mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(agent-mode): stop a stale state_changed from reverting the picked model
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 <noreply@anthropic.com>
This commit is contained in:
parent
6b8840b197
commit
2870d072e3
2 changed files with 119 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue