merge PR #5761 via eval/pr-5761: fix(utils): bounded default ptree stderr retention

2026-07-17 04:42:10 +02:00
parent 05e1314bd9 edba577e7a
commit f92d8feeec
3 changed files with 110 additions and 9 deletions
@@ -5,6 +5,7 @@
 ### Fixed

 - Fixed fatal cleanup failing to reach `process.exit()` when terminal stderr is revoked, and isolated rotating log files/audit state per process to prevent concurrent OMP instances from racing compression and rotation ([#5716](https://github.com/can1357/oh-my-pi/issues/5716)).
+- Bounded default `ptree.ChildProcess` stderr retention to the existing 32 KiB tail instead of retaining every raw chunk; long-lived subprocesses (LSP/DAP/RPC) no longer grow OMP memory with their stderr volume. Full capture must now be selected at spawn time via `spawn(cmd, { stderr: "full" })` / `exec(cmd, { stderr: "full" })`, and a retroactive `wait({ stderr: "full" })` on a default child throws instead of returning truncated data ([#5759](https://github.com/can1357/oh-my-pi/issues/5759)).

 ## [17.0.1] - 2026-07-16

@@ -72,6 +72,7 @@ export class TimeoutError extends AbortError {
 export interface WaitOptions {
 	allowNonZero?: boolean;
 	allowAbort?: boolean;
+	/** `full` requires upfront capture; `exec` enables it, while direct `spawn` callers pass `stderr: "full"`. */
 	stderr?: "full" | "buffer";
 }

@@ -97,7 +98,7 @@ export interface ExecResult {
 export class ChildProcess<In extends InMask = InMask> {
 	#nothrow = false;
 	#stderrTail = "";
-	#stderrChunks: Uint8Array[] = [];
+	#stderrChunks?: Uint8Array[];
 	#exitReason?: Exception;
 	#exitReasonPending?: Exception;
 	#stderrDone: Promise<void>;
@@ -107,8 +108,10 @@ export class ChildProcess<In extends InMask = InMask> {
 	constructor(
 		readonly proc: PipedSubprocess<In>,
 		readonly exposeStderr: boolean,
+		retainFullStderr = exposeStderr,
 	) {
-		// Eagerly drain stderr into a truncated tail string + raw chunks.
+		if (retainFullStderr) this.#stderrChunks = [];
+		// Eagerly drain stderr into a truncated tail, retaining raw chunks only for explicit full capture.
 		const dec = new TextDecoder();
 		const trim = () => {
 			if (this.#stderrTail.length > NonZeroExitError.MAX_TRACE)
@@ -123,7 +126,7 @@ export class ChildProcess<In extends InMask = InMask> {
 		this.#stderrDone = (async () => {
 			try {
 				for await (const chunk of stderrStream) {
-					this.#stderrChunks.push(chunk);
+					this.#stderrChunks?.push(chunk);
 					this.#stderrTail += dec.decode(chunk, { stream: true });
 					trim();
 				}
@@ -259,11 +262,15 @@ export class ChildProcess<In extends InMask = InMask> {

 	async wait(opts?: WaitOptions): Promise<ExecResult> {
 		const { allowNonZero = false, allowAbort = false, stderr: stderrMode = "buffer" } = opts ?? {};
+		const stderrChunks = this.#stderrChunks;
+		if (stderrMode === "full" && !stderrChunks) {
+			throw new Error('Full stderr capture must be requested when spawning the process (pass stderr: "full")');
+		}

 		const stdoutP = new Response(this.stdout).text();
 		const stderrP =
-			stderrMode === "full"
-				? this.#stderrDone.then(() => new TextDecoder().decode(Buffer.concat(this.#stderrChunks)))
+			stderrMode === "full" && stderrChunks
+				? this.#stderrDone.then(() => new TextDecoder().decode(Buffer.concat(stderrChunks)))
 				: this.#stderrDone.then(() => this.#stderrTail);

 		const [stdout, stderr] = await Promise.all([stdoutP, stderrP]);
@@ -328,11 +335,15 @@ type ChildSpawnOptions<In extends InMask = InMask> = Omit<
 > & {
 	signal?: AbortSignal;
 	detached?: boolean;
+	/** Expose and retain complete stderr for a later `wait({ stderr: "full" })`. */
 	stderr?: "full" | null;
 };

-/** Spawn a child process with piped stdout/stderr. */
-export function spawn<In extends InMask = InMask>(cmd: string[], opts?: ChildSpawnOptions<In>): ChildProcess<In> {
+function spawnInternal<In extends InMask = InMask>(
+	cmd: string[],
+	opts: ChildSpawnOptions<In> | undefined,
+	retainFullStderr: boolean,
+): ChildProcess<In> {
 	const { timeout = -1, signal, stderr, ...rest } = opts ?? {};
 	const child = Bun.spawn(cmd, {
 		stdin: "ignore",
@@ -341,12 +352,17 @@ export function spawn<In extends InMask = InMask>(cmd: string[], opts?: ChildSpa
 		windowsHide: true,
 		...rest,
 	});
-	const cp = new ChildProcess(child, stderr === "full");
+	const cp = new ChildProcess(child, stderr === "full", retainFullStderr);
 	if (signal) cp.attachSignal(signal);
 	if (timeout > 0) cp.attachTimeout(timeout);
 	return cp;
 }

+/** Spawn a child process with piped stdout/stderr. */
+export function spawn<In extends InMask = InMask>(cmd: string[], opts?: ChildSpawnOptions<In>): ChildProcess<In> {
+	return spawnInternal(cmd, opts, opts?.stderr === "full");
+}
+
 /** Options for exec. */
 export interface ExecOptions extends Omit<ChildSpawnOptions, "stderr" | "stdin">, WaitOptions {
 	input?: string | Buffer | Uint8Array;
@@ -357,7 +373,7 @@ export async function exec(cmd: string[], opts?: ExecOptions): Promise<ExecResul
 	const { input, stderr, allowAbort, allowNonZero, ...spawnOpts } = opts ?? {};
 	const stdin = typeof input === "string" ? Buffer.from(input) : input;
 	const resolved: ChildSpawnOptions = stdin === undefined ? spawnOpts : { ...spawnOpts, stdin };
-	using child = spawn(cmd, resolved);
+	using child = spawnInternal(cmd, resolved, stderr === "full");
 	return await child.wait({ stderr, allowAbort, allowNonZero });
 }

@@ -0,0 +1,84 @@
+import { describe, expect, it } from "bun:test";
+import { exec, NonZeroExitError, spawn } from "@oh-my-pi/pi-utils/ptree";
+
+const STDERR_LIMIT = NonZeroExitError.MAX_TRACE;
+const LARGE_STDERR_SIZE = 4 * 1024 * 1024;
+const STDERR_HEAD = "stderr-head\n";
+const STDERR_TAIL = "\nstderr-tail";
+const FULL_CAPTURE_ERROR = "Full stderr capture must be requested when spawning the process";
+
+function stderrFixture(size: number, exitCode = 0, stdout = ""): string[] {
+	const fillLength = size - STDERR_HEAD.length - STDERR_TAIL.length;
+	const script = [
+		`await Bun.stdout.write(${JSON.stringify(stdout)});`,
+		`await Bun.stderr.write(${JSON.stringify(STDERR_HEAD)} + "x".repeat(${fillLength}) + ${JSON.stringify(STDERR_TAIL)});`,
+		`process.exitCode = ${exitCode};`,
+	].join("\n");
+	return ["bun", "-e", script];
+}
+
+describe("ptree stderr capture", () => {
+	it("requires full stderr capture to be selected before spawning", async () => {
+		using child = spawn(stderrFixture(LARGE_STDERR_SIZE));
+		await child.exited;
+
+		let captureError: unknown;
+		try {
+			await child.wait({ stderr: "full" });
+		} catch (caught) {
+			captureError = caught;
+		}
+		expect(captureError).toBeInstanceOf(Error);
+		if (!(captureError instanceof Error)) throw new Error("Expected full capture error");
+		expect(captureError.message).toContain(FULL_CAPTURE_ERROR);
+		const result = await child.wait();
+
+		expect(result.stderr.length).toBe(STDERR_LIMIT);
+		expect(result.stderr).not.toContain(STDERR_HEAD);
+		expect(result.stderr).toEndWith(STDERR_TAIL);
+		expect(child.peekStderr()).toBe(result.stderr);
+	});
+
+	it("preserves complete stderr for explicit exec capture", async () => {
+		const result = await exec(stderrFixture(LARGE_STDERR_SIZE, 0, "stdout-ok"), { stderr: "full" });
+
+		expect(result.stdout).toBe("stdout-ok");
+		expect(result.stderr.length).toBe(LARGE_STDERR_SIZE);
+		expect(result.stderr).toStartWith(STDERR_HEAD);
+		expect(result.stderr).toEndWith(STDERR_TAIL);
+	});
+
+	it("preserves the live stream and retained stderr for explicit spawn capture", async () => {
+		const size = STDERR_LIMIT * 4;
+		using child = spawn(stderrFixture(size, 0, "spawn-ok"), { stderr: "full" });
+		const stderrStream = child.stderr;
+		if (!stderrStream) throw new Error("Expected exposed stderr stream");
+
+		const streamedStderr = new Response(stderrStream).text();
+		const [result, streamed] = await Promise.all([child.wait({ stderr: "full" }), streamedStderr]);
+
+		expect(result.stdout).toBe("spawn-ok");
+		expect(result.stderr.length).toBe(size);
+		expect(result.stderr).toStartWith(STDERR_HEAD);
+		expect(result.stderr).toEndWith(STDERR_TAIL);
+		expect(streamed).toBe(result.stderr);
+	});
+
+	it("keeps peek and nonzero errors on the bounded stderr tail", async () => {
+		using child = spawn(stderrFixture(STDERR_LIMIT * 4, 7));
+		let error: unknown;
+		try {
+			await child.exitedCleanly;
+		} catch (caught) {
+			error = caught;
+		}
+
+		expect(error).toBeInstanceOf(NonZeroExitError);
+		if (!(error instanceof NonZeroExitError)) throw new Error("Expected NonZeroExitError");
+		expect(error.stderr.length).toBe(STDERR_LIMIT);
+		expect(error.stderr).not.toContain(STDERR_HEAD);
+		expect(error.stderr).toEndWith(STDERR_TAIL);
+		expect(error.message).toContain(STDERR_TAIL);
+		expect(child.peekStderr()).toBe(error.stderr);
+	});
+});