From ef41317a2c759ccf101487253f700ab10a26127b Mon Sep 17 00:00:00 2001 From: Cline Date: Mon, 19 Oct 2026 15:03:45 +0000 Subject: [PATCH] fix(cli): keep ACP provider option out of the reserved model category The provider and model session config options both used category "model" with provider listed first. ACP clients break category ties by array order, so they treated provider as the model selector and wrote model ids into it. Move the builders to config-options.ts, give the provider option a custom "_provider" category, and add a regression test asserting only the model option carries "model". Fixes #15158 --- apps/cli/src/acp/acpAgent.ts | 32 ++-------- apps/cli/src/acp/config-options.test.ts | 80 +++++++++++++++++++++++++ apps/cli/src/acp/config-options.ts | 52 ++++++++++++++++ 3 files changed, 137 insertions(+), 27 deletions(-) create mode 100644 apps/cli/src/acp/config-options.test.ts create mode 100644 apps/cli/src/acp/config-options.ts diff --git a/apps/cli/src/acp/acpAgent.ts b/apps/cli/src/acp/acpAgent.ts index 6f041e17a2..8a6ffcb3f0 100644 --- a/apps/cli/src/acp/acpAgent.ts +++ b/apps/cli/src/acp/acpAgent.ts @@ -52,6 +52,10 @@ import { buildAutoApproveConfigOption, parseAutoApproveValue, } from "./auto-approve"; +import { + buildModelConfigOption, + buildProviderConfigOption as buildProviderConfigOptionFromChoices, +} from "./config-options"; import { buildOrganizationConfigOption, fetchClineOrganizations, @@ -869,33 +873,7 @@ async function buildProviderConfigOption( }; }), ); - return { - type: "select", - id: "provider", - name: "Provider", - description: "The authentication provider to use", - category: "model", - currentValue: currentProviderId, - options, - }; -} - -function buildModelConfigOption( - currentModelId: string, - providerModels: Record, -): SessionConfigOption { - return { - type: "select", - id: "model", - name: "Model", - category: "model", - currentValue: currentModelId, - options: Object.entries(providerModels).map(([modelId, info]) => ({ - value: modelId, - name: info.name ?? modelId, - description: info.description, - })), - }; + return buildProviderConfigOptionFromChoices(currentProviderId, options); } function buildModeConfigOption(currentMode: string): SessionConfigOption { diff --git a/apps/cli/src/acp/config-options.test.ts b/apps/cli/src/acp/config-options.test.ts new file mode 100644 index 0000000000..68cc335892 --- /dev/null +++ b/apps/cli/src/acp/config-options.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { + buildModelConfigOption, + buildProviderConfigOption, + MODEL_CONFIG_CATEGORY, + PROVIDER_CONFIG_CATEGORY, +} from "./config-options"; + +const providerChoices = [ + { value: "cline", name: "Cline" }, + { value: "openai-codex", name: "ChatGPT" }, +]; + +const providerModels = { + "anthropic/claude-sonnet-5.5": { name: "Claude Sonnet 5.5" }, + "openrouter/free": { name: "Free", description: "A free model" }, +}; + +describe("buildProviderConfigOption", () => { + it("builds the provider selector with the given choices", () => { + const option = buildProviderConfigOption("cline", providerChoices); + expect(option).toMatchObject({ + type: "select", + id: "provider", + name: "Provider", + currentValue: "cline", + }); + if (option.type !== "select") { + throw new Error(`expected a select option, got ${option.type}`); + } + expect(option.options).toEqual(providerChoices); + }); + + it("uses a custom `_`-prefixed category, never a spec-reserved one", () => { + const option = buildProviderConfigOption("cline", providerChoices); + expect(option.category).toBe(PROVIDER_CONFIG_CATEGORY); + // ACP reserves every category name that does not start with `_`. + expect(option.category).toMatch(/^_/); + }); +}); + +describe("buildModelConfigOption", () => { + it("maps the provider's catalog into select options", () => { + const option = buildModelConfigOption( + "anthropic/claude-sonnet-5.5", + providerModels, + ); + expect(option).toMatchObject({ + type: "select", + id: "model", + category: MODEL_CONFIG_CATEGORY, + currentValue: "anthropic/claude-sonnet-5.5", + }); + if (option.type !== "select") { + throw new Error(`expected a select option, got ${option.type}`); + } + expect(option.options).toEqual([ + { + value: "anthropic/claude-sonnet-5.5", + name: "Claude Sonnet 5.5", + description: undefined, + }, + { value: "openrouter/free", name: "Free", description: "A free model" }, + ]); + }); +}); + +describe("ACP config option categories", () => { + // Regression for cline/cline#15158: `provider` was listed first and also + // tagged `model`, so spec-following clients (which break category ties by + // array order) treated it as the model selector and wrote model ids into it. + it("only the model option carries the reserved `model` category", () => { + const configOptions = [ + buildProviderConfigOption("cline", providerChoices), + buildModelConfigOption("anthropic/claude-sonnet-5.5", providerModels), + ]; + const modelCategory = configOptions.filter((o) => o.category === "model"); + expect(modelCategory.map((o) => o.id)).toEqual(["model"]); + }); +}); diff --git a/apps/cli/src/acp/config-options.ts b/apps/cli/src/acp/config-options.ts new file mode 100644 index 0000000000..358394e405 --- /dev/null +++ b/apps/cli/src/acp/config-options.ts @@ -0,0 +1,52 @@ +import type { + SessionConfigOption, + SessionConfigSelectOption, +} from "@agentclientprotocol/sdk"; + +/** + * Category for the auth-provider selector. + * + * ACP reserves `model` for *the* model selector: when several options share a + * category, clients break the tie by array order and treat the earliest one + * as the model selector. The provider option is listed first, so tagging it + * `model` too made spec-following clients write a model id into `provider` + * (rejected by `setSessionConfigOption`). Names starting with `_` are free + * for custom use, so the provider gets its own category instead. + */ +export const PROVIDER_CONFIG_CATEGORY = "_provider"; + +/** Category reserved by ACP for the model selector. Only `model` may use it. */ +export const MODEL_CONFIG_CATEGORY = "model"; + +export function buildProviderConfigOption( + currentProviderId: string, + options: SessionConfigSelectOption[], +): SessionConfigOption { + return { + type: "select", + id: "provider", + name: "Provider", + description: "The authentication provider to use", + category: PROVIDER_CONFIG_CATEGORY, + currentValue: currentProviderId, + options, + }; +} + +export function buildModelConfigOption( + currentModelId: string, + providerModels: Record, +): SessionConfigOption { + return { + type: "select", + id: "model", + name: "Model", + category: MODEL_CONFIG_CATEGORY, + currentValue: currentModelId, + options: Object.entries(providerModels).map(([modelId, info]) => ({ + value: modelId, + name: info.name ?? modelId, + description: info.description, + })), + }; +}