From 4eaca82fa611935624b729f2c7459d55f1c9741b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korm=C3=A1kur?= Date: Sat, 11 Jul 2026 00:15:07 +0000 Subject: [PATCH] fix(tui): suppress unmanaged macOS stderr writes corrupting the viewport On macOS, libmalloc writes runtime diagnostics (e.g. "MallocStackLogging: can't turn off malloc stack logging because it was not enabled") directly to fd 2 of the running TUI process at arbitrary times, painting into the viewport. The existing env-strip only protects child processes. Add a fd-level stderr guard in pi-utils (suppressTerminalStderr / restoreTerminalStderr) that dup2-redirects fd 2 to the omp log file while the TUI owns the terminal, and restores it at every ownership handoff (external editor, Ctrl+Z suspend, shutdown, crash restore). Postmortem fatal handlers restore fd 2 before printing so crash reports stay visible. Mirrors openai/codex#24459. --- packages/coding-agent/CHANGELOG.md | 4 + packages/tui/CHANGELOG.md | 4 + packages/tui/src/terminal.ts | 23 +++- packages/utils/CHANGELOG.md | 4 + packages/utils/src/index.ts | 1 + packages/utils/src/postmortem.ts | 8 ++ packages/utils/src/stderr-guard.ts | 149 +++++++++++++++++++++++ packages/utils/test/stderr-guard.test.ts | 97 +++++++++++++++ 8 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 packages/utils/src/stderr-guard.ts create mode 100644 packages/utils/test/stderr-guard.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d8041c95c..c4b307f6b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed macOS runtime diagnostics (e.g. `MallocStackLogging: can't turn off malloc stack logging because it was not enabled`) written directly to fd 2 by libmalloc painting into the TUI viewport. While the TUI owns the terminal, stderr is now redirected to the omp log file and restored at every ownership handoff (external editor, Ctrl+Z suspend, shutdown, crash restore); fatal crash reports still reach the real terminal. Mirrors [openai/codex#24459](https://github.com/openai/codex/pull/24459). + ## [16.4.2] - 2026-07-10 ### Fixed diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 5d1a9980b..d2e74fc9c 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed unmanaged macOS stderr writes (libmalloc/framework diagnostics) corrupting the viewport: `ProcessTerminal` now suppresses fd 2 via the pi-utils stderr guard while it owns the terminal and restores it in `stop()` and the emergency-restore path. + ## [16.4.1] - 2026-07-10 ### Added diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index b35dfa1f5..c9125f9e9 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -1,6 +1,14 @@ import { dlopen, FFIType, ptr } from "bun:ffi"; import * as fs from "node:fs"; -import { $env, isBunTestRuntime, isTerminalHeadless, logger, postmortem } from "@oh-my-pi/pi-utils"; +import { + $env, + isBunTestRuntime, + isTerminalHeadless, + logger, + postmortem, + restoreTerminalStderr, + suppressTerminalStderr, +} from "@oh-my-pi/pi-utils"; import { setKittyProtocolActive } from "./keys"; import { StdinBuffer } from "./stdin-buffer"; import { @@ -275,6 +283,9 @@ function createConsoleCodepageGuard(): (() => void) | null { */ export function emergencyTerminalRestore(): void { try { + // Crash paths must surface subsequent stderr (fatal reports) on the + // real terminal; no-op when the stderr guard is inactive. + restoreTerminalStderr(); const terminal = activeTerminal; if (terminal) { terminal.stop(); @@ -551,6 +562,11 @@ export class ProcessTerminal implements Terminal { activeTerminal = this; terminalEverStarted = true; + // Keep unmanaged fd-2 writes (macOS libmalloc/framework diagnostics) off + // the viewport while we own the terminal; released in stop(). See + // stderr-guard in pi-utils (mirrors openai/codex#24459). + suppressTerminalStderr(); + // Save previous state and enable raw mode this.#wasRaw = process.stdin.isRaw || false; if (process.stdin.setRawMode) { @@ -1269,6 +1285,11 @@ export class ProcessTerminal implements Terminal { activeTerminal = null; } + // Release terminal ownership of fd 2 first so external programs, + // suspend, and shutdown see the real stderr even if a later teardown + // step throws. + restoreTerminalStderr(); + if (this.#clearProgressTimer()) { this.#safeWrite(TERMINAL_PROGRESS_CLEAR_SEQUENCE); } diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 5e14d664b..50b1835cd 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added a terminal stderr guard (`suppressTerminalStderr`/`restoreTerminalStderr`): dup2-redirects fd 2 to the omp log file while a TUI owns the terminal so macOS runtime diagnostics cannot paint into the viewport. The postmortem fatal handlers restore fd 2 before printing so crash reports stay visible. + ## [16.4.2] - 2026-07-10 ### Added diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 2816db33f..ebb86e412 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -26,6 +26,7 @@ export { AbortError, ChildProcess, Exception, NonZeroExitError } from "./ptree"; export * from "./runtime-install"; export * from "./sanitize-text"; export * from "./snowflake"; +export * from "./stderr-guard"; export * from "./stream"; export * from "./tab-spacing"; export * from "./temp"; diff --git a/packages/utils/src/postmortem.ts b/packages/utils/src/postmortem.ts index 42c37b42e..15bbff1b4 100644 --- a/packages/utils/src/postmortem.ts +++ b/packages/utils/src/postmortem.ts @@ -8,6 +8,7 @@ import inspector from "node:inspector"; import { isMainThread } from "node:worker_threads"; import { logger } from "."; +import { restoreTerminalStderr } from "./stderr-guard"; // Cleanup reasons, in order of priority/meaning. export enum Reason { @@ -166,6 +167,11 @@ if (isMainThread) { logger.warn("Ignoring expected cleanup exception", { err }); return; } + // fd 2 may be redirected to the log while a TUI owns the terminal + // (stderr-guard); re-point it at the real terminal so the fatal + // report is visible. Terminal modes are restored moments later by + // the terminal-restore cleanup callback inside runCleanup(). + restoreTerminalStderr(); process.stderr.write(formatFatalError("Uncaught Exception", err)); logger.error("Uncaught exception", { err }); await runCleanup(Reason.UNCAUGHT_EXCEPTION); @@ -199,6 +205,8 @@ if (isMainThread) { }); } } + // See uncaughtException above: surface the report on the real stderr. + restoreTerminalStderr(); process.stderr.write(formatFatalError("Unhandled Rejection", err)); logger.error("Unhandled rejection", { err }); await runCleanup(Reason.UNHANDLED_REJECTION); diff --git a/packages/utils/src/stderr-guard.ts b/packages/utils/src/stderr-guard.ts new file mode 100644 index 000000000..693552c27 --- /dev/null +++ b/packages/utils/src/stderr-guard.ts @@ -0,0 +1,149 @@ +/** + * Terminal stderr guard: keeps unmanaged fd-2 writes off the terminal while a + * TUI owns the viewport. + * + * On macOS, runtime diagnostics are written by the platform directly to file + * descriptor 2 at arbitrary times — e.g. libmalloc's "MallocStackLogging: + * can't turn off malloc stack logging because it was not enabled" when the OS + * broadcasts a memory-diagnostic event to long-lived processes. Those bytes + * bypass the renderer and paint straight into the viewport. Stripping the + * MallocStackLogging* env vars (cli.ts) only protects child processes; it + * cannot stop libmalloc inside THIS process from logging. + * + * Fix (mirrors openai/codex#24459): while the TUI owns the terminal, dup fd 2 + * aside and dup2 a redirect target over it; restore the saved fd whenever + * terminal ownership is released (external editor, Ctrl+Z suspend, shutdown, + * crash restore). Unlike codex we redirect to the omp log file — not + * /dev/null — so the diagnostics stay greppable and Bun native-crash reports + * (which abort before any JS cleanup can restore fd 2) are preserved. + * + * Only dup/dup2 go through bun:ffi. fcntl is deliberately avoided: it is + * variadic, and the arm64-darwin ABI passes variadic arguments on the stack, + * so a fixed-arity FFI signature would read garbage for the third argument. + */ +import { dlopen, FFIType } from "bun:ffi"; +import * as fs from "node:fs"; +import { getLogPath } from "./dirs"; + +const STDOUT_FILENO = 1; +const STDERR_FILENO = 2; + +interface LibcFdOps { + dup(fd: number): number; + dup2(oldFd: number, newFd: number): number; +} + +let libcFdOpsCache: LibcFdOps | null | undefined; + +function libcFdOps(): LibcFdOps | null { + if (libcFdOpsCache !== undefined) return libcFdOpsCache; + libcFdOpsCache = null; + if (process.platform === "win32") return null; + // Darwin: dyld resolves libSystem from the shared cache. Linux: glibc + // first, then the generic soname for musl-style layouts. + const candidates = + process.platform === "darwin" ? ["libSystem.B.dylib", "/usr/lib/libSystem.B.dylib"] : ["libc.so.6", "libc.so"]; + for (const candidate of candidates) { + try { + const libc = dlopen(candidate, { + dup: { args: [FFIType.i32], returns: FFIType.i32 }, + dup2: { args: [FFIType.i32, FFIType.i32], returns: FFIType.i32 }, + }); + libcFdOpsCache = libc.symbols; + return libcFdOpsCache; + } catch { + // Try the next candidate; the guard stays inert if none load. + } + } + return libcFdOpsCache; +} + +/** + * True when fd 2 writes would land on the same terminal the TUI paints to: + * both stdout and stderr are ttys backed by the same device file. A stderr + * the user already redirected (`2>file`, `2>/dev/null`, a different tty) must + * keep flowing untouched. + */ +function stderrSharesStdoutTerminal(): boolean { + if (!process.stdout.isTTY || !process.stderr.isTTY) return false; + try { + const stdoutStat = fs.fstatSync(STDOUT_FILENO); + const stderrStat = fs.fstatSync(STDERR_FILENO); + return stdoutStat.dev === stderrStat.dev && stdoutStat.ino === stderrStat.ino; + } catch { + return false; + } +} + +/** Saved dup of the real stderr while suppression is active, else null. */ +let savedStderrFd: number | null = null; + +export interface SuppressTerminalStderrOptions { + /** Redirect target path; defaults to today's omp log file, then /dev/null. */ + redirectPath?: string; + /** Bypass the macOS + same-terminal gate. Tests only. */ + force?: boolean; +} + +/** + * Redirect fd 2 away from the terminal while the TUI owns the viewport. + * Returns true when suppression is (already) active. No-op — returning + * false — off macOS, when stderr does not target the stdout terminal, or + * when the libc fd ops are unavailable. + */ +export function suppressTerminalStderr(options?: SuppressTerminalStderrOptions): boolean { + if (savedStderrFd !== null) return true; + if (!options?.force && (process.platform !== "darwin" || !stderrSharesStdoutTerminal())) { + return false; + } + const libc = libcFdOps(); + if (!libc) return false; + + let redirectFd: number; + try { + redirectFd = fs.openSync(options?.redirectPath ?? getLogPath(), "a"); + } catch { + try { + redirectFd = fs.openSync("/dev/null", "w"); + } catch { + return false; + } + } + + const saved = libc.dup(STDERR_FILENO); + if (saved === -1) { + fs.closeSync(redirectFd); + return false; + } + if (libc.dup2(redirectFd, STDERR_FILENO) === -1) { + fs.closeSync(redirectFd); + fs.closeSync(saved); + return false; + } + fs.closeSync(redirectFd); + savedStderrFd = saved; + return true; +} + +/** + * Re-point fd 2 at the saved terminal stderr. Safe to call unconditionally: + * no-op when suppression is not active. Called at every terminal-ownership + * release and by the postmortem fatal handlers before they print, so crash + * reports reach the real terminal. + */ +export function restoreTerminalStderr(): void { + if (savedStderrFd === null) return; + const saved = savedStderrFd; + savedStderrFd = null; + libcFdOps()?.dup2(saved, STDERR_FILENO); + try { + fs.closeSync(saved); + } catch { + // The dup'ed fd is process-owned; a close failure leaves nothing to recover. + } +} + +/** Whether fd 2 is currently redirected away from the terminal. */ +export function isTerminalStderrSuppressed(): boolean { + return savedStderrFd !== null; +} diff --git a/packages/utils/test/stderr-guard.test.ts b/packages/utils/test/stderr-guard.test.ts new file mode 100644 index 000000000..fa663d73f --- /dev/null +++ b/packages/utils/test/stderr-guard.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +/** + * Contract (issue: macOS libmalloc diagnostics painting into the TUI + * viewport; mirrors openai/codex#24459): while suppression is active, fd-2 + * writes land in the redirect target instead of the previous stderr; restore + * rejoins the saved stderr; without `force`, a stderr that is not the stdout + * terminal (here: a pipe) is left untouched. + * + * Runs in a subprocess so the test suite's own fd 2 is never mutated. + */ + +const GUARD_MODULE = path.resolve(import.meta.dir, "../src/stderr-guard.ts"); + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { force: true, recursive: true }); + } +}); + +interface ProbeReport { + gateResult: boolean; + forced: boolean; + secondSuppress: boolean; + suppressedWhileActive: boolean; + suppressedAfterRestore: boolean; +} + +describe("stderr guard", () => { + it("suppresses fd-2 writes only while active and refuses non-terminal stderr without force", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "stderr-guard-")); + tempDirs.push(dir); + const redirectPath = path.join(dir, "redirect.log"); + const probePath = path.join(dir, "probe.ts"); + fs.writeFileSync( + probePath, + [ + `import { isTerminalStderrSuppressed, restoreTerminalStderr, suppressTerminalStderr } from ${JSON.stringify(GUARD_MODULE)};`, + `import * as fs from "node:fs";`, + `const redirectPath = process.argv[2];`, + `fs.writeSync(2, "before\\n");`, + `// stderr is a pipe here, so the same-terminal gate must refuse.`, + `const gateResult = suppressTerminalStderr();`, + `const forced = suppressTerminalStderr({ force: true, redirectPath });`, + `const suppressedWhileActive = isTerminalStderrSuppressed();`, + `if (forced) fs.writeSync(2, "hidden\\n");`, + `// Idempotent while active: must not stack a second saved fd.`, + `const secondSuppress = suppressTerminalStderr({ force: true, redirectPath });`, + `restoreTerminalStderr();`, + `fs.writeSync(2, "after\\n");`, + `// Restore without active suppression is a no-op.`, + `restoreTerminalStderr();`, + `fs.writeSync(2, "still-visible\\n");`, + `process.stdout.write(JSON.stringify({`, + ` gateResult,`, + ` forced,`, + ` secondSuppress,`, + ` suppressedWhileActive,`, + ` suppressedAfterRestore: isTerminalStderrSuppressed(),`, + `}));`, + ].join("\n"), + ); + + const proc = Bun.spawn([process.execPath, probePath, redirectPath], { + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout as ReadableStream).text(), + new Response(proc.stderr as ReadableStream).text(), + proc.exited, + ]); + + expect(exitCode).toBe(0); + const report = JSON.parse(stdout) as ProbeReport; + // Piped stderr is not the stdout terminal → the non-forced gate refuses. + expect(report.gateResult).toBe(false); + expect(report.suppressedAfterRestore).toBe(false); + + if (report.forced) { + expect(report.suppressedWhileActive).toBe(true); + expect(report.secondSuppress).toBe(true); + expect(stderr).toBe("before\nafter\nstill-visible\n"); + expect(fs.readFileSync(redirectPath, "utf8")).toBe("hidden\n"); + } else { + // libc fd ops unavailable on this platform: the guard must stay + // inert and every write must reach the original stderr. + expect(stderr).toBe("before\nafter\nstill-visible\n"); + expect(fs.existsSync(redirectPath)).toBe(false); + } + }); +});