fix(orchestration): keep worker starts and recovery commands safe

This commit is contained in:
Brennan Benson
2026-08-24 11:41:04 -07:00
parent 24ce0be774
commit a1b6bee711
4 changed files with 119 additions and 3 deletions
@@ -1,5 +1,8 @@
import { describe, expect, it } from 'vitest'
import { orchestrationMutationRecoveryError } from './orchestration-mutation-recovery'
import {
orchestrationMutationRecoveryError,
renderCommand
} from './orchestration-mutation-recovery'
import { RuntimeClientError } from './runtime-client'
describe('orchestration mutation recovery', () => {
@@ -111,4 +114,27 @@ describe('orchestration mutation recovery', () => {
'request_4'
])
})
it('renders Windows cmd recovery guidance without quote drift or percent expansion', () => {
expect(
renderCommand(
['orca', 'orchestration', 'worker-start', '--comment', 'literal "quoted" %PATH% & safe'],
'win32',
{ ComSpec: 'C:\\Windows\\System32\\cmd.exe' }
)
).toBe(
'"orca" "orchestration" "worker-start" "--comment" "literal ""quoted"" "^%"PATH"^%" & safe"'
)
})
it('keeps PowerShell and POSIX recovery guidance literal', () => {
expect(
renderCommand(['orca', 'literal "quoted" $HOME'], 'win32', {
ComSpec: 'powershell.exe'
})
).toBe("& 'orca' 'literal \\\"quoted\\\" $HOME'")
expect(renderCommand(['orca', 'literal $(do-not-run)'], 'darwin')).toBe(
"orca 'literal $(do-not-run)'"
)
})
})
+36 -2
View File
@@ -1,5 +1,9 @@
import { RuntimeClientError } from './runtime-client'
import { 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'
import type { AgentStartupShell } from '../shared/tui-agent-startup-shell'
export function orchestrationMutationRecoveryError(error: unknown): unknown {
if (!(error instanceof RuntimeClientError) || !isUnknownMutationOutcomeCode(error.code)) {
@@ -129,8 +133,38 @@ function parseCommandLine(value: string): string[] | undefined {
return parts.length > 0 ? parts : undefined
}
function renderCommand(command: readonly string[]): string {
return command.map(shellQuote).join(' ')
export function renderCommand(
command: readonly string[],
platform: NodeJS.Platform = process.platform,
env: NodeJS.ProcessEnv = process.env
): string {
const shell = resolveRecoveryShell(platform, env)
const rendered = command.map((value) => quoteRecoveryArgument(value, shell)).join(' ')
return shell === 'powershell' && rendered ? `& ${rendered}` : rendered
}
function resolveRecoveryShell(
platform: NodeJS.Platform,
env: NodeJS.ProcessEnv
): AgentStartupShell {
if (platform !== 'win32') {
return 'posix'
}
return resolveWindowsShellStartupFamily(
env.ORCA_TERMINAL_WINDOWS_SHELL ?? env.ORCA_WINDOWS_SHELL ?? env.ComSpec ?? env.COMSPEC
)
}
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.
return quoteWindowsCmdArgument(value)
}
if (shell === 'powershell') {
return quotePowerShellNativeArgument(value)
}
return shellQuote(value)
}
function shellQuote(value: string): string {
@@ -32,6 +32,17 @@ vi.mock('../git/worktree', () => {
})
describe('OrcaRuntimeRpcServer', () => {
it('classifies worker-start as a keepalive-backed long poll', () => {
expect(
longPollClassOf({
id: 'req_worker_start',
authToken: 'token',
method: 'orchestration.workerStart',
params: { task: 'task_1', timeoutMs: 60_000 }
})
).toBe('wait')
})
it('keeps agent-prompt submission sockets alive during verification', () => {
expect(
longPollClassOf({
@@ -92,6 +103,45 @@ describe('OrcaRuntimeRpcServer', () => {
// Exercise the real socket (not a mock) so we catch buffer/flush regressions
// that a unit-level test would miss.
describe('long-poll transport (§3.1)', () => {
it('emits keepalives while orchestration.workerStart blocks', async () => {
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-'))
const runtime = new OrcaRuntimeService()
const server = new OrcaRuntimeRpcServer({
runtime,
userDataPath,
keepaliveIntervalMs: 30
})
const dispatch = server['dispatcher']
vi.spyOn(dispatch, 'dispatch').mockImplementation(async (request) => {
await sleep(120)
return {
id: request.id,
ok: true,
result: { dispatch: { id: 'dispatch_1' } },
_meta: { runtimeId: runtime.getRuntimeId() }
}
})
await server.start()
try {
const metadata = readRuntimeMetadata(userDataPath)
const session = openFramedSession(metadata!.transports[0]!.endpoint, {
id: 'req_worker_start',
authToken: metadata!.authToken,
method: 'orchestration.workerStart',
params: { task: 'task_1', timeoutMs: 60_000 }
})
await session.done
expect(
session.frames.filter((frame) => frame._keepalive === true).length
).toBeGreaterThanOrEqual(2)
expect(session.frames.filter((frame) => frame.ok !== undefined)).toHaveLength(1)
} finally {
await server.stop()
}
})
it('emits keepalive frames while a check --wait handler blocks', async () => {
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-'))
const runtime = new OrcaRuntimeService()
+6
View File
@@ -446,6 +446,12 @@ type LongPollClass = 'ask' | 'wait'
// Why: single classifier for long-poll requests (handlers that block on an external event), shared by counter/abort/keepalive. See §3.1.
export function longPollClassOf(request: RpcRequest): LongPollClass | null {
// Worker start waits for readiness and then verifies the submitted prompt;
// the complete operation can run for 90–110s. Keep every local transport
// (Unix sockets and Windows named pipes) alive for that long poll.
if (request.method === 'orchestration.workerStart') {
return 'wait'
}
if (request.method === 'terminal.wait') {
return 'wait'
}