diff --git a/src/acp-agent.ts b/src/acp-agent.ts index b5924d4d..f6390591 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -533,8 +533,8 @@ export type Session = { * without ever reaching the model). */ queryClosed?: boolean; cwd: string; - /** Serialized snapshot of session-defining params (cwd, mcpServers) used to - * detect when loadSession/resumeSession is called with changed values. */ + /** Serialized snapshot of session-defining params (cwd, mcpServers, skills) + * used to detect when loadSession/resumeSession is called with changed values. */ sessionFingerprint: string; /** Original ACP parameters used to recreate this query with a new provider. */ creationParams?: NewSessionRequest; @@ -830,16 +830,32 @@ function disarmForceCancel(session: Session): void { } } +/** Normalize skills without changing their SDK semantics. Array order and + * duplicates do not affect the selected skill set, so neither should rebuild + * the underlying Query process. */ +function normalizeSkills(skills: Options["skills"]): Options["skills"] { + return Array.isArray(skills) ? [...new Set(skills)].sort() : skills; +} + /** Compute a stable fingerprint of the session-defining params so we can * detect when a loadSession/resumeSession call requires tearing down and - * recreating the underlying Query process. MCP servers are sorted by name - * so that ordering differences don't trigger unnecessary recreations. */ + * recreating the underlying Query process. MCP servers are sorted by name, + * and skills are normalized as a set, so ordering differences don't trigger + * unnecessary recreations. */ function computeSessionFingerprint(params: { cwd: string; mcpServers?: NewSessionRequest["mcpServers"]; + _meta?: NewSessionRequest["_meta"]; }): string { const servers = [...(params.mcpServers ?? [])].sort((a, b) => a.name.localeCompare(b.name)); - return JSON.stringify({ cwd: params.cwd, mcpServers: servers }); + const skills = normalizeSkills( + (params._meta as NewSessionMeta | undefined)?.claudeCode?.options?.skills, + ); + return JSON.stringify({ + cwd: params.cwd, + mcpServers: servers, + ...(skills !== undefined && { skills }), + }); } export type SDKMessageFilter = { @@ -5935,8 +5951,9 @@ export class ClaudeAcpAgent { } // Session-defining params changed (e.g. cwd pointed at a git worktree, - // or MCP servers reconfigured). Tear down the existing session and - // recreate it so the underlying Query process picks up the new values. + // MCP servers reconfigured, or the skill set changed). Tear down the + // existing session and recreate it so the underlying Query process picks + // up the new values. await this.teardownSession(params.sessionId); } diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 26a043fd..a86dd55d 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -54,6 +54,7 @@ import { PermissionUpdate, query, SDKAssistantMessage, + type Options, } from "@anthropic-ai/claude-agent-sdk"; import { randomUUID } from "crypto"; import { readFile } from "node:fs/promises"; @@ -6391,10 +6392,15 @@ describe("getOrCreateSession param change detection", () => { function injectSession( agent: ClaudeAcpAgent, sessionId: string, - opts: { cwd?: string; mcpServers?: { name: string }[] } = {}, + opts: { + cwd?: string; + mcpServers?: { name: string }[]; + skills?: Options["skills"]; + } = {}, ) { const cwd = opts.cwd ?? "/test"; const mcpServers = (opts.mcpServers ?? []) as any[]; + const skills = Array.isArray(opts.skills) ? [...new Set(opts.skills)].sort() : opts.skills; function* empty() {} const gen = Object.assign(empty(), { interrupt: vi.fn(), @@ -6410,6 +6416,7 @@ describe("getOrCreateSession param change detection", () => { sessionFingerprint: JSON.stringify({ cwd, mcpServers: [...mcpServers].sort((a: any, b: any) => a.name.localeCompare(b.name)), + ...(skills !== undefined && { skills }), }), modes: { currentModeId: "default", availableModes: [] }, models: { currentModelId: "default", availableModels: [] }, @@ -6530,6 +6537,90 @@ describe("getOrCreateSession param change detection", () => { expect(agent.sessions["s1"]).toBe(session); expect(session.settingsManager.dispose).not.toHaveBeenCalled(); }); + + it.each<{ + label: string; + previousSkills: Options["skills"]; + nextSkills: Options["skills"]; + }>([ + { label: "CLI defaults to all skills", previousSkills: undefined, nextSkills: "all" }, + { label: "all skills to no skills", previousSkills: "all", nextSkills: [] }, + { label: "no skills to CLI defaults", previousSkills: [], nextSkills: undefined }, + { label: "one explicit skill list to another", previousSkills: ["pdf"], nextSkills: ["docx"] }, + ])( + "tears down the existing session when skills change from $label", + async ({ previousSkills, nextSkills }) => { + const agent = createMockAgent(); + const session = injectSession(agent, "s1", { + cwd: "/project", + skills: previousSkills, + }); + const meta = + nextSkills === undefined ? undefined : { claudeCode: { options: { skills: nextSkills } } }; + const createSessionSpy = vi + .spyOn(agent as any, "createSession") + .mockRejectedValue(new Error("mock")); + + await expect( + agent.resumeSession({ + sessionId: "s1", + cwd: "/project", + mcpServers: [], + _meta: meta, + }), + ).rejects.toThrow("mock"); + + expect(session.settingsManager.dispose).toHaveBeenCalled(); + expect(session.abortController.signal.aborted).toBe(true); + expect(session.query.interrupt).toHaveBeenCalled(); + expect(agent.sessions["s1"]).toBeUndefined(); + expect(createSessionSpy).toHaveBeenCalledWith( + expect.objectContaining({ _meta: meta }), + expect.objectContaining({ resume: "s1" }), + ); + }, + ); + + it("treats reordered and duplicate skills as unchanged", async () => { + const agent = createMockAgent(); + const session = injectSession(agent, "s1", { + cwd: "/project", + skills: ["pdf", "docx"], + }); + + await agent.resumeSession({ + sessionId: "s1", + cwd: "/project", + mcpServers: [], + _meta: { + claudeCode: { + options: { skills: ["pdf", "docx", "pdf"] }, + }, + }, + }); + + expect(agent.sessions["s1"]).toBe(session); + expect(session.settingsManager.dispose).not.toHaveBeenCalled(); + }); + + it("ignores unrelated Claude options when computing the fingerprint", async () => { + const agent = createMockAgent(); + const session = injectSession(agent, "s1", { cwd: "/project" }); + + await agent.resumeSession({ + sessionId: "s1", + cwd: "/project", + mcpServers: [], + _meta: { + claudeCode: { + options: { env: { CUSTOM_ENV: "changed" } }, + }, + }, + }); + + expect(agent.sessions["s1"]).toBe(session); + expect(session.settingsManager.dispose).not.toHaveBeenCalled(); + }); }); describe("usage_update computation", () => { diff --git a/src/tests/create-session-options.test.ts b/src/tests/create-session-options.test.ts index d0b5aaa7..65af6455 100644 --- a/src/tests/create-session-options.test.ts +++ b/src/tests/create-session-options.test.ts @@ -18,6 +18,8 @@ vi.mock("@anthropic-ai/claude-agent-sdk", async () => { query: (args: { prompt: unknown; options: Options }) => { capturedOptions = args.options; return makeMockQuery({ + interrupt: async () => undefined, + close: () => {}, initializationResult: async () => ({ models: [ { @@ -263,6 +265,36 @@ describe("createSession options merging", () => { expect(capturedOptions!.tools).toEqual([]); }); + it("recreates a resumed Query with changed skills and the same session ID", async () => { + const created = await agent.newSession({ + cwd: process.cwd(), + mcpServers: [], + _meta: { + claudeCode: { + options: { skills: ["pdf"] }, + }, + }, + }); + const initialQuery = agent.sessions[created.sessionId]!.query; + expect(capturedOptions!.skills).toEqual(["pdf"]); + + await agent.resumeSession({ + sessionId: created.sessionId, + cwd: process.cwd(), + mcpServers: [], + _meta: { + claudeCode: { + options: { skills: ["docx"] }, + }, + }, + }); + + expect(Object.keys(agent.sessions)).toEqual([created.sessionId]); + expect(agent.sessions[created.sessionId]!.query).not.toBe(initialQuery); + expect(capturedOptions!.resume).toBe(created.sessionId); + expect(capturedOptions!.skills).toEqual(["docx"]); + }); + describe("subagent transcript forwarding", () => { it("keeps the legacy default when neither the client nor caller opts in", async () => { await agent.newSession({ cwd: process.cwd(), mcpServers: [] });