fix(merge): classify a 429 secondary rate limit as infra-scoped

[shin-core]

What & why

classifyMergeFailure (src/services/merge-failure.ts) reached the self-healing infra scope for a rate-limit window only via status === 403 && isConvergenceForbiddenMessage(...). But this repo's own GitHub client documents that a secondary rate limit surfaces as 403 or 429 (src/github/client.ts:422), and a sustained window exhausts the retries so the 429 reaches the classifier as a thrown RequestError.

A 429 matched no status or message branch, so it fell through to { terminal: false, scope: "commit" }. Once attempts >= MERGE_RETRY_CAP the executor escalated it to terminal, and expiresAt is set only for scope === "infra" — so the block had no expiry and isMergeBlockInEffect stayed true until the head SHA advanced. A green, approved PR was stranded until the contributor pushed a commit they had no reason to push. Because a 429 window is fleet-wide, one window stranded every in-flight merge it caught — the exact outcome the module doc says cannot happen.

The fix

Add an exported isRateLimitMessage predicate (secondary rate limit, abuse, api rate limit exceeded) alongside the existing message helpers, and two branches placed before the terminal 403:

• status === 429 → { terminal: false, scope: "infra" }
• status === 403 && isRateLimitMessage(message) → { terminal: false, scope: "infra" }

So the 403 and 429 spellings of one condition are treated identically and lapse on INFRA_MERGE_BLOCK_TTL_MS with an autonomous re-probe. No other status's terminal changes, and the commit-scoped fall-through default at line 132 is untouched (per the DoD's "do not default the fall-through to infra"). No import from src/github/client.ts (that module is Response-shaped, not error-shaped).

Tests

test/unit/merge-failure.test.ts:

• A 429 and each rate-limited-403 spelling classify { terminal: false, scope: "infra" } (429 fails on main, which returns scope: "commit").
• isRateLimitMessage matches only rate-limit text.
• Split the existing convergence-403 test so the non-rate-limit 403 keeps the "converging" arm.

test/unit/merge-block-recovery.test.ts:

• Driving the classifier→markPullRequestMergeBlocked composition (as handleMergeFailure does) through MERGE_RETRY_CAP 429 failures persists a non-null mergeBlockedUntil that lapses at NOW + INFRA_MERGE_BLOCK_TTL_MS + 1.
• A 409 merge-conflict still persists a commit-scoped block with mergeBlockedUntil == null (unchanged).

Validation

• npm run typecheck green; both suites (41 tests) green.
• Diff coverage on merge-failure.ts is 100% line and branch (429 arm + both rate-limited-403 arms); whole-file branch 100%.
• git diff --check <base> HEAD clean; diff is three files, no route/schema/migration change.

Closes #9693

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-29 09:42:50 UTC}

3 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This PR adds two new branches to classifyMergeFailure so a 429 status or a rate-limit-worded 403 is classified as scope:"infra" (non-terminal, expiring on TTL) instead of falling through to scope:"commit", which previously stranded green PRs on a self-healing GitHub rate-limit window. The fix is placed correctly before the terminal 403 branch, doesn't touch the untouched fall-through default, and the new isRateLimitMessage predicate is straightforward and well-tested including both true and false cases. Tests exercise the classifier directly and one integration-style test drives the retry-cap/markPullRequestMergeBlocked composition the same way the real executor does, which is good coverage of the actual bug path.

Nits — 5 non-blocking

• The isRateLimitMessage regex uses bare `abuse` (src/services/merge-failure.ts:84), which is broader than the existing isConvergenceForbiddenMessage's `abuse detection` — could false-positive on unrelated 403 messages containing 'abuse' that aren't rate-limit related, worth tightening to match the existing convention.
• The magic numbers 403/429 in the new branches (src/services/merge-failure.ts:132-136) are consistent with the existing style in the same function (401, 405, 409 are also bare literals), so no action needed despite the external brief's flag.
• Per the PR description, this closes issue orb(merge): classify a 429 secondary rate limit as infra-scoped #9693, but I can't independently verify the issue's exact acceptance criteria from what's provided — worth confirming the linked issue is fully satisfied.
• Consider narrowing `abuse` to `abuse detection` in isRateLimitMessage (src/services/merge-failure.ts:84) to match the specificity of the sibling isConvergenceForbiddenMessage regex and avoid overly broad matches.
• The reason strings for the two new branches are near-duplicates differing only in '403'/'429' (src/services/merge-failure.ts:133,135) — minor, but could be templated if this pattern grows further.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#9693
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	❌ 5/25	Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload	✅ 10/10	Author activity: 50 registered-repo PR(s), 35 merged, 0 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	shin-core; Gittensor profile; 50 PR(s), 0 issue(s).
Improvement	✅ Minor	risk: clean · value: minor

Linked issue satisfaction

Partially addressed
The classifier changes correctly add a 429 branch and a rate-limited-403 branch ahead of the terminal 403 case, with an exported isRateLimitMessage predicate and well-targeted unit tests, satisfying most of the classification requirements. However, the issue's explicit deliverable that a named regression test 'drives handleMergeFailure through MERGE_RETRY_CAP 429 failures' is not met — the added t

Review context

• Author: shin-core
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
• Official Gittensor activity: 50 PR(s), 0 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Await review-lane availability.
• Then work through the remaining 2 steps in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record

• action: hold · clause: success
• config: 58fe2eec8fd6bf36804b470f264b62cc86b784a1940e1a7b6b4967ae7205a8bf · pack: oss-anti-slop · ci: passed
• record: d9a44edbc3fe3bdca8c2128203e7e80d1d7693b7b5027f3118a1568375ddda6d (schema v5, head 35c0d10)

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.70%. Comparing base (7df6148) to head (35c0d10).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9795      +/-   ##
==========================================
+ Coverage   76.69%   76.70%   +0.01%     
==========================================
  Files         283      284       +1     
  Lines       59729    59761      +32     
  Branches     6557     6573      +16     
==========================================
+ Hits        45808    45840      +32     
  Misses      13639    13639              
  Partials      282      282              

Flag	Coverage Δ
backend	100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/services/merge-failure.ts	100.00% <100.00%> (ø)

[loopover-orb]

Held for manual review: the gate and required CI are green, but GitHub reports this pull request's mergeable state as unstable because a non-required check or status is not passing, so LoopOver will not auto-merge. A maintainer can resolve the failing check or review and merge manually. This is an automated maintenance action.