From 4e0c18238dc2b50334894dce46bb6152aa767558 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 25 May 2026 08:35:55 +0100 Subject: [PATCH] fix: harden release model discovery --- .../openclaw-live-and-e2e-checks-reusable.yml | 8 +- .../e2e/lib/codex-media-path/write-config.mjs | 7 +- .../lib/codex-npm-plugin-live/assertions.mjs | 5 +- ...els-config.applies-config-env-vars.test.ts | 75 +++++++++++++++++++ src/agents/models-config.merge.test.ts | 31 ++++++++ src/agents/models-config.merge.ts | 10 +++ src/agents/models-config.plan.ts | 20 ++++- .../models-config.providers.secret-helpers.ts | 11 ++- .../gateway-codex-harness.live.test.ts | 8 +- .../gateway-models.profiles.live.test.ts | 24 +++++- .../provider-discovery.runtime.test.ts | 50 +++++++++++++ src/plugins/provider-discovery.runtime.ts | 3 + 12 files changed, 232 insertions(+), 20 deletions(-) diff --git a/.github/workflows/openclaw-live-and-e2e-checks-reusable.yml b/.github/workflows/openclaw-live-and-e2e-checks-reusable.yml index b88a5b20136..6f75ce25943 100644 --- a/.github/workflows/openclaw-live-and-e2e-checks-reusable.yml +++ b/.github/workflows/openclaw-live-and-e2e-checks-reusable.yml @@ -1986,7 +1986,7 @@ jobs: - suite_id: native-live-src-gateway-profiles-anthropic-opus suite_group: native-live-src-gateway-profiles-anthropic label: Native live gateway profiles Anthropic Opus - command: OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-opus-4-7 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles + command: OPENCLAW_LIVE_GATEWAY_THINKING=low OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-opus-4-7 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles timeout_minutes: 30 profile_env_only: false advisory: true @@ -1994,7 +1994,7 @@ jobs: - suite_id: native-live-src-gateway-profiles-anthropic-sonnet-haiku suite_group: native-live-src-gateway-profiles-anthropic label: Native live gateway profiles Anthropic Sonnet/Haiku - command: OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-sonnet-4-6,anthropic/claude-haiku-4-5 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles + command: OPENCLAW_LIVE_GATEWAY_THINKING=low OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-sonnet-4-6,anthropic/claude-haiku-4-5 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles timeout_minutes: 30 profile_env_only: false advisory: true @@ -2013,7 +2013,7 @@ jobs: profiles: stable full - suite_id: native-live-src-gateway-profiles-openai label: Native live gateway profiles OpenAI - command: OPENCLAW_LIVE_GATEWAY_PROVIDERS=openai OPENCLAW_LIVE_GATEWAY_MODELS=openai/gpt-5.5 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles + command: OPENCLAW_LIVE_GATEWAY_THINKING=low OPENCLAW_LIVE_GATEWAY_PROVIDERS=openai OPENCLAW_LIVE_GATEWAY_MODELS=openai/gpt-5.5 node .release-harness/scripts/test-live-shard.mjs native-live-src-gateway-profiles timeout_minutes: 60 profile_env_only: false profiles: beta minimum stable full @@ -2294,7 +2294,7 @@ jobs: profiles: beta minimum stable full - suite_id: live-gateway-anthropic-docker label: Docker live gateway Anthropic - command: OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-sonnet-4-6 OPENCLAW_LIVE_GATEWAY_MAX_MODELS=1 OPENCLAW_LIVE_GATEWAY_STEP_TIMEOUT_MS=90000 OPENCLAW_LIVE_GATEWAY_MODEL_TIMEOUT_MS=180000 OPENCLAW_LIVE_DOCKER_REPO_ROOT="$GITHUB_WORKSPACE" timeout --foreground --kill-after=30s 35m bash .release-harness/scripts/test-live-gateway-models-docker.sh + command: OPENCLAW_LIVE_GATEWAY_THINKING=low OPENCLAW_LIVE_GATEWAY_PROVIDERS=anthropic OPENCLAW_LIVE_GATEWAY_MODELS=anthropic/claude-sonnet-4-6 OPENCLAW_LIVE_GATEWAY_MAX_MODELS=1 OPENCLAW_LIVE_GATEWAY_STEP_TIMEOUT_MS=90000 OPENCLAW_LIVE_GATEWAY_MODEL_TIMEOUT_MS=180000 OPENCLAW_LIVE_DOCKER_REPO_ROOT="$GITHUB_WORKSPACE" timeout --foreground --kill-after=30s 35m bash .release-harness/scripts/test-live-gateway-models-docker.sh timeout_minutes: 40 profile_env_only: false profiles: stable full diff --git a/scripts/e2e/lib/codex-media-path/write-config.mjs b/scripts/e2e/lib/codex-media-path/write-config.mjs index c146daeea30..a1579bb2637 100644 --- a/scripts/e2e/lib/codex-media-path/write-config.mjs +++ b/scripts/e2e/lib/codex-media-path/write-config.mjs @@ -45,7 +45,6 @@ const config = { }, agents: { defaults: { - agentRuntime: { id: "codex" }, model: { primary: "codex/gpt-5.5", fallbacks: [] }, models: { "codex/gpt-5.5": { @@ -61,8 +60,12 @@ const config = { { id: "main", default: true, - agentRuntime: { id: "codex" }, model: { primary: "codex/gpt-5.5", fallbacks: [] }, + models: { + "codex/gpt-5.5": { + agentRuntime: { id: "codex" }, + }, + }, workspace: workspaceDir, }, ], diff --git a/scripts/e2e/lib/codex-npm-plugin-live/assertions.mjs b/scripts/e2e/lib/codex-npm-plugin-live/assertions.mjs index fed4d868d47..e3e768f5334 100644 --- a/scripts/e2e/lib/codex-npm-plugin-live/assertions.mjs +++ b/scripts/e2e/lib/codex-npm-plugin-live/assertions.mjs @@ -66,7 +66,10 @@ function configure() { defaults: { ...cfg.agents?.defaults, model: { primary: modelRef, fallbacks: [] }, - agentRuntime: { id: "codex" }, + models: { + ...cfg.agents?.defaults?.models, + [modelRef]: { agentRuntime: { id: "codex" } }, + }, workspace: path.join(state, "workspace"), skipBootstrap: true, timeoutSeconds: 420, diff --git a/src/agents/models-config.applies-config-env-vars.test.ts b/src/agents/models-config.applies-config-env-vars.test.ts index 447dc16db91..62c041fb406 100644 --- a/src/agents/models-config.applies-config-env-vars.test.ts +++ b/src/agents/models-config.applies-config-env-vars.test.ts @@ -30,6 +30,25 @@ function createImplicitOpenRouterProvider(): ProviderConfig { }; } +function createImplicitOpenAiProvider(overrides: Partial = {}): ProviderConfig { + return { + baseUrl: "https://api.openai.com/v1", + api: "openai-responses", + models: [ + { + id: "gpt-5.5", + name: "GPT-5.5", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 400000, + maxTokens: 128000, + }, + ], + ...overrides, + }; +} + async function resolveProvidersForConfigEnvTest(params: { cfg: OpenClawConfig; onResolveImplicitProviders: (env: NodeJS.ProcessEnv) => void; @@ -188,6 +207,62 @@ describe("models-config", () => { expect(observedSnapshot).toBe(pluginMetadataSnapshot); }); + it("does not write unauthenticated model providers that would invalidate models.json", async () => { + const plan = await planOpenClawModelsJsonWithDeps( + { + cfg: { models: { providers: {} } }, + agentDir: "/tmp/openclaw-models-config-env-vars-test", + env: {}, + existingRaw: "", + existingParsed: null, + }, + { + resolveImplicitProviders: async () => ({ + openai: createImplicitOpenAiProvider(), + "auth-only": createImplicitOpenAiProvider({ + baseUrl: "https://auth.example/v1", + api: "openai-responses", + models: [], + }), + }), + }, + ); + + expect(plan.action).toBe("write"); + if (plan.action !== "write") { + throw new Error("Expected models.json write plan"); + } + const parsed = JSON.parse(plan.contents) as { providers?: Record }; + expect(parsed.providers?.openai).toBeUndefined(); + expect(parsed.providers?.["auth-only"]).toBeDefined(); + }); + + it("falls back to canonical env markers when provider runtime has no api-key policy", async () => { + const plan = await planOpenClawModelsJsonWithDeps( + { + cfg: { models: { providers: {} } }, + agentDir: "/tmp/openclaw-models-config-env-vars-test", + env: { OPENAI_API_KEY: "sk-test" } as NodeJS.ProcessEnv, + existingRaw: "", + existingParsed: null, + }, + { + resolveImplicitProviders: async () => ({ + openai: createImplicitOpenAiProvider(), + }), + }, + ); + + expect(plan.action).toBe("write"); + if (plan.action !== "write") { + throw new Error("Expected models.json write plan"); + } + const parsed = JSON.parse(plan.contents) as { + providers?: Record; + }; + expect(parsed.providers?.openai?.apiKey).toBe("OPENAI_API_KEY"); + }); + it("normalizes retired Gemini ids preserved from existing models.json rows", async () => { const plan = await planOpenClawModelsJsonWithDeps( { diff --git a/src/agents/models-config.merge.test.ts b/src/agents/models-config.merge.test.ts index ae1ed7c1a81..9baa2b5866e 100644 --- a/src/agents/models-config.merge.test.ts +++ b/src/agents/models-config.merge.test.ts @@ -180,6 +180,37 @@ describe("models-config merge helpers", () => { expect(merged["custom-proxy"]?.baseUrl).toBe("http://localhost:4000/v1"); }); + it("drops stale invalid existing providers that would poison models.json", () => { + const merged = mergeWithExistingProviderSecrets({ + nextProviders: { + openai: createConfigProvider(), + }, + existingProviders: { + "claude-cli": { + api: "anthropic-messages", + models: [ + createModel({ + id: "claude-sonnet-4-6", + name: "Claude Sonnet", + reasoning: true, + }), + ], + } as unknown as ExistingProviderConfig, + "auth-only": { + baseUrl: "https://auth.example/v1", + api: "openai-responses", + apiKey: preservedApiKey, + models: [], + } as ExistingProviderConfig, + }, + secretRefManagedProviders: new Set(), + }); + + expect(merged["claude-cli"]).toBeUndefined(); + expect(merged["auth-only"]?.apiKey).toBe(preservedApiKey); + expect(merged.openai).toBeDefined(); + }); + it("preserves non-empty existing apiKey and baseUrl from models.json", () => { const merged = mergeWithExistingProviderSecrets({ nextProviders: { diff --git a/src/agents/models-config.merge.ts b/src/agents/models-config.merge.ts index c925b2d7463..139d04040f8 100644 --- a/src/agents/models-config.merge.ts +++ b/src/agents/models-config.merge.ts @@ -212,6 +212,13 @@ function shouldPreserveExistingBaseUrl(params: { return !existingApi || !nextApi || existingApi === nextApi; } +function isExistingProviderSelfContained(entry: ExistingProviderConfig): boolean { + if (!Array.isArray(entry.models) || entry.models.length === 0) { + return true; + } + return Boolean(entry.baseUrl?.trim() && entry.apiKey); +} + export function mergeWithExistingProviderSecrets(params: { nextProviders: Record; existingProviders: Record; @@ -220,6 +227,9 @@ export function mergeWithExistingProviderSecrets(params: { const { nextProviders, existingProviders, secretRefManagedProviders } = params; const mergedProviders: Record = {}; for (const [key, entry] of Object.entries(existingProviders)) { + if (!isExistingProviderSelfContained(entry)) { + continue; + } mergedProviders[key] = entry; } for (const [key, newEntry] of Object.entries(nextProviders)) { diff --git a/src/agents/models-config.plan.ts b/src/agents/models-config.plan.ts index 71334bf3d10..7f099ee182f 100644 --- a/src/agents/models-config.plan.ts +++ b/src/agents/models-config.plan.ts @@ -105,6 +105,22 @@ function resolveProvidersForMode(params: { }); } +function isWritableProviderConfig(provider: ProviderConfig): boolean { + if (!Array.isArray(provider.models) || provider.models.length === 0) { + return true; + } + return Boolean(provider.baseUrl?.trim() && provider.apiKey); +} + +function filterWritableProviders( + providers: Record, +): Record { + const next = Object.fromEntries( + Object.entries(providers).filter(([, provider]) => isWritableProviderConfig(provider)), + ); + return Object.keys(next).length === Object.keys(providers).length ? providers : next; +} + export async function planOpenClawModelsJsonWithDeps( params: { cfg: OpenClawConfig; @@ -181,7 +197,9 @@ export async function planOpenClawModelsJsonWithDeps( sourceSecretDefaults: params.sourceConfigForSecrets?.secrets?.defaults, secretRefManagedProviders, }) ?? normalizedMergedProviders; - const finalProviders = applyNativeStreamingUsageCompat(secretEnforcedProviders); + const finalProviders = applyNativeStreamingUsageCompat( + filterWritableProviders(secretEnforcedProviders), + ); const nextContents = `${JSON.stringify({ providers: finalProviders }, null, 2)}\n`; if (params.existingRaw === nextContents) { diff --git a/src/agents/models-config.providers.secret-helpers.ts b/src/agents/models-config.providers.secret-helpers.ts index a419bab9a0c..3beb405cb6d 100644 --- a/src/agents/models-config.providers.secret-helpers.ts +++ b/src/agents/models-config.providers.secret-helpers.ts @@ -287,13 +287,12 @@ export function resolveMissingProviderApiKey(params: { const authMode = params.provider.auth; if (params.providerApiKeyResolver && (!authMode || authMode === "aws-sdk")) { const resolvedApiKey = params.providerApiKeyResolver(params.env); - if (!resolvedApiKey) { - return params.provider; + if (resolvedApiKey) { + return { + ...params.provider, + apiKey: resolvedApiKey, + }; } - return { - ...params.provider, - apiKey: resolvedApiKey, - }; } if (authMode === "aws-sdk") { const awsEnvVar = resolveAwsSdkApiKeyVarName(params.env); diff --git a/src/gateway/gateway-codex-harness.live.test.ts b/src/gateway/gateway-codex-harness.live.test.ts index f3f8874a78d..4c7d2a23e73 100644 --- a/src/gateway/gateway-codex-harness.live.test.ts +++ b/src/gateway/gateway-codex-harness.live.test.ts @@ -237,16 +237,15 @@ async function writeLiveGatewayConfig(params: { }, }, }, - // The Codex plugin owns the `codex/*` catalog/auth marker. Keeping the - // fixture on that provider proves the app-server harness path instead of - // exercising legacy OpenAI-Codex provider overrides. + // The Codex plugin owns the `codex/*` catalog/auth marker. Keeping runtime + // policy on the model entry proves the app-server harness path. agents: { defaults: { workspace: params.workspace, - agentRuntime: { id: "codex" }, skipBootstrap: true, timeoutSeconds: CODEX_HARNESS_AGENT_TIMEOUT_SECONDS, model: { primary: params.modelKey }, + models: { [params.modelKey]: { agentRuntime: { id: "codex" } } }, sandbox: { mode: "off" }, }, list: [ @@ -254,7 +253,6 @@ async function writeLiveGatewayConfig(params: { id: "dev", default: true, workspace: params.workspace, - agentRuntime: { id: "codex" }, model: { primary: params.modelKey }, models: { [params.modelKey]: { agentRuntime: { id: "codex" } } }, }, diff --git a/src/gateway/gateway-models.profiles.live.test.ts b/src/gateway/gateway-models.profiles.live.test.ts index c65697c453c..641d5faaba5 100644 --- a/src/gateway/gateway-models.profiles.live.test.ts +++ b/src/gateway/gateway-models.profiles.live.test.ts @@ -68,7 +68,10 @@ const REQUIRE_PROFILE_KEYS = isLiveProfileKeyModeEnabled(); const LIVE_CREDENTIAL_PRECEDENCE = REQUIRE_PROFILE_KEYS ? "profile-first" : "env-first"; const PROVIDERS = parseFilter(process.env.OPENCLAW_LIVE_GATEWAY_PROVIDERS); const GATEWAY_LIVE_SMOKE = isTruthyEnvValue(process.env.OPENCLAW_LIVE_GATEWAY_SMOKE); -const THINKING_LEVEL = GATEWAY_LIVE_SMOKE ? "low" : "high"; +const THINKING_LEVEL = resolveGatewayLiveThinkingLevel({ + raw: process.env.OPENCLAW_LIVE_GATEWAY_THINKING, + smoke: GATEWAY_LIVE_SMOKE, +}); const ENABLE_EXTRA_TOOL_PROBES = !GATEWAY_LIVE_SMOKE; const ENABLE_EXTRA_IMAGE_PROBES = !GATEWAY_LIVE_SMOKE; const THINKING_TAG_RE = /<\s*\/?\s*(?:(?:antml:)?(?:think(?:ing)?|thought)|antthinking)\s*>/i; @@ -1005,6 +1008,13 @@ describe("providerScopedModelRegistryProviders", () => { }); describe("resolveGatewayLiveModelThinkingLevel", () => { + it("allows release lanes to lower gateway live thinking without smoke mode", () => { + expect(resolveGatewayLiveThinkingLevel({ raw: "low", smoke: false })).toBe("low"); + expect(resolveGatewayLiveThinkingLevel({ raw: undefined, smoke: false })).toBe("high"); + expect(resolveGatewayLiveThinkingLevel({ raw: undefined, smoke: true })).toBe("low"); + expect(resolveGatewayLiveThinkingLevel({ raw: "wat", smoke: false })).toBe("high"); + }); + it("clamps requested thinking to levels supported by model metadata", () => { expect( resolveGatewayLiveModelThinkingLevel({ @@ -2096,6 +2106,18 @@ function resolveGatewayLiveModelThinkingLevel(params: { return clampThinkingLevel(model, normalized); } +function resolveGatewayLiveThinkingLevel(params: { raw?: string; smoke: boolean }): string { + const raw = params.raw?.trim().toLowerCase(); + if (!raw) { + return params.smoke ? "low" : "high"; + } + return ["off", "minimal", "low", "medium", "high", "xhigh"].includes(raw) + ? raw + : params.smoke + ? "low" + : "high"; +} + function buildLiveGatewayConfig(params: { cfg: OpenClawConfig; candidates: Array; diff --git a/src/plugins/provider-discovery.runtime.test.ts b/src/plugins/provider-discovery.runtime.test.ts index 51732b614b6..919c11027f4 100644 --- a/src/plugins/provider-discovery.runtime.test.ts +++ b/src/plugins/provider-discovery.runtime.test.ts @@ -405,6 +405,56 @@ describe("resolvePluginDiscoveryProvidersRuntime", () => { }); }); + it("ignores manifest model catalogs that cannot form valid models.json providers", () => { + mocks.resolveDiscoveredProviderPluginIds.mockReturnValue(["anthropic"]); + mocks.loadPluginMetadataSnapshot.mockReturnValue({ + index: { plugins: [] }, + manifestRegistry: { + plugins: [ + { + ...createManifestPluginWithModelCatalog("anthropic"), + modelCatalog: { + providers: { + "claude-cli": { + models: [ + { + id: "claude-sonnet-4-6", + name: "Claude Sonnet 4.6", + reasoning: true, + input: ["text"], + contextWindow: 200000, + maxTokens: 64000, + }, + ], + }, + anthropic: { + baseUrl: "https://api.anthropic.com", + api: "anthropic-messages", + models: [ + { + id: "claude-sonnet-4-6", + name: "Claude Sonnet 4.6", + reasoning: true, + input: ["text"], + contextWindow: 200000, + maxTokens: 64000, + }, + ], + }, + }, + discovery: { "claude-cli": "static", anthropic: "static" }, + }, + }, + ], + diagnostics: [], + }, + }); + + const providers = resolvePluginDiscoveryProvidersRuntime({ discoveryEntriesOnly: true }); + + expect(providers.map((provider) => provider.id)).toEqual(["anthropic"]); + }); + it("keeps manifest catalogs and loads only scoped plugins that have no entry", () => { const dynamicProvider = createProvider({ id: "minimax", mode: "catalog" }); mocks.resolveDiscoveredProviderPluginIds.mockReturnValue(["minimax", "openai"]); diff --git a/src/plugins/provider-discovery.runtime.ts b/src/plugins/provider-discovery.runtime.ts index 76ae85c9f31..ed705eb95f4 100644 --- a/src/plugins/provider-discovery.runtime.ts +++ b/src/plugins/provider-discovery.runtime.ts @@ -134,6 +134,9 @@ function providerConfigFromManifestRows( rows: readonly NormalizedModelCatalogRow[], ): ModelProviderConfig | undefined { const firstRow = rows[0]; + if (!firstRow?.baseUrl || !firstRow.api) { + return undefined; + } const models = rows .map((row) => modelDefinitionFromManifestRow(row)) .filter((model): model is ModelDefinitionConfig => Boolean(model));