fix(providers): keep the chosen model across ACP provider changes

fx's provider option also selects that provider's saved model. Paseo applied
the model before the option, so a new agent lost the requested model. It also
kept the old model as the agent's preference after a switch, and reopening the
agent sent that model to the new connection.

Apply opening configuration in the order the agent lists its options, and store
the model an option moved the session to.
This commit is contained in:
Mohamed Boudra
2026-11-09 06:21:21 +01:00
parent 106278adad
commit 5f09b3f697
4 changed files with 124 additions and 16 deletions
@@ -695,15 +695,16 @@ class AcpRuntime {
async configure(
changes: Extract<ProviderInput, { type: "session.configure" }>["changes"],
): Promise<void> {
const ordered: AcpConfigChange[] = [];
if (changes.model !== undefined) ordered.push({ target: "model", value: changes.model });
if (changes.mode !== undefined) ordered.push({ target: "mode", value: changes.mode });
const requested: AcpConfigChange[] = [];
if (changes.model !== undefined) requested.push({ target: "model", value: changes.model });
if (changes.mode !== undefined) requested.push({ target: "mode", value: changes.mode });
if (changes.thinkingOption !== undefined) {
ordered.push({ target: "thinking", value: changes.thinkingOption });
requested.push({ target: "thinking", value: changes.thinkingOption });
}
for (const [id, value] of Object.entries(changes.settings ?? {})) {
ordered.push({ target: "setting", id, value });
requested.push({ target: "setting", id, value });
}
const ordered = this.inAgentOrder(requested);
const previousModes = this.modes ? structuredClone(this.modes) : null;
const previousOptions = structuredClone(this.configOptions);
this.configTransaction = true;
@@ -769,14 +770,36 @@ class AcpRuntime {
}
private async applyInitialConfig(config: ProviderSessionConfig): Promise<void> {
if (config.model) await this.applyConfigChange({ target: "model", value: config.model });
if (config.mode) await this.applyConfigChange({ target: "mode", value: config.mode });
const requested: AcpConfigChange[] = [];
if (config.model) requested.push({ target: "model", value: config.model });
if (config.mode) requested.push({ target: "mode", value: config.mode });
if (config.thinkingOption) {
await this.applyConfigChange({ target: "thinking", value: config.thinkingOption });
requested.push({ target: "thinking", value: config.thinkingOption });
}
for (const [id, value] of Object.entries(config.settings)) {
await this.applyConfigChange({ target: "setting", id, value });
requested.push({ target: "setting", id, value });
}
for (const change of this.inAgentOrder(requested)) await this.applyConfigChange(change);
}
// One option can reset another: choosing an fx provider also selects that provider's saved
// model, so a model applied first would be lost. Agents list the option that decides first.
private inAgentOrder(changes: readonly AcpConfigChange[]): AcpConfigChange[] {
const position = (change: AcpConfigChange): number => {
const configId = this.configIdFor(change);
const index = this.configOptions.findIndex((option) => option.id === configId);
return index === -1 ? this.configOptions.length : index;
};
return changes.toSorted((left, right) => position(left) - position(right));
}
private configIdFor(change: AcpConfigChange): string | undefined {
if (change.target === "setting") return change.id;
if (change.target === "model") return modelOption(this.configOptions)?.id;
if (change.target === "thinking") {
return this.configOptions.find((option) => option.category === "thought_level")?.id;
}
return this.configOptions.find((option) => option.category === "mode")?.id;
}
private async applyConfigChange(change: AcpConfigChange): Promise<void> {
@@ -804,12 +827,7 @@ class AcpRuntime {
this.modes = { ...this.modes, currentModeId: change.value };
return;
}
let configId: string | undefined;
if (change.target === "setting") configId = change.id;
if (change.target === "model") configId = modelOption(this.configOptions)?.id;
if (change.target === "thinking") {
configId = this.configOptions.find((option) => option.category === "thought_level")?.id;
}
const configId = this.configIdFor(change);
if (!configId) {
throw new Error(`ACP session does not expose ${change.target} configuration`);
}
+37
View File
@@ -1211,6 +1211,43 @@ lines.on("line", (line) => {
await connection.close();
});
it("applies opening configuration in the order the agent lists its options", async () => {
const executable = await fakeAcp(`const readline = require("node:readline");
const lines = readline.createInterface({ input: process.stdin });
const send = (message) => process.stdout.write(JSON.stringify(message) + "\\n");
let provider = "first";
let model = "first-saved";
const saved = { first: "first-saved", second: "second-saved" };
const select = (id, currentValue, values) => ({ id, name: id, category: "model", type: "select", currentValue, options: values.map((value) => ({ name: value, value })) });
const options = () => [
select("provider", provider, ["first", "second"]),
select("model", model, ["first-saved", "second-saved", "second-chosen"]),
];
lines.on("line", (line) => {
const message = JSON.parse(line);
if (message.method === "initialize") send({ jsonrpc: "2.0", id: message.id, result: { protocolVersion: message.params.protocolVersion, agentCapabilities: {} } });
else if (message.method === "session/new") send({ jsonrpc: "2.0", id: message.id, result: { sessionId: "native-1", configOptions: options() } });
else if (message.method === "session/set_config_option") {
if (message.params.configId === "provider") { provider = message.params.value; model = saved[provider]; }
if (message.params.configId === "model") model = message.params.value;
send({ jsonrpc: "2.0", id: message.id, result: { configOptions: options() } });
}
});`);
const { connection, events } = await connect(executable, [
"prompt.message",
"session.configure",
]);
const input = openInput({ provider: "second" });
await connection.send({ ...input, config: { ...input.config, model: "second-chosen" } });
await waitForEvent(events, (event) => event.type === "session.ready");
expect(events.findLast((event) => event.type === "session.config")).toMatchObject({
config: { model: "second-chosen" },
});
await connection.close();
});
it("gives discovery transformers the discovered session configuration facade", async () => {
const executable = await fakeAcp(`const readline = require("node:readline");
const lines = readline.createInterface({ input: process.stdin });
@@ -13511,6 +13511,9 @@ describe("agent options", () => {
/** What the session says it ended up with after the next apply. */
ended: Record<string, unknown> = {};
openOptionsPatch: Record<string, unknown> = {};
runtimeModel: string | null = null;
/** The model the next apply moves the session to, as a provider's connection option does. */
movesModelTo: string | null = null;
async applyOptions(options: ProviderOptions): Promise<Record<string, unknown>> {
if (this.rejectWith) throw this.rejectWith;
@@ -13518,13 +13521,20 @@ describe("agent options", () => {
this.applyGate = null;
await gate;
this.applied.push(options);
if (this.movesModelTo) this.runtimeModel = this.movesModelTo;
return this.ended;
}
override async getRuntimeInfo() {
const info = await super.getRuntimeInfo();
return this.runtimeModel ? { ...info, model: this.runtimeModel } : info;
}
}
class OptionsClient extends TestAgentClient {
session: OptionsSession | null = null;
openOptionsPatch: Record<string, unknown> = {};
runtimeModel: string | null = null;
// Stands in for the `config.json` provider defaults the registry wrapper merges in.
effectiveOptions(own: ProviderOptions | undefined): ProviderOptions {
@@ -13534,6 +13544,7 @@ describe("agent options", () => {
override async createSession(config: AgentSessionConfig): Promise<AgentSession> {
this.session = new OptionsSession(config);
this.session.openOptionsPatch = this.openOptionsPatch;
this.session.runtimeModel = this.runtimeModel;
return this.session;
}
}
@@ -13541,14 +13552,21 @@ describe("agent options", () => {
async function createOptionsAgent(
providerOptions?: ProviderOptions,
openOptionsPatch: Record<string, unknown> = {},
opened: { model?: string; runtimeModel?: string } = {},
) {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-options-"));
const storage = new AgentStorage(join(workdir, "agents"), logger);
const client = new OptionsClient();
client.openOptionsPatch = openOptionsPatch;
client.runtimeModel = opened.runtimeModel ?? null;
const manager = new AgentManager({ clients: { codex: client }, registry: storage, logger });
const agent = await manager.createAgent(
{ provider: "codex", cwd: workdir, ...(providerOptions ? { providerOptions } : {}) },
{
provider: "codex",
cwd: workdir,
...(opened.model ? { model: opened.model } : {}),
...(providerOptions ? { providerOptions } : {}),
},
undefined,
{ workspaceId: undefined },
);
@@ -13637,6 +13655,33 @@ describe("agent options", () => {
});
});
test("an option that moves the agent to another model reopens on that model", async () => {
const { manager, client, storage, agentId } = await createOptionsAgent(
undefined,
{},
{ model: "first-connection-model" },
);
client.session!.movesModelTo = "second-connection-model";
await manager.setAgentOptions(agentId, { web_search: true });
expect(manager.getAgent(agentId)?.config.model).toBe("second-connection-model");
await manager.flush();
expect((await storage.get(agentId))?.config?.model).toBe("second-connection-model");
});
test("an option that leaves the model alone keeps the model the user chose", async () => {
const { manager, agentId } = await createOptionsAgent(
undefined,
{},
{ model: "alias", runtimeModel: "canonical" },
);
await manager.setAgentOptions(agentId, { web_search: true });
expect(manager.getAgent(agentId)?.config.model).toBe("alias");
});
test("a patch key is resolved against the configured defaults, merging nested objects", async () => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-options-"));
const client = new OptionsClient();
@@ -2506,9 +2506,17 @@ export class AgentManager {
const set = Object.fromEntries(Object.entries(patch).filter(([, value]) => value != null));
const resolved = this.effectiveOptionsFor(agent.provider, set);
sent = Object.fromEntries(Object.keys(patch).map((key) => [key, resolved[key] ?? null]));
const modelBefore = agent.runtimeInfo?.model;
ended = (await agent.session.applyOptions(sent)) ?? {};
// Changes the provider made on its own while applying are older than its final answer.
await this.drainSessionEvents(agentId);
await this.refreshRuntimeInfo(agent, { emit: false });
// An option can choose the model too, as an fx connection does. Reopening the agent on
// the earlier preference would send that model to the new connection.
const modelAfter = agent.runtimeInfo?.model;
if (agent.config.model && modelAfter && modelAfter !== modelBefore) {
agent.config.model = modelAfter;
}
}
this.storeOptionsPatch({ agent, patch: { ...patch, ...ended } });
this.updateEffectiveOptions({ agent, sent, reported: ended });