mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
feat(orchestration): bundled improvements — check-wait, stale-base, preamble+ask, QoL (#1403)
* feat(orchestration): transport keepalive + delivered_at split for check --wait
Implements the four §3 fixes from the check-wait design doc:
- §3.1 Transport keepalive: long-poll RPCs (orchestration.check --wait) emit
`{"_keepalive":true}` frames every 10s so neither server nor client tears
the socket down on idle. A `longPoll` admission counter capped at 16 fails
fast with `runtime_busy` when saturated; an AbortController wired through
the RPC dispatcher cancels the inner waiter the moment the socket closes.
- §3.2 delivered_at split: push-on-idle now stamps `delivered_at` instead of
flipping `read`, so the check caller remains the sole consumer of its
queue. Adds a synchronous idempotent schema migration that hard-fails on
error.
- §3.3 inbox/check parity: `orchestration inbox --terminal <handle>` and
`orchestration check --all` agree on the same rows (sequence DESC, no
mark-read). `check --unread=false` kept for one release as a compat shim.
- §3.4 CLI heartbeat: `orca orchestration check --wait` emits JSON heartbeat
lines to stderr every 15s so Claude Code's Bash tool sees continuous
output and doesn't auto-background the subprocess.
Tests: extends runtime-rpc, orca-runtime, envelope-schema, orchestration
method, and formatter suites; adds a subprocess test that spawns the built
CLI and verifies stderr line-flushing, heartbeat cadence, and stdout
cleanliness end-to-end.
Co-authored-by: Orca <help@stably.ai>
* feat(orchestration): preamble rules + heartbeat schema
- Preamble (#7, #15, #9): worker_done body ("3-sentence summary" + reportPath),
BEHAVIOR RULE #1 forbidding AskUserQuestion, heartbeat every 5 minutes with
taskId+dispatchId payload, AFTER YOU SEND grace window.
- Schema v2 migration: adds 'heartbeat' to messages.type CHECK, adds
dispatch_contexts.last_heartbeat_at, gated by user_version PRAGMA with
transactional rebuild + explicit CREATE INDEX to avoid silent perf regress.
- DB helpers: recordHeartbeat (dispatched-only), getStaleDispatches,
getThreadMessagesFor (thread+handle scoped for ask).
Co-authored-by: Orca <help@stably.ai>
* feat(orchestration): coordinator heartbeat + stale detector
Handle incoming 'heartbeat' messages by calling recordHeartbeat keyed on
payload.dispatchId (strict — log-and-skip if missing, no taskId fallback so
a straggler heartbeat from a previously-failed dispatch cannot mask a hung
retry per §5.3.4). On every tick after the 10-minute threshold, emit one
log per stale dispatched row — no auto-fail.
Also threads dispatchId through buildDispatchPreamble so workers can
attribute their heartbeats back to the correct dispatch context.
Co-authored-by: Orca <help@stably.ai>
* feat(orchestration): orca orchestration ask verb
Adds a CLI verb that sends a decision_gate message and blocks on the
coordinator's reply, scoped to the outbound message's thread. Group
addresses (@all, @idle, …) are rejected — fan-out questions must use
send --type decision_gate explicitly.
--json emits bare single-line JSON (bypassing printResult) so workers can
pipe `orca orchestration ask … --json | jq -r .answer` without unwrapping
an RPC envelope; human mode prints just the answer. On timeout the verb
exits 1 and returns {answer: null, timedOut: true}.
This is the CLI surface BEHAVIOR RULE #1 in the dispatch preamble points
workers at instead of AskUserQuestion.
Co-authored-by: Orca <help@stably.ai>
* feat(orchestration): QoL bundle — preamble visibility, status enum, dispatch cross-ref, inbox --full
Addresses four items from ORCHESTRATOR_FEEDBACK:
- #5 preamble visibility: `dispatch-show --preamble` regenerates the preamble
text for a task; `dispatch --inject --dry-run` previews without mutating
state; `dispatch --return-preamble` echoes the injected preamble in the JSON
response so coordinators can audit what a worker received.
- #6 status enum validation: CLI rejects unknown `task-update --status` values
with `invalid status '<x>', expected one of: pending, ready, dispatched,
completed, failed, blocked` before the RPC's generic Zod message. Valid
statuses are listed under Notes in `task-update --help`.
- #13 task-list dispatch cross-ref: `task-list --json` now includes
`assignee_handle` and `dispatch_id` for tasks in status=dispatched via a
read-only LEFT JOIN on dispatch_contexts. Non-dispatched rows keep their
legacy shape so existing consumers are unaffected.
- #14 inbox body visibility: `inbox --full` prints body + payload verbatim;
default output is unchanged (id/from/to/subject only).
No DB migrations; join-only change on dispatch_contexts so the sibling
preamble PR's `last_heartbeat_at` column addition will not conflict.
Co-authored-by: Orca <help@stably.ai>
* fix(worktree): prevent stale-base worktree creation and dispatch
Addresses feedback #16 per DESIGN_DOC_STALE_BASE_FIX.md §0. Four v1
components coordinated by a single shared fetch cache on the runtime:
1. Concurrent-fetch-with-gate in UI create path: `createLocalWorktree`
fires `git fetch` BEFORE the suffix loop / PR probe / path
resolution, then awaits right before `addWorktree` so the new branch
always spawns from a fresh remote tip. Renderer sees a two-phase
spinner via the new `createWorktree:progress` IPC event. The cache
is a `Map<repoPath::remote, Promise<void>>` + 30s success-only
timestamp on `OrcaRuntimeService` (§7.1 — shared with dispatch).
2. Dispatch pre-flight drift guard in `Coordinator.dispatchTask`:
probes `rev-list --left-right --count` against the target worktree
and silently returns (preserves `ready`, no circuit-breaker burn)
when `behind > 20` unless the task spec carries
`allow-stale-base: true`. Parsing strips the flag so it never leaks
into the worker's `--- TASK ---` block.
3. Preamble drift section: populated only when dispatch detected drift.
Workers see `--- BASE DRIFT ---` with the N-most-recent subjects
they don't have, so they can pull them in before running.
4. §3.3 Lifecycle: `.finally()` evicts Map entries on BOTH success and
rejection; timestamp is written ONLY on success. Prevents a single
DNS hiccup from wedging every future create on the repo until
restart, and keeps the freshness window honest.
Defers the DB `allow_stale_base` column (§0.2) and the create-time
warn toast; both can layer in later without migration.
Tests: 35 new/updated unit tests covering drift preamble, dispatch
refusal, spec-text flag parsing, fetch Map eviction after rejection,
freshness-window short-circuit, and concurrent-caller serialization.
Co-authored-by: Orca <help@stably.ai>
* test(orchestration): seed v2 DB in migration hard-fail test
After consolidating the schema bump, fresh DBs are initialized directly at
v3 via createTables(), so the v2→v3 ALTER TABLE is skipped on new installs
and the prior test's stub never fired. Seed a v2-shape file on disk so the
guarded ALTER actually runs and the "simulated migration failure" stub
propagates as intended.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
/* eslint-disable max-lines -- Why: orchestration CLI handlers share flag-parsing helpers and dispatch/preamble logic; splitting by verb would fragment the RuntimeClient call shape without reducing complexity. */
|
||||
import type { CommandHandler } from '../dispatch'
|
||||
import { printResult } from '../format'
|
||||
import {
|
||||
@@ -5,14 +6,70 @@ import {
|
||||
getOptionalStringFlag,
|
||||
getRequiredStringFlag
|
||||
} from '../flags'
|
||||
import { RuntimeClientError } from '../runtime-client'
|
||||
import { getTerminalHandle } from '../selectors'
|
||||
|
||||
// Why: 15 s is well under Claude Code's empirical ~2 min Bash-tool silence
|
||||
// budget and generates only ~40 lines per 10 min wait — enough to assure the
|
||||
// parent process the subprocess is alive without flooding logs. See design
|
||||
// doc §3.4.
|
||||
const DEFAULT_HEARTBEAT_INTERVAL_MS = 15_000
|
||||
|
||||
// Why: test-only escape hatch so subprocess tests can verify the feature in
|
||||
// under 10 s rather than needing a full 15 s silence window. Production users
|
||||
// should never set this — there is no surface documentation. A bogus value
|
||||
// falls back to the default rather than disabling the heartbeat.
|
||||
function resolveHeartbeatIntervalMs(): number {
|
||||
const raw = process.env.ORCA_HEARTBEAT_INTERVAL_MS
|
||||
if (!raw) {
|
||||
return DEFAULT_HEARTBEAT_INTERVAL_MS
|
||||
}
|
||||
const parsed = Number(raw)
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) {
|
||||
return DEFAULT_HEARTBEAT_INTERVAL_MS
|
||||
}
|
||||
return parsed
|
||||
}
|
||||
|
||||
function startCheckHeartbeat(deadlineMs: number | undefined): () => void {
|
||||
const startedAt = Date.now()
|
||||
const interval = setInterval(() => {
|
||||
const payload = {
|
||||
_heartbeat: true,
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
deadlineMs: deadlineMs ?? null
|
||||
}
|
||||
// Why: `process.stderr.write` is line-flushed per-call in Node, whereas a
|
||||
// fully-buffered writer would hold all heartbeat lines until exit and
|
||||
// silently defeat the whole point of the ping. Subprocess test asserts
|
||||
// this by reading stderr incrementally. See §3.4.
|
||||
process.stderr.write(`${JSON.stringify(payload)}\n`)
|
||||
}, resolveHeartbeatIntervalMs())
|
||||
if (typeof interval.unref === 'function') {
|
||||
interval.unref()
|
||||
}
|
||||
return () => clearInterval(interval)
|
||||
}
|
||||
|
||||
// Why: mirrors TaskStatus (orchestration/types.ts) so the CLI can surface a
|
||||
// clear enum-aware error before the generic RPC Zod "Missing --status" message.
|
||||
const TASK_STATUS_VALUES = [
|
||||
'pending',
|
||||
'ready',
|
||||
'dispatched',
|
||||
'completed',
|
||||
'failed',
|
||||
'blocked'
|
||||
] as const
|
||||
|
||||
type MessageSummary = {
|
||||
id: string
|
||||
from_handle: string
|
||||
to_handle?: string
|
||||
subject: string
|
||||
type?: string
|
||||
body?: string
|
||||
payload?: string | null
|
||||
}
|
||||
|
||||
async function resolveOrchestrationTerminalHandle(
|
||||
@@ -62,18 +119,36 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
|
||||
'orchestration check': async ({ flags, client, cwd, json }) => {
|
||||
const terminal = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'terminal')
|
||||
const result = await client.call<{
|
||||
const wait = flags.has('wait')
|
||||
const timeoutMs = flags.has('timeout-ms') ? Number(flags.get('timeout-ms')) : undefined
|
||||
|
||||
// Why: Claude Code's Bash tool auto-backgrounds subprocesses that produce
|
||||
// no output for ~2 min (shorter on the non-interactive path). Emit a
|
||||
// heartbeat line to stderr every HEARTBEAT_INTERVAL_MS while the wait is
|
||||
// active so the parent process can see the subprocess is still alive.
|
||||
// Stderr rather than stdout so stdout stays a single final JSON payload,
|
||||
// and JSON-shaped rather than `# …` so `2>&1 | jq` pipelines still work
|
||||
// (jq refuses `#`-prefixed lines). See design doc §3.4.
|
||||
const stopHeartbeat = wait ? startCheckHeartbeat(timeoutMs) : null
|
||||
type CheckResult = {
|
||||
messages: MessageSummary[]
|
||||
count: number
|
||||
formatted?: string
|
||||
}>('orchestration.check', {
|
||||
terminal,
|
||||
unread: flags.has('unread') ? true : undefined,
|
||||
types: getOptionalStringFlag(flags, 'types'),
|
||||
inject: flags.has('inject') ? true : undefined,
|
||||
wait: flags.has('wait') ? true : undefined,
|
||||
timeoutMs: flags.has('timeout-ms') ? Number(flags.get('timeout-ms')) : undefined
|
||||
})
|
||||
}
|
||||
let result: Awaited<ReturnType<typeof client.call<CheckResult>>>
|
||||
try {
|
||||
result = await client.call<CheckResult>('orchestration.check', {
|
||||
terminal,
|
||||
unread: flags.has('unread') ? true : undefined,
|
||||
all: flags.has('all') ? true : undefined,
|
||||
types: getOptionalStringFlag(flags, 'types'),
|
||||
inject: flags.has('inject') ? true : undefined,
|
||||
wait: wait ? true : undefined,
|
||||
timeoutMs
|
||||
})
|
||||
} finally {
|
||||
stopHeartbeat?.()
|
||||
}
|
||||
printResult(result, json, (r) => {
|
||||
if (r.formatted) {
|
||||
return r.formatted
|
||||
@@ -98,19 +173,36 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
},
|
||||
|
||||
'orchestration inbox': async ({ flags, client, json }) => {
|
||||
const full = flags.has('full')
|
||||
const result = await client.call<{
|
||||
messages: MessageSummary[]
|
||||
count: number
|
||||
}>('orchestration.inbox', {
|
||||
limit: getOptionalPositiveIntegerFlag(flags, 'limit')
|
||||
limit: getOptionalPositiveIntegerFlag(flags, 'limit'),
|
||||
terminal: getOptionalStringFlag(flags, 'terminal')
|
||||
})
|
||||
printResult(result, json, (r) => {
|
||||
if (r.count === 0) {
|
||||
return 'No messages.'
|
||||
}
|
||||
// Why: default output omits body/payload for at-a-glance sweeps; --full
|
||||
// prints them verbatim so callers can audit without parsing --json.
|
||||
return r.messages
|
||||
.map((m) => `${m.id} ${m.from_handle} -> ${m.to_handle ?? '?'}: "${m.subject}"`)
|
||||
.join('\n')
|
||||
.map((m) => {
|
||||
const head = `${m.id} ${m.from_handle} -> ${m.to_handle ?? '?'}: "${m.subject}"`
|
||||
if (!full) {
|
||||
return head
|
||||
}
|
||||
const parts = [head]
|
||||
if (m.body && m.body.length > 0) {
|
||||
parts.push(m.body)
|
||||
}
|
||||
if (m.payload) {
|
||||
parts.push(`[payload] ${m.payload}`)
|
||||
}
|
||||
return parts.join('\n')
|
||||
})
|
||||
.join(full ? '\n\n' : '\n')
|
||||
})
|
||||
},
|
||||
|
||||
@@ -128,7 +220,13 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
|
||||
'orchestration task-list': async ({ flags, client, json }) => {
|
||||
const result = await client.call<{
|
||||
tasks: { id: string; spec: string; status: string }[]
|
||||
tasks: {
|
||||
id: string
|
||||
spec: string
|
||||
status: string
|
||||
assignee_handle?: string | null
|
||||
dispatch_id?: string | null
|
||||
}[]
|
||||
count: number
|
||||
}>('orchestration.taskList', {
|
||||
status: getOptionalStringFlag(flags, 'status'),
|
||||
@@ -138,16 +236,31 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
if (r.count === 0) {
|
||||
return 'No tasks.'
|
||||
}
|
||||
return r.tasks.map((t) => `${t.id} [${t.status}] ${t.spec.slice(0, 60)}`).join('\n')
|
||||
return r.tasks
|
||||
.map((t) => {
|
||||
const head = `${t.id} [${t.status}] ${t.spec.slice(0, 60)}`
|
||||
if (t.status === 'dispatched' && t.assignee_handle) {
|
||||
return `${head} -> ${t.assignee_handle} (${t.dispatch_id ?? '?'})`
|
||||
}
|
||||
return head
|
||||
})
|
||||
.join('\n')
|
||||
})
|
||||
},
|
||||
|
||||
'orchestration task-update': async ({ flags, client, json }) => {
|
||||
const status = getRequiredStringFlag(flags, 'status')
|
||||
if (!TASK_STATUS_VALUES.includes(status as (typeof TASK_STATUS_VALUES)[number])) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
`invalid status '${status}', expected one of: ${TASK_STATUS_VALUES.join(', ')}`
|
||||
)
|
||||
}
|
||||
const result = await client.call<{ task: { id: string; status: string } }>(
|
||||
'orchestration.taskUpdate',
|
||||
{
|
||||
id: getRequiredStringFlag(flags, 'id'),
|
||||
status: getRequiredStringFlag(flags, 'status'),
|
||||
status,
|
||||
result: getOptionalStringFlag(flags, 'result')
|
||||
}
|
||||
)
|
||||
@@ -156,29 +269,95 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
|
||||
'orchestration dispatch': async ({ flags, client, cwd, json }) => {
|
||||
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
const result = await client.call<{
|
||||
dispatch: { id: string; task_id: string; status: string }
|
||||
}>('orchestration.dispatch', {
|
||||
task: getRequiredStringFlag(flags, 'task'),
|
||||
to: getRequiredStringFlag(flags, 'to'),
|
||||
from,
|
||||
inject: flags.has('inject') ? true : undefined,
|
||||
devMode: isDevCliInvocation()
|
||||
})
|
||||
printResult(
|
||||
result,
|
||||
json,
|
||||
(r) => `Dispatched ${r.dispatch.task_id} -> ${r.dispatch.id} [${r.dispatch.status}]`
|
||||
)
|
||||
},
|
||||
|
||||
'orchestration dispatch-show': async ({ flags, client, json }) => {
|
||||
const dryRun = flags.has('dry-run') ? true : undefined
|
||||
const returnPreamble = flags.has('return-preamble') ? true : undefined
|
||||
// Why: --to is only required for non-dry-run; the RPC handler re-enforces.
|
||||
const to = dryRun ? getOptionalStringFlag(flags, 'to') : getRequiredStringFlag(flags, 'to')
|
||||
const result = await client.call<{
|
||||
dispatch: { id: string; task_id: string; status: string } | null
|
||||
}>('orchestration.dispatchShow', {
|
||||
task: getRequiredStringFlag(flags, 'task')
|
||||
injected?: boolean
|
||||
dryRun?: boolean
|
||||
preamble?: string
|
||||
}>('orchestration.dispatch', {
|
||||
task: getRequiredStringFlag(flags, 'task'),
|
||||
to,
|
||||
from,
|
||||
inject: flags.has('inject') ? true : undefined,
|
||||
dryRun,
|
||||
returnPreamble,
|
||||
devMode: isDevCliInvocation()
|
||||
})
|
||||
printResult(result, json, (r) => {
|
||||
if (r.dryRun) {
|
||||
return r.preamble ?? ''
|
||||
}
|
||||
const base = `Dispatched ${r.dispatch?.task_id} -> ${r.dispatch?.id} [${r.dispatch?.status}]`
|
||||
return r.preamble ? `${base}\n\n--- Preamble ---\n${r.preamble}` : base
|
||||
})
|
||||
},
|
||||
|
||||
'orchestration ask': async ({ flags, client, cwd, json }) => {
|
||||
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
const timeoutMs = flags.has('timeout-ms') ? Number(flags.get('timeout-ms')) : 600_000
|
||||
const result = await client.call<{
|
||||
answer: string | null
|
||||
messageId: string | null
|
||||
threadId: string
|
||||
timedOut: boolean
|
||||
}>(
|
||||
'orchestration.ask',
|
||||
{
|
||||
to: getRequiredStringFlag(flags, 'to'),
|
||||
question: getRequiredStringFlag(flags, 'question'),
|
||||
options: getOptionalStringFlag(flags, 'options'),
|
||||
timeoutMs: flags.has('timeout-ms') ? Number(flags.get('timeout-ms')) : undefined,
|
||||
from
|
||||
},
|
||||
// Why: the runtime's `waitForMessage` can block up to `timeoutMs`, but
|
||||
// the RPC transport has its own 60s default timeout that would fire
|
||||
// first. Extend the per-call timeout by a small grace window so the
|
||||
// RPC doesn't abort before the runtime's internal timeout resolves.
|
||||
{ timeoutMs: timeoutMs + 5_000 }
|
||||
)
|
||||
// Why: deliberate bypass of `printResult`. `--json` on `ask` emits a
|
||||
// single-line bare JSON object (no RPC envelope, no multi-line pretty-
|
||||
// print) so workers can pipe `orca orchestration ask … --json | jq -r
|
||||
// .answer` without reaching into a `result` envelope. This diverges from
|
||||
// every other orchestration verb; called out in the commit message and
|
||||
// guarded by a unit test in orchestration.test.ts.
|
||||
if (json) {
|
||||
console.log(JSON.stringify(result.result))
|
||||
} else if (result.result.answer !== null) {
|
||||
console.log(result.result.answer)
|
||||
}
|
||||
if (result.result.timedOut) {
|
||||
if (!json) {
|
||||
console.error(`ask timeout after ${timeoutMs}ms (thread ${result.result.threadId})`)
|
||||
}
|
||||
process.exitCode = 1
|
||||
}
|
||||
},
|
||||
|
||||
'orchestration dispatch-show': async ({ flags, client, cwd, json }) => {
|
||||
const showPreamble = flags.has('preamble') ? true : undefined
|
||||
// Why: resolve --from when previewing so the preamble embeds a real
|
||||
// coordinator handle, matching what an actual dispatch would produce.
|
||||
const from = showPreamble
|
||||
? await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
: undefined
|
||||
const result = await client.call<{
|
||||
dispatch: { id: string; task_id: string; status: string } | null
|
||||
preamble?: string
|
||||
}>('orchestration.dispatchShow', {
|
||||
task: getRequiredStringFlag(flags, 'task'),
|
||||
preamble: showPreamble,
|
||||
from,
|
||||
devMode: isDevCliInvocation()
|
||||
})
|
||||
printResult(result, json, (r) => {
|
||||
if (r.preamble && showPreamble) {
|
||||
return r.preamble
|
||||
}
|
||||
if (!r.dispatch) {
|
||||
return 'No dispatch context found.'
|
||||
}
|
||||
|
||||
@@ -197,6 +197,31 @@ describe('orca cli worktree awareness', () => {
|
||||
expect(logSpy).toHaveBeenCalledWith('Sent 2 messages to 2 recipients')
|
||||
})
|
||||
|
||||
it('rejects unknown task-update status with an enum-aware error', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_coord'
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const priorExitCode = process.exitCode
|
||||
|
||||
await main(
|
||||
['orchestration', 'task-update', '--id', 'task_x', '--status', 'complete'],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
const output = [...errSpy.mock.calls, ...logSpy.mock.calls]
|
||||
.flat()
|
||||
.map((v) => (typeof v === 'string' ? v : JSON.stringify(v)))
|
||||
.join('\n')
|
||||
expect(output).toContain("invalid status 'complete'")
|
||||
expect(output).toContain('pending, ready, dispatched, completed, failed, blocked')
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
expect(process.exitCode).toBe(1)
|
||||
|
||||
// Reset exitCode so subsequent tests don't inherit the failure.
|
||||
process.exitCode = priorExitCode
|
||||
errSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('passes dev mode to injected orchestration dispatches', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_sender'
|
||||
process.env.ORCA_USER_DATA_PATH = '/tmp/orca-dev'
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
// Why: subprocess-level test for the CLI heartbeat behavior described in
|
||||
// design doc §3.4. Spawns the real compiled CLI with no TTY, points it at a
|
||||
// real in-process runtime via ORCA_USER_DATA_PATH, and asserts:
|
||||
// - the first heartbeat line appears on stderr well under Claude Code's
|
||||
// ~2 min Bash-tool silence budget (we verify with a shortened interval;
|
||||
// production uses 15 s via the same code path)
|
||||
// - ≥3 heartbeats arrive during the wait window
|
||||
// - stderr is line-flushed (we observe each heartbeat as a separate chunk
|
||||
// before the process exits — not in one burst at the end)
|
||||
// - stdout stays a single clean JSON payload (no heartbeats leak to stdout)
|
||||
// - a `jq "select(._heartbeat|not)"` filter on the merged stream would
|
||||
// yield exactly the final result
|
||||
//
|
||||
// This test is skipped if the CLI hasn't been built yet (out/cli/index.js
|
||||
// missing) so `pnpm test` works on a fresh checkout without requiring a prior
|
||||
// `pnpm run build:cli`. The verification gate explicitly builds the CLI
|
||||
// before running this file.
|
||||
import { spawn } from 'child_process'
|
||||
import { existsSync, mkdtempSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { OrcaRuntimeService } from '../main/runtime/orca-runtime'
|
||||
import { OrchestrationDb } from '../main/runtime/orchestration/db'
|
||||
import { OrcaRuntimeRpcServer } from '../main/runtime/runtime-rpc'
|
||||
|
||||
// Why: Vitest runs tests with `process.cwd()` pinned to the repo root, so
|
||||
// join against it to locate the compiled CLI regardless of where this test
|
||||
// file itself lives.
|
||||
const CLI_PATH = join(process.cwd(), 'out', 'cli', 'index.js')
|
||||
|
||||
const describeIfBuilt = existsSync(CLI_PATH) ? describe : describe.skip
|
||||
|
||||
describeIfBuilt('orca orchestration check --wait subprocess (§3.4)', () => {
|
||||
it('emits newline-flushed JSON heartbeats to stderr while waiting', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-cli-sub-'))
|
||||
const runtime = new OrcaRuntimeService()
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
runtime.setOrchestrationDb(db)
|
||||
const server = new OrcaRuntimeRpcServer({ runtime, userDataPath })
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
// Why: use the ORCA_HEARTBEAT_INTERVAL_MS escape hatch to shrink the
|
||||
// test to ~1 s wall time. Production callers never set this; the
|
||||
// production default (15 s) is exercised by §3.4's own unit tests
|
||||
// and by the fact that this same code path runs with the real
|
||||
// constant when the env var is absent.
|
||||
const heartbeatMs = 200
|
||||
const waitTimeoutMs = 1200
|
||||
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
[
|
||||
CLI_PATH,
|
||||
'orchestration',
|
||||
'check',
|
||||
'--wait',
|
||||
'--timeout-ms',
|
||||
String(waitTimeoutMs),
|
||||
'--json'
|
||||
],
|
||||
{
|
||||
env: {
|
||||
...process.env,
|
||||
ORCA_USER_DATA_PATH: userDataPath,
|
||||
ORCA_TERMINAL_HANDLE: 'term_nobody',
|
||||
ORCA_HEARTBEAT_INTERVAL_MS: String(heartbeatMs)
|
||||
},
|
||||
// Why: explicit pipe for all three fds so we can watch stderr
|
||||
// in real time; no TTY attached (Bash-tool parity).
|
||||
stdio: ['ignore', 'pipe', 'pipe']
|
||||
}
|
||||
)
|
||||
|
||||
const stderrChunks: { at: number; data: string }[] = []
|
||||
const stdoutChunks: { at: number; data: string }[] = []
|
||||
const startedAt = Date.now()
|
||||
child.stderr.setEncoding('utf8')
|
||||
child.stdout.setEncoding('utf8')
|
||||
child.stderr.on('data', (d) => stderrChunks.push({ at: Date.now() - startedAt, data: d }))
|
||||
child.stdout.on('data', (d) => stdoutChunks.push({ at: Date.now() - startedAt, data: d }))
|
||||
|
||||
const exitCode = await new Promise<number>((resolveExit, rejectExit) => {
|
||||
child.once('exit', (code) => resolveExit(code ?? 1))
|
||||
child.once('error', rejectExit)
|
||||
})
|
||||
|
||||
expect(exitCode).toBe(0)
|
||||
|
||||
const stderr = stderrChunks.map((c) => c.data).join('')
|
||||
const stdout = stdoutChunks.map((c) => c.data).join('')
|
||||
|
||||
const heartbeatLines = stderr
|
||||
.split('\n')
|
||||
.filter((line) => line.trim().length > 0)
|
||||
.map((line) => {
|
||||
try {
|
||||
return JSON.parse(line) as Record<string, unknown>
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
})
|
||||
.filter((p): p is Record<string, unknown> => p !== null && p._heartbeat === true)
|
||||
|
||||
// ≥3 heartbeats in a 1.2s window with a 200ms interval
|
||||
expect(heartbeatLines.length).toBeGreaterThanOrEqual(3)
|
||||
expect(heartbeatLines[0]).toHaveProperty('elapsedMs')
|
||||
expect(heartbeatLines[0]).toHaveProperty('deadlineMs', waitTimeoutMs)
|
||||
|
||||
// Why: first heartbeat must arrive within one interval + scheduler
|
||||
// slack (300ms is generous); if the stream were fully buffered we'd
|
||||
// see everything only after exit.
|
||||
const firstHeartbeatChunk = stderrChunks.find((c) => c.data.includes('_heartbeat'))
|
||||
expect(firstHeartbeatChunk).toBeDefined()
|
||||
expect(firstHeartbeatChunk!.at).toBeLessThan(heartbeatMs + 300)
|
||||
|
||||
// Why: line-flushing proof — the *first* heartbeat chunk must arrive
|
||||
// strictly before the exit chunk; i.e. we got at least two separate
|
||||
// stderr data events (heartbeat + final). A single-chunk delivery
|
||||
// would indicate stderr was buffered until exit.
|
||||
const lastStderrAt = stderrChunks.at(-1)?.at ?? 0
|
||||
const firstStderrAt = stderrChunks.at(0)?.at ?? 0
|
||||
expect(lastStderrAt).toBeGreaterThan(firstStderrAt)
|
||||
|
||||
// Stdout: exactly one JSON payload, the terminal result. No heartbeats
|
||||
// leak, and the content parses as valid JSON.
|
||||
const stdoutTrimmed = stdout.trim()
|
||||
const stdoutPayload = JSON.parse(stdoutTrimmed) as Record<string, unknown>
|
||||
expect(stdoutPayload).not.toHaveProperty('_heartbeat')
|
||||
expect(stdoutTrimmed).not.toContain('_heartbeat')
|
||||
// Why: result should be an RPC success envelope with the expected
|
||||
// shape. `count: 0` and `messages: []` because the wait timed out
|
||||
// with no message for term_nobody.
|
||||
expect(stdoutPayload).toMatchObject({ ok: true })
|
||||
|
||||
// Why: the heartbeats-on-stderr design is meant to pair with shell
|
||||
// filters like `2>&1 | jq "select(._heartbeat|not)"`. jq is
|
||||
// line-oriented by default, but also accepts pretty-printed JSON
|
||||
// across multiple lines. What matters here is that every
|
||||
// heartbeat line on stderr is a standalone JSON object (so jq can
|
||||
// match it) and doesn't span multiple lines — assert that each
|
||||
// heartbeat is a single-line JSON with no embedded newlines.
|
||||
for (const line of stderr.split('\n')) {
|
||||
if (line.trim().length === 0) {
|
||||
continue
|
||||
}
|
||||
if (line.includes('_heartbeat')) {
|
||||
expect(() => JSON.parse(line)).not.toThrow()
|
||||
expect(line).not.toContain('\n')
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
db.close()
|
||||
await server.stop()
|
||||
}
|
||||
}, 30_000)
|
||||
})
|
||||
@@ -5,6 +5,14 @@ import { getCliStatus } from './status'
|
||||
import { sendRequest } from './transport'
|
||||
import { RuntimeClientError, RuntimeRpcFailureError, type RuntimeRpcSuccess } from './types'
|
||||
|
||||
// Why: for `orchestration.check --wait` the caller's method-level
|
||||
// `params.timeoutMs` is the inner waiter budget; we extend the client-side
|
||||
// socket timeout to `timeoutMs + GRACE_MS` so the client's own idle timer
|
||||
// never fires before the server-side waiter has had a chance to resolve and
|
||||
// emit its terminal frame. The 10 s grace absorbs round-trip + one final
|
||||
// keepalive window. See design doc §3.1.
|
||||
const LONG_POLL_CLIENT_GRACE_MS = 10_000
|
||||
|
||||
export class RuntimeClient {
|
||||
private readonly userDataPath: string
|
||||
private readonly requestTimeoutMs: number
|
||||
@@ -25,18 +33,30 @@ export class RuntimeClient {
|
||||
}
|
||||
): Promise<RuntimeRpcSuccess<TResult>> {
|
||||
const metadata = readMetadata(this.userDataPath)
|
||||
const response = await sendRequest<TResult>(
|
||||
metadata,
|
||||
method,
|
||||
params,
|
||||
options?.timeoutMs ?? this.requestTimeoutMs
|
||||
)
|
||||
const effectiveTimeoutMs = options?.timeoutMs ?? this.resolveMethodTimeoutMs(method, params)
|
||||
const response = await sendRequest<TResult>(metadata, method, params, effectiveTimeoutMs)
|
||||
if (!response.ok) {
|
||||
throw new RuntimeRpcFailureError(response)
|
||||
}
|
||||
return response
|
||||
}
|
||||
|
||||
// Why: centralises the per-method timeout policy. `orchestration.check` with
|
||||
// `wait: true` is the only long-poll today, and its inner waiter budget
|
||||
// lives in `params.timeoutMs`. We widen the client-side socket timeout to
|
||||
// `timeoutMs + grace` so it doesn't fire before the server has a chance to
|
||||
// resolve. Without this, a 5 min wait would still die at the 60 s default.
|
||||
// See design doc §3.1.
|
||||
private resolveMethodTimeoutMs(method: string, params?: unknown): number {
|
||||
if (method === 'orchestration.check' && isWaitingCheck(params)) {
|
||||
const inner = Number((params as { timeoutMs?: unknown }).timeoutMs)
|
||||
if (Number.isFinite(inner) && inner > 0) {
|
||||
return Math.max(inner + LONG_POLL_CLIENT_GRACE_MS, this.requestTimeoutMs)
|
||||
}
|
||||
}
|
||||
return this.requestTimeoutMs
|
||||
}
|
||||
|
||||
async getCliStatus(): Promise<RuntimeRpcSuccess<CliStatusResult>> {
|
||||
return getCliStatus(this.userDataPath)
|
||||
}
|
||||
@@ -67,3 +87,12 @@ export class RuntimeClient {
|
||||
function delay(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms))
|
||||
}
|
||||
|
||||
function isWaitingCheck(params: unknown): boolean {
|
||||
return (
|
||||
typeof params === 'object' &&
|
||||
params !== null &&
|
||||
'wait' in params &&
|
||||
(params as { wait: unknown }).wait === true
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { RuntimeRpcEnvelopeSchema } from './envelope-schema'
|
||||
import { RuntimeRpcEnvelopeSchema, isKeepaliveFrame } from './envelope-schema'
|
||||
|
||||
describe('RuntimeRpcEnvelopeSchema', () => {
|
||||
it('accepts a well-formed success envelope', () => {
|
||||
@@ -57,4 +57,37 @@ describe('RuntimeRpcEnvelopeSchema', () => {
|
||||
const parsed = RuntimeRpcEnvelopeSchema.safeParse({ hello: 'world' })
|
||||
expect(parsed.success).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts a keepalive frame', () => {
|
||||
const parsed = RuntimeRpcEnvelopeSchema.safeParse({ _keepalive: true })
|
||||
expect(parsed.success).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a keepalive frame with _keepalive !== true', () => {
|
||||
const parsed = RuntimeRpcEnvelopeSchema.safeParse({ _keepalive: false })
|
||||
expect(parsed.success).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isKeepaliveFrame', () => {
|
||||
it('returns true for a well-formed keepalive', () => {
|
||||
expect(isKeepaliveFrame({ _keepalive: true })).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false for a success envelope', () => {
|
||||
expect(isKeepaliveFrame({ id: 'x', ok: true, result: {}, _meta: { runtimeId: 'r' } })).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('returns false for non-object inputs', () => {
|
||||
expect(isKeepaliveFrame(null)).toBe(false)
|
||||
expect(isKeepaliveFrame(undefined)).toBe(false)
|
||||
expect(isKeepaliveFrame('keepalive')).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when _keepalive is not strictly true', () => {
|
||||
expect(isKeepaliveFrame({ _keepalive: 1 })).toBe(false)
|
||||
expect(isKeepaliveFrame({ _keepalive: 'true' })).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -36,4 +36,27 @@ const Failure = z.object({
|
||||
_meta: MetaFailure
|
||||
})
|
||||
|
||||
export const RuntimeRpcEnvelopeSchema = z.discriminatedUnion('ok', [Success, Failure])
|
||||
// Why: transport-layer keepalive frame (server→client only). Not a terminal
|
||||
// frame — the client reads past it and keeps waiting for the real
|
||||
// success/failure. `id` and `_meta` are deliberately absent: keepalives carry
|
||||
// no method-level semantics and aren't tied to a particular request (one
|
||||
// connection handles one request today). See design doc §3.1.
|
||||
const Keepalive = z.object({
|
||||
_keepalive: z.literal(true)
|
||||
})
|
||||
|
||||
// Why: switched from z.discriminatedUnion('ok', …) to z.union because
|
||||
// keepalives have no `ok` field. Client code must branch on
|
||||
// `'_keepalive' in frame` before treating the frame as Success/Failure.
|
||||
export const RuntimeRpcEnvelopeSchema = z.union([Success, Failure, Keepalive])
|
||||
|
||||
export type RuntimeRpcKeepaliveFrame = z.infer<typeof Keepalive>
|
||||
|
||||
export function isKeepaliveFrame(frame: unknown): frame is RuntimeRpcKeepaliveFrame {
|
||||
return (
|
||||
typeof frame === 'object' &&
|
||||
frame !== null &&
|
||||
'_keepalive' in frame &&
|
||||
(frame as { _keepalive: unknown })._keepalive === true
|
||||
)
|
||||
}
|
||||
|
||||
+100
-45
@@ -1,7 +1,7 @@
|
||||
import { createConnection } from 'net'
|
||||
import { randomUUID } from 'crypto'
|
||||
import type { RuntimeMetadata, RuntimeTransportMetadata } from '../../shared/runtime-bootstrap'
|
||||
import { RuntimeRpcEnvelopeSchema } from './envelope-schema'
|
||||
import { isKeepaliveFrame, RuntimeRpcEnvelopeSchema } from './envelope-schema'
|
||||
import { RuntimeClientError, type RuntimeRpcResponse } from './types'
|
||||
|
||||
export async function sendRequest<TResult>(
|
||||
@@ -13,9 +13,14 @@ export async function sendRequest<TResult>(
|
||||
return await new Promise((resolve, reject) => {
|
||||
const socket = createConnection(getTransportEndpoint(metadata.transport!))
|
||||
let buffer = ''
|
||||
let settled = false
|
||||
const requestId = randomUUID()
|
||||
|
||||
const timeout = setTimeout(() => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
socket.destroy()
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
@@ -25,28 +30,72 @@ export async function sendRequest<TResult>(
|
||||
)
|
||||
}, timeoutMs)
|
||||
|
||||
const finish = (
|
||||
result: { ok: true; response: RuntimeRpcResponse<TResult> } | { ok: false; error: Error }
|
||||
): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timeout)
|
||||
socket.end()
|
||||
if (result.ok) {
|
||||
resolve(result.response)
|
||||
} else {
|
||||
reject(result.error)
|
||||
}
|
||||
}
|
||||
|
||||
socket.setEncoding('utf8')
|
||||
socket.once('error', () => {
|
||||
clearTimeout(timeout)
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
finish({
|
||||
ok: false,
|
||||
error: new RuntimeClientError(
|
||||
'runtime_unavailable',
|
||||
'Could not connect to the running Orca app. Restart Orca and try again.'
|
||||
)
|
||||
)
|
||||
})
|
||||
})
|
||||
socket.on('data', (chunk) => {
|
||||
buffer += chunk
|
||||
const newlineIndex = buffer.indexOf('\n')
|
||||
if (newlineIndex === -1) {
|
||||
return
|
||||
}
|
||||
const message = buffer.slice(0, newlineIndex)
|
||||
socket.end()
|
||||
clearTimeout(timeout)
|
||||
let response: RuntimeRpcResponse<TResult>
|
||||
try {
|
||||
const raw: unknown = JSON.parse(message)
|
||||
// Why: the server may interleave `{"_keepalive":true}\n` frames with the
|
||||
// final success/failure frame to keep both idle timers alive during a
|
||||
// long-poll (see design doc §3.1). Read frames in a loop until we see a
|
||||
// terminal frame. Each keepalive refreshes the client-side timer so a
|
||||
// 10 min wait doesn't trip the 60 s default ceiling.
|
||||
let newlineIndex = buffer.indexOf('\n')
|
||||
while (newlineIndex !== -1 && !settled) {
|
||||
const line = buffer.slice(0, newlineIndex)
|
||||
buffer = buffer.slice(newlineIndex + 1)
|
||||
if (line.trim().length === 0) {
|
||||
newlineIndex = buffer.indexOf('\n')
|
||||
continue
|
||||
}
|
||||
|
||||
let raw: unknown
|
||||
try {
|
||||
raw = JSON.parse(line)
|
||||
} catch {
|
||||
finish({
|
||||
ok: false,
|
||||
error: new RuntimeClientError(
|
||||
'invalid_runtime_response',
|
||||
'The Orca runtime returned an invalid response frame.'
|
||||
)
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
// Fast-path: ignore keepalives without running the full schema.
|
||||
// setTimeout().refresh() is stable since Node 10 (Orca ships on
|
||||
// Node 20+ via Electron and the standalone CLI targets the same
|
||||
// major). See §7 risk #9.
|
||||
if (isKeepaliveFrame(raw)) {
|
||||
timeout.refresh()
|
||||
newlineIndex = buffer.indexOf('\n')
|
||||
continue
|
||||
}
|
||||
|
||||
// Why: validate the envelope shape (id, ok, result/error, _meta) at
|
||||
// the decode boundary so version skew between the CLI and the Orca
|
||||
// main runtime surfaces as a single invalid_runtime_response instead
|
||||
@@ -54,43 +103,49 @@ export async function sendRequest<TResult>(
|
||||
// unknown — the TResult generic is the caller's responsibility.
|
||||
const parsed = RuntimeRpcEnvelopeSchema.safeParse(raw)
|
||||
if (!parsed.success) {
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
finish({
|
||||
ok: false,
|
||||
error: new RuntimeClientError(
|
||||
'invalid_runtime_response',
|
||||
'The Orca runtime returned an invalid response frame.'
|
||||
)
|
||||
)
|
||||
})
|
||||
return
|
||||
}
|
||||
response = parsed.data as RuntimeRpcResponse<TResult>
|
||||
} catch {
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
'invalid_runtime_response',
|
||||
'The Orca runtime returned an invalid response frame.'
|
||||
)
|
||||
)
|
||||
|
||||
// Narrow out keepalive (already filtered above) so TS can see a
|
||||
// Success|Failure shape here.
|
||||
const frame = parsed.data
|
||||
if ('_keepalive' in frame) {
|
||||
timeout.refresh()
|
||||
newlineIndex = buffer.indexOf('\n')
|
||||
continue
|
||||
}
|
||||
|
||||
const response = frame as RuntimeRpcResponse<TResult>
|
||||
if (response.id !== requestId) {
|
||||
finish({
|
||||
ok: false,
|
||||
error: new RuntimeClientError(
|
||||
'invalid_runtime_response',
|
||||
'The Orca runtime returned a mismatched response id.'
|
||||
)
|
||||
})
|
||||
return
|
||||
}
|
||||
if (response._meta?.runtimeId && response._meta.runtimeId !== metadata.runtimeId) {
|
||||
finish({
|
||||
ok: false,
|
||||
error: new RuntimeClientError(
|
||||
'runtime_unavailable',
|
||||
'The Orca runtime changed while the request was in flight. Retry the command.'
|
||||
)
|
||||
})
|
||||
return
|
||||
}
|
||||
finish({ ok: true, response })
|
||||
return
|
||||
}
|
||||
if (response.id !== requestId) {
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
'invalid_runtime_response',
|
||||
'The Orca runtime returned a mismatched response id.'
|
||||
)
|
||||
)
|
||||
return
|
||||
}
|
||||
if (response._meta?.runtimeId && response._meta.runtimeId !== metadata.runtimeId) {
|
||||
reject(
|
||||
new RuntimeClientError(
|
||||
'runtime_unavailable',
|
||||
'The Orca runtime changed while the request was in flight. Retry the command.'
|
||||
)
|
||||
)
|
||||
return
|
||||
}
|
||||
resolve(response)
|
||||
})
|
||||
socket.on('connect', () => {
|
||||
socket.write(
|
||||
|
||||
@@ -23,8 +23,23 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
||||
path: ['orchestration', 'check'],
|
||||
summary: 'Check messages for a terminal',
|
||||
usage:
|
||||
'orca orchestration check [--terminal <handle>] [--unread] [--types <type,...>] [--inject] [--wait] [--timeout-ms <n>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'terminal', 'unread', 'types', 'inject', 'wait', 'timeout-ms']
|
||||
'orca orchestration check [--terminal <handle>] [--unread | --all] [--types <type,...>] [--inject] [--wait] [--timeout-ms <n>] [--json]\n' +
|
||||
' --unread (default): return only unread messages and mark them read.\n' +
|
||||
' --all: return every message for the handle; does not mark read.\n' +
|
||||
' --wait: block until a matching message arrives or --timeout-ms expires.\n' +
|
||||
' Emits JSON heartbeat lines to stderr every 15s so the caller can\n' +
|
||||
' tell the process is alive. Filter with `grep -v _heartbeat` or\n' +
|
||||
' `jq "select(._heartbeat|not)"` when merging streams with 2>&1.',
|
||||
allowedFlags: [
|
||||
...GLOBAL_FLAGS,
|
||||
'terminal',
|
||||
'unread',
|
||||
'all',
|
||||
'types',
|
||||
'inject',
|
||||
'wait',
|
||||
'timeout-ms'
|
||||
]
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'reply'],
|
||||
@@ -34,9 +49,9 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'inbox'],
|
||||
summary: 'Show all messages across recipients',
|
||||
usage: 'orca orchestration inbox [--limit <n>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'limit']
|
||||
summary: 'Show messages across (or for) recipients',
|
||||
usage: 'orca orchestration inbox [--limit <n>] [--terminal <handle>] [--full] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'limit', 'terminal', 'full']
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'task-create'],
|
||||
@@ -56,20 +71,29 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
||||
summary: 'Update a task status',
|
||||
usage:
|
||||
'orca orchestration task-update --id <task_id> --status <status> [--result <json>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'id', 'status', 'result']
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'id', 'status', 'result'],
|
||||
notes: ['Valid --status values: pending, ready, dispatched, completed, failed, blocked.']
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'dispatch'],
|
||||
summary: 'Dispatch a task to a terminal',
|
||||
usage:
|
||||
'orca orchestration dispatch --task <task_id> --to <handle> [--from <handle>] [--inject] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'task', 'to', 'from', 'inject']
|
||||
'orca orchestration dispatch --task <task_id> --to <handle> [--from <handle>] [--inject] [--dry-run] [--return-preamble] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'task', 'to', 'from', 'inject', 'dry-run', 'return-preamble']
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'dispatch-show'],
|
||||
summary: 'Show dispatch context for a task',
|
||||
usage: 'orca orchestration dispatch-show --task <task_id> [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'task']
|
||||
usage:
|
||||
'orca orchestration dispatch-show --task <task_id> [--preamble] [--from <handle>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'task', 'preamble', 'from']
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'ask'],
|
||||
summary: 'Ask the coordinator a question and block until answered',
|
||||
usage:
|
||||
'orca orchestration ask --to <handle> --question <text> [--options <csv>] [--timeout-ms <n>] [--from <handle>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'to', 'question', 'options', 'timeout-ms', 'from']
|
||||
},
|
||||
{
|
||||
path: ['orchestration', 'run'],
|
||||
|
||||
@@ -223,6 +223,63 @@ export async function getBaseRefDefault(path: string): Promise<string | null> {
|
||||
return getDefaultBaseRefAsync(path)
|
||||
}
|
||||
|
||||
/**
|
||||
* Return { ahead, behind } for localRef vs remoteRef, or null on git failure.
|
||||
*
|
||||
* Why: `rev-list --left-right --count A...B` emits `<ahead>\t<behind>` —
|
||||
* ahead = commits on A not reachable from B; behind = commits on B not
|
||||
* reachable from A. This is the merge-base-symmetric delta used by the
|
||||
* stale-base dispatch guard (§3.1). Returning null on any failure (bad
|
||||
* ref, corrupt repo, non-numeric output) lets callers degrade gracefully
|
||||
* instead of failing dispatch on a probe error.
|
||||
*/
|
||||
export function getRemoteDrift(
|
||||
repoPath: string,
|
||||
localRef: string,
|
||||
remoteRef: string
|
||||
): { ahead: number; behind: number } | null {
|
||||
try {
|
||||
const stdout = gitExecFileSync(
|
||||
['rev-list', '--left-right', '--count', `${localRef}...${remoteRef}`],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
const [aheadStr, behindStr] = stdout.trim().split(/\s+/)
|
||||
const ahead = Number(aheadStr)
|
||||
const behind = Number(behindStr)
|
||||
if (!Number.isFinite(ahead) || !Number.isFinite(behind)) {
|
||||
return null
|
||||
}
|
||||
return { ahead, behind }
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Up to `limit` commit subjects present on remoteRef but not localRef, in
|
||||
* recency order. Returns [] on git failure.
|
||||
*
|
||||
* Why: powers the preamble drift section (§3.2) so a worker dispatched
|
||||
* against an acknowledged-stale base can see at a glance whether the
|
||||
* drift touches their task area.
|
||||
*/
|
||||
export function getRecentDriftSubjects(
|
||||
repoPath: string,
|
||||
localRef: string,
|
||||
remoteRef: string,
|
||||
limit: number
|
||||
): string[] {
|
||||
try {
|
||||
const stdout = gitExecFileSync(
|
||||
['log', '--format=%s', '-n', String(limit), `${localRef}..${remoteRef}`],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
return stdout.split('\n').filter((s) => s.trim().length > 0)
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `git remote` stdout into a count of configured remotes.
|
||||
*
|
||||
|
||||
@@ -21,6 +21,7 @@ import { getPRForBranch } from '../github/client'
|
||||
import { listWorktrees, addWorktree, addSparseWorktree } from '../git/worktree'
|
||||
import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo'
|
||||
import { gitExecFileAsync } from '../git/runner'
|
||||
import type { OrcaRuntimeService } from '../runtime/orca-runtime'
|
||||
import { isWslPath, parseWslPath, getWslHome } from '../wsl'
|
||||
import { createSetupRunnerScript, getEffectiveHooks, shouldRunSetupForCreate } from '../hooks'
|
||||
import { getSshGitProvider } from '../providers/ssh-git-dispatch'
|
||||
@@ -44,6 +45,20 @@ export function notifyWorktreesChanged(mainWindow: BrowserWindow, repoId: string
|
||||
}
|
||||
}
|
||||
|
||||
// Why (§3.3): two-phase spinner. Main process fires `'fetching'` immediately
|
||||
// after kicking off `git fetch` and `'creating'` after that fetch resolves
|
||||
// (or is determined to be cache-fresh). Renderer swaps its spinner label in
|
||||
// response; fallback is the static "Creating worktree..." label if no event
|
||||
// arrives (e.g. renderer races destruction of the window).
|
||||
export function emitCreateWorktreeProgress(
|
||||
mainWindow: BrowserWindow,
|
||||
phase: 'fetching' | 'creating'
|
||||
): void {
|
||||
if (!mainWindow.isDestroyed()) {
|
||||
mainWindow.webContents.send('createWorktree:progress', { phase })
|
||||
}
|
||||
}
|
||||
|
||||
export async function createRemoteWorktree(
|
||||
args: CreateWorktreeArgs,
|
||||
repo: Repo,
|
||||
@@ -205,13 +220,55 @@ export async function createLocalWorktree(
|
||||
args: CreateWorktreeArgs,
|
||||
repo: Repo,
|
||||
store: Store,
|
||||
mainWindow: BrowserWindow
|
||||
mainWindow: BrowserWindow,
|
||||
runtime?: OrcaRuntimeService
|
||||
): Promise<CreateWorktreeResult> {
|
||||
const settings = store.getSettings()
|
||||
|
||||
const username = getGitUsername(repo.path)
|
||||
const requestedName = args.name
|
||||
const sanitizedName = sanitizeWorktreeName(args.name)
|
||||
|
||||
// Why (§3.3): determine the base branch (and therefore the remote we need to
|
||||
// fetch) FIRST, so the fetch can overlap all pre-create work below. Neither
|
||||
// of these calls depends on the suffix loop / PR probe / branch-conflict
|
||||
// resolution, so they are safe to hoist.
|
||||
const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path)
|
||||
if (!baseBranch) {
|
||||
// Why: getDefaultBaseRef may return null when none of origin/HEAD,
|
||||
// origin/main, origin/master, local main, or local master exist. Don't
|
||||
// fall back to a hardcoded 'origin/main' — passing a non-existent ref to
|
||||
// `git worktree add` produces an opaque error. Fail here with a clear
|
||||
// message so the UI can prompt the user to pick a base branch explicitly.
|
||||
throw new Error(
|
||||
'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.'
|
||||
)
|
||||
}
|
||||
|
||||
// Why (§3.3 Lifecycle): fire fetch via the shared 30s-window cache on the
|
||||
// runtime so repeat creates on the same repo reuse the in-flight promise
|
||||
// and dispatch probes benefit from the freshness window. Kicked off BEFORE
|
||||
// the suffix loop / PR probe / path resolution so those operations overlap
|
||||
// the network round-trip — the `await` right before `addWorktree` is the
|
||||
// only point that actually requires fetch completion.
|
||||
//
|
||||
// Why `runtime` is optional: a handful of legacy IPC test harnesses still
|
||||
// call createLocalWorktree without the runtime. In that case we fall back
|
||||
// to the old fire-and-forget behavior (which those tests already expect).
|
||||
// Production `worktrees.ts` always passes runtime, so the happy path
|
||||
// always gets the cache.
|
||||
const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin'
|
||||
const fetchPromise: Promise<void> = runtime
|
||||
? runtime.fetchRemoteWithCache(repo.path, remote)
|
||||
: gitExecFileAsync(['fetch', remote], { cwd: repo.path })
|
||||
.then(() => undefined)
|
||||
.catch(() => undefined)
|
||||
|
||||
// Why: emit a progress event so the renderer dialog can switch its spinner
|
||||
// label to "Checking for updates..." while the fetch is in flight, then
|
||||
// "Creating worktree..." after we await it. Renderer falls back to the
|
||||
// static "Creating worktree..." label if no event arrives.
|
||||
emitCreateWorktreeProgress(mainWindow, 'fetching')
|
||||
// Why: WSL worktrees live under ~/orca/workspaces inside the WSL
|
||||
// filesystem. Validate against that root, not the Windows workspace dir.
|
||||
// If WSL home lookup fails, keep using the configured workspace root so
|
||||
@@ -298,20 +355,6 @@ export async function createLocalWorktree(
|
||||
)
|
||||
}
|
||||
|
||||
// Determine base branch.
|
||||
//
|
||||
// Why: getDefaultBaseRef may return null when none of origin/HEAD,
|
||||
// origin/main, origin/master, local main, or local master exist. In that
|
||||
// case we must not fall back to a hardcoded 'origin/main' — passing a
|
||||
// non-existent ref to `git worktree add` produces an opaque error. Fail
|
||||
// here with a clear message so the UI can prompt the user to pick a base
|
||||
// branch explicitly.
|
||||
const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path)
|
||||
if (!baseBranch) {
|
||||
throw new Error(
|
||||
'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.'
|
||||
)
|
||||
}
|
||||
// Why: `ask` is a pre-create choice gate, not a post-create side effect.
|
||||
// Resolve it before mutating git state so missing UI input cannot strand
|
||||
// a real worktree on disk while the renderer reports "create failed". The
|
||||
@@ -348,16 +391,16 @@ export async function createLocalWorktree(
|
||||
}
|
||||
}
|
||||
|
||||
// Why: `git fetch` previously blocked worktree creation for 1–5s on every
|
||||
// click, even though the fetch result isn't actually required — the
|
||||
// subsequent `git worktree add` uses whatever local ref `baseBranch` points
|
||||
// at. Kicking fetch off in parallel lets the worktree be created off the
|
||||
// last-known tip while the fetch completes in the background; the next
|
||||
// user action (pull, diff, PR create) will see the refreshed remote state.
|
||||
const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin'
|
||||
void gitExecFileAsync(['fetch', remote], { cwd: repo.path }).catch(() => {
|
||||
// Fetch is best-effort — don't block worktree creation if offline
|
||||
})
|
||||
// Why (§3.3): gate on the fetch we fired at the top of this function.
|
||||
// Pre-create probes (branch-conflict, PR probe, path resolution, sparse
|
||||
// prep) already ran concurrently with the fetch; in the warm case this
|
||||
// await is a no-op. In the cold case the spinner has already shown
|
||||
// "Checking for updates..." so the user sees the wait is legible.
|
||||
//
|
||||
// `fetchRemoteWithCache` never rejects (log-and-proceed on offline
|
||||
// failure), so the bare `await` does not need a try/catch here.
|
||||
await fetchPromise
|
||||
emitCreateWorktreeProgress(mainWindow, 'creating')
|
||||
|
||||
await (sparseDirectories.length > 0
|
||||
? addSparseWorktree(
|
||||
|
||||
@@ -181,7 +181,14 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
|
||||
ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard')
|
||||
listWorktreesMock.mockResolvedValue([])
|
||||
|
||||
registerWorktreeHandlers(mainWindow as never, store as never, {} as never)
|
||||
// Why: createLocalWorktree routes `git fetch` through
|
||||
// `runtime.fetchRemoteWithCache` (§3.3 Lifecycle). Stub it for path tests.
|
||||
const runtimeStub = {
|
||||
fetchRemoteWithCache: async () => {
|
||||
/* noop */
|
||||
}
|
||||
}
|
||||
registerWorktreeHandlers(mainWindow as never, store as never, runtimeStub as never)
|
||||
})
|
||||
|
||||
it('accepts a newly created Windows worktree when git lists the same path with different separators', async () => {
|
||||
|
||||
@@ -248,7 +248,16 @@ describe('registerWorktreeHandlers', () => {
|
||||
ensurePathWithinWorkspaceMock.mockImplementation((targetPath: string) => targetPath)
|
||||
listWorktreesMock.mockResolvedValue([])
|
||||
|
||||
registerWorktreeHandlers(mainWindow as never, store as never, {} as never)
|
||||
// Why: createLocalWorktree routes `git fetch` through
|
||||
// `runtime.fetchRemoteWithCache` (§3.3 Lifecycle). A minimal stub
|
||||
// keeps these tests focused on create-flow semantics; the full
|
||||
// cache behavior is covered by fetch-remote-cache.test.ts.
|
||||
const runtimeStub = {
|
||||
fetchRemoteWithCache: async () => {
|
||||
/* noop — fetch mocked at gitExecFileAsync level via gitExecFileAsyncMock */
|
||||
}
|
||||
}
|
||||
registerWorktreeHandlers(mainWindow as never, store as never, runtimeStub as never)
|
||||
})
|
||||
|
||||
it('auto-suffixes the branch name when the first choice collides with a remote branch', async () => {
|
||||
|
||||
@@ -146,7 +146,7 @@ export function registerWorktreeHandlers(
|
||||
return createRemoteWorktree(args, repo, store, mainWindow)
|
||||
}
|
||||
|
||||
return createLocalWorktree(args, repo, store, mainWindow)
|
||||
return createLocalWorktree(args, repo, store, mainWindow, runtime)
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Why: these tests cover the §3.3 Lifecycle rules on
|
||||
// `OrcaRuntimeService.fetchRemoteWithCache` — in particular that a rejected
|
||||
// fetch evicts its Map entry AND does not advance the freshness timestamp,
|
||||
// and that two concurrent callers serialize on a single underlying fetch.
|
||||
// They live in a dedicated file so we can mock `gitExecFileAsync` cleanly
|
||||
// without disturbing the large orca-runtime.test.ts mock surface.
|
||||
|
||||
const gitExecFileAsyncMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('../git/runner', async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>
|
||||
return {
|
||||
...actual,
|
||||
gitExecFileAsync: gitExecFileAsyncMock
|
||||
}
|
||||
})
|
||||
|
||||
// Why: orca-runtime.ts imports heavy modules (hooks, ipc/*, etc.) at top
|
||||
// level. We only exercise the fetch cache, so we let those imports load
|
||||
// normally — none of them trigger IO until a runtime method is called.
|
||||
import { OrcaRuntimeService } from './orca-runtime'
|
||||
|
||||
describe('OrcaRuntimeService.fetchRemoteWithCache', () => {
|
||||
beforeEach(() => {
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('evicts the in-flight Map entry on rejection so the next caller re-fetches', async () => {
|
||||
// First call rejects, second call resolves. Without §3.3 Lifecycle
|
||||
// `.finally()` eviction, the second caller would await the rejected
|
||||
// promise forever (or throw the same error) — the regression pattern
|
||||
// described in §3.3.
|
||||
gitExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('network down'))
|
||||
.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
||||
|
||||
const runtime = new OrcaRuntimeService(null)
|
||||
|
||||
await runtime.fetchRemoteWithCache('/repo/a', 'origin')
|
||||
await runtime.fetchRemoteWithCache('/repo/a', 'origin')
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not advance the freshness timestamp when the fetch rejects', async () => {
|
||||
// A rejected fetch that wrote the timestamp would make the 30s freshness
|
||||
// cache "lie" — the next caller would skip the fetch on a repo whose
|
||||
// last real sync is unknown. §3.3 mandates success-only writes.
|
||||
gitExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('boom'))
|
||||
.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
||||
|
||||
const runtime = new OrcaRuntimeService(null)
|
||||
|
||||
await runtime.fetchRemoteWithCache('/repo/b', 'origin')
|
||||
// Immediately call again — if the freshness window were armed we would
|
||||
// short-circuit and skip the fetch. It must still dispatch a real fetch.
|
||||
await runtime.fetchRemoteWithCache('/repo/b', 'origin')
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('serializes two concurrent callers onto a single git fetch', async () => {
|
||||
// Two callers hitting the same repo+remote at the same time must share
|
||||
// one underlying fetch. Without the in-flight Map they would each
|
||||
// dispatch an independent `git fetch`, tripling the network load in the
|
||||
// worst case (renderer create + dispatch probe + CLI create).
|
||||
let resolveFetch!: () => void
|
||||
const pending = new Promise<{ stdout: string; stderr: string }>((resolve) => {
|
||||
resolveFetch = () => resolve({ stdout: '', stderr: '' })
|
||||
})
|
||||
gitExecFileAsyncMock.mockReturnValueOnce(pending)
|
||||
|
||||
const runtime = new OrcaRuntimeService(null)
|
||||
|
||||
const first = runtime.fetchRemoteWithCache('/repo/c', 'origin')
|
||||
const second = runtime.fetchRemoteWithCache('/repo/c', 'origin')
|
||||
|
||||
// Allow both callers to register before we resolve.
|
||||
await Promise.resolve()
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
resolveFetch()
|
||||
await Promise.all([first, second])
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('skips the fetch inside the 30s freshness window after a successful fetch', async () => {
|
||||
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
|
||||
|
||||
const runtime = new OrcaRuntimeService(null)
|
||||
|
||||
await runtime.fetchRemoteWithCache('/repo/d', 'origin')
|
||||
await runtime.fetchRemoteWithCache('/repo/d', 'origin')
|
||||
|
||||
// Second call must short-circuit on the freshness window (no new exec).
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -471,32 +471,44 @@ describe('OrcaRuntimeService', () => {
|
||||
|
||||
it('delivers pending orchestration messages to an already-idle agent', async () => {
|
||||
vi.useFakeTimers()
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
const write = vi.fn().mockReturnValue(true)
|
||||
runtime.setOrchestrationDb(db)
|
||||
runtime.setPtyController({
|
||||
write,
|
||||
kill: vi.fn(),
|
||||
getForegroundProcess: async () => null
|
||||
})
|
||||
syncSinglePty(runtime)
|
||||
try {
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
const write = vi.fn().mockReturnValue(true)
|
||||
runtime.setOrchestrationDb(db)
|
||||
runtime.setPtyController({
|
||||
write,
|
||||
kill: vi.fn(),
|
||||
getForegroundProcess: async () => null
|
||||
})
|
||||
syncSinglePty(runtime)
|
||||
|
||||
const [terminal] = (await runtime.listTerminals()).terminals
|
||||
runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07', 100)
|
||||
runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101)
|
||||
db.insertMessage({ from: 'term_sender', to: terminal.handle, subject: 'hello' })
|
||||
const [terminal] = (await runtime.listTerminals()).terminals
|
||||
runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07', 100)
|
||||
runtime.onPtyData('pty-1', '\x1b]0;Codex done\x07', 101)
|
||||
db.insertMessage({ from: 'term_sender', to: terminal.handle, subject: 'hello' })
|
||||
|
||||
runtime.deliverPendingMessagesForHandle(terminal.handle)
|
||||
runtime.deliverPendingMessagesForHandle(terminal.handle)
|
||||
|
||||
expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: hello'))
|
||||
// Why: markAsRead is deferred until the 500ms delayed Enter is confirmed,
|
||||
// so we must advance timers past the split-write delay.
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(write).toHaveBeenCalledWith('pty-1', '\r')
|
||||
expect(db.getUnreadMessages(terminal.handle)).toHaveLength(0)
|
||||
db.close()
|
||||
vi.useRealTimers()
|
||||
expect(write).toHaveBeenCalledWith('pty-1', expect.stringContaining('Subject: hello'))
|
||||
// Why: the split Enter write lands after the 500ms delay, so we advance
|
||||
// past it before asserting on delivered_at.
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(write).toHaveBeenCalledWith('pty-1', '\r')
|
||||
|
||||
// Why: design doc §3.2 splits delivered vs. read — push-on-idle stamps
|
||||
// `delivered_at` but must *not* flip `read`, since only the check caller
|
||||
// (the agent) is authorized to consume messages from its queue. The
|
||||
// injected banner is a courtesy; the rows stay unread so the agent can
|
||||
// still observe them via `check` and resolve the consumption race.
|
||||
const unread = db.getUnreadMessages(terminal.handle)
|
||||
expect(unread).toHaveLength(1)
|
||||
expect(unread[0].read).toBe(0)
|
||||
expect(unread[0].delivered_at).not.toBeNull()
|
||||
db.close()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('adopts preallocated ORCA_TERMINAL_HANDLE as a valid runtime handle', async () => {
|
||||
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
isShellProcess
|
||||
} from '../../shared/agent-detection'
|
||||
import type { AgentStatus } from '../../shared/agent-detection'
|
||||
import { gitExecFileAsync, gitExecFileSync } from '../git/runner'
|
||||
import { gitExecFileAsync } from '../git/runner'
|
||||
import { isWslPath, parseWslPath, getWslHome } from '../wsl'
|
||||
import { randomUUID } from 'crypto'
|
||||
import { join } from 'path'
|
||||
@@ -84,7 +84,9 @@ import {
|
||||
getBranchConflictKind,
|
||||
isGitRepo,
|
||||
getRepoName,
|
||||
searchBaseRefs
|
||||
searchBaseRefs,
|
||||
getRemoteDrift,
|
||||
getRecentDriftSubjects
|
||||
} from '../git/repo'
|
||||
import { listWorktrees, addWorktree, removeWorktree } from '../git/worktree'
|
||||
import { createSetupRunnerScript, getEffectiveHooks, runHook } from '../hooks'
|
||||
@@ -242,6 +244,23 @@ export class OrcaRuntimeService {
|
||||
private agentDetector: AgentDetector | null = null
|
||||
private _orchestrationDb: OrchestrationDb | null = null
|
||||
private messageWaitersByHandle = new Map<string, Set<MessageWaiter>>()
|
||||
// Why (§3.3 + §7.1): the renderer-create path and coordinator
|
||||
// `probeWorktreeDrift` share this cache so a create that already fetched
|
||||
// `origin` within the last 30s does not re-fetch during dispatch, and
|
||||
// vice-versa. Keyed by `<repoPath>::<remote>` so multi-remote repos (even
|
||||
// though v1 only uses `origin`) don't cross-contaminate. The in-flight Map
|
||||
// also provides serialization — two concurrent callers share a single
|
||||
// underlying `git fetch`. Lifecycle rules are enforced in
|
||||
// `fetchRemoteWithCache` and MUST NOT be duplicated elsewhere:
|
||||
// - entry inserted BEFORE await,
|
||||
// - `.finally()` removes the entry on BOTH success and rejection,
|
||||
// - timestamp written ONLY on success (rejection must not make the
|
||||
// 30s freshness cache lie).
|
||||
// A literal "insert before await / read-back after await" without these
|
||||
// three rules wedges all future creates on the same repo after a single
|
||||
// DNS hiccup until process restart (see §3.3 Lifecycle).
|
||||
private fetchInflight = new Map<string, Promise<void>>()
|
||||
private fetchLastCompletedAt = new Map<string, number>()
|
||||
private readonly getLocalProviderFn: (() => IPtyProvider) | null
|
||||
|
||||
constructor(
|
||||
@@ -1022,10 +1041,18 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
|
||||
const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin'
|
||||
// Why (§3.3 Lifecycle): route through the shared fetch cache so back-to-back
|
||||
// CLI creates on the same repo don't each pay the round-trip, and so a
|
||||
// subsequent dispatch probe within the 30s window reuses this result. The
|
||||
// helper swallows rejection (log-and-proceed) so a DNS hiccup never wedges
|
||||
// future creates and CLI creation stays usable offline — same intent as
|
||||
// the previous try/catch around gitExecFileSync.
|
||||
try {
|
||||
gitExecFileSync(['fetch', remote], { cwd: repo.path })
|
||||
await this.fetchRemoteWithCache(repo.path, remote)
|
||||
} catch {
|
||||
// Why: matching the editor behavior keeps CLI creation usable offline.
|
||||
// Why: belt-and-suspenders. fetchRemoteWithCache already logs and does
|
||||
// not throw; the outer try/catch guarantees create-path tolerance even
|
||||
// if future refactors change that contract.
|
||||
}
|
||||
|
||||
await addWorktree(
|
||||
@@ -1105,6 +1132,96 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch `remote` in `repoPath`, sharing the 30s freshness window + in-flight
|
||||
* serialization with all other callers (renderer-create path, CLI create,
|
||||
* dispatch drift probe). Never rejects — callers log-and-proceed on offline
|
||||
* failures (§3.3 Lifecycle).
|
||||
*
|
||||
* Why a shared cache on the runtime instead of module-scoped: §7.1 relies on
|
||||
* one cache for BOTH the renderer create path and `probeWorktreeDrift`. A
|
||||
* dispatch tick that reuses a just-completed create-path fetch is the
|
||||
* primary telemetry target; splitting the cache by call-site would double
|
||||
* the fetch load on warm repos.
|
||||
*/
|
||||
async fetchRemoteWithCache(repoPath: string, remote: string): Promise<void> {
|
||||
const key = `${repoPath}::${remote}`
|
||||
const lastAt = this.fetchLastCompletedAt.get(key)
|
||||
if (lastAt !== undefined && Date.now() - lastAt < FETCH_FRESHNESS_MS) {
|
||||
// Why: freshness window hit — skip the fetch entirely. Do NOT reuse any
|
||||
// in-flight promise here; the timestamp is only written on success, so
|
||||
// hitting this branch means a previous fetch did succeed recently.
|
||||
return
|
||||
}
|
||||
|
||||
const existing = this.fetchInflight.get(key)
|
||||
if (existing) {
|
||||
// Why: genuine serialization (not check-then-set). Two callers racing
|
||||
// on the same repo+remote share the single underlying `git fetch`.
|
||||
return existing
|
||||
}
|
||||
|
||||
const promise = gitExecFileAsync(['fetch', remote], { cwd: repoPath })
|
||||
.then(() => {
|
||||
// Why (§3.3 Lifecycle): timestamp on success ONLY. Writing on rejection
|
||||
// would make the freshness cache lie about the last known remote state.
|
||||
this.fetchLastCompletedAt.set(key, Date.now())
|
||||
})
|
||||
.catch((err) => {
|
||||
// Why: swallow here so awaiters don't throw at the await site. Outer
|
||||
// create/dispatch paths are already tolerant of offline fetch failure;
|
||||
// this is the behavioral contract of this helper.
|
||||
console.warn(`[fetchRemoteWithCache] ${remote} fetch failed for ${repoPath}:`, err)
|
||||
})
|
||||
.finally(() => {
|
||||
// Why (§3.3 Lifecycle): evict on BOTH success and rejection. A
|
||||
// rejected entry that survived in the Map would wedge every future
|
||||
// create on this repo until Orca restarted (the F2 bug §3.3 pins).
|
||||
this.fetchInflight.delete(key)
|
||||
})
|
||||
|
||||
this.fetchInflight.set(key, promise)
|
||||
return promise
|
||||
}
|
||||
|
||||
/**
|
||||
* Probe how far the worktree's HEAD is behind its tracking remote. Returns
|
||||
* null when the probe cannot establish a signal (no default base ref, or
|
||||
* git failure). Dispatch treats null as "unknown — proceed" (§3.1); only
|
||||
* knowing-and-stale refuses.
|
||||
*/
|
||||
async probeWorktreeDrift(worktreeSelector: string): Promise<{
|
||||
base: string
|
||||
behind: number
|
||||
recentSubjects: string[]
|
||||
} | null> {
|
||||
const wt = await this.resolveWorktreeSelector(worktreeSelector)
|
||||
if (!this.store) {
|
||||
return null
|
||||
}
|
||||
const repo = this.store.getRepos().find((r) => r.id === wt.repoId)
|
||||
if (!repo) {
|
||||
return null
|
||||
}
|
||||
const base = getDefaultBaseRef(repo.path)
|
||||
if (!base) {
|
||||
// Why: brand-new repo with no remote primary — nothing to compare
|
||||
// against, so there's no meaningful drift to report. Dispatch should
|
||||
// not block on a probe that cannot form an opinion.
|
||||
return null
|
||||
}
|
||||
const remote = base.includes('/') ? base.split('/')[0] : 'origin'
|
||||
// Why: fetch failures are non-fatal; we proceed with whatever the
|
||||
// last-known remote ref points at. `fetchRemoteWithCache` never throws.
|
||||
await this.fetchRemoteWithCache(wt.path, remote)
|
||||
const drift = getRemoteDrift(wt.path, 'HEAD', base)
|
||||
if (!drift) {
|
||||
return null
|
||||
}
|
||||
const recentSubjects = getRecentDriftSubjects(wt.path, 'HEAD', base, DRIFT_PROBE_SUBJECT_LIMIT)
|
||||
return { base, behind: drift.behind, recentSubjects }
|
||||
}
|
||||
|
||||
async updateManagedWorktreeMeta(
|
||||
worktreeSelector: string,
|
||||
updates: {
|
||||
@@ -1733,7 +1850,7 @@ export class OrcaRuntimeService {
|
||||
|
||||
waitForMessage(
|
||||
handle: string,
|
||||
options?: { typeFilter?: string[]; timeoutMs?: number }
|
||||
options?: { typeFilter?: string[]; timeoutMs?: number; signal?: AbortSignal }
|
||||
): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
const timeoutMs = options?.timeoutMs ?? MESSAGE_WAIT_DEFAULT_TIMEOUT_MS
|
||||
@@ -1745,7 +1862,27 @@ export class OrcaRuntimeService {
|
||||
timeout: null
|
||||
}
|
||||
|
||||
// Why: if the caller aborts (socket closed on the RPC side — see design
|
||||
// doc §3.1 counter-lifecycle), resolve immediately so the long-poll slot
|
||||
// is released instead of counting down the full timeoutMs with a dead
|
||||
// client on the other end.
|
||||
const signal = options?.signal
|
||||
const onAbort = (): void => {
|
||||
this.removeMessageWaiter(waiter)
|
||||
resolve()
|
||||
}
|
||||
if (signal) {
|
||||
if (signal.aborted) {
|
||||
resolve()
|
||||
return
|
||||
}
|
||||
signal.addEventListener('abort', onAbort, { once: true })
|
||||
}
|
||||
|
||||
waiter.timeout = setTimeout(() => {
|
||||
if (signal) {
|
||||
signal.removeEventListener('abort', onAbort)
|
||||
}
|
||||
this.removeMessageWaiter(waiter)
|
||||
resolve()
|
||||
}, timeoutMs)
|
||||
@@ -2002,8 +2139,15 @@ export class OrcaRuntimeService {
|
||||
|
||||
// Why: Claude Code treats large single PTY writes as paste events and
|
||||
// swallows a \r included in the same write. Send Enter separately after
|
||||
// a delay so the agent processes the pasted message first. Mark messages
|
||||
// as read only after \r is confirmed, so failed deliveries stay queued.
|
||||
// a delay so the agent processes the pasted message first. Stamp
|
||||
// `delivered_at` only after \r is confirmed, so failed deliveries stay
|
||||
// queued.
|
||||
//
|
||||
// Important (design doc §3.2, feedback #2): we stamp `delivered_at` here
|
||||
// instead of flipping `read`. `read` is reserved for "a check-caller
|
||||
// consumed this message." Flipping `read` on push-on-idle would hide the
|
||||
// message from the coordinator's next `check --unread`, which is the
|
||||
// exact bug feedback #2 reported. The two bits must stay independent.
|
||||
const ptyId = leaf.ptyId
|
||||
setTimeout(() => {
|
||||
try {
|
||||
@@ -2012,11 +2156,12 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
const submitted = this.ptyController?.write(ptyId, '\r') ?? false
|
||||
if (submitted) {
|
||||
this._orchestrationDb?.markAsRead(unread.map((m) => m.id))
|
||||
this._orchestrationDb?.markAsDelivered(unread.map((m) => m.id))
|
||||
}
|
||||
} catch {
|
||||
// Terminal may have closed during the delay — messages stay unread
|
||||
// and will be re-delivered on the next idle transition.
|
||||
// Terminal may have closed during the delay — messages stay queued
|
||||
// (delivered_at still NULL) and will be re-delivered on the next
|
||||
// idle transition.
|
||||
}
|
||||
}, 500)
|
||||
}
|
||||
@@ -3133,6 +3278,13 @@ const DEFAULT_TERMINAL_LIST_LIMIT = 200
|
||||
const DEFAULT_WORKTREE_LIST_LIMIT = 200
|
||||
const DEFAULT_WORKTREE_PS_LIMIT = 200
|
||||
const RESOLVED_WORKTREE_CACHE_TTL_MS = 1000
|
||||
// Why (§3.3): 30s freshness window. A second worktree-create or dispatch-probe
|
||||
// against the same repo+remote within this window reuses the previous successful
|
||||
// fetch instead of repeating the round-trip. Chosen so rapid "new worktree"
|
||||
// clicks and successive coordinator dispatches feel snappy, while still being
|
||||
// short enough that a genuinely-changed remote is observed on the next action.
|
||||
const FETCH_FRESHNESS_MS = 30_000
|
||||
const DRIFT_PROBE_SUBJECT_LIMIT = 5
|
||||
function buildPreview(lines: string[], partialLine: string): string {
|
||||
const previewLines = buildTailLines(lines, partialLine)
|
||||
.map((line) => line.trim())
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
|
||||
|
||||
exports[`buildDispatchPreamble > renders a stable snapshot of the full preamble 1`] = `
|
||||
"You are working inside Orca, a multi-agent IDE. You are a dispatched worker.
|
||||
Your coordinator's terminal handle is: term_COORD
|
||||
Your task ID is: task_SNAP
|
||||
|
||||
You talk to the coordinator only through the CLI commands below. Do not use
|
||||
Slack, GitHub comments, or any other channel to reach a human during the run.
|
||||
|
||||
=== CLI COMMANDS ===
|
||||
|
||||
# Report task completion (REQUIRED when done — even on failure).
|
||||
#
|
||||
# RULE: --body must be a 3-sentence executive summary (what you did,
|
||||
# what you found, what's left). Never send an empty body; the coordinator
|
||||
# reads the body first and only opens artifacts if it needs more detail.
|
||||
# If you produced a long-form artifact, include its path as
|
||||
# payload.reportPath so the coordinator can find it without a file search.
|
||||
#
|
||||
# RULE: send worker_done exactly once. Failure is still a worker_done
|
||||
# with subject like "Failed: <reason>" — never silently exit.
|
||||
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","filesModified":["path/a","path/b"],"reportPath":"<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
|
||||
# distinguish "still thinking" from "hung / crashed." Skip heartbeats only
|
||||
# while blocked inside \`check --wait\` or \`ask\` — those calls are
|
||||
# themselves liveness signals.
|
||||
#
|
||||
# Include BOTH taskId and dispatchId in the payload: the coordinator
|
||||
# attributes the heartbeat to the specific dispatch context, not just
|
||||
# the task, so a straggler heartbeat from a previously-failed dispatch
|
||||
# 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>"}'
|
||||
|
||||
# Ask the coordinator a question and block until it answers.
|
||||
#
|
||||
# BEHAVIOR RULE #1 (MUST NOT VIOLATE):
|
||||
# NEVER use AskUserQuestion; use \`orca orchestration ask\` or send
|
||||
# --type decision_gate. AskUserQuestion opens a local TUI prompt that the
|
||||
# coordinator cannot see and cannot answer — your session will hang forever
|
||||
# waiting on a human. Every interactive question goes through \`ask\` below.
|
||||
#
|
||||
# The \`ask\` verb is a thin wrapper: it sends a decision_gate message and
|
||||
# blocks on \`check --wait\` until the coordinator replies, then prints the
|
||||
# reply body. Use it anywhere you would otherwise have reached for
|
||||
# AskUserQuestion.
|
||||
orca orchestration ask --to term_COORD \\
|
||||
--question "<your question>" \\
|
||||
--options "<optional,comma,separated>" \\
|
||||
--timeout-ms 600000
|
||||
|
||||
# Escalate a blocker or failure (pre-completion, when you need the
|
||||
# coordinator to do something before you can continue):
|
||||
orca orchestration send --to term_COORD \\
|
||||
--type escalation --subject "Blocked: <reason>" \\
|
||||
--body "<details>" \\
|
||||
--payload '{"taskId":"task_SNAP"}'
|
||||
|
||||
# Check for messages from the coordinator:
|
||||
orca orchestration check
|
||||
|
||||
=== AFTER YOU SEND worker_done ===
|
||||
|
||||
Keep the shell session open for a grace period (10 minutes) in case the
|
||||
coordinator sends a follow-up or re-dispatches you. Poll with
|
||||
\`orca orchestration check\` every 2 minutes during that window. If no
|
||||
follow-up arrives, you may exit after the grace period — the coordinator
|
||||
will not expect further output from you.
|
||||
|
||||
If the coordinator re-dispatches you (you will receive a fresh preamble +
|
||||
TASK block), reset your polling and start the new task. Do not respond
|
||||
to the previous task's follow-ups after a re-dispatch.
|
||||
|
||||
=== TASK ===
|
||||
TASK_BODY"
|
||||
`;
|
||||
@@ -1,12 +1,27 @@
|
||||
/* eslint-disable max-lines -- Why: coordinator tests cover dispatch, DAG ordering, escalation, decision gates, concurrency, and stop — splitting by category would scatter shared setup without improving clarity. */
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { OrchestrationDb } from './db'
|
||||
import { Coordinator, type CoordinatorRuntime } from './coordinator'
|
||||
import {
|
||||
Coordinator,
|
||||
DISPATCH_STALE_THRESHOLD,
|
||||
parseAllowStaleBaseFromSpec,
|
||||
type CoordinatorRuntime
|
||||
} from './coordinator'
|
||||
|
||||
type DriftResult = {
|
||||
base: string
|
||||
behind: number
|
||||
recentSubjects: string[]
|
||||
} | null
|
||||
|
||||
function createMockRuntime(): CoordinatorRuntime & {
|
||||
sentMessages: { handle: string; text: string }[]
|
||||
terminals: { handle: string; worktreeId: string; connected: boolean; writable: boolean }[]
|
||||
createdTerminals: string[]
|
||||
probeDriftCalls: string[]
|
||||
probeDriftResult: DriftResult
|
||||
setProbeDrift(result: DriftResult): void
|
||||
throwProbeDrift: Error | null
|
||||
} {
|
||||
const mock = {
|
||||
sentMessages: [] as { handle: string; text: string }[],
|
||||
@@ -17,6 +32,12 @@ function createMockRuntime(): CoordinatorRuntime & {
|
||||
writable: boolean
|
||||
}[],
|
||||
createdTerminals: [] as string[],
|
||||
probeDriftCalls: [] as string[],
|
||||
probeDriftResult: null as DriftResult,
|
||||
throwProbeDrift: null as Error | null,
|
||||
setProbeDrift(result: DriftResult): void {
|
||||
mock.probeDriftResult = result
|
||||
},
|
||||
async sendTerminal(handle: string, action: { text?: string }) {
|
||||
mock.sentMessages.push({ handle, text: action.text ?? '' })
|
||||
return { handle, accepted: true, bytesWritten: 0 }
|
||||
@@ -32,6 +53,13 @@ function createMockRuntime(): CoordinatorRuntime & {
|
||||
},
|
||||
async waitForTerminal(handle: string) {
|
||||
return { handle, condition: 'exit' }
|
||||
},
|
||||
async probeWorktreeDrift(worktreeSelector: string): Promise<DriftResult> {
|
||||
mock.probeDriftCalls.push(worktreeSelector)
|
||||
if (mock.throwProbeDrift) {
|
||||
throw mock.throwProbeDrift
|
||||
}
|
||||
return mock.probeDriftResult
|
||||
}
|
||||
}
|
||||
return mock
|
||||
@@ -355,6 +383,88 @@ describe('Coordinator', () => {
|
||||
expect(result.status).toBe('completed')
|
||||
})
|
||||
|
||||
it('logs a stale warning for dispatched rows past the threshold and does not auto-fail', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
// No terminals available so dispatchReadyTasks creates one and we can
|
||||
// drive the stale-scan deterministically via SQL backdating.
|
||||
const task = db.createTask({ spec: 'work' })
|
||||
const ctx = db.createDispatchContext(task.id, 'term_stale')
|
||||
|
||||
// Backdate dispatched_at and last_heartbeat_at beyond the 10-min threshold
|
||||
// so getStaleDispatches returns this row on the first tick.
|
||||
const sqlite = (
|
||||
db as unknown as { db: { prepare: (s: string) => { run: (...a: unknown[]) => void } } }
|
||||
).db
|
||||
const iso = (ms: number) => new Date(Date.now() - ms).toISOString()
|
||||
sqlite
|
||||
.prepare('UPDATE dispatch_contexts SET dispatched_at = ?, last_heartbeat_at = ? WHERE id = ?')
|
||||
.run(iso(60 * 60 * 1000), iso(30 * 60 * 1000), ctx.id)
|
||||
|
||||
const logs: string[] = []
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 20,
|
||||
onLog: (m) => logs.push(m)
|
||||
})
|
||||
|
||||
// Drive one tick then stop — we only need the stale warning to have fired.
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 80)
|
||||
})
|
||||
coordinator.stop()
|
||||
await runPromise
|
||||
|
||||
expect(logs.some((l) => /has not sent a heartbeat/.test(l) && l.includes(task.id))).toBe(true)
|
||||
// Task status must NOT have been auto-failed — logging only.
|
||||
expect(db.getTask(task.id)?.status).toBe('dispatched')
|
||||
})
|
||||
|
||||
it('records heartbeat by dispatchId on worker heartbeat messages', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
|
||||
const task = db.createTask({ spec: 'work' })
|
||||
const ctx = db.createDispatchContext(task.id, 'term_a')
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 20
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'alive',
|
||||
type: 'heartbeat',
|
||||
payload: JSON.stringify({ taskId: task.id, dispatchId: ctx.id, phase: 'implementing' })
|
||||
})
|
||||
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 80)
|
||||
})
|
||||
|
||||
expect(db.getDispatchContext(task.id)?.last_heartbeat_at).toBeTruthy()
|
||||
|
||||
// Complete the task so the coordinator run finishes cleanly.
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
})
|
||||
|
||||
it('can be stopped', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
@@ -376,4 +486,287 @@ describe('Coordinator', () => {
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('failed')
|
||||
})
|
||||
|
||||
describe('stale-base dispatch guard', () => {
|
||||
it('threads drift into the preamble when behind > 0 and under threshold', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
runtime.setProbeDrift({
|
||||
base: 'origin/main',
|
||||
behind: 5,
|
||||
recentSubjects: ['fix A', 'fix B', 'fix C']
|
||||
})
|
||||
|
||||
const task = db.createTask({ spec: 'do the work' })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
worktree: 'wt1'
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 100)
|
||||
})
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
expect(runtime.probeDriftCalls).toContain('wt1')
|
||||
const sent = runtime.sentMessages.find((m) => m.handle === 'term_a')
|
||||
expect(sent).toBeDefined()
|
||||
expect(sent!.text).toContain('--- BASE DRIFT ---')
|
||||
expect(sent!.text).toContain('5 commits behind origin/main')
|
||||
expect(sent!.text).toContain('fix A')
|
||||
})
|
||||
|
||||
it('silently skips dispatch when drift > threshold and allow-stale-base is absent', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
runtime.setProbeDrift({
|
||||
base: 'origin/main',
|
||||
behind: DISPATCH_STALE_THRESHOLD + 10,
|
||||
recentSubjects: ['fix A']
|
||||
})
|
||||
|
||||
const task = db.createTask({ spec: 'do the work' })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
worktree: 'wt1'
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 250)
|
||||
})
|
||||
coordinator.stop()
|
||||
const result = await runPromise
|
||||
|
||||
// Why: silent-skip must NOT burn the circuit-breaker budget. Task must
|
||||
// stay in `ready`; failDispatch must NOT be called; sendTerminal must
|
||||
// NOT be called; no dispatch context should exist.
|
||||
expect(runtime.sentMessages).toHaveLength(0)
|
||||
expect(db.getTask(task.id)?.status).toBe('ready')
|
||||
expect(db.getDispatchContext(task.id)).toBeUndefined()
|
||||
// Coordinator was stopped externally, so overall status is 'failed'
|
||||
// because tasks are not complete — but the task itself never dispatched.
|
||||
expect(result.status).toBe('failed')
|
||||
expect(result.failedTasks).not.toContain(task.id)
|
||||
})
|
||||
|
||||
it('proceeds with stripped spec + drift section when allow-stale-base overrides', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
runtime.setProbeDrift({
|
||||
base: 'origin/main',
|
||||
behind: 200,
|
||||
recentSubjects: ['commit 1', 'commit 2']
|
||||
})
|
||||
|
||||
const spec = `Investigate issue #42
|
||||
allow-stale-base: true`
|
||||
const task = db.createTask({ spec })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
worktree: 'wt1'
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 100)
|
||||
})
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
const sent = runtime.sentMessages.find((m) => m.handle === 'term_a')
|
||||
expect(sent).toBeDefined()
|
||||
expect(sent!.text).toContain('--- BASE DRIFT ---')
|
||||
expect(sent!.text).toContain('200 commits behind origin/main')
|
||||
// Why (§3.4): stripped spec must not contain the infra flag line.
|
||||
expect(sent!.text).toContain('Investigate issue #42')
|
||||
expect(sent!.text).not.toContain('allow-stale-base: true')
|
||||
})
|
||||
|
||||
it('proceeds without drift section when probeWorktreeDrift returns null', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
runtime.setProbeDrift(null)
|
||||
|
||||
const task = db.createTask({ spec: 'do the work' })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
worktree: 'wt1'
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 100)
|
||||
})
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
const sent = runtime.sentMessages.find((m) => m.handle === 'term_a')
|
||||
expect(sent).toBeDefined()
|
||||
expect(sent!.text).not.toContain('--- BASE DRIFT ---')
|
||||
})
|
||||
|
||||
it('does not call probeWorktreeDrift when coordinator has no worktree selector', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
const logs: string[] = []
|
||||
|
||||
const task = db.createTask({ spec: 'do the work' })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
// worktree deliberately omitted
|
||||
onLog: (msg) => logs.push(msg)
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 100)
|
||||
})
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
expect(runtime.probeDriftCalls).toHaveLength(0)
|
||||
expect(logs.some((m) => m.includes('stale-base guard inert'))).toBe(true)
|
||||
// Dispatch still went through normally.
|
||||
expect(runtime.sentMessages.length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('proceeds without drift when probeWorktreeDrift throws', async () => {
|
||||
db = new OrchestrationDb(':memory:')
|
||||
const runtime = createMockRuntime()
|
||||
runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }]
|
||||
runtime.throwProbeDrift = new Error('boom')
|
||||
|
||||
const task = db.createTask({ spec: 'do the work' })
|
||||
|
||||
const coordinator = new Coordinator(db, runtime, {
|
||||
spec: 'go',
|
||||
coordinatorHandle: 'coord',
|
||||
pollIntervalMs: 50,
|
||||
worktree: 'wt1'
|
||||
})
|
||||
|
||||
const runPromise = coordinator.run()
|
||||
await new Promise((r) => {
|
||||
setTimeout(r, 100)
|
||||
})
|
||||
|
||||
db.insertMessage({
|
||||
from: 'term_a',
|
||||
to: 'coord',
|
||||
subject: 'Done',
|
||||
type: 'worker_done',
|
||||
payload: JSON.stringify({ taskId: task.id })
|
||||
})
|
||||
|
||||
const result = await runPromise
|
||||
expect(result.status).toBe('completed')
|
||||
const sent = runtime.sentMessages.find((m) => m.handle === 'term_a')
|
||||
expect(sent!.text).not.toContain('--- BASE DRIFT ---')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseAllowStaleBaseFromSpec', () => {
|
||||
it('matches canonical form on its own line and strips it', () => {
|
||||
const spec = `Do the work
|
||||
allow-stale-base: true`
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(true)
|
||||
expect(strippedSpec).toBe('Do the work\n')
|
||||
expect(strippedSpec).not.toContain('allow-stale-base')
|
||||
})
|
||||
|
||||
it('matches case-insensitively', () => {
|
||||
const spec = `Do the work
|
||||
Allow-Stale-Base: TRUE`
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(true)
|
||||
expect(strippedSpec).not.toMatch(/[Aa]llow-[Ss]tale-[Bb]ase/)
|
||||
})
|
||||
|
||||
it('does not match allow-stale-base: false', () => {
|
||||
const spec = `Do the work
|
||||
allow-stale-base: false`
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(false)
|
||||
expect(strippedSpec).toBe(spec)
|
||||
})
|
||||
|
||||
it('does not match allow-stale-base: truthy', () => {
|
||||
const spec = `Do the work
|
||||
allow-stale-base: truthy`
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(false)
|
||||
expect(strippedSpec).toBe(spec)
|
||||
})
|
||||
|
||||
it('does not match the flag embedded inside a sentence', () => {
|
||||
const spec = 'we allow-stale-base: true sometimes'
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(false)
|
||||
expect(strippedSpec).toBe(spec)
|
||||
})
|
||||
|
||||
it('handles the flag as the last line with no trailing newline', () => {
|
||||
const spec = 'line 1\nallow-stale-base: true'
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(spec)
|
||||
expect(allowStale).toBe(true)
|
||||
expect(strippedSpec).toBe('line 1\n')
|
||||
expect(strippedSpec.endsWith('allow-stale-base: true')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -19,6 +19,45 @@ export type CoordinatorRuntime = {
|
||||
handle: string,
|
||||
options?: { condition?: string; timeoutMs?: number }
|
||||
): Promise<{ handle: string; condition: string }>
|
||||
// Why (§3.1): dispatch pre-flight drift check lives on the runtime because
|
||||
// it needs to resolve a worktree selector, load the repo, and fetch. The
|
||||
// coordinator only knows about handles + specs; resolving a git worktree
|
||||
// from this layer would leak transport details here.
|
||||
probeWorktreeDrift(worktreeSelector: string): Promise<{
|
||||
base: string
|
||||
behind: number
|
||||
recentSubjects: string[]
|
||||
} | null>
|
||||
}
|
||||
|
||||
// Why (§3.1): single threshold, no warn/refuse split. Coordinator picked 20
|
||||
// in msg_eff3a646110d — lets normal day-of-velocity on active monorepos pass
|
||||
// while still tripping on the 168-commit harm observed in ORCHESTRATOR_FEEDBACK.md.
|
||||
export const DISPATCH_STALE_THRESHOLD = 20
|
||||
|
||||
// Why (§3.4): the flag is stashed in the task spec text rather than a DB
|
||||
// column in v1. The regex is intentionally narrow — only the canonical form
|
||||
// matches, so typos fail closed (dispatch refuses). Returning the stripped
|
||||
// spec alongside the boolean keeps this infra line out of the worker's
|
||||
// `--- TASK ---` block (workers would otherwise read it as an instruction).
|
||||
//
|
||||
// Trade-off (§7.9): the regex matches any line of the spec including lines
|
||||
// inside fenced code blocks. Acceptable v1 limitation — the failure mode is
|
||||
// "dispatches through when the author didn't intend to," which the preamble
|
||||
// drift section surfaces to the worker. Skill doc directs authors to place
|
||||
// the flag as the last line and avoid the literal flag in code examples.
|
||||
const ALLOW_STALE_BASE_RE = /^[ \t]*allow-stale-base:[ \t]*true[ \t]*\r?$/im
|
||||
const ALLOW_STALE_BASE_STRIP_RE = /^[ \t]*allow-stale-base:[ \t]*true[ \t]*\r?\n?/im
|
||||
|
||||
export function parseAllowStaleBaseFromSpec(spec: string): {
|
||||
allowStale: boolean
|
||||
strippedSpec: string
|
||||
} {
|
||||
if (!ALLOW_STALE_BASE_RE.test(spec)) {
|
||||
return { allowStale: false, strippedSpec: spec }
|
||||
}
|
||||
const strippedSpec = spec.replace(ALLOW_STALE_BASE_STRIP_RE, '')
|
||||
return { allowStale: true, strippedSpec }
|
||||
}
|
||||
|
||||
export type CoordinatorOptions = {
|
||||
@@ -41,6 +80,13 @@ type CoordinatorState = {
|
||||
const DEFAULT_POLL_MS = 2000
|
||||
const MAX_CONCURRENT_DEFAULT = 4
|
||||
|
||||
// Why: 10 min matches the preamble's documented heartbeat cadence (5 min) ×
|
||||
// 2, so a single missed heartbeat is the earliest a dispatch can look stale.
|
||||
// Keeping this in one place (not a per-call arg) ensures the preamble copy
|
||||
// and the detector logic stay aligned; moving it to a config would multiply
|
||||
// the places this constant must be kept in sync.
|
||||
const HUNG_THRESHOLD_MS = 10 * 60 * 1000
|
||||
|
||||
export class Coordinator {
|
||||
private db: OrchestrationDb
|
||||
private runtime: CoordinatorRuntime
|
||||
@@ -173,10 +219,27 @@ export class Coordinator {
|
||||
this.processMessages()
|
||||
this.processEscalations()
|
||||
this.processDecisionGates()
|
||||
this.warnStaleDispatches()
|
||||
await this.dispatchReadyTasks()
|
||||
return this.checkConvergence()
|
||||
}
|
||||
|
||||
// Why: emit a single warning per stale dispatch per tick. This intentionally
|
||||
// does NOT auto-fail the dispatch — the false-positive cost (a slow worker
|
||||
// producing correct output) is higher than the false-negative cost (a hung
|
||||
// worker keeps its terminal slot until a human notices). Auto-fail policy
|
||||
// is a separate decision documented in R6 of DESIGN_DOC_PREAMBLE_FIX.md.
|
||||
private warnStaleDispatches(): void {
|
||||
const thresholdIso = new Date(Date.now() - HUNG_THRESHOLD_MS).toISOString()
|
||||
const stale = this.db.getStaleDispatches(thresholdIso)
|
||||
for (const ctx of stale) {
|
||||
const minutes = Math.round(HUNG_THRESHOLD_MS / 60000)
|
||||
this.opts.onLog(
|
||||
`Warning: worker ${ctx.assignee_handle ?? '<unknown>'} on task ${ctx.task_id} has not sent a heartbeat in ~${minutes} min (dispatch ${ctx.id})`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private processMessages(): void {
|
||||
const messages = this.db.getUnreadMessages(this.opts.coordinatorHandle)
|
||||
if (messages.length === 0) {
|
||||
@@ -194,6 +257,9 @@ export class Coordinator {
|
||||
case 'decision_gate':
|
||||
this.handleDecisionGateMessage(msg)
|
||||
break
|
||||
case 'heartbeat':
|
||||
this.handleHeartbeat(msg)
|
||||
break
|
||||
case 'status':
|
||||
this.opts.onLog(`Status from ${msg.from_handle}: ${msg.subject}`)
|
||||
break
|
||||
@@ -205,6 +271,35 @@ export class Coordinator {
|
||||
this.db.markAsRead(messages.map((m) => m.id))
|
||||
}
|
||||
|
||||
// Why: attribute heartbeats to the specific dispatchId, not a
|
||||
// (taskId, from_handle) lookup. A task that gets retried after a failed
|
||||
// dispatch has multiple rows in dispatch_contexts — a late heartbeat from
|
||||
// the previous (failed) assignee arriving while the new dispatch is active
|
||||
// would falsely bump the new row's last_heartbeat_at if we resolved by
|
||||
// "latest dispatch for this task" (§5.3.4). If the worker drops dispatchId
|
||||
// from the payload, log-and-skip is the preferred failure mode: the stale
|
||||
// detector will correctly flag the dispatch as hung because nothing
|
||||
// refreshed last_heartbeat_at.
|
||||
private handleHeartbeat(msg: MessageRow): void {
|
||||
if (!msg.payload) {
|
||||
this.opts.onLog(`Heartbeat from ${msg.from_handle} missing payload; ignored`)
|
||||
return
|
||||
}
|
||||
let payload: { dispatchId?: unknown } = {}
|
||||
try {
|
||||
payload = JSON.parse(msg.payload)
|
||||
} catch {
|
||||
this.opts.onLog(`Heartbeat from ${msg.from_handle} has invalid JSON payload; ignored`)
|
||||
return
|
||||
}
|
||||
const dispatchId = payload.dispatchId
|
||||
if (typeof dispatchId !== 'string' || dispatchId.length === 0) {
|
||||
this.opts.onLog(`Heartbeat from ${msg.from_handle} missing dispatchId; ignored`)
|
||||
return
|
||||
}
|
||||
this.db.recordHeartbeat(dispatchId, msg.created_at)
|
||||
}
|
||||
|
||||
private handleWorkerDone(msg: MessageRow): void {
|
||||
this.opts.onLog(`Worker done: ${msg.from_handle} — ${msg.subject}`)
|
||||
|
||||
@@ -383,15 +478,69 @@ export class Coordinator {
|
||||
}
|
||||
|
||||
private async dispatchTask(task: TaskRow, targetHandle: string): Promise<void> {
|
||||
// Why (§3.1): pre-flight drift check BEFORE `createDispatchContext` so a
|
||||
// refusal does NOT increment failure_count. createDispatchContext carries
|
||||
// `MAX(failure_count)` forward across contexts (db.ts:301-306), so burning
|
||||
// the circuit-breaker budget here would convert a recoverable "fetch and
|
||||
// retry" into a hard `failed` task within ~6s of polling. Silent return
|
||||
// leaves the task in `ready`; the next `dispatchReadyTasks` tick retries
|
||||
// naturally, and once the coordinator's worktree has been refreshed
|
||||
// dispatch proceeds cleanly.
|
||||
const { allowStale, strippedSpec } = parseAllowStaleBaseFromSpec(task.spec)
|
||||
let baseDrift: {
|
||||
base: string
|
||||
behind: number
|
||||
recentSubjects: string[]
|
||||
} | null = null
|
||||
|
||||
if (!this.opts.worktree) {
|
||||
// Why (§7.4): CoordinatorOptions.worktree is optional. When undefined,
|
||||
// probeWorktreeDrift cannot resolve a selector; log once so operators
|
||||
// can see the guard did not run for this task and proceed. v2 may
|
||||
// always resolve a worktree via the coordinator-terminal handle.
|
||||
this.opts.onLog(`stale-base guard inert for ${task.id}: coordinator has no worktree selector`)
|
||||
} else {
|
||||
baseDrift = await this.runtime.probeWorktreeDrift(this.opts.worktree).catch((err) => {
|
||||
this.opts.onLog(`probeWorktreeDrift failed for ${this.opts.worktree}: ${err}`)
|
||||
return null
|
||||
})
|
||||
|
||||
if (baseDrift && baseDrift.behind > DISPATCH_STALE_THRESHOLD && !allowStale) {
|
||||
// Why (§3.1): silent-return, NOT failDispatch (which would burn the
|
||||
// circuit-breaker budget). The message lists three remediations so
|
||||
// the operator can recover via any of them.
|
||||
this.opts.onLog(
|
||||
`Skipping dispatch of ${task.id}: worktree is ${baseDrift.behind} commits ` +
|
||||
`behind ${baseDrift.base}. Pull/rebase the worktree, recreate it with ` +
|
||||
`--base-branch ${baseDrift.base}, or include 'allow-stale-base: true' ` +
|
||||
`in the task spec to override. Task remains in 'ready'; coordinator ` +
|
||||
`will retry on the next tick.`
|
||||
)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
const dispatch = this.db.createDispatchContext(task.id, targetHandle)
|
||||
|
||||
// Why: agents dispatched by the coordinator must use orca-dev in dev mode
|
||||
// so they talk to the dev runtime's socket, not production (Section 6.4).
|
||||
// Why (§3.4): `strippedSpec` drops the `allow-stale-base: true` line so
|
||||
// the worker's `--- TASK ---` block does not contain the infra flag (which
|
||||
// the worker would otherwise read as part of its instructions).
|
||||
const preamble = buildDispatchPreamble({
|
||||
taskId: task.id,
|
||||
taskSpec: task.spec,
|
||||
dispatchId: dispatch.id,
|
||||
// Why (§3.4, stale-base PR): use `strippedSpec` not `task.spec` so the
|
||||
// `allow-stale-base: true` line isn't rendered into the worker's
|
||||
// --- TASK --- block (worker would otherwise treat the infra flag as
|
||||
// part of its instructions).
|
||||
taskSpec: strippedSpec,
|
||||
coordinatorHandle: this.opts.coordinatorHandle,
|
||||
devMode: process.env.ORCA_USER_DATA_PATH?.includes('orca-dev')
|
||||
devMode: process.env.ORCA_USER_DATA_PATH?.includes('orca-dev'),
|
||||
// Why (§3.2): drift section fires only when behind > 0. The preamble
|
||||
// builder gates on this itself; passing the object unconditionally lets
|
||||
// the coordinator stay dumb about the display rule.
|
||||
...(baseDrift ? { baseDrift } : {})
|
||||
})
|
||||
|
||||
// Why: check if the task was previously blocked by a decision gate that
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
/* eslint-disable max-lines -- Why: DB tests cover messages, tasks, dispatch contexts, decision gates, coordinator runs, and lifecycle in one suite to share the createDb() helper and afterEach cleanup. */
|
||||
import Database from 'better-sqlite3'
|
||||
import { mkdtempSync, rmSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { OrchestrationDb } from './db'
|
||||
import type { MessageType } from './db'
|
||||
@@ -198,6 +202,36 @@ describe('OrchestrationDb', () => {
|
||||
expect(d.listTasks()).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('listTasksWithDispatch joins active dispatch metadata', () => {
|
||||
const d = createDb()
|
||||
const ready = d.createTask({ spec: 'ready task' })
|
||||
const dispatched = d.createTask({ spec: 'active task' })
|
||||
const ctx = d.createDispatchContext(dispatched.id, 'term_worker')
|
||||
|
||||
const rows = d.listTasksWithDispatch()
|
||||
const readyRow = rows.find((r) => r.id === ready.id)
|
||||
const dispatchedRow = rows.find((r) => r.id === dispatched.id)
|
||||
|
||||
expect(readyRow?.assignee_handle).toBeNull()
|
||||
expect(readyRow?.dispatch_id).toBeNull()
|
||||
expect(dispatchedRow?.assignee_handle).toBe('term_worker')
|
||||
expect(dispatchedRow?.dispatch_id).toBe(ctx.id)
|
||||
})
|
||||
|
||||
it('listTasksWithDispatch does not surface completed dispatches', () => {
|
||||
const d = createDb()
|
||||
const task = d.createTask({ spec: 'work' })
|
||||
d.createDispatchContext(task.id, 'term_worker')
|
||||
d.updateTaskStatus(task.id, 'completed')
|
||||
|
||||
const rows = d.listTasksWithDispatch()
|
||||
const row = rows.find((r) => r.id === task.id)
|
||||
// Task is completed — its dispatch is terminal and should not appear as
|
||||
// an "active" assignee.
|
||||
expect(row?.assignee_handle).toBeNull()
|
||||
expect(row?.dispatch_id).toBeNull()
|
||||
})
|
||||
|
||||
it('supports parent_id for task decomposition', () => {
|
||||
const d = createDb()
|
||||
const parent = d.createTask({ spec: 'parent' })
|
||||
@@ -454,4 +488,278 @@ describe('OrchestrationDb', () => {
|
||||
expect(d.listTasks()).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('heartbeat + thread helpers (fresh schema)', () => {
|
||||
it('insertMessage accepts type = heartbeat', () => {
|
||||
const d = createDb()
|
||||
const msg = d.insertMessage({
|
||||
from: 'worker',
|
||||
to: 'coord',
|
||||
subject: 'alive',
|
||||
type: 'heartbeat',
|
||||
payload: JSON.stringify({ taskId: 'task_x', dispatchId: 'ctx_x' })
|
||||
})
|
||||
expect(msg.type).toBe('heartbeat')
|
||||
})
|
||||
|
||||
it('recordHeartbeat updates last_heartbeat_at on dispatched rows', () => {
|
||||
const d = createDb()
|
||||
const task = d.createTask({ spec: 'work' })
|
||||
const ctx = d.createDispatchContext(task.id, 'term_a')
|
||||
|
||||
d.recordHeartbeat(ctx.id, '2026-05-04T00:00:00.000Z')
|
||||
const after = d.getDispatchContext(task.id)
|
||||
expect(after?.last_heartbeat_at).toBe('2026-05-04T00:00:00.000Z')
|
||||
})
|
||||
|
||||
it('recordHeartbeat is a no-op for completed rows (straggler ignored)', () => {
|
||||
const d = createDb()
|
||||
const task = d.createTask({ spec: 'work' })
|
||||
const ctx = d.createDispatchContext(task.id, 'term_a')
|
||||
d.completeDispatch(ctx.id)
|
||||
|
||||
d.recordHeartbeat(ctx.id, '2026-05-04T00:00:00.000Z')
|
||||
const after = d.getDispatchContext(task.id)
|
||||
expect(after?.last_heartbeat_at).toBeNull()
|
||||
})
|
||||
|
||||
it('getStaleDispatches returns only dispatched rows past the grace window', () => {
|
||||
const d = createDb()
|
||||
// Fixture: four rows, SQL-backdated timestamps (no fake clock):
|
||||
// (a) dispatched, heartbeated 5 min ago → not stale
|
||||
// (b) dispatched, heartbeated 12 min ago → STALE (expected result)
|
||||
// (c) dispatched, never heartbeated, dispatched 30s ago → not stale (grace)
|
||||
// (d) completed, heartbeated 30 min ago → not stale (status filter)
|
||||
const taskA = d.createTask({ spec: 'a' })
|
||||
const taskB = d.createTask({ spec: 'b' })
|
||||
const taskC = d.createTask({ spec: 'c' })
|
||||
const taskD = d.createTask({ spec: 'd' })
|
||||
const ctxA = d.createDispatchContext(taskA.id, 'term_a')
|
||||
const ctxB = d.createDispatchContext(taskB.id, 'term_b')
|
||||
const ctxC = d.createDispatchContext(taskC.id, 'term_c')
|
||||
const ctxD = d.createDispatchContext(taskD.id, 'term_d')
|
||||
d.completeDispatch(ctxD.id)
|
||||
|
||||
const now = Date.now()
|
||||
const iso = (ms: number) => new Date(now - ms).toISOString()
|
||||
|
||||
// Backdate dispatched_at for a, b, d to long ago so the grace doesn't
|
||||
// shield them. c keeps its default (≈now).
|
||||
const sqlite = (d as unknown as { db: Database.Database }).db
|
||||
sqlite
|
||||
.prepare(
|
||||
'UPDATE dispatch_contexts SET dispatched_at = ?, last_heartbeat_at = ? WHERE id = ?'
|
||||
)
|
||||
.run(iso(60 * 60 * 1000), iso(5 * 60 * 1000), ctxA.id)
|
||||
sqlite
|
||||
.prepare(
|
||||
'UPDATE dispatch_contexts SET dispatched_at = ?, last_heartbeat_at = ? WHERE id = ?'
|
||||
)
|
||||
.run(iso(60 * 60 * 1000), iso(12 * 60 * 1000), ctxB.id)
|
||||
sqlite
|
||||
.prepare('UPDATE dispatch_contexts SET dispatched_at = ? WHERE id = ?')
|
||||
.run(iso(30_000), ctxC.id)
|
||||
sqlite
|
||||
.prepare(
|
||||
'UPDATE dispatch_contexts SET dispatched_at = ?, last_heartbeat_at = ? WHERE id = ?'
|
||||
)
|
||||
.run(iso(60 * 60 * 1000), iso(30 * 60 * 1000), ctxD.id)
|
||||
|
||||
const stale = d.getStaleDispatches(iso(10 * 60 * 1000))
|
||||
expect(stale.map((s) => s.id)).toEqual([ctxB.id])
|
||||
})
|
||||
|
||||
it('getThreadMessagesFor returns only same-thread replies to a handle', () => {
|
||||
const d = createDb()
|
||||
const outbound = d.insertMessage({
|
||||
from: 'worker',
|
||||
to: 'coord',
|
||||
subject: 'Question',
|
||||
type: 'decision_gate',
|
||||
body: 'yes or no?'
|
||||
})
|
||||
// Reply in the same thread addressed to the worker
|
||||
const reply = d.insertMessage({
|
||||
from: 'coord',
|
||||
to: 'worker',
|
||||
subject: 'Re: Question',
|
||||
body: 'yes',
|
||||
threadId: outbound.id
|
||||
})
|
||||
// Distractor: different thread, same recipient
|
||||
d.insertMessage({
|
||||
from: 'coord',
|
||||
to: 'worker',
|
||||
subject: 'other',
|
||||
body: 'unrelated',
|
||||
threadId: 'thread_other'
|
||||
})
|
||||
// Distractor: same thread but not addressed to worker
|
||||
d.insertMessage({
|
||||
from: 'coord',
|
||||
to: 'someone_else',
|
||||
subject: 'cc',
|
||||
body: 'not yours',
|
||||
threadId: outbound.id
|
||||
})
|
||||
|
||||
const replies = d.getThreadMessagesFor(outbound.id, 'worker', outbound.sequence)
|
||||
expect(replies).toHaveLength(1)
|
||||
expect(replies[0].id).toBe(reply.id)
|
||||
})
|
||||
})
|
||||
|
||||
describe('schema migration from v1 → v2', () => {
|
||||
let dbPath: string
|
||||
let tempDir: string
|
||||
|
||||
afterEach(() => {
|
||||
if (tempDir) {
|
||||
rmSync(tempDir, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
function createV1Snapshot(): string {
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'orca-db-migrate-'))
|
||||
dbPath = join(tempDir, 'test.db')
|
||||
const raw = new Database(dbPath)
|
||||
// v1 schema: pre-heartbeat CHECK, no last_heartbeat_at column.
|
||||
raw.exec(`
|
||||
CREATE TABLE messages (
|
||||
id TEXT NOT NULL,
|
||||
from_handle TEXT NOT NULL,
|
||||
to_handle TEXT NOT NULL,
|
||||
subject TEXT NOT NULL,
|
||||
body TEXT NOT NULL DEFAULT '',
|
||||
type TEXT NOT NULL DEFAULT 'status'
|
||||
CHECK(type IN (
|
||||
'status', 'dispatch', 'worker_done', 'merge_ready',
|
||||
'escalation', 'handoff', 'decision_gate'
|
||||
)),
|
||||
priority TEXT NOT NULL DEFAULT 'normal'
|
||||
CHECK(priority IN ('normal', 'high', 'urgent')),
|
||||
thread_id TEXT,
|
||||
payload TEXT,
|
||||
read INTEGER NOT NULL DEFAULT 0,
|
||||
sequence INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
CREATE UNIQUE INDEX idx_messages_id ON messages(id);
|
||||
CREATE INDEX idx_inbox ON messages(to_handle, read);
|
||||
CREATE INDEX idx_thread ON messages(thread_id);
|
||||
|
||||
CREATE TABLE tasks (
|
||||
id TEXT PRIMARY KEY, parent_id TEXT, spec TEXT NOT NULL,
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
CHECK(status IN ('pending','ready','dispatched','completed','failed','blocked')),
|
||||
deps TEXT NOT NULL DEFAULT '[]', result TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
completed_at TEXT
|
||||
);
|
||||
|
||||
CREATE TABLE dispatch_contexts (
|
||||
id TEXT PRIMARY KEY, task_id TEXT NOT NULL, assignee_handle TEXT,
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
CHECK(status IN ('pending','dispatched','completed','failed','circuit_broken')),
|
||||
failure_count INTEGER NOT NULL DEFAULT 0, last_failure TEXT,
|
||||
dispatched_at TEXT, completed_at TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
|
||||
CREATE TABLE decision_gates (
|
||||
id TEXT PRIMARY KEY, task_id TEXT NOT NULL, question TEXT NOT NULL,
|
||||
options TEXT NOT NULL DEFAULT '[]',
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
CHECK(status IN ('pending','resolved','timeout')),
|
||||
resolution TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
resolved_at TEXT
|
||||
);
|
||||
|
||||
CREATE TABLE coordinator_runs (
|
||||
id TEXT PRIMARY KEY, spec TEXT NOT NULL,
|
||||
status TEXT NOT NULL DEFAULT 'idle'
|
||||
CHECK(status IN ('idle','running','completed','failed')),
|
||||
coordinator_handle TEXT NOT NULL,
|
||||
poll_interval_ms INTEGER NOT NULL DEFAULT 2000,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
completed_at TEXT
|
||||
);
|
||||
`)
|
||||
// Seed a pre-existing v1 message so migration must preserve data.
|
||||
raw
|
||||
.prepare(
|
||||
`INSERT INTO messages (id, from_handle, to_handle, subject, type) VALUES ('msg_v1', 'a', 'b', 'pre-migration', 'status')`
|
||||
)
|
||||
.run()
|
||||
raw.pragma('user_version = 0')
|
||||
raw.close()
|
||||
return dbPath
|
||||
}
|
||||
|
||||
it('migrates a v1 snapshot to v2, accepts heartbeat, preserves indexes', () => {
|
||||
const path = createV1Snapshot()
|
||||
const d = new OrchestrationDb(path)
|
||||
db = d
|
||||
|
||||
// (a) INSERT type='heartbeat' now succeeds
|
||||
expect(() =>
|
||||
d.insertMessage({
|
||||
from: 'w',
|
||||
to: 'c',
|
||||
subject: 'alive',
|
||||
type: 'heartbeat',
|
||||
payload: '{"taskId":"t","dispatchId":"ctx"}'
|
||||
})
|
||||
).not.toThrow()
|
||||
|
||||
// (b) last_heartbeat_at column exists on dispatch_contexts
|
||||
const task = d.createTask({ spec: 'work' })
|
||||
const ctx = d.createDispatchContext(task.id, 'term_a')
|
||||
d.recordHeartbeat(ctx.id, '2026-05-04T00:00:00.000Z')
|
||||
expect(d.getDispatchContext(task.id)?.last_heartbeat_at).toBe('2026-05-04T00:00:00.000Z')
|
||||
|
||||
// (c) Indexes still attached to messages post-rebuild.
|
||||
const sqlite = (d as unknown as { db: Database.Database }).db
|
||||
const indexes = sqlite
|
||||
.prepare(
|
||||
`SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'messages' AND name NOT LIKE 'sqlite_%'`
|
||||
)
|
||||
.all() as { name: string }[]
|
||||
const names = new Set(indexes.map((r) => r.name))
|
||||
expect(names.has('idx_messages_id')).toBe(true)
|
||||
expect(names.has('idx_inbox')).toBe(true)
|
||||
expect(names.has('idx_thread')).toBe(true)
|
||||
|
||||
// v1 data preserved
|
||||
expect(d.getMessageById('msg_v1')?.subject).toBe('pre-migration')
|
||||
})
|
||||
|
||||
it('is idempotent: opening an already-migrated DB is a no-op', () => {
|
||||
const path = createV1Snapshot()
|
||||
const first = new OrchestrationDb(path)
|
||||
first.insertMessage({
|
||||
from: 'w',
|
||||
to: 'c',
|
||||
subject: 'alive',
|
||||
type: 'heartbeat',
|
||||
payload: '{}'
|
||||
})
|
||||
first.close()
|
||||
|
||||
const second = new OrchestrationDb(path)
|
||||
db = second
|
||||
expect(() =>
|
||||
second.insertMessage({
|
||||
from: 'w',
|
||||
to: 'c',
|
||||
subject: 'again',
|
||||
type: 'heartbeat',
|
||||
payload: '{}'
|
||||
})
|
||||
).not.toThrow()
|
||||
const inbox = second.getInbox(10)
|
||||
expect(inbox.length).toBeGreaterThanOrEqual(2)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -33,6 +33,14 @@ function generateId(prefix: string): string {
|
||||
return `${prefix}_${randomBytes(6).toString('hex')}`
|
||||
}
|
||||
|
||||
// Why: v1 → v2 added `'heartbeat'` to messages.type CHECK + `last_heartbeat_at`
|
||||
// column (preamble-hardening PR). v2 → v3 adds `delivered_at` column so
|
||||
// push-on-idle can distinguish queued-but-undelivered from user-acknowledged
|
||||
// messages without touching the `read` bit (check-wait PR). Bumping together
|
||||
// since both PRs ship in the same release — existing on-disk DBs must clear
|
||||
// both gaps in one atomic migration.
|
||||
const SCHEMA_VERSION = 3
|
||||
|
||||
export class OrchestrationDb {
|
||||
private db: Database.Database
|
||||
|
||||
@@ -42,6 +50,7 @@ export class OrchestrationDb {
|
||||
this.db.pragma('synchronous = NORMAL')
|
||||
this.db.pragma('busy_timeout = 5000')
|
||||
this.createTables()
|
||||
this.migrate()
|
||||
}
|
||||
|
||||
private createTables(): void {
|
||||
@@ -55,7 +64,7 @@ export class OrchestrationDb {
|
||||
type TEXT NOT NULL DEFAULT 'status'
|
||||
CHECK(type IN (
|
||||
'status', 'dispatch', 'worker_done', 'merge_ready',
|
||||
'escalation', 'handoff', 'decision_gate'
|
||||
'escalation', 'handoff', 'decision_gate', 'heartbeat'
|
||||
)),
|
||||
priority TEXT NOT NULL DEFAULT 'normal'
|
||||
CHECK(priority IN ('normal', 'high', 'urgent')),
|
||||
@@ -63,7 +72,8 @@ export class OrchestrationDb {
|
||||
payload TEXT,
|
||||
read INTEGER NOT NULL DEFAULT 0,
|
||||
sequence INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
delivered_at TEXT
|
||||
);
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_messages_id ON messages(id);
|
||||
@@ -89,16 +99,17 @@ export class OrchestrationDb {
|
||||
CREATE INDEX IF NOT EXISTS idx_tasks_parent ON tasks(parent_id);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS dispatch_contexts (
|
||||
id TEXT PRIMARY KEY,
|
||||
task_id TEXT NOT NULL,
|
||||
assignee_handle TEXT,
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
id TEXT PRIMARY KEY,
|
||||
task_id TEXT NOT NULL,
|
||||
assignee_handle TEXT,
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
CHECK(status IN ('pending', 'dispatched', 'completed', 'failed', 'circuit_broken')),
|
||||
failure_count INTEGER NOT NULL DEFAULT 0,
|
||||
last_failure TEXT,
|
||||
dispatched_at TEXT,
|
||||
completed_at TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
failure_count INTEGER NOT NULL DEFAULT 0,
|
||||
last_failure TEXT,
|
||||
dispatched_at TEXT,
|
||||
completed_at TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
last_heartbeat_at TEXT
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_dispatch_task ON dispatch_contexts(task_id);
|
||||
@@ -132,6 +143,115 @@ export class OrchestrationDb {
|
||||
`)
|
||||
}
|
||||
|
||||
// Why: `CREATE TABLE IF NOT EXISTS` is a no-op against an existing on-disk
|
||||
// DB, so new schema shapes (added columns, widened CHECK constraints) do
|
||||
// not reach an upgraded user unless we migrate explicitly. The transaction
|
||||
// guarantees atomicity — a mid-migration crash leaves the DB at the prior
|
||||
// version because `user_version` is bumped only on success. Idempotent
|
||||
// re-invocation is a no-op (current >= SCHEMA_VERSION short-circuit).
|
||||
private migrate(): void {
|
||||
const current = this.db.pragma('user_version', { simple: true }) as number
|
||||
if (current >= SCHEMA_VERSION) {
|
||||
return
|
||||
}
|
||||
|
||||
this.db.exec('BEGIN')
|
||||
try {
|
||||
// v1 → v2: add last_heartbeat_at column; widen messages.type CHECK to
|
||||
// include 'heartbeat'. SQLite cannot ALTER a CHECK constraint, so we
|
||||
// rebuild the messages table. We also include `delivered_at` in the
|
||||
// rebuilt schema so DBs migrating from v1 pick up the v3 column in a
|
||||
// single table-rewrite pass (avoids a second messages-rebuild later).
|
||||
if (current < 2) {
|
||||
if (!this.hasColumn('dispatch_contexts', 'last_heartbeat_at')) {
|
||||
this.db.exec(`ALTER TABLE dispatch_contexts ADD COLUMN last_heartbeat_at TEXT`)
|
||||
}
|
||||
|
||||
if (!this.messagesTypeCheckAllowsHeartbeat()) {
|
||||
// Why — index list is not optional. createTables() already attached
|
||||
// idx_messages_id / idx_inbox / idx_thread to the old messages table;
|
||||
// DROP TABLE removes those indexes with it. CREATE INDEX IF NOT
|
||||
// EXISTS in createTables() only runs on the next process startup,
|
||||
// so skipping explicit recreation here would leave every
|
||||
// getUnreadMessages / getMessageById call full-scanning for the
|
||||
// rest of this process's lifetime — a silent O(N) perf regression.
|
||||
// The three CREATE INDEX statements below mirror createTables()
|
||||
// verbatim so the two definitions cannot drift.
|
||||
this.db.exec(`
|
||||
CREATE TABLE messages_new (
|
||||
id TEXT NOT NULL,
|
||||
from_handle TEXT NOT NULL,
|
||||
to_handle TEXT NOT NULL,
|
||||
subject TEXT NOT NULL,
|
||||
body TEXT NOT NULL DEFAULT '',
|
||||
type TEXT NOT NULL DEFAULT 'status'
|
||||
CHECK(type IN (
|
||||
'status', 'dispatch', 'worker_done', 'merge_ready',
|
||||
'escalation', 'handoff', 'decision_gate', 'heartbeat'
|
||||
)),
|
||||
priority TEXT NOT NULL DEFAULT 'normal'
|
||||
CHECK(priority IN ('normal', 'high', 'urgent')),
|
||||
thread_id TEXT,
|
||||
payload TEXT,
|
||||
read INTEGER NOT NULL DEFAULT 0,
|
||||
sequence INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
delivered_at TEXT
|
||||
);
|
||||
INSERT INTO messages_new (
|
||||
id, from_handle, to_handle, subject, body, type, priority,
|
||||
thread_id, payload, read, sequence, created_at
|
||||
)
|
||||
SELECT
|
||||
id, from_handle, to_handle, subject, body, type, priority,
|
||||
thread_id, payload, read, sequence, created_at
|
||||
FROM messages;
|
||||
DROP TABLE messages;
|
||||
ALTER TABLE messages_new RENAME TO messages;
|
||||
|
||||
CREATE UNIQUE INDEX idx_messages_id ON messages(id);
|
||||
CREATE INDEX idx_inbox ON messages(to_handle, read);
|
||||
CREATE INDEX idx_thread ON messages(thread_id);
|
||||
`)
|
||||
}
|
||||
}
|
||||
|
||||
// v2 → v3: add `delivered_at` column to messages. A DB that reached v2
|
||||
// via the v1 → v2 rebuild above already has the column (we included
|
||||
// it in messages_new); this handles DBs that were at v2 before this
|
||||
// release shipped (preamble PR deployed standalone, then check-wait
|
||||
// merged). ALTER TABLE is idempotent via the hasColumn probe — a
|
||||
// duplicate-column error would abort the whole transaction.
|
||||
if (current < 3) {
|
||||
if (!this.hasColumn('messages', 'delivered_at')) {
|
||||
this.db.exec(`ALTER TABLE messages ADD COLUMN delivered_at TEXT`)
|
||||
}
|
||||
}
|
||||
|
||||
this.db.pragma(`user_version = ${SCHEMA_VERSION}`)
|
||||
this.db.exec('COMMIT')
|
||||
} catch (err) {
|
||||
this.db.exec('ROLLBACK')
|
||||
throw err
|
||||
}
|
||||
}
|
||||
|
||||
private hasColumn(table: string, column: string): boolean {
|
||||
const rows = this.db.pragma(`table_info(${table})`) as { name: string }[]
|
||||
return rows.some((r) => r.name === column)
|
||||
}
|
||||
|
||||
// Why: sqlite_master stores the original CREATE TABLE SQL including the
|
||||
// CHECK clause. Inspecting that text is the cheapest reliable way to tell
|
||||
// whether the pre-rebuild schema already knows about 'heartbeat' without
|
||||
// needing a dedicated schema_meta row.
|
||||
private messagesTypeCheckAllowsHeartbeat(): boolean {
|
||||
const row = this.db
|
||||
.prepare("SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'messages'")
|
||||
.get() as { sql: string } | undefined
|
||||
return !!row && row.sql.includes("'heartbeat'")
|
||||
}
|
||||
|
||||
// ── Messages ──
|
||||
|
||||
insertMessage(msg: {
|
||||
@@ -195,12 +315,56 @@ export class OrchestrationDb {
|
||||
this.db.prepare(`UPDATE messages SET read = 1 WHERE id IN (${placeholders})`).run(...ids)
|
||||
}
|
||||
|
||||
// Why: `delivered_at` is stamped via SQLite's datetime('now') rather than a
|
||||
// JS ISO string so it uses the same 'YYYY-MM-DD HH:MM:SS' UTC shape as the
|
||||
// other SQL-default timestamps on this table. A future ORDER BY or
|
||||
// comparison against created_at relies on this format consistency.
|
||||
// See design doc §3.2.
|
||||
markAsDelivered(ids: string[]): void {
|
||||
if (ids.length === 0) {
|
||||
return
|
||||
}
|
||||
const placeholders = ids.map(() => '?').join(',')
|
||||
this.db
|
||||
.prepare(`UPDATE messages SET delivered_at = datetime('now') WHERE id IN (${placeholders})`)
|
||||
.run(...ids)
|
||||
}
|
||||
|
||||
getInbox(limit = 20): MessageRow[] {
|
||||
return this.db
|
||||
.prepare('SELECT * FROM messages ORDER BY sequence DESC LIMIT ?')
|
||||
.all(limit) as MessageRow[]
|
||||
}
|
||||
|
||||
// Why: used by `check --all` and `inbox --terminal <handle>` — returns every
|
||||
// message for a handle regardless of read/delivered state; never touches the
|
||||
// read bit. Stale-handle safe: if the handle no longer exists, the query
|
||||
// just returns whatever historical rows remain (§3.3).
|
||||
getAllMessagesForHandle(toHandle: string, limit = 100): MessageRow[] {
|
||||
return this.db
|
||||
.prepare('SELECT * FROM messages WHERE to_handle = ? ORDER BY sequence DESC LIMIT ?')
|
||||
.all(toHandle, limit) as MessageRow[]
|
||||
}
|
||||
|
||||
// Why: thread-scoped read for the `orchestration.ask` wait loop. Filtered
|
||||
// by `to_handle` so a worker only sees replies addressed to it (not
|
||||
// messages it sent), and ordered by `sequence` so the first post-ask
|
||||
// reply is returned first. `afterSequence` lets the caller resume past an
|
||||
// already-seen marker without re-reading the outbound ask itself. Uses
|
||||
// the existing idx_thread index (see createTables) — no new index.
|
||||
getThreadMessagesFor(threadId: string, toHandle: string, afterSequence?: number): MessageRow[] {
|
||||
if (afterSequence !== undefined) {
|
||||
return this.db
|
||||
.prepare(
|
||||
'SELECT * FROM messages WHERE thread_id = ? AND to_handle = ? AND sequence > ? ORDER BY sequence ASC'
|
||||
)
|
||||
.all(threadId, toHandle, afterSequence) as MessageRow[]
|
||||
}
|
||||
return this.db
|
||||
.prepare('SELECT * FROM messages WHERE thread_id = ? AND to_handle = ? ORDER BY sequence ASC')
|
||||
.all(threadId, toHandle) as MessageRow[]
|
||||
}
|
||||
|
||||
// ── Tasks ──
|
||||
|
||||
createTask(task: { spec: string; deps?: string[]; parentId?: string }): TaskRow {
|
||||
@@ -232,6 +396,50 @@ export class OrchestrationDb {
|
||||
return this.db.prepare('SELECT * FROM tasks ORDER BY created_at').all() as TaskRow[]
|
||||
}
|
||||
|
||||
// Why: surfaces the active dispatch (assignee handle + dispatch context id)
|
||||
// alongside each task so coordinators can answer "who is working on task X?"
|
||||
// from a single query. The LEFT JOIN keeps non-dispatched tasks in the result
|
||||
// with NULL assignee/dispatch fields so non-dispatched output stays stable.
|
||||
// The inner subquery picks the most recent active dispatch per task to match
|
||||
// the semantics of getDispatchContext for dispatched tasks.
|
||||
listTasksWithDispatch(filter?: { status?: TaskStatus; ready?: boolean }): (TaskRow & {
|
||||
assignee_handle: string | null
|
||||
dispatch_id: string | null
|
||||
})[] {
|
||||
const whereClauses: string[] = []
|
||||
const params: unknown[] = []
|
||||
if (filter?.ready) {
|
||||
whereClauses.push("t.status = 'ready'")
|
||||
} else if (filter?.status) {
|
||||
whereClauses.push('t.status = ?')
|
||||
params.push(filter.status)
|
||||
}
|
||||
const where = whereClauses.length > 0 ? `WHERE ${whereClauses.join(' AND ')}` : ''
|
||||
const sql = `
|
||||
SELECT
|
||||
t.*,
|
||||
d.assignee_handle AS assignee_handle,
|
||||
d.id AS dispatch_id
|
||||
FROM tasks t
|
||||
LEFT JOIN (
|
||||
SELECT dc.*
|
||||
FROM dispatch_contexts dc
|
||||
INNER JOIN (
|
||||
SELECT task_id, MAX(rowid) AS max_rowid
|
||||
FROM dispatch_contexts
|
||||
WHERE status IN ('pending', 'dispatched')
|
||||
GROUP BY task_id
|
||||
) latest ON latest.task_id = dc.task_id AND latest.max_rowid = dc.rowid
|
||||
) d ON d.task_id = t.id
|
||||
${where}
|
||||
ORDER BY t.created_at
|
||||
`
|
||||
return this.db.prepare(sql).all(...params) as (TaskRow & {
|
||||
assignee_handle: string | null
|
||||
dispatch_id: string | null
|
||||
})[]
|
||||
}
|
||||
|
||||
updateTaskStatus(id: string, status: TaskStatus, result?: string): TaskRow | undefined {
|
||||
const completedAt =
|
||||
status === 'completed' || status === 'failed' ? new Date().toISOString() : null
|
||||
@@ -362,6 +570,40 @@ export class OrchestrationDb {
|
||||
return active ? this.failDispatch(active.id, error) : undefined
|
||||
}
|
||||
|
||||
// Why: only touch rows that are currently dispatched. A straggler heartbeat
|
||||
// from a dispatch that already transitioned to `completed` / `failed` /
|
||||
// `circuit_broken` MUST NOT retroactively bump `last_heartbeat_at`, because
|
||||
// the stale-dispatch detector is the signal the coordinator uses to know a
|
||||
// newer dispatch for the same task has hung. Silently no-op'ing keeps the
|
||||
// zombie-heartbeat race from masking a hung retry (§5.3.4).
|
||||
recordHeartbeat(dispatchId: string, at: string): void {
|
||||
this.db
|
||||
.prepare(
|
||||
"UPDATE dispatch_contexts SET last_heartbeat_at = ? WHERE id = ? AND status = 'dispatched'"
|
||||
)
|
||||
.run(at, dispatchId)
|
||||
}
|
||||
|
||||
// Why: the query restricts to currently-dispatched contexts AND respects a
|
||||
// dispatched-at grace. Without `status = 'dispatched'`, every completed /
|
||||
// failed / circuit_broken row with an old-or-null last_heartbeat_at would
|
||||
// warn every tick (warning storm). Without `dispatched_at < :threshold`,
|
||||
// a freshly-dispatched worker would trip the warning during its first
|
||||
// heartbeat interval (false positive). Callers supply the threshold as an
|
||||
// ISO timestamp so the SQLite string-compare ordering works correctly
|
||||
// (ISO-8601 compares lexicographically in time order).
|
||||
getStaleDispatches(thresholdIso: string): DispatchContextRow[] {
|
||||
return this.db
|
||||
.prepare(
|
||||
`SELECT * FROM dispatch_contexts
|
||||
WHERE status = 'dispatched'
|
||||
AND dispatched_at IS NOT NULL
|
||||
AND dispatched_at < ?
|
||||
AND (last_heartbeat_at IS NULL OR last_heartbeat_at < ?)`
|
||||
)
|
||||
.all(thresholdIso, thresholdIso) as DispatchContextRow[]
|
||||
}
|
||||
|
||||
failDispatch(ctxId: string, error: string): DispatchContextRow | undefined {
|
||||
const ctx = this.db.prepare('SELECT * FROM dispatch_contexts WHERE id = ?').get(ctxId) as
|
||||
| DispatchContextRow
|
||||
|
||||
@@ -16,6 +16,7 @@ function makeMessage(overrides: Partial<MessageRow> = {}): MessageRow {
|
||||
read: 0,
|
||||
sequence: 1,
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
delivered_at: null,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,67 +1,118 @@
|
||||
import { spawnSync } from 'child_process'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildDispatchPreamble } from './preamble'
|
||||
|
||||
function baseParams(overrides: Partial<Parameters<typeof buildDispatchPreamble>[0]> = {}) {
|
||||
return {
|
||||
taskId: 'task_abc123',
|
||||
dispatchId: 'ctx_def456',
|
||||
taskSpec: 'Implement the login form',
|
||||
coordinatorHandle: 'term_coord',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('buildDispatchPreamble', () => {
|
||||
it('substitutes template variables', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_abc123',
|
||||
taskSpec: 'Implement the login form',
|
||||
coordinatorHandle: 'term_coord'
|
||||
})
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
|
||||
expect(result).toContain('task_abc123')
|
||||
expect(result).toContain('ctx_def456')
|
||||
expect(result).toContain('term_coord')
|
||||
expect(result).toContain('Implement the login form')
|
||||
expect(result).not.toContain('{{')
|
||||
})
|
||||
|
||||
it('includes worker_done command', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c'
|
||||
})
|
||||
it('includes worker_done command with --body 3-sentence summary prompt and reportPath', () => {
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
|
||||
expect(result).toContain('worker_done')
|
||||
expect(result).toContain('orchestration send')
|
||||
expect(result).toContain('orchestration check')
|
||||
expect(result).toContain('--body')
|
||||
expect(result).toMatch(/3-sentence summary/)
|
||||
expect(result).toContain('reportPath')
|
||||
})
|
||||
|
||||
it('includes the task spec after separator', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
taskSpec: 'refactor the auth module',
|
||||
coordinatorHandle: 'term_c'
|
||||
})
|
||||
it('CLI examples parse as valid shell (bash -n on the extracted block)', () => {
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
// Why: feeding `bash -n` the full preamble falsely fails on apostrophes
|
||||
// in the surrounding prose. Slice between the CLI markers and strip
|
||||
// shell-style comment lines so we only syntax-check the commands.
|
||||
const cliStart = result.indexOf('=== CLI COMMANDS ===')
|
||||
const cliEnd = result.indexOf('=== AFTER YOU SEND worker_done ===')
|
||||
expect(cliStart).toBeGreaterThan(-1)
|
||||
expect(cliEnd).toBeGreaterThan(cliStart)
|
||||
const block = result.slice(cliStart, cliEnd)
|
||||
const stripped = block
|
||||
.split('\n')
|
||||
.filter((line) => !line.trim().startsWith('#'))
|
||||
.filter((line) => !line.trim().startsWith('==='))
|
||||
.join('\n')
|
||||
|
||||
expect(result).toContain('--- TASK ---')
|
||||
const check = spawnSync('bash', ['-n'], { input: stripped, encoding: 'utf8' })
|
||||
expect(check.status).toBe(0)
|
||||
})
|
||||
|
||||
it('includes heartbeat CLI block with taskId and dispatchId and 5-minute cadence', () => {
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
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
|
||||
// (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')
|
||||
})
|
||||
|
||||
it('includes ask block with BEHAVIOR RULE #1 forbidding AskUserQuestion', () => {
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
expect(result).toContain('orchestration ask')
|
||||
expect(result).toContain('--question')
|
||||
expect(result).toContain('--timeout-ms 600000')
|
||||
// Why: the exact phrase is asserted so the rule can't be trimmed away by
|
||||
// accident. BEHAVIOR RULE #1 is the only place AskUserQuestion appears.
|
||||
expect(result).toContain('BEHAVIOR RULE #1')
|
||||
expect(result).toContain('NEVER use AskUserQuestion')
|
||||
// AskUserQuestion must appear ONLY inside the rule text, not anywhere
|
||||
// else (e.g., not in an example payload or header). Count occurrences
|
||||
// of the exact token as a sanity check.
|
||||
const occurrences = (result.match(/AskUserQuestion/g) ?? []).length
|
||||
// Three mentions: the one-liner ban, the TUI-prompt rationale, and the
|
||||
// "when tempted to reach for AskUserQuestion" closing line.
|
||||
expect(occurrences).toBe(3)
|
||||
})
|
||||
|
||||
it('includes AFTER YOU SEND block with 2-minute poll cadence and release signal', () => {
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
expect(result).toContain('=== AFTER YOU SEND worker_done ===')
|
||||
expect(result).toMatch(/2 minutes/)
|
||||
expect(result).toMatch(/may exit/)
|
||||
})
|
||||
|
||||
it('uses === TASK === separator with the task spec appended', () => {
|
||||
const result = buildDispatchPreamble(baseParams({ taskSpec: 'refactor the auth module' }))
|
||||
expect(result).toContain('=== TASK ===')
|
||||
expect(result).toContain('refactor the auth module')
|
||||
})
|
||||
|
||||
it('uses orca CLI by default when devMode is not set', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c'
|
||||
})
|
||||
|
||||
const result = buildDispatchPreamble(baseParams())
|
||||
expect(result).toContain('orca orchestration send')
|
||||
expect(result).toContain('orca orchestration check')
|
||||
expect(result).toContain('orca orchestration ask')
|
||||
})
|
||||
|
||||
it('uses orca-dev CLI when devMode is true', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c',
|
||||
devMode: true
|
||||
})
|
||||
|
||||
const result = buildDispatchPreamble(baseParams({ devMode: true }))
|
||||
expect(result).toContain('orca-dev orchestration send')
|
||||
expect(result).toContain('orca-dev orchestration check')
|
||||
// Ensure no bare "orca " (without -dev) appears as a CLI command.
|
||||
// We split on "orca-dev" first so those occurrences don't produce
|
||||
// false positives, then check the remaining fragments.
|
||||
expect(result).toContain('orca-dev orchestration ask')
|
||||
const fragments = result.split('orca-dev')
|
||||
for (const fragment of fragments) {
|
||||
expect(fragment).not.toMatch(/orca orchestration/)
|
||||
@@ -69,14 +120,92 @@ describe('buildDispatchPreamble', () => {
|
||||
})
|
||||
|
||||
it('uses orca CLI when devMode is false', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c',
|
||||
devMode: false
|
||||
})
|
||||
|
||||
const result = buildDispatchPreamble(baseParams({ devMode: false }))
|
||||
expect(result).toContain('orca orchestration send')
|
||||
expect(result).toContain('orca orchestration check')
|
||||
})
|
||||
|
||||
it('appends a BASE DRIFT section when baseDrift.behind > 0', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
dispatchId: 'ctx_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c',
|
||||
baseDrift: {
|
||||
base: 'origin/main',
|
||||
behind: 7,
|
||||
recentSubjects: ['fix: A', 'feat: B', 'chore: C']
|
||||
}
|
||||
})
|
||||
|
||||
expect(result).toContain('--- BASE DRIFT ---')
|
||||
expect(result).toContain('7 commits behind origin/main')
|
||||
expect(result).toContain(' - fix: A')
|
||||
expect(result).toContain(' - feat: B')
|
||||
expect(result).toContain(' - chore: C')
|
||||
// drift section must appear before the task spec
|
||||
expect(result.indexOf('--- BASE DRIFT ---')).toBeLessThan(result.indexOf('=== TASK ==='))
|
||||
})
|
||||
|
||||
it('omits the drift section when baseDrift.behind is 0', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
dispatchId: 'ctx_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c',
|
||||
baseDrift: {
|
||||
base: 'origin/main',
|
||||
behind: 0,
|
||||
recentSubjects: []
|
||||
}
|
||||
})
|
||||
|
||||
expect(result).not.toContain('--- BASE DRIFT ---')
|
||||
expect(result).not.toContain('commits behind')
|
||||
})
|
||||
|
||||
it('omits the drift section when baseDrift is undefined', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
dispatchId: 'ctx_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c'
|
||||
})
|
||||
|
||||
expect(result).not.toContain('--- BASE DRIFT ---')
|
||||
expect(result).not.toContain('commits behind')
|
||||
})
|
||||
|
||||
it('lists drift subjects in the order provided, each prefixed with two spaces and dash', () => {
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_x',
|
||||
dispatchId: 'ctx_x',
|
||||
taskSpec: 'do stuff',
|
||||
coordinatorHandle: 'term_c',
|
||||
baseDrift: {
|
||||
base: 'origin/main',
|
||||
behind: 3,
|
||||
recentSubjects: ['first', 'second', 'third']
|
||||
}
|
||||
})
|
||||
|
||||
const firstIdx = result.indexOf(' - first')
|
||||
const secondIdx = result.indexOf(' - second')
|
||||
const thirdIdx = result.indexOf(' - third')
|
||||
expect(firstIdx).toBeGreaterThanOrEqual(0)
|
||||
expect(secondIdx).toBeGreaterThan(firstIdx)
|
||||
expect(thirdIdx).toBeGreaterThan(secondIdx)
|
||||
})
|
||||
|
||||
it('renders a stable snapshot of the full preamble', () => {
|
||||
// Why: single strict snapshot catches any accidental regression in
|
||||
// formatting or rule presence in one line.
|
||||
const result = buildDispatchPreamble({
|
||||
taskId: 'task_SNAP',
|
||||
dispatchId: 'ctx_SNAP',
|
||||
taskSpec: 'TASK_BODY',
|
||||
coordinatorHandle: 'term_COORD'
|
||||
})
|
||||
expect(result).toMatchSnapshot()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,41 +1,147 @@
|
||||
export type PreambleParams = {
|
||||
taskId: string
|
||||
// Why: the heartbeat payload attributes liveness to a specific dispatch
|
||||
// context (not just a task). A retried task has multiple dispatch_contexts
|
||||
// rows; keying heartbeats on dispatchId prevents a straggler heartbeat from
|
||||
// a previously-failed dispatch from masking a hung retry. Both call sites
|
||||
// already have this value in scope (coordinator.ts dispatch.id, rpc
|
||||
// orchestration.ts ctx.id).
|
||||
dispatchId: string
|
||||
taskSpec: string
|
||||
coordinatorHandle: string
|
||||
devMode?: boolean
|
||||
// Why: populated by the coordinator's dispatch pre-flight (§3.1) only
|
||||
// when the target worktree is behind its tracking remote. When absent
|
||||
// or when `behind === 0`, the preamble emits no drift section. Callers
|
||||
// must NOT pre-populate this with empty data; the drift section is a
|
||||
// loud-but-rare signal tied to the `allow-stale-base: true` override
|
||||
// path, and polluting it for fresh worktrees would train workers to
|
||||
// ignore it.
|
||||
baseDrift?: {
|
||||
base: string
|
||||
behind: number
|
||||
recentSubjects: string[]
|
||||
}
|
||||
}
|
||||
|
||||
// Why: 5 minutes is frequent enough that the coordinator's stale-heartbeat
|
||||
// check (threshold 10 min) catches a hung worker within one tick, and
|
||||
// infrequent enough to avoid inbox spam on long tasks. One constant so
|
||||
// cadence tuning is a single-line change (Q1 in DESIGN_DOC_PREAMBLE_FIX.md).
|
||||
const HEARTBEAT_INTERVAL_MIN = 5
|
||||
|
||||
// Why: the dispatch preamble teaches agents about Orca's CLI commands for
|
||||
// structured communication. Agents don't need prior knowledge of Orca — they
|
||||
// treat these as shell tools the same way they use git or npm.
|
||||
// structured communication. Behavioral rules (body summary, heartbeat cadence,
|
||||
// no-AskUserQuestion) live as inline comments above the relevant CLI example,
|
||||
// not as a separate prose block — LLM readers anchor on examples and skim
|
||||
// trailing prose, so rules must land at the point of use.
|
||||
export function buildDispatchPreamble(params: PreambleParams): string {
|
||||
// Why: in dev mode, agents must use orca-dev to connect to the dev runtime's
|
||||
// socket. Without this, agents inside the dev Electron app would call the
|
||||
// production CLI and talk to the wrong Orca instance (Section 6.4).
|
||||
const cli = params.devMode ? 'orca-dev' : 'orca'
|
||||
|
||||
return `You are working inside Orca, a multi-agent IDE. You have access to these
|
||||
CLI commands for communicating with the coordinator:
|
||||
const header = `You are working inside Orca, a multi-agent IDE. You are a dispatched worker.
|
||||
Your coordinator's terminal handle is: ${params.coordinatorHandle}
|
||||
Your task ID is: ${params.taskId}
|
||||
|
||||
# Report task completion (REQUIRED when done):
|
||||
You talk to the coordinator only through the CLI commands below. Do not use
|
||||
Slack, GitHub comments, or any other channel to reach a human during the run.
|
||||
|
||||
=== CLI COMMANDS ===
|
||||
|
||||
# Report task completion (REQUIRED when done — even on failure).
|
||||
#
|
||||
# RULE: --body must be a 3-sentence executive summary (what you did,
|
||||
# what you found, what's left). Never send an empty body; the coordinator
|
||||
# reads the body first and only opens artifacts if it needs more detail.
|
||||
# If you produced a long-form artifact, include its path as
|
||||
# payload.reportPath so the coordinator can find it without a file search.
|
||||
#
|
||||
# RULE: send worker_done exactly once. Failure is still a worker_done
|
||||
# with subject like "Failed: <reason>" — never silently exit.
|
||||
${cli} orchestration send --to ${params.coordinatorHandle} \\
|
||||
--type worker_done --subject "Done" \\
|
||||
--payload '{"taskId":"${params.taskId}","filesModified":[...]}'
|
||||
--type worker_done --subject "<short status>" \\
|
||||
--body "<3-sentence summary: what you did, what you found, what's left>" \\
|
||||
--payload '{"taskId":"${params.taskId}","filesModified":["path/a","path/b"],"reportPath":"<optional: path to the full artifact>"}'
|
||||
|
||||
# Report a blocker or failure:
|
||||
# BEHAVIOR RULE: send a heartbeat every ${HEARTBEAT_INTERVAL_MIN} minutes
|
||||
# while actively working on the task. The coordinator uses this to
|
||||
# distinguish "still thinking" from "hung / crashed." Skip heartbeats only
|
||||
# while blocked inside \`check --wait\` or \`ask\` — those calls are
|
||||
# themselves liveness signals.
|
||||
#
|
||||
# Include BOTH taskId and dispatchId in the payload: the coordinator
|
||||
# attributes the heartbeat to the specific dispatch context, not just
|
||||
# the task, so a straggler heartbeat from a previously-failed dispatch
|
||||
# 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>"}'
|
||||
|
||||
# Ask the coordinator a question and block until it answers.
|
||||
#
|
||||
# BEHAVIOR RULE #1 (MUST NOT VIOLATE):
|
||||
# NEVER use AskUserQuestion; use \`${cli} orchestration ask\` or send
|
||||
# --type decision_gate. AskUserQuestion opens a local TUI prompt that the
|
||||
# coordinator cannot see and cannot answer — your session will hang forever
|
||||
# waiting on a human. Every interactive question goes through \`ask\` below.
|
||||
#
|
||||
# The \`ask\` verb is a thin wrapper: it sends a decision_gate message and
|
||||
# blocks on \`check --wait\` until the coordinator replies, then prints the
|
||||
# reply body. Use it anywhere you would otherwise have reached for
|
||||
# AskUserQuestion.
|
||||
${cli} orchestration ask --to ${params.coordinatorHandle} \\
|
||||
--question "<your question>" \\
|
||||
--options "<optional,comma,separated>" \\
|
||||
--timeout-ms 600000
|
||||
|
||||
# Escalate a blocker or failure (pre-completion, when you need the
|
||||
# coordinator to do something before you can continue):
|
||||
${cli} orchestration send --to ${params.coordinatorHandle} \\
|
||||
--type escalation --subject "Blocked: <reason>" \\
|
||||
--body "<details>"
|
||||
--body "<details>" \\
|
||||
--payload '{"taskId":"${params.taskId}"}'
|
||||
|
||||
# Check for messages from the coordinator or other agents:
|
||||
# Check for messages from the coordinator:
|
||||
${cli} orchestration check
|
||||
|
||||
Your assigned task ID is: ${params.taskId}
|
||||
=== AFTER YOU SEND worker_done ===
|
||||
|
||||
When you finish your task, run the worker_done command above with the
|
||||
list of files you modified. If you are blocked or need help, send an
|
||||
escalation. Do not exit the session.
|
||||
Keep the shell session open for a grace period (10 minutes) in case the
|
||||
coordinator sends a follow-up or re-dispatches you. Poll with
|
||||
\`${cli} orchestration check\` every 2 minutes during that window. If no
|
||||
follow-up arrives, you may exit after the grace period — the coordinator
|
||||
will not expect further output from you.
|
||||
|
||||
--- TASK ---
|
||||
If the coordinator re-dispatches you (you will receive a fresh preamble +
|
||||
TASK block), reset your polling and start the new task. Do not respond
|
||||
to the previous task's follow-ups after a re-dispatch.`
|
||||
|
||||
// Why: the drift section fires only when the coordinator allowed dispatch
|
||||
// against a stale worktree (via `allow-stale-base: true` in the task spec,
|
||||
// see §3.4) OR when behind>0 but under the refusal threshold. Either way
|
||||
// it is defense-in-depth: the worker sees the drift from line 1 instead
|
||||
// of discovering it via stale line numbers in artifacts later.
|
||||
const drift =
|
||||
params.baseDrift && params.baseDrift.behind > 0 ? buildDriftSection(params.baseDrift) : ''
|
||||
|
||||
return `${header}${drift}
|
||||
|
||||
=== TASK ===
|
||||
${params.taskSpec}`
|
||||
}
|
||||
|
||||
function buildDriftSection(drift: NonNullable<PreambleParams['baseDrift']>): string {
|
||||
const subjects = drift.recentSubjects.map((s) => ` - ${s}`).join('\n')
|
||||
return `
|
||||
|
||||
--- BASE DRIFT ---
|
||||
Your worktree HEAD is ${drift.behind} commits behind ${drift.base}. The 5 most recent
|
||||
subjects on ${drift.base} NOT in your worktree:
|
||||
${subjects}
|
||||
|
||||
If any look relevant to your task, either pull them in (\`git pull --rebase
|
||||
${drift.base}\` or equivalent) or escalate to the coordinator before starting.
|
||||
---`
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ export type MessageType =
|
||||
| 'escalation'
|
||||
| 'handoff'
|
||||
| 'decision_gate'
|
||||
| 'heartbeat'
|
||||
|
||||
export type MessagePriority = 'normal' | 'high' | 'urgent'
|
||||
|
||||
@@ -30,6 +31,7 @@ export type MessageRow = {
|
||||
read: number
|
||||
sequence: number
|
||||
created_at: string
|
||||
delivered_at: string | null
|
||||
}
|
||||
|
||||
export type TaskRow = {
|
||||
@@ -53,6 +55,7 @@ export type DispatchContextRow = {
|
||||
dispatched_at: string | null
|
||||
completed_at: string | null
|
||||
created_at: string
|
||||
last_heartbeat_at: string | null
|
||||
}
|
||||
|
||||
export type DecisionGateRow = {
|
||||
|
||||
@@ -38,6 +38,13 @@ export type RpcRequest = {
|
||||
|
||||
export type RpcContext = {
|
||||
runtime: OrcaRuntimeService
|
||||
// Why: long-poll handlers (e.g. orchestration.check with wait=true) need to
|
||||
// observe the underlying socket's lifetime so they can release their slot
|
||||
// and resolve their inner waiters immediately when a client disconnects
|
||||
// instead of running down the configured timeoutMs. Undefined outside the
|
||||
// runtime-rpc transport (direct in-process callers don't need it).
|
||||
// See design doc §3.1 counter-lifecycle.
|
||||
signal?: AbortSignal
|
||||
}
|
||||
|
||||
export type RpcHandler<TParams> = (params: TParams, ctx: RpcContext) => Promise<unknown> | unknown
|
||||
|
||||
@@ -30,7 +30,7 @@ export class RpcDispatcher {
|
||||
this.registry = buildRegistry(methods)
|
||||
}
|
||||
|
||||
async dispatch(request: RpcRequest): Promise<RpcResponse> {
|
||||
async dispatch(request: RpcRequest, options?: { signal?: AbortSignal }): Promise<RpcResponse> {
|
||||
const meta = this.meta()
|
||||
const method = this.registry.get(request.method)
|
||||
if (!method) {
|
||||
@@ -55,7 +55,10 @@ export class RpcDispatcher {
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await method.handler(parsedParams, { runtime: this.runtime })
|
||||
const result = await method.handler(parsedParams, {
|
||||
runtime: this.runtime,
|
||||
signal: options?.signal
|
||||
})
|
||||
return successResponse(request.id, meta, result)
|
||||
} catch (error) {
|
||||
// Why: browser methods throw BrowserError with a structured `code`;
|
||||
|
||||
@@ -38,7 +38,7 @@ describe('orchestration RPC methods', () => {
|
||||
|
||||
it('registers all expected methods', () => {
|
||||
const registry = buildRegistry(ORCHESTRATION_METHODS)
|
||||
expect(registry.size).toBe(15)
|
||||
expect(registry.size).toBe(16)
|
||||
expect(registry.has('orchestration.send')).toBe(true)
|
||||
expect(registry.has('orchestration.check')).toBe(true)
|
||||
expect(registry.has('orchestration.reply')).toBe(true)
|
||||
@@ -48,6 +48,7 @@ describe('orchestration RPC methods', () => {
|
||||
expect(registry.has('orchestration.taskUpdate')).toBe(true)
|
||||
expect(registry.has('orchestration.dispatch')).toBe(true)
|
||||
expect(registry.has('orchestration.dispatchShow')).toBe(true)
|
||||
expect(registry.has('orchestration.ask')).toBe(true)
|
||||
expect(registry.has('orchestration.run')).toBe(true)
|
||||
expect(registry.has('orchestration.runStop')).toBe(true)
|
||||
expect(registry.has('orchestration.gateCreate')).toBe(true)
|
||||
@@ -275,6 +276,79 @@ describe('orchestration RPC methods', () => {
|
||||
})
|
||||
).rejects.toThrow('Invalid --types')
|
||||
})
|
||||
|
||||
it('default (unread only) marks returned rows as read', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'two' })
|
||||
|
||||
const first = (await call('orchestration.check', { terminal: 'b' })) as {
|
||||
count: number
|
||||
}
|
||||
expect(first.count).toBe(2)
|
||||
|
||||
const second = (await call('orchestration.check', { terminal: 'b' })) as {
|
||||
count: number
|
||||
}
|
||||
expect(second.count).toBe(0)
|
||||
})
|
||||
|
||||
it('--all returns every message for the handle without marking read', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
const second = db.insertMessage({ from: 'a', to: 'b', subject: 'two' })
|
||||
db.markAsRead([second.id])
|
||||
|
||||
const result = (await call('orchestration.check', {
|
||||
terminal: 'b',
|
||||
all: true
|
||||
})) as { messages: { read: number }[]; count: number }
|
||||
|
||||
expect(result.count).toBe(2)
|
||||
// Must not have flipped the remaining unread row
|
||||
const stillUnread = db.getUnreadMessages('b')
|
||||
expect(stillUnread).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('--all returns rows with delivered_at set after push-on-idle stamped them', async () => {
|
||||
setup()
|
||||
const msg = db.insertMessage({ from: 'a', to: 'b', subject: 'hi' })
|
||||
// Why: simulate push-on-idle stamping delivered_at without the runtime loop.
|
||||
db.markAsDelivered([msg.id])
|
||||
|
||||
const result = (await call('orchestration.check', {
|
||||
terminal: 'b',
|
||||
all: true
|
||||
})) as { messages: { id: string; delivered_at: string | null }[]; count: number }
|
||||
|
||||
expect(result.count).toBe(1)
|
||||
expect(result.messages[0].delivered_at).not.toBeNull()
|
||||
})
|
||||
|
||||
it('--all --terminal <unknown> returns empty list', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
|
||||
const result = (await call('orchestration.check', {
|
||||
terminal: 'does_not_exist',
|
||||
all: true
|
||||
})) as { count: number }
|
||||
expect(result.count).toBe(0)
|
||||
})
|
||||
|
||||
it('unread:false compat shim behaves like --all (one-release bridge)', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
|
||||
const result = (await call('orchestration.check', {
|
||||
terminal: 'b',
|
||||
unread: false
|
||||
})) as { count: number }
|
||||
expect(result.count).toBe(1)
|
||||
|
||||
// Must not have marked read
|
||||
expect(db.getUnreadMessages('b')).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.reply', () => {
|
||||
@@ -310,6 +384,38 @@ describe('orchestration RPC methods', () => {
|
||||
const result = (await call('orchestration.inbox', {})) as { count: number }
|
||||
expect(result.count).toBe(2)
|
||||
})
|
||||
|
||||
it('--terminal <handle> matches check --all output for the same handle', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'two' })
|
||||
db.insertMessage({ from: 'a', to: 'c', subject: 'other' })
|
||||
|
||||
const inbox = (await call('orchestration.inbox', { terminal: 'b' })) as {
|
||||
messages: { id: string; to_handle: string }[]
|
||||
count: number
|
||||
}
|
||||
const check = (await call('orchestration.check', {
|
||||
terminal: 'b',
|
||||
all: true
|
||||
})) as { messages: { id: string; to_handle: string }[]; count: number }
|
||||
|
||||
expect(inbox.count).toBe(2)
|
||||
expect(check.count).toBe(2)
|
||||
// Same rows in the same order — both use sequence DESC
|
||||
expect(inbox.messages.map((m) => m.id)).toEqual(check.messages.map((m) => m.id))
|
||||
expect(inbox.messages.every((m) => m.to_handle === 'b')).toBe(true)
|
||||
})
|
||||
|
||||
it('--terminal <unknown_handle> returns empty list without erroring', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'a', to: 'b', subject: 'one' })
|
||||
|
||||
const result = (await call('orchestration.inbox', {
|
||||
terminal: 'does_not_exist'
|
||||
})) as { count: number }
|
||||
expect(result.count).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.taskCreate', () => {
|
||||
@@ -369,6 +475,33 @@ describe('orchestration RPC methods', () => {
|
||||
const method = findMethod('orchestration.taskList')
|
||||
expect(() => method.params!.parse({ status: 'done-ish' })).toThrow()
|
||||
})
|
||||
|
||||
it('includes assignee_handle and dispatch_id for dispatched tasks', async () => {
|
||||
setup()
|
||||
const t1 = db.createTask({ spec: 'ready work' })
|
||||
const t2 = db.createTask({ spec: 'active work' })
|
||||
const ctx = db.createDispatchContext(t2.id, 'term_worker')
|
||||
|
||||
const result = (await call('orchestration.taskList', {})) as {
|
||||
tasks: {
|
||||
id: string
|
||||
status: string
|
||||
assignee_handle?: string | null
|
||||
dispatch_id?: string | null
|
||||
}[]
|
||||
}
|
||||
|
||||
const ready = result.tasks.find((t) => t.id === t1.id)
|
||||
const dispatched = result.tasks.find((t) => t.id === t2.id)
|
||||
expect(ready).toBeDefined()
|
||||
expect(dispatched).toBeDefined()
|
||||
// Non-dispatched tasks keep the legacy shape — no assignee/dispatch fields.
|
||||
expect(ready).not.toHaveProperty('assignee_handle')
|
||||
expect(ready).not.toHaveProperty('dispatch_id')
|
||||
// Dispatched tasks surface the active dispatch.
|
||||
expect(dispatched?.assignee_handle).toBe('term_worker')
|
||||
expect(dispatched?.dispatch_id).toBe(ctx.id)
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.taskUpdate', () => {
|
||||
@@ -496,6 +629,50 @@ describe('orchestration RPC methods', () => {
|
||||
/already has an active dispatch/
|
||||
)
|
||||
})
|
||||
|
||||
it('dry-run returns the preamble without mutating state', async () => {
|
||||
setup()
|
||||
const task = db.createTask({ spec: 'work' })
|
||||
|
||||
const result = (await call('orchestration.dispatch', {
|
||||
task: task.id,
|
||||
to: 'term_a',
|
||||
inject: true,
|
||||
dryRun: true,
|
||||
from: 'term_coord'
|
||||
})) as {
|
||||
dispatch: null
|
||||
dryRun: boolean
|
||||
preamble: string
|
||||
injected: boolean
|
||||
}
|
||||
|
||||
expect(result.dryRun).toBe(true)
|
||||
expect(result.dispatch).toBeNull()
|
||||
expect(result.injected).toBe(false)
|
||||
expect(result.preamble).toContain('work')
|
||||
expect(result.preamble).toContain(task.id)
|
||||
expect(result.preamble).toContain('term_coord')
|
||||
// Task state must not change on dry-run.
|
||||
expect(db.getTask(task.id)?.status).toBe('ready')
|
||||
expect(db.getDispatchContext(task.id)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returnPreamble includes preamble in the response', async () => {
|
||||
setup()
|
||||
const task = db.createTask({ spec: 'work' })
|
||||
|
||||
const result = (await call('orchestration.dispatch', {
|
||||
task: task.id,
|
||||
to: 'term_a',
|
||||
returnPreamble: true,
|
||||
from: 'term_coord'
|
||||
})) as { dispatch: { id: string }; preamble: string }
|
||||
|
||||
expect(result.dispatch.id).toMatch(/^ctx_/)
|
||||
expect(result.preamble).toContain(task.id)
|
||||
expect(result.preamble).toContain('term_coord')
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.dispatchShow', () => {
|
||||
@@ -519,6 +696,44 @@ describe('orchestration RPC methods', () => {
|
||||
|
||||
expect(result.dispatch).toBeNull()
|
||||
})
|
||||
|
||||
it('--preamble returns the preamble text', async () => {
|
||||
setup()
|
||||
const task = db.createTask({ spec: 'refactor auth' })
|
||||
db.createDispatchContext(task.id, 'term_a')
|
||||
|
||||
const result = (await call('orchestration.dispatchShow', {
|
||||
task: task.id,
|
||||
preamble: true,
|
||||
from: 'term_coord'
|
||||
})) as { dispatch: { task_id: string } | null; preamble: string }
|
||||
|
||||
expect(result.preamble).toContain('refactor auth')
|
||||
expect(result.preamble).toContain(task.id)
|
||||
expect(result.preamble).toContain('term_coord')
|
||||
expect(result.dispatch?.task_id).toBe(task.id)
|
||||
})
|
||||
|
||||
it('--preamble works when no dispatch exists yet', async () => {
|
||||
setup()
|
||||
const task = db.createTask({ spec: 'build feature' })
|
||||
|
||||
const result = (await call('orchestration.dispatchShow', {
|
||||
task: task.id,
|
||||
preamble: true,
|
||||
from: 'term_coord'
|
||||
})) as { dispatch: null; preamble: string }
|
||||
|
||||
expect(result.dispatch).toBeNull()
|
||||
expect(result.preamble).toContain('build feature')
|
||||
})
|
||||
|
||||
it('--preamble throws for unknown task', async () => {
|
||||
setup()
|
||||
await expect(
|
||||
call('orchestration.dispatchShow', { task: 'task_fake', preamble: true })
|
||||
).rejects.toThrow('Task not found')
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.gateCreate', () => {
|
||||
@@ -621,6 +836,145 @@ describe('orchestration RPC methods', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.ask', () => {
|
||||
it('sends a decision_gate and returns the first thread reply', async () => {
|
||||
setup()
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'waitForMessage').mockImplementation(async () => {
|
||||
// Simulate coordinator replying in the thread during the wait
|
||||
const outbound = db.getInbox(10).find((m) => m.type === 'decision_gate')
|
||||
if (outbound) {
|
||||
db.insertMessage({
|
||||
from: 'term_coord',
|
||||
to: 'term_worker',
|
||||
subject: 'Re: Question',
|
||||
body: 'go ahead',
|
||||
threadId: outbound.id
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
const result = (await call('orchestration.ask', {
|
||||
from: 'term_worker',
|
||||
to: 'term_coord',
|
||||
question: 'proceed?',
|
||||
options: 'yes, no',
|
||||
timeoutMs: 500
|
||||
})) as {
|
||||
answer: string
|
||||
messageId: string
|
||||
threadId: string
|
||||
timedOut: boolean
|
||||
}
|
||||
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.answer).toBe('go ahead')
|
||||
expect(result.messageId).toMatch(/^msg_/)
|
||||
|
||||
// Outbound decision_gate message was persisted with parsed options.
|
||||
const outbound = db.getInbox(10).find((m) => m.type === 'decision_gate')
|
||||
expect(outbound).toBeTruthy()
|
||||
expect(outbound?.subject).toBe('Question')
|
||||
expect(outbound?.body).toBe('proceed?')
|
||||
const payload = JSON.parse(outbound!.payload ?? '{}')
|
||||
expect(payload.question).toBe('proceed?')
|
||||
expect(payload.options).toEqual(['yes', 'no'])
|
||||
})
|
||||
|
||||
it('returns timedOut when no reply arrives in the window', async () => {
|
||||
setup()
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'waitForMessage').mockResolvedValue()
|
||||
|
||||
const result = (await call('orchestration.ask', {
|
||||
from: 'term_worker',
|
||||
to: 'term_coord',
|
||||
question: 'still there?',
|
||||
timeoutMs: 1
|
||||
})) as { answer: string | null; timedOut: boolean; messageId: string | null }
|
||||
|
||||
expect(result.timedOut).toBe(true)
|
||||
expect(result.answer).toBeNull()
|
||||
expect(result.messageId).toBeNull()
|
||||
// Outbound message still persisted (coordinator can still see it).
|
||||
const outbound = db.getInbox(10).find((m) => m.type === 'decision_gate')
|
||||
expect(outbound).toBeTruthy()
|
||||
})
|
||||
|
||||
it('rejects group addresses with a dedicated error (no message persisted)', async () => {
|
||||
setup()
|
||||
await expect(
|
||||
call('orchestration.ask', {
|
||||
from: 'term_worker',
|
||||
to: '@reviewers',
|
||||
question: 'ok?'
|
||||
})
|
||||
).rejects.toThrow(/does not support group addresses/)
|
||||
expect(db.getInbox(10)).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('does not return distractor messages on a different thread', async () => {
|
||||
setup()
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {})
|
||||
let wakeCount = 0
|
||||
vi.spyOn(runtime, 'waitForMessage').mockImplementation(async () => {
|
||||
wakeCount++
|
||||
const outbound = db.getInbox(20).find((m) => m.type === 'decision_gate')
|
||||
if (wakeCount === 1 && outbound) {
|
||||
// First wake: distractor in a DIFFERENT thread — must be ignored.
|
||||
db.insertMessage({
|
||||
from: 'term_coord',
|
||||
to: 'term_worker',
|
||||
subject: 'unrelated',
|
||||
body: 'other',
|
||||
threadId: 'thread_other'
|
||||
})
|
||||
} else if (wakeCount === 2 && outbound) {
|
||||
// Second wake: correct thread reply.
|
||||
db.insertMessage({
|
||||
from: 'term_coord',
|
||||
to: 'term_worker',
|
||||
subject: 'Re: Question',
|
||||
body: 'correct answer',
|
||||
threadId: outbound.id
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
const result = (await call('orchestration.ask', {
|
||||
from: 'term_worker',
|
||||
to: 'term_coord',
|
||||
question: 'filter?',
|
||||
timeoutMs: 2_000
|
||||
})) as { answer: string; timedOut: boolean }
|
||||
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.answer).toBe('correct answer')
|
||||
})
|
||||
|
||||
it('parses options CSV with whitespace and empty entries', async () => {
|
||||
setup()
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {})
|
||||
vi.spyOn(runtime, 'waitForMessage').mockResolvedValue()
|
||||
|
||||
await call('orchestration.ask', {
|
||||
from: 'w',
|
||||
to: 'c',
|
||||
question: 'q',
|
||||
options: 'a, b ,,c',
|
||||
timeoutMs: 1
|
||||
})
|
||||
|
||||
const outbound = db.getInbox(10).find((m) => m.type === 'decision_gate')
|
||||
const payload = JSON.parse(outbound!.payload ?? '{}')
|
||||
expect(payload.options).toEqual(['a', 'b', 'c'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration.reset', () => {
|
||||
it('resets all state', async () => {
|
||||
setup()
|
||||
|
||||
@@ -15,7 +15,8 @@ const MESSAGE_TYPES: MessageType[] = [
|
||||
'merge_ready',
|
||||
'escalation',
|
||||
'handoff',
|
||||
'decision_gate'
|
||||
'decision_gate',
|
||||
'heartbeat'
|
||||
]
|
||||
|
||||
const TASK_STATUSES: TaskStatus[] = [
|
||||
@@ -40,7 +41,8 @@ const SendParams = z.object({
|
||||
'merge_ready',
|
||||
'escalation',
|
||||
'handoff',
|
||||
'decision_gate'
|
||||
'decision_gate',
|
||||
'heartbeat'
|
||||
])
|
||||
.optional(),
|
||||
priority: z.enum(['normal', 'high', 'urgent']).optional(),
|
||||
@@ -52,6 +54,10 @@ const SendParams = z.object({
|
||||
const CheckParams = z.object({
|
||||
terminal: OptionalString,
|
||||
unread: OptionalBoolean,
|
||||
// Why: `all` surfaces every message for the handle and skips mark-read.
|
||||
// Previously the only way to ask for "all" was the hidden RPC trick
|
||||
// `{unread: false}`. See design doc §3.2 / §3.3.
|
||||
all: OptionalBoolean,
|
||||
types: OptionalString,
|
||||
inject: OptionalBoolean,
|
||||
wait: OptionalBoolean,
|
||||
@@ -65,7 +71,11 @@ const ReplyParams = z.object({
|
||||
})
|
||||
|
||||
const InboxParams = z.object({
|
||||
limit: OptionalFiniteNumber
|
||||
limit: OptionalFiniteNumber,
|
||||
// Why: filters the inbox listing to a specific handle so coordinators can
|
||||
// ask "everything for this handle" with either `inbox` or `check --all`
|
||||
// and get agreeing results. See design doc §3.3.
|
||||
terminal: OptionalString
|
||||
})
|
||||
|
||||
const TaskCreateParams = z.object({
|
||||
@@ -99,14 +109,30 @@ const TaskUpdateParams = z.object({
|
||||
|
||||
const DispatchParams = z.object({
|
||||
task: requiredString('Missing --task'),
|
||||
to: requiredString('Missing --to'),
|
||||
// Why: --to is only required for real dispatches. When --dry-run is set the
|
||||
// caller is previewing the preamble and no terminal is targeted, so allow it
|
||||
// to be absent. The handler enforces presence before any side-effecting work.
|
||||
to: OptionalString,
|
||||
from: OptionalString,
|
||||
inject: OptionalBoolean,
|
||||
dryRun: OptionalBoolean,
|
||||
returnPreamble: OptionalBoolean,
|
||||
devMode: OptionalBoolean
|
||||
})
|
||||
|
||||
const DispatchShowParams = z.object({
|
||||
task: OptionalString
|
||||
task: OptionalString,
|
||||
preamble: OptionalBoolean,
|
||||
from: OptionalString,
|
||||
devMode: OptionalBoolean
|
||||
})
|
||||
|
||||
const AskParams = z.object({
|
||||
to: requiredString('Missing --to'),
|
||||
question: requiredString('Missing --question'),
|
||||
options: OptionalString,
|
||||
timeoutMs: OptionalFiniteNumber,
|
||||
from: OptionalString
|
||||
})
|
||||
|
||||
const ResetParams = z.object({
|
||||
@@ -177,7 +203,7 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
defineMethod({
|
||||
name: 'orchestration.check',
|
||||
params: CheckParams,
|
||||
handler: async (params, { runtime }) => {
|
||||
handler: async (params, { runtime, signal }) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
const handle = params.terminal ?? 'unknown'
|
||||
const typeFilter = params.types
|
||||
@@ -191,12 +217,17 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
throw new Error(`Invalid --types: ${invalidTypes.join(',')}`)
|
||||
}
|
||||
|
||||
const showUnread = params.unread !== false
|
||||
// Why: `all` short-circuits to "everything for the handle, no marking."
|
||||
// Explicit `unread: false` is also honored for one release as a compat
|
||||
// shim so in-flight callers don't break (see design doc §5). Otherwise
|
||||
// today's behavior is preserved: default is unread-only + mark-read.
|
||||
const showAll = params.all === true || params.unread === false
|
||||
const showUnread = !showAll
|
||||
|
||||
const readAndReturn = () => {
|
||||
const messages = showUnread
|
||||
? db.getUnreadMessages(handle, typeFilter)
|
||||
: db.getAllMessages(handle)
|
||||
: db.getAllMessagesForHandle(handle)
|
||||
|
||||
if (showUnread && messages.length > 0) {
|
||||
db.markAsRead(messages.map((m) => m.id))
|
||||
@@ -216,10 +247,15 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
}
|
||||
|
||||
// Why: blocking wait lets coordinators replace sleep+poll loops with a
|
||||
// single call that resolves when a message arrives or the timeout expires.
|
||||
// single call that resolves when a message arrives or the timeout
|
||||
// expires. The `signal` plumbed from the RPC transport aborts this
|
||||
// waiter the moment the client socket closes, so a killed client
|
||||
// releases its long-poll slot immediately rather than after the full
|
||||
// timeoutMs. See design doc §3.1 counter-lifecycle.
|
||||
await runtime.waitForMessage(handle, {
|
||||
typeFilter: typeFilter as string[] | undefined,
|
||||
timeoutMs: params.timeoutMs ?? undefined
|
||||
timeoutMs: params.timeoutMs ?? undefined,
|
||||
signal
|
||||
})
|
||||
return readAndReturn()
|
||||
}
|
||||
@@ -255,7 +291,13 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
params: InboxParams,
|
||||
handler: (params, { runtime }) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
const messages = db.getInbox(params.limit)
|
||||
// Why: when `terminal` is provided, mirror `check --all` output for that
|
||||
// handle (same rows in the same sequence order). Stale/unknown handles
|
||||
// return an empty list instead of erroring, matching the "historical
|
||||
// rows survive handle deletion" rule in design doc §3.3.
|
||||
const messages = params.terminal
|
||||
? db.getAllMessagesForHandle(params.terminal, params.limit)
|
||||
: db.getInbox(params.limit)
|
||||
return { messages, count: messages.length }
|
||||
}
|
||||
}),
|
||||
@@ -291,10 +333,21 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
params: TaskListParams,
|
||||
handler: (params, { runtime }) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
const tasks = db.listTasks({
|
||||
// Why: listTasksWithDispatch returns the same rows as listTasks plus
|
||||
// assignee_handle + dispatch_id joined in for tasks that currently have an
|
||||
// active dispatch. Non-dispatched tasks get NULL for those fields, so
|
||||
// consumers reading the legacy shape are unaffected.
|
||||
const joined = db.listTasksWithDispatch({
|
||||
status: params.status as TaskStatus,
|
||||
ready: params.ready
|
||||
})
|
||||
const tasks = joined.map((row) => {
|
||||
const { assignee_handle, dispatch_id, ...base } = row
|
||||
if (base.status === 'dispatched') {
|
||||
return { ...base, assignee_handle, dispatch_id }
|
||||
}
|
||||
return base
|
||||
})
|
||||
return { tasks, count: tasks.length }
|
||||
}
|
||||
}),
|
||||
@@ -321,6 +374,30 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
if (!task) {
|
||||
throw new Error(`Task not found: ${params.task}`)
|
||||
}
|
||||
|
||||
// Why: --inject --dry-run lets a coordinator preview the exact preamble
|
||||
// text that would be injected without mutating task state or touching the
|
||||
// target terminal. Skips the ready-status check so coordinators can inspect
|
||||
// the preamble for already-dispatched or blocked tasks too. No dispatch
|
||||
// context exists yet (that happens after the ready-status check), so
|
||||
// dispatchId is a placeholder — the real injected preamble gets a real
|
||||
// ctx.id below.
|
||||
if (params.dryRun) {
|
||||
const preamble = buildDispatchPreamble({
|
||||
taskId: task.id,
|
||||
dispatchId: 'ctx_dryrun',
|
||||
taskSpec: task.spec,
|
||||
coordinatorHandle: params.from ?? 'coordinator',
|
||||
devMode: params.devMode
|
||||
})
|
||||
return { dispatch: null, injected: false, dryRun: true, preamble }
|
||||
}
|
||||
|
||||
if (!params.to) {
|
||||
throw new Error('Missing --to')
|
||||
}
|
||||
const to = params.to
|
||||
|
||||
if (task.status !== 'ready') {
|
||||
throw new Error(`Task ${params.task} is ${task.status}; only ready tasks can be dispatched`)
|
||||
}
|
||||
@@ -330,28 +407,34 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
// status and foreground process — Claude Code doesn't emit recognized OSC
|
||||
// titles on startup, so title-only detection misses freshly spawned agents.
|
||||
if (params.inject) {
|
||||
const hasAgent = await runtime.isTerminalRunningAgent(params.to)
|
||||
const hasAgent = await runtime.isTerminalRunningAgent(to)
|
||||
if (!hasAgent) {
|
||||
throw new Error(
|
||||
`Cannot dispatch --inject to terminal ${params.to}: no recognized agent detected. ` +
|
||||
`Cannot dispatch --inject to terminal ${to}: no recognized agent detected. ` +
|
||||
'Start an agent CLI (e.g. claude, codex, gemini) in the terminal first, ' +
|
||||
'or dispatch without --inject and send the prompt manually.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
const ctx = db.createDispatchContext(params.task, params.to)
|
||||
const ctx = db.createDispatchContext(params.task, to)
|
||||
|
||||
// Why: preamble is built here (not before ctx) so `dispatchId` can be
|
||||
// the real ctx.id — the preamble-hardening PR made dispatchId required
|
||||
// so heartbeats can attribute liveness to a specific dispatch context,
|
||||
// not just a task.
|
||||
const preamble = buildDispatchPreamble({
|
||||
taskId: task.id,
|
||||
dispatchId: ctx.id,
|
||||
taskSpec: task.spec,
|
||||
coordinatorHandle: params.from ?? 'coordinator',
|
||||
devMode: params.devMode
|
||||
})
|
||||
|
||||
let injected = false
|
||||
if (params.inject) {
|
||||
try {
|
||||
const preamble = buildDispatchPreamble({
|
||||
taskId: task.id,
|
||||
taskSpec: task.spec,
|
||||
coordinatorHandle: params.from ?? 'coordinator',
|
||||
devMode: params.devMode
|
||||
})
|
||||
await runtime.sendTerminal(params.to, { text: preamble, enter: true })
|
||||
await runtime.sendTerminal(to, { text: preamble, enter: true })
|
||||
injected = true
|
||||
} catch (err) {
|
||||
db.failDispatch(ctx.id, err instanceof Error ? err.message : String(err))
|
||||
@@ -359,6 +442,12 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
}
|
||||
}
|
||||
|
||||
// Why: returnPreamble is opt-in because the preamble is several hundred
|
||||
// bytes and most callers don't need it in the response. Exposing it
|
||||
// supports coordinators that want to log what was injected for auditing.
|
||||
if (params.returnPreamble) {
|
||||
return { dispatch: ctx, injected, preamble }
|
||||
}
|
||||
return { dispatch: ctx, injected }
|
||||
}
|
||||
}),
|
||||
@@ -372,10 +461,100 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
throw new Error('Missing --task')
|
||||
}
|
||||
const ctx = db.getDispatchContext(params.task)
|
||||
|
||||
// Why: --preamble lets callers inspect the exact preamble text that was
|
||||
// (or would be) injected for this task. The preamble is derived from the
|
||||
// current task spec, so even after dispatch completes the text can be
|
||||
// regenerated deterministically.
|
||||
if (params.preamble) {
|
||||
const task = db.getTask(params.task)
|
||||
if (!task) {
|
||||
throw new Error(`Task not found: ${params.task}`)
|
||||
}
|
||||
const preamble = buildDispatchPreamble({
|
||||
taskId: task.id,
|
||||
// Why: prefer the existing dispatch context's id if we have one
|
||||
// (so the preview matches what was actually injected); fall back
|
||||
// to a placeholder when no dispatch has occurred yet.
|
||||
dispatchId: ctx?.id ?? 'ctx_preview',
|
||||
taskSpec: task.spec,
|
||||
coordinatorHandle: params.from ?? 'coordinator',
|
||||
devMode: params.devMode
|
||||
})
|
||||
return { dispatch: ctx ?? null, preamble }
|
||||
}
|
||||
|
||||
return { dispatch: ctx ?? null }
|
||||
}
|
||||
}),
|
||||
|
||||
defineMethod({
|
||||
name: 'orchestration.ask',
|
||||
params: AskParams,
|
||||
handler: async (params, { runtime }) => {
|
||||
// Why: group addresses have no unambiguous answer semantics (whose
|
||||
// reply wins? first? consensus?) and the ~60-LOC scope is not the
|
||||
// place to design that. Rejecting here closes the silent-timeout
|
||||
// footgun where a worker passing `--to @reviewers` would have the
|
||||
// decision_gate inserted against a literal string no one subscribes
|
||||
// to. Workers that need fan-out fall back to `send --type decision_gate`.
|
||||
if (isGroupAddress(params.to)) {
|
||||
throw new Error(
|
||||
'ask does not support group addresses; use send --type decision_gate for fan-out questions'
|
||||
)
|
||||
}
|
||||
|
||||
const db = runtime.getOrchestrationDb()
|
||||
const from = params.from ?? 'unknown'
|
||||
const timeoutMs = params.timeoutMs ?? 600_000
|
||||
const options =
|
||||
params.options
|
||||
?.split(',')
|
||||
.map((s) => s.trim())
|
||||
.filter(Boolean) ?? []
|
||||
|
||||
const payload = JSON.stringify({ question: params.question, options })
|
||||
const outbound = db.insertMessage({
|
||||
from,
|
||||
to: params.to,
|
||||
subject: 'Question',
|
||||
body: params.question,
|
||||
type: 'decision_gate',
|
||||
payload
|
||||
})
|
||||
runtime.deliverPendingMessagesForHandle(params.to)
|
||||
runtime.notifyMessageArrived(params.to)
|
||||
|
||||
const threadId = outbound.id
|
||||
const deadline = Date.now() + timeoutMs
|
||||
const afterSequence = outbound.sequence
|
||||
|
||||
// Why: loop with a remaining-budget guard so an unrelated distractor
|
||||
// message that wakes waitForMessage does not cause indefinite iteration.
|
||||
// waitForMessage is handle-scoped, so we re-query by thread on every
|
||||
// wake-up to separate "reply in my thread arrived" from "something
|
||||
// else was delivered to this handle."
|
||||
while (true) {
|
||||
const replies = db.getThreadMessagesFor(threadId, from, afterSequence)
|
||||
if (replies.length > 0) {
|
||||
const reply = replies[0]
|
||||
db.markAsRead([reply.id])
|
||||
return {
|
||||
answer: reply.body,
|
||||
messageId: reply.id,
|
||||
threadId,
|
||||
timedOut: false
|
||||
}
|
||||
}
|
||||
const remainingMs = deadline - Date.now()
|
||||
if (remainingMs <= 0) {
|
||||
return { answer: null, messageId: null, threadId, timedOut: true }
|
||||
}
|
||||
await runtime.waitForMessage(from, { timeoutMs: remainingMs })
|
||||
}
|
||||
}
|
||||
}),
|
||||
|
||||
...ORCHESTRATION_GATE_METHODS,
|
||||
|
||||
defineMethod({
|
||||
|
||||
@@ -2,9 +2,11 @@
|
||||
import { existsSync, mkdtempSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { createConnection } from 'net'
|
||||
import { createConnection, type Socket } from 'net'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import Database from 'better-sqlite3'
|
||||
import { OrcaRuntimeService } from './orca-runtime'
|
||||
import { OrchestrationDb } from './orchestration/db'
|
||||
import * as runtimeMetadataModule from './runtime-metadata'
|
||||
import { readRuntimeMetadata } from './runtime-metadata'
|
||||
import { createRuntimeTransportMetadata, OrcaRuntimeRpcServer } from './runtime-rpc'
|
||||
@@ -46,6 +48,62 @@ async function sendRequest(
|
||||
})
|
||||
}
|
||||
|
||||
// Why: long-poll keepalive tests need every frame, not just the first, because
|
||||
// we need to count `_keepalive` frames before the terminal success/failure.
|
||||
// Also exposes the socket so tests can close it mid-wait to exercise the
|
||||
// long-poll counter decrement path.
|
||||
type FramedSession = {
|
||||
socket: Socket
|
||||
frames: Record<string, unknown>[]
|
||||
done: Promise<void>
|
||||
}
|
||||
|
||||
function openFramedSession(endpoint: string, request: Record<string, unknown>): FramedSession {
|
||||
const frames: Record<string, unknown>[] = []
|
||||
const socket = createConnection(endpoint)
|
||||
let buffer = ''
|
||||
socket.setEncoding('utf8')
|
||||
const done = new Promise<void>((resolve, reject) => {
|
||||
socket.once('error', (err) => {
|
||||
// Why: ECONNRESET is expected when we deliberately destroy the socket
|
||||
// mid-wait to probe the counter decrement; surface other errors.
|
||||
if ((err as NodeJS.ErrnoException).code === 'ECONNRESET') {
|
||||
resolve()
|
||||
return
|
||||
}
|
||||
reject(err)
|
||||
})
|
||||
socket.on('close', () => resolve())
|
||||
socket.on('data', (chunk: string) => {
|
||||
buffer += chunk
|
||||
let newlineIndex = buffer.indexOf('\n')
|
||||
while (newlineIndex !== -1) {
|
||||
const raw = buffer.slice(0, newlineIndex).trim()
|
||||
buffer = buffer.slice(newlineIndex + 1)
|
||||
if (raw) {
|
||||
const frame = JSON.parse(raw) as Record<string, unknown>
|
||||
frames.push(frame)
|
||||
// Why: the server leaves the socket open after writing the terminal
|
||||
// frame (short RPCs expect the client to close); close the client
|
||||
// side so `done` resolves once we've captured the response.
|
||||
if (frame._keepalive !== true) {
|
||||
socket.end()
|
||||
}
|
||||
}
|
||||
newlineIndex = buffer.indexOf('\n')
|
||||
}
|
||||
})
|
||||
socket.on('connect', () => {
|
||||
socket.write(`${JSON.stringify(request)}\n`)
|
||||
})
|
||||
})
|
||||
return { socket, frames, done }
|
||||
}
|
||||
|
||||
function sleep(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms))
|
||||
}
|
||||
|
||||
describe('OrcaRuntimeRpcServer', () => {
|
||||
const makeStore = (overrides?: { isUnread?: boolean }) => ({
|
||||
getRepo: (id: string) =>
|
||||
@@ -502,4 +560,241 @@ describe('OrcaRuntimeRpcServer', () => {
|
||||
|
||||
await server.stop()
|
||||
})
|
||||
|
||||
// Why: §6 tests for the transport keepalive + long-poll counter path in §3.1.
|
||||
// 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 keepalive frames while a check --wait handler blocks', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-'))
|
||||
const runtime = new OrcaRuntimeService()
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
runtime.setOrchestrationDb(db)
|
||||
// Why: 50ms keepalive lets us collect ≥3 frames within a 300ms wait
|
||||
// window without slowing the suite.
|
||||
const server = new OrcaRuntimeRpcServer({
|
||||
runtime,
|
||||
userDataPath,
|
||||
keepaliveIntervalMs: 50
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
const metadata = readRuntimeMetadata(userDataPath)
|
||||
const session = openFramedSession(metadata!.transport!.endpoint, {
|
||||
id: 'req_wait',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: {
|
||||
terminal: 'term_nobody',
|
||||
wait: true,
|
||||
timeoutMs: 300
|
||||
}
|
||||
})
|
||||
await session.done
|
||||
|
||||
const keepalives = session.frames.filter((f) => f._keepalive === true)
|
||||
const terminals = session.frames.filter((f) => f.ok !== undefined)
|
||||
expect(terminals).toHaveLength(1)
|
||||
expect(terminals[0]).toMatchObject({ id: 'req_wait', ok: true })
|
||||
// Why: 300ms wait with 50ms keepalive → expect roughly 5 keepalives;
|
||||
// assert ≥3 to tolerate scheduler jitter without flaking.
|
||||
expect(keepalives.length).toBeGreaterThanOrEqual(3)
|
||||
} finally {
|
||||
db.close()
|
||||
await server.stop()
|
||||
}
|
||||
})
|
||||
|
||||
it('releases long-poll slot when client closes mid-wait', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-'))
|
||||
const runtime = new OrcaRuntimeService()
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
runtime.setOrchestrationDb(db)
|
||||
const server = new OrcaRuntimeRpcServer({
|
||||
runtime,
|
||||
userDataPath,
|
||||
keepaliveIntervalMs: 1000,
|
||||
longPollCap: 2
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
const metadata = readRuntimeMetadata(userDataPath)
|
||||
const endpoint = metadata!.transport!.endpoint
|
||||
|
||||
// Fill the cap with two long waits (10s each — we'll kill them).
|
||||
const a = openFramedSession(endpoint, {
|
||||
id: 'req_a',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: { terminal: 'term_a', wait: true, timeoutMs: 10_000 }
|
||||
})
|
||||
const b = openFramedSession(endpoint, {
|
||||
id: 'req_b',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: { terminal: 'term_b', wait: true, timeoutMs: 10_000 }
|
||||
})
|
||||
// Let the two waits land in the handler and increment the counter.
|
||||
await sleep(100)
|
||||
expect(server['activeLongPolls']).toBe(2)
|
||||
|
||||
// Kill one client mid-wait; counter must drop to 1.
|
||||
a.socket.destroy()
|
||||
await a.done
|
||||
// Give Node one tick to fire the close event on the server socket.
|
||||
await sleep(50)
|
||||
expect(server['activeLongPolls']).toBe(1)
|
||||
|
||||
// The freed slot must admit a new long-poll immediately.
|
||||
const c = openFramedSession(endpoint, {
|
||||
id: 'req_c',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: { terminal: 'term_c', wait: true, timeoutMs: 100 }
|
||||
})
|
||||
await c.done
|
||||
const cTerminal = c.frames.find((f) => f.ok !== undefined)
|
||||
expect(cTerminal).toMatchObject({ ok: true, id: 'req_c' })
|
||||
|
||||
b.socket.destroy()
|
||||
await b.done
|
||||
} finally {
|
||||
db.close()
|
||||
await server.stop()
|
||||
}
|
||||
})
|
||||
|
||||
it('responds runtime_busy once the long-poll cap is saturated', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-'))
|
||||
const runtime = new OrcaRuntimeService()
|
||||
const db = new OrchestrationDb(':memory:')
|
||||
runtime.setOrchestrationDb(db)
|
||||
const server = new OrcaRuntimeRpcServer({
|
||||
runtime,
|
||||
userDataPath,
|
||||
keepaliveIntervalMs: 1000,
|
||||
longPollCap: 1
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
const metadata = readRuntimeMetadata(userDataPath)
|
||||
const endpoint = metadata!.transport!.endpoint
|
||||
|
||||
const a = openFramedSession(endpoint, {
|
||||
id: 'req_a',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: { terminal: 'term_a', wait: true, timeoutMs: 5_000 }
|
||||
})
|
||||
await sleep(100)
|
||||
expect(server['activeLongPolls']).toBe(1)
|
||||
|
||||
// Second long-poll overflows the cap → runtime_busy.
|
||||
const overflow = await sendRequest(endpoint, {
|
||||
id: 'req_overflow',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'orchestration.check',
|
||||
params: { terminal: 'term_b', wait: true, timeoutMs: 5_000 }
|
||||
})
|
||||
expect(overflow).toMatchObject({
|
||||
id: 'req_overflow',
|
||||
ok: false,
|
||||
error: { code: 'runtime_busy' }
|
||||
})
|
||||
// The failing request must not have counted against the cap.
|
||||
expect(server['activeLongPolls']).toBe(1)
|
||||
|
||||
// Short RPCs still succeed even when the long-poll cap is full.
|
||||
const short = await sendRequest(endpoint, {
|
||||
id: 'req_short',
|
||||
authToken: metadata!.authToken,
|
||||
method: 'status.get'
|
||||
})
|
||||
expect(short).toMatchObject({ id: 'req_short', ok: true })
|
||||
|
||||
a.socket.destroy()
|
||||
await a.done
|
||||
} finally {
|
||||
db.close()
|
||||
await server.stop()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
// Why: §6 test for the idempotent + hard-fail schema migration. A broken
|
||||
// migration must crash startup loudly rather than serve traffic against a
|
||||
// schema missing the delivered_at column.
|
||||
describe('orchestration DB migration (§3.2)', () => {
|
||||
it('is idempotent when delivered_at already exists', () => {
|
||||
// First open creates the column; second open should be a no-op.
|
||||
const db1 = new OrchestrationDb(':memory:')
|
||||
db1.close()
|
||||
// File path reuse is meaningless with :memory:, so use a tmp file.
|
||||
const tmpPath = join(mkdtempSync(join(tmpdir(), 'orca-orch-mig-')), 'orch.sqlite')
|
||||
const a = new OrchestrationDb(tmpPath)
|
||||
a.close()
|
||||
// Second construction must not throw "duplicate column name".
|
||||
expect(() => {
|
||||
const b = new OrchestrationDb(tmpPath)
|
||||
b.close()
|
||||
}).not.toThrow()
|
||||
})
|
||||
|
||||
it('hard-fails startup when the migration cannot be applied', () => {
|
||||
// Simulate a migration error by monkey-patching better-sqlite3's exec.
|
||||
// If ALTER TABLE throws for any reason (e.g. disk full, permissions),
|
||||
// the constructor must propagate — not swallow and serve half-broken.
|
||||
//
|
||||
// Why the pre-seeded v2 DB: after the schema bundle, fresh DBs are
|
||||
// initialized directly at v3 via createTables() (which already includes
|
||||
// `delivered_at`), so the v2 → v3 ALTER is a no-op for new installs.
|
||||
// To exercise the hard-fail path we need a DB that actually has work
|
||||
// to migrate — a v2-shape file without the delivered_at column — so
|
||||
// the guarded ALTER runs and the stub can fire.
|
||||
const tmpPath = join(mkdtempSync(join(tmpdir(), 'orca-orch-mig-')), 'orch.sqlite')
|
||||
const seed = new Database(tmpPath)
|
||||
seed.exec(`
|
||||
CREATE TABLE messages (
|
||||
id TEXT NOT NULL,
|
||||
from_handle TEXT NOT NULL,
|
||||
to_handle TEXT NOT NULL,
|
||||
subject TEXT NOT NULL,
|
||||
body TEXT NOT NULL DEFAULT '',
|
||||
type TEXT NOT NULL DEFAULT 'status'
|
||||
CHECK(type IN (
|
||||
'status', 'dispatch', 'worker_done', 'merge_ready',
|
||||
'escalation', 'handoff', 'decision_gate', 'heartbeat'
|
||||
)),
|
||||
priority TEXT NOT NULL DEFAULT 'normal'
|
||||
CHECK(priority IN ('normal', 'high', 'urgent')),
|
||||
thread_id TEXT,
|
||||
payload TEXT,
|
||||
read INTEGER NOT NULL DEFAULT 0,
|
||||
sequence INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
`)
|
||||
seed.pragma('user_version = 2')
|
||||
seed.close()
|
||||
|
||||
const realPrototype = Database.prototype as unknown as {
|
||||
exec: (sql: string) => unknown
|
||||
}
|
||||
const originalExec = realPrototype.exec
|
||||
realPrototype.exec = function (sql: string) {
|
||||
if (sql.includes('ALTER TABLE messages ADD COLUMN delivered_at')) {
|
||||
throw new Error('simulated migration failure')
|
||||
}
|
||||
return originalExec.call(this, sql)
|
||||
}
|
||||
try {
|
||||
expect(() => new OrchestrationDb(tmpPath)).toThrow('simulated migration failure')
|
||||
} finally {
|
||||
realPrototype.exec = originalExec
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
+151
-11
@@ -1,3 +1,4 @@
|
||||
/* eslint-disable max-lines -- Why: this file is the single security boundary for the bundled CLI — transport setup, auth-token enforcement, admission control, keepalive framing, and orphan-socket sweeping all co-locate deliberately so a reviewer can audit the boundary in one sitting. Splitting this across files would scatter the invariants without reducing complexity. */
|
||||
// Why: this is the single security boundary for the bundled CLI. It owns
|
||||
// transport setup (unix socket / named pipe), auth-token enforcement, and
|
||||
// bootstrap-metadata publication so a running runtime is always discoverable
|
||||
@@ -19,12 +20,49 @@ type OrcaRuntimeRpcServerOptions = {
|
||||
userDataPath: string
|
||||
pid?: number
|
||||
platform?: NodeJS.Platform
|
||||
// Why: test-only overrides for the two time-bound constants below.
|
||||
// Production callers must not pass these — defaults are set by the design
|
||||
// doc (§3.1) and changing them in production would weaken the admission
|
||||
// fence or flood the socket with keepalive frames.
|
||||
keepaliveIntervalMs?: number
|
||||
longPollCap?: number
|
||||
}
|
||||
|
||||
const MAX_RUNTIME_RPC_MESSAGE_BYTES = 1024 * 1024
|
||||
const RUNTIME_RPC_SOCKET_IDLE_TIMEOUT_MS = 30_000
|
||||
const MAX_RUNTIME_RPC_CONNECTIONS = 32
|
||||
|
||||
// Why: after 10 s of a pending dispatch we emit a tiny `{"_keepalive":true}`
|
||||
// frame every 10 s until the handler resolves. Each write resets both the
|
||||
// server's own socket idle timer (30 s) and — once §3.1 ships on the client —
|
||||
// the client's idle timer, because any byte counts as socket activity. This
|
||||
// is the transport-layer fix for feedback #1: long-poll RPCs (i.e.
|
||||
// orchestration.check --wait) can now run past the 30 s/60 s idle caps
|
||||
// without either end tearing the socket down. See design doc §3.1.
|
||||
const KEEPALIVE_INTERVAL_MS = 10_000
|
||||
|
||||
// Why: long-poll slot cap. With keepalives a `check --wait --timeout-ms
|
||||
// 600000` can hold a connection for up to 10 minutes; unbounded that would
|
||||
// saturate MAX_RUNTIME_RPC_CONNECTIONS (32) with 32 waiting coordinators
|
||||
// and lock out normal short RPCs. Capping at half the connection budget
|
||||
// leaves the other half for short traffic. On overflow the server responds
|
||||
// immediately with `runtime_busy` (CLI exit 75) — fail fast, not silent
|
||||
// queuing. See design doc §3.1 + §7 risk #2.
|
||||
const LONG_POLL_CAP = 16
|
||||
|
||||
// Why: a long-poll request is one whose handler blocks for an unbounded
|
||||
// amount of time waiting for an external event (today, only
|
||||
// `orchestration.check` with `wait === true`). This function is the single
|
||||
// place that classifies it — the long-poll counter, abort wiring, and
|
||||
// runtime_busy admission check all share this decision. See §3.1.
|
||||
function isLongPollRequest(request: RpcRequest): boolean {
|
||||
if (request.method !== 'orchestration.check') {
|
||||
return false
|
||||
}
|
||||
const params = request.params as { wait?: unknown } | undefined
|
||||
return params?.wait === true
|
||||
}
|
||||
|
||||
export class OrcaRuntimeRpcServer {
|
||||
private readonly runtime: OrcaRuntimeService
|
||||
private readonly dispatcher: RpcDispatcher
|
||||
@@ -32,20 +70,30 @@ export class OrcaRuntimeRpcServer {
|
||||
private readonly pid: number
|
||||
private readonly platform: NodeJS.Platform
|
||||
private readonly authToken = randomBytes(24).toString('hex')
|
||||
private readonly keepaliveIntervalMs: number
|
||||
private readonly longPollCap: number
|
||||
private server: Server | null = null
|
||||
private transport: RuntimeTransportMetadata | null = null
|
||||
// Why: separate from Node's server.maxConnections because we need to count
|
||||
// only long-running dispatches, not every in-flight short RPC. See §3.1 +
|
||||
// §7 risk #2.
|
||||
private activeLongPolls = 0
|
||||
|
||||
constructor({
|
||||
runtime,
|
||||
userDataPath,
|
||||
pid = process.pid,
|
||||
platform = process.platform
|
||||
platform = process.platform,
|
||||
keepaliveIntervalMs = KEEPALIVE_INTERVAL_MS,
|
||||
longPollCap = LONG_POLL_CAP
|
||||
}: OrcaRuntimeRpcServerOptions) {
|
||||
this.runtime = runtime
|
||||
this.dispatcher = new RpcDispatcher({ runtime })
|
||||
this.userDataPath = userDataPath
|
||||
this.pid = pid
|
||||
this.platform = platform
|
||||
this.keepaliveIntervalMs = keepaliveIntervalMs
|
||||
this.longPollCap = longPollCap
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
@@ -174,37 +222,129 @@ export class OrcaRuntimeRpcServer {
|
||||
const rawMessage = buffer.slice(0, newlineIndex).trim()
|
||||
buffer = buffer.slice(newlineIndex + 1)
|
||||
if (rawMessage) {
|
||||
void this.handleMessage(rawMessage).then((response) => {
|
||||
socket.write(`${JSON.stringify(response)}\n`)
|
||||
})
|
||||
void this.handleRequest(socket, rawMessage)
|
||||
}
|
||||
newlineIndex = buffer.indexOf('\n')
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
private async handleMessage(rawMessage: string): Promise<RpcResponse> {
|
||||
// Why: a single entry point per inbound request so keepalive + long-poll
|
||||
// admission + AbortController wiring + response write all live in one
|
||||
// place. See design doc §3.1.
|
||||
private async handleRequest(socket: Socket, rawMessage: string): Promise<void> {
|
||||
const parsed = this.parseAndAuth(rawMessage)
|
||||
if ('error' in parsed) {
|
||||
this.safeWrite(socket, `${JSON.stringify(parsed.error)}\n`)
|
||||
return
|
||||
}
|
||||
const request = parsed.request
|
||||
|
||||
// Why: long-poll admission fence. Short RPCs bypass the counter entirely
|
||||
// — it only guards handlers that can block for minutes. See §7 risk #2.
|
||||
const longPoll = isLongPollRequest(request)
|
||||
if (longPoll) {
|
||||
if (this.activeLongPolls >= this.longPollCap) {
|
||||
const busy = this.buildError(
|
||||
request.id,
|
||||
'runtime_busy',
|
||||
'long-poll capacity reached; retry with backoff'
|
||||
)
|
||||
this.safeWrite(socket, `${JSON.stringify(busy)}\n`)
|
||||
socket.end()
|
||||
return
|
||||
}
|
||||
this.activeLongPolls += 1
|
||||
}
|
||||
|
||||
// Why: `decremented` must guard against double-decrement when both
|
||||
// `close` and a post-resolve cleanup path fire. `socket.on('close')` is
|
||||
// the only path that fires for every termination (normal end, destroy,
|
||||
// idle timer, client kill -9, OS reset), so it carries the decrement.
|
||||
// Tying it to `.finally` alone would leak a slot any time a client dies
|
||||
// mid-wait because the inner waitForMessage can keep counting down for
|
||||
// minutes after the socket is gone. See §3.1 counter-lifecycle.
|
||||
let decremented = !longPoll
|
||||
const abortController = new AbortController()
|
||||
const onClose = (): void => {
|
||||
if (!decremented) {
|
||||
decremented = true
|
||||
this.activeLongPolls = Math.max(0, this.activeLongPolls - 1)
|
||||
}
|
||||
abortController.abort()
|
||||
}
|
||||
socket.on('close', onClose)
|
||||
|
||||
// Why: for long-poll requests we start a keepalive ticker after 10 s. The
|
||||
// first frame at 10 s resets both the server's 30 s idle timer and the
|
||||
// client's configured timeout. Short RPCs never see a keepalive — the
|
||||
// ticker never fires because the handler resolves first.
|
||||
let keepaliveTimer: NodeJS.Timeout | null = null
|
||||
if (longPoll) {
|
||||
keepaliveTimer = setInterval(() => {
|
||||
if (socket.writable && !socket.destroyed) {
|
||||
socket.write('{"_keepalive":true}\n')
|
||||
}
|
||||
}, this.keepaliveIntervalMs)
|
||||
// Why: don't hold the process open solely on the keepalive interval —
|
||||
// .unref() lets the event loop exit when nothing else is pending.
|
||||
if (typeof keepaliveTimer.unref === 'function') {
|
||||
keepaliveTimer.unref()
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const response = await this.dispatcher.dispatch(request, {
|
||||
signal: longPoll ? abortController.signal : undefined
|
||||
})
|
||||
if (!socket.destroyed) {
|
||||
this.safeWrite(socket, `${JSON.stringify(response)}\n`)
|
||||
}
|
||||
} finally {
|
||||
if (keepaliveTimer) {
|
||||
clearInterval(keepaliveTimer)
|
||||
}
|
||||
// Why: the close-handler path is still what decrements the counter (the
|
||||
// socket may be closed by the client before the response write flushes).
|
||||
// We don't remove the listener here — `once` semantics are handled by
|
||||
// the boolean guard.
|
||||
}
|
||||
}
|
||||
|
||||
private parseAndAuth(rawMessage: string): { request: RpcRequest } | { error: RpcResponse } {
|
||||
let request: RpcRequest
|
||||
try {
|
||||
request = JSON.parse(rawMessage) as RpcRequest
|
||||
} catch {
|
||||
return this.buildError('unknown', 'bad_request', 'Invalid JSON request')
|
||||
return { error: this.buildError('unknown', 'bad_request', 'Invalid JSON request') }
|
||||
}
|
||||
|
||||
if (typeof request.id !== 'string' || request.id.length === 0) {
|
||||
return this.buildError('unknown', 'bad_request', 'Missing request id')
|
||||
return { error: this.buildError('unknown', 'bad_request', 'Missing request id') }
|
||||
}
|
||||
if (typeof request.method !== 'string' || request.method.length === 0) {
|
||||
return this.buildError(request.id, 'bad_request', 'Missing RPC method')
|
||||
return { error: this.buildError(request.id, 'bad_request', 'Missing RPC method') }
|
||||
}
|
||||
if (typeof request.authToken !== 'string' || request.authToken.length === 0) {
|
||||
return this.buildError(request.id, 'unauthorized', 'Missing auth token')
|
||||
return { error: this.buildError(request.id, 'unauthorized', 'Missing auth token') }
|
||||
}
|
||||
if (request.authToken !== this.authToken) {
|
||||
return this.buildError(request.id, 'unauthorized', 'Invalid auth token')
|
||||
return { error: this.buildError(request.id, 'unauthorized', 'Invalid auth token') }
|
||||
}
|
||||
|
||||
return this.dispatcher.dispatch(request)
|
||||
return { request }
|
||||
}
|
||||
|
||||
private safeWrite(socket: Socket, payload: string): void {
|
||||
if (socket.destroyed || !socket.writable) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
socket.write(payload)
|
||||
} catch {
|
||||
// Socket was closed in between the writable check and the write —
|
||||
// nothing we can do; the client already disconnected.
|
||||
}
|
||||
}
|
||||
|
||||
private buildError(id: string, code: string, message: string): RpcResponse {
|
||||
|
||||
Reference in New Issue
Block a user