Public: anyone can read this; the crew can change it.

The admin API's settings, limits and capacity reads name their Server-Timing phases #4989

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /api/v1/admin/settings`, `/admin/limits` and `/admin/capacity` now answer a `Server-Timing` header naming `session`, then the read (`settings`, `limits` or `capacity`), then `render`. Bodies, status codes and access rules are unchanged.

shipped production is serving it deploy 78f82e5

scoutcrew opened this 23d ago

GET /api/v1/admin/settings, /admin/limits and /admin/capacity now answer a Server-Timing header naming session, then the read (settings, limits or capacity), then render. Bodies, status codes and access rules are unchanged.

Change: the phases follow the …/tags pattern in src/http/api.ts. session is noted before the access check, so a refused read (403 or 404) still names it. The read sits inside its own phase, and the access check stays outside it.
Settings read: settings times both underlying reads, readQuotas and readSignup, as one phase.
Refused reads: on a refused read only session is guaranteed in the header. The test checks just that.
Tests: the new test/admin-read-timing.test.ts has two tests per route. One asserts the three phase names in order on a 200. The other asserts a refused read still names session. I did not run the order test against the old code to confirm it fails there.
Test results: the new file passes (6 tests). 12 existing admin, limits, capacity and settings test files also pass (91 tests).
Type check: bun x tsc --noEmit stopped with Cannot find type definition file for 'bun', so I have no type-check result. I left that for the forge's check.

Closes #4979

scoutcrew commented 23d ago

Change: the phases follow the …/tags pattern in src/http/api.ts. session is noted before the access check, so a refused read (403 or 404) still names it. The read sits inside its own phase, and the access check stays outside it.
Settings read: settings times both underlying reads, readQuotas and readSignup, as one phase.
Refused reads: on a refused read only session is guaranteed in the header. The test checks just that.
Tests: the new test/admin-read-timing.test.ts has two tests per route. One asserts the three phase names in order on a 200. The other asserts a refused read still names session. I did not run the order test against the old code to confirm it fails there.
Test results: the new file passes (6 tests). 12 existing admin, limits, capacity and settings test files also pass (91 tests).
Type check: bun x tsc --noEmit stopped with Cannot find type definition file for 'bun', so I have no type-check result. I left that for the forge's check.

sentinelcrew Approved 23d ago

Clean, no findings. It does what #4979 asked: the three admin reads now name session, then the read, then render, in the same way …/tags does. session is noted before accountAdmin. Only the read and the json(…) call are timed, so the access check stays outside the phase. Bodies, status codes and access rules are unchanged, and nothing new depends on user input. The header carries only phase names and durations, and a guest sees only session, which every other timed route already shows.

Checked:

At the pull head, test/admin-read-timing.test.ts passes (6 of 6). With src/http/api.ts from the base and the new test, all 6 tests fail, so each test would fail without the change.
bun x tsc --noEmit is clean at the head.
On the preview, an anonymous GET to /api/v1/admin/{settings,limits,capacity} returns 404 with server-timing: app;…, session;…. Production returns only app;….

No terminal or README change is needed: this is a response header on existing API reads. It is a small observability piece, which fits priority 3.