fix: defer model validation until after extensions register (#3089)

* fix: defer model validation until after extensions register (#2626) Extension-provided models (e.g. claude-code/claude-sonnet-4-6) were silently overwritten on every startup because the model validation ran before createAgentSession(), which is where extensions register their models in the ModelRegistry. At validation time, extension models did not exist in the registry, so the user's valid choice was replaced with a built-in fallback. Extract validation into validateConfiguredModel() and call it after createAgentSession() in both print-mode and interactive-mode paths. Closes #2626 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align MinimalSettingsManager interface with SettingsManager The MinimalSettingsManager interface used `string` for thinking level types, but SettingsManager uses a specific union type and returns `undefined`. This caused TS2345 at cli.ts lines 448 and 587. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-30 16:38:10 -04:00 · 2026-03-30 16:38:10 -04:00 · 6e22a20580
commit 6e22a20580
parent 2f3ffbfc10
3 changed files with 259 additions and 37 deletions
--- a/src/cli.ts
+++ b/src/cli.ts
@ -16,7 +16,8 @@ import { agentDir, sessionsDir, authFilePath } from './app-paths.js'
 import { initResources, buildResourceLoader, getNewerManagedResourceVersion } from './resource-loader.js'
 import { ensureManagedTools } from './tool-bootstrap.js'
 import { loadStoredEnvKeys } from './wizard.js'
-import { getPiDefaultModelAndProvider, migratePiCredentials } from './pi-migration.js'
+import { migratePiCredentials } from './pi-migration.js'
+import { validateConfiguredModel } from './startup-model-validation.js'
 import { shouldRunOnboarding, runOnboarding } from './onboarding.js'
 import chalk from 'chalk'
 import { checkForUpdates } from './update-check.js'
@ -391,42 +392,6 @@ if (cliFlags.listModels !== undefined) {
  process.exit(0)
 }

-// Validate configured model on startup — catches stale settings from prior installs
-// (e.g. grok-2 which no longer exists) and fresh installs with no settings.
-// Only resets the default when the configured model no longer exists in the registry;
-// never overwrites a valid user choice.
-const configuredProvider = settingsManager.getDefaultProvider()
-const configuredModel = settingsManager.getDefaultModel()
-const allModels = modelRegistry.getAll()
-const availableModels = modelRegistry.getAvailable()
-const configuredExists = configuredProvider && configuredModel &&
-  allModels.some((m) => m.provider === configuredProvider && m.id === configuredModel)
-const configuredAvailable = configuredProvider && configuredModel &&
-  availableModels.some((m) => m.provider === configuredProvider && m.id === configuredModel)
-
-if (!configuredModel || !configuredExists) {
-  // Model not configured at all, or removed from registry — pick a fallback.
-  // Only fires when the model is genuinely unknown (not just temporarily unavailable).
-  const piDefault = getPiDefaultModelAndProvider()
-  const preferred =
-    (piDefault
-      ? availableModels.find((m) => m.provider === piDefault.provider && m.id === piDefault.model)
-      : undefined) ||
-    availableModels.find((m) => m.provider === 'openai' && m.id === 'gpt-5.4') ||
-    availableModels.find((m) => m.provider === 'openai') ||
-    availableModels.find((m) => m.provider === 'anthropic' && m.id === 'claude-opus-4-6') ||
-    availableModels.find((m) => m.provider === 'anthropic' && m.id.includes('opus')) ||
-    availableModels.find((m) => m.provider === 'anthropic') ||
-    availableModels[0]
-  if (preferred) {
-    settingsManager.setDefaultModelAndProvider(preferred.provider, preferred.id)
-  }
-}
-
-if (settingsManager.getDefaultThinkingLevel() !== 'off' && !configuredExists) {
-  settingsManager.setDefaultThinkingLevel('off')
-}
-
 // GSD always uses quiet startup — the gsd extension renders its own branded header
 if (!settingsManager.getQuietStartup()) {
  settingsManager.setQuietStartup(true)
@ -477,6 +442,11 @@ if (isPrintMode) {
  })
  markStartup('createAgentSession')

+  // Validate configured model AFTER extensions have registered their models (#2626).
+  // Before this, extension-provided models (e.g. claude-code/*) were not yet in the
+  // registry, causing the user's valid choice to be silently overwritten.
+  validateConfiguredModel(modelRegistry, settingsManager)
+
  if (extensionsResult.errors.length > 0) {
    for (const err of extensionsResult.errors) {
      // Downgrade conflicts with built-in tools to warnings (#1347)
@ -625,6 +595,11 @@ const { session, extensionsResult } = await createAgentSession({
 })
 markStartup('createAgentSession')

+// Validate configured model AFTER extensions have registered their models (#2626).
+// Before this, extension-provided models (e.g. claude-code/*) were not yet in the
+// registry, causing the user's valid choice to be silently overwritten.
+validateConfiguredModel(modelRegistry, settingsManager)
+
 if (extensionsResult.errors.length > 0) {
  for (const err of extensionsResult.errors) {
    const isSuperseded = err.error.includes("supersedes");
--- a/src/startup-model-validation.ts
+++ b/src/startup-model-validation.ts
@ -0,0 +1,78 @@
+/**
+ * Startup model validation — extracted from cli.ts so it can be called
+ * AFTER extensions register their models in the ModelRegistry.
+ *
+ * Before this extraction (bug #2626), the validation ran before
+ * createAgentSession(), meaning extension-provided models (e.g.
+ * claude-code/claude-sonnet-4-6) were not yet in the registry.
+ * configuredExists was always false for extension models, causing the
+ * user's valid choice to be silently overwritten with a built-in fallback.
+ */
+
+import { getPiDefaultModelAndProvider } from './pi-migration.js'
+
+interface MinimalModel {
+  provider: string
+  id: string
+}
+
+interface MinimalModelRegistry {
+  getAll(): MinimalModel[]
+  getAvailable(): MinimalModel[]
+}
+
+type ThinkingLevel = 'off' | 'minimal' | 'low' | 'medium' | 'high' | 'xhigh'
+
+interface MinimalSettingsManager {
+  getDefaultProvider(): string | undefined
+  getDefaultModel(): string | undefined
+  getDefaultThinkingLevel(): ThinkingLevel | undefined
+  setDefaultModelAndProvider(provider: string, modelId: string): void
+  setDefaultThinkingLevel(level: ThinkingLevel): void
+}
+
+/**
+ * Validate the configured default model against the registry.
+ *
+ * If the configured model exists in the registry, this is a no-op — the
+ * user's choice is preserved.  If it does not exist (stale settings from a
+ * prior install, or genuinely removed model), a fallback is selected and
+ * written to settings.
+ *
+ * IMPORTANT: Call this AFTER createAgentSession() so that extension-
+ * provided models have been registered in the ModelRegistry.
+ */
+export function validateConfiguredModel(
+  modelRegistry: MinimalModelRegistry,
+  settingsManager: MinimalSettingsManager,
+): void {
+  const configuredProvider = settingsManager.getDefaultProvider()
+  const configuredModel = settingsManager.getDefaultModel()
+  const allModels = modelRegistry.getAll()
+  const availableModels = modelRegistry.getAvailable()
+  const configuredExists = configuredProvider && configuredModel &&
+    allModels.some((m) => m.provider === configuredProvider && m.id === configuredModel)
+
+  if (!configuredModel || !configuredExists) {
+    // Model not configured at all, or removed from registry — pick a fallback.
+    // Only fires when the model is genuinely unknown (not just temporarily unavailable).
+    const piDefault = getPiDefaultModelAndProvider()
+    const preferred =
+      (piDefault
+        ? availableModels.find((m) => m.provider === piDefault.provider && m.id === piDefault.model)
+        : undefined) ||
+      availableModels.find((m) => m.provider === 'openai' && m.id === 'gpt-5.4') ||
+      availableModels.find((m) => m.provider === 'openai') ||
+      availableModels.find((m) => m.provider === 'anthropic' && m.id === 'claude-opus-4-6') ||
+      availableModels.find((m) => m.provider === 'anthropic' && m.id.includes('opus')) ||
+      availableModels.find((m) => m.provider === 'anthropic') ||
+      availableModels[0]
+    if (preferred) {
+      settingsManager.setDefaultModelAndProvider(preferred.provider, preferred.id)
+    }
+  }
+
+  if (settingsManager.getDefaultThinkingLevel() !== 'off' && !configuredExists) {
+    settingsManager.setDefaultThinkingLevel('off')
+  }
+}
--- a/src/tests/extension-model-validation.test.ts
+++ b/src/tests/extension-model-validation.test.ts
@ -0,0 +1,169 @@
+/**
+ * Regression test for #2626: Extension-provided models silently overwritten on startup.
+ *
+ * The startup model-validation logic must run AFTER extensions register their
+ * models in the ModelRegistry.  When validation runs before extensions load,
+ * extension-provided models (e.g. claude-code/claude-sonnet-4-6) are not yet
+ * in the registry, so configuredExists is always false and the user's choice
+ * is silently replaced with a built-in fallback.
+ *
+ * This test exercises `validateConfiguredModel()` directly (once extracted) to
+ * verify that:
+ *   (a) extension models present in the registry are preserved,
+ *   (b) genuinely missing models still trigger fallback selection.
+ */
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+const { validateConfiguredModel } = await import("../startup-model-validation.js");
+
+/**
+ * Minimal stub of ModelRegistry with just getAll() / getAvailable().
+ */
+function fakeModelRegistry(models: Array<{ provider: string; id: string }>) {
+  const available = models.map((m) => ({
+    ...m,
+    name: m.id,
+    contextWindow: 128_000,
+    maxTokens: 4096,
+    reasoning: false,
+  }));
+  return {
+    getAll: () => available,
+    getAvailable: () => available,
+  };
+}
+
+/**
+ * Minimal stub of SettingsManager backed by plain objects.
+ */
+function fakeSettingsManager(initial: { provider?: string; model?: string }) {
+  let provider = initial.provider;
+  let model = initial.model;
+  let thinkingLevel = "off" as string;
+  return {
+    getDefaultProvider: () => provider,
+    getDefaultModel: () => model,
+    getDefaultThinkingLevel: () => thinkingLevel,
+    setDefaultModelAndProvider(p: string, m: string) {
+      provider = p;
+      model = m;
+    },
+    setDefaultThinkingLevel(level: string) {
+      thinkingLevel = level;
+    },
+    // Expose for assertions
+    get currentProvider() { return provider; },
+    get currentModel() { return model; },
+  };
+}
+
+// ──────────────────────────────────────────────────────────────────────
+// Test: extension-provided model in registry must NOT be overwritten
+// ──────────────────────────────────────────────────────────────────────
+test("validateConfiguredModel preserves extension-provided model when present in registry", () => {
+  const settings = fakeSettingsManager({
+    provider: "claude-code",
+    model: "claude-sonnet-4-6",
+  });
+
+  // Registry includes the extension model (simulating post-extension-load state)
+  const registry = fakeModelRegistry([
+    { provider: "openai", id: "gpt-5.4" },
+    { provider: "claude-code", id: "claude-sonnet-4-6" },
+  ]);
+
+  validateConfiguredModel(registry as any, settings as any);
+
+  assert.equal(settings.currentProvider, "claude-code",
+    "provider must remain the user-configured extension provider");
+  assert.equal(settings.currentModel, "claude-sonnet-4-6",
+    "model must remain the user-configured extension model");
+});
+
+// ──────────────────────────────────────────────────────────────────────
+// Test: genuinely removed model still triggers fallback
+// ──────────────────────────────────────────────────────────────────────
+test("validateConfiguredModel falls back when model is not in registry", () => {
+  const settings = fakeSettingsManager({
+    provider: "openai",
+    model: "grok-2",  // hypothetical removed model
+  });
+
+  const registry = fakeModelRegistry([
+    { provider: "openai", id: "gpt-5.4" },
+    { provider: "anthropic", id: "claude-opus-4-6" },
+  ]);
+
+  validateConfiguredModel(registry as any, settings as any);
+
+  // Should have been overwritten to one of the available models
+  assert.notEqual(settings.currentModel, "grok-2",
+    "stale model must be replaced by a fallback");
+  assert.ok(settings.currentProvider, "a fallback provider must be set");
+  assert.ok(settings.currentModel, "a fallback model must be set");
+});
+
+// ──────────────────────────────────────────────────────────────────────
+// Test: no configured model at all triggers fallback
+// ──────────────────────────────────────────────────────────────────────
+test("validateConfiguredModel picks a fallback when nothing is configured", () => {
+  const settings = fakeSettingsManager({
+    provider: undefined,
+    model: undefined,
+  });
+
+  const registry = fakeModelRegistry([
+    { provider: "openai", id: "gpt-5.4" },
+  ]);
+
+  validateConfiguredModel(registry as any, settings as any);
+
+  assert.equal(settings.currentProvider, "openai");
+  assert.equal(settings.currentModel, "gpt-5.4");
+});
+
+// ──────────────────────────────────────────────────────────────────────
+// Test: thinking level reset when model doesn't exist
+// ──────────────────────────────────────────────────────────────────────
+test("validateConfiguredModel resets thinking level when model was replaced", () => {
+  const settings = fakeSettingsManager({
+    provider: "openai",
+    model: "grok-2",
+  });
+  // Simulate non-off thinking level
+  settings.setDefaultThinkingLevel("high");
+
+  const registry = fakeModelRegistry([
+    { provider: "openai", id: "gpt-5.4" },
+  ]);
+
+  validateConfiguredModel(registry as any, settings as any);
+
+  assert.equal(settings.getDefaultThinkingLevel(), "off",
+    "thinking level must be reset to off when model was not found");
+});
+
+// ──────────────────────────────────────────────────────────────────────
+// Test: thinking level NOT reset when model exists
+// ──────────────────────────────────────────────────────────────────────
+test("validateConfiguredModel preserves thinking level when model exists", () => {
+  const settings = fakeSettingsManager({
+    provider: "openai",
+    model: "gpt-5.4",
+  });
+  settings.setDefaultThinkingLevel("high");
+
+  const registry = fakeModelRegistry([
+    { provider: "openai", id: "gpt-5.4" },
+  ]);
+
+  validateConfiguredModel(registry as any, settings as any);
+
+  assert.equal(settings.getDefaultThinkingLevel(), "high",
+    "thinking level must be preserved when configured model exists");
+});