mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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 <neil@stably.ai> Co-authored-by: wusiyuan05 <wusiyuan05@zuoyebang.com>
This commit is contained in:
co-authored by
Neil
wusiyuan05
parent
4fda57bb11
commit
d290035e6e
@@ -32,6 +32,20 @@ export function getOptionalStringFlag(
|
||||
return typeof value === 'string' && value.length > 0 ? value : undefined
|
||||
}
|
||||
|
||||
export function getPresentStringFlag(
|
||||
flags: Map<string, string | boolean>,
|
||||
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) {
|
||||
|
||||
@@ -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<string, string | boolean>([['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'
|
||||
|
||||
|
||||
@@ -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<string, CommandHandler> = {
|
||||
count: number
|
||||
}>('orchestration.inbox', {
|
||||
limit: getOptionalPositiveIntegerFlag(flags, 'limit'),
|
||||
terminal: getOptionalStringFlag(flags, 'terminal')
|
||||
terminal: getPresentStringFlag(flags, 'terminal')
|
||||
})
|
||||
printResult(result, json, (value) => {
|
||||
if (value.count === 0) {
|
||||
|
||||
@@ -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<string | undefined> {
|
||||
// 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
|
||||
|
||||
@@ -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<string, CommandHandler> = {
|
||||
'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<string, CommandHandler>
|
||||
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'),
|
||||
|
||||
@@ -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<string, string | boolean>,
|
||||
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<string, string | boolean>): TuiAgent | undefined {
|
||||
const agent = getPresentStringFlag(flags, 'agent')
|
||||
if (agent === undefined) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string> {
|
||||
const explicit = getOptionalStringFlag(flags, 'terminal')
|
||||
const explicit = getPresentStringFlag(flags, 'terminal')
|
||||
if (explicit) {
|
||||
return explicit
|
||||
}
|
||||
|
||||
@@ -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<string, string | boolean>,
|
||||
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<string, string | boolean>): boolean {
|
||||
return flags.has('project') || flags.has('host') || flags.has('project-host-setup')
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user