fix(actuation): classify a 401 merge failure as terminal, not transient - #2408
Conversation
withInstallationTokenRetry already evicts-and-retries once on a 401 inside the merge call itself, so a 401 reaching classifyMergeFailure means that retry also failed — a persistently unauthorized installation (App suspended or key rotated), not a one-off stale-token race. Previously this fell through to the generic terminal:false branch, burning the full MERGE_RETRY_CAP of 5 sweep-cycle retries against the same known-bad credential before finally holding. Add an explicit 401 branch returning terminal:true with a distinct "installation token rejected" reason, positioned before the 403 branch.
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-01 20:05:15 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 6 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2408 +/- ##
=======================================
Coverage 95.75% 95.75%
=======================================
Files 223 223
Lines 24812 24813 +1
Branches 9004 9005 +1
=======================================
+ Hits 23758 23759 +1
Misses 432 432
Partials 622 622
🚀 New features to boost your workflow:
|
What
classifyMergeFailuredecides whether a failed merge attempt is worth retrying (up toMERGE_RETRY_CAPsweep cycles) or should be held for a human. A 401 (Bad credentials) from the merge mutation had no explicit branch, so it fell through to the genericterminal: falsecase.Fix
withInstallationTokenRetry(src/github/app.ts) already evicts the cached installation token and retries once on a 401 inside the merge call itself. So a 401 reachingclassifyMergeFailuremeans that retry already failed — a persistently unauthorized installation (the App was suspended, or its private key was rotated), not a one-off stale-token race that a re-attempt would fix.Previously this burned the full
MERGE_RETRY_CAPof 5 sweep-cycle retries against the same known-bad credential before finally holding — wasted calls against an installation that cannot succeed until a human re-consents or restores the key.Added an explicit 401 branch returning
terminal: truewith a distinct "installation token rejected" reason, positioned before the 403 branch. Updated the top-of-file doc comment to document the new case alongside the existing 403/405/409/conflict cases.Tests
terminal: truewith a reason matching "installation token rejected" / "suspended or key rotated", distinct from the existing 403/409 assertions.npx tsc --noEmitclean.npx vitest run test/unit/merge-failure.test.ts— 6/6 pass.src/services/merge-failure.tsfully covered, zero gaps.npx vitest run test/unit/agent-action-executor.test.ts— 29/29 pass.npm run test:coverage— 309 files / 5658 tests pass.npm audit --audit-level=moderate— 0 vulnerabilities.Advances #1936. Closes #2264.