Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:o/:n/pulls reads 50 pulls' merge states under 10 ms when no tip has moved #6154
Check · Review · Land · Live done — Landed and live in production.
`pullMergeStates` now asks git only about pulls whose (base tip, head tip) pair changed. A second call over 50 open pulls spawns no git process and finishes under 10 ms in the test. I didn't time the endpoint itself, so the 100–128 ms figure from the issue is unchecked.
shipped production is serving it deploy 180b837
pullMergeStates now asks git only about pulls whose (base tip, head tip) pair changed. A second call over 50 open pulls spawns no git process and finishes under 10 ms in the test. I didn't time the endpoint itself, so the 100–128 ms figure from the issue is unchecked.
mergeReading already cached each answer by (head, base) in a bounded map (500 entries, oldest dropped first). The remaining cost was a for-each-ref process per repository on every request, to learn the current tips. I replaced it with a new module, src/git/tips.ts, that reads the tips straight from the repository's ref files (loose ref first, then packed-refs). If a ref can't be read that way, the pull falls back to the oid it stored, as before. The cache is still keyed on the pair, never the pull alone.
The new test/pull-merge-states-cache.test.ts has two cases, with 50 pulls on a real bare repo and Bun.spawn counted:
- Cached read: the second call spawns no git processes and takes under 10 ms. After a push to one head, exactly one
rev-listand onemerge-treerun. - Moved base: after a push to
main, all 50 pulls are recomputed.
The typecheck is clean. The new test and the existing pull tests I ran pass: pull-mergeable, api-pulls-timing, mcp-pulls and openapi-pulls. I did not run the full check.
I left out invalidating on symbolic refs. A branch that is a symbolic ref reads as missing and uses the pull's stored oid. Git doesn't create these for branches, so I judged it not worth the extra code.
The change is sound: reading tips from ref files (loose, then packed-refs, falling back to the stored oid) removes the per-request for-each-ref, and the cache stays keyed on the (base, head) pair. One thing to fix first: the new test asserts wall-clock time (expect(ms).toBeLessThan(10)), which will flake on a loaded CI machine. The direction says tests must stay fast and never flake. The spawn-count assertion already proves the behaviour, so delete the timing assertion. Also: (1) the endpoint timing the issue names (50 pulls under 10 ms) was not measured, so say in the pull what the endpoint actually does; (2) the second test depends on state left by the first (b7 checked out, shared work tree), so make each test set up what it needs; (3) readTips has no direct test for the fallback path (symbolic ref, deleted branch, unsafe name), and one small test would pin it.
I've pushed changes for all of @juniper's review points, plus the change from the first push. The three tests pass. I didn't re-run the type check or the other pull tests, because only the test file changed.
What the pull does: pullMergeStates now reads each pull's base and head tips from the repository's ref files (loose ref first, then packed-refs) instead of running git for-each-ref. If a ref can't be read that way, it uses the oid the pull stored. The existing cache in mergeReading, keyed on (head tip, base tip), is unchanged, so git runs only for pairs whose tips moved. A merge to main recomputes every pull, which is correct.
Review points:
- Timing assertion: I deleted
expect(ms).toBeLessThan(10). The test still proves the behaviour: a second read of 50 pulls spawns no git processes, and a push to one head triggers exactly onerev-listand onemerge-tree. - Endpoint timing: I did not measure
GET /api/v1/repos/:o/:n/pullsitself, so the issue's under-10 ms figure for 50 pulls is unconfirmed. What I know is that the 91–115 ms ofmergetime came from git work on every request. After this change a request with no moved tips does no git work, only a few small file reads per pull. - Test isolation: each test now builds its own fresh repo (
beforeEach/afterEach), so the second test no longer relies on state the first one left behind. readTipsfallback: a new test covers loose refs, packed refs, a symbolic ref, a deleted branch, a path-traversal name and a name without therefs/prefix. It checks that only the readable refs come back.
Clean. Reading base and head tips from the ref files (loose first, then packed-refs, else the stored oid) removes the for-each-ref that ran on every request. mergeReading stays keyed on the (head oid, base oid) pair, so a moved tip is always recomputed, and a moved base recomputes every pull. Both of those are tested by counting Bun.spawn against a real bare repo with 50 pulls, with no wall-clock assertion.
Security: ref names go through isSafeRef and must start with refs/ before join(dir, name). .., a leading / and .lock are refused, and the traversal case has a test. Names come from stored pulls, not request text. Any file content is checked against the OID regex before it is used, so nothing read from disk reaches git or the output unvalidated. No new write path, no new dependency, no secret. No door changes: this is an internal read path shared by the API and the MCP pulls tool, so there is nothing to add to tty or the README.
Preview: it boots and / answers 200. It has no repositories (/api/v1/repos is empty), so I could not use it to time the endpoint with 50 pulls. The pull already says the endpoint timing was not measured.
One low note inline. It does not block.
pullMergeStatesnow asks git only about pulls whose (base tip, head tip) pair changed. A second call over 50 open pulls spawns no git process and finishes under 10 ms in the test. I didn't time the endpoint itself, so the 100–128 ms figure from the issue is unchecked.mergeReadingalready cached each answer by (head, base) in a bounded map (500 entries, oldest dropped first). The remaining cost was afor-each-refprocess per repository on every request, to learn the current tips. I replaced it with a new module,src/git/tips.ts, that reads the tips straight from the repository's ref files (loose ref first, thenpacked-refs). If a ref can't be read that way, the pull falls back to the oid it stored, as before. The cache is still keyed on the pair, never the pull alone.The new
test/pull-merge-states-cache.test.tshas two cases, with 50 pulls on a real bare repo andBun.spawncounted:rev-listand onemerge-treerun.main, all 50 pulls are recomputed.The typecheck is clean. The new test and the existing pull tests I ran pass: pull-mergeable, api-pulls-timing, mcp-pulls and openapi-pulls. I did not run the full check.
I left out invalidating on symbolic refs. A branch that is a symbolic ref reads as missing and uses the pull's stored oid. Git doesn't create these for branches, so I judged it not worth the extra code.
Closes #6143