mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
fix(cli): make selector_not_found name the offending worktree selector
A bare repo id passed to --worktree returned a content-free `selector_not_found` with no value and no grammar, so a caller could not tell what was wrong. Shape it at the CLI boundary — the only layer that still knows what was typed — in the same selector/suggestions/nextSteps shape as an unknown-flag error, and point `--from` on `orchestration check` at `--terminal`, which edit distance cannot reach.
This commit is contained in:
@@ -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[] = [
|
||||
{
|
||||
|
||||
@@ -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<Record<string, string>> = { 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.
|
||||
|
||||
@@ -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::<absolute-path>')
|
||||
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', {
|
||||
|
||||
+35
-1
@@ -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<TResult>(
|
||||
@@ -85,6 +101,10 @@ export function printResult<TResult>(
|
||||
|
||||
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
|
||||
|
||||
@@ -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::<absolute-path>')
|
||||
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')
|
||||
|
||||
|
||||
+5
-1
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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:<repo-id>::<absolute-path>',
|
||||
'path:<absolute-path>',
|
||||
'name:<display-name>',
|
||||
'branch:<branch>',
|
||||
'identity:<identity-key>',
|
||||
'issue:<number>',
|
||||
'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 `<repo-id>::<path>`; the repo id alone names no checkout.
|
||||
return selector.includes('::')
|
||||
? []
|
||||
: [`id:${selector.slice(3)}::<absolute-path>`, 'path:<absolute-path>']
|
||||
}
|
||||
if (PREFIXES.some((prefix) => selector.startsWith(prefix))) {
|
||||
return []
|
||||
}
|
||||
return selector.startsWith('/') || /^[A-Za-z]:[\\/]/.test(selector)
|
||||
? [`path:${selector}`]
|
||||
: [`id:${selector}::<absolute-path>`, `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.'
|
||||
]
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user