From 07df4bf0bef3a9426afc26116eb155fa7bc8170d Mon Sep 17 00:00:00 2001 From: Chen Date: Thu, 27 Aug 2026 16:20:43 +0800 Subject: [PATCH] fix(orchestration): recover stripped task deps --- .../orchestration-task-create-cli.test.ts | 54 +++++++ ...ative-cli-powershell-argv-boundary.test.ts | 60 ++++++++ .../session-worktree-ownership.ts | 1 + .../orchestration/task-deps-flag.test.ts | 51 +++++++ .../runtime/orchestration/task-deps-flag.ts | 34 +++++ ...orchestration-cli-runtime-boundary.test.ts | 138 ++++++++++++++++++ .../orchestration-tasks-dispatch.test.ts | 13 ++ src/main/runtime/rpc/methods/orchestration.ts | 14 +- 8 files changed, 353 insertions(+), 12 deletions(-) create mode 100644 src/cli/handlers/orchestration-task-create-cli.test.ts create mode 100644 src/main/cli/native-cli-powershell-argv-boundary.test.ts create mode 100644 src/main/runtime/orchestration/task-deps-flag.test.ts create mode 100644 src/main/runtime/orchestration/task-deps-flag.ts create mode 100644 src/main/runtime/rpc/methods/orchestration-cli-runtime-boundary.test.ts diff --git a/src/cli/handlers/orchestration-task-create-cli.test.ts b/src/cli/handlers/orchestration-task-create-cli.test.ts new file mode 100644 index 00000000000..839ccabdcd6 --- /dev/null +++ b/src/cli/handlers/orchestration-task-create-cli.test.ts @@ -0,0 +1,54 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const callMock = vi.hoisted(() => vi.fn()) +const getTerminalHandleMock = vi.hoisted(() => vi.fn()) +const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE + +// Why: isolate flag-to-RPC mapping; printResult only writes output. +vi.mock('../format', () => ({ printResult: vi.fn() })) +vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock })) + +import { ORCHESTRATION_HANDLERS } from './orchestration' + +describe('orchestration task-create CLI mapping', () => { + beforeEach(() => { + callMock.mockReset() + getTerminalHandleMock.mockReset() + process.env.ORCA_TERMINAL_HANDLE = 'term_creator' + }) + + afterEach(() => { + if (originalTerminalHandle === undefined) { + delete process.env.ORCA_TERMINAL_HANDLE + } else { + process.env.ORCA_TERMINAL_HANDLE = originalTerminalHandle + } + }) + + it('passes PowerShell-stripped deps through to the runtime', async () => { + callMock + .mockResolvedValueOnce({ result: { terminal: { handle: 'term_creator' } } }) + .mockResolvedValueOnce({ result: { task: { id: 'task_2', status: 'pending' } } }) + + await ORCHESTRATION_HANDLERS['orchestration task-create']({ + flags: new Map([ + ['spec', 'do child work'], + ['deps', '[task_b2a580db74d8]'] + ]), + client: { call: callMock }, + cwd: '/tmp/repo', + json: true + } as never) + + expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', { + spec: 'do child work', + taskTitle: undefined, + displayName: undefined, + deps: '[task_b2a580db74d8]', + parent: undefined, + run: undefined, + callerTerminalHandle: 'term_creator' + }) + expect(getTerminalHandleMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/cli/native-cli-powershell-argv-boundary.test.ts b/src/main/cli/native-cli-powershell-argv-boundary.test.ts new file mode 100644 index 00000000000..2c701f0ca47 --- /dev/null +++ b/src/main/cli/native-cli-powershell-argv-boundary.test.ts @@ -0,0 +1,60 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { runProcess } from '../../shared/child-process/run-process' +import { windowsPowerShellPath } from '../../shared/child-process/windows-system-binary' +import { parseOrchestrationTaskDepsFlag } from '../runtime/orchestration/task-deps-flag' + +const PARENT_TASK_ID = 'task_b2a580db74d8' + +describe('native CLI PowerShell argv boundary', () => { + it.skipIf(process.platform !== 'win32')( + 'recovers ConvertTo-Json dependencies after PowerShell 5.1 builds native argv', + async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-native-powershell-argv-')) + const scriptPath = join(root, 'invoke-deps.ps1') + const targetPath = join(root, 'argv-target.cjs') + + try { + await writeFile( + scriptPath, + [ + `$deps = ConvertTo-Json -Compress @('${PARENT_TASK_ID}')`, + '& $args[0] $args[1] --deps $deps', + 'exit $LASTEXITCODE' + ].join('\n'), + 'utf8' + ) + await writeFile( + targetPath, + 'process.stdout.write(JSON.stringify(process.argv.slice(2)))\n', + 'utf8' + ) + + const result = await runProcess({ + program: windowsPowerShellPath(), + args: [ + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'Bypass', + '-File', + scriptPath, + process.execPath, + targetPath + ] + }) + + expect(result.code).toBe(0) + expect(result.stderr).toBe('') + const argv = JSON.parse(result.stdout.trim()) as string[] + expect(argv[0]).toBe('--deps') + expect(argv[1]).toBe(`[${PARENT_TASK_ID}]`) + expect(parseOrchestrationTaskDepsFlag(argv[1])).toEqual([PARENT_TASK_ID]) + } finally { + await rm(root, { recursive: true, force: true }) + } + } + ) +}) diff --git a/src/main/persistence/restoring-sessions/session-worktree-ownership.ts b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts index dc572e79278..e06e39dc5c5 100644 --- a/src/main/persistence/restoring-sessions/session-worktree-ownership.ts +++ b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts @@ -38,6 +38,7 @@ export const WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND = { clientHostedBrowserCloseIntentsByEnvironment: 'row-arrays', activeTabTypeByWorktree: 'owner-keyed', browserUrlHistory: 'none', + workspaceDocHistory: 'none', activeTabIdByWorktree: 'owner-keyed', unifiedTabs: 'owner-keyed-row-arrays', tabGroups: 'owner-keyed-row-arrays', diff --git a/src/main/runtime/orchestration/task-deps-flag.test.ts b/src/main/runtime/orchestration/task-deps-flag.test.ts new file mode 100644 index 00000000000..193fa5deafc --- /dev/null +++ b/src/main/runtime/orchestration/task-deps-flag.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { generateId } from './db/generated-id' +import { parseOrchestrationTaskDepsFlag } from './task-deps-flag' + +describe('parseOrchestrationTaskDepsFlag', () => { + it('accepts canonical JSON string arrays', () => { + expect(parseOrchestrationTaskDepsFlag('["task_b2a580db74d8"]')).toEqual(['task_b2a580db74d8']) + expect(parseOrchestrationTaskDepsFlag('[]')).toEqual([]) + }) + + it('recovers quote-stripped generated task IDs', () => { + expect(parseOrchestrationTaskDepsFlag('[task_b2a580db74d8]')).toEqual(['task_b2a580db74d8']) + expect(parseOrchestrationTaskDepsFlag('[ task_b2a580db74d8 , task_907c556bfed6 ]')).toEqual([ + 'task_b2a580db74d8', + 'task_907c556bfed6' + ]) + }) + + it('rejects valid JSON with the wrong shape without entering argv recovery', () => { + const recoveryProbe = { + [Symbol.toPrimitive]: () => '[1]', + trim: () => { + throw new Error('argv recovery ran') + } + } as unknown as string + + expect(() => parseOrchestrationTaskDepsFlag(recoveryProbe)).toThrow( + 'Invalid --deps: must be a JSON array of task IDs' + ) + }) + + it.each([ + 'not-json', + 'task_b2a580db74d8', + '[task_example]', + '[not_a_task]', + '[task_b2a580db74d8,]', + '[task_b2a580db74d8,,task_907c556bfed6]', + '[task_b2a580db74d8 task_907c556bfed6]', + '{"deps":["task_b2a580db74d8"]}', + '[{"id":"task_b2a580db74d8"}]', + '[1]' + ])('rejects unsupported input %s', (raw) => { + expect(() => parseOrchestrationTaskDepsFlag(raw)).toThrow('Invalid --deps') + }) + + it('tracks the generated task ID contract', () => { + expect(parseOrchestrationTaskDepsFlag(`[${generateId('task')}]`)).toHaveLength(1) + expect(() => parseOrchestrationTaskDepsFlag('[task_abc]')).toThrow('Invalid --deps') + }) +}) diff --git a/src/main/runtime/orchestration/task-deps-flag.ts b/src/main/runtime/orchestration/task-deps-flag.ts new file mode 100644 index 00000000000..18627c8372b --- /dev/null +++ b/src/main/runtime/orchestration/task-deps-flag.ts @@ -0,0 +1,34 @@ +const INVALID_DEPS_ERROR = 'Invalid --deps: must be a JSON array of task IDs' +const GENERATED_TASK_ID_PATTERN = /^task_[0-9a-f]{12}$/i + +// Windows PowerShell 5.1 strips the quotes in `["task_x"]` at the native argv boundary. +export function parseOrchestrationTaskDepsFlag(raw: string): string[] { + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + const recovered = recoverArgvStrippedTaskDeps(raw) + if (recovered) { + return recovered + } + throw new Error(INVALID_DEPS_ERROR) + } + + if (!Array.isArray(parsed) || !parsed.every((entry) => typeof entry === 'string')) { + throw new Error(INVALID_DEPS_ERROR) + } + return parsed +} + +function recoverArgvStrippedTaskDeps(raw: string): string[] | null { + const trimmed = raw.trim() + if (!trimmed.startsWith('[') || !trimmed.endsWith(']')) { + return null + } + const body = trimmed.slice(1, -1).trim() + if (body.length === 0) { + return [] + } + const ids = body.split(',').map((entry) => entry.trim()) + return ids.every((id) => GENERATED_TASK_ID_PATTERN.test(id)) ? ids : null +} diff --git a/src/main/runtime/rpc/methods/orchestration-cli-runtime-boundary.test.ts b/src/main/runtime/rpc/methods/orchestration-cli-runtime-boundary.test.ts new file mode 100644 index 00000000000..8ffa47fc34a --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration-cli-runtime-boundary.test.ts @@ -0,0 +1,138 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RpcContext } from '../core' +import { createOrchestrationRpcHarness } from './orchestration-rpc-test-harness' +import type { OrchestrationDb } from '../../orchestration/db' + +type CliRuntimeClient = { + isRemote?: boolean + call: (method: string, params?: unknown, options?: unknown) => Promise<{ result: T }> +} + +type CliHandler = (ctx: { + flags: Map + client: CliRuntimeClient + cwd: string + json: boolean +}) => Promise + +const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE + +describe('orchestration CLI/runtime boundary', () => { + const h = createOrchestrationRpcHarness() + const { findMethod } = h + let db: OrchestrationDb + let ctx: RpcContext + + afterEach(() => { + h.cleanup() + restoreTerminalHandle() + vi.doUnmock('../../../../cli/format') + vi.resetModules() + }) + + /** Bridges CLI client calls into the in-memory RPC harness without bypassing RPC param parsing. */ + async function callRpc(name: string, params: Record) { + const method = findMethod(name) + const parsed = method.params ? method.params.parse(params) : undefined + return method.handler(parsed, ctx) + } + + /** Builds the fake RuntimeClient used by the real CLI handlers in this boundary test. */ + function client(): CliRuntimeClient { + return { + isRemote: false, + /** Preserves terminal-handle validation while routing other calls through runtime RPC. */ + async call(method: string, params?: unknown): Promise<{ result: T }> { + if (method === 'terminal.show') { + return { result: { terminal: { handle: objectParams(params).terminal } } as T } + } + return { result: (await callRpc(method, objectParams(params))) as T } + } + } + } + + it('creates and gates a PowerShell-stripped dependency through CLI and runtime', async () => { + ;({ db, ctx } = h.setup()) + process.env.ORCA_TERMINAL_HANDLE = 'term_coord' + const handlers = await loadOrchestrationHandlers() + const runtimeClient = client() + + await runCli(handlers['orchestration task-create'], runtimeClient, [['spec', 'parent work']]) + const parent = taskBySpec('parent work') + + await runCli(handlers['orchestration task-create'], runtimeClient, [ + ['spec', 'child work'], + ['deps', `[${parent.id}]`] + ]) + const child = taskBySpec('child work') + + expect(child.status).toBe('pending') + expect(child.deps).toBe(JSON.stringify([parent.id])) + await expect( + runCli(handlers['orchestration dispatch'], runtimeClient, [ + ['task', child.id], + ['to', 'term_worker'] + ]) + ).rejects.toThrow('only ready tasks can be dispatched') + + await runCli(handlers['orchestration task-update'], runtimeClient, [ + ['id', parent.id], + ['status', 'completed'] + ]) + expect(db.getTask(child.id)?.status).toBe('ready') + + await runCli(handlers['orchestration dispatch'], runtimeClient, [ + ['task', child.id], + ['to', 'term_worker'] + ]) + expect(db.getTask(child.id)?.status).toBe('dispatched') + expect(db.getDispatchContext(child.id)?.assignee_handle).toBe('term_worker') + }) + + /** Looks up real DB-created tasks by unique fixture spec after the CLI allocates their IDs. */ + function taskBySpec(spec: string) { + const task = db.listTasks().find((candidate) => candidate.spec === spec) + if (!task) { + throw new Error(`Expected task with spec: ${spec}`) + } + return task + } +}) + +/** Imports orchestration handlers after mocking output so the test observes state, not stdout. */ +async function loadOrchestrationHandlers(): Promise> { + vi.doMock('../../../../cli/format', () => ({ printResult: vi.fn() })) + const cliModulePath = '../../../../cli/handlers/orchestration' + const module = (await import(cliModulePath)) as { + ORCHESTRATION_HANDLERS: Record + } + return module.ORCHESTRATION_HANDLERS +} + +/** Executes one CLI handler with argv-like flags against the supplied runtime client. */ +async function runCli( + handler: CliHandler, + client: CliRuntimeClient, + entries: [string, string | boolean][] +): Promise { + await handler({ + flags: new Map(entries), + client, + cwd: process.cwd(), + json: true + }) +} + +/** Narrows optional RPC params into the object shape expected by the RPC parser. */ +function objectParams(params: unknown): Record { + return params && typeof params === 'object' ? (params as Record) : {} +} + +/** Restores the caller terminal environment so later CLI tests do not inherit this fixture. */ +function restoreTerminalHandle(): void { + if (originalTerminalHandle === undefined) { + delete process.env.ORCA_TERMINAL_HANDLE + } else { + process.env.ORCA_TERMINAL_HANDLE = originalTerminalHandle + } +} diff --git a/src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts b/src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts index 4a87c53059d..a216b4c7a4d 100644 --- a/src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts @@ -52,6 +52,19 @@ describe('orchestration RPC methods', () => { expect(result.task.status).toBe('pending') }) + it('persists a dependency from a PowerShell-stripped CLI deps payload', async () => { + setup() + const t1 = db.createTask({ spec: 'first' }) + + const result = (await call('orchestration.taskCreate', { + spec: 'second', + deps: `[${t1.id}]` + })) as { task: { id: string; status: string } } + + expect(result.task.status).toBe('pending') + expect(db.getTask(result.task.id)?.deps).toBe(JSON.stringify([t1.id])) + }) + it('records the caller pane, process, and Run generation when creating a task', async () => { setup() vi.spyOn(runtime, 'getTerminalPaneKey').mockImplementation((handle) => diff --git a/src/main/runtime/rpc/methods/orchestration.ts b/src/main/runtime/rpc/methods/orchestration.ts index 4045ce11409..88734efeb74 100644 --- a/src/main/runtime/rpc/methods/orchestration.ts +++ b/src/main/runtime/rpc/methods/orchestration.ts @@ -29,6 +29,7 @@ import { type SendRecipientWarning } from './orchestration-recipient-routing' import { buildInjectRejectionMessage } from './orchestration-inject-rejection-message' +import { parseOrchestrationTaskDepsFlag } from '../../orchestration/task-deps-flag' import { resolveRunScope } from './orchestration-run-scope' import { ORCHESTRATION_RUN_METHODS } from './orchestration-runs' import { ORCHESTRATION_WORKER_METHODS } from './orchestration-worker-methods' @@ -1478,18 +1479,7 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [ params: TaskCreateParams, handler: (params, { orchestrationCompatibilityEvidence, runtime, legacyCoordinatorRunId }) => { const db = runtime.getOrchestrationDb() - let deps: string[] | undefined - if (params.deps) { - try { - const parsed = JSON.parse(params.deps) - if (!Array.isArray(parsed) || !parsed.every((d) => typeof d === 'string')) { - throw new Error('not an array of strings') - } - deps = parsed - } catch { - throw new Error('Invalid --deps: must be a JSON array of task IDs') - } - } + const deps = params.deps ? parseOrchestrationTaskDepsFlag(params.deps) : undefined const run = resolveRunScope(runtime, { runId: params.run, callerTerminalHandle: params.callerTerminalHandle,