mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 18:10:45 +00:00
fix: wizard no clobber model.primary on re-run
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
This commit is contained in:
committed by
Peter Steinberger
parent
d795000377
commit
8d57d745cf
@@ -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 <id> --set-default`
|
||||
or `openclaw models set <model>` 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
|
||||
|
||||
@@ -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 <id> --set-default` and
|
||||
`openclaw models set <model>` still replace `agents.defaults.model.primary`.
|
||||
|
||||
## "Model is not allowed" (and why replies stop)
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ export type ApplyAuthChoiceParams = {
|
||||
runtime: RuntimeEnv;
|
||||
agentDir?: string;
|
||||
setDefaultModel: boolean;
|
||||
preserveExistingDefaultModel?: boolean;
|
||||
agentId?: string;
|
||||
opts?: Partial<OnboardOptions>;
|
||||
};
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -152,6 +152,7 @@ export async function promptAuthConfig(
|
||||
prompter,
|
||||
runtime,
|
||||
setDefaultModel: true,
|
||||
preserveExistingDefaultModel: true,
|
||||
});
|
||||
next = applied.config;
|
||||
if (applied.retrySelection) {
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
@@ -847,7 +847,7 @@ export function applyModelFallbacksFromSelection(
|
||||
...defaults,
|
||||
model: {
|
||||
...(typeof existingModel === "object" ? existingModel : undefined),
|
||||
primary: existingPrimary ?? resolvedKey,
|
||||
...(existingPrimary != null ? { primary: existingPrimary } : {}),
|
||||
fallbacks,
|
||||
},
|
||||
},
|
||||
|
||||
@@ -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": {},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -33,6 +33,7 @@ export type ApplyProviderAuthChoiceParams = {
|
||||
runtime: RuntimeEnv;
|
||||
agentDir?: string;
|
||||
setDefaultModel: boolean;
|
||||
preserveExistingDefaultModel?: boolean;
|
||||
agentId?: string;
|
||||
opts?: Partial<ProviderAuthOptionBag>;
|
||||
};
|
||||
@@ -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<void> {
|
||||
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<void>;
|
||||
}): Promise<OpenClawConfig> {
|
||||
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 };
|
||||
|
||||
Reference in New Issue
Block a user