diff --git a/src/cli.ts b/src/cli.ts index 255cbf3ed..d09152543 100644 --- 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"); diff --git a/src/startup-model-validation.ts b/src/startup-model-validation.ts new file mode 100644 index 000000000..1a4141f00 --- /dev/null +++ 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') + } +} diff --git a/src/tests/extension-model-validation.test.ts b/src/tests/extension-model-validation.test.ts new file mode 100644 index 000000000..22ae05c1a --- /dev/null +++ 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"); +});