Repository navigation
fix(output): don't panic when a consumer stops reading stdout - #3766
Conversation
`wt list | head -3` exited 101 with `failed printing to stdout: Broken pipe`, and so did `wt list statusline | head -1` — the surface a shell prompt runs on every redraw. std's `print!`/`println!` panic on a `BrokenPipe`; anstream's drop it, which is why the `--format=json` surfaces #3746 converted already exit cleanly. Seven stdout surfaces were still on std's macros. Five of them — the `wt list` table, `--version`, `--help-md`, `--help-description`, and `wt config update --print` — are read by a person, so they go through anstream's printer, the canonical color-aware one. For `wt list` that also settles a second question. `wt --help` documents `NO_COLOR` ("Disable colored output") and `CLICOLOR_FORCE` ("Force colored output even when not a TTY"), a contract that only makes sense if color is off when stdout isn't a terminal — and the test suite sets `CLICOLOR_FORCE=1` precisely so ANSI still shows up in snapshots. But `wt list` wrote escapes through std's macros, so it colored a pipe unconditionally and neither variable reached it. It now behaves as documented: color on a terminal (both the progressive and buffered paths), plain on a pipe, `CLICOLOR_FORCE` to override, `NO_COLOR` to suppress. The five `BareRepoTest` snapshots that change are the ones whose helper already strips `CLICOLOR_FORCE` to capture "plain output" and, until now, didn't get it; the diffs remove ANSI and nothing else. Two surfaces do need the escapes on a pipe, because there the pipe is a courier rather than the destination: the statusline, which a shell prompt or Claude Code captures and renders, and the `--help-page` document, whose escapes the docs pipeline converts into HTML spans. Neither consumer is ever a tty, so anstream would strip them every time — and no test would catch that, since the suite forces color. They get `crate::output::println_verbatim!`, a new sibling of `print_json` that writes the bytes through unchanged. It drops a `BrokenPipe` and still panics on any other write error, matching anstream, so the two printers fail the same way on a full disk. Both are byte-identical to before: the help snapshots and `test_docs_are_in_sync` pass untouched. `test_stdout_surfaces_survive_a_closed_consumer` drives nine invocations across both printers with their stdout pipe closed before the child is waited on, so the first write has no reader — deterministic rather than racing a consumer's exit, and `--version`'s few dozen bytes trip it just as readily as the help page's 23KB. Each case was checked to fail against the unfixed code. `statusline.rs` leaves `STDOUT_ALLOWED_PATHS`, since it no longer writes stdout directly; a stray `println!` there fails the guard again. `print_first_buffered_line` is gone too — its one caller is the `WORKTRUNK_FIRST_OUTPUT` benchmark hook, which wrote the same header line through a third path, and a LineWriter flushes on the newline the measurement is timing.
worktrunk-bot
left a comment
There was a problem hiding this comment.
The printer split is right and println_verbatim! matches anstream's policy exactly (I diffed it against anstream-1.0.0/src/_macros.rs — same Err(e) if e.kind() != BrokenPipe => panic! arm), so the two printers really do fail identically on a full disk. The --help-page rewrite is byte-identical too: print!(x); println!() collapsing into println_verbatim!("{}", x) emits the same bytes, which is why the help snapshots and test_docs_are_in_sync are untouched.
One claim the code doesn't back up, though. The added src/commands/CLAUDE.md paragraph (and the PR description) says wt list now honors NO_COLOR — but NO_COLOR only matters when stdout is a terminal, and that's exactly the case this change doesn't route through anstream. RenderTarget::detect picks Table { progressive: true } purely on stdout().is_terminal(), and show_progress gates on that, so on a tty the rows never reach print_buffered_table; they go through ProgressiveTable, whose print_all / redraw_line do writeln!(stdout, "{}", line) against std::io::stdout() imported from std::io — no anstream in the path. The skeleton strings already carry the escapes (Style::new().dimmed() is baked in at render_skeleton_row time), so NO_COLOR=1 wt list in a terminal still emits ANSI. The CLICOLOR_FORCE-on-a-pipe half is genuinely fixed; it's the NO_COLOR half that isn't. Inline suggestion narrows the sentence — happy to instead push a commit teaching ProgressiveTable to strip when anstream's choice says so, if you'd rather close the gap than document it. (Verified by source reading, not a PTY run.)
Smaller, and outside the diff: check_no_stdout_in_commands roots its scan at src/commands/, so src/help.rs — the file this PR just converted — has no static guard. It now imports print from worktrunk::styling but not println, so a bare println! added there later silently picks up std's macro and reintroduces the exact panic. test_stdout_surfaces_survive_a_closed_consumer covers today's four help surfaces but not a new one. Widening the scan root would need STDOUT_ALLOWED_PATHS extended for the #[cfg(test)] println!s in src/display.rs and the printer modules themselves — say the word and I'll push it.
`RenderTarget::detect` sets `progressive = is_tty`, so `ProgressiveTable` renders every terminal case and writes to a raw `std::io::stdout()` with the escapes already baked into each row. `print_buffered_table` — the one this branch routed through anstream — runs only when stdout isn't a terminal, where color is off regardless. So the change turns off color on a pipe and makes `CLICOLOR_FORCE` turn it back on; `NO_COLOR` still doesn't reach the table. A 4-row PTY doesn't exercise the buffered path either: a short terminal only shrinks `visible_row_count`. Measured both paths again with a PTY — `NO_COLOR=1` changes nothing on a terminal, byte-identical at 1835 escapes. Thanks to worktrunk-bot for catching the claim.
|
Confirmed and fixed in 2ec645a — you were right, and the verification behind the claim was weaker than the claim. I'd checked
Not taking the offer to close the gap in this PR. Stripping SGR from the progressive rows has to leave two things alone: the cursor control the redraw depends on, and the URL column's OSC 8, since Same for the guard scope, which is a real gap and one I'd already noted: widening
|
worktrunk-bot
left a comment
There was a problem hiding this comment.
The correction is right about the progressive path, but it overshoots on one case, and in the direction of underselling the change: --no-progressive on a tty does now honor NO_COLOR, so "for every tty" / "only ever sees the piped case" isn't quite the rule.
RenderTarget::detect resolves the flag as Some(p) => p && is_tty, so --no-progressive gives Table { progressive: false } on a terminal too. show_progress is matches!(render_target, Table { progressive: true }), so that case takes print_buffered_table — the function this PR just moved onto anstream by adding use worktrunk::styling::println to collect/mod.rs. And anstream strips at the stream level, not at format time: anstream::stdout() resolves through auto.rs's choice(), whose first arm is if anstyle_query::no_color() { ColorChoice::Never } — checked before the raw.is_terminal() arm — and Never builds a StripStream, which strips the escapes baked into each row string regardless of tty. So NO_COLOR=1 wt list --no-progressive in a terminal is plain after this PR and wasn't before. The flag is hidden but reachable, and --progressive's own help points at it ("Use --no-progressive to force buffered rendering").
Suggestions narrow both claims to that shape. (Verified by source reading — anstream's choice() from a clone of rust-cli/anstyle, the rest from this tree; I didn't run a PTY.)
The previous correction overshot. `RenderTarget::detect` resolves the
flag as `Some(p) => p && is_tty`, so `--no-progressive` gives
`Table { progressive: false }` on a terminal, which takes
`print_buffered_table` — the function this branch moved onto anstream.
anstream strips at the stream level, and its `choice()` checks
`no_color()` before the `is_terminal()` arm, so the escapes baked into
each row come off.
PTY-measured on a terminal: `wt list --no-progressive` emits 322
escapes, `NO_COLOR=1` the same command emits 0, and `NO_COLOR=1
CLICOLOR_FORCE=1` also emits 0 — `no_color()` is the first arm.
Thanks to worktrunk-bot for catching it.
|
Right on both counts, and I ran the PTY you didn't — it confirms your source reading exactly. Narrowed in 3303c5e. On a terminal with
That last row is the ordering you pointed at:
The docstring,
|
Both rework features this release already documents, so they fold into the existing bullets rather than adding new ones: #3766 extends #3746's broken-pipe fix to every remaining stdout surface, and #3767 narrows the plugin removal hook's guard from #3754. #3766 also stops `wt list` coloring a pipe, which is user-visible on its own, so that gets its own entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…#3771) ## Problem `worktrunk::styling`'s `eprint!` is anstream's and strips ANSI when stderr isn't a terminal; std's prelude macro of the same name keeps it. A file that imports one but not the other — or neither — gets a mix, and adjacent lines of the same message block disagree about whether a redirected stderr carries escapes. `wt list 2>&1 >/dev/null | cat -v` in a repo with a deprecated `[ci]` block, on `main`: ``` ^[[33mM-bM-^VM-2^[[39m ^[[33mProject config: ^[[1m[ci]^[[22m is deprecated in favor of ^[[1m[forge]^[[22m^[[39m M-bM-^FM-3 To see details, run wt config show; to apply updates, run wt config update ``` The warning is `eprint!("{warnings}")` at `src/config/deprecation.rs`, which resolved to std's macro; the hint directly beneath it is the `eprintln!` imported from `styling` four lines later. Anyone redirecting `wt` narration to a file gets escapes on one line and not the next. This is the stderr counterpart of what #3746 and #3766 fixed on stdout, and it went unnoticed for the reason named in `verbatim.rs`'s own docstring: the suite sets `CLICOLOR_FORCE=1`, which forces color on *both* printers, so no snapshot could disagree no matter which macro was in scope. `output_system_guard` doesn't cover it either — it scans for `print!`/`println!` tokens under `src/commands/`, not for which `eprint!` a file imported. ## Solution The rule is now structural rather than per-site. `check_stderr_macros_come_from_styling` in `output_system_guard.rs` walks every `.rs` file under `src/` and flags a bare `eprint!`/`eprintln!` whose file lacks the matching `worktrunk::styling` import. A call satisfies it either way — importing the macro, or qualifying the call as `styling::eprintln!(…)`, which several files (`git/repository/mod.rs`, `config/user/mod.rs`, `commands/config/alias.rs`) already do. Two files are allowlisted with a reason: `testing/mock_stub.rs` relays a stub's captured stderr verbatim, so its bytes are fixture data; `remove_dir.rs`'s one call is a `#[cfg(test)]` skip diagnostic, not narration a user redirects. Reverting the source fixes below makes it name exactly those five lines and nothing else. The sites it fixes: - `src/config/deprecation.rs` — the deprecation warning block above. - `src/commands/config/update.rs` — the `format_update_preview` block shown before `wt config update`'s prompt, reachable with a tty stdin and a redirected stderr. - `src/output/prompt.rs` — the `[y/N/?]` prompt; its blank-line `eprintln!` was already explicitly qualified as `worktrunk::styling::eprintln!`, so the two disagreed within four lines. Both now come from one import. - `src/output/global.rs` — the file the first scan couldn't see, because that scan looked for "imported `eprintln` but not `eprint`" and this file imports neither. Its four styled `eprintln!` calls (`print_outdated_shell_wrapper_hint_once`, `warn_retired_exec_once`, `warn_exec_scrubbed_once`) all resolve to std's, so a user mid-upgrade running `wt … 2>log` gets `ESC[…m` around the shell-wrapper repair hint. The module's own docstring already claimed the contract the code didn't have — *"Regular output still uses `eprintln!`/`println!` directly (from `worktrunk::styling` for color support)"*. One added import makes it true; under the suite's `CLICOLOR_FORCE=1` no snapshot moves. - `src/commands/for_each.rs` — the pre-spawn ANSI reset. `output/handlers.rs` runs the identical three lines (`stderr().flush()?`, `eprint!("{}", anstyle::Reset)`, `stderr().flush().ok()`) immediately before building its `Cmd`, but through anstream's `eprint` *and* anstream's `stderr`; `for_each` used std's for both, so the same operation wrote a literal `ESC[0m` into a redirected stderr where `handlers` dropped it. Both halves move together — the flushes have to name the stream the reset was written to, so switching `eprint!` alone would flush std's handle while anstream's buffer held the write. The `std::io::stderr()` handed to `Stdio::from` four lines down is a different thing and stays. ## Tests `test_stderr_narration_strips_ansi_when_piped` in `output_system_guard.rs`, alongside the closed-consumer test #3766 added. It clears `CLICOLOR_FORCE` and sets `NO_COLOR` (which only anstream honors), triggers the `[ci]` deprecation, and asserts stderr carries the warning and no `\x1b`. Confirmed to fail on the pre-fix source with exactly the escapes quoted above, and to pass with it. That test proves what the property buys at one site; `check_stderr_macros_come_from_styling` is what holds it at all of them. No runtime test can: the suite's `CLICOLOR_FORCE=1` forces color on both printers, so a snapshot agrees whichever macro is in scope, and the property is about every stderr write in the binary rather than any one path. The module docstring is updated for both — the `Allowed:` list no longer reads flatly as "`eprintln!` / `eprint!` (stderr is safe)", which was the sentence someone skims before making this exact mistake. ## Verification `cargo clippy --all-targets`, `cargo fmt --check`, `cargo test --lib --bins` (2,454 passed), and the integration suite (1,961 passed) all run locally. One integration test fails in this sandbox and is unrelated: `test_copy_ignored_preserves_file_executable_permissions` expects `0644` and sees `0664`, because the sandbox's umask is `002` rather than the runner's `022` (confirmed by `umask` → `0002`). It touches none of these files; CI will confirm. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
worktrunk 0.72.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## Release Notes ### Improved - **A host carrying a forge's name anywhere resolves to that forge again**: 0.71.0 required `github`, `gitlab`, or `gitea` as a whole dot-separated label, which read as an ownership check but wasn't one — an attacker controls their own DNS — while shutting out self-hosters with hyphenated names. `github-enterprise.acme.com`, `mygithub.com`, and the `github-personal` SSH alias classify again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ([#3673](max-sixty/worktrunk#3673)) - **One `[projects."…"]` entry can cover every repository on a host, and can set the forge**: A key containing `*` matches any run of characters, `/` included, so `[projects."git.company.example/*"]` covers a whole host; every matching entry applies, least- to most-specific. The table also gained `forge.platform` and `forge.hostname`, so a self-hosted host needs one entry here rather than a block in every repo. [Docs](https://worktrunk.dev/config/#user-project-specific-settings) ([#3701](max-sixty/worktrunk#3701), thanks @chrishas35 for the request and @witt-bit for the workspace-scoped case it partly serves) - **`wt merge` and `wt step push` leave the target worktree's uncommitted changes in place**: The autostash that held a dirty target's changes restored them as unstaged; both now advance the target with a compare-and-swap `update-ref` and `read-tree -m -u`, which leaves uncommitted work untouched. (Breaking: the fast-forward path no longer runs `git push`, so `pre-push` and the receive-side hooks no longer fire, and a failed sync errors with the ref rolled back.) ([#3703](max-sixty/worktrunk#3703), [#3684](max-sixty/worktrunk#3684), [#3693](max-sixty/worktrunk#3693), thanks @gubasso for reporting) - **Approval state and branch-removal outcomes are machine-readable**: `wt config approvals list --format=json` reports whether a non-interactive run would stop for approval. `wt remove` and `wt step prune` replace `branch_deleted` with `branch_outcome`: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. (Breaking.) ([#3710](max-sixty/worktrunk#3710), thanks @NathanaelRea for the requests) - **Every commit hash worktrunk prints follows `core.abbrev`**: The `wt list` table, `wt switch --prs`'s `log` tab, and `wt config state`'s CI cache table sliced to 8 characters while `--format=json` carried git's `%h`. All now ask git how wide it abbreviates in this repo. ([#3676](max-sixty/worktrunk#3676), [#3677](max-sixty/worktrunk#3677)) - **`wt list --format=json` schema 2 has a published JSON Schema**: [worktrunk.dev/schema/list-v2.json](https://worktrunk.dev/schema/list-v2.json) holds the contract, and `wt list --print-schema` prints the same document. Four fields that were bare strings are now enumerated vocabularies; the emitted JSON is unchanged. ([#3747](max-sixty/worktrunk#3747)) - **A detached worktree is named by its commit, not `-`**: The Branch cell hardcoded `-`, which reads as missing data rather than a state; it now carries the row's abbreviated HEAD, and the picker and statusline name the worktree the same way. ([#3675](max-sixty/worktrunk#3675)) ### Fixed - **Piped output is plain and no longer panics**: `wt list | head -3` exited 101 with `failed printing to stdout: Broken pipe`, and `wt list` wrote ANSI to a pipe unconditionally. Every stdout surface now exits cleanly, and the human-read ones are plain when piped unless `CLICOLOR_FORCE=1`. ([#3746](max-sixty/worktrunk#3746), [#3766](max-sixty/worktrunk#3766)) - **A CI check that hasn't finished no longer reads as passed**: Each forge's status parser missed documented values, so a GitHub PR parked on an approval gate showed green, and GitLab's `canceling` and Azure DevOps's `postponed` read as no CI at all. ([#3741](max-sixty/worktrunk#3741), [#3740](max-sixty/worktrunk#3740)) - **The shell wrappers survive an `rm` alias and a failing `--execute`**: Aliases bake into the wrapper at parse time, so `alias rm='rm -v'` reached its cleanup — noise on zsh and bash, and on nushell an abort that leaked three temp files, as a failing `--execute` body also did. ([#3714](max-sixty/worktrunk#3714), thanks @Ar4l), ([#3732](max-sixty/worktrunk#3732), [#3734](max-sixty/worktrunk#3734)) - **An alias or hook wrapping `wt switch` or `wt remove` keeps your subdirectory**: The user's position came from the `wt` process's cwd, which inside an alias body is the worktree root, so an aliased `wt remove` from `feature/apps/gateway` landed at the primary worktree's root. Fixes [#3723](max-sixty/worktrunk#3723). ([#3724](max-sixty/worktrunk#3724), thanks @vivienm for reporting) - **`wt merge` and `wt step push` refuse a target worktree parked mid-operation**: The two-tree sync refuses an unmerged index but not a stopped cherry-pick or rebase whose conflict was already staged, so the push range could land in a paused target and be committed by `--continue`. ([#3759](max-sixty/worktrunk#3759)) - **The Claude plugin's worktree-remove hook resolves against the worktree path**: The hook anchored at `CLAUDE_PROJECT_DIR`, which the `claude agents` view routinely leaves outside any repository, so `wt remove` died with `not a git repository` and the session became undeletable. Its guard now also requires a `.git` entry. ([#3754](max-sixty/worktrunk#3754), [#3767](max-sixty/worktrunk#3767), thanks @judewang for the fix and the report) - **A Gitea API error is reported as one, not as a parse failure**: `tea api` exits 0 whatever the HTTP status, so both call sites guessed from the body's shape and blamed an API change for an API error. `--include` surfaces the status instead. ([#3713](max-sixty/worktrunk#3713), [#3600](max-sixty/worktrunk#3600)) - **`--print-schema` and the doc-generation help flags name the right command**: All three found the subcommand by scanning `argv` for a `/wt` suffix, which never matches `wt.exe` under a backslash path, so on Windows they read the binary's own path as the command. ([#3762](max-sixty/worktrunk#3762)) - **A multibyte shell name no longer panics**: `extract_filename_from_path` sliced at `len() - 4` to test for `.exe` with no char-boundary check: `SHELL=/bin/日本語 wt config show` panicked, and on macOS every process name goes through it during shell detection. ([#3727](max-sixty/worktrunk#3727)) - **`wt` installed under a dotted name generates shell integration for that name**: `binary_name` used `file_stem`, which cuts at the last dot, so `wt config shell init bash` under `wt.old` emitted a wrapper for `wt`. It now strips only the executable suffix. ([#3719](max-sixty/worktrunk#3719)) - **Concurrent `wt step prune` removals no longer race the worktree registry**: `git worktree remove` reads every sibling under `.git/worktrees/`, so two overlapping removals could have one read a sibling mid-teardown. Registry-mutating removals now serialize behind a second lock. ([#3692](max-sixty/worktrunk#3692)) - **The `wt switch` first-run offer previews the legacy files it removes**: Accepting "Install shell integration?" could delete a deprecated worktrunk-managed wrapper the prompt never named. What gets removed is unchanged. ([#3656](max-sixty/worktrunk#3656)) - **`wt config create --project` writes a resolvable link**: The comment it writes into `.config/wt.toml` carried a raw Zola target, because the link-conversion regex stopped at the first `]` — here the one closing a nested code span. ([#3731](max-sixty/worktrunk#3731)) - **`wt list --branches` counts a local branch containing `/` as local**: The summary tally classified branch-only rows by `branch.contains('/')`, so a local `feature/login` counted under "N remote branches". ([#3687](max-sixty/worktrunk#3687)) - **`wt step relocate`'s human summary counts template-error branches as skipped**: `--format=json` already folded them into `skipped`; the human tally undercounted by the number of branches whose `worktree-path` template failed to expand. ([#3688](max-sixty/worktrunk#3688)) ### Documentation - **SignPath attribution appears with the artifacts it describes**: The code-signing notice and a route to the policy now sit in the install section's Windows block on the README and the docs landing page, as SignPath Foundation's OSS program requires. ([#3709](max-sixty/worktrunk#3709)) - **`wt step copy-ignored`'s `--require-include` example renders as a terminal block**: It was the only `console` block in the command's long help missing the `$ ` prefix. ([#3706](max-sixty/worktrunk#3706)) ### Internal - **Library API rework** (Breaking library API): `cargo-semver-checks` fails five lints — `LegacyForgeAlias` and `Repository::legacy_forge_alias` removed with the forge-classification revert, `Repository::forge_platform_override` removed for one shared resolver, `stage_worktree_removal` gained two parameters, `UserProjectOverrides` gained a `forge` field, and the `real-repo-benches` feature was removed. ([#3673](max-sixty/worktrunk#3673), [#3694](htt See merge request: Harmonybrew/homebrew-core!16275
wt list | head -3exited 101 withfailed printing to stdout: Broken pipe, and so didwt list statusline | head -1— the surface a shell prompt runs on every redraw. std'sprint!/println!panic on aBrokenPipe; anstream's drop it, which is why the--format=jsonsurfaces #3746 converted already exit cleanly.Seven stdout surfaces were still on std's macros. Five of them — the
wt listtable,--version,--help-md,--help-description, andwt config update --print— are read by a person, so they go through anstream's printer, the canonical color-aware one.wt listalso stops coloring a pipewt --helpdocumentsNO_COLOR("Disable colored output") andCLICOLOR_FORCE("Force colored output even when not a TTY"). That second row only means something if color is off when stdout isn't a terminal — andsrc/testing/mod.rssetsCLICOLOR_FORCE=1inSTATIC_TEST_ENV_VARSprecisely so ANSI still appears in snapshots. Butwt listwrote its escapes through std's macros, so it colored a pipe unconditionally and neither variable ever reached it.It now behaves as documented for the piped case: color on a terminal, plain on a pipe,
CLICOLOR_FORCE=1to keep it on a pipe.NO_COLORis fixed whereverprint_buffered_tableruns, which is the piped case plus--no-progressiveon a terminal. It does not reach the default terminal rendering:RenderTarget::detecthands every tty that didn't pass--no-progressivetoProgressiveTable, which writes to a rawstd::io::stdout()with each row's escapes already baked in.Closing that last gap means stripping SGR from the progressive rows while leaving the redraw's cursor control and the URL column's OSC 8 intact (
src/styling/hyperlink.rsstatesNO_COLORmust not affect hyperlink support), so it's a separate change on the most visible surface in the tool.PTY measurements
On a terminal, with
CLICOLOR_FORCEunset:wt list(progressive, default)NO_COLOR=1 wt listwt list --no-progressiveNO_COLOR=1 wt list --no-progressiveNO_COLOR=1 CLICOLOR_FORCE=1 wt list --no-progressiveno_color()firstPiped: 0 escapes by default, 17 under
CLICOLOR_FORCE=1.Two earlier drafts of this description got this wrong in both directions, and @worktrunk-bot caught each.
The five changed snapshots all belong to
BareRepoTest, whoseconfigure_wt_cmdstripsCLICOLOR_FORCEto capture "the terminal's plain output" and, until now, didn't get it. The diffs remove ANSI and change nothing else — same content, same column widths.Two surfaces keep the escapes
For the statusline and
--help-page, the pipe is a courier rather than the destination. A shell prompt or Claude Code captures the statusline and renders it; the docs pipeline converts the help page's escapes into HTML spans. Neither consumer is ever a tty, so anstream would strip them every time in production — and no test would catch it, because the suite forces color.They get
crate::output::println_verbatim!, a sibling ofprint_jsonthat writes the bytes through unchanged. It drops aBrokenPipeand still panics on any other write error, matching anstream, so both printers fail the same way on a full disk. Output is byte-identical: the help snapshots andtest_docs_are_in_syncpass untouched.Test, and two cleanups that fall out
test_stdout_surfaces_survive_a_closed_consumerdrives nine invocations across both printers with the child's stdout pipe closed before it is waited on, so the first write has no reader. That's deterministic rather than racing a consumer's exit, and--version's few dozen bytes trip it as readily as the help page's 23KB —EPIPEis about whether a reader is attached, not about filling the buffer. Every case was checked to fail against the unfixed code.statusline.rsleavesSTDOUT_ALLOWED_PATHS, since it no longer writes stdout directly; a strayprintln!there fails the guard again.print_first_buffered_lineis gone. Its one caller is theWORKTRUNK_FIRST_OUTPUTbenchmark hook, which wrote the same header line through a third path; a LineWriter flushes on the newline that measurement is timing, so the hook is unaffected.