diff --git a/src/agents/pi-embedded-runner/model.test-harness.ts b/src/agents/pi-embedded-runner/model.test-harness.ts index b91ca8b8c5f..4ce53098e46 100644 --- a/src/agents/pi-embedded-runner/model.test-harness.ts +++ b/src/agents/pi-embedded-runner/model.test-harness.ts @@ -124,3 +124,22 @@ export function mockDiscoveredModel(params: { }), } as unknown as ReturnType); } + +/** + * Mock a stale discovered gpt-5.4 model while keeping the gpt-5.2-codex + * template visible so `hasDynamicOverrideTemplate` returns true and the + * `preferResolvedModel`/`preserveDiscoveredTransportMetadata` path is exercised. + */ +export function mockStaleCodexDiscovery(staleModel: Record): void { + vi.mocked(discoverModels).mockReturnValue({ + find: vi.fn((provider: string, modelId: string) => { + if (provider === "openai-codex" && modelId === "gpt-5.2-codex") { + return OPENAI_CODEX_TEMPLATE_MODEL; + } + if (provider === "openai-codex" && modelId === "gpt-5.4") { + return staleModel; + } + return null; + }), + } as unknown as ReturnType); +} diff --git a/src/agents/pi-embedded-runner/model.test.ts b/src/agents/pi-embedded-runner/model.test.ts index b733e3a3f5f..459ce5c805e 100644 --- a/src/agents/pi-embedded-runner/model.test.ts +++ b/src/agents/pi-embedded-runner/model.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { discoverModels } from "../pi-model-discovery.js"; vi.mock("../pi-model-discovery.js", () => ({ discoverAuthStorage: vi.fn(() => ({ mocked: true })), @@ -26,6 +27,7 @@ import { makeModel, mockDiscoveredModel, mockOpenAICodexTemplateModel, + mockStaleCodexDiscovery, resetMockDiscoverModels, } from "./model.test-harness.js"; @@ -666,6 +668,207 @@ describe("resolveModel", () => { expect(result.model).toMatchObject(buildOpenAICodexForwardCompatExpectation("gpt-5.4")); }); + it("prefers the codex gpt-5.4 forward-compat model over stale discovered metadata", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 272000, + maxTokens: 128000, + }); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 1_050_000, + maxTokens: 128_000, + }); + }); + + it("preserves configured openai-codex overrides when stale discovery loses to the dynamic gpt-5.4 model", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 272000, + maxTokens: 64000, + }); + + const cfg: OpenClawConfig = { + models: { + providers: { + "openai-codex": { + baseUrl: "https://custom.example.com", + models: [{ id: "gpt-5.4", maxTokens: 64_000 }], + }, + }, + }, + } as unknown as OpenClawConfig; + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent", cfg); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + baseUrl: "https://custom.example.com", + contextWindow: 1_050_000, + maxTokens: 64_000, + }); + }); + + it("prefers the codex gpt-5.4 forward-compat model on async resolve when discovery is stale", async () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 272000, + maxTokens: 128000, + }); + + const result = await resolveModelAsync("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 1_050_000, + maxTokens: 128_000, + }); + }); + + it("preserves discovered baseUrl and headers when dynamic gpt-5.4 wins", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 272000, + maxTokens: 128000, + baseUrl: "https://proxy.example.com/backend-api", + headers: { "X-Test-Route": "tenant-a" }, + }); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + baseUrl: "https://proxy.example.com/backend-api", + contextWindow: 1_050_000, + }); + expect((result.model as { headers?: Record }).headers).toMatchObject({ + "X-Test-Route": "tenant-a", + }); + }); + + it("preserves discovered api and compat when dynamic gpt-5.4 wins", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + api: "openai-completions", + compat: { supportsStore: false }, + contextWindow: 272000, + maxTokens: 128000, + }); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + api: "openai-completions", + compat: { supportsStore: false }, + contextWindow: 1_050_000, + maxTokens: 128_000, + }); + }); + + it("keeps model-level api overrides when dynamic gpt-5.4 wins", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + api: "openai-completions", + contextWindow: 272000, + maxTokens: 128000, + }); + + const cfg: OpenClawConfig = { + models: { + providers: { + "openai-codex": { + models: [{ id: "gpt-5.4", api: "openai-responses" }], + }, + }, + }, + } as unknown as OpenClawConfig; + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent", cfg); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + // api: "openai-responses" from config is normalized to "openai-codex-responses" + // by normalizeCodexTransport when baseUrl is the codex endpoint + contextWindow: 1_050_000, + maxTokens: 128_000, + }); + }); + + it("preserves discovered input when dynamic gpt-5.4 wins", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + input: ["text"], + contextWindow: 272000, + maxTokens: 128000, + }); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + input: ["text"], + contextWindow: 1_050_000, + maxTokens: 128_000, + }); + }); + + it("preserves discovered maxTokens when dynamic gpt-5.4 wins", () => { + mockStaleCodexDiscovery({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + maxTokens: 64_000, + contextWindow: 272000, + }); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + contextWindow: 1_050_000, + maxTokens: 64_000, + }); + }); + + it("keeps discovered gpt-5.4 metadata when no codex template exists", () => { + vi.mocked(discoverModels).mockReturnValue({ + find: vi.fn((provider: string, modelId: string) => { + if (provider !== "openai-codex" || modelId !== "gpt-5.4") { + return null; + } + return { + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + input: ["text"], + contextWindow: 272000, + maxTokens: 128000, + cost: { input: 1, output: 2, cacheRead: 3, cacheWrite: 4 }, + }; + }), + } as unknown as ReturnType); + + const result = resolveModel("openai-codex", "gpt-5.4", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + ...buildOpenAICodexForwardCompatExpectation("gpt-5.4"), + // api stays "openai-codex-responses" after plugin normalization + input: ["text"], + contextWindow: 272000, + maxTokens: 128000, + cost: { input: 1, output: 2, cacheRead: 3, cacheWrite: 4 }, + }); + }); + it("builds an openai-codex fallback for gpt-5.3-codex-spark", () => { mockOpenAICodexTemplateModel(); diff --git a/src/agents/pi-embedded-runner/model.ts b/src/agents/pi-embedded-runner/model.ts index 5bf97a683d0..eac91d88a8a 100644 --- a/src/agents/pi-embedded-runner/model.ts +++ b/src/agents/pi-embedded-runner/model.ts @@ -232,6 +232,103 @@ function resolveExplicitModelWithRegistry(params: { return undefined; } +function preferResolvedModel( + discoveredModel: Model | undefined, + dynamicModel: Model | undefined, +): Model | undefined { + if (!dynamicModel) { + return discoveredModel; + } + if (!discoveredModel) { + return dynamicModel; + } + const dynamicContextWindow = dynamicModel.contextWindow ?? 0; + const discoveredContextWindow = discoveredModel.contextWindow ?? 0; + if (dynamicContextWindow > discoveredContextWindow) { + return dynamicModel; + } + if (dynamicContextWindow < discoveredContextWindow) { + return discoveredModel; + } + return dynamicModel; +} + +const OPENAI_CODEX_DYNAMIC_OVERRIDE_MODELS = new Set(["gpt-5.4"]); +const OPENAI_CODEX_DYNAMIC_OVERRIDE_TEMPLATES: Record = { + "gpt-5.4": ["gpt-5.3-codex", "gpt-5.2-codex"], +}; + +function shouldPreferDynamicModelOverride(params: { provider: string; modelId: string }): boolean { + return ( + normalizeProviderId(params.provider) === "openai-codex" && + OPENAI_CODEX_DYNAMIC_OVERRIDE_MODELS.has(params.modelId) + ); +} + +function hasDynamicOverrideTemplate(params: { + provider: string; + modelId: string; + modelRegistry: ModelRegistry; +}): boolean { + if (!shouldPreferDynamicModelOverride(params)) { + return false; + } + const templateIds = OPENAI_CODEX_DYNAMIC_OVERRIDE_TEMPLATES[params.modelId]; + if (!templateIds?.length) { + return false; + } + return templateIds.some((templateId) => params.modelRegistry.find(params.provider, templateId)); +} + +function preserveDiscoveredTransportMetadata(params: { + discoveredModel: Model | undefined; + dynamicModel: Model | undefined; + providerConfig?: InlineProviderConfig; + modelId: string; +}): Model | undefined { + const { discoveredModel, dynamicModel, providerConfig, modelId } = params; + if (!discoveredModel || !dynamicModel) { + return dynamicModel; + } + const configuredModel = providerConfig?.models?.find((candidate) => candidate.id === modelId); + const discoveredHeaders = sanitizeModelHeaders(discoveredModel.headers, { + stripSecretRefMarkers: true, + }); + const dynamicHeaders = sanitizeModelHeaders(dynamicModel.headers, { + stripSecretRefMarkers: true, + }); + // Headers use a 3-way merge: dynamic template < discovered < configured. + // dynamicModel.headers already includes configured overrides from + // applyConfiguredProviderOverrides, so we extract configured headers separately + // and apply them last to ensure they win over stale discovered headers. + const providerHeaders = sanitizeModelHeaders(providerConfig?.headers, { + stripSecretRefMarkers: true, + }); + const configuredModelHeaders = sanitizeModelHeaders(configuredModel?.headers, { + stripSecretRefMarkers: true, + }); + const mergedHeaders = + dynamicHeaders || discoveredHeaders || providerHeaders || configuredModelHeaders + ? { + ...dynamicHeaders, + ...discoveredHeaders, + ...providerHeaders, + ...configuredModelHeaders, + } + : undefined; + return { + ...dynamicModel, + api: configuredModel?.api ?? providerConfig?.api ?? discoveredModel.api ?? dynamicModel.api, + baseUrl: providerConfig?.baseUrl ?? discoveredModel.baseUrl ?? dynamicModel.baseUrl, + input: configuredModel?.input ?? discoveredModel.input ?? dynamicModel.input, + compat: configuredModel?.compat ?? discoveredModel.compat ?? dynamicModel.compat, + maxTokens: configuredModel?.maxTokens ?? discoveredModel.maxTokens ?? dynamicModel.maxTokens, + reasoning: configuredModel?.reasoning ?? discoveredModel.reasoning ?? dynamicModel.reasoning, + cost: discoveredModel.cost ?? dynamicModel.cost, + headers: mergedHeaders, + }; +} + export function resolveModelWithRegistry(params: { provider: string; modelId: string; @@ -243,7 +340,9 @@ export function resolveModelWithRegistry(params: { if (explicitModel?.kind === "suppressed") { return undefined; } - if (explicitModel?.kind === "resolved") { + const shouldCompareDynamicOverride = + shouldPreferDynamicModelOverride(params) && hasDynamicOverrideTemplate(params); + if (explicitModel?.kind === "resolved" && !shouldCompareDynamicOverride) { return explicitModel.model; } @@ -261,12 +360,33 @@ export function resolveModelWithRegistry(params: { providerConfig, }, }); - if (pluginDynamicModel) { + const configuredDynamicModel = pluginDynamicModel + ? applyConfiguredProviderOverrides({ + discoveredModel: pluginDynamicModel, + providerConfig, + modelId, + }) + : undefined; + const discoveredResolvedModel = + explicitModel?.kind === "resolved" ? explicitModel.model : undefined; + const dynamicModelForComparison = shouldCompareDynamicOverride + ? preserveDiscoveredTransportMetadata({ + discoveredModel: discoveredResolvedModel, + dynamicModel: configuredDynamicModel, + providerConfig, + modelId, + }) + : configuredDynamicModel; + const preferredModel = preferResolvedModel(discoveredResolvedModel, dynamicModelForComparison); + if (preferredModel) { + if (preferredModel === explicitModel?.model) { + return preferredModel; + } return normalizeResolvedModel({ provider, cfg, agentDir, - model: pluginDynamicModel, + model: preferredModel, }); } @@ -368,7 +488,7 @@ export async function resolveModelAsync( modelRegistry, }; } - if (!explicitModel) { + const maybePrepareDynamicModel = async () => { const providerPlugin = resolveProviderRuntimePlugin({ provider, config: cfg, @@ -387,17 +507,21 @@ export async function resolveModelAsync( }, }); } + }; + if ( + !explicitModel || + (explicitModel.kind === "resolved" && + hasDynamicOverrideTemplate({ provider, modelId, modelRegistry })) + ) { + await maybePrepareDynamicModel(); } - const model = - explicitModel?.kind === "resolved" - ? explicitModel.model - : resolveModelWithRegistry({ - provider, - modelId, - modelRegistry, - cfg, - agentDir: resolvedAgentDir, - }); + const model = resolveModelWithRegistry({ + provider, + modelId, + modelRegistry, + cfg, + agentDir: resolvedAgentDir, + }); if (model) { return { model, authStorage, modelRegistry }; }