diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 84c0d602b..543a737a6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -219,6 +219,9 @@ ### Removed - Removed the bundled plan subagent from available task agents. +### 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. ## [16.4.2] - 2026-07-10 diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 2a6e16a07..e0a0443c3 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -34,6 +34,9 @@ - Fixed an issue where the mid-prompt `/` autocomplete popup lingered indefinitely on non-path and non-skill tokens. Autocomplete matching is now properly gated to explicit skill namespaces, queries, and prefixes, preventing stale popups from incorrectly rewriting input on Tab or Enter. - Fixed idle Loader animation driving the full TUI render pipeline on every spinner tick by directly rewriting the Loader's visible rows when geometry is unchanged, reducing idle render work while preserving fallback repaint paths ([#5192](https://github.com/can1357/oh-my-pi/issues/5192)). +### 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 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 992cfb967..3e0546297 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -7,6 +7,9 @@ ### Added - Added `AsyncDrain`, the deferred write-batching helper previously private to the coding-agent's prompt-history storage; now shared with model-perf recording. +### 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 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..fc1643b9c --- /dev/null +++ b/packages/utils/src/stderr-guard.ts @@ -0,0 +1,155 @@ +/** + * 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 * as path from "node:path"; +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 { + const redirectPath = options?.redirectPath ?? getLogPath(); + // getLogsDir() only computes the path; the logger creates it lazily, so + // on a fresh profile ~/.omp/logs may not exist yet. Create it here so + // diagnostics land in the log instead of falling through to /dev/null. + fs.mkdirSync(path.dirname(redirectPath), { recursive: true }); + redirectFd = fs.openSync(redirectPath, "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); + } + }); +});