Skip to content

fix(core): forward stderr from stdout-only filters on every exit (adopt upstream #3772) - #159

Merged
kylehgc merged 5 commits into
developfrom
adopt/3772-stderr-passthrough
Sep 3, 2026
Merged

kylehgc merged 5 commits into
developfrom
adopt/3772-stderr-passthrough

Conversation

@kylehgc

@kylehgc kylehgc commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Closes #135. Adoption of upstream rtk-ai#3772 (Nicolas Le Cam / KuSh): every RunOptions::stdout_only() filter dropped the tool's stderr, and with stdout empty the never-worse guard collapsed the run to silence on both streams. Around 30 call sites (ruff, pytest, rspec, rubocop, gh, glab, go test, golangci-lint, tree, wc, psql, …).

Commits

Commit Author Kind
12db983 fix(core): forward stderr from stdout-only filters and count it Nicolas Le Cam Cherry-pick of 2eee910. One deviation, folded into the pick: the import hunk conflicted with the fork's use regex::Regex; (from upstream's bun/deno runner work); both imports kept. Body otherwise byte-identical.
4d3b417 test(core): pin stdout-only stderr handling Nicolas Le Cam Cherry-pick of 7b89028, byte-identical (tests/stderr_only_failure_test.rs, unix-only).
9ed2e49 fix(runner): drop the failure-only stderr forward superseded by rtk-ai#3772 kylehgc Amendment. The fork carries an adoption of upstream rtk-ai#3029 (georgyia, c27bbd0, fork PR #17) that forwards stderr on non-zero exits only. rtk-ai#3772 subsumes it; left in place the two blocks print the diagnostics twice on failure. Its test file tests/stderr_passthrough_test.rs keeps the failure case; the success case flips from "stays suppressed" to "reaches the user" — the old assertion encoded rtk-ai#3029's scoping, which is exactly what rtk-ai#3772 changes.
90008ee test(core): pin the prettier stderr case to this fork's prettier contract kylehgc Amendment. Upstream's prettier is stdout-only, so rtk-ai#3772's prettier test expects the stderr report forwarded verbatim and an empty stdout. This fork's prettier (PR #117, upstream rtk-ai#2878) runs on the combined stream and summarises the [warn] report on stdout, so that test would fail on fork CI. Re-pinned to the intent: the report reaches the user (either stream) and no clean run is invented; exit code still checked.

Why the rtk-ai#3029 adoption goes

Upstream rtk-ai#3029 is still open, so this is not a heal. It is a partial supersession inside the fork: rtk-ai#3772 does everything rtk-ai#3029 did plus the success-exit case, and the two cannot coexist without a double print. git show upstream/develop:src/core/runner.rs | grep 3029 is blank — the block was fork-only.

Repro, real binaries (before = installed develop build, after = this branch), fake ruff.cmd / prettier.cmd first on PATH

ruff: stderr "ruff diagnostics on stderr", exit 0
before: stdout=[] stderr=[]                                  ← the bug: silence on both streams
after:  stdout=[] stderr=[ruff diagnostics on stderr]

