Skip to content

fix(pr-pipeline): stop a GitHub API pool at its stop floor, not only at zero - #15305

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/rate-guard-stop-near-zero
Nov 11, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/rate-guard-stop-near-zero

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hub lessons-ledger finding L532 (breaker trip leak, GitHub 403 on 2026-11-11 07:26:48Z)

Summary

On 2026-11-11 the GITHUB_TOKEN window went from 475 remaining at 07:17Z to 0 at 07:25Z, and GitHub then answered 403 "API rate limit exceeded for installation". The Node rate guard only warned under its read floor ("reads continue") and stopped a pool only after GitHub refused a call. GitHub's docs say "Continuing to make requests while you are rate limited may result in the banning of your integration" (rate limits for the REST API).

Fix

  • .github/actions/log-api-budget/rate-guard.mjs: classify takes a stopFloor option (off by default, so the shared vectors and the shell and Python ports are unchanged). The latch passes limits.api.<pool>.stop-floor, read through the same margin() path as read-floor. A response whose x-ratelimit-remaining is at or under that share of x-ratelimit-limit now latches like a remaining of 0: the stop is written to the job's stop file, later calls to that pool are refused until x-ratelimit-reset, and the step holds under API_BUDGET_HOLD, else fails. The log line says it stopped at the floor ("left GitHub's core API 100 of 5000 requests, at or under its stop-floor share 0.02"), so nobody reads it as a refusal. A response with no readable limit, remaining or future reset never stops on the floor (fail open). The refusal path is unchanged.
  • .github/actions/log-api-budget/rate-limits.json: stop-floor: 0.02 for every pool, mirrored in the built-in SHARES. No existing share, TRIP_PCT or CLEAR_PCT changed.
  • docs/conventions/pr-pipeline/pr-pipeline.schema.json: stop-floor added to api-pool and api-content-pool (both additionalProperties: false), so a limits.api override in pr-pipeline.yaml validates. Descriptions in the schema, the README api row and the pr-pipeline.yaml comment are updated where they said the guard stops only at zero.
  • Changelog fragment .changes/pr-pipeline/fix-rate-guard-stop-near-zero-d2c8e526.md (minor: a new config field).

Cost per run: no added API calls. The check reads rate-limit headers on responses the guard already receives.

Verification

  • node --test --test-concurrency=4 ".github/actions/log-api-budget/*.test.mjs" ".github/actions/resolve-config/*.test.mjs": 1248 pass, 0 fail.
  • New cases in rate-guard.test.mjs (threshold 100 = 0.02 x 5000, by hand): remaining 100, 99 and 1 stop, record core 1792751600 in the stop file, refuse the next call and fail the step; 101 sends on with no error; a holding latch logs a notice and fails nothing; missing limit, unparseable remaining or limit, missing reset, or a reset already past never stop; a limits.api override of 0 stops nothing at 1 left; a 403 refusal still logs "hit GitHub's core API rate limit".
  • Suites with low-remaining fixtures (report-check-run, request-budget, publish-lane-level, lane-hold, dispatch, refire-refused, feedback-dispatch, label-unblocked, resolve-pr): 1002 pass, 0 fail.
  • python3 -m unittest on test_babysit_gh_rate_guard.py: 46 pass.

Related

  • Not in this PR: a kill-switch step at the top of work-resolve-issue.yml's admit and pick-slot jobs. A run stopped there would end with admit green and its blocker step skipped, which didNoWork in .github/scripts/work-unblock/label-unblocked.mjs classifies as no stall, so work-unblock's sweep would never re-fire the issue once the switch clears. Today such a run reaches broker-check's kill-switch step and is re-fired as a breaker trip. That change needs the sweep to learn the new stop first; it is left to the hub.
  • api-budget.bash (the shell guard) and babysit_gh.py still stop only at zero; both read the shared vectors, which this change leaves alone.

session: GitHub Support

🤖 Generated with Claude Code

…at zero

The Node rate guard warned under the read floor and kept sending until
GitHub refused a call. On 2026-11-11 the GITHUB_TOKEN window went from 475
remaining to 0 in eight minutes and GitHub answered 403 "API rate limit
exceeded for installation".

A response whose x-ratelimit-remaining is at or under the pool's
limits.api stop-floor share of x-ratelimit-limit (default 0.02 in
rate-limits.json) now latches like a remaining of 0: the stop goes in the
job's stop file, later calls to the pool are refused until the header's
reset, and the step holds under API_BUDGET_HOLD, else fails. A response
with no readable limit, remaining or future reset never stops on it. The
check reads headers already returned, so it adds no API call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 11, 2026 08:19
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-11T08:21:22.110387Z eff99eb Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eff99eb4a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed eff99eb4a01b1c9da4dc0983f92c1b70b9645c74: read 7 of 7 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #15305, head eff99eb4

No security issues found. I read every hunk of all 7 changed files.

What the change does. It adds an optional stop-floor share (0.02 by default) to the Node rate guard. A GitHub response whose x-ratelimit-remaining is at or under stop-floor × x-ratelimit-limit now trips the latch the same way a remaining of 0 does: the stop goes into the job's stop file, later calls to that pool are refused until x-ratelimit-reset, and the step holds under API_BUDGET_HOLD or else fails. The default is mirrored in rate-limits.json, in the built-in SHARES, and in the api-pool / api-content-pool schema entries, with matching doc updates.

