mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-27 11:42:52 +00:00
Start a Cursor agent on a model that has no Fast variant (#5274)
* Start an ACP session when a stored feature value no longer applies Replaying stored config onto a new ACP session already tolerates a model the session cannot select, warning and keeping the provider default. Feature values were the exception: they were written unconditionally, and either Paseo's own "does not expose ACP feature" guard or the provider's -32602 rejection escaped out of initializeNewSession, so no agent was created. Cursor's `fast` is a per-model option, so switching a draft from a fast-capable model to one without a fast variant left a stored value the new session cannot take, and agent creation failed. Apply stored feature values best-effort at session start, matching the model path. A user-initiated setFeature still reports the failure. Co-authored-by: gengjiawen <technicalcute@gmail.com> * Tolerate a provider's invalid-params only after a model switch A stale option snapshot is only possible when the replay switched the model, since the model write answers with an empty response. Without a switch the options came straight from session/new, so a rejected write is a real failure the user needs to see. --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> Co-authored-by: gengjiawen <technicalcute@gmail.com>
This commit is contained in:
co-authored by
gengjiawen
parent
34c9fd03c9
commit
ba4595d4e8
@@ -144,6 +144,10 @@ function isACPError(value: unknown): value is ACPError {
|
||||
return isRecord(value) && typeof value.message === "string" && typeof value.code === "number";
|
||||
}
|
||||
|
||||
function isACPInvalidParams(error: unknown): boolean {
|
||||
return isACPError(error) && error.code === -32602;
|
||||
}
|
||||
|
||||
function extractACPErrorDataMessage(data: unknown): string | null {
|
||||
if (!isRecord(data)) {
|
||||
return null;
|
||||
@@ -2277,7 +2281,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
|
||||
|
||||
const option = findSelectConfigFeatureOption(this.configOptions, featureOption);
|
||||
if (!option) {
|
||||
throw new Error(`${this.provider} does not expose ACP feature '${featureId}'`);
|
||||
throw new Error(this.featureUnavailableMessage(featureId));
|
||||
}
|
||||
|
||||
const requestedValue = normalizeConfigFeatureValue(value);
|
||||
@@ -2819,6 +2823,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
|
||||
await this.setModeWithSelection({ modeId: configuredModeId, selection });
|
||||
}
|
||||
const configuredModelId = this.config.model;
|
||||
let switchedModel = false;
|
||||
if (configuredModelId && configuredModelId !== this.currentModel) {
|
||||
const selection = resolveACPModelSelection({
|
||||
modelId: configuredModelId,
|
||||
@@ -2827,6 +2832,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
|
||||
});
|
||||
try {
|
||||
await this.setModelWithSelection({ modelId: configuredModelId, selection });
|
||||
switchedModel = true;
|
||||
} catch (error) {
|
||||
if (!this.isModelSelectionUnavailableError(error)) {
|
||||
throw error;
|
||||
@@ -2845,10 +2851,39 @@ export class ACPAgentSession implements AgentSession, ACPClient {
|
||||
if (!Object.prototype.hasOwnProperty.call(configuredFeatureValues, featureOption.id)) {
|
||||
continue;
|
||||
}
|
||||
await this.setFeature(featureOption.id, configuredFeatureValues[featureOption.id]);
|
||||
try {
|
||||
await this.setFeature(featureOption.id, configuredFeatureValues[featureOption.id]);
|
||||
} catch (error) {
|
||||
if (!this.isStaleFeatureValueError(error, featureOption.id, switchedModel)) {
|
||||
throw error;
|
||||
}
|
||||
this.logger.warn(
|
||||
{ err: error, featureId: featureOption.id, model: this.currentModel },
|
||||
`${this.provider} cannot apply ACP feature '${featureOption.id}' to the current model; using the provider default`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A stored feature value is a preference carried over from whichever model the user
|
||||
* last configured, so the session it lands on may have no such option. Paseo's own
|
||||
* guard says so when the session's options are accurate. A model switch answers with
|
||||
* an empty response, leaving Paseo holding the previous model's options, and then the
|
||||
* provider is the one that rejects the write as invalid params. Outside those two
|
||||
* cases the write failed for a reason the user needs to see.
|
||||
*/
|
||||
private isStaleFeatureValueError(
|
||||
error: unknown,
|
||||
featureId: string,
|
||||
switchedModel: boolean,
|
||||
): boolean {
|
||||
if (this.isFeatureUnavailableError(error, featureId)) {
|
||||
return true;
|
||||
}
|
||||
return switchedModel && isACPInvalidParams(error);
|
||||
}
|
||||
|
||||
private warnInvalidSelection(value: string, message: string): void {
|
||||
this.logger.warn({ value }, message);
|
||||
}
|
||||
@@ -2861,6 +2896,14 @@ export class ACPAgentSession implements AgentSession, ACPClient {
|
||||
return error instanceof Error && error.message === this.modelSelectionUnavailableMessage();
|
||||
}
|
||||
|
||||
private featureUnavailableMessage(featureId: string): string {
|
||||
return `${this.provider} does not expose ACP feature '${featureId}'`;
|
||||
}
|
||||
|
||||
private isFeatureUnavailableError(error: unknown, featureId: string): boolean {
|
||||
return error instanceof Error && error.message === this.featureUnavailableMessage(featureId);
|
||||
}
|
||||
|
||||
private translateSessionUpdate(update: SessionUpdate): AgentStreamEvent[] {
|
||||
if (update.sessionUpdate === "user_message_chunk") {
|
||||
return this.handleUserMessageChunk(update);
|
||||
|
||||
@@ -1,22 +1,42 @@
|
||||
import type { SessionConfigOption } from "@agentclientprotocol/sdk";
|
||||
import { describe, expect, test, vi } from "vitest";
|
||||
|
||||
import { ACPAgentSession } from "./acp-agent.js";
|
||||
import type { SpawnedACPProcess, SessionStateResponse } from "./acp-agent.js";
|
||||
import type { AgentSessionConfig } from "../agent-sdk-types.js";
|
||||
import { CURSOR_FAST_FEATURE_OPTION, CursorACPAgentClient } from "./cursor-acp-agent.js";
|
||||
import { createTestLogger } from "../../../test-utils/test-logger.js";
|
||||
|
||||
function captureWarnings(): {
|
||||
logger: ReturnType<typeof createTestLogger>;
|
||||
messages: () => string[];
|
||||
} {
|
||||
const logged: string[] = [];
|
||||
const logger = createTestLogger();
|
||||
const child = {
|
||||
trace: vi.fn(),
|
||||
warn: (_context: unknown, message: string) => logged.push(message),
|
||||
};
|
||||
vi.spyOn(logger, "child").mockReturnValue(
|
||||
child as unknown as ReturnType<typeof createTestLogger>,
|
||||
);
|
||||
return { logger, messages: () => logged };
|
||||
}
|
||||
|
||||
function fastConfigOption(currentValue: "false" | "true"): SessionConfigOption {
|
||||
return {
|
||||
id: "fast",
|
||||
name: "Fast",
|
||||
type: "select",
|
||||
currentValue,
|
||||
options: [
|
||||
{ value: "false", name: "Off" },
|
||||
{ value: "true", name: "Fast" },
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
describe("CursorACPAgentClient model discovery", () => {
|
||||
function fastConfigOption(currentValue: "false" | "true") {
|
||||
return {
|
||||
id: "fast",
|
||||
name: "Fast",
|
||||
type: "select" as const,
|
||||
currentValue,
|
||||
options: [
|
||||
{ value: "false", name: "Off" },
|
||||
{ value: "true", name: "Fast" },
|
||||
],
|
||||
};
|
||||
}
|
||||
class TestCursorACPAgentClient extends CursorACPAgentClient {
|
||||
constructor(response: SessionStateResponse) {
|
||||
super({
|
||||
@@ -181,3 +201,157 @@ describe("CursorACPAgentClient model discovery", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("CursorACPAgentClient session start", () => {
|
||||
// Kimi K3 exposes reasoning but no fast option.
|
||||
const KIMI_K3_CONFIG_OPTIONS: SessionConfigOption[] = [
|
||||
{
|
||||
id: "reasoning",
|
||||
name: "Reasoning",
|
||||
category: "thought_level",
|
||||
type: "select",
|
||||
currentValue: "max",
|
||||
options: [
|
||||
{ value: "low", name: "Low" },
|
||||
{ value: "high", name: "High" },
|
||||
{ value: "max", name: "Max" },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
function createCursorSession(
|
||||
config: Partial<AgentSessionConfig>,
|
||||
session: {
|
||||
currentModelId: string;
|
||||
configOptions: SessionConfigOption[];
|
||||
setSessionConfigOption?: () => Promise<{ configOptions: SessionConfigOption[] }>;
|
||||
} = { currentModelId: "kimi-k3", configOptions: KIMI_K3_CONFIG_OPTIONS },
|
||||
logger = createTestLogger(),
|
||||
): ACPAgentSession {
|
||||
class StubbedCursorSession extends ACPAgentSession {
|
||||
protected override async spawnProcess(): Promise<SpawnedACPProcess> {
|
||||
return {
|
||||
child: { kill: vi.fn(), exitCode: 0, signalCode: null, once: vi.fn() },
|
||||
connection: {
|
||||
newSession: vi.fn().mockResolvedValue({
|
||||
sessionId: "session-1",
|
||||
models: {
|
||||
currentModelId: session.currentModelId,
|
||||
availableModels: [
|
||||
{ modelId: "composer-2.5", name: "Composer 2.5", description: null },
|
||||
{ modelId: "kimi-k3", name: "Kimi K3", description: null },
|
||||
],
|
||||
},
|
||||
configOptions: session.configOptions,
|
||||
}),
|
||||
// cursor-agent switches the model without returning refreshed config options.
|
||||
unstable_setSessionModel: vi.fn().mockResolvedValue(undefined),
|
||||
setSessionConfigOption:
|
||||
session.setSessionConfigOption ??
|
||||
vi.fn().mockResolvedValue({ configOptions: session.configOptions }),
|
||||
},
|
||||
initialize: { agentCapabilities: {} },
|
||||
} as SpawnedACPProcess;
|
||||
}
|
||||
}
|
||||
|
||||
return new StubbedCursorSession(
|
||||
{ provider: "acp", cwd: "/tmp/cursor", ...config },
|
||||
{
|
||||
provider: "acp",
|
||||
logger,
|
||||
defaultCommand: ["cursor-agent", "acp"],
|
||||
defaultModes: [],
|
||||
capabilities: { supportsStreaming: true, supportsSessionPersistence: true },
|
||||
configFeatureOptions: [CURSOR_FAST_FEATURE_OPTION],
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
test("starts on a model without a fast variant while Fast is still stored", async () => {
|
||||
// The composer stored Fast while a fast-capable model was selected, and it is
|
||||
// still stored after the draft moved to a model that has no fast variant.
|
||||
const warn = captureWarnings();
|
||||
const session = createCursorSession(
|
||||
{ model: "kimi-k3", featureValues: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } },
|
||||
{ currentModelId: "kimi-k3", configOptions: KIMI_K3_CONFIG_OPTIONS },
|
||||
warn.logger,
|
||||
);
|
||||
|
||||
await session.initializeNewSession();
|
||||
|
||||
expect(session.id).toBe("session-1");
|
||||
expect(warn.messages()).toContain(
|
||||
"acp cannot apply ACP feature 'fast' to the current model; using the provider default",
|
||||
);
|
||||
});
|
||||
|
||||
test("starts when the CLI rejects a stored feature the switched-to model dropped", async () => {
|
||||
// The CLI keeps the pre-switch config options for the session, so Fast is
|
||||
// still listed after the model moves to one that has no fast variant.
|
||||
const warn = captureWarnings();
|
||||
const session = createCursorSession(
|
||||
{ model: "kimi-k3", featureValues: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } },
|
||||
{
|
||||
currentModelId: "composer-2.5",
|
||||
configOptions: [fastConfigOption("false"), ...KIMI_K3_CONFIG_OPTIONS],
|
||||
setSessionConfigOption: vi.fn().mockRejectedValue(
|
||||
Object.assign(new Error("Invalid params"), {
|
||||
code: -32602,
|
||||
data: { message: "Unknown model config option: fast" },
|
||||
}),
|
||||
),
|
||||
},
|
||||
warn.logger,
|
||||
);
|
||||
|
||||
await session.initializeNewSession();
|
||||
|
||||
expect(session.id).toBe("session-1");
|
||||
expect(warn.messages()).toContain(
|
||||
"acp cannot apply ACP feature 'fast' to the current model; using the provider default",
|
||||
);
|
||||
});
|
||||
|
||||
test("still fails session start when a stored feature write fails for another reason", async () => {
|
||||
const session = createCursorSession(
|
||||
{ model: "kimi-k3", featureValues: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } },
|
||||
{
|
||||
currentModelId: "composer-2.5",
|
||||
configOptions: [fastConfigOption("false"), ...KIMI_K3_CONFIG_OPTIONS],
|
||||
setSessionConfigOption: vi.fn().mockRejectedValue(new Error("write EPIPE")),
|
||||
},
|
||||
);
|
||||
|
||||
await expect(session.initializeNewSession()).rejects.toThrow("write EPIPE");
|
||||
});
|
||||
|
||||
test("reports the failure when the user turns Fast on for a model without it", async () => {
|
||||
const session = createCursorSession({ model: "kimi-k3" });
|
||||
await session.initializeNewSession();
|
||||
|
||||
await expect(session.setFeature(CURSOR_FAST_FEATURE_OPTION.id, "true")).rejects.toThrow(
|
||||
"acp does not expose ACP feature 'fast'",
|
||||
);
|
||||
});
|
||||
|
||||
test("still fails session start when the provider rejects a write it should accept", async () => {
|
||||
// No model switch, so the session's options came straight from session/new and the
|
||||
// provider disagreeing about a feature it just advertised is a real failure.
|
||||
const session = createCursorSession(
|
||||
{ model: "composer-2.5", featureValues: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } },
|
||||
{
|
||||
currentModelId: "composer-2.5",
|
||||
configOptions: [fastConfigOption("false")],
|
||||
setSessionConfigOption: vi.fn().mockRejectedValue(
|
||||
Object.assign(new Error("Invalid params"), {
|
||||
code: -32602,
|
||||
data: { message: "sessionId is required" },
|
||||
}),
|
||||
),
|
||||
},
|
||||
);
|
||||
|
||||
await expect(session.initializeNewSession()).rejects.toThrow("Invalid params");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user