fix(pi-coding-agent): resolve model fallback race that ignores configured provider (#3534)
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
This commit is contained in:
parent
4cb6252b9b
commit
9fe13da3f2
4 changed files with 164 additions and 3 deletions
|
|
@ -13,7 +13,7 @@ import type { ModelRegistry } from "./model-registry.js";
|
|||
/** Default model IDs for each known provider */
|
||||
const defaultModelPerProvider: Record<KnownProvider, string> = {
|
||||
"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<KnownProvider, string> = {
|
|||
"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)) {
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
26
src/cli.ts
26
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");
|
||||
|
|
|
|||
124
src/tests/startup-model-validation.test.ts
Normal file
124
src/tests/startup-model-validation.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue