From 9fe13da3f2c93579d80291a724dae659984089d7 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 5 Apr 2026 07:14:24 -0500 Subject: [PATCH 1/2] fix(pi-coding-agent): resolve model fallback race that ignores configured provider (#3534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extension-provided models (e.g. claude-code/*) were unavailable during findInitialModel() because pendingProviderRegistrations had not been flushed yet, causing the fallback chain to select Google Gemini even when the user explicitly configured claude-code as their default. Three compounding issues fixed: (A) Flush pendingProviderRegistrations in createAgentSession() before findInitialModel() runs, so extension models are in the registry when initial model selection happens. (B) Re-apply the validated model to the session after validateConfiguredModel() in both print and interactive CLI paths. Previously, validation updated settingsManager but never called session.setModel(), leaving the session on the wrong model. (C) Update defaultModelPerProvider.anthropic from "claude-opus-4-6[1m]" to "claude-opus-4-6" — the [1m] variant was removed from the model registry when the base model was upgraded to 1M context, causing the Anthropic fallback to silently fail and skip to Google. Closes #3534 --- .../src/core/model-resolver.ts | 6 +- packages/pi-coding-agent/src/core/sdk.ts | 11 ++ src/cli.ts | 26 ++++ src/tests/startup-model-validation.test.ts | 124 ++++++++++++++++++ 4 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 src/tests/startup-model-validation.test.ts diff --git a/packages/pi-coding-agent/src/core/model-resolver.ts b/packages/pi-coding-agent/src/core/model-resolver.ts index 6d07b940b..3e3b266f7 100644 --- a/packages/pi-coding-agent/src/core/model-resolver.ts +++ b/packages/pi-coding-agent/src/core/model-resolver.ts @@ -13,7 +13,7 @@ import type { ModelRegistry } from "./model-registry.js"; /** Default model IDs for each known provider */ const defaultModelPerProvider: Record = { "amazon-bedrock": "us.anthropic.claude-opus-4-6-v1", - anthropic: "claude-opus-4-6[1m]", + anthropic: "claude-opus-4-6", "anthropic-vertex": "claude-sonnet-4-6", openai: "gpt-5.4", "azure-openai-responses": "gpt-5.2", @@ -24,7 +24,7 @@ const defaultModelPerProvider: Record = { "google-vertex": "gemini-3-pro-preview", "github-copilot": "gpt-4o", openrouter: "openai/gpt-5.1-codex", - "vercel-ai-gateway": "anthropic/claude-opus-4-6[1m]", + "vercel-ai-gateway": "anthropic/claude-opus-4-6", xai: "grok-4-fast-non-reasoning", groq: "openai/gpt-oss-120b", cerebras: "zai-glm-4.6", @@ -507,7 +507,7 @@ export async function findInitialModel(options: { const found = modelRegistry.find(defaultProvider, defaultModelId); if (found) { // Check if the provider's recommended default is a higher-capability variant - // of the saved model (e.g. saved "claude-opus-4-6" vs recommended "claude-opus-4-6[1m]"). + // of the saved model (e.g. saved "claude-opus-4-6" vs recommended "claude-opus-4-6-extended"). // If so, prefer the recommended variant to avoid using a smaller context window (#1125). const recommendedId = defaultModelPerProvider[defaultProvider as KnownProvider]; if (recommendedId && recommendedId !== defaultModelId && recommendedId.startsWith(defaultModelId)) { diff --git a/packages/pi-coding-agent/src/core/sdk.ts b/packages/pi-coding-agent/src/core/sdk.ts index 55e80dfc8..8d8f8cf04 100644 --- a/packages/pi-coding-agent/src/core/sdk.ts +++ b/packages/pi-coding-agent/src/core/sdk.ts @@ -214,6 +214,17 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} } } + // Flush extension provider registrations so extension-provided models (e.g. claude-code/*) + // are available in the registry before model resolution. Without this, findInitialModel() + // cannot find extension models and falls back to built-in providers (#3534). + const extensionsForModelResolution = resourceLoader.getExtensions(); + for (const { name, config } of extensionsForModelResolution.runtime.pendingProviderRegistrations) { + modelRegistry.registerProvider(name, config); + } + // Note: we do NOT clear pendingProviderRegistrations here — bindCore() will iterate + // an empty array harmlessly, and clearing here would require the runtime to track + // whether the flush already happened. + // If still no model, use findInitialModel (checks settings default, then provider defaults) if (!model) { const result = await findInitialModel({ diff --git a/src/cli.ts b/src/cli.ts index ac25bb18c..3350529c2 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -467,6 +467,19 @@ if (isPrintMode) { // registry, causing the user's valid choice to be silently overwritten. validateConfiguredModel(modelRegistry, settingsManager) + // Re-apply the validated model to the session. findInitialModel() may have picked + // a fallback if extension models weren't yet registered at that point (#3534). + { + const validatedProvider = settingsManager.getDefaultProvider() + const validatedModelId = settingsManager.getDefaultModel() + if (validatedProvider && validatedModelId) { + const correctModel = modelRegistry.find(validatedProvider, validatedModelId) + if (correctModel) { + session.setModel(correctModel) + } + } + } + if (extensionsResult.errors.length > 0) { for (const err of extensionsResult.errors) { // Downgrade conflicts with built-in tools to warnings (#1347) @@ -620,6 +633,19 @@ markStartup('createAgentSession') // registry, causing the user's valid choice to be silently overwritten. validateConfiguredModel(modelRegistry, settingsManager) +// Re-apply the validated model to the session. findInitialModel() may have picked +// a fallback if extension models weren't yet registered at that point (#3534). +{ + const validatedProvider = settingsManager.getDefaultProvider() + const validatedModelId = settingsManager.getDefaultModel() + if (validatedProvider && validatedModelId) { + const correctModel = modelRegistry.find(validatedProvider, validatedModelId) + if (correctModel) { + session.setModel(correctModel) + } + } +} + if (extensionsResult.errors.length > 0) { for (const err of extensionsResult.errors) { const isConflict = err.error.includes("supersedes") || err.error.includes("conflicts with"); diff --git a/src/tests/startup-model-validation.test.ts b/src/tests/startup-model-validation.test.ts new file mode 100644 index 000000000..fc124a132 --- /dev/null +++ b/src/tests/startup-model-validation.test.ts @@ -0,0 +1,124 @@ +/** + * GSD-2 — Regression tests for startup model validation (#3534) + * + * Verifies that validateConfiguredModel() correctly handles extension-provided + * models and that stale model IDs (e.g. claude-opus-4-6[1m]) trigger fallback. + */ + +import { describe, it, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { validateConfiguredModel } from "../startup-model-validation.js"; + +// ─── Helpers ──────────────────────────────────────────────────────────────── + +interface MockModel { + provider: string; + id: string; +} + +function createMockRegistry(allModels: MockModel[], availableModels?: MockModel[]) { + return { + getAll: () => allModels, + getAvailable: () => availableModels ?? allModels, + }; +} + +function createMockSettings(defaults: { provider?: string; model?: string; thinking?: "off" | "high" }) { + let currentProvider = defaults.provider; + let currentModel = defaults.model; + let currentThinking: "off" | "minimal" | "low" | "medium" | "high" | "xhigh" = defaults.thinking ?? "off"; + + return { + getDefaultProvider: () => currentProvider, + getDefaultModel: () => currentModel, + getDefaultThinkingLevel: () => currentThinking, + setDefaultModelAndProvider: (provider: string, modelId: string) => { + currentProvider = provider; + currentModel = modelId; + }, + setDefaultThinkingLevel: (level: "off" | "minimal" | "low" | "medium" | "high" | "xhigh") => { + currentThinking = level; + }, + // Expose for assertions + get _provider() { return currentProvider; }, + get _model() { return currentModel; }, + get _thinking() { return currentThinking; }, + }; +} + +// ─── Tests ────────────────────────────────────────────────────────────────── + +describe("validateConfiguredModel — regression #3534", () => { + it("preserves valid extension-provided model without overwriting", () => { + // Simulate: user configured claude-code/claude-opus-4-6, extension has registered it + const registry = createMockRegistry([ + { provider: "claude-code", id: "claude-opus-4-6" }, + { provider: "google", id: "gemini-2.5-pro" }, + ]); + const settings = createMockSettings({ provider: "claude-code", model: "claude-opus-4-6" }); + + validateConfiguredModel(registry, settings); + + // Should NOT have changed the settings — the model is valid + assert.equal(settings._provider, "claude-code"); + assert.equal(settings._model, "claude-opus-4-6"); + }); + + it("falls back when configured model ID does not exist in registry", () => { + // Simulate: user configured claude-opus-4-6[1m] but registry only has claude-opus-4-6 + const registry = createMockRegistry([ + { provider: "anthropic", id: "claude-opus-4-6" }, + { provider: "google", id: "gemini-2.5-pro" }, + ]); + const settings = createMockSettings({ provider: "anthropic", model: "claude-opus-4-6[1m]" }); + + validateConfiguredModel(registry, settings); + + // Should have replaced with a fallback — the [1m] variant doesn't exist + assert.notEqual(settings._model, "claude-opus-4-6[1m]"); + }); + + it("does not fall back to google when anthropic models are available", () => { + // Simulate: stale setting triggers fallback, anthropic should be preferred over google + const registry = createMockRegistry([ + { provider: "anthropic", id: "claude-opus-4-6" }, + { provider: "google", id: "gemini-2.5-pro" }, + ]); + const settings = createMockSettings({ provider: "anthropic", model: "nonexistent-model" }); + + validateConfiguredModel(registry, settings); + + // Should pick anthropic fallback, not google + assert.equal(settings._provider, "anthropic"); + assert.equal(settings._model, "claude-opus-4-6"); + }); + + it("resets thinking level when model is replaced", () => { + const registry = createMockRegistry([ + { provider: "anthropic", id: "claude-opus-4-6" }, + ]); + const settings = createMockSettings({ + provider: "anthropic", + model: "nonexistent-model", + thinking: "high", + }); + + validateConfiguredModel(registry, settings); + + assert.equal(settings._thinking, "off"); + }); + + it("is a no-op when no model is configured at all", () => { + const registry = createMockRegistry([ + { provider: "anthropic", id: "claude-opus-4-6" }, + { provider: "google", id: "gemini-2.5-pro" }, + ]); + const settings = createMockSettings({ provider: undefined, model: undefined }); + + validateConfiguredModel(registry, settings); + + // Should pick a fallback since nothing was configured + assert.ok(settings._provider); + assert.ok(settings._model); + }); +}); From e3cd354d58f2e8f1ad1bdb6c274916044d63179a Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 5 Apr 2026 07:27:26 -0500 Subject: [PATCH 2/2] fix(cli): guard model re-apply against session restore and async rejection Address Codex adversarial review findings: 1. Only re-apply the validated model when createAgentSession() signals a fallback (modelFallbackMessage is truthy). This prevents silently overriding the persisted model of resumed conversations. 2. Use modelRegistry.getAvailable() instead of find() to ensure the model's provider is request-ready before calling setModel(). 3. Await session.setModel() and wrap in try/catch so provider auth failures don't surface as unhandled promise rejections at startup. Applies to both print-mode and interactive-mode startup paths. --- src/cli.ts | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 3350529c2..28e7e12d6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -453,7 +453,7 @@ if (isPrintMode) { await resourceLoader.reload() markStartup('resourceLoader.reload') - const { session, extensionsResult } = await createAgentSession({ + const { session, extensionsResult, modelFallbackMessage } = await createAgentSession({ authStorage, modelRegistry, settingsManager, @@ -467,15 +467,21 @@ if (isPrintMode) { // registry, causing the user's valid choice to be silently overwritten. validateConfiguredModel(modelRegistry, settingsManager) - // Re-apply the validated model to the session. findInitialModel() may have picked - // a fallback if extension models weren't yet registered at that point (#3534). - { + // Re-apply the validated model to the session only when findInitialModel() used a + // fallback (not when restoring an existing session's model). This prevents silently + // overriding the persisted model of resumed conversations (#3534). + if (modelFallbackMessage) { const validatedProvider = settingsManager.getDefaultProvider() const validatedModelId = settingsManager.getDefaultModel() if (validatedProvider && validatedModelId) { - const correctModel = modelRegistry.find(validatedProvider, validatedModelId) + const correctModel = modelRegistry.getAvailable() + .find((m) => m.provider === validatedProvider && m.id === validatedModelId) if (correctModel) { - session.setModel(correctModel) + try { + await session.setModel(correctModel) + } catch { + // Provider not ready — leave session on its current model + } } } } @@ -619,7 +625,7 @@ const resourceLoadPromise = resourceLoader.reload() await resourceLoadPromise markStartup('resourceLoader.reload') -const { session, extensionsResult } = await createAgentSession({ +const { session, extensionsResult, modelFallbackMessage: interactiveFallbackMsg } = await createAgentSession({ authStorage, modelRegistry, settingsManager, @@ -633,15 +639,21 @@ markStartup('createAgentSession') // registry, causing the user's valid choice to be silently overwritten. validateConfiguredModel(modelRegistry, settingsManager) -// Re-apply the validated model to the session. findInitialModel() may have picked -// a fallback if extension models weren't yet registered at that point (#3534). -{ +// Re-apply the validated model to the session only when findInitialModel() used a +// fallback (not when restoring an existing session's model). This prevents silently +// overriding the persisted model of resumed conversations (#3534). +if (interactiveFallbackMsg) { const validatedProvider = settingsManager.getDefaultProvider() const validatedModelId = settingsManager.getDefaultModel() if (validatedProvider && validatedModelId) { - const correctModel = modelRegistry.find(validatedProvider, validatedModelId) + const correctModel = modelRegistry.getAvailable() + .find((m) => m.provider === validatedProvider && m.id === validatedModelId) if (correctModel) { - session.setModel(correctModel) + try { + await session.setModel(correctModel) + } catch { + // Provider not ready — leave session on its current model + } } } }