From f3d8ae3eb389ee9367f48c8762562a55280ade0a Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 17:37:21 +0200 Subject: [PATCH] fix: Preserve live model on resumed sessions (#848) Closes #845 --- src/acp-agent.ts | 212 +++++++++++++++++++++-- src/tests/model-resolution.test.ts | 105 ++++++++++- src/tests/session-config-options.test.ts | 23 +++ src/tests/session-load.test.ts | 54 +++++- 4 files changed, 372 insertions(+), 22 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 1ae1b865..4f6cad14 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -2643,6 +2643,17 @@ export class ClaudeAcpAgent { : []; let validValue = allValues.find((o) => o.value === params.value); + // The option's reported currentValue is always a valid target, even when + // it has no options entry: a session running an out-of-picker model + // (resumed onto an allowlist-excluded model, or a refusal fallback) + // reports a currentValue that isn't selectable, and a client + // round-tripping it must not get "Invalid value". It flows through the + // normal apply path below — re-asserting an already-current value is + // harmless and can repair SDK drift. + if (!validValue && option.currentValue === params.value) { + validValue = { value: params.value, name: params.value }; + } + // For model options, fall back to resolveModelPreference when the exact // value doesn't match. This lets callers use human-friendly aliases like // "opus" or "sonnet" instead of full model IDs like "claude-opus-4-6". @@ -3802,11 +3813,45 @@ export class ClaudeAcpAgent { initializationResult.models, settingsManager, this.logger, + creationOpts.resume !== undefined, ); // Gate `auto` (and future model-specific modes) on the resolved model's // `ModelInfo`. See `buildAvailableModes` for the canonical SDK signal. - const currentModelInfo = allowedModels.find((m) => m.value === models.currentModelId); + // A resumed session can be running a model outside the `availableModels` + // allowlist (currentModelId is then the verbatim live id, see + // `matchResumedModel`); its capabilities are still known to the SDK's + // unfiltered list, so fall back to that before treating the model as + // unknown — otherwise auto mode would be spuriously clamped and the + // Fast-mode/Effort options hidden for a model that supports them. + const allowlistedModelInfo = allowedModels.find((m) => m.value === models.currentModelId); + const fallbackModelInfo = allowlistedModelInfo + ? undefined + : (resolveModelPreference(initializationResult.models, models.currentModelId) ?? undefined); + const currentModelInfo = allowlistedModelInfo ?? fallbackModelInfo; + // Register the fallback-resolved capabilities under the verbatim live id + // so every modelInfos consumer (buildConfigOptions' effort lookup, later + // rebuilds via session.modelInfos) agrees with the gating below. The + // picker options themselves come from `models.availableModels`, so this + // adds no selectable entry. Capability flags only: the fuzzy-matched + // sibling's resolvedModel/displayName/description can describe a + // different context lane and would poison later resolvedModel matching + // (syncModelAfterRefusalFallback) and context-window inference + // (applyConfigOptionValue) if they traveled under this id. + const modelInfos = fallbackModelInfo + ? [ + ...allowedModels, + { + value: models.currentModelId, + displayName: models.currentModelId, + description: "", + supportsAutoMode: fallbackModelInfo.supportsAutoMode, + supportsFastMode: fallbackModelInfo.supportsFastMode, + supportsEffort: fallbackModelInfo.supportsEffort, + supportedEffortLevels: fallbackModelInfo.supportedEffortLevels, + }, + ] + : allowedModels; const availableModes = buildAvailableModes(currentModelInfo); // Clamp `permissionMode` if the resolved session does not offer it. The @@ -3872,7 +3917,7 @@ export class ClaudeAcpAgent { const configOptions = buildConfigOptions( modes, models, - allowedModels, + modelInfos, settingsManager.getSettings().effortLevel, agents, currentAgent, @@ -3905,7 +3950,7 @@ export class ClaudeAcpAgent { }, modes, models, - modelInfos: allowedModels, + modelInfos, configOptions, agents, currentAgent, @@ -3913,10 +3958,15 @@ export class ClaudeAcpAgent { abortController, emitRawSDKMessages: sessionMeta?.claudeCode?.emitRawSDKMessages ?? false, contextWindowSize: + // Deliberately keyed to the allowlisted entry: a fallback-resolved + // sibling's displayName/description can describe a different context + // lane than the verbatim live id (e.g. an "opus[1m]" row matched for + // a bare 200k id), so on the fallback path only the id itself is a + // trustworthy window signal. inferContextWindowFromModel( models.currentModelId, - currentModelInfo?.displayName, - currentModelInfo?.description, + allowlistedModelInfo?.displayName, + allowlistedModelInfo?.description, ) ?? DEFAULT_CONTEXT_WINDOW, taskState, toolUseCache: {}, @@ -4334,15 +4384,31 @@ export function buildConfigOptions( // but the SDK model list uses IDs like "claude-opus-4-6-1m". const MODEL_CONTEXT_HINT_PATTERN = /\[(\d+m)\]$/i; +// The id-suffix spelling of a context hint ("-1m" in "claude-opus-4-6-1m"); +// shared by the strip and canonicalize helpers below so the two can't drift. +const CONTEXT_HINT_SUFFIX_PATTERN = /-(\d+m)$/i; + +/** Remove context-window hints — the display form "[1m]" and the SDK id + * suffix form "-1m" — from a model string. Those digits describe context + * size, not model identity or generation version. */ +function stripContextHints(s: string): string { + return s.replace(/\[\d+m\]/gi, "").replace(CONTEXT_HINT_SUFFIX_PATTERN, ""); +} + +/** Canonicalize a model id for exact comparison: trimmed, lowercased, with + * the id-suffix hint spelling unified to the bracket form ("-1m" → "[1m]"). + * The hint itself is kept — bare and 1M ids must stay distinct. */ +function canonicalizeModelId(s: string): string { + return s.trim().toLowerCase().replace(CONTEXT_HINT_SUFFIX_PATTERN, "[$1]"); +} + // Captures a model family version: `4-6`/`4.7` for dated generations, or a // bare `5` for single-number ones like "Sonnet 5". Used to keep a pinned // `claude-opus-4-6` from matching the `opus` alias once it points at 4.7. const MODEL_FAMILY_VERSION_PATTERN = /\b(\d+)(?:[-.](\d+))?\b/; function extractModelFamilyVersion(s: string): string | null { - // Strip "[1m]"-style context hints first — that digit is context window - // size, not a model generation version. - const match = s.replace(/\[\d+m\]/gi, "").match(MODEL_FAMILY_VERSION_PATTERN); + const match = stripContextHints(s).match(MODEL_FAMILY_VERSION_PATTERN); if (!match) return null; return match[2] ? `${match[1]}.${match[2]}` : match[1]; } @@ -4408,13 +4474,18 @@ export function resolveModelPreference(models: ModelInfo[], preference: string): // Exact match on the alias's canonical resolved id (e.g. a pinned // "claude-sonnet-5" against the "sonnet" row's `resolvedModel`). SDK- // reported and unambiguous, so it's tried before the fuzzier tiers below. - // "default" is skipped first since it shares a resolvedModel with - // whichever alias the CLI currently recommends — a specific pin should - // land on that named alias, not "default". + // Compared on the canonical hint spelling so a "-1m"-suffix pin matches a + // "[1m]"-spelled resolvedModel instead of falling into the substring tier + // (which would land on the bare 200k sibling). "default" is skipped first + // since it shares a resolvedModel with whichever alias the CLI currently + // recommends — a specific pin should land on that named alias, not + // "default". + const canonicalPreference = canonicalizeModelId(trimmed); + const matchesResolved = (model: ModelInfo) => + model.resolvedModel != null && canonicalizeModelId(model.resolvedModel) === canonicalPreference; const resolvedMatch = - models.find( - (model) => model.value !== "default" && model.resolvedModel?.toLowerCase() === lower, - ) ?? models.find((model) => model.resolvedModel?.toLowerCase() === lower); + models.find((model) => model.value !== "default" && matchesResolved(model)) ?? + models.find(matchesResolved); if (resolvedMatch) return resolvedMatch; // Substring match @@ -4444,6 +4515,61 @@ export function resolveModelPreference(models: ModelInfo[], preference: string): return bestMatch; } +/** Map the live model reported by a resumed session onto the picker's model + * list. The CLI restores a resumed session's model from the transcript's + * last assistant message, which records the concrete API id (e.g. + * "claude-opus-4-6") with any "[1m]" context hint dropped. Tiers, in order: + * 1. Exact match with the Default entry's resolution — when a named alias + * shares Default's resolvedModel verbatim, the live id can't tell the + * two apart, and a never-customized session should stay on Default. + * 2. Exact resolvedModel match on a named row. Checked before the + * hint-stripped Default comparison so a live "claude-sonnet-5[1m]" lands + * on the "sonnet[1m]" row rather than a Default that resolves to the + * bare "claude-sonnet-5" — the two rows differ in context window, which + * drives `contextWindowSize` and capability gating downstream. + * 3. Hint-stripped match with Default's resolution — a session that never + * left the default resumes as the bare transcript id, and shouldn't show + * a concrete picker entry. + * 4. `resolveModelPreference` over the picker entries. + * 5. A model with no picker counterpart (e.g. excluded by an + * `availableModels` allowlist) is tracked verbatim, mirroring + * `syncModelAfterRefusalFallback`: the picker shows no selection, but the + * model-dependent bookkeeping stays truthful to what the SDK is running. */ +export function matchResumedModel(models: ModelInfo[], liveModel: string): ModelInfo { + const live = canonicalizeModelId(liveModel); + const defaultEntry = models.find((m) => m.value === "default"); + const defaultResolved = defaultEntry?.resolvedModel + ? canonicalizeModelId(defaultEntry.resolvedModel) + : undefined; + + if (defaultEntry && defaultResolved === live) { + return defaultEntry; + } + + // No default-row exclusion needed: a default row matching `live` exactly + // already returned at the tier above. + const exactMatch = models.find( + (m) => m.resolvedModel && canonicalizeModelId(m.resolvedModel) === live, + ); + if (exactMatch) return exactMatch; + + if ( + defaultEntry && + defaultResolved && + stripContextHints(defaultResolved) === stripContextHints(live) + ) { + return defaultEntry; + } + + return ( + resolveModelPreference(models, liveModel) ?? { + value: liveModel, + displayName: liveModel, + description: "", + } + ); +} + function resolveSettingsModel( models: ModelInfo[], settingsModel: unknown, @@ -4536,12 +4662,32 @@ export function applyAvailableModelsAllowlist( return result; } +/** Read the model a resumed session is actually running (via the + * `getContextUsage` control request — the same source `/context` prints) and + * map it onto the picker. Best-effort: a control-request failure is logged + * and returns null so callers keep their current choice; failing the whole + * session/load over an unreadable report would be worse. */ +async function readResumedLiveModel( + query: Query, + models: ModelInfo[], + logger: Logger, +): Promise { + try { + const liveModel = (await query.getContextUsage()).model; + return liveModel ? matchResumedModel(models, liveModel) : null; + } catch (error) { + logger.error("Failed to read the resumed session's live model:", error); + return null; + } +} + async function getAvailableModels( query: Query, models: ModelInfo[], sdkModels: ModelInfo[], settingsManager: SettingsManager, logger: Logger, + isResumedSession: boolean, ): Promise { const settings = settingsManager.getSettings(); @@ -4551,7 +4697,8 @@ async function getAvailableModels( // Model priority (highest to lowest): // 1. ANTHROPIC_MODEL environment variable // 2. settings.model (user configuration) - // 3. models[0] (default first model) + // 3. the resumed session's live model (resumed sessions only) + // 4. models[0] (default first model) if (process.env.ANTHROPIC_MODEL) { const match = resolveModelPreference(models, process.env.ANTHROPIC_MODEL); if (match) { @@ -4566,22 +4713,49 @@ async function getAvailableModels( } } + // A resumed session restores the model it was previously running (the CLI + // re-reads it from the transcript), so without an env/settings override the + // freshly-computed default above can disagree with what the session actually + // runs — session/load then reports a model the session isn't using (issue + // #845). Ask the CLI for the live model and reflect it. No `setModel` here: + // the SDK is already running this model, and pushing a picker alias back + // (e.g. "opus[1m]") could change the live model rather than describe it. + if (resolvedFromInput === undefined && isResumedSession) { + currentModel = (await readResumedLiveModel(query, models, logger)) ?? currentModel; + } + // Skip the setModel round-trip when we can prove the SDK has already landed // on the same model. Two cases qualify: - // (a) No override applied — currentModel stayed at models[0]; the SDK is on - // its own default and we have nothing to sync. + // (a) No override applied — currentModel is the SDK's own default (or, on + // resume, the live model read back from the SDK above); nothing to sync. // (b) The resolver returned the user's input verbatim AND that value exists // in the SDK's original model list — meaning no fuzzy match or // allowlist rewrite was involved, and the SDK (which reads the same // ANTHROPIC_MODEL / settings.json) will have arrived at the same entry. + // This only holds for fresh sessions: a resumed session lands on the + // transcript's model regardless of env/settings, so the override must + // be re-asserted to keep the reported model truthful. // Anything else (fuzzy match, allowlist-synthesized value, alias) gets a // setModel call so we don't drift from the user's intended pin. const sdkSawSameValue = sdkModels.some((m) => m.value === currentModel.value); const skipSetModel = resolvedFromInput === undefined || - (currentModel.value === resolvedFromInput && sdkSawSameValue); + (!isResumedSession && currentModel.value === resolvedFromInput && sdkSawSameValue); if (!skipSetModel) { - await query.setModel(currentModel.value); + try { + await query.setModel(currentModel.value); + } catch (error) { + // On a fresh session the pin is a defining option — fail loudly. A + // resumed session already runs fine on the transcript's model, so + // failing the whole session/load over the re-assert would be worse + // than loading with the pin unapplied (mirrors the setPermissionMode + // containment in createSession). The SDK then stayed on the + // transcript's model, so read that back rather than reporting the + // pin the session isn't running. + if (!isResumedSession) throw error; + logger.error(`Failed to re-assert model "${currentModel.value}" on resume:`, error); + currentModel = (await readResumedLiveModel(query, models, logger)) ?? currentModel; + } } return { diff --git a/src/tests/model-resolution.test.ts b/src/tests/model-resolution.test.ts index 10a7137a..8fe8b36f 100644 --- a/src/tests/model-resolution.test.ts +++ b/src/tests/model-resolution.test.ts @@ -2,7 +2,11 @@ import { describe, it, expect } from "vitest"; import { randomUUID } from "crypto"; import { query } from "@anthropic-ai/claude-agent-sdk"; import type { ModelInfo } from "@anthropic-ai/claude-agent-sdk"; -import { resolveModelPreference, applyAvailableModelsAllowlist } from "../acp-agent.js"; +import { + resolveModelPreference, + applyAvailableModelsAllowlist, + matchResumedModel, +} from "../acp-agent.js"; // Mirrors a real `supportedModels()` response: alias rows carry // `resolvedModel`, and "Sonnet 5" has no `major.minor` dot unlike older @@ -68,6 +72,105 @@ describe("resolveModelPreference - resolvedModel matching", () => { it("returns null for a preference that matches nothing", () => { expect(resolveModelPreference(LIVE_SHAPED_MODELS, "claude-gpt-99")).toBeNull(); }); + + it("matches a '-1m' id-suffix preference against a '[1m]'-spelled resolvedModel", () => { + // Without hint canonicalization the exact tier misses and the substring + // tier lands on the bare 200k sibling, silently downgrading a 1M pin. + const models: ModelInfo[] = [ + { + value: "sonnet", + resolvedModel: "claude-sonnet-5", + displayName: "Sonnet", + description: "Sonnet 5 · Efficient for routine tasks", + }, + { + value: "sonnet[1m]", + resolvedModel: "claude-sonnet-5[1m]", + displayName: "Sonnet", + description: "Sonnet 5 with 1M context", + }, + ]; + expect(resolveModelPreference(models, "claude-sonnet-5-1m")?.value).toBe("sonnet[1m]"); + }); +}); + +describe("matchResumedModel", () => { + // Environment shape from issue #845: the default resolves to Sonnet, the + // session was explicitly switched to Opus before the process restart. + const SONNET_DEFAULT_MODELS: ModelInfo[] = [ + { + value: "default", + resolvedModel: "claude-sonnet-5", + displayName: "Default (recommended)", + description: "Use the default model (currently Sonnet 5)", + }, + { + value: "opus", + resolvedModel: "claude-opus-4-6[1m]", + displayName: "Opus", + description: "Opus 4.6 · Most capable for complex work", + }, + { + value: "sonnet", + resolvedModel: "claude-sonnet-5", + displayName: "Sonnet", + description: "Sonnet 5 · Efficient for routine tasks", + }, + { + value: "sonnet[1m]", + resolvedModel: "claude-sonnet-5[1m]", + displayName: "Sonnet", + description: "Sonnet 5 with 1M context", + }, + ]; + + it("maps an explicitly-switched session's live model to its picker alias", () => { + expect(matchResumedModel(SONNET_DEFAULT_MODELS, "claude-opus-4-6").value).toBe("opus"); + expect(matchResumedModel(SONNET_DEFAULT_MODELS, "claude-opus-4-6[1m]").value).toBe("opus"); + }); + + it("keeps Default selected when the live model is what Default resolves to", () => { + expect(matchResumedModel(SONNET_DEFAULT_MODELS, "claude-sonnet-5").value).toBe("default"); + }); + + it("prefers an exact named-row resolvedModel match over the hint-stripped Default tier", () => { + // "claude-sonnet-5[1m]" differs from Default's "claude-sonnet-5" only by + // the context hint; stripping both would misreport the explicit 1M row as + // Default (and infer the wrong context window downstream). Both hint + // spellings must land on the row regardless of which one it uses. + expect(matchResumedModel(SONNET_DEFAULT_MODELS, "claude-sonnet-5[1m]").value).toBe( + "sonnet[1m]", + ); + expect(matchResumedModel(SONNET_DEFAULT_MODELS, "claude-sonnet-5-1m").value).toBe("sonnet[1m]"); + }); + + it("normalizes the '-1m' id-suffix hint form and casing against Default's resolution", () => { + expect(matchResumedModel(LIVE_SHAPED_MODELS, "claude-opus-4-8-1m").value).toBe("default"); + expect(matchResumedModel(LIVE_SHAPED_MODELS, "Claude-Opus-4-8[1M]").value).toBe("default"); + }); + + it("prefers Default over a named row sharing the identical resolvedModel", () => { + // "opus[1m]" carries the same resolvedModel as Default; the live id can't + // tell the two apart, so a never-customized session stays on Default. + expect(matchResumedModel(LIVE_SHAPED_MODELS, "claude-opus-4-8[1m]").value).toBe("default"); + }); + + it("ignores '[1m]' context hints when comparing against Default's resolution", () => { + // The transcript-restored model drops the "[1m]" context hint, so a + // session that never left the 1M-context default resumes as the bare id. + expect(matchResumedModel(LIVE_SHAPED_MODELS, "claude-opus-4-8").value).toBe("default"); + }); + + it("still resolves non-default live models against the picker entries", () => { + expect(matchResumedModel(LIVE_SHAPED_MODELS, "claude-sonnet-5").value).toBe("sonnet"); + expect(matchResumedModel(LIVE_SHAPED_MODELS, "claude-haiku-4-5-20251001").value).toBe("haiku"); + }); + + it("tracks a live model with no picker counterpart verbatim", () => { + const result = matchResumedModel(LIVE_SHAPED_MODELS, "claude-gpt-99"); + expect(result.value).toBe("claude-gpt-99"); + expect(result.displayName).toBe("claude-gpt-99"); + }); }); describe("applyAvailableModelsAllowlist - resolvedModel matching", () => { diff --git a/src/tests/session-config-options.test.ts b/src/tests/session-config-options.test.ts index a8846e02..f18a272e 100644 --- a/src/tests/session-config-options.test.ts +++ b/src/tests/session-config-options.test.ts @@ -819,6 +819,29 @@ describe("session config options", () => { expect(setModelSpy).toHaveBeenCalledWith("claude-sonnet-4-6"); }); + // A session can be running a model with no picker entry (resumed onto a + // model excluded by the availableModels allowlist, or a refusal + // fallback); its verbatim id is then the option's currentValue. A client + // round-tripping that reported value must not get "Invalid value". + it("accepts the reported currentValue even when it has no options entry", async () => { + const session = (agent as unknown as { sessions: Record }).sessions[SESSION_ID]; + session.models = { ...session.models, currentModelId: "claude-offlist-9" }; + session.configOptions = session.configOptions.map((o: { id: string }) => + o.id === "model" ? { ...o, currentValue: "claude-offlist-9" } : o, + ); + + const response = await agent.setSessionConfigOption({ + sessionId: SESSION_ID, + configId: "model", + value: "claude-offlist-9", + }); + + expect(setModelSpy).toHaveBeenCalledWith("claude-offlist-9"); + expect(response.configOptions?.find((o) => o.id === "model")?.currentValue).toBe( + "claude-offlist-9", + ); + }); + it("setSessionMode also syncs configOptions", async () => { await agent.setSessionMode({ sessionId: SESSION_ID, modeId: "plan" }); diff --git a/src/tests/session-load.test.ts b/src/tests/session-load.test.ts index 4c5e2672..ea7625cc 100644 --- a/src/tests/session-load.test.ts +++ b/src/tests/session-load.test.ts @@ -6,9 +6,11 @@ import { AcpClient, ClaudeAcpAgent } from "../acp-agent.js"; import { Pushable } from "../utils.js"; import type { SDKUserMessage } from "@anthropic-ai/claude-agent-sdk"; -function createMockClient(): AcpClient { +function createMockClient( + sessionUpdate: (notification: SessionNotification) => Promise = async () => {}, +): AcpClient { return { - sessionUpdate: async (_notification: SessionNotification) => {}, + sessionUpdate, requestPermission: async () => ({ outcome: { outcome: "cancelled" } }), readTextFile: async () => ({ content: "" }), writeTextFile: async () => ({}), @@ -330,4 +332,52 @@ describe.skipIf(!process.env.RUN_INTEGRATION_TESTS)("session load/resume lifecyc } expect(recordedUserChunks.some((c) => c.includes("hi"))).toBe(true); }, 60000); + + // Regression test for https://github.com/agentclientprotocol/claude-agent-acp/issues/845 + // On resume the CLI restores the model the session's transcript was running, + // so loadSession must report that live model instead of recomputing the + // env/settings/default choice from scratch. Haiku is used because no account + // resolves its default there, so the restored model provably differs from + // the freshly-computed default. Requires no ANTHROPIC_MODEL env var and no + // "model" key in settings.json (either would pin the model on resume and + // mask the reconciliation this test covers). + it("ACP: loadSession reports the model the resumed session is actually running", async () => { + const agentChunks: string[] = []; + const client = createMockClient(async (notification) => { + if (notification.update.sessionUpdate === "agent_message_chunk") { + const content = notification.update.content; + if (content.type === "text") agentChunks.push(content.text); + } + }); + + const modelOf = (opts: { id: string; currentValue?: unknown }[] | null | undefined) => + opts?.find((o) => o.id === "model")?.currentValue; + + const agent = new ClaudeAcpAgent(client); + try { + const cwd = process.cwd(); + const { sessionId, configOptions } = await agent.newSession({ cwd, mcpServers: [] }); + expect(modelOf(configOptions)).not.toBe("haiku"); + + await agent.setSessionConfigOption({ sessionId, configId: "model", value: "haiku" }); + // A real turn on Haiku, so the transcript's last assistant message — + // which is what the CLI restores the model from — records it. + await agent.prompt({ + sessionId, + prompt: [{ type: "text", text: "Reply with only the word OK" }], + }); + await agent.closeSession({ sessionId }); + + const loadResp = await agent.loadSession({ sessionId, cwd, mcpServers: [] }); + expect(modelOf(loadResp.configOptions)).toBe("haiku"); + + // And the reported option agrees with the live session (`/context`). + agentChunks.length = 0; + await agent.prompt({ sessionId, prompt: [{ type: "text", text: "/context" }] }); + const liveModel = agentChunks.join("\n").match(/\*\*Model:\*\*\s*(\S+)/)?.[1]; + expect(liveModel).toContain("haiku"); + } finally { + await agent.dispose(); + } + }, 120000); });