Fix orchestration payloads on Windows (#4608)
This commit is contained in:
parent
089fbd6951
commit
62600ef808
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { parseArgs, validateCommandAndFlags } from './args'
|
||||
import { parseArgs, supportsBrowserPageFlag, validateCommandAndFlags } from './args'
|
||||
|
||||
describe('parseArgs', () => {
|
||||
it('keeps an empty string as a flag value', () => {
|
||||
|
|
@ -39,6 +39,12 @@ describe('parseArgs', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('supportsBrowserPageFlag', () => {
|
||||
it('does not expose browser page targeting on orchestration commands', () => {
|
||||
expect(supportsBrowserPageFlag(['orchestration', 'send'])).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('validateCommandAndFlags', () => {
|
||||
const specs = [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -81,6 +81,7 @@ export function supportsBrowserPageFlag(commandPath: string[]): boolean {
|
|||
'worktree',
|
||||
'terminal',
|
||||
'file',
|
||||
'orchestration',
|
||||
'computer',
|
||||
'note',
|
||||
'diagnostics'
|
||||
|
|
|
|||
|
|
@ -56,6 +56,68 @@ describe('orchestration reset CLI handler', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('orchestration send structured payload flags', () => {
|
||||
beforeEach(() => {
|
||||
callMock.mockReset().mockResolvedValue({ result: { message: { id: 'msg_1' } } })
|
||||
delete process.env.ORCA_TERMINAL_HANDLE
|
||||
})
|
||||
|
||||
const invokeSend = (flags: Map<string, string | boolean>) =>
|
||||
ORCHESTRATION_HANDLERS['orchestration send']({
|
||||
flags,
|
||||
client: { call: callMock },
|
||||
cwd: '/tmp/repo',
|
||||
json: true
|
||||
} as never)
|
||||
|
||||
it('serializes common worker payload fields as JSON', async () => {
|
||||
await invokeSend(
|
||||
new Map<string, string | boolean>([
|
||||
['from', 'term_worker'],
|
||||
['to', 'term_coord'],
|
||||
['subject', 'done'],
|
||||
['type', 'worker_done'],
|
||||
['task-id', 'task_1'],
|
||||
['dispatch-id', 'ctx_1'],
|
||||
['files-modified', 'src/a.ts, src/b.ts'],
|
||||
['report-path', 'reports/done.md']
|
||||
])
|
||||
)
|
||||
|
||||
expect(callMock).toHaveBeenCalledWith('orchestration.send', {
|
||||
from: 'term_worker',
|
||||
to: 'term_coord',
|
||||
subject: 'done',
|
||||
body: undefined,
|
||||
type: 'worker_done',
|
||||
priority: undefined,
|
||||
threadId: undefined,
|
||||
payload: JSON.stringify({
|
||||
taskId: 'task_1',
|
||||
dispatchId: 'ctx_1',
|
||||
filesModified: ['src/a.ts', 'src/b.ts'],
|
||||
reportPath: 'reports/done.md'
|
||||
}),
|
||||
devMode: false
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects mixing raw payload with structured payload flags', async () => {
|
||||
await expect(
|
||||
invokeSend(
|
||||
new Map<string, string | boolean>([
|
||||
['from', 'term_worker'],
|
||||
['to', 'term_coord'],
|
||||
['subject', 'done'],
|
||||
['payload', '{"taskId":"task_1"}'],
|
||||
['task-id', 'task_1']
|
||||
])
|
||||
)
|
||||
).rejects.toThrow(/structured payload/)
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration timeout flag validation', () => {
|
||||
const invalidTimeoutValues: [string, string | boolean][] = [
|
||||
['missing', true],
|
||||
|
|
|
|||
|
|
@ -72,6 +72,54 @@ type MessageSummary = {
|
|||
payload?: string | null
|
||||
}
|
||||
|
||||
function getOptionalStructuredMessagePayload(
|
||||
flags: Map<string, string | boolean>
|
||||
): string | undefined {
|
||||
const rawPayload = getOptionalStringFlag(flags, 'payload')
|
||||
const taskId = getOptionalStringFlag(flags, 'task-id')
|
||||
const dispatchId = getOptionalStringFlag(flags, 'dispatch-id')
|
||||
const filesModified = getOptionalStringFlag(flags, 'files-modified')
|
||||
const reportPath = getOptionalStringFlag(flags, 'report-path')
|
||||
const phase = getOptionalStringFlag(flags, 'phase')
|
||||
const hasStructuredPayload =
|
||||
taskId !== undefined ||
|
||||
dispatchId !== undefined ||
|
||||
filesModified !== undefined ||
|
||||
reportPath !== undefined ||
|
||||
phase !== undefined
|
||||
if (!hasStructuredPayload) {
|
||||
return rawPayload
|
||||
}
|
||||
if (rawPayload !== undefined) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
'Use either --payload or structured payload flags, not both.'
|
||||
)
|
||||
}
|
||||
// Why: raw JSON arguments are fragile in Windows PowerShell; these flags let
|
||||
// workers send parseable orchestration payloads without shell-specific quoting.
|
||||
const payload: Record<string, string | string[]> = {}
|
||||
if (taskId) {
|
||||
payload.taskId = taskId
|
||||
}
|
||||
if (dispatchId) {
|
||||
payload.dispatchId = dispatchId
|
||||
}
|
||||
if (filesModified) {
|
||||
payload.filesModified = filesModified
|
||||
.split(',')
|
||||
.map((file) => file.trim())
|
||||
.filter(Boolean)
|
||||
}
|
||||
if (reportPath) {
|
||||
payload.reportPath = reportPath
|
||||
}
|
||||
if (phase) {
|
||||
payload.phase = phase
|
||||
}
|
||||
return JSON.stringify(payload)
|
||||
}
|
||||
|
||||
async function resolveOrchestrationTerminalHandle(
|
||||
flags: Map<string, string | boolean>,
|
||||
cwd: string,
|
||||
|
|
@ -127,7 +175,7 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
|||
type: getOptionalStringFlag(flags, 'type'),
|
||||
priority: getOptionalStringFlag(flags, 'priority'),
|
||||
threadId: getOptionalStringFlag(flags, 'thread-id'),
|
||||
payload: getOptionalStringFlag(flags, 'payload'),
|
||||
payload: getOptionalStructuredMessagePayload(flags),
|
||||
devMode: isDevCliInvocation()
|
||||
})
|
||||
printResult(result, json, (r) => {
|
||||
|
|
|
|||
|
|
@ -390,6 +390,11 @@ export function formatFlagHelp(flag: string): string {
|
|||
terminal: '--terminal <handle> Runtime-issued terminal handle',
|
||||
text: '--text <text> Text payload to send or type',
|
||||
'text-stdin': '--text-stdin Read text payload from stdin',
|
||||
'task-id': '--task-id <id> Task id to include in orchestration payload JSON',
|
||||
'dispatch-id': '--dispatch-id <id> Dispatch id to include in orchestration payload JSON',
|
||||
'files-modified': '--files-modified <csv> Comma-separated files for orchestration payload JSON',
|
||||
'report-path': '--report-path <path> Report path to include in orchestration payload JSON',
|
||||
phase: '--phase <text> Worker phase to include in orchestration payload JSON',
|
||||
'timeout-ms': '--timeout-ms <ms> Maximum wait time before timing out',
|
||||
'to-element-index': '--to-element-index <n> Destination element index from get-app-state',
|
||||
'to-x': '--to-x <x> Destination window-local x coordinate',
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
|||
path: ['orchestration', 'send'],
|
||||
summary: 'Send an inter-agent message',
|
||||
usage:
|
||||
'orca orchestration send --to <handle> --subject <text> [--from <handle>] [--body <text>] [--type <type>] [--priority <level>] [--thread-id <id>] [--payload <json>] [--json]',
|
||||
'orca orchestration send --to <handle> --subject <text> [--from <handle>] [--body <text>] [--type <type>] [--priority <level>] [--thread-id <id>] [--payload <json>] [--task-id <id>] [--dispatch-id <id>] [--files-modified <csv>] [--report-path <path>] [--phase <text>] [--json]',
|
||||
allowedFlags: [
|
||||
...GLOBAL_FLAGS,
|
||||
'to',
|
||||
|
|
@ -16,7 +16,16 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
|||
'type',
|
||||
'priority',
|
||||
'thread-id',
|
||||
'payload'
|
||||
'payload',
|
||||
'task-id',
|
||||
'dispatch-id',
|
||||
'files-modified',
|
||||
'report-path',
|
||||
'phase'
|
||||
],
|
||||
notes: [
|
||||
'On Windows PowerShell, quote group addresses such as --to "@all" or --to "@worktree:<id>".',
|
||||
'Prefer --task-id/--dispatch-id/etc. over raw --payload JSON in worker commands; PowerShell strips JSON quotes easily.'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
|
@ -39,6 +48,9 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
|||
'inject',
|
||||
'wait',
|
||||
'timeout-ms'
|
||||
],
|
||||
notes: [
|
||||
'On Windows PowerShell, quote comma-separated type filters, e.g. --types "worker_done,escalation".'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -25,7 +25,9 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
orca orchestration send --to term_COORD \\
|
||||
--type worker_done --subject "<short status>" \\
|
||||
--body "<3-sentence summary: what you did, what you found, what's left>" \\
|
||||
--payload '{"taskId":"task_SNAP","dispatchId":"ctx_SNAP","filesModified":["path/a","path/b"],"reportPath":"<optional: path to the full artifact>"}'
|
||||
--task-id task_SNAP --dispatch-id ctx_SNAP \\
|
||||
--files-modified "path/a,path/b" \\
|
||||
--report-path "<optional: path to the full artifact>"
|
||||
|
||||
# BEHAVIOR RULE: send a heartbeat every 5 minutes
|
||||
# while actively working on the task. The coordinator uses this to
|
||||
|
|
@ -39,7 +41,8 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
# cannot mask a hung retry.
|
||||
orca orchestration send --to term_COORD \\
|
||||
--type heartbeat --subject "alive" \\
|
||||
--payload '{"taskId":"task_SNAP","dispatchId":"ctx_SNAP","phase":"<short: investigating|implementing|reviewing|waiting>"}'
|
||||
--task-id task_SNAP --dispatch-id ctx_SNAP \\
|
||||
--phase "<short: investigating|implementing|reviewing|waiting>"
|
||||
|
||||
# Ask the coordinator a question and block until it answers.
|
||||
#
|
||||
|
|
@ -63,7 +66,7 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
orca orchestration send --to term_COORD \\
|
||||
--type escalation --subject "Blocked: <reason>" \\
|
||||
--body "<details>" \\
|
||||
--payload '{"taskId":"task_SNAP"}'
|
||||
--task-id task_SNAP
|
||||
|
||||
# Check for messages from the coordinator:
|
||||
orca orchestration check
|
||||
|
|
|
|||
|
|
@ -32,11 +32,10 @@ describe('buildDispatchPreamble', () => {
|
|||
expect(result).toContain('--body')
|
||||
expect(result).toMatch(/3-sentence summary/)
|
||||
expect(result).toContain('reportPath')
|
||||
const workerDoneLine = result
|
||||
.split('\n')
|
||||
.find((line) => line.includes('"taskId"') && line.includes('filesModified'))
|
||||
expect(workerDoneLine).toContain('dispatchId')
|
||||
expect(workerDoneLine).toContain('ctx_def456')
|
||||
expect(result).toContain('--task-id task_abc123')
|
||||
expect(result).toContain('--dispatch-id ctx_def456')
|
||||
expect(result).toContain('--files-modified "path/a,path/b"')
|
||||
expect(result).toContain('--report-path "<optional: path to the full artifact>"')
|
||||
})
|
||||
|
||||
it('CLI examples parse as valid shell (bash -n on the extracted block)', () => {
|
||||
|
|
@ -64,15 +63,12 @@ describe('buildDispatchPreamble', () => {
|
|||
expect(result).toContain('--type heartbeat')
|
||||
expect(result).toContain('--subject "alive"')
|
||||
expect(result).toMatch(/5 minutes/)
|
||||
// Both taskId and dispatchId are rendered inside the payload template
|
||||
// Both taskId and dispatchId are rendered as structured payload flags
|
||||
// (regression guard for §5.3.4 attribution — dispatchId attribution
|
||||
// prevents the zombie-heartbeat-masks-hung-retry race).
|
||||
const heartbeatLine = result
|
||||
.split('\n')
|
||||
.find((line) => line.includes('"taskId"') && line.includes('dispatchId'))
|
||||
expect(heartbeatLine).toBeTruthy()
|
||||
expect(heartbeatLine).toContain('task_abc123')
|
||||
expect(heartbeatLine).toContain('ctx_def456')
|
||||
expect(result).toContain('--task-id task_abc123')
|
||||
expect(result).toContain('--dispatch-id ctx_def456')
|
||||
expect(result).toContain('--phase "<short: investigating|implementing|reviewing|waiting>"')
|
||||
})
|
||||
|
||||
it('includes ask block with BEHAVIOR RULE #1 forbidding AskUserQuestion', () => {
|
||||
|
|
|
|||
|
|
@ -64,7 +64,9 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
${cli} orchestration send --to ${params.coordinatorHandle} \\
|
||||
--type worker_done --subject "<short status>" \\
|
||||
--body "<3-sentence summary: what you did, what you found, what's left>" \\
|
||||
--payload '{"taskId":"${params.taskId}","dispatchId":"${params.dispatchId}","filesModified":["path/a","path/b"],"reportPath":"<optional: path to the full artifact>"}'
|
||||
--task-id ${params.taskId} --dispatch-id ${params.dispatchId} \\
|
||||
--files-modified "path/a,path/b" \\
|
||||
--report-path "<optional: path to the full artifact>"
|
||||
|
||||
# BEHAVIOR RULE: send a heartbeat every ${HEARTBEAT_INTERVAL_MIN} minutes
|
||||
# while actively working on the task. The coordinator uses this to
|
||||
|
|
@ -78,7 +80,8 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
# cannot mask a hung retry.
|
||||
${cli} orchestration send --to ${params.coordinatorHandle} \\
|
||||
--type heartbeat --subject "alive" \\
|
||||
--payload '{"taskId":"${params.taskId}","dispatchId":"${params.dispatchId}","phase":"<short: investigating|implementing|reviewing|waiting>"}'
|
||||
--task-id ${params.taskId} --dispatch-id ${params.dispatchId} \\
|
||||
--phase "<short: investigating|implementing|reviewing|waiting>"
|
||||
|
||||
# Ask the coordinator a question and block until it answers.
|
||||
#
|
||||
|
|
@ -102,7 +105,7 @@ Slack, GitHub comments, or any other channel to reach a human during the run.
|
|||
${cli} orchestration send --to ${params.coordinatorHandle} \\
|
||||
--type escalation --subject "Blocked: <reason>" \\
|
||||
--body "<details>" \\
|
||||
--payload '{"taskId":"${params.taskId}"}'
|
||||
--task-id ${params.taskId}
|
||||
|
||||
# Check for messages from the coordinator:
|
||||
${cli} orchestration check
|
||||
|
|
|
|||
|
|
@ -46,7 +46,9 @@ describe('ACTIVE_TAB_INDICATOR_CLASSES', () => {
|
|||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain('absolute')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain('bottom-0')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain('h-[2px]')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain('bg-foreground')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain(
|
||||
'bg-[color-mix(in_srgb,var(--foreground)_60%,var(--card))]'
|
||||
)
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).toContain('pointer-events-none')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).not.toContain('-top-px')
|
||||
expect(ACTIVE_TAB_INDICATOR_CLASSES).not.toContain('bg-[#1e3d9c]')
|
||||
|
|
@ -56,7 +58,7 @@ describe('ACTIVE_TAB_INDICATOR_CLASSES', () => {
|
|||
describe('getTabRootStateClasses', () => {
|
||||
it('returns the shared selected-tab surface treatment', () => {
|
||||
const classes = getTabRootStateClasses(true)
|
||||
expect(classes).toContain('bg-[color-mix(in_srgb,var(--foreground)_10%,var(--card))]')
|
||||
expect(classes).toContain('bg-[color-mix(in_srgb,var(--foreground)_6%,var(--card))]')
|
||||
expect(classes).toContain('text-foreground')
|
||||
expect(classes).not.toContain('hover:text-foreground')
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue