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

GET /api/v1/repos/:owner/:name/forks gives each fork's ahead, behind and last push, and the parent, as the forks page and the MCP forks tool do #9124

Merged opened this

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

Merged

`GET /api/v1/repos/:owner/:name/forks` now returns each fork's drift and last push, plus the parent. The new drift test and the related fork, OpenAPI and MCP-size tests pass, as do `bun scripts/gate.ts` and `tsc`. I did not run the full suite; the forge's check does that on the push.

shipped production is serving it deploy 2fa0c24

opened this

GET /api/v1/repos/:owner/:name/forks now returns each fork's drift and last push, plus the parent. The new drift test and the related fork, OpenAPI and MCP-size tests pass, as do bun scripts/gate.ts and tsc. I did not run the full suite; the forge's check does that on the push.

  • Items: every existing field stays. Each item adds ahead and behind (both null when git cannot say) and updatedAt, the last push.
  • Paging: the route pages first, then reads drift for that page only, in runs of MAX_RANKED_FORKS. It uses driftOfForks, the same function the forks page and the MCP forks tool use, so it also reuses its cache.
  • Parent: the page now has parent: "owner/name" | null next to items and next_cursor.
  • Callers: visibleForks is unchanged for its other callers.
  • Timing: Server-Timing now reads session, forks, drift, render. I updated test/forks-api-timing.test.ts to expect that, because it pinned the old three phases.
  • OpenAPI: the route summary in src/contract/openapi/routes/getApiV1ReposOwnerNameForks.ts names the new fields. No separate snapshot pinned it.
  • Test: test/forks-api-drift.test.ts forks a repo and pushes one commit to the fork. It reads ahead: 1, behind: 0, a non-null updatedAt, and parent naming alice/app from the fork's own route. It also checks the phase order. I did not run it against main to confirm it fails there.

Left out: I changed nothing in the MCP tool or its size budget. I also added no drift cache beyond the one driftOfForks already has; I did not check whether #6146 has landed.

Closes #9099

commented

GET /api/v1/repos/:owner/:name/forks now returns each fork's drift and last push, plus the parent. The new drift test and the related fork, OpenAPI and MCP-size tests pass, as do bun scripts/gate.ts and tsc. I did not run the full suite; the forge's check does that on the push.

  • Items: every existing field stays. Each item adds ahead and behind (both null when git cannot say) and updatedAt, the last push.
  • Paging: the route pages first, then reads drift for that page only, in runs of MAX_RANKED_FORKS. It uses driftOfForks, the same function the forks page and the MCP forks tool use, so it also reuses its cache.
  • Parent: the page now has parent: "owner/name" | null next to items and next_cursor.
  • Callers: visibleForks is unchanged for its other callers.
  • Timing: Server-Timing now reads session, forks, drift, render. I updated test/forks-api-timing.test.ts to expect that, because it pinned the old three phases.
  • OpenAPI: the route summary in src/contract/openapi/routes/getApiV1ReposOwnerNameForks.ts names the new fields. No separate snapshot pinned it.
  • Test: test/forks-api-drift.test.ts forks a repo and pushes one commit to the fork. It reads ahead: 1, behind: 0, a non-null updatedAt, and parent naming alice/app from the fork's own route. It also checks the phase order. I did not run it against main to confirm it fails there.

Left out: I changed nothing in the MCP tool or its size budget. I also added no drift cache beyond the one driftOfForks already has; I did not check whether #6146 has landed.

commented

GET /api/v1/repos/:owner/:name/forks now returns each fork's ahead, behind and updatedAt, and a parent field next to items and next_cursor. The check failed on a missing Try: line in the commit messages. I added an empty follow-up commit carrying it and pushed. bun scripts/gate.ts now passes locally. I did not rerun tsc or the tests, because the only change since the last run is that empty commit.

Try: GET /api/v1/repos/<owner>/<name>/forks on the preview and read ahead, behind, updatedAt and parent.

  • Items: every existing field stays. ahead and behind are null when git cannot say. Drift is read for the requested page only.
  • Timing: the phases are now session, forks, drift, render. I updated test/forks-api-timing.test.ts to expect that, because it pinned the old three.
  • Tests: test/forks-api-drift.test.ts forks a repo, pushes one commit to the fork and reads ahead: 1, behind: 0. It also checks that parent is alice/app from the fork's own route. I did not run it against main to confirm it fails there.
  • OpenAPI: the route summary names the new fields. No separate snapshot pinned it.
  • Left out: I made no change to the MCP tool or its size budget. I added no drift cache beyond the one driftOfForks already has, and did not check whether #6146 has landed.
commented

The failing check was a test that pinned the old response shape, and it passes now. test/fork-counts.test.ts expected the forks list to be exactly { items: [], next_cursor: null }. The new parent field is intended, so I updated that one line to expect { parent: null, items: [], next_cursor: null } and pushed. The test was wrong, not the code.

I re-ran test/fork-counts.test.ts and test/mcp/forks.test.ts, and both pass. I did not re-run the full suite locally; the forge's check does that on the push.

Try: GET /api/v1/repos/<owner>/<name>/forks on the preview and read parent beside items.

What the pull does:

  • Fields: each fork item keeps its existing fields and adds ahead, behind (null when git cannot say) and updatedAt, the last push.
  • Parent and timing: the page gains parent: "owner/name" | null. Drift is read for the requested page only, and a drift timing phase now appears between forks and render.
  • Spec and tests: the OpenAPI summary for the route names the new fields. test/forks-api-drift.test.ts forks a repo, pushes one commit to the fork, and reads ahead: 1, behind: 0. It also checks that parent names the source.

Left out: I made no change to the MCP tool or its size budget. I added no drift cache beyond the one driftOfForks already has.

Changes requested — before the latest push

The route and the OpenAPI summary are right, and drift is read after paging. Two things to fix in one round:

  1. The paging logic is copied out of pageOf instead of reused. See the comment on upstream.ts.
  2. The "drift is read only for the page being answered" promise has no test. See the comment on the test.

No high-severity security finding. Visibility goes through visibleForks and forkParentFor with the caller, so a guest sees no private fork or parent.

commented

Both review points are fixed and pushed. Locally, tsc, the gate and the fork and OpenAPI tests I ran pass; the forge's check runs the rest.

  • Paging: src/http/api.ts now has slicePage(rows, page), which returns { items, next_cursor } or the 422 invalid_cursor response. pageOf calls it, and the forks route in src/http/api/upstream.ts uses it, so the paging logic lives in one place.
  • Drift test: a new case in test/forks-api-drift.test.ts makes a repo with two forks, one commit ahead and one two ahead. It reads the list with limit=1, then with the returned cursor. Each page has one item with its own ahead, the first next_cursor points at that item, and the last is null. A spy on driftOfForks shows it is called with only that page's fork each time. Reading drift for every fork would now fail the test.

Try: GET /api/v1/repos/<owner>/<name>/forks?limit=1 on the preview, then again with the returned next_cursor. Each page carries its own fork's ahead and behind.

Approved

Both earlier asks are met. Paging now lives in one place (slicePage, with pageOf calling it). A new test spies on driftOfForks and shows drift is read only for the requested page. Auth, the invalid_cursor 422 and the security floor are unchanged. Non-blocking: the first test in test/forks-api-drift.test.ts uses very long lines.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs