diff --git a/apps/cli/src/acp/acpAgent.ts b/apps/cli/src/acp/acpAgent.ts index 6f041e17a2..1469f2c9cc 100644 --- a/apps/cli/src/acp/acpAgent.ts +++ b/apps/cli/src/acp/acpAgent.ts @@ -14,6 +14,7 @@ import type { PromptRequest, PromptResponse, SessionConfigOption, + SessionConfigSelectOption, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModelRequest, @@ -47,11 +48,8 @@ import { authenticateAcpProvider, isAcpAuthMethodId, } from "./auth"; -import { - AUTO_APPROVE_CONFIG_ID, - buildAutoApproveConfigOption, - parseAutoApproveValue, -} from "./auto-approve"; +import { AUTO_APPROVE_CONFIG_ID, parseAutoApproveValue } from "./auto-approve"; +import { buildConfigOptions } from "./config-options"; import { buildOrganizationConfigOption, fetchClineOrganizations, @@ -236,13 +234,15 @@ export class AcpAgent implements Agent { availableModels, currentModelId: defaultModelId, }, - configOptions: [ - await buildProviderConfigOption(providerId), - buildModelConfigOption(defaultModelId, providerModels), - buildModeConfigOption(defaultMode), - buildAutoApproveConfigOption(this.defaultAutoApproveTools), - ...(organizationOption ? [organizationOption] : []), - ], + configOptions: buildConfigOptions({ + currentProviderId: providerId, + providerOptions: await getProviderOptions(), + currentModelId: defaultModelId, + providerModels, + currentMode: defaultMode, + autoApproveTools: this.defaultAutoApproveTools, + organizationOption, + }), }; } @@ -562,13 +562,13 @@ export class AcpAgent implements Agent { ); } - const configOptions = await buildAllConfigOptions(session); const organizationOption = await this.getOrganizationConfigOption( session.currentProviderId, ); - if (organizationOption) { - configOptions.push(organizationOption); - } + const configOptions = await buildAllConfigOptions( + session, + organizationOption, + ); sendConfigOptionUpdate(this.conn, params.sessionId, configOptions); return { configOptions }; } @@ -857,10 +857,8 @@ function toAcpPromptError(error: Error): RequestError { : RequestError.internalError({ message }, message); } -async function buildProviderConfigOption( - currentProviderId: string, -): Promise { - const options = await Promise.all( +async function getProviderOptions(): Promise { + return Promise.all( ACP_AUTH_METHODS.map(async (m) => { const provider = await Llms.getProvider(m.id); return { @@ -869,75 +867,28 @@ 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, - })), - }; -} - -function buildModeConfigOption(currentMode: string): SessionConfigOption { - return { - type: "select", - id: "mode", - name: "Session Mode", - description: "Controls whether the agent can modify files", - category: "mode", - currentValue: currentMode, - options: [ - { - value: "plan", - name: "Plan", - description: - "Explore the codebase and plan changes without modifying files", - }, - { - value: "act", - name: "Act", - description: "Make changes to the codebase", - }, - ], - }; } async function buildAllConfigOptions( session: SessionState, + organizationOption?: SessionConfigOption, ): Promise { - const [providerOption, providerModels] = await Promise.all([ - buildProviderConfigOption(session.currentProviderId), + const [providerOptions, providerModels] = await Promise.all([ + getProviderOptions(), Llms.getModelsForProvider( session.currentProviderId, CHAT_MODEL_QUERY_OPTIONS, ), ]); - return [ - providerOption, - buildModelConfigOption(session.currentModelId, providerModels), - buildModeConfigOption(session.currentMode), - buildAutoApproveConfigOption(session.autoApproveTools), - ]; + return buildConfigOptions({ + currentProviderId: session.currentProviderId, + providerOptions, + currentModelId: session.currentModelId, + providerModels, + currentMode: session.currentMode, + autoApproveTools: session.autoApproveTools, + organizationOption, + }); } function extractTextFromContentBlocks(blocks: ContentBlock[]): string { 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..0d3ebe794e --- /dev/null +++ b/apps/cli/src/acp/config-options.test.ts @@ -0,0 +1,110 @@ +import type { SessionConfigOption } from "@agentclientprotocol/sdk"; +import { describe, expect, it } from "vitest"; +import { + buildConfigOptions, + buildProviderConfigOption, + MODEL_CONFIG_CATEGORY, + PROVIDER_CONFIG_CATEGORY, +} from "./config-options"; + +// ACP reserves every category name that does not start with `_` for the spec. +const ACP_RESERVED_CATEGORIES = [ + "mode", + "model", + "model_config", + "thought_level", +]; + +const providerModels = { + "anthropic/claude-sonnet-5.5": { name: "Claude Sonnet 5.5" }, + "openrouter/free": { name: "Free" }, +}; + +const providerOptions = [ + { value: "cline", name: "Cline" }, + { value: "openrouter", name: "OpenRouter" }, +]; + +const organizationOption: SessionConfigOption = { + type: "select", + id: "organization", + name: "Account", + category: "_account", + currentValue: "personal", + options: [{ value: "personal", name: "Personal" }], +}; + +describe("buildProviderConfigOption", () => { + it("builds the provider selector from the current provider", () => { + const option = buildProviderConfigOption("cline", providerOptions); + + expect(option).toMatchObject({ + type: "select", + id: "provider", + currentValue: "cline", + options: providerOptions, + }); + }); + + it("uses a custom category instead of a spec-reserved one", () => { + const option = buildProviderConfigOption("cline", providerOptions); + + expect(option.category).toBe(PROVIDER_CONFIG_CATEGORY); + expect(ACP_RESERVED_CATEGORIES).not.toContain(option.category); + expect(option.category).toMatch(/^_/); + }); +}); + +describe.each([ + false, + true, +])("ACP config options (organization: %s)", (includeOrganization) => { + const options = buildConfigOptions({ + currentProviderId: "cline", + providerOptions, + currentModelId: "anthropic/claude-sonnet-5.5", + providerModels, + currentMode: "act", + autoApproveTools: false, + organizationOption: includeOrganization ? organizationOption : undefined, + }); + + it("tags only the model selector with the model category", () => { + expect( + options + .filter((option) => option.category === MODEL_CONFIG_CATEGORY) + .map((option) => option.id), + ).toEqual(["model"]); + }); + + it("uses reserved or custom category names for every option", () => { + for (const { category } of options) { + expect( + category === undefined || + category === null || + ACP_RESERVED_CATEGORIES.includes(category) || + category.startsWith("_"), + ).toBe(true); + } + }); + + it("does not repeat reserved categories in the assembled list", () => { + const categories = options + .map((option) => option.category) + .filter( + (category) => category && ACP_RESERVED_CATEGORIES.includes(category), + ); + + expect(new Set(categories).size).toBe(categories.length); + }); + + it("preserves the option order", () => { + expect(options.map((option) => option.id)).toEqual([ + "provider", + "model", + "mode", + "auto_approve", + ...(includeOrganization ? ["organization"] : []), + ]); + }); +}); diff --git a/apps/cli/src/acp/config-options.ts b/apps/cli/src/acp/config-options.ts new file mode 100644 index 0000000000..f44eba30f7 --- /dev/null +++ b/apps/cli/src/acp/config-options.ts @@ -0,0 +1,95 @@ +import type { + SessionConfigOption, + SessionConfigSelectOption, +} from "@agentclientprotocol/sdk"; +import { buildAutoApproveConfigOption } from "./auto-approve"; + +// Clients break category ties by array order, so the provider must not claim +// the model selector's category when it comes first. +export const PROVIDER_CONFIG_CATEGORY = "_provider"; +export const MODEL_CONFIG_CATEGORY = "model"; + +interface ConfigOptionsInput { + currentProviderId: string; + providerOptions: SessionConfigSelectOption[]; + currentModelId: string; + providerModels: Record; + currentMode: string; + autoApproveTools: boolean; + organizationOption?: SessionConfigOption; +} + +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, + })), + }; +} + +function buildModeConfigOption(currentMode: string): SessionConfigOption { + return { + type: "select", + id: "mode", + name: "Session Mode", + description: "Controls whether the agent can modify files", + category: "mode", + currentValue: currentMode, + options: [ + { + value: "plan", + name: "Plan", + description: + "Explore the codebase and plan changes without modifying files", + }, + { + value: "act", + name: "Act", + description: "Make changes to the codebase", + }, + ], + }; +} + +export function buildConfigOptions({ + currentProviderId, + providerOptions, + currentModelId, + providerModels, + currentMode, + autoApproveTools, + organizationOption, +}: ConfigOptionsInput): SessionConfigOption[] { + return [ + buildProviderConfigOption(currentProviderId, providerOptions), + buildModelConfigOption(currentModelId, providerModels), + buildModeConfigOption(currentMode), + buildAutoApproveConfigOption(autoApproveTools), + ...(organizationOption ? [organizationOption] : []), + ]; +} diff --git a/apps/cli/src/acp/organizations.test.ts b/apps/cli/src/acp/organizations.test.ts index 41ca89ab7f..40c648cc40 100644 --- a/apps/cli/src/acp/organizations.test.ts +++ b/apps/cli/src/acp/organizations.test.ts @@ -28,6 +28,7 @@ describe("buildOrganizationConfigOption", () => { activeOrganizationId: "org-2", }); expect(option.id).toBe("organization"); + expect(option.category).toBe("_account"); if (option.type !== "select") { throw new Error(`expected a select option, got ${option.type}`); } diff --git a/apps/cli/src/acp/organizations.ts b/apps/cli/src/acp/organizations.ts index 3b8a1e48bc..68b3d7d4ff 100644 --- a/apps/cli/src/acp/organizations.ts +++ b/apps/cli/src/acp/organizations.ts @@ -94,7 +94,7 @@ export function buildOrganizationConfigOption( name: "Account", description: "The Cline account usage is billed to — your personal account or an organization", - category: "account", + category: "_account", currentValue: state.activeOrganizationId ?? PERSONAL_ACCOUNT_VALUE, options: [ { value: PERSONAL_ACCOUNT_VALUE, name: "Personal" }, diff --git a/sdk/packages/core/src/extensions/tools/helpers.run-commands.test.ts b/sdk/packages/core/src/extensions/tools/helpers.run-commands.test.ts new file mode 100644 index 0000000000..321a47d787 --- /dev/null +++ b/sdk/packages/core/src/extensions/tools/helpers.run-commands.test.ts @@ -0,0 +1,38 @@ +import { zodToJsonSchema } from "@cline/shared"; +import { describe, expect, it } from "vitest"; +import { normalizeRunCommandsInput } from "./helpers"; +import { RunCommandsInputSchema } from "./schemas"; + +describe("normalizeRunCommandsInput", () => { + it("accepts the documented shapes", () => { + expect(normalizeRunCommandsInput({ commands: ["ls", "pwd"] })).toEqual([ + "ls", + "pwd", + ]); + expect(normalizeRunCommandsInput({ commands: "ls" })).toEqual(["ls"]); + expect(normalizeRunCommandsInput(["ls"])).toEqual(["ls"]); + expect(normalizeRunCommandsInput("ls")).toEqual(["ls"]); + expect(normalizeRunCommandsInput({ command: "ls" })).toEqual(["ls"]); + expect( + normalizeRunCommandsInput({ command: "git", args: ["status"] }), + ).toEqual([{ command: "git", args: ["status"] }]); + }); + + it.each([ + { label: "an object with an empty commands list", input: { commands: [] } }, + { label: "a bare empty list", input: [] }, + ])("rejects $label instead of reporting an empty success", ({ input }) => { + expect(() => normalizeRunCommandsInput(input)).toThrow( + "run_commands requires at least one command", + ); + }); +}); + +describe("RunCommandsInputSchema", () => { + it("advertises the command list as non-empty", () => { + const schema = zodToJsonSchema(RunCommandsInputSchema) as { + properties: { commands: { minItems?: number } }; + }; + expect(schema.properties.commands.minItems).toBe(1); + }); +}); diff --git a/sdk/packages/core/src/extensions/tools/helpers.ts b/sdk/packages/core/src/extensions/tools/helpers.ts index a9a3373e4b..dc6c6cd956 100644 --- a/sdk/packages/core/src/extensions/tools/helpers.ts +++ b/sdk/packages/core/src/extensions/tools/helpers.ts @@ -1,4 +1,5 @@ import { validateWithZod } from "@cline/shared"; +import type { z } from "zod"; import { type EditFileInput, INPUT_ARG_CHAR_LIMIT, @@ -137,8 +138,24 @@ export function coalesceOrphanReadRanges(input: unknown): unknown { export function normalizeRunCommandsInput( input: unknown, ): Array { - const validate = validateWithZod(RunCommandsInputUnionSchema, input); + const commands = collectRunCommands( + validateWithZod(RunCommandsInputUnionSchema, input), + ); + // An empty list is well-formed but never a real request: nothing would + // run and the call would still report success. Fail it so the model sees + // that no command was given instead of an empty result it has to read + // into. + if (commands.length === 0) { + throw new Error( + "run_commands requires at least one command; the commands list was empty.", + ); + } + return commands; +} +function collectRunCommands( + validate: z.infer, +): Array { if (typeof validate === "string") { return [validate]; } diff --git a/sdk/packages/core/src/extensions/tools/schemas.ts b/sdk/packages/core/src/extensions/tools/schemas.ts index b067cb61a3..8ebffd1160 100644 --- a/sdk/packages/core/src/extensions/tools/schemas.ts +++ b/sdk/packages/core/src/extensions/tools/schemas.ts @@ -149,6 +149,10 @@ export const StructuredCommandEntrySchema = z.union([ export const RunCommandsInputSchema = z.object({ commands: z .array(CommandInputSchema) + // Advertised to the model as minItems: 1. The lenient union below still + // accepts an empty list so the normalizer can reject it with one clear + // message instead of a union error. + .min(1) .describe("Array of complete shell command strings to execute."), });