Skip to content

Adopt rtk-ai/rtk#3029: runner - surface failing tool's stderr under stdout-only filters - #17

Merged
kylehgc merged 2 commits into
developfrom
claude/issue-6-adopt-3029
Jul 17, 2026
Merged

kylehgc merged 2 commits into
developfrom
claude/issue-6-adopt-3029

Conversation

@kylehgc

@kylehgc kylehgc commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Closes #6.

Adopts rtk-ai/rtk#3029 (cherry-pick, authorship preserved) plus one fork-authored amendment.

Commits

  • c27bbd0 fix(runner): surface a failing tool's stderr under stdout-only filtering — cherry-pick of upstream 328bb53, author georgyia (@georgyia). Callers pairing stdout_only with early_exit_on_failure (ls, tree) already printed stderr on failure; bare stdout_only callers (pytest, ruff, prettier, go test, golangci-lint, phpunit, phpstan, pint, rubocop) captured it and dropped it. The fix emits raw_stderr when the child fails; success stays suppressed. + tests/stderr_passthrough_test.rs (cfg(unix), shim-based, failing-before).
  • afc5258 test: use expect with reason instead of bare unwrap in stderr test — amendment: .claude/rules/rust-patterns.md requires expect("reason") over bare unwrap() in tests; mechanical rename of the five call sites in the new test file.

Adoption checklist

  • Sync: no-op — upstream/develop is already an ancestor of fork develop
  • Repro on pre-cherry-pick binary (below)
  • Cherry-pick preserving authorship; fixed behavior confirmed with real output (below)
  • Quality gate: cargo fmt --all clean, cargo clippy --all-targets zero warnings, cargo test --all 2466 passed / 0 failed
  • Amendment as separate fork-authored commit

Repro (real output, this Windows machine)

This machine has python but no pytest — the exact rtk-ai#3026 quadrant, no uv needed: rtk pytest falls back to python -m pytest, the spawn succeeds, and the child's diagnostic goes to the captured-and-dropped stderr.

Before (develop @ 1018b8b):

$ rtk pytest --version
exit 1, stdout 1 byte (a single newline), stderr 0 bytes

A completely mute failure — reads as a quiet pass.

After (this branch):

$ rtk pytest --version
exit 1, stderr 48 bytes:
C:\Python314\python.exe: No module named pytest

Review

Two-axis review (standards + spec) run pre-PR.

  • Spec: clean — fix hunk byte-identical to upstream and correctly placed (early-return ordering verified), no-double-print claim verified against FilterMode::CaptureOnly and both raw_stderr print sites, both claimed tests present and asserting what they claim, no scope creep (git diff pr-3029 HEAD empty on both files).
  • Standards: one hard finding (bare unwrap() in tests vs rust-patterns.md) fixed by the amendment. Known ceilings, noted not fixed: the new tests are #![cfg(unix)] so Windows CI never runs them (the runner change is platform-neutral and the Windows behavior was verified manually above); emitted stderr is not counted in rtk gain tracking on failure (consistent with the pre-existing skip_filter_on_failure branch).

🤖 Generated with Claude Code

georgyia and others added 2 commits July 17, 2026 17:48
`uv run rtk pytest` produced no output at all when pytest was missing from the
venv: exit 1, a single newline on stdout, nothing on stderr. A test run that
prints nothing and reports no error reads as a quiet pass, leaving the exit code
as the only signal.

The trigger is uv, but the defect is in the runner. Filters that compress stdout
pass RunOptions::stdout_only, which keeps stderr out of the filter input — and
run_captured_filter only ever emitted stderr inside its skip_filter_on_failure
branch. Callers that pair stdout_only with early_exit_on_failure (ls, tree) hit
that branch and print stderr; callers that don't (pytest, ruff, prettier, go
test, golangci-lint, phpunit, phpstan, pint, rubocop) captured stderr and
dropped it on the floor.

Standalone `rtk pytest` looks fine only because python is absent there: the spawn
fails outright and the anyhow error prints. Under `uv run` the venv supplies
python, so rtk falls back to `python -m pytest`, the spawn succeeds, and the
child's "No module named pytest" goes to the stderr that was being discarded.

Emit stderr when the child fails. Only on failure: on a successful run stderr is
the incidental noise RTK exists to suppress, and filtered stdout already carries
the result.

Fixes rtk-ai#3026
Amendment to the adopted rtk-ai#3029: .claude/rules/rust-patterns.md
requires expect(reason) over bare unwrap() in tests. Mechanical rename of
the five bare unwrap() calls in tests/stderr_passthrough_test.rs.

Co-Authored-By: Claude Fable 5 <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 upstream #3029: runner - surface failing tool's stderr under stdout-only filters

2 participants