From 7c2774cd1b617f288a0db18dd1249d8a9c722057 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 16:13:28 +0200 Subject: [PATCH] Add ACP session delete support --- src/CodexAcpClient.ts | 4 + src/CodexAcpServer.ts | 36 +++++ src/CodexAppServerClient.ts | 6 + .../data/session-delete-active-turn.json | 31 ++++ .../data/session-delete-idle.json | 20 +++ .../data/session-delete-unknown-local.json | 10 ++ .../CodexACPAgent/initialize.test.ts | 1 + .../CodexACPAgent/session-delete.test.ts | 147 ++++++++++++++++++ 8 files changed, 255 insertions(+) create mode 100644 src/__tests__/CodexACPAgent/data/session-delete-active-turn.json create mode 100644 src/__tests__/CodexACPAgent/data/session-delete-idle.json create mode 100644 src/__tests__/CodexACPAgent/data/session-delete-unknown-local.json create mode 100644 src/__tests__/CodexACPAgent/session-delete.test.ts diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 0f873e4b..6fc34438 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -270,6 +270,10 @@ export class CodexAcpClient { } } + async deleteSession(sessionId: string): Promise { + await this.codexClient.threadArchive({threadId: sessionId}); + } + async awaitMcpServerStartup(serverNames: Array, afterVersion: number): Promise { return await this.codexClient.awaitMcpServerStartup(serverNames, afterVersion); } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 0003c058..9b92c1fe 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -160,6 +160,7 @@ export class CodexAcpServer implements acp.Agent { resume: { }, list: { }, close: { }, + delete: { }, }, mcpCapabilities: { acp: false, @@ -455,6 +456,41 @@ export class CodexAcpServer implements acp.Agent { return {}; } + async unstable_deleteSession(params: acp.DeleteSessionRequest): Promise { + logger.log("Deleting session...", {sessionId: params.sessionId}); + const sessionId = params.sessionId; + const shouldCloseLocalSession = this.hasLocalSession(sessionId); + + this.beginSessionCloseFence(sessionId); + try { + if (shouldCloseLocalSession) { + await this.closeSession({sessionId}); + } else { + this.bumpSessionGeneration(sessionId); + } + + await this.runWithProcessCheck(() => this.codexAcpClient.deleteSession(sessionId)); + logger.log("Session deleted", {sessionId}); + } finally { + this.endSessionCloseFence(sessionId); + } + + return {}; + } + + private hasLocalSession(sessionId: string): boolean { + return this.sessions.has(sessionId) + || this.pendingMcpStartupSessions.has(sessionId) + || this.pendingTurnStarts.has(sessionId) + || this.activePrompts.has(sessionId) + || this.hasPendingSessionOpen(sessionId) + || this.sessionIsClosing(sessionId); + } + + private hasPendingSessionOpen(sessionId: string): boolean { + return this.sessionOpenGenerations.get(sessionId) === this.getSessionGeneration(sessionId); + } + async newSession( params: acp.NewSessionRequest, ): Promise { diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index d5145346..0bb5e1a4 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -24,6 +24,8 @@ import type { SkillsExtraRootsSetParams, SkillsListParams, SkillsListResponse, + ThreadArchiveParams, + ThreadArchiveResponse, ThreadLoadedListParams, ThreadLoadedListResponse, ThreadListParams, @@ -241,6 +243,10 @@ export class CodexAppServerClient { return await this.sendRequest({ method: "thread/read", params: params }); } + async threadArchive(params: ThreadArchiveParams): Promise { + return await this.sendRequest({ method: "thread/archive", params: params }); + } + async threadUnsubscribe(params: ThreadUnsubscribeParams): Promise { return await this.sendRequest({ method: "thread/unsubscribe", params: params }); } diff --git a/src/__tests__/CodexACPAgent/data/session-delete-active-turn.json b/src/__tests__/CodexACPAgent/data/session-delete-active-turn.json new file mode 100644 index 00000000..658df344 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/session-delete-active-turn.json @@ -0,0 +1,31 @@ +{ + "eventType": "request", + "method": "turn/interrupt", + "params": { + "threadId": "session-id", + "turnId": "turn-id" + } +} +{ + "eventType": "response" +} +{ + "eventType": "request", + "method": "thread/unsubscribe", + "params": { + "threadId": "session-id" + } +} +{ + "eventType": "response" +} +{ + "eventType": "request", + "method": "thread/archive", + "params": { + "threadId": "session-id" + } +} +{ + "eventType": "response" +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/session-delete-idle.json b/src/__tests__/CodexACPAgent/data/session-delete-idle.json new file mode 100644 index 00000000..1cbaca3b --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/session-delete-idle.json @@ -0,0 +1,20 @@ +{ + "eventType": "request", + "method": "thread/unsubscribe", + "params": { + "threadId": "session-id" + } +} +{ + "eventType": "response" +} +{ + "eventType": "request", + "method": "thread/archive", + "params": { + "threadId": "session-id" + } +} +{ + "eventType": "response" +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/session-delete-unknown-local.json b/src/__tests__/CodexACPAgent/data/session-delete-unknown-local.json new file mode 100644 index 00000000..5862191d --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/session-delete-unknown-local.json @@ -0,0 +1,10 @@ +{ + "eventType": "request", + "method": "thread/archive", + "params": { + "threadId": "session-id" + } +} +{ + "eventType": "response" +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/initialize.test.ts b/src/__tests__/CodexACPAgent/initialize.test.ts index 0d6e3af2..0cbc45e6 100644 --- a/src/__tests__/CodexACPAgent/initialize.test.ts +++ b/src/__tests__/CodexACPAgent/initialize.test.ts @@ -50,6 +50,7 @@ describe('CodexACPAgent - initialize', () => { resume: {}, list: {}, close: {}, + delete: {}, }, mcpCapabilities: { acp: false, diff --git a/src/__tests__/CodexACPAgent/session-delete.test.ts b/src/__tests__/CodexACPAgent/session-delete.test.ts new file mode 100644 index 00000000..9b551da2 --- /dev/null +++ b/src/__tests__/CodexACPAgent/session-delete.test.ts @@ -0,0 +1,147 @@ +import {describe, expect, it, vi} from "vitest"; +import { + createCodexMockTestFixture, + createTestModel, + type CodexMockTestFixture, +} from "../acp-test-utils"; +import type {CodexAcpServer} from "../../CodexAcpServer"; +import type {CodexAcpClient} from "../../CodexAcpClient"; + +const sessionId = "session-id"; + +describe("ACP session delete", () => { + it("advertises session delete support", async () => { + const fixture = createCodexMockTestFixture(); + + const response = await fixture.getCodexAcpAgent().initialize({protocolVersion: 1}); + + expect(response.agentCapabilities?.sessionCapabilities?.delete).toEqual({}); + }); + + it("archives sessions that are not active locally", async () => { + const fixture = createCodexMockTestFixture(); + + await expect(fixture.getCodexAcpAgent().unstable_deleteSession({sessionId})).resolves.toEqual({}); + + await expect(fixture.getCodexConnectionDump([])).toMatchFileSnapshot( + "data/session-delete-unknown-local.json" + ); + }); + + it("closes local session resources before archiving", async () => { + const {fixture, codexAcpAgent} = await createSession(); + + await expect(codexAcpAgent.unstable_deleteSession({sessionId})).resolves.toEqual({}); + + await expect(fixture.getCodexConnectionDump([])).toMatchFileSnapshot( + "data/session-delete-idle.json" + ); + expect(() => codexAcpAgent.getSessionState(sessionId)).toThrow(`Session ${sessionId} not found`); + }); + + it("does not close again when deleting a previously closed session", async () => { + const {fixture, codexAcpAgent} = await createSession(); + + await codexAcpAgent.closeSession({sessionId}); + fixture.clearCodexConnectionDump(); + + await expect(codexAcpAgent.unstable_deleteSession({sessionId})).resolves.toEqual({}); + + await expect(fixture.getCodexConnectionDump([])).toMatchFileSnapshot( + "data/session-delete-unknown-local.json" + ); + }); + + it("interrupts active turns before archiving", async () => { + const {fixture, codexAcpAgent} = await createSession(); + codexAcpAgent.getSessionState(sessionId).currentTurnId = "turn-id"; + + await expect(codexAcpAgent.unstable_deleteSession({sessionId})).resolves.toEqual({}); + + await expect(fixture.getCodexConnectionDump([])).toMatchFileSnapshot( + "data/session-delete-active-turn.json" + ); + expect(() => codexAcpAgent.getSessionState(sessionId)).toThrow(`Session ${sessionId} not found`); + }); + + it("rejects resume while unknown-local delete archive is in flight", async () => { + const fixture = createCodexMockTestFixture(); + const codexAcpAgent = fixture.getCodexAcpAgent(); + const codexAcpClient = fixture.getCodexAcpClient(); + const archive = deferred(); + vi.spyOn(codexAcpClient, "authRequired").mockResolvedValue(false); + vi.spyOn(codexAcpClient, "deleteSession").mockReturnValue(archive.promise); + const resumeSessionSpy = vi.spyOn(codexAcpClient, "resumeSession"); + + const deletePromise = codexAcpAgent.unstable_deleteSession({sessionId}); + await vi.waitFor(() => { + expect(codexAcpClient.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + await expect(codexAcpAgent.resumeSession({ + sessionId, + cwd: "/test/cwd", + mcpServers: [], + })).rejects.toThrow("Invalid request"); + expect(resumeSessionSpy).not.toHaveBeenCalled(); + + archive.resolve(undefined); + await expect(deletePromise).resolves.toEqual({}); + }); + + it("rejects load after local close completes while delete archive is in flight", async () => { + const {codexAcpAgent, codexAcpClient} = await createSession(); + const archive = deferred(); + vi.spyOn(codexAcpClient, "deleteSession").mockReturnValue(archive.promise); + const loadSessionSpy = vi.spyOn(codexAcpClient, "loadSession"); + + const deletePromise = codexAcpAgent.unstable_deleteSession({sessionId}); + await vi.waitFor(() => { + expect(codexAcpClient.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + await expect(codexAcpAgent.loadSession({ + sessionId, + cwd: "/test/cwd", + mcpServers: [], + })).rejects.toThrow("Invalid request"); + expect(loadSessionSpy).not.toHaveBeenCalled(); + + archive.resolve(undefined); + await expect(deletePromise).resolves.toEqual({}); + }); +}); + +async function createSession(): Promise<{ + fixture: CodexMockTestFixture, + codexAcpAgent: CodexAcpServer, + codexAcpClient: CodexAcpClient, +}> { + const fixture = createCodexMockTestFixture(); + const codexAcpAgent = fixture.getCodexAcpAgent(); + const codexAcpClient = fixture.getCodexAcpClient(); + const model = createTestModel(); + + vi.spyOn(codexAcpClient, "authRequired").mockResolvedValue(false); + vi.spyOn(codexAcpClient, "getAccount").mockResolvedValue({account: null, requiresOpenaiAuth: false}); + vi.spyOn(codexAcpClient, "newSession").mockResolvedValue({ + sessionId, + currentModelId: "model-id[medium]", + models: [model], + currentServiceTier: null, + }); + + await codexAcpAgent.newSession({cwd: "/test/cwd", mcpServers: []}); + fixture.clearCodexConnectionDump(); + fixture.clearAcpConnectionDump(); + + return {fixture, codexAcpAgent, codexAcpClient}; +} + +function deferred(): {promise: Promise, resolve: (value: T) => void} { + let resolve: (value: T) => void = () => {}; + const promise = new Promise((innerResolve) => { + resolve = innerResolve; + }); + return {promise, resolve}; +}