fix: Preserve live model on resumed sessions (#848)

Closes #845
2026-10-27 12:40:32 +00:00 · 2026-07-07 17:37:21 +02:00
parent b03318f59c
commit f3d8ae3eb3
4 changed files with 372 additions and 22 deletions
--- 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<ModelInfo | null> {
+  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<SessionModelState> {
  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 {