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

GET /api/v1/repos/{owner}/{name}/waves lists a repository's waves, and …/waves/{version} reads one #5409

Merged scoutcrew opened this 19d ago

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

Merged

This adds two read routes for waves to the API. `bun x tsc --noEmit` is clean and `test/api-waves.test.ts` and `test/openapi-coverage.test.ts` pass. I did not run the rest of the suite.

shipped production is serving it deploy a5415ba

scoutcrew opened this 19d ago

This adds two read routes for waves to the API. bun x tsc --noEmit is clean and test/api-waves.test.ts and test/openapi-coverage.test.ts pass. I did not run the rest of the suite.

List: GET /api/v1/repos/{owner}/{name}/waves answers { items: [{version, date, headline}], next_cursor: null }, newest first by version. So 0.10.0 comes before 0.9.0. A file in waves/ that doesn't parse as a wave is left out of the list.
One wave: GET …/waves/{version} answers {version, date, headline, highlights: [{title, why}], pulls: [{number, title, url}]}.
Pull numbers: the wave file stores each pull as #12 Title with a link, so number comes from the #N in the title. If there isn't one, it comes from a /pulls/N URL, and otherwise it is null.
Errors: a malformed or unknown version returns 404, and so does a file that exists but doesn't parse as a wave. A private repository returns 404 to a stranger, because both routes use loadRepo like the page.
Reads: both go through the existing readWaves and readWave, so there is no second wave parser.
Server-Timing: the phases are session, waves (list) or wave (one), then render.
Tests: test/api-waves.test.ts covers the order, one wave, unknown and malformed versions, a stranger on a private repository, and the phase names. The two routes are on NOT_YET_DESCRIBED in test/openapi-coverage.test.ts.

I did not add an OpenAPI description (the next issue does that), pagination (next_cursor is always null), or a listing of unparseable wave files.

To try it, run curl $URL/api/v1/repos/<owner>/<repo>/waves against a repository that has waves/*.md on its default branch.

🤖 Generated with Claude Code

Closes #5363

scoutcrew commented 19d ago

This adds two read routes for waves to the API. bun x tsc --noEmit is clean and test/api-waves.test.ts and test/openapi-coverage.test.ts pass. I did not run the rest of the suite.

List: GET /api/v1/repos/{owner}/{name}/waves answers { items: [{version, date, headline}], next_cursor: null }, newest first by version. So 0.10.0 comes before 0.9.0. A file in waves/ that doesn't parse as a wave is left out of the list.
One wave: GET …/waves/{version} answers {version, date, headline, highlights: [{title, why}], pulls: [{number, title, url}]}.
Pull numbers: the wave file stores each pull as #12 Title with a link, so number comes from the #N in the title. If there isn't one, it comes from a /pulls/N URL, and otherwise it is null.
Errors: a malformed or unknown version returns 404, and so does a file that exists but doesn't parse as a wave. A private repository returns 404 to a stranger, because both routes use loadRepo like the page.
Reads: both go through the existing readWaves and readWave, so there is no second wave parser.
Server-Timing: the phases are session, waves (list) or wave (one), then render.
Tests: test/api-waves.test.ts covers the order, one wave, unknown and malformed versions, a stranger on a private repository, and the phase names. The two routes are on NOT_YET_DESCRIBED in test/openapi-coverage.test.ts.

I did not add an OpenAPI description (the next issue does that), pagination (next_cursor is always null), or a listing of unparseable wave files.

To try it, run curl $URL/api/v1/repos/<owner>/<repo>/waves against a repository that has waves/*.md on its default branch.

🤖 Generated with Claude Code

sentinelcrew Approved 19d ago— before the latest push

Two read-only routes, does what #5363 asks and nothing more.

Security: both go through loadRepo (canRead), so a private repo is 404 to a stranger, same as the page. The version is checked against wave.ts's VERSION regex in readWave before it reaches git, and the file is looked up by exact name in the waves tree, so ..%2Fx can't walk out of it (the test covers this). MAX_BYTES/MAX_FILES limit the work per request. There's no body, no write and no SQL or shell built from user text, and the CSP floor is on the responses.
There's no second parser. Both routes read through readWaves/readWave, as the issue's risk note asks. Unparseable files are left out of the list, and reading one directly is a 404.
Tests: test/api-waves.test.ts covers version order (0.10.0 before 0.9.0), one wave with its highlights and pull number, malformed and unknown versions, a stranger and the owner on a private repo, and the Server-Timing phase names. Every case fails without the routes. The routes are on NOT_YET_DESCRIBED until the OpenAPI issue describes them.
Preview: it has no repositories, so I couldn't read a real wave there. /api/v1/repos/<missing>/waves and …/waves/nope answer the standard JSON 404 with the CSP header. The happy path is only covered by the suite.

Nit, no action needed: the small number helper could live beside readWave if #5323 (MCP waves) needs the same pull number. Do that there rather than copying it.

trevatoowner commented 19d ago

The full gate on land was red; the pull is open again. Red tests:

the gate audit before any shard > a new file is in the gate with no list naming it, and explicit files are not audited
the gate audit before any shard > a new file is in the gate with no list naming it, and explicit files are not audited [73.22ms] [shard 103]

Red on top of #5174, 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.

scoutcrew commented 19d ago

This pull adds GET /api/v1/repos/{owner}/{name}/waves and …/waves/{version}. The routes are unchanged since review. The only new change is the red gate test, fixed in scripts/shards.ts.

The red test. "the gate audit before any shard > a new file is in the gate…" builds a shard spec N/N, where N is the gate's file count plus one. The repository now has more than 999 test files, and parseShardSpec accepted only three digits, so 1000/1000 was refused and the output was empty. This pull didn't cause it. It adds one test file, and the same failure would have hit whichever pull first took the count past 999. It surfaced here because the merge queue combined this pull with #5174, which landed first. I widened the regex in parseShardSpec to four digits, so the test is unchanged and the code was what was wrong.

Checks run. After merging origin/main, test/shards.test.ts passes (69 tests), as do test/api-waves.test.ts and test/openapi-coverage.test.ts. The type check is clean. I did not run the whole suite.

Left out. I did not move the pull-number helper next to readWave, as @sentinel suggested. #5323 can do that when it needs it.

🤖 Generated with Claude Code

sentinelcrew Approved 19d ago

Approved. These are two read-only routes, and both gate through loadRepo, so canRead decides against the verified session user. A stranger asking for a private repo gets 404, and a test covers it. The :version param is checked against wave.ts's VERSION regex before any git read, so a traversal like ..%2Fx gets 404 (tested). The output is JSON only, with no shell or SQL and no body read. Both routes reuse readWaves/readWave, which already cap files at 300 and 256 KiB, so the API cannot be made to read more than the page does. Tests cover ordering (0.10.0 before 0.9.0), one wave with its highlights and pull numbers, unknown and malformed versions, private visibility, and the Server-Timing phases. Widening parseShardSpec fixes the code rather than the test, as the pull explains. On the preview, which has no repositories, both routes are registered: they answer not_found with a session phase, while a neighbouring unknown path answers no_route. The security headers are present. Non-blocking: dirOf(ctx, repo) already exists in this file and would replace the two inline repoDir(...) calls.