From fe76087854374213afbdc42dcf2468cb4d7a8d28 Mon Sep 17 00:00:00 2001 From: Josh Duffney Date: Tue, 29 Sep 2026 10:09:43 -0500 Subject: [PATCH 1/4] fix(tests): select advertised ACP model Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../coder-acp-copilot/src/acp-client.test.ts | 75 ++++++++++++++++++- .../coder-acp-copilot/src/acp-client.ts | 62 +++++++++++++-- .../src/copilot-cli.integration.test.ts | 41 ++++++---- .../coder-acp-copilot/src/test-worker.ts | 13 +++- 4 files changed, 166 insertions(+), 25 deletions(-) diff --git a/apps/workers/coder-acp-copilot/src/acp-client.test.ts b/apps/workers/coder-acp-copilot/src/acp-client.test.ts index f4a126ca7..827ca12ee 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.test.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.test.ts @@ -2,7 +2,18 @@ // Licensed under the MIT License. import { describe, it, expect, vi } from "vitest"; -import { runACPSession, selectModel, selectReasoningEffort, selectPermissionMode, formatModeError, formatToolArgs, formatToolContent, AUTOPILOT_MODE_ID } from "./acp-client.js"; +import { + runACPSession, + selectModel, + resolveRequestedModel, + selectFirstAvailableNonDefaultModel, + selectReasoningEffort, + selectPermissionMode, + formatModeError, + formatToolArgs, + formatToolContent, + AUTOPILOT_MODE_ID, +} from "./acp-client.js"; import type * as acp from "@agentclientprotocol/sdk"; import os from "node:os"; @@ -288,6 +299,68 @@ describe("selectModel", () => { }); }); +describe("selectFirstAvailableNonDefaultModel", () => { + function makeSession( + overrides?: Partial + ): acp.NewSessionResponse { + return { + sessionId: "session-1", + ...overrides, + } as acp.NewSessionResponse; + } + + it("selects the first advertised model different from the current model", () => { + const session = makeSession({ + models: { + currentModelId: "default-model", + availableModels: [ + { modelId: "default-model", name: "Default Model" }, + { modelId: "alternate-model", name: "Alternate Model" }, + { modelId: "another-model", name: "Another Model" }, + ], + }, + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBe( + "alternate-model" + ); + }); + + it("returns undefined when the session does not advertise models", () => { + expect(selectFirstAvailableNonDefaultModel(makeSession())).toBeUndefined(); + }); + + it("returns undefined when only the current model is available", () => { + const session = makeSession({ + models: { + currentModelId: "default-model", + availableModels: [ + { modelId: "default-model", name: "Default Model" }, + ], + }, + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBeUndefined(); + }); +}); + +describe("resolveRequestedModel", () => { + const session = { + sessionId: "session-1", + } as acp.NewSessionResponse; + + it("preserves a fixed model id", () => { + expect(resolveRequestedModel("fixed-model", session)).toBe("fixed-model"); + }); + + it("resolves a selector with the new session response", () => { + const selector = vi.fn().mockReturnValue("selected-model"); + + expect(resolveRequestedModel(selector, session)).toBe("selected-model"); + expect(selector).toHaveBeenCalledWith(session); + }); +}); + describe("selectReasoningEffort", () => { function makeConnection(overrides?: Partial): acp.ClientSideConnection { return { diff --git a/apps/workers/coder-acp-copilot/src/acp-client.ts b/apps/workers/coder-acp-copilot/src/acp-client.ts index 4116977ba..6f00f0e9a 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.ts @@ -127,8 +127,8 @@ export interface ACPClientOptions { mcpServers?: McpServerConfig[]; /** Session timeout in milliseconds (default: 30 min). Set to 0 to disable. */ sessionTimeoutMs?: number; - /** Model to select after the session is created (e.g. "gpt-5.4"). */ - model?: string; + /** Model to select after the session is created, or a selector resolved from the new session response. */ + model?: string | ACPModelSelector; /** Reasoning effort level to apply via ACP config option (e.g. "low", "medium", "high"). */ reasoningEffort?: string; /** Use shell to spawn the process (required on Windows for .cmd shim resolution). */ @@ -142,6 +142,36 @@ export interface ACPSessionResult { confirmedModel?: string; } +export type ACPModelSelector = ( + sessionResult: acp.NewSessionResponse +) => string | undefined; + +export function resolveRequestedModel( + model: string | ACPModelSelector | undefined, + sessionResult: acp.NewSessionResponse +): string | undefined { + return typeof model === "function" ? model(sessionResult) : model; +} + +/** + * Choose the first advertised model that differs from the session default. + * + * Intended for integration coverage that must exercise model switching without + * depending on a server-controlled model id. + */ +export function selectFirstAvailableNonDefaultModel( + sessionResult: acp.NewSessionResponse +): string | undefined { + const models = sessionResult.models; + if (!models) { + return undefined; + } + + return models.availableModels.find( + (candidate) => candidate.modelId !== models.currentModelId + )?.modelId; +} + /** * ACP Client implementation that handles permission requests and session updates */ @@ -571,10 +601,32 @@ export async function runACPSession( onLog(`Session config options: ${sessionResult.configOptions.map((o: acp.SessionConfigOption) => `${o.id}${o.category ? ` (${o.category})` : ""}`).join(", ")}`); } - // Select model if requested + // Select model if requested. A selector is resolved only after newSession + // because the current and available models are session-scoped. let confirmedModel: string | undefined; - if (model) { - confirmedModel = await selectModel(connection, sessionResult, model, onLog); + const requestedModel = resolveRequestedModel(model, sessionResult); + if (requestedModel) { + confirmedModel = await selectModel( + connection, + sessionResult, + requestedModel, + onLog + ); + } else if (typeof model === "function") { + const hasModelCapability = + sessionResult.models !== undefined || + sessionResult.configOptions?.some( + (option: acp.SessionConfigOption) => option.category === "model" + ) === true; + if (hasModelCapability) { + onLog( + "Warning: model selector did not choose a model from the advertised model options" + ); + } else { + onLog( + "Warning: agent does not advertise model selection capability (no models field or model config option)" + ); + } } // Set reasoning effort if requested diff --git a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts index 61b43081f..45359d600 100644 --- a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts +++ b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts @@ -89,7 +89,6 @@ describe("coder-acp-copilot integration", async () => { env.push( `GITHUB_TOKEN=${GITHUB_TOKEN}`, "TEST_PROMPT=Generate a Hello World REST API in Python using Flask.", - "TEST_MODEL=claude-opus-4.6", ); } workerResult = await runTestWorker(docker, { image: IMAGE_TAG, workingDir: WORKING_DIR, env }); @@ -145,11 +144,11 @@ describe("coder-acp-copilot integration", async () => { ); // ----------------------------------------------------------------------- - // Model selection test: ACP set_model actually changes the active model + // Model selection test: ACP set_model changes to an advertised non-default model // ----------------------------------------------------------------------- it.skipIf(!canRun)( - "honours the requested model via ACP set_model", + "selects an advertised non-default model via ACP set_model", { timeout: 300_000 }, async () => { const { result } = workerResult!; @@ -157,24 +156,34 @@ describe("coder-acp-copilot integration", async () => { log(`confirmedModel=${first.confirmedModel}`); - // KNOWN LIMITATION: model selection depends on server-side capability - // advertisement. The ACP newSession response must include either a `models` - // field or a `configOptions` entry with category "model". If the server - // stops advertising these (which can change independently of CLI version), - // selectModel() returns undefined and we can only verify the graceful - // fallback path rather than asserting a confirmed model. if (first.confirmedModel === undefined) { - // Server did not advertise model selection — verify logs show the warning - const hasWarning = result.logs?.some((l) => - l.includes("does not advertise model selection capability") + const selectionUnavailable = result.logs?.some( + (line) => + line.includes("does not advertise model selection capability") || + line.includes( + "model selector did not choose a model from the advertised model options" + ) ); + const selectionFailed = result.logs?.some((line) => + line.includes("session/set_model failed") + ); + + expect( + selectionFailed, + "Dynamic model selection reached session/set_model but failed", + ).toBe(false); expect( - hasWarning, - "selectModel() returned undefined but expected a capability warning in logs", + selectionUnavailable, + "Expected an explicit warning when no alternate advertised model can be selected", ).toBe(true); - log("SKIPPED (server did not advertise model selection capability)"); + log("SKIPPED (server did not advertise an alternate selectable model)"); } else { - expect(first.confirmedModel).toBe("claude-opus-4.6"); + expect( + result.logs?.some((line) => + line.includes(`Model set to "${first.confirmedModel}" via session/set_model`) + ), + `Expected session/set_model success log for "${first.confirmedModel}"`, + ).toBe(true); } }, ); diff --git a/apps/workers/coder-acp-copilot/src/test-worker.ts b/apps/workers/coder-acp-copilot/src/test-worker.ts index 2b6b5d287..06d0c46a5 100644 --- a/apps/workers/coder-acp-copilot/src/test-worker.ts +++ b/apps/workers/coder-acp-copilot/src/test-worker.ts @@ -13,13 +13,20 @@ * * Env vars: * GITHUB_TOKEN — GitHub PAT for Copilot auth (required to run prompts) - * TEST_PROMPT / TEST_PROMPT_2 / TEST_MODEL — see test-utils/harness + * TEST_PROMPT / TEST_PROMPT_2 — see test-utils/harness */ import { runTestHarness } from "test-utils/harness"; -import { runACPSession } from "./acp-client.js"; +import { + runACPSession, + selectFirstAvailableNonDefaultModel, +} from "./acp-client.js"; await runTestHarness({ - runSession: runACPSession, + runSession: (prompt, options) => + runACPSession(prompt, { + ...options, + model: selectFirstAvailableNonDefaultModel, + }), command: "copilot", args: ["--acp", "--yolo"], getCredentialEnv: () => { From 14a3e2acdedc92e01c7a0a7252387a06b9359ae5 Mon Sep 17 00:00:00 2001 From: Josh Duffney Date: Tue, 29 Sep 2026 16:14:47 -0500 Subject: [PATCH 2/4] refactor(acp): expose model selection state Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../coder-acp-copilot/src/acp-client.test.ts | 58 +++++++++++++++++++ .../coder-acp-copilot/src/acp-client.ts | 41 ++++++++++--- packages/test-utils/src/harness.ts | 2 + packages/test-utils/src/types.ts | 2 + 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/apps/workers/coder-acp-copilot/src/acp-client.test.ts b/apps/workers/coder-acp-copilot/src/acp-client.test.ts index 827ca12ee..c65ade8fd 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.test.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.test.ts @@ -6,6 +6,8 @@ import { runACPSession, selectModel, resolveRequestedModel, + getCurrentModelId, + hasModelSelectionCapability, selectFirstAvailableNonDefaultModel, selectReasoningEffort, selectPermissionMode, @@ -361,6 +363,62 @@ describe("resolveRequestedModel", () => { }); }); +describe("model selection session metadata", () => { + function makeSession( + overrides?: Partial + ): acp.NewSessionResponse { + return { + sessionId: "session-1", + ...overrides, + } as acp.NewSessionResponse; + } + + it("returns the current model advertised by the session", () => { + const session = makeSession({ + models: { + currentModelId: "default-model", + availableModels: [ + { modelId: "default-model", name: "Default Model" }, + ], + }, + }); + + expect(getCurrentModelId(session)).toBe("default-model"); + }); + + it("detects models-field capability", () => { + const session = makeSession({ + models: { + currentModelId: "default-model", + availableModels: [], + }, + }); + + expect(hasModelSelectionCapability(session)).toBe(true); + }); + + it("detects model config-option capability", () => { + const session = makeSession({ + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "default-model", + options: [], + type: "select", + }, + ], + }); + + expect(hasModelSelectionCapability(session)).toBe(true); + }); + + it("returns false when no model selection mechanism is advertised", () => { + expect(hasModelSelectionCapability(makeSession())).toBe(false); + }); +}); + describe("selectReasoningEffort", () => { function makeConnection(overrides?: Partial): acp.ClientSideConnection { return { diff --git a/apps/workers/coder-acp-copilot/src/acp-client.ts b/apps/workers/coder-acp-copilot/src/acp-client.ts index 6f00f0e9a..653defbad 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.ts @@ -127,7 +127,10 @@ export interface ACPClientOptions { mcpServers?: McpServerConfig[]; /** Session timeout in milliseconds (default: 30 min). Set to 0 to disable. */ sessionTimeoutMs?: number; - /** Model to select after the session is created, or a selector resolved from the new session response. */ + /** + * Model to select after the session is created, or a selector resolved from + * the new session response for session-scoped policies and integration probes. + */ model?: string | ACPModelSelector; /** Reasoning effort level to apply via ACP config option (e.g. "low", "medium", "high"). */ reasoningEffort?: string; @@ -138,10 +141,16 @@ export interface ACPClientOptions { export interface ACPSessionResult { response: string; stopReason: string; + /** Model active when the ACP session was created, when advertised by the agent. */ + initialModel?: string; /** Model that was successfully activated via ACP set_model, or undefined if model selection was not requested or did not succeed. */ confirmedModel?: string; } +/** + * Select a model after ACP session creation, when session-scoped model + * capabilities are available. + */ export type ACPModelSelector = ( sessionResult: acp.NewSessionResponse ) => string | undefined; @@ -153,6 +162,23 @@ export function resolveRequestedModel( return typeof model === "function" ? model(sessionResult) : model; } +export function getCurrentModelId( + sessionResult: acp.NewSessionResponse +): string | undefined { + return sessionResult.models?.currentModelId; +} + +export function hasModelSelectionCapability( + sessionResult: acp.NewSessionResponse +): boolean { + return ( + sessionResult.models != null || + sessionResult.configOptions?.some( + (option: acp.SessionConfigOption) => option.category === "model" + ) === true + ); +} + /** * Choose the first advertised model that differs from the session default. * @@ -344,7 +370,9 @@ export async function selectModel( } // Neither mechanism available — warn and continue - onLog(`Warning: agent does not advertise model selection capability (no models field or model config option); model "${model}" may not be honoured`); + if (!hasModelSelectionCapability(sessionResult)) { + onLog(`Warning: agent does not advertise model selection capability (no models field or model config option); model "${model}" may not be honoured`); + } return undefined; } @@ -603,6 +631,7 @@ export async function runACPSession( // Select model if requested. A selector is resolved only after newSession // because the current and available models are session-scoped. + const initialModel = getCurrentModelId(sessionResult); let confirmedModel: string | undefined; const requestedModel = resolveRequestedModel(model, sessionResult); if (requestedModel) { @@ -613,12 +642,7 @@ export async function runACPSession( onLog ); } else if (typeof model === "function") { - const hasModelCapability = - sessionResult.models !== undefined || - sessionResult.configOptions?.some( - (option: acp.SessionConfigOption) => option.category === "model" - ) === true; - if (hasModelCapability) { + if (hasModelSelectionCapability(sessionResult)) { onLog( "Warning: model selector did not choose a model from the advertised model options" ); @@ -658,6 +682,7 @@ export async function runACPSession( return { response: clientHandler.getResponse(), stopReason: promptResult.stopReason, + initialModel, confirmedModel, }; }; diff --git a/packages/test-utils/src/harness.ts b/packages/test-utils/src/harness.ts index a1ce4e955..2a1b0ee24 100644 --- a/packages/test-utils/src/harness.ts +++ b/packages/test-utils/src/harness.ts @@ -30,6 +30,7 @@ export interface HarnessSessionOptions { export interface HarnessSessionResult { response: string; stopReason: string; + initialModel?: string; confirmedModel?: string; } @@ -149,6 +150,7 @@ export async function runTestHarness(config: TestHarnessConfig): Promise promptResult.success = true; promptResult.response = acpResult.response; promptResult.stopReason = acpResult.stopReason; + promptResult.initialModel = acpResult.initialModel; promptResult.confirmedModel = acpResult.confirmedModel; } catch (error) { const msg = error instanceof Error ? error.message : String(error); diff --git a/packages/test-utils/src/types.ts b/packages/test-utils/src/types.ts index 30317c9d7..fece36048 100644 --- a/packages/test-utils/src/types.ts +++ b/packages/test-utils/src/types.ts @@ -11,6 +11,8 @@ export interface PromptResult { response?: string; stopReason?: string; error?: string; + /** Model active when the ACP session was created, when advertised by the agent. */ + initialModel?: string; /** Model confirmed active by ACP set_model, or undefined if not requested/unavailable. */ confirmedModel?: string; } From 53f6d6438f12877d2e44ba0cb35b4e34c323d48b Mon Sep 17 00:00:00 2001 From: Josh Duffney Date: Tue, 29 Sep 2026 16:15:16 -0500 Subject: [PATCH 3/4] test(copilot): isolate model selection probe Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/copilot-cli.integration.test.ts | 46 ++++++++++++++----- .../coder-acp-copilot/src/test-worker.ts | 13 +++++- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts index 45359d600..1154ba74b 100644 --- a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts +++ b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts @@ -63,6 +63,7 @@ describe("coder-acp-copilot integration", async () => { const docker = new Docker(); let workerResult: { result: TestResult; exitCode: number } | undefined; + let modelSelectionResult: { result: TestResult; exitCode: number } | undefined; beforeAll(async () => { if (!dockerAvailable) return; @@ -93,7 +94,25 @@ describe("coder-acp-copilot integration", async () => { } workerResult = await runTestWorker(docker, { image: IMAGE_TAG, workingDir: WORKING_DIR, env }); log(`exit=${workerResult.exitCode} lastStep=${workerResult.result.lastStep}`); - }, 600_000); // 10 min for Docker build + + // Keep the real coding prompt on the session default. Exercise model + // switching in a separate short session so server-side model ordering + // cannot make the coding assertion flaky. + if (hasCredentials) { + modelSelectionResult = await runTestWorker(docker, { + image: IMAGE_TAG, + workingDir: WORKING_DIR, + env: [ + `GITHUB_TOKEN=${GITHUB_TOKEN}`, + "TEST_PROMPT=Reply with OK.", + "TEST_SELECT_NON_DEFAULT_MODEL=true", + ], + }); + log( + `model selection exit=${modelSelectionResult.exitCode} lastStep=${modelSelectionResult.result.lastStep}` + ); + } + }, 900_000); // 15 min for Docker build plus two authenticated sessions afterAll(async () => { // Image kept for faster re-runs. Use `docker system prune` to clean up. @@ -144,16 +163,21 @@ describe("coder-acp-copilot integration", async () => { ); // ----------------------------------------------------------------------- - // Model selection test: ACP set_model changes to an advertised non-default model + // Model selection test: ACP changes to an advertised non-default model // ----------------------------------------------------------------------- it.skipIf(!canRun)( - "selects an advertised non-default model via ACP set_model", + "selects an advertised non-default model via ACP", { timeout: 300_000 }, async () => { - const { result } = workerResult!; + const { result } = modelSelectionResult!; const first = result.prompts[0]; + expect( + first.error, + `Model selection probe failed: ${first?.error}` + ).toBeUndefined(); + expect(first.success).toBe(true); log(`confirmedModel=${first.confirmedModel}`); if (first.confirmedModel === undefined) { @@ -164,8 +188,10 @@ describe("coder-acp-copilot integration", async () => { "model selector did not choose a model from the advertised model options" ) ); - const selectionFailed = result.logs?.some((line) => - line.includes("session/set_model failed") + const selectionFailed = result.logs?.some( + (line) => + line.includes("session/set_model failed") || + line.includes("session/set_config_option failed for model") ); expect( @@ -178,12 +204,8 @@ describe("coder-acp-copilot integration", async () => { ).toBe(true); log("SKIPPED (server did not advertise an alternate selectable model)"); } else { - expect( - result.logs?.some((line) => - line.includes(`Model set to "${first.confirmedModel}" via session/set_model`) - ), - `Expected session/set_model success log for "${first.confirmedModel}"`, - ).toBe(true); + expect(first.initialModel, "Expected the session's initial model").toBeTruthy(); + expect(first.confirmedModel).not.toBe(first.initialModel); } }, ); diff --git a/apps/workers/coder-acp-copilot/src/test-worker.ts b/apps/workers/coder-acp-copilot/src/test-worker.ts index 06d0c46a5..bfcb9eb5e 100644 --- a/apps/workers/coder-acp-copilot/src/test-worker.ts +++ b/apps/workers/coder-acp-copilot/src/test-worker.ts @@ -13,7 +13,9 @@ * * Env vars: * GITHUB_TOKEN — GitHub PAT for Copilot auth (required to run prompts) - * TEST_PROMPT / TEST_PROMPT_2 — see test-utils/harness + * TEST_PROMPT / TEST_PROMPT_2 / TEST_MODEL — see test-utils/harness + * TEST_SELECT_NON_DEFAULT_MODEL — select an advertised non-default model + * when TEST_MODEL is not set */ import { runTestHarness } from "test-utils/harness"; import { @@ -21,11 +23,18 @@ import { selectFirstAvailableNonDefaultModel, } from "./acp-client.js"; +const selectNonDefaultModel = + process.env.TEST_SELECT_NON_DEFAULT_MODEL === "true"; + await runTestHarness({ runSession: (prompt, options) => runACPSession(prompt, { ...options, - model: selectFirstAvailableNonDefaultModel, + model: + options.model ?? + (selectNonDefaultModel + ? selectFirstAvailableNonDefaultModel + : undefined), }), command: "copilot", args: ["--acp", "--yolo"], From cd2c166c5b946a46f9b7722c159fdab03634ca8f Mon Sep 17 00:00:00 2001 From: Josh Duffney Date: Tue, 29 Sep 2026 16:15:51 -0500 Subject: [PATCH 4/4] test(acp): select non-default config option model Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../coder-acp-copilot/src/acp-client.test.ts | 114 ++++++++++++++++++ .../coder-acp-copilot/src/acp-client.ts | 74 ++++++++---- .../src/copilot-cli.integration.test.ts | 2 +- packages/test-utils/src/types.ts | 2 +- 4 files changed, 164 insertions(+), 28 deletions(-) diff --git a/apps/workers/coder-acp-copilot/src/acp-client.test.ts b/apps/workers/coder-acp-copilot/src/acp-client.test.ts index c65ade8fd..715381440 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.test.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.test.ts @@ -344,6 +344,103 @@ describe("selectFirstAvailableNonDefaultModel", () => { expect(selectFirstAvailableNonDefaultModel(session)).toBeUndefined(); }); + + it("selects a non-default model from a model config option", () => { + const session = makeSession({ + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "default-model", + options: [ + { value: "default-model", name: "Default Model" }, + { value: "alternate-model", name: "Alternate Model" }, + ], + type: "select", + }, + ], + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBe( + "alternate-model" + ); + }); + + it("selects a non-default model from a grouped model config option", () => { + const session = makeSession({ + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "default-model", + options: [ + { + group: "recommended", + name: "Recommended", + options: [ + { value: "default-model", name: "Default Model" }, + { value: "alternate-model", name: "Alternate Model" }, + ], + }, + ], + type: "select", + }, + ], + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBe( + "alternate-model" + ); + }); + + it("returns undefined when a model config option only lists the current model", () => { + const session = makeSession({ + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "default-model", + options: [ + { value: "default-model", name: "Default Model" }, + ], + type: "select", + }, + ], + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBeUndefined(); + }); + + it("prefers the models field when both representations are advertised", () => { + const session = makeSession({ + models: { + currentModelId: "models-default", + availableModels: [ + { modelId: "models-default", name: "Models Default" }, + { modelId: "models-alternate", name: "Models Alternate" }, + ], + }, + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "config-default", + options: [ + { value: "config-alternate", name: "Config Alternate" }, + ], + type: "select", + }, + ], + }); + + expect(selectFirstAvailableNonDefaultModel(session)).toBe( + "models-alternate" + ); + }); }); describe("resolveRequestedModel", () => { @@ -386,6 +483,23 @@ describe("model selection session metadata", () => { expect(getCurrentModelId(session)).toBe("default-model"); }); + it("returns the current model from a model config option", () => { + const session = makeSession({ + configOptions: [ + { + id: "model-picker", + category: "model", + name: "Model", + currentValue: "default-model", + options: [], + type: "select", + }, + ], + }); + + expect(getCurrentModelId(session)).toBe("default-model"); + }); + it("detects models-field capability", () => { const session = makeSession({ models: { diff --git a/apps/workers/coder-acp-copilot/src/acp-client.ts b/apps/workers/coder-acp-copilot/src/acp-client.ts index 653defbad..7d3c60721 100644 --- a/apps/workers/coder-acp-copilot/src/acp-client.ts +++ b/apps/workers/coder-acp-copilot/src/acp-client.ts @@ -143,7 +143,7 @@ export interface ACPSessionResult { stopReason: string; /** Model active when the ACP session was created, when advertised by the agent. */ initialModel?: string; - /** Model that was successfully activated via ACP set_model, or undefined if model selection was not requested or did not succeed. */ + /** Model successfully activated through ACP, or undefined if selection was not requested or did not succeed. */ confirmedModel?: string; } @@ -165,7 +165,28 @@ export function resolveRequestedModel( export function getCurrentModelId( sessionResult: acp.NewSessionResponse ): string | undefined { - return sessionResult.models?.currentModelId; + return ( + sessionResult.models?.currentModelId ?? + getModelConfigOption(sessionResult)?.currentValue + ); +} + +function getModelConfigOption( + sessionResult: acp.NewSessionResponse +): acp.SessionConfigOption | undefined { + return sessionResult.configOptions?.find( + (option: acp.SessionConfigOption) => option.category === "model" + ); +} + +function getConfigOptionValues( + configOption: acp.SessionConfigOption +): string[] { + return configOption.options.flatMap((option) => + "value" in option + ? [option.value] + : option.options.map((groupedOption) => groupedOption.value) + ); } export function hasModelSelectionCapability( @@ -173,9 +194,7 @@ export function hasModelSelectionCapability( ): boolean { return ( sessionResult.models != null || - sessionResult.configOptions?.some( - (option: acp.SessionConfigOption) => option.category === "model" - ) === true + getModelConfigOption(sessionResult) !== undefined ); } @@ -189,13 +208,20 @@ export function selectFirstAvailableNonDefaultModel( sessionResult: acp.NewSessionResponse ): string | undefined { const models = sessionResult.models; - if (!models) { + if (models) { + return models.availableModels.find( + (candidate) => candidate.modelId !== models.currentModelId + )?.modelId; + } + + const modelConfigOption = getModelConfigOption(sessionResult); + if (!modelConfigOption) { return undefined; } - return models.availableModels.find( - (candidate) => candidate.modelId !== models.currentModelId - )?.modelId; + return getConfigOptionValues(modelConfigOption).find( + (value) => value !== modelConfigOption.currentValue + ); } /** @@ -349,23 +375,19 @@ export async function selectModel( } // Path 2: stable session/set_config_option with category "model" - if (sessionResult.configOptions) { - const modelConfigOption = sessionResult.configOptions.find( - (o) => o.category === "model" - ); - if (modelConfigOption) { - try { - await connection.setSessionConfigOption({ - sessionId: sessionResult.sessionId, - configId: modelConfigOption.id, - value: model, - }); - onLog(`Model set to "${model}" via session/set_config_option (configId: ${modelConfigOption.id})`); - return model; - } catch (err) { - onLog(`Warning: session/set_config_option failed for model "${model}": ${err instanceof Error ? err.message : String(err)}`); - return undefined; - } + const modelConfigOption = getModelConfigOption(sessionResult); + if (modelConfigOption) { + try { + await connection.setSessionConfigOption({ + sessionId: sessionResult.sessionId, + configId: modelConfigOption.id, + value: model, + }); + onLog(`Model set to "${model}" via session/set_config_option (configId: ${modelConfigOption.id})`); + return model; + } catch (err) { + onLog(`Warning: session/set_config_option failed for model "${model}": ${err instanceof Error ? err.message : String(err)}`); + return undefined; } } diff --git a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts index 1154ba74b..0ebdb8825 100644 --- a/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts +++ b/apps/workers/coder-acp-copilot/src/copilot-cli.integration.test.ts @@ -196,7 +196,7 @@ describe("coder-acp-copilot integration", async () => { expect( selectionFailed, - "Dynamic model selection reached session/set_model but failed", + "Dynamic model selection reached an ACP selection method but failed", ).toBe(false); expect( selectionUnavailable, diff --git a/packages/test-utils/src/types.ts b/packages/test-utils/src/types.ts index fece36048..aea787381 100644 --- a/packages/test-utils/src/types.ts +++ b/packages/test-utils/src/types.ts @@ -13,7 +13,7 @@ export interface PromptResult { error?: string; /** Model active when the ACP session was created, when advertised by the agent. */ initialModel?: string; - /** Model confirmed active by ACP set_model, or undefined if not requested/unavailable. */ + /** Model confirmed active through ACP, or undefined if not requested/unavailable. */ confirmedModel?: string; }