From d0e1edca7f5d8addfe7461c788f506c536294e80 Mon Sep 17 00:00:00 2001 From: Felipe Fidelix <309092+fidelix@users.noreply.github.com> Date: Sat, 12 Sep 2026 03:50:09 -0300 Subject: [PATCH] fix(server): show the right thinking options for each Cursor model (#4180) * fix(server): probe Cursor thinking options per model Cursor ACP only reports thought_level for the current model, so the catalog was copying Haiku's Off/On onto Grok 4.6 until the next probe happened to land on Grok. Co-authored-by: Cursor * fix(server): drop inherited thinking options when an ACP catalog probe fails A rejected model switch was leaving the probe session's thinking list on the failed model, so Grok could still advertise Haiku's Off/On. Co-authored-by: Cursor * fix(server): probe per-model thinking when session/new has no thought_level Composer/Auto as the Cursor ACP default omits thought_level, so the catalog helper skipped every model switch and Grok kept an empty thinking list. Co-authored-by: Cursor * fix(server): discover model options without changing preferences Use the provider model catalog extension instead of probing model switches that persist user preferences. Keep provider-specific discovery in each shim and validate extension responses at the boundary. --------- Co-authored-by: Cursor Co-authored-by: Mohamed Boudra --- docs/providers.md | 7 + .../server/agent/providers/acp-agent.test.ts | 7 +- .../src/server/agent/providers/acp-agent.ts | 9 +- .../agent/providers/cursor-acp-agent.test.ts | 7 + .../agent/providers/cursor-acp-agent.ts | 62 ++++- .../providers/cursor-acp-catalog.test.ts | 213 ++++++++++++++++++ .../agent/providers/kimi-acp-agent.test.ts | 32 ++- .../server/agent/providers/kimi-acp-agent.ts | 40 ++-- 8 files changed, 340 insertions(+), 37 deletions(-) create mode 100644 packages/server/src/server/agent/providers/cursor-acp-catalog.test.ts diff --git a/docs/providers.md b/docs/providers.md index f47d408d54..992b6c555f 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -49,6 +49,13 @@ Copilot custom agents are exposed through ACP session config, not the slash-comm ACP permission options are rendered as ordered actions and Paseo returns the selected option's exact `optionId`. Agents can therefore encode a single-choice question as multiple options of the same allow kind. Auto-accept does not resolve those chooser requests; they always wait for the user. +ACP shims can own model discovery through `catalogModelResolver`; the shared client owns the probe +process and refresh deadline. Keep vendor RPCs in the shim. Cursor uses +`cursor/list_available_models` because switching models during discovery writes its saved CLI +preferences and selection history. Cursor versions without that extension must be updated. Kimi +still probes model selections in its own shim. The initial session supplies modes and the current +model; it does not override the model list returned by a resolver. + ### Direct Implement the `AgentClient` and `AgentSession` interfaces from `agent-sdk-types.ts` yourself. This gives full control but requires you to handle process management, streaming, permissions, and session persistence from scratch. diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 1c77a6a25d..237e98892c 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1794,10 +1794,9 @@ describe("ACPAgentClient modelTransformer", () => { describe("ACPAgentClient catalog discovery without a model resolver", () => { test("never switches models during catalog discovery even with multiple models and a thinking picker", async () => { - // The per-model probing that switches models lives on KimiACPAgentClient - // (see kimi-acp-agent.test.ts). The base client ships no catalog model resolver, so a - // slow or nonconforming ACP can't stall its catalog probe on extra setSessionConfigOption - // round trips. + // Model discovery extensions live on providers that opt in. The base + // client ships no catalog model resolver, so a slow or nonconforming ACP can't + // stall its catalog probe on extra setSessionConfigOption round trips. const setSessionConfigOption = vi.fn(); class TestACPAgentClient extends ACPAgentClient { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 0adb620136..4b14f924ef 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -393,7 +393,7 @@ export type ACPExtensionCommandsParser = ( * the catalog plumbing. */ export interface ACPCatalogModelResolverContext { - connection: ClientSideConnection; + connection: Pick; sessionId: string; models: AgentModelDefinition[]; configOptions: SessionConfigOption[] | null | undefined; @@ -404,10 +404,9 @@ export interface ACPCatalogModelResolverContext { } /** - * Optional hook that refines the catalog's model list using the live probe session. - * The base client ships no resolver — catalog discovery derives models from the initial - * session response and never mutates the probe. Providers that need per-model data (Kimi) - * inject a resolver so the extra round trips stay off every other ACP. + * Providers own model discovery through this hook, including extension RPCs. Without a + * resolver, the base client derives models from the initial session response. The client + * owns the probe lifetime and refresh deadline; resolvers return the complete model list. */ export type ACPCatalogModelResolver = ( context: ACPCatalogModelResolverContext, diff --git a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts index a38fb77128..fbe4c762e1 100644 --- a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts @@ -33,6 +33,13 @@ describe("CursorACPAgentClient model discovery", () => { child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() }, connection: { newSession: vi.fn().mockResolvedValue(this.response), + extMethod: async () => ({ + models: (this.response.models?.availableModels ?? []).map((model) => ({ + value: model.modelId, + name: model.name, + configOptions: [], + })), + }), }, initialize: { agentCapabilities: {} }, } as SpawnedACPProcess; diff --git a/packages/server/src/server/agent/providers/cursor-acp-agent.ts b/packages/server/src/server/agent/providers/cursor-acp-agent.ts index 6dce76c973..0a7ce428b3 100644 --- a/packages/server/src/server/agent/providers/cursor-acp-agent.ts +++ b/packages/server/src/server/agent/providers/cursor-acp-agent.ts @@ -1,6 +1,13 @@ +import { zSessionConfigOption } from "@agentclientprotocol/sdk/dist/schema/zod.gen.js"; import type { Logger } from "pino"; +import { z } from "zod"; -import type { ACPConfigFeatureOption } from "./acp-agent.js"; +import type { AgentModelDefinition } from "../agent-sdk-types.js"; +import { + deriveSelectorOptions, + type ACPCatalogModelResolverContext, + type ACPConfigFeatureOption, +} from "./acp-agent.js"; import { GenericACPAgentClient } from "./generic-acp-agent.js"; interface CursorACPAgentClientOptions { @@ -26,6 +33,58 @@ export const CURSOR_FAST_FEATURE_OPTION: ACPConfigFeatureOption = { icon: "zap", }; +const CursorModelCatalogSchema = z.object({ + models: z.array( + z.object({ + value: z.string().min(1), + name: z.string(), + configOptions: z.array(zSessionConfigOption), + }), + ), +}); + +// Cursor model switches persist CLI preferences, even in a throwaway probe session. +// Its extension returns each model's parameter definitions without selecting it. +export async function resolveCursorCatalogModels({ + connection, + models, + provider, + runRequest, +}: ACPCatalogModelResolverContext): Promise { + const catalog = await runRequest(() => fetchCursorModelCatalog(connection)); + const currentModelId = models.find((model) => model.isDefault)?.id; + + return catalog.models.map((model) => { + const thinkingOptions = deriveSelectorOptions(model.configOptions, "thought_level"); + const defaultThinkingOptionId = thinkingOptions.find((option) => option.isDefault)?.id; + return { + provider, + id: model.value, + label: model.name, + isDefault: model.value === currentModelId, + thinkingOptions: thinkingOptions.length > 0 ? thinkingOptions : undefined, + defaultThinkingOptionId, + }; + }); +} + +async function fetchCursorModelCatalog(connection: ACPCatalogModelResolverContext["connection"]) { + try { + const response = await connection.extMethod("cursor/list_available_models", {}); + return CursorModelCatalogSchema.parse(response); + } catch (error) { + const extensionUnavailable = + typeof error === "object" && error !== null && "code" in error && error.code === -32601; + if (extensionUnavailable) { + throw new Error( + "Update Cursor CLI: this version does not support cursor/list_available_models.", + { cause: error }, + ); + } + throw error; + } +} + export class CursorACPAgentClient extends GenericACPAgentClient { constructor(options: CursorACPAgentClientOptions) { super({ @@ -40,6 +99,7 @@ export class CursorACPAgentClient extends GenericACPAgentClient { initialCommandsWaitTimeoutMs: CURSOR_INITIAL_COMMANDS_WAIT_TIMEOUT_MS, clientCapabilityMeta: CURSOR_CLIENT_CAPABILITY_META, configFeatureOptions: [CURSOR_FAST_FEATURE_OPTION], + catalogModelResolver: resolveCursorCatalogModels, }); } } diff --git a/packages/server/src/server/agent/providers/cursor-acp-catalog.test.ts b/packages/server/src/server/agent/providers/cursor-acp-catalog.test.ts new file mode 100644 index 0000000000..9facfda374 --- /dev/null +++ b/packages/server/src/server/agent/providers/cursor-acp-catalog.test.ts @@ -0,0 +1,213 @@ +import type { SessionConfigOption } from "@agentclientprotocol/sdk"; +import { expect, test } from "vitest"; +import { ZodError } from "zod"; + +import { createTestLogger } from "../../../test-utils/test-logger.js"; +import { deriveModelDefinitionsFromACP, type ACPCatalogModelResolverContext } from "./acp-agent.js"; +import { resolveCursorCatalogModels } from "./cursor-acp-agent.js"; + +const modelOption: SessionConfigOption = { + id: "model", + name: "Model", + category: "model", + type: "select", + currentValue: "claude-haiku-4-5", + options: [ + { value: "claude-haiku-4-5", name: "Haiku 4.5" }, + { value: "grok-4.6", name: "Grok 4.6" }, + ], +}; + +const booleanThinking: SessionConfigOption = { + id: "thinking", + name: "Thinking", + category: "thought_level", + type: "select", + currentValue: "true", + options: [ + { value: "false", name: "Off" }, + { value: "true", name: "On" }, + ], +}; + +const effortThinking: SessionConfigOption = { + id: "effort", + name: "Effort", + category: "thought_level", + type: "select", + currentValue: "xhigh", + options: [ + { value: "low", name: "Low" }, + { value: "medium", name: "Medium" }, + { value: "high", name: "High" }, + { value: "xhigh", name: "Extra High" }, + ], +}; + +class CursorCatalogConnection { + selectedModel = "claude-haiku-4-5"; + selectionHistory = ["claude-haiku-4-5"]; + error: unknown = null; + catalog: Record = { + models: [ + { value: "claude-haiku-4-5", name: "Haiku 4.5", configOptions: [booleanThinking] }, + { value: "grok-4.6", name: "Grok 4.6", configOptions: [effortThinking] }, + ], + }; + + async extMethod(method: string, params: Record) { + if (method !== "cursor/list_available_models" || Object.keys(params).length !== 0) { + throw new Error(`Unsupported catalog request: ${method}`); + } + if (this.error) throw this.error; + return this.catalog; + } + + async setSessionConfigOption({ value }: { value: string }) { + this.selectedModel = value; + this.selectionHistory.push(value); + const thinking = value === "grok-4.6" ? effortThinking : booleanThinking; + return { configOptions: [thinking] }; + } +} + +function createCatalogContext( + connection: CursorCatalogConnection, + configOptions: SessionConfigOption[] = [modelOption, booleanThinking], +): ACPCatalogModelResolverContext { + return { + connection, + sessionId: "catalog-session", + models: deriveModelDefinitionsFromACP("acp", null, configOptions), + configOptions, + runRequest: (request) => request(), + transformConfigOptions: (options) => options, + logger: createTestLogger(), + provider: "acp", + }; +} + +test("lists each model's thinking options without changing Cursor preferences", async () => { + const connection = new CursorCatalogConnection(); + const context = createCatalogContext(connection); + + const models = await resolveCursorCatalogModels(context); + + expect(connection.selectedModel).toBe("claude-haiku-4-5"); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); + expect( + models.map(({ id, isDefault, thinkingOptions, defaultThinkingOptionId }) => ({ + id, + isDefault, + thinking: thinkingOptions?.map((option) => option.id), + defaultThinkingOptionId, + })), + ).toEqual([ + { + id: "claude-haiku-4-5", + isDefault: true, + thinking: ["false", "true"], + defaultThinkingOptionId: "true", + }, + { + id: "grok-4.6", + isDefault: false, + thinking: ["low", "medium", "high", "xhigh"], + defaultThinkingOptionId: "xhigh", + }, + ]); +}); + +test("reports that Cursor must be updated when the model extension is unavailable", async () => { + const connection = new CursorCatalogConnection(); + connection.error = { code: -32601, message: "Method not found" }; + + await expect(resolveCursorCatalogModels(createCatalogContext(connection))).rejects.toThrow( + "Update Cursor CLI", + ); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); +}); + +test("discovers thinking models when the session default has no thinking selector", async () => { + const connection = new CursorCatalogConnection(); + const context = createCatalogContext(connection, [modelOption]); + + const models = await resolveCursorCatalogModels(context); + + expect(models[1].thinkingOptions?.map((option) => option.id)).toEqual([ + "low", + "medium", + "high", + "xhigh", + ]); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); +}); + +test("uses the extension's complete catalog even when session/new lists only one model", async () => { + const connection = new CursorCatalogConnection(); + const context = createCatalogContext(connection); + context.models = context.models.slice(0, 1); + + const models = await resolveCursorCatalogModels(context); + + expect(models.map((model) => model.id)).toEqual(["claude-haiku-4-5", "grok-4.6"]); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); +}); + +test("does not copy session thinking options onto a model without a selector", async () => { + const connection = new CursorCatalogConnection(); + connection.catalog = { + models: [{ value: "composer-2", name: "Composer", configOptions: [] }], + }; + + const models = await resolveCursorCatalogModels(createCatalogContext(connection)); + + expect(models).toEqual([ + { + provider: "acp", + id: "composer-2", + label: "Composer", + isDefault: false, + thinkingOptions: undefined, + defaultThinkingOptionId: undefined, + }, + ]); +}); + +test("keeps an empty extension catalog empty", async () => { + const connection = new CursorCatalogConnection(); + connection.catalog = { models: [] }; + + await expect(resolveCursorCatalogModels(createCatalogContext(connection))).resolves.toEqual([]); +}); + +test.each([ + {}, + { models: [{ value: "grok-4.6", name: "Grok 4.6" }] }, + { + models: [ + { + value: "grok-4.6", + name: "Grok 4.6", + configOptions: [{ ...effortThinking, currentValue: 1 }], + }, + ], + }, +])("rejects malformed extension responses: %j", async (catalog) => { + const connection = new CursorCatalogConnection(); + connection.catalog = catalog; + + await expect(resolveCursorCatalogModels(createCatalogContext(connection))).rejects.toBeInstanceOf( + ZodError, + ); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); +}); + +test("propagates extension failures without changing preferences or returning stale options", async () => { + const connection = new CursorCatalogConnection(); + const error = new Error("Connection closed"); + connection.error = error; + + await expect(resolveCursorCatalogModels(createCatalogContext(connection))).rejects.toBe(error); + expect(connection.selectionHistory).toEqual(["claude-haiku-4-5"]); +}); diff --git a/packages/server/src/server/agent/providers/kimi-acp-agent.test.ts b/packages/server/src/server/agent/providers/kimi-acp-agent.test.ts index ce296726a1..b388fdb01a 100644 --- a/packages/server/src/server/agent/providers/kimi-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/kimi-acp-agent.test.ts @@ -156,8 +156,13 @@ describe("KimiACPAgentClient per-model thinking options", () => { expect(setSessionConfigOption).not.toHaveBeenCalled(); }); - test("skips per-model probing when the provider has no thinking picker", async () => { - const setSessionConfigOption = vi.fn(); + test("probes each model when session/new has no thought_level so a later model can still expose thinking options", async () => { + const setSessionConfigOption = vi.fn(async ({ value }: { value: string }) => ({ + configOptions: + value === "kimi-k3" + ? [modelConfigOption(value), effortThinkingConfigOption()] + : [modelConfigOption(value)], + })); const client = createKimiClient( async () => @@ -174,16 +179,28 @@ describe("KimiACPAgentClient per-model thinking options", () => { }) as unknown as SpawnedACPProcess, ); - await client.fetchCatalog({ + const catalog = await client.fetchCatalog({ scope: "workspace", cwd: "/tmp/acp-kimi-no-thinking", force: false, }); - expect(setSessionConfigOption).not.toHaveBeenCalled(); + expect(setSessionConfigOption).toHaveBeenCalledTimes(2); + + const kimiForCoding = catalog.models.find((model) => model.id === "kimi-for-coding"); + const kimiK3 = catalog.models.find((model) => model.id === "kimi-k3"); + expect(kimiForCoding?.thinkingOptions).toBeUndefined(); + expect(kimiForCoding?.defaultThinkingOptionId).toBeUndefined(); + expect(kimiK3?.thinkingOptions).toEqual([ + expect.objectContaining({ id: "off", isDefault: false }), + expect.objectContaining({ id: "low", isDefault: false }), + expect.objectContaining({ id: "medium", isDefault: true }), + expect.objectContaining({ id: "high", isDefault: false }), + ]); + expect(kimiK3?.defaultThinkingOptionId).toBe("medium"); }); - test("keeps a model's default thinking options when its probe fails", async () => { + test("omits thinking options when a model's probe fails instead of keeping another model's list", async () => { const setSessionConfigOption = vi.fn(async ({ value }: { value: string }) => { if (value === "kimi-k3") { throw new Error("probe rejected model switch"); @@ -212,10 +229,13 @@ describe("KimiACPAgentClient per-model thinking options", () => { force: false, }); + const kimiForCoding = catalog.models.find((model) => model.id === "kimi-for-coding"); const kimiK3 = catalog.models.find((model) => model.id === "kimi-k3"); - expect(kimiK3?.thinkingOptions).toEqual([ + expect(kimiForCoding?.thinkingOptions).toEqual([ expect.objectContaining({ id: "off", isDefault: true }), expect.objectContaining({ id: "on", isDefault: false }), ]); + expect(kimiK3?.thinkingOptions).toBeUndefined(); + expect(kimiK3?.defaultThinkingOptionId).toBeUndefined(); }); }); diff --git a/packages/server/src/server/agent/providers/kimi-acp-agent.ts b/packages/server/src/server/agent/providers/kimi-acp-agent.ts index e8ff7ce252..50effdf33a 100644 --- a/packages/server/src/server/agent/providers/kimi-acp-agent.ts +++ b/packages/server/src/server/agent/providers/kimi-acp-agent.ts @@ -18,23 +18,8 @@ interface KimiACPAgentClientOptions { providerParams?: unknown; } -/** - * Kimi reports different thinking-effort levels per model (a boolean on/off toggle for one - * model, a multi-level select for another), but only exposes the *currently selected* - * model's levels through `configOptions` — the model list itself carries no per-model - * effort metadata. `deriveModelDefinitionsFromACP` can only see whichever model the probe - * session defaulted to, so every other model would otherwise inherit that one model's - * thinking options. - * - * Reuse the single catalog probe session to switch through each candidate model in turn and - * read back its real thinking options, rather than spawning a probe per model. Skipped - * entirely when the provider reports one model or no thinking picker, so a misbehaving ACP - * that only advertises a single model pays no extra round trips. - * - * This lives on the Kimi client, not the base ACP adapter: only Kimi needs the extra - * `setSessionConfigOption` round trips, so no other ACP provider risks a slow or - * nonconforming agent stalling its catalog probe on model switching. - */ +// Kimi exposes thinking options only for the selected model. Keep its model-switching +// discovery here: other providers can supply a read-only catalog instead. export async function resolveKimiCatalogModels({ connection, sessionId, @@ -49,7 +34,7 @@ export async function resolveKimiCatalogModels({ return models; } const modelOption = findSelectConfigOption({ configOptions, category: "model" }); - if (!modelOption || !findSelectConfigOption({ configOptions, category: "thought_level" })) { + if (!modelOption) { return models; } @@ -72,11 +57,24 @@ export async function resolveKimiCatalogModels({ thinkingOptions.find((option) => option.isDefault)?.id ?? undefined, }); } catch (error) { + const errorMessage = toDiagnosticErrorMessage(error); + if (model.isDefault) { + logger.warn( + { modelId: model.id, error: errorMessage }, + `${provider} catalog probe could not refresh thinking options for current model "${model.id}"; keeping session options`, + ); + resolved.push(model); + continue; + } logger.warn( - { modelId: model.id, error: toDiagnosticErrorMessage(error) }, - `${provider} catalog probe could not resolve thinking options for model "${model.id}"; keeping its default options`, + { modelId: model.id, error: errorMessage }, + `${provider} catalog probe could not resolve thinking options for model "${model.id}"; omitting thinking options`, ); - resolved.push(model); + resolved.push({ + ...model, + thinkingOptions: undefined, + defaultThinkingOptionId: undefined, + }); } } return resolved;