Repository navigation
fix(bin): use conditional GitHub REST reads and stop sweeps below a quota floor - #88
Merged
Merged
Conversation
…and contributions sweeps Add bin/fm-gh-rest.sh: GET with If-None-Match from a per-URL ETag cache under state/ (a 304 serves the cached body and is not counted against the rate limit), recording X-RateLimit-* from every response. Route fm-contributions, fm_open_loops, fm-pr-state and fm-pr-reviewers REST reads through it. Below the floor (FM_GH_RATE_FLOOR_PERCENT, default 15) the contributions poll keeps each row's last observation marked stale with the reset time, and the ledger republishes its last rows marked stale instead of publishing partial rows.
…and contributions sweeps Add bin/fm-gh-rest.sh: GET with If-None-Match from a per-URL ETag cache under state/ (a 304 serves the cached body and is not counted against the rate limit), recording X-RateLimit-* from every response. Route fm-contributions, fm_open_loops, fm-pr-state and fm-pr-reviewers REST reads through it. Below the floor (FM_GH_RATE_FLOOR_PERCENT, default 15) the contributions poll keeps each row's last observation marked stale with the reset time, and the ledger republishes its last rows marked stale instead of publishing partial rows.
…ral regression coverage
…s-observer unregistration to the shared bounded-watcher fixture, preventing unrelated observer wakes from preempting PR-security scenarios. Removed redundant per-case cleanup and documented the isolation. The previously failing self-merge scenario and focused transition, retry, and concurrent-publication scenarios pass. Check-unregister tests, shell syntax, and documentation audience checks pass. The scoped CI runner progressed through the reported failure and subsequent retirement/authority cases, but the full security suite remains red at a later, separate teardown fixture failure: it records a nonexistent project, preventing nested-worktree ownership verification. That fixture and teardown implementation are unchanged from the base commit; they were left untouched. Production behavior was not changed
…and contributions sweeps Add bin/fm-gh-rest.sh: GET with If-None-Match from a per-URL ETag cache under state/ (a 304 serves the cached body and is not counted against the rate limit), recording X-RateLimit-* from every response. Route fm-contributions, fm_open_loops, fm-pr-state and fm-pr-reviewers REST reads through it. Below the floor (FM_GH_RATE_FLOOR_PERCENT, default 15) the contributions poll keeps each row's last observation marked stale with the reset time, and the ledger republishes its last rows marked stale instead of publishing partial rows.
…ral regression coverage
…s-observer unregistration to the shared bounded-watcher fixture, preventing unrelated observer wakes from preempting PR-security scenarios. Removed redundant per-case cleanup and documented the isolation. The previously failing self-merge scenario and focused transition, retry, and concurrent-publication scenarios pass. Check-unregister tests, shell syntax, and documentation audience checks pass. The scoped CI runner progressed through the reported failure and subsequent retirement/authority cases, but the full security suite remains red at a later, separate teardown fixture failure: it records a nonexistent project, preventing nested-worktree ownership verification. That fixture and teardown implementation are unchanged from the base commit; they were left untouched. Production behavior was not changed
… its isolation proof is recorded
…or in fm-gh-rest get
…g with fm-gh-rest
This was referenced Oct 10, 2026
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.
Intent
I would like all bugs to be fixed so tomorrow can be focused entirely to Vernant and not problems preventning Vernant from getting built.
Context: GitHub rate limiting has repeatedly stopped fleet work (no-mistakes push and CI steps failing on gh pr view, a Vernant lane PR report published incomplete with rows missing). The measured scout report data/fm-fleet-github-graphql-budget/report.md (in the main firstmate home) found: the core (REST) bucket runs at about 80 percent of 5000/hour with no unusual load; the open-work ledger in 7 lane homes spent 88 calls per run until those homes were updated to PR #56 (done 2026-10-08); the contributions poll spends about 1150 core calls/hour re-reading 12 unchanged PRs; conditional GETs with If-None-Match that return 304 do not count against the limit; nothing in tracked code reads the remaining quota or degrades when it is low; gh api rate_limit reads wrong on this account - use X-RateLimit-* headers. GraphQL was exhausted on 2026-10-06 ~21:26Z, 2026-10-08 02:53Z and ~04:45Z by a burst of about 8x baseline whose consumer is unknown (the ledger and no-mistakes are ruled out).
What Changed
bin/fm-gh-rest.sh, a shared REST read helper with two commands:getandguard.getkeeps a per-URL ETag cache instate/gh-rest-cache/and sendsIf-None-Match. When GitHub replies 304, the helper returns the cached body. It followsLink rel="next"pagination. If a 304 has no Link header and the cached page is full, it reads that page again without a condition. It records theX-RateLimit-*response headers instate/gh-ratelimit.<resource>.json. Concurrent writers keep the lowest remaining value for the newest reset window. Before each request, the helper checks a fixed floor of 15% of the core limit. Below the floor, it exits 75 and prints no partial output.bin/fm-contributions.shnow sends its REST reads through the helper, and it runsguardbefore its GraphQLghreads. When the quota is below the floor, the poll stops reading the forge. It keeps the last observation of each owner that it did not measure, and it sets that record's error to the quota-reset reason. It announces this once per reset window.bin/fm_open_loops.pynow reads PRs throughfm-gh-rest.sh get --paginate --slurp, in place ofgh-axi apiwith base64. On a quota refusal it drops the partial collection. It returns the last published ledger, with each row markedstale: true. It also setscomplete: false,stale_reason, andstale_until_epoch, and adds aledger stalecoverage row. If no earlier ledger exists, it adds aledger degradedrow.tests/fm-gh-rest.test.sh, an HTTP-faithfulghtest double (tests/assets/gh-http-shim.sh, installed withfm_gh_http_shimintests/lib.sh), and new quota and conditional-read cases intests/fm-contributions.test.shandtests/fm-open-loops.test.sh. Registers the new test in thebin/fm-test-run.shfamily and path routing. The bounded watcher intests/fm-pr-check-security.test.shnow unregisters the contributions observer before it starts. Documents the cache, quota recording, and degradation indocs/configuration.md.Risk Assessment
Testing
I ran the real helper, the contributions poll, and the open-work ledger in a disposable lab FM_HOME. Each one used the machine's real gh login against real GitHub. A pass-through gh spy recorded every real call with its HTTP status and X-RateLimit-Used. Results: (1) warm reads were all 304 replies, and sequential 304s left X-RateLimit-Used unchanged. (2) Pagination and the full-last-page re-read worked, and real GitHub 304s carry no Link header, so that path is real. (3) With the recorded quota below the floor, the helper exited 75 and the poll and ledger made zero gh calls. They kept the last observations, marked them stale, and announced the episode once. (4) Both sweeps recovered after the window reset. (5) The R1 case (a real 404 after a quota episode still wakes firstmate) passed. (6) The R2 prune case passed. I could not make real GitHub report low quota on a 304, so R3 is untested live. The focused tests/fm-gh-rest.test.sh covers R3 with a shim and passed 17/17. This is a CLI change with no UI, so I captured CLI transcripts and no screenshots. I tore down the lab. The worktree is clean.
bash tests/fm-gh-rest.test.shcase ('ok - a full cached last page is not re-read once its 304 re…Evidence: Live conditional GET: 200 then two uncounted 304s
Source: Live conditional GET: 200 then two uncounted 304s
Evidence: Live paginated conditional read (3 pages)
Source: Live paginated conditional read (3 pages)
Evidence: Live full-last-page re-read (real 304 omits Link)
Source: Live full-last-page re-read (real 304 omits Link)
Evidence: Quota floor guard/get refusal, boundary, expiry
Source: Quota floor guard/get refusal, boundary, expiry
Evidence: Live contributions poll: warm poll is 7/7 REST 304
Source: Live contributions poll: warm poll is 7/7 REST 304
Evidence: Live contributions poll below floor: zero calls, stale mark, one announcement, recovery
Source: Live contributions poll below floor: zero calls, stale mark, one announcement, recovery
Evidence: R1: real 404 after quota episode still prints observation unavailable
Source: R1: real 404 after quota episode still prints observation unavailable
Evidence: Live open-work ledger: 304s, retained stale result, recovery
Source: Live open-work ledger: 304s, retained stale result, recovery
Evidence: Ledger below floor with no prior ledger: degraded row
Source: Ledger below floor with no prior ledger: degraded row
Evidence: R2: aged staged temp files pruned, fresh kept
Source: R2: aged staged temp files pruned, fresh kept
Evidence: tests/fm-gh-rest.test.sh output (17/17 ok)
Source: tests/fm-gh-rest.test.sh output (17/17 ok)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-contributions.sh:461- A quota-stale mark hides the next real failure. mark_quota_stale (bin/fm-contributions.sh:252-278) writes the quota reason into each owner's.error. The once-per-episode wake check at bin/fm-contributions.sh:461-463 prints 'observation unavailable' only when every owner has.error == null. Concrete sequence: poll N runs below the floor and marks PR 8 stale. The window resets. In poll N+1, guard returns empty, observe() gets a real HTTP 502, and no quota-low file exists. The all(.error == null) test is false because the quota reason is still saved. So the new genuine failure episode is never announced and firstmate is not woken. The row is still written with the generic error, so later polls also stay silent. Fix at the same boundary: in the jq test at line 461-463, treat a saved quota-reason error (one whose text contains ' quota low (') as no prior failure. The same invariant must hold in the announce check in mark_quota_stale at bin/fm-contributions.sh:257-263, which already scopes itself to quota errors and needs no change.bin/fm-gh-rest.sh:92- Staged temp files can leak forever. fetch_page stages cache entries as$CACHE/.entry.XXXXXX(bin/fm-gh-rest.sh:165, :182). record_rate stages$STATE/.gh-ratelimit.XXXXXX(:72). These files are removed only on the normal path. Callers kill the helper on purpose: contributions forge() wraps it in fm_run_timed with a bound of 5 s or less, and the ledger's stop_command sends SIGKILL. The staged entry also lives through record_response's lock wait of up to 2 s (:113), so a kill inside that window is reachable. prune_cache (:92) deletes only*.json, so each leaked full-body.entry.*copy stays in state/gh-rest-cache for good. Fix: in the same prune pass, also delete.entry.*in $CACHE and.gh-ratelimit.*in $STATE that are older than a short age (for example -mmin +60).bin/fm-gh-rest.sh:161- The full-last-page re-read skips the quota floor. A 304 with no Link header for a full cached last page records that 304's headers (:159), then at once makes a counted unconditional GET (:161). It does not return to cmd_get's per-page quota_low check (:224). If the 304 reported quota below the floor, the helper still spends one more counted call. docs/configuration.md says 'every REST read checks the floor before each page'. Fix: call quota_low core after record_response at :159, and exit 75 with the reason before the unconditional fetch_page.bin/fm-contributions.sh:316- GraphQL quota is still not tracked. The intent says GraphQL was used up by a burst from an unknown consumer. This change records and enforces only the RESTcorebucket. The contributions poll's per-PR GraphQL read (gh pr viewat :316) is gated on the core bucket, not the graphql bucket. This does not contradict the intent, because the burst consumer is unknown and the documented scope is REST. It only means this change does not prevent the GraphQL exhaustion that broke no-mistakesgh pr view.🔧 Fix applied.
3 issues (1 warning, 2 infos) still open:
bin/fm-contributions.sh:461- A quota-stale mark hides the next real failure. mark_quota_stale (bin/fm-contributions.sh:252-278) writes the quota reason into each owner's.error. The once-per-episode wake check at bin/fm-contributions.sh:461-463 prints 'observation unavailable' only when every owner has.error == null. Concrete sequence: poll N runs below the floor and marks PR 8 stale. The window resets. In poll N+1, guard returns empty, observe() gets a real HTTP 502, and no quota-low file exists. The all(.error == null) test is false because the quota reason is still saved. So the new genuine failure episode is never announced and firstmate is not woken. The row is still written with the generic error, so later polls also stay silent. Fix at the same boundary: in the jq test at line 461-463, treat a saved quota-reason error (one whose text contains ' quota low (') as no prior failure. The same invariant must hold in the announce check in mark_quota_stale at bin/fm-contributions.sh:257-263, which already scopes itself to quota errors and needs no change.bin/fm-contributions.sh:316- GraphQL quota is still not tracked. The intent says GraphQL was used up by a burst from an unknown consumer. This change records and enforces only the RESTcorebucket. The contributions poll's per-PR GraphQL read (gh pr viewat :316) is gated on the core bucket, not the graphql bucket. This does not contradict the intent, because the burst consumer is unknown and the documented scope is REST. It only means this change does not prevent the GraphQL exhaustion that broke no-mistakesgh pr view.bin/fm-contributions.sh:463- Round 1 fix R1 is correct. It has one side effect. Sequence: a real forge failure wakes firstmate. Then a quota-low episode replaces the saved error with the quota reason (mark_quota_stale, bin/fm-contributions.sh:275). Then the window resets and the forge still fails. The poll now prints 'observation unavailable' a second time for the same outage. This is at most one more wake per quota window, and it is the safe direction (no hidden failure). Two comments still state the old rule 'no prior owner has an error': bin/fm-contributions.sh:75-76 and bin/fm-contributions.sh:460. No action is necessary.✅ **Test** - passed
✅ No issues found.
bash tests/fm-gh-rest.test.shcase ('ok - a full cached last page is not re-read once its 304 re…bin/fm-lab-home.sh create $LAB(disposable marked lab FM_HOME, removed withrm -rf $LABat the end)Pass-throughghspy on PATH that runs the real /opt/homebrew/bin/gh and logs HTTP status plus X-RateLimit-Used/Remaining for each callFM_HOME=$LAB bin/fm-gh-rest.sh get repos/MrGTV-love/firstmate/pulls/81x3 (cold then warm) against real GitHubbin/fm-gh-rest.sh get 'repos/MrGTV-love/firstmate/pulls?state=all&per_page=40' --paginate --slurpcold and warm (3 pages)bin/fm-gh-rest.sh get 'repos/MrGTV-love/firstmate/pulls?state=all&per_page=42' --paginate --slurpcold and warm (exactly-full last page)bin/fm-gh-rest.sh guardandgetwith a lab quota record below, at, and above the floor, with an expired window, and with no recordbin/fm-contributions.sh pollin the lab home (private TMUX_TMPDIR) owning real PR MrGTV-love/firstmate#84: cold, warm, below floor x2, after window resetR1 sequence:fm-contributions.sh pollowning nonexistent PR #999999: quota-stale poll, then reset poll with a real 404, then repeat pollbin/fm-open-loops.sh --json --heartbeatin the lab home: cold, warm, below floor (JSON and TOON), after reset, and below floor with no prior ledgerR2: aged.entry.*and.gh-ratelimit.*staged files plus fresh ones, then a realfm-gh-rest.sh getbash tests/fm-gh-rest.test.sh(17/17 ok, including the R3 case: a 304 that reports low quota on a full last page; this is a shim-backed test, not a live run)✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → no changes applied ✅
🔧 No changes applied.
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.