From 940dbfd242ce132d18113fa596c6115d8e96e442 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 28 Sep 2026 22:32:35 +0100 Subject: [PATCH] fix(opencode): clear unsupported thinking when changing models (#5587) Resolve retained variants against the destination model before applying a model change. Publish the effective thinking choice and refresh the provider resume handle immediately so a reload cannot restore the old selection. --- .../opencode-model-switch.real.spec.ts | 78 +++++++++++++++++ .../src/server/agent/agent-manager.test.ts | 47 ++++++++++ .../server/src/server/agent/agent-manager.ts | 1 + .../agent/providers/opencode-agent.test.ts | 78 ++++++++++++++++- .../server/agent/providers/opencode-agent.ts | 21 +++++ .../agent/providers/opencode/v2/agent.test.ts | 85 ++++++++++++++++++- .../agent/providers/opencode/v2/session.ts | 47 +++++++--- 7 files changed, 343 insertions(+), 14 deletions(-) create mode 100644 packages/app/e2e/browser/opencode-model-switch.real.spec.ts diff --git a/packages/app/e2e/browser/opencode-model-switch.real.spec.ts b/packages/app/e2e/browser/opencode-model-switch.real.spec.ts new file mode 100644 index 0000000000..e83a962fb4 --- /dev/null +++ b/packages/app/e2e/browser/opencode-model-switch.real.spec.ts @@ -0,0 +1,78 @@ +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { test, expect, type Page } from "../support/fixtures"; +import { launchAgent, cleanupRewindFlow, type AgentHandle } from "../support/helpers/rewind-flow"; +import { submitMessage } from "../support/helpers/composer"; + +const binary = process.env.OPENCODE_TEST_BINARY ?? "opencode"; +test.use({ + e2eDaemonConfig: { + version: 1, + agents: { providers: { opencode: { enabled: true, command: [binary] } } }, + }, +}); + +async function selectMedium(page: Page) { + await page + .getByRole("button", { name: /^Select thinking option/ }) + .filter({ visible: true }) + .click(); + await page.getByText("Medium", { exact: true }).filter({ visible: true }).click(); + await expect( + page + .getByRole("button", { name: "Select thinking option (Medium)", exact: true }) + .filter({ visible: true }), + ).toBeVisible(); +} +async function selectLongcat(page: Page) { + await page + .getByRole("button", { name: /^Select model \(/ }) + .filter({ visible: true }) + .click(); + await page + .getByText("LongCat 2.5 Preview Free", { exact: true }) + .filter({ visible: true }) + .click(); +} +async function askForReply(handle: AgentHandle) { + await submitMessage(handle.page, "Reply exactly OK. Do not use tools."); + return handle.client.waitForFinish(handle.agentId, 120_000); +} + +test("switching an existing OpenCode session clears unsupported thinking", async ({ + page, +}, testInfo) => { + test.setTimeout(240_000); + const cwd = mkdtempSync(path.join(tmpdir(), "paseo-variant-ui-")); + let handle: AgentHandle | undefined; + try { + handle = await launchAgent({ + page, + cwd, + provider: "opencode", + mode: "full-access", + providerConfig: { model: "opencode/space-bunny-free" }, + }); + await selectMedium(page); + expect((await askForReply(handle)).status).toBe("idle"); + await selectLongcat(page); + await expect( + page.getByRole("button", { name: /^Select thinking option/ }).filter({ visible: true }), + ).toHaveCount(0); + const finish = await askForReply(handle); + const screenshot = testInfo.outputPath("model-switch.png"); + await page.screenshot({ path: screenshot }); + await testInfo.attach("After switching to Longcat", { + path: screenshot, + contentType: "image/png", + }); + expect(finish.status).toBe("idle"); + expect(finish.final?.lastError).toBeFalsy(); + await expect( + page.getByTestId("assistant-message").filter({ visible: true }).last(), + ).toContainText("OK"); + } finally { + await cleanupRewindFlow({ handle, cwd }); + } +}); diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 0af62655ae..2f9cd425ae 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -4851,6 +4851,53 @@ test("persists live mode, model, and thinking changes without an external snapsh expect(persisted?.runtimeInfo?.model).toBe("gpt-5.4"); }); +test("model changes persist the resolved thinking and fresh provider handle before another turn", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-model-selection-")); + class ModelSession extends TestAgentSession { + selection = { model: "source", thinkingOptionId: "medium" as string | undefined }; + async setModel(model: string | null) { + this.selection = { model: model ?? "default", thinkingOptionId: undefined }; + this.pushEvent({ + type: "thinking_option_changed", + provider: "codex", + thinkingOptionId: null, + }); + } + describePersistence() { + return { provider: "codex" as const, sessionId: this.id, metadata: { ...this.selection } }; + } + } + class ModelClient extends TestAgentClient { + async createSession(config: AgentSessionConfig) { + return new ModelSession(config); + } + } + const storage = new AgentStorage(join(workdir, "agents"), logger); + const manager = new AgentManager({ + clients: { codex: new ModelClient() }, + registry: storage, + logger, + }); + try { + const agent = await manager.createAgent( + { provider: "codex", cwd: workdir, model: "source", thinkingOptionId: "medium" }, + undefined, + { workspaceId: undefined }, + ); + await manager.setAgentModel(agent.id, "target"); + await manager.flush(); + const stored = await storage.get(agent.id); + expect(stored?.config?.model).toBe("target"); + expect(stored?.config?.thinkingOptionId).toBeUndefined(); + expect(stored?.persistence?.metadata?.model).toBe("target"); + expect(stored?.persistence?.metadata?.thinkingOptionId).toBeUndefined(); + } finally { + for (const agent of manager.listAgents()) await manager.closeAgent(agent.id); + await manager.flush(); + rmSync(workdir, { recursive: true, force: true }); + } +}); + test("later explicit config mutations win over events emitted by earlier mutations", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-config-mutation-order-")); class ConfigMutationSession extends TestAgentSession { diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 061c00e2ca..87d3b290aa 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -1943,6 +1943,7 @@ export class AgentManager { if (agent.runtimeInfo) { agent.runtimeInfo = { ...agent.runtimeInfo, model: normalizedModelId }; } + this.refreshSessionPersistence(agent); this.touchUpdatedAt(agent); this.emitState(agent); } diff --git a/packages/server/src/server/agent/providers/opencode-agent.test.ts b/packages/server/src/server/agent/providers/opencode-agent.test.ts index 9406ddbce4..0634ec25cb 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.test.ts @@ -333,6 +333,76 @@ describe("OpenCodeAgentClient adapter smoke tests", () => { model: TEST_MODEL, }); + test.each([ + { variants: { medium: {}, custom: {} }, selected: "medium", expected: "medium" }, + { variants: { custom: {} }, selected: "custom", expected: "custom" }, + { variants: { high: {} }, selected: "medium", expected: undefined }, + { variants: {}, selected: "medium", expected: undefined }, + ])( + "model switches resolve the retained variant: $selected / $variants", + async ({ variants, selected, expected }) => { + const cwd = tmpCwd(); + const runtime = new TestOpenCodeHarness(); + const upstream = new TestOpenCodeClient(); + upstream.providerListResponse = { + data: { + connected: ["test"], + all: [ + { + id: "test", + name: "Test", + source: "api", + models: { target: { name: "Target", variants } }, + }, + ], + }, + }; + runtime.enqueueClient(upstream); + const client = new OpenCodeAgentClient(logger, undefined, { + serverManager: runtime, + createClient: runtime.createClient, + }); + const session = await client.createSession({ + provider: "opencode", + cwd, + model: "test/source", + thinkingOptionId: selected, + }); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + try { + await session.setModel!("test/target"); + expect((await session.getRuntimeInfo()).thinkingOptionId).toBe(expected ?? null); + expect(events).toContainEqual({ + type: "thinking_option_changed", + provider: "opencode", + thinkingOptionId: expected ?? null, + }); + upstream.sessionPromptAsyncEvents = [ + { type: "session.idle", properties: { sessionID: "session-1" } }, + ]; + await collectTurnEvents(streamSession(session, "Use the selected model")); + expect(upstream.calls.sessionPromptAsync).toEqual([ + expect.objectContaining({ + model: { providerID: "test", modelID: "target" }, + ...(expected ? { variant: expected } : {}), + }), + ]); + if (!expected) expect(upstream.calls.sessionPromptAsync[0]).not.toHaveProperty("variant"); + await session.setThinkingOption!("medium"); + await expect(session.setModel!("test/missing")).rejects.toThrow( + "OpenCode model unavailable", + ); + upstream.providerListResponse = { error: "Catalog unavailable" }; + await expect(session.setModel!("test/source")).rejects.toThrow("Catalog unavailable"); + expect((await session.getRuntimeInfo()).model).toBe("test/target"); + } finally { + await session.close(); + rmSync(cwd, { recursive: true, force: true }); + } + }, + ); + test("usage reference follows the active OpenCode model and OAuth account", async () => { const cwd = tmpCwd(); const runtime = new TestOpenCodeHarness(); @@ -386,7 +456,13 @@ describe("OpenCodeAgentClient adapter smoke tests", () => { expect(events).toContainEqual({ type: "model_changed", provider: "opencode", - runtimeInfo: { provider: "opencode", sessionId, model: "openai/gpt-5", modeId: null }, + runtimeInfo: { + provider: "opencode", + sessionId, + model: "openai/gpt-5", + modeId: null, + thinkingOptionId: null, + }, }); await session.close(); rmSync(cwd, { recursive: true, force: true }); diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index e92a424ee6..b2cab99c6e 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -3616,6 +3616,7 @@ class OpenCodeAgentSession implements AgentSession { sessionId: this.sessionId, model: this.config.model ?? null, modeId: this.currentMode, + thinkingOptionId: this.config.thinkingOptionId ?? null, }; } @@ -3627,12 +3628,32 @@ class OpenCodeAgentSession implements AgentSession { } async setModel(modelId: string | null): Promise { + await this.reconnectIfServerExited(); const normalizedModelId = typeof modelId === "string" && modelId.trim().length > 0 ? modelId : null; + let variant = this.config.thinkingOptionId; + if (!normalizedModelId) { + variant = undefined; + } else if (variant) { + const model = this.parseModel(normalizedModelId); + const response = await this.client.provider.list({ directory: this.config.cwd }); + if (response.error) + throw new Error(`Failed to fetch OpenCode providers: ${JSON.stringify(response.error)}`); + const provider = response.data?.all.find((entry) => entry.id === model?.providerID); + const target = model && provider?.models[model.modelID]; + if (!target) throw new Error(`OpenCode model unavailable: ${normalizedModelId}`); + if (!Object.hasOwn(target.variants ?? {}, variant)) variant = undefined; + } this.config.model = normalizedModelId ?? undefined; + this.config.thinkingOptionId = variant; this.selectedModelContextWindowMaxTokens = this.resolveConfiguredModelContextWindowMaxTokens( this.config.model, ); + this.notifySubscribers({ + type: "thinking_option_changed", + provider: "opencode", + thinkingOptionId: variant ?? null, + }); } async setThinkingOption(thinkingOptionId: string | null): Promise { diff --git a/packages/server/src/server/agent/providers/opencode/v2/agent.test.ts b/packages/server/src/server/agent/providers/opencode/v2/agent.test.ts index 9a0e696e32..29223073bb 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/agent.test.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/agent.test.ts @@ -1,4 +1,4 @@ -import type { SessionMessageInfo } from "@opencode/client"; +import type { ModelInfo, SessionMessageInfo } from "@opencode/client"; import { describe, expect, test } from "vitest"; import { createTestLogger } from "../../../../../test-utils/test-logger.js"; @@ -7,6 +7,89 @@ import { OpenCodeV2AgentClient } from "./agent.js"; import { V2Harness } from "../test-utils/v2-harness.js"; describe("OpenCode v2 session lifecycle", () => { + test.each([ + { variants: ["medium", "custom"], selected: "medium", expected: "medium" }, + { variants: ["custom"], selected: "custom", expected: "custom" }, + { variants: ["high"], selected: "medium", expected: undefined }, + { variants: [], selected: "medium", expected: undefined }, + { variants: ["medium"], selected: "default", expected: undefined }, + ])( + "model switches retain only supported variants: $selected / $variants", + async ({ variants, selected, expected }) => { + const harness = new V2Harness(); + const target: ModelInfo = { + id: "target", + modelID: "target", + providerID: "test", + name: "Target", + capabilities: { tools: true, input: ["text"], output: ["text"] }, + variants: variants.map((id) => ({ id })), + time: { released: 1 }, + cost: [], + status: "active", + enabled: true, + limit: { context: 200000, output: 10000 }, + }; + harness.info.model = { providerID: "test", id: "source", variant: selected }; + harness.api.model.list = async () => ({ location: harness.info.location, data: [target] }); + harness.api.session.switchModel = async ({ model }) => { + harness.info.model = model; + }; + const client = new OpenCodeV2AgentClient({ + logger: createTestLogger(), + runtime: harness.runtime, + }); + const session = await client.createSession({ + provider: "opencode", + cwd: "/tmp/project", + model: "test/source", + thinkingOptionId: selected, + }); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + try { + await session.setModel!("test/target"); + expect(await session.getRuntimeInfo()).toMatchObject({ + model: "test/target", + thinkingOptionId: expected ?? null, + }); + expect(session.describePersistence().metadata?.thinkingOptionId).toBe(expected); + expect(events).toContainEqual({ + type: "thinking_option_changed", + provider: "opencode", + thinkingOptionId: expected ?? null, + }); + await expect(session.setModel!("test/missing")).rejects.toThrow( + "OpenCode model unavailable", + ); + harness.api.session.switchModel = async () => { + throw new Error("Switch failed"); + }; + await expect(session.setModel!("test/target")).rejects.toThrow("Switch failed"); + expect(session.describePersistence().metadata?.thinkingOptionId).toBe(expected); + expect(harness.info.model).toEqual({ + providerID: "test", + id: "target", + ...(expected ? { variant: expected } : {}), + }); + harness.api.session.switchModel = async ({ model }) => { + harness.info.model = model; + }; + harness.api.model.default = async () => ({ location: harness.info.location, data: target }); + await session.setThinkingOption!("unsupported"); + await session.setModel!(null); + expect((await session.getRuntimeInfo()).thinkingOptionId).toBeNull(); + harness.api.model.list = async () => { + throw new Error("Catalog unavailable"); + }; + await expect(session.setModel!("test/source")).rejects.toThrow("Catalog unavailable"); + expect((await session.getRuntimeInfo()).model).toBe("test/target"); + } finally { + await session.close(); + } + }, + ); + test("restores each same-directory agent environment on reconnect and resume", async () => { const first = new V2Harness(); const second = new V2Harness(); diff --git a/packages/server/src/server/agent/providers/opencode/v2/session.ts b/packages/server/src/server/agent/providers/opencode/v2/session.ts index 315e75410c..60ede6b3f5 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/session.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/session.ts @@ -276,25 +276,48 @@ export class OpenCodeV2Session implements AgentSession { } async setModel(model: string | null) { await this.reconnectIfExited(); - const selected = model - ? modelRef(model, this.config.thinkingOptionId) - : (await this.client.model.default({ location: { directory: this.config.cwd } })).data; + const location = { directory: this.config.cwd }; + const selected = model ? modelRef(model) : (await this.client.model.default({ location })).data; if (!selected) throw new Error("OpenCode has no default model"); - await this.client.session.switchModel({ - sessionID: this.id, - model: { - id: selected.id, - providerID: selected.providerID, - variant: this.config.thinkingOptionId, - }, - }); + const catalog = await this.client.model.list({ location }); + const target = catalog.data.find( + (entry) => + entry.enabled && entry.providerID === selected.providerID && entry.id === selected.id, + ); + if (!target) + throw new Error(`OpenCode model unavailable: ${selected.providerID}/${selected.id}`); + const retainedVariant = this.config.thinkingOptionId; + const variant = + retainedVariant !== "default" && target.variants.some((entry) => entry.id === retainedVariant) + ? retainedVariant + : undefined; + const nextModel = { + id: selected.id, + providerID: selected.providerID, + ...(variant ? { variant } : {}), + }; + await this.client.session.switchModel({ sessionID: this.id, model: nextModel }); + this.info.model = nextModel; this.config.model = model ?? undefined; + this.config.thinkingOptionId = variant; + this.emit({ + type: "thinking_option_changed", + provider: "opencode", + thinkingOptionId: variant ?? null, + }); this.emit({ type: "model_changed", provider: "opencode", - runtimeInfo: await this.getRuntimeInfo(), + runtimeInfo: { + provider: "opencode", + sessionId: this.id, + model: `${selected.providerID}/${selected.id}`, + modeId: this.info.agent ?? null, + thinkingOptionId: variant ?? null, + }, }); } + async setThinkingOption(variant: string | null) { await this.reconnectIfExited(); const model = this.info.model;