Skip to content

fix: don't abort when stderr or stdout is closed early (#40) - #44

Merged
igorls merged 13 commits into
masterfrom
fix/40-closed-stderr
Oct 11, 2026
Merged

igorls merged 13 commits into
masterfrom
fix/40-closed-stderr

Conversation

@igorls

@igorls igorls commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Fixes #40.

  • New console module with errln!/err!/outln! that drop write errors; all non-test eprintln!/println! calls use them, so 2>&1 | head -3 no longer panics (exit 101).
  • A BrokenPipe while writing the document to -o - is reported as BrokenPipe (not a pile of per-file errors) and main exits 0, like other pipeline tools.
  • E2E tests spawn the built binary with a pipe whose read end is already closed (stderr, then stdout) and check exit status, output file and no panic. They fail on master (verified) and pass here.

Follow-up: the demo script (#43) pipes through sed -n because of this bug; it can go back to head after this lands.
Note: CHANGELOG gets an ## Unreleased entry; later PRs in this series add to the same section, so merge in order or ask me to rebase.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 01:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The token-budget truncation notice can still convert a closed stdout pipe into a nonzero exit.

1 open finding
What changed in this PR

Prevents closed stdout/stderr pipes from panicking or prematurely aborting document generation.

Changes:

  • Adds best-effort console output macros.
  • Treats stdout BrokenPipe as successful pipeline termination.
  • Adds end-to-end closed-pipe regression tests.
File Description
src/​console.rs Adds non-panicking console macros.
src/​main.rs Handles BrokenPipe as success.
src/​lib.rs Migrates console output to safe macros.
src/​markdown.rs Propagates parallel output pipe closure.
src/​tree.rs Uses safe stdout output.
src/​file_utils.rs Uses safe stderr prompting.
src/​content_filter.rs Uses safe skip reporting.
src/​config.rs Uses safe configuration warnings.
src/​cache.rs Uses safe migration notices.
tests/​test_closed_pipes.rs Tests closed stdout/stderr behavior.
CHANGELOG.md Documents the fix.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/markdown.rs Outdated
@igorls

igorls commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Addressed review: the truncation-notice write now also records BrokenPipe. Note: with a reader that is already gone, the header write fails first, so the added token-budget test is a sanity check; the mid-stream case is covered by the shared pipe_closed flag on all three per-file write sites. Also fixed a Windows-only test assertion (path separator).

@igorls
igorls merged commit e72a3cb into master Oct 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Process exits 101 (panic) when stderr is closed early, e.g. context-builder … 2>&1 | head -3

2 participants