fix(git): stream push output to avoid spurious 30s timeout (#963)

[ousamabenyounes]

Problem (#963)

`rtk git push` reportedly times out: users see `bash tool terminated command after exceeding timeout 30000 ms` while plain `git push` to the same remote completes fine. P1-critical because every Claude Code git push goes through rtk.

Root cause

`run_push` used `cmd.stdin(Stdio::inherit()).output()`. `Command::output()` captures both stdout and stderr until the child exits.

Git push prints its progress (`Counting objects` / `Compressing objects` / `Writing objects`) to stderr and may prompt for SSH passphrases or HTTPS credentials. With stderr captured nothing reached the terminal until the process finished, so:

1. Claude Code's bash tool saw zero output for 30+ seconds and killed the command — exactly the 30000 ms message in the issue.
2. Long pushes / slow remotes / 2FA prompts looked like a hang to humans.
3. Auth prompts that git writes to stderr (rather than `/dev/tty`) were invisible until the user gave up and Ctrl+C'd.

The original code traded interactivity for a one-line compact summary (`ok master`). Push output is small — a handful of lines — so the saving was negligible while the cost (timeout, hidden prompts) was severe.

Fix

Inherit all three streams and use `cmd.status()` instead of `.output()`. Progress and auth prompts now flow through in real time, the bash tool keeps seeing output, and the exit code propagates via the existing `exit_code_from_status` helper. Tracking still records the invocation; raw/filtered token counts deliberately collapse to 0 because we don't capture.

Test plan

• `cargo fmt --all`
• `cargo test --all` — 1681 passed, 0 failed
• `cargo build --release` clean
• No new unit test: the bug is wall-clock + stream buffering against a real network remote, which can't be exercised hermetically. Existing git fetch/pull/clone tests cover the surrounding plumbing; this change only swaps the IO mode for `push`.

Notes

• Behaviour change: users will now see git's native progress output instead of `ok master`. This matches plain `git push` behaviour and is what the issue reporter expects.
• No filter regressions: `run_pull`, `run_fetch`, `run_clone` are untouched; only `run_push` is modified.

Closes #963

🤖 Generated with Claude Code

---

Vibe Coded by Ousama Ben Younes
Developed With Ora Studio (Claude Code)

[pszymkowiak]

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type	bug-fix
🟡 Risk	medium

Summary

Fixes a critical timeout issue where rtk git push would be killed by Claude Code's 30-second bash timeout because stdout/stderr were buffered until process exit. The fix switches from Command::output() (which captures streams) to Command::status() with all three stdio streams inherited, allowing git's progress output and auth prompts to flow through in real time.

Review Checklist

• Tests present
• Breaking change
• Docs updated

Linked issues: #963

---

Analyzed automatically by wshm · This is an automated analysis, not a human review.

[aeppling]

Hello @ousamabenyounes , thanks for addressing this.

The fix is not correct because loose of filtering, to fix this we need to use run_streaming + FilterMode::Streaming existing infrastructure that has been added in 0.37.0 for streaming commands.

[ousamabenyounes]

Thanks for the steer @aeppling — rewritten on top of stream::run_streaming + FilterMode::Streaming in a402997.

GitPushStreamFilter (StreamFilter) now:

• feed_line drops the high-volume progress phases (Enumerating / Counting / Compressing / Writing objects, Delta compression, Total) and passes through everything else (remote: messages, To <url>, ref updates, errors). Lines flow to the terminal as git emits them — so the bash tool never sees the 30 s silence — while still cutting the bulk of the noise.
• on_exit appends a compact ok <ref> / ok (up-to-date) summary on success, restoring the token-saving payoff of the previous filter.
• StdinMode::Inherit so SSH passphrase / HTTPS credential prompts still reach the user.
• Tracking now records the real raw + filtered output (the previous force-passthrough commit was zeroing both).

Six new unit tests in src/cmds/git/git.rs cover progress-prefix drop, up-to-date, remote passthrough, no-summary-on-failure, first-ref-wins-for-summary, and a token-savings assertion (>=40% on a representative payload).

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, and cargo test --all (1876 passed, 0 failed) all green locally. Ready for re-review.

[aeppling]

Hey @ousamabenyounes , good follow-up, the rewrite correctly uses run_streaming + FilterMode::Streaming as requested and the filtering intent is right.

Architectural issue

GitPushStreamFilter implements StreamFilter directly, but stream.rs already has a pattern for this: BlockStreamFilter<H: BlockHandler>. The problem is BlockStreamFilter cannot be reused here because its default behavior is DROP (only collected blocks are emitted), while git push needs the opposite: default KEEP, suppress only noise.

The fix would be to add LineStreamFilter<H: LineHandler> to stream.rs as the counterpart to BlockStreamFilter , default KEEP, suppress only noise ; then rewrite GitPushStreamFilter as a LineHandler. Mirror BlockHandler conventions for the trait methods.

This will allow for futur stream command to reuse this behavior.

Additional fixes:

• Token savings threshold is 40%, actual savings on the test fixture are 68% you can raise to 60%
• if !dest.is_empty() after split_whitespace().next() is unreachable, remove it

Details

• The comment inside run_push is a history comment referencing the bug and the old implementation, remove it, it belongs in the commit message (where it already is)
• The comment above GIT_PUSH_NOISE_PREFIXES re-explains what the constant name already says, trim to one line covering only the non-obvious invariant

[ousamabenyounes]

All points addressed in be51783.

Architectural — Added LineStreamFilter<H: LineHandler> to src/core/stream.rs as the counterpart to BlockStreamFilter<H: BlockHandler>. Trait surface mirrors BlockHandler:

```rust
pub trait LineHandler {
fn should_skip(&mut self, _line: &str) -> bool { false } // default KEEP
fn observe_line(&mut self, _line: &str) {} // default no-op
fn format_summary(&self, exit_code: i32, raw: &str) -> Option;
}
```

Default is KEEP — handlers opt into dropping noise via `should_skip`. `GitPushLineHandler` is now a small `LineHandler` impl (no direct `StreamFilter` plumbing), and future line-oriented streaming commands can reuse the pattern. Added 4 unit tests in `stream.rs` covering: default-keep-all, skip-drops-matching, summary-propagates-exit-code, observe-only-called-for-kept-lines.

Additional fixes:

• Token savings threshold raised `40% → 60%`.
• Removed the unreachable `if !dest.is_empty()` after `split_whitespace().next()`.
• Stripped the `When using the rtk git push command, it sometimes times out #963:` history block inside `run_push` (it's in the commit message).
• Trimmed the `GIT_PUSH_NOISE_PREFIXES` doc comment to one line.

`cargo fmt`, `cargo clippy --all-targets -- -D warnings`, and `cargo test --all` (1880 passed, 0 failed) green locally. Re-ready for review.