From d5e93573107106c51fe401080bb5afeff1d7ee43 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 5 Jul 2026 22:40:51 -0700 Subject: [PATCH] Validate ORCA_TERMINAL_HANDLE and fall back to active terminal if stale Long-lived shells can retain a stale ORCA_TERMINAL_HANDLE environment variable after the runtime remints a pane handle. This can cause commands to bake obsolete terminal handles into coordinator preambles or tasks. - Check if the environment-provided handle is live via terminal.show before using it in dispatch, task-create, or run operations. - Fall back to resolving the active terminal/implicit sender if the environment handle is stale. - Map raw "no_active_terminal" errors to a helpful user-facing error message suggesting the use of the "--from" flag. --- src/cli/handlers/orchestration.test.ts | 232 +++++++++++++++++++++++++ src/cli/handlers/orchestration.ts | 113 ++++++++++-- 2 files changed, 335 insertions(+), 10 deletions(-) diff --git a/src/cli/handlers/orchestration.test.ts b/src/cli/handlers/orchestration.test.ts index 15a4791c2df..c3df37b8f10 100644 --- a/src/cli/handlers/orchestration.test.ts +++ b/src/cli/handlers/orchestration.test.ts @@ -12,6 +12,7 @@ vi.mock('../format', () => ({ printResult: vi.fn() })) vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock })) import { ORCHESTRATION_HANDLERS } from './orchestration' +import { RuntimeClientError } from '../runtime-client' afterEach(() => { getTerminalHandleMock.mockReset() @@ -186,6 +187,237 @@ describe('orchestration send structured payload flags', () => { devMode: false }) }) + + it('continues to use ORCA_TERMINAL_HANDLE as worker lifecycle sender authority', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_worker_env' + + await invokeSend( + new Map([ + ['to', 'term_coord'], + ['subject', 'done'], + ['type', 'worker_done'] + ]) + ) + + expect(callMock).toHaveBeenCalledTimes(1) + expect(callMock).toHaveBeenCalledWith('orchestration.send', { + from: 'term_worker_env', + to: 'term_coord', + subject: 'done', + body: undefined, + type: 'worker_done', + priority: undefined, + threadId: undefined, + payload: undefined, + devMode: false + }) + }) + + it('reports sender resolution failure instead of raw no_active_terminal', async () => { + getTerminalHandleMock.mockRejectedValue( + new RuntimeClientError('no_active_terminal', 'no_active_terminal') + ) + + await expect( + invokeSend( + new Map([ + ['to', 'term_coord'], + ['subject', 'done'], + ['type', 'worker_done'] + ]) + ) + ).rejects.toMatchObject({ + code: 'no_active_sender_terminal', + message: expect.stringContaining('Pass --from') + }) + expect(callMock).not.toHaveBeenCalled() + }) +}) + +describe('orchestration dispatch coordinator handle', () => { + beforeEach(() => { + callMock.mockReset() + getTerminalHandleMock.mockReset() + delete process.env.ORCA_TERMINAL_HANDLE + }) + + const invokeDispatch = (flags: Map) => + ORCHESTRATION_HANDLERS['orchestration dispatch']({ + flags, + client: { call: callMock }, + cwd: '/tmp/repo', + json: true + } as never) + + const invokeDispatchShow = (flags: Map) => + ORCHESTRATION_HANDLERS['orchestration dispatch-show']({ + flags, + client: { call: callMock }, + cwd: '/tmp/repo', + json: true + } as never) + + it('falls back to active sender resolution when env handle is stale', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_stale_coord' + callMock + .mockRejectedValueOnce( + new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale') + ) + .mockResolvedValueOnce({ + result: { dispatch: { id: 'ctx_1', task_id: 'task_1', status: 'dispatched' } } + }) + getTerminalHandleMock.mockResolvedValue('term_live_coord') + + await invokeDispatch( + new Map([ + ['task', 'task_1'], + ['to', 'term_worker'], + ['inject', true] + ]) + ) + + expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { + terminal: 'term_stale_coord' + }) + expect(getTerminalHandleMock).toHaveBeenCalled() + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.dispatch', { + task: 'task_1', + to: 'term_worker', + from: 'term_live_coord', + inject: true, + dryRun: undefined, + returnPreamble: undefined, + devMode: false + }) + }) + + it('uses a live coordinator handle for dispatch-show preamble previews', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_stale_coord' + callMock + .mockRejectedValueOnce( + new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale') + ) + .mockResolvedValueOnce({ + result: { dispatch: null, preamble: 'preamble' } + }) + getTerminalHandleMock.mockResolvedValue('term_live_coord') + + await invokeDispatchShow( + new Map([ + ['task', 'task_1'], + ['preamble', true] + ]) + ) + + expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { + terminal: 'term_stale_coord' + }) + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.dispatchShow', { + task: 'task_1', + preamble: true, + from: 'term_live_coord', + devMode: false + }) + }) +}) + +describe('orchestration task-create caller handle', () => { + beforeEach(() => { + callMock.mockReset() + getTerminalHandleMock.mockReset() + delete process.env.ORCA_TERMINAL_HANDLE + }) + + const invokeTaskCreate = (flags: Map) => + ORCHESTRATION_HANDLERS['orchestration task-create']({ + flags, + client: { call: callMock }, + cwd: '/tmp/repo', + json: true + } as never) + + it('records a live env terminal handle as task creator', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_creator' + callMock + .mockResolvedValueOnce({ result: { terminal: { handle: 'term_creator' } } }) + .mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } }) + + await invokeTaskCreate(new Map([['spec', 'do work']])) + + expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_creator' }) + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', { + spec: 'do work', + taskTitle: undefined, + displayName: undefined, + deps: undefined, + parent: undefined, + callerTerminalHandle: 'term_creator' + }) + }) + + it('does not persist a stale env terminal handle as task creator', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_stale' + callMock + .mockRejectedValueOnce( + new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale') + ) + .mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } }) + getTerminalHandleMock.mockRejectedValue( + new RuntimeClientError('no_active_terminal', 'no_active_terminal') + ) + + await invokeTaskCreate(new Map([['spec', 'do work']])) + + expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_stale' }) + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', { + spec: 'do work', + taskTitle: undefined, + displayName: undefined, + deps: undefined, + parent: undefined, + callerTerminalHandle: undefined + }) + }) + + it('propagates unexpected active fallback failures after a stale env handle', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_stale' + callMock.mockRejectedValueOnce( + new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale') + ) + getTerminalHandleMock.mockRejectedValue( + new RuntimeClientError('runtime_unavailable', 'runtime_unavailable') + ) + + await expect( + invokeTaskCreate(new Map([['spec', 'do work']])) + ).rejects.toMatchObject({ + code: 'runtime_unavailable' + }) + + expect(callMock).toHaveBeenCalledTimes(1) + }) + + it('falls back to the active terminal when a stale env handle has a live replacement', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_stale' + callMock + .mockRejectedValueOnce( + new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale') + ) + .mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } }) + getTerminalHandleMock.mockResolvedValue('term_live') + + await invokeTaskCreate(new Map([['spec', 'do work']])) + + expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_stale' }) + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', { + spec: 'do work', + taskTitle: undefined, + displayName: undefined, + deps: undefined, + parent: undefined, + callerTerminalHandle: 'term_live' + }) + }) }) describe('orchestration timeout flag validation', () => { diff --git a/src/cli/handlers/orchestration.ts b/src/cli/handlers/orchestration.ts index 2476cdf836d..e0b162df316 100644 --- a/src/cli/handlers/orchestration.ts +++ b/src/cli/handlers/orchestration.ts @@ -127,7 +127,8 @@ async function resolveOrchestrationTerminalHandle( flags: Map, cwd: string, client: Parameters[0]['client'], - flagName: 'from' | 'terminal' + flagName: 'from' | 'terminal', + options: { validateEnvHandle?: boolean } = {} ): Promise { const explicit = getOptionalStringFlag(flags, flagName) if (explicit) { @@ -135,11 +136,107 @@ async function resolveOrchestrationTerminalHandle( } const envHandle = process.env.ORCA_TERMINAL_HANDLE if (envHandle && envHandle.length > 0) { + if (flagName === 'from' && options.validateEnvHandle) { + // Why: long-lived shells can retain an ORCA_TERMINAL_HANDLE after the + // runtime remints the pane handle; do not bake that stale id into + // coordinator preambles. + const live = await isLiveTerminalHandle(envHandle, client) + if (!live) { + return await resolveImplicitOrchestrationSender(flags, cwd, client) + } + } return envHandle } + if (flagName === 'from') { + return await resolveImplicitOrchestrationSender(flags, cwd, client) + } return await getTerminalHandle(flags, cwd, client) } +async function resolveTaskCreatorTerminalHandle( + flags: Map, + cwd: string, + client: Parameters[0]['client'] +): Promise { + const envHandle = process.env.ORCA_TERMINAL_HANDLE + if (!envHandle || envHandle.length === 0) { + return undefined + } + const live = await isLiveTerminalHandle(envHandle, client) + if (live) { + return envHandle + } + try { + return await resolveImplicitOrchestrationSender(flags, cwd, client) + } catch (err) { + if (getClientErrorCode(err) !== 'no_active_sender_terminal') { + throw err + } + return undefined + } +} + +async function isLiveTerminalHandle( + handle: string, + client: Parameters[0]['client'] +): Promise { + try { + await client.call('terminal.show', { terminal: handle }) + return true + } catch (err) { + if (isStaleTerminalIdentityError(err)) { + return false + } + throw err + } +} + +function getClientErrorCode(err: unknown): string | undefined { + if (!err || typeof err !== 'object') { + return undefined + } + const code = (err as { code?: unknown }).code + return typeof code === 'string' ? code : undefined +} + +function isStaleTerminalIdentityError(err: unknown): boolean { + const code = getClientErrorCode(err) + return code === 'terminal_handle_stale' || code === 'terminal_gone' +} + +function isNoActiveTerminalError(err: unknown): boolean { + return getClientErrorCode(err) === 'no_active_terminal' +} + +async function resolveCoordinatorTerminalHandle( + flags: Map, + cwd: string, + client: Parameters[0]['client'] +): Promise { + return await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from', { + validateEnvHandle: true + }) +} + +async function resolveImplicitOrchestrationSender( + flags: Map, + cwd: string, + client: Parameters[0]['client'] +): Promise { + try { + return await getTerminalHandle(flags, cwd, client) + } catch (err) { + if (!isNoActiveTerminalError(err)) { + throw err + } + 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.' + ) + } +} + function isDevCliInvocation(): boolean { return process.env.ORCA_USER_DATA_PATH?.includes('orca-dev') ?? false } @@ -288,12 +385,8 @@ export const ORCHESTRATION_HANDLERS: Record = { }) }, - 'orchestration task-create': async ({ flags, client, json }) => { - const callerTerminalHandle = - typeof process.env.ORCA_TERMINAL_HANDLE === 'string' && - process.env.ORCA_TERMINAL_HANDLE.length > 0 - ? process.env.ORCA_TERMINAL_HANDLE - : undefined + 'orchestration task-create': async ({ flags, client, cwd, json }) => { + const callerTerminalHandle = await resolveTaskCreatorTerminalHandle(flags, cwd, client) const result = await client.call<{ task: { id: string; status: string } }>( 'orchestration.taskCreate', { @@ -361,7 +454,7 @@ export const ORCHESTRATION_HANDLERS: Record = { }, 'orchestration dispatch': async ({ flags, client, cwd, json }) => { - const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from') + const from = await resolveCoordinatorTerminalHandle(flags, cwd, client) const dryRun = flags.has('dry-run') ? true : undefined const returnPreamble = flags.has('return-preamble') ? true : undefined // Why: --to is only required for non-dry-run; the RPC handler re-enforces. @@ -437,7 +530,7 @@ export const ORCHESTRATION_HANDLERS: Record = { // Why: resolve --from when previewing so the preamble embeds a real // coordinator handle, matching what an actual dispatch would produce. const from = showPreamble - ? await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from') + ? await resolveCoordinatorTerminalHandle(flags, cwd, client) : undefined const result = await client.call<{ dispatch: { id: string; task_id: string; status: string } | null @@ -460,7 +553,7 @@ export const ORCHESTRATION_HANDLERS: Record = { }, 'orchestration run': async ({ flags, client, cwd, json }) => { - const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from') + const from = await resolveCoordinatorTerminalHandle(flags, cwd, client) const result = await client.call<{ runId: string status: string