From d290035e6ec3b39d1375e23ce4db087b508df109 Mon Sep 17 00:00:00 2001 From: programmerWsy <113076850+Cunninger@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:30:34 +0800 Subject: [PATCH] fix(cli): reject empty --terminal/--from handles instead of falling back to defaults (#16745) An explicit empty --terminal (or orchestration --from) was treated as absent, so `orca terminal send --terminal ""` targeted the active pane and orchestration check/inbox/worker-start fell back to the ambient ORCA_TERMINAL_HANDLE mailbox. Hoist the existing getPresentStringFlag into flags.ts and use it for these handles so a present-but-empty value fails with invalid_argument. Fixes #16694 Co-authored-by: Neil Co-authored-by: wusiyuan05 --- src/cli/flags.ts | 14 ++++++++++++++ .../orchestration-check-identity.test.ts | 9 +++++++++ .../orchestration/message-inbox-handlers.ts | 3 ++- .../handlers/orchestration/terminal-identity.ts | 4 ++-- .../orchestration/worker-launch-handler.ts | 5 +++-- src/cli/handlers/worktree.ts | 16 +--------------- src/cli/index-terminal-commands.test.ts | 13 +++++++++++++ src/cli/selectors.ts | 4 ++-- src/cli/worktree-project-target.ts | 15 +-------------- 9 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/cli/flags.ts b/src/cli/flags.ts index 30fc0a72628..2fe2973fa26 100644 --- a/src/cli/flags.ts +++ b/src/cli/flags.ts @@ -32,6 +32,20 @@ export function getOptionalStringFlag( return typeof value === 'string' && value.length > 0 ? value : undefined } +export function getPresentStringFlag( + flags: Map, + name: string +): string | undefined { + if (!flags.has(name)) { + return undefined + } + const value = flags.get(name) + if (typeof value === 'string' && value.length > 0) { + return value + } + throw new RuntimeClientError('invalid_argument', `Missing value for --${name}`) +} + // A missing valued flag must fail: silently regenerating mutation IDs can submit prompts twice. export function rejectValuelessFlag(value: string | boolean | undefined, name: string): void { if (value === true) { diff --git a/src/cli/handlers/orchestration-check-identity.test.ts b/src/cli/handlers/orchestration-check-identity.test.ts index 4d9962a3b10..d6c6a1d6f7f 100644 --- a/src/cli/handlers/orchestration-check-identity.test.ts +++ b/src/cli/handlers/orchestration-check-identity.test.ts @@ -59,6 +59,15 @@ describe('orchestration check identity', () => { ) }) + it('rejects an empty --terminal instead of consuming the ambient mailbox', async () => { + process.env.ORCA_TERMINAL_HANDLE = 'term_coord' + + await expect( + invokeCheck(new Map([['terminal', '']])) + ).rejects.toMatchObject({ code: 'invalid_argument' }) + expect(callMock).not.toHaveBeenCalled() + }) + it('keeps an explicit legacy terminal handle scoped to that handle', async () => { process.env.ORCA_TERMINAL_HANDLE = 'term_stale_env' diff --git a/src/cli/handlers/orchestration/message-inbox-handlers.ts b/src/cli/handlers/orchestration/message-inbox-handlers.ts index 8623b5db08c..a19c3bbbf09 100644 --- a/src/cli/handlers/orchestration/message-inbox-handlers.ts +++ b/src/cli/handlers/orchestration/message-inbox-handlers.ts @@ -3,6 +3,7 @@ import { printResult } from '../../format' import { getOptionalPositiveIntegerFlag, getOptionalStringFlag, + getPresentStringFlag, getRequiredStringFlag } from '../../flags' import { @@ -36,7 +37,7 @@ export const ORCHESTRATION_INBOX_HANDLERS: Record = { count: number }>('orchestration.inbox', { limit: getOptionalPositiveIntegerFlag(flags, 'limit'), - terminal: getOptionalStringFlag(flags, 'terminal') + terminal: getPresentStringFlag(flags, 'terminal') }) printResult(result, json, (value) => { if (value.count === 0) { diff --git a/src/cli/handlers/orchestration/terminal-identity.ts b/src/cli/handlers/orchestration/terminal-identity.ts index cc929523116..9191189fc38 100644 --- a/src/cli/handlers/orchestration/terminal-identity.ts +++ b/src/cli/handlers/orchestration/terminal-identity.ts @@ -1,5 +1,5 @@ import type { RuntimeClient } from '../../runtime-client' -import { getOptionalStringFlag } from '../../flags' +import { getOptionalStringFlag, getPresentStringFlag } from '../../flags' import { RuntimeClientError } from '../../runtime-client' import { getTerminalHandle } from '../../selectors' import { hasStructuredSessionMarker } from '../../../shared/structured-session-marker' @@ -21,7 +21,7 @@ export async function resolveOrchestrationTerminalHandle( options: { validateEnvHandle?: boolean } = {} ): Promise { // CLI entry already fences caller flags using the command spec. - const explicit = getOptionalStringFlag(flags, flagName) + const explicit = getPresentStringFlag(flags, flagName) const sessionId = readInjectedAgentSessionId() if (sessionId) { return explicit ? sessionAddressForHost(explicit, sessionId) : undefined diff --git a/src/cli/handlers/orchestration/worker-launch-handler.ts b/src/cli/handlers/orchestration/worker-launch-handler.ts index b6e4d92799c..747d9801e4c 100644 --- a/src/cli/handlers/orchestration/worker-launch-handler.ts +++ b/src/cli/handlers/orchestration/worker-launch-handler.ts @@ -1,6 +1,6 @@ import type { CommandHandler } from '../../dispatch' import { printResult } from '../../format' -import { getOptionalStringFlag } from '../../flags' +import { getOptionalStringFlag, getPresentStringFlag } from '../../flags' import { RuntimeClientError } from '../../runtime-client' import type { RuntimeStatus } from '../../../shared/runtime-types' import { ORCHESTRATION_WORKER_LAUNCH_PREFERENCES_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' @@ -13,6 +13,7 @@ import { renderResolvedOrchestrationCommand } from '../../orchestration-mutation export const ORCHESTRATION_WORKER_LAUNCH_HANDLER: Record = { 'orchestration worker-start': async ({ flags, client, cwd, json }) => { + const terminal = getPresentStringFlag(flags, 'terminal') const model = getOptionalStringFlag(flags, 'model') const effort = getOptionalStringFlag(flags, 'effort') if (model || effort) { @@ -62,7 +63,7 @@ export const ORCHESTRATION_WORKER_LAUNCH_HANDLER: Record agent: getOptionalStringFlag(flags, 'agent'), model, effort, - terminal: getOptionalStringFlag(flags, 'terminal'), + terminal, retryOf: getOptionalStringFlag(flags, 'retry-of'), timeoutMs: getOptionalPositiveIntegerValueFlag(flags, 'timeout-ms'), run: getOptionalStringFlag(flags, 'run'), diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index 9c79eada3e1..a0bc349ca5a 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -18,6 +18,7 @@ import { RuntimeClientError } from '../runtime-client' import { getOptionalPositiveIntegerFlag, getOptionalStringFlag, + getPresentStringFlag, getRequiredStringFlag } from '../flags' import { @@ -54,21 +55,6 @@ import { resolveReferenceInputs } from '../reference-input' -function getPresentStringFlag( - flags: Map, - name: string, - options: { allowEmpty?: boolean } = {} -): string | undefined { - if (!flags.has(name)) { - return undefined - } - const value = flags.get(name) - if (typeof value === 'string' && (options.allowEmpty || value.length > 0)) { - return value - } - throw new RuntimeClientError('invalid_argument', `Missing value for --${name}`) -} - function getOptionalStartupAgent(flags: Map): TuiAgent | undefined { const agent = getPresentStringFlag(flags, 'agent') if (agent === undefined) { diff --git a/src/cli/index-terminal-commands.test.ts b/src/cli/index-terminal-commands.test.ts index 73464747632..e57a08515be 100644 --- a/src/cli/index-terminal-commands.test.ts +++ b/src/cli/index-terminal-commands.test.ts @@ -316,6 +316,19 @@ describe('orca cli worktree awareness', () => { expect(process.exitCode).toBe(1) }) + it('rejects an empty --terminal instead of sending to the active pane', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main(['terminal', 'send', '--terminal', '', '--text', 'x', '--json'], '/tmp/repo') + + expect(callMock).not.toHaveBeenCalled() + expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toMatchObject({ + ok: false, + error: { code: 'invalid_argument' } + }) + expect(process.exitCode).toBe(1) + }) + it('sends explicit remote terminal create worktree selectors unchanged', async () => { queueFixtures( callMock, diff --git a/src/cli/selectors.ts b/src/cli/selectors.ts index 57c17d7a656..34bc4bf3ed7 100644 --- a/src/cli/selectors.ts +++ b/src/cli/selectors.ts @@ -7,7 +7,7 @@ import { import { parseWslUncPath } from '../shared/wsl-paths' import type { RuntimeClient } from './runtime-client' import { RuntimeClientError } from './runtime/types' -import { getOptionalStringFlag, getRequiredStringFlag } from './flags' +import { getOptionalStringFlag, getPresentStringFlag, getRequiredStringFlag } from './flags' import { getCallerExecutionHostId, resolveCallerPaneWorktreeSelector, @@ -187,7 +187,7 @@ export async function getTerminalHandle( client: RuntimeClient, options: { requireUnambiguous?: boolean } = {} ): Promise { - const explicit = getOptionalStringFlag(flags, 'terminal') + const explicit = getPresentStringFlag(flags, 'terminal') if (explicit) { return explicit } diff --git a/src/cli/worktree-project-target.ts b/src/cli/worktree-project-target.ts index 198d8ac7242..3beb836ca21 100644 --- a/src/cli/worktree-project-target.ts +++ b/src/cli/worktree-project-target.ts @@ -4,6 +4,7 @@ import { hostFilterMatchesHostId, resolveHostFlagTarget } from './execution-host import { findProjectSetups, setupHostId } from './project-selector' import type { RuntimeClient } from './runtime-client' import { RuntimeClientError } from './runtime-client' +import { getPresentStringFlag } from './flags' export type ProjectCreateTarget = { repoSelector: string @@ -12,20 +13,6 @@ export type ProjectCreateTarget = { setup: ProjectHostSetup } -function getPresentStringFlag( - flags: Map, - name: string -): string | undefined { - if (!flags.has(name)) { - return undefined - } - const value = flags.get(name) - if (typeof value === 'string' && value.length > 0) { - return value - } - throw new RuntimeClientError('invalid_argument', `Missing value for --${name}`) -} - export function hasWorkspaceProjectTarget(flags: Map): boolean { return flags.has('project') || flags.has('host') || flags.has('project-host-setup') }