From cf326922b2f2b6307aca80c5de5db6ec1830fcfd Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 9 Sep 2026 23:34:09 -0400 Subject: [PATCH] fix(orchestration): revalidate legacy takeover before the recipient verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A legacy coordinator taken over while `listTerminals` was in flight reported `runtime_error` instead of `legacy_read_only`: Run scoping made "no live Dispatch in this Run" the first thing the group send could fail on, and that threw before the takeover check ran. Takeover is a precondition, not a commit-time detail — the sender must be told it is read-only whatever else is wrong with its recipient set. Revalidation moves to immediately after the only `await` in the path. Everything below it is synchronous, so the commit-time window it used to guard is unchanged; only the error paths now see it. The legacy partition test gave `term_current_worker` no Dispatch, so under Run scoping it is correctly not a recipient. It now holds a real current-contract Dispatch in the same adopted Run, which is what the test is named for: one `legacy_direct` and one `current_delivery` recipient in one fan-out. Claude-Session: run-scoped-group-addresses --- .../orchestration/messaging/send-group.ts | 5 ++++- ...rchestration-legacy-coordinator-race.test.ts | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/send-group.ts b/src/main/runtime/rpc/methods/orchestration/messaging/send-group.ts index e6718b63541..edeed33a515 100644 --- a/src/main/runtime/rpc/methods/orchestration/messaging/send-group.ts +++ b/src/main/runtime/rpc/methods/orchestration/messaging/send-group.ts @@ -106,6 +106,10 @@ export async function sendGroupMessage(args: { const { terminals } = await runtime.listTerminals(undefined, undefined, { includeVisualLayouts: false }) + // Immediately after the only await: a coordinator taken over during terminal discovery is + // read-only, and must be told that whatever else is wrong with its recipient set. Everything + // below is synchronous, so no takeover can interleave between here and the insert. + revalidateLegacyCoordinator?.() // Structured workers are on no PTY surface, so `listTerminals` cannot see them and a broadcast // silently missed every one. Composed here rather than inside `listTerminals`, whose result is // published to paired clients and to consumers that assume a summary is writable. @@ -165,7 +169,6 @@ export async function sendGroupMessage(args: { ) } - revalidateLegacyCoordinator?.() const threadId = params.threadId ?? `thread_${Date.now()}` const messages = db.insertMessages( uniqueRecipients.map((resolution) => ({ diff --git a/src/main/runtime/rpc/orchestration-legacy-coordinator-race.test.ts b/src/main/runtime/rpc/orchestration-legacy-coordinator-race.test.ts index 71daf09b0e3..8ef1825f558 100644 --- a/src/main/runtime/rpc/orchestration-legacy-coordinator-race.test.ts +++ b/src/main/runtime/rpc/orchestration-legacy-coordinator-race.test.ts @@ -315,13 +315,26 @@ describe('legacy coordinator takeover races', () => { it('partitions a coordinator group send by legacy recipient contract', async () => { const harness = createHarness() + // A second worker on the CURRENT contract in the same adopted Run. Group addresses reach a + // Run's Dispatches, so the partition needs two Dispatches, not a Dispatch and a loose pane. + const currentTask = harness.db.createTask({ + runId: harness.adoptedRunId, + spec: 'current-contract assignment', + createdByTerminalHandle: COORDINATOR_HANDLE + }) + const currentDispatch = createRootDispatch( + harness.db, + currentTask.id, + 'term_current_worker', + 'tab_current_worker:22222222-2222-4222-8222-222222222222' + ) vi.mocked(harness.runtime.getTerminalPaneKey).mockImplementation((handle) => handle === COORDINATOR_HANDLE ? COORDINATOR_PANE : handle === WORKER_HANDLE ? WORKER_PANE : handle === 'term_current_worker' - ? 'tab_current_worker:leaf_current_worker' + ? 'tab_current_worker:22222222-2222-4222-8222-222222222222' : null ) vi.spyOn(harness.runtime, 'listTerminals').mockResolvedValue({ @@ -355,7 +368,7 @@ describe('legacy coordinator takeover races', () => { }), expect.objectContaining({ run_id: harness.adoptedRunId, - to_handle: 'term_current_worker', + to_handle: `dispatch:${currentDispatch.id}`, delivery_contract: 'current_delivery' }) ])