fix(opencode): prevent same-parent assistant siblings during concurrent prompts (#28202)#28488
Open
ririnto wants to merge 2 commits into
Open
fix(opencode): prevent same-parent assistant siblings during concurrent prompts (#28202)#28488ririnto wants to merge 2 commits into
ririnto wants to merge 2 commits into
Conversation
…gressions (anomalyco#28202) Three bun:test cases covering the assistant-sibling race described in anomalyco#28202: 1. 'does not fan out assistant siblings when a new prompt arrives during a tool round-trip' — demonstrates the original bug: U2 arrives mid tool round-trip and U1's tool branch is silently orphaned or U2 produces no response under its parentID. 2. 'U1->U2->U3 chain produces exactly one terminal assistant per parent' — forward-looking coverage that overlapping but serially-resolved prompts each receive exactly one terminal assistant under the correct parentID. 3. 'U2 with tool round-trip lands under U2, not U1' — after a U1->U2 turn transition, U2's own tool continuation must stay bound to U2. Determinism uses Deferred-gated llm.hold, llm.wait, pollWithTimeout, and Effect.forkChild — no fixed sleeps. Tests at this commit demonstrate the bug; the next commit fixes runLoop and turns them green.
…Loop (anomalyco#28202) runLoop now runs as nested loops: an outer per-turn loop and an inner per-iteration loop. Each turn captures initialUser as a const at outer- block entry so all iteration sites in the turn (parentID, model resolution, agent lookup, format, summary, system reminders, compaction) reference a single pinned user message. A nextUser carrier propagates U(n)->U(n+1) transitions between turns; a done sentinel exits the outer loop so the trailing compaction.prune cleanup remains reachable. The terminal-exit branch additionally requires lastAssistantMsg.parentID === initialUser.id. Without this, the very first iteration of a newly transitioned turn observes the prior turn's terminal assistant via MessageV2.latest() and exits before ever calling the model for the new user, leaving U(n+1) without a response. Function lastAssistant (the helper that returns the most recent assistant) is renamed to finalAssistant to remove a shadow against the destructured 'assistant: lastAssistantMsg' inside the inner loop — previously an early 'return yield* lastAssistant(sessionID)' inside the inner block resolved to the destructured message object and was unreachable as a callable. Resolves the same-parentID assistant fan-out and the orphaned tool branch described in issue anomalyco#28202. Three regression tests added in the previous commit now pass.
Author
|
Simply, this PR solves the issue. When a new prompt arrives at the same time as a tool call output, two assistant messages get created under the same parent. |
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.
Issue for this PR
Closes #28202
Type of change
What does this PR do?
runLoopre-resolvedlastUserfromMessageV2.latest(msgs)on every iteration. When a second user prompt landed during U1's tool round-trip, U1's post-tool continuation rebound to U2'sparentID, orphaning U1's tool branch and producing same-parent assistant siblings.The fix restructures
runLoopinto an outer per-turn loop withinitialUserpinned as aconstand an inner per-iteration loop, with anextUsercarrier for U(n)→U(n+1) transitions and adonesentinel socompaction.prunecleanup stays reachable. The terminal-exit branch additionally requireslastAssistantMsg.parentID === initialUser.idso the first iteration of a newly transitioned turn cannot exit on the previous turn's terminal assistant before calling the model. Multi-turn within a singlerunLoopcovers the second prompt without spinning up a concurrent runLoop that would write under the same parent.Helper
lastAssistantis renamed tofinalAssistantto remove a shadow against the destructuredassistant: latestAssistantInfo, which had made the earlyreturn yield* lastAssistant(sessionID)paths inside the inner loop resolve to the message object rather than the function.CLI was not affected because its transport in
cli/cmd/run/stream.transport.tsenforces a client-side single-turn queue viastate.wait+ the session-idle event; only the Web path can fire concurrent prompts atpromptAsync.How did you verify your code works?
Three regression tests in
test/session/prompt.test.ts:Determinism via
Deferred-gatedllm.hold,llm.wait,pollWithTimeout, andEffect.forkChild(no fixed sleeps).Manual verification of the red→green chain: checking out the test commit alone (before the fix) reproduces all 3 failures; applying the fix turns them green.
bun run typecheckclean;bun test test/session/prompt.test.ts56/56 pass.Checklist