From 8d57d745cf5f1d83648e3a755957825805f6d77b Mon Sep 17 00:00:00 2001 From: Sathvik-1007 Date: Fri, 24 Apr 2026 03:12:51 +0530 Subject: [PATCH] fix: wizard no clobber model.primary on re-run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit two bugs. both squash user model choice silently. bug 1: applyDefaultModel() unconditional primary: model overwrite. wizard calls with setDefaultModel=true, provider returns its default (e.g. openrouter/auto), bam user primary gone. fix: existingPrimary ?? model. bug 2: applyModelFallbacksFromSelection() phantom primary injection. when no primary configured, resolvedKey (hardcoded default) written as primary via nullish coalescing fallback. fix: conditional spread — only include primary key when one actually existed. tests for both. closes #70696 --- docs/cli/configure.md | 3 + docs/concepts/models.md | 4 + src/commands/auth-choice.apply.types.ts | 1 + src/commands/auth-choice.test.ts | 29 +++++ src/commands/configure.gateway-auth.ts | 1 + src/commands/model-picker.test.ts | 17 +++ src/commands/models/auth.test.ts | 23 ++++ src/flows/model-picker.ts | 2 +- .../provider-auth-choice-helpers.test.ts | 73 +++++++++++- src/plugins/provider-auth-choice-helpers.ts | 14 ++- src/plugins/provider-auth-choice.ts | 110 ++++++++++++++---- 11 files changed, 252 insertions(+), 25 deletions(-) diff --git a/docs/cli/configure.md b/docs/cli/configure.md index ee11f506cf4..119dfc613a1 100644 --- a/docs/cli/configure.md +++ b/docs/cli/configure.md @@ -13,6 +13,9 @@ Note: The **Model** section now includes a multi-select for the `agents.defaults.models` allowlist (what shows up in `/model` and the model picker). Provider-scoped setup choices merge their selected models into the existing allowlist instead of replacing unrelated providers already in the config. +Re-running provider auth from configure preserves an existing +`agents.defaults.model.primary`; use `openclaw models auth login --provider --set-default` +or `openclaw models set ` when you intentionally want to change the default model. When configure starts from a provider auth choice, the default-model and allowlist pickers prefer that provider automatically. For paired providers such diff --git a/docs/concepts/models.md b/docs/concepts/models.md index 771f9ee513a..2c5796dbb49 100644 --- a/docs/concepts/models.md +++ b/docs/concepts/models.md @@ -82,6 +82,10 @@ provided value should become the complete target value. Interactive provider setup and `openclaw configure --section model` also merge provider-scoped selections into the existing allowlist, so adding Codex, Ollama, or another provider does not drop unrelated model entries. +Configure preserves an existing `agents.defaults.model.primary` when provider +auth is re-applied. Explicit default-setting commands such as +`openclaw models auth login --provider --set-default` and +`openclaw models set ` still replace `agents.defaults.model.primary`. ## "Model is not allowed" (and why replies stop) diff --git a/src/commands/auth-choice.apply.types.ts b/src/commands/auth-choice.apply.types.ts index fe135787196..db21a184413 100644 --- a/src/commands/auth-choice.apply.types.ts +++ b/src/commands/auth-choice.apply.types.ts @@ -11,6 +11,7 @@ export type ApplyAuthChoiceParams = { runtime: RuntimeEnv; agentDir?: string; setDefaultModel: boolean; + preserveExistingDefaultModel?: boolean; agentId?: string; opts?: Partial; }; diff --git a/src/commands/auth-choice.test.ts b/src/commands/auth-choice.test.ts index bc59fbfab52..e778023e41b 100644 --- a/src/commands/auth-choice.test.ts +++ b/src/commands/auth-choice.test.ts @@ -996,6 +996,35 @@ describe("applyAuthChoice", () => { } }); + it("keeps an existing default model when configure re-applies provider auth", async () => { + await setupTempState(); + vi.stubEnv("OPENROUTER_API_KEY", "sk-openrouter-test"); + const note = vi.fn(); + const confirm = vi.fn(async () => true); + const text = vi.fn(); + const existingPrimary = "anthropic/claude-opus-4-6"; + const prompter = createPrompter({ text, confirm, note }); + + const result = await applyAuthChoice({ + authChoice: "openrouter-api-key", + config: { agents: { defaults: { model: { primary: existingPrimary } } } }, + prompter, + runtime: createExitThrowingRuntime(), + setDefaultModel: true, + preserveExistingDefaultModel: true, + }); + + expect(resolveAgentModelPrimaryValue(result.config.agents?.defaults?.model)).toBe( + existingPrimary, + ); + expect(result.config.agents?.defaults?.models?.["openrouter/auto"]).toEqual({}); + expect(runProviderModelSelectedHook).not.toHaveBeenCalled(); + expect(note).toHaveBeenCalledWith( + "Kept existing default model anthropic/claude-opus-4-6; openrouter/auto is available.", + "Model configured", + ); + }); + it("uses explicit env for plugin auth resolution instead of host env", async () => { await setupTempState(); process.env.OPENAI_API_KEY = "sk-openai-host"; // pragma: allowlist secret diff --git a/src/commands/configure.gateway-auth.ts b/src/commands/configure.gateway-auth.ts index 65dd16f017e..62ce30169e3 100644 --- a/src/commands/configure.gateway-auth.ts +++ b/src/commands/configure.gateway-auth.ts @@ -152,6 +152,7 @@ export async function promptAuthConfig( prompter, runtime, setDefaultModel: true, + preserveExistingDefaultModel: true, }); next = applied.config; if (applied.retrySelection) { diff --git a/src/commands/model-picker.test.ts b/src/commands/model-picker.test.ts index aa91d4b8c8a..9cc35d2789c 100644 --- a/src/commands/model-picker.test.ts +++ b/src/commands/model-picker.test.ts @@ -566,6 +566,23 @@ describe("applyModelFallbacksFromSelection", () => { }); }); + it("does not inject a phantom primary when none was configured", () => { + const config = { + agents: { + defaults: {}, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, [ + "openai/gpt-5.5", + "anthropic/claude-sonnet-4-6", + ]); + expect(next.agents?.defaults?.model).toEqual({ + fallbacks: ["anthropic/claude-sonnet-4-6"], + }); + expect(next.agents?.defaults?.model).not.toHaveProperty("primary"); + }); + it("keeps existing fallbacks when the primary is not selected", () => { const config = { agents: { diff --git a/src/commands/models/auth.test.ts b/src/commands/models/auth.test.ts index 173ddba7c63..cf3dd1c3e7a 100644 --- a/src/commands/models/auth.test.ts +++ b/src/commands/models/auth.test.ts @@ -614,6 +614,29 @@ describe("modelsAuthLoginCommand", () => { }); }); + it("overwrites an existing primary when login uses --set-default", async () => { + const runtime = createRuntime(); + currentConfig = { + agents: { + defaults: { + model: { primary: "anthropic/claude-opus-4-6" }, + models: { "anthropic/claude-opus-4-6": {} }, + }, + }, + }; + + await modelsAuthLoginCommand({ provider: "openai-codex", setDefault: true }, runtime); + + expect(lastUpdatedConfig?.agents?.defaults?.model).toEqual({ + primary: "openai-codex/gpt-5.5", + }); + expect(lastUpdatedConfig?.agents?.defaults?.models).toEqual({ + "anthropic/claude-opus-4-6": {}, + "openai-codex/gpt-5.5": {}, + }); + expect(runtime.log).toHaveBeenCalledWith("Default model set to openai-codex/gpt-5.5"); + }); + it("survives lockout clearing failure without blocking login", async () => { const runtime = createRuntime(); mocks.loadAuthProfileStoreForRuntime.mockImplementation(() => { diff --git a/src/flows/model-picker.ts b/src/flows/model-picker.ts index af6ea1590a8..f4f84e7d54a 100644 --- a/src/flows/model-picker.ts +++ b/src/flows/model-picker.ts @@ -847,7 +847,7 @@ export function applyModelFallbacksFromSelection( ...defaults, model: { ...(typeof existingModel === "object" ? existingModel : undefined), - primary: existingPrimary ?? resolvedKey, + ...(existingPrimary != null ? { primary: existingPrimary } : {}), fallbacks, }, }, diff --git a/src/plugins/provider-auth-choice-helpers.test.ts b/src/plugins/provider-auth-choice-helpers.test.ts index c1f28b1a791..141adc2b210 100644 --- a/src/plugins/provider-auth-choice-helpers.test.ts +++ b/src/plugins/provider-auth-choice-helpers.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { applyProviderAuthConfigPatch } from "./provider-auth-choice-helpers.js"; +import { applyDefaultModel, applyProviderAuthConfigPatch } from "./provider-auth-choice-helpers.js"; describe("applyProviderAuthConfigPatch", () => { const base = { @@ -102,3 +102,74 @@ describe("applyProviderAuthConfigPatch", () => { }); }); }); + +describe("applyDefaultModel", () => { + it("sets the primary when none exists", () => { + const config = { + agents: { defaults: {} }, + } as OpenClawConfig; + const next = applyDefaultModel(config, "openrouter/auto"); + expect(next.agents?.defaults?.model).toEqual({ primary: "openrouter/auto" }); + }); + + it("overwrites an existing primary by default", () => { + const config = { + agents: { + defaults: { + model: { primary: "anthropic/claude-opus-4-6" }, + }, + }, + } as OpenClawConfig; + const next = applyDefaultModel(config, "openrouter/auto"); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openrouter/auto", + }); + }); + + it("preserves an existing primary when requested", () => { + const config = { + agents: { + defaults: { + model: { primary: "anthropic/claude-opus-4-6" }, + }, + }, + } as OpenClawConfig; + const next = applyDefaultModel(config, "openrouter/auto", { + preserveExistingPrimary: true, + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "anthropic/claude-opus-4-6", + }); + }); + + it("preserves an existing primary and keeps fallbacks", () => { + const config = { + agents: { + defaults: { + model: { + primary: "anthropic/claude-opus-4-6", + fallbacks: ["openai/gpt-5.4"], + }, + }, + }, + } as OpenClawConfig; + const next = applyDefaultModel(config, "openrouter/auto", { + preserveExistingPrimary: true, + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "anthropic/claude-opus-4-6", + fallbacks: ["openai/gpt-5.4"], + }); + }); + + it("adds the model to the allowlist", () => { + const config = { + agents: { defaults: { models: { "anthropic/claude-sonnet-4-6": {} } } }, + } as OpenClawConfig; + const next = applyDefaultModel(config, "openrouter/auto"); + expect(next.agents?.defaults?.models).toEqual({ + "anthropic/claude-sonnet-4-6": {}, + "openrouter/auto": {}, + }); + }); +}); diff --git a/src/plugins/provider-auth-choice-helpers.ts b/src/plugins/provider-auth-choice-helpers.ts index 4e1623b4f96..92e2e0dfa82 100644 --- a/src/plugins/provider-auth-choice-helpers.ts +++ b/src/plugins/provider-auth-choice-helpers.ts @@ -119,11 +119,21 @@ export function applyProviderAuthConfigPatch( }; } -export function applyDefaultModel(cfg: OpenClawConfig, model: string): OpenClawConfig { +export function applyDefaultModel( + cfg: OpenClawConfig, + model: string, + opts?: { preserveExistingPrimary?: boolean }, +): OpenClawConfig { const models = { ...cfg.agents?.defaults?.models }; models[model] = models[model] ?? {}; const existingModel = cfg.agents?.defaults?.model; + const existingPrimary = + typeof existingModel === "string" + ? existingModel + : existingModel && typeof existingModel === "object" + ? (existingModel as { primary?: string }).primary + : undefined; return { ...cfg, agents: { @@ -135,7 +145,7 @@ export function applyDefaultModel(cfg: OpenClawConfig, model: string): OpenClawC ...(existingModel && typeof existingModel === "object" && "fallbacks" in existingModel ? { fallbacks: (existingModel as { fallbacks?: string[] }).fallbacks } : undefined), - primary: model, + primary: opts?.preserveExistingPrimary === true ? (existingPrimary ?? model) : model, }, }, }, diff --git a/src/plugins/provider-auth-choice.ts b/src/plugins/provider-auth-choice.ts index 05388760d56..1a758603d39 100644 --- a/src/plugins/provider-auth-choice.ts +++ b/src/plugins/provider-auth-choice.ts @@ -33,6 +33,7 @@ export type ApplyProviderAuthChoiceParams = { runtime: RuntimeEnv; agentDir?: string; setDefaultModel: boolean; + preserveExistingDefaultModel?: boolean; agentId?: string; opts?: Partial; }; @@ -83,6 +84,65 @@ function restoreConfiguredPrimaryModel( }; } +function resolveConfiguredDefaultModelPrimary(cfg: OpenClawConfig): string | undefined { + const model = cfg.agents?.defaults?.model; + if (typeof model === "string") { + return model; + } + if (model && typeof model === "object" && typeof model.primary === "string") { + return model.primary; + } + return undefined; +} + +async function noteDefaultModelResult(params: { + previousPrimary: string | undefined; + selectedModel: string; + preserveExistingDefaultModel: boolean | undefined; + prompter: WizardPrompter; +}): Promise { + if ( + params.preserveExistingDefaultModel === true && + params.previousPrimary && + params.previousPrimary !== params.selectedModel + ) { + await params.prompter.note( + `Kept existing default model ${params.previousPrimary}; ${params.selectedModel} is available.`, + "Model configured", + ); + return; + } + + await params.prompter.note(`Default model set to ${params.selectedModel}`, "Model configured"); +} + +async function applyDefaultModelFromAuthChoice(params: { + config: OpenClawConfig; + selectedModel: string; + preserveExistingDefaultModel: boolean | undefined; + prompter: WizardPrompter; + runSelectedModelHook: (config: OpenClawConfig) => Promise; +}): Promise { + const previousPrimary = resolveConfiguredDefaultModelPrimary(params.config); + const preservesDifferentPrimary = + params.preserveExistingDefaultModel === true && + previousPrimary !== undefined && + previousPrimary !== params.selectedModel; + const nextConfig = applyDefaultModel(params.config, params.selectedModel, { + preserveExistingPrimary: params.preserveExistingDefaultModel === true, + }); + if (!preservesDifferentPrimary) { + await params.runSelectedModelHook(nextConfig); + } + await noteDefaultModelResult({ + previousPrimary, + selectedModel: params.selectedModel, + preserveExistingDefaultModel: params.preserveExistingDefaultModel, + prompter: params.prompter, + }); + return nextConfig; +} + type ProviderAuthChoiceRuntime = typeof import("./provider-auth-choice.runtime.js"); const defaultProviderAuthChoiceDeps = { @@ -281,23 +341,27 @@ export async function applyAuthChoiceLoadedPluginProvider( nextConfig = applied.config; let agentModelOverride: string | undefined; if (applied.defaultModel) { + const selectedModel = applied.defaultModel; if (params.setDefaultModel) { - nextConfig = applyDefaultModel(nextConfig, applied.defaultModel); - await runProviderModelSelectedHook({ + nextConfig = await applyDefaultModelFromAuthChoice({ config: nextConfig, - model: applied.defaultModel, + selectedModel, + preserveExistingDefaultModel: params.preserveExistingDefaultModel, prompter: params.prompter, - agentDir: params.agentDir, - workspaceDir, + runSelectedModelHook: async (config) => { + await runProviderModelSelectedHook({ + config, + model: selectedModel, + prompter: params.prompter, + agentDir: params.agentDir, + workspaceDir, + }); + }, }); - await params.prompter.note( - `Default model set to ${applied.defaultModel}`, - "Model configured", - ); return { config: nextConfig }; } nextConfig = restoreConfiguredPrimaryModel(nextConfig, params.config); - agentModelOverride = applied.defaultModel; + agentModelOverride = selectedModel; } return { config: nextConfig, agentModelOverride }; @@ -368,29 +432,33 @@ export async function applyAuthChoicePluginProvider( nextConfig = applied.config; if (applied.defaultModel) { + const selectedModel = applied.defaultModel; if (params.setDefaultModel) { - nextConfig = applyDefaultModel(nextConfig, applied.defaultModel); - await runProviderModelSelectedHook({ + nextConfig = await applyDefaultModelFromAuthChoice({ config: nextConfig, - model: applied.defaultModel, + selectedModel, + preserveExistingDefaultModel: params.preserveExistingDefaultModel, prompter: params.prompter, - agentDir, - workspaceDir, + runSelectedModelHook: async (config) => { + await runProviderModelSelectedHook({ + config, + model: selectedModel, + prompter: params.prompter, + agentDir, + workspaceDir, + }); + }, }); - await params.prompter.note( - `Default model set to ${applied.defaultModel}`, - "Model configured", - ); return { config: nextConfig }; } if (params.agentId) { await params.prompter.note( - `Default model set to ${applied.defaultModel} for agent "${params.agentId}".`, + `Default model set to ${selectedModel} for agent "${params.agentId}".`, "Model configured", ); } nextConfig = restoreConfiguredPrimaryModel(nextConfig, params.config); - return { config: nextConfig, agentModelOverride: applied.defaultModel }; + return { config: nextConfig, agentModelOverride: selectedModel }; } return { config: nextConfig };