From db0b787505ac7d053abb58e6f8e12cbf963082d5 Mon Sep 17 00:00:00 2001 From: issacboy123 Date: Thu, 20 Aug 2026 03:53:37 -0700 Subject: [PATCH 1/2] feat(coding-agent): persist subagent lifecycle --- packages/coding-agent/src/core/sdk.ts | 51 +++- .../coding-agent/src/core/session-manager.ts | 34 +++ .../src/core/subagents/controller.ts | 141 +++++++++-- .../src/core/subagents/services.ts | 22 +- .../core/subagents/session-manager-factory.ts | 5 +- .../coding-agent/src/core/subagents/types.ts | 18 +- .../test/suite/subagent-persistence.test.ts | 238 +++++++++++++++++- 7 files changed, 470 insertions(+), 39 deletions(-) diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 8b967821e..fe78236e2 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -23,9 +23,10 @@ import { ModelRuntime } from "./model-runtime.ts"; import { mergeProviderAttributionHeaders } from "./provider-attribution.ts"; import type { ResourceLoader } from "./resource-loader.ts"; import { DefaultResourceLoader } from "./resource-loader.ts"; -import { getDefaultSessionDir, SessionManager } from "./session-manager.ts"; +import { getDefaultSessionDir, persistSessionState, SessionManager } from "./session-manager.ts"; import { SettingsManager } from "./settings-manager.ts"; import { SubagentController } from "./subagents/controller.ts"; +import { encodeSubagentRosterSnapshot, SUBAGENT_ROSTER_CUSTOM_TYPE } from "./subagents/persistence.ts"; import { captureSubagentSessionConfig, createSubagentSession } from "./subagents/services.ts"; import { RootLocalEphemeralSubagentSessionManagerFactory } from "./subagents/session-manager-factory.ts"; import { time } from "./timings.ts"; @@ -198,6 +199,7 @@ function createRootSubagentController( maxConcurrent: number, agentDir: string, telemetryContext: TelemetryContext, + parentSessionManager: SessionManager, ): SubagentController { const sessionManagerFactory = new RootLocalEphemeralSubagentSessionManagerFactory(); return new SubagentController({ @@ -205,14 +207,41 @@ function createRootSubagentController( telemetryContext, captureSession: (parent, childName, origin) => { const config = captureSubagentSessionConfig(parent, agentDir); - return (signal) => - createSubagentSession(config, { - signal, + const sessionManager = sessionManagerFactory.create( + Object.freeze({ childName, + sourceSessionManager: config.sourceSessionManager, + cwd: config.cwd, + parentSessionId: config.parentSession, origin, - sessionManagerFactory, - }); + }), + ); + return Object.freeze({ + sessionManager, + config: Object.freeze({ + model: Object.freeze({ provider: config.model.provider, id: config.model.id }), + thinkingLevel: config.thinkingLevel, + tools: config.tools, + harnessRevision: config.harnessVersionBundle?.manifest.version ?? null, + }), + createSession: (signal: AbortSignal) => + createSubagentSession(config, { + signal, + childName, + origin, + sessionManager, + }), + }); }, + persistRoster: parentSessionManager.isPersisted() + ? (agents, admission) => { + parentSessionManager.appendCustomEntry( + SUBAGENT_ROSTER_CUSTOM_TYPE, + encodeSubagentRosterSnapshot(agents), + ); + if (admission) persistSessionState(parentSessionManager); + } + : undefined, }); } @@ -260,11 +289,6 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const subagentOptions = normalizeSubagentOptions(options.subagents); const telemetryContext = options.telemetryContext ?? NOOP_TELEMETRY_CONTEXT; assertNoReservedSubagentTools(options.customTools); - const subagentController = - subagentOptions === false - ? undefined - : createRootSubagentController(subagentOptions.maxConcurrent, agentDir, telemetryContext); - const customTools = [...(options.customTools ?? []), ...(subagentController?.createToolDefinitions() ?? [])]; let resourceLoader = options.resourceLoader; const authPath = options.agentDir ? join(agentDir, "auth.json") : undefined; @@ -273,6 +297,11 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const settingsManager = options.settingsManager ?? SettingsManager.create(cwd, agentDir); const sessionManager = options.sessionManager ?? SessionManager.create(cwd, getDefaultSessionDir(cwd, agentDir)); + const subagentController = + subagentOptions === false + ? undefined + : createRootSubagentController(subagentOptions.maxConcurrent, agentDir, telemetryContext, sessionManager); + const customTools = [...(options.customTools ?? []), ...(subagentController?.createToolDefinitions() ?? [])]; if (!resourceLoader) { resourceLoader = new DefaultResourceLoader({ cwd, agentDir, settingsManager }); diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index e687b8614..a259e3bf4 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -229,6 +229,7 @@ function createSessionId(): string { } const PERSIST_SESSION_HEADER = Symbol("persistSessionHeader"); +const PERSIST_SESSION_STATE = Symbol("persistSessionState"); export function assertValidSessionId(id: string): void { if (!/^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$/.test(id)) { @@ -1056,6 +1057,34 @@ export class SessionManager { this.flushed = true; } + /** Flush the complete admitted session state before accepting dependent work. */ + [PERSIST_SESSION_STATE](): void { + if (!this.persist || !this.sessionFile) { + throw new Error("Cannot persist an in-memory session"); + } + + if (!this.flushed) { + const fd = openSync(this.sessionFile, "wx"); + try { + for (const entry of this.fileEntries) { + writeFileSync(fd, `${JSON.stringify(entry)}\n`); + } + fsyncSync(fd); + } finally { + closeSync(fd); + } + this.flushed = true; + return; + } + + const fd = openSync(this.sessionFile, "r+"); + try { + fsyncSync(fd); + } finally { + closeSync(fd); + } + } + _persist(entry: SessionEntry): void { if (!this.persist || !this.sessionFile) return; @@ -1814,3 +1843,8 @@ export class SessionManager { export function persistSessionHeader(sessionManager: SessionManager): void { sessionManager[PERSIST_SESSION_HEADER](); } + +/** Internal seam for durably admitting state that normal creation may still buffer. */ +export function persistSessionState(sessionManager: SessionManager): void { + sessionManager[PERSIST_SESSION_STATE](); +} diff --git a/packages/coding-agent/src/core/subagents/controller.ts b/packages/coding-agent/src/core/subagents/controller.ts index 49ecfb285..4cc3bc02b 100644 --- a/packages/coding-agent/src/core/subagents/controller.ts +++ b/packages/coding-agent/src/core/subagents/controller.ts @@ -4,7 +4,9 @@ import type { AgentSession } from "../agent-session.ts"; import type { ToolDefinition } from "../extensions/types.ts"; import { startCodingAgentSpan } from "../telemetry.ts"; import { createAgentActionTool, createAgentsTool, createSpawnAgentTool } from "../tools/subagents.ts"; +import type { ChildSnapshotV1 } from "./persistence.ts"; import type { + CapturedSubagentSession, CaptureSubagentSession, CreateSubagentSession, SubagentActionResult, @@ -102,6 +104,10 @@ interface SubagentRecord { coordinatedRun?: CoordinatedRun; lastRetryCycleId?: number; telemetryContext?: TelemetryContext; + sessionManager?: CapturedSubagentSession["sessionManager"]; + persistenceConfig?: CapturedSubagentSession["config"]; + persistenceError?: string; + leafUnsubscribe?: () => void; } function isTerminalStatus(status: SubagentStatus): status is SubagentTerminalStatus { @@ -147,6 +153,7 @@ const MAX_TIMER_MS = 2_147_483_647; export class SubagentController { readonly #maxConcurrent: number; readonly #captureSession: CaptureSubagentSession | undefined; + readonly #persistRoster: ((agents: readonly ChildSnapshotV1[], admission: boolean) => void) | undefined; readonly #telemetryContext: TelemetryContext; readonly #records = new Map(); readonly #listeners = new Set(); @@ -168,6 +175,7 @@ export class SubagentController { maxConcurrent: number; captureSession?: CaptureSubagentSession; telemetryContext?: TelemetryContext; + persistRoster?: (agents: readonly ChildSnapshotV1[], admission: boolean) => void; }) { if (!Number.isInteger(options.maxConcurrent) || options.maxConcurrent <= 0) { throw new RangeError("maxConcurrent must be a positive integer"); @@ -175,18 +183,20 @@ export class SubagentController { this.#maxConcurrent = options.maxConcurrent; this.#captureSession = options.captureSession; this.#telemetryContext = options.telemetryContext ?? NOOP_TELEMETRY_CONTEXT; + this.#persistRoster = options.persistRoster; } createToolDefinitions(): ToolDefinition[] { const definitions = [ createSpawnAgentTool((name, task, toolCallId) => { const parent = this.#getBoundParent(); + this.#assertReservable(name); const origin = captureSpawnOrigin(parent, toolCallId); - const createSession = this.#captureSession?.(parent, name, origin); - if (!createSession) { + const captured = this.#captureSession?.(parent, name, origin); + if (!captured) { throw new Error("Subagent session capture is unavailable"); } - this.spawn(name, task, createSession, origin, parent.agent.telemetryContext); + this.spawn(name, task, captured.createSession, origin, parent.agent.telemetryContext, captured); }), createAgentActionTool((request) => { const parent = this.#getBoundParent(); @@ -217,13 +227,7 @@ export class SubagentController { } reserve(name: string, task: string): void { - this.#assertOpen(); - if (!SUBAGENT_NAME_PATTERN.test(name)) { - throw new Error(`Invalid subagent name: ${name}`); - } - if (this.#records.has(name)) { - throw new Error(`Subagent already reserved: ${name}`); - } + this.#assertReservable(name); const timestamp = Date.now(); const record: SubagentRecord = { @@ -250,14 +254,43 @@ export class SubagentController { createSession: CreateSubagentSession, origin?: SubagentSpawnOrigin, telemetryContext: TelemetryContext = this.#telemetryContext, + captured?: CapturedSubagentSession, ): void { - this.reserve(name, task); - const record = this.#records.get(name)!; + this.#assertReservable(name); + if (this.#persistRoster && !captured) { + throw new Error("Persistent subagent admission requires captured session state"); + } + + const timestamp = Date.now(); + const record: SubagentRecord = { + id: uuidv7(), + name, + task, + status: "queued", + createdAt: timestamp, + updatedAt: timestamp, + }; record.createSession = createSession; record.telemetryContext = telemetryContext; + record.sessionManager = captured?.sessionManager; + record.persistenceConfig = captured?.config; if (origin?.type === "agent_tool_call") { record.coordinatedRun = this.#createCoordinatedRun("initial"); } + this.#records.set(name, record); + try { + this.#persistRoster?.(this.#createPersistenceSnapshot(), true); + } catch (error) { + this.#records.delete(name); + throw error; + } + this.#publish((revision) => ({ + type: "subagent_reserved", + revision, + name, + status: "queued", + timestamp, + })); void startCodingAgentSpan( telemetryContext, "diffex.coding_agent.subagent", @@ -293,6 +326,11 @@ export class SubagentController { throw new Error(`Subagent session already attached: ${name}`); } record.session = session; + record.sessionManager = session.sessionManager; + record.leafUnsubscribe = session.subscribe((event) => { + if (event.type !== "message_end" && event.type !== "entry_appended") return; + queueMicrotask(() => this.#persistLeafChange(name)); + }); } trackRun(name: string, run: Promise): void { @@ -600,6 +638,7 @@ export class SubagentController { this.#shutdownPromise = shutdownPromise; try { + this.#persistRosterBestEffort(undefined, true); this.#closed = true; const closedError = this.#closedError(); this.#rejectPermitQueue(closedError); @@ -695,6 +734,7 @@ export class SubagentController { status, timestamp, })); + this.#persistRosterBestEffort(name); } #isValidTransition(previousStatus: SubagentStatus, status: SubagentStatus): boolean { @@ -792,7 +832,9 @@ export class SubagentController { status: agent.status, ...(record.coordinatedRun === undefined ? {} : { runId: record.coordinatedRun.id }), ...(record.finalText === undefined ? {} : { result: record.finalText }), - ...(record.error === undefined ? {} : { error: record.error }), + ...((record.error ?? record.persistenceError) === undefined + ? {} + : { error: record.error ?? record.persistenceError }), }), ]; }); @@ -827,10 +869,70 @@ export class SubagentController { attempt: run.attempt, retryAvailable, ...(record.finalText === undefined ? {} : { result: record.finalText }), - ...(record.error === undefined ? {} : { error: record.error }), + ...((record.error ?? record.persistenceError) === undefined + ? {} + : { error: record.error ?? record.persistenceError }), }); } + #createPersistenceSnapshot(interruptActive = false): ChildSnapshotV1[] { + const interruptionTimestamp = interruptActive ? Date.now() : undefined; + return [...this.#records.values()] + .sort((left, right) => left.name.localeCompare(right.name)) + .map((record) => { + const config = record.persistenceConfig; + const sessionManager = record.sessionManager; + if (!config || !sessionManager) { + throw new Error(`Persistent subagent state is unavailable: ${record.name}`); + } + const interrupted = interruptActive && (record.status === "queued" || record.status === "running"); + const snapshotTimestamp = interruptionTimestamp ?? record.updatedAt; + const error = record.error ?? record.persistenceError; + return { + name: record.name, + task: record.task, + status: interrupted ? "interrupted" : record.status, + createdAt: record.createdAt, + updatedAt: interrupted ? snapshotTimestamp : record.updatedAt, + ...(record.startedAt === undefined ? {} : { startedAt: record.startedAt }), + ...(interrupted + ? { completedAt: snapshotTimestamp } + : record.completedAt === undefined + ? {} + : { completedAt: record.completedAt }), + ...(record.finalText === undefined ? {} : { result: record.finalText }), + ...(error === undefined ? {} : { error }), + childSessionId: sessionManager.getSessionId(), + childLeafId: sessionManager.getLeafId(), + config, + }; + }); + } + + #persistRosterBestEffort(changedName?: string, interruptActive = false): void { + if (!this.#persistRoster || this.#records.size === 0) return; + try { + this.#persistRoster(this.#createPersistenceSnapshot(interruptActive), false); + } catch (error) { + const message = `Subagent roster persistence failed: ${normalizeError(error)}`; + if (changedName) { + const record = this.#records.get(changedName); + if (record) record.persistenceError = message; + return; + } + for (const record of this.#records.values()) { + if (record.status === "queued" || record.status === "running") { + record.persistenceError = message; + } + } + } + } + + #persistLeafChange(name: string): void { + if (this.#closed || this.#resetting || !this.#records.has(name)) return; + this.#persistRosterBestEffort(name); + } + #queryConditionMet(agents: readonly SubagentResultView[], until: "any" | "all"): boolean { return until === "any" ? agents.some((agent) => isTerminalStatus(agent.status)) @@ -1188,6 +1290,7 @@ export class SubagentController { async #cleanupRecords(records: SubagentRecord[]): Promise { const failures: unknown[] = []; + for (const record of records) record.leafUnsubscribe?.(); const abortResults = await Promise.allSettled( records.flatMap((record) => { const session = record.session; @@ -1268,6 +1371,16 @@ export class SubagentController { if (this.#resetting) throw this.#resetError(); } + #assertReservable(name: string): void { + this.#assertOpen(); + if (!SUBAGENT_NAME_PATTERN.test(name)) { + throw new Error(`Invalid subagent name: ${name}`); + } + if (this.#records.has(name)) { + throw new Error(`Subagent already reserved: ${name}`); + } + } + #resetError(): Error { return new Error("Subagent controller is resetting"); } diff --git a/packages/coding-agent/src/core/subagents/services.ts b/packages/coding-agent/src/core/subagents/services.ts index 2a6a64ac4..628cac3cb 100644 --- a/packages/coding-agent/src/core/subagents/services.ts +++ b/packages/coding-agent/src/core/subagents/services.ts @@ -47,6 +47,7 @@ export interface CreateSubagentSessionOptions { readonly childName?: string; readonly origin?: SubagentSpawnOrigin; readonly sessionManagerFactory?: SubagentSessionManagerFactory; + readonly sessionManager?: SessionManager; } const SUBAGENT_SYSTEM_PROMPT = [ @@ -124,16 +125,17 @@ export async function createSubagentSession( }); options.signal?.throwIfAborted(); - const sessionManagerFactory = options.sessionManagerFactory ?? new RootLocalEphemeralSubagentSessionManagerFactory(); - const sessionManager = sessionManagerFactory.create( - Object.freeze({ - childName: options.childName ?? "subagent", - sourceSessionManager: config.sourceSessionManager, - cwd: config.cwd, - parentSessionId: config.parentSession, - origin: options.origin ?? Object.freeze({ type: "programmatic", contextEntryId: null }), - }), - ); + const sessionManager = + options.sessionManager ?? + (options.sessionManagerFactory ?? new RootLocalEphemeralSubagentSessionManagerFactory()).create( + Object.freeze({ + childName: options.childName ?? "subagent", + sourceSessionManager: config.sourceSessionManager, + cwd: config.cwd, + parentSessionId: config.parentSession, + origin: options.origin ?? Object.freeze({ type: "programmatic", contextEntryId: null }), + }), + ); const { session } = await createAgentSessionFromServices({ services, sessionManager, diff --git a/packages/coding-agent/src/core/subagents/session-manager-factory.ts b/packages/coding-agent/src/core/subagents/session-manager-factory.ts index 3c4573ae8..a7224a78e 100644 --- a/packages/coding-agent/src/core/subagents/session-manager-factory.ts +++ b/packages/coding-agent/src/core/subagents/session-manager-factory.ts @@ -1,4 +1,5 @@ import { SessionManager } from "../session-manager.ts"; +import { ParentOwnedSubagentSessionStore } from "./persistence.ts"; import type { SubagentSpawnOrigin } from "./types.ts"; export interface SubagentSessionManagerRequest { @@ -16,7 +17,9 @@ export interface SubagentSessionManagerFactory { export class RootLocalEphemeralSubagentSessionManagerFactory implements SubagentSessionManagerFactory { create(request: SubagentSessionManagerRequest): SessionManager { // Future context inheritance will be opt-in and use request.origin.contextEntryId with request.sourceSessionManager.forkBranch(). - // Future persistence will replace or extend this internal manager factory, add root metadata, and implement restoration without changing controller scheduling. + if (request.sourceSessionManager.isPersisted()) { + return new ParentOwnedSubagentSessionStore(request.sourceSessionManager).createChild(request.childName); + } return SessionManager.inMemory(request.cwd, { parentSession: request.parentSessionId }); } } diff --git a/packages/coding-agent/src/core/subagents/types.ts b/packages/coding-agent/src/core/subagents/types.ts index ecaa1de90..efb203654 100644 --- a/packages/coding-agent/src/core/subagents/types.ts +++ b/packages/coding-agent/src/core/subagents/types.ts @@ -1,4 +1,7 @@ +import type { ThinkingLevel } from "@earendil-works/diffex-agent-core"; import type { AgentSession } from "../agent-session.ts"; +import type { SessionManager } from "../session-manager.ts"; +import type { ToolName } from "../tools/index.ts"; export const SUBAGENT_NAME_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; @@ -12,6 +15,19 @@ export interface SubagentSummary { export type CreateSubagentSession = (signal: AbortSignal) => Promise; +export interface CapturedSubagentConfig { + readonly model: { readonly provider: string; readonly id: string }; + readonly thinkingLevel: ThinkingLevel; + readonly tools: readonly ToolName[]; + readonly harnessRevision: string | null; +} + +export interface CapturedSubagentSession { + readonly createSession: CreateSubagentSession; + readonly sessionManager: SessionManager; + readonly config: CapturedSubagentConfig; +} + export type SubagentSpawnOrigin = | { readonly type: "agent_tool_call"; @@ -33,7 +49,7 @@ export type CaptureSubagentSession = ( parent: AgentSession, childName: string, origin: SubagentSpawnOrigin, -) => CreateSubagentSession; +) => CapturedSubagentSession; export type SubagentMessageDelivery = "steer" | "follow_up" | "prompt"; diff --git a/packages/coding-agent/test/suite/subagent-persistence.test.ts b/packages/coding-agent/test/suite/subagent-persistence.test.ts index 4ace05414..d3e8c5a14 100644 --- a/packages/coding-agent/test/suite/subagent-persistence.test.ts +++ b/packages/coding-agent/test/suite/subagent-persistence.test.ts @@ -10,8 +10,12 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { persistSessionHeader, SessionManager } from "../../src/core/session-manager.ts"; +import type { AgentTool } from "@earendil-works/diffex-agent-core"; +import { fauxAssistantMessage } from "@earendil-works/diffex-ai"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { AgentSession } from "../../src/core/agent-session.ts"; +import { createAgentSessionFromServices, createAgentSessionServices } from "../../src/core/agent-session-services.ts"; +import { loadEntriesFromFile, persistSessionHeader, SessionManager } from "../../src/core/session-manager.ts"; import { type ChildSnapshotV1, decodeSubagentRosterSnapshot, @@ -20,6 +24,8 @@ import { readSelectedBranchSubagentRoster, SUBAGENT_ROSTER_CUSTOM_TYPE, } from "../../src/core/subagents/persistence.ts"; +import { createHarness, type Harness } from "./harness.ts"; +import { deferred } from "./subagent-harness.ts"; const CHILD_ID_A = "018f4f7c-43b7-7abc-8def-0123456789ab"; const CHILD_ID_B = "018f4f7c-43b7-7abc-8def-0123456789ac"; @@ -215,3 +221,231 @@ describe("parent-owned subagent session storage", () => { expect(() => store.openChild("beta", CHILD_ID_B, null)).toThrow("not a safe file"); }); }); + +describe("durable subagent lifecycle", () => { + const harnesses: Harness[] = []; + const sessions: AgentSession[] = []; + + afterEach(async () => { + vi.restoreAllMocks(); + while (sessions.length > 0) await sessions.pop()?.dispose(); + while (harnesses.length > 0) await harnesses.pop()?.cleanup(); + }); + + async function createRoot(options: { persist?: boolean; subagents?: false } = {}) { + const harness = await createHarness({ + models: [{ id: "faux-lifecycle", name: "Lifecycle", reasoning: true }], + settings: { retry: { enabled: false } }, + }); + harnesses.push(harness); + const cwd = join(harness.tempDir, "root"); + const agentDir = join(harness.tempDir, "agent"); + const sessionDir = join(harness.tempDir, "sessions"); + mkdirSync(cwd, { recursive: true }); + mkdirSync(agentDir, { recursive: true }); + const services = await createAgentSessionServices({ + cwd, + agentDir, + modelRuntime: harness.session.modelRuntime, + settingsManager: harness.settingsManager, + resourceLoaderOptions: { + noExtensions: true, + noSkills: true, + noPromptTemplates: true, + noThemes: true, + reloadSettings: false, + }, + }); + const sessionManager = + options.persist === false ? SessionManager.inMemory(cwd) : SessionManager.create(cwd, sessionDir); + const { session } = await createAgentSessionFromServices({ + services, + sessionManager, + model: harness.getModel("faux-lifecycle"), + thinkingLevel: "high", + subagents: options.subagents, + }); + sessions.push(session); + return { harness, session, sessionManager, cwd, sessionDir }; + } + + function getTool(session: AgentSession, name: string): AgentTool { + const tool = session.agent.state.tools.find((candidate) => candidate.name === name); + if (!tool) throw new Error(`Missing tool: ${name}`); + return tool; + } + + function readSnapshots(sessionFile: string): ChildSnapshotV1[][] { + return loadEntriesFromFile(sessionFile).flatMap((entry) => { + if (entry.type !== "custom" || entry.customType !== SUBAGENT_ROSTER_CUSTOM_TYPE) return []; + const decoded = decodeSubagentRosterSnapshot(entry.data); + return [decoded.children.flatMap((child) => (child.kind === "restorable" ? [child.snapshot] : []))]; + }); + } + + it("persists the child header and queued roster before direct spawn starts provider work", async () => { + const started = deferred(); + const { harness, session, sessionManager, cwd, sessionDir } = await createRoot(); + const sessionFile = sessionManager.getSessionFile(); + if (!sessionFile) throw new Error("Missing parent session file"); + let admissionObserved = false; + harness.setResponses([ + async (_context, request) => { + const snapshots = readSnapshots(sessionFile); + const queued = snapshots.find((snapshot) => snapshot[0]?.status === "queued")?.[0]; + const ownedDirectory = resolve(dirname(sessionFile), ".subagents", session.sessionId); + admissionObserved = + queued !== undefined && + existsSync(ownedDirectory) && + readdirSync(ownedDirectory).some((name) => name.endsWith(`_${queued.childSessionId}.jsonl`)); + started.resolve(); + await new Promise((resolvePromise) => { + request?.signal?.addEventListener("abort", () => resolvePromise(), { once: true }); + }); + return fauxAssistantMessage("stopped"); + }, + ]); + + await getTool(session, "spawn_agent").execute("direct-spawn", { + name: "worker", + task: "inspect durability", + }); + const reopened = SessionManager.open(sessionFile, sessionDir, cwd); + const admitted = readSelectedBranchSubagentRoster(reopened); + expect(admitted.children[0]).toMatchObject({ + kind: "restorable", + snapshot: { + name: "worker", + task: "inspect durability", + config: { + model: { provider: "faux", id: "faux-lifecycle" }, + thinkingLevel: "high", + tools: ["read", "bash", "edit", "write", "update_plan"], + harnessRevision: session.activeHarnessVersion ?? null, + }, + }, + }); + await started.promise; + expect(admissionObserved).toBe(true); + }); + + it("rejects spawn without provider work when the initial roster cannot persist", async () => { + const { harness, session, sessionManager } = await createRoot(); + let providerCalls = 0; + harness.setResponses([ + () => { + providerCalls += 1; + return fauxAssistantMessage("unexpected"); + }, + ]); + vi.spyOn(sessionManager, "appendCustomEntry").mockImplementationOnce(() => { + throw new Error("parent roster unavailable"); + }); + + await expect( + getTool(session, "spawn_agent").execute("failed-spawn", { name: "worker", task: "do not run" }), + ).rejects.toThrow("parent roster unavailable"); + await new Promise((resolvePromise) => setTimeout(resolvePromise, 10)); + expect(providerCalls).toBe(0); + await expect(getTool(session, "agents").execute("list", { timeout_ms: 0 })).resolves.toMatchObject({ + details: { agents: [] }, + }); + }); + + it("rejects spawn without provider work when the child header cannot persist", async () => { + const { harness, session } = await createRoot(); + let providerCalls = 0; + harness.setResponses([ + () => { + providerCalls += 1; + return fauxAssistantMessage("unexpected"); + }, + ]); + vi.spyOn(ParentOwnedSubagentSessionStore.prototype, "createChild").mockImplementationOnce(() => { + throw new Error("child header unavailable"); + }); + + await expect( + getTool(session, "spawn_agent").execute("failed-header", { name: "worker", task: "do not run" }), + ).rejects.toThrow("child header unavailable"); + await new Promise((resolvePromise) => setTimeout(resolvePromise, 10)); + expect(providerCalls).toBe(0); + await expect(getTool(session, "agents").execute("list", { timeout_ms: 0 })).resolves.toMatchObject({ + details: { agents: [] }, + }); + }); + + it("records lifecycle and child leaf snapshots and exposes later persistence errors", async () => { + const { harness, session, sessionManager } = await createRoot(); + const sessionFile = sessionManager.getSessionFile(); + if (!sessionFile) throw new Error("Missing parent session file"); + harness.setResponses([fauxAssistantMessage("child complete")]); + const append = vi.spyOn(sessionManager, "appendCustomEntry"); + const originalAppend = append.getMockImplementation(); + let calls = 0; + append.mockImplementation((customType, data) => { + calls += 1; + if (calls === 2) throw new Error("later snapshot unavailable"); + return originalAppend + ? originalAppend(customType, data) + : SessionManager.prototype.appendCustomEntry.call(sessionManager, customType, data); + }); + + await getTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "finish" }); + const result = await getTool(session, "agents").execute("wait", { timeout_ms: 1_000, until: "all" }); + expect(result.details).toMatchObject({ + agents: [ + { + name: "worker", + status: "idle", + result: "child complete", + error: "Subagent roster persistence failed: later snapshot unavailable", + }, + ], + }); + const snapshots = readSnapshots(sessionFile); + expect(snapshots.map((snapshot) => snapshot[0]?.status)).toEqual( + expect.arrayContaining(["queued", "running", "idle"]), + ); + expect(snapshots.some((snapshot) => snapshot[0]?.childLeafId !== null)).toBe(true); + expect(snapshots.at(-1)?.[0]).toMatchObject({ + status: "idle", + result: "child complete", + }); + }); + + it("persists active children as interrupted before graceful disposal", async () => { + const started = deferred(); + const { harness, session, sessionManager } = await createRoot(); + const sessionFile = sessionManager.getSessionFile(); + if (!sessionFile) throw new Error("Missing parent session file"); + harness.setResponses([ + async (_context, request) => { + started.resolve(); + await new Promise((resolvePromise) => { + request?.signal?.addEventListener("abort", () => resolvePromise(), { once: true }); + }); + return fauxAssistantMessage("stopped"); + }, + ]); + + await getTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "keep running" }); + await started.promise; + await session.dispose(); + const latest = readSnapshots(sessionFile).at(-1)?.[0]; + expect(latest).toMatchObject({ name: "worker", status: "interrupted" }); + expect(latest?.completedAt).toBeTypeOf("number"); + }); + + it("keeps in-memory, disabled, and unsaved modes ephemeral", async () => { + const inMemory = await createRoot({ persist: false }); + inMemory.harness.setResponses([fauxAssistantMessage("done")]); + await getTool(inMemory.session, "spawn_agent").execute("spawn", { name: "worker", task: "ephemeral" }); + await getTool(inMemory.session, "agents").execute("wait", { timeout_ms: 1_000, until: "all" }); + expect(inMemory.sessionManager.getSessionFile()).toBeUndefined(); + + const disabled = await createRoot({ subagents: false }); + expect(disabled.session.getToolDefinition("spawn_agent")).toBeUndefined(); + expect(existsSync(resolve(disabled.sessionDir, ".subagents"))).toBe(false); + }); +}); From 2877a2e907c59c89d8cac97a0cd025c561deb4a2 Mon Sep 17 00:00:00 2001 From: issacboy123 Date: Thu, 20 Aug 2026 04:00:53 -0700 Subject: [PATCH 2/2] fix(coding-agent): roll back failed roster snapshots --- packages/coding-agent/src/core/sdk.ts | 17 +++++++---- .../test/suite/subagent-persistence.test.ts | 29 ++++++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index fe78236e2..f7ff0ba30 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -235,11 +235,18 @@ function createRootSubagentController( }, persistRoster: parentSessionManager.isPersisted() ? (agents, admission) => { - parentSessionManager.appendCustomEntry( - SUBAGENT_ROSTER_CUSTOM_TYPE, - encodeSubagentRosterSnapshot(agents), - ); - if (admission) persistSessionState(parentSessionManager); + const previousLeafId = parentSessionManager.getLeafId(); + try { + parentSessionManager.appendCustomEntry( + SUBAGENT_ROSTER_CUSTOM_TYPE, + encodeSubagentRosterSnapshot(agents), + ); + if (admission) persistSessionState(parentSessionManager); + } catch (error) { + if (previousLeafId === null) parentSessionManager.resetLeaf(); + else parentSessionManager.branch(previousLeafId); + throw error; + } } : undefined, }); diff --git a/packages/coding-agent/test/suite/subagent-persistence.test.ts b/packages/coding-agent/test/suite/subagent-persistence.test.ts index d3e8c5a14..705d89f4b 100644 --- a/packages/coding-agent/test/suite/subagent-persistence.test.ts +++ b/packages/coding-agent/test/suite/subagent-persistence.test.ts @@ -338,7 +338,9 @@ describe("durable subagent lifecycle", () => { return fauxAssistantMessage("unexpected"); }, ]); - vi.spyOn(sessionManager, "appendCustomEntry").mockImplementationOnce(() => { + const append = vi.spyOn(sessionManager, "appendCustomEntry"); + append.mockImplementationOnce((customType, data) => { + SessionManager.prototype.appendCustomEntry.call(sessionManager, customType, data); throw new Error("parent roster unavailable"); }); @@ -350,6 +352,7 @@ describe("durable subagent lifecycle", () => { await expect(getTool(session, "agents").execute("list", { timeout_ms: 0 })).resolves.toMatchObject({ details: { agents: [] }, }); + expect(readSelectedBranchSubagentRoster(sessionManager)).toMatchObject({ found: false, children: [] }); }); it("rejects spawn without provider work when the child header cannot persist", async () => { @@ -437,6 +440,30 @@ describe("durable subagent lifecycle", () => { expect(latest?.completedAt).toBeTypeOf("number"); }); + it("still disposes gracefully when the interrupted roster cannot persist", async () => { + const started = deferred(); + const { harness, session, sessionManager } = await createRoot(); + harness.setResponses([ + async (_context, request) => { + started.resolve(); + await new Promise((resolvePromise) => { + request?.signal?.addEventListener("abort", () => resolvePromise(), { once: true }); + }); + return fauxAssistantMessage("stopped"); + }, + ]); + + await getTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "keep running" }); + await started.promise; + const previousLeafId = sessionManager.getLeafId(); + vi.spyOn(sessionManager, "appendCustomEntry").mockImplementationOnce(() => { + throw new Error("graceful snapshot unavailable"); + }); + + await expect(session.dispose()).resolves.toBeUndefined(); + expect(sessionManager.getLeafId()).toBe(previousLeafId); + }); + it("keeps in-memory, disabled, and unsaved modes ephemeral", async () => { const inMemory = await createRoot({ persist: false }); inMemory.harness.setResponses([fauxAssistantMessage("done")]);