diff --git a/src/cli/args.test.ts b/src/cli/args.test.ts index 1ac86d99e12..d94b8447de2 100644 --- a/src/cli/args.test.ts +++ b/src/cli/args.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import type { CommandSpec } from './args' +import { COMMAND_SPECS } from './specs' import { REPEATED_FLAG_SEPARATOR, findCommandSpec, @@ -325,6 +326,25 @@ describe('validateCommandAndFlags', () => { } }) + it('points --from at --terminal on the one verb that renamed the caller flag', () => { + const parsed = parseArgs(['orchestration', 'check', '--from', 'term_a']) + + try { + validateCommandAndFlags(COMMAND_SPECS, parsed) + throw new Error('expected validateCommandAndFlags to throw') + } catch (error) { + const data = (error as { data?: { suggestions: string[]; nextSteps: string[] } }).data + expect(data?.suggestions[0]).toBe('terminal') + expect(data?.nextSteps[0]).toContain('--terminal') + } + }) + + it('leaves --from alone where the command actually accepts it', () => { + const parsed = parseArgs(['orchestration', 'reply', '--from', 'term_a']) + + expect(() => validateCommandAndFlags(COMMAND_SPECS, parsed)).not.toThrow() + }) + it('attaches did-you-mean suggestions to unknown-command errors', () => { const suggestSpecs: CommandSpec[] = [ { diff --git a/src/cli/command-suggestion.ts b/src/cli/command-suggestion.ts index 7b80138e2f3..09113c853c8 100644 --- a/src/cli/command-suggestion.ts +++ b/src/cli/command-suggestion.ts @@ -105,10 +105,20 @@ export type FlagErrorData = { nextSteps: string[] } +// Why: edit distance cannot recover a rename. `orchestration check` is the one verb +// that identifies its caller with `--terminal` while every sibling uses `--from`, so +// the near-miss ranking answered `--json`/`--run` and left the caller stuck (#16904). +// A synonym only fires where the typed flag is rejected and its partner is accepted. +const FLAG_SYNONYMS: Readonly> = { from: 'terminal' } + function suggestFlags(flag: string, validFlags: string[]): string[] { - return rankByDistance( + const synonym = FLAG_SYNONYMS[flag] + const ranked = rankByDistance( validFlags.map((candidate) => ({ label: candidate, distance: levenshtein(flag, candidate) })) ) + return synonym && validFlags.includes(synonym) + ? [synonym, ...ranked.filter((name) => name !== synonym)].slice(0, MAX_SUGGESTIONS) + : ranked } // Why: include the accepted set so agents can recover without another help call. diff --git a/src/cli/format-recovery.test.ts b/src/cli/format-recovery.test.ts index fea52395cd8..4355b728a1f 100644 --- a/src/cli/format-recovery.test.ts +++ b/src/cli/format-recovery.test.ts @@ -1,8 +1,55 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { formatCliError } from './format' +import { formatCliError, reportCliError } from './format' import { RuntimeClientError, RuntimeRpcFailureError } from './runtime-client' +function selectorNotFound(): RuntimeRpcFailureError { + return new RuntimeRpcFailureError({ + id: 'req_selector', + ok: false, + error: { code: 'selector_not_found', message: 'selector_not_found' }, + _meta: { runtimeId: 'runtime_local' } + }) +} + +describe('worktree selector recovery', () => { + it('names the offending value and the valid forms on a bare repo id', () => { + const output = formatCliError(selectorNotFound(), { + commandPath: ['orchestration', 'worker-start'], + worktreeSelector: 'id:github:stablyai/orca' + }) + + expect(output).toContain('No Orca workspace matched the worktree selector') + expect(output).toContain('id:github:stablyai/orca') + expect(output).toContain('Did you mean: id:github:stablyai/orca::') + expect(output).toContain('Valid selector forms:') + expect(output).toContain('a bare repository id is not a worktree id') + }) + + it('carries the same recovery into the --json failure envelope', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + + reportCliError(selectorNotFound(), true, { + commandPath: ['terminal', 'create'], + worktreeSelector: 'path:/nope' + }) + + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toMatchObject({ + error: { + code: 'selector_not_found', + data: { selector: 'path:/nope', validSelectorForms: expect.arrayContaining(['current']) } + } + }) + log.mockRestore() + }) + + it('stays silent when no worktree selector was passed', () => { + expect(formatCliError(selectorNotFound(), { commandPath: ['worktree', 'show'] })).toBe( + 'selector_not_found' + ) + }) +}) + describe('CLI error recovery', () => { it('prints did-you-mean next steps for an unknown-command error carrying data', () => { const error = new RuntimeClientError('invalid_argument', 'Unknown command: worktree remov', { diff --git a/src/cli/format.ts b/src/cli/format.ts index 0a297138364..968a0910d76 100644 --- a/src/cli/format.ts +++ b/src/cli/format.ts @@ -5,6 +5,7 @@ import { stripAutomationOwnerConflictCode } from '../shared/automation-owner-conflict' import { automationOwnerConflictRecovery } from './automation-owner-conflict-recovery' +import { worktreeSelectorRecovery } from './worktree-selector-recovery' import { prepareComputerCliJsonResult } from './computer-format' import type { RuntimeRpcFailure, RuntimeRpcSuccess } from './runtime-client' import { RuntimeClientError, RuntimeRpcFailureError } from './runtime/types' @@ -69,6 +70,21 @@ export { type CliErrorContext = { commandPath?: readonly string[] + /** The `--worktree` value this invocation sent; the runtime's error never echoes it. */ + worktreeSelector?: string +} + +function selectorRecovery(code: string | undefined, context: CliErrorContext) { + return code === 'selector_not_found' && context.worktreeSelector + ? worktreeSelectorRecovery(context.worktreeSelector) + : undefined +} + +function errorCode(error: unknown): string | undefined { + if (error instanceof RuntimeRpcFailureError) { + return error.response.error.code + } + return error instanceof RuntimeClientError ? error.code : undefined } export function printResult( @@ -85,6 +101,10 @@ export function printResult( export function formatCliError(error: unknown, context: CliErrorContext = {}): string { const message = error instanceof Error ? error.message : String(error) + const selector = selectorRecovery(errorCode(error), context) + if (selector) { + return formatMessageWithNextSteps(message, selector.nextSteps) + } if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') { if (hasOrchestrationRequestId(error.data)) { return message @@ -130,9 +150,19 @@ function hasOrchestrationRequestId(data: unknown): boolean { } export function reportCliError(error: unknown, json: boolean, context: CliErrorContext = {}): void { + const selector = selectorRecovery(errorCode(error), context) if (json) { if (error instanceof RuntimeRpcFailureError) { - console.log(JSON.stringify(withAutomationOwnerConflictRecovery(error.response), null, 2)) + const response = withAutomationOwnerConflictRecovery(error.response) + console.log( + JSON.stringify( + selector + ? { ...response, error: { ...response.error, data: response.error.data ?? selector } } + : response, + null, + 2 + ) + ) } else { const response: RuntimeRpcFailure = { id: 'local', @@ -201,6 +231,10 @@ function localCliErrorData(error: unknown, context: CliErrorContext): unknown { if (error instanceof RuntimeClientError && error.data !== undefined) { return error.data } + const selector = selectorRecovery(errorCode(error), context) + if (selector) { + return selector + } const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error)) if (conflict) { return conflict diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 8504f647392..7308d39dac6 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -228,6 +228,29 @@ describe('unknown command surfaces a suggestion', () => { expect(stderr).toContain('--json') }) + it('names the offending --worktree value and the valid forms on selector_not_found', async () => { + const { RuntimeRpcFailureError } = await import('./runtime/types.js') + callMock.mockRejectedValue( + new RuntimeRpcFailureError({ + id: 'req_selector', + ok: false, + error: { code: 'selector_not_found', message: 'selector_not_found' }, + _meta: { runtimeId: 'runtime_local' } + }) + ) + + await main( + ['orchestration', 'worker-start', '--task', 't1', '--worktree', 'repo-1', '--agent', 'codex'], + '/tmp/repo' + ) + + expect(process.exitCode).toBe(1) + const stderr = errorSpy.mock.calls.map((call) => String(call[0])).join('\n') + expect(stderr).toContain('No Orca workspace matched the worktree selector "repo-1"') + expect(stderr).toContain('id:repo-1::') + expect(stderr).toContain('Valid selector forms:') + }) + it('reports a pre-command flag that belongs to another command', async () => { await main(['--workspace', 'worktree', 'list'], '/tmp/repo') diff --git a/src/cli/index.ts b/src/cli/index.ts index 9389113b195..01aa1d9e4be 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -176,7 +176,11 @@ export async function main( json }) } catch (error) { - reportCliError(error, json, { commandPath: parsed.commandPath }) + const worktreeSelector = parsed.flags.get('worktree') + reportCliError(error, json, { + commandPath: parsed.commandPath, + ...(typeof worktreeSelector === 'string' ? { worktreeSelector } : {}) + }) process.exitCode = 1 } } diff --git a/src/cli/worktree-selector-recovery.ts b/src/cli/worktree-selector-recovery.ts new file mode 100644 index 00000000000..ac597fc6c8c --- /dev/null +++ b/src/cli/worktree-selector-recovery.ts @@ -0,0 +1,55 @@ +// Why: the runtime answers an unresolvable `--worktree` with a bare +// `selector_not_found` — no offending value and no grammar — so a caller who passed +// a repo id where a worktree id belongs cannot tell what was wrong (#16904). The CLI +// is the only layer that still knows what the caller typed, so it shapes the recovery +// here, in the same validFlags/suggestions/nextSteps shape as an unknown-flag error. + +export const WORKTREE_SELECTOR_FORMS = [ + 'id:::', + 'path:', + 'name:', + 'branch:', + 'identity:', + 'issue:', + 'current', + 'active' +] as const + +export type WorktreeSelectorRecovery = { + selector: string + validSelectorForms: readonly string[] + suggestions: readonly string[] + nextSteps: readonly string[] +} + +const PREFIXES = ['id:', 'path:', 'name:', 'branch:', 'identity:', 'issue:'] + +function suggestForms(selector: string): string[] { + if (selector.startsWith('id:')) { + // A worktree id is `::`; the repo id alone names no checkout. + return selector.includes('::') + ? [] + : [`id:${selector.slice(3)}::`, 'path:'] + } + if (PREFIXES.some((prefix) => selector.startsWith(prefix))) { + return [] + } + return selector.startsWith('/') || /^[A-Za-z]:[\\/]/.test(selector) + ? [`path:${selector}`] + : [`id:${selector}::`, `name:${selector}`, `branch:${selector}`] +} + +export function worktreeSelectorRecovery(selector: string): WorktreeSelectorRecovery { + const suggestions = suggestForms(selector) + return { + selector, + validSelectorForms: WORKTREE_SELECTOR_FORMS, + suggestions, + nextSteps: [ + `No Orca workspace matched the worktree selector "${selector}".`, + ...(suggestions.length > 0 ? [`Did you mean: ${suggestions.join(', ')}`] : []), + `Valid selector forms: ${WORKTREE_SELECTOR_FORMS.join(', ')}.`, + 'List the exact values with `orca worktree list --json`; a bare repository id is not a worktree id.' + ] + } +}