fix(reviewer): make the escalation-budget guard restart-safe - #338
Merged
Conversation
#335 stopped the fleet's own fix-push from resetting the fix_attempts budget by matching the recorded last_fix_push_sha. But that SHA is recorded only AFTER the fix pass returns — so if the watcher is interrupted between the push and the record (a long fix pass killing the process, as seen on #337), the SHA is lost and the next poll mistakes our own push for an external one, resetting the budget and re-opening the #334 infinite-loop risk. Add a restart-safe fallback using state that survives the pre-fix save: when there is an active fix cycle (fix_attempts > 0) but the pass outcome was never recorded (last_fix_pass_pushed absent — popped at dispatch start, re-set only on completion), a head move is our interrupted fix's push, not an external one — so do not reset. A poll never observes this mid-dispatch (the dispatch is synchronous within one poll), so the fallback only triggers after a restart. External pushes after a COMPLETED pass still reset for a fresh review. Tests: restart-mid-fix preserves the budget (→2); the self/external-push tests now set last_fix_pass_pushed=True to model a completed prior pass. Reviewer suite 124 pass; audit clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Closes the residual edge in #335 that #337 exposed live.
#335 stops the fleet's own fix-push from resetting the
fix_attemptsbudget bymatching the recorded
last_fix_push_sha. But that SHA is recorded only afterthe fix pass returns — so if the watcher is interrupted between the push and the
record (a long fix pass killing the process, as happened on #337), the SHA is lost
and the next poll mistakes our own push for an external one, resetting the budget
and re-opening the #334 infinite-loop risk.
Fix: a restart-safe fallback driven by state that survives the pre-fix save —
when there is an active fix cycle (
fix_attempts > 0) but the pass outcome wasnever recorded (
last_fix_pass_pushedabsent; it's popped at dispatch start andre-set only on completion), a head move is our interrupted fix's push, not
external → don't reset. A poll never observes this mid-dispatch (the dispatch is
synchronous within one poll), so the fallback only fires post-restart. External
pushes after a completed pass still reset for a fresh review.
Tests:
test_phase1_restart_mid_fix_does_not_reset_budget(→ budget preserved);the self/external-push tests now set
last_fix_pass_pushed=Trueto model acompleted prior pass. Reviewer suite 124 pass; audit clean.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code