From 2870d072e321dd61d1d1a5fcf9af4461aafa5114 Mon Sep 17 00:00:00 2001 From: Zero Liu Date: Tue, 7 Jul 2026 15:41:53 -0700 Subject: [PATCH] fix(agent-mode): stop a stale state_changed from reverting the picked model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opencode (an ACP subprocess) broadcasts config_option_update notifications that the ACP layer synthesizes into state_changed events. One can race a model switch and carry the pre-switch default; because handleSessionEvent applied state_changed unconditionally, the late push clobbered the user's just-applied model back to the default — intermittently. These backends never self-switch the model, so a state_changed that regresses the model away from the last user-applied selection is a stale echo. Track the last applied model and, when an incoming state reverts it (and the backend still offers it), keep it; every other dimension (effort, mode, availableModels) stays authoritative. Co-Authored-By: Claude Opus 4.8 --- src/agentMode/session/AgentSession.test.ts | 81 ++++++++++++++++++++++ src/agentMode/session/AgentSession.ts | 39 ++++++++++- 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/agentMode/session/AgentSession.test.ts b/src/agentMode/session/AgentSession.test.ts index d02d1f1f..7be314fd 100644 --- a/src/agentMode/session/AgentSession.test.ts +++ b/src/agentMode/session/AgentSession.test.ts @@ -1798,6 +1798,87 @@ describe("AgentSession.create (via start)", () => { expect(session.getState()?.model?.current.baseModelId).toBe("sonnet"); }); + it("keeps the user-applied model when a late state_changed would revert it", async () => { + // Regression: opencode's config_option_update broadcasts (synthesized into + // state_changed) can race a switch and carry the pre-switch default; the old + // unconditional overwrite clobbered the user's pick back to the default. + const mock = makeMockBackend(); + const reported: BackendState = { + model: { + current: { baseModelId: "gpt-5", effort: null }, + apply: { kind: "setModel" }, + availableModels: [ + { baseModelId: "gpt-5", name: "GPT-5", provider: "openai", effortOptions: [] }, + { baseModelId: "sonnet", name: "Sonnet", provider: "anthropic", effortOptions: [] }, + ], + }, + mode: null, + }; + const switched: BackendState = { + model: { ...reported.model!, current: { baseModelId: "sonnet", effort: null } }, + mode: null, + }; + mock.newSession.mockResolvedValueOnce({ sessionId: "acp-1", state: reported }); + mock.setSessionModel.mockResolvedValueOnce(switched); + const session = AgentSession.start({ + backend: mock.asBackend, + cwd: "/vault", + internalId: "internal-1", + backendId: "opencode", + defaultModelSelection: { baseModelId: "sonnet", effort: null }, + getDescriptor: () => makeWireOnlyDescriptor(), + }); + await session.ready; + expect(session.getState()?.model?.current.baseModelId).toBe("sonnet"); + mock.emit({ sessionId: "acp-1", update: { sessionUpdate: "state_changed", state: reported } }); + expect(session.getState()?.model?.current.baseModelId).toBe("sonnet"); + }); + + it("honors a state_changed that keeps the applied model but changes effort", async () => { + const mock = makeMockBackend(); + const reported: BackendState = { + model: { + current: { baseModelId: "gpt-5", effort: null }, + apply: { kind: "setModel" }, + availableModels: [ + { baseModelId: "gpt-5", name: "GPT-5", provider: "openai", effortOptions: [] }, + { + baseModelId: "sonnet", + name: "Sonnet", + provider: "anthropic", + effortOptions: [{ value: "high", label: "High" }], + }, + ], + }, + mode: null, + }; + const switched: BackendState = { + model: { ...reported.model!, current: { baseModelId: "sonnet", effort: null } }, + mode: null, + }; + mock.newSession.mockResolvedValueOnce({ sessionId: "acp-1", state: reported }); + mock.setSessionModel.mockResolvedValueOnce(switched); + const session = AgentSession.start({ + backend: mock.asBackend, + cwd: "/vault", + internalId: "internal-1", + backendId: "opencode", + defaultModelSelection: { baseModelId: "sonnet", effort: null }, + getDescriptor: () => makeWireOnlyDescriptor(), + }); + await session.ready; + const withEffort: BackendState = { + model: { ...switched.model!, current: { baseModelId: "sonnet", effort: "high" } }, + mode: null, + }; + mock.emit({ + sessionId: "acp-1", + update: { sessionUpdate: "state_changed", state: withEffort }, + }); + expect(session.getState()?.model?.current.baseModelId).toBe("sonnet"); + expect(session.getState()?.model?.current.effort).toBe("high"); + }); + it("seeds config-option opencode effort via the effort option, not the model id", async () => { // Regression: a cross-backend pick to config-option opencode (≥1.15.13) // must set the bare model on the model config option and the effort on the diff --git a/src/agentMode/session/AgentSession.ts b/src/agentMode/session/AgentSession.ts index 7b7b2e0e..4408d732 100644 --- a/src/agentMode/session/AgentSession.ts +++ b/src/agentMode/session/AgentSession.ts @@ -348,6 +348,12 @@ export class AgentSession { * `setSession*` responses. */ private currentState: BackendState | null = null; + /** + * The model base id the user/seed last explicitly applied via setModel / + * setConfigOption. Used to reject a stale `state_changed` push that would + * silently revert it (these backends never self-switch the model). + */ + private lastAppliedModelBaseId: string | null = null; private label: string | null = null; // Tracks who set the current label so an agent-pushed `session_info_update` // can't clobber a label the user explicitly chose via Rename. @@ -559,9 +565,39 @@ export class AgentSession { modelId, }); this.currentState = next; + this.rememberAppliedModel(next); this.notifyModelChanged(); } + /** Record the model just applied so a stale backend push can't revert it. */ + private rememberAppliedModel(state: BackendState): void { + const base = state.model?.current.baseModelId; + if (base) this.lastAppliedModelBaseId = base; + } + + /** + * A `state_changed` push must not silently revert the user's just-applied + * model. opencode broadcasts config_option_update notifications (synthesized + * into state_changed by the ACP layer) that can race a model switch and carry + * the pre-switch default. These backends never self-switch the model, so when + * an incoming state regresses the model away from the last applied selection — + * and the backend still offers that model — keep it. Every other dimension + * (effort, mode, availableModels) stays authoritative. + */ + private reconcileAppliedModel(incoming: BackendState): BackendState { + const applied = this.lastAppliedModelBaseId; + if (applied === null || !incoming.model) return incoming; + if (incoming.model.current.baseModelId === applied) return incoming; + if (!incoming.model.availableModels.some((m) => m.baseModelId === applied)) return incoming; + return { + ...incoming, + model: { + ...incoming.model, + current: { ...incoming.model.current, baseModelId: applied }, + }, + }; + } + /** * Apply an encoded model wire id through whichever channel the current model * state declares. Backends whose catalog comes from a `category:"model"` @@ -652,6 +688,7 @@ export class AgentSession { value, }); this.currentState = next; + this.rememberAppliedModel(next); this.notifyModelChanged(); this.clearCurrentPlanIfModeLeft(); } @@ -1541,7 +1578,7 @@ export class AgentSession { return; } if (update.sessionUpdate === "state_changed") { - this.currentState = update.state; + this.currentState = this.reconcileAppliedModel(update.state); this.notifyModelChanged(); this.clearCurrentPlanIfModeLeft(); return;