diff --git a/src/cli/handlers/orchestration-gate-cli.test.ts b/src/cli/handlers/orchestration-gate-cli.test.ts index a2793ac9323..217e2f359f1 100644 --- a/src/cli/handlers/orchestration-gate-cli.test.ts +++ b/src/cli/handlers/orchestration-gate-cli.test.ts @@ -34,6 +34,9 @@ import { okFixture, queueFixtures } from '../test-fixtures' 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 const restoreEnv = (name: string, value: string | undefined): void => { if (value === undefined) { @@ -54,6 +57,7 @@ describe('orchestration gate commands carry caller identity', () => { errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) delete process.env.ORCA_TERMINAL_HANDLE delete process.env.ORCA_PANE_KEY + delete process.env.ORCA_STRUCTURED_SESSION process.exitCode = 0 }) @@ -62,6 +66,7 @@ describe('orchestration gate commands carry caller identity', () => { errorSpy.mockRestore() restoreEnv('ORCA_TERMINAL_HANDLE', originalTerminalHandle) restoreEnv('ORCA_PANE_KEY', originalPaneKey) + restoreEnv('ORCA_STRUCTURED_SESSION', originalStructuredSession) process.exitCode = 0 }) @@ -192,7 +197,7 @@ describe('orchestration gate commands carry caller identity', () => { expect(process.exitCode).toBe(1) const stderr = errorSpy.mock.calls.map((call) => String(call[0])).join('\n') - expect(stderr).toContain('Pass --from ') + expect(stderr).toContain("Pass --from with your own terminal's handle") expect(callMock).not.toHaveBeenCalledWith('orchestration.gateCreate', expect.anything()) }) 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 505bfaac262..e693d99079d 100644 --- a/src/cli/handlers/orchestration/terminal-identity.ts +++ b/src/cli/handlers/orchestration/terminal-identity.ts @@ -36,11 +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()) { - throw new RuntimeClientError( - 'no_active_sender_terminal', - `This chat session has no orchestration identity of its own, so --${flagName} cannot be inferred. ` + - `Pass --${flagName} explicitly; guessing would act on another pane's mailbox.` - ) + throw structuredSessionRefusal(flagName) } if (flagName === 'from') { return await resolveImplicitOrchestrationSender(flags, cwd, client) @@ -188,10 +184,33 @@ 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. ' + - '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.' ) } 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'