[comment audit] Shorten remaining CLI root comments (#32273)

* 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

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-10-23 08:07:03 -07:00
committed by GitHub
co-authored by OrcaWin
parent 120f01331c
commit 9aa208a4ab
64 changed files with 111 additions and 568 deletions
+2 -12
View File
@@ -5,16 +5,9 @@ import { describe, expect, it } from 'vitest'
import { buildHandlerRoutes, dispatch, type HandlerContext } from './dispatch'
import { HANDLER_GROUPS, type HandlerGroup } from './handler-group-manifest'
// Why: dispatch trusts the manifest's eager key lists to route without loading a
// group. These tests are the only thing standing between that trust and a
// silently unreachable command, so they load every group for real.
// Why: __dirname works under both Vitest and the CommonJS tsc emit that
// build:cli type-checks this file against; import.meta.dirname does not.
// import.meta.dirname is unavailable in the CommonJS build:cli emit.
const HANDLERS_DIR = join(__dirname, 'handlers')
// Why: both the plural records and the single-command `*_HANDLER` ones that
// nested modules export get spread into a group, so both must route.
const HANDLER_RECORD_EXPORT = /_HANDLERS?$/
function listHandlerModules(dir: string): string[] {
@@ -49,10 +42,7 @@ describe('handler group manifest', () => {
expect(notCallable).toEqual([])
})
// Why: dropping a group from the manifest silently unregisters its commands —
// walk the tree so a new or forgotten handler file fails here, not in prod.
// Nested modules are spread into a parent group rather than registered under
// their own name, so routability, not file name, is the invariant that holds.
// Nested modules are spread into parent groups; verify routability rather than group names.
it('routes every command exported by a handler module', async () => {
const routes = buildHandlerRoutes(HANDLER_GROUPS)
const unroutable: string[] = []
+2 -4
View File
@@ -6,14 +6,12 @@ import { REFERENCE_HANDLER_GROUP } from './reference-handler-group'
export type HandlerGroup = {
name: string
// Why: eager string keys let dispatch build (and duplicate-check) the whole
// command table without loading any group's transitive module graph.
// Eager keys allow routing and duplicate checks without importing each handler graph.
keys: readonly string[]
load: () => Promise<Record<string, CommandHandler>>
}
// Why: `keys` mirrors each group's exported record and is verified against the
// real exports by handler-group-manifest.test.ts, so drift fails CI, not dispatch.
// handler-group-manifest.test.ts checks keys against the real exports to prevent drift.
export const HANDLER_GROUPS: readonly HandlerGroup[] = [
JIRA_HANDLER_GROUP,
LINEAR_HANDLER_GROUP,
-3
View File
@@ -88,7 +88,6 @@ function formatCommandFlagHelp(flag: string, commandPath: string[]): string {
if (command === 'terminal close' && flag === 'tab') {
return '--tab Close the whole tab and wait for durable persistence'
}
// Why: the shared --title help says "terminal tab" — correct for `rename`, wrong for one pane.
if (command === 'terminal set-pane-title' && flag === 'title') {
return '--title <text> Custom title for one pane (pass "" to reset)'
}
@@ -182,8 +181,6 @@ function formatCommandFlagHelp(flag: string, commandPath: string[]): string {
if (command === 'orchestration task-create' && flag === 'display-name') {
return '--display-name <text> UI label shown for dispatched worker rows'
}
// Why: the shared --agent help describes launching a TUI agent in a terminal,
// which is the wrong meaning here — this selects the account provider.
if (command === 'account add' && flag === 'agent') {
return '--agent <id> Account provider: claude, codex, opencode, or devin (default claude)'
}
@@ -20,8 +20,6 @@ function clientReturning(targets: { id: string; label: string }[]): RuntimeClien
}
describe('findSshTargetByName', () => {
// Why: ids are machine-generated `ssh-<timestamp>-<random>`, so the label is the only name a
// person or agent ever has. Matching ids alone reports "not found" for a target that exists.
it('matches the human label as well as the generated id', () => {
expect(findSshTargetByName(SSH_TARGETS, 'openclaw')?.id).toBe('ssh-1755000000000-a1b2c3')
expect(findSshTargetByName(SSH_TARGETS, 'ssh-1755000000000-a1b2c3')?.label).toBe('openclaw')
@@ -61,7 +59,6 @@ describe('crossKindNextSteps', () => {
expect(crossKindNextSteps('nowhere', alternatives, 'ssh')).toEqual([])
})
// Why: suggesting the axis the caller already used would be noise, not help.
it('does not suggest the axis that was already requested', () => {
expect(crossKindNextSteps('openclaw', alternatives, 'ssh')).toEqual([])
expect(crossKindNextSteps('awin', alternatives, 'environment')).toEqual([])
@@ -75,8 +72,6 @@ describe('resolveSshHostTargetId', () => {
).resolves.toBe('ssh-1755000000000-a1b2c3')
})
// Why: this used to answer ok:true with an empty list — the same silent wrong-machine result
// that unknown runtime ids gave before they were rejected.
it('rejects an unknown target instead of letting it filter to nothing', async () => {
await expect(
resolveSshHostTargetId(clientReturning(SSH_TARGETS), 'nowhere', ENVIRONMENTS)
@@ -103,8 +98,6 @@ describe('resolveSshHostTargetId', () => {
})
describe('listSshTargets', () => {
// Why: an older host answers listTargets but not listTargetSummaries. Treating that as "no
// targets" would reject an ssh id that is valid on that host.
it('falls back to the older listing when the newer method is absent', async () => {
const { RuntimeClientError } = await import('./runtime/types.js')
const call = vi.fn(async (method: string) => {
@@ -136,8 +129,6 @@ describe('listSshTargets', () => {
expect(call).toHaveBeenCalledWith('ssh.getState', { targetId: SSH_TARGETS[0].id })
})
// Why: this only ever runs to enrich an error we are already reporting; a failure here must
// not replace that error with a confusing one about SSH enumeration.
it('returns nothing rather than masking the error it was enriching', async () => {
const client = {
call: vi.fn(async () => {
@@ -159,8 +150,6 @@ describe('ambiguous names never resolve silently', () => {
{ id: 'env-2', name: 'awin' }
]
// Why: picking the first would choose a machine on the caller's behalf — the exact failure the
// whole selector path exists to prevent.
it('refuses to guess between two ssh targets sharing a label', () => {
expect(findSshTargetByName(twoOpenclaw, 'openclaw')).toBeUndefined()
expect(ambiguousSshTargets(twoOpenclaw, 'openclaw')).toHaveLength(2)
@@ -171,7 +160,6 @@ describe('ambiguous names never resolve silently', () => {
expect(ambiguousEnvironments(twoAwin, 'awin')).toHaveLength(2)
})
// An exact id is never ambiguous, even when labels collide.
it('still resolves an exact id past a colliding label', () => {
expect(findSshTargetByName(twoOpenclaw, 'ssh-2-b')?.id).toBe('ssh-2-b')
expect(findEnvironmentByName(twoAwin, 'env-2')?.id).toBe('env-2')
+2 -29
View File
@@ -10,7 +10,6 @@ export type SshTargetSummary = {
}
export type EnvironmentSummary = { id: string; name: string }
/** The name `orca host list` prints for its own `--host local` row. */
export const LOCAL_HOST_LIST_NAME = 'this machine'
export type HostAlternatives = {
@@ -18,9 +17,6 @@ export type HostAlternatives = {
environments: readonly EnvironmentSummary[]
}
// Why: SSH target ids are machine-generated (`ssh-<timestamp>-<random>`), so the name a person
// or an agent actually knows is the label. Looking a name up as an id alone reports "not found"
// for a target that is sitting right there.
export function findSshTargetByName(
targets: readonly SshTargetSummary[],
name: string
@@ -30,14 +26,9 @@ export function findSshTargetByName(
return byId
}
const byLabel = matchesByLabel(targets, name)
// Why: two targets can share a label. Picking the first would silently choose a machine for
// the caller — the failure this whole selector path exists to prevent — so an ambiguous name
// resolves to nothing and the caller is told to use an id.
return byLabel.length === 1 ? byLabel[0] : undefined
}
// Why: cmd.exe passes single quotes through literally, so `--host 'my box'` from a POSIX habit
// arrives quoted; a host or project name never legitimately starts and ends with the same quote.
export { normalizeTypedName, stripWrappingQuotes }
function matchesByLabel(targets: readonly SshTargetSummary[], name: string): SshTargetSummary[] {
@@ -62,8 +53,6 @@ export function findEnvironmentByName(
return byId
}
const byName = matchesByEnvironmentName(environments, name)
// Why: the environment store itself refuses an ambiguous name rather than guessing; resolving
// one here would quietly reintroduce the guess it exists to prevent.
return byName.length === 1 ? byName[0] : undefined
}
@@ -83,9 +72,6 @@ export function ambiguousEnvironments(
return byName.length > 1 ? byName : []
}
// Why: a paired Orca server and an SSH target are different machines reached different ways, but
// a caller only knows "the machine called X". When X misses on one axis, the useful answer is
// almost always that it exists on the other — so say which, and give the exact flag.
export function crossKindNextSteps(
name: string,
alternatives: HostAlternatives,
@@ -107,11 +93,7 @@ export function crossKindNextSteps(
return steps
}
/**
* The host's SSH target summaries; throws what the host reports. Why the fallback: hosts
* predating listTargetSummaries still answer listTargets from the same summariser, and without it
* an old host looks like one with no SSH targets, rejecting a target id that is valid there.
*/
/** Older hosts expose summaries through listTargets; propagate errors other than method_not_found. */
export async function fetchSshTargetSummaries<T extends { id: string }>(
client: RuntimeClient
): Promise<{ targets: T[]; legacy: boolean }> {
@@ -127,8 +109,7 @@ export async function fetchSshTargetSummaries<T extends { id: string }>(
}
}
// Why: only display identity crosses this boundary — the RPC deliberately withholds addresses
// and credentials — and an enumeration failure must never mask the error we are explaining.
// Best-effort discovery avoids replacing selector errors with SSH enumeration failures.
export async function listSshTargets(client: RuntimeClient): Promise<SshTargetSummary[]> {
try {
const { targets, legacy } = await fetchSshTargetSummaries<SshTargetSummary>(client)
@@ -166,19 +147,11 @@ async function enrichLegacySshTargetStates(
)
}
// Why: `--host ssh:<id>` was never validated, so an unknown target answered ok:true with an
// empty list — the same silent wrong-machine answer that `runtime:` ids used to give. And since
// target ids are machine-generated (`ssh-<timestamp>-<random>`), the label a caller actually
// knows never matches one, so this fires on the common spelling rather than a rare typo.
// Resolving the label and naming the alternative is what makes the failure recoverable.
// `flagSpelling` names the flag the way the caller typed it, so a bare `--host trader-local`
// is not echoed back as `--host ssh:trader-local` — a spelling they did not use.
export async function resolveSshHostTargetId(
client: RuntimeClient,
targetId: string,
environments: readonly EnvironmentSummary[],
flagSpelling = `--host ssh:${targetId}`,
// A bare name was also tried as this machine and as a paired server, so say "host".
bareName = false
): Promise<string> {
const targets = await listSshTargets(client)
-1
View File
@@ -15,7 +15,6 @@ import { main } from './index'
import { okFixture } from './test-fixtures'
import { RuntimeRpcFailureError } from './runtime/types'
// What a headless server (`orca serve`, orcad) answers today: the bare token, flattened (#29822).
function headlessRefusal(): RuntimeRpcFailureError {
return new RuntimeRpcFailureError({
id: 'req_open',
@@ -50,7 +50,6 @@ import { pairRuntimeEnvironment, useWorktreeAwarenessEnvironment } from './index
const SSH_TARGET = { id: 'ssh-1777360569033-yvz2mp', label: 'openclaw', remotePlatform: 'win32' }
/** Every SSH-target lookup answers with the one target only this machine's runtime knows about. */
function queueSshTargetLookups(count: number): void {
queueFixtures(
callMock,
@@ -58,11 +57,7 @@ function queueSshTargetLookups(count: number): void {
)
}
/**
* A runtime that answers `status.get` with the name it currently publishes: the stored override
* when one is set, else the detected name. `settings.update` replies with the real `{ settings }`
* envelope so a handler reading a bare `machineName` off it prints `undefined`.
*/
/** Mirror the settings.update envelope and publish the effective name through status.get. */
function fakeMachineNameRuntime(detectedName: string): { updates: unknown[] } {
const updates: unknown[] = []
let override = ''
@@ -114,7 +109,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
expect(
printed.result.hosts.find((host: { id: string }) => host.id === SSH_TARGET.id).platform
).toBe('win32')
// The tell: `runtimeId: local` is only honest if no routed client was ever built.
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(null, null)
})
@@ -131,10 +125,8 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
.slice(0, 3)
.map((call) => JSON.parse(String(call[0])))
expect(first.result).toEqual({ machineName: 'm4airs-Air', platform: 'darwin' })
// The write reply is `{ settings }`; the printed name must be what the runtime publishes now.
expect(second.result).toEqual({ machineName: 'build-server', platform: 'darwin' })
expect(second._meta.runtimeId).toBe('runtime-1')
// A blank `--name` returns to the detected name — and prints it, not an empty string.
expect(third.result.machineName).toBe('m4airs-Air')
expect(logSpy.mock.calls[3]?.[0]).toBe('m4airs-Air (darwin)')
expect(runtime.updates).toEqual([{ machineName: ' build-server ' }, { machineName: '' }])
@@ -156,8 +148,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(undefined, 'm4air')
})
// Why: the pre-check matches names loosely but the connection lookup is exact, so `build` for a
// server saved as `Build` passed the check and then failed with "Unknown environment: build".
it('connects to the server an --environment name in another case resolved to', async () => {
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'M4Air')
fakeMachineNameRuntime('M4 Air')
@@ -169,7 +159,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(undefined, 'env-m4air')
})
/** The selectors an agent's environment carries: set empty, which the CLI reads as unset. */
function agentSelectors(): void {
process.env.ORCA_ENVIRONMENT = ''
process.env.ORCA_PAIRING_CODE = ''
@@ -203,8 +192,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
})
it('routes `host name` through an ambient ORCA_ENVIRONMENT, unlike the pinned `host list`', async () => {
// Why: the pin used to cover the whole `host` family, which silently answered for this machine
// when the shell was pointed at another one. `host name` describes one runtime, so it routes.
process.env.ORCA_ENVIRONMENT = 'm4air'
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'm4air')
fakeMachineNameRuntime('M4 Air')
@@ -217,14 +204,12 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
expect(printed.ok).toBe(true)
expect(printed.result.machineName).toBe('M4 Air')
expect(printed._meta.runtimeId).toBe('runtime-1')
// The ambient selector is left for the client to honour; a `null` here would pin it local.
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(undefined, undefined)
expect(runtimeClientConstructorMock).not.toHaveBeenCalledWith(null, null)
})
it('refuses to rename a runtime that does not publish a machine name', async () => {
// Why: an older runtime's strict settings schema answers the write with a bare `invalid_params`;
// the field's absence from status is the tell, so the CLI refuses before writing anything.
// Missing status support lets the CLI reject the rename before an older settings schema rejects it.
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'm4air')
callMock.mockImplementation(async (method: string) => {
if (method === 'status.get') {
@@ -263,9 +248,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
})
it('rejects `host list --environment` instead of answering with a half-routed listing', async () => {
// Why: pre-fix this routed the SSH lookup to m4air while reading paired servers from this
// machine, dropped the openclaw row, and still stamped `_meta.runtimeId: "local"` — one
// listing describing two hosts, which reads as "m4air has no SSH targets".
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'm4air')
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
runtimeClientConstructorMock.mockClear()
@@ -317,8 +299,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
})
it('keeps `host list` local when ORCA_ENVIRONMENT is set ambiently', async () => {
// Why: the ambient variable produced the same two-machine listing as the explicit flag, with
// no flag to reject. Pinning the family is what makes `runtimeId: local` true in both cases.
process.env.ORCA_ENVIRONMENT = 'm4air'
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'm4air')
queueSshTargetLookups(1)
@@ -336,7 +316,6 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
})
it('still treats --environment as the selector argument on `environment show` and `rm`', async () => {
// Why: the guard must not fire where the flag names the row to act on rather than a route.
const environment = {
id: 'env-m4air',
name: 'm4air',
-1
View File
@@ -113,7 +113,6 @@ describe('orca cli worktree awareness', () => {
expect(output).toContain('hostAvailable: 2.0 MB (free-memory)')
expect(output).toContain('app: 1.0 MB')
expect(output).toContain('- feature 1.0 MB 2.5% 1 session')
// Back-compat: a host that never heard of committed bytes prints none.
expect(output).not.toContain('totalPrivateMemory')
expect(output).not.toContain('processCommitMetric')
})
@@ -72,9 +72,6 @@ describe('omittedHostIds selector annotation', () => {
})
it('marks a stale runtime host that no caller can select', async () => {
// Why: `omittedHostIds` is built from the runtime's own bookkeeping, so it names `runtime:`
// ids for servers that are no longer paired. An agent looping over the list to complete a
// partial listing hard-errors on those — 6 of 9 in the recorded QA run.
pairRuntimeEnvironment(listEnvironmentsMock, 'env-paired', 'm4air')
queueFixtures(
callMock,
@@ -133,8 +130,6 @@ describe('omittedHostIds selector annotation', () => {
] as const)(
'only suggests flags that `%s %s` accepts',
async (group, action, requestId, rows) => {
// Why: `--host local` / `--host ssh:<id>` were suggested here, and the listing rejected
// them as unknown flags, breaking the agent loop the annotation exists for (#29901).
pairRuntimeEnvironment(listEnvironmentsMock, 'env-paired', 'm4air')
queueFixtures(
callMock,
@@ -166,7 +161,7 @@ describe('omittedHostIds selector annotation', () => {
expect(spec?.allowedFlags).toContain(selector.split(' ')[0].replace(/^--/, ''))
}
}
// An SSH host is reached through the answering runtime, so no target lookup is needed.
// SSH hosts are reached through the answering runtime, without another target lookup.
expect(callMock).toHaveBeenCalledTimes(1)
}
)
@@ -192,8 +187,6 @@ describe('omittedHostIds selector annotation', () => {
})
it('never keeps a host id out of omittedHostIds', async () => {
// Why: filtering the unreachable ones would shrink what the listing admits it did not cover.
// The gap is real whether or not this machine can name the host that owns it.
listEnvironmentsMock.mockReturnValue([])
queueFixtures(
callMock,
-2
View File
@@ -133,7 +133,6 @@ describe('orca cli worktree awareness', () => {
})
it("refuses an agent session's caller flag naming another caller before any request", async () => {
// One chokepoint for every verb: the spec says which flag names the caller.
process.env.ORCA_AGENT_SESSION_ID = 'f7a1c0de-1111-4222-8333-444455556666'
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
try {
@@ -213,7 +212,6 @@ describe('orca cli worktree awareness', () => {
expect(callMock).not.toHaveBeenCalled()
expect(process.exitCode).toBe(1)
// Reset exitCode so subsequent tests don't inherit the failure.
process.exitCode = priorExitCode
errSpy.mockRestore()
})
@@ -54,8 +54,6 @@ const ORCA_PROJECT = {
}
const PROJECT_LIST_FIXTURE = okFixture('req_project_list', { projects: [ORCA_PROJECT] })
// Why: #30367 — the names `orca project list` and `orca host list` print were rejected by the
// commands that take --project and --host; only the generated ids worked.
describe('orca cli project and host names', () => {
useWorktreeAwarenessEnvironment({
callMock,
@@ -66,8 +64,6 @@ describe('orca cli project and host names', () => {
spawnMock
})
// Why: the issue's own repro — the names `orca project list` and `orca host list` print were
// rejected, and only the generated ids worked.
it('filters project setups by the display name and host name the list commands print', async () => {
queueFixtures(
callMock,
@@ -162,8 +158,6 @@ describe('orca cli project and host names', () => {
)
})
// Why: a project whose only setup is not ready must not fall through to another project whose
// display name happens to equal the id that was typed.
it('keeps an exact project id even when another project is named that id', async () => {
queueFixtures(
callMock,
-15
View File
@@ -202,8 +202,6 @@ describe('orca cli worktree awareness', () => {
expect(logSpy.mock.calls[0]?.[0]).not.toContain('setup-by-client')
})
// Why: --host runtime:<id> routes to a paired server, so an older one is reachable without the
// caller meaning to. A raw method_not_found reads as an Orca bug rather than a version gap.
it('names the version gap when the server predates project host setup', async () => {
pairRuntimeEnvironment(listEnvironmentsMock, 'old-server')
const { RuntimeClientError } = await import('./runtime/types.js')
@@ -231,11 +229,9 @@ describe('orca cli worktree awareness', () => {
await main(['project', 'setups', '--host', 'runtime:not-a-real-env', '--json'], '/tmp/repo')
// The command itself never reached a runtime; only the suggestion lookup did.
expect(callMock).not.toHaveBeenCalledWith('projectHostSetup.list')
const printed = [...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n')
expect(printed).toContain('no paired Orca server is named or has id not-a-real-env')
// An agent reads the code and the retry candidates, not the prose.
expect(JSON.parse(printed).error.code).toBe('invalid_argument')
expect(JSON.parse(printed).error.data.knownEnvironments).toEqual([])
expect(process.exitCode).toBe(1)
@@ -243,8 +239,6 @@ describe('orca cli worktree awareness', () => {
process.exitCode = priorExitCode
})
// Why: ssh: was never validated, so an unknown target answered ok:true with an empty list —
// the same silent wrong-machine answer unknown runtime ids used to give.
it('rejects an unknown ssh host instead of answering empty', async () => {
queueFixtures(callMock, okFixture('req_ssh_targets', { targets: [] }))
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
@@ -307,9 +301,6 @@ describe('orca cli worktree awareness', () => {
expect(logSpy.mock.calls[0]?.[0]).toContain('setup-openclaw')
})
// Why: `runtime:<id>` is a persisted token — it lands in ProjectHostSetup.hostId and is
// embedded in generated setup ids. Accepting a name is only safe because it is canonicalized
// to the id before anything downstream sees it; this pins that.
it('never lets an environment name reach a persisted host id', async () => {
pairRuntimeEnvironment(listEnvironmentsMock, 'env-uuid-1', 'awin')
queueFixtures(
@@ -495,8 +486,6 @@ describe('orca cli worktree awareness', () => {
})
it('rejects SSH project setup relative paths, which name the client filesystem', async () => {
// A local CLI reaching an `ssh:*` host is still off-client: resolving `./orca` against the
// CLI cwd would register a path that exists on the wrong machine.
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const priorExitCode = process.exitCode
@@ -593,9 +582,6 @@ describe('orca cli worktree awareness', () => {
}
)
// Why: STA-4792 defect 2. `--host runtime:<id>` used to leave the client local, so a Windows
// destination fell into resolve(cwd, ...) and became a literal directory next to the caller.
// Routing makes the client remote, which is what sends the path through untouched.
it('sends a windows destination to the routed host instead of joining it to the local cwd', async () => {
pairRuntimeEnvironment(listEnvironmentsMock, 'awin')
queueFixtures(
@@ -794,7 +780,6 @@ describe('orca cli worktree awareness', () => {
it.each([
{ flags: [], force: false, hostId: undefined },
{ flags: ['--force'], force: true, hostId: undefined },
// Why: a project on two hosts has two setups with one id; --host names the twin.
{ flags: ['--host', 'local'], force: false, hostId: 'local' }
])('deletes project host setup metadata with $flags', async ({ flags, force, hostId }) => {
queueFixtures(
-1
View File
@@ -73,7 +73,6 @@ describe('orca serve restore resume', () => {
expect(serveOrcaAppMock).not.toHaveBeenCalled()
expect(callMock).toHaveBeenCalledWith('runtime.automaticWork.resume')
// Why null: `serve` commands always target this machine, never an ambient paired host.
expect(runtimeClientConstructorMock).toHaveBeenLastCalledWith(null, null)
expect(logSpy.mock.calls[0]?.[0]).toContain('Automatic restores resumed')
expect(logSpy.mock.calls[0]?.[0]).toContain('stopped unexpectedly')
@@ -89,8 +89,7 @@ describe('orca terminal list host scope', () => {
expect(printed.result.hostScope).toEqual({
hostIds: ['ssh:box-1'],
omittedHostIds: ['local'],
// `terminal list` takes no --host, so no flag reaches the answering runtime's own hosts
// (#29901); see index-omitted-host-scope-selectors.test.ts.
// These hosts are reached through the answering runtime; listing takes no --host flag.
omittedHostSelectors: [{ hostId: 'local', selector: null }]
})
})
+2 -8
View File
@@ -17,10 +17,7 @@ export type WorktreeAwarenessMocks = {
}
export async function createRuntimeClientModuleMock(mocks: RuntimeClientModuleMocks) {
// 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 the mock must preserve the production classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('./runtime/types.js')
class RuntimeClient {
@@ -83,7 +80,6 @@ export async function createChildProcessModuleMock(spawnMock: Mock) {
}
}
/** Registers a paired environment so `--host runtime:<id>` routes there instead of being rejected. */
export function pairRuntimeEnvironment(listEnvironmentsMock: Mock, id: string, name = id): void {
listEnvironmentsMock.mockReturnValue([
{
@@ -108,7 +104,6 @@ export function pairRuntimeEnvironment(listEnvironmentsMock: Mock, id: string, n
])
}
/** Installs the env-var save/restore + mock-reset hooks shared by the CLI worktree-awareness suites. */
export function useWorktreeAwarenessEnvironment(mocks: WorktreeAwarenessMocks): void {
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalUserDataPath = process.env.ORCA_USER_DATA_PATH
@@ -126,8 +121,7 @@ export function useWorktreeAwarenessEnvironment(mocks: WorktreeAwarenessMocks):
delete process.env.ORCA_DEV_CLI_INVOCATION
delete process.env.ORCA_WORKSPACE_ID
delete process.env.ORCA_WORKTREE_ID
// Isolate the pane key so claude-teams tests that set it don't leak a
// senderPaneKey into later orchestration.send assertions.
// Prevent claude-teams tests from leaking senderPaneKey into later assertions.
delete process.env.ORCA_PANE_KEY
mocks.serveOrcaAppMock.mockReset()
mocks.getDefaultUserDataPathMock.mockClear()
-3
View File
@@ -251,7 +251,6 @@ describe('orca cli worktree awareness', () => {
expect.objectContaining({ id: 'recipe.destroy.run', status: 'pass' })
])
)
// The transcript carries both stages so the agent can self-diagnose.
expect(output.provisionTranscript?.provision.exitCode).toBe(0)
expect(output.provisionTranscript?.destroy?.exitCode).toBe(0)
const cleanupPayload = JSON.parse(
@@ -288,7 +287,6 @@ describe('orca cli worktree awareness', () => {
stdin: { write: vi.fn(), end: vi.fn() },
kill: vi.fn()
})
// create emits a non-JSON line to stdout + a real diagnostic to stderr, then exits 0
spawnMock.mockImplementationOnce(() => {
process.nextTick(() => {
startChild.stdout.emit('data', Buffer.from('Provisioning sandbox...\n'))
@@ -330,7 +328,6 @@ describe('orca cli worktree awareness', () => {
expect.objectContaining({ id: 'recipe.provision', status: 'fail' })
])
)
// The agent gets the full create output, not a 500-char tail.
expect(output.provisionTranscript?.provision.stdout).toContain('Provisioning sandbox...')
expect(output.provisionTranscript?.provision.stderr).toContain('missing scope')
expect(output.provisionTranscript?.provision.parseError).toBeTruthy()
@@ -80,7 +80,6 @@ describe('orca cli worktree awareness', () => {
comment: undefined,
runHooks: true,
activate: true,
// CLI activation targets the host desktop.
navigation: 'host',
parentWorktree: undefined,
cwdParentWorktree: 'id:repo-1::/tmp/repo',
+1 -3
View File
@@ -145,9 +145,7 @@ describe('orca cli worktree awareness', () => {
linkedIssue: undefined,
linkedLinearIssue: 'STA-335',
linkedLinearIssueWorkspaceId: null,
// Why: a bare identifier carries no org, and the two scoping fields
// describe the issue being replaced. Inheriting a stale org key would
// pin the lookup to a workspace this issue does not live in.
// A bare identifier supplies no org; inheriting the replaced issue's org would misroute lookup.
linkedLinearIssueOrganizationUrlKey: null,
comment: undefined,
runHooks: false,
@@ -56,7 +56,7 @@ function buildSetup(overrides: { id: string; hostId: string; repoId: string; pat
}
}
// Why one repo id on two setups: a project copied between hosts keeps its id (Codex review 6 #6).
// Copies on different hosts retain the same repo id.
const TWIN_SETUPS = [
buildSetup({ id: 'setup-1', hostId: 'local', repoId: 'repo-1', path: '/tmp/orca' }),
buildSetup({ id: 'setup-1', hostId: 'runtime:gpu', repoId: 'repo-1', path: '/srv/orca' })
@@ -233,9 +233,6 @@ describe('orca cli worktree awareness', () => {
)
})
// Why: swallowing every project.list failure made a network blip or a permission error
// indistinguishable from "no project by that name", and the caller was then told their project
// was not set up on any host.
it('surfaces a project.list failure instead of reporting the project as not set up', async () => {
const { RuntimeClientError } = await import('./runtime/types.js')
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
@@ -265,8 +262,6 @@ describe('orca cli worktree awareness', () => {
process.exitCode = priorExitCode
})
// Why: the legacy fallback matches a repo folder name, and two projects can hold repos with the
// same folder name. Selecting either would target a project the caller never named.
it('refuses a legacy display-name match that spans two projects', async () => {
const { RuntimeClientError } = await import('./runtime/types.js')
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
@@ -312,7 +307,6 @@ describe('orca cli worktree awareness', () => {
process.exitCode = priorExitCode
})
// Only a version gap earns the weaker match against the setup's own display name.
it('falls back to the setup display name when the server has no project.list', async () => {
const { RuntimeClientError } = await import('./runtime/types.js')
queueFixtures(
@@ -354,9 +348,6 @@ describe('orca cli worktree awareness', () => {
)
})
// Why: the reported failure. `orca project list` shows the display name, so that is what gets
// typed; matching the provider-scoped projectId alone reported "not set up" for a project that
// was sitting right there, and blamed the missing --host for it.
it('resolves --project by display name and infers the only host it is set up on', async () => {
queueFixtures(
callMock,
@@ -397,7 +388,6 @@ describe('orca cli worktree awareness', () => {
)
})
// Why: a project that matched nothing used to be reported as a host problem.
it('reports an unknown project as a project miss, not a host one', async () => {
queueFixtures(
callMock,
@@ -420,8 +410,6 @@ describe('orca cli worktree awareness', () => {
process.exitCode = priorExitCode
})
// Why: one host is not a choice. Two are, and picking one would create the worktree on a
// machine the caller never named.
it('asks which host only when the project is set up on more than one', async () => {
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
@@ -470,8 +458,6 @@ describe('orca cli worktree awareness', () => {
process.exitCode = priorExitCode
})
// Why: `orca host list` prints `trader-local`, so that is what gets typed. Only the generated
// `ssh:ssh-<timestamp>-<random>` spelling used to be accepted.
it('accepts the host alias that `orca host list` prints', async () => {
queueFixtures(
callMock,
@@ -589,8 +575,6 @@ describe('orca cli worktree awareness', () => {
})
it('marks every worktree.create as CLI-created even from an external shell', async () => {
// Why: the sidebar badge/filter must catch hand-typed creates too, so the
// provenance request is sent with no terminal handle rather than omitted.
delete process.env.ORCA_TERMINAL_HANDLE
queueFixtures(
callMock,
@@ -100,9 +100,7 @@ describe('orca cli worktree awareness', () => {
})
it('resolves the invocation cwd from ORCA_CLI_CWD when no cwd is passed', async () => {
// Why: the SSH relay bridge runs the CLI on the Orca host with the remote
// shell's cwd carried in ORCA_CLI_CWD (#7716); cwd-based selectors must
// resolve against it, not the host process cwd.
// ORCA_CLI_CWD carries the remote shell's cwd when the bridge executes the CLI on the host.
process.env.ORCA_CLI_CWD = '/tmp/repo/feature/src'
try {
queueFixtures(
+1 -1
View File
@@ -449,7 +449,7 @@ describe('orca cli worktree awareness', () => {
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const priorExitCode = process.exitCode
// `active` resolves through worktree.list, so any RPC here means validation ran too late.
// Resolving active makes an RPC, so no calls proves validation ran first.
await main(['worktree', 'set', '--worktree', 'active', ...flags, '--json'], '/tmp/repo')
expect(callMock).not.toHaveBeenCalled()
+2 -8
View File
@@ -46,7 +46,7 @@ import { okFixture, queueFixtures } from './test-fixtures'
describe('COMMAND_SPECS collision check', () => {
it('has no duplicate command or alias paths', () => {
// Why: first-match resolution would silently shadow duplicate aliases.
// First-match resolution would silently shadow duplicate aliases.
const seen = new Set<string>()
for (const spec of COMMAND_SPECS) {
for (const path of specPaths(spec)) {
@@ -82,8 +82,7 @@ describe('command aliases dispatch to the canonical handler', () => {
afterEach(() => {
callMock.mockReset()
// Why: restore console.log so a downstream describe's vi.spyOn starts from a
// clean spy — otherwise this block's --json output leaks into its calls[0].
// Restore the spy so later describe blocks cannot inherit these calls.
logSpy.mockRestore()
})
@@ -148,8 +147,6 @@ describe('command aliases dispatch to the canonical handler', () => {
}
})
// #19334: a failed archive hook blocks removal, so the CLI must exit non-zero rather than
// report a delete that did not happen — and the waiver must ride its own flag, never --force.
it('exits non-zero when worktree removal is refused by a failed archive hook', async () => {
queueFixtures(callMock, okFixture('req_show', { worktree: { hostId: 'local' } }))
callMock.mockRejectedValueOnce(
@@ -179,7 +176,6 @@ describe('command aliases dispatch to the canonical handler', () => {
}
})
// #19334 S4: the waiver only applies to a hook that ran, so alone it silently does nothing.
it('rejects the archive-hook waiver without --run-hooks instead of ignoring it', async () => {
queueFixtures(callMock, okFixture('req_show', { worktree: { hostId: 'local' } }))
const priorExitCode = process.exitCode
@@ -191,7 +187,6 @@ describe('command aliases dispatch to the canonical handler', () => {
)
expect(process.exitCode).toBe(1)
// The removal must never have been attempted.
expect(callMock).not.toHaveBeenCalledWith('worktree.rm', expect.anything())
} finally {
process.exitCode = priorExitCode
@@ -240,7 +235,6 @@ describe('command aliases dispatch to the canonical handler', () => {
runtimeClientConstructorMock.mockClear()
await main(['agent-context', '--json'], '/tmp/repo')
// Why: pure local read — proves the SSH/offline property (no RPC).
expect(runtimeClientConstructorMock).not.toHaveBeenCalled()
expect(callMock).not.toHaveBeenCalled()
const schema = JSON.parse(String(logSpy.mock.calls.at(-1)?.[0]))
+11 -37
View File
@@ -32,9 +32,7 @@ function shouldIgnoreRemoteSelection(commandPath: string[]): boolean {
commandPath[0] === 'account' ||
commandPath[0] === 'artifacts' ||
commandPath[0] === 'environment' ||
// Why: `host list` answers "what can this machine target, and with what flag". Half of that
// answer (paired servers) is read from this machine's own pairing store and cannot be routed,
// so routing the other half produced one listing describing two machines at once.
// Paired servers come from the local store; routing SSH lookup would mix two machines in one list.
commandPath.join(' ') === 'host list' ||
commandPath[0] === 'serve' ||
(commandPath[0] === 'agent' && commandPath[1] !== 'launch') ||
@@ -44,19 +42,12 @@ function shouldIgnoreRemoteSelection(commandPath: string[]): boolean {
)
}
// Why: the RuntimeClient graph is 153 of the CLI's 199 eager modules (zod via
// shared/pairing, ws + tweetnacl via websocket-transport). Loading it here
// rather than at module scope means --help, `help <cmd>`, and command/flag
// errors — which all return before this call — never pay for it. Awaited
// before dispatch so `ctx.client` stays a synchronous getter.
// Defer the runtime graph until after help and validation; await it to keep ctx.client synchronous.
async function loadRuntimeClientClass(): Promise<typeof RuntimeClient> {
return (await import('./runtime-client.js')).RuntimeClient
}
// Why: the SSH relay bridge executes this CLI on the Orca host while the
// caller's shell cwd lives on the remote machine (which cannot be chdir'd
// into). ORCA_CLI_CWD carries that remote cwd so cwd-based selectors like
// `--worktree active` resolve against the caller's directory.
// The SSH bridge runs here, but ORCA_CLI_CWD carries the remote caller's directory.
function resolveInvocationCwd(): string {
const override = process.env.ORCA_CLI_CWD
return typeof override === 'string' && override.length > 0 ? override : process.cwd()
@@ -108,9 +99,7 @@ export async function main(
const json = parsed.flags.has('json')
try {
// Why: CLI syntax and flag errors should be reported before any runtime
// lookup so users do not get misleading "Orca is not running" failures for
// simple command typos or unsupported flags.
// Report invalid commands before runtime lookup can mask them with connection errors.
validateCommandAndFlags(COMMAND_SPECS, parsed)
const commandSpec = findCommandSpec(COMMAND_SPECS, parsed.commandPath)
refuseConflictingSessionCallerFlags(commandSpec, parsed.flags)
@@ -118,27 +107,18 @@ export async function main(
const ignoreRemoteSelection = shouldIgnoreRemoteSelection(parsed.commandPath)
const pairingCode = ignoreRemoteSelection ? null : parsed.flags.get('pairing-code')
const environmentSelector = ignoreRemoteSelection ? null : parsed.flags.get('environment')
// Why: only the explicit flag is asserted eagerly. An ambient ORCA_ENVIRONMENT is background
// config, and failing local-only commands because of a stale one would be a regression; the
// explicit flag means the caller named that machine, so a bad name should fail immediately
// with the cross-kind hint rather than a bare store error at first use.
// Eagerly resolve explicit --environment; stale ambient config must not break local-only commands.
const listSshTargetsForSuggestion = async (): Promise<{ id: string; label: string }[]> =>
listSshTargets(new RuntimeClientClass(undefined, undefined, null, null))
const storeEnvironmentSelector =
typeof environmentSelector === 'string'
? await resolveEnvironmentSelectorForStore(environmentSelector, listSshTargetsForSuggestion)
: null
// Why: --host runtime:<id> names a paired server, not a filter over this
// runtime's rows, so it has to pick the connection before the client exists.
// An ambient ORCA_ENVIRONMENT is checked for disagreement too — silently
// retargeting a mutation to another server is the bug this flag already had.
// An ambient pairing code cannot be resolved to an id to compare, so the
// explicit flag simply wins there.
// Select the paired connection before constructing the client; reject conflicting environments.
const hostEnvironmentId = ignoreRemoteSelection
? null
: await resolveHostFlagEnvironmentId(parsed.flags, {
// Why: only consulted when the name missed, and against this machine's own runtime —
// SSH targets are registered there, not in the paired server we failed to find.
// SSH targets belong to this machine, even when the missing name was a paired server.
listSshTargets: listSshTargetsForSuggestion,
verifySshTargets: async () =>
(
@@ -154,18 +134,13 @@ export async function main(
? { value: process.env.ORCA_ENVIRONMENT, label: 'ORCA_ENVIRONMENT' }
: null
})
// Why: --host runtime:<name> is canonicalized to the environment's id so downstream host-id
// comparisons against stored rows still match; rewrite the flag once, here, rather than
// resolving the name again at every consumer.
// Stored host identities use ids, so canonicalize names before any downstream comparison.
if (hostEnvironmentId !== null) {
parsed.flags.set('host', `runtime:${hostEnvironmentId}`)
}
// Why: pass `null` (not `undefined`) when remote selection is suppressed
// so the RuntimeClient default parameter does not re-activate the
// ORCA_PAIRING_CODE / ORCA_ENVIRONMENT env-var fallback for commands
// that must run locally (environment / serve).
// null suppresses RuntimeClient's ORCA_PAIRING_CODE / ORCA_ENVIRONMENT default parameters.
const suppressed = ignoreRemoteSelection ? null : undefined
// An explicit --host runtime:<id> outranks an ambient pairing code or environment.
// An explicit runtime host overrides an ambient pairing code.
const remotePairingCode =
hostEnvironmentId !== null ? null : typeof pairingCode === 'string' ? pairingCode : suppressed
const remoteEnvironment = hostEnvironmentId ?? storeEnvironmentSelector ?? suppressed
@@ -208,8 +183,7 @@ export async function main(
async function runClaudeTeams(argv: string[], cwd: string): Promise<void> {
try {
// Why: everything after `orca claude-teams` belongs to Claude Code, not
// Orca's own flag parser, so new Claude flags work without Orca changes.
// Forward Claude Code arguments verbatim so new flags work without Orca changes.
const client = new (await loadRuntimeClientClass())(undefined, undefined, null, null)
await dispatch(['claude-teams'], {
flags: new Map(),
-1
View File
@@ -99,7 +99,6 @@ export function formatJiraCommentList(comments: JiraComment[]): string {
.join('\n\n')
}
// Why: the handler throws on ok:false, so only successful creates reach here.
export function formatJiraCreate(result: Extract<JiraCreateIssueResult, { ok: true }>): string {
return [`Created ${result.key}`, `URL: ${result.url}`].join('\n')
}
+5 -21
View File
@@ -8,11 +8,11 @@ import { findEnvironmentByName } from './host-selector-alternatives'
export type OmittedHostScopeSelector = {
hostId: ExecutionHostId
/** The flag that routes a follow-up query to this host, or null when it names nothing here. */
/** null means this machine has no selector for the omitted host. */
selector: string | null
}
/** A host scope annotated on this machine. The runtime never sends `omittedHostSelectors`. */
/** omittedHostSelectors is added locally, never sent by the runtime. */
export type ListingHostScopeWithSelectors = RuntimeListingHostScope & {
omittedHostSelectors?: OmittedHostScopeSelector[]
}
@@ -21,23 +21,7 @@ export type WithAnnotatedHostScope<TResult> = Omit<TResult, 'hostScope'> & {
hostScope?: ListingHostScopeWithSelectors
}
/**
* Resolves how to reach each host a listing did not cover.
*
* `hostScope` is the documented way to complete a partial listing, but `omittedHostIds` is built
* from the runtime's own bookkeeping — repos, folder workspaces, and workspace sessions — so it
* names `runtime:` ids for servers that are no longer paired. An agent looping over the list to
* finish the job hard-errors on those.
*
* The ids are kept rather than filtered: dropping one would shrink what the listing admits it did
* not cover, and `docs/reference/ssh-execution-boundary.md` requires a listing to name its gaps.
* A `null` selector marks the ones this machine cannot name, which is the part a caller needs.
* Only the local pairing store is consulted, so this answers "can I select it", never "is it up" —
* no host is claimed live or exited on this path.
*
* `local` and `ssh:` hosts get no selector: the listing commands take no `--host` (#29901), and
* the runtime that answered is the one that reaches them, so no flag would route elsewhere.
*/
// Preserve unreachable hosts as coverage gaps; pairing proves selectability, not liveness.
export async function resolveOmittedHostScopeSelectors(
omittedHostIds: readonly ExecutionHostId[]
): Promise<OmittedHostScopeSelector[]> {
@@ -73,10 +57,11 @@ function resolveSelector(
const environment = findEnvironmentByName(environments, host.environmentId)
return environment ? `--environment ${environment.name}` : null
}
// Listings take no --host flag; local and SSH hosts are reached through the answering runtime.
return null
}
/** Renders a scope line; an absent scope means the host never reported one, not full coverage. */
/** An absent scope means coverage is unknown. */
export function formatListingHostScope(scope: ListingHostScopeWithSelectors | undefined): string {
if (!scope) {
return 'scope: unverifiable — this host does not report which hosts it lists'
@@ -104,7 +89,6 @@ export function formatListingHostScope(scope: ListingHostScopeWithSelectors | un
return `scope: ${covered} — not covered: ${omitted.join(', ')}`
}
/** Attaches the resolved selectors in place; a listing with no omitted hosts pays nothing. */
export async function annotateOmittedHostScope(result: {
hostScope?: ListingHostScopeWithSelectors
}): Promise<void> {
@@ -12,9 +12,7 @@ afterEach(() => {
vi.restoreAllMocks()
})
// Why: these are the exact envelopes the RPC dispatcher test proved the runtime emits. This
// checkout's formatter never enumerates codes (verified below with a code no build has defined),
// which is what lets a client that predates a new code still print its message and nextSteps.
// Runtime envelopes may contain codes introduced after this client; formatting must preserve them.
describe('orchestration dispatch refusals through the CLI error boundary', () => {
it.each([
{
@@ -49,8 +47,6 @@ describe('orchestration dispatch refusals through the CLI error boundary', () =>
})
})
// Why: a code this build has never defined stands in for a future host's new code; if the
// formatter ever starts gating on known codes, this is the assertion that catches it.
it('prints an unknown code with its message and nextSteps unchanged', () => {
const failure: RuntimeRpcFailure = {
id: 'rpc_1',
@@ -315,7 +315,6 @@ describe('orchestration mutation recovery', () => {
expect((result.data as { recovery: object }).recovery).not.toHaveProperty('retryCommand')
})
// Pins the outcome-unknown codes (round-2 S5): only these get recovery guidance.
it.each([
['runtime_unavailable', true],
['remote_runtime_unavailable', true],
+3 -6
View File
@@ -29,8 +29,7 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
? [...originalCommand, '--retry-request', requestId]
: undefined
const executable = originalCommand?.[0] ?? resolveOrchestrationCliExecutable()
// Why: a lost response usually carries no dispatch id, and the old code then emitted
// no read-only step at all — leaving retry as the only way to learn what happened.
// Without a dispatch id, query by request id to inspect the outcome before retrying a mutation.
const queryCommand = dispatchId
? [executable, 'orchestration', 'worker-show', '--dispatch', dispatchId, '--json']
: [executable, 'orchestration', 'request-show', '--request', requestId, '--json']
@@ -65,8 +64,7 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
recovery,
nextSteps
}
// Preserve the RPC failure envelope so --json callers retain the request id and
// runtime metadata while receiving the structured recovery guidance.
// Preserve request id and runtime metadata for --json recovery.
if (error instanceof RuntimeRpcFailureError) {
return new RuntimeRpcFailureError({
...error.response,
@@ -195,8 +193,7 @@ function resolveRecoveryShell(
function quoteRecoveryArgument(value: string, shell: AgentStartupShell): string {
if (shell === 'cmd') {
// cmd has no single-quote syntax and expands %VAR% inside double quotes;
// use the argv encoder that keeps quote parity and escapes percent pairs.
// cmd expands %VAR% inside double quotes; its argv encoder must escape percent pairs too.
return quoteWindowsCmdArgument(value)
}
if (shell === 'powershell') {
@@ -1,16 +1,4 @@
/**
* A command that runs inside a structured agent session is that session: the injected
* `ORCA_AGENT_SESSION_ID` names the caller, and nothing resolves or guesses a terminal for it.
*
* One rule for every verb that names a caller: a caller flag may restate the session, but a flag
* naming anyone else is refused — never silently dropped, never allowed to win. A session address
* the CLI cannot place is left to the host, whose `/clear` lineage decides.
* The #21097 accident was a chat that named a sibling's terminal and consumed that sibling's mail.
*
* The session env here is the hardest case, a chat that inherited a pane's `ORCA_TERMINAL_HANDLE`
* and `ORCA_PANE_KEY` (an Orca launched from an Orca terminal), and the implicit-terminal guess has
* a sibling to find.
*/
// Inherited pane identity and an available sibling must never override the injected session caller.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@ -30,7 +18,7 @@ import { formatCliError, reportCliError } from './cli-error'
import { RuntimeRpcFailureError } from './runtime/types'
const SESSION = 'f7a1c0de-1111-4222-8333-444455556666'
/** The session a `/clear` continued as SESSION: the chat's orchestration address stays this one's. */
/** SESSION continues ROOT after /clear, retaining ROOT's orchestration address. */
const ROOT = '0b5e2d7c-9a41-4c3e-8f62-7d1a3e5b9c08'
const IDENTITY_ENV = [
'ORCA_AGENT_SESSION_ID',
@@ -40,7 +28,6 @@ const IDENTITY_ENV = [
] as const
const originalEnv = Object.fromEntries(IDENTITY_ENV.map((name) => [name, process.env[name]]))
/** Enough of every receipt shape that each handler finishes after its RPC. */
const RESULT = {
result: {
run: { id: 'run_1', objective: 'o', consumer_generation: 1 },
@@ -73,14 +60,12 @@ const RESULT = {
type Verb = {
command: string
flags: Record<string, string | true>
/** The flag that names the caller, when the verb has one. */
callerFlag?: 'from' | 'terminal'
method: string
callerParam: 'from' | 'terminal' | 'callerTerminalHandle'
}
/** Every verb whose request names its caller: the host's caller-param map, from the CLI side.
* `dispatch-show` is not one: its --from only fills preview text, so it is pinned on its own. */
/** dispatch-show is excluded: its --from fills preview text rather than naming the caller. */
const CALLER_VERBS: Verb[] = [
{
command: 'run-create',
@@ -175,11 +160,9 @@ const CALLER_VERBS: Verb[] = [
method: 'workerStart',
callerParam: 'from'
},
// No caller flag: its spec takes none. It asks runCurrent for the caller's Run.
{ command: 'worker-list', flags: {}, method: 'runCurrent', callerParam: 'from' }
]
/** Enough flags for any verb to get past its own validation to identity resolution. */
const EVERY_REQUIRED_FLAG = {
objective: 'o',
id: 'id_1',
@@ -199,7 +182,6 @@ function flagMap(flags: Record<string, string | true>): Map<string, string | boo
return new Map(Object.entries(flags))
}
/** What `main()` does between parsing and dispatch: the spec-driven caller check, then the handler. */
async function invoke(
command: string,
flags: Map<string, string | boolean>,
@@ -243,7 +225,6 @@ function setEnv(env: Partial<Record<(typeof IDENTITY_ENV)[number], string>>): vo
}
}
/** A chat with its id, plus a pane identity it inherited from the Orca that launched it. */
function asSessionWithInheritedPane(): void {
setEnv({
ORCA_AGENT_SESSION_ID: SESSION,
@@ -274,7 +255,6 @@ describe.each(CALLER_VERBS)('orchestration $command run as an agent session', (v
const [params] = callsTo(verb.method)
expect(params, 'the verb reached its method').toBeDefined()
expect(params?.[verb.callerParam]).toBeUndefined()
// An inherited pane is not the session's identity.
expect(params?.terminalPaneKey).toBeUndefined()
expect(params?.senderPaneKey).toBeUndefined()
expect(getTerminalHandleMock).not.toHaveBeenCalled()
@@ -325,7 +305,6 @@ describe.each(CALLER_VERBS.filter((verb) => verb.callerFlag !== undefined))(
beforeEach(asSessionWithInheritedPane)
it('sends the address it had before the clear for the host to place, instead of refusing it', async () => {
// The chat's address is its lineage root's, which only the host's session records know.
const flags = flagMap({
...verb.flags,
[verb.callerFlag ?? 'from']: `orca_session_id:${ROOT}`
@@ -367,8 +346,7 @@ describe.each([
it.each([`orca_session_id:${ROOT}`, 'orca_session_id:9d4c1b2a-3e5f-4a6b-8c7d-0e1f2a3b4c5d'])(
'sends a session address it cannot place (%s) for the host to check, never dropping it',
async (declared) => {
// A /clear-ed chat's root is accepted by the host; anyone else is refused there, before any
// effect. Dropped here, a conflicting caller would pass unchecked.
// Forward session addresses for host lineage checks; dropping them would bypass caller fencing.
await invoke(command, flagMap({ run: 'run_1', from: declared }))
expect(callsTo(method)[0]).toMatchObject({ run: 'run_1', [callerParam]: declared })
}
@@ -445,7 +423,6 @@ describe('the identity a session presents', () => {
}
asSessionWithInheritedPane()
expect(await preview({})).toBe(`orca_session_id:${SESSION}`)
// Not a caller flag: it names the text to preview, so it is never fenced.
expect(await preview({ from: 'term_sibling' })).toBe('term_sibling')
setEnv({ ORCA_AGENT_SESSION_ID: SESSION, ORCA_TERMINAL_HANDLE: 'structworker_self' })
expect(await preview({})).toBe('structworker_self')
@@ -528,9 +505,7 @@ describe('the orchestration envelope', () => {
})
it('binds whichever current Orca CLI the agent reached, not only the one the session names', () => {
// A login shell can put another install's `orca` first, and the packaged Windows launcher
// rewrites ORCA_CLI_COMMAND in its own process. Neither matters: the id rides the envelope
// and the pinned instance is the one dialed.
// Another CLI install may rewrite ORCA_CLI_COMMAND; the envelope must still pin the session instance.
vi.stubEnv('ORCA_USER_DATA_PATH', '/data/session-orca')
try {
const envelope = createOrchestrationCompatibilityEnvelope({
@@ -573,8 +548,6 @@ describe('which flag names the caller, declared on every spec', () => {
const ORCHESTRATION_SPECS = COMMAND_SPECS.filter((spec) => spec.path[0] === 'orchestration')
it('classifies every --from and --terminal an orchestration verb accepts', () => {
// A new verb cannot take either flag without saying whether it names the caller, so the entry
// check covers it by construction instead of each handler remembering to refuse.
const unclassified = ORCHESTRATION_SPECS.flatMap((spec) =>
(['from', 'terminal'] as const)
.filter((flag) => spec.allowedFlags.includes(flag) && !spec.identityFlagRoles?.[flag])
@@ -625,7 +598,6 @@ describe('which flag names the caller, declared on every spec', () => {
})
describe('every orchestration verb, enumerated', () => {
/** Runs every verb once; returns the ones that guessed an implicit terminal. */
async function verbsThatGuess(): Promise<string[]> {
const guessed: string[] = []
for (const command of Object.keys(ORCHESTRATION_HANDLERS)) {
@@ -641,8 +613,7 @@ describe('every orchestration verb, enumerated', () => {
}
it('guesses a terminal for no verb when the session id is present', async () => {
// Positive control first: with no identity at all the same harness sees the guess, so an empty
// result below is about the id, not a harness that cannot observe a guess.
// Prove the harness observes guesses without an injected identity before testing their absence.
setEnv({})
const population = await verbsThatGuess()
expect(population).toEqual([
@@ -1,13 +1,3 @@
/**
* The env-handle path, with NO `--from`.
*
* Every other orchestration CLI test passes `--from term_coord` explicitly, so the resolver a real
* worker actually goes through — `ORCA_TERMINAL_HANDLE` plus `validateEnvHandle` — was never
* exercised. That is why twelve coordinator verbs could fail for a structured worker while the
* whole suite stayed green, and why the worker's own preamble (which tells it to run these with no
* `--from`) failed on its first line.
*/
import { describe, expect, it, vi } from 'vitest'
const {
@@ -55,13 +45,7 @@ import { useWorktreeAwarenessEnvironment } from './index-test-harness'
const STRUCTURED_HANDLE = 'structworker_a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d'
/**
* Every command below is one of the twelve that resolve their sender through
* `resolveCoordinatorTerminalHandle`. All twelve share ONE resolver and one liveness probe, so the
* set is sufficient if it covers the distinct shapes that reach it: a read, a mutation, a
* run-scoped verb, a dispatch verb and a worker-start. A per-verb sweep would pin the argv parsing
* of twelve handlers and still tell us nothing more about the seam that actually broke.
*/
// Cover distinct handler shapes that share sender resolution and liveness checks.
const SENDER_VERBS: { argv: string[]; method: string }[] = [
{ argv: ['orchestration', 'run-current'], method: 'orchestration.runCurrent' },
{ argv: ['orchestration', 'run-create', '--objective', 'x'], method: 'orchestration.runCreate' },
@@ -97,18 +81,13 @@ describe('a structured worker running orchestration commands as itself', () => {
it.each(SENDER_VERBS)(
'resolves its own identity for $method with no --from',
async ({ argv, method }) => {
// The defect this pins: the sender resolver validated the env handle with `terminal.show`, a
// PTY verb that misses for a structured worker and answers `terminal_handle_stale`. The pane
// remint that would have recovered it needs `ORCA_PANE_KEY`, which a structured child
// deliberately does not carry, so the command died on `no_active_sender_terminal`.
process.env.ORCA_TERMINAL_HANDLE = STRUCTURED_HANDLE
answerCalls()
vi.spyOn(console, 'log').mockImplementation(() => {})
await expect(main(argv)).resolves.not.toThrow()
const called = callMock.mock.calls.map((call) => call[0] as string)
expect(called).toContain(method)
// Never through `terminal.show`: teaching that verb structured handles would hand every
// public terminal verb something that looks writable and is not.
// Structured workers lack writable PTYs; terminal.show must not validate them.
expect(called).not.toContain('terminal.show')
}
)
@@ -123,8 +102,6 @@ describe('a structured worker running orchestration commands as itself', () => {
})
it('still refuses a handle the runtime reports dead, with no pane key to remint from', async () => {
// The invariant the fix must not break: a stale `ORCA_TERMINAL_HANDLE` in a long-lived shell
// must keep failing rather than being baked into a coordinator preamble.
process.env.ORCA_TERMINAL_HANDLE = 'term_stale'
callMock.mockImplementation(async (method: string) => {
if (method === 'terminal.resolveIdentity') {
@@ -1,14 +1,3 @@
/**
* A structured chat session with NO orchestration identity must refuse, not guess.
*
* Non-worker structured sessions get no `ORCA_TERMINAL_HANDLE`, so `orchestration check` fell
* through to the active-terminal guess — and `check` is destructive by default, so it consumed
* another pane's oldest unread batch and marked it read. The rightful worker never saw that mail.
*
* The case pinned here is ONE terminal pane in the worktree, because that is the case
* `requireUnambiguous` misses: with a single candidate the guess still resolves, to a sibling.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { ORCA_STRUCTURED_SESSION_ENV } from '../shared/structured-session-marker'
@@ -38,8 +27,7 @@ describe('a structured chat session with no orchestration identity', () => {
getTerminalHandleMock.mockReset()
delete process.env.ORCA_TERMINAL_HANDLE
process.env[ORCA_STRUCTURED_SESSION_ENV] = '1'
// Exactly ONE terminal pane in the worktree: the single-candidate case, where
// `requireUnambiguous` still resolves and would hand this session a sibling's handle.
// Even one candidate can belong to a sibling, so requireUnambiguous cannot protect this session.
getTerminalHandleMock.mockResolvedValue('term_sibling')
})
@@ -61,7 +49,6 @@ describe('a structured chat session with no orchestration identity', () => {
code: 'no_active_sender_terminal',
message: expect.stringContaining('--terminal')
})
// Neither guessed nor sent: a destructive read must not reach the runtime at all.
expect(getTerminalHandleMock).not.toHaveBeenCalled()
expect(callMock).not.toHaveBeenCalled()
})
@@ -58,7 +58,7 @@ describe('revealPairedTerminalOnLocalDesktop', () => {
expect(mocks.resolveEnvironment).toHaveBeenCalledWith('/profile', 'winpc')
expect(serverCall).toHaveBeenCalledWith('terminal.show', { terminal: HANDLE })
// Why null selectors: the reveal must reach this machine's runtime, not the environment again.
// null bypasses ambient remote selection so reveal reaches the local desktop.
expect(mocks.desktopClient).toHaveBeenCalledWith('/profile', undefined, null, null)
expect(mocks.desktopCall).toHaveBeenCalledWith('terminal.reveal', {
worktreeId: shownTerminal.worktreeId,
+1 -6
View File
@@ -4,11 +4,7 @@ import type { RuntimeTerminalReveal } from '../shared/terminal-pane-reveal'
import { RuntimeClient } from './runtime-client'
import { resolveEnvironment } from './runtime/environments'
/**
* A paired server has no window, so `terminal.focus` there never navigates (#24688). The desktop
* app on this machine shows that server's panes under the same ids and can reveal one.
* Resolves false when no local desktop shows the environment; the server's answer then stands.
*/
/** Paired servers have no window; reveal their pane through a local desktop if one is available. */
export async function revealPairedTerminalOnLocalDesktop(
client: RuntimeClient,
handle: string
@@ -30,7 +26,6 @@ export async function revealPairedTerminalOnLocalDesktop(
})
return revealed.result.reveal.navigated
} catch {
// Why: no desktop here, one without terminal.reveal, a headless one, or a handle gone since.
return false
}
}
+6 -18
View File
@@ -12,9 +12,6 @@ export function setupHostId(setup: ProjectHostSetup): string {
return normalizeExecutionHostId(setup.hostId) ?? setup.hostId
}
// Why: `orca project list` shows display names, and the stored projectId is a provider-scoped
// string ("github:stablyai/orca") nobody keeps in their head — so the name is what gets typed.
// Matching ids alone reported "not set up" for a project sitting right there in the listing.
export async function findProjectSetups(
client: RuntimeClient,
setups: readonly ProjectHostSetup[],
@@ -28,7 +25,7 @@ export async function findProjectSetups(
if (projects === null) {
return legacyNameMatchedSetups(setups, projectSelector)
}
// Why: an exact id with no setups names that project, not another one whose name equals it.
// An exact id must not fall through to another project whose name equals it.
if (projects.some((project) => project.id === projectSelector)) {
return []
}
@@ -40,10 +37,7 @@ export async function findProjectSetups(
: setups.filter((candidate) => candidate.projectId === matchedId)
}
/**
* The project id a `--project` value names, for commands that send the id itself. An id or an
* unknown value passes through unchanged so the server still answers for it, as it always did.
*/
/** Unknown selectors pass through for the server to resolve. */
export async function resolveProjectIdSelector(
client: RuntimeClient,
projectSelector: string
@@ -52,8 +46,7 @@ export async function resolveProjectIdSelector(
try {
projects = (await client.call<{ projects: Project[] }>('project.list')).result.projects
} catch {
// Why: falling back sends the value exactly as before this lookup existed, so a failed or
// missing project.list costs only the name convenience, never a working id.
// Failed name lookup must not prevent a valid id from reaching the server.
return projectSelector
}
if (projects.some((project) => project.id === projectSelector)) {
@@ -78,17 +71,14 @@ function assertProjectNameUnambiguous(named: readonly Project[], projectSelector
)
}
// Why: a server predating project.list still answers projectHostSetup.list, and those rows carry
// a display name of their own. It is the repo's name rather than the project's, so this is the
// weaker match — reachable only on that older server.
// Older servers expose only setup display names, which name repos rather than projects.
function legacyNameMatchedSetups(
setups: readonly ProjectHostSetup[],
projectSelector: string
): ProjectHostSetup[] {
const matched = setups.filter((candidate) => matchesName(candidate.displayName, projectSelector))
const projectIds = [...new Set(matched.map((candidate) => candidate.projectId))]
// Why: two projects can hold repos with the same folder name, and this match is by folder name.
// Selecting either would target a project the caller did not name.
// Repo folder names can collide across projects; never guess between them.
if (projectIds.length <= 1) {
return matched
}
@@ -107,9 +97,7 @@ async function listProjectsUnlessUnsupported(client: RuntimeClient): Promise<Pro
try {
return (await client.call<{ projects: Project[] }>('project.list')).result.projects
} catch (error) {
// Why: only a version gap justifies the weaker fallback. Swallowing everything reported a
// network blip or a permission error as "no project by that name", which then surfaced as
// "Project is not set up on the selected host" for a project that exists and is set up.
// Only a version gap permits the weaker name match; propagate network and permission failures.
if (error instanceof RuntimeClientError && error.code === 'method_not_found') {
return null
}
+1 -5
View File
@@ -4,8 +4,7 @@ import { describeQuoteStrippedJsonFlag, looksQuoteStripped } from './quote-strip
describe('quote-stripped JSON flag detection', () => {
it('flags the exact shape measured from Windows PowerShell 5.1', () => {
// Measured on a real Windows host: ConvertTo-Json emitted
// ["task_b2a580db74d8","task_c3b691ec85e9"] and argv received the value below.
// PowerShell 5.1 passed this shape after ConvertTo-Json emitted quoted task ids.
expect(looksQuoteStripped('[task_b2a580db74d8,task_c3b691ec85e9]')).toBe(true)
expect(looksQuoteStripped('[a,b]')).toBe(true)
expect(looksQuoteStripped('{a:b}')).toBe(true)
@@ -26,7 +25,6 @@ describe('quote-stripped JSON flag detection', () => {
})
it('requires a key:value pair per entry before calling an object stripped', () => {
// Quoting these cannot produce a valid object, so they are ordinary invalid JSON.
expect(looksQuoteStripped('{a,b}')).toBe(false)
expect(looksQuoteStripped('{a:b,c}')).toBe(false)
expect(looksQuoteStripped('{:b}')).toBe(false)
@@ -40,14 +38,12 @@ describe('quote-stripped JSON flag detection', () => {
expect(message).toContain('--payload is not valid JSON')
expect(message).toContain('PowerShell 5.1')
expect(message).toContain('$v')
// A payload can carry secrets, and this reaches --json output.
expect(message).not.toContain('hunter2')
expect(message).not.toContain('secret')
expect(describeQuoteStrippedJsonFlag('options', '["a","b"]')).toBeNull()
})
it('hedges the shell attribution, since it inspects only the value shape', () => {
// The same shape occurs when a macOS/Linux user simply forgets to quote.
expect(describeQuoteStrippedJsonFlag('options', '[a,b]')).toContain('If you ran this from')
})
})
+4 -22
View File
@@ -1,14 +1,4 @@
/**
* Windows PowerShell 5.1 does not escape inner quotes when it builds a native command line, so
* `--payload '{"a":"b"}'` reaches the exe as `--payload {a:b}` (#16706). The value is correct when
* printed and damaged by the time argv is parsed, which makes the resulting "invalid JSON" error
* point at the user's input rather than at the shell that mangled it.
*
* Detection only — recovery is deliberately not attempted. Un-quoting is lossy for anything but a
* strict grammar: `["1","2"]` and `[1,2]` arrive identically, so a general repair would silently
* turn strings into numbers. `--deps` is recoverable only because generated task IDs have a fixed
* shape (see `task-deps-flag.ts`).
*/
// PowerShell 5.1 can strip JSON quotes at the native argv boundary; repair would lose type information.
const BARE_TOKEN = /^[A-Za-z0-9_.:+-]+$/
const BARE_OBJECT_ENTRY = /^([A-Za-z0-9_.+-]+):([A-Za-z0-9_.+-]+)$/
@@ -17,7 +7,6 @@ function splitEntries(body: string): string[] {
return body.split(',').map((entry) => entry.trim())
}
/** Whether `raw` looks like JSON whose quotes a native argv boundary stripped. */
export function looksQuoteStripped(raw: string): boolean {
const trimmed = raw.trim()
if (trimmed.includes('"')) {
@@ -36,26 +25,19 @@ export function looksQuoteStripped(raw: string): boolean {
if (body.length === 0) {
return false
}
// Why: only claim mangling when re-quoting every entry would actually produce valid JSON.
// An object needs a `key:value` pair per entry; `{a,b}` is not a stripped object.
// Re-quoting must yield valid JSON; {a,b} is not a stripped object.
return isObject
? splitEntries(body).every((entry) => BARE_OBJECT_ENTRY.test(entry))
: splitEntries(body).every((entry) => BARE_TOKEN.test(entry))
}
}
/**
* Error text for a JSON flag whose quotes were stripped. Returns null when `raw` is not that shape.
*
* The value itself is never echoed: a `--payload` can carry secrets, and this message reaches both
* human and `--json` output. The shape is already described by the flag name and the guidance.
*/
/** Never echo the value: JSON payloads can contain secrets. */
export function describeQuoteStrippedJsonFlag(flagName: string, raw: string): string | null {
if (!looksQuoteStripped(raw)) {
return null
}
// Why conditional wording: this inspects only the value's shape, so it also fires when someone
// types an unquoted `[a,b]` on macOS or Linux, where PowerShell is not involved.
// Shape alone cannot prove PowerShell was involved; other shells can receive the same invalid input.
return (
`--${flagName} is not valid JSON: its quotes are missing.\n` +
'If you ran this from Windows PowerShell 5.1, it strips inner quotes when building a native ' +
+2 -10
View File
@@ -1,17 +1,9 @@
import { RuntimeClientError } from './runtime/types'
/**
* The flags that pick which runtime answers a command. `shouldIgnoreRemoteSelection`
* in `src/cli/index.ts` pins some command families to the local runtime, which drops
* these silently — so every pinned family pairs the pin with this rejection instead.
*/
// Local-only command families reject explicit runtime selectors rather than silently ignoring them.
export const REMOTE_SELECTION_FLAGS = ['environment', 'pairing-code'] as const
/**
* Fails a pinned command that was given a runtime selector, rather than answering
* for a machine the caller did not name. `suffix` completes "`--<flag>` does not
* retarget …" and should say what the command answers for and where to run it.
*/
/** suffix completes "--<flag> does not retarget …" with the command's actual scope. */
export function rejectRemoteSelectionFlags(
flags: ReadonlyMap<string, string | boolean>,
suffix: string,
+1 -2
View File
@@ -19,8 +19,7 @@ export function resolveRepoPathArgument(
if (!isRemote) {
return resolvePath(cwd, inputPath)
}
// Why: the local CLI cwd is unrelated to a paired runtime's filesystem.
// Relative remote paths would silently target the wrong machine.
// The client cwd belongs to a different filesystem from a paired runtime's relative paths.
if (!isAbsoluteServerPath(inputPath)) {
throw new RuntimeClientError(
'invalid_argument',
+1 -5
View File
@@ -5,11 +5,7 @@ import {
RETRY_REQUEST_ID_GUIDANCE
} from '../shared/orchestration-retry-request-id'
/**
* `--retry-request` carries the mutation identity that makes a replay idempotent. A damaged value
* must never fall through to `undefined`, because the client would then mint a fresh identity and
* re-apply a mutation that may already have taken effect (#15180).
*/
/** Reject damaged identities: falling back to a fresh id could replay an already-applied mutation. */
export function readRetryRequestFlag(flags: Map<string, string | boolean>): string | undefined {
const value = flags.get('retry-request')
rejectValuelessFlag(value, 'retry-request')
+4 -37
View File
@@ -17,11 +17,7 @@ const {
testUserDataPathRef: { current: '' }
}))
// Why: `main` reaches RuntimeClient through `await import('./runtime-client.js')`
// now. Mocking the same specifier the eager import used proves the dynamic
// import still resolves to the module the 10 existing vi.mock suites target.
// Why: --environment is now resolved against the paired-environment store before the client is
// constructed, so a forwarding assertion needs an environment that actually resolves.
// Explicit environment selectors must resolve before the client is constructed.
vi.mock('./runtime/environments', async (importOriginal) => {
const actual = await importOriginal<Record<string, unknown>>()
return {
@@ -30,14 +26,7 @@ vi.mock('./runtime/environments', async (importOriginal) => {
}
})
// Why: this suite runs the REAL `main()`, and `agent hooks off` below reaches the production
// handler, which calls removeManagedAgentHooks() against the developer's OWN ~/.claude and
// ~/.cursor — a green test run silently deleted every Orca-managed hook on the machine, so agent
// status stopped reporting until the next Orca restart (STA-5679). The byte-for-byte equivalence
// twin already refuses these tokens for exactly this reason
// (config/scripts/cli-runtime-client-deferral-equivalence.mjs); this is the same guard for vitest.
// Stubbed, not dropped: the row is the only case that reads ctx.client, so it carries the
// null-vs-undefined coverage the rest of the table cannot.
// Stub hook writes: this suite runs real handlers that would otherwise edit the developer's agent configs.
vi.mock('../main/agent-hooks/managed-agent-hook-controls', () => ({
applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock,
getManagedAgentHookStatuses: vi.fn(() => [])
@@ -92,18 +81,7 @@ describe('RuntimeClient module-graph deferral', () => {
expect(constructorArgsMock).not.toHaveBeenCalled()
})
// Why: `agent hooks on|off` are the only commands that both sit in a
// suppressed group and touch ctx.client, and they rewrite the real ~/.claude
// hook config — so the byte-for-byte equivalence script cannot invoke them.
// Assert the constructor arguments directly instead: `null` (not `undefined`)
// is what stops the ORCA_* env fallback re-activating for local-only groups.
//
// `constructs` is declared per case and asserted BEFORE the args, because
// only `agent hooks off` reads ctx.client. Looping over `mock.calls` alone
// would pass vacuously for the other four, and would keep passing if the one
// case that carries the null-vs-undefined coverage stopped constructing at
// all. The zero rows are not filler: they assert the deferral itself — a
// local-only group must reach its handler without building a client.
// Assert construction counts before arguments; otherwise an unused getter would pass vacuously.
const SUPPRESSED_GROUPS: [name: string, argv: string[], constructs: number][] = [
['agent', ['agent', 'hooks', 'off'], 1],
['environment', ['environment', 'list'], 0],
@@ -146,10 +124,7 @@ describe('RuntimeClient module-graph deferral', () => {
}
)
// Why: four of the five groups above never read ctx.client, so they can only
// assert that no client is built — not that suppression forwards `null`.
// Stub dispatch and read the getter directly so every group asserts the
// constructor arguments unconditionally, exactly once.
// Probe the getter directly: handlers that never use it cannot verify null selector forwarding.
it.each(SUPPRESSED_GROUPS.map(([name, argv]) => [name, argv] as const))(
'forwards null remote selection to the client %s would build',
async (_name, argv) => {
@@ -172,9 +147,6 @@ describe('RuntimeClient module-graph deferral', () => {
}
)
// Why: the mirror — the same stubbed-dispatch probe must show `undefined`
// (not `null`) for a non-suppressed group, or the assertion above would pass
// for a build that suppressed EVERY command's env fallback.
it('forwards undefined remote selection for a non-suppressed group', async () => {
vi.stubEnv('ORCA_PAIRING_CODE', 'pairing-code')
const dispatchSpy = vi.spyOn(dispatchModule, 'dispatch').mockResolvedValue(undefined)
@@ -191,9 +163,6 @@ describe('RuntimeClient module-graph deferral', () => {
}
})
// Why: the mirror of the above — for every other command the env fallback
// MUST stay live, which the RuntimeClient default parameters implement. That
// only works if `undefined` is forwarded.
it('forwards undefined for non-suppressed commands so the env fallback applies', async () => {
callMock.mockResolvedValue({ result: { worktrees: [] } })
@@ -218,8 +187,6 @@ describe('RuntimeClient module-graph deferral', () => {
expect(constructorArgsMock.mock.calls[0][3]).toBe('env-1')
})
// Why: the getter is memoised; a dynamic import inside it would have made it
// async and changed every handler signature.
it('reuses one client instance across repeated ctx.client reads', async () => {
callMock.mockResolvedValue({ result: { worktrees: [] } })
+3 -9
View File
@@ -62,9 +62,7 @@ function writeMetadata(
}
function findUnusedPid(seed = 200_000): number {
// Why: the stale-bootstrap test must point metadata at a definitely-dead
// process. Hard-coding a small PID is host-dependent and flakes when that
// PID happens to be alive on the machine running the suite.
// Find a dead PID at runtime; a fixed PID may belong to a live process on this host.
let pid = Math.max(seed, process.pid + 10_000)
while (pid < 2_000_000) {
try {
@@ -77,9 +75,7 @@ function findUnusedPid(seed = 200_000): number {
return 2_000_000
}
// Why: these tests create Unix domain socket servers in temp directories.
// Windows does not support Unix domain sockets in the same way, causing
// EACCES errors on listen(), so the suite is skipped on that platform.
// Temp-path Unix sockets fail with EACCES on Windows.
describe.skipIf(process.platform === 'win32')('RuntimeClient', () => {
it('adds an opaque durable request ID only to orchestration mutations', async () => {
const userDataPath = makeRuntimeClientFixture()
@@ -398,7 +394,6 @@ describe.skipIf(process.platform === 'win32')('RuntimeClient', () => {
await expect(client.openOrca(100)).rejects.toMatchObject({
code: 'desktop_activation_blocked'
})
// A blocked runtime can't promote, so we bail before spawning the app.
expect(launchOrcaApp).not.toHaveBeenCalled()
})
@@ -408,8 +403,7 @@ describe.skipIf(process.platform === 'win32')('RuntimeClient', () => {
const server = createServer((socket) => {
sockets.add(socket)
socket.once('close', () => sockets.delete(socket))
// Why: keep the socket open without replying so the client timeout path
// is exercised against a real hung runtime connection.
// Leave the connection open without a reply to exercise the timeout against a real socket.
})
servers.add(server)
await new Promise<void>((resolve) => server.listen(endpoint, resolve))
-1
View File
@@ -1,4 +1,3 @@
// Why: one import surface (and one vi.mock target) for CLI handlers over ./runtime/*.
export { RuntimeClient } from './runtime/client'
export { serveOrcaApp } from './runtime/launch'
export { getDefaultUserDataPath } from './runtime/metadata'
-1
View File
@@ -11,7 +11,6 @@ import { COMMAND_SPECS } from './specs'
const COMMAND_PATHS = COMMAND_SPECS.flatMap((spec) => specPaths(spec))
/** The real registry pipeline, so these assertions cover the shipped spec too. */
function parseCli(argv: string[]): ParsedArgs {
const parsed = normalizeCommandPositionals(
COMMAND_SPECS,
+2 -6
View File
@@ -16,8 +16,7 @@ export type SearchCommand =
| { kind: 'index-status' }
| { kind: 'search'; request: AiVaultSearchRequest }
// Why enumerated: --index-status calls a different RPC that reads none of these,
// so ignoring one would answer a question the caller did not ask.
// --index-status uses a different RPC; query flags would otherwise be silently ignored.
const QUERY_ONLY_FLAGS = [
'query',
'scope',
@@ -40,7 +39,6 @@ function readEnum<TValue extends string>(
if (value === undefined) {
return undefined
}
// Why find and not includes: the match carries the narrow type, so nothing is asserted.
const matched = allowed.find((candidate) => candidate === value)
if (matched === undefined) {
throw new RuntimeClientError(
@@ -86,8 +84,7 @@ function readScopePaths(flags: Map<string, string | boolean>): string[] | undefi
return paths
}
// Why the contract's own schema: the offset requirement lives there, and a
// second copy here would drift from what the host accepts.
// Reuse the host schema so timestamp offset requirements cannot drift.
function readSince(flags: Map<string, string | boolean>): string | undefined {
const since = getOptionalStringFlag(flags, 'since')
if (since === undefined) {
@@ -129,7 +126,6 @@ function readFilters(
return Object.keys(filters).length > 0 ? filters : undefined
}
/** Maps `orca search` flags onto the session-search contract; nothing it does not have. */
export function parseSearchCommand(flags: Map<string, string | boolean>): SearchCommand {
if (flags.has('index-status')) {
const conflicting = QUERY_ONLY_FLAGS.filter((flag) => flags.has(flag))
+2 -6
View File
@@ -1,6 +1,4 @@
// Why: the runtime answers a selector that matches several rows (two projects both named `app`)
// with a bare `selector_ambiguous`, naming neither the value nor a way out. Only the CLI still
// knows which flag the caller typed, so it says which selector was ambiguous and what is exact.
// The runtime omits the typed selector from ambiguity errors; only the CLI can supply it.
export type SelectorAmbiguityRecovery = {
ambiguousSelectors: { flag: string; selector: string }[]
@@ -9,14 +7,12 @@ export type SelectorAmbiguityRecovery = {
const EXACT_FORMS = {
repo: {
// Why the --project route: a local and an SSH project can share both repo id and path.
forms:
'id:<repo-id> or path:<absolute-path>; if two hosts share that id or path, use --project <id> --host <host> where the command takes them',
list: 'orca repo list --json',
noun: 'project'
},
worktree: {
// Why identity first: a local and an SSH checkout can share both repo id and path.
forms:
'identity:<identity.key> (host-qualified), or id:<repo-id>::<absolute-path> when only one host has that checkout',
list: 'orca worktree list --json',
@@ -27,7 +23,7 @@ const EXACT_FORMS = {
export function selectorAmbiguityRecovery(selectors: {
repo?: string
worktree?: string
/** The command chose a paired server, so the listing must ask that server, not this machine. */
/** Route recovery listings to the selected server. */
remoteServerSelected?: boolean
}): SelectorAmbiguityRecovery | undefined {
const entries = (['worktree', 'repo'] as const).flatMap((flag) => {
+6 -26
View File
@@ -37,21 +37,13 @@ export function normalizeWorktreeSelector(selector: string, cwd: string): string
return selector
}
/**
* Rewrites the Linux path a WSL shell prints into the UNC path the runtime stored (#16628).
*
* Why here and not in the runtime: only the CLI sits in the distro, so only it can
* prove which distro the typed path belongs to. Translating once at this chokepoint
* covers `worktree show`, `terminal list --worktree` and `worktree rm --worktree`.
*/
/** Resolve Linux selectors to stored UNC paths here, where the caller's distro is known. */
export async function resolveCallerDistroPathSelector(
selector: string,
cwd: string,
client: RuntimeClient
): Promise<string> {
// Why not WSL_DISTRO_NAME: it is also set for a Linux-native CLI whose runtime stores
// POSIX paths, so it would name a distro for a caller that has none. ORCA_CLI_CWD, which
// the WSL launcher always sets and which arrives here as the invocation cwd, proves it.
// A UNC cwd proves Windows/WSL routing; WSL_DISTRO_NAME is also set for Linux-native runtimes.
const callerDistro = parseWslUncPath(cwd)?.distro
const linuxPath = selector.startsWith('path:') ? selector.slice(5) : ''
const isLinuxMountedPath = /^\/mnt\/[A-Za-z](?:\/|$)/.test(linuxPath)
@@ -73,8 +65,7 @@ export async function resolveCallerDistroPathSelector(
? isWslUncPathForLinuxMountedPath(worktree.path, linuxPath)
: isWslUncPathForCallerLinuxPath(worktree.path, linuxPath, callerDistro!)
)
// Why the stored spelling rather than a synthesized UNC path: an unmatched selector must
// reach the runtime verbatim and fail as the caller typed it, never as a guessed distro.
// Preserve unmatched input verbatim; synthesizing a UNC path could guess the wrong distro.
return match ? `path:${match.path}` : selector
}
@@ -94,8 +85,7 @@ function assertLocalCwdWorktreeSelector(selector: string, client: RuntimeClient)
if (!client.isRemote) {
return
}
// Why: a paired CLI's cwd belongs to the client machine, not the runtime
// server, so cwd-derived worktree selectors are only valid locally.
// A paired CLI's cwd belongs to the client filesystem, so it cannot identify server worktrees.
throw new RuntimeClientError(
'invalid_argument',
`${selector} is a local cwd shortcut and cannot be resolved against a remote runtime. Pass an explicit server-side worktree selector such as identity:<identity>, id:<repo-id>::<path>, name:<displayName>, branch:<branch>, issue:<number>, or path:<absolute-server-path>.`
@@ -118,10 +108,7 @@ export async function resolveCurrentWorktreeSelection(
const worktrees = await client.call<RuntimeWorktreeListResult>('worktree.list', {
limit: 10_000
})
// Why: users expect "active/current" to mean the enclosing managed worktree
// even from nested subdirectories. Resolve to the concrete runtime row here:
// duplicate repo registrations can expose the same Git worktree path, and a
// path selector would throw selector_ambiguous after losing the repo id.
// Preserve the repo id: duplicate registrations can make a bare path selector ambiguous.
const selection = selectEnclosingWorktreeSelectionOnHost(
worktrees.result.worktrees,
cwd,
@@ -167,8 +154,7 @@ export async function getRequiredWorktreeSelector(
return await normalizeWorktreeSelectorForCaller(value, cwd, client)
}
// Why: local browser commands default to the current worktree by auto-resolving
// from cwd. Remote commands omit worktree so the runtime uses server-side focus.
// Remote commands leave worktree unset to use server-side focus.
export async function getBrowserWorktreeSelector(
flags: Map<string, string | boolean>,
cwd: string,
@@ -188,18 +174,13 @@ export async function getBrowserWorktreeSelector(
if (client.isRemote) {
return undefined
}
// Default: auto-resolve from cwd
try {
return await resolveCurrentWorktreeSelector(cwd, client)
} catch {
// Not inside a managed worktree — no filter
return undefined
}
}
// Why: mirrors browser's implicit active-tab targeting. When --terminal is
// omitted, resolve the active terminal in the current worktree so commands
// like `orca terminal send --text "hello" --enter` Just Work.
export async function getTerminalHandle(
flags: Map<string, string | boolean>,
cwd: string,
@@ -270,7 +251,6 @@ export async function getComputerCommandTarget(
}
}
// Match browser targeting: workspace by default, explicit device/emulator/worktree overrides.
export type EmulatorCliTarget = {
worktree?: string
device?: string
+4 -14
View File
@@ -3,15 +3,9 @@ import { normalizeServeModeArgv } from '../main/startup/serve-mode-argv'
import { BOOLEAN_FLAGS } from './args'
import { SERVE_COMMAND_SPECS } from './specs/serve'
// Why this test lives under src/cli: the Electron rewrite duplicates the CLI's serve flag list because
// the main tsconfig cannot import the CLI project, so the parity check has to run from the CLI side.
// It fails when `orca serve` grows a flag that a direct `<binary> serve …` launch would silently drop (#12677).
// The main tsconfig cannot import CLI specs, so test the duplicated serve flag lists from here.
// Global flags with no `--serve-*` counterpart:
// - help: a help launch is refused outright, not translated (see serve-mode-argv's HELP_FLAGS).
// - pairing-code / environment: nothing in src/main reads them, so they ride through untouched.
// - force: overrides the CLI's own pre-exec check; a direct `<binary> serve` launch has none to override.
// - supervise: the CLI is the supervisor; a direct `<binary> serve` launch has no parent to restart it.
// Help is refused; pairing selectors pass through; force and supervise apply only to the CLI parent.
const UNTRANSLATED_GLOBAL_FLAGS = new Set([
'help',
'pairing-code',
@@ -41,20 +35,16 @@ describe('serve flag parity between the CLI spec and the Electron argv rewrite',
expect(normalizeServeModeArgv(argv)).toEqual(expected)
if (takesValue) {
// The equals form is the other shape `orca serve` accepts; normalize it to the internal shape.
expect(normalizeServeModeArgv(['/AppRun', 'serve', `--${flag}=value`])).toEqual(expected)
} else {
// A boolean with an attached value is not a truthy assertion: the CLI reads these as
// `flags.get(name) === true`, so `--no-pairing=false` must not disable pairing here either.
// (`--json` is the one flag the CLI reads with `.has()`; erring toward the literal value is
// the safe direction and only changes output format.)
// Keep attached boolean values literal, as the CLI does for flags such as --no-pairing.
expect(normalizeServeModeArgv(['/AppRun', 'serve', `--${flag}=false`])).toEqual([
'/AppRun',
'--serve',
`--${flag}=false`
])
}
// Idempotent: `orca serve` spawns the app already in this shape, and the rewrite runs over it too.
// The app also normalizes argv already rewritten by the CLI launcher.
expect(normalizeServeModeArgv(expected)).toEqual(expected)
})
})
+3 -17
View File
@@ -1,15 +1,4 @@
/**
* An agent session is its own orchestration caller. A flag that names the caller may restate that
* session, but one naming anyone else is refused before any effect — never dropped, never allowed
* to win. Against a host that predates session callers this is the only guard: such a host
* would honor the flag as the caller, which is how a chat consumed a sibling's mail (#21097).
*
* Whether `--from`/`--terminal` names the caller or a target is declared on the command's spec
* (`identityFlagRoles`) and checked once, here, at the CLI entry, so a handler cannot forget it.
*
* A `/clear`ed chat keeps its lineage root's address, which only the host's records can place, so
* a session address that is not this session's own spelling goes to the host's canonical-id check.
*/
// Fence caller flags before dispatch: older hosts would let a session act as another caller.
import type { CommandSpec, IdentityFlag } from './command-spec'
import { RuntimeClientError } from './runtime/types'
@@ -48,10 +37,7 @@ export function refuseConflictingSessionCallerFlags(
const IDENTITY_FLAGS: readonly IdentityFlag[] = ['from', 'terminal']
/**
* The session's own spellings, plus the handle a structured worker session was minted. Plain
* strings: this runs at the CLI entry for every command, before the address codec's module graph.
*/
// Avoid loading the address codec graph at CLI entry for every command.
function namesInjectedSession(value: string, sessionId: string, env: NodeJS.ProcessEnv): boolean {
return (
value === sessionId ||
@@ -60,7 +46,7 @@ function namesInjectedSession(value: string, sessionId: string, env: NodeJS.Proc
)
}
/** A session address only the host can place: this session's `/clear` root, or someone else. */
/** Only the host can distinguish this session's /clear lineage root from another caller. */
export function sessionAddressForHost(
value: string,
sessionId: string,
+3 -5
View File
@@ -23,8 +23,7 @@ const sha = (text: string) => createHash('sha256').update(text).digest('hex')
const { detectAgentsMock } = vi.hoisted(() => ({ detectAgentsMock: vi.fn() }))
// Why: the real installer runs against a temp home; only the shipped bytes and the
// host's agent probe are pinned so the test does not depend on this machine.
// Use the real installer in a temporary home; pin bundled bytes and agent detection for portability.
vi.mock('../main/skills/bundled-skill-install/bundled-skill-sources', () => ({
BUNDLED_SKILL_SOURCES: [
{ name: NAME, text: NEW_TEXT, releasedDigests: [sha(OLD_TEXT), sha(NEW_TEXT)] }
@@ -33,7 +32,7 @@ vi.mock('../main/skills/bundled-skill-install/bundled-skill-sources', () => ({
vi.mock('../main/preflight/installed-agent-detection', () => ({
detectInstalledAgentsWithShellPathHydration: detectAgentsMock
}))
// Why: a held lock should fail fast here instead of waiting out the real 5s default.
// Avoid the production five-second lock wait in tests.
vi.mock('../main/skills/bundled-skill-install/local-host', async (importOriginal) => {
const actual = await importOriginal<typeof LocalHost>()
return {
@@ -130,7 +129,7 @@ describe('orca skills install/update', () => {
const text = await run(['update', '--skill', NAME, '--agent', 'claude', '--dry-run'])
// Why no Claude line: update never adds the skill for an agent that does not have it.
// Updating must not install skills for an agent that lacks them.
expect(text).toContain(`${NAME}\n ${paths.shared}: Update\n\n`)
expect(text).toMatch(/Plan digest: [0-9a-f]{64}\nRerun without --dry-run to update now\.\n$/u)
expect(await skillText(paths.shared)).toBe(OLD_TEXT)
@@ -204,7 +203,6 @@ describe('orca skills install/update', () => {
it('exits 1 when the run cannot apply the plan', async () => {
await writeCopy(paths.shared, OLD_TEXT)
// Why: another install holding the target's lock is a run that wrote nothing.
const release = await acquireSkillInstallLock({
path: skillInstallLockPath(join(paths.userData, 'skill-installs'), paths.shared)
})
+2 -5
View File
@@ -6,7 +6,7 @@ const { guideModuleLoadMock, previewMock, runtimeClientConstructorMock } = vi.ho
runtimeClientConstructorMock: vi.fn()
}))
// Why: these cases stop before planning; the installer itself is covered in skills-install.test.ts.
// Installer behavior is covered separately in skills-install.test.ts.
vi.mock('../main/skills/bundled-skill-install/cli-entry', () => ({
localBundledSkillInput: previewMock,
previewBundledSkillInstall: previewMock,
@@ -45,10 +45,7 @@ vi.mock('./bundled-skill-guides.js', () => {
})
vi.mock('./runtime-client', async () => {
// 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 the mock must preserve the production classes.
const { RuntimeClientError, RuntimeRpcFailureError } = await import('./runtime/types.js')
class RuntimeClient {
-1
View File
@@ -1,4 +1,3 @@
/** Write one newline-terminated payload to stdout without doubling an existing newline. */
export function writeStdoutLine(value: string): void {
process.stdout.write(value.endsWith('\n') ? value : `${value}\n`)
}
+2 -6
View File
@@ -200,8 +200,7 @@ describe('formatTerminalSend', () => {
})
})
// Why: an older host still publishes the codex-* tokens for dialogs its matcher never proved were
// Codex's, so a Gemini/Cursor/Antigravity user reads a Codex label unless the CLI names the neutral one.
// Older hosts can mislabel other providers' dialogs with codex-* tokens; include the neutral names.
describe('blocked-reason rendering against a mixed-version host', () => {
function showResult(reason?: RuntimeTerminalWaitBlockedReason): {
terminal: RuntimeTerminalShow
@@ -242,16 +241,13 @@ describe('blocked-reason rendering against a mixed-version host', () => {
}
}
// Why one assertion over every reason: a test that only asserts the *absence* of an alias suffix
// passes when the aliasing code is deleted, so each case is paired with a legacy token that must
// gain one.
// Pair alias additions with unchanged tokens so missing alias logic cannot pass on absence alone.
it.each([
['codex-trust-workspace', 'codex-trust-workspace (agent-trust-workspace)'],
['codex-update-prompt', 'codex-update-prompt (agent-update-prompt)'],
['codex-cwd-prompt', 'codex-cwd-prompt (agent-cwd-prompt)'],
['codex-hooks-review-prompt', 'codex-hooks-review-prompt (agent-hooks-review-prompt)'],
['codex-interactive-prompt', 'codex-interactive-prompt (agent-interactive-prompt)'],
// This build published these itself, so there is nothing to reinterpret.
['agent-trust-workspace', 'agent-trust-workspace'],
['codex-model-migration-prompt', 'codex-model-migration-prompt']
] as const)('renders %s as %s on both wait and show', (reason, rendered) => {
+1 -6
View File
@@ -106,8 +106,6 @@ export function formatTerminalShow(result: { terminal: RuntimeTerminalShow }): s
`ptyId: ${terminal.ptyId ?? 'none'}`,
`connected: ${terminal.connected}`,
`writable: ${terminal.writable}`,
// Why listed above the preview: the preview is where a reader would otherwise have to
// spot the prompt by eye, which is the work this line exists to remove.
`agentWait: ${formatAgentWait(terminal.agentWait)}`,
`preview: ${terminal.preview || '<empty>'}`
].join('\n')
@@ -143,8 +141,6 @@ export function formatTerminalRead(result: { terminal: RuntimeTerminalRead }): s
...latestCursor,
...(terminal.truncated ? ['warning: older output is no longer retained'] : []),
...(limitedWarning ? [limitedWarning] : []),
// Why: the caller asked for the rendered screen; say plainly that this is not it rather
// than let repaint fragments be read as what the terminal displayed.
...(terminal.source === 'screen-unavailable'
? [
'warning: no rendered screen was available, so this is accumulated output; repainted lines may appear as stacked fragments'
@@ -208,8 +204,7 @@ function promptObservationWarning(
if (prompt.observation === 'incarnation_replaced') {
return 'delivery was not observed because the terminal process was replaced. Inspect the current terminal before sending a new prompt; do not retry with this request ID.'
}
// Ordered before the unsupported arm: an agent provider that never reached turn_started
// needs the swallowed-Enter recovery even if this host could not observe the submit.
// Providers need swallowed-Enter recovery without turn_started, even if submit observation failed.
if (prompt.provider !== 'unsupported' && prompt.provider !== 'old-host') {
return prompt.stages.includes('turn_started')
? null
+1 -4
View File
@@ -100,8 +100,6 @@ describe('orca terminal read --screen', () => {
expect(String(logSpy.mock.calls[0]?.[0])).toContain('source: screen')
})
// Why: the whole defect is a stream being read as if it were the screen. A fallback has to
// announce itself rather than look like a successful screen read.
it('warns when a screen was asked for but only the stream was available', async () => {
queueFixtures(
callMock,
@@ -116,8 +114,7 @@ describe('orca terminal read --screen', () => {
expect(printed).toContain('stacked fragments')
})
// Why: zod strips the unknown `screen` key on an older host, so it answers with an ordinary
// stream read. Handing that back would be the original defect wearing the new flag's name.
// Older hosts strip the unknown screen key and return a stream read.
it("refuses to pass an older host's stream off as a screen read", async () => {
queueFixtures(callMock, readFixture({ tail: ['cclclecleaclear'] }))
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+1 -2
View File
@@ -63,12 +63,11 @@ export function okFixture(id: string, result: unknown): OkFixture {
return { id, ok: true, result, _meta: { runtimeId: 'runtime-1' } }
}
/** The project read a CLI automation write makes to fence the host it lands on. */
/** Repo lookup fences automation writes to the destination host. */
export function localRepoDestinationFixtures(repoId = 'repo-1'): OkFixture[] {
return [okFixture('req_repo_show', { repo: { id: repoId, connectionId: null } })]
}
/** A workspace selector resolves through its workspace before its project. */
export function workspaceDestinationFixtures(
repoId = 'repo-1',
worktreePath = '/tmp/repo/feature'
+1 -4
View File
@@ -1,7 +1,6 @@
import type { CommandSpec } from './args'
// Why: predictable verbs prevent failed agent guesses; existing divergences stay
// grandfathered because renaming them would break compatibility.
// Grandfather existing verb divergences to avoid breaking compatibility.
type VerbFamily = {
name: string
@@ -61,8 +60,6 @@ export function findVocabularyViolations(specs: CommandSpec[]): VocabularyViolat
if (family.allowlist.has(command)) {
continue
}
// Why: an alias on the canonical verb makes the command reachable the
// canonical way, which satisfies the policy without a rename.
if (hasCanonicalAlias(spec, family.canonical)) {
continue
}
+1 -2
View File
@@ -63,8 +63,7 @@ function formatWorktreeMemoryLine(worktree: WorktreeMemory): string {
].join(' ')
}
// Why omitted rather than zeroed: a host that predates the field, or cannot read
// commit at all, must not be printed as agents committing nothing.
// Missing commit fields mean unknown usage, not zero.
function formatCommitLines(snapshot: MemorySnapshot): string[] {
if (typeof snapshot.totalPrivateMemory !== 'number') {
return []
+5 -15
View File
@@ -7,7 +7,7 @@ import { RuntimeClientError } from './runtime-client'
export type ProjectCreateTarget = {
repoSelector: string
/** The selected setup's host; a repo id alone is ambiguous once it is set up on two hosts. */
/** Repo ids can be shared across hosts. */
repoHostId: string
setup: ProjectHostSetup
}
@@ -46,8 +46,7 @@ export function assertWorkspaceTargetFlagsCompatible(flags: Map<string, string |
}
}
// Why: the routed runtime can hold both an exact `runtime:<id>` row and a `local` row for the
// same project; the exact stamp is the one the caller named, so it wins the selection.
// Prefer the exact runtime stamp over its local alias when both setups exist.
function selectSetupOnHost(
candidates: readonly ProjectHostSetup[],
host: ParsedExecutionHost | undefined,
@@ -59,8 +58,7 @@ function selectSetupOnHost(
candidates.find((candidate) => hostFilterMatchesHostId(host, candidate.hostId))
)
}
// Why: one host is not a choice, so demanding --host for it was ceremony. Two are a choice, and
// picking one would run the command on a machine the caller never named.
// Infer a sole host, but never choose between multiple hosts without an explicit selector.
const hostIds = new Set(candidates.map(setupHostId))
if (hostIds.size <= 1) {
return candidates[0]
@@ -102,8 +100,6 @@ export async function resolveProjectCreateTarget(
try {
result = await client.call<{ setups: ProjectHostSetup[] }>('projectHostSetup.list')
} catch (error) {
// Why: --host runtime:<id> routes here, so an older server is reachable without the caller
// meaning to; name the version gap rather than surfacing a raw method_not_found.
if (error instanceof RuntimeClientError && error.code === 'method_not_found') {
throw new RuntimeClientError(
'incompatible_runtime',
@@ -120,7 +116,7 @@ export async function resolveProjectCreateTarget(
: (await findProjectSetups(client, result.result.setups, projectId)).filter(
(candidate) => candidate.setupState === 'ready'
)
// Why --host applies to both: setup ids are per host, so the same id can name two setups.
// Setup ids are scoped to a host, so --host must disambiguate them too.
const setup = projectHostSetupId
? selectSetupOnHost(
ready.filter((candidate) => candidate.id === projectHostSetupId),
@@ -135,7 +131,6 @@ export async function resolveProjectCreateTarget(
`Project host setup is not ready or was not found: ${projectHostSetupId}${host ? ` on ${host.id}` : ''}`
)
}
// Why: blaming the host when no project matched sent callers to debug the wrong flag.
if (candidates.length === 0) {
throw new RuntimeClientError(
'invalid_argument',
@@ -150,8 +145,6 @@ export async function resolveProjectCreateTarget(
throw new RuntimeClientError(
'invalid_argument',
`Project is not set up on the selected host: ${projectId}${host ? ` on ${host.id}` : ''}`,
// Why: when the project is set up somewhere just not here, the recoverable answer is which
// hosts do have it — otherwise the caller has to go re-derive the listing by hand.
{
knownProjectHostSetups: candidates.map(summarizeSetup),
nextSteps: candidates.map(
@@ -176,10 +169,7 @@ function getRepoSelectorFromWorktreeId(worktreeId: string | undefined): string |
return `id:${worktreeId.slice(0, separatorIndex)}`
}
/**
* The repo a create names, as RPC params: spread it whole into every call that looks the repo up,
* so the host a project target picked is never dropped. `--repo` stays selector-only.
*/
/** Forward repoHostId with the repo in every lookup; the same repo can exist on multiple hosts. */
export type CliRepoTarget = { repo: string; repoHostId?: string }
export async function getCreateRepoTarget(
+1 -1
View File
@@ -4,7 +4,7 @@ import {
resolveProgramStatusPresentation
} from '../shared/program-status/presentation'
/** Compact OSC 7501 suffix for a `worktree ps` line: a block beats progress; '' when neither. */
/** A block takes precedence over progress. */
export function formatWorktreePsProgramStatus(
agents: readonly RuntimeWorktreeAgentRow[] | undefined
): string {
+1 -5
View File
@@ -19,7 +19,6 @@ function terminalVerdict(row: TerminalCounts): TerminalVerdict {
return 'none'
}
/** `live:` and `pty:` words for one row; lost contact never reads as zero or no. */
export function formatWorktreePsTerminalFields(row: TerminalCounts): string {
if (terminalVerdict(row) === 'unverifiable') {
return 'live:unverifiable pty:unverifiable'
@@ -31,10 +30,7 @@ export function formatWorktreePsTerminalFields(row: TerminalCounts): string {
return `live:${row.liveTerminalCount}+${unverifiable} unverifiable pty:${row.hasAttachedPty ? 'yes' : 'unverifiable'}`
}
/**
* JSON counterpart. The count fields keep their number/boolean types for existing scripts, so
* `terminalVerdict` is what says a 0/false came from a host that could not be asked.
*/
/** Preserve numeric/boolean fields for existing scripts; terminalVerdict distinguishes unknown values. */
export function projectWorktreePsTerminalVerdict<TRow extends TerminalCounts>(
row: TRow
): TRow & { terminalVerdict: TerminalVerdict } {
@@ -1,7 +1,3 @@
/**
* #21210: several SSH hosts hold a worktree of one repo at the same path. A CLI on host D must
* resolve `current` (and a pane's forwarded worktree id) to host D's row, never another host's twin.
*/
import { describe, expect, it } from 'vitest'
import {
getCallerExecutionHostId,
@@ -37,7 +33,6 @@ describe('implicit worktree selection is scoped to the caller host (#21210)', ()
it('reads the caller host from the relay stamp, encoding the SSH target', () => {
expect(getCallerExecutionHostId(SSH_CALLER_D)).toBe('ssh:host%20d')
expect(getCallerExecutionHostId({})).toBe('local')
// A WSL stamp is still this machine.
expect(
getCallerExecutionHostId({
ORCA_ORCHESTRATION_COMPATIBILITY_HOST_KIND: 'wsl',
@@ -57,7 +52,6 @@ describe('implicit worktree selection is scoped to the caller host (#21210)', ()
expect(selectEnclosingWorktreeOnHost(TWINS, PATH, 'local')).toBe(`id:repo-l::${PATH}`)
})
// A runtime-stamped row in this runtime's own catalog is a checkout it holds locally.
it('keeps a runtime-stamped local checkout for a local caller', () => {
const rows = [
TWINS[0]!,
+1 -5
View File
@@ -1,8 +1,4 @@
// Why: the runtime answers an unresolvable `--worktree` with a bare
// `selector_not_found` — no offending value and no grammar — so a caller who passed
// a repo id where a worktree id belongs cannot tell what was wrong (#16904). The CLI
// is the only layer that still knows what the caller typed, so it shapes the recovery
// here, in the same validFlags/suggestions/nextSteps shape as an unknown-flag error.
// The runtime omits the typed selector from not-found errors; add it and its grammar at the CLI boundary.
export const WORKTREE_SELECTOR_FORMS = [
'id:<repo-id>::<absolute-path>',
@@ -1,8 +1,4 @@
/**
* The CLI half of #16628: inside WSL the user types `/home/neil/qa-repo`, the runtime stored the
* UNC path Windows sees. Only this process sits in the distro, so only it may translate — and the
* same selector reaches `worktree rm`, so an unprovable distro must leave the path alone.
*/
// These selectors also reach worktree rm, so translate only with proof of the caller's distro.
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { RuntimeWorktreeRecord } from '../shared/runtime-types'
import type { RuntimeClient } from './runtime-client'
@@ -65,7 +61,7 @@ describe('normalizeWorktreeSelectorForCaller in a WSL shell (#16628)', () => {
it('leaves the path alone when only another distro spells it', async () => {
const { client } = makeClient([uncPath(UBUNTU, LINUX_PATH)])
// Interop forwards WSL_DISTRO_NAME, so it must not outvote the cwd that proves the distro.
// Interop can forward WSL_DISTRO_NAME from a different distro than the cwd proves.
vi.stubEnv('WSL_DISTRO_NAME', UBUNTU)
await expect(
@@ -103,7 +99,7 @@ describe('normalizeWorktreeSelectorForCaller in a WSL shell (#16628)', () => {
it.each([
['a branch selector', 'branch:qa'],
['a relative path', 'path:qa-repo'],
// A UNC path is already the runtime's spelling; a backslash inside a Linux path has no UNC form.
// A Linux filename containing a backslash cannot be represented as a UNC path.
['a UNC path selector', `path:${uncPath(UBUNTU, LINUX_PATH)}`],
['a Linux path containing a backslash', 'path:/home/neil/qa\\repo']
])('leaves %s untranslated', async (_label, selector) => {