Repository navigation
fix: gstack's GitHub waits poll REST at most once a minute and wait out rate limits (GST-1713) - #4109
Merged
Merged
Conversation
…ut rate limits (GST-1713) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
|
😎 This pull request was merged. |
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
ci-gate --registration-wait re-polls while time is left in the window; a rate-limited gh pr merge fails at once under the land lock; a bot PR author reads without its [bot] suffix; ci-carry wait documents its one-poll overrun. Review-Finding: p1/specialist-batched#1 reader=testing Review-Finding: p1/specialist-batched#2 reader=testing Review-Finding: p1/specialist-batched#3 reader=testing Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
No image: this is a CLI and library change with no visible surface, so the before/after is in text.
gstack-pr-waitread the PR through a GraphQL query plusgh pr checksevery 20 seconds (as often as every 5), and other GitHub waits polled every 3 to 30 seconds. A rate-limitedghcall failed, or was retried within seconds.lib/gh-poll.ts).gstack-pr-waitreads PR state, base head and head checks from REST and makes one cached GraphQL conversation read. A rate-limited call readsgh api rate_limit, sleeps until the reset and retries at most twice. The one exception isgh pr merge, which runs under the land lock and still fails at once.Estimated GitHub calls (handoff audit: handoffs/GST-1713/audit.md)
--pinTests
test/gh-poll-tripwire.test.ts: in bin/, lib/ and scripts/, any wait in a GitHub-reading module must take its duration frompollDelayMs(or carry agh-poll-exempt:reason, ratcheted at 4: two GitHub Actions-only scripts and pr-wait's 5xx retry). No shipped file may set theGSTACK_GH_POLL_FLOOR_MSfixture seam.test/gh-poll.test.ts: the floor and jitter, rate-limit classification, sleep-until-reset with no immediate retry and a bounded give-up, and REST parsing from trimmed recordings of real replies (test/fixtures/gh-poll/: fix: release stamps and a docs-audit edit after the last fix-check are no gate-class edit (GST-1702) #4101 merged, feat: cut standalone /plan-eng-review; /autoplan keeps its eng phase (GST-1698) #4108 open, fix: release stamps and a docs-audit edit after the last fix-check are no gate-class edit (GST-1702) #4101's 64 check runs and status,rate_limit).test/pr-wait.test.ts: the fake gh now serves REST shapes; new cases prove one GraphQL read across unchanged polls, rereads on change, ten minutes or green-under-ready, and ≥60 s sleeps. ci-gate, greptile-early and ci-carry timing expectations move to the 60 s floor; two fixtures that drive real binaries set the floor seam to 0.Blast radius
This touches every GitHub wait in gstack: ci-gate, ci-carry, ship-release accept, greptile-early, the ship-queue land, train, post-merge and dispatch waits, release-gate and eval-bg. Each wait now takes at least 60 seconds per poll. The merge gate itself is unchanged:
gstack-ship-queue landstill checks CI at the pin, and a rate-limitedgh pr mergestill fails at once instead of sleeping while it holds the land lock. Evidence: focused tests ran (gh-poll, gh-poll-tripwire, pr-wait, gstack-ci-gate, ci-carry, ship-ci-carry, land-and-deploy-ci-gate and ship-queue-land*, all passing), andgstack-ci-gatewas run once at 5549950 against this PR (VERDICT PASS).Why
"The Capy GitHub app installation's GraphQL budget runs out right after each reset" (GST-1713). Every crew's watchers shared one 7,700-point hourly budget, so merges stalled across crews. With REST reads, a 60-second poll floor and waiting out rate limits, a watcher costs about one GraphQL read every 10 minutes instead of about 360 an hour.
Approvals
No approvals recorded for branch capy/gstacks-github-waits-poll.
Summary
The Capy app installation's GraphQL budget ran dry because gstack's watchers polled
gh pr viewandgh pr checksevery few seconds. This PR moves the watchers onto REST, puts a 60-second floor under every GitHub wait, and makes a rate-limited call wait for its reset. /ship's review then fixed four problems in the first version:gstack-ci-gate --registration-wait 60never re-polled, a rate-limitedgh pr mergecould sleep up to about 130 minutes while holding the land lock, a bot-authored PR's own comments counted as events, andgstack-ci-carry waitnow documents that it can overrun its timeout by one poll.Test Coverage
Tests: 1751 → 1751 test files (the review fixes added 3 test cases to existing files; Step 7 ran audit-only and generated none).
Coverage: 67% value-weighted (75% including 1 weakly covered path).
Test value: 0 tests written, 0 rejected by the authoring gate, 3 existing tests extended, 1 path weakly covered.
Gap 3 in the diagram is not real. The audit diffed against the moved
origin/main, so it read two newer main commits as deletions. This PR's change tobin/gstack-ship-receiptis only theghSpawnSyncswap (git diff dd97420 HEAD -- bin/gstack-ship-receipt).Regression proof for the review fixes: the 3 new cases fail at 5549950 and pass at ea06cce (
test/gh-poll.test.ts,test/gstack-ci-gate.test.ts).Tests ran: free 1626 (files; carried from CI run 42343130375/1)
Pre-Landing Review
Pass 1 (full panel: batched testing, security and state-machines specialist, then Red Team) found 5 findings. Four were fixed in ea06cce and one was skipped:
lib/ci-gate.ts:221:--registration-wait 60broke before any re-poll once the first read took any time. The loop now polls while time is left in the window.lib/ship-queue-land-direct.ts:237: a rate-limitedgh pr mergeslept through the reset while holding the land lock, which becomes breakable after 10 minutes.gh pr mergenow fails at once.lib/pr-wait.ts:504: a REST[bot]author did not match GraphQL's login, so the bot's own comments counted as events. The suffix is now stripped.lib/ci-carry.ts:151:waitForRuncan overrun--timeoutby one poll. This is now documented.lib/gh-poll.ts:159: superseded check runs from another check suite on the same head still count (see Deferred findings).Pass 2 read the fix range only (5549950..ea06cce). The specialist returned NO FINDINGS, and the Red Team raised one note (below). Then Step 11 ran the native adversarial voice and the outside voice (openai/gpt-6.1-sol, completed). Both raised findings above the tool's blocking line, and they are listed under Deferred findings: the crew caps /ship at one fix round and two review rounds, and both were used.
REVIEW_LOOP: passes=2 blocking_found=4 deferred=2 noted=1 fixed=4 scope=frozen ledgers_read=4 dispatches=8
Gate edits: none detected (21 listed, 12 read; patterns: RH-1,RH-3,RH-4,RH-12,RH-13,RH-14,RH-15,RH-16; unlisted idioms: none)
Outside review: completed (openai/gpt-6.1-sol via host subagent). Its recommendation was request changes, for the two deferred findings below.
Deferred findings
These are left for the lane and Captain to decide. No issue was filed, because the crew files nothing on GitHub as garrytan.
lib/gh-poll.ts:161/lib/pr-wait-github.ts:185:parseHeadChecksdedupes runs per check suite, andfetchChecksthen collapses each name to its worst bucket. An older failed or cancelled run from another suite on the same head, such as apull_requestrun followed by aworkflow_dispatchrerun, keepsgstack-pr-waitat PR_WAIT_BLOCKED whilegstack-ci-gateand the land gate read green. It fails closed, so nothing untested merges, but it stalls the waiter on the crew's own tree_mismatch recovery path. Three readers agreed. Recommended follow-up: supersede runs per app and name, aslatestRunPerNamedoes.lib/pr-wait.ts:497: a push between the state read and the checks read can reportgreen on <old sha>. Lead judgment: the verdict names the head it checked, the oldgh pr checksread had the same race just after its read, and land re-checks CI at its--pin. Not fixed.Review notes
Below the blocking floor (advisory, informational, test-gap or wording items); recorded here and not fixed in this PR.
lib/ship-queue-land-direct.ts:241The pr merge fail-fast is only a partial fix. Its stated reason is that nothing should sleep under the land lock past PUSH_STALE_MS (gh-poll.ts:79-83), but the post-mergegh pr viewreadback runs inside the same locked section with phase 'merging' (set at line 235). It uses the same ghSpawnSync/withRateLimit runner (bin/gstack-ship-queue:170), and that runner was added on this branch in 8d10186.pr view --json ...mergeCommitis a GraphQL call, and the shared installation budget is exactly the one the changelog says other crews drain right after each reset. So a rate-limited readback can sleep up to MAX_LIMIT_WAITS x 65 min (about 2 h 10 min) after a successful merge. Once 10 minutes pass,--break-lockis allowed, and a second lander can take the lock and merge while the first run is still in verifyMerge (merged checks on a stale tip, land note). The CAS release (ship-queue-land-lock.ts:137-143) stops the first run from clobbering the new holder, so this is a broken-exclusion window, not lost data. pr merge itself is fixed. The same pattern can apply to any other gh call made while the lock is held (train land paths).lib/ship-queue-land-direct.ts:2412. INVESTIGATE: rate-limit sleeps now run while the land lock is held in themergingphase.failsFast(gh-poll.ts) exempts onlygh pr merge, and its own comment names the hazard: the lock becomes breakable afterPUSH_STALE_MS(10 min). But land-direct.ts:235-246 sets the phase tomergingand then runs the readbackview(), which isgh pr view(GraphQL, the budget most likely to belib/gh-poll.ts:1063. INVESTIGATE: per-call timeouts no longer bound wall time.ghSpawnSyncand the wrappedspawnGhturn thetimeout/timeoutMsthat callers pass (run-evidencegh(args, timeoutMs), gstack-ship-receipt's 120 s, ship-bin-common's 60 s) into a per-spawn limit, so one rate limit can stretch a call to about 131 min. Separately,runCiGate(ci-gate.ts:~219) now sleeps a full 60-75 s even whenlib/gh-poll.ts:954. FIXABLE: rate-limit detection reads stdout from commands whose stdout carries CI-controlled text.withRateLimitrunsrateLimitKind(args, stderr + stdout)whenever the exit status is nonzero.gh pr checksexits 1 or 8 with stdout listing check names and descriptions, which workflows (including fork PRs) control. A check titled with "rate limit exceeded" makes ci-gate readrate_limit,lib/pr-wait-github.ts:1365. FIXABLE:fetchBaseOidinterpolates the branch name into the REST path without encoding. It buildsrepos/${repo}/git/ref/heads/${base}raw (pr-wait-github.ts:~136). A base containing#,?or%yields a 404, which is treated as non-transient and returns null. That silently turns off the "base moved and touches this PR's files" event, which the old GraphQLbaseRef.target.oidread dlib/pr-wait-github.ts:1366. INVESTIGATE: load moves from GraphQL onto the shared REST core budget. Each pr-wait poll now makes at least 4 REST calls (pull, paginated check-runs, status, git ref) every 60-75 s, about 200-240 an hour per watcher, before compare calls. Under--until ready, once CI is green the GraphQL conversation read is forced on every poll, even while waiting on BEHIND or a gating bot. With many creExploratory QA
Smoke only, report-only. The full exploratory method files were not read in this unattended run, so this is partial coverage.
gstack-ci-gate --repo garrytan/gstack --pr 4109 --expect-head 5549950(one read, no wait) printedVERDICT PASS 5549950b66c1733503743af131ce7e482c3518fe.gstack-pr-wait --helpshows the new interval floor and the REST-plus-cached-GraphQL read.gstack-pr-waitwatch, which takes at least two 60-second polls.Design Review
No frontend files changed — design review skipped.
Eval Results
Not run. The eval selector (
scripts/eval-select.ts --base main, pr profile) picks 115 paid behavioral probes, becausetest/helpers/free-fixtures-data.tsand the gate code select every dependent case. That is far past the lane's $10 paid-run cap, so running it needs the Captain's decision. Quality judges: none.Review bot comments
No review bot comments. The only bot comment on the PR is trunk-io[bot]'s merge-queue placeholder.
Scope Drift
Scope Check: CLEAN. Every change serves GST-1713 (poll floor, rate-limit wait, REST reads, call-site swaps, tests, fragment), and the review fixes stay in the same modules.
Plan Completion
Plan completion audit: not run (no plan is bound to this branch and no docs/designs/ file matches). Fix: add "Plan: " to the PR body, or run /autoplan.
Verification Results
No Step 8.1 plan obligations (no plan bound). The repro for each review fix is a failing-first test: 3 FAIL at 5549950, 3 PASS at ea06cce.
TODOS
No TODO items completed in this PR.
Documentation
Status: current. No documentation edits were needed.
Scope: dd97420..ea06cce (3 commits, 42 files) plus the unstaged changelog.d fragment. I audited docs/ship-policy.md, docs/standalone-binaries.md, docs/troubleshooting.md (gstack-pr-wait), the land-and-deploy templates and their generated outputs, and ship/sections/greptile.md.tmpl against lib/gh-poll.ts, lib/pr-wait.ts, lib/greptile-early.ts and scripts/eval-bg.ts.
Documentation health:
--interval 60matches the code)Coverage debt: the
lib/gh-poll.tspolicy (60 s floor plus jitter, waiting out rate limits and retrying at most twice) is described only in the changelog fragment and the module header, which is reference-only coverage with no how-to or explanation. No critical gaps. Diagram drift: none.Test plan
bun run test(free lane): 1626 test files carried from CI run 42343130375/1 (free-tests.yml workflow_dispatch at tree 3a67da058b71; 36161 tests), EVIDENCE FRESHbun testgh-poll (13), gstack-ci-gate (29), pr-wait (52), gh-poll-tripwire (3), ci-carry (7), ship-ci-carry (50), land-and-deploy-ci-gate (46), ship-queue-land-direct (53), ship-queue-land-lock (14), ship-queue-land (36): all pass at ea06cce's codebun run typecheckclean,bun run typecheck:testno new diagnostics, module-size ratchet and spawnsync tripwire pass, conflict markers none🤖 Generated with Claude Code