Trusted paths I checked closely.

  • .github/actions/log-api-budget/rate-guard.mjs
    • The new floored condition only adds ways to latch. Every path that stopped at base still stops: remaining === 0, retry-after, a refusal message, a GraphQL rate-limit error and 429.
    • floored stays false unless the limit, remaining and a future reset are all readable. That only limits when the new stop fires; it removes no base stop.
    • stopFloor arrives through the existing margin() path (rate-guard.mjs:557-568), which only returns numbers. FIELDS checks it as a share in [0, 1].
    • refusedForLimit(response) now runs every time the latch trips, not behind the resend && waits(url) short-circuit. It only reads a cloned body (rate-guard.mjs:481-491), and resent is computed the same way as before.
    • The new log text uses header values parsed with Number() and the numeric stopFloor, so header text cannot inject into the ::error:: / ::notice:: workflow command beyond what the log line already showed at base.
  • .github/actions/log-api-budget/rate-limits.json: the only change is a new stop-floor: 0.02 key on each pool. No existing share, cap or pace value changed.
  • docs/conventions/pr-pipeline.yaml: comment text only. No lane slot, activity effect or merge setting changed, so nothing the token broker mints is affected.

Trust controls. Nothing here widens who passes the trusted-trigger check, any token's scope, the reach of the kill switch, or any merge hold. The change makes the API stop stricter, and it fails closed: a stop under the floor holds or fails the step.

Instruction surface and manifests. No instruction file or plugin manifest is touched.

Spend. No added API calls; the check reads rate-limit headers on responses the guard already gets. Nothing in this change can bill.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed eff99eb4a01b1c9da4dc0983f92c1b70b9645c74: read 7 of 7 changed file(s); 0 finding(s), 0 as inline comments.

Code review: stop a GitHub API pool at its stop floor

No blocking or advisory findings. I read every hunk of all 7 changed files. I also read the unchanged code the changed lines call in rate-guard.mjs: readDefaults, apiLimits, margin, classify, refusedForLimit, observe, and the test helpers latchFor and fakeGitHub.

What I checked

  • The stop check (classify, floored). A pool only stops on the floor when all of these hold:

    • the header's limit is greater than 0
    • remaining and reset are integers
    • the reset is still in the future

    If any header is missing or unreadable, it does not stop (fail open), as the PR says. The reset time it records comes only from x-ratelimit-reset, which is always in the future here, so the step never falls back to a guessed wait. With no retry-after or secondary-limit signal, the pool comes from x-ratelimit-resource, so the stop holds that pool only, not every pool. stopFloor defaults to null, so the shared vectors and the shell and Python ports behave as before.

  • Config path. margin(pool, "stop-floor", token) gives null for anything that isn't a number. apiLimits merges field by field, so if limits.api.<pool> is set without stop-floor, the field still comes from rate-limits.json. readDefaults accepts the new key because it is listed in FIELDS and CONTENT_FIELDS, and it is not in REQUIRED. The schema adds stop-floor to both api-pool and api-content-pool (both additionalProperties: false), so an override validates. resolve-config has no field list of its own. The built-in SHARES match rate-limits.json.

  • Other readers of rate-limits.json. babysit_gh.py reads only LIMITS_FIELDS = ("writer-pause", "read-floor") plus limit-cap, so the extra key does not affect it. api-budget.bash reads no file. I found no log parser that depends on the old hit GitHub's … rate limit text in the Node guard's log line, so the new left GitHub's … wording breaks no consumer.

  • observe changes. refusedForLimit is now called on every response that triggers a stop, where before it ran only on the resend path. It reads a clone of the body, so the body the caller gets back is still intact. On a 200 response that hits the floor, refused is false, so the call is never resent and the step holds or fails as described. The log message is worked out again from the raw headers. It can only call a stop a floor stop when no refusal happened and the remaining is between 1 and the floor, which matches floored in every case where only the floor triggered the stop.

  • Tests. The new cases cover:

    • the boundary: 100, 99 and 1 stop; 101 does not
    • the stop being recorded in the stop file, and the next call refused
    • the holding latch: a notice, and nothing fails
    • fail-open on a missing or unparsable limit, remaining or reset, and on a reset already past
    • an override of 0
    • a refusal still logged as "hit GitHub's … rate limit"

    I did not run them. The PR's pass counts (1248 Node, 1002 for the suites with low-remaining fixtures, 46 Python) are the author's claim and I have not verified them.

Closing issue

No closing issue: the PR cites hub lessons-ledger finding L532, so there are no issue acceptance criteria to check. The changelog fragment's bump: minor matches a new config field.

Notes

  • Spend: none. The check reads rate-limit headers on responses the guard already gets, so it adds no API calls.
  • Merge gate: rate-limits.json changes a default that the merge hold lists floor, so the repository's own merge gate will wait for a person on that. This lane is not holding the PR for it.
  • Embedded instructions: none found in the PR context.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

pr-unblock / address-feedback finished round 1 of max 4 on eff99eb.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Nov 11, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The pr-unblock lane stopped on this pull request and needs a person: an adjudicator found that a disputed review finding turns on a design choice, which a person decides.

Reason: design

  • Reason: feedback-question-design
  • Resume key: eff99eb4a01b1c9da4dc0983f92c1b70b9645c74 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=15305 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=15305 -f activity=address-feedback

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Nov 11, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit 375aba3 Nov 11, 2026
137 of 139 checks passed
@kyle-sexton
kyle-sexton deleted the fix/rate-guard-stop-near-zero branch November 11, 2026 13:04
@github-actions github-actions Bot added the merge-path: owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-path: owner-agent Post-merge accounting record (README.md, Merge paths)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant