diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 80c6e5cf8..4d65d0409 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -26,6 +26,7 @@ - Fixed `omp plugin list --json` omitting locally linked plugins that exist only in `omp-plugins.lock.json` and `node_modules` symlinks. ([#2742](https://github.com/can1357/oh-my-pi/issues/2742)) - Fixed task subagents to install their configured ordered model candidates as child-session retry fallback chains, so retryable provider failures can advance to the next subagent model instead of failing the worker ([#2750](https://github.com/can1357/oh-my-pi/issues/2750)). - Fixed empty reasonless aborted assistant turns to auto-retry without switching model fallback, so transient provider-side aborts after tool results do not end headless sessions ([#2685](https://github.com/can1357/oh-my-pi/issues/2685)). +- Fixed startup model fallback choosing the plain OpenAI `gpt-5.5` provider before the Codex OAuth provider when both shared the same default model id, which could surface a misleading OpenAI 401 despite valid Codex credentials ([#2807](https://github.com/can1357/oh-my-pi/issues/2807)). ## [16.0.1] - 2026-06-15 diff --git a/packages/coding-agent/src/config/model-resolver.ts b/packages/coding-agent/src/config/model-resolver.ts index 79ee2253b..a28a2c3c6 100644 --- a/packages/coding-agent/src/config/model-resolver.ts +++ b/packages/coding-agent/src/config/model-resolver.ts @@ -34,15 +34,28 @@ import { isAuthenticated, kNoAuth, type ModelRegistry } from "./model-registry"; import { MODEL_ROLE_IDS, type ModelRole } from "./model-roles"; import type { Settings } from "./settings"; +function isKnownProvider(provider: string): provider is KnownProvider { + return provider in DEFAULT_MODEL_PER_PROVIDER; +} + /** - * Pick the first available model matching a known provider's default id - * (catalog table order), falling back to the first available model. + * Pick the best available provider-default model before falling back to raw model order. + * + * Provider-default matches are ranked by canonical provider priority so native/OAuth + * providers win over API-compatible mirrors that expose the same default model id. */ -function pickDefaultAvailableModel(availableModels: Model[]): Model | undefined { - for (const provider of Object.keys(DEFAULT_MODEL_PER_PROVIDER) as KnownProvider[]) { - const defaultId = DEFAULT_MODEL_PER_PROVIDER[provider]; - const match = availableModels.find(m => m.provider === provider && m.id === defaultId); - if (match) return match; +export function pickDefaultAvailableModel(availableModels: Model[]): Model | undefined { + const providerPriority = buildModelProviderPriorityRank(); + const defaultMatches = availableModels.filter( + model => isKnownProvider(model.provider) && DEFAULT_MODEL_PER_PROVIDER[model.provider] === model.id, + ); + if (defaultMatches.length > 0) { + return [...defaultMatches].sort((a, b) => { + const aRank = providerPriority.get(a.provider.toLowerCase()) ?? Number.POSITIVE_INFINITY; + const bRank = providerPriority.get(b.provider.toLowerCase()) ?? Number.POSITIVE_INFINITY; + if (aRank !== bRank) return aRank - bRank; + return availableModels.indexOf(a) - availableModels.indexOf(b); + })[0]; } return availableModels[0]; } @@ -447,10 +460,28 @@ function findExactCanonicalModelMatch( }); } +function resolveCodexPreferredCanonicalMatch( + model: Model, + availableModels: Model[], + modelRegistry: CanonicalModelRegistry | undefined, +): Model | undefined { + if (model.provider !== "openai") return undefined; + const canonicalId = modelRegistry?.getCanonicalId?.(model); + if (!canonicalId || canonicalId !== model.id) return undefined; + const preferred = modelRegistry?.resolveCanonicalModel?.(canonicalId, { + availableOnly: false, + candidates: availableModels, + }); + if (!preferred || modelsAreEqual(preferred, model)) return undefined; + if (preferred.provider !== "openai-codex" || preferred.id !== model.id) return undefined; + return preferred; +} + /** * The single model-matching engine. Tries, in order: * 1. exact `provider/id` reference (variant-alias and OpenRouter routed/date - * fallbacks included), + * fallbacks included; stale `openai/` defaults may still coalesce + * to Codex when the canonical resolver prefers the OAuth transport), * 2. exact canonical id (coalesces provider variants), * 3. exact bare id (preference-ranked), * 4. retired effort-tier variant alias (collapsed catalog entries), @@ -464,10 +495,11 @@ function matchModel( context: ModelPreferenceContext, options?: { modelRegistry?: CanonicalModelRegistry }, ): Model | undefined { - // Explicit provider/model selectors always bypass canonical coalescing. const exactRefMatch = findExactModelReferenceMatch(modelPattern, availableModels); if (exactRefMatch) { - return exactRefMatch; + return ( + resolveCodexPreferredCanonicalMatch(exactRefMatch, availableModels, options?.modelRegistry) ?? exactRefMatch + ); } // Exact canonical ids coalesce provider variants before bare-id matching. diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 46f55ab9e..fae378788 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -21,7 +21,6 @@ import { getOpenAICodexTransportDetails, prewarmOpenAICodexResponses, } from "@oh-my-pi/pi-ai/providers/openai-codex-responses"; -import { DEFAULT_MODEL_PER_PROVIDER } from "@oh-my-pi/pi-catalog/provider-models"; import type { Component } from "@oh-my-pi/pi-tui"; import { $env, @@ -49,6 +48,7 @@ import { getModelMatchPreferences, parseModelPattern, parseModelString, + pickDefaultAvailableModel, resolveAllowedModels, resolveModelRoleValue, } from "./config/model-resolver"; @@ -1928,29 +1928,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} // Re-resolve the allowed set: extension factories above may have // registered providers/models that weren't visible at startup. const fallbackCandidates = await resolveAllowedModels(modelRegistry, settings, modelMatchPreferences); - // Prefer each provider's configured default model - // (DEFAULT_MODEL_PER_PROVIDER) over raw catalog order. Without this the - // first-run fallback picks whatever model sorts first in models.json for - // the winning provider (e.g. anthropic's claude-3-5-sonnet-20240620) - // instead of the intended provider default (claude-sonnet-4-6). Mirrors - // findInitialModel's precedence. - for (const [provider, defaultId] of Object.entries(DEFAULT_MODEL_PER_PROVIDER)) { - const preferred = fallbackCandidates.find( - candidate => candidate.provider === provider && candidate.id === defaultId, - ); - if (preferred && hasModelAuth(preferred)) { - model = preferred; - break; - } - } - // Otherwise, first available model with a valid API key. - if (!model) { - for (const candidate of fallbackCandidates) { - if (hasModelAuth(candidate)) { - model = candidate; - break; - } - } + const defaultModel = pickDefaultAvailableModel(fallbackCandidates.filter(hasModelAuth)); + if (defaultModel) { + model = defaultModel; } if (model) { if (modelFallbackMessage) { diff --git a/packages/coding-agent/test/model-resolver.test.ts b/packages/coding-agent/test/model-resolver.test.ts index 08b3bf831..02376f3b9 100644 --- a/packages/coding-agent/test/model-resolver.test.ts +++ b/packages/coding-agent/test/model-resolver.test.ts @@ -3,10 +3,12 @@ import { type Api, Effort, type Model } from "@oh-my-pi/pi-ai"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; import type { CanonicalModelVariant } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { + type CanonicalModelRegistry, expandRoleAlias, filterAvailableModelsByEnabledPatterns, parseModelPattern, parseModelString, + pickDefaultAvailableModel, resolveAgentModelPatterns, resolveCliModel, resolveModelFromString, @@ -170,6 +172,49 @@ const mockCodexOverlapModels: Model<"anthropic-messages">[] = [ }), ]; +const openaiGpt55Models: Model[] = [ + buildModel({ + id: "gpt-5.5", + name: "GPT-5.5", + api: "openai-responses", + provider: "openai", + baseUrl: "https://api.openai.com", + reasoning: true, + thinking: { + mode: "effort", + efforts: [Effort.Low, Effort.Medium, Effort.High, Effort.XHigh], + }, + input: ["text"], + cost: { input: 1, output: 4, cacheRead: 0.1, cacheWrite: 1 }, + contextWindow: 400000, + maxTokens: 128000, + }), + buildModel({ + id: "gpt-5.5", + name: "GPT-5.5 Codex", + api: "openai-codex-responses", + provider: "openai-codex", + baseUrl: "https://chatgpt.com/backend-api/codex/responses", + reasoning: true, + thinking: { + mode: "effort", + efforts: [Effort.Low, Effort.Medium, Effort.High, Effort.XHigh], + }, + input: ["text"], + cost: { input: 1, output: 4, cacheRead: 0.1, cacheWrite: 1 }, + contextWindow: 400000, + maxTokens: 128000, + }), +]; + +const codexCanonicalRegistry: CanonicalModelRegistry = { + resolveCanonicalModel: (canonicalId: string, options?: { candidates?: Model[] }) => { + if (canonicalId !== "gpt-5.5") return undefined; + return options?.candidates?.find(model => model.provider === "openai-codex" && model.id === canonicalId); + }, + getCanonicalId: (model: Model) => (model.id === "gpt-5.5" ? "gpt-5.5" : undefined), +}; + function createOpusModel(provider: string, id: string, name: string): Model<"anthropic-messages"> { return buildModel({ id, @@ -249,6 +294,28 @@ const canonicalRegistry = { const allModels = [...mockModels, ...mockOpenRouterModels, ...mockProviderOverlapModels, ...mockCodexOverlapModels]; +describe("pickDefaultAvailableModel", () => { + test("prefers Codex OAuth over plain OpenAI for the shared GPT default", () => { + const result = pickDefaultAvailableModel(openaiGpt55Models); + + expect(result?.provider).toBe("openai-codex"); + expect(result?.id).toBe("gpt-5.5"); + }); +}); + +describe("resolveModelRoleValue", () => { + test("reroutes stale OpenAI GPT defaults through Codex canonical selection", () => { + const result = resolveModelRoleValue("openai/gpt-5.5:xhigh", openaiGpt55Models, { + modelRegistry: codexCanonicalRegistry, + }); + + expect(result.model?.provider).toBe("openai-codex"); + expect(result.model?.id).toBe("gpt-5.5"); + expect(result.thinkingLevel).toBe(Effort.XHigh); + expect(result.explicitThinkingLevel).toBe(true); + }); +}); + describe("parseModelPattern", () => { describe("simple patterns without colons", () => { test("exact match returns model with undefined thinking level", () => { diff --git a/packages/coding-agent/test/sdk-model-selection.test.ts b/packages/coding-agent/test/sdk-model-selection.test.ts index e11416e6e..5719946ef 100644 --- a/packages/coding-agent/test/sdk-model-selection.test.ts +++ b/packages/coding-agent/test/sdk-model-selection.test.ts @@ -283,6 +283,45 @@ describe("createAgentSession deferred model pattern resolution", () => { } }); + test("prefers Codex OAuth over plain OpenAI for the shared startup default", async () => { + const openaiDefault = getBundledModel("openai", "gpt-5.5"); + const codexDefault = getBundledModel("openai-codex", "gpt-5.5"); + if (!openaiDefault || !codexDefault) { + throw new Error("Expected bundled OpenAI and Codex GPT-5.5 defaults"); + } + + const authStorage = await AuthStorage.create(path.join(tempDir, "codex-fallback-auth.db")); + authStoragesToClose.push(authStorage); + authStorage.setRuntimeApiKey("openai", "sk-or-v1-invalid-openai-key"); + authStorage.setRuntimeApiKey("openai-codex", "codex-oauth-token"); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "models.yml")); + + const { session } = await createAgentSession({ + cwd: tempDir, + agentDir: tempDir, + authStorage, + modelRegistry, + settings: Settings.isolated(), + sessionManager: SessionManager.inMemory(), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + skipPythonPreflight: true, + }); + + try { + expect(session.model?.provider).toBe("openai-codex"); + expect(session.model?.id).toBe(codexDefault.id); + expect(session.model?.id).toBe(openaiDefault.id); + } finally { + await session.dispose(); + } + }); + test("restores role model from extension provider after startup resume", async () => { const defaultModel = getBundledModel("anthropic", "claude-sonnet-4-5"); if (!defaultModel) {