Keep the provider's actual model after unavailable selections (#8957)

* fix(server): keep the model selected by the provider

A live model setter can retain or resolve a different model than the requested ID. Read its effective selection before persisting manager state so an ignored selection cannot overwrite the actual model. Preserve explicit preference resets.

* fix(server): preserve completed model selections

Return the effective selection from the provider mutation so a separate status request cannot reject an already completed switch or leave configured and runtime models inconsistent.

---------

Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
2026-11-07 04:18:27 +00:00
committed by GitHub
parent c5cf6cbb4c
commit daf26892d3
10 changed files with 150 additions and 15 deletions
@@ -700,6 +700,71 @@ test("stops an OpenCode execution already active when its conversation is reopen
}
});
test("a completed OpenCode model switch does not depend on another session read", async () => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-opencode-model-"));
const harness = new V2Harness();
harness.info.location.directory = workdir;
harness.info.model = { providerID: "provider", id: "source" };
let observationUnavailable = false;
harness.api.model.list = async () => ({
location: { directory: workdir },
data: [
{
providerID: "provider",
id: "target",
modelID: "target",
name: "Target",
capabilities: { tools: true, input: ["text"], output: ["text"] },
enabled: true,
variants: [],
time: { released: 1 },
cost: [],
status: "active",
limit: { context: 200000, output: 10000 },
},
],
});
harness.api.session.switchModel = async ({ model }) => {
harness.info.model = model;
observationUnavailable = true;
};
harness.api.session.get = async () => {
if (observationUnavailable) throw new Error("Transport");
return harness.info;
};
const storage = new AgentStorage(join(workdir, "agents"), logger);
const client = new OpenCodeV2AgentClient({ logger, runtime: harness.runtime });
const manager = new AgentManager({ clients: { opencode: client }, registry: storage, logger });
const agent = await manager.resumeAgentFromPersistence({
provider: "opencode",
sessionId: "session",
nativeHandle: "session",
metadata: { cwd: workdir },
});
try {
await manager.setAgentModel(agent.id, "provider/target");
await manager.flush();
expect(harness.info.model?.id).toBe("target");
expect(manager.getAgent(agent.id)?.config.model).toBe("provider/target");
expect(manager.getAgent(agent.id)?.runtimeInfo?.model).toBe("provider/target");
const stored = await storage.get(agent.id);
expect(stored?.config?.model).toBe("provider/target");
expect(stored?.runtimeInfo?.model).toBe("provider/target");
expect(stored?.persistence?.metadata?.model).toBe("provider/target");
observationUnavailable = false;
for await (const _event of manager.streamAgent(agent.id, "continue after switching")) {
// A successful mutation leaves a usable session.
}
expect(harness.prompts).toEqual(["continue after switching"]);
} finally {
observationUnavailable = false;
await manager.closeAgent(agent.id);
await manager.flush();
await client.shutdown();
rmSync(workdir, { recursive: true, force: true });
}
});
test("uses an injected timeline store without making it a production requirement", async () => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-timeline-store-"));
const store = new RecordingTimelineStore();
@@ -5415,6 +5480,9 @@ test("model changes persist the resolved thinking and fresh provider handle befo
thinkingOptionId: null,
});
}
override async getRuntimeInfo() {
return { ...(await super.getRuntimeInfo()), ...this.selection };
}
describePersistence() {
return { provider: "codex" as const, sessionId: this.id, metadata: { ...this.selection } };
}
@@ -5497,6 +5565,63 @@ test("later explicit config mutations win over events emitted by earlier mutatio
expect(manager.getAgent(snapshot.id)?.config.thinkingOptionId).toBe("high");
});
test.each([
{ requested: "unavailable", effective: "source" },
{ requested: "alias", effective: "canonical" },
{ requested: null, effective: null },
])(
"setAgentModel stores effective selections and preserves resets for $requested",
async ({ requested, effective }) => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-effective-model-"));
class SelectingSession extends TestAgentSession {
private model: string | null = "source";
async setModel(model: string | null) {
if (model && model !== "unavailable") this.model = model === "alias" ? "canonical" : model;
return { model: this.model };
}
override async getRuntimeInfo() {
return { ...(await super.getRuntimeInfo()), model: this.model };
}
}
class SelectingClient extends TestAgentClient {
override async createSession(config: AgentSessionConfig): Promise<AgentSession> {
return new SelectingSession(config);
}
}
const storage = new AgentStorage(join(workdir, "agents"), logger);
const manager = new AgentManager({
clients: { codex: new SelectingClient() },
registry: storage,
logger,
});
try {
const snapshot = await manager.createAgent(
{ provider: "codex", cwd: workdir, model: "source" },
undefined,
{ workspaceId: undefined },
);
await manager.setAgentModel(snapshot.id, requested);
await manager.flush();
expect(manager.getAgent(snapshot.id)?.config.model).toBe(effective ?? undefined);
expect(manager.getAgent(snapshot.id)?.runtimeInfo?.model).toBe(effective);
const persisted = await storage.get(snapshot.id);
expect(persisted?.config?.model).toBe(effective ?? undefined);
expect(persisted?.runtimeInfo?.model).toBe(effective);
await manager.setAgentModel(snapshot.id, "target");
expect(manager.getAgent(snapshot.id)?.config.model).toBe("target");
expect(manager.getAgent(snapshot.id)?.runtimeInfo?.model).toBe("target");
} finally {
for (const agent of manager.listAgents()) await manager.closeAgent(agent.id);
await manager.flush();
rmSync(workdir, { recursive: true, force: true });
}
},
);
test("setAgentThinkingOption adopts the session's effective clamped level", async () => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-thinking-clamp-"));
class ClampingSession extends TestAgentSession {
@@ -2334,14 +2334,16 @@ export class AgentManager {
const normalizedModelId =
typeof modelId === "string" && modelId.trim().length > 0 ? modelId : null;
if (agent.session.setModel) {
await agent.session.setModel(normalizedModelId);
}
const selection = await agent.session.setModel?.(normalizedModelId);
await this.drainSessionEvents(agentId);
agent.config.model = normalizedModelId ?? undefined;
// A reset clears the preference even when the provider retains its active model.
let effectiveModelId = normalizedModelId;
if (normalizedModelId !== null && selection) effectiveModelId = selection.model;
agent.config.model = effectiveModelId ?? undefined;
if (agent.runtimeInfo) {
agent.runtimeInfo = { ...agent.runtimeInfo, model: normalizedModelId };
agent.runtimeInfo = { ...agent.runtimeInfo, model: effectiveModelId };
}
this.refreshSessionPersistence(agent);
this.touchUpdatedAt(agent);
@@ -720,7 +720,9 @@ export interface AgentSession {
/** Release live runtime resources without archiving or deleting the durable native session. */
close(): Promise<void>;
listCommands?(): Promise<AgentSlashCommand[]>;
setModel?(modelId: string | null): Promise<void>;
/** A returned model is the effective selection committed by this mutation.
* Returning void means the requested selection was accepted. No status read is required. */
setModel?(modelId: string | null): Promise<void | { model: string | null }>;
setThinkingOption?(thinkingOptionId: string | null): Promise<void | AgentProviderNotice>;
setFeature?(featureId: string, value: unknown): Promise<void>;
revertConversation?(input: { messageId: string }): Promise<void>;
@@ -1400,9 +1400,10 @@ class PluginAgentSession implements AgentSession {
}));
}
async setModel(modelId: string | null): Promise<void> {
async setModel(modelId: string | null): Promise<{ model: string | null }> {
this.assertReplayComplete();
await this.bridge.configure({ model: modelId });
return { model: this.bridge.config.model ?? null };
}
async setThinkingOption(thinkingOptionId: string | null): Promise<void> {
@@ -520,7 +520,7 @@ test("ACP setModel only uses config-option fallback when the matching select cho
setSessionConfigOption.mockClear();
await expect(session.setModel("new-provider-model")).resolves.toBeUndefined();
await expect(session.setModel("new-provider-model")).resolves.toEqual({ model: "sonnet" });
expect(childLogger.warn).toHaveBeenCalledWith(
{ value: "new-provider-model" },
expect.stringContaining("is not a valid claude-acp model config option"),
@@ -2281,16 +2281,17 @@ export class ACPAgentSession implements AgentSession, ACPClient {
};
}
async setModel(modelId: string | null): Promise<void> {
async setModel(modelId: string | null): Promise<{ model: string | null }> {
if (!this.connection || !this.sessionId) {
throw new Error("ACP session not initialized");
}
if (!modelId) {
this.currentModel = null;
return;
return { model: this.currentModel };
}
await this.setModelWithSelection({ modelId, selection: this.resolveModelSelection(modelId) });
return { model: this.currentModel };
}
private resolveModelSelection(modelId: string): ACPModelSelection {
@@ -2782,7 +2782,7 @@ class ClaudeAgentSession implements AgentSession {
this.currentMode = normalized;
}
async setModel(modelId: string | null): Promise<void> {
async setModel(modelId: string | null): Promise<{ model: string | null }> {
const normalizedModelId =
typeof modelId === "string" && modelId.trim().length > 0 ? modelId.trim() : null;
const activeQuery = await this.ensureQuery();
@@ -2800,6 +2800,7 @@ class ClaudeAgentSession implements AgentSession {
this.cachedRuntimeInfo = null;
// Model change affects persistence metadata, so invalidate cached handle.
this.persistence = null;
return { model: this.lastOptionsModel ?? this.config.model ?? null };
}
private reconcileThinkingOptionForModel(modelId: string | null): void {
@@ -1302,10 +1302,10 @@ export class OmpAgentSession implements AgentSession {
return null;
}
async setModel(modelId: string | null): Promise<void> {
async setModel(modelId: string | null): Promise<{ model: string | null }> {
const parsedReference = parseModelReference(modelId);
if (!parsedReference) {
return;
return { model: modelToId(this.state.model) };
}
if (!parsedReference.provider) {
throw new Error(`OMP model id must include a provider: ${modelId}`);
@@ -1314,6 +1314,7 @@ export class OmpAgentSession implements AgentSession {
const model = await this.runtimeSession.setModel(parsedReference.provider, parsedReference.id);
this.config.model = `${model.provider}/${model.id}`;
await this.refreshState();
return { model: modelToId(this.state.model) };
}
async setThinkingOption(thinkingOptionId: string | null): Promise<void> {
@@ -367,6 +367,7 @@ export class OpenCodeV2Session implements AgentSession {
thinkingOptionId: variant ?? null,
},
});
return { model: `${selected.providerID}/${selected.id}` };
}
async setThinkingOption(variant: string | null) {
@@ -1704,10 +1704,10 @@ export class PiRpcAgentSession implements AgentSession {
return null;
}
async setModel(modelId: string | null): Promise<void> {
async setModel(modelId: string | null): Promise<{ model: string | null }> {
const parsedReference = parseModelReference(modelId);
if (!parsedReference) {
return;
return { model: modelToId(this.state.model) };
}
if (!parsedReference.provider) {
throw new Error(`Pi model id must include a provider: ${modelId}`);
@@ -1716,6 +1716,7 @@ export class PiRpcAgentSession implements AgentSession {
const model = await this.runtimeSession.setModel(parsedReference.provider, parsedReference.id);
await this.refreshState();
this.config.model = `${model.provider}/${model.id}`;
return { model: modelToId(this.state.model) };
}
async setThinkingOption(thinkingOptionId: string | null): Promise<void> {