Fix overlapping turns silently dropping a reply (CL-6670) - #325
Merged
Conversation
Proves the fix for overlapping turns silently landing on the wrong row: findRunningTurn's "newest running turn" pick is only safe when at most one turn is ever running per (workbench, agent), and nothing previously enforced that.
…ext (CL-6670) Fixes overlapping turns silently dropping a reply: the sidecar's agent.event stream carries only the agent's address, so a second occurrence opened for an agent while its first was still running left findRunningTurn guessing which real reply belonged to which row — one reply would land stamped onto the wrong turn, or trigger a spurious "didn't manage to answer" notice for a turn that actually answered. Different agents named in the same room are unaffected: the wait is scoped per (workbench, agent), so two agents mentioned back to back still turn concurrently, as they always have. Updates the two existing tests that relied on the old, ambiguous behavior (multiple simultaneously-running turns for one agent) to finish each turn before its next message, matching the corrected contract.
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
CL-6670: in a multi-agent room,
@agent-Aa question, then — while A isstill generating —
@agent-Ba different question; A's reply neverlanded, silently.
Verified mechanism:
AgentTurnStore.findRunningTurn(packages/chat/src/agent-turns.ts)picks the newest
runningturn for a (workbench, agent) pair as theone a reply belongs to, because the sidecar's
agent.eventstreamcarries only the agent's address, never a turn id. That pick is only
safe if at most one turn is ever
runningfor that pair at a time —but nothing enforced that:
dispatchTurnopens a fresh occurrence foran agent as soon as its own workbench-level dispatch claim
(
turn-queue.ts, CL-6331) frees up, which only spans the fast mailhandoff, not the agent's actual reply. Two messages reaching the same
agent a few seconds apart (a slow reply, or a second message arriving
mid-generation) could each open their own
runningrow, and whicheverof the agent's two real replies arrived first got misattributed to the
other row — closing it early and leaving the true owner
runninguntil it either got the next reply misattributed to it too, or aged
out as a stale, "never finished" turn with no trace of what actually
happened.
Cross-agent state in
chat-orchestrator.ts(repliedAddresses,replyParts, the turn-drop notice guard) is correctly scoped byagentAddress, so two different agents were never at risk ofclobbering each other's bookkeeping — the ambiguity was specifically
whenever the same agent had more than one turn open at once.
Fix
AgentTurnStoregainswaitUntilFree(tenantId, workbenchId, agentAddress): resolves immediately if the agent has no running turn,otherwise once its current turn closes via
finishTurn(or, as abackstop, once it would age out as stale anyway).
dispatchTurnBatchnow awaits it — scoped per (workbench, agent) — before opening a new
occurrence for a recipient, placed outside the per-hop dispatch
deadline so a legitimately slow reply is never mistaken for an
unreachable agent.
This makes
findRunningTurn's "newest running turn" pick correct byconstruction (never more than one candidate) rather than a coincidence
that held only under light load. Two different agents mentioned in
the same room still turn fully concurrently — the wait is keyed
per-agent, never per-workbench — matching CL-6670's requirement that
overlapping turns for different agents must both complete and both
land, correctly attributed.
Test plan
packages/chat/src/agent-turns.test.ts—waitUntilFreeresolves immediately when free, blocks until
finishTurn(or afailed turn) frees it, and never blocks a different agent's own
wait.
packages/chat/test/agent-turn-dispatch.test.ts— a secondagent's turn starts immediately while the first agent's turn is
still open (CL-6670's literal repro shape); a second message to
the same agent now waits for the first turn to close instead of
opening a second running row.
agent-turn-dispatch.test.tsandcommands.test.tstests that relied on the old, ambiguous "multiple simultaneously
running turns for one agent" behavior to finish each turn first.
WORKBENCH_CHECK_SINCE=origin/main bun run typecheck— passbun run lint— pass (pre-existing warnings only, unrelated)WORKBENCH_CHECK_SINCE=origin/main bun run test— pass (662/662in
@corbits/chat, full monorepo gate green)Live browser-driven proof on a scratch stack (per the ticket) was not
completed in this pass — flagging for a follow-up rather than skipping
silently.