Key in-flight turn state by turn, not agent address (CL-7196) - #478
Conversation
resolveMemberWorkbenches returns workbench ids plural by design: an agent can be mid-turn in two benches at once. Prove the orchestrator doesn't let one turn's inference/tool events, reply, or bracket-close observe or consume the other's in-flight state.
resolveMemberWorkbenches returns workbench ids plural: one agent address can be mid-turn in several benches at once, each on its own sidecar session. repliedAddresses, notifiedDropAddresses, and the reply-parts accumulator's partsByAddress all keyed on the address alone, so two concurrent turns for the same agent shared one bucket and could consume or drop each other's state. Key all three on agent address + sidecar sessionId instead.
8793ae1 to
8069b47
Compare
|
Posting review summary for CL-7196 turn-keying fix. |
Review notes (code review pass)Re-keys in-flight turn state ( sessionId coverage
Findings (fixed)Both commit subjects carried a trailing Checks run (foreground)
No other issues found. Not merging. |
1 participant
Summary
resolveMemberWorkbenchesdeliberately returns workbench ids plural: one agent address can be mid-turn in several benches at once, each running its own sidecar session.repliedAddresses,notifiedDropAddresses, and the reply-parts accumulator'spartsByAddressall keyed in-flight turn state on the agent address alone, so two concurrent turns for the same agent shared one bucket —replyParts.take(agentAddress)could swallow the other turn's accumulated content, and either turn's bracket-close could steal the other's drop-notice guard.turnKeyFor(agentAddress, sessionId)and re-keyed all three collections on it.sessionIdis carried on everyagent.eventframe (SidecarEventMap["agent.event"]), scoped to the one sidecar connection that turn runs on.Out of scope (flagging, not fixing here)
postedApprovalIds(process-lifetimeSet) andpendingDelegationThreads(aMapentry leaked per delegation whose specialist never replies) are both unbounded, but neither is part of the turn-keying bug and neither has a natural eviction point in this diff — bounding them needs its own design call (TTL vs. LRU vs. event-driven eviction). Left for a separate ticket.Test plan
packages/chattest suite (723 pass, 0 fail), including the new overlapping-turns regression test already committed on this branchWORKBENCH_CHECK_SINCE-scoped and fullbun run typecheck— cleanbun run lint— 0 errors (pre-existing unrelated warnings only)bun run check:structural— clean