From 1377baee1a0466bd723ee3dae7311890b190e864 Mon Sep 17 00:00:00 2001 From: Ruben Cuevas Date: Sat, 25 Apr 2026 07:56:42 -0400 Subject: [PATCH] fix(configure): clear deselected model fallbacks --- ...re.gateway-auth.prompt-auth-config.test.ts | 46 +++ src/commands/configure.gateway-auth.ts | 4 +- src/commands/model-picker.test.ts | 353 ++++++++++++++++++ src/flows/model-picker.ts | 203 +++++++++- 4 files changed, 587 insertions(+), 19 deletions(-) diff --git a/src/commands/configure.gateway-auth.prompt-auth-config.test.ts b/src/commands/configure.gateway-auth.prompt-auth-config.test.ts index 54c49ab45d2..466655958b4 100644 --- a/src/commands/configure.gateway-auth.prompt-auth-config.test.ts +++ b/src/commands/configure.gateway-auth.prompt-auth-config.test.ts @@ -198,6 +198,52 @@ describe("promptAuthConfig", () => { }); }); + it("resolves fallback aliases before scoped allowlist pruning", async () => { + vi.clearAllMocks(); + mocks.promptAuthChoiceGrouped.mockResolvedValue("token"); + mocks.applyAuthChoice.mockResolvedValue({ + config: { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["mini"], + }, + models: { + "openai/gpt-5.5": { alias: "GPT" }, + "openai/gpt-5.4-mini": { alias: "mini" }, + "anthropic/claude-sonnet-4-6": { alias: "Sonnet" }, + }, + }, + }, + }, + }); + mocks.promptModelAllowlist.mockResolvedValue({ + models: ["openai/gpt-5.5"], + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4-mini"], + }); + mocks.resolveProviderPluginChoice.mockReturnValue({ + provider: { id: "openai", label: "OpenAI", auth: [] }, + method: { id: "setup-token", label: "setup-token", kind: "token" }, + wizard: { + modelAllowlist: { + allowedKeys: ["openai/gpt-5.5", "openai/gpt-5.4-mini"], + initialSelections: ["openai/gpt-5.5"], + }, + }, + }); + + const result = await promptAuthConfig({}, makeRuntime(), noopPrompter); + + expect(result.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + }); + expect(result.agents?.defaults?.models).toEqual({ + "openai/gpt-5.5": { alias: "GPT" }, + "anthropic/claude-sonnet-4-6": { alias: "Sonnet" }, + }); + }); + it("scopes the allowlist picker to the selected provider when available", async () => { mocks.promptAuthChoiceGrouped.mockResolvedValue("openai-api-key"); mocks.resolvePreferredProviderForAuthChoice.mockResolvedValue("openai"); diff --git a/src/commands/configure.gateway-auth.ts b/src/commands/configure.gateway-auth.ts index 62ce30169e3..7ce8e3f7b76 100644 --- a/src/commands/configure.gateway-auth.ts +++ b/src/commands/configure.gateway-auth.ts @@ -177,10 +177,12 @@ export async function promptAuthConfig( preferredProvider, }); if (allowlistSelection.models) { + next = applyModelFallbacksFromSelection(next, allowlistSelection.models, { + scopeKeys: allowlistSelection.scopeKeys, + }); next = applyModelAllowlist(next, allowlistSelection.models, { scopeKeys: allowlistSelection.scopeKeys, }); - next = applyModelFallbacksFromSelection(next, allowlistSelection.models); } } diff --git a/src/commands/model-picker.test.ts b/src/commands/model-picker.test.ts index 5869b83e90b..93f63f95a16 100644 --- a/src/commands/model-picker.test.ts +++ b/src/commands/model-picker.test.ts @@ -434,6 +434,179 @@ describe("promptModelAllowlist", () => { "openai/gpt-5.4-mini", ]); }); + + it("seeds existing model fallbacks into unscoped allowlist selections", async () => { + loadModelCatalog.mockResolvedValue([ + { + provider: "openai", + id: "gpt-5.5", + name: "GPT-5.5", + }, + ]); + + const multiselect = vi.fn(async (params) => params.initialValues ?? []); + const prompter = makePrompter({ multiselect }); + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["anthropic/claude-sonnet-4-6"], + }, + models: { + "openai/gpt-5.5": { alias: "gpt" }, + }, + }, + }, + } as OpenClawConfig; + + const result = await promptModelAllowlist({ config, prompter }); + const call = multiselect.mock.calls[0]?.[0]; + expect(call?.options.map((option: { value: string }) => option.value)).toEqual([ + "openai/gpt-5.5", + "anthropic/claude-sonnet-4-6", + ]); + expect(call?.initialValues).toEqual(["openai/gpt-5.5", "anthropic/claude-sonnet-4-6"]); + expect(result.models).toEqual(["openai/gpt-5.5", "anthropic/claude-sonnet-4-6"]); + }); + + it("resolves bare fallback seeds against the primary model provider", async () => { + loadModelCatalog.mockResolvedValue([ + { + provider: "anthropic", + id: "claude-opus-4-6", + name: "Claude Opus 4.5", + }, + { + provider: "anthropic", + id: "claude-sonnet-4-6", + name: "Claude Sonnet 4.5", + }, + { + provider: "openai", + id: "claude-sonnet-4-6", + name: "Wrong provider", + }, + ]); + + const multiselect = vi.fn(async (params) => params.initialValues ?? []); + const prompter = makePrompter({ multiselect }); + const config = { + agents: { + defaults: { + model: { + primary: "anthropic/claude-opus-4-6", + fallbacks: ["claude-sonnet-4-6"], + }, + }, + }, + } as OpenClawConfig; + + const result = await promptModelAllowlist({ config, prompter }); + const call = multiselect.mock.calls[0]?.[0]; + + expect(call?.initialValues).toEqual([ + "anthropic/claude-opus-4-6", + "anthropic/claude-sonnet-4-6", + ]); + expect(result.models).toEqual(["anthropic/claude-opus-4-6", "anthropic/claude-sonnet-4-6"]); + }); + + it("keeps the no-catalog allowlist prompt blank when no allowlist exists", async () => { + loadModelCatalog.mockResolvedValue([]); + + const text = vi.fn(async (params) => params.initialValue ?? ""); + const prompter = makePrompter({ text }); + const config = { + agents: { + defaults: { + model: "openai/gpt-5.5", + }, + }, + } as OpenClawConfig; + + const result = await promptModelAllowlist({ config, prompter }); + + expect(text.mock.calls[0]?.[0]?.initialValue).toBe(""); + expect(result).toEqual({}); + }); + + it("shows existing fallbacks in the no-catalog allowlist prompt when an allowlist exists", async () => { + loadModelCatalog.mockResolvedValue([]); + + const text = vi.fn(async (params) => params.initialValue ?? ""); + const prompter = makePrompter({ text }); + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["anthropic/claude-sonnet-4-6"], + }, + models: { + "openai/gpt-5.5": { alias: "gpt" }, + }, + }, + }, + } as OpenClawConfig; + + const result = await promptModelAllowlist({ config, prompter }); + + expect(text.mock.calls[0]?.[0]?.initialValue).toBe( + "openai/gpt-5.5, anthropic/claude-sonnet-4-6", + ); + expect(result.models).toEqual(["openai/gpt-5.5", "anthropic/claude-sonnet-4-6"]); + }); + + it("keeps provider-scoped fallback supplements within scope", async () => { + loadModelCatalog.mockResolvedValue([ + { + provider: "openai", + id: "gpt-5.5", + name: "GPT-5.5", + }, + { + provider: "openai", + id: "gpt-5.4", + name: "GPT-5.4", + }, + { + provider: "anthropic", + id: "claude-sonnet-4-6", + name: "Claude Sonnet 4.5", + }, + ]); + + const multiselect = vi.fn(async (params) => params.initialValues ?? []); + const prompter = makePrompter({ multiselect }); + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["anthropic/claude-sonnet-4-6"], + }, + }, + }, + } as OpenClawConfig; + + const result = await promptModelAllowlist({ + config, + prompter, + preferredProvider: "openai", + }); + + const call = multiselect.mock.calls[0]?.[0]; + expect(call?.options.map((option: { value: string }) => option.value)).toEqual([ + "openai/gpt-5.5", + "openai/gpt-5.4", + ]); + expect(call?.initialValues).toEqual(["openai/gpt-5.5"]); + expect(result).toEqual({ + models: ["openai/gpt-5.5"], + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4"], + }); + }); }); describe("runtime model picker visibility", () => { @@ -601,6 +774,186 @@ describe("applyModelFallbacksFromSelection", () => { expect(next.agents?.defaults?.model).not.toHaveProperty("primary"); }); + it("does not write an empty model object for singleton default selections", () => { + const config = { + agents: { + defaults: {}, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5"]); + expect(next).toBe(config); + }); + + it("clears existing fallbacks when only the primary remains selected", () => { + const config = { + agents: { + defaults: { + model: { + primary: "anthropic/claude-opus-4-6", + fallbacks: ["anthropic/claude-sonnet-4-6"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["anthropic/claude-opus-4-6"]); + expect(next.agents?.defaults?.model).toEqual({ + primary: "anthropic/claude-opus-4-6", + }); + }); + + it("drops malformed fallback refs instead of preserving raw strings", () => { + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["openai/"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5"]); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + }); + }); + + it("preserves hidden fallbacks during unscoped selections", () => { + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["claude-cli/claude-sonnet-4-6", "anthropic/claude-sonnet-4-6"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5"]); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + fallbacks: ["claude-cli/claude-sonnet-4-6"], + }); + }); + + it("preserves out-of-scope fallbacks during scoped selections", () => { + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["openai/gpt-5.4", "anthropic/claude-sonnet-4-6"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5"], { + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4"], + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + fallbacks: ["anthropic/claude-sonnet-4-6"], + }); + }); + + it("removes scoped fallbacks for empty scoped selections", () => { + const config = { + agents: { + defaults: { + model: { + primary: "anthropic/claude-opus-4-6", + fallbacks: ["openai/gpt-5.5", "google/gemini-3-pro-preview"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, [], { + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4"], + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "anthropic/claude-opus-4-6", + fallbacks: ["google/gemini-3-pro-preview"], + }); + }); + + it("does not add new scoped fallbacks when the primary is outside scope", () => { + const config = { + agents: { + defaults: { + model: { + primary: "anthropic/claude-opus-4-6", + fallbacks: ["openai/gpt-5.5"], + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5", "openai/gpt-5.4"], { + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4"], + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "anthropic/claude-opus-4-6", + fallbacks: ["openai/gpt-5.5"], + }); + }); + + it("removes existing scoped fallback aliases when deselected", () => { + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["mini"], + }, + models: { + "openai/gpt-5.4-mini": { alias: "mini" }, + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection(config, ["openai/gpt-5.5"], { + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4-mini"], + }); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + }); + }); + + it("canonicalizes existing scoped fallback aliases when kept selected", () => { + const config = { + agents: { + defaults: { + model: { + primary: "openai/gpt-5.5", + fallbacks: ["mini"], + }, + models: { + "openai/gpt-5.4-mini": { alias: "mini" }, + }, + }, + }, + } as OpenClawConfig; + + const next = applyModelFallbacksFromSelection( + config, + ["openai/gpt-5.5", "openai/gpt-5.4-mini"], + { + scopeKeys: ["openai/gpt-5.5", "openai/gpt-5.4-mini"], + }, + ); + expect(next.agents?.defaults?.model).toEqual({ + primary: "openai/gpt-5.5", + fallbacks: ["openai/gpt-5.4-mini"], + }); + }); + it("keeps existing fallbacks when the primary is not selected", () => { const config = { agents: { diff --git a/src/flows/model-picker.ts b/src/flows/model-picker.ts index f4f84e7d54a..7bb6d951392 100644 --- a/src/flows/model-picker.ts +++ b/src/flows/model-picker.ts @@ -9,12 +9,17 @@ import { import { buildAllowedModelSet, buildModelAliasIndex, + type ModelAliasIndex, modelKey, normalizeProviderId, resolveConfiguredModelRef, + resolveModelRefFromString, } from "../agents/model-selection.js"; import { formatTokenK } from "../commands/models/shared.js"; -import { resolveAgentModelPrimaryValue } from "../config/model-input.js"; +import { + resolveAgentModelFallbackValues, + resolveAgentModelPrimaryValue, +} from "../config/model-input.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { applyPrimaryModel } from "../plugins/provider-model-primary.js"; import { resolveOwningPluginIdsForProvider } from "../plugins/providers.js"; @@ -121,6 +126,48 @@ function normalizeModelKeys(values: string[]): string[] { return next; } +function resolveFallbackModelKey(params: { + cfg: OpenClawConfig; + raw: string; + defaultProvider: string; + aliasIndex: ModelAliasIndex; +}): string | undefined { + const raw = normalizeOptionalString(params.raw); + if (!raw) { + return undefined; + } + const resolved = resolveModelRefFromString({ + cfg: params.cfg, + raw, + defaultProvider: params.defaultProvider, + aliasIndex: params.aliasIndex, + }); + if (!resolved) { + return undefined; + } + return modelKey(resolved.ref.provider, resolved.ref.model); +} + +function resolveFallbackModelKeys(params: { + cfg: OpenClawConfig; + rawFallbacks: string[]; + defaultProvider: string; + aliasIndex: ModelAliasIndex; +}): string[] { + return normalizeModelKeys( + params.rawFallbacks + .map((raw) => + resolveFallbackModelKey({ + cfg: params.cfg, + raw, + defaultProvider: params.defaultProvider, + aliasIndex: params.aliasIndex, + }), + ) + .filter((key): key is string => Boolean(key)), + ); +} + function resolveModelRouteHint(provider: string): string | undefined { const normalized = normalizeProviderId(provider); if (normalized === "openai") { @@ -615,14 +662,29 @@ export async function promptModelAllowlist(params: { defaultModel: DEFAULT_MODEL, }); const resolvedKey = modelKey(resolved.provider, resolved.model); + const aliasIndex = buildModelAliasIndex({ + cfg, + defaultProvider: DEFAULT_PROVIDER, + }); + const fallbackAliasIndex = + resolved.provider === DEFAULT_PROVIDER + ? aliasIndex + : buildModelAliasIndex({ + cfg, + defaultProvider: resolved.provider, + }); + const fallbackKeys = resolveFallbackModelKeys({ + cfg, + rawFallbacks: resolveAgentModelFallbackValues(cfg.agents?.defaults?.model), + defaultProvider: resolved.provider, + aliasIndex: fallbackAliasIndex, + }); const initialSeeds = normalizeModelKeys([ ...existingKeys, resolvedKey, + ...fallbackKeys, ...(params.initialSelections ?? []), ]); - const initialKeys = allowedKeySet - ? initialSeeds.filter((key) => allowedKeySet.has(key)) - : initialSeeds.filter(isModelPickerVisibleModelRef); const allowlistProgress = params.prompter.progress("Loading available models"); let catalog: Awaited>; @@ -632,11 +694,13 @@ export async function promptModelAllowlist(params: { allowlistProgress.stop(); } if (catalog.length === 0 && allowedKeys.length === 0) { + const noCatalogInitialKeys = + existingKeys.length > 0 ? normalizeModelKeys([...existingKeys, ...fallbackKeys]) : []; const raw = await params.prompter.text({ message: params.message ?? "Allowlist models (comma-separated provider/model; blank to keep current)", - initialValue: existingKeys.join(", "), + initialValue: noCatalogInitialKeys.join(", "), placeholder: "provider/model, other-provider/model", }); const parsed = (raw ?? "") @@ -649,10 +713,6 @@ export async function promptModelAllowlist(params: { return { models: normalizeModelKeys(parsed) }; } - const aliasIndex = buildModelAliasIndex({ - cfg, - defaultProvider: DEFAULT_PROVIDER, - }); const hasAuth = createProviderAuthChecker({ cfg, agentDir: params.agentDir }); const matchesPreferredProvider = preferredProvider ? createPreferredProviderMatcher({ @@ -678,12 +738,20 @@ export async function promptModelAllowlist(params: { : preferredProvider ? filteredCatalog.map((entry) => modelKey(entry.provider, entry.id)) : undefined; + const scopeKeySet = scopeKeys ? new Set(scopeKeys) : null; + const selectableInitialSeeds = + scopeKeySet && !allowedKeySet + ? initialSeeds.filter((key) => scopeKeySet.has(key)) + : initialSeeds; + const initialKeys = allowedKeySet + ? initialSeeds.filter((key) => allowedKeySet.has(key)) + : selectableInitialSeeds.filter(isModelPickerVisibleModelRef); for (const entry of filteredCatalog) { addModelSelectOption({ entry, options, seen, aliasIndex, hasAuth }); } - const supplementalKeys = (allowedKeySet ? allowedKeys : existingKeys).filter( + const supplementalKeys = (allowedKeySet ? allowedKeys : selectableInitialSeeds).filter( isModelPickerVisibleModelRef, ); for (const key of supplementalKeys) { @@ -813,9 +881,12 @@ export function applyModelAllowlist( export function applyModelFallbacksFromSelection( cfg: OpenClawConfig, selection: string[], + opts: { scopeKeys?: string[] } = {}, ): OpenClawConfig { const normalized = normalizeModelKeys(selection); - if (normalized.length <= 1) { + const scopeKeys = opts.scopeKeys ? normalizeModelKeys(opts.scopeKeys) : []; + const scopeKeySet = scopeKeys.length > 0 ? new Set(scopeKeys) : null; + if (normalized.length === 0 && !scopeKeySet) { return cfg; } @@ -825,7 +896,8 @@ export function applyModelFallbacksFromSelection( defaultModel: DEFAULT_MODEL, }); const resolvedKey = modelKey(resolved.provider, resolved.model); - if (!normalized.includes(resolvedKey)) { + const includesResolvedPrimary = normalized.includes(resolvedKey); + if (!includesResolvedPrimary && !scopeKeySet) { return cfg; } @@ -837,20 +909,115 @@ export function applyModelFallbacksFromSelection( : existingModel && typeof existingModel === "object" ? existingModel.primary : undefined; + const preservedModelFields = + existingModel && typeof existingModel === "object" + ? (({ fallbacks: _oldFallbacks, ...rest }) => rest)(existingModel) + : {}; - const fallbacks = normalized.filter((key) => key !== resolvedKey); + const aliasIndex = buildModelAliasIndex({ + cfg, + defaultProvider: resolved.provider, + }); + const existingFallbacks = + existingModel && typeof existingModel === "object" && Array.isArray(existingModel.fallbacks) + ? resolveFallbackModelKeys({ + cfg, + rawFallbacks: existingModel.fallbacks, + defaultProvider: resolved.provider, + aliasIndex, + }) + : []; + const existingFallbackSet = new Set(existingFallbacks); + const rawSelectedFallbacks = normalized.filter((key) => key !== resolvedKey); + const selectedFallbacks = + scopeKeySet && !includesResolvedPrimary + ? rawSelectedFallbacks.filter((key) => existingFallbackSet.has(key)) + : rawSelectedFallbacks; + const fallbacks = scopeKeySet + ? mergeScopedFallbackSelection({ + existingFallbacks, + selectedFallbacks, + scopeKeySet, + }) + : mergeUnscopedFallbackSelection({ + existingFallbacks, + selectedFallbacks, + }); + const nextModel = { + ...preservedModelFields, + ...(existingPrimary != null ? { primary: existingPrimary } : {}), + ...(fallbacks.length > 0 ? { fallbacks } : {}), + }; + if (Object.keys(nextModel).length === 0) { + if (!defaults || !Object.hasOwn(defaults, "model")) { + return cfg; + } + const { model: _ignoredModel, ...restDefaults } = defaults; + return { + ...cfg, + agents: { + ...cfg.agents, + defaults: restDefaults, + }, + }; + } return { ...cfg, agents: { ...cfg.agents, defaults: { ...defaults, - model: { - ...(typeof existingModel === "object" ? existingModel : undefined), - ...(existingPrimary != null ? { primary: existingPrimary } : {}), - fallbacks, - }, + model: nextModel, }, }, }; } + +function mergeScopedFallbackSelection(params: { + existingFallbacks: string[]; + selectedFallbacks: string[]; + scopeKeySet: Set; +}): string[] { + const selected = new Set(params.selectedFallbacks); + const fallbacks: string[] = []; + for (const fallback of params.existingFallbacks) { + if (!params.scopeKeySet.has(fallback)) { + fallbacks.push(fallback); + continue; + } + if (selected.delete(fallback)) { + fallbacks.push(fallback); + } + } + for (const fallback of params.selectedFallbacks) { + if (selected.has(fallback)) { + fallbacks.push(fallback); + } + } + return fallbacks; +} + +function mergeUnscopedFallbackSelection(params: { + existingFallbacks: string[]; + selectedFallbacks: string[]; +}): string[] { + const selected = new Set(params.selectedFallbacks); + const fallbacks: string[] = []; + for (const fallback of params.existingFallbacks) { + if (!isModelPickerVisibleModelRef(fallback)) { + fallbacks.push(fallback); + // Defensive: future callers may pass a hidden fallback in the selected list. + selected.delete(fallback); + continue; + } + if (selected.delete(fallback)) { + fallbacks.push(fallback); + } + } + for (const fallback of params.selectedFallbacks) { + if (selected.has(fallback)) { + fallbacks.push(fallback); + } + } + return fallbacks; +}