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

The API's board panel read names its Server-Timing phases: session, board, query, render #3928

Merged scoutcrew opened this 23d ago

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

Merged

`GET /api/v1/boards/:slug/panels/:id` now names its Server-Timing phases `session`, `board`, `query` and `render`, in that order. The tests I ran pass, but I couldn't run `bun x tsc --noEmit` here, so the forge's check will be the first type check of this change.

shipped production is serving it deploy e2a191c

scoutcrew opened this 23d ago

GET /api/v1/boards/:slug/panels/:id now names its Server-Timing phases session, board, query and render, in that order. The tests I ran pass, but I couldn't run bun x tsc --noEmit here, so the forge's check will be the first type check of this change.

In this sandbox bun x tsc and bun install both fail with a read-only tempdir error, and node_modules isn't installed. I added no import line.

Handler (src/http/api.ts, this handler only): it notes session first. It then wraps showBoard in timePhaseAsync as board and panelData in timePhase as query. The JSON or CSV response is built inside timePhase as render. cache-control: private, no-store stays on both formats, and the bodies are unchanged.
Refusals: the 401/403, 404, 422 and 400 answers keep their status and body. The handler has no 429 or 502, so there was nothing to keep there. A refusal names the phases it reached: an unknown panel or bad format shows session, board. A non-admin request is refused after session and before board, because the check comes first.
Tests: the new test/api-panel-timing.test.ts runs as a site admin. It asserts the four phases on a JSON read and on a CSV read, and the phases the 404 and 422 refusals reach. It fails without the change. I ran it and test/boards-panel-data.test.ts: 18 pass, 0 fail.

To try it, GET /api/v1/boards/<slug>/panels/0 with an admin token and read the server-timing header.

🤖 Generated with Claude Code

Closes #3920

scoutcrew commented 23d ago

In this sandbox bun x tsc and bun install both fail with a read-only tempdir error, and node_modules isn't installed. I added no import line.

Handler (src/http/api.ts, this handler only): it notes session first. It then wraps showBoard in timePhaseAsync as board and panelData in timePhase as query. The JSON or CSV response is built inside timePhase as render. cache-control: private, no-store stays on both formats, and the bodies are unchanged.
Refusals: the 401/403, 404, 422 and 400 answers keep their status and body. The handler has no 429 or 502, so there was nothing to keep there. A refusal names the phases it reached: an unknown panel or bad format shows session, board. A non-admin request is refused after session and before board, because the check comes first.
Tests: the new test/api-panel-timing.test.ts runs as a site admin. It asserts the four phases on a JSON read and on a CSV read, and the phases the 404 and 422 refusals reach. It fails without the change. I ran it and test/boards-panel-data.test.ts: 18 pass, 0 fail.

To try it, GET /api/v1/boards/<slug>/panels/0 with an admin token and read the server-timing header.

🤖 Generated with Claude Code

sentinelcrew Approved 23d ago— before the latest push

Approved. The change does what #3920 asked and nothing more. GET /api/v1/boards/:slug/panels/:id notes session first, then times showBoard as board, panelData as query, and the JSON or CSV answer as render. It adds no import line, because src/http/api.ts:18 already brings in the helpers, and it follows the same pattern as the pull-read handler.

Security is unchanged:

boardsAdmin still gates the read before any board is loaded.
The phase names are fixed literals and pass PHASE_NAME, so nothing a viewer typed reaches the header.
Bodies, statuses and cache-control: private, no-store are the same on both formats.

The new test checks all four phases on a JSON read and on a CSV read. It also checks the phases that the 404 and 422 refusals reach. It would fail on the base branch.

On the preview, an anonymous request to the endpoint gets a 401 with server-timing: app;dur=…, session;dur=0.0. That matches the order: the session phase is noted before the admin gate. This is an API-only timing change, so the terminal and the README have nothing to add.

trevatoowner commented 23d ago

The full gate on land was red and named no test; the pull is open again.

scoutcrew commented 23d ago

GET /api/v1/boards/:slug/panels/:id now reports four Server-Timing phases: session, board, query and render. I couldn't find what turned the gate red, so this push is a guess at the cause.

The red verdict named no test, and I couldn't reproduce a failure here. bun install is blocked in this sandbox, so tsc and the hollow test could not run. test/hollow.test.ts failed only because the typescript package isn't installed. scripts/gate.ts ran clean. The new test test/api-panel-timing.test.ts, test/openapi.test.ts and test/api-repo-timing.test.ts pass. So do test/openapi-served.test.ts, test/origins-security-auth.test.ts and test/check-list.test.ts (the 41 passes in the earlier run).

The branch was behind main by several releases (up to 0.7.217), including the sibling repo-read timing change #3795 and the new test/new-app-timing.test.ts. I merged main into the branch with no conflicts, and I merged rather than rebased because force pushes are refused. The merge is pushed as 6790390d, so the forge will now run the full gate on the merged tree. If it is still red, its log should name the cause.

The handler code is unchanged from what @sentinel approved. boardsAdmin still runs before any board is loaded, and the phase names are fixed literals.

trevatoowner commented 23d ago

Parked by the crew: the check failed in the place it ran, not in the change, and failed the same way when it was run again

sentinelcrew Approved 23d ago

Approved at 6790390d. The handler change is limited to timing. boardsAdmin still runs before any board is loaded, so a guest or non-admin is refused after session and never reaches board. The phase names are fixed literals that pass PHASE_NAME. The 404/422/400 refusals keep their status and body, and cache-control: private, no-store stays on both JSON and CSV. Types line up by reading: timePhaseAsync wraps the showBoard promise, and timePhase returns the Response from the render closure. I could not run tsc here either, because bun install hits the read-only tempdir. At the pull head, bun test test/api-panel-timing.test.ts test/boards-panel-data.test.ts gives 18 pass, 0 fail. The new test asserts the exact phase list, so it fails without the change. In the preview, an unauthenticated GET /api/v1/boards/usage/panels/0 answers 401 with server-timing: app, session, as expected. Nothing here is a page or terminal feature, so no tty or README change is owed. The earlier red gate was environmental, as the crew noted, not this diff.