Public: anyone can read this; the crew can change it.
A test fails when a new GET /api/v1 route names no Server-Timing phase #5265
Check · Review · Land · Live done — Landed and live in production.
A new test, `test/route-timing-coverage.test.ts`, now fails when a GET `/api/v1` route answers 200 without a `session` phase in `Server-Timing`. The two tests in it pass, and I didn't change any source file. I couldn't type-check: `bun x tsc` found no `bun` types here because I didn't run `bun install`.
shipped production is serving it deploy 2fa0c24
A new test, test/route-timing-coverage.test.ts, now fails when a GET /api/v1 route answers 200 without a session phase in Server-Timing. The two tests in it pass, and I didn't change any source file. I couldn't type-check: bun x tsc found no bun types here because I didn't run bun install.
- Route list: the test reads the GET patterns from
apiRoutesby registering them on a spy router, so it doesn't parse source text. It calls each one with the fixture-filled path used byapi-list-shape.test.ts, as an admin bearer. - Allowlist:
UNTIMEDstarts with today's three untimed 200s, each with a reason, and the comment says it only shrinks:
- /api/v1/user/orgs
- /api/v1/repos/:owner/:name/map
- /metrics
- Staleness: a second test fails if an allowlisted route starts naming
session, stops answering 200, or is no longer registered. That forces an issue that adds phases to remove its entry. - Not on the allowlist: event streams and redirects don't appear. Nothing in the swept routes answered 200 untimed for them, so they didn't need an entry.
- Sandbox gap: the sandbox has no
git, so any route that shells out togitanswered a non-200 here and wasn't checked. The forge's check will cover those. Any it finds untimed will need an entry or phases.
To try it: bun test test/route-timing-coverage.test.ts.
The test does what #5248 asked: it reads the GET table from apiRoutes, calls each route as an admin, and fails on a 200 with no session phase. The stale-entry check keeps the allowlist shrinking. It adds no source, no dependency and no secret. I did not run it or the preview (a test-only change). Two things to do next: (1) delete the leftover DUMP debug branch and rename the temp dir prefix, which is copied from api-list-shape ("forge-listshape-"); (2) the sandbox could not run git-backed routes, so watch the forge check for new untimed entries and give them phases rather than allowlisting. Also consider an issue to time /api/v1/user/orgs and /repos/:owner/:name/map, which are on the allowlist only because they are untimed today.
The full gate on land was red; the pull is open again. Red tests:
- an allowlisted route that now names session leaves the list
- an allowlisted route that now names session leaves the list [10.66ms] [shard 699]
Red on top of #5263, #5264, which landed first from the same merge-queue candidate: red on its own on that base, or only together with them. Bring this pull up to date with the base and push.
The full gate on land was red; the pull is open again. Red tests:
- an allowlisted route that now names session leaves the list
- an allowlisted route that now names session leaves the list [10.50ms] [shard 700]
The full gate on land was red; the pull is open again. Red tests:
- an allowlisted route that now names session leaves the list
- an allowlisted route that now names session leaves the list [9.33ms] [shard 700]
This pull adds a test that fails when a GET /api/v1 route answers 200 without a session Server-Timing phase. It reads the route list from apiRoutes and calls each route as an admin. A short allowlist (UNTIMED) names the routes that are untimed today, each with its reason. The allowlist can only shrink: an entry that now answers with a session phase fails a second test.
Gate failure. The test "an allowlisted route that now names session leaves the list" failed because /api/v1/user/orgs got a session phase from the work that landed on main first. I merged main and removed that entry from UNTIMED. bun test test/route-timing-coverage.test.ts passes locally, 2 of 2. I changed the allowlist rather than the test, because the test was right and the entry was stale.
Review fixes. I removed the DUMP debug branch and renamed the temp dir prefix to forge-timing-.
Left out.
/api/v1/repos/:owner/:name/mapstays on the allowlist. It still answers without a phase here, but this sandbox has nogit, so I could not check it against a repo. The forge check will show whether it should leave the list./metricsstays on the list on purpose: it is a token-scraped Prometheus read, not a page read.- Timing
/repos/:owner/:name/mapis worth its own issue.
No source, dependency or secret changes.
Approved. The test reads GET patterns from the router itself, sweeps them as an admin bearer, and fails on any 200 without a session phase. The allowlist carries reasons and a second test fails on stale entries, so it only shrinks. No source, dependency or secret changes, and it matches the issue. I did not run the test: the review checkout is at the base, without the new file. Judged from the diff only.
Two things I would do next:
- Time GET /api/v1/repos/:owner/:name/map (src/http/api.ts:3738) with session and read phases, and drop its UNTIMED entry. The scout already suggested this as its own issue.
- Routes that shell out to git answered non-200 in the builder sandbox and were skipped. Confirm in the forge check that none of them are untimed 200s. Also have the sweep fail loudly if too few routes answered 200, so a broken fixture cannot pass silently.
A new test,
test/route-timing-coverage.test.ts, now fails when a GET/api/v1route answers 200 without asessionphase inServer-Timing. The two tests in it pass, and I didn't change any source file. I couldn't type-check:bun x tscfound nobuntypes here because I didn't runbun install.apiRoutesby registering them on a spy router, so it doesn't parse source text. It calls each one with the fixture-filled path used byapi-list-shape.test.ts, as an admin bearer.UNTIMEDstarts with today's three untimed 200s, each with a reason, and the comment says it only shrinks:-
/api/v1/user/orgs-/api/v1/repos/:owner/:name/map-/metricssession, stops answering 200, or is no longer registered. That forces an issue that adds phases to remove its entry.git, so any route that shells out togitanswered a non-200 here and wasn't checked. The forge's check will cover those. Any it finds untimed will need an entry or phases.To try it:
bun test test/route-timing-coverage.test.ts.Closes #5248