From 68e1e583e87b93b6a1eee4f0885410092caaa5dd Mon Sep 17 00:00:00 2001 From: bo Date: Wed, 29 Jul 2026 17:15:21 +0800 Subject: [PATCH] fix(settings): improve model configuration resilience --- .../features/SettingsDialog.interaction.tsx | 80 ++++++++++++- .../components/features/SettingsDialog.tsx | 50 ++++---- .../components/features/settings-helpers.ts | 30 +++++ .../components/features/settings-panels.tsx | 108 ++++++++++++++++-- docs/configuration.md | 1 + packages/agent-core/src/config/config.test.ts | 38 ++++++ packages/agent-core/src/config/provider.ts | 2 +- packages/agent-core/src/config/schema.ts | 4 +- .../src/config/server-config-service.test.ts | 42 ++++++- .../src/config/server-config-service.ts | 3 - .../src/models/model-runtime-snapshot.ts | 47 +++++--- .../models/model-selection-resolver.test.ts | 19 ++- 12 files changed, 366 insertions(+), 58 deletions(-) diff --git a/apps/web/src/components/features/SettingsDialog.interaction.tsx b/apps/web/src/components/features/SettingsDialog.interaction.tsx index 9b85d037..5f741304 100644 --- a/apps/web/src/components/features/SettingsDialog.interaction.tsx +++ b/apps/web/src/components/features/SettingsDialog.interaction.tsx @@ -54,6 +54,11 @@ function input(label: string, index = 0) { if (!element) throw new Error(`Missing input ${label}[${index}]`); return element; } +function modality(label: "Input modalities" | "Output modalities", value: "text" | "image" | "audio" | "video") { + const element = container.querySelector(`input[aria-label="${label}: ${value}"]`) as HTMLInputElement | null; + if (!element) throw new Error(`Missing modality ${label}: ${value}`); + return element; +} function change(element: HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement, value: string) { act(() => { const previous = element.value; @@ -102,6 +107,20 @@ beforeEach(() => installDom()); afterEach(() => { act(() => root.unmount()); dom.window.close(); }); describe("SettingsDialog interactions", () => { + test("keeps the apply notice and Settings workspace inside one bounded column", () => { + const withNotice = structuredClone(snapshot); + withNotice.restartRequiredSections = ["mcp"]; + act(() => root.render( {}} />)); + + const layout = container.querySelector("[data-settings-layout]") as HTMLElement; + const workspace = container.querySelector("[data-settings-workspace]") as HTMLElement; + expect(layout.className).toContain("flex-col"); + expect(layout.className).toContain("h-full"); + expect(workspace.className).toContain("flex-1"); + expect(workspace.className).toContain("min-h-0"); + expect(workspace.className).not.toContain("h-full"); + }); + test("opens the requested section and follows an external section change", () => { act(() => root.render( {}} section="profiles" />)); expect(container.textContent).toContain("Principal, deep, and fast model bindings"); @@ -216,8 +235,25 @@ describe("SettingsDialog interactions", () => { act(() => root.render( {}} />)); expect(input("Provider package").value).toBe("@ai-sdk/openai-compatible"); expect(input("Output limit").value).toBe("500"); - expect(input("Input modalities").value).toBe("text"); - expect(input("Output modalities").value).toBe("text"); + expect(modality("Input modalities", "text").checked).toBe(true); + expect(modality("Input modalities", "image").checked).toBe(false); + expect(modality("Output modalities", "text").checked).toBe(true); + expect(modality("Output modalities", "image").checked).toBe(false); + expect(modality("Input modalities", "text").disabled).toBe(true); + }); + + test("selects model modalities without comma-delimited text editing", () => { + act(() => root.render( {}} />)); + + act(() => modality("Input modalities", "image").click()); + expect(modality("Input modalities", "text").checked).toBe(true); + expect(modality("Input modalities", "text").disabled).toBe(false); + expect(modality("Input modalities", "image").checked).toBe(true); + + act(() => modality("Input modalities", "text").click()); + expect(modality("Input modalities", "text").checked).toBe(false); + expect(modality("Input modalities", "image").checked).toBe(true); + expect(modality("Input modalities", "image").disabled).toBe(true); }); test("selects packages from the server catalog and preserves adapter-specific advanced options", () => { @@ -263,7 +299,8 @@ describe("SettingsDialog interactions", () => { act(() => root.render( {}} />)); click("Add provider"); await act(async () => { click("Save changes"); await Promise.resolve(); }); - expect(container.textContent).toContain("Must be a URL"); + expect(container.querySelector("footer [role=\"alert\"]")?.textContent) + .toBe("Configuration validation failed: Must be a URL"); expect(container.textContent).toContain("provider-2"); }); @@ -305,7 +342,7 @@ describe("SettingsDialog interactions", () => { )) }); act(() => root.render( {}} />)); click("Memory"); - const enabled = container.querySelector('input[type="checkbox"]') as HTMLInputElement; + const enabled = container.querySelector('input[aria-label="Memory extraction"]') as HTMLInputElement; act(() => enabled.click()); await act(async () => { click("Save changes"); await Promise.resolve(); }); expect(container.textContent).toContain("Restart required for: Memory"); @@ -396,6 +433,41 @@ describe("SettingsDialog interactions", () => { expect([...(input("Variant") as HTMLSelectElement).querySelectorAll("option")].map((option) => option.value)).toContain("deep"); }); + test("saves missing Profile variant references and marks Profiles for attention", async () => { + const referenced = structuredClone(snapshot); + referenced.config.profiles.principal.variant = "fast"; + referenced.config.profiles.deep.variant = "fast"; + let request: Record | undefined; + Object.defineProperty(globalThis, "fetch", { configurable: true, value: mock(async (_url: string, init?: RequestInit) => { + request = JSON.parse(String(init?.body)); + const submitted = request?.config as typeof snapshot.config; + const response = successfulSaveResponse(); + response.config.provider.local.models.demo.variants = structuredClone(submitted.provider.local.models.demo.variants); + response.config.profiles = structuredClone(submitted.profiles); + return Response.json(response); + }) }); + act(() => root.render( {}} />)); + + change(input("Variants JSON"), JSON.stringify({ high: { temperature: 0.2 } })); + const profilesButton = [...container.querySelectorAll("button")].find((button) => button.textContent === "Profiles"); + expect(profilesButton?.getAttribute("data-invalid-count")).toBe("2"); + expect(profilesButton?.getAttribute("aria-label")).toBe("Profiles, 2 variant references need attention"); + + click("Profiles"); + expect(container.textContent).toContain('Variant "fast" no longer exists. This Profile is using the model default.'); + expect(input("Variant").getAttribute("aria-invalid")).toBe("true"); + expect(input("Variant").value).toBe("fast"); + const principalSummary = [...container.querySelectorAll("summary")].find((summary) => summary.textContent?.includes("principal")); + expect(principalSummary?.getAttribute("aria-label")).toBe('principal, local:demo, variant "fast" is missing; using model default'); + + await act(async () => { click("Save changes"); await Promise.resolve(); }); + const config = request?.config as typeof snapshot.config; + expect(config.profiles.principal.variant).toBe("fast"); + expect(config.profiles.deep.variant).toBe("fast"); + expect(profilesButton?.getAttribute("data-invalid-count")).toBe("2"); + expect(input("Variant").value).toBe("fast"); + }); + test("locks secret-bearing identities and still renames entries without preserved secrets", () => { act(() => root.render( {}} />)); expect((input("Provider ID") as HTMLInputElement).readOnly).toBe(true); diff --git a/apps/web/src/components/features/SettingsDialog.tsx b/apps/web/src/components/features/SettingsDialog.tsx index bf4ac736..1ab28832 100644 --- a/apps/web/src/components/features/SettingsDialog.tsx +++ b/apps/web/src/components/features/SettingsDialog.tsx @@ -5,7 +5,7 @@ import { ApiError } from "../../api/client"; import { getProviderAdapterCatalog, getServerConfig, saveServerConfig, toConfigDraft, type ServerConfigSnapshot } from "../../api/config"; import { useMcpStatusStore } from "../../store/mcp-status-store"; import { DialogContent, DialogDescription, DialogRoot, DialogTitle } from "../ui/Dialog"; -import { cloneConfig, hasConfigChanges, toFieldErrors, type SettingsSection } from "./settings-helpers"; +import { cloneConfig, hasConfigChanges, missingProfileVariants, toFieldErrors, type SettingsSection } from "./settings-helpers"; import { SettingsProfilesPanel, SettingsGithubPanel, SettingsMcpPanel, SettingsMemoryPanel, SettingsModelsPanel, SettingsNavigation } from "./settings-panels"; import { SettingsSecurityPanel } from "./SettingsSecurityPanel"; import { SettingsUpdatesPanel } from "./SettingsUpdatesPanel"; @@ -22,7 +22,7 @@ const restartSectionLabels: Record = { export function SettingsApplyNotice({ modelsAppliedLive, restartRequiredSections }: { modelsAppliedLive: boolean; restartRequiredSections: readonly RestartRequiredSection[] }) { if (!modelsAppliedLive && restartRequiredSections.length === 0) return null; - return
0 ? "border-warning/30 bg-warning-muted text-warning" : "border-success/30 bg-success-muted text-success"}`}> + return
0 ? "border-warning/30 bg-warning-muted text-warning" : "border-success/30 bg-success-muted text-success"}`}> {modelsAppliedLive && Model and Profile changes applied live.} {modelsAppliedLive && restartRequiredSections.length > 0 ? " " : null} {restartRequiredSections.length > 0 && Restart required for: {restartRequiredSections.map((section) => restartSectionLabels[section]).join(", ")}.} @@ -58,6 +58,7 @@ export function SettingsBody({ snapshot, adapterCatalog, servers, onReload, sect }, [requestedSection]); const dirty = useMemo(() => hasConfigChanges(draft, snapshot.config), [draft, snapshot.config]); + const invalidProfileCount = useMemo(() => missingProfileVariants(draft).length, [draft]); const onJsonValidationChange = useCallback((path: string, error?: string) => { setJsonErrors((current) => { if (error === undefined) { @@ -89,10 +90,14 @@ export function SettingsBody({ snapshot, adapterCatalog, servers, onReload, sect await onReload(); setModelsAppliedLive(modelSettingsChanged); } catch (error) { - setErrors(toFieldErrors(error)); + const nextErrors = toFieldErrors(error); + setErrors(nextErrors); + const firstValidationMessage = Object.values(nextErrors)[0]; setSaveError(error instanceof ApiError && error.code === "CONFIG_REVISION_CONFLICT" ? "This configuration was changed elsewhere. Reload the latest version before saving." - : error instanceof Error ? error.message : "Unable to save settings"); + : firstValidationMessage !== undefined + ? `Configuration validation failed: ${firstValidationMessage}` + : error instanceof Error ? error.message : "Unable to save settings"); } finally { setSaving(false); } @@ -103,21 +108,24 @@ export function SettingsBody({ snapshot, adapterCatalog, servers, onReload, sect onSectionChange?.(next); }; - return <>{section !== "updates" && }
- - {section === "updates" - ?
- :
-
- - - {section === "security" && } - - - -
{saveError || reloadError ?
{saveError ?? reloadError}
: {hasJsonErrors ? "Fix invalid JSON before saving" : dirty ? "Unsaved changes" : "All changes saved"}}
-
} -
; + return
+ {section !== "updates" && } +
+ + {section === "updates" + ?
+ :
+
+ + + {section === "security" && } + + + +
{saveError || reloadError ?
{saveError ?? reloadError}
: {hasJsonErrors ? "Fix invalid JSON before saving" : dirty ? "Unsaved changes" : "All changes saved"}}
+
} +
+
; } export function SettingsDialog({ open, section = "models", onClose }: { open: boolean; section?: SettingsSection; onClose: () => void }) { @@ -178,8 +186,8 @@ export function SettingsDialog({ open, section = "models", onClose }: { open: bo : "Loading settings…"}}; } -function SettingsSidebar({ section, onSelect }: { section: SettingsSection; onSelect: (section: SettingsSection) => void }) { - return ; +function SettingsSidebar({ section, onSelect, invalidProfileCount = 0 }: { section: SettingsSection; onSelect: (section: SettingsSection) => void; invalidProfileCount?: number }) { + return ; } function UpdatesOnlyWorkspace({ section, onSelect }: { section: SettingsSection; onSelect: (section: SettingsSection) => void }) { diff --git a/apps/web/src/components/features/settings-helpers.ts b/apps/web/src/components/features/settings-helpers.ts index 01f62d3a..cf5c46f8 100644 --- a/apps/web/src/components/features/settings-helpers.ts +++ b/apps/web/src/components/features/settings-helpers.ts @@ -11,6 +11,36 @@ export const PROFILE_NAMES = [ export const BUILT_IN_MCP_NAMES = BUILTIN_MCP_SERVER_NAMES; +export interface MissingProfileVariant { + profile: typeof PROFILE_NAMES[number]; + model: string; + variant: string; +} + +export function missingProfileVariant( + config: ServerConfig, + profile: typeof PROFILE_NAMES[number], +): MissingProfileVariant | undefined { + const item = config.profiles[profile]; + if (item.variant === undefined) return undefined; + const separator = item.model.indexOf(":"); + if (separator < 0) return undefined; + const providerId = item.model.slice(0, separator); + const modelId = item.model.slice(separator + 1); + const model = config.provider[providerId]?.models[modelId]; + if (!model || Object.prototype.hasOwnProperty.call(model.variants ?? {}, item.variant)) { + return undefined; + } + return { profile, model: item.model, variant: item.variant }; +} + +export function missingProfileVariants(config: ServerConfig): MissingProfileVariant[] { + return PROFILE_NAMES.flatMap((profile) => { + const issue = missingProfileVariant(config, profile); + return issue === undefined ? [] : [issue]; + }); +} + export function cloneConfig(config: ServerConfig): ServerConfig { return structuredClone(config); } diff --git a/apps/web/src/components/features/settings-panels.tsx b/apps/web/src/components/features/settings-panels.tsx index eaf1d1fd..44c338b4 100644 --- a/apps/web/src/components/features/settings-panels.tsx +++ b/apps/web/src/components/features/settings-panels.tsx @@ -2,7 +2,7 @@ import type { ConfigSecretMutation, McpServerStatus, ProviderAdapterCatalog, Pro import { ChevronRight, Plus, Trash2 } from "lucide-react"; import type { ModelCallOptions, ServerConfig, ServerMcpConfig, ServerModelConfig } from "../../api/config"; import { Field, JsonObjectField, NumberField, RenameInput, SecretField, SecretRecordEditor, TextInput } from "./settings-fields"; -import { PROFILE_NAMES, BUILT_IN_MCP_NAMES, defaultMemoryConfig, errorAtOrBelow, type FieldErrors, type SettingsSection, withDraft } from "./settings-helpers"; +import { PROFILE_NAMES, BUILT_IN_MCP_NAMES, defaultMemoryConfig, errorAtOrBelow, missingProfileVariant, type FieldErrors, type SettingsSection, withDraft } from "./settings-helpers"; type JsonValidationChange = (path: string, error?: string) => void; @@ -10,6 +10,8 @@ const secondaryActionClass = "inline-flex h-8 items-center justify-center gap-2 const subtleActionClass = "inline-flex h-7 items-center justify-center gap-2 rounded-sm px-2 text-[12px] font-medium text-brand transition-colors duration-[var(--motion-hover)] hover:bg-brand-subtle focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand"; const dangerActionClass = "inline-flex h-7 items-center justify-center gap-2 rounded-sm px-2 text-[12px] font-medium text-error transition-colors duration-[var(--motion-hover)] hover:bg-error-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand"; const selectClass = "h-8 w-full rounded-sm border border-border-control bg-bg-base px-3 text-[12px] text-text-primary outline-none transition-colors duration-[var(--motion-hover)] hover:border-text-secondary focus:border-brand focus:ring-2 focus:ring-brand-subtle"; +const MODEL_MODALITIES = ["text", "image", "audio", "video"] as const; +type ModelModality = typeof MODEL_MODALITIES[number]; function PanelHeader({ title, description }: { title: string; description: string }) { return
@@ -21,7 +23,7 @@ function PanelHeader({ title, description }: { title: string; description: strin function SettingsToggle({ checked, onChange, label, description }: { checked: boolean; onChange: (checked: boolean) => void; label: string; description: string }) { return ; } +function ModalityField({ + label, + value, + onChange, + error, +}: { + label: string; + value: readonly ModelModality[]; + onChange: (value: ModelModality[]) => void; + error?: string; +}) { + const selected = new Set(value); + return
+ {label} +
+ {MODEL_MODALITIES.map((modality) => { + const checked = selected.has(modality); + const onlySelection = checked && selected.size === 1; + return ; + })} +
+ {error &&

{error}

} +
; +} + function nextGeneratedId(prefix: string, entries: Record): string { let index = Object.keys(entries).length + 1; while (entries[`${prefix}-${index}`] !== undefined) index += 1; @@ -134,11 +177,39 @@ const MCP_STATUS_META: Record void }) { +export function SettingsNavigation({ + activeSection, + onSelect, + invalidProfileCount = 0, +}: { + activeSection: SettingsSection; + onSelect: (section: SettingsSection) => void; + invalidProfileCount?: number; +}) { const entries: Array<[SettingsSection, string]> = [["models", "Models"], ["profiles", "Profiles"], ["security", "Security"], ["mcp", "MCP"], ["memory", "Memory"], ["github", "GitHub"], ["updates", "About & Updates"]]; return ; } @@ -232,8 +303,18 @@ function ModelEditor({ config, onChange, providerId, modelId, model, errors, onJ update((draft) => { draft.name = next; })} /> update((draft) => { draft.limit.context = next ?? 0; })} /> update((draft) => { draft.limit.output = next ?? 0; })} /> - update((draft) => { draft.modalities.input = next.split(",").map((entry) => entry.trim()).filter(Boolean) as ServerModelConfig["modalities"]["input"]; })} /> - update((draft) => { draft.modalities.output = next.split(",").map((entry) => entry.trim()).filter(Boolean) as ServerModelConfig["modalities"]["output"]; })} /> + update((draft) => { draft.modalities.input = next; })} + error={errorAtOrBelow(errors, `${path}.modalities.input`)} + /> + update((draft) => { draft.modalities.output = next; })} + error={errorAtOrBelow(errors, `${path}.modalities.output`)} + />
| undefined} onChange={(next) => update((draft) => { draft.options = next as ModelCallOptions | undefined; })} error={errorAtOrBelow(errors, `${path}.options`)} validationPath={`${path}.options`} onValidationChange={onJsonValidationChange} resetVersion={jsonResetVersion} /> | undefined} onChange={(next) => update((draft) => { draft.variants = next as Record | undefined; })} error={errorAtOrBelow(errors, `${path}.variants`)} validationPath={`${path}.variants`} onValidationChange={onJsonValidationChange} resetVersion={jsonResetVersion} /> @@ -252,16 +333,27 @@ export function SettingsProfilesPanel({ config, onChange, errors, onJsonValidati const model = separator < 0 ? "" : item.model.slice(separator + 1); const variants = config.provider[provider]?.models[model]?.variants ?? {}; const optionsPath = `profiles.${profile}.options`; + const missingVariant = missingProfileVariant(config, profile); + const variantPath = `profiles.${profile}.variant`; + const variantError = errors[variantPath] ?? (missingVariant === undefined + ? undefined + : `Variant "${missingVariant.variant}" no longer exists. This Profile is using the model default.`); return
- +
- +
| undefined} onChange={(next) => onChange(withDraft(config, (draft) => { draft.profiles[profile].options = next as ModelCallOptions | undefined; }))} error={errorAtOrBelow(errors, optionsPath)} validationPath={optionsPath} onValidationChange={onJsonValidationChange} resetVersion={jsonResetVersion} />
diff --git a/docs/configuration.md b/docs/configuration.md index 9733b01d..ca87b0f7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -70,6 +70,7 @@ The catalog contains exactly these official AI SDK language Provider packages: - Analyst requires `deep`; Explore and Librarian require `fast`; Build accepts `deep` or `fast` selected by Lead at delegation time. - A Profile changes model resources only. Agent identity controls tools and delegation; Skills control workflow guidance. - Profile-default call options merge as `model.options → variants[profile.variant] → profiles[profile].options`. `providerOptions` is shallow-replaced like any other top-level key. +- If a Profile still names a removed variant, Settings marks that Profile for attention but allows the document to save. Runtime omits the missing variant and uses the model default plus Profile options until the reference is repaired. An empty Profile variant is normalized to Default; an unknown Profile model remains a validation error. - A root Lead Session override is a complete alternative selection. It resolves the chosen model and variant without inheriting `principal` options; clearing it returns to `principal`. Saving in **Settings → Models / Profiles** validates the complete document, prepares the new Provider registry, writes atomically, then publishes Models and Profile defaults immediately. It returns the disk revision, the published model-runtime revision, and any restart-required sections: `mcp`, `memory`, or `integrations.github`. diff --git a/packages/agent-core/src/config/config.test.ts b/packages/agent-core/src/config/config.test.ts index 694edf20..2af4fb7e 100644 --- a/packages/agent-core/src/config/config.test.ts +++ b/packages/agent-core/src/config/config.test.ts @@ -293,6 +293,44 @@ describe("parseConfig", () => { expect(parsed.profiles.fast.options?.temperature).toBe(0.5); }); + test("normalizes an empty Profile variant to the model default", () => { + const config = { + ...VALID_CONFIG_WITH_PROFILES, + profiles: { + ...VALID_CONFIG_WITH_PROFILES.profiles, + principal: { + ...VALID_CONFIG_WITH_PROFILES.profiles.principal, + variant: "", + }, + }, + }; + + const parsed = parseConfig(config); + + expect(parsed.profiles.principal.variant).toBeUndefined(); + }); + + test("rejects an empty model variant name", () => { + const config = { + ...VALID_CONFIG_WITH_PROFILES, + provider: { + xxx: { + ...VALID_CONFIG_WITH_PROFILES.provider.xxx, + models: { + "gpt-5.2": { + ...VALID_CONFIG_WITH_PROFILES.provider.xxx.models["gpt-5.2"], + variants: { + "": { temperature: 0.2 }, + }, + }, + }, + }, + }, + }; + + expect(() => parseConfig(config)).toThrow(ConfigValidationError); + }); + test("memory config is optional", () => { const parsed = parseConfig(VALID_CONFIG_WITH_PROFILES); diff --git a/packages/agent-core/src/config/provider.ts b/packages/agent-core/src/config/provider.ts index 62f9a98a..b3f3ae61 100644 --- a/packages/agent-core/src/config/provider.ts +++ b/packages/agent-core/src/config/provider.ts @@ -122,7 +122,7 @@ export const modelConfigSchema = z limit: modelLimitSchema, modalities: modelModalitiesSchema, options: modelCallOptionsSchema.optional(), - variants: z.record(z.string(), modelCallOptionsSchema).optional(), + variants: z.record(z.string().min(1), modelCallOptionsSchema).optional(), }) .strict(); diff --git a/packages/agent-core/src/config/schema.ts b/packages/agent-core/src/config/schema.ts index a4a6a608..3296faa8 100644 --- a/packages/agent-core/src/config/schema.ts +++ b/packages/agent-core/src/config/schema.ts @@ -23,7 +23,9 @@ export const integrationsConfigSchema = z export const profileConfigSchema = z .object({ model: z.string().min(1), - variant: z.string().optional(), + variant: z.string().transform((variant) => + variant === "" ? undefined : variant + ).optional(), options: modelCallOptionsSchema.optional(), }) .strict(); diff --git a/packages/agent-core/src/config/server-config-service.test.ts b/packages/agent-core/src/config/server-config-service.test.ts index f164ecec..1e23aea8 100644 --- a/packages/agent-core/src/config/server-config-service.test.ts +++ b/packages/agent-core/src/config/server-config-service.test.ts @@ -499,10 +499,9 @@ describe("ServerConfigService", () => { expect(await readFile(service.configPath, "utf8")).toBe(before); }); - test("rejects unsupported provider packages, unknown variants, and invalid MCP URLs before writing", async () => { + test("rejects unsupported provider packages and invalid MCP URLs before writing", async () => { for (const mutate of [ (draft: ServerConfigUpdate) => { draft.provider.local.npm = "unsupported"; }, - (draft: ServerConfigUpdate) => { draft.profiles.principal.variant = "missing"; }, (draft: ServerConfigUpdate) => { draft.mcp!.servers.custom.url = "file:///not-http"; }, ]) { const service = await createService(); @@ -515,6 +514,45 @@ describe("ServerConfigService", () => { } }); + test("saves a missing Profile variant and publishes its model-default fallback", async () => { + const service = await createService(); + const snapshot = await service.getSnapshot(); + const update = preserveSecrets(snapshot.config); + update.profiles.fast.variant = "removed"; + + const saved = await service.save({ + expectedRevision: snapshot.revision, + config: update, + }); + + expect(saved.config.profiles.fast.variant).toBe("removed"); + expect(service.modelRuntime.current.getProfileDefault("fast")).toEqual({ + model: "local:test-model", + }); + expect(service.modelRuntime.current.catalog.profileDefaults.fast).toEqual({ + model: "local:test-model", + }); + expect(JSON.parse(await readFile(service.configPath, "utf8")).profiles.fast.variant).toBe("removed"); + }); + + test("normalizes an empty Profile variant to the model default on save", async () => { + const service = await createService(); + const snapshot = await service.getSnapshot(); + const update = preserveSecrets(snapshot.config); + update.profiles.fast.variant = ""; + + const saved = await service.save({ + expectedRevision: snapshot.revision, + config: update, + }); + + expect(saved.config.profiles.fast.variant).toBeUndefined(); + expect(service.modelRuntime.current.getProfileDefault("fast")).toEqual({ + model: "local:test-model", + }); + expect(JSON.parse(await readFile(service.configPath, "utf8")).profiles.fast).not.toHaveProperty("variant"); + }); + test("rejects built-in MCP names before writing", async () => { const service = await createService(); const snapshot = await service.getSnapshot(); diff --git a/packages/agent-core/src/config/server-config-service.ts b/packages/agent-core/src/config/server-config-service.ts index 395cb1eb..eb9fd8a1 100644 --- a/packages/agent-core/src/config/server-config-service.ts +++ b/packages/agent-core/src/config/server-config-service.ts @@ -415,9 +415,6 @@ function validateConfig(value: unknown): ArchCodeConfig { issues.push({ path: `profiles.${profileName}.model`, message: `Unknown model reference "${profile.model}"` }); continue; } - if (profile.variant !== undefined && model.variants?.[profile.variant] === undefined) { - issues.push({ path: `profiles.${profileName}.variant`, message: `Unknown variant "${profile.variant}" for model "${profile.model}"` }); - } } for (const name of Object.keys(config.mcp?.servers ?? {})) { diff --git a/packages/agent-core/src/models/model-runtime-snapshot.ts b/packages/agent-core/src/models/model-runtime-snapshot.ts index 6bde5223..cc002ec4 100644 --- a/packages/agent-core/src/models/model-runtime-snapshot.ts +++ b/packages/agent-core/src/models/model-runtime-snapshot.ts @@ -76,9 +76,9 @@ export class ModelRuntimeSnapshot { ); for (const [profileName, profile] of profiles) { - if (!this.tryResolveSelection({ model: profile.model, variant: profile.variant })) { + if (!this.#models.has(profile.model)) { throw new InvalidModelRuntimeSnapshotError( - `Profile "${profileName}" has invalid default selection "${formatSelection(profile)}"`, + `Profile "${profileName}" has unknown default model "${profile.model}"`, ); } } @@ -92,7 +92,11 @@ export class ModelRuntimeSnapshot { if (!profile) { throw new InvalidModelRuntimeSnapshotError(`Profile "${profileName}" is absent from the model runtime snapshot`); } - return freezeSelection({ model: profile.model, variant: profile.variant }); + const model = this.#models.get(profile.model); + if (!model) { + throw new InvalidModelRuntimeSnapshotError(`Profile "${profileName}" has unknown default model "${profile.model}"`); + } + return freezeSelection(resolvedProfileSelection(profile, model.config)); } getProfileOptions(profileName: ProfileName): ProfileConfig["options"] { @@ -143,18 +147,35 @@ function buildCatalog( })), })); const profileDefaults = { - principal: selectionFromProfile(config.profiles.principal), - deep: selectionFromProfile(config.profiles.deep), - fast: selectionFromProfile(config.profiles.fast), + principal: selectionFromProfile(config, config.profiles.principal), + deep: selectionFromProfile(config, config.profiles.deep), + fast: selectionFromProfile(config, config.profiles.fast), }; return deepFreeze({ revision, providers, profileDefaults }); } -function selectionFromProfile(profile: ProfileConfig): ModelSelectionRef { - return profile.variant === undefined - ? { model: profile.model } - : { model: profile.model, variant: profile.variant }; +function selectionFromProfile( + config: Omit, + profile: ProfileConfig, +): ModelSelectionRef { + const [providerId = "", ...modelIdParts] = profile.model.split(":"); + const model = config.provider[providerId]?.models[modelIdParts.join(":")]; + if (!model) return { model: profile.model }; + return resolvedProfileSelection(profile, model); +} + +function resolvedProfileSelection( + profile: ProfileConfig, + model: ModelConfig, +): ModelSelectionRef { + if ( + profile.variant === undefined + || !Object.prototype.hasOwnProperty.call(model.variants ?? {}, profile.variant) + ) { + return { model: profile.model }; + } + return { model: profile.model, variant: profile.variant }; } function freezeSelection(selection: ModelSelectionRef): ModelSelectionRef { @@ -163,9 +184,3 @@ function freezeSelection(selection: ModelSelectionRef): ModelSelectionRef { ...(selection.variant === undefined ? {} : { variant: selection.variant }), }); } - -function formatSelection(selection: ModelSelectionRef): string { - return selection.variant === undefined - ? selection.model - : `${selection.model} (${selection.variant})`; -} diff --git a/packages/agent-core/src/models/model-selection-resolver.test.ts b/packages/agent-core/src/models/model-selection-resolver.test.ts index 5595d595..b37b7f1f 100644 --- a/packages/agent-core/src/models/model-selection-resolver.test.ts +++ b/packages/agent-core/src/models/model-selection-resolver.test.ts @@ -156,11 +156,26 @@ describe("ModelRuntimeSnapshot", () => { expect(Object.isFrozen(runtimeSnapshot.catalog)).toBe(true); }); - test("rejects an invalid Profile binding and a registry missing a configured model", () => { + test("falls back from a missing Profile variant to the model default", () => { const invalidProfile = config(); invalidProfile.profiles.deep.variant = "removed"; - expect(() => snapshot(invalidProfile)).toThrow(InvalidModelRuntimeSnapshotError); + const runtimeSnapshot = snapshot(invalidProfile); + const binding = new ModelSelectionResolver().resolve({ + snapshot: runtimeSnapshot, + profile: "deep", + }); + + expect(runtimeSnapshot.getProfileDefault("deep")).toEqual({ model: "local:alpha" }); + expect(runtimeSnapshot.catalog.profileDefaults.deep).toEqual({ model: "local:alpha" }); + expect(binding.summary.selection).toEqual({ model: "local:alpha" }); + expect(binding.options).toEqual({ + temperature: 0.25, + topP: 0.7, + providerOptions: { local: { layer: "model" } }, + }); + }); + test("rejects a registry missing a configured model", () => { const missingModel = config(); const missingRegistry = registry(missingModel); (missingRegistry.models as Map).delete("local:beta");