From 8e0f495197c19c8b1ddfe219b86df9bab51907f5 Mon Sep 17 00:00:00 2001 From: zssggle-rgb Date: Wed, 1 Apr 2026 09:10:04 +0800 Subject: [PATCH] fix(acpx): preserve control command error details (#58613) --- extensions/acpx/src/runtime.test.ts | 78 +++++++++++++++++++ extensions/acpx/src/runtime.ts | 31 +++++++- .../acpx/src/test-utils/runtime-fixtures.ts | 7 +- 3 files changed, 113 insertions(+), 3 deletions(-) diff --git a/extensions/acpx/src/runtime.test.ts b/extensions/acpx/src/runtime.test.ts index 99d77cd5e10..0094f121805 100644 --- a/extensions/acpx/src/runtime.test.ts +++ b/extensions/acpx/src/runtime.test.ts @@ -300,6 +300,84 @@ describe("AcpxRuntime", () => { }); }); + it("surfaces structured control-command errors from sessions ensure", async () => { + const previousEnsureExit = process.env.MOCK_ACPX_ENSURE_EXIT_1; + const previousStatusSignal = process.env.MOCK_ACPX_STATUS_SIGNAL; + process.env.MOCK_ACPX_ENSURE_EXIT_1 = "1"; + process.env.MOCK_ACPX_STATUS_SIGNAL = "SIGTERM"; + + try { + const { runtime } = await createMockRuntimeFixture(); + await expect( + runtime.ensureSession({ + sessionKey: "agent:codex:acp:ensure-structured-error", + agent: "codex", + mode: "persistent", + }), + ).rejects.toMatchObject({ + code: "ACP_SESSION_INIT_FAILED", + message: "-32603: mock ensure failure", + }); + } finally { + if (previousEnsureExit === undefined) { + delete process.env.MOCK_ACPX_ENSURE_EXIT_1; + } else { + process.env.MOCK_ACPX_ENSURE_EXIT_1 = previousEnsureExit; + } + if (previousStatusSignal === undefined) { + delete process.env.MOCK_ACPX_STATUS_SIGNAL; + } else { + process.env.MOCK_ACPX_STATUS_SIGNAL = previousStatusSignal; + } + } + }); + + it("appends stderr details when control-command errors are generic", async () => { + const previousEnsureExit = process.env.MOCK_ACPX_ENSURE_EXIT_1; + const previousEnsureMessage = process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE; + const previousEnsureStderr = process.env.MOCK_ACPX_ENSURE_STDERR; + const previousStatusSignal = process.env.MOCK_ACPX_STATUS_SIGNAL; + process.env.MOCK_ACPX_ENSURE_EXIT_1 = "1"; + process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE = "Internal error"; + process.env.MOCK_ACPX_ENSURE_STDERR = "usage limit exceeded"; + process.env.MOCK_ACPX_STATUS_SIGNAL = "SIGTERM"; + + try { + const { runtime } = await createMockRuntimeFixture(); + await expect( + runtime.ensureSession({ + sessionKey: "agent:codex:acp:ensure-generic-error", + agent: "codex", + mode: "persistent", + }), + ).rejects.toMatchObject({ + code: "ACP_SESSION_INIT_FAILED", + message: "-32603: Internal error | usage limit exceeded", + }); + } finally { + if (previousEnsureExit === undefined) { + delete process.env.MOCK_ACPX_ENSURE_EXIT_1; + } else { + process.env.MOCK_ACPX_ENSURE_EXIT_1 = previousEnsureExit; + } + if (previousEnsureMessage === undefined) { + delete process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE; + } else { + process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE = previousEnsureMessage; + } + if (previousEnsureStderr === undefined) { + delete process.env.MOCK_ACPX_ENSURE_STDERR; + } else { + process.env.MOCK_ACPX_ENSURE_STDERR = previousEnsureStderr; + } + if (previousStatusSignal === undefined) { + delete process.env.MOCK_ACPX_STATUS_SIGNAL; + } else { + process.env.MOCK_ACPX_STATUS_SIGNAL = previousStatusSignal; + } + } + }); + it("serializes text plus image attachments into ACP prompt blocks", async () => { const { runtime, logPath } = await createMockRuntimeFixture(); diff --git a/extensions/acpx/src/runtime.ts b/extensions/acpx/src/runtime.ts index 944b4462bd6..1df6089e468 100644 --- a/extensions/acpx/src/runtime.ts +++ b/extensions/acpx/src/runtime.ts @@ -14,6 +14,7 @@ import type { import { AcpRuntimeError } from "../runtime-api.js"; import { toAcpMcpServers, type ResolvedAcpxPluginConfig } from "./config.js"; import { checkAcpxVersion, type AcpxVersionCheckResult } from "./ensure.js"; +import { parseControlJsonError } from "./runtime-internals/control-errors.js"; import { parseJsonLines, parsePromptEventLine, @@ -128,6 +129,25 @@ function shouldRetainNamedSessionForDeadStatus(detail: AcpxJsonObject | undefine return summary?.includes("queue owner unavailable") ?? false; } +function formatAcpxControlErrorMessage(params: { + code?: string; + message: string; + stderr: string; +}): string { + const baseMessage = params.code ? `${params.code}: ${params.message}` : params.message; + const stderrSummary = summarizeLogText(params.stderr); + if (!stderrSummary) { + return baseMessage; + } + if ( + /^(?:internal error|acpx reported an error)$/i.test(params.message) && + !baseMessage.includes(stderrSummary) + ) { + return `${baseMessage} | ${stderrSummary}`; + } + return baseMessage; +} + function findSessionIdentifierEvent(events: AcpxJsonObject[]): AcpxJsonObject | undefined { return events.find( (event) => @@ -1037,14 +1057,21 @@ export class AcpxRuntime implements AcpRuntime { } const events = parseJsonLines(result.stdout); - const errorEvent = events.map((event) => toAcpxErrorEvent(event)).find(Boolean) ?? null; + const errorEvent = + events + .map((event) => toAcpxErrorEvent(event) ?? parseControlJsonError(event)) + .find(Boolean) ?? null; if (errorEvent) { if (params.ignoreNoSession && errorEvent.code === "NO_SESSION") { return events; } throw new AcpRuntimeError( params.fallbackCode, - errorEvent.code ? `${errorEvent.code}: ${errorEvent.message}` : errorEvent.message, + formatAcpxControlErrorMessage({ + code: errorEvent.code, + message: errorEvent.message, + stderr: result.stderr, + }), ); } diff --git a/extensions/acpx/src/test-utils/runtime-fixtures.ts b/extensions/acpx/src/test-utils/runtime-fixtures.ts index bc9ffd5f341..1195e13e7a5 100644 --- a/extensions/acpx/src/test-utils/runtime-fixtures.ts +++ b/extensions/acpx/src/test-utils/runtime-fixtures.ts @@ -84,13 +84,16 @@ const setValue = command === "set" ? String(args[commandIndex + 2] || "") : ""; if (command === "sessions" && args[commandIndex + 1] === "ensure") { writeLog({ kind: "ensure", agent, args, sessionName: ensureName }); + if (process.env.MOCK_ACPX_ENSURE_STDERR) { + process.stderr.write(String(process.env.MOCK_ACPX_ENSURE_STDERR) + "\n"); + } if (process.env.MOCK_ACPX_ENSURE_EXIT_1 === "1") { return emitJsonAndExit({ jsonrpc: "2.0", id: null, error: { code: -32603, - message: "mock ensure failure", + message: process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE || "mock ensure failure", }, }, 1); } @@ -419,7 +422,9 @@ export async function readMockRuntimeLogEntries( export async function cleanupMockRuntimeFixtures(): Promise { delete process.env.MOCK_ACPX_LOG; delete process.env.MOCK_ACPX_CONFIG_SHOW_AGENTS; + delete process.env.MOCK_ACPX_ENSURE_ERROR_MESSAGE; delete process.env.MOCK_ACPX_ENSURE_EXIT_1; + delete process.env.MOCK_ACPX_ENSURE_STDERR; delete process.env.MOCK_ACPX_STATUS_STATUS; delete process.env.MOCK_ACPX_STATUS_SUMMARY; sharedMockCliScriptPath = null;