From 98ad3839a377ff2dbd45476e8d736e088cc9bb59 Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Fri, 16 Oct 2026 12:35:17 +0300 Subject: [PATCH 01/18] orchestrate: kill-tree descends into handed-off subplanner branches like crawl crawlBranch followed subplanners whose status is running or handed-off; collectRunningAgentsInTree (kill-tree) followed only running ones, so a verifier or worker still running under a handed-off subplanner was listed by crawl but reported as "nothing to kill". Both walkers now use one predicate, descendsIntoSubplanner, so they cannot drift again. Test builds a two-branch repo with a handed-off subplanner and a running child and asserts crawl and kill-tree both see it. Fixes #641. --- .../__tests__/kill-tree-handed-off.test.ts | 93 +++++++++++++++++++ .../skills/orchestrate/scripts/cli/util.ts | 21 ++++- 2 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts new file mode 100644 index 000000000..d7f000064 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + collectRunningAgentsInTree, + crawlBranch, + descendsIntoSubplanner, +} from "../cli/util.ts"; + +function git(repo: string, ...args: string[]): string { + return execFileSync("git", ["-C", repo, ...args], { + stdio: ["ignore", "pipe", "pipe"], + }).toString(); +} + +/** + * Builds a repo where `origin/:.orchestrate//state.json` exists + * for a root planner and one subplanner. The root marks the subplanner + * `handed-off`; the subplanner's own branch still has a `running` child. + */ +function fixtureRepo(): { repoPath: string; rootBranch: string; rootSlug: string } { + const repoPath = mkdtempSync(join(tmpdir(), "orch-kill-tree-")); + git(repoPath, "init", "-q", "-b", "main"); + git(repoPath, "config", "user.email", "t@example.com"); + git(repoPath, "config", "user.name", "t"); + + const rootSlug = "ship-refactor"; + // The root planner lives on its own branch; subplanners go under orch//. + const rootBranch = "planner/ship-refactor"; + const childName = "child-planner"; + + const commitState = (branch: string, slug: string, state: unknown) => { + git(repoPath, "checkout", "-q", "--orphan", branch); + const dir = join(repoPath, ".orchestrate", slug); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "state.json"), JSON.stringify(state, null, 2)); + git(repoPath, "add", "-A"); + git(repoPath, "commit", "-q", "-m", `state for ${slug}`); + // The walkers read `origin/`; point that ref at the local commit. + git(repoPath, "update-ref", `refs/remotes/origin/${branch}`, "HEAD"); + }; + + commitState(rootBranch, rootSlug, { + rootSlug, + tasks: [ + { + name: childName, + type: "subplanner", + status: "handed-off", + agentId: "agent-child", + runId: "run-child", + }, + ], + }); + commitState(`orch/${rootSlug}/${childName}`, childName, { + rootSlug: childName, + tasks: [ + { + name: "verifier", + type: "worker", + status: "running", + agentId: "agent-verifier", + runId: "run-verifier", + }, + ], + }); + return { repoPath, rootBranch, rootSlug }; +} + +describe("kill-tree walks handed-off subplanners like crawl", () => { + test("descendsIntoSubplanner follows running and handed-off subplanners only", () => { + const base = { name: "x", agentId: null, runId: null, parentAgentId: null }; + expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "running" } as never)).toBe(true); + expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "handed-off" } as never)).toBe(true); + expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "done" } as never)).toBe(false); + expect(descendsIntoSubplanner({ ...base, type: "worker", status: "running" } as never)).toBe(false); + }); + + test("a running agent under a handed-off subplanner is both crawled and collected", () => { + const { repoPath, rootBranch, rootSlug } = fixtureRepo(); + + const out: string[] = []; + crawlBranch({ repoPath, branch: rootBranch, slug: rootSlug }, 0, out, new Set()); + expect(out.join("\n")).toContain("verifier"); + + const victims = collectRunningAgentsInTree({ repoPath, branch: rootBranch, slug: rootSlug }); + expect(victims.map(v => v.agentId)).toEqual(["agent-verifier"]); + expect(victims[0]?.branch).toBe(`orch/${rootSlug}/child-planner`); + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/cli/util.ts b/orchestrate/skills/orchestrate/scripts/cli/util.ts index bffd24078..cbfcda4a4 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/util.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/util.ts @@ -100,10 +100,7 @@ export function crawlBranch( out.push( `${indent(depth + 1)}${t.name.padEnd(28)} ${t.type.padEnd(11)} ${t.status.padEnd(11)} ${t.agentId ?? ""}${lineage}` ); - if ( - t.type === "subplanner" && - (t.status === "running" || t.status === "handed-off") - ) { + if (descendsIntoSubplanner(t)) { // `ownSlug` (from loaded state.json) is authoritative over the param. crawlBranch( { repoPath, branch: `orch/${ownSlug}/${t.name}`, slug: t.name }, @@ -119,6 +116,20 @@ export function indent(depth: number): string { return " ".repeat(depth); } +/** + * Shared by `crawl` and `kill-tree` so the two walkers cannot disagree about + * which subplanner branches to descend into. A `handed-off` subplanner has + * finished its own run, but the state.json on its branch can still hold + * `running` children (a verifier or respawn it started, workers in flight), + * so both walkers must follow it. + */ +export function descendsIntoSubplanner(t: TreeTask): boolean { + return ( + t.type === "subplanner" && + (t.status === "running" || t.status === "handed-off") + ); +} + /** * Walk the tree the same way `crawl` does and collect every `running` task's * (agentId, runId). Reads only the fields needed for containment so unrelated @@ -171,7 +182,7 @@ export function collectRunningAgentsInTree( parentAgentId: t.parentAgentId ?? null, }); } - if (t.type === "subplanner" && t.status === "running") { + if (descendsIntoSubplanner(t)) { collectRunningAgentsInTree( { repoPath, branch: `orch/${ownSlug}/${t.name}`, slug: t.name }, collected, From b0346afdce596e55fe79df52f6784cf8fffcf77b Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:36:23 -0400 Subject: [PATCH 02/18] fix(orchestrate): fail kill-tree when cancellation fails Signed-off-by: Rudy Celekli (cherry picked from commit fce5c0f543dfbb63196faebd54785d31817f95a3) --- .../scripts/__tests__/kill-tree-exit.test.ts | 54 +++++++++++++++++++ .../orchestrate/scripts/cli/forensics.ts | 1 + 2 files changed, 55 insertions(+) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-exit.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-exit.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-exit.test.ts new file mode 100644 index 000000000..a5b90a2f6 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-exit.test.ts @@ -0,0 +1,54 @@ +import { expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +function killTree(failures: number) { + const script = ` + import { mock } from "bun:test"; + import { Command } from "commander"; + let attempts = 0; + mock.module("./core/agent-manager.ts", () => ({ + cancelCloudRun: async () => { + attempts++; + if (attempts <= ${failures}) throw new Error("backend cancellation failed"); + } + })); + mock.module("./cli/util.ts", () => ({ + collectRunningAgentsInTree: () => [ + { taskName:"one", agentId:"a1", runId:"r1", branch:"orch/a/one", parentAgentId:null }, + { taskName:"two", agentId:"a2", runId:"r2", branch:"orch/a/two", parentAgentId:null } + ], + filterVictimsToSubtree: victims => victims, + crawlBranch: () => {}, + errorMessage: error => error.message, + })); + const { registerForensicsCommands } = await import("./cli/forensics.ts"); + const program = new Command(); + registerForensicsCommands(program); + await program.parseAsync(["bun", "cli.ts", "kill-tree", "/offline", "main", "root", "--no-fetch", "-y"]); + `; + return spawnSync(process.execPath, ["-e", script], { + cwd: fileURLToPath(new URL("../", import.meta.url)), + encoding: "utf8", + env: { ...process.env, CURSOR_API_KEY: "offline-test-key" }, + }); +} + +test("kill-tree exits successfully when every cancellation succeeds", () => { + const result = killTree(0); + expect(result.status).toBe(0); + expect(result.stdout).toContain("killed 2 cloud agent(s); 0 failed."); +}); + +test("kill-tree reports failure while continuing to cancel other agents", () => { + const result = killTree(1); + expect(result.status).toBe(1); + expect(result.stdout).toContain("killed 1 cloud agent(s); 1 failed."); + expect(result.stderr).toContain("backend cancellation failed"); +}); + +test("kill-tree reports failure when every cancellation fails", () => { + const result = killTree(2); + expect(result.status).toBe(1); + expect(result.stdout).toContain("killed 0 cloud agent(s); 2 failed."); +}); diff --git a/orchestrate/skills/orchestrate/scripts/cli/forensics.ts b/orchestrate/skills/orchestrate/scripts/cli/forensics.ts index c5ee55675..82a466ea7 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/forensics.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/forensics.ts @@ -161,6 +161,7 @@ export function registerForensicsCommands(program: Command): void { } } console.log(`killed ${cancelled} cloud agent(s); ${failed} failed.`); + if (failed > 0) process.exitCode = 1; } ); } From e3b7785c5e9c77fad99a018e62a1dbe046a6f7cb Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:37:34 -0400 Subject: [PATCH 03/18] fix(orchestrate): cancel completed inspection timers Signed-off-by: Rudy Celekli (cherry picked from commit 28f2b4f6ea403af49062833634ee70beb2c35727) --- .../scripts/__tests__/inspect-timer.test.ts | 54 +++++++++++++++++++ .../orchestrate/scripts/core/agent-manager.ts | 11 ++-- 2 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts new file mode 100644 index 000000000..ecaed8691 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts @@ -0,0 +1,54 @@ +import { expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +test("completed stream inspection releases its deadline timers", () => { + const child = spawnSync(process.execPath, ["-e", ` + import { inspectRunStream } from "./core/agent-manager.ts"; + const run = { stream: async function* () { + yield { type: "assistant", message: { content: [{ type:"text", text:"done" }] } }; + } }; + const result = await inspectRunStream({ run, task:"one", agentId:"a", runId:"r", timeoutMs:30000 }); + console.log(JSON.stringify(result)); + `], { + cwd: fileURLToPath(new URL("../", import.meta.url)), + encoding: "utf8", + timeout: 2000, + }); + expect(child.stdout).toContain('"last_assistant_text_snippet":"done"'); + expect(child.error).toBeUndefined(); + expect(child.status).toBe(0); +}); + +test("silent inspection still reaches its deadline and closes the iterator", async () => { + const { inspectRunStream } = await import("../core/agent-manager.ts"); + let returned = false; + const iterator = { + next: () => new Promise>(() => {}), + return: async () => { returned = true; return { done: true as const, value: undefined }; }, + [Symbol.asyncIterator]() { return this; }, + }; + const result = await inspectRunStream({ + run: { stream: () => iterator } as never, + task: "silent", agentId: "a", runId: "r", timeoutMs: 25, + }); + expect(returned).toBe(true); + expect(result.tool_calls_total).toBe(0); + expect(result.drainedMs).toBeGreaterThanOrEqual(20); + expect(result.drainedMs).toBeLessThan(1000); +}); + +test("stream rejection is preserved and the iterator is closed", async () => { + const { inspectRunStream } = await import("../core/agent-manager.ts"); + let returned = false; + const iterator = { + next: async () => { throw new Error("stream failed"); }, + return: async () => { returned = true; return { done: true as const, value: undefined }; }, + [Symbol.asyncIterator]() { return this; }, + }; + await expect(inspectRunStream({ + run: { stream: () => iterator } as never, + task: "failed", agentId: "a", runId: "r", timeoutMs: 30000, + })).rejects.toThrow("stream failed"); + expect(returned).toBe(true); +}); diff --git a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts index 32ce7199f..1b28da226 100644 --- a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts +++ b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts @@ -1950,13 +1950,14 @@ export async function inspectRunStream(args: { try { while (Date.now() - startedAt < args.timeoutMs) { const remaining = Math.max(0, args.timeoutMs - (Date.now() - startedAt)); + const deadline = new AbortController(); const next = await Promise.race([ iterator.next(), - sleep(remaining).then(() => ({ + sleep(remaining, undefined, { signal: deadline.signal }).then(() => ({ done: true as const, value: undefined, })), - ]); + ]).finally(() => deadline.abort()); if (next.done) break; const event = next.value; if (event.type === "assistant") { @@ -1982,7 +1983,11 @@ export async function inspectRunStream(args: { } } } finally { - await Promise.race([iterator.return?.(), sleep(1_000)]); + const cleanupDeadline = new AbortController(); + await Promise.race([ + iterator.return?.(), + sleep(1_000, undefined, { signal: cleanupDeadline.signal }), + ]).finally(() => cleanupDeadline.abort()); } const toolCallWindowStart = Date.now() - 5 * 60_000; From 6e2e8395bad672b6021e6210dca6d6d0428bfa17 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:39:30 -0400 Subject: [PATCH 04/18] fix(orchestrate): paginate Andon reason replies Signed-off-by: Rudy Celekli (cherry picked from commit 97b0ddc680911a65e0b087627c371352a90f5ebb) --- .../__tests__/andon-root-cache.test.ts | 60 +++++++++++++++++++ .../skills/orchestrate/scripts/core/andon.ts | 27 ++++++--- 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts index 9a380195f..f23f989e0 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts @@ -124,6 +124,66 @@ describe("SlackReactionAndonSource", () => { expect(calls).toEqual([{ limit: 20, latest: expect.any(String) }]); }); + test("Finds the newest reason beyond the first reply page", async () => { + const cursors: (string | undefined)[] = []; + const latestBounds: (string | undefined)[] = []; + const source = new SlackReactionAndonSource({ + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + cursors.push(args.cursor); + latestBounds.push(args.latest); + return args.cursor === undefined ? { + messages: [ + { ts: "111.222", text: "🚨 ANDON RAISED by parent: ignore parent" }, + { ts: "111.300", text: "🚨 ANDON RAISED by operator: old reason" }, + ], + nextCursor: "page-two", + } : { + messages: [ + { ts: "111.400", text: ":rotating_light: ANDON RAISED by operator: newest reason" }, + { ts: "111.500", text: "unrelated newest reply" }, + ], + }; + }, + }, { channel: "C123", ts: "111.222" }); + await expect(source.snapshot()).resolves.toMatchObject({ active: true, reason: "newest reason" }); + expect(cursors).toEqual([undefined, "page-two"]); + expect(latestBounds[0]).toBe(latestBounds[1]); + }); + + test("Applies the recent reply window across pages", async () => { + const source = new SlackReactionAndonSource({ + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + return args.cursor === undefined ? { + messages: [{ ts: "111.300", text: "🚨 ANDON RAISED by operator: outside recent window" }], + nextCursor: "page-two", + } : { + messages: Array.from({ length: 20 }, (_, i) => ({ ts: `112.${100 + i}`, text: "unrelated" })), + }; + }, + }, { channel: "C123", ts: "111.222" }); + const state = await source.snapshot(); + expect(state.active).toBe(true); + expect(state).not.toHaveProperty("reason", "outside recent window"); + }); + + test("Keeps Andon active when a later reply page fails", async () => { + const source = new SlackReactionAndonSource({ + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + if (args.cursor) throw new Error("slack_replies_unavailable"); + return { + messages: [{ ts: "111.300", text: "🚨 ANDON RAISED by operator: incomplete history" }], + nextCursor: "page-two", + }; + }, + }, { channel: "C123", ts: "111.222" }); + const state = await source.snapshot(); + expect(state.active).toBe(true); + expect(state).not.toHaveProperty("reason", "incomplete history"); + }); + test("Keeps Andon active when reason reply fetch fails", async () => { const source = new SlackReactionAndonSource( { diff --git a/orchestrate/skills/orchestrate/scripts/core/andon.ts b/orchestrate/skills/orchestrate/scripts/core/andon.ts index ae6e84cae..8a28f1123 100644 --- a/orchestrate/skills/orchestrate/scripts/core/andon.ts +++ b/orchestrate/skills/orchestrate/scripts/core/andon.ts @@ -185,15 +185,24 @@ export class SlackReactionAndonSource implements AndonSource { } private async latestRaisedReason(): Promise { - const page = await this.slack.getThreadReplies({ - ...this.ref, - limit: ANDON_REASON_REPLY_SCAN_LIMIT, - latest: String(Date.now() / 1000), - }); - const recentReplies = page.messages - .filter(message => message.ts !== this.ref.ts) - .sort((a, b) => compareSlackTsDesc(a.ts, b.ts)) - .slice(0, ANDON_REASON_REPLY_SCAN_LIMIT); + // Slack returns the earliest replies first, even with `latest`. Follow + // cursors before selecting the recent window; keep at most 20 candidates. + const latest = String(Date.now() / 1000); + let cursor: string | undefined; + let recentReplies: { ts: string; text: string }[] = []; + do { + const page = await this.slack.getThreadReplies({ + ...this.ref, + limit: ANDON_REASON_REPLY_SCAN_LIMIT, + latest, + ...(cursor ? { cursor } : {}), + }); + recentReplies = [...recentReplies, ...page.messages] + .filter(message => message.ts !== this.ref.ts) + .sort((a, b) => compareSlackTsDesc(a.ts, b.ts)) + .slice(0, ANDON_REASON_REPLY_SCAN_LIMIT); + cursor = page.nextCursor; + } while (cursor); for (const message of recentReplies) { const reason = parseRaisedReason(message.text); if (reason !== undefined) return reason; From 2e1fe73ca49445fc86d5229f3c770a63c932346c Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:42:30 -0400 Subject: [PATCH 05/18] fix(orchestrate): paginate root planner lookup Signed-off-by: Rudy Celekli (cherry picked from commit d4052b9782defa3ee48715aa3377fafa387b0614) --- .../__tests__/kickoff-pagination.test.ts | 47 +++++++++++++++++ .../skills/orchestrate/scripts/cli/task.ts | 52 +++++++++++-------- 2 files changed, 77 insertions(+), 22 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts new file mode 100644 index 000000000..a97aa2a5d --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts @@ -0,0 +1,47 @@ +import { expect, test } from "bun:test"; +import { findActiveRootPlanner } from "../cli/task.ts"; + +const now = Date.parse("2026-10-16T16:00:00Z"); +const matching = { + agentId: "matching", name: "refactor-root", createdAt: now - 1000, + latestRun: { id: "run-matching", status: "running" }, +}; + +test("finds an active matching planner on a later list page", async () => { + const cursors: (string | undefined)[] = []; + const api = { + async list(opts: { cursor?: string }) { + cursors.push(opts.cursor); + return opts.cursor ? { items: [matching] } : { + items: [{ ...matching, agentId: "other", name: "different-root" }], + nextCursor: "next-page", + }; + }, + }; + const result = await findActiveRootPlanner(api, "refactor", now); + expect(result?.agentId).toBe("matching"); + expect(cursors).toEqual([undefined, "next-page"]); +}); + +test("follows cursors even when an intermediate page is empty", async () => { + const api = { + async list(opts: { cursor?: string }) { + return opts.cursor ? { items: [matching] } : { items: [], nextCursor: "next-page" }; + }, + }; + expect((await findActiveRootPlanner(api, "refactor", now))?.runId).toBe("run-matching"); +}); + +test("an exhausted empty listing has no active planner", async () => { + expect(await findActiveRootPlanner({ list: async () => ({ items: [] }) }, "refactor", now)).toBeNull(); +}); + +test("propagates later-page lookup failures instead of authorizing a duplicate", async () => { + const api = { + async list(opts: { cursor?: string }) { + if (opts.cursor) throw new Error("list unavailable"); + return { items: [], nextCursor: "next-page" }; + }, + }; + await expect(findActiveRootPlanner(api, "refactor", now)).rejects.toThrow("list unavailable"); +}); diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 3093c913a..45f2cb276 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -32,7 +32,7 @@ export const MAX_BOOT_MS = 30 * 60 * 1000; const ACTIVE_ROOT_RUN_STATUSES = new Set(["pending", "running"]); type AgentListApi = { - list: (opts: { runtime: "cloud"; limit: number }) => Promise; + list: (opts: { runtime: "cloud"; limit: number; cursor?: string }) => Promise; listRuns?: ( agentId: string, opts: { runtime: "cloud"; limit: number } @@ -480,30 +480,38 @@ export async function findActiveRootPlanner( rootSlug: string, nowMs: number = Date.now() ): Promise { - const list = await agentApi.list({ runtime: "cloud", limit: 50 }); // Exact match, not startsWith: a prefix slug would otherwise adopt another // goal's `-root` planner (e.g. "auth" adopting "auth-ui-root"). const rootPlannerName = `${rootSlug}-root`; - for (const item of listItems(list)) { - const name = stringField(item, "name"); - if (!name) continue; - if (name !== rootPlannerName) continue; - const createdAt = timeMs(field(item, "createdAt")); - if (createdAt === null || nowMs - createdAt > MAX_BOOT_MS) continue; - const agentId = - stringField(item, "agentId") ?? stringField(item, "id") ?? null; - if (!agentId) continue; - const embeddedRun = latestRunFromItem(item); - const run = embeddedRun ?? (await latestRunForAgent(agentApi, agentId)); - const status = runStatus(run); - if (!ACTIVE_ROOT_RUN_STATUSES.has(status)) continue; - return { - agentId, - runId: stringField(run, "id") ?? null, - status, - name, - }; - } + let cursor: string | undefined; + do { + const list = await agentApi.list({ + runtime: "cloud", + limit: 50, + ...(cursor ? { cursor } : {}), + }); + for (const item of listItems(list)) { + const name = stringField(item, "name"); + if (!name) continue; + if (name !== rootPlannerName) continue; + const createdAt = timeMs(field(item, "createdAt")); + if (createdAt === null || nowMs - createdAt > MAX_BOOT_MS) continue; + const agentId = + stringField(item, "agentId") ?? stringField(item, "id") ?? null; + if (!agentId) continue; + const embeddedRun = latestRunFromItem(item); + const run = embeddedRun ?? (await latestRunForAgent(agentApi, agentId)); + const status = runStatus(run); + if (!ACTIVE_ROOT_RUN_STATUSES.has(status)) continue; + return { + agentId, + runId: stringField(run, "id") ?? null, + status, + name, + }; + } + cursor = isRecord(list) ? stringField(list, "nextCursor") : undefined; + } while (cursor); return null; } From 478ef440165d35892aaf092ba849240b226f2b15 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 16:26:49 -0400 Subject: [PATCH 06/18] fix(orchestrate): keep empty canonical verification authoritative Signed-off-by: Rudy Celekli (cherry picked from commit 8dc53ec011bcf0fc0f1a5b5d74d4b54a04e9213f) --- .../__tests__/handoff-empty-canonical.test.ts | 17 +++++++++++++++++ .../skills/orchestrate/scripts/core/handoff.ts | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts new file mode 100644 index 000000000..16c36439e --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from "bun:test"; +import { parseHandoffVerification } from "../core/handoff.ts"; + +describe("canonical verification remains authoritative when empty", () => { + test("empty canonical verdict cannot inherit legacy pass", () => { + expect(parseHandoffVerification("## Verification\n\n## Verdict\npass\n")).toBeNull(); + }); + test("empty terminal canonical section cannot inherit earlier legacy pass", () => { + expect(parseHandoffVerification("## Verdict\npass\n\n## Verification\n")).toBeNull(); + }); + test("legacy-only verdict remains supported", () => { + expect(parseHandoffVerification("## Verdict\npass\n")).toBe("type-check-only"); + }); + test("recognized canonical verdict retains precedence", () => { + expect(parseHandoffVerification("## Verification\nverifier-blocked\n\n## Verdict\npass\n")).toBe("verifier-blocked"); + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/core/handoff.ts b/orchestrate/skills/orchestrate/scripts/core/handoff.ts index 666e2dd61..9f3c84073 100644 --- a/orchestrate/skills/orchestrate/scripts/core/handoff.ts +++ b/orchestrate/skills/orchestrate/scripts/core/handoff.ts @@ -61,7 +61,8 @@ export function resolveRunBranch(args: { */ export function parseHandoffVerification(handoff: string): Verification | null { const fromCanonical = readSectionValue({ handoff, heading: "Verification" }); - if (fromCanonical) { + if (/^##[\t ]+Verification[\t ]*\r?$/m.test(handoff)) { + if (!fromCanonical) return null; const normalized = normalizeEnumValue(fromCanonical); if (isVerification(normalized)) return normalized; return null; From e51644f7384a60ac3ef80d3bfa0e0610a3caff34 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 16:28:07 -0400 Subject: [PATCH 07/18] fix(orchestrate): prefer the canonical handoff PR section (cherry picked from commit af03d4cedbdd4218a3aa6df5c068ed97a363fa4c) --- .../handoff-verification-parser.test.ts | 26 ++++++++++++++++++ .../orchestrate/scripts/core/handoff.ts | 27 ++++++++++++++----- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts index c17c656e5..197122753 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts @@ -101,6 +101,32 @@ describe("parseHandoffFailureMode", () => { }); describe("parseHandoffPrNumber", () => { + test("Explicit PR sections win over dependency links", () => { + for (const heading of ["PR", "Pull Request"]) { + for (const value of ["#42", "https://github.com/example/project/pull/42", ""]) { + const body = `## What I did\nFollowed https://github.com/example/dependency/pull/7\n## ${heading}\n${value}`; + expect(parseHandoffPrNumber(body)).toBe(42); + } + } + }); + + test("Explicit absent or invalid PRs do not adopt reference links", () => { + for (const value of ["(none)", "", "#42garbage", "0", "9007199254740993"]) { + const body = `## PR\n${value}\n## References\nhttps://github.com/example/dependency/pull/7`; + expect(parseHandoffPrNumber(body)).toBe(null); + } + }); + + test("Canonical section whitespace and CRLF match the section reader", () => { + const body = "## What I did\r\nhttps://github.com/example/dependency/pull/7\r\n##\tPR \r\n\r\n`#42`\r\n"; + expect(parseHandoffPrNumber(body)).toBe(42); + }); + + test("Lowercase headings retain the legacy URL fallback", () => { + const body = "## pr\n#42\n## References\nhttps://github.com/example/dependency/pull/7"; + expect(parseHandoffPrNumber(body)).toBe(7); + }); + test("Reads GitHub pull request URLs", () => { const body = "Opened https://github.com/example-org/example-repo/pull/109301"; diff --git a/orchestrate/skills/orchestrate/scripts/core/handoff.ts b/orchestrate/skills/orchestrate/scripts/core/handoff.ts index 9f3c84073..74d78de3c 100644 --- a/orchestrate/skills/orchestrate/scripts/core/handoff.ts +++ b/orchestrate/skills/orchestrate/scripts/core/handoff.ts @@ -83,16 +83,29 @@ export function parseHandoffFailureMode(handoff: string): FailureMode | null { } export function parseHandoffPrNumber(handoff: string): number | null { - for (const rawUrl of handoff.match(/https?:\/\/[^\s<>|)]+/g) ?? []) { - const number = prNumberFromUrl(rawUrl); - if (number !== null) return number; - } + // An explicit artifact section wins over links to dependencies or examples. + // If it says no PR, do not attribute an unrelated reference to this task. + const hasSection = /^##\s+(?:PR|Pull Request)\s*\r?\n/m.test(handoff); const fromSection = readSectionValue({ handoff, heading: "PR" }) ?? readSectionValue({ handoff, heading: "Pull Request" }); - if (!fromSection) return null; - const number = Number.parseInt(fromSection.replace(/^#/, ""), 10); - return Number.isInteger(number) && number > 0 ? number : null; + if (hasSection) { + if (!fromSection) return null; + const fromUrl = firstPrNumberFromUrl(fromSection); + if (fromUrl !== null) return fromUrl; + if (!/^#?\d+$/.test(fromSection)) return null; + const number = Number(fromSection.replace(/^#/, "")); + return Number.isSafeInteger(number) && number > 0 ? number : null; + } + return firstPrNumberFromUrl(handoff); +} + +function firstPrNumberFromUrl(text: string): number | null { + for (const rawUrl of text.match(/https?:\/\/[^\s<>|)]+/g) ?? []) { + const number = prNumberFromUrl(rawUrl); + if (number !== null) return number; + } + return null; } function prNumberFromUrl(rawUrl: string): number | null { From 49f41924a4c4523b4dc0f33b30861305e8f07040 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 16:54:28 -0400 Subject: [PATCH 08/18] fix(orchestrate): recognize empty PR headings at end of handoff (cherry picked from commit d87e457e3b1d4db8dda36f550cfdc3b40cd4cf09) --- .../scripts/__tests__/handoff-verification-parser.test.ts | 8 ++++++++ orchestrate/skills/orchestrate/scripts/core/handoff.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts index 197122753..b1686e3f4 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts @@ -117,6 +117,14 @@ describe("parseHandoffPrNumber", () => { } }); + test("Empty trailing PR headings do not inherit dependency links", () => { + for (const heading of ["## PR", "## Pull Request", "##\tPR ", "## Pull Request\t"]) { + const body = `## What I did\nFollowed https://github.com/example/dependency/pull/7\n${heading}`; + expect(parseHandoffPrNumber(body)).toBe(null); + expect(parseHandoffPrNumber(body.trim())).toBe(null); + } + }); + test("Canonical section whitespace and CRLF match the section reader", () => { const body = "## What I did\r\nhttps://github.com/example/dependency/pull/7\r\n##\tPR \r\n\r\n`#42`\r\n"; expect(parseHandoffPrNumber(body)).toBe(42); diff --git a/orchestrate/skills/orchestrate/scripts/core/handoff.ts b/orchestrate/skills/orchestrate/scripts/core/handoff.ts index 74d78de3c..f3c386d8a 100644 --- a/orchestrate/skills/orchestrate/scripts/core/handoff.ts +++ b/orchestrate/skills/orchestrate/scripts/core/handoff.ts @@ -85,7 +85,7 @@ export function parseHandoffFailureMode(handoff: string): FailureMode | null { export function parseHandoffPrNumber(handoff: string): number | null { // An explicit artifact section wins over links to dependencies or examples. // If it says no PR, do not attribute an unrelated reference to this task. - const hasSection = /^##\s+(?:PR|Pull Request)\s*\r?\n/m.test(handoff); + const hasSection = /^##\s+(?:PR|Pull Request)\s*(?:\r?\n|$)/m.test(handoff); const fromSection = readSectionValue({ handoff, heading: "PR" }) ?? readSectionValue({ handoff, heading: "Pull Request" }); From 223bbe8180d428734967ddbffb4e39d570996fdc Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 16:55:43 -0400 Subject: [PATCH 09/18] fix(orchestrate): preserve invalid required comment queues (cherry picked from commit 1c5cfbf8e9c1c1d939e0cfa4e09e8ef30958ff50) --- .../__tests__/comment-retry-queue.test.ts | 100 +++++++++++++++++- .../scripts/core/comment-retry-queue.ts | 38 +++++-- 2 files changed, 128 insertions(+), 10 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts index a0078a018..82b121cb0 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts @@ -1,5 +1,11 @@ import { describe, expect, test } from "bun:test"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -392,3 +398,95 @@ describe("required comment retry queue", () => { } }); }); + +describe("invalid persisted required comment queues", () => { + const valid = { + id: "persisted-required-comment", + destination: `slack:${TEST_SLACK_CHANNEL}:111.222`, + body: "keep this required comment", + sender: "agent", + attempts: 0, + createdAt: "1970-01-01T00:00:00.000Z", + nextAttemptAt: "1970-01-01T00:00:00.000Z", + }; + for (const [name, content] of [ + ["malformed JSON", '[{"body":"private fixture content"}'], + ["non-array JSON", JSON.stringify({ pending: [valid] })], + [ + "invalid entry beside a valid comment", + JSON.stringify([valid, { id: "broken" }]), + ], + ]) { + for (const operation of ["enqueue", "drain"] as const) { + test(`${operation} preserves ${name} without posting`, async () => { + const workspace = mkdtempSync(join(tmpdir(), "orch-comment-corruption-")); + const posts: SlackPost[] = []; + const queuePath = commentRetryQueuePath(workspace); + try { + writeFileSync(queuePath, content); + const destinations = { slack: slackAdapter({ posts }) }; + const run = + operation === "drain" + ? drainCommentRetryQueue({ workspace, destinations }) + : postOrQueueComment({ + workspace, + destinations, + destination: valid.destination, + body: "new required comment", + sender: "agent", + criticality: "required", + }); + await expect(run).rejects.toThrow(/comment queue/); + expect(readFileSync(queuePath, "utf8")).toBe(content); + expect(posts).toEqual([]); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }); + } + } + + test("read failures preserve the queue path without posting", async () => { + const workspace = mkdtempSync(join(tmpdir(), "orch-comment-read-error-")); + const posts: SlackPost[] = []; + try { + mkdirSync(commentRetryQueuePath(workspace)); + await expect( + postOrQueueComment({ + workspace, + destinations: { slack: slackAdapter({ posts }) }, + destination: valid.destination, + body: "new comment", + sender: "agent", + criticality: "required", + }) + ).rejects.toThrow(/comment queue/); + expect(posts).toEqual([]); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }); + + test("a valid empty queue can enqueue and deliver normally", async () => { + const workspace = mkdtempSync(join(tmpdir(), "orch-comment-empty-")); + const posts: SlackPost[] = []; + try { + writeFileSync(commentRetryQueuePath(workspace), "[]"); + const result = await postOrQueueComment({ + workspace, + destinations: { slack: slackAdapter({ posts }) }, + destination: valid.destination, + body: "new comment", + sender: "agent", + criticality: "required", + }); + expect(result).toBe("posted"); + expect(posts.map(post => post.text)).toEqual(["new comment"]); + expect( + JSON.parse(readFileSync(commentRetryQueuePath(workspace), "utf8")) + ).toEqual([]); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts index f9a3b3bf3..2172748b8 100644 --- a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts +++ b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts @@ -1,6 +1,5 @@ import { randomUUID } from "node:crypto"; import { - existsSync, mkdirSync, readFileSync, renameSync, @@ -198,17 +197,38 @@ function enqueueComment(args: { } function readQueue(path: string): CommentRetryEntry[] { - if (!existsSync(path)) return []; + let content: string; try { - const raw = JSON.parse(readFileSync(path, "utf8")); - if (!Array.isArray(raw)) return []; - return raw.flatMap(entry => { - const parsed = parseEntry(entry); - return parsed ? [parsed] : []; - }); + content = readFileSync(path, "utf8"); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT") return []; + throw new Error( + `Cannot read required comment queue ${path} (${code ?? "read error"}); existing queue preserved.` + ); + } + let raw: unknown; + try { + raw = JSON.parse(content); } catch { - return []; + throw new Error( + `Invalid JSON in required comment queue ${path}; restore or repair the file before retrying.` + ); } + if (!Array.isArray(raw)) { + throw new Error( + `Required comment queue ${path} must contain an array; restore or repair the file before retrying.` + ); + } + return raw.map((entry, index) => { + const parsed = parseEntry(entry); + if (!parsed) { + throw new Error( + `Invalid entry ${index} in required comment queue ${path}; restore or repair the file before retrying.` + ); + } + return parsed; + }); } function writeQueue(path: string, queue: CommentRetryEntry[]): void { From bb32daf2f02bd3987d915898ec1c2f2ff018457d Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:34:58 -0400 Subject: [PATCH 10/18] fix(orchestrate): parse exponent-form measurement values Signed-off-by: Rudy Celekli (cherry picked from commit 95fc9e28cea8a670ae51ee61fc06bdfca812b05a) --- .../__tests__/measurements-exponents.test.ts | 28 +++++++++++++++++++ .../orchestrate/scripts/measurements.ts | 4 +-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts new file mode 100644 index 000000000..8178b0480 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts @@ -0,0 +1,28 @@ +import { expect, test } from "bun:test"; +import { compareMeasurement } from "../measurements.ts"; + +function check(measured: string, claimed: string) { + return compareMeasurement({ spec: { name: "bytes", command: "printf metric" }, measured, claim: { name: "bytes", before: "0", op: "→", after: claimed } }); +} + +test("native large-number output uses numeric tolerance", () => { + const result = check(String(1.05e21), String(1e21)); + expect(result.outcome).toBe("match"); + expect(result.measuredNumeric).toBe(1.05e21); +}); + +test("native small-number output reports its full numeric value", () => { + const result = check(String(1e-7), String(1e-7)); + expect(result.outcome).toBe("match"); + expect(result.measuredNumeric).toBe(1e-7); +}); + +test("equivalent decimal and exponent output compare equally with units", () => { + expect(check("1e3 MB", "1000 MB").outcome).toBe("match"); +}); + +test("out-of-tolerance, unit mismatch, and nonnumeric strings remain rejected", () => { + expect(check("2e21", "1e21").outcome).toBe("value-mismatch"); + expect(check("1e3 KB", "1000 MB").outcome).toBe("value-mismatch"); + expect(check("answer", "other").outcome).toBe("value-mismatch"); +}); diff --git a/orchestrate/skills/orchestrate/scripts/measurements.ts b/orchestrate/skills/orchestrate/scripts/measurements.ts index 2cc008e9c..e6b687ee1 100644 --- a/orchestrate/skills/orchestrate/scripts/measurements.ts +++ b/orchestrate/skills/orchestrate/scripts/measurements.ts @@ -204,7 +204,7 @@ interface NumericWithUnit { * fall through to string comparison instead of pretending it's numeric. */ function parseNumericWithUnit(value: string): NumericWithUnit | null { - const match = /^([-+]?\d+(?:\.\d+)?)\s*([a-zA-Z%]+)?$/.exec(value.trim()); + const match = /^([-+]?\d+(?:\.\d+)?(?:[eE][-+]?\d+)?)\s*([a-zA-Z%]+)?$/.exec(value.trim()); if (!match) return null; const numeric = Number.parseFloat(match[1]); if (!Number.isFinite(numeric)) return null; @@ -214,7 +214,7 @@ function parseNumericWithUnit(value: string): NumericWithUnit | null { function parseFiniteNumber(value: string): number | null { // Used for the `measuredNumeric` display field: lenient leading-numeric // strip, separate from the end-anchored unit-aware comparison. - const match = /^[-+]?\d+(?:\.\d+)?/.exec(value.trim()); + const match = /^[-+]?\d+(?:\.\d+)?(?:[eE][-+]?\d+)?/.exec(value.trim()); if (!match) return null; const parsed = Number.parseFloat(match[0]); return Number.isFinite(parsed) ? parsed : null; From a38d6966e64a6d9dc8969e8f6e56b181e24cf87f Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 17:54:18 -0400 Subject: [PATCH 11/18] test(orchestrate): cover relative exponent comparisons Signed-off-by: Rudy Celekli (cherry picked from commit 9becd10edbb284b4efd9f5942f02ddb8f06180bc) --- .../scripts/__tests__/measurements-exponents.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts index 8178b0480..d998384a1 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts @@ -26,3 +26,8 @@ test("out-of-tolerance, unit mismatch, and nonnumeric strings remain rejected", expect(check("1e3 KB", "1000 MB").outcome).toBe("value-mismatch"); expect(check("answer", "other").outcome).toBe("value-mismatch"); }); + +test("subunit exponents respect relative drift", () => { + expect(check("1e-6", "1e-7").outcome).toBe("value-mismatch"); + expect(check("1.05e-7", "1e-7").outcome).toBe("match"); +}); From f5be1743ad99564bee1bdd22739ef153e92df9a6 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 16 Oct 2026 14:47:19 -0400 Subject: [PATCH 12/18] fix(orchestrate): compare fractional measurements relatively (cherry picked from commit 204320767c3d21faf2d18331629f183bd5499d54) --- .../__tests__/measurements-compare.test.ts | 17 +++++++++++++++++ .../skills/orchestrate/scripts/measurements.ts | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts index 64f437490..cee60d29f 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts @@ -44,6 +44,23 @@ describe("compareMeasurement / applyMeasurementParser / buildMeasurementEnv", () expect((result.driftFraction ?? 0) > 1).toBeTruthy(); }); + test("Fractional measurements use relative tolerance at either unit scale", () => { + for (const [after, measured, outcome] of [ + ["0.01 s", "0.1 s", "value-mismatch"], + ["10 ms", "100 ms", "value-mismatch"], + ["-0.01 s", "-0.1 s", "value-mismatch"], + ["0.01 s", "0.0105 s", "match"], + ["10 ms", "10.5 ms", "match"], + ["0 s", "0.09 s", "match"], + ["0 s", "0.2 s", "value-mismatch"], + ] as const) { + const result = compareMeasurement({ spec: baseSpec, measured, claim: claim(after) }); + expect(result.outcome).toBe(outcome); + } + const result = compareMeasurement({ spec: baseSpec, measured: "0.1 s", claim: claim("0.01 s") }); + expect(result.driftFraction).toBeCloseTo(9); + }); + test("Custom tolerance widens matching window", () => { const result = compareMeasurement({ spec: { ...baseSpec, toleranceFraction: 0.5 }, diff --git a/orchestrate/skills/orchestrate/scripts/measurements.ts b/orchestrate/skills/orchestrate/scripts/measurements.ts index e6b687ee1..582e89691 100644 --- a/orchestrate/skills/orchestrate/scripts/measurements.ts +++ b/orchestrate/skills/orchestrate/scripts/measurements.ts @@ -163,7 +163,10 @@ export function compareMeasurement( detail, }; } - const denom = Math.max(Math.abs(claimParsed.numeric), 1); + // Keep relative tolerance independent of the unit scale. Flooring at 1 + // accepts large errors for fractional measurements (e.g. seconds). + // A zero claim retains the existing absolute-tolerance fallback. + const denom = Math.abs(claimParsed.numeric) || 1; const drift = Math.abs(measuredParsed.numeric - claimParsed.numeric) / denom; const outcome: MeasurementCheck["outcome"] = From f8fb6d5a4cb9811428d0e1ed9c50675746205320 Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Thu, 22 Oct 2026 18:50:11 +0300 Subject: [PATCH 13/18] fix(orchestrate): regex measurement parser returns the wrong value under the g flag The regex parser schema accepts any subset of gimsuy, and the parser used String.prototype.match. With g, match returns every whole match and no capture groups, so the documented contract (capture group 1, else the whole match) silently became "the second whole match": for pattern 'passing: (\\d+)' with flags 'g' on two-line test output the parser returned 'passing: 40' instead of '12', and compareMeasurement then failed the worker's correct claim with a unit mismatch. Drop g when building the RegExp (one value is extracted, global has no meaning here) and use exec, which returns the first match with its groups under any flags. Test covers both the grouped and the whole-match case with g set; it fails on the previous code with 'passing: 40'. --- .../__tests__/measurements-compare.test.ts | 17 +++++++++++++++++ .../skills/orchestrate/scripts/measurements.ts | 9 +++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts index cee60d29f..e3d580779 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts @@ -168,6 +168,23 @@ describe("compareMeasurement / applyMeasurementParser / buildMeasurementEnv", () if (result.ok) expect(result.value).toBe("bytes"); }); + test("regex parser with the g flag still returns capture group 1 of the first match", () => { + const stdout = "passing: 12 failing: 3\npassing: 40 failing: 0\n"; + const grouped = applyMeasurementParser( + { kind: "regex", pattern: "passing: (\\d+)", flags: "g" }, + stdout + ); + expect(grouped.ok).toBe(true); + if (grouped.ok) expect(grouped.value).toBe("12"); + + const whole = applyMeasurementParser( + { kind: "regex", pattern: "passing: \\d+", flags: "gm" }, + stdout + ); + expect(whole.ok).toBe(true); + if (whole.ok) expect(whole.value).toBe("passing: 12"); + }); + test("regex parser without match returns ok false", () => { const result = applyMeasurementParser( { kind: "regex", pattern: "totally-not-here" }, diff --git a/orchestrate/skills/orchestrate/scripts/measurements.ts b/orchestrate/skills/orchestrate/scripts/measurements.ts index 582e89691..f9d00a843 100644 --- a/orchestrate/skills/orchestrate/scripts/measurements.ts +++ b/orchestrate/skills/orchestrate/scripts/measurements.ts @@ -368,14 +368,19 @@ export function applyMeasurementParser( } let re: RegExp; try { - re = new RegExp(parser.pattern, parser.flags ?? ""); + // The schema accepts `g`, but a global regex changes what + // `String.prototype.match` returns: every whole match and no + // groups, so "capture group 1" would silently become the second + // match. One value is extracted here, so drop `g` and use `exec`, + // which returns the first match with its groups under any flags. + re = new RegExp(parser.pattern, (parser.flags ?? "").replace(/g/g, "")); } catch (err) { return { ok: false, reason: `invalid regex: ${errorMessage(err)}`, }; } - const match = stdout.match(re); + const match = re.exec(stdout); if (!match) { return { ok: false, reason: "regex did not match command stdout" }; } From 126c8810de6e08a761e732fdec1823aa559bb2cc Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Fri, 23 Oct 2026 11:05:04 +0300 Subject: [PATCH 14/18] fix(orchestrate): read a handoff section value written as a list item or in bold readSectionValue and parseHandoffBranch took the first line under a heading and stripped only surrounding backticks. A worker that writes the value the way it writes the Measurements section, as a one-item list ('- unit-test-verified') or in bold, produced a value that normalizes to '--unit-test-verified' or '**unit-test-verified**', which is not an enum member, so parseHandoffVerification returned null and recordHandoffVerification silently recorded nothing; parseHandoffBranch returned '- `agent/x`' as the branch name. parseHandoffMeasurements in the same module already strips list markers. One helper now strips a leading list marker, bold, and backticks before the enum or branch check. Tests for both parsers; they fail on main. --- .../__tests__/handoff-branch-parser.test.ts | 5 +++++ .../handoff-verification-parser.test.ts | 12 ++++++++++ .../orchestrate/scripts/core/handoff.ts | 22 ++++++++++++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts index fe9413582..4896ed2a8 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts @@ -3,6 +3,11 @@ import { describe, expect, test } from "bun:test"; import { parseHandoffBranch, resolveRunBranch } from "../core/handoff.ts"; describe("parseHandoffBranch / resolveRunBranch", () => { + test("A branch written as a one-item list is read", () => { + const body = ["## Branch", "- `agent/listed-abc1`", "## What I did"].join("\n"); + expect(parseHandoffBranch(body)).toBe("agent/listed-abc1"); + }); + test("Worker template wraps branch value in single backticks", () => { const body = [ "## Status", diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts index b1686e3f4..5168b0c0c 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts @@ -37,6 +37,18 @@ describe("parseHandoffVerification", () => { expect(parseHandoffVerification(body)).toBe("verifier-failed"); }); + test("A one-item list or bold around the value is stripped", () => { + expect(parseHandoffVerification(["## Verification", "- unit-test-verified", ""].join("\n"))).toBe( + "unit-test-verified" + ); + expect(parseHandoffVerification(["## Verification", "* `live-ui-verified`", ""].join("\n"))).toBe( + "live-ui-verified" + ); + expect(parseHandoffVerification(["## Verification", "**type-check-only**", ""].join("\n"))).toBe( + "type-check-only" + ); + }); + test("Unknown value returns null without falling through to legacy", () => { const body = ["## Verification", "totally-made-up", ""].join("\n"); expect(parseHandoffVerification(body)).toBe(null); diff --git a/orchestrate/skills/orchestrate/scripts/core/handoff.ts b/orchestrate/skills/orchestrate/scripts/core/handoff.ts index f3c386d8a..96b37ca03 100644 --- a/orchestrate/skills/orchestrate/scripts/core/handoff.ts +++ b/orchestrate/skills/orchestrate/scripts/core/handoff.ts @@ -18,8 +18,9 @@ export function parseHandoffBranch(handoff: string): string | null { if (raw.length === 0) return null; if (raw.startsWith("##")) return null; // Worker template wraps the value in backticks (`agent/foo-abc1`); the - // operator-facing hint also tolerates a bare branch name. - const unwrapped = raw.replace(/^`([^`]+)`$/, "$1").trim(); + // operator-facing hint also tolerates a bare branch name, and workers + // sometimes write the value as a one-item list or in bold. + const unwrapped = unwrapValue(raw); if (unwrapped.length === 0) return null; if (/^\(no branch\)$/i.test(unwrapped)) return null; return unwrapped; @@ -139,7 +140,22 @@ function readSectionValue(args: { const raw = match[1].trim(); if (raw.length === 0) return null; if (raw.startsWith("##")) return null; - return raw.replace(/^`([^`]+)`$/, "$1").trim(); + const unwrapped = unwrapValue(raw); + return unwrapped.length === 0 ? null : unwrapped; +} + +/** + * Strip the decoration a worker may put around a single-value section: + * a leading list marker (`- ` / `* `), bold (`**x**`), and backticks, in that + * order, the same tolerance `parseHandoffMeasurements` has for list markers. + */ +function unwrapValue(raw: string): string { + return raw + .replace(/^[-*]\s+/, "") + .replace(/^\*\*([^*]+)\*\*$/, "$1") + .trim() + .replace(/^`([^`]+)`$/, "$1") + .trim(); } function readKeyValue(args: { handoff: string; key: string }): string | null { From 6607aca0dbc525b0b1ed8211f87ce12566109478 Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Fri, 23 Oct 2026 18:07:51 +0300 Subject: [PATCH 15/18] fix(orchestrate): redact mid-line local paths and env-style secret assignments in Slack comment bodies redactBody gates every Slack comment the orchestrator posts. Three shapes got through it: - /Users/... and /workspace/... were matched only at a line start (^ with the m flag), so "tests failed in /Users/me/repo/x.ts:12" passed as is. The .pnpm pattern next to them had no anchor, so the three disagreed. - The secret matcher wrapped the bare word in \b, and _ is a word character, so GITHUB_TOKEN=ghp_... and SLACK_BOT_TOKEN=xoxb-... never matched while token=... did. - The value was \S+, so "Authorization: Bearer xyz" became "Authorization=[redacted] xyz" with the credential still in place. Paths now match anywhere with a lookbehind that keeps URL segments such as https://host/Users/x out. The key may carry a prefix or suffix, and the value runs to the end of the line. Two of the three new tests fail on main; 214/214 with the fix. --- .../scripts/__tests__/redact-body.test.ts | 31 +++++++++++++++++++ .../orchestrate/scripts/core/redact-body.ts | 12 +++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/redact-body.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/redact-body.test.ts index a7fbbd4a8..fdb36af0d 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/redact-body.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/redact-body.test.ts @@ -14,6 +14,37 @@ describe("redactBody", () => { expect(result.text).toContain("[redacted-path]"); }); + test("refuses local paths in the middle of a line", () => { + const result = redactBody( + "tests failed in /Users/example/repo/src/foo.test.ts:12, see /workspace/app/out.log" + ); + + expect(result.reasons).toContain("contains /Users path"); + expect(result.reasons).toContain("contains /workspace path"); + expect(result.text).toBe( + "tests failed in [redacted-path] see [redacted-path]" + ); + }); + + test("leaves URL segments that happen to contain /Users alone", () => { + expect(redactBody("see https://example.com/Users/profile").reasons).toEqual( + [] + ); + }); + + test("refuses env-style secret assignments and whole header values", () => { + const envStyle = redactBody( + "export SLACK_BOT_TOKEN=xoxb-1-abc GITHUB_TOKEN=ghp_def" + ); + expect(envStyle.reasons).toContain("contains sensitive key"); + expect(envStyle.text).not.toContain("xoxb-1-abc"); + expect(envStyle.text).not.toContain("ghp_def"); + + const header = redactBody("Authorization: Bearer eyJhbGciOi"); + expect(header.reasons).toContain("contains sensitive key"); + expect(header.text).toBe("Authorization=[redacted]"); + }); + test("refuses log dumps", () => { const result = redactBody( [ diff --git a/orchestrate/skills/orchestrate/scripts/core/redact-body.ts b/orchestrate/skills/orchestrate/scripts/core/redact-body.ts index 4623f1074..f309a32bf 100644 --- a/orchestrate/skills/orchestrate/scripts/core/redact-body.ts +++ b/orchestrate/skills/orchestrate/scripts/core/redact-body.ts @@ -1,10 +1,16 @@ const MAX_BODY_CHARS = 2_048; const SENSITIVE_KEY_RE = /token|secret|password|api[_-]?key|authorization/i; +// The key may carry a prefix or suffix (GITHUB_TOKEN, slack-bot-token, +// api_key_prod); `\b` around the bare word missed all of those. The value +// runs to the end of the line so "Authorization: Bearer xyz" loses xyz too, +// not only the scheme word. const SENSITIVE_ASSIGNMENT_RE = - /\b(token|secret|password|api[_-]?key|authorization)\b\s*[:=]\s*\S+/gi; + /\b([\w-]*(?:token|secret|password|api[_-]?key|authorization)[\w-]*)\s*[:=][ \t]*[^\n]+/gi; +// A path is redacted wherever it appears, not only at a line start. The +// lookbehind keeps URL segments such as "https://host/Users/x" out. const PATH_PATTERNS = [ - { re: /^\/workspace\/\S*/gm, reason: "contains /workspace path" }, - { re: /^\/Users\/\S*/gm, reason: "contains /Users path" }, + { re: /(? Date: Tue, 27 Oct 2026 08:03:41 +0300 Subject: [PATCH 16/18] fix(orchestrate): serialise the required-comment retry queue across callers The queue file was read, changed and written back by whoever posted a required comment, with no lock. Two callers that overlap (the orchestrator and a local worker, or two local workers at a handoff) each wrote their own snapshot; the later write dropped the other's entry, and postOrQueueComment then reported "posted" for a comment that was in neither the file nor Slack. A mkdir lock now covers enqueue, drain and the final read; a holder that dies leaves a lock that is taken over after 30 s. Test fails on main. --- .../comment-retry-queue-lock.test.ts | 78 ++++++++++++ .../scripts/core/comment-retry-queue.ts | 118 ++++++++++++++---- 2 files changed, 175 insertions(+), 21 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue-lock.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue-lock.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue-lock.test.ts new file mode 100644 index 000000000..5ef51d0df --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue-lock.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import type { SlackAdapter, SlackMessageRef } from "../adapters/types.ts"; +import { + commentRetryQueuePath, + postOrQueueComment, +} from "../core/comment-retry-queue.ts"; + +const TEST_SLACK_CHANNEL = "C123TEST"; + +// Slack is down for everyone. The first post hangs for a moment so that a +// second caller reads and writes the queue file while the first is waiting. +function slowThenFailingSlack(): SlackAdapter { + let calls = 0; + const notUsed = async (): Promise => { + throw new Error("not used"); + }; + return { + postRunKickoff: notUsed, + lookupFirstNameByEmail: async () => undefined, + postInThread: notUsed, + editThreadMessage: notUsed, + uploadFileToThread: notUsed, + getReactions: notUsed, + getThreadReplies: notUsed, + async postCommentInThread(): Promise { + calls++; + if (calls === 1) await new Promise(resolve => setTimeout(resolve, 60)); + throw new Error("slack_unavailable"); + }, + addReaction: notUsed, + removeReaction: notUsed, + }; +} + +describe("required comment retry queue under concurrent callers", () => { + // Two required comments posted at the same time, as two local workers (or the + // orchestrator and a worker) do at a handoff. + test("Neither entry is lost and neither caller reports a comment it never delivered", async () => { + const workspace = mkdtempSync(join(tmpdir(), "orch-comment-queue-lock-")); + try { + const slack = slowThenFailingSlack(); + const destination = `slack:${TEST_SLACK_CHANNEL}:111.222`; + const first = postOrQueueComment({ + destinations: { slack }, + workspace, + destination, + body: "first worker is done", + sender: "worker-1", + criticality: "required", + }); + await new Promise(resolve => setTimeout(resolve, 10)); + const second = postOrQueueComment({ + destinations: { slack }, + workspace, + destination, + body: "second worker is done", + sender: "worker-2", + criticality: "required", + }); + const results = await Promise.all([first, second]); + + const queue = JSON.parse( + readFileSync(commentRetryQueuePath(workspace), "utf8") + ) as { body: string }[]; + expect(queue.map(entry => entry.body).sort()).toEqual([ + "first worker is done", + "second worker is done", + ]); + expect(results).toEqual(["queued", "queued"]); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts index 2172748b8..c387ff3ff 100644 --- a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts +++ b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts @@ -3,13 +3,27 @@ import { mkdirSync, readFileSync, renameSync, + rmSync, + statSync, writeFileSync, } from "node:fs"; import { dirname, join, resolve } from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; import type { CommentCriticality, SlackAdapter } from "../adapters/types.ts"; const COMMENT_RETRY_BACKOFF_MS = [1_000, 5_000, 30_000, 300_000, 1_800_000]; +// The queue file is read, changed and written back by whoever posts a +// required comment: the orchestrator, and every local worker that runs +// `comment --criticality required` in the same workspace. Without a lock, two +// callers that overlap (one is still waiting on Slack while the other reads +// the file) each write their own snapshot, the later write drops the other's +// entry, and `postOrQueueComment` then reports "posted" for a comment that +// is in neither the file nor Slack. `mkdir` is atomic on every platform, so a +// directory is the lock. +const QUEUE_LOCK_WAIT_MS = 5_000; +const QUEUE_LOCK_POLL_MS = 10; +const QUEUE_LOCK_STALE_MS = 30_000; interface CommentRetryEntry { id: string; @@ -62,28 +76,34 @@ export async function postOrQueueComment(args: { if (!args.workspace) { throw new Error("--criticality required needs --workspace for retry state"); } - const entryId = enqueueComment({ - workspace: args.workspace, - destination: args.destination, - body: args.body, - sender: args.sender, - }); - await drainCommentRetryQueue({ - workspace: args.workspace, - destinations: args.destinations, - logAttention: args.logAttention, - maxAttemptsThisDrain: 1, - allowedSlackThread: args.allowedSlackThread, + const workspace = args.workspace; + // Enqueue, drain and the "did ours land" read happen under one lock, so a + // caller that overlaps with another neither loses its entry nor reads a + // file the other caller is about to overwrite. + return withQueueLock(commentRetryQueuePath(workspace), async () => { + const entryId = enqueueComment({ + workspace, + destination: args.destination, + body: args.body, + sender: args.sender, + }); + await drainLocked({ + workspace, + destinations: args.destinations, + logAttention: args.logAttention, + maxAttemptsThisDrain: 1, + allowedSlackThread: args.allowedSlackThread, + }); + // Drain processes FIFO with maxAttemptsThisDrain=1, so an older queued + // entry can consume the single attempt and leave ours pending. A successful + // post removes the entry from the file, so absence means we landed. + // Exhausted-on-disk counts as "queued"; the comment never delivered, so + // claiming "posted" would be a worse lie. + const stillQueued = readQueue(commentRetryQueuePath(workspace)).some( + entry => entry.id === entryId + ); + return stillQueued ? "queued" : "posted"; }); - // Drain processes FIFO with maxAttemptsThisDrain=1, so an older queued - // entry can consume the single attempt and leave ours pending. A successful - // post removes the entry from the file, so absence means we landed. - // Exhausted-on-disk counts as "queued"; the comment never delivered, so - // claiming "posted" would be a worse lie. - const stillQueued = readQueue(commentRetryQueuePath(args.workspace)).some( - entry => entry.id === entryId - ); - return stillQueued ? "queued" : "posted"; } export async function drainCommentRetryQueue(args: { @@ -96,6 +116,20 @@ export async function drainCommentRetryQueue(args: { * catches destinations written directly into `comment-retry-queue.json`. */ allowedSlackThread?: { channel: string; threadTs: string }; +}): Promise<{ posted: number; queued: number }> { + if (!args.destinations) return { posted: 0, queued: 0 }; + return withQueueLock(commentRetryQueuePath(args.workspace), () => + drainLocked(args) + ); +} + +/** `drainCommentRetryQueue` with the lock already held by the caller. */ +async function drainLocked(args: { + workspace: string; + destinations?: CommentDestinations; + logAttention?: (line: string) => void; + maxAttemptsThisDrain?: number; + allowedSlackThread?: { channel: string; threadTs: string }; }): Promise<{ posted: number; queued: number }> { if (!args.destinations) return { posted: 0, queued: 0 }; const path = commentRetryQueuePath(args.workspace); @@ -196,6 +230,48 @@ function enqueueComment(args: { return id; } +async function withQueueLock( + queuePath: string, + fn: () => Promise +): Promise { + const lockDir = `${queuePath}.lock`; + mkdirSync(dirname(lockDir), { recursive: true }); + const giveUpAt = Date.now() + QUEUE_LOCK_WAIT_MS; + for (;;) { + try { + mkdirSync(lockDir); + break; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err; + // A holder that died mid-drain leaves the directory behind; past the + // stale window the lock is taken over rather than waited on forever. + if (lockAgeMs(lockDir) > QUEUE_LOCK_STALE_MS) { + rmSync(lockDir, { recursive: true, force: true }); + continue; + } + if (Date.now() >= giveUpAt) { + throw new Error( + `comment retry queue is locked by another process (${lockDir}); try again` + ); + } + await sleep(QUEUE_LOCK_POLL_MS); + } + } + try { + return await fn(); + } finally { + rmSync(lockDir, { recursive: true, force: true }); + } +} + +function lockAgeMs(lockDir: string): number { + try { + return Date.now() - statSync(lockDir).mtimeMs; + } catch { + return 0; + } +} + function readQueue(path: string): CommentRetryEntry[] { let content: string; try { From e892e6a1020a59326cc363a1fbe079d22e13de3e Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Mon, 26 Oct 2026 20:20:00 +0300 Subject: [PATCH 17/18] fix(orchestrate): do not wait on a dead SSE stream after the watchdog has the result waitAndHandoff raced run.wait() against an Agent.getRun poll so a wedged stream could not hang the reconcile loop, then awaited the stream reader unconditionally. When the poll won, run.stream() was still parked on the dead connection and never ended, so the task never reached handed-off and the watchdog bought nothing. The stream now gets a 2 s grace to deliver what it has and the loop moves on with the authoritative result. The poll interval is overridable via ORCHESTRATE_WATCHDOG_POLL_MS so the test can run in seconds; the default is unchanged. --- .../wait-handoff-dead-stream.test.ts | 107 ++++++++++++++++++ .../orchestrate/scripts/core/agent-manager.ts | 23 +++- 2 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/wait-handoff-dead-stream.test.ts diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/wait-handoff-dead-stream.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/wait-handoff-dead-stream.test.ts new file mode 100644 index 000000000..9bad5b1ca --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/wait-handoff-dead-stream.test.ts @@ -0,0 +1,107 @@ +import { afterAll, beforeAll, describe, expect, mock, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// A run whose SSE has died: `stream()` never yields again and never ends, and +// `wait()` (which rides the same stream) never settles. The REST side has +// already moved the run to `finished`, which is what the watchdog poll sees. + +const SDK_MOCK = "@cursor/sdk"; + +const finishedResult = { + id: "run-dead-sse", + status: "finished" as const, + result: "## Status\nhanded-off\n\n## Branch\nagent/worker-one\n", + durationMs: 10_000, + git: { branches: [{ branch: "agent/worker-one" }] }, +}; + +const never = () => new Promise(() => {}); + +function deadStreamRun(status: "running" | "finished") { + return { + id: "run-dead-sse", + agentId: "bc-dead-sse", + status, + stream: async function* () { + yield { + type: "assistant", + message: { content: [{ type: "text", text: "working..." }] }, + }; + await never(); + }, + wait: status === "finished" ? async () => finishedResult : never, + }; +} + +mock.module(SDK_MOCK, () => ({ + Agent: { + create: async () => ({ + agentId: "bc-dead-sse", + send: async () => deadStreamRun("running"), + [Symbol.asyncDispose]: () => Promise.resolve(), + }), + // The poll sees the server-side truth. + getRun: async () => deadStreamRun("finished"), + }, + CursorAgentError: class CursorAgentError extends Error {}, +})); + +const { AgentManager, __resetSDKForTests } = await import( + "../core/agent-manager.ts" +); +__resetSDKForTests(); + +const ORIGINAL_API_KEY = process.env.CURSOR_API_KEY; +const ORIGINAL_POLL = process.env.ORCHESTRATE_WATCHDOG_POLL_MS; + +beforeAll(() => { + process.env.CURSOR_API_KEY = "test-key"; + process.env.ORCHESTRATE_WATCHDOG_POLL_MS = "50"; +}); + +afterAll(() => { + if (ORIGINAL_API_KEY === undefined) delete process.env.CURSOR_API_KEY; + else process.env.CURSOR_API_KEY = ORIGINAL_API_KEY; + if (ORIGINAL_POLL === undefined) + delete process.env.ORCHESTRATE_WATCHDOG_POLL_MS; + else process.env.ORCHESTRATE_WATCHDOG_POLL_MS = ORIGINAL_POLL; +}); + +describe("waitAndHandoff with a dead SSE stream", () => { + test("Hands the task off from the watchdog's result instead of waiting on the stream forever", async () => { + const workspace = mkdtempSync(join(tmpdir(), "orch-dead-sse-")); + writeFileSync( + join(workspace, "plan.json"), + JSON.stringify({ + goal: "survive a dead stream", + rootSlug: "dead-sse", + baseBranch: "main", + repoUrl: "https://github.com/example-org/example-repo", + tasks: [ + { name: "worker-one", type: "worker", scopedGoal: "Do the work." }, + ], + }) + ); + try { + const mgr = await AgentManager.load(workspace); + const def = mgr.plan.tasks?.find(t => t.name === "worker-one"); + if (!def) throw new Error("missing task def"); + const spawned = await mgr.spawnTask(def); + if (!spawned) throw new Error("spawn failed"); + + const outcome = await Promise.race([ + mgr.waitAndHandoff(spawned).then(() => "handed-off" as const), + new Promise<"hung">(resolve => + setTimeout(() => resolve("hung"), 8_000) + ), + ]); + + expect(outcome).toBe("handed-off"); + expect(mgr.getTask("worker-one")?.status).toBe("handed-off"); + } finally { + rmSync(workspace, { recursive: true, force: true }); + } + }, 15_000); +}); diff --git a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts index 1b28da226..e4c5370b9 100644 --- a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts +++ b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts @@ -41,6 +41,7 @@ import type { CommentDestinations } from "./comment-retry-queue.ts"; const SPAWN_MAX_ATTEMPTS = 3; const SPAWN_RETRY_BACKOFF_MS = 2_000; +// Overridable for tests via ORCHESTRATE_WATCHDOG_POLL_MS. const WAIT_WATCHDOG_POLL_INTERVAL_MS = 60_000; // Internal watchdog "noticed idle" debug logs. Lower threshold = earlier // visibility into stalled SSE / tool-call streams without disturbing the @@ -55,6 +56,10 @@ const STUCK_ESCALATION_MS = 1_800_000; // still reports running. Without it, state.attention and saveState frequency // would blow up. Negligible cost when retries take ~1-2 minutes. const WAIT_RECOVERY_RETRY_BACKOFF_MS = 5_000; +// How long waitAndHandoff waits for `run.stream()` to end after `run.wait()` +// (or the watchdog poll) has already produced the result. A live stream ends +// within milliseconds of the run; a dead one never does. +const WAIT_STREAM_DRAIN_GRACE_MS = 2_000; export type RespawnSource = "local-cli" | "self-planner" | "script-auto-retry"; @@ -883,7 +888,12 @@ export class AgentManager { lastWaitError = errorMessage(waitErr); throw waitErr; } - await streamPromise; + // When the watchdog poll won the race, `run.stream()` is still parked + // on a dead SSE and never ends; awaiting it here would hang the + // reconcile loop that the watchdog exists to free. `rr` is + // authoritative either way, so the stream gets a short grace to + // deliver what it already has, then the loop moves on without it. + await Promise.race([streamPromise, sleep(WAIT_STREAM_DRAIN_GRACE_MS)]); if (rr.status !== "error") break; lastWaitError = runResultErrorMessage(rr) ?? lastWaitError; @@ -1864,7 +1874,7 @@ async function waitRunWithWatchdog(args: { const pollLoop = async (): Promise => { while (!ac.signal.aborted) { try { - await sleep(WAIT_WATCHDOG_POLL_INTERVAL_MS, undefined, { + await sleep(watchdogPollIntervalMs(), undefined, { signal: ac.signal, }); } catch { @@ -2018,6 +2028,15 @@ export function formatToolCallIdleWarning(args: { return `${args.taskLabel}: tool_call idle ${args.idleMs}ms; last=${lastCall}`; } +function watchdogPollIntervalMs(): number { + const raw = process.env.ORCHESTRATE_WATCHDOG_POLL_MS; + if (!raw) return WAIT_WATCHDOG_POLL_INTERVAL_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 + ? parsed + : WAIT_WATCHDOG_POLL_INTERVAL_MS; +} + function toolCallStuckThresholdMs(): number { const raw = process.env.ORCHESTRATE_TOOL_CALL_IDLE_MS; if (!raw) return STUCK_ESCALATION_MS; From 8a3e024c3041d1221fb29bcfef4d5d7498eb67e8 Mon Sep 17 00:00:00 2001 From: Dean Rie Date: Tue, 27 Oct 2026 08:06:51 +0300 Subject: [PATCH 18/18] style(orchestrate): biome format and import order for the bundled files --- .../__tests__/andon-root-cache.test.ts | 114 ++++++++++++------ .../__tests__/comment-retry-queue.test.ts | 4 +- .../__tests__/handoff-branch-parser.test.ts | 4 +- .../__tests__/handoff-empty-canonical.test.ts | 18 ++- .../handoff-verification-parser.test.ts | 43 +++++-- .../scripts/__tests__/inspect-timer.test.ts | 59 ++++++--- .../__tests__/kickoff-pagination.test.ts | 34 ++++-- .../__tests__/kill-tree-handed-off.test.ts | 53 ++++++-- .../__tests__/measurements-compare.test.ts | 12 +- .../__tests__/measurements-exponents.test.ts | 6 +- .../skills/orchestrate/scripts/cli/task.ts | 6 +- .../orchestrate/scripts/measurements.ts | 4 +- 12 files changed, 262 insertions(+), 95 deletions(-) diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts index f23f989e0..6f797f9ee 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/andon-root-cache.test.ts @@ -127,58 +127,96 @@ describe("SlackReactionAndonSource", () => { test("Finds the newest reason beyond the first reply page", async () => { const cursors: (string | undefined)[] = []; const latestBounds: (string | undefined)[] = []; - const source = new SlackReactionAndonSource({ - ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), - async getThreadReplies(args) { - cursors.push(args.cursor); - latestBounds.push(args.latest); - return args.cursor === undefined ? { - messages: [ - { ts: "111.222", text: "🚨 ANDON RAISED by parent: ignore parent" }, - { ts: "111.300", text: "🚨 ANDON RAISED by operator: old reason" }, - ], - nextCursor: "page-two", - } : { - messages: [ - { ts: "111.400", text: ":rotating_light: ANDON RAISED by operator: newest reason" }, - { ts: "111.500", text: "unrelated newest reply" }, - ], - }; + const source = new SlackReactionAndonSource( + { + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + cursors.push(args.cursor); + latestBounds.push(args.latest); + return args.cursor === undefined + ? { + messages: [ + { + ts: "111.222", + text: "🚨 ANDON RAISED by parent: ignore parent", + }, + { + ts: "111.300", + text: "🚨 ANDON RAISED by operator: old reason", + }, + ], + nextCursor: "page-two", + } + : { + messages: [ + { + ts: "111.400", + text: ":rotating_light: ANDON RAISED by operator: newest reason", + }, + { ts: "111.500", text: "unrelated newest reply" }, + ], + }; + }, }, - }, { channel: "C123", ts: "111.222" }); - await expect(source.snapshot()).resolves.toMatchObject({ active: true, reason: "newest reason" }); + { channel: "C123", ts: "111.222" } + ); + await expect(source.snapshot()).resolves.toMatchObject({ + active: true, + reason: "newest reason", + }); expect(cursors).toEqual([undefined, "page-two"]); expect(latestBounds[0]).toBe(latestBounds[1]); }); test("Applies the recent reply window across pages", async () => { - const source = new SlackReactionAndonSource({ - ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), - async getThreadReplies(args) { - return args.cursor === undefined ? { - messages: [{ ts: "111.300", text: "🚨 ANDON RAISED by operator: outside recent window" }], - nextCursor: "page-two", - } : { - messages: Array.from({ length: 20 }, (_, i) => ({ ts: `112.${100 + i}`, text: "unrelated" })), - }; + const source = new SlackReactionAndonSource( + { + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + return args.cursor === undefined + ? { + messages: [ + { + ts: "111.300", + text: "🚨 ANDON RAISED by operator: outside recent window", + }, + ], + nextCursor: "page-two", + } + : { + messages: Array.from({ length: 20 }, (_, i) => ({ + ts: `112.${100 + i}`, + text: "unrelated", + })), + }; + }, }, - }, { channel: "C123", ts: "111.222" }); + { channel: "C123", ts: "111.222" } + ); const state = await source.snapshot(); expect(state.active).toBe(true); expect(state).not.toHaveProperty("reason", "outside recent window"); }); test("Keeps Andon active when a later reply page fails", async () => { - const source = new SlackReactionAndonSource({ - ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), - async getThreadReplies(args) { - if (args.cursor) throw new Error("slack_replies_unavailable"); - return { - messages: [{ ts: "111.300", text: "🚨 ANDON RAISED by operator: incomplete history" }], - nextCursor: "page-two", - }; + const source = new SlackReactionAndonSource( + { + ...slackWithReactions([{ name: "rotating_light", users: ["U123"] }]), + async getThreadReplies(args) { + if (args.cursor) throw new Error("slack_replies_unavailable"); + return { + messages: [ + { + ts: "111.300", + text: "🚨 ANDON RAISED by operator: incomplete history", + }, + ], + nextCursor: "page-two", + }; + }, }, - }, { channel: "C123", ts: "111.222" }); + { channel: "C123", ts: "111.222" } + ); const state = await source.snapshot(); expect(state.active).toBe(true); expect(state).not.toHaveProperty("reason", "incomplete history"); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts index 82b121cb0..04381001b 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/comment-retry-queue.test.ts @@ -419,7 +419,9 @@ describe("invalid persisted required comment queues", () => { ]) { for (const operation of ["enqueue", "drain"] as const) { test(`${operation} preserves ${name} without posting`, async () => { - const workspace = mkdtempSync(join(tmpdir(), "orch-comment-corruption-")); + const workspace = mkdtempSync( + join(tmpdir(), "orch-comment-corruption-") + ); const posts: SlackPost[] = []; const queuePath = commentRetryQueuePath(workspace); try { diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts index 4896ed2a8..314eaeb5b 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-branch-parser.test.ts @@ -4,7 +4,9 @@ import { parseHandoffBranch, resolveRunBranch } from "../core/handoff.ts"; describe("parseHandoffBranch / resolveRunBranch", () => { test("A branch written as a one-item list is read", () => { - const body = ["## Branch", "- `agent/listed-abc1`", "## What I did"].join("\n"); + const body = ["## Branch", "- `agent/listed-abc1`", "## What I did"].join( + "\n" + ); expect(parseHandoffBranch(body)).toBe("agent/listed-abc1"); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts index 16c36439e..4d7f67c8a 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-empty-canonical.test.ts @@ -3,15 +3,25 @@ import { parseHandoffVerification } from "../core/handoff.ts"; describe("canonical verification remains authoritative when empty", () => { test("empty canonical verdict cannot inherit legacy pass", () => { - expect(parseHandoffVerification("## Verification\n\n## Verdict\npass\n")).toBeNull(); + expect( + parseHandoffVerification("## Verification\n\n## Verdict\npass\n") + ).toBeNull(); }); test("empty terminal canonical section cannot inherit earlier legacy pass", () => { - expect(parseHandoffVerification("## Verdict\npass\n\n## Verification\n")).toBeNull(); + expect( + parseHandoffVerification("## Verdict\npass\n\n## Verification\n") + ).toBeNull(); }); test("legacy-only verdict remains supported", () => { - expect(parseHandoffVerification("## Verdict\npass\n")).toBe("type-check-only"); + expect(parseHandoffVerification("## Verdict\npass\n")).toBe( + "type-check-only" + ); }); test("recognized canonical verdict retains precedence", () => { - expect(parseHandoffVerification("## Verification\nverifier-blocked\n\n## Verdict\npass\n")).toBe("verifier-blocked"); + expect( + parseHandoffVerification( + "## Verification\nverifier-blocked\n\n## Verdict\npass\n" + ) + ).toBe("verifier-blocked"); }); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts index 5168b0c0c..a08a2ec65 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/handoff-verification-parser.test.ts @@ -38,15 +38,21 @@ describe("parseHandoffVerification", () => { }); test("A one-item list or bold around the value is stripped", () => { - expect(parseHandoffVerification(["## Verification", "- unit-test-verified", ""].join("\n"))).toBe( - "unit-test-verified" - ); - expect(parseHandoffVerification(["## Verification", "* `live-ui-verified`", ""].join("\n"))).toBe( - "live-ui-verified" - ); - expect(parseHandoffVerification(["## Verification", "**type-check-only**", ""].join("\n"))).toBe( - "type-check-only" - ); + expect( + parseHandoffVerification( + ["## Verification", "- unit-test-verified", ""].join("\n") + ) + ).toBe("unit-test-verified"); + expect( + parseHandoffVerification( + ["## Verification", "* `live-ui-verified`", ""].join("\n") + ) + ).toBe("live-ui-verified"); + expect( + parseHandoffVerification( + ["## Verification", "**type-check-only**", ""].join("\n") + ) + ).toBe("type-check-only"); }); test("Unknown value returns null without falling through to legacy", () => { @@ -115,7 +121,11 @@ describe("parseHandoffFailureMode", () => { describe("parseHandoffPrNumber", () => { test("Explicit PR sections win over dependency links", () => { for (const heading of ["PR", "Pull Request"]) { - for (const value of ["#42", "https://github.com/example/project/pull/42", ""]) { + for (const value of [ + "#42", + "https://github.com/example/project/pull/42", + "", + ]) { const body = `## What I did\nFollowed https://github.com/example/dependency/pull/7\n## ${heading}\n${value}`; expect(parseHandoffPrNumber(body)).toBe(42); } @@ -130,7 +140,12 @@ describe("parseHandoffPrNumber", () => { }); test("Empty trailing PR headings do not inherit dependency links", () => { - for (const heading of ["## PR", "## Pull Request", "##\tPR ", "## Pull Request\t"]) { + for (const heading of [ + "## PR", + "## Pull Request", + "##\tPR ", + "## Pull Request\t", + ]) { const body = `## What I did\nFollowed https://github.com/example/dependency/pull/7\n${heading}`; expect(parseHandoffPrNumber(body)).toBe(null); expect(parseHandoffPrNumber(body.trim())).toBe(null); @@ -138,12 +153,14 @@ describe("parseHandoffPrNumber", () => { }); test("Canonical section whitespace and CRLF match the section reader", () => { - const body = "## What I did\r\nhttps://github.com/example/dependency/pull/7\r\n##\tPR \r\n\r\n`#42`\r\n"; + const body = + "## What I did\r\nhttps://github.com/example/dependency/pull/7\r\n##\tPR \r\n\r\n`#42`\r\n"; expect(parseHandoffPrNumber(body)).toBe(42); }); test("Lowercase headings retain the legacy URL fallback", () => { - const body = "## pr\n#42\n## References\nhttps://github.com/example/dependency/pull/7"; + const body = + "## pr\n#42\n## References\nhttps://github.com/example/dependency/pull/7"; expect(parseHandoffPrNumber(body)).toBe(7); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts index ecaed8691..a69338a1e 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/inspect-timer.test.ts @@ -3,18 +3,25 @@ import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; test("completed stream inspection releases its deadline timers", () => { - const child = spawnSync(process.execPath, ["-e", ` + const child = spawnSync( + process.execPath, + [ + "-e", + ` import { inspectRunStream } from "./core/agent-manager.ts"; const run = { stream: async function* () { yield { type: "assistant", message: { content: [{ type:"text", text:"done" }] } }; } }; const result = await inspectRunStream({ run, task:"one", agentId:"a", runId:"r", timeoutMs:30000 }); console.log(JSON.stringify(result)); - `], { - cwd: fileURLToPath(new URL("../", import.meta.url)), - encoding: "utf8", - timeout: 2000, - }); + `, + ], + { + cwd: fileURLToPath(new URL("../", import.meta.url)), + encoding: "utf8", + timeout: 2000, + } + ); expect(child.stdout).toContain('"last_assistant_text_snippet":"done"'); expect(child.error).toBeUndefined(); expect(child.status).toBe(0); @@ -25,12 +32,20 @@ test("silent inspection still reaches its deadline and closes the iterator", asy let returned = false; const iterator = { next: () => new Promise>(() => {}), - return: async () => { returned = true; return { done: true as const, value: undefined }; }, - [Symbol.asyncIterator]() { return this; }, + return: async () => { + returned = true; + return { done: true as const, value: undefined }; + }, + [Symbol.asyncIterator]() { + return this; + }, }; const result = await inspectRunStream({ run: { stream: () => iterator } as never, - task: "silent", agentId: "a", runId: "r", timeoutMs: 25, + task: "silent", + agentId: "a", + runId: "r", + timeoutMs: 25, }); expect(returned).toBe(true); expect(result.tool_calls_total).toBe(0); @@ -42,13 +57,25 @@ test("stream rejection is preserved and the iterator is closed", async () => { const { inspectRunStream } = await import("../core/agent-manager.ts"); let returned = false; const iterator = { - next: async () => { throw new Error("stream failed"); }, - return: async () => { returned = true; return { done: true as const, value: undefined }; }, - [Symbol.asyncIterator]() { return this; }, + next: async () => { + throw new Error("stream failed"); + }, + return: async () => { + returned = true; + return { done: true as const, value: undefined }; + }, + [Symbol.asyncIterator]() { + return this; + }, }; - await expect(inspectRunStream({ - run: { stream: () => iterator } as never, - task: "failed", agentId: "a", runId: "r", timeoutMs: 30000, - })).rejects.toThrow("stream failed"); + await expect( + inspectRunStream({ + run: { stream: () => iterator } as never, + task: "failed", + agentId: "a", + runId: "r", + timeoutMs: 30000, + }) + ).rejects.toThrow("stream failed"); expect(returned).toBe(true); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts index a97aa2a5d..3e0291875 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/kickoff-pagination.test.ts @@ -3,7 +3,9 @@ import { findActiveRootPlanner } from "../cli/task.ts"; const now = Date.parse("2026-10-16T16:00:00Z"); const matching = { - agentId: "matching", name: "refactor-root", createdAt: now - 1000, + agentId: "matching", + name: "refactor-root", + createdAt: now - 1000, latestRun: { id: "run-matching", status: "running" }, }; @@ -12,10 +14,12 @@ test("finds an active matching planner on a later list page", async () => { const api = { async list(opts: { cursor?: string }) { cursors.push(opts.cursor); - return opts.cursor ? { items: [matching] } : { - items: [{ ...matching, agentId: "other", name: "different-root" }], - nextCursor: "next-page", - }; + return opts.cursor + ? { items: [matching] } + : { + items: [{ ...matching, agentId: "other", name: "different-root" }], + nextCursor: "next-page", + }; }, }; const result = await findActiveRootPlanner(api, "refactor", now); @@ -26,14 +30,24 @@ test("finds an active matching planner on a later list page", async () => { test("follows cursors even when an intermediate page is empty", async () => { const api = { async list(opts: { cursor?: string }) { - return opts.cursor ? { items: [matching] } : { items: [], nextCursor: "next-page" }; + return opts.cursor + ? { items: [matching] } + : { items: [], nextCursor: "next-page" }; }, }; - expect((await findActiveRootPlanner(api, "refactor", now))?.runId).toBe("run-matching"); + expect((await findActiveRootPlanner(api, "refactor", now))?.runId).toBe( + "run-matching" + ); }); test("an exhausted empty listing has no active planner", async () => { - expect(await findActiveRootPlanner({ list: async () => ({ items: [] }) }, "refactor", now)).toBeNull(); + expect( + await findActiveRootPlanner( + { list: async () => ({ items: [] }) }, + "refactor", + now + ) + ).toBeNull(); }); test("propagates later-page lookup failures instead of authorizing a duplicate", async () => { @@ -43,5 +57,7 @@ test("propagates later-page lookup failures instead of authorizing a duplicate", return { items: [], nextCursor: "next-page" }; }, }; - await expect(findActiveRootPlanner(api, "refactor", now)).rejects.toThrow("list unavailable"); + await expect(findActiveRootPlanner(api, "refactor", now)).rejects.toThrow( + "list unavailable" + ); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts index d7f000064..6b38979c2 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/kill-tree-handed-off.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { execFileSync } from "node:child_process"; -import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -21,7 +21,11 @@ function git(repo: string, ...args: string[]): string { * for a root planner and one subplanner. The root marks the subplanner * `handed-off`; the subplanner's own branch still has a `running` child. */ -function fixtureRepo(): { repoPath: string; rootBranch: string; rootSlug: string } { +function fixtureRepo(): { + repoPath: string; + rootBranch: string; + rootSlug: string; +} { const repoPath = mkdtempSync(join(tmpdir(), "orch-kill-tree-")); git(repoPath, "init", "-q", "-b", "main"); git(repoPath, "config", "user.email", "t@example.com"); @@ -73,20 +77,53 @@ function fixtureRepo(): { repoPath: string; rootBranch: string; rootSlug: string describe("kill-tree walks handed-off subplanners like crawl", () => { test("descendsIntoSubplanner follows running and handed-off subplanners only", () => { const base = { name: "x", agentId: null, runId: null, parentAgentId: null }; - expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "running" } as never)).toBe(true); - expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "handed-off" } as never)).toBe(true); - expect(descendsIntoSubplanner({ ...base, type: "subplanner", status: "done" } as never)).toBe(false); - expect(descendsIntoSubplanner({ ...base, type: "worker", status: "running" } as never)).toBe(false); + expect( + descendsIntoSubplanner({ + ...base, + type: "subplanner", + status: "running", + } as never) + ).toBe(true); + expect( + descendsIntoSubplanner({ + ...base, + type: "subplanner", + status: "handed-off", + } as never) + ).toBe(true); + expect( + descendsIntoSubplanner({ + ...base, + type: "subplanner", + status: "done", + } as never) + ).toBe(false); + expect( + descendsIntoSubplanner({ + ...base, + type: "worker", + status: "running", + } as never) + ).toBe(false); }); test("a running agent under a handed-off subplanner is both crawled and collected", () => { const { repoPath, rootBranch, rootSlug } = fixtureRepo(); const out: string[] = []; - crawlBranch({ repoPath, branch: rootBranch, slug: rootSlug }, 0, out, new Set()); + crawlBranch( + { repoPath, branch: rootBranch, slug: rootSlug }, + 0, + out, + new Set() + ); expect(out.join("\n")).toContain("verifier"); - const victims = collectRunningAgentsInTree({ repoPath, branch: rootBranch, slug: rootSlug }); + const victims = collectRunningAgentsInTree({ + repoPath, + branch: rootBranch, + slug: rootSlug, + }); expect(victims.map(v => v.agentId)).toEqual(["agent-verifier"]); expect(victims[0]?.branch).toBe(`orch/${rootSlug}/child-planner`); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts index e3d580779..c290e486c 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-compare.test.ts @@ -54,10 +54,18 @@ describe("compareMeasurement / applyMeasurementParser / buildMeasurementEnv", () ["0 s", "0.09 s", "match"], ["0 s", "0.2 s", "value-mismatch"], ] as const) { - const result = compareMeasurement({ spec: baseSpec, measured, claim: claim(after) }); + const result = compareMeasurement({ + spec: baseSpec, + measured, + claim: claim(after), + }); expect(result.outcome).toBe(outcome); } - const result = compareMeasurement({ spec: baseSpec, measured: "0.1 s", claim: claim("0.01 s") }); + const result = compareMeasurement({ + spec: baseSpec, + measured: "0.1 s", + claim: claim("0.01 s"), + }); expect(result.driftFraction).toBeCloseTo(9); }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts index d998384a1..e28848c18 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/measurements-exponents.test.ts @@ -2,7 +2,11 @@ import { expect, test } from "bun:test"; import { compareMeasurement } from "../measurements.ts"; function check(measured: string, claimed: string) { - return compareMeasurement({ spec: { name: "bytes", command: "printf metric" }, measured, claim: { name: "bytes", before: "0", op: "→", after: claimed } }); + return compareMeasurement({ + spec: { name: "bytes", command: "printf metric" }, + measured, + claim: { name: "bytes", before: "0", op: "→", after: claimed }, + }); } test("native large-number output uses numeric tolerance", () => { diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 45f2cb276..b052a7cfa 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -32,7 +32,11 @@ export const MAX_BOOT_MS = 30 * 60 * 1000; const ACTIVE_ROOT_RUN_STATUSES = new Set(["pending", "running"]); type AgentListApi = { - list: (opts: { runtime: "cloud"; limit: number; cursor?: string }) => Promise; + list: (opts: { + runtime: "cloud"; + limit: number; + cursor?: string; + }) => Promise; listRuns?: ( agentId: string, opts: { runtime: "cloud"; limit: number } diff --git a/orchestrate/skills/orchestrate/scripts/measurements.ts b/orchestrate/skills/orchestrate/scripts/measurements.ts index f9d00a843..726c48a6e 100644 --- a/orchestrate/skills/orchestrate/scripts/measurements.ts +++ b/orchestrate/skills/orchestrate/scripts/measurements.ts @@ -207,7 +207,9 @@ interface NumericWithUnit { * fall through to string comparison instead of pretending it's numeric. */ function parseNumericWithUnit(value: string): NumericWithUnit | null { - const match = /^([-+]?\d+(?:\.\d+)?(?:[eE][-+]?\d+)?)\s*([a-zA-Z%]+)?$/.exec(value.trim()); + const match = /^([-+]?\d+(?:\.\d+)?(?:[eE][-+]?\d+)?)\s*([a-zA-Z%]+)?$/.exec( + value.trim() + ); if (!match) return null; const numeric = Number.parseFloat(match[1]); if (!Number.isFinite(numeric)) return null;