mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
[comment audit] Shorten CLI parsing and host-routing comments (#32265)
* chore: shorten process-host comments * chore: audit CLI parsing and host-routing comments * chore: remove stale host-selector restriction comment --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
@@ -1,9 +1,6 @@
|
||||
import type { CommandSpec } from './args'
|
||||
import { effectiveAllowedFlags } from './args'
|
||||
|
||||
// Why: serialize the live spec table so agent discovery cannot drift from the
|
||||
// command surface it describes.
|
||||
|
||||
const SCHEMA_VERSION = 1
|
||||
|
||||
export type AgentContextCommand = {
|
||||
@@ -29,7 +26,7 @@ export type AgentContextSchema = {
|
||||
|
||||
export function buildAgentContext(specs: CommandSpec[]): AgentContextSchema {
|
||||
const commands = specs
|
||||
// Why: hidden specs dispatch but stay off every discovery surface, including this one.
|
||||
// Hidden commands dispatch but stay off discovery surfaces.
|
||||
.filter((spec) => spec.hidden !== true)
|
||||
.map((spec) => ({
|
||||
command: spec.path.join(' '),
|
||||
@@ -38,8 +35,7 @@ export function buildAgentContext(specs: CommandSpec[]): AgentContextSchema {
|
||||
argumentMode: spec.argumentMode ?? 'parsed',
|
||||
summary: spec.summary,
|
||||
usage: spec.usage,
|
||||
// Why: the effective accepted set (globals + conditional --page), not just
|
||||
// allowedFlags — otherwise agents treat --json/--help as unsupported.
|
||||
// Include globals and conditional flags so discovery matches validation.
|
||||
flags: effectiveAllowedFlags(spec),
|
||||
positionalArgs: spec.positionalArgs ?? [],
|
||||
...(spec.variadicPositional ? { variadicPositional: true } : {}),
|
||||
@@ -47,7 +43,6 @@ export function buildAgentContext(specs: CommandSpec[]): AgentContextSchema {
|
||||
examples: spec.examples ?? [],
|
||||
notes: spec.notes ?? []
|
||||
}))
|
||||
// Why: deterministic ordering so the JSON diffs cleanly across runs.
|
||||
.sort((a, b) => a.command.localeCompare(b.command))
|
||||
return {
|
||||
schemaVersion: SCHEMA_VERSION,
|
||||
@@ -57,8 +52,6 @@ export function buildAgentContext(specs: CommandSpec[]): AgentContextSchema {
|
||||
}
|
||||
|
||||
export function formatAgentContextSummary(schema: AgentContextSchema): string {
|
||||
// Why: keep the default (human) output bounded — the full surface is large, so
|
||||
// point the reader at --json rather than dumping every command.
|
||||
return [
|
||||
`${schema.commandCount} commands (schema v${schema.schemaVersion}).`,
|
||||
'Run `orca agent-context --json` for the full machine-readable command schema.'
|
||||
|
||||
@@ -1,8 +1,4 @@
|
||||
/**
|
||||
* How the CLI starts an agent: through `agent.launch`, the host's one launch point that the desktop
|
||||
* and phone use too. `--surface chat|terminal` picks a chat or a terminal for this launch; without
|
||||
* it the user's Chat UI setting does. Either way the receipt says which ran and why.
|
||||
*/
|
||||
// agent.launch shares desktop/mobile policy; absent --surface uses the host's Chat UI setting.
|
||||
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import type { AgentLaunchPrompt, AgentLaunchResult } from '../shared/agent-launch-intent'
|
||||
@@ -31,7 +27,7 @@ import { withCliAgentLaunchRecovery } from './agent-launch-recovery'
|
||||
|
||||
type Flags = Map<string, string | boolean>
|
||||
|
||||
/** `create` is the `worktree.create` request the CLI already builds; the host strips agent fields. */
|
||||
/** The host strips agent fields from the worktree.create payload. */
|
||||
export type CliAgentLaunchTarget =
|
||||
| { kind: 'existing'; worktree: string }
|
||||
| { kind: 'create-worktree'; create: Record<string, unknown> }
|
||||
@@ -63,8 +59,6 @@ export function parseCliAgent(value: string): TuiAgent {
|
||||
return value
|
||||
}
|
||||
|
||||
/** The agent, prompt, model and effort a CLI start asks for; refuses a model or effort the launch
|
||||
* could not apply, by the same rule as `orchestration worker-start`. */
|
||||
export function readCliAgentLaunch(
|
||||
flags: Flags,
|
||||
agent: TuiAgent,
|
||||
@@ -94,7 +88,6 @@ export function readCliAgentLaunch(
|
||||
agent,
|
||||
...(surface ? { surface } : {}),
|
||||
...(operationId ? { operationId } : {}),
|
||||
// A blank prompt is no prompt: the agent starts with nothing sent.
|
||||
...(typeof prompt === 'string' && prompt.trim()
|
||||
? { prompt: { text: prompt, delivery: 'submit' as const } }
|
||||
: {}),
|
||||
|
||||
@@ -1,12 +1,10 @@
|
||||
import { AGENT_LAUNCH_RUNTIME_CAPABILITIES } from '../shared/agent-launch-runtime-capability'
|
||||
import { okFixture } from './test-fixtures'
|
||||
|
||||
/** A host that runs `agent.launch`, as `status.get` reports it. */
|
||||
export function agentLaunchHostFixture() {
|
||||
return okFixture('req_status', { capabilities: [...AGENT_LAUNCH_RUNTIME_CAPABILITIES] })
|
||||
}
|
||||
|
||||
/** What `agent.launch` answers when the user's Chat UI setting opened a chat. */
|
||||
export function agentLaunchFixture(worktreeId: string, warning?: string) {
|
||||
return okFixture('req_launch', {
|
||||
outcome: { kind: 'structured', sessionId: 'codex_1', handle: 'agent_1' },
|
||||
|
||||
@@ -16,12 +16,7 @@ export type PrintableSessionSearchResponse = Exclude<
|
||||
{ kind: 'malformed-cursor' }
|
||||
>
|
||||
|
||||
/**
|
||||
* Transcript text reaches the terminal verbatim, so an OSC 52 or cursor sequence
|
||||
* inside a tool log would otherwise run on the reader's terminal. The `[[` `]]`
|
||||
* match marks are left as the engine wrote them: this CLI emits no ANSI anywhere,
|
||||
* so a colour scheme invented here would be the only one in the surface.
|
||||
*/
|
||||
// Strip terminal controls from untrusted transcripts, including OSC 52 clipboard writes.
|
||||
export function terminalSafe(value: string): string {
|
||||
return stripAnsiEscapeSequences(value).replace(TERMINAL_CONTROL_CHARACTER_PATTERN, '')
|
||||
}
|
||||
@@ -85,8 +80,7 @@ function formatUnavailable(
|
||||
if (reason === 'unsupported-agent') {
|
||||
return 'This host does not support history search for the selected agent. Update Orca on that host or select another agent.'
|
||||
}
|
||||
// The CLI scopes with --path, never with an identity, so this only reaches a
|
||||
// caller that built a request by hand.
|
||||
// Only hand-built requests can select an unknown identity; the CLI scopes by path.
|
||||
if (reason === 'scope-unknown') {
|
||||
return 'This host does not know the workspace or project that search was scoped to.'
|
||||
}
|
||||
|
||||
+6
-12
@@ -38,7 +38,7 @@ function setFlagValue(
|
||||
flags.set(name, value)
|
||||
}
|
||||
|
||||
/** The most specific spec whose path prefixes `path`, so a group never shadows a leaf. */
|
||||
// Prefer the longest path so command groups cannot shadow leaf commands.
|
||||
function specForPathPrefix(
|
||||
specs: readonly CommandSpec[],
|
||||
path: readonly string[]
|
||||
@@ -75,9 +75,7 @@ export function parseArgs(
|
||||
}
|
||||
|
||||
const assignment = token.slice(2)
|
||||
// Why: `--flag=value` is the only unambiguous way to pass a value that
|
||||
// itself starts with `--` (e.g. `--text=--help`); the space-separated form
|
||||
// treats a `--`-leading next token as a new flag, so it can't express one.
|
||||
// --flag=value permits values starting with -- without treating them as another flag.
|
||||
const equalsIndex = assignment.indexOf('=')
|
||||
if (equalsIndex !== -1) {
|
||||
flagEntries.push([assignment.slice(0, equalsIndex), assignment.slice(equalsIndex + 1)])
|
||||
@@ -89,7 +87,7 @@ export function parseArgs(
|
||||
flagEntries.push([flag, true])
|
||||
continue
|
||||
}
|
||||
// Why: a pre-command flag must not consume a registry-resolvable command path.
|
||||
// Pre-command flags must not consume a registered command path.
|
||||
if (commandPath.length === 0 && i + 1 === commandIndex) {
|
||||
flagEntries.push([flag, true])
|
||||
continue
|
||||
@@ -176,7 +174,6 @@ export function supportsBrowserPageFlag(commandPath: string[]): boolean {
|
||||
].includes(joined)
|
||||
}
|
||||
|
||||
// Why: validation and agent discovery must expose the same effective flag set.
|
||||
export function effectiveAllowedFlags(spec: CommandSpec): string[] {
|
||||
if (spec.argumentMode === 'passthrough') {
|
||||
return []
|
||||
@@ -208,14 +205,12 @@ export function isCommandGroup(specs: CommandSpec[], commandPath: string[]): boo
|
||||
export function normalizeCommandPositionals(specs: CommandSpec[], parsed: ParsedArgs): ParsedArgs {
|
||||
for (const spec of specs) {
|
||||
const positionalArgs = spec.positionalArgs ?? []
|
||||
// Why: aliased paths still need canonicalization when there are no positionals.
|
||||
// Alias-only paths still need canonicalization without positional arguments.
|
||||
if (positionalArgs.length === 0 && !spec.aliases) {
|
||||
continue
|
||||
}
|
||||
// Why: canonicalize aliases before validation and dispatch so both use one key.
|
||||
for (const base of specPaths(spec)) {
|
||||
// Why: `< 0` (not `<= 0`) so an exact base match with zero positionals
|
||||
// still canonicalizes an aliased path; upper bound guards over-consumption.
|
||||
// Zero positionals still permit an exact alias match.
|
||||
const positionalCount = parsed.commandPath.length - base.length
|
||||
if (
|
||||
positionalCount < 0 ||
|
||||
@@ -228,8 +223,7 @@ export function normalizeCommandPositionals(specs: CommandSpec[], parsed: Parsed
|
||||
}
|
||||
const flags = new Map(parsed.flags)
|
||||
const values = parsed.commandPath.slice(base.length)
|
||||
// Why: validation runs inside main's error-reporting path, so normalization
|
||||
// records ambiguity instead of throwing before CLI errors can be formatted.
|
||||
// Record ambiguity here; validation reports it through the CLI error formatter.
|
||||
const providedPositionals = values.map(
|
||||
(_, index) => positionalArgs[Math.min(index, positionalArgs.length - 1)]
|
||||
)
|
||||
|
||||
@@ -1,14 +1,4 @@
|
||||
/**
|
||||
* Where a CLI create — or an edit that moves the record's project/workspace —
|
||||
* wants the automation to land.
|
||||
*
|
||||
* The CLI cannot project a host itself, so it asks the same authority that will
|
||||
* perform the write to resolve the selector it was given and to name its
|
||||
* current SSH registrations. That captured incarnation rides along on the
|
||||
* request, and the authority re-checks it inside the write, so a target that
|
||||
* disappears in between fails the write instead of silently repointing the
|
||||
* record at a dead or re-registered host.
|
||||
*/
|
||||
// Capture the authority's SSH generation so it can reject removed or re-registered targets at write time.
|
||||
|
||||
import {
|
||||
AUTOMATION_OWNER_CONFLICT_CODES,
|
||||
@@ -21,7 +11,7 @@ import type { Repo } from '../shared/repo-types'
|
||||
import type { RuntimeClient } from './runtime-client'
|
||||
import { fetchSshTargetSummaries } from './host-selector-alternatives'
|
||||
|
||||
/** The project/workspace selectors the same request carries; both empty means the host cannot change. */
|
||||
/** Empty selectors mean the execution host cannot change. */
|
||||
export type AutomationDestinationTarget = { repo?: string; workspace?: string }
|
||||
|
||||
async function resolveTargetRepo(
|
||||
@@ -40,12 +30,7 @@ async function resolveTargetRepo(
|
||||
.repo
|
||||
}
|
||||
|
||||
/**
|
||||
* `undefined` means the authority registers the target but assigns no
|
||||
* generation, which is a host with nothing to fence — not a host to fail
|
||||
* against. A target the authority does not register at all is positive
|
||||
* evidence that the destination is gone.
|
||||
*/
|
||||
// A registered target without a generation needs no fence; an unregistered target is gone.
|
||||
async function sshTargetGeneration(
|
||||
client: RuntimeClient,
|
||||
targetId: string
|
||||
|
||||
@@ -31,10 +31,6 @@ function automation(overrides: Partial<Automation> = {}): Automation {
|
||||
}
|
||||
}
|
||||
|
||||
// `orca automations run` sends users to `orca automations show` to find out where
|
||||
// an automation lives. The stored `executionTarget*` fields survive their host's
|
||||
// removal untouched, so printing only those renders a dead automation byte-for-byte
|
||||
// like a healthy one and the named recovery step cannot be completed.
|
||||
describe('formatAutomationShow reports the host the authority projects', () => {
|
||||
it('distinguishes a live SSH host from a replaced one by its incarnation', () => {
|
||||
const live = formatAutomationShow({
|
||||
@@ -58,7 +54,7 @@ describe('formatAutomationShow reports the host the authority projects', () => {
|
||||
})
|
||||
|
||||
expect(output).toContain('host: orphan (no host can run this automation)')
|
||||
// The stored pin stays visible; it is the record's history, not its current owner.
|
||||
// Keep the stored target as history, separately from current host ownership.
|
||||
expect(output).toContain('target: ssh:box-1')
|
||||
})
|
||||
|
||||
@@ -71,8 +67,6 @@ describe('formatAutomationShow reports the host the authority projects', () => {
|
||||
expect(output).toContain('host: self')
|
||||
})
|
||||
|
||||
// An older authority projects no owner; inventing one from the stored fields would
|
||||
// report a removed host as healthy.
|
||||
it('omits the host line when the authority reports no owner', () => {
|
||||
const output = formatAutomationShow({ automation: automation() })
|
||||
|
||||
|
||||
@@ -1,12 +1,4 @@
|
||||
/**
|
||||
* Human-readable automation output for the CLI.
|
||||
*
|
||||
* The host lines come from the authority's own projection, never from the
|
||||
* stored `executionTarget*` fields: a target that was removed — or removed and
|
||||
* re-registered under the same id — leaves those fields untouched, so printing
|
||||
* them alone would render a dead host identically to a healthy one and leave
|
||||
* `orca automations show` unable to complete the recovery it is named in.
|
||||
*/
|
||||
// Use authority-projected owners: stored target fields survive host removal and re-registration.
|
||||
|
||||
import type { Automation, AutomationRun } from '../shared/automations-types'
|
||||
import type { AutomationListItem } from '../shared/automation-list-scope'
|
||||
@@ -95,7 +87,6 @@ export function formatAutomationShow(result: AutomationShowPayload): string {
|
||||
`baseBranch: ${automation.baseBranch ?? 'null'}`,
|
||||
`reuseSession: ${automation.reuseSession}`,
|
||||
`target: ${automation.executionTargetType}:${automation.executionTargetId}`,
|
||||
// Omitted rather than guessed: a host that reports no owner has none to report.
|
||||
...(result.owner ? [`host: ${formatOwnerSelector(result.owner.selector)}`] : []),
|
||||
`prompt: ${automation.prompt}`
|
||||
].join('\n')
|
||||
|
||||
@@ -25,8 +25,6 @@ describe('automation owner conflicts read as CLI outcomes', () => {
|
||||
expect(output).toContain('Next step: Run the command again')
|
||||
})
|
||||
|
||||
// A removed SSH host fails identically forever, so proposing a retry would send
|
||||
// the user in a loop instead of to the two things that actually resolve it.
|
||||
it('never proposes a retry for a removed target', () => {
|
||||
const output = formatCliError(
|
||||
conflict(AUTOMATION_OWNER_CONFLICT_CODES.targetRemoved, 'Host removed.')
|
||||
@@ -58,9 +56,6 @@ describe('automation owner conflicts read as CLI outcomes', () => {
|
||||
expect(output).toBe('Something else: runtime_error')
|
||||
})
|
||||
|
||||
// The trailing token is the convention precisely because a hop can flatten the
|
||||
// error class. Classifying on `.code` alone reads only the hop that preserved it,
|
||||
// and the user loses the recovery on every other one.
|
||||
it('recovers the conflict from the message when a hop flattened the code', () => {
|
||||
const flattened = new RuntimeRpcFailureError({
|
||||
id: 'request-1',
|
||||
@@ -100,7 +95,6 @@ describe('automation owner conflicts read as CLI outcomes', () => {
|
||||
expect(payload.error.data.nextSteps[0]).toContain('no longer registered')
|
||||
})
|
||||
|
||||
// A local throw never reaches the envelope, so the token is all the CLI has.
|
||||
it('classifies a locally thrown conflict the same way', () => {
|
||||
const output = formatCliError(
|
||||
new Error(
|
||||
|
||||
@@ -1,11 +1,4 @@
|
||||
/**
|
||||
* What a user should do about each automation owner conflict, phrased for the
|
||||
* CLI rather than for a window that can re-render itself.
|
||||
*
|
||||
* `automation_target_removed` deliberately offers no retry: the SSH host the
|
||||
* record is pinned to is gone, so every retry fails identically until the user
|
||||
* re-adds that host or deletes the automation.
|
||||
*/
|
||||
// Removed SSH targets cannot recover through retry; users must re-add the host or delete the automation.
|
||||
|
||||
import {
|
||||
AUTOMATION_OWNER_CONFLICT_CODES,
|
||||
|
||||
@@ -19,7 +19,6 @@ function run(index: number, snapshotBytes = 0): AutomationRun {
|
||||
} as unknown as AutomationRun
|
||||
}
|
||||
|
||||
/** A paired host: pages when asked, and refuses any reply a remote connection cannot carry. */
|
||||
function pairedHost(runs: AutomationRun[], options: { pages: boolean } = { pages: true }) {
|
||||
return vi.fn(async (params: { automationId?: string; limit?: number; cursor?: string }) => {
|
||||
const result =
|
||||
@@ -35,7 +34,6 @@ function pairedHost(runs: AutomationRun[], options: { pages: boolean } = { pages
|
||||
|
||||
describe('readAllAutomationRuns', () => {
|
||||
it('reads a history larger than one remote reply, in order and without gaps', async () => {
|
||||
// 430 runs of ~23 KB is ~10 MB, which a single unpaged reply could not carry.
|
||||
const runs = Array.from({ length: 430 }, (_, index) => run(index, 23_000))
|
||||
const host = pairedHost(runs)
|
||||
const result = await readAllAutomationRuns(host, undefined)
|
||||
|
||||
@@ -2,7 +2,7 @@ import type { AutomationRun } from '../shared/automations-types'
|
||||
import type { RuntimeRpcSuccess } from '../shared/runtime-rpc-envelope'
|
||||
import { RuntimeClientError, type RuntimeClient } from './runtime-client'
|
||||
|
||||
// Why 25: a snapshot can reach ~200 KB, so 25 runs stay under a remote reply's 4 MB in practice.
|
||||
// Keep typical snapshot pages below the remote reply limit; retry smaller when needed.
|
||||
export const AUTOMATION_RUNS_PAGE_SIZE = 25
|
||||
const REPLY_TOO_LARGE = 'reply_too_large'
|
||||
|
||||
@@ -10,7 +10,6 @@ type RunsReply = { runs: AutomationRun[]; nextCursor?: string | null }
|
||||
|
||||
type RunsQuery = { automationId?: string; limit: number; cursor?: string }
|
||||
|
||||
/** Sends one `automation.runs` request. */
|
||||
export type AutomationRunsRpc = (
|
||||
params: Partial<RunsQuery>
|
||||
) => Promise<RuntimeRpcSuccess<RunsReply>>
|
||||
@@ -44,7 +43,6 @@ async function readShrinkingPage(
|
||||
}
|
||||
}
|
||||
|
||||
/** One page, halving the page size while the remote host refuses the reply as too large. */
|
||||
export async function readAutomationRunsPage(
|
||||
rpc: AutomationRunsRpc,
|
||||
query: RunsQuery
|
||||
@@ -52,10 +50,7 @@ export async function readAutomationRunsPage(
|
||||
return (await readShrinkingPage(rpc, query)).page
|
||||
}
|
||||
|
||||
/**
|
||||
* Every run, read page by page so no single reply has to carry the whole history (#28254).
|
||||
* A host older than paging ignores `limit` and returns everything with no cursor, ending the loop.
|
||||
*/
|
||||
/** Older hosts ignore paging and return all runs without a cursor, ending the loop. */
|
||||
export async function readAllAutomationRuns(
|
||||
rpc: AutomationRunsRpc,
|
||||
automationId: string | undefined
|
||||
@@ -65,7 +60,7 @@ export async function readAllAutomationRuns(
|
||||
let cursor: string | undefined
|
||||
let limit = AUTOMATION_RUNS_PAGE_SIZE
|
||||
for (;;) {
|
||||
// Why carry the limit: the next page is likely as heavy as the one that had to shrink.
|
||||
// Later pages may need the same reduced limit.
|
||||
const read = await readShrinkingPage(rpc, { automationId, limit, cursor })
|
||||
const page = read.page
|
||||
limit = read.limit
|
||||
|
||||
@@ -37,7 +37,6 @@ vi.mock('./runtime-client', () => {
|
||||
import { main } from './index'
|
||||
import { okFixture, queueFixtures } from './test-fixtures'
|
||||
|
||||
// Why: StorageKeyValue requires a non-empty key but accepts any string value, empty included.
|
||||
describe('orca cli storage set preserves an empty value', () => {
|
||||
beforeEach(() => {
|
||||
callMock.mockReset()
|
||||
@@ -82,7 +81,7 @@ describe('orca cli storage set preserves an empty value', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: PowerShell 5.1 drops empty native-exe args, so `--value=` is the Windows escape hatch.
|
||||
// PowerShell 5.1 drops empty native arguments; equals form preserves them.
|
||||
it('passes an empty --value= (equals form) through to browser.storage.local.set', async () => {
|
||||
queueFixtures(callMock, okFixture('req_storage_local_set', { success: true }))
|
||||
vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
@@ -133,8 +132,6 @@ describe('orca cli storage set preserves an empty value', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: CookieSet.value and SetCredentials.pass accept any string, empty included,
|
||||
// while name and user require a non-empty one.
|
||||
describe('orca cli cookie set and set credentials preserve an empty value', () => {
|
||||
beforeEach(() => {
|
||||
callMock.mockReset()
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
import type { HandlerGroup } from './handler-group-manifest'
|
||||
|
||||
// Why split out: the browser command surface is a third of the CLI's groups and
|
||||
// changes as a unit, so it keeps handler-group-manifest.ts readable at a glance.
|
||||
export const BROWSER_HANDLER_GROUPS: readonly HandlerGroup[] = [
|
||||
{
|
||||
name: 'browser-identity',
|
||||
|
||||
@@ -14,7 +14,7 @@ type CatalogRow = Pick<RuntimeWorktreeRecord, 'id' | 'path' | 'hostId'> & {
|
||||
identity?: { key: string }
|
||||
}
|
||||
|
||||
/** The execution host this CLI process runs on: the SSH target its relay stamped, else this machine. */
|
||||
/** SSH relays stamp the caller host; otherwise use this machine. */
|
||||
export function getCallerExecutionHostId(
|
||||
env: Readonly<Record<string, string | undefined>> = process.env
|
||||
): ExecutionHostId {
|
||||
@@ -22,9 +22,7 @@ export function getCallerExecutionHostId(
|
||||
return host?.kind === 'ssh' ? toSshExecutionHostId(host.targetId) : LOCAL_EXECUTION_HOST_ID
|
||||
}
|
||||
|
||||
// Why: the catalog lists SSH hosts' worktrees too and two hosts can hold the same path, so an SSH
|
||||
// caller may only take its own host's rows and a local caller none of an SSH host's. `runtime:*`
|
||||
// rows in this runtime's catalog are checkouts it holds itself, and unstamped rows predate stamping.
|
||||
// Paths may exist on multiple hosts; runtime:* and unstamped rows belong to this runtime.
|
||||
function isOnCallerHost(row: CatalogRow, callerHostId: ExecutionHostId): boolean {
|
||||
const rowHostId = row.hostId ?? LOCAL_EXECUTION_HOST_ID
|
||||
return parseExecutionHostId(callerHostId)?.kind === 'ssh'
|
||||
@@ -32,17 +30,15 @@ function isOnCallerHost(row: CatalogRow, callerHostId: ExecutionHostId): boolean
|
||||
: parseExecutionHostId(rowHostId)?.kind !== 'ssh'
|
||||
}
|
||||
|
||||
// Why: `id:` keeps repo inference and the runtime's scoped fast path; only a bare id that another
|
||||
// host's row shares needs the exact identity, or the runtime would refuse it as ambiguous.
|
||||
// Prefer id: for repo inference; shared IDs need an exact host-qualified identity.
|
||||
function toHostScopedSelector(row: CatalogRow, catalog: readonly CatalogRow[]): string {
|
||||
const sharedId = catalog.some((other) => other !== row && other.id === row.id)
|
||||
return sharedId && row.identity?.key ? `identity:${row.identity.key}` : `id:${row.id}`
|
||||
}
|
||||
|
||||
/** A host-scoped selector plus the worktree id it names, which an `identity:` selector hides. */
|
||||
export type HostScopedWorktreeSelection = { selector: string; worktreeId: string }
|
||||
|
||||
/** The caller host's innermost worktree enclosing `cwd`; undefined when none does. */
|
||||
/** Prefer the innermost enclosing worktree on the caller's host. */
|
||||
export function selectEnclosingWorktreeSelectionOnHost(
|
||||
catalog: readonly CatalogRow[],
|
||||
cwd: string,
|
||||
@@ -67,7 +63,6 @@ export function selectEnclosingWorktreeSelectionOnHost(
|
||||
: undefined
|
||||
}
|
||||
|
||||
/** The caller host's innermost worktree enclosing `cwd`, as a selector; undefined when none does. */
|
||||
export function selectEnclosingWorktreeOnHost(
|
||||
catalog: readonly CatalogRow[],
|
||||
cwd: string,
|
||||
@@ -76,7 +71,6 @@ export function selectEnclosingWorktreeOnHost(
|
||||
return selectEnclosingWorktreeSelectionOnHost(catalog, cwd, hostId)?.selector
|
||||
}
|
||||
|
||||
/** Scopes a pane's forwarded bare worktree id to the caller's host; undefined when it has none. */
|
||||
export function selectWorktreeIdOnHost(
|
||||
catalog: readonly CatalogRow[],
|
||||
worktreeId: string,
|
||||
@@ -86,7 +80,6 @@ export function selectWorktreeIdOnHost(
|
||||
return match ? toHostScopedSelector(match, catalog) : undefined
|
||||
}
|
||||
|
||||
/** Scopes a pane's forwarded bare `ORCA_WORKTREE_ID` to the caller's host before it is used. */
|
||||
export async function resolveCallerPaneWorktreeSelector(
|
||||
worktreeId: string,
|
||||
client: RuntimeClient
|
||||
|
||||
@@ -13,7 +13,7 @@ export type CliErrorContext = {
|
||||
commandPath?: readonly string[]
|
||||
/** The `--worktree` value this invocation sent; the runtime's error never echoes it. */
|
||||
worktreeSelector?: string
|
||||
/** The `--repo` value this invocation sent, for the same reason. */
|
||||
/** The runtime error omits the original --repo selector. */
|
||||
repoSelector?: string
|
||||
/** True when --environment, --pairing-code, --host runtime:, or their env vars chose a server. */
|
||||
remoteServerSelected?: boolean
|
||||
@@ -38,7 +38,7 @@ function contextRecovery(
|
||||
remoteServerSelected: context.remoteServerSelected
|
||||
})
|
||||
}
|
||||
// Why: a paired server's handle sent to this machine reads as a dead terminal, not a missing flag.
|
||||
// A remote handle sent locally looks stale rather than missing a flag.
|
||||
if (
|
||||
code === 'terminal_handle_stale' &&
|
||||
context.commandPath?.[0] === 'terminal' &&
|
||||
@@ -78,8 +78,7 @@ export function formatCliError(error: unknown, context: CliErrorContext = {}): s
|
||||
}
|
||||
return `${message}\nOrca is not running. Run 'orca open' first.`
|
||||
}
|
||||
// Why: error-specific recovery must win over the generic computer fallback.
|
||||
// Classified from the whole error, not just `.code`: a hop that flattens the class leaves only the token.
|
||||
// Match before generic recovery; flattened errors may retain only the conflict token.
|
||||
const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error))
|
||||
if (conflict) {
|
||||
return formatMessageWithNextSteps(stripAutomationOwnerConflictCode(message), conflict.nextSteps)
|
||||
@@ -167,7 +166,6 @@ export function reportCliError(error: unknown, json: boolean, context: CliErrorC
|
||||
}
|
||||
}
|
||||
|
||||
/** Machine-readable half of the same recovery the human message carries. */
|
||||
function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): RuntimeRpcFailure {
|
||||
const code = matchAutomationOwnerConflict(response)
|
||||
const conflict = automationOwnerConflictRecovery(code)
|
||||
@@ -178,7 +176,7 @@ function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): Runti
|
||||
...response,
|
||||
error: {
|
||||
...response.error,
|
||||
// Restores the classification a flattening hop dropped, so --json consumers read the conflict, not the transport.
|
||||
// Restore conflict classification lost during transport flattening.
|
||||
code,
|
||||
message: stripAutomationOwnerConflictCode(response.error.message),
|
||||
data: response.error.data ?? conflict
|
||||
@@ -193,7 +191,7 @@ function formatMessageWithNextSteps(message: string, nextSteps: readonly string[
|
||||
return `${message}\n${nextSteps.map((step) => `Next step: ${step}`).join('\n')}`
|
||||
}
|
||||
|
||||
/** Why merge: a mutation error already carries its request id, and `??` dropped the selector grammar. */
|
||||
// Preserve both mutation request IDs and selector recovery hints.
|
||||
function mergeContextRecovery(
|
||||
data: unknown,
|
||||
selector: ReturnType<typeof contextRecovery>
|
||||
@@ -224,7 +222,7 @@ function nextStepsFromData(data: unknown): string[] {
|
||||
return []
|
||||
}
|
||||
|
||||
/** Read here, not from the error's data: the servers that send this predate the hint (#31777). */
|
||||
// Older servers do not include this recovery hint in error data.
|
||||
function computerSidecarExitRecovery(error: unknown, context: CliErrorContext) {
|
||||
if (
|
||||
context.commandPath?.[0] !== 'computer' ||
|
||||
@@ -243,7 +241,7 @@ function localCliErrorData(error: unknown, context: CliErrorContext): unknown {
|
||||
return sidecarExit
|
||||
}
|
||||
const selector = contextRecovery(errorCode(error), context)
|
||||
// Why: error-specific recovery must win over the generic computer fallback.
|
||||
// Error-specific recovery takes precedence over the generic fallback.
|
||||
if (error instanceof RuntimeClientError && error.data !== undefined) {
|
||||
return mergeContextRecovery(error.data, selector)
|
||||
}
|
||||
|
||||
@@ -231,8 +231,7 @@ function isNonInteractiveCodexSubcommand(tokens: string[]): boolean {
|
||||
|
||||
const normalizedSubcommand = subcommand.value.toLowerCase()
|
||||
if (normalizedSubcommand === 'login') {
|
||||
// Why: bare `codex login` displays an auth flow; only explicit status/help
|
||||
// or stdin-fed token modes are safe to leave in a background PTY.
|
||||
// Bare codex login opens an auth flow; only status/help or stdin-token modes are background-safe.
|
||||
const loginStartIndex = subcommand.index + 1
|
||||
const loginSubcommand = findCodexSubcommand(
|
||||
tokens,
|
||||
@@ -250,8 +249,7 @@ function isNonInteractiveCodexSubcommand(tokens: string[]): boolean {
|
||||
)
|
||||
}
|
||||
if (normalizedSubcommand === 'cloud') {
|
||||
// Why: bare `codex cloud` opens the interactive cloud browser, while its
|
||||
// named child commands are plain one-shot commands.
|
||||
// Bare codex cloud is interactive; named subcommands are one-shot commands.
|
||||
const cloudStartIndex = subcommand.index + 1
|
||||
const cloudSubcommand = findCodexSubcommand(
|
||||
tokens,
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
// Why: the shared --focus line describes terminal create's terminal session.
|
||||
// File focus differs from terminal create's shared --focus help.
|
||||
const FILE_OPEN_FOCUS_HELP =
|
||||
"--focus Bring the user to the file (switches Orca's window to its worktree)"
|
||||
|
||||
/** Per-command flag help, kept out of the shared help chain it would crowd. */
|
||||
const COMMAND_SCOPED_FLAG_HELP: Record<string, Record<string, string>> = {
|
||||
'reference add': {
|
||||
url: '--url <url-or-jira-key> Full review/issue URL or Jira key; use positionals or repeated --url',
|
||||
|
||||
@@ -1,23 +1,20 @@
|
||||
export type CommandSpec = {
|
||||
path: string[]
|
||||
// Why: conventional alternate verbs should resolve without duplicating specs or handlers.
|
||||
aliases?: string[][]
|
||||
argumentMode?: 'parsed' | 'passthrough'
|
||||
// Why: typo recovery must never steer a benign mistake into destructive state changes.
|
||||
// Prevent typo recovery from suggesting destructive actions for unrelated commands.
|
||||
destructive?: boolean
|
||||
hidden?: boolean
|
||||
summary: string
|
||||
usage: string
|
||||
allowedFlags: string[]
|
||||
// Why: repeatability is per-command vocabulary. `--agent` repeats for `search`
|
||||
// and is single-valued for `worktree create`, which one global set cannot say.
|
||||
// Flag repeatability varies by command, even for the same flag name.
|
||||
repeatableFlags?: string[]
|
||||
positionalArgs?: string[]
|
||||
variadicPositional?: boolean
|
||||
examples?: string[]
|
||||
notes?: string[]
|
||||
// Why: `--from`/`--terminal` names either the acting caller or a target, and only the spec can
|
||||
// say which. An agent session refuses a caller flag naming anyone else before dispatch.
|
||||
// Caller flags must match the acting agent session; target flags may name another session.
|
||||
identityFlagRoles?: Partial<Record<IdentityFlag, 'caller' | 'target'>>
|
||||
}
|
||||
|
||||
|
||||
@@ -28,8 +28,6 @@ const specs: CommandSpec[] = [
|
||||
allowedFlags: []
|
||||
},
|
||||
{
|
||||
// A destructive command outside the delete-family, to prove the guard keys
|
||||
// off the spec flag rather than a hardcoded verb list.
|
||||
path: ['emulator', 'kill'],
|
||||
destructive: true,
|
||||
summary: 'Kill the emulator',
|
||||
@@ -64,8 +62,6 @@ describe('suggestCommands', () => {
|
||||
})
|
||||
|
||||
it('never suggests a destructive command for a benign non-destructive typo', () => {
|
||||
// `worktree move` sits distance 2 from `worktree remove`; without the
|
||||
// guard it would sole-suggest an irreversible delete on blind retry. #6303
|
||||
const result = suggestCommands(specs, ['worktree', 'move'])
|
||||
expect(result).not.toContain('worktree remove')
|
||||
expect(result).not.toContain('worktree rm')
|
||||
@@ -83,10 +79,7 @@ describe('suggestCommands', () => {
|
||||
})
|
||||
|
||||
it('guards destructive commands outside the delete-family via the spec flag', () => {
|
||||
// `emulator ball` is a benign token, distance 2 from the flagged `emulator
|
||||
// kill` — close enough to otherwise rank, so the guard must exclude it.
|
||||
expect(suggestCommands(specs, ['emulator', 'ball'])).not.toContain('emulator kill')
|
||||
// A genuine near-miss of the destructive verb still recovers.
|
||||
expect(suggestCommands(specs, ['emulator', 'kil'])).toContain('emulator kill')
|
||||
})
|
||||
|
||||
|
||||
@@ -1,21 +1,17 @@
|
||||
import { specPaths, type CommandSpec } from './command-spec'
|
||||
import { levenshtein } from '../shared/edit-distance'
|
||||
|
||||
// Why: rank the live registry so typo recovery cannot drift from accepted paths.
|
||||
|
||||
const SUGGESTION_THRESHOLD = 3
|
||||
const MAX_SUGGESTIONS = 3
|
||||
|
||||
// Why: a close typo of a destructive verb (`remov`→`remove`) still signals that
|
||||
// intent, but `move` (distance 2 from `remove`) does not — keep this at 1 so
|
||||
// genuine recovery works while unrelated verbs stay locked out. #6303
|
||||
// Distance 1 recovers remov→remove without suggesting deletion for move.
|
||||
const DESTRUCTIVE_INTENT_THRESHOLD = 1
|
||||
|
||||
function finalToken(path: string[]): string {
|
||||
return path.at(-1) ?? ''
|
||||
}
|
||||
|
||||
// Why: declared aliases connect equivalent destructive verbs without unlocking unrelated actions.
|
||||
// Aliases join destructive verb families without admitting unrelated verbs.
|
||||
function destructiveVerbFamilies(specs: CommandSpec[]): Map<string, Set<string>> {
|
||||
const families = new Map<string, Set<string>>()
|
||||
for (const spec of specs) {
|
||||
@@ -48,7 +44,6 @@ export type CommandErrorData = {
|
||||
nextSteps: string[]
|
||||
}
|
||||
|
||||
// Why: one bounded near-match ranking keeps command and flag recovery consistent.
|
||||
function rankByDistance(scored: { label: string; distance: number }[]): string[] {
|
||||
return scored
|
||||
.filter((entry) => entry.distance > 0 && entry.distance <= SUGGESTION_THRESHOLD)
|
||||
@@ -57,7 +52,7 @@ function rankByDistance(scored: { label: string; distance: number }[]): string[]
|
||||
.map((entry) => entry.label)
|
||||
}
|
||||
|
||||
// Why: same-depth matching avoids suggesting parent groups or unrelated commands.
|
||||
// Match depth to avoid suggesting parent groups or unrelated commands.
|
||||
export function suggestCommands(specs: CommandSpec[], commandPath: string[]): string[] {
|
||||
const input = commandPath.join(' ')
|
||||
const families = destructiveVerbFamilies(specs)
|
||||
@@ -107,10 +102,7 @@ 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.
|
||||
// Recover renamed flags only when the typed flag is rejected and its synonym is accepted.
|
||||
const FLAG_SYNONYMS: Readonly<Record<string, string>> = { from: 'terminal' }
|
||||
|
||||
function suggestFlags(flag: string, validFlags: string[]): string[] {
|
||||
@@ -127,7 +119,6 @@ function suggestFlags(flag: string, validFlags: string[]): string[] {
|
||||
: ranked
|
||||
}
|
||||
|
||||
// Why: include the accepted set so agents can recover without another help call.
|
||||
export function unknownFlagData(flag: string, validFlags: string[]): FlagErrorData {
|
||||
const sortedValid = [...validFlags].sort((a, b) => a.localeCompare(b))
|
||||
const suggestions = suggestFlags(flag, sortedValid)
|
||||
|
||||
@@ -95,8 +95,7 @@ export function prepareComputerCliJsonResult<TResult>(
|
||||
}
|
||||
} as RuntimeRpcSuccess<TResult>
|
||||
} catch {
|
||||
// Why: temp-file export is an ergonomics optimization; keep inline screenshot
|
||||
// data when disk, permissions, or path validation would otherwise fail --json.
|
||||
// Preserve inline screenshot data if the optional temp-file export fails.
|
||||
return response
|
||||
}
|
||||
}
|
||||
@@ -124,8 +123,7 @@ function cleanupComputerScreenshots(outputDir: string): void {
|
||||
const now = Date.now()
|
||||
const markerPath = join(outputDir, COMPUTER_SCREENSHOT_CLEANUP_MARKER)
|
||||
try {
|
||||
// Why: agents can call computer-use CLI commands in loops; a marker keeps
|
||||
// temp cleanup from becoming a synchronous directory scan per screenshot.
|
||||
// Throttle synchronous cleanup scans across repeated screenshots.
|
||||
if (statSync(markerPath).mtimeMs > now - COMPUTER_SCREENSHOT_CLEANUP_INTERVAL_MS) {
|
||||
return
|
||||
}
|
||||
@@ -144,13 +142,13 @@ function cleanupComputerScreenshots(outputDir: string): void {
|
||||
rmSync(path, { force: true })
|
||||
}
|
||||
} catch {
|
||||
// Best-effort cleanup only; formatting should not fail because a temp file raced.
|
||||
// A raced temp file must not fail screenshot formatting.
|
||||
}
|
||||
}
|
||||
try {
|
||||
writeFileSync(markerPath, `${now}\n`, { mode: 0o600 })
|
||||
} catch {
|
||||
// Best-effort marker only; stale cleanup state should not hide a screenshot.
|
||||
// A failed cleanup marker must not hide the screenshot.
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-5
@@ -12,8 +12,7 @@ export type HandlerContext = {
|
||||
|
||||
export type CommandHandler = (ctx: HandlerContext) => Promise<void>
|
||||
|
||||
// Why: routing only needs key→group, so every CLI invocation can skip the
|
||||
// transitive module graph of the 24 groups it does not dispatch into.
|
||||
// Load only the selected handler group to avoid importing unrelated command dependencies.
|
||||
function buildRoutes(groups: readonly HandlerGroup[]): Map<string, HandlerGroup> {
|
||||
const table = new Map<string, HandlerGroup>()
|
||||
for (const group of groups) {
|
||||
@@ -32,7 +31,6 @@ function buildRoutes(groups: readonly HandlerGroup[]): Map<string, HandlerGroup>
|
||||
|
||||
const ROUTES = buildRoutes(HANDLER_GROUPS)
|
||||
|
||||
// Why: exposes only the canonical command keys, not the handler internals.
|
||||
export const HANDLER_COMMAND_KEYS: ReadonlySet<string> = new Set(ROUTES.keys())
|
||||
|
||||
export async function dispatch(commandPath: string[], ctx: HandlerContext): Promise<void> {
|
||||
@@ -42,8 +40,7 @@ export async function dispatch(commandPath: string[], ctx: HandlerContext): Prom
|
||||
throw new RuntimeClientError('invalid_argument', `Unknown command: ${key}`)
|
||||
}
|
||||
const handler = (await group.load())[key]
|
||||
// Why: the manifest key list is verified against the real exports in CI, so a
|
||||
// miss here means the group changed without the manifest — fail loudly.
|
||||
// Missing handlers indicate drift from the CI-verified manifest.
|
||||
if (!handler) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
|
||||
@@ -54,8 +54,6 @@ describe('parseHostFlag', () => {
|
||||
expect(() => parseHostFlag(flags({ host: true }))).toThrow('Missing value for --host')
|
||||
})
|
||||
|
||||
// A value that claims a kind but does not parse is malformed, not a name someone could have
|
||||
// meant — only an unprefixed value can be a host alias.
|
||||
it.each(['runtime:', 'ssh:', 'ssh:a|b', ' '])('rejects the unparseable host id %s', (value) => {
|
||||
expect(() => parseHostFlag(flags({ host: value }))).toThrow(`Invalid --host value: ${value}`)
|
||||
})
|
||||
@@ -70,8 +68,6 @@ describe('parseHostFlag', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: `orca host list` prints the alias, so the alias is what gets typed. Rejecting it sent
|
||||
// people hunting for a machine-generated id the listing showed in a different column.
|
||||
it('carries an unprefixed host name through as an alias', () => {
|
||||
expect(parseHostFlag(flags({ host: 'trader-local' }))).toEqual({
|
||||
kind: 'alias',
|
||||
@@ -111,8 +107,6 @@ describe('resolveHostFlagEnvironmentId', () => {
|
||||
).rejects.toThrow('no paired Orca server is named or has id env-missing')
|
||||
})
|
||||
|
||||
// Why: the name is what a person or agent actually knows; requiring the raw uuid made the
|
||||
// obvious spelling fail against a server that is sitting right there.
|
||||
it('accepts the environment name as well as its id', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')])
|
||||
|
||||
@@ -213,8 +207,6 @@ describe('resolveHostFlagEnvironmentId', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: the reported failure. `orca host list` shows `trader-local`, so `--host trader-local` is
|
||||
// what gets typed, and only the machine-generated `ssh:ssh-1783599077486-fyasux` was accepted.
|
||||
describe('unprefixed --host aliases', () => {
|
||||
function clientWithTargets(targets: { id: string; label: string }[]): RuntimeClient {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: SSH target resolution reads only client.call.
|
||||
@@ -232,7 +224,6 @@ describe('unprefixed --host aliases', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// The SSH axis lives on the connected runtime, so a miss here is not yet a failure.
|
||||
it('leaves an alias that names no paired server to the SSH axis', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')])
|
||||
|
||||
@@ -260,8 +251,6 @@ describe('unprefixed --host aliases', () => {
|
||||
).rejects.toThrow('Unknown host in --host trader-local')
|
||||
})
|
||||
|
||||
// Why: a paired server and an SSH target are different machines. Picking either would run the
|
||||
// command somewhere the caller never named.
|
||||
it('refuses an alias that names a machine on both axes', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')])
|
||||
listSshTargetsMock.mockResolvedValue([{ id: 'ssh-1-a', label: 'awin' }])
|
||||
@@ -279,9 +268,6 @@ describe('unprefixed --host aliases', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: findSshTargetByName returns undefined for a duplicated label, which reads the same as
|
||||
// "no SSH target by that name" — so this case used to look conflict-free and resolve to the
|
||||
// paired server, the one direction that still guessed.
|
||||
it('refuses an alias naming a server and two SSH targets sharing a label', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')])
|
||||
listSshTargetsMock.mockResolvedValue([
|
||||
@@ -329,7 +315,6 @@ describe('unprefixed --host aliases', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: `orca host list` prints `this machine` as the local row's name, with a space.
|
||||
it('resolves the local row name to --host local', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')])
|
||||
await expect(
|
||||
@@ -375,7 +360,7 @@ describe('unprefixed --host aliases', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Why: cmd.exe does not strip single quotes, so `--host 'gpu'` arrives with them.
|
||||
// cmd.exe passes wrapping single quotes through literally.
|
||||
it('ignores one pair of wrapping quotes around a host name', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')])
|
||||
listSshTargetsMock.mockResolvedValue([])
|
||||
@@ -385,8 +370,6 @@ describe('unprefixed --host aliases', () => {
|
||||
expect(parseHostFlag(flags({ host: '"local"' }))).toEqual({ kind: 'local', id: 'local' })
|
||||
})
|
||||
|
||||
// Why: with --environment the command runs on that server, so a bare name is a machine it
|
||||
// reaches; a locally paired server sharing the name, even the routed one, is not a host there.
|
||||
it('leaves an alias to the routed server SSH targets when --environment picks the server', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([
|
||||
environment('env-1', 'openclaw'),
|
||||
@@ -482,8 +465,7 @@ describe('resolveEnvironmentSelectorForStore', () => {
|
||||
expect(listSshTargetsMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: the store matches names exactly, so a loosely matched name has to leave here as the id;
|
||||
// an exact one stays as typed, which `orca status` shows as the target.
|
||||
// The store matches names exactly; canonicalize loose matches to IDs.
|
||||
it('hands back the id for a name typed in another case or quoted', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'Build')])
|
||||
|
||||
@@ -495,8 +477,6 @@ describe('resolveEnvironmentSelectorForStore', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// Why: the inverse of the --host case, and the one the report actually hit — a name that is
|
||||
// an SSH target dead-ended with a bare "unknown environment".
|
||||
it('names the SSH target when the selector is one', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')])
|
||||
listSshTargetsMock.mockResolvedValue([{ id: 'ssh-1-a', label: 'openclaw' }])
|
||||
|
||||
@@ -23,21 +23,16 @@ import type { RuntimeClient } from './runtime-client'
|
||||
import { RuntimeClientError } from './runtime/types'
|
||||
|
||||
export type HostFlagRoutingSelection = {
|
||||
// Why: SSH targets live in the running Orca host, not on disk, so enumerating them needs a
|
||||
// client. Injected as a thunk so the lookup only happens on the error path we are explaining.
|
||||
// Enumerating SSH targets needs the running host; defer lookup until error recovery.
|
||||
listSshTargets: () => Promise<SshTargetSummary[]>
|
||||
// Why: unlike the suggestion listing, this throws when the targets cannot be read. A bare name
|
||||
// may only route to a paired server once no SSH target is known to share it.
|
||||
// Fail on unreadable targets: routing a bare name requires ruling out SSH collisions.
|
||||
verifySshTargets: () => Promise<SshTargetSummary[]>
|
||||
pairingCode: string | null
|
||||
// Why: an ambient ORCA_ENVIRONMENT counts as a selection too, so carry the label to
|
||||
// name the real source in the conflict message.
|
||||
// Preserve the selection source, including ORCA_ENVIRONMENT, for conflict messages.
|
||||
environmentSelector: { value: string; label: string } | null
|
||||
}
|
||||
|
||||
// Why: `orca host list` prints the alias a caller actually knows (`trader-local`), so that is what
|
||||
// they type — but which machine it names needs the host listings, which only the async resolvers
|
||||
// below can read. Carry the alias unresolved rather than rejecting the obvious spelling.
|
||||
// Host labels need async catalog lookup before they can resolve to a machine.
|
||||
export type HostFlagAlias = { kind: 'alias'; alias: string }
|
||||
export type HostFlagSelection = ParsedExecutionHost | HostFlagAlias
|
||||
|
||||
@@ -59,8 +54,7 @@ export function parseHostFlag(flags: Map<string, string | boolean>): HostFlagSel
|
||||
return parsed
|
||||
}
|
||||
const alias = value.trim()
|
||||
// A value that claims a kind but does not parse is malformed, not an alias: `ssh:` names no
|
||||
// target, and a `|` would rebind the id downstream. Only an unprefixed name can be a label.
|
||||
// Prefixed values must parse as IDs, never aliases; malformed IDs could rebind downstream.
|
||||
if (!alias || alias.startsWith('ssh:') || alias.startsWith('runtime:')) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
@@ -70,10 +64,7 @@ export function parseHostFlag(flags: Map<string, string | boolean>): HostFlagSel
|
||||
return { kind: 'alias', alias }
|
||||
}
|
||||
|
||||
// Why: `runtime:<environment-id>` ids are minted by this machine's pairing store, so they
|
||||
// only name a machine relative to it. Filtering or mutating the locally connected runtime
|
||||
// with one answers for the wrong host, and an id that matches nothing is indistinguishable
|
||||
// from a real host with no rows — so resolve the id here and send the command to it.
|
||||
// Pairing IDs are local to this machine; route to the paired host before applying its filters.
|
||||
export async function resolveHostFlagEnvironmentId(
|
||||
flags: Map<string, string | boolean>,
|
||||
selection: HostFlagRoutingSelection
|
||||
@@ -93,25 +84,18 @@ export async function resolveHostFlagEnvironmentId(
|
||||
}))
|
||||
const wanted = host.kind === 'runtime' ? host.environmentId : host.alias
|
||||
const flagSpelling = host.kind === 'runtime' ? `--host ${host.id}` : `--host ${host.alias}`
|
||||
// Why: with --environment or --pairing-code the command already runs on a chosen server, so a
|
||||
// bare name is this machine or an SSH target that server reaches, never a second server.
|
||||
// An explicit server selection scopes bare aliases to that server's local/SSH hosts.
|
||||
if (host.kind === 'alias' && (selection.environmentSelector || selection.pairingCode)) {
|
||||
return null
|
||||
}
|
||||
// Why: --environment has always taken a name or an id, and the name is what people and agents
|
||||
// actually know. Requiring the raw uuid here made the obvious spelling fail; accept either and
|
||||
// canonicalize to the id so stored host ids still compare correctly downstream.
|
||||
// Accept environment names but canonicalize them to IDs for stored-host comparisons.
|
||||
const environment = findEnvironmentByName(environments, wanted)
|
||||
if (!environment) {
|
||||
assertEnvironmentNameUnambiguous(environments, wanted, flagSpelling)
|
||||
// A bare alias has axes left to try: this machine, and SSH targets on the connected runtime,
|
||||
// which resolveHostFlagTarget answers once a client exists. Missing here is not a failure.
|
||||
// A missing server alias may still name the local machine or one of its SSH targets.
|
||||
if (host.kind === 'alias') {
|
||||
return null
|
||||
}
|
||||
// Why: `runtime:<id>` is a host id that also appears in stored rows, so it resolves by id
|
||||
// only — unlike --environment, which also accepts a name. Say so, and hand back the ids an
|
||||
// agent can retry with instead of making it scrape the sentence.
|
||||
const sshTargets = await selection.listSshTargets()
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
@@ -163,9 +147,7 @@ export async function resolveHostFlagEnvironmentId(
|
||||
return environment.id
|
||||
}
|
||||
|
||||
// Why: a paired server and an SSH target are different machines, and one alias can name both.
|
||||
// Picking either would send the command to a machine the caller did not choose — the failure the
|
||||
// prefixed spellings exist to prevent — so hand back both flags and let them say which.
|
||||
// An alias naming both a paired server and an SSH target must not choose either silently.
|
||||
async function assertAliasNamesOneHost(
|
||||
alias: string,
|
||||
environment: EnvironmentSummary,
|
||||
@@ -186,9 +168,7 @@ async function assertAliasNamesOneHost(
|
||||
}
|
||||
)
|
||||
}
|
||||
// Why: findSshTargetByName collapses a duplicated label to undefined, which reads here as "no
|
||||
// SSH target by that name" — so two colliding targets would have looked like no conflict at all
|
||||
// and resolved to the server. Duplicates are more ambiguity, not less.
|
||||
// Duplicate SSH labels are ambiguous, not evidence that no target matches.
|
||||
const matched = findSshTargetByName(sshTargets, alias)
|
||||
const conflicts = matched ? [matched] : ambiguousSshTargets(sshTargets, alias)
|
||||
if (conflicts.length === 0) {
|
||||
@@ -212,9 +192,7 @@ async function assertAliasNamesOneHost(
|
||||
)
|
||||
}
|
||||
|
||||
// Why: a runtime stamps its own setups `local` when they were made on the box and
|
||||
// `runtime:<id>` when a paired client made them. Once --host routed the command to that
|
||||
// runtime both spellings mean the same machine, so a host filter has to accept both.
|
||||
// After routing, local and runtime:<id> setups both belong to the selected server.
|
||||
export function hostFilterMatchesHostId(
|
||||
filter: ParsedExecutionHost,
|
||||
candidateHostId: string | null | undefined
|
||||
@@ -226,10 +204,7 @@ export function hostFilterMatchesHostId(
|
||||
return filter.kind === 'runtime' && candidate === LOCAL_EXECUTION_HOST_ID
|
||||
}
|
||||
|
||||
// Why: `ssh:` reaches a machine the connected runtime owns, so it can only be checked against
|
||||
// that runtime — unlike `runtime:`, which is resolved from this machine's pairing store before a
|
||||
// client exists. Callers that act on a --host value run this so an unknown target fails loudly
|
||||
// instead of quietly filtering to nothing.
|
||||
// Validate SSH IDs against the connected runtime, not this machine's pairing store.
|
||||
export async function resolveHostFlagTarget(
|
||||
flags: Map<string, string | boolean>,
|
||||
client: RuntimeClient
|
||||
@@ -249,8 +224,7 @@ export async function resolveHostFlagTarget(
|
||||
if (host.kind === 'alias' && isLocalHostListName(host.alias)) {
|
||||
return resolveLocalHostListName(host.alias, client)
|
||||
}
|
||||
// A bare alias reaches here only after resolveHostFlagEnvironmentId found no paired server by
|
||||
// that name, so SSH is the one axis left and an unknown name is a real failure.
|
||||
// Server lookup already missed; SSH is the remaining alias scope.
|
||||
const wanted = host.kind === 'ssh' ? host.targetId : host.alias
|
||||
const flagSpelling = host.kind === 'ssh' ? `--host ${host.id}` : `--host ${host.alias}`
|
||||
const targetId = await resolveSshHostTargetId(
|
||||
@@ -289,9 +263,7 @@ async function resolveLocalHostListName(
|
||||
return { kind: 'local', id: LOCAL_EXECUTION_HOST_ID }
|
||||
}
|
||||
|
||||
// Why: the store refuses an ambiguous environment name rather than guessing which server was
|
||||
// meant. Resolving one here would put the guess back, in the flag whose whole purpose is to stop
|
||||
// a command reaching a machine the caller did not choose.
|
||||
// Preserve the store's refusal to guess between duplicate environment names.
|
||||
function assertEnvironmentNameUnambiguous(
|
||||
environments: readonly { id: string; name: string }[],
|
||||
name: string,
|
||||
@@ -313,11 +285,7 @@ function assertEnvironmentNameUnambiguous(
|
||||
)
|
||||
}
|
||||
|
||||
// Why: the inverse of the --host case. `--environment openclaw` failed with a bare "Unknown
|
||||
// environment", when openclaw is very often an SSH target — a different axis, not a typo. The
|
||||
// store's own error cannot carry the hint (translateStoreError forwards code and message only,
|
||||
// dropping data), so resolve the selector here where the payload survives. Returns what the store
|
||||
// resolves to the same server: the store matches names exactly, so a loose match leaves as the id.
|
||||
// Resolve here so cross-host suggestions survive store errors that discard structured data.
|
||||
export async function resolveEnvironmentSelectorForStore(
|
||||
selector: string,
|
||||
listSshTargetsForSuggestion: () => Promise<SshTargetSummary[]>
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
/** One-line flag descriptions shared by every command's help output. */
|
||||
export const FLAG_HELP_TEXT: Record<string, string> = {
|
||||
'external-worktree-visibility':
|
||||
'--external-worktree-visibility show|hide|inherit Override or inherit non-Orca worktree visibility',
|
||||
@@ -113,7 +112,6 @@ export const FLAG_HELP_TEXT: Record<string, string> = {
|
||||
'value-stdin': '--value-stdin Read set-value payload from stdin',
|
||||
'window-id': '--window-id <id> Target a window id from list-windows',
|
||||
'window-index': '--window-index <n> Target a window index from list-windows',
|
||||
// Browser automation flags
|
||||
element: '--element <ref> Element ref from snapshot (e.g. e3)',
|
||||
url: '--url <url> URL to navigate to',
|
||||
value: '--value <text> Value to fill or select',
|
||||
|
||||
+2
-10
@@ -32,11 +32,7 @@ export function getOptionalStringFlag(
|
||||
return typeof value === 'string' && value.length > 0 ? value : undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* A valued flag whose value the shell (or a missing variable) ate parses as `true`. Dropping it
|
||||
* silently mints a fresh mutation identity and can deliver a prompt twice (#15180), so every
|
||||
* valued-flag accessor refuses the damaged shape by 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) {
|
||||
throw new RuntimeClientError(
|
||||
@@ -46,11 +42,7 @@ export function rejectValuelessFlag(value: string | boolean | undefined, name: s
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A JSON-valued flag, rejected up front when a native argv boundary stripped its quotes so the
|
||||
* error names the shell instead of the user's value (#16706). The value itself is still parsed
|
||||
* downstream; this only catches the damaged shape.
|
||||
*/
|
||||
/** Detect shell-stripped JSON quotes before downstream parsing so the error names the argv boundary. */
|
||||
export function getOptionalJsonFlag(
|
||||
flags: Map<string, string | boolean>,
|
||||
name: string
|
||||
|
||||
@@ -120,7 +120,7 @@ describe('worktree selector recovery', () => {
|
||||
error: {
|
||||
code: 'selector_not_found',
|
||||
message: 'selector_not_found',
|
||||
// The mutation-recovery layer already attached its request id.
|
||||
// Preserve the request ID already attached by mutation recovery.
|
||||
data: { orchestrationRequestId: 'req_abc', nextSteps: ['Run request-show first.'] }
|
||||
},
|
||||
_meta: { runtimeId: 'runtime_local' }
|
||||
@@ -218,7 +218,7 @@ describe('CLI error recovery', () => {
|
||||
})
|
||||
|
||||
it('tells an older server to reprovision when its computer sidecar is missing', () => {
|
||||
// An older orcad answers with the generic provider steps, which never name the server install.
|
||||
// Older servers return generic steps without naming the server installation.
|
||||
const error = new RuntimeRpcFailureError({
|
||||
id: 'req_sidecar_exit',
|
||||
ok: false,
|
||||
|
||||
+1
-2
@@ -89,8 +89,7 @@ export type HostListEntry = {
|
||||
connectionStatus?: string
|
||||
}
|
||||
|
||||
// Why: the selector column is the point of this command — the name alone is what callers already
|
||||
// had, and passing it on the wrong axis is the mistake this output exists to prevent.
|
||||
// Explicit host selectors prevent callers from using a display name on the wrong host scope.
|
||||
export function formatHostList(result: { hosts: HostListEntry[] }): string {
|
||||
const kindLabel: Record<HostListEntry['kind'], string> = {
|
||||
local: 'local',
|
||||
|
||||
Reference in New Issue
Block a user