fix(orchestration): preserve recovery executable identity

This commit is contained in:
Brennan Benson
2026-08-24 11:22:32 -07:00
parent fec52e441e
commit 24ce0be774
6 changed files with 264 additions and 25 deletions
+4 -1
View File
@@ -18,6 +18,7 @@ import { reportCliError } from './format'
import { printHelp } from './help'
import type { RuntimeClient } from './runtime-client'
import { COMMAND_SPECS } from './specs'
import { resolveOrchestrationCliExecutable } from './runtime/orchestration-recovery-command'
export { COMMAND_SPECS } from './specs'
export { buildCurrentWorktreeSelector, normalizeWorktreeSelector } from './selectors'
@@ -149,7 +150,9 @@ export async function main(
undefined,
undefined,
remotePairingCode,
remoteEnvironment
remoteEnvironment,
resolveOrchestrationCliExecutable(),
argv
)
return client
},
@@ -36,9 +36,13 @@ describe('orchestration mutation recovery', () => {
workerDeathInferred: false
}
})
expect(result.message.indexOf('worker-show')).toBeLessThan(
result.message.indexOf('exact original')
expect(result.message.indexOf('orca orchestration worker-show')).toBeLessThan(
result.message.indexOf('orca orchestration worker-start')
)
expect((result.data as { nextSteps?: string[] }).nextSteps).toEqual([
'Run orca orchestration worker-show --dispatch dispatch_1 --json before retrying.',
'Run orca orchestration worker-start --task task_1 --retry-request request_1.'
])
})
it('does not invent a dispatch for an old-client-shaped error', () => {
@@ -59,4 +63,52 @@ describe('orchestration mutation recovery', () => {
expect((result.data as Record<string, unknown>).recovery).not.toHaveProperty('dispatchId')
expect(result.message).not.toContain('worker death')
})
it('renders the exact executable and safely quotes original arguments', () => {
const result = orchestrationMutationRecoveryError(
new RuntimeClientError('runtime_timeout', 'request timed out', {
orchestrationRequestId: 'request_3',
dispatchId: 'dispatch_3',
originalCommand: [
'orca-dev',
'orchestration',
'worker-start',
'--task',
'task 3',
'--comment',
'literal $(do-not-run)'
]
})
) as RuntimeClientError
expect((result.data as { nextSteps?: string[] }).nextSteps).toEqual([
'Run orca-dev orchestration worker-show --dispatch dispatch_3 --json before retrying.',
"Run orca-dev orchestration worker-start --task 'task 3' --comment 'literal $(do-not-run)' --retry-request request_3."
])
expect(result.message).toContain("'literal $(do-not-run)'")
})
it('parses legacy command text without losing quoted arguments', () => {
const result = orchestrationMutationRecoveryError(
new RuntimeClientError('runtime_timeout', 'request timed out', {
orchestrationRequestId: 'request_4',
originalCommand:
'orca-ide orchestration worker-stop --dispatch dispatch_4 --comment "quoted value"'
})
) as RuntimeClientError
expect(
(result.data as { recovery?: { retryCommand?: string[] } }).recovery?.retryCommand
).toEqual([
'orca-ide',
'orchestration',
'worker-stop',
'--dispatch',
'dispatch_4',
'--comment',
'quoted value',
'--retry-request',
'request_4'
])
})
})
+71 -5
View File
@@ -1,4 +1,5 @@
import { RuntimeClientError } from './runtime-client'
import { resolveOrchestrationCliExecutable } from './runtime/orchestration-recovery-command'
export function orchestrationMutationRecoveryError(error: unknown): unknown {
if (!(error instanceof RuntimeClientError) || !isUnknownMutationOutcomeCode(error.code)) {
@@ -14,8 +15,9 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
const retryCommand = originalCommand
? [...originalCommand, '--retry-request', requestId]
: undefined
const executable = originalCommand?.[0] ?? resolveOrchestrationCliExecutable()
const queryCommand = dispatchId
? ['orca', 'orchestration', 'worker-show', '--dispatch', dispatchId, '--json']
? [executable, 'orchestration', 'worker-show', '--dispatch', dispatchId, '--json']
: undefined
const recovery = {
orchestrationRequestId: requestId,
@@ -27,10 +29,10 @@ export function orchestrationMutationRecoveryError(error: unknown): unknown {
workerDeathInferred: false
}
const retryStep = retryCommand
? `Then re-issue the exact original command with --retry-request ${requestId}.`
: `Recovery is blocked until the exact original command is available; if it is, re-issue it with --retry-request ${requestId}.`
? `Run ${renderCommand(retryCommand)}.`
: 'Recovery is blocked until the exact original command is available; no retry command was emitted.'
const nextSteps = queryCommand
? [`Run ${queryCommand.join(' ')} before retrying.`, retryStep]
? [`Run ${renderCommand(queryCommand)} before retrying.`, retryStep]
: [retryStep]
const message = [
stripUnsafeRetryAdvice(error.message, requestId),
@@ -69,11 +71,75 @@ function commandParts(value: unknown): string[] | undefined {
return [...value]
}
if (typeof value === 'string' && value.length > 0) {
return value.trim().split(/\s+/)
return parseCommandLine(value)
}
return undefined
}
function parseCommandLine(value: string): string[] | undefined {
const parts: string[] = []
let part = ''
let quote: "'" | '"' | undefined
let tokenStarted = false
for (let index = 0; index < value.length; index += 1) {
const character = value[index]
if (quote === "'") {
if (character === "'") {
quote = undefined
} else {
part += character
}
tokenStarted = true
continue
}
if (quote === '"') {
if (character === '"') {
quote = undefined
} else if (character === '\\' && ['"', '\\'].includes(value[index + 1] ?? '')) {
part += value[++index]
} else {
part += character
}
tokenStarted = true
continue
}
if (character === "'" || character === '"') {
quote = character
tokenStarted = true
} else if (/\s/.test(character)) {
if (tokenStarted) {
parts.push(part)
part = ''
tokenStarted = false
}
} else if (character === '\\' && [' ', '\\', "'", '"'].includes(value[index + 1] ?? '')) {
part += value[++index]
tokenStarted = true
} else {
part += character
tokenStarted = true
}
}
if (quote !== undefined) {
return undefined
}
if (tokenStarted) {
parts.push(part)
}
return parts.length > 0 ? parts : undefined
}
function renderCommand(command: readonly string[]): string {
return command.map(shellQuote).join(' ')
}
function shellQuote(value: string): string {
if (/^[A-Za-z0-9_./:@%+=,-]+$/.test(value)) {
return value
}
return `'${value.replaceAll("'", "'\\\"'\\\"'")}'`
}
function stripUnsafeRetryAdvice(message: string, requestId: string): string {
return message
.replace(' Restart Orca and try again.', '')
+13 -7
View File
@@ -20,7 +20,10 @@ import { RemoteRuntimeCompatGate } from './remote-runtime-compat-gate'
import { createOrchestrationCompatibilityEnvelope } from './orchestration-compatibility-envelope'
import { getTimeoutMsParam, isWaitingCheck } from './runtime-request-timeout'
import { resolveWorkerStartClientTimeoutMs } from '../../shared/orchestration-timing-budgets'
import { buildOrchestrationRecoveryCommand } from './orchestration-recovery-command'
import {
buildOrchestrationRecoveryCommand,
resolveOrchestrationCliExecutable
} from './orchestration-recovery-command'
// Why: for long-poll methods the caller's method-level
// `params.timeoutMs` is the inner waiter budget; we extend the client-side
@@ -42,6 +45,8 @@ export class RuntimeClient {
private readonly requestTimeoutMs: number
private readonly remotePairing: PairingOffer | null
private readonly environmentSelector: string | null
private readonly cliExecutable: string
private readonly originalArgs: readonly string[] | undefined
private readonly remoteCompat: RemoteRuntimeCompatGate
private orchestrationContractCheck: Promise<void> | null = null
private readonly orchestrationCompatibility = createOrchestrationCompatibilityEnvelope(
@@ -55,11 +60,15 @@ export class RuntimeClient {
userDataPath = getDefaultUserDataPath(),
requestTimeoutMs = 60_000,
remotePairingCode = process.env.ORCA_PAIRING_CODE ?? process.env.ORCA_REMOTE_PAIRING ?? null,
environmentSelector = process.env.ORCA_ENVIRONMENT ?? null
environmentSelector = process.env.ORCA_ENVIRONMENT ?? null,
cliExecutable = resolveOrchestrationCliExecutable(),
originalArgs?: readonly string[]
) {
this.userDataPath = userDataPath
this.requestTimeoutMs = requestTimeoutMs
this.environmentSelector = environmentSelector
this.cliExecutable = cliExecutable
this.originalArgs = originalArgs ? [...originalArgs] : undefined
this.remotePairing = resolveRemotePairing(userDataPath, remotePairingCode, environmentSelector)
this.remoteCompat = new RemoteRuntimeCompatGate(userDataPath, environmentSelector)
}
@@ -82,7 +91,7 @@ export class RuntimeClient {
? (options?.orchestrationRequestId ?? randomUUID())
: undefined
const originalCommand = orchestrationMutation
? buildOrchestrationRecoveryCommand(method, params)
? buildOrchestrationRecoveryCommand(method, params, this.cliExecutable, this.originalArgs)
: undefined
const compatibilityEnvelope = method.startsWith('orchestration.')
? {
@@ -272,10 +281,7 @@ function attachMutationRecovery(
{
...(error.data && typeof error.data === 'object' ? error.data : {}),
orchestrationRequestId: requestId,
...(originalCommand &&
!(error.data && typeof error.data === 'object' && 'originalCommand' in error.data)
? { originalCommand }
: {})
...(originalCommand ? { originalCommand } : {})
}
)
}
@@ -1,16 +1,37 @@
import { describe, expect, it } from 'vitest'
import { buildOrchestrationRecoveryCommand } from './orchestration-recovery-command'
import {
buildOrchestrationRecoveryCommand,
resolveOrchestrationCliExecutable
} 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('reconstructs the keyed worker-start command from its RPC params', () => {
expect(
buildOrchestrationRecoveryCommand('orchestration.workerStart', {
task: 'task_1',
on: 'windows',
worktree: 'new-top-level',
timeoutMs: 90_000,
devMode: false
})
buildOrchestrationRecoveryCommand(
'orchestration.workerStart',
{
task: 'task_1',
on: 'windows',
worktree: 'new-top-level',
timeoutMs: 90_000,
devMode: false
},
'orca'
)
).toEqual([
'orca',
'orchestration',
@@ -26,6 +47,42 @@ describe('orchestration recovery command identity', () => {
])
})
it('preserves the selected executable and raw command arguments', () => {
expect(
buildOrchestrationRecoveryCommand(
'orchestration.workerStart',
{ task: 'task_1' },
'orca-dev',
[
'orchestration',
'worker-start',
'--task',
'task_1',
'--comment',
'literal $(do-not-run) "quoted"',
'--json'
]
)
).toEqual([
'orca-dev',
'orchestration',
'worker-start',
'--task',
'task_1',
'--comment',
'literal $(do-not-run) "quoted"',
'--json'
])
})
it('supports the explicit executable-first form', () => {
expect(
buildOrchestrationRecoveryCommand('orca-ide', 'orchestration.workerStop', {
dispatch: 'dispatch_1'
})
).toEqual(['orca-ide', 'orchestration', 'worker-stop', '--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()
@@ -1,7 +1,43 @@
export function resolveOrchestrationCliExecutable(
env: NodeJS.ProcessEnv = process.env,
platform: NodeJS.Platform = process.platform
): string {
const configured = env.ORCA_CLI_COMMAND?.trim()
if (configured) {
return configured
}
if (env.ORCA_DEV_REPO_ROOT) {
return 'orca-dev'
}
return platform === 'linux' ? 'orca-ide' : 'orca'
}
export function buildOrchestrationRecoveryCommand(
method: string,
params: unknown
params: unknown,
executable?: string,
originalArgs?: readonly string[]
): string[] | undefined
export function buildOrchestrationRecoveryCommand(
executable: string,
method: string,
params: unknown,
originalArgs?: readonly string[]
): string[] | undefined
export function buildOrchestrationRecoveryCommand(
methodOrExecutable: string,
paramsOrMethod: unknown,
executableOrParams?: string | unknown,
originalArgs?: readonly string[]
): string[] | undefined {
const legacyOrder = typeof paramsOrMethod === 'string' && executableOrParams !== undefined
const method = (legacyOrder ? paramsOrMethod : methodOrExecutable) as string
const params = legacyOrder ? executableOrParams : paramsOrMethod
const executable = legacyOrder
? methodOrExecutable
: typeof executableOrParams === 'string'
? executableOrParams
: resolveOrchestrationCliExecutable()
const command =
method === 'orchestration.workerStart'
? 'worker-start'
@@ -20,7 +56,10 @@ export function buildOrchestrationRecoveryCommand(
if (typeof record[requiredKey] !== 'string' || record[requiredKey].length === 0) {
return undefined
}
const args = ['orca', 'orchestration', command]
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) {
continue
@@ -34,3 +73,19 @@ export function buildOrchestrationRecoveryCommand(
}
return args
}
function withoutRetryRequest(args: readonly string[]): string[] {
const result: string[] = []
for (let index = 0; index < args.length; index += 1) {
const arg = args[index]
if (arg === '--retry-request') {
index += 1
continue
}
if (arg.startsWith('--retry-request=')) {
continue
}
result.push(arg)
}
return result
}