From f70ed8c8d682a68a7fd2b4699e781fef578b6ba6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 18 Sep 2026 02:48:08 -0400 Subject: [PATCH] fix(orchestration): route the lifecycle-send refusal to the structured message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `orchestration send --type worker_done|heartbeat` refuses in the send handler before `resolveOrchestrationTerminalHandle` runs, so the structured guard never saw the case a structured session hits most: the canonical worker lifecycle report. That caller was still told to pass `--from` with "your own terminal's handle" — which it does not have, so any handle it picked would belong to another pane. `throwNoActiveSenderTerminal` now derives which refusal fits instead of each call site deciding: marker set AND no handle means no identity exists, so the structured refusal applies. A stale `ORCA_TERMINAL_HANDLE` is deliberately excluded — that caller does have an identity, it just went stale, and keeps the advice to re-run under a live one. Also corrects the guidance itself (`--agent` is a `worktree create` flag; `terminal create` has no such flag), aligns the SSH fallback wording with its local twin, and pins ORCA_STRUCTURED_SESSION in the send tests, which until now decided which refusal they exercised from ambient environment. --- src/cli/handlers/orchestration.test.ts | 33 +++++++++++++++++++ .../orchestration/terminal-identity.ts | 32 +++++++++++++----- src/main/ssh/ssh-remote-orchestration-send.ts | 3 +- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/cli/handlers/orchestration.test.ts b/src/cli/handlers/orchestration.test.ts index d8cf528671d..e0a20541f91 100644 --- a/src/cli/handlers/orchestration.test.ts +++ b/src/cli/handlers/orchestration.test.ts @@ -4,6 +4,9 @@ const callMock = vi.fn() const getTerminalHandleMock = vi.hoisted(() => vi.fn()) const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE const originalPaneKey = process.env.ORCA_PANE_KEY +// Why: a structured-session marker inherited from the runner diverts these cases to the +// structured refusal, so which branch they exercise would depend on who ran them. +const originalStructuredSession = process.env.ORCA_STRUCTURED_SESSION function lifecycleGroupRecipientError(type: 'worker_done' | 'heartbeat'): string { return `${type} messages belong to one exact Dispatch and cannot target a group address.` } @@ -28,6 +31,11 @@ afterEach(() => { } else { process.env.ORCA_PANE_KEY = originalPaneKey } + if (originalStructuredSession === undefined) { + delete process.env.ORCA_STRUCTURED_SESSION + } else { + process.env.ORCA_STRUCTURED_SESSION = originalStructuredSession + } }) describe('orchestration send structured payload flags', () => { @@ -36,6 +44,7 @@ describe('orchestration send structured payload flags', () => { getTerminalHandleMock.mockReset() delete process.env.ORCA_TERMINAL_HANDLE delete process.env.ORCA_PANE_KEY + delete process.env.ORCA_STRUCTURED_SESSION }) const invokeSend = (flags: Map) => @@ -292,6 +301,29 @@ describe('orchestration send structured payload flags', () => { expect(callMock).not.toHaveBeenCalled() }) + it('refuses a structured session without naming a handle it could pass', async () => { + process.env.ORCA_STRUCTURED_SESSION = '1' + getTerminalHandleMock.mockResolvedValue('term_sibling_pane') + + // The refusal must not recommend --from: the explicit-flag branch returns before this guard, + // so the advice would succeed against a handle that necessarily belongs to another pane. + await expect( + invokeSend( + new Map([ + ['to', 'term_coord'], + ['subject', 'done'], + ['type', 'worker_done'], + ['outcome', 'succeeded'] + ]) + ) + ).rejects.toMatchObject({ + code: 'no_active_sender_terminal', + message: expect.not.stringContaining('Pass --from') + }) + expect(getTerminalHandleMock).not.toHaveBeenCalled() + expect(callMock).not.toHaveBeenCalled() + }) + it.each(['worker_done', 'heartbeat'] as const)( 'does not resolve an identity-less %s sender from the active terminal', async (type) => { @@ -327,6 +359,7 @@ describe('orchestration timeout flag validation', () => { callMock.mockReset() delete process.env.ORCA_TERMINAL_HANDLE delete process.env.ORCA_PANE_KEY + delete process.env.ORCA_STRUCTURED_SESSION }) const invokeCheck = (flags: Map) => diff --git a/src/cli/handlers/orchestration/terminal-identity.ts b/src/cli/handlers/orchestration/terminal-identity.ts index 83097e02126..e693d99079d 100644 --- a/src/cli/handlers/orchestration/terminal-identity.ts +++ b/src/cli/handlers/orchestration/terminal-identity.ts @@ -36,15 +36,7 @@ export async function resolveOrchestrationTerminalHandle( // rightful worker never saw its mail. Refusing is the only honest answer: this child genuinely // cannot infer its own identity. if (isStructuredSessionWithoutIdentity()) { - // Why not suggest --${flagName}: the explicit-flag branch above returns before this guard, so - // that advice succeeds — against a handle that necessarily belongs to another pane. - throw new RuntimeClientError( - 'no_active_sender_terminal', - `This chat session has no orchestration identity of its own, so --${flagName} cannot be inferred, ` + - `and no terminal handle names it — every live handle belongs to a different pane, whose mailbox ` + - `passing it would consume. Drive a worker directly instead: create a worktree, create a terminal ` + - `in it with an agent, then use terminal send and terminal read.` - ) + throw structuredSessionRefusal(flagName) } if (flagName === 'from') { return await resolveImplicitOrchestrationSender(flags, cwd, client) @@ -192,7 +184,29 @@ async function resolveImplicitOrchestrationSender( } } +/** + * Why no flag is suggested: every caller reaches a refusal only after the explicit-flag branch has + * already returned, so `--from` advice would succeed — against a handle that necessarily belongs to + * another pane, whose unread mail the next `check` consumes. + */ +function structuredSessionRefusal(flagName: 'from' | 'terminal'): RuntimeClientError { + return new RuntimeClientError( + 'no_active_sender_terminal', + `This chat session has no orchestration identity of its own, so --${flagName} cannot be inferred, ` + + `and no terminal handle names it — every live handle belongs to a different pane, and passing one ` + + `would consume that pane's mailbox. Drive a worker directly instead: create a worktree with ` + + `--agent to launch one in its first terminal, then use terminal send and terminal read.` + ) +} + export function throwNoActiveSenderTerminal(): never { + // Lifecycle sends refuse here before the structured guard above ever runs, so this is the only + // place left that would tell an identity-less session to pass a handle it does not have. A stale + // ORCA_TERMINAL_HANDLE is a different case — that caller HAS an identity, so it keeps the advice + // to re-run under a live one. + if (isStructuredSessionWithoutIdentity() && !process.env.ORCA_TERMINAL_HANDLE) { + throw structuredSessionRefusal('from') + } throw new RuntimeClientError( 'no_active_sender_terminal', 'Could not determine the sender terminal for this orchestration command. ' + diff --git a/src/main/ssh/ssh-remote-orchestration-send.ts b/src/main/ssh/ssh-remote-orchestration-send.ts index 391be6499df..bf03be2b867 100644 --- a/src/main/ssh/ssh-remote-orchestration-send.ts +++ b/src/main/ssh/ssh-remote-orchestration-send.ts @@ -27,7 +27,8 @@ export function resolveRemoteOrchestrationSender( throw new RemoteCliArgumentError( 'no_active_sender_terminal', 'Could not determine the sender terminal for this orchestration command. ' + - 'Pass --from or run the command inside a live Orca terminal with ORCA_TERMINAL_HANDLE set.' + "Pass --from with your own terminal's handle — another pane's handle would act on its mailbox — " + + 'or run the command inside a live Orca terminal with ORCA_TERMINAL_HANDLE set.' ) } return explicit ?? envHandle ?? 'unknown'