diff --git a/docs/agent-lifecycle.md b/docs/agent-lifecycle.md index ee763aa1f9..d699d8519a 100644 --- a/docs/agent-lifecycle.md +++ b/docs/agent-lifecycle.md @@ -151,11 +151,12 @@ lifecycle queue. Resume chooses its history or interactive purpose from the dura entering that queue. Shutdown must finish before the manager releases runtime ownership; a failed close retains the runtime for cleanup and blocks replacement through that close operation. -Authoritative timeline catch-up can use a runtime-only `history` resume purpose. For Codex, that -purpose initializes a temporary app-server, reads the persisted thread and child histories, and -releases the process before returning. It never loads, resumes, or unarchives a native thread, -including legacy records whose native archive failed. The retained history session contains only -the read results. Interactive resume remains responsible for repairing a provider session archived +Authoritative timeline catch-up can use a runtime-only `history` resume purpose. The retained +history session contains only the read results and must release its provider process once replay +finishes. Codex reads the persisted thread and child histories on a temporary app-server and never +loads, resumes, or unarchives a native thread, including legacy records whose native archive failed. +ACP providers spawn the CLI, replay the session through `session/load`, and terminate the process +before returning. Interactive resume remains responsible for repairing a provider session archived outside Paseo while its Paseo agent is active. Provider session connection owns every process it spawns until the session is registered with diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 6bded49feb..169421fca9 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1750,6 +1750,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private fallbackAssistantMessageId: string | null = null; private closed = false; private historyPending = false; + private historyRuntimeReleased = false; private replayingHistory = false; private bootstrapThreadEventPending = false; private readonly terminateProcess: ProcessTerminator; @@ -1867,11 +1868,38 @@ export class ACPAgentSession implements AgentSession, ACPClient { } await this.applyConfiguredOverrides(); + + if (this.resumePurpose === "history") { + await this.releaseHistoryRuntime(); + } } catch (error) { await this.closeAfterInitializationFailure(error); } } + // History resumes exist to replay persisted events into the canonical + // timeline; the provider process is released once replay finishes so + // archived agents hold no resident runtime. Interactive work goes through + // reloadAgentSession, which spawns a fresh process. + private async releaseHistoryRuntime(): Promise { + const child = this.child; + if (child) { + const result = await this.terminateProcess(child, { + gracefulTimeoutMs: 2_000, + forceTimeoutMs: 2_000, + }); + if (result === "kill-timeout") { + // Keep the child registered so close() can retry the shutdown. + this.logger.warn("ACP history resume could not release the provider process"); + return; + } + } + this.historyRuntimeReleased = true; + this.connection = null; + this.child = null; + this.settleCommandsReady(); + } + private async closeAfterInitializationFailure(error: unknown): Promise { try { await this.close(); @@ -2841,7 +2869,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { }); const spawnError = rejectOnSpawnError(child, stderrChunks); child.once("exit", (code, signal) => { - if (this.closed) { + if (this.closed || this.historyRuntimeReleased) { return; } if (this.activeForegroundTurnId) { diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.archived-history.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.archived-history.test.ts index ec17dd7ecf..fdddb4941c 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.archived-history.test.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.archived-history.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -68,8 +68,60 @@ describe("GenericACPAgentClient archived history", () => { ).rejects.toThrow("ENOENT"); }); }); + + test("releases the provider process once an archived session's history is replayed", async () => { + await withFakeACPAgent(async ({ command, removedCwd, pidPath }) => { + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + + const session = await client.resumeSession( + { provider: "acp", sessionId: "archived-session", metadata: { cwd: removedCwd } }, + undefined, + undefined, + { purpose: "history" }, + ); + try { + const pid = Number(await readFile(pidPath, "utf8")); + expect(isProcessAlive(pid)).toBe(false); + expect(await collectTimelineText(session.streamHistory())).toEqual([ + "user_message:What is in the README?", + "assistant_message:It describes the project.", + ]); + } finally { + await session.close(); + } + }); + }); + + test("keeps the provider process for an interactive resume", async () => { + await withFakeACPAgent(async ({ command, removedCwd, pidPath }) => { + await mkdir(removedCwd, { recursive: true }); + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + + const session = await client.resumeSession( + { provider: "acp", sessionId: "archived-session", metadata: { cwd: removedCwd } }, + undefined, + undefined, + { purpose: "interactive" }, + ); + try { + const pid = Number(await readFile(pidPath, "utf8")); + expect(isProcessAlive(pid)).toBe(true); + } finally { + await session.close(); + } + }); + }); }); +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + async function collectTimelineText(events: AsyncGenerator): Promise { const text: string[] = []; for await (const event of events) { @@ -88,17 +140,20 @@ async function withFakeACPAgent( command: [string, ...string[]]; removedCwd: string; loadRequestsPath: string; + pidPath: string; }) => Promise, ): Promise { const testDir = await mkdtemp(path.join(tmpdir(), "paseo-acp-archived-history-")); try { const scriptPath = path.join(testDir, "fake-acp-agent.cjs"); const loadRequestsPath = path.join(testDir, "load-requests.json"); + const pidPath = path.join(testDir, "agent.pid"); await writeFile(scriptPath, fakeACPAgentScript, "utf8"); await run({ - command: [process.execPath, scriptPath, loadRequestsPath], + command: [process.execPath, scriptPath, loadRequestsPath, pidPath], removedCwd: path.join(testDir, "removed-worktree"), loadRequestsPath, + pidPath, }); } finally { await rm(testDir, { recursive: true, force: true }); @@ -110,6 +165,10 @@ const fs = require("node:fs"); const readline = require("node:readline"); const loadRequestsPath = process.argv[2]; +const pidPath = process.argv[3]; +if (pidPath) { + fs.writeFileSync(pidPath, String(process.pid)); +} const loadRequests = []; const rl = readline.createInterface({ input: process.stdin });