Repository navigation
fix(reactor): persist resolved approval removal with its result turn - #1497
Closed
TheGreatAxios wants to merge 5 commits into
Closed
TheGreatAxios wants to merge 5 commits into
TheGreatAxios wants to merge 5 commits into
Conversation
Tool cycles write turns only and defer metadata to the next terminal commit. An approved or rejected call drops its pending operation from memory, but a teardown before that terminal left it in metadata.json; a reactor rebuilt from the store re-armed the answered call's gate and, at the original deadline, appended a second tool_result for the same call id, which the prompt tripwire rejects. The turns-only write now also writes metadata when a resolved approval changed the pending operations, so the removal lands with the turn that answers the call. Writing metadata in tryCorrelate instead would drop the op before the re-dispatched result is on disk, leaving a dangling tool_use with no gate to answer it. Cycles that leave pending operations untouched still skip the metadata write.
The pending-operation removal reaches metadata in a second await after the result turn, so a crash between the two leaves a stale op for a call history already answers. A reactor rebuilt from that store re-armed its gate and, at the original deadline, appended a second tool_result for the same call id, which the prompt tripwire rejects. Startup now drops and persists the removal of any loaded op whose parked call already has a result, and the approval timeout injects no synthetic result for such a call.
Crash recovery could reload a pending operation for a call the turns already answer, re-arming its gate and injecting a second tool result at the deadline. load() now drops any pending operation whose suspended call id already has a tool result, warning per drop; marker operations without a suspended call pass through. The removal converges to disk on the next metadata write.
A run opens only when the loop dequeues message.received, and a correlated approval never becomes one. A parked call rehydrated by a new reactor has no run from the process that parked it, so its approval re-dispatched the call and re-inferred with no run open, and consumers that treat no open run as idle could rebuild the agent under the running continuation. The approval now opens a run keyed on its own message id when none is open. A resume inside the still-open parking run is unchanged.
TheGreatAxios
commented
Oct 17, 2026
TheGreatAxios
left a comment
TheGreatAxios
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review · Comment
Persists a resolved approval's op-removal with its result turn, filters stale ops at load, and opens a run on approval resume — landable as a whole.
Findings
No blocking findings. The mid-write second-approval race from the earlier review re-verified as a real mechanism hole (writeMetadata snapshots the op set, then clears the flag unconditionally), but its on-disk consequence (result present, op still listed) is exactly what the stacked load-time filter drops, so no rebuild path re-arms the stale gate. Two follow-ups owed, fittable right after merge: compare-and-clear hardening of the flag clear, and a two-parked-approval interleaving test with a deferred metadata write.
Contributor
|
Triage: Fail @TheGreatAxios, please address the following:
|
Collaborator
Author
|
Superseded by #1495, which implements the turn-log approval design on main covering this stack's fixes (removal persistence, stale-op filtering, resume coverage, reload e2e). No cherry-picks outstanding. |
TheGreatAxios
deleted the
cl-10764-persist-pending-operation-removal-when-an-approved-call-is
branch
October 17, 2026 17:29
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
metadata.jsontogether with the turn that answers it, so a reactor rebuilt from the session store never re-arms the answered call's gate.ask, rebuilds the agent from the same store, and advances past the original gate deadline.In the interactive TUI without yolo, a reload right after an approval rebuilt the agent while
metadata.jsonstill listed the pending operation. One hour later the stale gate injectedapproval timed outfor a call that already had a result, and the session died withMalformed tool sequence: duplicate tool_result.Verification
bun run checkpasses (9159 tests, 0 fail)Fixes CL-10764
Fixes CL-10767
Fixes CL-10765
Fixes CL-10777
Part of CL-10763