diff --git a/docs/air-extensions.md b/docs/air-extensions.md index cc8a4045..831894b7 100644 --- a/docs/air-extensions.md +++ b/docs/air-extensions.md @@ -1423,7 +1423,8 @@ It is idempotent, and an unknown id also returns `{}`. - a `sessionIndex` client: deletes every transcript of the session (empty ones too) and every `/` directory, also one left without a transcript by an earlier failed delete. Anything left behind fails the request. A session that runs here is closed first. As with the CLI, the session is deleted also when another Claude Code process has it open. An unknown session is `-32002`. - an AIR client without `sessionIndex`: archives the session instead of deleting it (the title records of [Archive](#archive)), because AIR uses delete for "Done" and can reopen the session. A session that runs here is closed first. The list hides archived sessions from such a client too: those whose custom title, as the SDK list reports it, is archived. A session without a transcript fails with the error of the SDK delete, as before. A `sessionIndex` client sees such a session as archived (`archived: true`), as AIR's native Claude path does. -- any other client: the SDK delete, as before. +- any other client: the SDK delete. A session without a transcript (unknown, deleted before, or + never prompted) succeeds, as ACP asks. `session/close` of a session that is not loaded returns `{}` for every client. diff --git a/src/session-index/connection.ts b/src/session-index/connection.ts index 1eb82160..ee25b8e8 100644 --- a/src/session-index/connection.ts +++ b/src/session-index/connection.ts @@ -16,7 +16,6 @@ import { type ListSessionsRequest, type ListSessionsResponse, } from "@agentclientprotocol/sdk"; -import { deleteSession as sdkDeleteSession } from "@anthropic-ai/claude-agent-sdk"; import type { Query } from "@anthropic-ai/claude-agent-sdk"; import type { AcpClient, Logger, Session } from "../acp-agent.js"; import { @@ -41,6 +40,7 @@ import { } from "./list-subscriptions.js"; import { archiveInsteadOfDelete, + deleteSessionIfStored, archiveTo, renameTo, parseRenameSessionRequest, @@ -336,7 +336,8 @@ export class SessionIndexConnection { * - Another AIR client uses delete to mark a session done, and may reopen * it later: the adapter archives it instead (as `_session/archive` does, * after the CLI closed), so the transcript survives. - * - Every other client: the SDK delete, as before. + * - Every other client: the SDK delete. A session without a transcript + * succeeds (see {@link deleteSessionIfStored}). */ async deleteSession(params: DeleteSessionRequest): Promise { const loaded = this.host.agent.sessions[params.sessionId] !== undefined; @@ -350,7 +351,7 @@ export class SessionIndexConnection { } else if (this.host.isAirClient()) { await archiveInsteadOfDelete(params.sessionId, this.airArchiver()); } else { - await sdkDeleteSession(params.sessionId); + await deleteSessionIfStored(params.sessionId); } return {}; } diff --git a/src/session-index/service.ts b/src/session-index/service.ts index 5f893942..db96d317 100644 --- a/src/session-index/service.ts +++ b/src/session-index/service.ts @@ -383,6 +383,28 @@ function sdkDeleteError(sessionId: string): Error { ); } +/** + * `session/delete` of a client that is neither AIR nor a `sessionIndex` client: + * the SDK delete of the session's transcript. A session without one succeeds, + * as ACP asks ("Deleting an already-deleted session, or a session that never + * existed, SHOULD succeed silently"): an id that is not a session id, a + * session deleted before, or a session this connection ran but never prompted, + * since Claude Code writes the transcript at the first prompt. Other failures + * still fail the request. + */ +export async function deleteSessionIfStored( + sessionId: string, + deleteSession: (sessionId: string) => Promise = sdkDeleteSession, +): Promise { + if (!isSessionId(sessionId)) return; + try { + await deleteSession(sessionId); + } catch (error) { + if (isSdkNotFound(error)) return; + throw error; + } +} + /** * `session/delete` of an AIR client without `sessionIndex`, which uses delete * to mark a session done: archives it instead, so the transcript survives. A diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 8b3cdb8f..3c3cce67 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -13269,6 +13269,14 @@ describe("session/delete", () => { return agent.sessions[sessionId]!; } + // Session ids are UUIDs: the SDK delete rejects any other id. + const SESSION_1 = "6f0c5f0e-1a2b-4c3d-8e9f-0a1b2c3d4e5f"; + const SESSION_2 = "7a1d6e1f-2b3c-4d4e-9f0a-1b2c3d4e5f60"; + + /** The error the SDK delete throws for a session without a transcript. */ + const sdkNotFound = (sessionId: string) => + new Error(`Session ${sessionId} not found in any project directory`); + beforeEach(() => { vi.mocked(deleteSession).mockReset(); vi.mocked(deleteSession).mockResolvedValue(undefined); @@ -13276,45 +13284,70 @@ describe("session/delete", () => { it("tears down the active session and deletes it from disk", async () => { const agent = createMockAgent(); - const session = injectSession(agent, "session-1"); + const session = injectSession(agent, SESSION_1); - const result = await agent.deleteSession({ sessionId: "session-1" }); + const result = await agent.deleteSession({ sessionId: SESSION_1 }); expect(result).toEqual({}); - expect(agent.sessions["session-1"]).toBeUndefined(); + expect(agent.sessions[SESSION_1]).toBeUndefined(); expect(session.query.interrupt).toHaveBeenCalled(); expect(session.settingsManager.dispose).toHaveBeenCalled(); expect(session.abortController.signal.aborted).toBe(true); - expect(deleteSession).toHaveBeenCalledWith("session-1"); + expect(deleteSession).toHaveBeenCalledWith(SESSION_1); }); it("deletes a session from disk that is not currently active", async () => { const agent = createMockAgent(); - const result = await agent.deleteSession({ sessionId: "not-active" }); + const result = await agent.deleteSession({ sessionId: SESSION_1 }); expect(result).toEqual({}); - expect(deleteSession).toHaveBeenCalledWith("not-active"); + expect(deleteSession).toHaveBeenCalledWith(SESSION_1); }); - it("propagates errors from the SDK delete call", async () => { + it("deletes an active session that was never prompted, which has no transcript", async () => { const agent = createMockAgent(); - vi.mocked(deleteSession).mockRejectedValueOnce(new Error("Session not found on disk")); + injectSession(agent, SESSION_1); + vi.mocked(deleteSession).mockRejectedValueOnce(sdkNotFound(SESSION_1)); - await expect(agent.deleteSession({ sessionId: "missing" })).rejects.toThrow( - "Session not found on disk", + await expect(agent.deleteSession({ sessionId: SESSION_1 })).resolves.toEqual({}); + expect(agent.sessions[SESSION_1]).toBeUndefined(); + }); + + // ACP: "Deleting an already-deleted session, or a session that never + // existed, SHOULD succeed silently." + it("succeeds for a session that does not exist", async () => { + const agent = createMockAgent(); + vi.mocked(deleteSession).mockRejectedValueOnce(sdkNotFound(SESSION_1)); + + await expect(agent.deleteSession({ sessionId: SESSION_1 })).resolves.toEqual({}); + }); + + it("succeeds for an id that is not a session id, without asking the SDK", async () => { + const agent = createMockAgent(); + + await expect(agent.deleteSession({ sessionId: "tck-never-created" })).resolves.toEqual({}); + expect(deleteSession).not.toHaveBeenCalled(); + }); + + it("propagates other errors from the SDK delete call", async () => { + const agent = createMockAgent(); + vi.mocked(deleteSession).mockRejectedValueOnce(new Error("EACCES: permission denied")); + + await expect(agent.deleteSession({ sessionId: SESSION_1 })).rejects.toThrow( + "EACCES: permission denied", ); }); it("does not affect other sessions when deleting one", async () => { const agent = createMockAgent(); - injectSession(agent, "session-a"); - injectSession(agent, "session-b"); + injectSession(agent, SESSION_1); + injectSession(agent, SESSION_2); - await agent.deleteSession({ sessionId: "session-a" }); + await agent.deleteSession({ sessionId: SESSION_1 }); - expect(agent.sessions["session-a"]).toBeUndefined(); - expect(agent.sessions["session-b"]).toBeDefined(); + expect(agent.sessions[SESSION_1]).toBeUndefined(); + expect(agent.sessions[SESSION_2]).toBeDefined(); }); });