fix(orchestration): revalidate legacy takeover before the recipient verdict

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
This commit is contained in:
Jinwoo-H
2026-09-09 23:34:09 -04:00
parent 725ccb8c43
commit cf326922b2
2 changed files with 19 additions and 3 deletions
@@ -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) => ({
@@ -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'
})
])