mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(acpx): preserve Unicode in wrapper diagnostics (#103738)
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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<void> {
|
||||
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");
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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 "";
|
||||
|
||||
Reference in New Issue
Block a user