⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
classifyMergeFailure (src/services/merge-failure.ts:118-132) reaches the infra scope for a rate-limit window only via status === 403:
if (status === 403 && isConvergenceForbiddenMessage(message))
return { terminal: false, scope: "infra", reason: `merge forbidden for now (403 ...): ${message}` };
...
return { terminal: false, scope: "commit", reason: message }; // line 132
But this repo's own GitHub client documents that a secondary limit surfaces as 403 or 429 — src/github/client.ts:422: if (response.status !== 403 && response.status !== 429) return false; — and :400-401 states that a sustained limit exhausts the retries and the response is returned to the caller, i.e. a 429 does reach handleMergeFailure as a thrown RequestError.
Mechanically: a merge fails with HTTP 429 "You have exceeded a secondary rate limit". No status branch and no message branch matches, so line 132 returns { terminal: false, scope: "commit" }. In src/services/agent-action-executor.ts:1198-1212, once attempts >= MERGE_RETRY_CAP (5) the failure is escalated to terminal, and expiresAt is only set when scope === "infra". So expiresAt is undefined, and isMergeBlockInEffect (merge-failure.ts:147-149) returns true until the head SHA advances. A green, approved PR is stranded until the contributor pushes a commit they have no reason to push.
That is exactly the outcome the module doc promises cannot happen — merge-failure.ts:113-117: "It is meaningful on the non-terminal classes too: the executor carries it through the retry-cap exhaustion path, so a sustained rate-limit window that burns MERGE_RETRY_CAP still recovers on its own rather than terminally stranding everything that passed through it" — restated verbatim at agent-action-executor.ts:1207-1210. A fleet-wide 429 window catches every in-flight merge at once, so a single window strands every PR it touches.
Requirements
classifyMergeFailure returns { terminal: false, scope: "infra" } for status === 429, placed with the other status branches and before the 403 branches so ordering is explicit.
- A 403 whose message signals a rate limit — specifically matching
secondary rate limit, abuse, or api rate limit exceeded — must also classify scope: "infra", so the 403 and 429 spellings of the same condition are treated identically. Add a single exported predicate for that message test in src/services/merge-failure.ts alongside the existing isConvergenceForbiddenMessage / isMergeConflictMessage helpers; do not import from src/github/client.ts (that module is Response-shaped, not error-shaped).
- The rate-limit 403 branch must be evaluated before the existing
if (status === 403) return { terminal: true, scope: "commit", ... } at line 124, so a rate-limited 403 no longer terminally blocks on the first failure.
- No change to
terminal for any status that is terminal today other than the rate-limited-403 case named above.
- No change to
isMergeBlockInEffect, activeMergeBlockedSha, or INFRA_MERGE_BLOCK_TTL_MS.
⚠️ Required pattern: mirror the existing status === 401 infra branch at src/services/merge-failure.ts:121 and the 403 + isConvergenceForbiddenMessage branch at :122-123 — a status/message test returning the infra scope, with the message predicate as a small named helper in this file. It does NOT satisfy this issue to make the fall-through at line 132 default to scope: "infra" (that would give every unclassified failure a TTL and defeat the commit-scoped default); to special-case 429 inside handleMergeFailure instead of in the classifier; or to give markPullRequestMergeBlocked an unconditional expiry.
Deliverables
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example handling 429 but leaving the rate-limited 403 terminally commit-scoped — does not resolve this issue.
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on src/**. src/services/merge-failure.ts and src/services/agent-action-executor.ts are both inside coverage.include. Every new conditional needs both arms: 429 vs not, rate-limited-403 message vs ordinary-403 message. The retry-cap-exhaustion test is the required named regression test.
Expected Outcome
A GitHub secondary-rate-limit window no longer converts every in-flight merge into a permanent, head-scoped block. Blocks caused by a 429 (or a rate-limited 403) expire on INFRA_MERGE_BLOCK_TTL_MS and are re-probed autonomously, which is what the module already documents.
Links & Resources
src/services/merge-failure.ts:100-132, :147-149; src/services/agent-action-executor.ts:1190-1212; src/github/client.ts:395-428.
Context
classifyMergeFailure(src/services/merge-failure.ts:118-132) reaches theinfrascope for a rate-limit window only viastatus === 403:But this repo's own GitHub client documents that a secondary limit surfaces as 403 or 429 —
src/github/client.ts:422:if (response.status !== 403 && response.status !== 429) return false;— and:400-401states that a sustained limit exhausts the retries and the response is returned to the caller, i.e. a 429 does reachhandleMergeFailureas a thrownRequestError.Mechanically: a merge fails with HTTP 429 "You have exceeded a secondary rate limit". No status branch and no message branch matches, so line 132 returns
{ terminal: false, scope: "commit" }. Insrc/services/agent-action-executor.ts:1198-1212, onceattempts >= MERGE_RETRY_CAP(5) the failure is escalated to terminal, andexpiresAtis only set whenscope === "infra". SoexpiresAtisundefined, andisMergeBlockInEffect(merge-failure.ts:147-149) returnstrueuntil the head SHA advances. A green, approved PR is stranded until the contributor pushes a commit they have no reason to push.That is exactly the outcome the module doc promises cannot happen —
merge-failure.ts:113-117: "It is meaningful on the non-terminal classes too: the executor carries it through the retry-cap exhaustion path, so a sustained rate-limit window that burns MERGE_RETRY_CAP still recovers on its own rather than terminally stranding everything that passed through it" — restated verbatim atagent-action-executor.ts:1207-1210. A fleet-wide 429 window catches every in-flight merge at once, so a single window strands every PR it touches.Requirements
classifyMergeFailurereturns{ terminal: false, scope: "infra" }forstatus === 429, placed with the other status branches and before the403branches so ordering is explicit.secondary rate limit,abuse, orapi rate limit exceeded— must also classifyscope: "infra", so the 403 and 429 spellings of the same condition are treated identically. Add a single exported predicate for that message test insrc/services/merge-failure.tsalongside the existingisConvergenceForbiddenMessage/isMergeConflictMessagehelpers; do not import fromsrc/github/client.ts(that module is Response-shaped, not error-shaped).if (status === 403) return { terminal: true, scope: "commit", ... }at line 124, so a rate-limited 403 no longer terminally blocks on the first failure.terminalfor any status that is terminal today other than the rate-limited-403 case named above.isMergeBlockInEffect,activeMergeBlockedSha, orINFRA_MERGE_BLOCK_TTL_MS.Deliverables
classifyMergeFailurewith a 429RequestErrorreturns{ terminal: false, scope: "infra" }— asserted by a new case in the merge-failure unit test file.classifyMergeFailurewith a 403 whose message containssecondary rate limitreturnsscope: "infra"andterminal: false, while a 403 with an ordinary permission message still returns{ terminal: true, scope: "commit" }— both arms asserted.handleMergeFailurethroughMERGE_RETRY_CAP429 failures and asserts the persisted merge block carries a non-nullmergeBlockedUntil, and thatisMergeBlockInEffect(block, headSha, nowMs + INFRA_MERGE_BLOCK_TTL_MS + 1)isfalse.mergeBlockedUntil == null(commit-scoped behaviour unchanged).All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example handling 429 but leaving the rate-limited 403 terminally commit-scoped — does not resolve this issue.
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on
src/**.src/services/merge-failure.tsandsrc/services/agent-action-executor.tsare both insidecoverage.include. Every new conditional needs both arms: 429 vs not, rate-limited-403 message vs ordinary-403 message. The retry-cap-exhaustion test is the required named regression test.Expected Outcome
A GitHub secondary-rate-limit window no longer converts every in-flight merge into a permanent, head-scoped block. Blocks caused by a 429 (or a rate-limited 403) expire on
INFRA_MERGE_BLOCK_TTL_MSand are re-probed autonomously, which is what the module already documents.Links & Resources
src/services/merge-failure.ts:100-132,:147-149;src/services/agent-action-executor.ts:1190-1212;src/github/client.ts:395-428.