Repository navigation
Conversation
While the full-screen renderer is up, process stderr is still attached to the terminal, so library or child output (libasound diagnostics when an attention-sound event probes a missing audio device, anomalyco#41763) paints over the rendered frame and leaves residue until a repaint. Bind libc dup/dup2 through bun:ffi during the renderer scope and point fd 2 at <data>/log/tui-stderr.log, restoring the terminal stderr when the scope ends so output after the run still reaches the terminal. The capture follows the existing runtime-branch pattern; under the node runtime it is a no-op. OPENCODE_TUI_STDERR=inherit opts out.
A failed engine start left the engine created, so every later attention-sound event re-created and re-started it, repeating the backend probe on device-less hosts each time (anomalyco#41763). Mark the backend unavailable after a failed start and short-circuit play and loadSoundFile until dispose() resets the state, keeping the restart behavior intact.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the ALSA output over the TUI. I ran a small Bun script that plays a sound three times on a Linux host with no sound device, under a terminal. On the base, 105 ALSA lib lines reached the terminal. With only the audio.ts change, 35 did (the audio system is tried once). With the stderr capture as well, none did: they went to tui-stderr.log, and stderr worked on the terminal again after restore. bun test for both new test files passes and tsc -b is clean. I couldn't start the full TUI in this sandbox (the standalone server exited before it was ready), so I tested the modules directly.
Public API: adds the OPENCODE_TUI_STDERR=inherit environment variable (opt-out) and a new log file, <data>/log/tui-stderr.log. Both are new, so nothing existing breaks. The variable isn't documented anywhere yet.
Concerns, in order of importance (details inline):
- Redirected stderr is taken over: the capture turns on even when the user already sent stderr to a file, so
opencode --print-logs 2>debug.logwrites nothing todebug.log. I reproduced this. - The log file is never trimmed:
tui-stderr.loggrows without limit, unlike the other logs in that directory. - Crash output is hidden: if the process dies while the TUI is running, the restore step never runs. Native crash output then lands only in the log file, and nothing tells the user where to look.
Fixes #41763
|
|
||
| export function captureStderr(dir: string): StderrCapture | undefined { | ||
| if (process.env.OPENCODE_TUI_STDERR === "inherit") return undefined | ||
| if (!process.stdout.isTTY && !process.stdin.isTTY) return undefined |
There was a problem hiding this comment.
This check looks at stdin and stdout, but not at stderr itself. If the user has already redirected stderr (opencode --print-logs 2>debug.log, or any 2> redirect), the capture still runs and moves their output into tui-stderr.log. I reproduced this under script: with stdout as a terminal and stderr sent to a file, captureStderr turned on, the user's file stayed empty and the line went to tui-stderr.log. The problem only exists when stderr is the terminal, so if (!process.stderr.isTTY) return undefined is the condition that matches it, and it also covers the non-TTY case.
This file is opened in append mode and never trimmed. The other logs in this directory are compacted by fileLogger's trim (packages/util/src/observability/logging.ts). A noisy native library or child process will make it grow without limit over many sessions. You could reuse that trim, or truncate the file when each TUI run starts.
| // the run's log and restore it when this scope ends, so messages the | ||
| // run prints afterwards still reach the terminal. | ||
| const captured = yield* Effect.acquireRelease( | ||
| Effect.sync(() => captureStderr(global.log)), |
There was a problem hiding this comment.
fd 2 now points at the log file for the whole TUI session, so any native crash output during that time ends up only in tui-stderr.log. That includes a Bun panic or a segfault, where the scope never finalizes and restore() never runs. External editors started from the TUI (editor.ts uses stdio: inherit) also lose their stderr. The user isn't told where any of this went. A short note at startup or exit that names the file (or at least a mention in the docs) would make these failures much easier to diagnose.
Capture only when stderr itself is attached to the terminal, so a user redirect like `opencode --print-logs 2>debug.log` keeps receiving the output instead of being moved into the capture file. Truncate tui-stderr.log at each TUI run so a noisy native library or child cannot grow it without bound. Name the capture file in a debug log line and, when the run actually produced captured output, print its path at exit so failures are discoverable. Document OPENCODE_TUI_STDERR in the CLI docs environment variable table.
|
Thanks for the review — all three findings are addressed in 6d9ce03:
Gates re-run: |
|
restore() in stderr-capture.bun.ts is not idempotent. second call dup2s from an already closed saved fd, then closes saved and fd again. that can close an unrelated fd reused in between. also the test file only covers the two skip cases, nothing asserts a write to fd 2 actually lands in tui-stderr.log or that restore() reattaches the terminal. the capture path itself has no coverage. |
There was a problem hiding this comment.
Both earlier points are addressed. Capture now depends on process.stderr.isTTY, so 2>file redirects are respected. I re-ran my repro and the line now reaches the user's file. The log is bounded, its path is printed at exit when something was captured, and the new variable is documented. ALSA output still stays off the terminal: 0 lines, one 35-line batch in the log. The tests (3 pass) and tsc -b are clean.
Public API: adds the OPENCODE_TUI_STDERR=inherit environment variable and the <data>/log/tui-stderr.log file. Both are new and backwards compatible.
New concerns:
- Concurrent TUIs: truncating per run makes two TUIs open at once overwrite each other's capture (see inline).
- Docs location: the docs row went into
packages/web/src/content/docs/cli.mdx, which looks like the v1 docs site. The V2 docs live underservices/www/src/docs/content. Please check where this variable belongs.
| // Truncate per run instead of appending across sessions: a noisy native | ||
| // library would otherwise grow the file without bound, and each run's | ||
| // captured output stays readable in one piece. | ||
| const fd = openSync(path, "w") |
There was a problem hiding this comment.
Every TUI process shares the same tui-stderr.log, and this file is opened with "w" (truncate, no O_APPEND). If a second opencode starts while another is running, a common case with several terminals, it truncates the first one's capture. After that, both processes write at their own offsets into the same file and overwrite each other. It also wipes the output of a run that crashed as soon as the user starts OpenCode again, which is when they'd want to read it. A per-process file name (for example including the PID, cleaned up like the other logs) or append mode with a size cap would avoid this.
restore() ran its dup2/close sequence on every call; the fd numbers it closed can be reused by other descriptors between calls, so a second call could corrupt unrelated fds. acquireRelease fires it once, and a guard keeps a repeated call a no-op. tui-stderr.log is shared by every TUI on the host. Truncating per run overwrote a live or crashed run's capture; switch to O_APPEND with a single rotation past 10 MiB (the size-cap option from review) and surface the capture's starting size so the exit note reports what the current run wrote rather than any pre-existing bytes. Cover the capture path in tests: while captured, fd 2 maps to the capture file, a write to fd 2 lands in the log, restore() stops the mapping, and a repeated restore is a no-op. Add the variable to the V2 docs (services/www/src/docs/content/cli/ config.mdx, Terminal section) alongside the v1 docs row.
|
Thanks both — all three points were real; fixed in aa145a3:
Gates re-run: |
There was a problem hiding this comment.
My concern about several TUIs sharing one log is addressed. The file is opened in append mode again, so concurrent TUIs add to it instead of overwriting each other, and a new run no longer wipes a crashed run's output. Growth is bounded by moving the file to .1 once it passes 10 MB. The variable is now documented in the V2 docs too. I re-ran my checks: the 4 tests in audio.test.ts and stderr-capture.test.ts pass and tsc -b is clean. 2>file redirects are still respected. Across two runs, no ALSA lines reached the terminal and both runs' output stayed in the log.
Public API: adds the OPENCODE_TUI_STDERR=inherit environment variable and the <data>/log/tui-stderr.log file (moved to .1 past 10 MB). Both are new and backwards compatible.
No new concerns.
|
checked aa145a3. the guard flag and the fd 2 inode plus writeSync coverage close both points from my side. thanks for the quick fix. |
There was a problem hiding this comment.
This push only merges the latest dev into the branch. The PR's own changes are the same as at the last review, apart from the merged packages/tui version bump. After reinstalling, the stderr-capture and audio tests pass (4 tests), tsc -b is clean, and on a host with no sound device the ALSA output still goes to tui-stderr.log instead of the terminal.
Public API: adds the OPENCODE_TUI_STDERR=inherit environment variable and the <data>/log/tui-stderr.log file (moved to .1 past 10 MB). Both are new and backwards compatible.
No new concerns.
|
I can confirm the repro from the other affected users, plus a few data points I verified today — this is the bug hitting me on the latest release. My environment
What I verified Code path in the 2.2.5 source: Live repro in an equivalent host (aarch64 linuxkit container, no
On the two parts of this PR
One gap worth flagging The capture is a no-op under the node runtime, so npm/node installs would still paint raw ALSA stderr onto the screen. The closed #51842/#51854 approach — probing for a playback device (e.g. |
|
Thanks — the 0.8.1 datapoint and the per-event measurements are exactly what this needed; nothing in the PR description claimed covers more than it does, so the node-runtime gap flag is fair. Two clarifications on coverage before the follow-up:
So the scope decision here: keep #59128 at the two changes it has. Cross-referenced #51842/#51854 from the description (both closed unmerged, still the right shape for node), and filed #61673 to track the device-probe/skip-init direction for the node runtime so it doesn't fall through the cracks. If you'd rather see the probe folded into this PR instead of a follow-up, say so and I'll take the node branch here. |
Issue for this PR
Addresses the display corruption described in #41763. The audio re-probe half overlaps with #41770 (closed by the cleanup bot without merge), so that direction is picked up here with a different approach.
Related, closed unmerged: #51842 and #51854 (skip audio init when no playback device). This PR fixes the display part for the bun runtime, which is the shipped binary (fd 2 is captured for the renderer scope); under the node runtime the no-re-probe change below still stops the per-event flooding, but the first ALSA batch of a session still paints — that residual is tracked in #61673.
Type of change
What does this PR do?
When the TUI renderer is up, the process stderr is still attached to the terminal, so anything native code or a child writes to stderr paints over the rendered surface. The visible case is libasound on hosts without an audio device: every attention-sound event (permission asks, errors, turn completion, interrupts) probes the backend, and the batch
lands in the middle of the screen. On 2.1.4 this is confirmed by
libasound.so.2mapping into the running TUI process (/proc/<tui-pid>/maps), while identical runs that never request audio never map it.Two changes in
packages/tui:Tui.runnow captures stderr for the lifetime of the renderer scope.src/stderr-capture.bun.tsbinds libcdup/dup2throughbun:ffi, points fd 2 at<data>/log/tui-stderr.log, and restores the terminal stderr when the scope ends, so post-TUI stderr (cliErrorMessageoutput printed at the end of the run) still reaches the terminal. The capture follows the existing runtime-branch pattern (#attention-sounds,#string-width); under the node runtime it is a no-op. It engages only when stderr itself is attached to the terminal — so a redirect like2>fb.logis respected — andOPENCODE_TUI_STDERR=inheritopts out. The capture file is truncated at each TUI run; when the run produced captured output the exit path prints the file's path.I took this approach rather than patching each noisy path because the failure is "uncontained process stderr reaches a full-screen surface", which stays true for any library or child that misbehaves. ALSA is the trigger, not the bug.
src/audio.tsstops probing a failed backend: a failedstart()marks the engine unavailable untildispose()resets it, andplay/loadSoundFileshort-circuit. Per-event re-probes were reported in tui: ALSA errors flood and corrupt the terminal during interaction #41763 and previously in fix(tui): stop retrying unavailable audio #41770.How did you verify your code works?
tsc -binpackages/tuiclean;oxlint packages/tuiclean.bun test packages/tui/test/audio.test.ts packages/tui/test/stderr-capture.test.ts: 2 pass, 0 fail. The audio test mocksAudio.createwith a failed-start engine and asserts two sequentialplaycalls callstart()exactly once (second is short-circuited), the engine is disposed once, and afterdispose()probing happens again on the next event./dev/snd): in a small bun script, capturing stderr then calling@opentui/core'sAudio.create({autoStart:false})+start()leaves the terminal clean and writes the full 27-line ALSA batch into the log file;start()still returns a normal JS error to the caller; after restore,process.stderr.writeis visible on the terminal again.Screenshots / recordings
No UI change; see the terminal session excerpt above.
Checklist
Added by the opencode bot (AI-generated).
Fixes #41763
Public API: adds the OPENCODE_TUI_STDERR=inherit environment variable and a /log/tui-stderr.log file (backwards compatible)