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

The admin API's accounts, people, invites and teams lists name their Server-Timing phases #4988

Merged scoutcrew opened this 22d ago

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

Merged

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

shipped production is serving it deploy 93e874d

scoutcrew opened this 22d ago

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

Where the phases sit: each handler notes session first, before the access check, so a refused 403/404 still names it. The read runs inside its own timePhase, and json(…) inside render; the access check is outside both. The change is in src/http/api.ts.
Tests: test/admin-lists-timing.test.ts has two tests per route. One asserts the ordered phases on a 200. The other checks that an unauthenticated request still names session. All 8 pass. I didn't run the suite against the unchanged code to confirm each test fails without the change.
Test detail: the server adds its own app phase to every response. The order assertion filters it out and compares only session, the read and render.
Not checked: bun x tsc --noEmit couldn't run here. It fails on the missing bun type definitions, and bun install didn't fix that.

Closes #4978

scoutcrew commented 22d ago

Where the phases sit: each handler notes session first, before the access check, so a refused 403/404 still names it. The read runs inside its own timePhase, and json(…) inside render; the access check is outside both. The change is in src/http/api.ts.
Tests: test/admin-lists-timing.test.ts has two tests per route. One asserts the ordered phases on a 200. The other checks that an unauthenticated request still names session. All 8 pass. I didn't run the suite against the unchanged code to confirm each test fails without the change.
Test detail: the server adds its own app phase to every response. The order assertion filters it out and compares only session, the read and render.
Not checked: bun x tsc --noEmit couldn't run here. It fails on the missing bun type definitions, and bun install didn't fix that.

sentinelcrew Approved 22d ago

Clean, and it does what #4978 asked.

Security: access rules are unchanged. accountAdmin still runs before any read. Each timePhase wraps only the read or json(...), never the access check, so a refused request does no list work. Phase names are fixed literals that PHASE_NAME accepts, so the header carries nothing a client wrote. Adding session before the access check leaks nothing: it is the same server-measured number every other /api/v1 route that calls notePhase already exposes.
Tests: I applied the diff to a scratch copy and ran bun test test/admin-lists-timing.test.ts: 8 pass. Against unchanged main, all 8 fail, so each test fails without the change.
Preview: unauthenticated GET /api/v1/admin/{accounts,people,invites,teams} on lab--4988 answers 404 with server-timing: app;dur=…, session;dur=…. Production answers app only.
Doors and README: the change only adds response-header instrumentation, so nothing new is needed in the terminal or the README.

No findings.