[comment audit] Shorten CLI handler comments (#32282)

* chore: shorten process-host comments

* chore: audit CLI parsing and host-routing comments

* chore: remove stale host-selector restriction comment

* chore: shorten remaining CLI root comments

* Shorten CLI handler comments

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-10-23 08:23:48 -07:00
committed by GitHub
co-authored by OrcaWin
parent 846851237c
commit 6d1aaa4e26
90 changed files with 184 additions and 545 deletions
-3
View File
@@ -1,7 +1,5 @@
import type { ManagedDataAccountsState } from '../../shared/managed-account-types'
// Why: Claude and Codex managed-account summaries both carry id+email+active id,
// so one formatter renders either provider's block.
type AccountsBlock = {
accounts: readonly { id: string; email: string; needsSignIn?: true }[]
activeAccountId: string | null
@@ -24,7 +22,6 @@ export function formatDataAccounts(label: string, state: ManagedDataAccountsStat
.join('\n')}`
}
/** Renders a provider's managed-account list as a human-readable block, marking the active account. */
export function formatAccountsBlock(label: string, block: AccountsBlock): string {
if (block.accounts.length === 0) {
return `No managed ${label} accounts.`
+8 -27
View File
@@ -25,8 +25,7 @@ const {
writeKeychainMock: vi.fn()
}))
// Why: keep real temp-dir cleanup by default so leak assertions stay honest,
// while allowing deterministic Windows EBUSY coverage.
// Use real cleanup by default so leaks remain visible; override it only for Windows EBUSY cases.
vi.mock('node:fs', async (importOriginal) => {
const actual = await importOriginal<typeof NodeFs>()
rmSyncMock.mockImplementation(actual.rmSync)
@@ -43,8 +42,7 @@ vi.mock('../../main/claude-accounts/keychain', () => ({
readActiveClaudeKeychainCredentialsStrict: readKeychainMock,
writeActiveClaudeKeychainCredentials: writeKeychainMock
}))
// Why importOriginal: withCliRuntimeOnPath is a pure filesystem-probing helper,
// and the PATH assertions below are only meaningful against the real one.
// PATH assertions must exercise the real withCliRuntimeOnPath filesystem probes.
vi.mock('../../shared/node-cli-command-resolution', async (importOriginal) => ({
...(await importOriginal<typeof NodeCliCommandResolutionModule>()),
getVersionManagerBinPaths: getVersionManagerBinPathsMock,
@@ -73,9 +71,7 @@ function successfulChild(): EventEmitter {
return child
}
// Why: identify the handler under test by set difference, not by position —
// `.at(-1)` picks up any listener a later registration appends (vitest installs
// its own once-wrapped SIGINT teardown), which made assertions flake.
// Diff listener sets: Vitest may append its own wrapped SIGINT listener after ours.
function newSignalListener(
signal: NodeJS.Signals,
before: readonly unknown[]
@@ -146,8 +142,7 @@ describe('account CLI handlers', () => {
)
logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
process.env.ELECTRON_RUN_AS_NODE = '1'
// Why: a test run inside a real distro carries WSL_DISTRO_NAME, which would
// leak WSL attribution into every faked-win32 add below.
// Clear real WSL attribution before simulating win32.
delete process.env.WSL_DISTRO_NAME
delete process.env.ORCA_CLI_WSL_DISTRO
})
@@ -389,8 +384,7 @@ describe('account CLI handlers', () => {
it.each(['linux', 'darwin'])(
'keeps host attribution when the CLI runs on %s',
async (platform) => {
// Why: a Linux CLI talks to a Linux runtime whose accounts are host-lane;
// WSL_DISTRO_NAME there names the CLI's own environment, not a target lane.
// Linux runtimes use host-lane accounts even when the CLI itself runs in WSL.
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
process.env.WSL_DISTRO_NAME = 'Ubuntu-22.04'
process.env.ORCA_CLI_WSL_DISTRO = 'Ubuntu-22.04'
@@ -403,7 +397,7 @@ describe('account CLI handlers', () => {
)
it('waits for physical child close before removing interrupted login credentials', async () => {
// Why: deleting first lets the still-live login recreate credentials afterward.
// A live login can recreate credentials after cleanup; wait for close before deleting them.
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
const kill = vi.fn()
const child = Object.assign(new EventEmitter(), { kill })
@@ -436,8 +430,7 @@ describe('account CLI handlers', () => {
})
it('cleans up when an SSH hangup ends the login', async () => {
// Why: this flow targets headless/SSH hosts, where a dropped connection
// delivers SIGHUP — Node's default terminates without running cleanup.
// Node's default SIGHUP exit would bypass cleanup when an SSH connection drops.
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
const child = Object.assign(new EventEmitter(), { kill: vi.fn() })
let codexHome = ''
@@ -467,8 +460,7 @@ describe('account CLI handlers', () => {
})
it('warns that the account may already be registered when interrupted mid-RPC', async () => {
// Why: the runtime finishes the add independently of this process, so an
// interrupt after sign-in cannot honestly be reported as "not added".
// Registration continues at the runtime after the CLI is interrupted.
const child = Object.assign(new EventEmitter(), { kill: vi.fn() })
spawnMock.mockImplementation(() => {
queueMicrotask(() => child.emit('exit', 0))
@@ -524,7 +516,6 @@ describe('account CLI handlers', () => {
})
it('fails before the login when the runtime is unreachable', async () => {
// Why: discovering a dead runtime after sign-in wastes a full OAuth round trip.
callMock.mockRejectedValue(new Error('runtime not running'))
await expect(ACCOUNT_HANDLERS['account add'](context('codex'))).rejects.toThrow(
@@ -571,8 +562,6 @@ describe('account CLI handlers', () => {
it.each(['environment', 'pairing-code'])(
'rejects --%s instead of silently ignoring it',
async (flag) => {
// Why: account commands are pinned to the local runtime, so honoring these
// silently would register the account on the wrong host.
await expect(
ACCOUNT_HANDLERS['account add']({
...context('codex'),
@@ -589,8 +578,6 @@ describe('account CLI handlers', () => {
it.each(['environment', 'pairing-code'])(
'rejects --%s on `account list` instead of listing the local host',
async (flag) => {
// Why: listing is read-only, but answering with the LOCAL machine's accounts
// when the user named a remote host is the specific wrong answer they'd act on.
await expect(
ACCOUNT_HANDLERS['account list']({
...context('claude'),
@@ -602,8 +589,6 @@ describe('account CLI handlers', () => {
)
it('keeps the original add error when cleanup also fails', async () => {
// Why: cleanup runs in a `finally`, so an unguarded rejection there replaces
// the error that actually explains why the add failed.
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
rmSyncMock.mockImplementationOnce(() => {
throw new Error('EBUSY: resource busy or locked')
@@ -648,8 +633,6 @@ describe('account CLI handlers', () => {
})
it('rejects `--agent` with no value instead of defaulting to Claude', async () => {
// Why: the parser turns a valueless flag into boolean true, so a silent
// default would run a full OAuth login for the wrong provider.
await expect(
ACCOUNT_HANDLERS['account add']({ ...context('claude'), flags: new Map([['agent', true]]) })
).rejects.toThrow(
@@ -705,8 +688,6 @@ describe('account CLI handlers', () => {
})
it('lists accounts without forcing a provider usage refresh', async () => {
// Why: the forced lane bypasses the poll throttle and costs one serial
// round-trip per managed account, and this output shows no usage numbers.
callMock.mockResolvedValue({
id: 'test',
ok: true,
+8 -32
View File
@@ -39,7 +39,6 @@ import { getWslAccountTarget } from './account-wsl-location'
import { addDataAccount, listDataAccounts, mutateDataAccount } from './data-account-commands'
import { formatAccountsBlock, formatDataAccounts } from './account-list-format'
// Why: add returns just that provider's state; list returns the full snapshot.
type AccountsListSnapshot = {
opencode?: ManagedDataAccountsState
devin?: ManagedDataAccountsState
@@ -61,11 +60,7 @@ function addAgentNodePaths(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
return env
}
/**
* Runs the real agent login attached to the user's terminal so the OAuth
* URL/device-code prompt is visible and the code can be pasted back — the desktop
* GUI flow drives this via a browser Orca can't reach on a headless host.
*/
/** Attach login to the terminal so device-code authentication works on headless hosts. */
async function runAgentLoginInTerminal(
command: string,
args: string[],
@@ -80,8 +75,6 @@ async function runAgentLoginInTerminal(
try {
;({ spawnCmd, spawnArgs } = getSpawnArgsForWindows(resolvedCommand, args))
} catch (error) {
// Why: the bare sentinel message reaches the user verbatim otherwise, with
// nothing naming the path or the characters that made it unspawnable.
rejectPromise(
error instanceof UnsafeWindowsBatchArgumentsError
? new RuntimeClientError(
@@ -94,9 +87,7 @@ async function runAgentLoginInTerminal(
)
return
}
// Why paired after the seed: addAgentNodePaths prepends the *newest* version
// manager bin, which is not necessarily where this CLI lives. Pairing last puts
// the CLI's own node in front of that seed (stablyai/orca#10932).
// Put the CLI's own Node ahead of newer version-manager bins to keep the command/runtime pair intact.
const env = withCliRuntimeOnPath(
resolvedCommand,
addAgentNodePaths({ ...stripElectronRunAsNode(process.env), ...extraEnv })
@@ -105,8 +96,7 @@ async function runAgentLoginInTerminal(
let child: ReturnType<typeof spawn>
try {
child = spawn(spawnCmd, spawnArgs, {
// Why: JSON mode reserves stdout for the response envelope while keeping
// the interactive login attached to the user's terminal via stderr.
// Reserve stdout for JSON; keep interactive login on the terminal via stderr.
stdio: consoleStdio.stdio,
env
})
@@ -140,7 +130,6 @@ async function runAgentLoginInTerminal(
})
}
/** Signs in to a new account folder the host created, then registers it (superset AddAccountDialog). */
async function addClaudeAccount({ client, cwd, json }: HandlerContext): Promise<void> {
const { result: signIn } = await client.call<ClaudeAccountSignIn>(
'accounts.beginClaudeSignIn',
@@ -154,8 +143,7 @@ async function addClaudeAccount({ client, cwd, json }: HandlerContext): Promise<
}
const result = await withInteractiveLoginCleanup(
session,
// Why always: the host keeps a folder that became an account, and deletes an abandoned one.
// Never thrown: an older running Orca lacks the method, and the add itself already finished.
// Cancel cleans abandoned folders and preserves registered accounts; older hosts may lack the method.
async () => {
const { accountId, runtime, wslDistro } = signIn
await client
@@ -192,7 +180,6 @@ async function addClaudeAccount({ client, cwd, json }: HandlerContext): Promise<
printResult(result, json, (state) => formatAccountsBlock('Claude', state))
}
/** Logs into a Codex account in a temp CODEX_HOME, then registers it with the local runtime. */
async function addCodexAccount({ client, cwd, json }: HandlerContext): Promise<void> {
const codexHome = mkdtempSync(join(tmpdir(), 'orca-account-add-codex-'))
const session: InteractiveLoginSession = {
@@ -206,8 +193,7 @@ async function addCodexAccount({ client, cwd, json }: HandlerContext): Promise<v
rmSync(codexHome, { recursive: true, force: true })
},
async () => {
// Why: plain OAuth binds a loopback callback the user's browser cannot reach
// on a headless/SSH host; device auth is explicitly designed for this flow.
// Device auth works over SSH where the browser cannot reach the host's loopback OAuth callback.
await runAgentLoginInTerminal(
'codex',
['login', '--device-auth'],
@@ -225,13 +211,6 @@ async function addCodexAccount({ client, cwd, json }: HandlerContext): Promise<v
printResult(result, json, (state) => formatAccountsBlock('Codex', state))
}
/**
* Rejects the runtime-selector flags instead of ignoring them. shouldIgnoreRemoteSelection
* pins account commands to the local runtime, so honoring `--environment homelab`
* silently would target the laptop rather than the host the user named — the exact
* mistake this feature exists to avoid. A `--help` note does not reach someone who
* already typed the flag.
*/
function rejectAccountRemoteSelectionFlags(ctx: HandlerContext, command: string): void {
rejectRemoteSelectionFlags(
ctx.flags,
@@ -254,12 +233,10 @@ async function assertAccountImportSupported(
}
}
/** CLI handlers for managed account enrollment and listing. */
export const ACCOUNT_HANDLERS: Record<string, CommandHandler> = {
'account add': async (ctx) => {
const agentFlag = ctx.flags.get('agent')
// Why: a valueless `--agent` parses as boolean true; defaulting it to claude
// would silently run a full OAuth login for the provider the user did not ask for.
// A valueless flag is true; never silently default it to a different provider's OAuth flow.
if (agentFlag !== undefined && typeof agentFlag !== 'string') {
throw new RuntimeClientError(
'invalid_argument',
@@ -278,7 +255,7 @@ export const ACCOUNT_HANDLERS: Record<string, CommandHandler> = {
await addDataAccount(ctx, agent, runAgentLoginInTerminal)
return
}
// Why: fail on runtime version skew before burning a full OAuth round trip.
// Check runtime support before spending an OAuth round trip.
await assertAccountImportSupported(ctx, agent)
await ctx.client.call('accounts.list', { refreshUsage: false })
await (agent === 'claude' ? addClaudeAccount(ctx) : addCodexAccount(ctx))
@@ -291,8 +268,7 @@ export const ACCOUNT_HANDLERS: Record<string, CommandHandler> = {
return
}
const { client, json } = ctx
// Why: this command renders no usage numbers, so skip the forced provider
// refresh — it is one serial network round-trip per managed account.
// Listing shows no usage, so avoid a serial provider refresh for every account.
const result = await client.call<AccountsListSnapshot>('accounts.list', {
refreshUsage: false
})
@@ -42,7 +42,6 @@ let appHome: string
let realHome: string
let managedHome: string
// Why: stand in for a missing codex binary so no real app-server ever starts.
function stubMissingCodexBinary(): never {
throw Object.assign(new Error('spawn codex ENOENT'), { code: 'ENOENT' })
}
@@ -73,7 +72,7 @@ beforeEach(() => {
realHome = join(root, 'real-home')
managedHome = join(appUserData, 'codex-runtime-home', 'home')
mkdirSync(managedHome, { recursive: true })
// The app, running with its own HOME, installed the pane's home at spawn.
// Simulate an app that installed the pane home under a different HOME at spawn.
const appCommand = getManagedCommand(join(appHome, '.orca', 'agent-hooks', SCRIPT_NAME))
writeFileSync(
join(managedHome, 'hooks.json'),
@@ -102,8 +101,7 @@ afterEach(() => {
rmSync(root, { recursive: true, force: true })
})
// Why: the pane shell can run under the real HOME (login(1)) or the app's own;
// on a native host the pane step neither writes a Codex home nor asks the app to.
// Native pane preparation must leave Codex homes unchanged, whichever HOME its login shell uses.
it.each([
['the real HOME', 'real'],
["the app's HOME", 'app']
+2 -2
View File
@@ -140,7 +140,7 @@ function localSuccess<TResult>(result: TResult): RuntimeRpcSuccess<TResult> {
}
function formatAgentHookCommandResult(result: AgentHookCommandResult): string {
// Why the path: it names the home a status reports on, which the CLI's process may not share with the app's panes.
// The CLI and app panes may use different homes; identify the one this status describes.
const statusSummary = result.statuses
.map(
(status) =>
@@ -181,7 +181,7 @@ export const AGENT_HOOK_HANDLERS: Record<string, CommandHandler> = {
'agent hooks prepare-codex': async ({ client, flags }) => {
rejectRemoteHookSelection(flags)
if (!process.env.WSL_DISTRO_NAME?.trim()) {
// Why a no-op: native pane wrappers from builds up to v1.4.216 still call it; delete once no supported build's wrapper does.
// Native wrappers through v1.4.216 still call this; retain the no-op until they are unsupported.
return
}
try {
-1
View File
@@ -106,7 +106,6 @@ describe('artifact CLI handlers', () => {
json: false
})
).rejects.toThrow(/too large/)
// The capability preflight is the only permitted call; the oversized body never ships.
expect(call).not.toHaveBeenCalledWith('artifacts.share', expect.anything())
})
+1 -6
View File
@@ -78,12 +78,7 @@ async function readStdinWithinLimit(maxBytes: number): Promise<string> {
return Buffer.concat(chunks).toString('utf8')
}
/**
* Publishing is off by default, so denial is the common outcome. A tiny `settings.get` read
* answers it before we load (or pipe in) up to the full RPC byte budget the host would reject.
* Only an explicit `false` denies: a host predating the capability omits the field entirely,
* and an unreachable host stays the publish RPC's problem, not the preflight's.
*/
// Check before reading a large body; only explicit false denies, preserving older-host compatibility.
async function preflightPublishCapability(ctx: HandlerContext): Promise<void> {
let enabled: unknown
try {
@@ -13,19 +13,14 @@ function ok(result: unknown): unknown {
return { id: 'request-1', ok: true, result, _meta: { runtimeId: 'runtime-1' } }
}
/**
* Stands in for the storing authority: it answers the reads the CLI makes and
* enforces the destination exactly where persistence does — with the real
* assertions, and only when the client actually supplies one.
*/
/** Exercise the real destination assertions at write time, only when the client supplies a fence. */
function authority(options: {
/** What `ssh.listTargetSummaries` reports while the CLI captures its destination. */
registered: SshRegistration[]
/** Generation the same target carries by the time the write lands. */
generationAtWrite?: number
repoConnectionId?: string | null
targetSummaryError?: Error
/** A host predating `ssh.listTargetSummaries`, which answers only `ssh.listTargets`. */
/** Older hosts expose only ssh.listTargets. */
legacyHost?: boolean
}): { call: ReturnType<typeof vi.fn>; writes: { method: string; params: unknown }[] } {
const writes: { method: string; params: unknown }[] = []
@@ -82,10 +77,7 @@ const CREATE_FLAGS: [string, string][] = [
afterEach(() => vi.restoreAllMocks())
// The expected owner only fences the host the record is leaving. Without the
// destination the arrival is never checked, so a move onto a host that is gone —
// or gone and re-registered as a different machine under the same id — is
// accepted and only surfaces later, at dispatch, as a dead automation.
// Source-owner checks cannot protect a destination removed or re-registered before the write.
describe('CLI automation writes fence the host they land on', () => {
it('refuses a selector-moving edit onto an SSH host the authority no longer registers', async () => {
const { call, writes } = authority({ registered: [], repoConnectionId: 'box-1' })
@@ -208,8 +200,7 @@ describe('CLI automation writes fence the host they land on', () => {
})
})
// A host that predates SSH generations has no incarnation to capture, so the
// write must still land — failing closed there would break a host that is only old.
// Older hosts cannot provide generations; omit the fence to preserve compatibility.
it('omits the destination when the authority registers the target without a generation', async () => {
const { call, writes } = authority({
registered: [{ id: 'box-1', label: 'box 1' }],
@@ -244,7 +235,6 @@ describe('CLI automation writes fence the host they land on', () => {
expect(writes[0]?.params).toMatchObject({ destination: { selector: { kind: 'self' } } })
})
// Nothing moves, so there is no arrival to fence and no reason to spend the reads.
it('sends no destination when the edit names no project or workspace', async () => {
const { call, writes } = authority({ registered: [] })
vi.spyOn(console, 'log').mockImplementation(() => undefined)
@@ -1,6 +1,6 @@
import { RuntimeClientError } from '../runtime-client'
/** Returns the text unchanged; an explicit empty value clears saved extras on edit. */
/** An explicit empty value clears saved extras. */
export function getExtraAgentArgsFlag(flags: Map<string, string | boolean>): string | undefined {
if (!flags.has('extra-agent-args')) {
return undefined
@@ -18,8 +18,7 @@ function clientAnsweringShow(owner: unknown): { call: ReturnType<typeof vi.fn> }
afterEach(() => vi.restoreAllMocks())
// Why: the fence refuses an unfenced mutation on a generation-bearing SSH record, so
// every CLI mutation has to read the owner first or it cannot touch its own host.
// Generation-bearing SSH records reject mutations that omit the owner precondition.
describe('CLI automation mutations carry the owner they read', () => {
it.each([
['automations remove', 'automation.delete'],
@@ -60,8 +59,7 @@ describe('CLI automation mutations carry the owner they read', () => {
)
})
// An older host has no fence to satisfy, so a missing owner must not become a
// fabricated one — and must not stop the mutation either.
// Older hosts supply no owner and require no precondition.
it('omits the precondition when the host reports no owner', async () => {
const { call } = clientAnsweringShow(undefined)
vi.spyOn(console, 'log').mockImplementation(() => undefined)
+3 -9
View File
@@ -137,13 +137,7 @@ async function getExplicitTarget(
return { repo, workspace }
}
/**
* Reads the record's current owner so the mutation that follows can name it.
*
* The CLI cannot project an owner itself — it has no SSH target registry — so it
* asks the authority that stores the record. A host too old to answer sends
* nothing, and that host has no fence to satisfy either.
*/
// Read the authoritative owner for fencing; older hosts omit it and impose no precondition.
async function resolveExpectedOwner(
client: Parameters<CommandHandler>[0]['client'],
id: string
@@ -169,7 +163,7 @@ function buildAutomationRunContextFromSetup(setup: ProjectHostSetup): WorkspaceR
return runContext
}
// Why: an older runtime strips the field and would run the automation without it.
// Older runtimes silently strip extraAgentArgs and would run without the requested arguments.
async function assertExtraAgentArgsSupported(
client: RuntimeClient,
extraAgentArgs: string | undefined
@@ -256,7 +250,7 @@ export const AUTOMATION_HANDLERS: Record<string, CommandHandler> = {
} satisfies AutomationUpdateParams
await assertExtraAgentArgsSupported(client, updates.extraAgentArgs)
const expectedOwner = await resolveExpectedOwner(client, id)
// Why: expectedOwner only fences the host the record is leaving; an edit that moves it needs the arrival fenced too.
// Owner fencing covers the source host; moves must fence the destination too.
const destination = await resolveAutomationDestination(client, target)
const result = await client.call<{ automation: Automation }>('automation.update', {
id,
+1 -1
View File
@@ -45,7 +45,7 @@ export const BROWSER_COOKIE_HANDLERS: Record<string, CommandHandler> = {
},
'cookie set': async ({ flags, client, cwd, json }) => {
const name = getRequiredStringFlag(flags, 'name')
// CookieSet.value accepts '' (clearing a cookie's value is a real operation).
// An empty cookie value clears its content without removing the cookie.
const value = getRequiredStringFlagAllowingEmpty(flags, 'value')
const params: Record<string, unknown> = { name, value }
const domain = getOptionalStringFlag(flags, 'domain')
+1 -1
View File
@@ -71,7 +71,7 @@ export const BROWSER_ENV_HANDLERS: Record<string, CommandHandler> = {
},
'set credentials': async ({ flags, client, cwd, json }) => {
const user = getRequiredStringFlag(flags, 'user')
// SetCredentials.pass accepts '' (empty passwords are legal in HTTP basic auth).
// HTTP basic auth permits empty passwords.
const pass = getRequiredStringFlagAllowingEmpty(flags, 'pass')
const target = await getBrowserCommandTarget(flags, cwd, client)
const result = await client.call<unknown>('browser.setCredentials', {
+1 -4
View File
@@ -19,10 +19,7 @@ import {
import { RuntimeClientError } from '../runtime-client'
import { getBrowserCommandTarget } from '../selectors'
// Why: selector/text/url waits can legitimately take longer than a normal RPC
// round-trip, even when Orca is healthy. Give browser.wait an explicit timeout
// budget so slow waits do not get mislabeled as "Orca is not running" by the
// generic client timeout path.
// Browser waits can outlast ordinary RPCs without indicating a disconnected runtime.
const DEFAULT_BROWSER_WAIT_RPC_TIMEOUT_MS = 60_000
export const BROWSER_NAV_HANDLERS: Record<string, CommandHandler> = {
+1 -3
View File
@@ -41,9 +41,7 @@ export const BROWSER_PROFILE_HANDLERS: Record<string, CommandHandler> = {
scope
})
if (result.result.profile === null) {
// Why: registry refuses non-isolated/imported scopes; we already validated
// the scope client-side, so a null here means a server-side rejection we
// shouldn't silently report as success.
// Client validation cannot guarantee the server accepted creation; null must not read as success.
throw new RuntimeClientError(
'runtime_error',
`Failed to create browser profile (label=${label}, scope=${scope})`
+1 -1
View File
@@ -12,7 +12,7 @@ export const BROWSER_STORAGE_HANDLERS: Record<string, CommandHandler> = {
},
'storage local set': async ({ flags, client, cwd, json }) => {
const key = getRequiredStringFlag(flags, 'key')
// Why: the server accepts any string value; setItem(key, '') differs from an unset key.
// An empty value differs from an absent key.
const value = getRequiredStringFlagAllowingEmpty(flags, 'value')
const target = await getBrowserCommandTarget(flags, cwd, client)
const result = await client.call<unknown>('browser.storage.local.set', {
+2 -8
View File
@@ -49,15 +49,9 @@ export const BROWSER_TAB_HANDLERS: Record<string, CommandHandler> = {
if (index === undefined && !page) {
throw new RuntimeClientError('invalid_argument', 'Missing required --index or --page')
}
// Why: a stable browser page id is globally unique across Orca, so page-
// targeted tab switches should match the rest of the --page command model:
// global by default, with --worktree only acting as explicit validation.
// Page ids are globally unique; worktree is optional validation rather than an implicit filter.
const target = await getBrowserCommandTarget(flags, cwd, client)
// Why: --focus is an opt-in side effect. The renderer's handler is
// worktree-scoped: it surfaces the browser pane only when the user is
// already on the targeted worktree, otherwise it pre-stages silently.
// Spread conditionally so the RPC payload stays shape-identical to the
// pre-flag form when --focus is absent.
// Focus surfaces the pane only in the current worktree; otherwise the renderer pre-stages it.
const result = await client.call<BrowserTabSwitchResult>('browser.tabSwitch', {
index,
page,
@@ -20,10 +20,7 @@ function canonicalGuides(guides: readonly BundledSkillGuide[]): BundledSkillGuid
)
}
/**
* Load the embedded guide table in canonical order. Deferred because the table is
* large and unrelated CLI commands must not pay its module-load cost at startup.
*/
/** Defer the large guide table so unrelated commands avoid its startup cost. */
export async function loadCanonicalGuides(): Promise<BundledSkillGuide[]> {
const { BUNDLED_SKILL_GUIDES } = await import('../bundled-skill-guides.js')
return canonicalGuides(BUNDLED_SKILL_GUIDES)
@@ -41,8 +38,7 @@ export function requireTopic(
`Missing skill topic. Available topics: ${availableTopics}`
)
}
// Why: installed stubs may retain an old topic forever, so aliases and canonical
// names share one lookup table instead of being treated as transient CLI aliases.
// Installed stubs can retain old topics indefinitely, so aliases must remain valid lookup keys.
const guideByTopic = new Map<string, BundledSkillGuide>(
guides.flatMap((guide) => [guide.name, ...guide.aliases].map((name) => [name, guide]))
)
@@ -34,7 +34,7 @@ export type SkillMutationVerb = 'install' | 'update'
const SHARED_FOLDER_ONLY = 'universal'
// Why: earlier releases passed `--agent` straight to the skills CLI, so scripts still use its keys.
// Preserve --agent keys forwarded to the external skills CLI by earlier releases.
const SKILLS_CLI_KEY_ALIASES = skillsCliAgentAliases(SKILLS_CLI_AGENT_KEY_BY_TUI_AGENT)
/** `undefined` means detect; `[]` is `--agent universal`, the shared folder alone. */
@@ -48,7 +48,7 @@ function requestedAgents(
if (typeof requested !== 'string') {
return undefined
}
// Why one comma-separated value: `agent` is a single-value flag on other commands.
// Other commands treat --agent as a single-value flag, so accept multiple providers with commas.
const values = requested
.split(',')
.map((entry) => entry.trim())
@@ -156,8 +156,7 @@ function describePlan(plan: BundledSkillPlan): string[] {
}
async function workspaceDirectory(): Promise<string> {
// Why the real path: the planner dedupes targets by real path, so the root it is given must
// be the same spelling every time this folder is planned.
// Match the real-path spelling used to deduplicate planner targets.
return realpath(process.cwd())
}
@@ -175,8 +174,7 @@ export function createSkillMutationHandler(verb: SkillMutationVerb): CommandHand
return
}
// Why before target resolution: the answer belongs to the other machine, and agents
// detected here would be the wrong host's.
// Local agent detection would target the wrong machine for a bridged invocation.
if (process.env.ORCA_CLI_CWD) {
throw new RuntimeClientError(
'invalid_environment',
@@ -200,13 +198,12 @@ export function createSkillMutationHandler(verb: SkillMutationVerb): CommandHand
selection: {
names: skillNames,
...(flags.get('force') === true ? { force: 'all' as const } : {}),
// Why: update never adds a skill; install is the verb that does.
installedOnly: verb === 'update'
}
})
const plan = await previewBundledSkillInstall(input)
const planned = new Set(plan.skills.map((entry) => entry.name))
// Why --all is exempt: it names every bundled skill, so skipping absent ones is expected.
// --all is allowed to skip bundled skills that are not installed.
const notInstalled =
verb === 'update' && flags.get('all') !== true
? skillNames.filter((name) => !planned.has(name))
@@ -9,10 +9,7 @@ vi.mock('../runtime-client', async () => {
openOrca = vi.fn()
}
// Why: re-export the REAL error classes rather than redefining them. format.ts
// narrows with `instanceof` against ./runtime/types, so a look-alike class
// here would make every CLI error fall through to the generic `runtime_error`
// shape — mirroring the barrel keeps the mock faithful to production.
// Error formatting uses instanceof, so preserve the production error classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('../runtime/types.js')
return { RuntimeClient, RuntimeClientError, RuntimeRpcFailureError }
})
+1 -3
View File
@@ -9,9 +9,7 @@ vi.mock('../runtime-client', async () => {
openOrca = vi.fn()
}
// Why: re-export the REAL error classes; format.ts narrows with `instanceof`
// against ./runtime/types, so a look-alike would collapse every CLI error
// code into the generic `runtime_error` shape.
// Error formatting uses instanceof, so preserve the production error classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('../runtime/types.js')
return { RuntimeClient, RuntimeClientError, RuntimeRpcFailureError }
})
+4 -12
View File
@@ -2,12 +2,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }))
// The claude-teams handler spawns `claude` via node:child_process; mock it so we
// can inspect the child env without launching a real process.
vi.mock('node:child_process', () => ({ spawn: spawnMock }))
// Keep the socket runtime client out of the import graph; only the error type
// and serveOrcaApp binding are referenced by the module under test.
// Exclude the socket runtime graph; this handler needs only errors and serveOrcaApp.
vi.mock('../runtime-client', () => ({
RuntimeClientError: class RuntimeClientError extends Error {
readonly code: string
@@ -25,8 +22,7 @@ import type { RuntimeClient } from '../runtime-client'
type SpawnEnv = Record<string, string | undefined>
// Minimal child stub: the handler only awaits `exit`, so resolve it on the next
// microtask to complete the spawned-process promise deterministically.
// Emit exit on the next microtask so the spawned-process promise settles deterministically.
function mockClaudeChild(): { once: (event: string, cb: (...args: unknown[]) => void) => unknown } {
const child = {
once(event: string, cb: (...args: unknown[]) => void) {
@@ -71,8 +67,7 @@ describe('orca claude-teams CLI handler', () => {
previousRunAsNode = process.env.ELECTRON_RUN_AS_NODE
previousPaneKey = process.env.ORCA_PANE_KEY
previousExitCode = process.exitCode
// The `orca` launcher runs Orca's Electron binary as Node, so the CLI process
// itself carries ELECTRON_RUN_AS_NODE=1. Reproduce that inherited flag here.
// Reproduce the Electron-as-Node flag inherited from the orca launcher.
process.env.ELECTRON_RUN_AS_NODE = '1'
process.env.ORCA_PANE_KEY = 'tab-1:leaf-1'
})
@@ -91,8 +86,7 @@ describe('orca claude-teams CLI handler', () => {
process.exitCode = previousExitCode
})
// Guarded to non-Windows: the handler early-returns unsupported_platform on
// win32, so the leak path never runs there.
// Windows refuses the command before reaching child-process creation.
it.skipIf(isWindows)(
'does not leak ELECTRON_RUN_AS_NODE into the spawned claude child',
async () => {
@@ -102,8 +96,6 @@ describe('orca claude-teams CLI handler', () => {
const spawnEnv = spawnMock.mock.calls.at(-1)?.[2].env as SpawnEnv
expect(spawnEnv.ELECTRON_RUN_AS_NODE).toBeUndefined()
// The prepareLaunch request env is built from the same helper, so it must
// be sanitized too.
const prepareLaunchEnv = (callMock.mock.calls[0][1] as { env: SpawnEnv }).env
expect(prepareLaunchEnv.ELECTRON_RUN_AS_NODE).toBeUndefined()
}
+1 -4
View File
@@ -9,10 +9,7 @@ import { getOptionalServePort } from '../runtime/serve-port-flag'
import { getServeOptionValidationError } from '../../shared/serve-option-validation'
function envRecord(): Record<string, string> {
// Why: the `orca` launcher runs Orca's Electron binary as Node, so this CLI
// process carries ELECTRON_RUN_AS_NODE=1. Strip it before it reaches the
// spawned `claude` (and any nested Electron it launches), which would
// otherwise be forced into headless plain-Node mode.
// Inherited ELECTRON_RUN_AS_NODE would force nested Electron children into plain Node mode.
const env = stripElectronRunAsNode(process.env)
return Object.fromEntries(
Object.entries(env).filter((entry): entry is [string, string] => entry[1] !== undefined)
+1 -3
View File
@@ -18,9 +18,7 @@ vi.mock('../runtime-client', async () => {
}
}
// Why: re-export the REAL error classes; format.ts narrows with `instanceof`
// against ./runtime/types, so a look-alike would collapse every CLI error
// code into the generic `runtime_error` shape.
// Error formatting uses instanceof, so preserve the production error classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('../runtime/types.js')
return { RuntimeClient, RuntimeClientError, RuntimeRpcFailureError }
})
+1 -2
View File
@@ -128,8 +128,7 @@ export const EMULATOR_HANDLERS: Record<string, CommandHandler> = {
const target = await getEmulatorCommandTarget(flags, cwd, client)
const device = getOptionalStringFlag(flags, 'device')
const focus = flags.get('focus') === true
// Why: attach may cold-boot or recycle a wedged simulator (shutdown + boot +
// helper restart), which can legitimately exceed the 60s default budget.
// Cold boots and wedged-simulator recovery can exceed the normal RPC timeout.
const res = await client.call(
'emulator.attach',
{ device, worktree: target.worktree, focus },
+4 -16
View File
@@ -29,8 +29,7 @@ export const ENVIRONMENT_HANDLERS: Record<string, CommandHandler> = {
throw new RuntimeClientError('invalid_argument', 'Missing value for --name')
}
if (typeof requestedName === 'string') {
// Why: an older runtime rejects the unknown settings field with a bare `invalid_params`;
// a runtime that does not publish a name cannot store one either, so say so plainly.
// Check name support before an older settings schema can reject the write as invalid_params.
const current = await client.call<RuntimeStatus>('status.get')
if (current.result.machineName === undefined) {
throw new RuntimeClientError(
@@ -40,8 +39,7 @@ export const ENVIRONMENT_HANDLERS: Record<string, CommandHandler> = {
}
await client.call('settings.update', { machineName: requestedName })
}
// Why: print what the runtime publishes after the write (a blank `--name` means the detected
// name), inside the runtime's own envelope so a routed answer is stamped with that runtime.
// A blank override restores the detected name; read it from the runtime after writing.
const status = await client.call<RuntimeStatus>('status.get')
printResult({ ...status, result: describeRuntimeHost(status.result) }, json, formatHostName)
},
@@ -61,9 +59,6 @@ export const ENVIRONMENT_HANDLERS: Record<string, CommandHandler> = {
`Saved environment ${result.environment.name} (${result.environment.id}).`
)
},
// Why: an agent told "run it on <name>" had nowhere to look. `orca environment list` showed
// paired servers only, and nothing in the CLI listed SSH targets at all, so the wrong-axis
// guess was the only move available. This is the one place that answers both.
'host list': async ({ client, flags, json }) => {
rejectLocalPairingStoreRetargeting(
flags,
@@ -79,8 +74,7 @@ export const ENVIRONMENT_HANDLERS: Record<string, CommandHandler> = {
...(target.connectionStatus ? { connectionStatus: target.connectionStatus } : {}),
...(target.remotePlatform ? { platform: target.remotePlatform } : {})
}))
// Why: this listing answers from the local pairing store; dialing each paired server would make
// it slow and offline-fragile. `orca host name --environment <name>` asks one server directly.
// Read the local pairing store without dialing every server, so listing works offline.
const localStatus = await readLocalHostDescriptor(client)
const environments = listEnvironments(getDefaultUserDataPath()).map((environment) => ({
kind: 'environment' as const,
@@ -148,13 +142,7 @@ async function readLocalHostDescriptor(client: RuntimeClient): Promise<HostNameR
}
}
/**
* These two listings are pinned local by `shouldIgnoreRemoteSelection`, so a runtime selector is
* dropped for routing. It used to still reach the SSH half of `host list` through the routed
* client, producing a listing whose SSH rows came from the named server and whose paired-server
* rows came from this machine — one answer describing two hosts, stamped `runtimeId: local`.
* Failing is the only answer that is true of a single machine.
*/
// Both listings read the local pairing store; reject flags that imply a different runtime.
function rejectLocalPairingStoreRetargeting(
flags: Map<string, string | boolean>,
suffix: string,
+4 -9
View File
@@ -39,8 +39,7 @@ describe('absolute file CLI paths', () => {
vi.restoreAllMocks()
callMock.mockReset()
process.exitCode = undefined
// Why: a contributor running this suite inside WSL inherits a real
// WSL_DISTRO_NAME, which would flip the rewrite on for every case below.
// Prevent a real WSL_DISTRO_NAME from enabling the rewrite in unrelated cases.
vi.stubEnv('WSL_DISTRO_NAME', '')
vi.spyOn(console, 'log').mockImplementation(() => {})
vi.spyOn(console, 'error').mockImplementation(() => {})
@@ -142,8 +141,7 @@ describe('absolute file CLI paths', () => {
expectOpenedWith('xxx/xxx.ts')
})
// Why: backslash is a legal Linux filename character, but reads as a separator
// once the path is UNC — rewriting `a\b.ts` would open `a/b.ts` instead.
// Backslashes are filename characters on Linux but separators in UNC paths.
it('leaves a Linux path containing a backslash for the runtime guard', async () => {
vi.stubEnv('WSL_DISTRO_NAME', 'Ubuntu')
mockWorktreeShow(uncRoot)
@@ -162,8 +160,7 @@ describe('absolute file CLI paths', () => {
expectOpenedWith('/root/orca/workspaces/xxx/xxx/xxx.ts')
})
// Why: WSL_DISTRO_NAME is also set for a plain Linux CLI inside the distro, where
// roots are POSIX; prefixing there would strand every absolute path (#11393 regression).
// WSL_DISTRO_NAME also exists for Linux-native runtimes with POSIX roots.
it('keeps relativizing POSIX roots while WSL_DISTRO_NAME is set', async () => {
vi.stubEnv('WSL_DISTRO_NAME', 'Ubuntu')
mockWorktreeShow('/root/orca/workspaces/xxx')
@@ -182,7 +179,6 @@ describe('absolute file CLI paths', () => {
expectOpenedWith('/root/orca/workspaces/xxx-2/xxx.ts')
})
// Why: Windows folds the distro segment, so only its case may differ from the root.
it('matches when only the distro case differs', async () => {
vi.stubEnv('WSL_DISTRO_NAME', 'ubuntu')
mockWorktreeShow(uncRoot)
@@ -210,8 +206,7 @@ describe('absolute file CLI paths', () => {
expectOpenedWith('xxx/xxx.ts')
})
// Why: WSL_DISTRO_NAME only arrives if interop forwards it, but the launcher
// always sets ORCA_CLI_CWD, which reaches the handler as the invocation cwd.
// ORCA_CLI_CWD remains available when interop omits WSL_DISTRO_NAME.
it('falls back to the distro named by a UNC cwd', async () => {
mockWorktreeShow(uncRoot)
+1 -5
View File
@@ -13,11 +13,7 @@ function isRendererUnavailable(error: unknown): boolean {
)
}
/**
* Why: a headless server (`orca serve`, orcad) has no window to put the tab in and answers with
* the bare `renderer_unavailable` token (#29822). The wire answer stays as is — the mobile app
* keys its preview fallback on it — so only the CLI turns it into something an agent can act on.
*/
/** Keep renderer_unavailable on the wire for mobile fallback; add CLI-specific recovery here. */
export async function callFileOpen(
ctx: HandlerContext,
method: 'files.open' | 'files.openDiff',
+1 -2
View File
@@ -177,7 +177,6 @@ describe('orca file CLI handlers', () => {
process.exitCode = undefined
})
// Why: an older host still answers a PDF with opened:false (STA-9113); that must not read as success.
it('fails an unopened file open with ok:false in --json mode', async () => {
queueFixtures(
callMock,
@@ -520,7 +519,7 @@ describe('orca file CLI handlers', () => {
'/tmp/elsewhere'
)
// Why: an older host answers a binary edit open with opened:false, so focus carries to the next open.
// Declined binary opens must leave focus available for the next successful open.
expect(callMock).toHaveBeenNthCalledWith(2, 'files.open', {
worktree: 'id:wt-1',
relativePath: 'assets/logo.png',
+6 -19
View File
@@ -50,20 +50,9 @@ async function getFileWorktreeSelector({ flags, cwd, client }: HandlerContext):
return await resolveCurrentWorktreeSelector(cwd, client)
}
/**
* Why: a WSL workspace stores its worktree root as a Windows UNC path while the
* user types the Linux path they see inside the distro, so the two never match
* (#11393). Only the distro the caller is sitting in can name that Linux path, and
* only a UNC root wants the rewrite — a POSIX root already matches, so rewriting
* it would strand every absolute path.
*
* Backslash is a legal Linux filename character but a separator once the path
* reads as UNC, so `a\b.ts` would relativize to a different file, `a/b.ts`.
* Such a path has no Windows spelling; leave it to fail the match instead.
*/
// Only UNC roots need Linux-path translation; backslash filenames have no equivalent Windows spelling.
function toWorktreeRootPathFlavor(rootPath: string, cwd: string, path: string): string {
// Why: WSL_DISTRO_NAME reaches this process only if interop forwards it, but the
// WSL launcher always sets ORCA_CLI_CWD, and its UNC form names the distro itself.
// Interop may omit WSL_DISTRO_NAME; the launcher's UNC cwd also identifies the caller's distro.
const distro = process.env.WSL_DISTRO_NAME || parseWslUncPath(cwd)?.distro
if (
!distro ||
@@ -85,7 +74,7 @@ async function resolveFilePath(
if (!isRuntimePathAbsolute(path)) {
return path
}
// Why: only in-worktree absolute paths should be relativized here; outside paths must reach the runtime guard unchanged.
// Relativize only paths inside the worktree; leave others for the runtime guard.
const result = await ctx.client.call<{ worktree: RuntimeWorktreeRecord }>('worktree.show', {
worktree
})
@@ -119,9 +108,7 @@ function getOpenChangedMode(flags: Map<string, string | boolean>): OpenChangedMo
throw new RuntimeClientError('invalid_argument', 'Invalid --mode. Use edit, diff, or both.')
}
// Why: the CLI has no view of its own, so 'caller' moves nothing. --focus sends 'all', but unlike
// worktree create --activate a file open moves only the host's own window (intended; paired
// clients are not navigated). Hosts treat a missing field as the legacy switch.
// caller leaves views unchanged; --focus moves only the host window, preserving paired viewers.
function getFileOpenNavigation(flags: Map<string, string | boolean>): RuntimeNavigationTarget {
return flags.get('focus') === true ? 'all' : 'caller'
}
@@ -193,7 +180,7 @@ function formatOpenChangedResult(result: FileOpenChangedResult): string {
return lines.join('\n')
}
// Why: opened:false (older hosts decline binaries) must fail a single-file command, not exit 0.
// A declined single-file open must exit unsuccessfully even when the RPC succeeded.
function requireOpened(result: RuntimeFileOpenResult, target: string): void {
if (!result.opened) {
throw new RuntimeClientError(
@@ -248,7 +235,7 @@ export const FILE_HANDLERS: Record<string, CommandHandler> = {
const opened: FileOpenRecord[] = []
const skipped: FileOpenRecord[] = []
const openedEditPaths = new Set<string>()
// Why: switch once — the first tab that actually opens carries --focus; later ones join the now-viewed worktree.
// Apply focus to the first successful open only.
let navigation = getFileOpenNavigation(ctx.flags)
const openWithFocusOnce = async (
open: () => Promise<FileOpenRecord>
@@ -61,10 +61,7 @@ export function terminateWindowsLoginProcessTree(
})
}
/**
* Stops an interrupted interactive login before its credential directory is removed.
* The final close wait is bounded because a broken child handle must not hang Ctrl-C.
*/
/** Wait for close before credential cleanup, bounded so a broken child cannot hang Ctrl-C. */
export async function terminateInteractiveLoginProcess(
child: ChildProcess,
signal: NodeJS.Signals,
@@ -112,9 +109,6 @@ export async function terminateInteractiveLoginProcess(
}
}
/**
* Runs an account add with signal-safe cleanup and primary-error precedence.
*/
export async function withInteractiveLoginCleanup<T>(
session: InteractiveLoginSession,
cleanup: () => Promise<void>,
@@ -147,7 +141,7 @@ export async function withInteractiveLoginCleanup<T>(
.catch(reportCleanupFailure)
.finally(() => process.exit(INTERRUPT_EXIT_CODES[signal] ?? 1))
}
// Why: repeated signals must keep awaiting the same cleanup instead of restoring Node defaults.
// Repeated signals must await the same cleanup instead of restoring Node's termination default.
for (const signal of INTERRUPT_SIGNALS) {
process.on(signal, onSignal)
}
@@ -172,7 +166,6 @@ export async function withInteractiveLoginCleanup<T>(
}
if (addFailed) {
if (cleanupError !== undefined) {
// Why: retain the error that explains the failed account add.
reportCleanupFailure(cleanupError)
}
throw addError
+1 -2
View File
@@ -3,8 +3,7 @@ import { COMMAND_SPECS } from '../specs'
import { buildAgentContext, formatAgentContextSummary } from '../agent-context'
export const INTROSPECTION_HANDLERS: Record<string, CommandHandler> = {
// Why: pure local command — reads the static spec registry and prints it, with
// no runtime RPC, so it works when the Orca app is not running (SSH/headless).
// Keep schema output runtime-free so it works offline and over SSH.
'agent-context': async ({ json }) => {
const schema = buildAgentContext(COMMAND_SPECS)
if (json) {
-1
View File
@@ -31,7 +31,6 @@ async function clearAssigneeAfter(failure: Error): Promise<unknown> {
throw new Error('expected the write to fail')
}
// Pins which write failures say "may have been applied" (round-2 S5), so a dropped code is deliberate.
describe('Jira write outcome-unknown codes', () => {
it.each([
'runtime_timeout',
+2 -6
View File
@@ -19,9 +19,7 @@ vi.mock('../runtime-client', async () => {
}
}
// Why: re-export the REAL error classes; format.ts narrows with `instanceof`
// against ./runtime/types, so a look-alike would collapse every CLI error
// code into the generic `runtime_error` shape.
// Error formatting uses instanceof, so preserve the production error classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('../runtime/types.js')
return {
@@ -41,9 +39,7 @@ describe('orca linear CLI handlers', () => {
vi.restoreAllMocks()
callMock.mockReset()
process.env = { ...originalEnv }
// Why: these tests can run inside an Orca-managed terminal, which exports
// real worktree/terminal/pairing env hints; clear them so handler context
// assertions stay deterministic.
// Clear real terminal/worktree/pairing hints inherited from the test runner.
delete process.env.ORCA_WORKTREE_ID
delete process.env.ORCA_TERMINAL_HANDLE
delete process.env.ORCA_PAIRING_CODE
+1 -5
View File
@@ -1,8 +1,4 @@
/**
* An update or rollback restarts the managed server, and a CLI talking to that runtime then sees
* its connection close mid-call. That is expected, not a failure: read the host's status once the
* runtime answers again and report what the action left behind.
*/
// Updates can close this connection mid-call; report the outcome from status after the runtime returns.
import type { OrcadManagedRuntimeStatus } from '../../shared/orcad-managed-runtime'
import type { HandlerContext } from '../dispatch'
import { printResult } from '../format'
+1 -2
View File
@@ -97,7 +97,7 @@ describe('managed server CLI verbs', () => {
const log = vi.spyOn(console, 'log').mockImplementation(() => undefined)
const confirmed = client(forgotten)
await run('environment forget', confirmed, [['yes', true]])
// Never managedServer.stop: an older desktop would drop a flag and decommission instead.
// Older desktops ignore flags; using stop for forget would decommission the server.
expect(confirmed).toHaveBeenCalledWith(
'managedServer.forget',
{ selector: 'build-box' },
@@ -193,7 +193,6 @@ describe('managed server CLI verbs', () => {
})
})
// BUG-21: an update restarts the server, so the call's connection closes before it answers.
it('reports what a restarting update left behind instead of failing on the closed connection', async () => {
vi.useFakeTimers()
const log = vi.spyOn(console, 'log').mockImplementation(() => {})
+3 -12
View File
@@ -1,8 +1,3 @@
/**
* `orca environment status|update|rollback|recover|stop|forget|cancel-stop`: the Managed servers
* actions over runtime RPC. Each call is gated on the runtime's managedServer.v1 capability, and
* an older runtime's method_not_found reads the same as a missing capability.
*/
import type {
OrcadManagedCancelStopResult,
OrcadManagedDeployResult,
@@ -25,8 +20,7 @@ import { reportAfterClosedConnection } from './managed-server-reconnect'
const UNSUPPORTED_MESSAGE =
'This Orca runtime cannot manage servers over SSH. Run this on the computer whose Orca desktop app deployed the server, after updating Orca there.'
// Why 20 minutes: the desktop runs the whole action inline, and a Windows runtime promotion (5 min)
// plus a readiness wait (5 min) on a slow host already outlast the 60 s RPC default.
// Windows promotion and readiness waits can take ten minutes; allow headroom on slow hosts.
export const MANAGED_SERVER_ACTION_TIMEOUT_MS = 20 * 60_000
const READ_ONLY_METHODS = new Set(['managedServer.status'])
@@ -54,7 +48,7 @@ async function callManagedServer<TResult>(
if (error instanceof RuntimeClientError && error.code === 'method_not_found') {
throw unsupported()
}
// Why not a failure: the desktop keeps running the action after this client stops waiting.
// The runtime continues the action after this client times out.
if (!readOnly && error instanceof RuntimeClientError && error.code === 'runtime_timeout') {
throw new RuntimeClientError(
'managed_server_in_progress',
@@ -88,10 +82,7 @@ function selectorOf(context: HandlerContext): { selector: string } {
type Outcome = { outcome: string; code?: string; reason?: string }
/**
* Prints a result whose outcome is in `settled`; throws any other so scripts see a non-zero exit.
* Why an allow-list: an outcome a newer desktop adds must fail loudly, not print `undefined`.
*/
/** Reject unknown outcomes so newer-server responses cannot silently print undefined. */
function report<TResult extends Outcome, TSettled extends TResult['outcome']>(
response: RuntimeRpcSuccess<TResult>,
json: boolean,
@@ -1,18 +1,9 @@
/**
* How the orchestration CLI decides WHO is speaking.
*
* Split out of `orchestration.test.ts`, which sat exactly on the test-file line ceiling: these two
* suites are one subject — the coordinator and task-creator identity a command carries — and both
* exercise the env-handle validation and pane-remint chain rather than flag-to-param mapping.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
const getTerminalHandleMock = vi.hoisted(() => vi.fn())
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalPaneKey = process.env.ORCA_PANE_KEY
// Why: isolate the handler's flag-to-param mapping; printResult only writes output.
vi.mock('../format', () => ({ printResult: vi.fn() }))
vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock }))
@@ -24,8 +15,6 @@ function staleHandleError(): RuntimeClientError {
return new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
}
// Queues the stale-handle remint chain shared by coordinator commands:
// `terminal.resolveIdentity` answers not-live → resolvePane returns liveHandle → downstream RPC.
function stubStaleHandleRemint(liveHandle: string, downstream: unknown): void {
callMock
.mockResolvedValueOnce(notLiveIdentity())
@@ -33,12 +22,10 @@ function stubStaleHandleRemint(liveHandle: string, downstream: unknown): void {
.mockResolvedValueOnce(downstream)
}
// Queues a not-live identity followed by a resolvePane remint that fails with `error`.
function stubStaleHandleRemintFailure(error: RuntimeClientError): void {
callMock.mockResolvedValueOnce(notLiveIdentity()).mockRejectedValueOnce(error)
}
/** What the runtime answers for a handle whose leaf check reports `terminal_handle_stale`. */
function notLiveIdentity(): { result: { identity: { live: false } } } {
return { result: { identity: { live: false } } }
}
@@ -6,7 +6,7 @@ const { callMock, getTerminalHandleMock } = vi.hoisted(() => ({
}))
vi.mock('../runtime-client', async () => {
// Why: re-export the REAL error classes so format.ts `instanceof` narrowing still matches.
// Error formatting uses instanceof, so preserve the production error classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('../runtime/types.js')
class RuntimeClient {
readonly isRemote = false
@@ -34,8 +34,7 @@ import { okFixture, queueFixtures } from '../test-fixtures'
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalPaneKey = process.env.ORCA_PANE_KEY
// Why: a structured-session marker inherited from the runner diverts these cases to the
// structured refusal, so which branch they exercise would depend on who ran them.
// Isolate inherited session markers so the runner cannot change the refusal branch.
const originalStructuredSession = process.env.ORCA_STRUCTURED_SESSION
const originalCodexThreadId = process.env.CODEX_THREAD_ID
@@ -167,7 +166,6 @@ describe('orchestration gate commands carry caller identity', () => {
})
it('inspects a named Run without resolving a caller terminal', async () => {
// Why: read-only inspection must stay reachable from a pane with no bound Run.
getTerminalHandleMock.mockRejectedValue(
new RuntimeClientError('no_active_terminal', 'no active terminal')
)
@@ -1,7 +1,7 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { ORCHESTRATION_HANDLERS } from './orchestration'
// worker_done carries a CLI-minted request id so its runtime_unavailable retries replay one mutation.
// Reuse one mutation identity when retrying worker_done after runtime_unavailable.
const WORKER_DONE_REQUEST = { orchestrationRequestId: expect.any(String) }
const originalPaneKey = process.env.ORCA_PANE_KEY
@@ -1,5 +1,3 @@
// `request-show` is the recovery path a lost mutation response sends you down, so it must
// stay read-only, render the honest reading, and name a version gap instead of a raw RPC error.
import { beforeEach, describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
@@ -4,7 +4,6 @@ const callMock = vi.hoisted(() => vi.fn())
const getTerminalHandleMock = vi.hoisted(() => vi.fn())
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
// Why: isolate flag-to-RPC mapping; printResult only writes output.
vi.mock('../format', () => ({ printResult: vi.fn() }))
vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock }))
@@ -2,7 +2,6 @@ import { describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
// Why: isolate the handler's flag-to-param mapping; printResult only writes output.
vi.mock('../format', () => ({ printResult: vi.fn() }))
import { ORCHESTRATION_HANDLERS } from './orchestration'
@@ -26,7 +25,7 @@ describe('orchestration task-list brief output', () => {
it('requests server-side brief and falls back client-side for older runtimes', async () => {
callMock.mockReset().mockResolvedValue({
result: {
// No spec_truncated field — the pre-brief-runtime signature.
// Older runtimes omit spec_truncated.
tasks: [{ id: 'task_1', spec: `First line\n${'detail '.repeat(40)}`, status: 'ready' }],
count: 1
}
@@ -50,8 +49,7 @@ describe('orchestration task-list brief output', () => {
const response = await runTaskListBrief()
// Why: re-abbreviating a server-truncated spec would flip spec_truncated
// back to false (the truncated text fits the cap).
// Re-abbreviating would reset spec_truncated because the already-truncated text fits the cap.
expect(response.result.tasks).toBe(serverTasks)
})
})
@@ -572,7 +572,6 @@ describe('orchestration worker-start CLI contract', () => {
} as never)
).rejects.toMatchObject({ code: 'incompatible_runtime' })
// The extra call is the bound-Run lookup; the enumeration itself must not be retried.
expect(
callMock.mock.calls.filter(([method]) => method === 'orchestration.workerList')
).toHaveLength(1)
@@ -1,4 +1,3 @@
// `worker-show` text output must keep "nobody is waiting" apart from "nobody looked".
import { beforeEach, describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
@@ -58,8 +57,6 @@ describe('orchestration worker-show interactive wait output', () => {
})
it('says unknown when the field is absent, not none', async () => {
// An older host, or a worker whose identity could not be verified. Printing the same
// thing as an evaluated `null` is the false negative this field exists to remove.
const line = await showWorker({ status: 'identity_changed', exactWorker: false })
expect(line).toContain('Interactive wait: unknown (not evaluated)')
+6 -14
View File
@@ -1,13 +1,12 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
// worker_done carries a CLI-minted request id so its runtime_unavailable retries replay one mutation.
// Reuse one mutation identity when retrying worker_done after runtime_unavailable.
const WORKER_DONE_REQUEST = { orchestrationRequestId: expect.any(String) }
const getTerminalHandleMock = vi.hoisted(() => vi.fn())
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalPaneKey = process.env.ORCA_PANE_KEY
// Why: a structured-session marker inherited from the runner diverts these cases to the
// structured refusal, so which branch they exercise would depend on who ran them.
// Isolate inherited session markers so the runner cannot change the refusal branch.
const originalStructuredSession = process.env.ORCA_STRUCTURED_SESSION
const originalCliCommand = process.env.ORCA_CLI_COMMAND
const originalExitCode = process.exitCode
@@ -15,7 +14,6 @@ function lifecycleGroupRecipientError(type: 'worker_done' | 'heartbeat'): string
return `${type} messages belong to one exact Dispatch and cannot target a group address.`
}
// Why: isolate the handler's flag-to-param mapping; printResult only writes output.
vi.mock('../format', () => ({ printResult: vi.fn() }))
vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock }))
@@ -274,10 +272,7 @@ describe('orchestration send structured payload flags', () => {
])
)
// Why: pre-payload-authority runtimes only complete a worker_done whose
// sender equals the recorded (equally stale) assignee handle, and
// coordinator replies route to the sender row the worker's env-handle
// `check` actually reads — so lifecycle sends must stay env-verbatim.
// Older hosts match lifecycle senders to stale assignee handles, so preserve the env spelling.
expect(callMock).toHaveBeenCalledTimes(1)
expect(callMock).toHaveBeenCalledWith(
'orchestration.send',
@@ -332,8 +327,7 @@ describe('orchestration send structured payload flags', () => {
process.env.ORCA_STRUCTURED_SESSION = '1'
getTerminalHandleMock.mockResolvedValue('term_sibling_pane')
// The refusal must not recommend --from: the explicit-flag branch returns before this guard,
// so the advice would succeed against a handle that necessarily belongs to another pane.
// --from bypasses this guard and could select another pane; never recommend it as recovery.
await expect(
invokeSend(
new Map<string, string | boolean>([
@@ -440,8 +434,7 @@ describe('orchestration timeout flag validation', () => {
])
)
// Why: --peek rides with unread:false so pre-peek runtimes fall back to
// the non-consuming all mode instead of the destructive mark-read default.
// unread:false keeps --peek non-consuming when older runtimes ignore the peek field.
expect(callMock).toHaveBeenCalledWith('orchestration.check', {
terminal: 'term_worker',
terminalPaneKey: undefined,
@@ -479,8 +472,7 @@ describe('orchestration timeout flag validation', () => {
}
expect(response.result.messages.map((m) => m.id)).toEqual(['msg_new'])
expect(response.result.count).toBe(1)
// Why: the pre-peek runtime built `formatted` from all rows, including
// the read one the filter just removed.
// Older formatted output includes read rows removed by the client filter.
expect(response.result.formatted).toBeUndefined()
})
@@ -1,6 +1,6 @@
const DEFAULT_KEEPALIVE_INTERVAL_MS = 15_000
// Why: test-only escape hatch so subprocess tests avoid the full 15 s window; bogus values fall back to the default.
// Tests override the interval to avoid the full 15-second wait.
function resolveKeepaliveIntervalMs(): number {
const raw = process.env.ORCA_KEEPALIVE_INTERVAL_MS ?? process.env.ORCA_HEARTBEAT_INTERVAL_MS
if (!raw) {
@@ -18,7 +18,7 @@ export function startCheckKeepalive(deadlineMs: number | undefined): () => void
const interval = setInterval(() => {
const payload = {
_keepalive: true,
// Why: retain the old marker for scripts still filtering it while callers migrate to _keepalive.
// Preserve the old marker for scripts migrating to _keepalive.
_heartbeat: true,
elapsedMs: Date.now() - startedAt,
deadlineMs: deadlineMs ?? null
@@ -13,7 +13,6 @@ export const ORCHESTRATION_DISPATCH_HANDLER: Record<string, CommandHandler> = {
const from = await resolveCoordinatorTerminalHandle(flags, cwd, client)
const dryRun = flags.has('dry-run') ? true : undefined
const returnPreamble = flags.has('return-preamble') ? true : undefined
// Why: --to is only required for non-dry-run; the RPC handler re-enforces.
const to = dryRun ? getOptionalStringFlag(flags, 'to') : getRequiredStringFlag(flags, 'to')
const result = await callOrchestrationMutation<{
dispatch: { id: string; task_id: string; status: string } | null
@@ -43,8 +42,7 @@ export const ORCHESTRATION_DISPATCH_HANDLER: Record<string, CommandHandler> = {
export const ORCHESTRATION_DISPATCH_INSPECTION_HANDLERS: Record<string, CommandHandler> = {
'orchestration dispatch-show': async ({ flags, client, cwd, json }) => {
const showPreamble = flags.has('preamble') ? true : undefined
// Why: a preview must embed the same real coordinator handle as an actual dispatch. Its --from
// only fills preview text and names no caller, so it passes through unfenced.
// --from fills preview text rather than asserting caller identity, so it is intentionally unfenced.
const from = showPreamble
? (getOptionalStringFlag(flags, 'from') ??
injectedSessionAddress() ??
@@ -12,7 +12,6 @@ export const ORCHESTRATION_GATE_HANDLERS: Record<string, CommandHandler> = {
task: getRequiredStringFlag(flags, 'task'),
question: getRequiredStringFlag(flags, 'question'),
options: getOptionalJsonFlag(flags, 'options'),
// Why: gates are Run-scoped, so the coordinator handle is the authorized caller identity.
from: await resolveCoordinatorTerminalHandle(flags, cwd, client)
})
printResult(
@@ -36,7 +35,7 @@ export const ORCHESTRATION_GATE_HANDLERS: Record<string, CommandHandler> = {
'orchestration gate-list': async ({ flags, client, cwd, json }) => {
const run = getOptionalStringFlag(flags, 'run')
// Why: named runs remain inspectable without a pane; only implicit runs resolve identity.
// Named runs can be inspected without resolving a pane identity.
const from = run
? runScopedSessionCaller(flags)
: await resolveCoordinatorTerminalHandle(flags, cwd, client)
@@ -31,7 +31,7 @@ export const ORCHESTRATION_CHECK_HANDLER: Record<string, CommandHandler> = {
'orchestration check': async ({ flags, client, cwd, json }) => {
const wait = flags.has('wait')
const peek = flags.has('peek')
// Why: older runtimes strip unknown peek and run --unread --peek as destructive mark-read.
// Older runtimes ignore peek, making --unread --peek destructive.
if ([flags.has('unread'), peek, flags.has('all')].filter(Boolean).length > 1) {
throw new RuntimeClientError(
'invalid_argument',
@@ -41,7 +41,7 @@ export const ORCHESTRATION_CHECK_HANDLER: Record<string, CommandHandler> = {
const timeoutMs = getOptionalPositiveIntegerValueFlag(flags, 'timeout-ms')
const explicitTerminal = getOptionalStringFlag(flags, 'terminal')
const terminal = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'terminal')
// Why: a session names itself by its id alone; a pane key it inherited is not its identity.
// Sessions use their injected id; inherited pane keys belong to a different caller.
const paneKey =
explicitTerminal || terminal === undefined ? undefined : process.env.ORCA_PANE_KEY
const callerLabel = orchestrationCallerLabel(terminal)
@@ -51,7 +51,7 @@ export const ORCHESTRATION_CHECK_HANDLER: Record<string, CommandHandler> = {
result = await callOrchestrationMutation<CheckResult>(client, flags, 'orchestration.check', {
terminal,
terminalPaneKey: paneKey || undefined,
// Why: old runtimes degrade peek to non-consuming all mode instead of destructive mark-read.
// unread:false makes hosts that ignore peek fall back to non-consuming all mode.
unread: flags.has('unread') ? true : peek ? false : undefined,
peek: peek ? true : undefined,
all: flags.has('all') ? true : undefined,
@@ -99,7 +99,7 @@ function filterLegacyPeekResult(
const rawRowCount = result.result.messages.length
const unreadOnly = result.result.messages.filter((message) => message.read !== 1)
const removedReadRows = unreadOnly.length !== rawRowCount
// Why: read rows prove a pre-peek runtime cannot honor wait, so fail instead of returning early.
// Read rows reveal an older runtime returning before the requested unread wait is satisfied.
if (wait && removedReadRows && unreadOnly.length === 0) {
throw new RuntimeClientError(
'peek_wait_unsupported',
@@ -115,7 +115,7 @@ function filterLegacyPeekResult(
...result,
result: {
...result.result,
// Why: a pre-peek runtime formats all rows, which no longer matches the filtered set.
// Filtered rows invalidate the older host's preformatted output.
...(removedReadRows ? { formatted: undefined } : {}),
messages: unreadOnly,
count: unreadOnly.length
@@ -42,7 +42,6 @@ export const ORCHESTRATION_INBOX_HANDLERS: Record<string, CommandHandler> = {
if (value.count === 0) {
return 'No messages.'
}
// Why: default output omits body/payload for at-a-glance sweeps; --full prints them for auditing.
return value.messages
.map((message) => {
const head = `${message.id}${formatMessageReadOnlyTag(message)} ${message.from_handle} -> ${message.to_handle ?? '?'}: "${message.subject}"`
@@ -27,7 +27,7 @@ export function getOptionalStructuredMessagePayload(
'Use either --payload or structured payload flags, not both.'
)
}
// Why: raw JSON args are fragile in Windows PowerShell; these flags avoid shell-specific quoting.
// Structured flags avoid fragile PowerShell JSON quoting.
const payload: Record<string, string | string[]> = {}
if (taskId) {
payload.taskId = taskId
@@ -81,11 +81,11 @@ export const ORCHESTRATION_SEND_HANDLER: Record<string, CommandHandler> = {
!process.env.ORCA_TERMINAL_HANDLE &&
!readInjectedAgentSessionId()
) {
// Why: focus isn't lifecycle authority — an identity-less subprocess must fail closed rather than guess the worker.
// Focus cannot establish lifecycle ownership; never guess a worker for an identity-less subprocess.
throwNoActiveSenderTerminal()
}
// Why: lifecycle senders preserve ORCA_TERMINAL_HANDLE across restarts for older runtimes.
// Preserve the env handle across remints for older runtimes' lifecycle ownership checks.
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
const sendParams = {
from,
@@ -97,8 +97,7 @@ export const ORCHESTRATION_SEND_HANDLER: Record<string, CommandHandler> = {
priority: getOptionalStringFlag(flags, 'priority'),
threadId: getOptionalStringFlag(flags, 'thread-id'),
payload: getOptionalStructuredMessagePayload(flags),
// Why: pane key is the remint-stable sender identity the runtime verifies lifecycle ownership against; older runtimes strip it.
// A session names itself by its id alone.
// Pane keys survive remints; sessions instead authenticate by injected id, never inherited pane keys.
senderPaneKey:
from === undefined || readInjectedAgentSessionId()
? undefined
@@ -113,7 +112,7 @@ export const ORCHESTRATION_SEND_HANDLER: Record<string, CommandHandler> = {
'orchestration.send',
sendParams,
dispatchCapability ? { orchestrationCapability: dispatchCapability } : undefined,
// Why: a worker reports once and ends its turn, so a brief app outage must delay worker_done, not drop it.
// Workers report once before ending their turn; retry brief outages so worker_done is not lost.
type === 'worker_done' ? WORKER_DONE_UNAVAILABLE_RETRY_MS : 0
)
await requireWorkerDoneSettlement(client, type, sendParams.payload, result.result)
@@ -10,8 +10,6 @@ export const ORCHESTRATION_REQUEST_SHOW_HANDLER: Record<string, CommandHandler>
const result = await client
.call<OrchestrationMutationRequestShowResult>('orchestration.requestShow', { request })
.catch((error: unknown) => {
// Why: an Orca server older than request-show answers method_not_found, which reads
// as a bug rather than a version gap on the very path a lost response sends you down.
if (error instanceof RuntimeClientError && error.code === 'method_not_found') {
throw new RuntimeClientError(
'incompatible_runtime',
@@ -176,7 +176,7 @@ describe('callOrchestrationMutation runtime_unavailable retry', () => {
})
})
// Why: the fake runtime listens on a Unix socket path, which Windows does not accept.
// Windows cannot use this temporary Unix socket path.
it.skipIf(process.platform === 'win32')(
'reaches a runtime that comes back after the first attempt found it down',
async () => {
@@ -191,7 +191,7 @@ describe('callOrchestrationMutation runtime_unavailable retry', () => {
if (request.method === 'orchestration.send') {
sendRequestIds.push(String(request.orchestrationRequestId))
if (sendRequestIds.length === 1) {
// A runtime that drops the connection mid-request leaves the outcome unknown.
// Dropping the connection mid-request leaves the mutation outcome unknown.
socket.destroy()
return
}
@@ -219,7 +219,7 @@ describe('callOrchestrationMutation runtime_unavailable retry', () => {
undefined,
RETRY_MS
)
// The runtime is down for the first attempt: no metadata, so even the contract probe fails.
// Initial attempts have no metadata, so the contract probe fails until this runtime starts.
await new Promise<void>((resolve) => server.listen(endpoint, resolve))
writeFileSync(
join(userDataPath, 'orca-runtime.json'),
@@ -13,7 +13,7 @@ export async function callOrchestrationMutation<TResult>(
options?: { timeoutMs?: number; orchestrationCapability?: string },
unavailableRetryMs = 0
) {
// Why: every retry reuses one request id, so the host replays instead of applying the mutation twice.
// Reuse one request id so retries replay the mutation instead of applying it twice.
const requestId =
readRetryRequestFlag(flags) ?? (unavailableRetryMs > 0 ? randomUUID() : undefined)
const deadline = Date.now() + unavailableRetryMs
@@ -35,7 +35,7 @@ export async function callOrchestrationMutation<TResult>(
sentError = error
}
if (!unavailable || Date.now() + delayMs > deadline) {
// Why: a later attempt can fail before its request id is attached, though an earlier one may have landed.
// A later failure may lack the request id even though an earlier attempt landed.
throw orchestrationMutationRecoveryError(
carriesRequestId(error) ? error : (sentError ?? error)
)
@@ -69,13 +69,12 @@ export const ORCHESTRATION_QUESTION_HANDLER: Record<string, CommandHandler> = {
compatibilityCliCommand: resolveCompatibilityCliCommand(),
compatibilityWindowsCommand: resolvePackagedWindowsCompatibilityCommand()
},
// Why: extend past timeoutMs so transport does not abort before the runtime resolves its timeout.
// Outlive the server timeout so its result arrives before the transport aborts.
{
timeoutMs: resolveOrchestrationAskClientTimeoutMs(parsedTimeoutMs),
orchestrationCapability: getOptionalStringFlag(flags, 'dispatch-capability')
}
)
// Why: same {ok, result} envelope as every sibling verb; ask used to print a bare object.
if (json) {
printResult(result, true, () => '')
} else if (result.result.legacyCompatibility?.resumeRequired) {
@@ -99,7 +98,7 @@ export const ORCHESTRATION_QUESTION_HANDLER: Record<string, CommandHandler> = {
}
if (result.result.timedOut) {
if (!json) {
// Why: report the server's clamped effective budget rather than overstating the wait.
// Report the server's effective budget, which may be clamped below the requested timeout.
const waitedMs = result.result.timeoutMs ?? timeoutMs
const messageId = result.result.messageId
const dispatchCapability = getOptionalStringFlag(flags, 'dispatch-capability')
@@ -62,7 +62,7 @@ export const ORCHESTRATION_TASK_HANDLERS: Record<string, CommandHandler> = {
run,
callerTerminalHandle
})
// Why: only older runtimes (no spec_truncated) skip server-side abbreviation and need this client-side fallback.
// Only hosts that omit spec_truncated need client-side abbreviation.
const needsClientAbbreviation =
brief && result.result.tasks.some((task) => task.spec_truncated === undefined)
const output = needsClientAbbreviation
@@ -9,11 +9,7 @@ import {
} from '../../../shared/agent-session-caller-env'
import { sessionAddressForHost } from '../../session-caller-flags'
/**
* The caller's terminal handle, or `undefined` when an injected agent session id names the caller:
* the orchestration envelope carries that id and the host binds the caller param to it, so nothing
* is resolved or guessed here, except a session address only the host can place.
*/
/** undefined delegates caller identity to the injected session; the host resolves /clear addresses. */
export async function resolveOrchestrationTerminalHandle(
flags: Map<string, string | boolean>,
cwd: string,
@@ -21,7 +17,7 @@ export async function resolveOrchestrationTerminalHandle(
flagName: 'from' | 'terminal',
options: { validateEnvHandle?: boolean } = {}
): Promise<string | undefined> {
// A caller flag naming anyone else was already refused at the CLI entry, from the command's spec.
// CLI entry already fences caller flags using the command spec.
const explicit = getOptionalStringFlag(flags, flagName)
const sessionId = readInjectedAgentSessionId()
if (sessionId) {
@@ -33,7 +29,7 @@ export async function resolveOrchestrationTerminalHandle(
const envHandle = process.env.ORCA_TERMINAL_HANDLE
if (envHandle && envHandle.length > 0) {
if (flagName === 'from' && options.validateEnvHandle) {
// Why: long-lived shells can retain a stale ORCA_TERMINAL_HANDLE after remint; don't bake it into coordinator preambles.
// Shells can retain stale handles after remint; validate before embedding them in coordinator preambles.
const live = await isLiveTerminalHandle(envHandle, client)
if (!live) {
const reminted = await resolveOrchestrationPaneTerminalHandle(client)
@@ -45,11 +41,7 @@ export async function resolveOrchestrationTerminalHandle(
}
return envHandle
}
// Past this point every remaining route GUESSES an implicit terminal, and a structured session
// has no pane for the guess to land on — so it lands on a sibling. `check` is destructive by
// default, so that guess consumed another pane's oldest unread batch and marked it read, and the
// rightful worker never saw its mail. Refusing is the only honest answer: this child genuinely
// cannot infer its own identity.
// Implicit guesses can select a sibling pane and consume its mail; identity-less sessions must refuse.
if (hasStructuredSessionMarker()) {
throw structuredSessionRefusal(flagName)
}
@@ -59,14 +51,7 @@ export async function resolveOrchestrationTerminalHandle(
return await getTerminalHandle(flags, cwd, client)
}
/**
* Whether the handle this process was born with still names a live identity.
*
* `terminal.resolveIdentity`, never `terminal.show`: `show` is a PTY verb, so it missed for a
* structured worker and reported `terminal_handle_stale` for a handle that was perfectly live —
* which then failed every coordinator verb, because the pane remint below needs an `ORCA_PANE_KEY`
* a structured child deliberately does not carry.
*/
// terminal.show requires a PTY; resolveIdentity also recognizes structured workers without panes.
async function isLiveTerminalHandle(handle: string, client: RuntimeClient): Promise<boolean> {
try {
const response = await client.call<{ identity?: { live?: boolean } }>(
@@ -81,9 +66,7 @@ async function isLiveTerminalHandle(handle: string, client: RuntimeClient): Prom
return false
}
if (getClientErrorCode(err) === 'method_not_found') {
// Clients and remote hosts update independently, so a host that predates the identity probe
// is the normal mixed-version state. Fall back to what it does have — which is correct for
// that host, because a host without the probe also has no structured workers to miss.
// Hosts without the identity probe also predate structured workers, so terminal.show is sufficient.
return await showResolvesTerminalHandle(handle, client)
}
throw err
@@ -128,7 +111,7 @@ async function resolveOrchestrationPaneTerminalHandle(
return undefined
}
try {
// Why: pane-key reminting preserves caller identity; focus-based active-terminal fallback can point at a different pane.
// Remint by pane key; focus-based fallback can select another caller.
const response = await client.call<{ terminal: { handle: string } }>('terminal.resolvePane', {
paneKey
})
@@ -172,15 +155,11 @@ function getClientErrorMessage(err: unknown): string | undefined {
return typeof message === 'string' ? message : undefined
}
/** How check output names its caller: the handle, or the session's address. */
export function orchestrationCallerLabel(handle: string | undefined): string {
return handle ?? injectedSessionAddress() ?? 'unknown'
}
/**
* The caller for a read that `--run` scopes: no terminal is resolved, but a session's declared
* address still goes to the host, which places a `/clear` root and refuses anyone else.
*/
/** Named-run reads need no terminal lookup, but the host must still verify declared session lineage. */
export function runScopedSessionCaller(flags: Map<string, string | boolean>): string | undefined {
const declared = getOptionalStringFlag(flags, 'from')
const sessionId = readInjectedAgentSessionId()
@@ -203,8 +182,7 @@ async function resolveImplicitOrchestrationSender(
client: RuntimeClient
): Promise<string> {
try {
// Unambiguous: naming the sender is an identity claim, so an arbitrary pick would let this
// command speak as a sibling worker.
// An arbitrary active-terminal choice would let this command speak as a sibling worker.
return await getTerminalHandle(flags, cwd, client, { requireUnambiguous: true })
} catch (err) {
if (!isNoActiveTerminalError(err)) {
@@ -214,11 +192,7 @@ async function resolveImplicitOrchestrationSender(
}
}
/**
* Why no flag is suggested: every caller reaches a refusal only after the explicit-flag branch has
* already returned, so `--from` advice would succeed — against a handle that necessarily belongs to
* another pane, whose unread mail the next `check` consumes.
*/
// --from bypasses implicit resolution and could consume another pane's mailbox; never suggest it here.
function structuredSessionRefusal(flagName: 'from' | 'terminal'): RuntimeClientError {
return new RuntimeClientError(
'no_active_sender_terminal',
@@ -230,14 +204,11 @@ function structuredSessionRefusal(flagName: 'from' | 'terminal'): RuntimeClientE
}
export function throwNoActiveSenderTerminal(): never {
// Lifecycle sends refuse here before the structured guard above ever runs, so this is the only
// place left that would tell an identity-less session to pass a handle it does not have. A stale
// ORCA_TERMINAL_HANDLE is a different case — that caller HAS an identity, so it keeps the advice
// to re-run under a live one.
// Lifecycle sends can refuse before the structured guard; distinguish missing identity from stale handles.
if (hasStructuredSessionMarker() && !process.env.ORCA_TERMINAL_HANDLE) {
throw structuredSessionRefusal('from')
}
// Why: Codex sets CODEX_THREAD_ID after its env filter, which is what drops Orca's (#25775).
// Codex sets CODEX_THREAD_ID after the env filter that can drop Orca identity (#25775).
if (process.env.CODEX_THREAD_ID && !process.env.ORCA_TERMINAL_HANDLE) {
throw new RuntimeClientError(
'no_active_sender_terminal',
@@ -3,9 +3,6 @@ import { ORCHESTRATION_HANDLERS } from '../orchestration'
type Call = { name: string; params: Record<string, unknown> }
/** The runtime half of this seam (`runCurrent` from a coordinator handle, `workerList` filtered by
* `run`) is proven in `rpc/methods/orchestration/worker/worker-list-run-scope-rpc.test.ts`; this
* half proves the handler asks exactly those two questions and reports what it decided. */
describe('orchestration worker-list Run scope (CLI handler)', () => {
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
let calls: Call[]
@@ -2,15 +2,9 @@ import { getOptionalStringFlag } from '../../flags'
import type { RuntimeClient } from '../../runtime-client'
import { resolveOrchestrationTerminalHandle } from './terminal-identity'
/** Which Run `worker-list` enumerated, and why. Additive: old readers ignore it. */
export type WorkerListRunScope = { run?: string; source: 'flag' | 'bound' | 'all' }
/**
* Unscoped `worker-list` returned every Dispatch the database has ever held. The bound Run is
* the same Run `check` reads from the calling terminal, so it is the default and `--run`
* overrides it. With no binding to read, listing everything stays the answer and the receipt
* says which of the three it was.
*/
// Default to the caller's bound Run; an explicit --run overrides it and unbound callers see all Runs.
export async function resolveWorkerListRunScope(
flags: Map<string, string | boolean>,
cwd: string,
@@ -27,8 +21,7 @@ export async function resolveWorkerListRunScope(
})
return current.result.run ? { run: current.result.run.id, source: 'bound' } : { source: 'all' }
} catch {
// No live terminal, no stable pane, or a runtime that predates runCurrent: the caller asked
// for an inventory, so answer with the whole one rather than failing the enumeration.
// A missing caller binding or older runtime must not prevent a full inventory.
return { source: 'all' }
}
}
@@ -26,15 +26,14 @@ export const ORCHESTRATION_WORKER_OBSERVATION_HANDLERS: Record<string, CommandHa
const lines = [
`${value.dispatch?.id ?? 'unknown'} task=${value.dispatch?.taskId ?? 'unknown'} [${value.worker.state}] stage=${value.worker.stage}`
]
// Why: PTY status alone read `live` for an agent that died at a trust prompt, so the
// fleet verdict and its next action print beside it rather than in another command.
// A live PTY can contain a dead agent; show the independent agent verdict and recovery action.
if (value.projection) {
lines.push(
`Agent liveness: ${value.projection.liveness.verdict}`,
`Next action: ${value.projection.nextAction.argv.join(' ') || 'none'}`
)
}
// Why: absent means unknown on older runtimes, distinct from an evaluated null wait.
// Older runtimes omit agentWait; absence is unknown, distinct from an evaluated null.
if (value.observation === undefined || !('agentWait' in value.observation)) {
lines.push('Interactive wait: unknown (not evaluated)')
} else if (value.observation.agentWait) {
@@ -329,13 +329,7 @@ function occurrences(haystack: string, needle: string): number {
}
describe('formatWorkerRead', () => {
// The replay case this row is durable for: SQLite-backed, re-sent on every
// reconnect, and read here by a client that draws no roster block, runs no
// reconciliation, and cannot re-check whether those children still exist. A
// sentence frozen mid-flight outlives the process that wrote it, so it must
// not keep asserting a liveness only that process could have observed —
// `docs/reference/ssh-execution-boundary.md` calls that loss of contact
// reported as a live state.
// Replayed journal rows cannot prove current child liveness.
it('replays a mid-flight roster row without claiming a child is still working', () => {
const midFlight: readonly NativeChatSubagentEntry[] = [
{ id: 'child-1', label: 'read', state: 'working' },
@@ -354,10 +348,7 @@ describe('formatWorkerRead', () => {
expect(output).not.toMatch(/\bworking\b/)
})
// The body `codexSubagentGroupBody` actually writes: the plain-text twin, then
// the block it stands in for. The twin exists for clients that cannot draw the
// block, so a client printing the block must not print the twin beside it —
// the renderer drops the twin for the same reason, from the other side.
// Producers emit a text twin for clients that cannot render roster blocks; print only one copy.
it('prints the roster sentence once for the two-block row the producer writes', () => {
const sentence = subagentGroupFallbackText(ROSTER)
const output = formatWorkerRead(
@@ -374,9 +365,6 @@ describe('formatWorkerRead', () => {
expect(occurrences(output, sentence)).toBe(1)
})
// Suppression is per twin, not per message. One twin beside two roster blocks
// silenced BOTH groups and printed one sentence, so the second roster vanished
// with no marker — the same silent drop the missing-twin case above avoids.
it('stands in for the second roster block when only one twin accompanies two', () => {
const other: readonly NativeChatSubagentEntry[] = [
{ id: 'child-3', label: 'plan', state: 'completed' }
@@ -396,10 +384,6 @@ describe('formatWorkerRead', () => {
expect(output).toContain(`[subagents] ${subagentGroupFallbackText(other)}`)
})
// Which group a lone twin belongs to is decided by its TEXT, not its position.
// Claiming positionally silenced whichever group came first, so a twin
// belonging to a LATER group erased the earlier group's roster and printed the
// later one's sentence twice — the same silent drop, one permutation over.
it('claims a lone twin for the group it names, not the first group in the message', () => {
const other: readonly NativeChatSubagentEntry[] = [
{ id: 'child-3', label: 'plan', state: 'completed' }
@@ -420,8 +404,6 @@ describe('formatWorkerRead', () => {
expect(output).toContain(`[subagents] ${subagentGroupFallbackText(ROSTER)}`)
})
// The same claim, with the twin written after both blocks: nothing about the
// ORDER of a twin and its group is guaranteed by the block schema.
it('claims a trailing twin for the group it names', () => {
const other: readonly NativeChatSubagentEntry[] = [
{ id: 'child-3', label: 'plan', state: 'completed' }
@@ -442,9 +424,6 @@ describe('formatWorkerRead', () => {
expect(output).toContain(`[subagents] ${subagentGroupFallbackText(ROSTER)}`)
})
// A group with no twin beside it is a shape the block schema admits and no
// producer writes. Dropping it would lose the roster entirely, so the block
// itself carries the sentence when nothing else does.
it('stands in for a roster block that arrived without its twin', () => {
const output = formatWorkerRead(
transcriptRead([{ type: 'subagent-group', groupId: 'thread:turn-1', agents: [...ROSTER] }])
@@ -453,11 +432,7 @@ describe('formatWorkerRead', () => {
expect(output).toContain(`[assistant] [subagents] ${subagentGroupFallbackText(ROSTER)}`)
})
// A roster from a newer build holds a state this build does not know, which
// `summarizeSubagentGroup` reads as `unverifiable`. Recomputing the sentence
// to compare it against the frozen twin therefore produced a DIFFERENT string,
// and the CLI printed the roster twice: the twin's own wording plus a
// `[subagents]` line contradicting it.
// Unknown roster states can change recomputed text; preserve the producer's twin without duplicating it.
it('prints the roster once when the twin names a state this build cannot reproduce', () => {
const frozenTwin = 'Ran 2 subagents (1 cancelled)'
const output = formatWorkerRead(
@@ -482,10 +457,7 @@ describe('formatWorkerRead', () => {
expect(output).not.toContain('unverifiable')
})
// The journal admits block types this build does not know, and `client.call`
// casts the RPC result rather than validating it — so a newer remote host's
// block reaches this formatter as-is. Reading fields off it threw a TypeError
// and took down the whole `worker read`.
// RPC results are not schema-validated here, so newer hosts' unknown block types reach the formatter.
it('degrades an unknown block type from a newer host instead of throwing', () => {
const output = formatWorkerRead(
transcriptRead([
@@ -22,8 +22,7 @@ export type WorkerStartReceipt = {
export function formatWorkerStart(value: WorkerStartReceipt): string {
const lines = [`Worker ${value.dispatchId} [${value.state}] for ${value.taskId}`]
// Settings-driven rather than requested, so the human line always names the mode that ran: a
// fallback from the user's structured default is never silent.
// Show the mode that actually ran so a fallback from the user's default is visible.
if (value.mode) {
lines.push(value.mode.detail)
}
@@ -74,12 +73,11 @@ function formatWorkerReadDetails(value: OrchestrationWorkerReadResult): string {
? `Source: transcript (provider=${value.provider})`
: 'Source: terminal'
const lines = [source]
// A released archive read otherwise prints identically to a live one.
if (value.status?.worker) {
lines.push(`Worker: ${value.status.worker}`)
}
lines.push(`Archived: ${value.archived === true}`)
// Two different verdicts: status.liveness is the PTY's, the fleet projection is the agent's.
// PTY liveness and agent liveness are separate verdicts.
if (value.status?.liveness) {
lines.push(`Terminal liveness: ${value.status.liveness}`)
}
@@ -67,7 +67,7 @@ export const ORCHESTRATION_WORKER_TERMINAL_HANDLERS: Record<string, CommandHandl
'orchestration.workerRelease',
{ dispatch: getRequiredStringFlag(flags, 'dispatch') }
)
// Why: only an unprovable close is a failure; retained/pending/already-released are settled answers.
// Retained, pending, and already-released are settled outcomes; only release_unknown fails.
if (result.result.state === 'release_unknown') {
process.exitCode = 1
}
@@ -164,7 +164,6 @@ export const ORCHESTRATION_WORKER_TERMINAL_HANDLERS: Record<string, CommandHandl
const details = projection
? `/${stage}] attention=${attention} liveness=${liveness} provider=${provider} host=${projection.host.id} workspace=${workspace}`
: `]`
// Why: the enumerating command owes the literal argv the guides tell callers to run.
const next = projection
? ` next=${projection.nextAction.argv.join(' ') || 'none'}`
: ''
+6 -20
View File
@@ -34,18 +34,13 @@ import { findProjectSetups, resolveProjectIdSelector } from '../project-selector
import { resolveRepoPathArgument } from '../repo-path-arguments'
import { RuntimeClientError, type RuntimeRpcSuccess } from '../runtime-client'
// Why: an Orca server that predates project host setup answers `method_not_found`, which reads
// as an Orca bug rather than a version gap — and since --host runtime:<id> now routes these
// commands to that server, a client can reach an older host without meaning to. The desktop
// already names this case; match it instead of surfacing the raw dispatcher error.
async function callProjectHostSetup<TResult>(
client: HandlerContext['client'],
method: string,
params?: unknown
): Promise<RuntimeRpcSuccess<TResult>> {
try {
// Why: forward the exact arity the caller used; passing an explicit undefined would change
// the request shape for the no-params methods.
// Preserve absent params; explicitly passing undefined changes the request shape.
return params === undefined
? await client.call<TResult>(method)
: await client.call<TResult>(method, params)
@@ -71,9 +66,7 @@ async function getResolvedHostId(
return host.id
}
// Why: the setup paths keep the unresolved id on purpose. The runtime rejects every `ssh:` host
// for those operations regardless of whether it exists, so resolving first would answer "no such
// target" and imply the command would have worked with the right id.
// The runtime rejects SSH setup operations regardless of target existence; preserve that refusal.
async function getRequiredHostId(
flags: Map<string, string | boolean>,
client: HandlerContext['client']
@@ -85,8 +78,7 @@ async function getRequiredHostId(
if (host.kind !== 'alias') {
return host.id
}
// A bare alias has no unresolved spelling to forward — naming the machine is the lookup — so
// this one case resolves even on the paths that otherwise pass the id through untouched.
// Bare aliases must resolve to an id before they can be forwarded.
const resolved = await resolveHostFlagTarget(flags, client)
if (!resolved) {
throw new RuntimeClientError('invalid_argument', 'Missing required --host')
@@ -94,8 +86,7 @@ async function getRequiredHostId(
return resolved.id
}
// Why: a project on two hosts has two setups with the same id; `--host` picks one. An older
// host strips the field and acts on the first twin, so refuse rather than let it guess.
// Older hosts drop hostId and pick the first setup twin; refuse unsupported host targeting.
async function getOptionalSetupHostId(
flags: Map<string, string | boolean>,
client: HandlerContext['client']
@@ -149,8 +140,7 @@ export const PROJECT_HANDLERS: Record<string, CommandHandler> = {
'project setup-existing-folder': async ({ flags, client, cwd, json }) => {
const rawPath = getRequiredStringFlag(flags, 'path')
const hostId = await getRequiredHostId(flags, client)
// An SSH host's filesystem is not the CLI's, so resolving a relative path against the client
// cwd would register a path that names the wrong machine.
// SSH paths belong to the target filesystem, even when the runtime connection itself is local.
const pathIsOffClient = client.isRemote || getSshTargetIdForExecutionHost(hostId) !== null
const args: ProjectHostSetupExistingFolderArgs = {
hostId,
@@ -191,11 +181,7 @@ export const PROJECT_HANDLERS: Record<string, CommandHandler> = {
'project setup-create': async ({ flags, client, cwd, json }) => {
const path = getOptionalStringFlag(flags, 'path')
const args: ProjectHostSetupCreateArgs = {
// Why: unlike the setup paths below, the runtime does not reject `ssh:` here — this records
// independent metadata — so an unknown target would persist a row pointing at a machine
// that does not exist. Resolving catches that. `local` and `runtime:` pass through
// untouched, because this is also the provisioning path and a runtime host legitimately
// may not exist yet when its metadata is written.
// Resolve SSH targets to reject nonexistent hosts; runtime metadata may precede provisioning.
hostId: await getResolvedHostId(flags, client),
setupId: getOptionalStringFlag(flags, 'setup-id'),
path:
+3 -6
View File
@@ -24,8 +24,7 @@ import {
} from '../reference-input'
import { RuntimeClientError, type RuntimeRpcSuccess } from '../runtime-client'
// Why: hosts from before reference.* still advertise the linked-items delta capability, so the
// raw method_not_found would read as an Orca bug rather than a version gap.
// Older hosts may advertise linked-item deltas without supporting reference.*.
async function callReference<TResult>(
client: HandlerContext['client'],
method: 'reference.list' | 'reference.find',
@@ -116,7 +115,7 @@ async function mutateReferences(context: HandlerContext, operation: 'add' | 'rem
: `id:${workspace.id}`
const stored = before.result.references.map(({ key, selected: _selected, ...item }) => ({
item,
// Why: --key values come from the host's `reference list`; the CLI may be a different build.
// Reference keys come from the host, which may run a different build.
keys: new Set([key, getWorkspaceReferenceIdentity(item)])
}))
const base = stored.map(({ item }) => item)
@@ -132,9 +131,7 @@ async function mutateReferences(context: HandlerContext, operation: 'add' | 'rem
}
})
if (changes.some((change) => change.changed)) {
// Why: storage may hold same-URL twins with different source contexts; derive from `base`
// so the host's delta merge never sees an unrequested twin as removed.
// Input is already deduped by identity.
// Preserve same-URL twins with distinct source contexts when preparing the host's delta merge.
const linkedItems =
operation === 'add'
? [...base, ...input.filter((item) => !isLinked(getWorkspaceReferenceIdentity(item)))]
-2
View File
@@ -90,7 +90,6 @@ async function runSearch(
type CliFlags = [string, string | boolean][]
/** The printed envelope, narrowed by shape rather than asserted. */
function printedEnvelope(output: string): { keys: string[]; result: unknown } {
const parsed: unknown = JSON.parse(output)
if (typeof parsed !== 'object' || parsed === null || !('result' in parsed)) {
@@ -99,7 +98,6 @@ function printedEnvelope(output: string): { keys: string[]; result: unknown } {
return { keys: Object.keys(parsed), result: parsed.result }
}
/** Re-reads the printed result through the contract, so the shape is checked, not claimed. */
function printedResults(output: string): Extract<AiVaultSearchResponse, { kind: 'results' }> {
const parsed = AiVaultSearchResponseSchema.parse(printedEnvelope(output).result)
if (parsed.kind !== 'results') {
+2 -9
View File
@@ -13,12 +13,7 @@ import { parseSearchCommand } from '../search-command-arguments'
export const MALFORMED_CURSOR_MESSAGE =
'The host did not recognise that --cursor value. Cursors belong to one query on one host; re-run the search without --cursor.'
/**
* The shared contract client over the CLI's runtime RPC. Reusing it is what makes
* an old host's unknown-method refusal an `unavailable/no-service` answer instead
* of a raw JSON-RPC error, and it applies the same exposure policy the host did:
* a paired runtime is a relay caller, a local one is not.
*/
// Reuse contract compatibility and exposure policy: paired runtimes are relay callers.
function createCliSessionSearch(client: RuntimeClient) {
let lastEnvelope: RuntimeRpcSuccess<unknown> | undefined
const search = createSessionSearchClient(
@@ -28,8 +23,7 @@ function createCliSessionSearch(client: RuntimeClient) {
},
client.isRemote ? 'relay' : 'runtime'
)
// Why `client`: an answer synthesised from a refusal had no successful call, so
// no runtime produced it and none of its identifiers may be claimed here.
// A synthesized refusal has no runtime envelope; do not claim an unobserved runtime identity.
const envelope = <TResult>(result: TResult): RuntimeRpcSuccess<TResult> =>
lastEnvelope
? { ...lastEnvelope, result }
@@ -37,7 +31,6 @@ function createCliSessionSearch(client: RuntimeClient) {
return { search, envelope }
}
/** `orca search` over `aiVault.searchSessions` / `aiVault.searchStatus` on one host. */
export const SEARCH_HANDLERS: Record<string, CommandHandler> = {
search: async ({ client, flags, json }) => {
const command = parseSearchCommand(flags)
+1 -2
View File
@@ -152,7 +152,7 @@ describe('orca serve service', () => {
).toThrow(/PATH/)
})
it('names the command to run when only a server-only orca exists here', async () => {
// The `orca` an SSH-launched Orca server puts on a remote host runs on plain Node.
// SSH server launchers run on plain Node, without an installed Electron desktop.
const serverCli = (platform: NodeJS.Platform, path: string): ServeServiceDeps => {
const base = deps(platform)
const { ELECTRON_RUN_AS_NODE: _electron, ...env } = base.env
@@ -169,7 +169,6 @@ describe('orca serve service', () => {
installServeService({ command: null, dryRun: true }, serverCli('darwin', scoped))
).rejects.toThrow(/`orca serve service install`/)
expect(calls).toEqual([])
// An installed app's launcher on PATH (or named by --command) still works from that terminal.
await expect(
installServeService({ command: null, dryRun: true }, serverCli('linux', `${scoped}:${bin}`))
).resolves.toMatchObject({ dryRun: true })
+3 -4
View File
@@ -116,7 +116,7 @@ export async function installServeService(
if (spec.logPath) {
mkdirSync(dirname(spec.logPath), { recursive: true, mode: 0o700 })
}
// Why: installing is the documented way to retry after the crash-loop brake gave up.
// Reinstalling retries a supervisor stopped by the crash-loop brake.
clearServeSupervisorBrake(resolveLaunchUserDataPath(deps.env))
if (spec.manager === 'launchd') {
await deps.run('launchctl', ['bootout', `gui/${deps.uid}/${spec.name}`])
@@ -170,7 +170,7 @@ export async function readServeServiceStatus(
if (result.code === null) {
return { ...base, verdict: 'unverifiable', pid: null, detail: 'launchctl did not answer' }
}
// Why only 113: other failures (no GUI session over SSH, a refused request) prove nothing.
// Other launchctl failures cannot establish an exit, e.g. a missing GUI session over SSH.
if (result.code === 113) {
return { ...base, verdict: 'exited', pid: null, detail: 'not loaded' }
}
@@ -228,7 +228,6 @@ export async function uninstallServeService(
return { removed: existed, manager: spec.manager, name: spec.name, unitPath: spec.unitPath }
}
/** Local, runtime-free output: the same envelope shape `--json` callers get elsewhere. */
function emit<T>(result: T, json: boolean, format: (value: T) => string): void {
console.log(json ? JSON.stringify({ ok: true, result }, null, 2) : format(result))
}
@@ -241,7 +240,7 @@ export const SERVE_SERVICE_HANDLERS: Record<string, CommandHandler> = {
'serve service install': async ({ flags, json }) => {
const result = await installServeService({
command: optionalString(flags.get('command')),
// Why validate here: a bad value fails before the supervisor's brake and launchd would loop.
// Validate before launchd can loop on a bad value and trip the supervisor brake.
port: getOptionalServePort(flags),
pairingAddress: optionalString(flags.get('pairing-address')),
dryRun: flags.get('dry-run') === true
+1 -2
View File
@@ -10,8 +10,7 @@ import {
type GuideSelection = { full: boolean; reference: string | null; listReferences: boolean }
// Why: the kernel's gate table names each document as `references/<file>.md`, so that
// exact string must resolve as well as the bare name an agent is likely to retype.
// Accept references/<file>.md exactly as published in the kernel's gate table.
function normalizeReferenceSelector(value: string): string {
return value
.trim()
+1 -2
View File
@@ -6,8 +6,7 @@ import { createSkillMutationHandler } from './bundled-skill-install-command'
export const SKILL_HANDLERS: Record<string, CommandHandler> = {
'skills list': async ({ json }) => {
// Why: generated registry order is not a user-facing contract, while stable
// canonical sorting keeps agent-visible output reproducible across builds.
// Canonical sorting keeps output stable across generated-registry changes.
const topics = (await loadCanonicalGuides()).map((guide) => ({
name: guide.name,
description: guide.description.replace(/\s+/g, ' ').trim()
-1
View File
@@ -17,7 +17,6 @@ describe('orca speech openai-key set', () => {
await expect(readOpenAiKeyFromStdin(piped('sk-test-', 'abc\n'))).resolves.toBe('sk-test-abc')
})
// Why: a key typed as an argument would sit in shell history and process listings.
it('refuses an interactive terminal instead of waiting on it', async () => {
const terminal = Object.assign(chunks(), { isTTY: true })
await expect(readOpenAiKeyFromStdin(terminal)).rejects.toThrow('Pipe the key on stdin')
+1 -2
View File
@@ -14,7 +14,7 @@ function formatKeyStatus(status: OpenAiSpeechKeyStatus): string {
: 'OpenAI key saved on this host.'
}
// Why stdin and never a flag: argv shows up in shell history and process listings.
// Keep secrets out of argv, shell history, and process listings.
export async function readOpenAiKeyFromStdin(
stdin: AsyncIterable<unknown> & { isTTY?: boolean } = process.stdin
): Promise<string> {
@@ -44,7 +44,6 @@ export async function readOpenAiKeyFromStdin(
return key
}
// Why: `--host runtime:<id>` can reach an older server, whose raw `method_not_found` reads as a bug.
export async function callOpenAiKeyMethod(
client: Pick<HandlerContext['client'], 'call'>,
method: string,
+2 -5
View File
@@ -21,7 +21,6 @@ function terminalCloseFailure(close: RuntimeTerminalClose): RuntimeClientError |
{ close }
)
}
// Why: the close itself committed; only the process stop is unproven, so say which.
return new RuntimeClientError(
'terminal_stop_unverifiable',
`Closed terminal ${close.handle}. ${describeUnconfirmedCloseStop(close)}`,
@@ -91,12 +90,10 @@ export const terminalCloseHandler: CommandHandler = async ({ flags, client, cwd,
const result = await client.call<{ close: RuntimeTerminalClose }>(method, {
terminal: await getTerminalHandle(flags, cwd, client)
})
// Why: a transport-level success must not hide a live or unverifiable PTY. Keep the receipt in
// error.data so JSON callers retain the host's exact evidence while receiving a failing outcome.
// A successful RPC can leave a live or unverified PTY; retain the host receipt in error.data.
const failure = terminalCloseFailure(result.result.close)
if (failure) {
// Keep the established human receipt (including its liveness warning); JSON needs the
// standard failure envelope so callers do not mistake transport success for a stopped PTY.
// Keep human liveness warnings; JSON needs a failure envelope to distinguish an unconfirmed stop.
if (json) {
reportCliError(failure, true)
} else {
+1 -1
View File
@@ -57,7 +57,7 @@ function formatDaemonLiveness(daemon: DaemonLiveness | undefined): string[] {
export function formatSeveredDaemonStatus(result: SeveredStatusResult): string {
const severed = result.severed
const liveness = formatDaemonLiveness(result.daemon)
// Why: a severed daemon that has since died hosts nothing; the liveness line says so.
// An exited daemon hosts nothing; do not print stale severed-state details.
if (!severed || result.daemon?.verdict === 'exited') {
return liveness.join('\n')
}
-1
View File
@@ -100,7 +100,6 @@ export const terminalSendHandler: CommandHandler = async ({ flags, client, cwd,
baselineWorkingSequence: 0
}
}
// Why: the delivery warnings only existed in the text formatter, so --json callers never saw them.
const warnings = terminalSendWarnings(result.result.send)
printResult(
warnings.length > 0 ? { ...result, result: { ...result.result, warnings } } : result,
+2 -4
View File
@@ -785,8 +785,7 @@ describe('terminal create --shell', () => {
expect(call).not.toHaveBeenCalled()
})
// An older host strips the unknown param and answers with its default shell, which reads as a
// successful create. Creating the wrong shell silently is worse than refusing.
// Older hosts ignore shell and report a successful create using their default shell.
it('refuses rather than creating a default-shell terminal on a host without the capability', async () => {
const call = vi.fn()
@@ -796,8 +795,7 @@ describe('terminal create --shell', () => {
expect(call).not.toHaveBeenCalled()
})
// A status probe that fails or times out reports no capabilities either; blaming the host
// version would send the caller to update a host that may already be current.
// Missing capabilities on an unreachable runtime do not establish a version gap.
it('reports an unreachable host as unavailable rather than incompatible', async () => {
const call = vi.fn()
+7 -18
View File
@@ -52,9 +52,7 @@ import { revealPairedTerminalOnLocalDesktop } from '../paired-terminal-desktop-r
import { terminalCloseHandler } from './terminal-close'
import { terminalSendHandler } from './terminal-send'
// Why: terminal wait legitimately needs to outlive the CLI's default RPC
// timeout. Even without an explicit server timeout, the client must allow
// long waits instead of failing at the generic 15s transport cap.
// Long terminal waits must outlive the ordinary transport timeout.
const DEFAULT_TERMINAL_WAIT_RPC_TIMEOUT_MS = 5 * 60 * 1000
const terminalFocusHandler: CommandHandler = async ({ flags, client, cwd, json }) => {
@@ -76,7 +74,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
{
worktree: await getOptionalWorktreeSelector(flags, 'worktree', cwd, client),
limit: getOptionalPositiveIntegerFlag(flags, 'limit'),
// Why: agent JSON calls dominate; topology stays available through an explicit opt-in.
// Omit topology from JSON unless requested to keep agent responses small.
includeVisualLayouts: !json || flags.has('include-visual-layouts')
}
)
@@ -99,8 +97,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
throw new RuntimeClientError('invalid_argument', '--cursor must be a non-negative integer')
}
const screen = flags.get('screen') === true
// Why: a cursor pages through accumulated output. A screen read is the current frame and has
// nothing behind it to page, so accepting both would imply history that is not there.
// A screen is one current frame; a cursor would falsely imply pageable history.
if (screen && cursorFlag !== undefined) {
throw new RuntimeClientError(
'invalid_argument',
@@ -113,9 +110,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
...(screen ? { screen: true } : {}),
limit: getOptionalPositiveIntegerFlag(flags, 'limit')
})
// Why: an older host drops the unknown `screen` param and answers with its ordinary stream
// read, which carries no source. Returning that silently is the exact failure this flag
// exists to prevent, so refuse rather than hand back the other question's answer.
// Older hosts ignore screen and return a stream without source; never present that as a screen.
if (screen && result.result.terminal.source === undefined) {
throw new RuntimeClientError(
'incompatible_runtime',
@@ -140,8 +135,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
)
printResult(result, json, formatTerminalWait)
if (result.result.wait.satisfied === false) {
// Why: callers commonly chain `terminal wait && terminal send`; a
// structured blocked result is still an unsatisfied wait condition.
// Chained commands such as wait && send need a failing exit for an unsatisfied condition.
process.exitCode = 1
}
},
@@ -185,9 +179,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
`--shell must be one of: ${listSupportedWindowsShellOverrides().join(', ')}`
)
}
// Why refused rather than sent hopefully: an older host strips the unknown param and hands
// back a healthy terminal running its DEFAULT shell. Nothing in that reply says the shell
// was ignored, so a caller that wanted cmd would drive a PowerShell session believing it won.
// Older hosts ignore shell and successfully create the wrong default shell; check support first.
const status = await client.getCliStatus()
// An unreachable host reports no capabilities at all; that is not evidence it lacks --shell.
if (!status.result.runtime.reachable) {
@@ -214,14 +206,11 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
title: getOptionalStringFlag(flags, 'title'),
focus,
...(focus ? { presentation: 'focused' } : {}),
// Compat: hosts before v1.4.211 (#22384) leave a background-started agent's startup queries
// unanswered, so they need this to start it in the window; newer hosts ignore it. Delete
// once the minimum supported host version is v1.4.211 or later.
// Compat: hosts before v1.4.211 (#22384) need renderer startup replies; remove once unsupported.
...(useRendererBackedInteractiveTerminal ? { rendererBacked: true, activate: focus } : {})
})
printResult(result, json, formatTerminalCreate)
},
// `focus` resolves to this canonical path via CommandSpec.aliases before dispatch.
'terminal switch': terminalFocusHandler,
'terminal close': terminalCloseHandler,
'terminal split': async ({ flags, client, cwd, json }) => {
+3 -11
View File
@@ -12,8 +12,7 @@ import {
getEphemeralVmRecipeResultWarnings,
redactEphemeralVmRecipeDiagnosticText
} from '../../shared/ephemeral-vm-recipe-diagnostics'
// Why: import directly from the doctor module (not the barrel) — it uses Node
// fs/path and must stay out of the browser bundle that imports the barrel.
// This module uses Node fs/path; exporting it through the shared barrel would break browser bundles.
import { doctorEphemeralVmRecipe } from '../../shared/ephemeral-vm-recipe-doctor'
import {
runEphemeralVmRecipeCleanup,
@@ -85,9 +84,6 @@ function readTextFile(path: string): string {
return readFileSync(path, 'utf8')
}
// Why: give the agent the full create/destroy output so it can self-diagnose a
// failed provision instead of relaying logs through the user. Each stream is
// redacted and capped (head+tail) so a huge log stays readable but complete enough.
type ProvisionStageTranscript = {
exitCode: number | null
signal: NodeJS.Signals | null
@@ -107,8 +103,7 @@ type DoctorResult = EphemeralVmRecipeDoctorResult & {
const MAX_TRANSCRIPT_STREAM_CHARS = 16_000
// Why: keep both ends of a long log — the script's setup context (head) and the
// failure itself (tail) — rather than only the last 500 chars.
// Keep setup context and final diagnostics while bounding and redacting each stream.
function capTranscriptStream(value: string): string {
const redacted = redactEphemeralVmRecipeDiagnosticText(value)
if (redacted.length <= MAX_TRANSCRIPT_STREAM_CHARS) {
@@ -158,8 +153,6 @@ async function doctorRecipeWithProvision(
remediation: buildProvisionFailureRemediation(start.stderr, start.stdout)
}
],
// Why: the create script failed — hand the agent the complete output (not a
// tail) so it can see what the script printed and why parsing/exit failed.
provisionTranscript: {
provision: {
exitCode: start.exitCode,
@@ -222,8 +215,7 @@ async function doctorRecipeWithProvision(
})
}
// Why: include both stages' full output even on success — the agent can confirm
// pairing/teardown looked right, and diagnose a destroy failure without re-running.
// Retain both stages for diagnosis even when creation succeeds and teardown fails.
const provisionTranscript: ProvisionTranscript = {
provision: {
exitCode: 0,
@@ -1,8 +1,4 @@
/**
* `worktree create --agent`: the worktree and its agent in one `agent.launch`, so the agent follows
* `--surface`, else the user's Chat UI setting, like every other launch. A host older than
* `agent.launch` gets the create's own startup terminal, which cannot take a model, effort or surface.
*/
// Use agent.launch for surface/model/effort; older hosts fall back to the create startup terminal.
import type { AgentLaunchResult } from '../../shared/agent-launch-intent'
import {
@@ -43,8 +43,7 @@ export async function resolveCreateParentSelector(
const parentWorkspace = getWorkspaceKeyParentSelector(rawParentWorktree)
if (parentWorkspace) {
// Why: create exposes one public parent flag, while the runtime still needs
// workspace keys to preserve folder/worktree lineage accurately.
// The public parent flag can name either kind; runtime workspace keys preserve folder lineage.
return { parentWorkspace }
}
@@ -53,7 +52,7 @@ export async function resolveCreateParentSelector(
? getWorkspaceKeyParentSelector(parentWorktree)
: undefined
if (resolvedParentWorkspace) {
// Why: active/current may resolve to a folder workspace pseudo-worktree id.
// active/current can resolve to a folder workspace pseudo-worktree id.
return { parentWorkspace: resolvedParentWorkspace }
}
@@ -44,8 +44,6 @@ describe('getOptionalGitLabLinkFlag', () => {
).toBe(77)
})
// Issues and merge requests are separate namespaces on GitLab, so a reference
// to one must never be taken as the other's number.
it('refuses a merge request reference in the issue flag', () => {
expect(() => getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': '!42' }), 'issue')).toThrow(
/GitLab issue number/
@@ -92,8 +90,7 @@ describe('getOptionalGitLabLinkFlag', () => {
}
)
// A URL without GitLab's `/-/` separator is not a GitLab link, and a project
// path needs a group segment — neither may fall through to a number.
// GitLab URLs need /-/ and a grouped project path; malformed URLs must not fall back to numbers.
it.each(['https://gitlab.com/group/project/issues/42', 'https://gitlab.com/project/-/issues/42'])(
'refuses the non-GitLab URL shape %s',
(input) => {
@@ -101,8 +98,7 @@ describe('getOptionalGitLabLinkFlag', () => {
}
)
// `/^\d+$/` accepts 400 digits, which parseInt turns into Infinity — persisting
// that writes `null` over the link it meant to set.
// parseInt turns huge digit strings into Infinity, which JSON would persist as null.
it('refuses a number too large to be an integer', () => {
expect(() =>
getOptionalGitLabLinkFlag(flags({ 'gitlab-issue': '9'.repeat(400) }), 'issue')
@@ -23,10 +23,7 @@ export function getOptionalLinearIssueLinkFlag(
return { ...LINEAR_ISSUE_LINK_CLEARED }
}
// Why: the shared builder treats empty input as "clear", which is right for a
// text field the user can blank but wrong for a flag — `--linear-issue " "`
// is a mistyped argument, not a request to unlink. Only the literal `null`
// clears, and that is handled above.
// Only literal null unlinks; blank flag values must not inherit the text-field builder's clear behavior.
const updates = value.trim() === '' ? null : buildLinearIssueLinkUpdates(value)
if (!updates) {
throw new RuntimeClientError(
@@ -21,8 +21,7 @@ export function printHookWarning(result: HookWarningResult, json: boolean): void
if (result.warning) {
console.error(`warning: ${result.warning}`)
}
// Why (#19334): a waived archive-hook failure is the one case where Orca deleted a checkout
// whose archive step did not succeed. It has to stay visible in human output.
// A waived archive failure must remain visible after the checkout is deleted.
if (result.archiveHookOverride) {
console.error(`warning: ${formatArchiveHookOverride(result.archiveHookOverride)}`)
}
@@ -10,7 +10,7 @@ type ReviewTargetLinks = {
linkedGitLabMR?: number | null
}
// Why: only `set` may clear a link, so `create` parses the same flags non-nullable.
// Only set may clear a link; create uses non-nullable flags.
export function getReviewTargetLinkFlags(
flags: Map<string, string | boolean>,
options: { nullable?: boolean } = {}
@@ -1,13 +1,11 @@
import { RuntimeClientError, RuntimeRpcFailureError } from '../runtime-client'
// Why match the text: every host version refuses an undecided `ask` create with exactly this
// message and the generic runtime_error code, so it is the only stable signal.
// Hosts use generic runtime_error here; this message is the stable discriminator.
const SETUP_DECISION_REQUIRED = 'Setup decision required for this repository'
const SETUP_DECISION_NEXT_STEP =
'Pass --setup run to run the setup script, or --setup skip to create without it.'
/** Why here, not on the host: desktop and phone choose setup in their own UI, so the host's
* shared refusal names no flag; only the CLI user answers it with --setup. */
/** Only the CLI answers setup prompts with --setup; desktop and mobile have their own UI. */
export async function withSetupDecisionRecovery<T>(
create: Promise<T>,
replayingLaunch = false
+1 -2
View File
@@ -1,12 +1,11 @@
import { RuntimeClientError } from '../runtime-client'
/** `--unread` / `--read` as `worktree.set`'s `isUnread`; absent leaves it unchanged. */
export function getOptionalWorktreeUnreadFlag(
flags: Map<string, string | boolean>
): boolean | undefined {
const unread = flags.get('unread')
const read = flags.get('read')
// Why: `--unread=x` and `--read x` parse as values and would otherwise be silently dropped.
// Reject valued booleans instead of silently ignoring --unread=x or --read x.
for (const [name, value] of [
['unread', unread],
['read', read]
+4 -10
View File
@@ -167,9 +167,7 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
needsCwdRepoInference
) {
try {
// Why: agent shells can lose ORCA_TERMINAL_HANDLE while still running
// inside an Orca worktree. Cwd keeps CLI-created children nestable and
// lets create infer the repo for the common current-workspace case.
// Cwd preserves parent/repo inference when agent shells lose ORCA_TERMINAL_HANDLE.
cwdParentWorktree = await resolveCurrentWorktreeSelection(cwd, client)
} catch {
cwdParentWorktree = undefined
@@ -208,8 +206,7 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
...(cwdParentWorktree ? { cwdParentWorktree: cwdParentWorktree.selector } : {}),
noParent,
callerTerminalHandle,
// Why: marks the workspace as CLI-created so the sidebar can badge and
// filter it. Sent on every `worktree create` — hand-typed or agent-run.
// Always request CLI provenance, including hand-typed creates without a terminal handle.
cliProvenanceRequest: callerTerminalHandle ? { callerTerminalHandle } : {}
}
if (agentLaunch) {
@@ -265,8 +262,6 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
'Orca cannot tell which host owns this workspace. Refresh projects and try again.'
)
}
// Why (#19334): the waiver only ever applies to a hook that ran, so without --run-hooks it
// silently does nothing. Rejecting it beats letting someone believe they waived something.
if (flags.get('allow-failed-archive-hook') === true && flags.get('run-hooks') !== true) {
throw new RuntimeClientError(
'invalid_argument',
@@ -277,11 +272,10 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
worktree,
hostId,
force: flags.get('force') === true,
// Why (#11960): --force is explicit here, so it may also waive PTY-stop proof.
// --force also waives PTY-stop proof.
allowUnverifiedPtyStop: flags.get('force') === true,
runHooks: flags.get('run-hooks') === true,
// Why (#19334): deliberately NOT coupled to --force, which above already waives PTY-stop
// proof. Waiving a failed archive hook is a separate decision about the user's data.
// Archive failure waivers protect user data separately from --force's PTY-stop waiver.
allowFailedArchiveHook: flags.get('allow-failed-archive-hook') === true
})
printHookWarning(result.result, json)