diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 2d47bb3b6b..b097f8b1e5 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -200,15 +200,22 @@ export function summarizeACPRequestError(error: unknown): { return { message: String(error) }; } +// Keeps the JSON-RPC code so callers can still tell invalid params from other failures. +class ACPRequestError extends Error { + readonly code: number; + + constructor(error: ACPError) { + super(summarizeACPRequestError(error).message, { cause: error }); + this.name = "ACPRequestError"; + this.code = error.code; + } +} + function toACPRequestError(error: unknown): Error { if (!isACPError(error)) { return error instanceof Error ? error : new Error(String(error)); } - - const summary = summarizeACPRequestError(error); - const next = new Error(summary.message); - next.name = "ACPRequestError"; - return next; + return new ACPRequestError(error); } function resolveTerminalCommand( @@ -2021,8 +2028,9 @@ export class ACPAgentSession implements AgentSession, ACPClient { } const context = this.createProviderModeWriterContext(modeId, selection); - const providerResult = this.providerModeWriter - ? await this.providerModeWriter(context) + const providerModeWriter = this.providerModeWriter; + const providerResult = providerModeWriter + ? await this.runACPRequest(() => providerModeWriter(context)) : { handled: false }; if (providerResult.handled) { this.currentMode = providerResult.currentModeId ?? modeId; @@ -2067,15 +2075,17 @@ export class ACPAgentSession implements AgentSession, ACPClient { } } - if (this.beforeModeWriter) { - const beforeResult = await this.beforeModeWriter(context); + const beforeModeWriter = this.beforeModeWriter; + if (beforeModeWriter) { + const beforeResult = await this.runACPRequest(() => beforeModeWriter(context)); if (beforeResult?.configOptions) { this.configOptions = this.transformConfigOptions(beforeResult.configOptions); } } if (selection.hasAvailableModes) { - await this.connection.setSessionMode({ sessionId: this.sessionId, modeId }); + const { connection, sessionId } = this; + await this.runACPRequest(() => connection.setSessionMode({ sessionId, modeId })); this.currentMode = modeId; this.pushEvent({ type: "mode_changed", @@ -2091,11 +2101,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { throw new Error(`${this.provider} does not expose ACP mode switching`); } - const response = await this.connection.setSessionConfigOption({ - sessionId: this.sessionId, - configId: modeOption.id, - value: modeId, - }); + const { connection, sessionId } = this; + const response = await this.runACPRequest(() => + connection.setSessionConfigOption({ + sessionId, + configId: modeOption.id, + value: modeId, + }), + ); this.currentMode = this.applyConfigOptionResponse({ response, configId: modeOption.id, @@ -2206,11 +2219,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { return; } - const response = await this.connection.setSessionConfigOption({ - sessionId: this.sessionId, - configId: modelOption.id, - value: modelId, - }); + const { connection, sessionId } = this; + const response = await this.runACPRequest(() => + connection.setSessionConfigOption({ + sessionId, + configId: modelOption.id, + value: modelId, + }), + ); this.currentModel = this.applyConfigOptionResponse({ response, configId: modelOption.id, @@ -2234,8 +2250,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { return; } - if (this.thinkingOptionWriter) { - await this.thinkingOptionWriter(this.connection, this.sessionId, thinkingOptionId); + const thinkingOptionWriter = this.thinkingOptionWriter; + if (thinkingOptionWriter) { + const { connection, sessionId } = this; + await this.runACPRequest(() => thinkingOptionWriter(connection, sessionId, thinkingOptionId)); this.thinkingOptionId = thinkingOptionId; this.pushEvent({ type: "thinking_option_changed", @@ -2252,11 +2270,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { if (!option) { throw new Error(`${this.provider} does not expose ACP thought-level selection`); } - const response = await this.connection.setSessionConfigOption({ - sessionId: this.sessionId, - configId: option.id, - value: thinkingOptionId, - }); + const { connection, sessionId } = this; + const response = await this.runACPRequest(() => + connection.setSessionConfigOption({ + sessionId, + configId: option.id, + value: thinkingOptionId, + }), + ); this.thinkingOptionId = this.applyConfigOptionResponse({ response, configId: option.id, @@ -2302,11 +2323,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { ); } - const response = await this.connection.setSessionConfigOption({ - sessionId: this.sessionId, - configId: option.id, - value: requestedValue, - }); + const { connection, sessionId } = this; + const response = await this.runACPRequest(() => + connection.setSessionConfigOption({ + sessionId, + configId: option.id, + value: requestedValue, + }), + ); const currentValue = this.applyConfigOptionResponse({ response, configId: option.id, diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.request-errors.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.request-errors.test.ts new file mode 100644 index 0000000000..9e652a37d5 --- /dev/null +++ b/packages/server/src/server/agent/providers/generic-acp-agent.request-errors.test.ts @@ -0,0 +1,121 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { describe, expect, test } from "vitest"; + +import { createTestLogger } from "../../../test-utils/test-logger.js"; +import { GenericACPAgentClient } from "./generic-acp-agent.js"; + +describe("GenericACPAgentClient request errors", () => { + test("reports the agent's message when it rejects the configured mode", async () => { + await withFakeACPAgent("reject-set-mode", async (command, cwd) => { + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + + const created = client.createSession({ provider: "acp", cwd, modeId: "plan" }); + + await expect(created).rejects.toBeInstanceOf(Error); + await expect(created).rejects.toThrow("Internal error: Mode plan is not allowed"); + }); + }); + + test("reports the agent's message when it rejects the configured thinking option", async () => { + await withFakeACPAgent("reject-set-config-option", async (command, cwd) => { + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + + const created = client.createSession({ provider: "acp", cwd, thinkingOptionId: "high" }); + + await expect(created).rejects.toBeInstanceOf(Error); + await expect(created).rejects.toThrow("Internal error: Effort high is not allowed"); + }); + }); +}); + +async function withFakeACPAgent( + mode: "reject-set-mode" | "reject-set-config-option", + run: (command: [string, ...string[]], cwd: string) => Promise, +): Promise { + const testDir = await mkdtemp(path.join(tmpdir(), "paseo-acp-request-errors-")); + try { + const scriptPath = path.join(testDir, "fake-acp-agent.cjs"); + await writeFile(scriptPath, fakeACPAgentScript, "utf8"); + await run([process.execPath, scriptPath, mode], testDir); + } finally { + await rm(testDir, { recursive: true, force: true }); + } +} + +const fakeACPAgentScript = ` +const readline = require("node:readline"); + +const mode = process.argv[2]; +const rl = readline.createInterface({ input: process.stdin }); + +function write(message) { + process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...message }) + "\\n"); +} + +rl.on("line", (line) => { + const message = JSON.parse(line); + if (message.method === "initialize") { + write({ + id: message.id, + result: { + protocolVersion: message.params?.protocolVersion ?? 1, + agentCapabilities: { sessionCapabilities: { close: {} } }, + }, + }); + return; + } + + if (message.method === "session/new") { + write({ + id: message.id, + result: { + sessionId: "session-1", + modes: { + currentModeId: "agent", + availableModes: [ + { id: "agent", name: "Agent" }, + { id: "plan", name: "Plan" }, + ], + }, + configOptions: [ + { + id: "effort", + name: "Effort", + category: "thought_level", + type: "select", + currentValue: "low", + options: [ + { value: "low", name: "Low" }, + { value: "high", name: "High" }, + ], + }, + ], + }, + }); + return; + } + + if (message.method === "session/set_mode" && mode === "reject-set-mode") { + write({ + id: message.id, + error: { code: -32603, message: "Internal error", data: { details: "Mode plan is not allowed" } }, + }); + return; + } + + if (message.method === "session/set_config_option" && mode === "reject-set-config-option") { + write({ + id: message.id, + error: { code: -32603, message: "Internal error", data: { details: "Effort high is not allowed" } }, + }); + return; + } + + if (message.id !== undefined) { + write({ id: message.id, result: {} }); + } +}); +`;