fix(executor): write the crash ledger before cancelling, so a dead roll is loud (#519) - #522
Merged
Merged
Conversation
…ll is loud (#519) _roll_stop cancelled the resting bracket and then placed the replacement without ever writing an unbracketed: record -- place_bracket writes one only when a PLACEMENT is refused. A process dying between the cancel and the place therefore left no resting bracket AND no intent, so reconcile_unbracketed_positions took the branch that exists for DCA and skipped the position SILENTLY. Naked, with no CRITICAL, and indistinguishable from a tranche that carries no stop by design. That is the worst shape this failure could take: not merely unprotected, but unprotected and invisible. The record is now written BEFORE the venue is touched and cleared only once the replacement rests, so every death in that window lands in a state the existing sweep already converges -- next cycle it finds no bracket, finds these levels, and re-places from them. Deliberately the SAME unbracketed: key place_bracket uses, not a new roll_intent: one. The sweep, its ledger-sized qty, its escalation and its clear-on-success semantics are written and tested once; a second key would mean a second healer to keep correct. A record left by an aborted roll is harmless -- the sweep skips any product whose bracket still rests. The CRITICAL on the failed-placement path STAYS, and the record is retained alongside it. Automatic recovery next cycle is not a reason to downgrade an alert about a position unprotected right now: this deployment cycles once per UTC day, so 'next cycle' can be a day away. Found by the #502 design pass rather than by an incident -- _roll_stop has no automated caller today, so the window is entered only by a manual roll. #502's proposed per-cycle stop management would enter it on every ratchet, which is what turned a latent hole into a blocker. Gates: pytest 4553 passed / 3 skipped; ruff check keel tests packages clean; mypy clean across 353 source files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6
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 #519.
The hole
_roll_stopcancelled the resting bracket and then placed the replacement without ever writing anunbracketed:record —place_bracketwrites one only when a placement is refused.So a process dying between the cancel and the place left no resting bracket and no intent. On the next cycle
reconcile_unbracketed_positionshitif not intent: continue— the branch that exists for DCA, whose docstring explains that escalating it "would fire a CRITICAL for every DCA tranche on every cycle, which trains the alert to be ignored."The position stayed naked with no CRITICAL, indistinguishable from a tranche that carries no stop by design. That's the worst shape this failure could take: not merely unprotected, but unprotected and invisible.
The fix
Write the record before the venue is touched; clear it only once the replacement rests. Every death in that window now lands in a state the existing sweep already converges — next cycle it finds no bracket, finds these levels, re-places from them.
Deliberately the same
unbracketed:keyplace_bracketuses, not a newroll_intent:one. The sweep, its ledger-sized qty, its escalation and its clear-on-success semantics are written and tested once; a second key would mean a second healer to keep correct. A record left by an aborted roll is harmless — the sweep skips any product whose bracket still rests.The CRITICAL stays, and the record is retained alongside it. Automatic recovery next cycle is not a reason to downgrade an alert about a position unprotected right now: this deployment cycles once per UTC day, so "next cycle" can be a day away.
Tests
Three, pinning the contract at the crash point rather than around it:
test_a_roll_writes_its_crash_ledger_before_touching_the_venue— spies on_cancel_at_exchange(the moment the old bracket stops resting) and asserts the record already exists with the right levels.test_a_successful_roll_clears_its_crash_ledger— left standing, the sweep would re-place a bracket that already rests.test_a_failed_roll_RETAINS_its_crash_ledger_for_the_sweep— the naked case: record survives and the CRITICAL still fires.Verification
pytest -q— 4,553 passed, 3 skippedruff check keel tests packages— cleanmypy— clean, 353 source filesHow it was found
The #502 design pass, not an incident.
_roll_stophas no automated caller today, so the window is entered only by a manual roll — which is why this has never bitten. #502's proposed per-cycle stop management would enter it on every ratchet, which is what turned a latent hole into a blocker.Worth fixing on its own merits regardless of whether that step ever ships.