From df38cccae04ce4abb525447c5e8d9db78ffec61c Mon Sep 17 00:00:00 2001 From: Jake Gaylor Date: Wed, 23 Sep 2026 06:01:37 -0400 Subject: [PATCH] fix: recreate resumed sessions when any session option changes (#1097) A warm `session/resume` or `session/load` currently ignores changed `maxTurns` and `maxBudgetUsd`: the session fingerprint compares only cwd and MCP servers, so the SDK query retains its old options. A client can request a tighter limit and receive success while the old limit remains active. Include these two SDK execution limits in the session fingerprint. Adding, changing or removing a limit recreates the query with the supplied options and the same session ID. Identical limits reuse the existing query; object key order does not trigger recreation. Budget accounting across recreated SDK processes remains the client's responsibility. Validation: - Four regression cases failed before the fix; six new cases cover both methods, both limits, tightening/removal, and unchanged reuse. - `npm run check` and `npm run build` passed. - Focused suites: 551 passed, 9 skipped. - Full suite with `--maxWorkers=2`: 1,240 passed, 27 skipped. An initial unrestricted run hit one existing provider-switch test's 5s timeout and macOS `EMFILE` watcher errors; bounding workers passed without changing tests or timeouts. Prepared while integrating typed runtime limits in BinaryBourbon/fountain#1732. No live SDK budget-enforcement claim is made by these adapter tests. --------- Signed-off-by: Jake Gaylor Co-authored-by: Ben Brandt Co-authored-by: Claude Opus 5.5 (1M context) --- src/acp-agent.ts | 135 +++++++++++++++++++++-- src/tests/acp-agent.test.ts | 72 +++++++++++- src/tests/create-session-options.test.ts | 42 +++++++ 3 files changed, 232 insertions(+), 17 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 23da70b3..e0f7e312 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -1208,25 +1208,138 @@ function normalizeSkills(skills: Options["skills"]): Options["skills"] { return Array.isArray(skills) ? [...new Set(skills)].sort() : skills; } +/** Whether a changed `_meta.claudeCode.options` key on session/load or + * session/resume must rebuild the Query process. Almost every option is read + * only when the process starts, so the default is `true`. Every key of the + * SDK's `Options` must be classified: an SDK bump that adds or removes an + * option fails to compile until it is decided here. */ +const OPTION_REBUILDS_SESSION = { + additionalDirectories: true, + agentProgressSummaries: true, + agents: true, + allowDangerouslySkipPermissions: true, + allowedTools: true, + betas: true, + debug: true, + debugFile: true, + disallowedTools: true, + effort: true, + enableFileCheckpointing: true, + env: true, + executableArgs: true, + extraArgs: true, + fallbackModel: true, + forwardSubagentText: true, + includeHookEvents: true, + managedSettings: true, + maxBudgetUsd: true, + maxThinkingTokens: true, + maxTurns: true, + mcpServers: true, + model: true, + outputFormat: true, + pathToClaudeCodeExecutable: true, + permissionPrompts: true, + permissionPromptToolName: true, + persistSession: true, + perTaskStopAffordance: true, + planModeInstructions: true, + pluginDelivery: true, + plugins: true, + projectConfigRoot: true, + promptSuggestions: true, + sandbox: true, + sessionStoreFlush: true, + settings: true, + settingSources: true, + skills: true, + strictMcpConfig: true, + supportedDialogKinds: true, + systemPrompt: true, + taskBudget: true, + thinking: true, + toolAliases: true, + toolConfig: true, + tools: true, + verbatimPrompts: true, + // Managed or ignored by the adapter. + agent: false, + cwd: false, + executable: false, + includePartialMessages: false, + permissionMode: false, + // Per-call controls for attaching to the stored conversation. + continue: false, + forkSession: false, + resume: false, + resumeDropsTurn: false, + resumeSessionAt: false, + sessionId: false, + // Callbacks and live objects: they cannot cross JSON-RPC and have no stable serialized form. + abortController: false, + canUseTool: false, + hooks: false, + onElicitation: false, + onUserDialog: false, + sessionStore: false, + spawnClaudeCodeProcess: false, + stderr: false, + // No effect on an already-created session. + loadTimeoutMs: false, + title: false, +} as const satisfies Record; + +/** A JSON value with object keys sorted, so key order never changes a + * fingerprint. Functions and class instances (hook callbacks, abort + * controllers, in-process MCP servers) cannot cross JSON-RPC and have no + * stable serialized form, so they are left out. */ +function canonicalJson(value: unknown): unknown { + if (typeof value === "function") return undefined; + if (value === null || typeof value !== "object") return value; + if (Array.isArray(value)) return value.map(canonicalJson); + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return undefined; + return Object.fromEntries( + Object.keys(value) + .sort() + .map((key) => [key, canonicalJson((value as Record)[key])]) + .filter(([, entry]) => entry !== undefined), + ); +} + /** 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, - * and skills are normalized as a set, so ordering differences don't trigger - * unnecessary recreations. */ -function computeSessionFingerprint(params: { + * recreating the underlying Query process. Every option classified in + * {@link OPTION_REBUILDS_SESSION} is covered, so a warm resume never silently + * keeps stale values. MCP servers are sorted by name, and skills are + * normalized as a set, so ordering differences don't trigger unnecessary + * recreations. */ +export function computeSessionFingerprint(params: { cwd: string; mcpServers?: NewSessionRequest["mcpServers"]; + additionalDirectories?: NewSessionRequest["additionalDirectories"]; _meta?: NewSessionRequest["_meta"]; }): string { const servers = [...(params.mcpServers ?? [])].sort((a, b) => a.name.localeCompare(b.name)); - const skills = normalizeSkills( - (params._meta as NewSessionMeta | undefined)?.claudeCode?.options?.skills, + const meta = params._meta as (NewSessionMeta & Record) | undefined; + const options: Record = Object.fromEntries( + Object.entries(meta?.claudeCode?.options ?? {}).filter( + ([key]) => (OPTION_REBUILDS_SESSION as Record)[key] === true, + ), + ); + options.skills = normalizeSkills(options.skills as Options["skills"]); + return JSON.stringify( + canonicalJson({ + cwd: params.cwd, + mcpServers: servers, + additionalDirectories: params.additionalDirectories, + additionalRoots: meta?.additionalRoots, + systemPrompt: meta?.systemPrompt, + disableBuiltInTools: meta?.disableBuiltInTools, + emitRawSDKMessages: meta?.claudeCode?.emitRawSDKMessages, + options, + }), ); - return JSON.stringify({ - cwd: params.cwd, - mcpServers: servers, - ...(skills !== undefined && { skills }), - }); } export type SDKMessageFilter = { diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index d3bbaf93..0c484fe7 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -37,6 +37,7 @@ import { stripLocalCommandMetadata, ClaudeAcpAgent, claudeCliPath, + computeSessionFingerprint, streamEventToAcpNotifications, messageIdForGrouping, buildConfigOptions, @@ -9419,11 +9420,15 @@ describe("getOrCreateSession param change detection", () => { cwd?: string; mcpServers?: { name: string }[]; skills?: Options["skills"]; + options?: Options; } = {}, ) { const cwd = opts.cwd ?? "/test"; const mcpServers = (opts.mcpServers ?? []) as any[]; - const skills = Array.isArray(opts.skills) ? [...new Set(opts.skills)].sort() : opts.skills; + const options = { + ...opts.options, + ...(opts.skills !== undefined && { skills: opts.skills }), + }; function* empty() {} const gen = Object.assign(empty(), { interrupt: vi.fn(), @@ -9436,10 +9441,10 @@ describe("getOrCreateSession param change detection", () => { cancelled: false, titles: new SessionTitles(agent, sessionId), cwd, - sessionFingerprint: JSON.stringify({ + sessionFingerprint: computeSessionFingerprint({ cwd, - mcpServers: [...mcpServers].sort((a: any, b: any) => a.name.localeCompare(b.name)), - ...(skills !== undefined && { skills }), + mcpServers, + _meta: { claudeCode: { options } }, }), modes: { currentModeId: "default", availableModes: [] }, models: { currentModelId: "default", availableModels: [] }, @@ -9623,9 +9628,54 @@ describe("getOrCreateSession param change detection", () => { expect(session.settingsManager.dispose).not.toHaveBeenCalled(); }); - it("ignores unrelated Claude options when computing the fingerprint", async () => { + it("tears down the existing session when any other Claude option changes", async () => { + const agent = createMockAgent(); + const session = injectSession(agent, "s1", { + cwd: "/project", + options: { env: { CUSTOM_ENV: "original" } }, + }); + const createSessionSpy = vi + .spyOn(agent as any, "createSession") + .mockRejectedValue(new Error("mock")); + + await expect( + agent.resumeSession({ + sessionId: "s1", + cwd: "/project", + mcpServers: [], + _meta: { claudeCode: { options: { env: { CUSTOM_ENV: "changed" } } } }, + }), + ).rejects.toThrow("mock"); + + expect(session.settingsManager.dispose).toHaveBeenCalled(); + expect(agent.sessions["s1"]).toBeUndefined(); + expect(createSessionSpy).toHaveBeenCalled(); + }); + + it("tears down the existing session when additionalDirectories change", async () => { const agent = createMockAgent(); const session = injectSession(agent, "s1", { cwd: "/project" }); + vi.spyOn(agent as any, "createSession").mockRejectedValue(new Error("mock")); + + await expect( + agent.resumeSession({ + sessionId: "s1", + cwd: "/project", + mcpServers: [], + additionalDirectories: ["/other"], + }), + ).rejects.toThrow("mock"); + + expect(session.settingsManager.dispose).toHaveBeenCalled(); + expect(agent.sessions["s1"]).toBeUndefined(); + }); + + it("ignores option key order, per-call resume controls and creation-only options", async () => { + const agent = createMockAgent(); + const session = injectSession(agent, "s1", { + cwd: "/project", + options: { maxTurns: 10, env: { A: "1", B: "2" } }, + }); await agent.resumeSession({ sessionId: "s1", @@ -9633,7 +9683,17 @@ describe("getOrCreateSession param change detection", () => { mcpServers: [], _meta: { claudeCode: { - options: { env: { CUSTOM_ENV: "changed" } }, + options: { + env: { B: "2", A: "1" }, + maxTurns: 10, + resume: "s1", + sessionId: "s1", + forkSession: false, + resumeSessionAt: "msg-1", + resumeDropsTurn: "msg-1", + title: "Renamed", + abortController: new AbortController(), + }, }, }, }); diff --git a/src/tests/create-session-options.test.ts b/src/tests/create-session-options.test.ts index 9c2865fc..11f55fae 100644 --- a/src/tests/create-session-options.test.ts +++ b/src/tests/create-session-options.test.ts @@ -99,6 +99,48 @@ describe("createSession options merging", () => { agent = new ClaudeAcpAgent(createMockClient()); }); + for (const method of ["resumeSession", "loadSession"] as const) { + for (const limit of ["maxTurns", "maxBudgetUsd"] as const) { + it(`${method} applies added, tightened and removed ${limit} to the SDK query`, async () => { + const cwd = process.cwd(); + const { sessionId } = await agent.newSession({ cwd, mcpServers: [] }); + + for (const value of [10, 1, undefined]) { + const previousOptions = capturedOptions; + await agent[method]({ + sessionId, + cwd, + mcpServers: [], + _meta: { claudeCode: { options: { [limit]: value } } }, + }); + + expect(capturedOptions).not.toBe(previousOptions); + expect(capturedOptions?.[limit]).toBe(value); + expect(capturedOptions?.resume).toBe(sessionId); + } + }); + } + + it(`${method} reuses the SDK query when execution limits are unchanged`, async () => { + const cwd = process.cwd(); + const { sessionId } = await agent.newSession({ + cwd, + mcpServers: [], + _meta: { claudeCode: { options: { maxTurns: 10, maxBudgetUsd: 1 } } }, + }); + const previousOptions = capturedOptions; + + await agent[method]({ + sessionId, + cwd, + mcpServers: [], + _meta: { claudeCode: { options: { maxBudgetUsd: 1, maxTurns: 10 } } }, + }); + + expect(capturedOptions).toBe(previousOptions); + }); + } + describe("allowDangerouslySkipPermissions", () => { it("requests bypass capability by default and mirrors it in the mode catalog", async () => { const response = await agent.newSession({ cwd: process.cwd(), mcpServers: [] });