ruff: stderr "ruff failed hard", exit 2
before: stderr=[ruff failed hard]           (#3029 path)
after:  stderr=[ruff failed hard]  1 line   (once — the amendment matters here)

ruff: stdout "stdout line" + stderr "stderr diagnostic", exit 1
after:  stdout=[stdout line] stderr=[stderr diagnostic]      (no replay onto stdout)

ruff: nothing, exit 0
after:  stdout=[] stderr=[]                                  (silent stays silent)

prettier: stderr-only "[warn] src/a.js" report, exit 1
before/after: stdout=[Prettier: 1 files need formatting / 1. src/a.js] stderr=[]   (fork #117 contract, unchanged)

Quality gate

x64 host toolchain: cargo fmt --all --check clean · cargo clippy --all-targets 0 warnings · cargo test --all 3233 unit tests passed, 0 failed, 8 ignored; all integration suites green. The two stderr integration test files are #![cfg(unix)] and compile away on Windows; fork CI (ubuntu, macos) is their verification. git diff --check clean.

Not done

🤖 Generated with Claude Code

KuSh and others added 5 commits September 3, 2026 18:51
`RunOptions::stdout_only()` is documented as "stdout-only to filter, stderr
passthrough", but the passthrough half was never implemented: stderr was captured
and dropped on every path except `skip_filter_on_failure`. A tool that reports on
stderr and leaves stdout empty therefore produced nothing at all.

golangci-lint is the visible case -- a config or build error goes to stderr, the
filter is handed an empty stdout, and the never-worse guard replaces its
"JSON parse failed" fallback with the empty string, so both streams end up
silent. The same shape applies to every other stdout-only filter: ruff, pytest,
rspec, rubocop, gh, glab, prettier, tree, wc.

Savings are also measured against running the command directly, so the stderr
that is forwarded has to be counted as emitted. Comparing stdout to stdout while
printing stderr as well booked a passed-through stream as if it had been
filtered away.

The guard keeps comparing against stdout. stderr is forwarded verbatim on both
sides, so it cancels: never-worse reduces to `filtered <= stdout`, and widening
the baseline to the combined output would let rtk emit more than the command it
replaces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six cases over a fake tool on PATH, covering both halves of the contract: the
tool's own stderr has to reach the user, and rtk must not invent a stdout
message in its place.

The prettier case is the one that is easy to get wrong. It writes its report to
stderr and nothing to stdout even on a successful run, so a filter's empty-input
placeholder ("Error: prettier produced no output") would be printed over a run
that worked. An empty stdout from a command that printed nothing on stdout is
the correct answer.

Also pinned: stderr is not replayed onto stdout on top of being forwarded, a
genuinely silent command stays silent, forwarding is not golangci-specific, and
exit codes propagate -- including golangci-lint's exit 1, which means issues
were found and is reported without failing the build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…i#3772

The fork adopted upstream rtk-ai#3029 (georgyia, c27bbd0) to surface a failing
tool's stderr under stdout-only filtering, scoped to non-zero exits. rtk-ai#3772
forwards stderr on every exit and counts it, so the earlier block now prints
the same diagnostics twice on failure. Remove it; its test file keeps the
failure case and its success case now pins the new contract: a success exit
with stderr-only output is no longer collapsed to silence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ract

Upstream's prettier is a stdout-only filter, so rtk-ai#3772's test expects the
stderr report forwarded verbatim and an empty stdout. This fork's prettier
(PR #117, upstream rtk-ai#2878) runs on the combined stream and reads the
"[warn] <file>" report itself, printing a summary on stdout. Keep the
intent of the test — the report reaches the user and no clean run is
invented — without pinning which stream carries it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The six tests adopted from upstream rtk-ai#3772 spawn rtk without RTK_DB_PATH, so
every run booked fake-tool savings into the developer's history.db. Point
them at a per-test file in the tempdir, as tests/stderr_passthrough_test.rs
already does. Also names rtk-ai#2878 as the upstream issue it is.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@kylehgc

kylehgc commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Review round

Independent read-only reviewer pass over the branch, applying .claude/agents/code-reviewer.md. Verdict: approve, 0 blocking, 2 nits.

Verified

  • Cherry-pick fidelity: 12db983 differs from upstream 2eee910 only in the import hunk's context lines; 4d3b417 is byte-identical to 7b89028. Authorship preserved.
  • Every path through run_captured_filter emits stderr exactly once and never onto stdout: skip_filter_on_failure (early return), stdout-only with and without tee, no_trailing_newline, and the default combined-stream mode (stderr travels inside the filter input, not forwarded separately). Guard baseline stays stdout; tracking baseline is the full raw output plus forwarded stderr, as the PR states.
  • All 30 RunOptions::stdout_only() call sites hand off to the runner and print nothing themselves; the eprint! hits elsewhere are exec_capture paths that never enter the runner. No double print anywhere. The removed fix(runner): surface a failing tool's stderr under stdout-only filtering rtk-ai/rtk#3029 block (9ed2e49) was the only other eprint! of raw_stderr on the filtered path, so its removal was required, not optional.
  • tests/stderr_passthrough_test.rs::successful_tool_stderr_reaches_the_user fails on the pre-branch binary and passes after: a real regression test for the success-exit case, which fix(core): stdout-only filters silently swallow a tool's stderr rtk-ai/rtk#3772's own tests do not cover (all its fake tools exit non-zero except the silent one).
  • No unix-gated unit test in src/** asserts stderr suppression or tracking numbers through the runner, so nothing the Windows gate skipped would flip.

Said plainly: the re-pinned prettier test (90008ee) passes both before and after the runner change, because this fork's prettier runs on the combined stream. It now pins the fork's prettier contract (PR #117), not the runner. The runner's success-exit case is covered by the passthrough test above.

Nits taken (e98a14b): upstream's six adopted tests spawned rtk without RTK_DB_PATH and booked fake-tool savings into the developer's real history database; they now use a per-test file in the tempdir. The prettier comment names rtk-ai#2878 as an upstream issue.

Nits declined: no Windows-runnable success-exit stderr test (both files are #![cfg(unix)]; fork CI runs ubuntu and macos). Behavioural note for the record: go test / golangci-lint stderr chatter on a success exit is now forwarded verbatim, as intended by rtk-ai#3772.

Gate re-run after the amendment: fmt clean, clippy 0 warnings, all tests green on the x64 host toolchain.

@kylehgc
kylehgc merged commit 301551c into develop Sep 3, 2026
10 checks passed
@kylehgc
kylehgc deleted the adopt/3772-stderr-passthrough branch September 3, 2026 23:06
kylehgc added a commit that referenced this pull request Sep 6, 2026
The Fork docs workflow pushed a delta refresh (5f2f4cf) to develop after #159
merged; .github/README.md is regenerated by scripts/fork-delta.sh here.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

adopt core: stdout-only filters swallow a tool's stderr on success exits (upstream #3772)

2 participants