From 622eec9c364fe79dc623f6ad7408e0b7bec71fbe Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 14 Aug 2026 21:14:50 -0700 Subject: [PATCH] fix(ui): stage every gateway-honored agent model shape instead of silently dropping input (#123995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agents-page fallback editor gated staging on a resolvable primary model, but the gateway resolver honors { fallbacks } with no primary (agent-scope.ts, including the explicit empty-array-disables-global contract). With a fully implicit default model, typing a fallback chip cleared the input and staged nothing — no chip, no error, no write. Clearing the primary had the sibling bug: removeFormValue deleted the whole model node including authored agent fallbacks. One stageModelShape owner now writes the smallest representable shape (bare string, { primary, fallbacks }, { fallbacks }, or removal), and both entry points share existingModelParts instead of duplicating existing-shape probing. --- ui/src/pages/agents/model-config.test.ts | 63 ++++++++++++++++ ui/src/pages/agents/model-config.ts | 92 +++++++++++++----------- 2 files changed, 114 insertions(+), 41 deletions(-) diff --git a/ui/src/pages/agents/model-config.test.ts b/ui/src/pages/agents/model-config.test.ts index f69eca9a11c7..f86a97d44b53 100644 --- a/ui/src/pages/agents/model-config.test.ts +++ b/ui/src/pages/agents/model-config.test.ts @@ -60,4 +60,67 @@ describe("agent model config", () => { expect(runtimeConfig.state.configForm?.agents).not.toHaveProperty("list"); runtimeConfig.dispose(); }); + + it("stages fallbacks without a primary when no model is authored anywhere", async () => { + // Fully implicit default model: typing a fallback chip must stage the + // { fallbacks }-only shape the gateway honors, not silently no-op. + const runtimeConfig = createRuntimeConfig({ + agents: { entries: { main: { default: true } } }, + }); + await runtimeConfig.ensureLoaded(); + + stageAgentModelFallbacks(runtimeConfig, "main", ["openai/gpt-5.4"]); + + expect(runtimeConfig.state.configForm).toEqual({ + agents: { + entries: { + main: { default: true, model: { fallbacks: ["openai/gpt-5.4"] } }, + }, + }, + }); + runtimeConfig.dispose(); + }); + + it("keeps authored fallbacks when the primary model is cleared", async () => { + const runtimeConfig = createRuntimeConfig({ + agents: { + defaults: { model: { primary: "openai/gpt-5.4" } }, + entries: { + main: { + default: true, + model: { primary: "anthropic/claude-sonnet-4-6", fallbacks: ["openai/gpt-5.4"] }, + }, + }, + }, + }); + await runtimeConfig.ensureLoaded(); + + stageAgentPrimaryModel(runtimeConfig, "main", null); + + expect(runtimeConfig.state.configForm).toEqual({ + agents: { + defaults: { model: { primary: "openai/gpt-5.4" } }, + entries: { + main: { default: true, model: { fallbacks: ["openai/gpt-5.4"] } }, + }, + }, + }); + runtimeConfig.dispose(); + }); + + it("still removes the model node when clearing a primary with no fallbacks", async () => { + const runtimeConfig = createRuntimeConfig({ + agents: { + entries: { main: { default: true, model: "anthropic/claude-sonnet-4-6" } }, + }, + }); + await runtimeConfig.ensureLoaded(); + + stageAgentPrimaryModel(runtimeConfig, "main", null); + + expect(runtimeConfig.state.configForm).toEqual({ + agents: { entries: { main: { default: true } } }, + }); + runtimeConfig.dispose(); + }); }); diff --git a/ui/src/pages/agents/model-config.ts b/ui/src/pages/agents/model-config.ts index 36a49a1bc53f..96e40fc51ba0 100644 --- a/ui/src/pages/agents/model-config.ts +++ b/ui/src/pages/agents/model-config.ts @@ -21,6 +21,44 @@ function modelEntry(target: AgentConfigEntryTarget) { }; } +// Stage the smallest config shape that expresses the selection. The gateway +// resolver honors a bare string, { primary, fallbacks }, and { fallbacks } +// with no primary (agent-scope.ts); staging must write all three or an +// authored piece of the selection silently disappears. +function stageModelShape( + runtimeConfig: RuntimeConfig, + path: Array, + primary: string | null, + fallbacks: string[] | null, +) { + if (primary && fallbacks) { + runtimeConfig.patchForm(path, { primary, fallbacks }); + } else if (primary) { + runtimeConfig.patchForm(path, primary); + } else if (fallbacks) { + runtimeConfig.patchForm(path, { fallbacks }); + } else { + runtimeConfig.removeFormValue(path); + } +} + +function existingModelParts(existing: unknown): { + primary: string | null; + fallbacks: string[] | null; +} { + if (typeof existing === "string") { + return { primary: existing.trim() || null, fallbacks: null }; + } + if (existing && typeof existing === "object") { + const record = existing as { primary?: unknown; fallbacks?: unknown }; + return { + primary: typeof record.primary === "string" ? record.primary.trim() || null : null, + fallbacks: Array.isArray(record.fallbacks) ? (record.fallbacks as string[]) : null, + }; + } + return { primary: null, fallbacks: null }; +} + /** Stage a primary-model change; clearing falls back to the inherited default. */ export function stageAgentPrimaryModel( runtimeConfig: RuntimeConfig, @@ -32,17 +70,9 @@ export function stageAgentPrimaryModel( return; } const entry = modelEntry(target); - if (!modelId) { - runtimeConfig.removeFormValue(entry.path); - } else if (entry.existing && typeof entry.existing === "object") { - const fallbacks = (entry.existing as { fallbacks?: unknown }).fallbacks; - runtimeConfig.patchForm(entry.path, { - primary: modelId, - ...(Array.isArray(fallbacks) ? { fallbacks } : {}), - }); - } else { - runtimeConfig.patchForm(entry.path, modelId); - } + // Clearing the primary must not delete authored agent fallbacks: the + // { fallbacks }-only shape stays representable. + stageModelShape(runtimeConfig, entry.path, modelId, existingModelParts(entry.existing).fallbacks); } /** Stage fallback-list edits, preserving the effective primary model shape. */ @@ -54,40 +84,20 @@ export function stageAgentModelFallbacks( const config = currentConfigObject(runtimeConfig.state); const normalized = normalizeStringEntries(fallbacks); const resolved = resolveAgentConfig(config, agentId); - const primary = - resolveModelPrimary(resolved.entry?.model) ?? resolveModelPrimary(resolved.defaults?.model); const effective = resolveEffectiveModelFallbacks(resolved.entry?.model, resolved.defaults?.model); const existingTarget = runtimeConfig.agentEntry(agentId); - const target = - normalized.length > 0 - ? primary - ? (existingTarget ?? runtimeConfig.agentEntry(agentId, { ensure: true })) - : null - : (effective?.length ?? 0) > 0 || existingTarget - ? (existingTarget ?? runtimeConfig.agentEntry(agentId, { ensure: true })) - : null; + const mustWrite = normalized.length > 0 || (effective?.length ?? 0) > 0 || existingTarget; + const target = mustWrite + ? (existingTarget ?? runtimeConfig.agentEntry(agentId, { ensure: true })) + : null; if (!target) { return; } const entry = modelEntry(target); - const currentPrimary = - typeof entry.existing === "string" - ? entry.existing.trim() - : entry.existing && - typeof entry.existing === "object" && - typeof (entry.existing as { primary?: unknown }).primary === "string" - ? (entry.existing as { primary: string }).primary.trim() - : ""; - if (normalized.length === 0) { - if (currentPrimary || primary) { - runtimeConfig.patchForm(entry.path, currentPrimary || primary); - } else { - runtimeConfig.removeFormValue(entry.path); - } - } else if (currentPrimary || primary) { - runtimeConfig.patchForm(entry.path, { - primary: currentPrimary || primary, - fallbacks: normalized, - }); - } + const primary = + existingModelParts(entry.existing).primary ?? + resolveModelPrimary(resolved.entry?.model) ?? + resolveModelPrimary(resolved.defaults?.model) ?? + null; + stageModelShape(runtimeConfig, entry.path, primary, normalized.length > 0 ? normalized : null); }