From d89457e938439e309e91830c202b003bb8756bee Mon Sep 17 00:00:00 2001 From: Eva Ho Date: Tue, 27 Oct 2026 12:37:30 -0400 Subject: [PATCH] fix(cli): honor explicit startup model selection --- packages/cli/src/commands/commands.ts | 5 + packages/cli/src/commands/handlers/default.ts | 1 + packages/cli/test/startup-model.test.ts | 28 ++++ packages/tui/src/app.tsx | 7 +- packages/tui/src/context/local.tsx | 14 +- packages/tui/test/app-lifecycle.test.tsx | 153 ++++++++++-------- .../tui/test/context/local-selection.test.tsx | 41 +++++ 7 files changed, 166 insertions(+), 83 deletions(-) create mode 100644 packages/cli/test/startup-model.test.ts diff --git a/packages/cli/src/commands/commands.ts b/packages/cli/src/commands/commands.ts index 060b6160057a..d6cd40c03238 100644 --- a/packages/cli/src/commands/commands.ts +++ b/packages/cli/src/commands/commands.ts @@ -54,6 +54,11 @@ const Root = Spec.make(typeof OPENCODE_CLI_NAME === "string" ? OPENCODE_CLI_NAME Flag.withDescription("Session ID to continue, or to create if it does not exist"), Flag.optional, ), + model: Flag.String("model").pipe( + Flag.withAlias("m"), + Flag.withDescription("Model to use in the format provider/model"), + Flag.optional, + ), prompt: Flag.String("prompt").pipe(Flag.withDescription("Prompt to use"), Flag.optional), }, commands: [ diff --git a/packages/cli/src/commands/handlers/default.ts b/packages/cli/src/commands/handlers/default.ts index 426f5f7f5b75..a1c319bfa832 100644 --- a/packages/cli/src/commands/handlers/default.ts +++ b/packages/cli/src/commands/handlers/default.ts @@ -109,6 +109,7 @@ export default Runtime.handler(Commands, (input) => : undefined, }, args: { + model: Option.getOrUndefined(input.model), continue: input.continue, sessionID: sessionExists ? session : undefined, newSessionID: sessionExists ? undefined : session, diff --git a/packages/cli/test/startup-model.test.ts b/packages/cli/test/startup-model.test.ts new file mode 100644 index 000000000000..de655d0c6ae7 --- /dev/null +++ b/packages/cli/test/startup-model.test.ts @@ -0,0 +1,28 @@ +import { NodeServices } from "@effect/platform-node" +import { expect, test } from "bun:test" +import { Effect, Option } from "effect" +import { Command } from "effect/cli" +import { Commands } from "../src/commands/commands" + +test.each([ + { args: [], model: undefined, standalone: false }, + { args: ["--model", "ollama/selected"], model: "ollama/selected", standalone: false }, + { args: ["-m", "ollama/selected"], model: "ollama/selected", standalone: false }, + { args: ["--standalone", "--model", "ollama/selected"], model: "ollama/selected", standalone: true }, + { + args: ["--server", "http://127.0.0.1:9", "-m", "ollama/selected"], + model: "ollama/selected", + standalone: false, + }, +])("the startup command accepts a model selection: $args", async ({ args, model, standalone }) => { + const received: { model?: string; standalone: boolean }[] = [] + const command = Commands.spec.pipe( + Command.withHandler((input) => + Effect.sync( + () => void received.push({ model: Option.getOrUndefined(input.model), standalone: input.standalone }), + ), + ), + ) + await Effect.runPromise(Command.runWith(command, { version: "test" })(args).pipe(Effect.provide(NodeServices.layer))) + expect(received).toEqual([{ model, standalone }]) +}) diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index 577cfa11b095..64a9799d5d9a 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -629,13 +629,14 @@ function App() { if (args.agent) local.agent.set(args.agent) if (args.model) { const { providerID, modelID } = Model.parse(args.model) - if (!providerID || !modelID) - return toast.show({ + const valid = providerID.length > 0 && modelID.length > 0 + if (!valid) + toast.show({ variant: "warning", message: `Invalid model format: ${args.model}`, duration: 3000, }) - local.model.set({ providerID, modelID }, { recent: true }) + if (valid) 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 54eabb6e1d5e..a7f2d20a3b75 100644 --- a/packages/tui/src/context/local.tsx +++ b/packages/tui/src/context/local.tsx @@ -178,16 +178,11 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ : { providerID: configured.providerID, modelID: configured.model, variant: configured.variant } }) + const cliModel = createMemo(() => (args.model ? parse(args.model) : undefined)) + const fallbackModel = createMemo(() => { - if (args.model) { - const { providerID, modelID } = parse(args.model) - if (isModelValid({ providerID, modelID })) { - return { - providerID, - modelID, - } - } - } + const requested = cliModel() + if (requested && isModelValid(requested)) return requested const configured = configuredModel() if (configured && isModelValid(configured)) return configured @@ -210,6 +205,7 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ const a = agent.current() return getFirstValidModel( () => a && selectionState.newSessionModelByLocationAgent[locationAgentKey(a.id)], + cliModel, () => a?.model && { providerID: a.model.providerID, modelID: a.model.id }, fallbackModel, ) diff --git a/packages/tui/test/app-lifecycle.test.tsx b/packages/tui/test/app-lifecycle.test.tsx index f958b8c3d929..6a442f75f3a3 100644 --- a/packages/tui/test/app-lifecycle.test.tsx +++ b/packages/tui/test/app-lifecycle.test.tsx @@ -803,81 +803,92 @@ test("keeps assistant footer metrics current after prepend, same-length refresh, expect(reverted).not.toContain("50.0 tok/s") }) -test("session startup prompt is submitted exactly once", async () => { - await using state = await tmpdir() - const setup = await createTestRenderer({ width: 80, height: 24, useThread: false }) - const events = createEventStream() - const cwd = process.cwd() - const location = { directory: cwd, project: { id: "project", directory: cwd } } - const session = { - id: "dummy", - title: "Demo session", - projectID: "project", - location: { directory: cwd }, - agent: "build", - model: { providerID: "provider", id: "model" }, - cost: 0, - tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, - time: { created: 0, updated: 0 }, - } - const bodies: unknown[] = [] - const promptSubmitted = Promise.withResolvers() - const calls = createFetch(async (url, request) => { - if (url.pathname === "/api/location") return json(location) - if (url.pathname === "/api/session") return json({ data: [session], cursor: {} }) - if (url.pathname === "/api/session/dummy") return json({ data: session }) - if (url.pathname === "/api/session/dummy/message") return json({ data: [], cursor: {} }) - if (url.pathname === "/api/session/dummy/inbox") return json({ data: [] }) - if (url.pathname === "/api/session/dummy/permission") return json({ data: [] }) - if (url.pathname === "/api/agent") - return json({ - location, - data: [{ id: "build", mode: "primary", hidden: false, permissions: [] }], - }) - if (url.pathname === "/api/model") - return json({ - location, - data: [{ id: "model", providerID: "provider", name: "Model", variants: [] }], - }) - if (url.pathname === "/api/session/dummy/prompt") { - bodies.push(await request.json()) - promptSubmitted.resolve() - return json({ data: {} }) +test.each([undefined, "provider/", "/model", "model-without-provider"])( + "session startup prompt is submitted exactly once with model %s", + async (model) => { + await using state = await tmpdir() + const setup = await createTestRenderer({ width: 80, height: 24, useThread: false }) + const events = createEventStream() + const cwd = process.cwd() + const location = { directory: cwd, project: { id: "project", directory: cwd } } + const session = { + id: "dummy", + title: "Demo session", + projectID: "project", + location: { directory: cwd }, + agent: "build", + model: { providerID: "provider", id: "model" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + time: { created: 0, updated: 0 }, } - }, events) - const server = Bun.serve({ port: 0, fetch: (request) => calls.fetch(request) }) + const bodies: unknown[] = [] + let createdSessions = 0 + const promptSubmitted = Promise.withResolvers() + const calls = createFetch(async (url, request) => { + if (url.pathname === "/api/location") return json(location) + if (url.pathname === "/api/session") { + if (request.method === "POST") { + createdSessions++ + return new Response("Unexpected new session", { status: 500 }) + } + return json({ data: [session], cursor: {} }) + } + if (url.pathname === "/api/session/dummy") return json({ data: session }) + if (url.pathname === "/api/session/dummy/message") return json({ data: [], cursor: {} }) + if (url.pathname === "/api/session/dummy/inbox") return json({ data: [] }) + if (url.pathname === "/api/session/dummy/permission") return json({ data: [] }) + if (url.pathname === "/api/agent") + return json({ + location, + data: [{ id: "build", mode: "primary", hidden: false, permissions: [] }], + }) + if (url.pathname === "/api/model") + return json({ + location, + data: [{ id: "model", providerID: "provider", name: "Model", variants: [] }], + }) + if (url.pathname === "/api/session/dummy/prompt") { + bodies.push(await request.json()) + promptSubmitted.resolve() + return json({ data: {} }) + } + }, events) + const server = Bun.serve({ port: 0, fetch: (request) => calls.fetch(request) }) - try { - const { run } = await import("../src/app") - const task = Effect.runPromise( - run({ - app: { name: "test", version: "test", channel: "test" }, - server: { endpoint: { url: server.url.toString() } }, - config: { get: async () => ({}), update: async () => ({}) }, - packages: { prepare: async () => ({ directory: "" }) }, - terminalHandoff: async () => ({ renderer: setup.renderer, mode: "dark", complete: () => {} }), - args: { sessionID: "dummy", prompt: "RESUME_READY" }, - log: () => {}, - }).pipe(Effect.provide(Global.layerWith({ state: state.path })), Effect.provide(FileSystem.layerNoop({}))), - ) + try { + const { run } = await import("../src/app") + const task = Effect.runPromise( + run({ + app: { name: "test", version: "test", channel: "test" }, + server: { endpoint: { url: server.url.toString() } }, + config: { get: async () => ({}), update: async () => ({}) }, + packages: { prepare: async () => ({ directory: "" }) }, + terminalHandoff: async () => ({ renderer: setup.renderer, mode: "dark", complete: () => {} }), + args: { sessionID: "dummy", model, prompt: "RESUME_READY" }, + log: () => {}, + }).pipe(Effect.provide(Global.layerWith({ state: state.path })), Effect.provide(FileSystem.layerNoop({}))), + ) - await Promise.race([ - promptSubmitted.promise, - Bun.sleep(2000).then(() => { - throw new Error("startup prompt was not submitted") - }), - ]) - await Bun.sleep(20) - setup.renderer.destroy() - await task + await Promise.race([ + promptSubmitted.promise, + Bun.sleep(2000).then(() => { + throw new Error("startup prompt was not submitted") + }), + ]) + await Bun.sleep(20) + setup.renderer.destroy() + await task - expect(bodies).toHaveLength(1) - expect(bodies[0]).toMatchObject({ text: "RESUME_READY" }) - } finally { - if (!setup.renderer.isDestroyed) setup.renderer.destroy() - await server.stop() - } -}) + expect(bodies).toHaveLength(1) + expect(bodies[0]).toMatchObject({ text: "RESUME_READY" }) + expect(createdSessions).toBe(0) + } finally { + if (!setup.renderer.isDestroyed) setup.renderer.destroy() + await server.stop() + } + }, +) test("home startup prompt is submitted exactly once", async () => { await using state = await tmpdir() diff --git a/packages/tui/test/context/local-selection.test.tsx b/packages/tui/test/context/local-selection.test.tsx index b6d7ddaa2d6a..bd6f4617b72d 100644 --- a/packages/tui/test/context/local-selection.test.tsx +++ b/packages/tui/test/context/local-selection.test.tsx @@ -34,6 +34,47 @@ test("uses the last configured model and variant ahead of recents", async () => expect(setup.local.model.selection()).toEqual({ providerID: "provider", modelID: "second", variant: "high" }) }) +test.each(["build", "reviewer"])( + "an explicit CLI model overrides the %s agent default until the user selects another model", + async (id) => { + await using setup = await renderLocal({ + models: [model("first"), model("second"), model("third")], + args: { model: "provider/second" }, + agents: [agent(id, { providerID: "provider", id: "first" }), agent("plan")], + }) + expect(setup.local.agent.current()?.id).toBe(id) + expect(setup.local.model.current()?.modelID).toBe("second") + setup.local.model.set({ providerID: "provider", modelID: "third" }) + expect(setup.local.model.current()?.modelID).toBe("third") + setup.local.agent.set("plan") + expect(setup.local.model.current()?.modelID).toBe("second") + setup.local.agent.set(id) + expect(setup.local.model.current()?.modelID).toBe("third") + expect(setup.local.agent.current()?.model).toEqual({ providerID: "provider", id: "first" }) + }, +) + +test.each([undefined, "provider/missing"])("retains the agent model when the CLI model is %s", async (selected) => { + await using setup = await renderLocal({ + models: [model("first"), model("second")], + args: { model: selected }, + agents: [agent("build", { providerID: "provider", id: "second" })], + }) + expect(setup.local.model.current()?.modelID).toBe("second") +}) + +test("an explicit CLI model preserves the model of a resumed session", async () => { + await using setup = await renderLocal({ + models: [model("first"), model("second")], + args: { model: "provider/second" }, + sessions: [session("ses_first", { providerID: "provider", id: "first" })], + }) + await setup.data.session.sync("ses_first") + setup.route.navigate({ type: "session", sessionID: "ses_first" }) + expect(setup.local.model.current()?.modelID).toBe("first") + expect(setup.data.session.get("ses_first")?.model).toEqual({ providerID: "provider", id: "first" }) +}) + test("switching agents restores their model and variant within the session", async () => { await using setup = await renderLocal({ models: [model("first", ["low", "high"]), model("second", ["low", "high"]), model("third", ["low", "high"])],