mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(orchestration): recover stripped task deps
This commit is contained in:
@@ -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<string, string | boolean>([
|
||||
['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()
|
||||
})
|
||||
})
|
||||
@@ -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 })
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
@@ -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',
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
@@ -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: <T>(method: string, params?: unknown, options?: unknown) => Promise<{ result: T }>
|
||||
}
|
||||
|
||||
type CliHandler = (ctx: {
|
||||
flags: Map<string, string | boolean>
|
||||
client: CliRuntimeClient
|
||||
cwd: string
|
||||
json: boolean
|
||||
}) => Promise<void>
|
||||
|
||||
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<string, unknown>) {
|
||||
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<T>(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<Record<string, CliHandler>> {
|
||||
vi.doMock('../../../../cli/format', () => ({ printResult: vi.fn() }))
|
||||
const cliModulePath = '../../../../cli/handlers/orchestration'
|
||||
const module = (await import(cliModulePath)) as {
|
||||
ORCHESTRATION_HANDLERS: Record<string, CliHandler>
|
||||
}
|
||||
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<void> {
|
||||
await handler({
|
||||
flags: new Map<string, string | boolean>(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<string, unknown> {
|
||||
return params && typeof params === 'object' ? (params as Record<string, unknown>) : {}
|
||||
}
|
||||
|
||||
/** 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
|
||||
}
|
||||
}
|
||||
@@ -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) =>
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user