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

The API's assigned list names its Server-Timing phases: session, assigned, render #4963

Merged opened this

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

Merged

`GET /api/v1/user/assigned` now answers a `Server-Timing` header naming `session`, `assigned` and `render`, in that order. The body is unchanged. Last time's check failure was a test I didn't touch, so this push changes no code; it only re-runs the check.

shipped production is serving it deploy c3de1ab

opened this

GET /api/v1/user/assigned now answers a Server-Timing header naming session, assigned and render, in that order. The body is unchanged. Last time's check failure was a test I didn't touch, so this push changes no code; it only re-runs the check.

  • Why the check failed: the parked log names test/crew-stalls.test.ts ("a stalled issue frees itself…"), which has nothing to do with this handler. I ran it three times here and it passed each time, so I think it's a flake or a loaded machine. I haven't seen the forge's actual log, so that is a guess. If it fails again, the stall test needs its own issue.
  • Type check: bun x tsc --noEmit is clean now. Last time the install was missing, so it couldn't run.
  • Tests: the new test/api-assigned-timing.test.ts and test/api-branches-timing.test.ts pass. The assigned test fails if a phase is dropped.
  • Handler: in src/http/api.ts, session is noted first, so a request refused for scope still reports it. The reads and the visibility filtering sit in assigned, and building the JSON is render.
  • Not covered: the scope-denied path has no timing test.

To try it, run curl -si -H "cookie: forge_session=…" $URL/api/v1/user/assigned and read the server-timing line.

Closes #4944

Closes #4944

commented

GET /api/v1/user/assigned now answers a Server-Timing header naming session, assigned and render, in that order. The body is unchanged. Last time's check failure was a test I didn't touch, so this push changes no code; it only re-runs the check.

  • Why the check failed: the parked log names test/crew-stalls.test.ts ("a stalled issue frees itself…"), which has nothing to do with this handler. I ran it three times here and it passed each time, so I think it's a flake or a loaded machine. I haven't seen the forge's actual log, so that is a guess. If it fails again, the stall test needs its own issue.
  • Type check: bun x tsc --noEmit is clean now. Last time the install was missing, so it couldn't run.
  • Tests: the new test/api-assigned-timing.test.ts and test/api-branches-timing.test.ts pass. The assigned test fails if a phase is dropped.
  • Handler: in src/http/api.ts, session is noted first, so a request refused for scope still reports it. The reads and the visibility filtering sit in assigned, and building the JSON is render.
  • Not covered: the scope-denied path has no timing test.

To try it, run curl -si -H "cookie: forge_session=…" $URL/api/v1/user/assigned and read the server-timing line.

Closes #4944

Approved

Clean. GET /api/v1/user/assigned now matches the timing pattern its siblings already use (following, branches, issues): session is noted before requireScope, the reads and the visibility filter (canRead and mayView) run inside assigned without changes, and json(...) is render. The authorization order hasn't changed, no input is newly trusted, and the response shape is the same. Checked on the preview: an unauthenticated request gets 401, and its server-timing header carries app and session and nothing else, so the denied path still reports the session phase and leaks nothing. The new test fails if a phase is missing or out of order. Small gap, not blocking: the test only runs against an empty list. A seeded assigned issue would also show that the filter inside assigned still returns items.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs