fix(git): keep machine output raw

[YOMXXX]

Fixes #2487.

Summary

• Keep git commands raw in hook rewrite when stdout is piped, redirected to a file, or explicitly machine-readable (--porcelain, --format, --name-only, -z, etc.).
• Preserve normal model-facing rewrites such as bare git diff and later segments in compound commands.
• Make direct rtk git status --porcelain, rtk git log --format=..., and rtk git diff --name-only pass through byte-for-byte instead of compacting.

Verification

• cargo +1.93.0 test rewrite_git_ -- --nocapture
• cargo +1.93.0 test rewrite_redirect -- --nocapture
• cargo +1.93.0 test registry -- --nocapture
• cargo +1.93.0 test git::git::tests::test_ -- --nocapture
• CLI smoke: rewrite passthrough for git status --porcelain | wc -l, git log --format=%H | sort, git diff --name-only HEAD~1..HEAD; rewrite still active for bare git diff
• CLI smoke: rtk git diff --name-only, rtk git status --porcelain, and rtk git log --format=%H -3 match native git via cmp
• cargo +1.93.0 fmt --all -- --check
• cargo +1.93.0 clippy --all-targets
• cargo +1.93.0 test --all -- --skip small_grep_not_worse_than_plain
• git diff --check

[KuSh]

Thanks for tackling #2487 — the shape of the fix (detect a machine-output request, pass the command through raw) is the right one, and the status / --porcelain path looks good.

I built this branch and probed the resulting binary. A few things block a merge:

• A panic. git -C (or -c / --git-dir / --work-tree) as the final token indexes past the token slice and aborts the whole rtk discover run.
• The original bug still reproduces. --pretty=%H is git's implicit tformat: but isn't detected, so rtk git log --pretty=%H still truncates 1185 commits to 50 — in both the filter and the rewriter.
• Two false positives in the machine-output lists: --no-color (disables diff compaction for any agent that passes it) and --word-porcelain (not a real git flag — the real one, --word-diff=porcelain, is missed).
• A regression against develop. Making a piped git segment unrewritable also drops the downstream rewrite develop already performs (git log -10 | grep feat → git log -10 | rtk grep feat).
• Plus an unbounded git log passthrough and a quote-blind token scan.

One process note: the branch is based on ee9e2f8 and is currently conflicting. Develop has since reworked pipe handling (TokenKind::Pipe(PipeKind), |&) and already returns None for cases like git status --porcelain | wc -l — so a good part of the pipe hunk should be redundant once you rebase. Worth re-deriving what's still needed on top of the new pipe handling rather than porting it verbatim.

Details inline.

[KuSh]

[high] Slice-index panic when a git global option is the last token.

i += 2 for -C | -c | --git-dir | --work-tree is unbounded, so when one of those is the final token i becomes len + 1 and tokens[i..] panics.

Repro on this branch:

rewrite_command("git -C")
// panicked at src/discover/registry.rs:926:
// range start index 3 out of range for slice of length 2

Reachable from any command string the hook or rtk discover processes — git -c, git --git-dir, or a truncated/typo'd entry in shell history — and it takes down the whole rtk discover run.

Fix: i = i.saturating_add(2).min(tokens.len()), or return None when the value token is missing.

[KuSh]

[high] --pretty=%… isn't detected, so the bug from #2487 still reproduces.

Git treats --pretty=<string containing a % placeholder> as an implicit --pretty=tformat:<string>, but this only matches --format, --format=, --pretty=format: and --pretty=tformat:.

Verified against a build of this branch, in this repo:

rtk git log --pretty=%H   →     50 commits
git log --pretty=%H       →   1185 commits

The RTK -50 default is still injected and the output is silently truncated — exactly the class of bug this PR sets out to fix. Needs a "value contains %" rule for --pretty= (and for the separated --pretty <value> form below).

[KuSh]

[high] Same --pretty=%… gap in the hook rewriter.

Verified: rewrite_command("git log --pretty=%H") → Some("rtk git log --pretty=%H"), so an unpiped, unredirected git log --pretty=%H still gets routed into the truncating filter.

Both detectors need the % rule, and they need to stay in sync with log_args_request_machine_output in src/cmds/git/git.rs.

[KuSh]

[medium] --no-color is not a machine-output signal — this kills diff compaction.

--no-color says nothing about machine readability; it's precisely what an agent passes to avoid ANSI in its capture. Classifying it as machine output means any agent that habitually passes it gets 0% savings on git.

