mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix(session): recreate query when configured skills change (#968)
## Summary - include normalized Claude SDK `skills` in the resumed-session fingerprint - preserve the distinct semantics of omitted, `'all'`, empty, and explicit skill selections - avoid unnecessary Query recreation for reordered or duplicate skill names - keep unrelated `_meta.claudeCode.options` fields out of the fingerprint - recreate the Query with the same native session ID when skills change - merge the latest `origin/main` so the change is validated against the current session logic and SDK 0.3.238 ## Testing - clean Linux Node 22 environment with `@anthropic-ai/claude-agent-sdk@0.3.238` - `npm run build` - `npm run lint` - `npm run check` - targeted lifecycle tests: `425 passed, 9 skipped` - full test suite: `978 passed, 27 skipped` - successful lifecycle case: `newSession(['pdf']) -> resumeSession(['docx'])` Closes #955
This commit is contained in:
@@ -815,8 +815,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;
|
||||
@@ -1201,16 +1201,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 = {
|
||||
@@ -7915,8 +7931,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);
|
||||
}
|
||||
|
||||
|
||||
@@ -59,6 +59,7 @@ import {
|
||||
PermissionUpdate,
|
||||
query,
|
||||
SDKAssistantMessage,
|
||||
type Options,
|
||||
type SDKControlGetUsageResponse,
|
||||
} from "@anthropic-ai/claude-agent-sdk";
|
||||
import { createHash, randomUUID } from "crypto";
|
||||
@@ -9309,10 +9310,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(),
|
||||
@@ -9328,6 +9334,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: [] },
|
||||
@@ -9445,6 +9452,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", () => {
|
||||
|
||||
@@ -34,6 +34,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: initModels ?? [
|
||||
{
|
||||
@@ -319,6 +321,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: [] });
|
||||
|
||||
Reference in New Issue
Block a user