From 9bfecf46ee3eb08173c97dd7d307363f67483edf Mon Sep 17 00:00:00 2001 From: wings1029 Date: Sat, 11 Jul 2026 07:06:14 +0800 Subject: [PATCH] fix(acpx): preserve Unicode in wrapper diagnostics (#103738) Co-authored-by: Peter Steinberger --- extensions/acpx/src/codex-auth-bridge.test.ts | 119 ++++++++++++------ extensions/acpx/src/codex-auth-bridge.ts | 28 ++++- extensions/acpx/src/runtime.test.ts | 22 +++- extensions/acpx/src/runtime.ts | 3 +- 4 files changed, 122 insertions(+), 50 deletions(-) diff --git a/extensions/acpx/src/codex-auth-bridge.test.ts b/extensions/acpx/src/codex-auth-bridge.test.ts index e7ff33b1ce37..1c31aaccc5f3 100644 --- a/extensions/acpx/src/codex-auth-bridge.test.ts +++ b/extensions/acpx/src/codex-auth-bridge.test.ts @@ -10,6 +10,7 @@ import { resolveAcpxPluginConfig } from "./config.js"; import { OPENCLAW_ACPX_LEASE_ID_ARG, OPENCLAW_GATEWAY_INSTANCE_ID_ARG } from "./process-lease.js"; const execFileAsync = promisify(execFile); +const WRAPPER_STDERR_LOG_MAX_CHARS = 256 * 1024; const tempDirs: string[] = []; const previousEnv = { CODEX_HOME: process.env.CODEX_HOME, @@ -88,6 +89,49 @@ async function expectPathMissing(targetPath: string): Promise { expect((error as NodeJS.ErrnoException).code).toBe("ENOENT"); } +async function captureGeneratedCodexWrapperStderr( + source: string, + expectedExitCode = 0, +): Promise<{ log: string; stateDir: string }> { + const root = await makeTempDir(); + const stateDir = path.join(root, "state"); + const generated = generatedCodexPaths(stateDir); + const stderrScript = path.join(root, "emit-stderr.mjs"); + await fs.writeFile(stderrScript, source, "utf8"); + const pluginConfig = resolveAcpxPluginConfig({ rawConfig: {}, workspaceDir: root }); + await prepareAcpxCodexAuthConfig({ + pluginConfig, + stateDir, + resolveInstalledCodexAcpBinPath: async () => path.join(root, "unused-codex-acp.js"), + }); + + const leaseId = "lease-unicode"; + const execution = execFileAsync( + process.execPath, + [ + generated.wrapperPath, + "--openclaw-run-configured", + process.execPath, + stderrScript, + OPENCLAW_ACPX_LEASE_ID_ARG, + leaseId, + OPENCLAW_GATEWAY_INSTANCE_ID_ARG, + "gateway-test", + ], + { maxBuffer: WRAPPER_STDERR_LOG_MAX_CHARS * 2 }, + ); + if (expectedExitCode === 0) { + await execution; + } else { + await expect(execution).rejects.toMatchObject({ code: expectedExitCode }); + } + const log = await fs.readFile( + path.join(stateDir, "acpx", `codex-acp-wrapper.stderr.${leaseId}.log`), + "utf8", + ); + return { log, stateDir }; +} + afterEach(async () => { vi.restoreAllMocks(); restoreEnv("CODEX_HOME"); @@ -664,12 +708,7 @@ describe("prepareAcpxCodexAuthConfig", () => { }); it("captures Codex wrapper stderr in a stream-aware redacted per-lease log", async () => { - const root = await makeTempDir(); - const stateDir = path.join(root, "state"); - const generated = generatedCodexPaths(stateDir); - const stderrScript = path.join(root, "emit-stderr.mjs"); - await fs.writeFile( - stderrScript, + const { log, stateDir } = await captureGeneratedCodexWrapperStderr( `const chunks = [ "token=sk-test", "secret1234567890\\n", @@ -699,41 +738,7 @@ describe("prepareAcpxCodexAuthConfig", () => { setTimeout(writeNext, 5); } writeNext();`, - "utf8", - ); - const pluginConfig = resolveAcpxPluginConfig({ - rawConfig: { - agents: { - codex: { - command: `${process.execPath} ${stderrScript}`, - }, - }, - }, - workspaceDir: root, - }); - - await prepareAcpxCodexAuthConfig({ - pluginConfig, - stateDir, - resolveInstalledCodexAcpBinPath: async () => path.join(root, "codex-acp.js"), - }); - - await expect( - execFileAsync(process.execPath, [ - generated.wrapperPath, - "--openclaw-run-configured", - process.execPath, - stderrScript, - OPENCLAW_ACPX_LEASE_ID_ARG, - "lease-secret", - OPENCLAW_GATEWAY_INSTANCE_ID_ARG, - "gateway-test", - ]), - ).rejects.toMatchObject({ code: 1 }); - - const log = await fs.readFile( - path.join(stateDir, "acpx", "codex-acp-wrapper.stderr.lease-secret.log"), - "utf8", + 1, ); expect(log).toContain("token=[REDACTED]"); expect(log).toContain("Authorization: Bearer [REDACTED]"); @@ -758,6 +763,38 @@ describe("prepareAcpxCodexAuthConfig", () => { await expectPathMissing(path.join(stateDir, "acpx", "codex-acp-wrapper.stderr.log")); }); + it("keeps the persisted stderr line tail UTF-16 safe at the 256 KiB boundary", async () => { + const { log } = await captureGeneratedCodexWrapperStderr(` + const maxChars = 256 * 1024; + process.stderr.write("🚀\\n"); + process.stderr.write("a".repeat(maxChars - 3) + "\\n"); + `); + + expect(log).toBe(`\n${"a".repeat(WRAPPER_STDERR_LOG_MAX_CHARS - 3)}\n`); + expect(log).not.toContain("�"); + }); + + it("keeps the pending no-newline stderr tail UTF-16 safe", async () => { + const { log } = await captureGeneratedCodexWrapperStderr(` + const maxChars = 256 * 1024; + process.stderr.write("🚀" + "a".repeat(maxChars - 1)); + `); + + expect(log).toBe("a".repeat(WRAPPER_STDERR_LOG_MAX_CHARS - 1)); + expect(log).not.toContain("�"); + }); + + it("decodes split UTF-8 and flushes a partial final sequence", async () => { + const { log } = await captureGeneratedCodexWrapperStderr(` + process.stderr.write(Buffer.from([0xf0, 0x9f])); + setTimeout(() => { + process.stderr.write(Buffer.from([0x9a, 0x80, 0x0a, 0xe2])); + }, 50); + `); + + expect(log).toBe("🚀\n�"); + }); + it("leaves a custom Claude agent command alone", async () => { const root = await makeTempDir(); const stateDir = path.join(root, "state"); diff --git a/extensions/acpx/src/codex-auth-bridge.ts b/extensions/acpx/src/codex-auth-bridge.ts index ec4699d9495b..8320e88232b3 100644 --- a/extensions/acpx/src/codex-auth-bridge.ts +++ b/extensions/acpx/src/codex-auth-bridge.ts @@ -235,6 +235,7 @@ function buildAdapterWrapperScript(params: { import { appendFileSync, existsSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; import { spawn } from "node:child_process"; +import { StringDecoder } from "node:string_decoder"; import { fileURLToPath } from "node:url"; ${params.envSetup} @@ -285,7 +286,25 @@ function redactDiagnosticText(text) { return redacted; } +function tailUtf16Safe(text, maxChars) { + let start = Math.max(0, text.length - maxChars); + const startsInsideSurrogatePair = + start > 0 && + start < text.length && + text.charCodeAt(start) >= 0xdc00 && + text.charCodeAt(start) <= 0xdfff && + text.charCodeAt(start - 1) >= 0xd800 && + text.charCodeAt(start - 1) <= 0xdbff; + if (startsInsideSurrogatePair) { + start += 1; + } + return text.slice(start); +} + let pendingStderrLogText = ""; +// Pipe chunks can split a UTF-8 sequence. Preserve decoder state so diagnostic +// capture does not manufacture replacement characters between chunks. +const stderrDecoder = new StringDecoder("utf8"); const stderrPrivateKeyEndPattern = /-----END [A-Z ]*PRIVATE KEY-----/; function hasUnclosedPrivateKeyBlock(text) { @@ -310,7 +329,7 @@ function writeRedactedStderrLog(text) { appendFileSync(stderrLogPath, redactDiagnosticText(text), "utf8"); const current = readFileSync(stderrLogPath, "utf8"); if (current.length > stderrLogMaxChars) { - writeFileSync(stderrLogPath, current.slice(-stderrLogMaxChars), "utf8"); + writeFileSync(stderrLogPath, tailUtf16Safe(current, stderrLogMaxChars), "utf8"); } } catch { // Stderr capture is diagnostic-only; never break the ACP adapter. @@ -329,7 +348,7 @@ function flushFinalizedStderrLogText() { const lastLineBreak = pendingStderrLogText.lastIndexOf("\\n"); if (lastLineBreak === -1) { if (pendingStderrLogText.length > stderrLogMaxChars) { - pendingStderrLogText = pendingStderrLogText.slice(-stderrLogMaxChars); + pendingStderrLogText = tailUtf16Safe(pendingStderrLogText, stderrLogMaxChars); } return; } @@ -342,7 +361,7 @@ function flushFinalizedStderrLogText() { } if (flushEnd <= 0) { if (pendingStderrLogText.length > stderrLogMaxChars) { - pendingStderrLogText = pendingStderrLogText.slice(-stderrLogMaxChars); + pendingStderrLogText = tailUtf16Safe(pendingStderrLogText, stderrLogMaxChars); } return; } @@ -352,7 +371,7 @@ function flushFinalizedStderrLogText() { } function appendStderrLog(chunk) { - const text = typeof chunk === "string" ? chunk : chunk.toString("utf8"); + const text = stderrDecoder.write(chunk); if (!text) { return; } @@ -361,6 +380,7 @@ function appendStderrLog(chunk) { } function finishStderrLog() { + pendingStderrLogText += stderrDecoder.end(); const text = redactIncompletePrivateKeyTail(pendingStderrLogText); pendingStderrLogText = ""; writeRedactedStderrLog(text); diff --git a/extensions/acpx/src/runtime.test.ts b/extensions/acpx/src/runtime.test.ts index 435396e5ed1b..d07c715425d2 100644 --- a/extensions/acpx/src/runtime.test.ts +++ b/extensions/acpx/src/runtime.test.ts @@ -411,7 +411,21 @@ describe("AcpxRuntime fresh reset wrapper", () => { expect(ensureInput).not.toHaveProperty("thinking"); }); - it("adds Codex wrapper stderr tail to generic session initialization failures", async () => { + it.each([ + { + name: "adds the redacted Codex wrapper stderr tail to session initialization failures", + stderr: + "noise\nUnhandled error during session/new: deployment missing token=[REDACTED] sk-testsecret1234567890\n", + expectedFragment: "deployment missing", + forbiddenFragment: "sk-testsecret1234567890", + }, + { + name: "keeps the 6,000-unit Codex wrapper stderr tail UTF-16 safe", + stderr: `🚀${"a".repeat(5_999)}`, + expectedFragment: `Internal error: ${"a".repeat(5_999)}`, + forbiddenFragment: "\ude80", + }, + ])("$name", async ({ stderr, expectedFragment, forbiddenFragment }) => { const wrapperRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acpx-runtime-")); const leaseStore = makeLeaseStore(); const wrapperCommand = `node "${path.join(wrapperRoot, "codex-acp-wrapper.mjs")}"`; @@ -432,7 +446,7 @@ describe("AcpxRuntime fresh reset wrapper", () => { const leaseId = String(Array.from(leaseStore.leases.values())[0]?.leaseId); await fs.writeFile( path.join(wrapperRoot, `codex-acp-wrapper.stderr.${leaseId}.log`), - "noise\nUnhandled error during session/new: deployment missing token=[REDACTED] sk-testsecret1234567890\n", + stderr, "utf8", ); throw new Error("Internal error"); @@ -456,14 +470,14 @@ describe("AcpxRuntime fresh reset wrapper", () => { expect(outcome.error).toMatchObject({ name: "AcpRuntimeError", code: "ACP_SESSION_INIT_FAILED", - message: expect.stringContaining("deployment missing"), + message: expect.stringContaining(expectedFragment), }); const error = outcome.error; expect(error).toBeInstanceOf(AcpRuntimeError); if (!(error instanceof AcpRuntimeError)) { throw new Error("expected AcpRuntimeError"); } - expect(error.message).not.toContain("sk-testsecret1234567890"); + expect(error.message).not.toContain(forbiddenFragment); }); it("adds Codex wrapper stderr tail to generic first-turn failures", async () => { diff --git a/extensions/acpx/src/runtime.ts b/extensions/acpx/src/runtime.ts index 469ef89335a8..beb4bfae312f 100644 --- a/extensions/acpx/src/runtime.ts +++ b/extensions/acpx/src/runtime.ts @@ -27,6 +27,7 @@ import { import { parseStrictPositiveInteger } from "openclaw/plugin-sdk/number-runtime"; import { redactSensitiveText } from "openclaw/plugin-sdk/security-runtime"; import { normalizeStringEntries } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { sliceUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import { AcpRuntimeError, type AcpRuntime, type AcpRuntimeErrorCode } from "../runtime-api.js"; import { splitCommandParts } from "./command-line.js"; import { @@ -139,7 +140,7 @@ async function readCodexWrapperStderrTail(params: { "utf8", ); return compactDiagnosticText( - redactSensitiveText(text.slice(-CODEX_WRAPPER_ERROR_TAIL_MAX_CHARS)), + redactSensitiveText(sliceUtf16Safe(text, -CODEX_WRAPPER_ERROR_TAIL_MAX_CHARS)), ); } catch { return "";