fix(interactive-subagents): harden parallel lifecycle delivery
This commit is contained in:
@@ -158,13 +158,16 @@ async function emitMockEvent(
|
||||
|
||||
type SharedRuntimeTestState = {
|
||||
runningSubagents: Map<string, unknown>;
|
||||
resumingNames: Set<string>;
|
||||
latestCtx: unknown;
|
||||
latestPi: unknown;
|
||||
widgetInterval: ReturnType<typeof setInterval> | null;
|
||||
statusInterval: ReturnType<typeof setInterval> | null;
|
||||
standaloneWidgetSyncScheduled: boolean;
|
||||
pendingDeliveries: { length: number };
|
||||
resultDeliveryStates: Map<string, "queued" | "delivered">;
|
||||
deliveryFlushScheduled: boolean;
|
||||
parentGeneration: number;
|
||||
activeRuntimeToken: symbol | null;
|
||||
registeredRuntimeToken: symbol | null;
|
||||
statusWidgetClaim: unknown;
|
||||
@@ -178,11 +181,14 @@ function resetSharedRuntimeForTest(shared: SharedRuntimeTestState): void {
|
||||
shared.statusInterval = null;
|
||||
shared.standaloneWidgetSyncScheduled = false;
|
||||
shared.runningSubagents.clear();
|
||||
shared.resumingNames.clear();
|
||||
shared.latestCtx = null;
|
||||
shared.latestPi = null;
|
||||
shared.statusWidgetClaim = null;
|
||||
shared.pendingDeliveries.length = 0;
|
||||
shared.resultDeliveryStates.clear();
|
||||
shared.deliveryFlushScheduled = false;
|
||||
shared.parentGeneration = 0;
|
||||
shared.activeRuntimeToken = null;
|
||||
shared.registeredRuntimeToken = null;
|
||||
}
|
||||
@@ -1201,28 +1207,53 @@ describe("subagent discovery", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("resolveEffectiveInteractive defaults to the inverse of auto-exit", () => {
|
||||
// Autonomous agents (auto-exit: true) are NOT interactive — parent gets stall pings.
|
||||
it("defaults unspecified launch behavior to autonomous while preserving explicit interactive profiles", () => {
|
||||
assert.equal(testApi.resolveEffectiveAutoExit({ autoExit: true }), true);
|
||||
assert.equal(testApi.resolveEffectiveAutoExit({ autoExit: false }), false);
|
||||
assert.equal(testApi.resolveEffectiveAutoExit({}), true);
|
||||
assert.equal(testApi.resolveEffectiveAutoExit(null), true);
|
||||
assert.equal(testApi.autoExitEnvAssignment(true), "PI_SUBAGENT_AUTO_EXIT=1");
|
||||
assert.equal(testApi.autoExitEnvAssignment(false), "PI_SUBAGENT_AUTO_EXIT=0");
|
||||
assert.match(testApi.resolveClaudeSentinelFile("child", true), /pi-claude-child-done$/);
|
||||
assert.equal(testApi.resolveClaudeSentinelFile("child", false), undefined);
|
||||
|
||||
// Autonomous agents are not user-driven, so parent status transitions stay enabled.
|
||||
assert.equal(
|
||||
testApi.resolveEffectiveInteractive({ name: "A", task: "T" }, { autoExit: true }),
|
||||
false,
|
||||
);
|
||||
// Agents without auto-exit ARE interactive — parent does not receive status transition pings.
|
||||
// Explicitly non-auto-exit profiles remain interactive.
|
||||
assert.equal(
|
||||
testApi.resolveEffectiveInteractive({ name: "A", task: "T" }, { autoExit: false }),
|
||||
true,
|
||||
);
|
||||
// Missing auto-exit follows the fire-and-forget tool contract.
|
||||
assert.equal(
|
||||
testApi.resolveEffectiveInteractive({ name: "A", task: "T" }, {}),
|
||||
true,
|
||||
false,
|
||||
);
|
||||
// Bare spawn with no agent defs (e.g. /iterate fork) is interactive by default.
|
||||
assert.equal(
|
||||
testApi.resolveEffectiveInteractive({ name: "A", task: "T" }, null),
|
||||
true,
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("resolves relative launch cwd from the parent session rather than the process", () => {
|
||||
const resolved = testApi.resolveSubagentPaths(
|
||||
{ agent: "worker", task: "T", cwd: "roles/reviewer" },
|
||||
{},
|
||||
"/workspace/current-session",
|
||||
);
|
||||
assert.equal(resolved.effectiveCwd, "/workspace/current-session/roles/reviewer");
|
||||
|
||||
const inherited = testApi.resolveSubagentPaths(
|
||||
{ agent: "worker", task: "T" },
|
||||
{},
|
||||
"/workspace/current-session",
|
||||
);
|
||||
assert.equal(inherited.effectiveCwd, null);
|
||||
});
|
||||
|
||||
it("resolveEffectiveInteractive honors explicit frontmatter over the auto-exit default", () => {
|
||||
// Autonomous agent that still wants to be treated as interactive.
|
||||
assert.equal(
|
||||
@@ -1851,6 +1882,42 @@ describe("surface exit signals", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("terminates after repeated liveness failures when a completion-file surface is closed", async () => {
|
||||
await assert.rejects(
|
||||
pollForExit(
|
||||
"closed-surface",
|
||||
new AbortController().signal,
|
||||
{
|
||||
interval: 1,
|
||||
completionFile: "/tmp/nonexistent-subagent-completion",
|
||||
livenessCheck: async () => false,
|
||||
livenessIntervalMs: 0,
|
||||
livenessFailureThreshold: 2,
|
||||
},
|
||||
),
|
||||
/surface closed before completion/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("aborts an in-flight completion-file liveness probe", async () => {
|
||||
const controller = new AbortController();
|
||||
const polling = pollForExit(
|
||||
"blocked-surface",
|
||||
controller.signal,
|
||||
{
|
||||
interval: 1,
|
||||
completionFile: "/tmp/nonexistent-subagent-completion",
|
||||
livenessIntervalMs: 0,
|
||||
livenessCheck: async (_surface: string, signal?: AbortSignal) =>
|
||||
await new Promise<boolean>((_resolve, reject) => {
|
||||
signal?.addEventListener("abort", () => reject(new Error("probe aborted")), { once: true });
|
||||
}),
|
||||
},
|
||||
);
|
||||
setTimeout(() => controller.abort(), 10);
|
||||
await assert.rejects(polling, /aborted/i);
|
||||
});
|
||||
|
||||
it("bounds an asynchronous Orca screen read and kills its CLI process", async () => {
|
||||
const dir = createTestDir();
|
||||
const pidFile = join(dir, "reader.pid");
|
||||
@@ -1966,6 +2033,182 @@ describe("reload lifecycle", () => {
|
||||
beforeEach(() => resetSharedRuntimeForTest(shared));
|
||||
after(() => resetSharedRuntimeForTest(shared));
|
||||
|
||||
it("delivers parallel terminal results exactly once across active and temporarily unbound runtimes", async () => {
|
||||
const activeRuntime = createMockExtensionApi();
|
||||
(subagentsModule as any).default(activeRuntime.api);
|
||||
await emitMockEvent(activeRuntime, "session_start", { reason: "startup" }, ctx);
|
||||
|
||||
const deliver = (name: string) => (pi: any) => {
|
||||
pi.sendMessage({ customType: "subagent_result", content: name, display: true });
|
||||
};
|
||||
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-a", deliver("child-a")), true);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-a", deliver("child-a duplicate")), false);
|
||||
|
||||
let transientAttempts = 0;
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-retry", (pi: any) => {
|
||||
transientAttempts += 1;
|
||||
if (transientAttempts === 1) throw new Error("stale runtime");
|
||||
pi.sendMessage({ customType: "subagent_result", content: "child-retry", display: true });
|
||||
}), true);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-independent", deliver("child-independent")), true);
|
||||
assert.deepEqual(
|
||||
activeRuntime.sentMessages.map(({ message }) => message.content),
|
||||
["child-a", "child-independent"],
|
||||
"one failing delivery must not block its sibling",
|
||||
);
|
||||
|
||||
// Registration happens before session_start during reload. Results in this
|
||||
// gap must queue rather than use the still-bound old ExtensionAPI.
|
||||
const freshRuntime = createMockExtensionApi();
|
||||
(subagentsModule as any).default(freshRuntime.api);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-b", deliver("child-b")), true);
|
||||
assert.equal(activeRuntime.sentMessages.length, 2, "the stale runtime must not receive child-b");
|
||||
|
||||
await emitMockEvent(activeRuntime, "session_shutdown", { reason: "reload" }, ctx);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-c", deliver("child-c failed")), true);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-b", deliver("child-b duplicate")), false);
|
||||
assert.equal(shared.pendingDeliveries.length, 3);
|
||||
|
||||
await emitMockEvent(freshRuntime, "session_start", { reason: "reload" }, ctx);
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
|
||||
assert.deepEqual(
|
||||
freshRuntime.sentMessages.map(({ message }) => message.content),
|
||||
["child-retry", "child-b", "child-c failed"],
|
||||
);
|
||||
testApi.flushPendingDeliveries();
|
||||
assert.equal(freshRuntime.sentMessages.length, 3);
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady("child-c", deliver("post-flush duplicate")), false);
|
||||
});
|
||||
|
||||
it("bounds delivered result tombstones without evicting queued results", () => {
|
||||
shared.latestPi = { sendMessage() {} };
|
||||
shared.activeRuntimeToken = Symbol.for("test-runtime");
|
||||
shared.registeredRuntimeToken = shared.activeRuntimeToken;
|
||||
for (let index = 0; index < 1_100; index += 1) {
|
||||
assert.equal(testApi.sendResultWhenRuntimeReady(`finished-${index}`, () => {}), true);
|
||||
}
|
||||
assert.ok(shared.resultDeliveryStates.size <= 1_024);
|
||||
});
|
||||
|
||||
it("rejects launches after cancellation or runtime replacement", () => {
|
||||
const controller = new AbortController();
|
||||
const runtimeToken = Symbol("runtime");
|
||||
shared.activeRuntimeToken = runtimeToken;
|
||||
shared.registeredRuntimeToken = runtimeToken;
|
||||
assert.doesNotThrow(() => testApi.assertLaunchActive(controller.signal, runtimeToken));
|
||||
|
||||
controller.abort();
|
||||
assert.throws(
|
||||
() => testApi.assertLaunchActive(controller.signal, runtimeToken),
|
||||
/parent session ended/i,
|
||||
);
|
||||
|
||||
const currentController = new AbortController();
|
||||
shared.registeredRuntimeToken = Symbol("replacement");
|
||||
assert.throws(
|
||||
() => testApi.assertLaunchActive(currentController.signal, runtimeToken),
|
||||
/parent session ended/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a completion from a replaced parent even after its watcher left the running map", () => {
|
||||
const completed = {
|
||||
id: "old-child",
|
||||
name: "old-child",
|
||||
task: "done",
|
||||
surface: "old-pane",
|
||||
startTime: 0,
|
||||
sessionFile: "/tmp/old-child.jsonl",
|
||||
interactive: false,
|
||||
parentGeneration: shared.parentGeneration,
|
||||
statusState: createStatusState({ source: "pi", startTimeMs: 0 }),
|
||||
};
|
||||
assert.equal(testApi.shouldDeliverRunningResult(completed), true);
|
||||
shared.parentGeneration += 1;
|
||||
assert.equal(testApi.shouldDeliverRunningResult(completed), false);
|
||||
});
|
||||
|
||||
it("drops a queued subagent question after parent replacement", () => {
|
||||
const dir = createTestDir();
|
||||
const sessionFile = join(dir, "question-child.jsonl");
|
||||
const running = {
|
||||
id: "question-child",
|
||||
name: "question-child",
|
||||
task: "ask",
|
||||
agent: "worker",
|
||||
surface: "question-pane",
|
||||
startTime: 0,
|
||||
sessionFile,
|
||||
interactive: false,
|
||||
parentGeneration: shared.parentGeneration,
|
||||
statusState: createStatusState({ source: "pi", startTimeMs: 0 }),
|
||||
};
|
||||
writeFileSync(`${sessionFile}.ask`, "{");
|
||||
|
||||
try {
|
||||
testApi.deliverPendingQuestion(running);
|
||||
assert.equal(existsSync(`${sessionFile}.ask`), true, "partial signals must be retried");
|
||||
assert.equal(shared.pendingDeliveries.length, 0);
|
||||
|
||||
writeFileSync(`${sessionFile}.ask`, JSON.stringify({ question: "Still there?" }));
|
||||
testApi.deliverPendingQuestion(running);
|
||||
assert.equal(existsSync(`${sessionFile}.ask`), false);
|
||||
assert.equal(shared.pendingDeliveries.length, 1);
|
||||
shared.parentGeneration += 1;
|
||||
const replacement = createMockExtensionApi();
|
||||
shared.latestPi = replacement.api;
|
||||
shared.activeRuntimeToken = Symbol.for("replacement-runtime");
|
||||
shared.registeredRuntimeToken = shared.activeRuntimeToken;
|
||||
testApi.flushPendingDeliveries();
|
||||
assert.equal(replacement.sentMessages.length, 0);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("invalidates old children when replacement session_start wins the shutdown race", async () => {
|
||||
const oldRuntime = createMockExtensionApi();
|
||||
(subagentsModule as any).default(oldRuntime.api);
|
||||
await emitMockEvent(oldRuntime, "session_start", { reason: "startup" }, ctx);
|
||||
|
||||
const abortController = new AbortController();
|
||||
shared.runningSubagents.set("replaced-child", {
|
||||
id: "replaced-child",
|
||||
name: "replaced-child",
|
||||
task: "old work",
|
||||
surface: "old-pane",
|
||||
startTime: 0,
|
||||
sessionFile: "/tmp/replaced-child.jsonl",
|
||||
interactive: false,
|
||||
parentGeneration: shared.parentGeneration,
|
||||
statusState: createStatusState({ source: "pi", startTimeMs: 0 }),
|
||||
abortController,
|
||||
});
|
||||
testApi.sendWhenRuntimeReady(() => {
|
||||
throw new Error("queue this old callback");
|
||||
});
|
||||
assert.equal(shared.pendingDeliveries.length, 1);
|
||||
|
||||
const freshRuntime = createMockExtensionApi();
|
||||
(subagentsModule as any).default(freshRuntime.api);
|
||||
await emitMockEvent(
|
||||
freshRuntime,
|
||||
"session_start",
|
||||
{ reason: "new", previousSessionFile: "/tmp/old-parent.jsonl" },
|
||||
ctx,
|
||||
);
|
||||
|
||||
assert.equal(abortController.signal.aborted, true);
|
||||
assert.equal(shared.runningSubagents.size, 0);
|
||||
assert.equal(shared.pendingDeliveries.length, 0);
|
||||
assert.equal(shared.parentGeneration, 1);
|
||||
|
||||
await emitMockEvent(oldRuntime, "session_shutdown", { reason: "new" }, ctx);
|
||||
assert.equal(shared.parentGeneration, 1, "delayed old shutdown must not invalidate twice");
|
||||
});
|
||||
|
||||
it("preserves a live subagent and delivers its queued result through the fresh runtime", async () => {
|
||||
const oldRuntime = createMockExtensionApi();
|
||||
(subagentsModule as any).default(oldRuntime.api);
|
||||
@@ -1981,6 +2224,7 @@ describe("reload lifecycle", () => {
|
||||
sessionFile: string;
|
||||
statusState: ReturnType<typeof createStatusState>;
|
||||
interactive: boolean;
|
||||
parentGeneration: number;
|
||||
abortController: AbortController;
|
||||
suppressDelivery?: boolean;
|
||||
} = {
|
||||
@@ -1992,6 +2236,7 @@ describe("reload lifecycle", () => {
|
||||
sessionFile: "/tmp/reload-child.jsonl",
|
||||
statusState: createStatusState({ source: "pi", startTimeMs: Date.now() }),
|
||||
interactive: false,
|
||||
parentGeneration: shared.parentGeneration,
|
||||
abortController,
|
||||
};
|
||||
shared.runningSubagents.set(running.id, running);
|
||||
@@ -2032,9 +2277,13 @@ describe("reload lifecycle", () => {
|
||||
});
|
||||
|
||||
describe("tool registration", () => {
|
||||
it("always resumes subagents as autonomous (auto-exit, non-interactive tracking)", () => {
|
||||
it("always resumes subagents as autonomous with reliable follow-up delivery", () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
|
||||
assert.deepEqual(testApi.AUTOMATIC_DELIVERY_OPTIONS, {
|
||||
triggerTurn: true,
|
||||
deliverAs: "followUp",
|
||||
});
|
||||
assert.deepEqual(testApi.resolveResumeLaunchBehavior(), {
|
||||
autoExit: true,
|
||||
interactive: false,
|
||||
@@ -2367,7 +2616,7 @@ describe("subagent interruption", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("uniqueRunningName suffixes defaulted names that collide with running subagents", () => {
|
||||
it("uniqueRunningName suffixes explicit or defaulted names that collide with running subagents", () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
const runningMap = testApi.runningSubagents as Map<string, any>;
|
||||
runningMap.clear();
|
||||
@@ -2432,6 +2681,61 @@ describe("subagent interruption", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("serializes concurrent finished-session resumes by name", () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
const resumingNames = testApi.resumingNames as Set<string>;
|
||||
resumingNames.clear();
|
||||
try {
|
||||
assert.equal(testApi.reserveResumeName("Worker"), true);
|
||||
assert.equal(testApi.reserveResumeName("Worker"), false);
|
||||
assert.equal(testApi.reserveResumeName("Scout"), true);
|
||||
} finally {
|
||||
resumingNames.clear();
|
||||
}
|
||||
});
|
||||
|
||||
it("clears stale completion signals before resuming a finished session", () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
const dir = createTestDir();
|
||||
const sessionFile = join(dir, "finished.jsonl");
|
||||
writeFileSync(`${sessionFile}.exit`, JSON.stringify({ type: "error" }));
|
||||
writeFileSync(`${sessionFile}.ask`, JSON.stringify({ question: "stale" }));
|
||||
try {
|
||||
testApi.clearStaleSessionSignals(sessionFile);
|
||||
assert.equal(existsSync(`${sessionFile}.exit`), false);
|
||||
assert.equal(existsSync(`${sessionFile}.ask`), false);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("recognizes terminal completion and waits for the watcher before late steering", async () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
const runningMap = testApi.runningSubagents as Map<string, any>;
|
||||
const dir = createTestDir();
|
||||
const completionFile = join(dir, "child.exit-code");
|
||||
const running: any = makeRunning({ sessionFile: join(dir, "child.jsonl"), completionFile });
|
||||
runningMap.set(running.id, running);
|
||||
|
||||
try {
|
||||
assert.equal(testApi.hasTerminalCompletionSignal(running), false);
|
||||
writeFileSync(completionFile, "0\n");
|
||||
assert.equal(testApi.hasTerminalCompletionSignal(running), true);
|
||||
rmSync(completionFile);
|
||||
running.completionObserved = true;
|
||||
assert.equal(
|
||||
testApi.hasTerminalCompletionSignal(running),
|
||||
true,
|
||||
"watcher state must preserve completion after consuming the file",
|
||||
);
|
||||
setTimeout(() => runningMap.delete(running.id), 10);
|
||||
assert.equal(await testApi.waitForWatcherCleanup(running, new AbortController().signal, 200), true);
|
||||
} finally {
|
||||
runningMap.clear();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("steers a running subagent by typing into its pane (newlines flattened)", () => {
|
||||
const testApi = (subagentsModule as any).__test__;
|
||||
let sentSurface = "";
|
||||
|
||||
Reference in New Issue
Block a user