fix(recovery): _recover rides out the wipe's 503 with bounded retry (v2.9.2)#52
Merged
Conversation
…v2.9.2) At a universe wipe, SpaceTraders 503s on /register for the first minutes while it rebuilds. `_recover` (the supervisor's on_crash) re-registered ONCE, so that 503 made it return False and the supervisor stopped — stranding the fleet on a dead token. The supervisor gives on_crash exactly one shot per crash (its `recovered` latch won't re-invoke it until the runner runs again, and the runner can't run without a fresh token), so a single failed re-register is terminal. Observed live 2026-07-05: engine stopped at the wipe and did not restart until a manual start ~24 min later; registration only recovered via the agent's own out-of-band st_register retry, which does not restart the ops engine. Fix: retry the re-register IN PLACE across the rebuild window (bounded backoff, ~9 min, first try immediate). Give up (return False → supervisor stops the storm) only on a TERMINAL failure — call sign claimed / no account token / no call sign — or after the budget. Cancellation-safe: an operator stop cancels at the sleep checkpoint (CancelledError is BaseException, not caught by the except Exception). Follow-up (filed separately): if recovery lands out-of-band (agent st_register) after the supervisor already stopped, nothing restarts the engine; and the supervisor's one-shot on_crash is a host-level limitation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
There was a problem hiding this comment.
QA Audit — PR #52 | fix(recovery): _recover rides out the wipe's 503 with bounded retry (v2.9.2)
VERDICT: WARN (CI still settling — will auto-promote to APPROVED on terminal-green)
CI Status
- test: ⏳ in_progress
Diff Review
fleet.py:_recover()gains a bounded retry loop (_RESET_REREGISTER_BACKOFF≈ 9 min) aroundrecover_from_reset(), riding out the server's post-wipe rebuild 503s. Terminal failures (claimed call sign, no account token, no call sign) abort immediately — no wasted retries._reset_recovery_terminal(): clean substring matcher for the unrecoverable statuses.test_wipe_recovery.py: 5 focused cases covering transient-recovery, terminal-no-retry, budget-exhaustion, fast-path, and non-reset pass-through. 331 suite passed, ruff clean.- Version bump 2.9.1 → 2.9.2 in
protoagent.plugin.yaml.
Observations
- LOW: clawpatch structural pass (gateway) returned 0 findings across 7 mapped features — no cross-file hazards detected.
- LOW:
except Exceptionaroundrecover_from_reset()catches all non-BaseException types (including potential code bugs). Bounded retry (9 attempts) limits blast radius;_recordlogs each attempt for debugging. Acceptable for the acknowledged one-shot-per-crash constraint. - No unresolved CodeRabbit threads. Clean diff, well-tested, addresses an observed live incident (2026-07-05 wipe).
Approve-on-green policy: once test completes terminal-green, this COMMENT review auto-promotes to APPROVED and the PR merges. No re-review needed unless the head moves.
— Quinn, QA Engineer
|
Submitted COMMENT review on |
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.
Summary
/registerfor the first minutes while it rebuilds._recover(the supervisor'son_crash) re-registered once, so that 503 made it returnFalseand the supervisor stopped — stranding the fleet on a dead token.on_crashexactly one shot per crash (itsrecoveredlatch won't re-invoke it until the runner runs again, and the runner can't run without a fresh token), so a single failed re-register is terminal.st_registerretry — which does not restart the ops engine.Fix
(0, 15, 30, 45, 60, 90, 90, 90, 90)≈ 9 min; first try immediate so a server that's already back recovers with no wait).return False→ supervisor stops the storm) only on a terminal failure — call sign claimed / no account token / no call sign — or after the budget.asyncio.sleepcheckpoint (CancelledErrorisBaseException, not caught by theexcept Exceptionaround the re-register).Test plan
test_wipe_recovery.py(5 cases): transient-503-then-recover, claimed→terminal-no-retry, persistent-503→give-up-after-budget, fast-path-first-try, non-reset-crash-rekicksFollow-up (filed separately)
If recovery lands out-of-band (agent
st_register) after the supervisor already stopped, nothing restarts the engine; and the one-shoton_crashis a host-level (protoAgent) limitation. Tracked for a separate change.🤖 Generated with Claude Code