mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(qa-lab): bound retained child output without changing verdicts (#108981)
Reuse the canonical settlement owner for boundary helpers, reject truncated verification JSON, and preserve complete live output streams. Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -10,6 +10,13 @@ import {
|
||||
} from "./child-output.js";
|
||||
|
||||
describe("qa child output", () => {
|
||||
it("does not mark an exactly full first stderr chunk as truncated", () => {
|
||||
const tail = createQaChildOutputTail(4);
|
||||
appendQaChildOutputTail(tail, Buffer.from("tail"));
|
||||
expect(tail.truncated).toBe(false);
|
||||
expect(formatQaChildOutputTail(tail, "stderr")).toBe("tail");
|
||||
});
|
||||
|
||||
it("keeps capped stdout UTF-8 safe when the byte cap splits a code point", () => {
|
||||
const text = "ok \u{1f600} done";
|
||||
const capture = createQaChildOutputCapture(Buffer.byteLength("ok \u{1f600}", "utf8") - 1);
|
||||
|
||||
@@ -77,8 +77,8 @@ export function createQaChildOutputTail(maxBytes = QA_CHILD_STDERR_TAIL_BYTES) {
|
||||
export function appendQaChildOutputTail(tail: QaChildOutputTail, chunk: unknown) {
|
||||
const buffer = toBuffer(chunk);
|
||||
if (buffer.byteLength >= tail.maxBytes) {
|
||||
tail.truncated ||= tail.buffer.byteLength > 0 || buffer.byteLength > tail.maxBytes;
|
||||
tail.buffer = Buffer.from(buffer.subarray(buffer.byteLength - tail.maxBytes));
|
||||
tail.truncated = true;
|
||||
return;
|
||||
}
|
||||
const next = Buffer.concat([tail.buffer, buffer], tail.buffer.byteLength + buffer.byteLength);
|
||||
@@ -90,8 +90,12 @@ export function appendQaChildOutputTail(tail: QaChildOutputTail, chunk: unknown)
|
||||
tail.truncated = true;
|
||||
}
|
||||
|
||||
export function readQaChildOutputTail(tail: QaChildOutputTail) {
|
||||
return decodeUtf8Tail(tail.buffer, tail.truncated);
|
||||
}
|
||||
|
||||
export function formatQaChildOutputTail(tail: QaChildOutputTail, label: string) {
|
||||
const text = decodeUtf8Tail(tail.buffer, tail.truncated).trim();
|
||||
const text = readQaChildOutputTail(tail).trim();
|
||||
if (!text) {
|
||||
return "";
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js";
|
||||
import {
|
||||
assertQaGatewayCredentialLeaseQuarantine,
|
||||
createQaGatewayProcessBoundaryController,
|
||||
@@ -121,6 +122,19 @@ describe("gateway process boundary", () => {
|
||||
expect.objectContaining({ generation: prepared.generation }),
|
||||
]);
|
||||
|
||||
// Whitespace leaves JSON valid, but oversized verification output must not
|
||||
// authenticate a truncated proof as a complete response.
|
||||
await fs.appendFile(
|
||||
`${prepared.identityFilePath}.runtime`,
|
||||
" ".repeat(QA_CHILD_STDOUT_MAX_BYTES),
|
||||
);
|
||||
await expect(controller.markReady(identity)).rejects.toThrow("proxy stdout exceeded");
|
||||
|
||||
await fs.writeFile(launcherPath, '#!/bin/sh\ncat "$3.runtime" >&2\nexit 7\n');
|
||||
await expect(controller.markReady(identity)).rejects.toThrow(
|
||||
"proxy exited 7 (stderr truncated)",
|
||||
);
|
||||
|
||||
const malformed = await controller.prepare({
|
||||
args: ["gateway", "run"],
|
||||
cwd: tempRoot,
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { spawn, type ChildProcess } from "node:child_process";
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { constants as fsConstants } from "node:fs";
|
||||
import fs from "node:fs/promises";
|
||||
@@ -7,6 +7,8 @@ import { setTimeout as sleep } from "node:timers/promises";
|
||||
import { isRecord } from "openclaw/plugin-sdk/channel-secret-basic-runtime";
|
||||
import { isPathInside } from "openclaw/plugin-sdk/file-access-runtime";
|
||||
import { replaceFileAtomic } from "openclaw/plugin-sdk/security-runtime";
|
||||
import { QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js";
|
||||
import { runQaScenarioCommandLifecycle } from "./test-file-scenario-command-lifecycle.js";
|
||||
|
||||
const PROCESS_BOUNDARY_VERSION = 1;
|
||||
const PROCESS_BOUNDARY_START_TIMEOUT_MS = 30_000;
|
||||
@@ -300,45 +302,30 @@ async function runBoundaryLauncherCommand(params: {
|
||||
launcherPath: string;
|
||||
timeoutMs: number;
|
||||
}) {
|
||||
const child = spawn(params.launcherPath, params.args, {
|
||||
// The proxy gets its own process group; the verified SUT identity and its
|
||||
// UID-quiescence checks remain owned by the launcher, never this cleanup.
|
||||
const result = await runQaScenarioCommandLifecycle({
|
||||
command: params.launcherPath,
|
||||
args: [...params.args],
|
||||
cwd: process.cwd(),
|
||||
timeoutMs: params.timeoutMs,
|
||||
env: {
|
||||
HOME: process.env.HOME,
|
||||
LANG: process.env.LANG ?? "C.UTF-8",
|
||||
PATH: process.env.PATH,
|
||||
},
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
const stdout: Buffer[] = [];
|
||||
const stderr: Buffer[] = [];
|
||||
child.stdout.on("data", (chunk) => stdout.push(Buffer.from(chunk)));
|
||||
child.stderr.on("data", (chunk) => stderr.push(Buffer.from(chunk)));
|
||||
let timeout: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeoutPromise = new Promise<never>((_resolve, reject) => {
|
||||
timeout = setTimeout(() => {
|
||||
child.kill("SIGKILL");
|
||||
reject(new Error(`process-boundary ${params.label} proxy timed out`));
|
||||
}, params.timeoutMs);
|
||||
});
|
||||
let exitCode: number;
|
||||
try {
|
||||
exitCode = await Promise.race([
|
||||
new Promise<number>((resolve, reject) => {
|
||||
child.once("error", reject);
|
||||
child.once("close", (code) => resolve(code ?? 1));
|
||||
}),
|
||||
timeoutPromise,
|
||||
]);
|
||||
} finally {
|
||||
if (timeout) {
|
||||
clearTimeout(timeout);
|
||||
}
|
||||
}
|
||||
if (exitCode !== 0) {
|
||||
if (result.exitCode !== 0) {
|
||||
throw new Error(
|
||||
`process-boundary ${params.label} proxy exited ${exitCode}: ${Buffer.concat(stderr).toString("utf8").trim()}`,
|
||||
`process-boundary ${params.label} proxy exited ${result.exitCode}${result.stderrTruncated ? " (stderr truncated)" : ""}: ${result.failureMessage ?? result.stderr.trim()}`,
|
||||
);
|
||||
}
|
||||
return Buffer.concat(stdout).toString("utf8");
|
||||
if (result.stdoutTruncated) {
|
||||
throw new Error(
|
||||
`process-boundary ${params.label} proxy stdout exceeded ${QA_CHILD_STDOUT_MAX_BYTES} bytes`,
|
||||
);
|
||||
}
|
||||
return result.stdout;
|
||||
}
|
||||
|
||||
async function runBoundaryVerification(params: {
|
||||
|
||||
@@ -1,5 +1,15 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import path from "node:path";
|
||||
import {
|
||||
appendQaChildOutput,
|
||||
appendQaChildOutputTail,
|
||||
createQaChildOutputCapture,
|
||||
createQaChildOutputTail,
|
||||
QA_CHILD_STDERR_TAIL_BYTES,
|
||||
QA_CHILD_STDOUT_MAX_BYTES,
|
||||
readQaChildOutput,
|
||||
readQaChildOutputTail,
|
||||
} from "./child-output.js";
|
||||
import { createQaPosixCommandSettlement } from "./posix-command-settlement.js";
|
||||
import { runQaWindowsTaskkill } from "./windows-system-tools.js";
|
||||
|
||||
@@ -18,6 +28,8 @@ export type QaScenarioCommandResult = {
|
||||
signal?: NodeJS.Signals | null;
|
||||
stdout: string;
|
||||
stderr: string;
|
||||
stdoutTruncated?: true;
|
||||
stderrTruncated?: true;
|
||||
};
|
||||
|
||||
type QaScenarioCommandTerminalResult = Pick<
|
||||
@@ -41,8 +53,10 @@ export function runQaScenarioCommandLifecycle(
|
||||
env: execution.env,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
const stdout: Buffer[] = [];
|
||||
const stderr: Buffer[] = [];
|
||||
// Logs are diagnostics, not native test verdicts: bound retention without
|
||||
// failing noisy commands or truncating their live onOutput stream.
|
||||
const stdout = createQaChildOutputCapture();
|
||||
const stderr = createQaChildOutputTail();
|
||||
const commandLabel = path.basename(execution.command);
|
||||
createQaPosixCommandSettlement({
|
||||
child,
|
||||
@@ -104,8 +118,10 @@ export function runQaScenarioCommandLifecycle(
|
||||
resolve({
|
||||
...result,
|
||||
...(settlementFailure && result.exitCode === 0 ? { exitCode: 1 } : {}),
|
||||
stdout: Buffer.concat(stdout).toString("utf8"),
|
||||
stderr: Buffer.concat(stderr).toString("utf8"),
|
||||
stdout: readQaChildOutput(stdout),
|
||||
stderr: readQaChildOutputTail(stderr),
|
||||
...(stdout.exceeded ? { stdoutTruncated: true } : {}),
|
||||
...(stderr.truncated ? { stderrTruncated: true } : {}),
|
||||
...(settlementFailure
|
||||
? result.failureMessage
|
||||
? { failureMessage: `${result.failureMessage}; settlement: ${settlementFailure}` }
|
||||
@@ -115,12 +131,12 @@ export function runQaScenarioCommandLifecycle(
|
||||
},
|
||||
onStderrData: (chunk) => {
|
||||
const buffered = Buffer.from(chunk);
|
||||
stderr.push(buffered);
|
||||
appendQaChildOutputTail(stderr, buffered);
|
||||
execution.onOutput?.("stderr", buffered);
|
||||
},
|
||||
onStdoutData: (chunk) => {
|
||||
const buffered = Buffer.from(chunk);
|
||||
stdout.push(buffered);
|
||||
appendQaChildOutput(stdout, buffered);
|
||||
execution.onOutput?.("stdout", buffered);
|
||||
},
|
||||
processGroupId: isWindows ? undefined : child.pid,
|
||||
@@ -129,6 +145,19 @@ export function runQaScenarioCommandLifecycle(
|
||||
});
|
||||
}
|
||||
|
||||
export function formatQaScenarioCommandOutput(result: QaScenarioCommandResult): string {
|
||||
return [
|
||||
result.stdoutTruncated
|
||||
? `[stdout truncated to first ${QA_CHILD_STDOUT_MAX_BYTES} bytes]\n`
|
||||
: "",
|
||||
result.stdout,
|
||||
result.stderrTruncated
|
||||
? `\n[stderr truncated to last ${QA_CHILD_STDERR_TAIL_BYTES} bytes]\n`
|
||||
: "",
|
||||
result.stderr,
|
||||
].join("");
|
||||
}
|
||||
|
||||
export function resetQaScenarioCommandCleanupTimings() {
|
||||
timeoutKillGraceMs = QA_SCENARIO_COMMAND_TIMEOUT_KILL_GRACE_MS;
|
||||
timeoutForceSettleMs = QA_SCENARIO_COMMAND_TIMEOUT_FORCE_SETTLE_MS;
|
||||
|
||||
@@ -5,6 +5,7 @@ import { z } from "zod";
|
||||
import type { QaSeedScenarioWithSource } from "./scenario-catalog.js";
|
||||
import { shellQuote } from "./shell-quote.js";
|
||||
import {
|
||||
formatQaScenarioCommandOutput,
|
||||
runQaScenarioCommandLifecycle,
|
||||
type QaScenarioCommandExecution,
|
||||
} from "./test-file-scenario-command-lifecycle.js";
|
||||
@@ -201,7 +202,7 @@ export async function runDockerE2eBatch(params: {
|
||||
}
|
||||
await fs.writeFile(
|
||||
logPath,
|
||||
`$ ${shellQuote(process.execPath)} scripts/test-docker-all.mjs\n${commandResult.stdout}${commandResult.stderr}`,
|
||||
`$ ${shellQuote(process.execPath)} scripts/test-docker-all.mjs\n${formatQaScenarioCommandOutput(commandResult)}`,
|
||||
"utf8",
|
||||
);
|
||||
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { runQaScenarioCommandLifecycle } from "./test-file-scenario-command-lifecycle.js";
|
||||
import { QA_CHILD_STDERR_TAIL_BYTES, QA_CHILD_STDOUT_MAX_BYTES } from "./child-output.js";
|
||||
import {
|
||||
formatQaScenarioCommandOutput,
|
||||
runQaScenarioCommandLifecycle,
|
||||
} from "./test-file-scenario-command-lifecycle.js";
|
||||
import {
|
||||
runQaTestFileScenarios,
|
||||
type QaScenarioCommandExecution,
|
||||
@@ -22,6 +26,46 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("qa test file scenario runner", () => {
|
||||
it.each([0, 7])(
|
||||
"bounds retained child logs without changing exit %i or live output",
|
||||
async (exitCode) => {
|
||||
const streamed = { stdout: 0, stderr: 0 };
|
||||
const result = await runQaScenarioCommandLifecycle({
|
||||
command: process.execPath,
|
||||
args: [
|
||||
"-e",
|
||||
[
|
||||
`process.stdout.write('x'.repeat(${QA_CHILD_STDOUT_MAX_BYTES * 2}));`,
|
||||
`process.stderr.write('🦞'.repeat(${QA_CHILD_STDERR_TAIL_BYTES / 2 + 1}) + '\\nfinal diagnostic\\n');`,
|
||||
`process.exitCode = ${exitCode};`,
|
||||
].join("\n"),
|
||||
],
|
||||
cwd: process.cwd(),
|
||||
env: process.env,
|
||||
onOutput: (stream, chunk) => {
|
||||
streamed[stream] += chunk.byteLength;
|
||||
},
|
||||
timeoutMs: 5_000,
|
||||
});
|
||||
|
||||
expect(Buffer.byteLength(result.stdout)).toBe(QA_CHILD_STDOUT_MAX_BYTES);
|
||||
expect(Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(QA_CHILD_STDERR_TAIL_BYTES);
|
||||
expect(result.exitCode).toBe(exitCode);
|
||||
expect(result.failureMessage).toBeUndefined();
|
||||
expect(result.stdoutTruncated).toBe(true);
|
||||
expect(result.stderrTruncated).toBe(true);
|
||||
const log = formatQaScenarioCommandOutput(result);
|
||||
expect(log.startsWith("[stdout truncated to first")).toBe(true);
|
||||
expect(log.includes("[stderr truncated to last")).toBe(true);
|
||||
expect(result.stderr).toContain("final diagnostic");
|
||||
expect(result.stderr).not.toContain("�");
|
||||
expect(streamed).toEqual({
|
||||
stdout: QA_CHILD_STDOUT_MAX_BYTES * 2,
|
||||
stderr: QA_CHILD_STDERR_TAIL_BYTES * 2 + 4 + Buffer.byteLength("\nfinal diagnostic\n"),
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("streams real native subprocess output before command settlement", async () => {
|
||||
const observed: Array<{ stream: "stderr" | "stdout"; value: string }> = [];
|
||||
let settled = false;
|
||||
|
||||
@@ -22,6 +22,7 @@ import type { QaSeedScenarioWithSource } from "./scenario-catalog.js";
|
||||
import type { QaScorecardEvidenceMode } from "./scorecard-taxonomy.js";
|
||||
import { shellQuote } from "./shell-quote.js";
|
||||
import {
|
||||
formatQaScenarioCommandOutput,
|
||||
runQaScenarioCommandLifecycle,
|
||||
type QaScenarioCommandExecution,
|
||||
type QaScenarioCommandResult,
|
||||
@@ -293,12 +294,7 @@ async function runScenarioCommandSteps(params: {
|
||||
: {}),
|
||||
timeoutMs,
|
||||
});
|
||||
if (result.stdout) {
|
||||
logChunks.push(result.stdout);
|
||||
}
|
||||
if (result.stderr) {
|
||||
logChunks.push(result.stderr);
|
||||
}
|
||||
logChunks.push(formatQaScenarioCommandOutput(result));
|
||||
if (result.failureMessage || result.exitCode !== 0 || result.signal) {
|
||||
failureMessage =
|
||||
result.failureMessage ??
|
||||
|
||||
Reference in New Issue
Block a user