mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-25 10:42:45 +00:00
fix(acp): report the agent's error when session setup is rejected (#5765)
ACP agents reject mode, model, thinking, and feature writes as plain JSON-RPC error objects. Those calls skipped the request error normalization, so schedule runs and metadata generation recorded "[object Object]" instead of the agent's message. Refs #4666 Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<void>,
|
||||
): Promise<void> {
|
||||
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: {} });
|
||||
}
|
||||
});
|
||||
`;
|
||||
Reference in New Issue
Block a user