fix(#2053): prevent corrupted session history from breaking Anthropic API calls#2058
Draft
fix(#2053): prevent corrupted session history from breaking Anthropic API calls#2058
Conversation
…sistence During task transfers, sub-agent streaming events (AgentChoiceEvent, AgentChoiceReasoningEvent, MessageAddedEvent) were forwarded through the parent session's event channel. The PersistentRuntime's handleEvent checked sess.IsSubSession(), but sess was always the parent session, so the guard never triggered. This caused sub-agent assistant messages to be persisted directly into the parent session's message history, corrupting it. On session restore, the parent session contained interleaved sub-agent messages with tool_use blocks that had no corresponding tool_result messages in the parent context, causing Anthropic API errors: "unexpected tool_use_id found in tool_result blocks". Add SessionID field to AgentChoiceEvent and AgentChoiceReasoningEvent, and filter all streaming/message events by comparing the event's SessionID against the parent session's ID. Events from sub-sessions are now silently skipped during persistence (they are persisted separately via SubSessionCompletedEvent). Assisted-By: docker-agent
The non-beta convertMessages has a pendingAssistantToolUse flag that only includes tool_result user messages when they immediately follow an assistant message with tool_use blocks. Orphan tool results from corrupted session history are silently dropped. The beta convertBetaMessages had no such guard — every tool role message was unconditionally converted to a tool_result block. When the session history contained orphan tool results (e.g. from sub-agent messages that leaked into the parent session), they passed straight through to the Anthropic API, causing: "unexpected tool_use_id found in tool_result blocks" Add the same pendingAssistantToolUse tracking to convertBetaMessages to match the non-beta converter behavior. Assisted-By: docker-agent
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
aheritier
force-pushed
the
fix/issue-2053
branch
from
ded3735 to
9f7f944
Compare
…quencing validateSequencing only checked the forward direction: every assistant tool_use must have a matching tool_result in the next user message. It did not check the reverse: every tool_result must reference a tool_use in the immediately preceding assistant message. Orphan tool_results passed validation silently. Additionally, repairSequencing inserted a synthetic user message with missing tool_results *before* an existing user message that already had partial tool_results. This split tool_results across two user messages, causing the existing results to become orphaned: assistant(tool_use: A, B) synthetic_user(tool_result: B) <- repair inserted this user(tool_result: A) <- A now orphaned (previous is synthetic_user) Two fixes: 1. validateSequencing now also checks the reverse direction: for each user message containing tool_result blocks, verify the immediately preceding message is an assistant with corresponding tool_use IDs. 2. repairSequencing now merges missing tool_results into the existing next user message instead of inserting a separate synthetic message. This keeps all tool_results in a single user message adjacent to the assistant, satisfying both forward and reverse invariants. Assisted-By: docker-agent
Add tests covering: - Reverse validation: orphan tool_results referencing non-existent tool_use in the preceding assistant message are detected. - Reverse validation: tool_results with no preceding assistant message at all are detected. - Repair merge: when partial tool_results exist in the next user message, missing results are merged into it (not inserted as a separate synthetic message that would break sequencing). - Repair insert: when there is no next user message at all, a synthetic user message is correctly inserted. - Beta converter: orphan tool_results (no preceding assistant tool_use) are dropped by the pendingAssistantToolUse guard. - Beta converter: normal tool_use/tool_result flow continues to work correctly with the guard in place. - Runtime event tests updated for SessionID field added to AgentChoiceEvent and AgentChoiceReasoningEvent. Assisted-By: docker-agent
aheritier
force-pushed
the
fix/issue-2053
branch
from
9f7f944 to
7220241
Compare
Contributor
Author
|
/review |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
This PR effectively fixes issue #2053 by preventing corrupted session history from breaking Anthropic API calls. The implementation is sound:
Root cause fix (SessionID filtering):
- ✅ Adds
SessionIDfield toAgentChoiceEventandAgentChoiceReasoningEvent - ✅ Correctly filters sub-agent streaming events using
if e.SessionID != sess.ID { return } - ✅ Consistent pattern across all event types (AgentChoice, AgentChoiceReasoning, MessageAdded)
Defense layer (orphan tool result dropping):
- ✅ Adds
pendingAssistantToolUseboolean flag to beta converter - ✅ Correctly drops orphan tool results when flag is false
- ✅ Flag is properly reset after processing tool results
Validation + repair hardening:
- ✅ Reverse validation checks user tool_results reference existing assistant tool_use blocks
- ✅ Merge-based repair correctly checks next message role before merging
- ✅ Handles consecutive assistants by inserting synthetic user message (not merging)
Test coverage:
- ✅ 6 new tests cover reverse validation, merge behavior, orphan dropping, and normal flow
- ✅ Runtime event tests updated for SessionID field
The logic is correct and well-structured. All edge cases identified during review are properly handled.
Findings
No issues found. The changes are ready to merge.
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
1 participant
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
Fixes #2053 — After restoring a session that involved
transfer_task(sub-agent delegation), all subsequent prompts fail with:Root Cause
PersistentRuntime.handleEvent()persists all streaming events to the parent session, including events from sub-agent executions during task transfers. The existingsess.IsSubSession()guard never triggers becausesessis always the parent session. This corrupts the parent session's message history with interleaved sub-agent messages containing orphantool_use/tool_resultblocks.Changes
1.
fix(#2053): filter sub-agent streaming events from parent session persistenceRoot cause fix. Adds
SessionIDfield toAgentChoiceEventandAgentChoiceReasoningEvent. All streaming and message events inhandleEventare now filtered by comparing the event'sSessionIDagainst the parent session's ID. Sub-agent events are silently skipped (persisted separately viaSubSessionCompletedEvent).Files:
pkg/runtime/event.go,pkg/runtime/streaming.go,pkg/runtime/persistent_runtime.go2.
fix(#2053): add pendingAssistantToolUse guard to beta message converterDefense layer. The non-beta
convertMessagesalready had apendingAssistantToolUseflag that drops orphan tool results. The betaconvertBetaMessageslacked this guard — everytoolrole message was unconditionally sent to the API. Now both converters behave consistently.File:
pkg/model/provider/anthropic/beta_converter.go3.
fix(#2053): add reverse validation and merge-based repair for tool sequencingValidation + repair hardening.
validateSequencingnow checks both directions: forward (assistanttool_use→ next user hastool_results) and reverse (usertool_results→ previous assistant hastool_use).repairSequencingnow merges missingtool_resultsinto the existing next user message instead of inserting a separate synthetic message (which would orphan the existing results).File:
pkg/model/provider/anthropic/client.go4.
test(#2053): add coverage for tool_use/tool_result sequencing fixes6 new tests covering reverse validation, repair merge behavior, beta converter orphan dropping, and the normal tool_use/tool_result flow. Runtime event tests updated for the new
SessionIDfield.Files:
pkg/model/provider/anthropic/client_test.go,pkg/runtime/runtime_test.goTesting
task test— all tests passgo vet— clean