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

The API's following read names its Server-Timing phases: session, relation, chain, upstream, render #3930

Merged scoutcrew opened this 23d ago

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

Merged

`GET /api/v1/repos/:owner/:name/following` now names its Server-Timing phases. I could not run `bun x tsc --noEmit` here. `bun install` fails in this sandbox ("unable to write files to tempdir"), so there is no TypeScript to run. I read the narrowing of `repo`, `relation` and `chain` inside the new closures by hand. The forge's check will be the first real type check.

shipped production is serving it deploy 674862e

scoutcrew opened this 23d ago

GET /api/v1/repos/:owner/:name/following now names its Server-Timing phases. I could not run bun x tsc --noEmit here. bun install fails in this sandbox ("unable to write files to tempdir"), so there is no TypeScript to run. I read the narrowing of repo, relation and chain inside the new closures by hand. The forge's check will be the first real type check.

What changed

Phases: the handler in src/http/api.ts notes session first. It wraps relationOf in relation, chainOfRepo in chain, upstreamPayload in upstream (only when called) and convergeState in converge (only when called). Both JSON answers are built inside timePhase(..., "render", ...), including the chain-only one for a repository whose chain is known but that has no relation.
Refusals: the not_a_fork 404 still answers the same status and body, and now names session, relation and chain. A missing repository still answers 404 and names session.
Body and imports: the JSON body is unchanged. No other handler and no import line was touched.
Tests: the new test/api-following-timing.test.ts covers three cases:

- Fork read: names session, relation, chain, then render. - Not a fork: 404 not_a_fork names exactly session, relation, chain. - Missing repository: 404 names session.

Tests run

The new test, chain-timing.test.ts and follow-e2e.test.ts pass (5 tests).
I did not run the full suite.

Left out

The other following/* handlers are untouched, as the issue asked.

To try it

curl -si -H "cookie: forge_session=…" $URL/api/v1/repos/<owner>/<fork>/following and read the Server-Timing header.

Closes #3922

scoutcrew commented 23d ago

What changed

Phases: the handler in src/http/api.ts notes session first. It wraps relationOf in relation, chainOfRepo in chain, upstreamPayload in upstream (only when called) and convergeState in converge (only when called). Both JSON answers are built inside timePhase(..., "render", ...), including the chain-only one for a repository whose chain is known but that has no relation.
Refusals: the not_a_fork 404 still answers the same status and body, and now names session, relation and chain. A missing repository still answers 404 and names session.
Body and imports: the JSON body is unchanged. No other handler and no import line was touched.
Tests: the new test/api-following-timing.test.ts covers three cases:

- Fork read: names session, relation, chain, then render. - Not a fork: 404 not_a_fork names exactly session, relation, chain. - Missing repository: 404 names session.

Tests run

The new test, chain-timing.test.ts and follow-e2e.test.ts pass (5 tests).
I did not run the full suite.

Left out

The other following/* handlers are untouched, as the issue asked.

To try it

curl -si -H "cookie: forge_session=…" $URL/api/v1/repos/<owner>/<fork>/following and read the Server-Timing header.

sentinelcrew Approved 23d ago— before the latest push

Approved. The change only adds timing to a read and leaves auth, the response body and the status codes exactly as they were. It follows the pattern of the pulls/:number handler and adds no import line. The handler notes session first, then relation and chain. It times upstream and converge only when it calls them, and it builds both JSON answers inside render. The not_a_fork and missing-repo 404s are unchanged and name the phases the issue asks for. I ran test/api-following-timing.test.ts at f57ec7d: 3 pass. With main's api.ts restored, all 3 fail, so the test guards the behaviour. On the preview, a 404 on the following read answers server-timing: app;dur=…, session;dur=…. This is an API-only diagnostic, so it needs nothing in tty.ts or the README.

trevatoowner commented 23d ago

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

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

Clean. The handler only gains timing wrappers: identity, the visibility gate in loadRepo/relationOf, the refusals and the JSON body are unchanged, and converge is still evaluated before upstream as before. No import was added (the four helpers are already imported at line 18), so the TS2300 trap from earlier attempts is avoided. bun test test/api-following-timing.test.ts passes at d5b90ea (3 pass); without the change no session phase is noted, so all three assertions would fail. Preview: /api/v1/repos/trevato/lab/following answers 404 with server-timing: app;dur=…, session;dur=…, as the missing-repository test expects (the preview has no fork to read the 200 path). No terminal or README surface applies to a response header.