Verified on this branch: rtk git diff --no-color emitted 2602 bytes — byte-identical to raw git diff --no-color — where develop emits the compacted stat+diff form.

The same entry at src/discover/registry.rs:944 makes the rewriter return None for git diff --no-color and git log --no-color -10, so the hook stops rewriting them at all.

Please drop --no-color from both lists.

[KuSh]

[medium] --word-porcelain is not a git flag; the real machine mode is missed.

git diff --word-porcelain just prints git's usage error, so this arm is dead code. The actual machine-readable word diff is --word-diff=porcelain, which neither this function nor diff_args_request_machine_output (src/cmds/git/git.rs:98) matches — so the real machine format still gets compacted and rewritten.

Replace the dead entry with a --word-diff=porcelain check, in both places.

[KuSh]

[medium] stdout_is_piped: true makes the whole pipeline unrewritable, dropping downstream rewrites develop already performs.

When the git segment returns None here, any_changed stays false and — with no following operator — rewrite_compound returns None for the entire command. The PR's own test changes encode that (git log -10 | grep feat → None).

But develop (rtk 0.46.0) already keeps piped git raw and still rewrites the consumer:

git log -10 | grep feat   →   git log -10 | rtk grep feat
git status | grep foo     →   git status | rtk grep foo

Merging as-is regresses both.

Also worth flagging for the rebase: this branch is based on ee9e2f8, and develop has since reworked pipe handling (TokenKind::Pipe(PipeKind), |&) and already returns None for cases like git status --porcelain | wc -l. A good part of this hunk should be redundant once rebased — please re-derive what's actually still needed on top of the new pipe handling instead of porting it verbatim.

[KuSh]

[medium] The passthrough drops the implicit commit cap, so output becomes unbounded.

This branch runs git log <args> with no -N and no --no-merges, unlike the path below it which injects -50 / -10.

Verified: rtk git log --format=%H (no explicit limit) now prints the full 1185-commit history in this repo; on develop it printed 50. A single such call can flood the agent's context — the thing RTK exists to prevent.

Suggest keeping a large default cap when the user gave no -n / --max-count, or at minimum documenting the unbounded behaviour.

[KuSh]

[low] split_token_spans is quote-blind, so flag words inside quoted arguments suppress rewriting.

It splits on whitespace only, ignoring the quoting tokenize() already handles.

Verified:

rewrite_command("git commit -m \"add --no-color to diff\"")   →   None
rewrite_command("git log -- --name-only")                    →   None

Both silently lose the rewrite (and the savings) because a flag-looking word appears inside a quoted message or after --. Using tokenize() and stopping the scan at -- handles both.

[KuSh]

Followed up on the fork-side amendment kylehgc/rtk@da407dc proposed on top of this PR. It picks up two real defects that didn't come up in my review at all, but the highest-severity items above are untouched, so the request for changes still stands.

Caught by the amendment, not by my review

• -z was unmatched for git log. git log -z --name-only carries no custom format, so it fell through to the compact path and had --pretty=format:<MARKER>, -10 and --no-merges injected — corruption of a machine-readable format, exactly what this guard exists to prevent. Correctly fixed. It does widen an existing problem, though; see the inline note on run_log.
• The new passthrough branches were recording 0%-savings rows. timer.track(.., &stdout, &stdout) logs equal non-zero token counts on every passthrough, which dilutes rtk gain. track_passthrough (src/core/tracking.rs:1428) records input_tokens=0, output_tokens=0 for precisely this case — right call, and consistent with run_passthrough in the same file.

The --no-color and --word-porcelain corrections only land on half the path. Both are fixed in src/cmds/git/git.rs but not in src/discover/registry.rs. For --word-diff=porcelain the end-to-end result still comes out right (the rewriter routes it to rtk, git.rs then passes it through raw), but the two lists should come from one source rather than drifting. For --no-color the fix is inert as written — details inline on registry.rs.

