fix(orchestration): harden mutation recovery

This commit is contained in:
Brennan Benson
2026-08-25 02:33:43 -07:00
parent 7f5d59f9fa
commit b08acd340c
8 changed files with 207 additions and 23 deletions
@@ -206,6 +206,16 @@ describe('orchestration gate commands carry caller identity', () => {
'The Orca runtime closed the connection before responding. Restart Orca and try again. Orchestration mutation request ID: mutation_1.',
{
orchestrationRequestId: 'mutation_1',
originalCommand: [
'orca',
'orchestration',
'gate-create',
'--task',
'task_1',
'--question',
'ship?',
'--json'
],
failedStage: 'dispatch_input',
residualResources: [
{ kind: 'worktree', id: 'repo::child' },
@@ -1,4 +1,5 @@
import { describe, expect, it } from 'vitest'
import { runProcess } from '../shared/child-process/run-process'
import {
orchestrationMutationRecoveryError,
renderCommand
@@ -115,6 +116,56 @@ describe('orchestration mutation recovery', () => {
])
})
it.each([
[
'gate-create',
['orca', 'orchestration', 'gate-create', '--task', 'task_1', '--question', 'ship?']
],
[
'worker-retain',
['orca', 'orchestration', 'worker-retain', '--dispatch', 'dispatch_1', '--json']
]
])('replays exact %s argv with the keyed retry', (_name, originalCommand) => {
const result = orchestrationMutationRecoveryError(
new RuntimeClientError('runtime_timeout', 'request timed out', {
orchestrationRequestId: 'request_exact',
originalCommand
})
) as RuntimeClientError
expect(
(result.data as { recovery?: { retryCommand?: string[] } }).recovery?.retryCommand
).toEqual([...originalCommand, '--retry-request', 'request_exact'])
})
it('reuses the request identity without duplicating an existing retry flag', () => {
const result = orchestrationMutationRecoveryError(
new RuntimeClientError('runtime_timeout', 'request timed out', {
orchestrationRequestId: 'request_reused',
originalCommand: [
'orca',
'orchestration',
'worker-retain',
'--dispatch',
'dispatch_1',
'--retry-request=request_reused'
]
})
) as RuntimeClientError
expect(
(result.data as { recovery?: { retryCommand?: string[] } }).recovery?.retryCommand
).toEqual([
'orca',
'orchestration',
'worker-retain',
'--dispatch',
'dispatch_1',
'--retry-request',
'request_reused'
])
})
it('renders Windows cmd recovery guidance without quote drift or percent expansion', () => {
expect(
renderCommand(
@@ -137,4 +188,57 @@ describe('orchestration mutation recovery', () => {
"orca 'literal $(do-not-run)'"
)
})
it.runIf(process.platform !== 'win32')(
'round trips POSIX recovery argv through /bin/sh',
async () => {
const values = ['with spaces', "apostrophe's", 'literal $(do-not-run)', 'line one\nline two']
const command = renderCommand(
[
process.execPath,
'-e',
'process.stdout.write(JSON.stringify(process.argv.slice(1)))',
...values
],
'darwin'
)
const result = await runProcess({ program: '/bin/sh', args: ['-c', command] })
expect(result).toMatchObject({ code: 0, stderr: '', timedOut: false })
expect(JSON.parse(result.stdout)).toEqual(values)
}
)
it.each([
[
'split',
['orca', 'orchestration', 'send', '--pairing-code', 'split-secret', '--subject', 'status'],
'split-secret'
],
[
'equals',
['orca', 'orchestration', 'send', '--pairing-code=equals-secret', '--subject', 'status'],
'equals-secret'
]
])('blocks recovery and removes %s pairing credentials', (_name, originalCommand, secret) => {
const result = orchestrationMutationRecoveryError(
new RuntimeClientError('runtime_timeout', 'request timed out', {
orchestrationRequestId: 'request_secret',
originalCommand
})
) as RuntimeClientError
const output = JSON.stringify({ message: result.message, data: result.data })
expect(output).not.toContain(secret)
expect(result.message).toContain('Recovery is blocked')
expect(result.data).toMatchObject({
recovery: {
orchestrationRequestId: 'request_secret',
recoveryBlocked: true
}
})
expect(result.data).not.toHaveProperty('originalCommand')
expect((result.data as { recovery: object }).recovery).not.toHaveProperty('retryCommand')
})
})
+13 -4
View File
@@ -1,5 +1,8 @@
import { RuntimeClientError } from './runtime-client'
import { resolveOrchestrationCliExecutable } from './runtime/orchestration-recovery-command'
import {
recoverableOrchestrationArgs,
resolveOrchestrationCliExecutable
} from './runtime/orchestration-recovery-command'
import { quoteWindowsCmdArgument } from '../shared/child-process/windows-command-line'
import { quotePowerShellNativeArgument } from '../shared/powershell-native-argument'
import { resolveWindowsShellStartupFamily } from '../shared/windows-terminal-shell'
@@ -15,7 +18,12 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
return error
}
const dispatchId = typeof data?.dispatchId === 'string' ? data.dispatchId : undefined
const originalCommand = commandParts(data?.originalCommand)
const parsedOriginalCommand = commandParts(data?.originalCommand)
const originalCommand = parsedOriginalCommand
? recoverableOrchestrationArgs(parsedOriginalCommand)
: undefined
const safeData = { ...data }
delete safeData.originalCommand
const retryCommand = originalCommand
? [...originalCommand, '--retry-request', requestId]
: undefined
@@ -48,8 +56,9 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
: undefined
].filter((line): line is string => line !== undefined)
return new RuntimeClientError(error.code, message.join('\n'), {
...data,
...safeData,
orchestrationRequestId: requestId,
...(originalCommand ? { originalCommand } : {}),
recovery,
nextSteps
})
@@ -171,7 +180,7 @@ function shellQuote(value: string): string {
if (/^[A-Za-z0-9_./:@%+=,-]+$/.test(value)) {
return value
}
return `'${value.replaceAll("'", "'\\\"'\\\"'")}'`
return `'${value.replaceAll("'", `'"'"'`)}'`
}
function stripUnsafeRetryAdvice(message: string, requestId: string): string {
+1 -1
View File
@@ -69,7 +69,7 @@ describe('RuntimeClient orchestration recovery identity', () => {
})
)
const client = new RuntimeClient(userDataPath, 500)
const client = new RuntimeClient(userDataPath, 500, null, null, 'orca')
try {
await client.call('orchestration.workerStart', { task: 'task_1' })
throw new Error('expected worker-start failure')
@@ -5,18 +5,15 @@ import {
} from './orchestration-recovery-command'
describe('orchestration recovery command identity', () => {
it('resolves dev, packaged, and execution-host CLI names without rewriting them', () => {
expect(resolveOrchestrationCliExecutable({ ORCA_CLI_COMMAND: 'orca-dev' }, 'darwin')).toBe(
'orca-dev'
)
expect(resolveOrchestrationCliExecutable({ ORCA_CLI_COMMAND: 'orca-ide' }, 'win32')).toBe(
'orca-ide'
)
expect(resolveOrchestrationCliExecutable({ ORCA_DEV_REPO_ROOT: '/repo' }, 'darwin')).toBe(
'orca-dev'
)
expect(resolveOrchestrationCliExecutable({}, 'linux')).toBe('orca-ide')
expect(resolveOrchestrationCliExecutable({}, 'darwin')).toBe('orca')
it.each([
['configured dev', { ORCA_CLI_COMMAND: 'orca-dev' }, 'darwin', 'orca-dev'],
['configured WSL', { ORCA_CLI_COMMAND: 'orca-ide' }, 'linux', 'orca-ide'],
['dev checkout', { ORCA_DEV_REPO_ROOT: '/repo' }, 'darwin', 'orca-dev'],
['packaged Linux', {}, 'linux', 'orca-ide'],
['local macOS', {}, 'darwin', 'orca'],
['local Windows', {}, 'win32', 'orca']
] as const)('resolves the %s CLI identity', (_name, env, platform, expected) => {
expect(resolveOrchestrationCliExecutable(env, platform)).toBe(expected)
})
it('reconstructs the keyed worker-start command from its RPC params', () => {
@@ -75,6 +72,30 @@ describe('orchestration recovery command identity', () => {
])
})
it.each([
[
'gate-create',
'orchestration.gateCreate',
['orchestration', 'gate-create', '--task', 'task_1', '--question', 'ship?']
],
[
'worker-retain',
'orchestration.workerRetain',
['orchestration', 'worker-retain', '--dispatch', 'dispatch_1', '--json']
]
] as const)('preserves exact raw argv for %s recovery', (_name, method, args) => {
expect(buildOrchestrationRecoveryCommand(method, {}, 'orca', args)).toEqual(['orca', ...args])
})
it.each([
['split', ['orchestration', 'send', '--pairing-code', 'secret', '--subject', 'status']],
['equals', ['orchestration', 'send', '--pairing-code=secret', '--subject', 'status']]
])('blocks %s pairing-code argv instead of exposing a credential', (_name, args) => {
expect(
buildOrchestrationRecoveryCommand('orchestration.send', {}, 'orca', args)
).toBeUndefined()
})
it('supports the explicit executable-first form', () => {
expect(
buildOrchestrationRecoveryCommand('orca-ide', 'orchestration.workerStop', {
@@ -83,6 +104,20 @@ describe('orchestration recovery command identity', () => {
).toEqual(['orca-ide', 'orchestration', 'worker-stop', '--dispatch', 'dispatch_1'])
})
it('reconstructs worker-retain when raw argv is unavailable', () => {
expect(
buildOrchestrationRecoveryCommand('orchestration.workerRetain', {
dispatch: 'dispatch_1'
})
).toEqual([
resolveOrchestrationCliExecutable(),
'orchestration',
'worker-retain',
'--dispatch',
'dispatch_1'
])
})
it('does not fabricate identity for unsupported mutations or malformed params', () => {
expect(buildOrchestrationRecoveryCommand('orchestration.send', {})).toBeUndefined()
expect(buildOrchestrationRecoveryCommand('orchestration.workerStart', null)).toBeUndefined()
@@ -27,7 +27,7 @@ export function buildOrchestrationRecoveryCommand(
export function buildOrchestrationRecoveryCommand(
methodOrExecutable: string,
paramsOrMethod: unknown,
executableOrParams?: string | unknown,
executableOrParams?: unknown,
originalArgs?: readonly string[]
): string[] | undefined {
const legacyOrder = typeof paramsOrMethod === 'string' && executableOrParams !== undefined
@@ -38,6 +38,10 @@ export function buildOrchestrationRecoveryCommand(
: typeof executableOrParams === 'string'
? executableOrParams
: resolveOrchestrationCliExecutable()
if (originalArgs && originalArgs.length > 0) {
const recoverableArgs = recoverableOrchestrationArgs(originalArgs)
return recoverableArgs ? [executable, ...recoverableArgs] : undefined
}
const command =
method === 'orchestration.workerStart'
? 'worker-start'
@@ -47,7 +51,9 @@ export function buildOrchestrationRecoveryCommand(
? 'worker-abandon'
: method === 'orchestration.workerRelease'
? 'worker-release'
: undefined
: method === 'orchestration.workerRetain'
? 'worker-retain'
: undefined
if (!command || params === null || typeof params !== 'object') {
return undefined
}
@@ -56,9 +62,6 @@ export function buildOrchestrationRecoveryCommand(
if (typeof record[requiredKey] !== 'string' || record[requiredKey].length === 0) {
return undefined
}
if (originalArgs && originalArgs.length > 0) {
return [executable, ...withoutRetryRequest(originalArgs)]
}
const args = [executable, 'orchestration', command]
for (const [key, value] of Object.entries(record)) {
if (value === undefined || value === null || value === false) {
@@ -74,7 +77,10 @@ export function buildOrchestrationRecoveryCommand(
return args
}
function withoutRetryRequest(args: readonly string[]): string[] {
export function recoverableOrchestrationArgs(args: readonly string[]): string[] | undefined {
if (args.some((arg) => arg === '--pairing-code' || arg.startsWith('--pairing-code='))) {
return undefined
}
const result: string[] = []
for (let index = 0; index < args.length; index += 1) {
const arg = args[index]
@@ -106,11 +106,29 @@ describe('buildHostCliEnv', () => {
expect(env.PATH).toBe('/host/bin')
expect(env.ORCA_USER_DATA_PATH).toBe('/host/user-data')
expect(env.ORCA_CLI_CWD).toBe('/home/alice/wt/sub')
expect(env.ORCA_CLI_COMMAND).toBe('orca')
expect(env.ELECTRON_RUN_AS_NODE).toBe('1')
expect(env.NODE_OPTIONS).toBeUndefined()
expect(env.ORCA_NODE_OPTIONS).toBe('--inspect')
})
it.each([
['dev host', { ORCA_DEV_REPO_ROOT: '/repo', ORCA_CLI_COMMAND: 'orca-dev' }],
['packaged Linux host', { ORCA_CLI_COMMAND: 'orca-ide' }],
['local host', {}],
['WSL host', { WSL_DISTRO_NAME: 'Ubuntu', ORCA_CLI_COMMAND: 'orca-ide' }],
['Windows host', { ComSpec: 'C:\\Windows\\System32\\cmd.exe' }]
])('pins %s recovery to the remote shim', (_name, hostEnv) => {
const env = buildHostCliEnv({
hostEnv,
remoteEnv: { ORCA_CLI_COMMAND: 'untrusted-remote-command' },
userDataPath: '/host/user-data',
remoteCwd: '/srv/repo'
})
expect(env.ORCA_CLI_COMMAND).toBe('orca')
})
it('namespaces identical remote artifact paths by stable SSH target', () => {
const artifactInput = {
sourceKey: '/srv/repo/report.html',
@@ -159,6 +159,8 @@ export function buildHostCliEnv(args: {
// subprocess cwd cannot be chdir'd there; ORCA_CLI_CWD carries it for
// cwd-based selectors like `--worktree active`.
env.ORCA_CLI_CWD = args.remoteCwd
// Why: recovery commands run on the SSH execution host through its relay shim.
env.ORCA_CLI_COMMAND = 'orca'
// Why: same node-mode hygiene as the shipped CLI launchers — stash and clear
// NODE_OPTIONS so Electron's node bootstrap does not inherit them.
env.ORCA_NODE_OPTIONS = args.hostEnv.NODE_OPTIONS ?? ''