diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index 6b6db4718220..c884d7fe6bdb 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -640,7 +640,7 @@ function App(props: { pair?: DialogPairCredentials }) { onMount(() => { batch(() => { if (args.agent) local.agent.set(args.agent) - if (args.model) { + if (args.model && typeof args.model === "string") { const { providerID, modelID } = Model.parse(args.model) if (!providerID || !modelID) return toast.show({ @@ -648,7 +648,6 @@ function App(props: { pair?: DialogPairCredentials }) { message: `Invalid model format: ${args.model}`, duration: 3000, }) - local.model.set({ providerID, modelID }, { recent: true }) } if (args.sessionID && !args.fork) { route.navigate({ diff --git a/packages/tui/src/context/local.tsx b/packages/tui/src/context/local.tsx index c10baed3090e..8669fe4a12c9 100644 --- a/packages/tui/src/context/local.tsx +++ b/packages/tui/src/context/local.tsx @@ -1,7 +1,7 @@ import { createStore } from "solid-js/store" import { dedupeWith } from "effect/Array" import { createSimpleContext } from "./helper" -import { batch, createMemo, onCleanup } from "solid-js" +import { batch, createEffect, createMemo, createSignal, onCleanup } from "solid-js" import { useEvent } from "./event" import path from "path" import { useTuiPaths } from "./runtime" @@ -54,7 +54,8 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ const models = () => data.location.model.list(location.ref) const providers = () => data.location.provider.list(location.ref) - function isModelValid(model: ModelPreferenceModel) { + function isModelValid(model?: ModelPreferenceModel) { + if (!model?.providerID || !model?.modelID) return false return !!models()?.some((item) => item.providerID === model.providerID && item.id === model.modelID) } @@ -165,6 +166,7 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ const repository = createModelPreferenceRepository(path.join(paths.state, "model.json")) const pendingSelectionCommits = new Map() + const [cliSuperseded, setCliSuperseded] = createSignal(false) const selectionKey = (value: ModelSelection) => `${modelPreferenceKey(value)}:${normalizeModelVariant(value.variant) ?? "default"}` const saveState = { @@ -205,22 +207,36 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ ?.findLast((entry) => entry.type === "document" && entry.info.model !== undefined) const configured = entry?.type === "document" ? entry.info.model : undefined if (!configured) return - return typeof configured === "string" - ? { ...parse(configured), variant: undefined } - : { providerID: configured.providerID, modelID: configured.model, variant: configured.variant } + if (typeof configured === "string") { + const parsed = parse(configured) + if (!parsed.providerID || !parsed.modelID) return undefined + return parsed + } + if (typeof configured === "object" && configured !== null) { + const record = configured as Record + const providerID = typeof record.providerID === "string" ? record.providerID : undefined + const modelID = + typeof record.model === "string" + ? record.model + : typeof record.modelID === "string" + ? record.modelID + : undefined + const variant = typeof record.variant === "string" ? record.variant : undefined + if (!providerID || !modelID) return undefined + return { providerID, modelID, variant } + } + return undefined }) - const fallbackModel = createMemo(() => { - if (args.model) { - const { providerID, modelID } = parse(args.model) - if (isModelValid({ providerID, modelID })) { - return { - providerID, - modelID, - } - } - } + const cliModel = createMemo(() => { + if (!args.model || typeof args.model !== "string") return undefined + const parsed = parse(args.model) + if (!parsed.providerID || !parsed.modelID) return undefined + if (!isModelValid({ providerID: parsed.providerID, modelID: parsed.modelID })) return undefined + return parsed + }) + const fallbackModel = createMemo(() => { const configured = configuredModel() if (configured && isModelValid(configured)) return configured @@ -242,11 +258,23 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ const a = agent.current() return getFirstValidModel( () => a && selectionState.newSessionModelByLocationAgent[locationAgentKey(a.id)], - () => a?.model && { providerID: a.model.providerID, modelID: a.model.id }, + cliModel, + () => a?.model && { providerID: a.model.providerID, modelID: a.model.id, variant: a.model.variant }, fallbackModel, ) }) + createEffect(() => { + const cli = cliModel() + if (!cli) return + if (!models()) return + if (!preferences.ready) return + const key = modelPreferenceKey(cli) + if (preferences.recent.some((item) => modelPreferenceKey(item) === key)) return + setPreferences("recent", recentModels({ providerID: cli.providerID, modelID: cli.modelID }, preferences.recent)) + savePreferences() + }) + const currentSelection = createMemo(() => { if (route.data.type === "session") return sessionSelection(route.data.sessionID) const model = newSessionModel() @@ -265,12 +293,17 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ return `${JSON.stringify([ref.directory, ref.workspaceID])}:${agentID}` } - function preferredSelection(model: ModelPreferenceModel): ModelSelection { + function preferredSelection(model: ModelSelection): ModelSelection { const configured = agent.current()?.model const fallback = configuredModel() + const cli = cliSuperseded() ? undefined : cliModel() + const cliVariant = + cli && cli.providerID === model.providerID && cli.modelID === model.modelID ? cli.variant : undefined const preferred = preferences.variant[modelPreferenceKey(model)] const variant = normalizeModelVariant( - preferred ?? + cliVariant ?? + preferred ?? + model.variant ?? (configured?.providerID === model.providerID && configured.id === model.modelID ? configured.variant : undefined) ?? @@ -338,6 +371,7 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ if (route.data.type === "session") { const sessionID = route.data.sessionID const current = sessionSelection(sessionID) + setCliSuperseded(true) setSessionDraft( sessionID, current?.providerID === model.providerID && current.modelID === model.modelID @@ -349,6 +383,7 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ const current = agent.current() if (!current) return false setSelectionState("newSessionModelByLocationAgent", locationAgentKey(current.id), model) + setCliSuperseded(true) return true } @@ -523,6 +558,7 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ set(value: string | undefined) { const m = currentSelection() if (!m) return + setCliSuperseded(true) if (route.data.type === "session") { setSessionDraft(route.data.sessionID, { ...m, variant: normalizeModelVariant(value) }) } diff --git a/packages/tui/src/util/model.ts b/packages/tui/src/util/model.ts index 215602845fd9..deeca2f09f9a 100644 --- a/packages/tui/src/util/model.ts +++ b/packages/tui/src/util/model.ts @@ -1,6 +1,19 @@ -export function parse(value: string) { - const [providerID, ...modelID] = value.split("/") - return { providerID, modelID: modelID.join("/") } +import { Model } from "@opencode/schema/model" + +export function parse(value?: string | null): { providerID: string; modelID: string; variant?: string } { + if (typeof value !== "string" || value.length === 0) { + return { providerID: "", modelID: "" } + } + try { + const ref = Model.Ref.parse(value) + return { + providerID: ref.providerID, + modelID: ref.id, + ...(ref.variant ? { variant: ref.variant } : {}), + } + } catch { + return { providerID: "", modelID: "" } + } } export function formatRef(model: { providerID: string; id: string; variant?: string }) { @@ -12,6 +25,7 @@ export function switchLabel( models?: readonly { providerID: string; id: string; name: string }[], previous?: { providerID: string; id: string; variant?: string }, ) { + if (!model?.providerID || !model?.id) return "" if (previous?.providerID === model.providerID && previous.id === model.id) return `Switched variant to ${model.variant ?? "default"}` const display = models?.find((item) => item.providerID === model.providerID && item.id === model.id)?.name diff --git a/packages/tui/test/context/local-selection.test.tsx b/packages/tui/test/context/local-selection.test.tsx index 4c7376476bae..e4d8d34ba42b 100644 --- a/packages/tui/test/context/local-selection.test.tsx +++ b/packages/tui/test/context/local-selection.test.tsx @@ -1,4 +1,5 @@ import { expect, test } from "bun:test" +import path from "node:path" import { agent, model, renderLocal, session } from "../fixture/local" import { json } from "../fixture/tui-client" @@ -147,6 +148,53 @@ test("same-model agent switches clear drafts without a model acknowledgment", as expect(setup.local.model.current()?.modelID).toBe("second") }) +test("handles malformed config models and non-string CLI model args safely", async () => { + await using setup = await renderLocal({ + models: [model("first"), model("second", ["low", "high"])], + args: { model: true as any }, + fetch: (url) => { + if (url.pathname === "/api/config") + return json([ + { type: "document", info: { model: { providerID: "incomplete" } } }, + { type: "document", info: { model: null } }, + { type: "document", info: { model: 123 } }, + { type: "document", info: { model: "provider/second#high" } }, + ]) + }, + }) + expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "high" }) +}) + +test("preserves CLI model and variant ahead of recents and config and allows in-session variant switches", async () => { + await using setup = await renderLocal({ + models: [model("first"), model("second", ["low", "high"])], + args: { model: "provider/second#high" }, + preferences: { recent: [{ providerID: "provider", modelID: "first" }] }, + }) + expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "high" }) + setup.local.model.variant.set("low") + expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "low" }) +}) + +test("CLI variant wins transiently over stored preference without persisting", async () => { + await using setup = await renderLocal({ + models: [model("first"), model("second", ["low", "high"])], + args: { model: "provider/second#high" }, + preferences: { variant: { "provider/second": "low" } }, + }) + expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "high" }) + await setup.waitFor(async () => { + await Bun.sleep(10) + return setup.local.model.recent().some((item) => item.providerID === "provider" && item.modelID === "second") + }) + const stored = (await Bun.file(path.join(setup.state, "model.json")) + .json() + .catch(() => ({}))) as { variant?: Record } + expect(stored.variant?.["provider/second"]).toBe("low") + setup.local.model.variant.set("low") + expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "low" }) +}) + async function publishSelection( setup: Awaited>, agent: string, diff --git a/packages/tui/test/util/model.test.ts b/packages/tui/test/util/model.test.ts index 888b102013b3..a20ff65e2f68 100644 --- a/packages/tui/test/util/model.test.ts +++ b/packages/tui/test/util/model.test.ts @@ -4,7 +4,34 @@ import { formatRef, parse, switchLabel } from "../../src/util/model" describe("util.model", () => { test("splits provider from a nested model identifier", () => { expect(parse("provider/org/model")).toEqual({ providerID: "provider", modelID: "org/model" }) - expect(parse("invalid")).toEqual({ providerID: "invalid", modelID: "" }) + expect(parse("invalid")).toEqual({ providerID: "", modelID: "" }) + }) + + test("parses variant from model identifier if present", () => { + expect(parse("anthropic/claude-3-5-sonnet#thinking")).toEqual({ + providerID: "anthropic", + modelID: "claude-3-5-sonnet", + variant: "thinking", + }) + expect(parse("openrouter/anthropic/claude-3.5-sonnet#high")).toEqual({ + providerID: "openrouter", + modelID: "anthropic/claude-3.5-sonnet", + variant: "high", + }) + }) + + test("rejects empty variants and extra separators as invalid", () => { + expect(parse("provider/model#")).toEqual({ providerID: "", modelID: "" }) + expect(parse("provider/mo#de#l")).toEqual({ providerID: "", modelID: "" }) + expect(parse("openai/gpt-5#high#extra")).toEqual({ providerID: "", modelID: "" }) + }) + + test("handles undefined, null, non-string, and empty model identifiers safely", () => { + expect(parse(undefined as any)).toEqual({ providerID: "", modelID: "" }) + expect(parse(null as any)).toEqual({ providerID: "", modelID: "" }) + expect(parse({} as any)).toEqual({ providerID: "", modelID: "" }) + expect(parse(true as any)).toEqual({ providerID: "", modelID: "" }) + expect(parse("")).toEqual({ providerID: "", modelID: "" }) }) test("includes the selected variant in model refs", () => { @@ -46,4 +73,10 @@ describe("util.model", () => { "Switched model to anthropic/sonnet/high", ) }) + + test("handles empty or invalid model in switchLabel safely", () => { + expect(switchLabel(undefined as any)).toBe("") + expect(switchLabel({} as any)).toBe("") + expect(switchLabel({ providerID: "anthropic" } as any)).toBe("") + }) })