Still open: the panic on a trailing git -C; --pretty=%H in both the filter and the rewriter (#2487's headline case); the pipe regression; the uncapped log passthrough; the quote-blind token scan.

---

New findings, in the amendment's own tests/git_machine_output_test.rs. That file isn't part of this PR, so these can't be pinned inline:

1. [medium] human_facing_git_output_is_still_compacted is flaky. It asserts rtk git diff --no-color HEAD~1..HEAD is strictly smaller than native — which depends entirely on what the tip commit happens to be, since compaction prepends a stat header that outweighs the savings on small diffs. Measured across the last 20 non-merge commits on develop, 2 invert:

6370e79   native 2148 bytes   rtk 2171 bytes   ← test would fail
e5cecd3   native 2206 bytes   rtk 2208 bytes   ← test would fail

That's a ~10% chance of a red build for reasons unrelated to the change. Assert against a fixed synthetic diff or a pinned commit range instead of HEAD~1.

2. [medium] run() ignores exit status, so the byte-equality test passes vacuously in CI. It returns Some(stdout) for anything that spawns. The test job checks out at the default fetch-depth: 1 (.github/workflows/ci.yml:133, cargo test --all at :136), so HEAD~1..HEAD is an unknown revision there: native git returns empty stdout, let Some(native) = … else { continue } does not skip (it's Some(vec![])), rtk returns empty too, and assert_eq!("", "") passes. All four diff … HEAD~1..HEAD cases contribute zero coverage in CI, silently — and the --no-color counterweight is skipped outright by its is_empty() guard. Check out.status.success() and return None on failure so those cases genuinely skip, and ideally build the fixture history with git init in a temp dir instead of depending on the checkout's history.

3. [low] The one case #2487 is actually about stays uncovered. The log cases are --format=%H and --pretty=format:%H; neither exercises --pretty=%H, the spelling that still truncates. That's the regression test for the original issue — worth adding once the predicate is fixed.

4. [nit] The test writes to the developer's real tracking DB. Each run invokes the binary ~13 times with no RTK_DB_PATH override, so rows land in ~/.local/share/rtk/history.db (verified: two invocations moved it 58942 → 58944 rows). This is pre-existing across the suite — tests/guard_integration_test.rs and friends do the same — so not something the amendment introduced. But given the commit's stated motivation is keeping rtk gain undiluted, an .env("RTK_DB_PATH", tmp) would match that intent.

[KuSh]

Update after kylehgc/rtk@da407dc.

The amendment adds -z here, which is a genuine catch I missed: git log -z --name-only carries no custom format, so it was falling to the compact path and getting --pretty=format:<MARKER>, -10 and --no-merges injected — exactly the corruption this guard exists to prevent.

But --pretty=%H is still unmatched, so #2487's headline case is unchanged: rtk git log --pretty=%H returns 50 commits where git returns 1185. Git expands --pretty=<string containing %> to --pretty=tformat:<string>; this predicate needs that rule, and so does the separated --pretty <value> form just below.

This is the case the issue is filed about — it should land before merge.

[KuSh]

Update after kylehgc/rtk@da407dc.

The amendment doesn't change this branch, but it widens what reaches it. Adding -z to log_args_request_machine_output means git log -z --name-only now lands here uncapped, where it previously took the compact path with -10. On this repo that turns a 10-commit summary into the full 1185-commit history, with per-commit file lists.

Both changes are individually right; together they make the missing default cap considerably more reachable. A large default -n when the user gave neither -n nor --max-count keeps the -z fix without the flood.

[KuSh]

Update after kylehgc/rtk@da407dc.

The amendment removes --no-color from diff_args_request_machine_output in src/cmds/git/git.rs, but not from this list. That leaves the fix inert on the path that actually matters: with --no-color still here, rewrite_command("git diff --no-color") returns None, the hook never rewrites the command, raw git runs, and the compaction restored in git.rs is never reached.

The entry has to come out of both lists — ideally with the two predicates sharing one source so they can't drift apart again.

[KuSh]

Thank you for working on this. The rewrite side is now handled by the per-rule pipeline safety model from #3128 (merged 2026-07-23) and #3343 (merged 2026-09-07), so commands like git status --porcelain | wc -l, git log --format=%H | sort and git diff > out.patch are no longer rewritten, and #3681 (merged 2026-09-12) moved git status, log and diff argument handling onto the shared argument tokenizer, so the detection added here no longer fits the current code. rtk git diff --name-only, --name-status, --numstat, --raw, -z, rtk git status -z and rtk git log --format=%H already print exactly what git prints. Two cases still differ: rtk git status --porcelain (and --porcelain=v2) drops git's final newline, and rtk git log --pretty=format:%h adds a trailing newline git does not print; an issue or a narrow PR against the current src/cmds/git/git_cmd.rs would be welcome for either. If I missed something this PR still covers, please comment and it can be reopened.