[YOMXXX]

Summary

`rtk git diff` and `rtk diff` were emitting decorative output that broke `git apply`, `patch`, shell loops over `--name-only`, and exit-code-based predicates. Because the `PreToolUse` hook silently rewrites `git diff` to `rtk git diff` for hooked agents, those decorations silently distorted any patch / script workflow that used to work. Issue #1918 (and its duplicates #1081 / #1869) ask for the POSIX/git contract to be honored.

Two surfaces are fixed:

`rtk git diff` (proxy for `git diff`)

• Detects non-TTY stdout (`std::io::IsTerminal::is_terminal`) and machine-friendly flags (`--name-only`, `--name-status`, `--check`, `--exit-code`, `--raw`, `-z`, `--no-color`, `--numstat`, `--shortstat`, `--dirstat`, `--patch-with-raw`, `--patch-with-stat`, `--output[=…]`).
• When either is true: emit raw `git diff` output verbatim, with byte-for-byte fidelity (`print!` not `println!`) so trailing newlines are preserved for `git apply`.
• Propagate git's actual exit code rather than forcing 0.

`rtk diff` (RTK's GNU-diff-like file compare)

• Returns `0` when files are identical, `1` when they differ, `2` on missing-file / I/O error — the GNU diff exit-code contract.
• The `[ok] Files are identical` status now goes to stderr, so `rtk diff a b > patch.diff` produces an empty patch on identical files (which is what scripts expect) rather than text in stdout.

Reproduction

```bash

rtk git diff round-trip

mkdir -p /tmp/g && cd /tmp/g && git init -q
git config user.email t@t.t && git config user.name t
printf '1\n2\n3\n' > f && git add f && git commit -q -m x
printf '1\n2x\n3\n' > f

rtk git diff > rtk.patch
git stash -q
git apply --check rtk.patch && echo "applies"

Before this PR: 'No valid patches in input'

After this PR: 'applies'

rtk git diff --name-only

Before this PR: 'f\n\n--- Changes ---\n f (...) ...'

After this PR: 'f'

rtk diff exit codes

printf 'a\n' > /tmp/a && printf 'b\n' > /tmp/b
rtk diff /tmp/a /tmp/b; echo "exit=$?"

Before this PR: exit=0 (silently wrong)

After this PR: exit=1 (GNU diff convention)

rtk diff /tmp/nonexistent /tmp/a; echo "exit=$?"

Before this PR: exit=1

After this PR: exit=2 (GNU diff convention)

```

Behavior changes

• Non-TTY `rtk git diff`: no longer prepends a stat summary, no longer wraps in `--- Changes ---`. Output is byte-for-byte `git diff`. The compact path is only taken in interactive (TTY) sessions without machine-readable flags.
• Machine-friendly flags (`--name-only` etc.): same — clean passthrough.
• `rtk diff` exit codes now match GNU `diff`. The "Files are identical" message moves from stdout to stderr.

The compact-diff experience for interactive `rtk git diff` users is unchanged.

Test plan

• `test_has_machine_friendly_diff_flag_detects_name_only`.
• `test_has_machine_friendly_diff_flag_detects_machine_variants` (covers 12 flags).
• `test_has_machine_friendly_diff_flag_ignores_normal_args` (negative).
• `test_has_machine_friendly_diff_flag_detects_output_arg` (covers both `--output=...` and `--output ...`).
• `test_run_returns_0_on_identical_files`.
• `test_run_returns_1_on_different_files`.
• `test_run_returns_2_on_missing_file`.
• All 51 existing diff tests still pass.
• `cargo fmt --all` clean.
• `cargo clippy --all-targets` zero warnings.
• `cargo test --bin rtk -- --test-threads=8` 1908 passed, 0 failed.

Out of scope (deferred to a follow-up)

• `[hooks].exclude_commands` subcommand matching ("git diff" must skip the hook) — issue [hooks] exclude_commands does not cover subcommands (e.g. "git diff") #1919. Wider scope; deserves its own PR.
• TTY detection for `rtk diff` (the GNU-like file compare) is not added — it's a content-transforming command, not a passthrough, so the "non-TTY emits unified diff" semantics don't apply cleanly. Exit-code fix is sufficient for the headline scripting use case.

Fixes #1918, #1869
Refs #1081 (duplicate of #1918)

[YOMXXX]

Review note: I see two blocking issues before this should merge.

1. The passthrough branch drops stdout on non-zero exit. For git diff --exit-code, a real diff returns exit code 1 and still writes the patch to stdout. The current code returns as soon as !result.success() and only prints stderr, so it loses the diff content in exactly the programmatic-consumer case this PR is trying to preserve. Please print result.stdout before returning the real exit code, and add a test for --exit-code with differences.

2. !stdout.is_terminal() is too broad as a passthrough trigger. Claude/Codex-style agents usually capture stdout through a non-TTY, so ordinary rtk git diff in the agent path would now fall back to raw git diff, which defeats RTK's token-reduction behavior. I would keep passthrough tied to machine-friendly flags and explicit opt-outs, not non-TTY alone.

[YOMXXX]

Follow-up to my self-review note above — both blocking issues are now addressed in ed86670 (force-pushed).

1. --exit-code no longer drops stdout on non-zero exit.

The passthrough branch now forwards stdout and stderr independently of result.success(). git diff --exit-code returns exit 1 with the patch on stdout when files differ — the previous shape returned early on !result.success() and only printed stderr, losing the patch in exactly the programmatic-consumer case this PR is supposed to preserve. The exit code is still propagated unchanged.

if !result.stdout.is_empty() {
    print!("{}", result.stdout);
}
if !result.stderr.is_empty() {
    eprint!("{}", result.stderr);
}
return Ok(result.exit_code);

2. !stdout.is_terminal() is no longer a passthrough trigger.

The trigger is now purely flag-driven via has_machine_friendly_diff_flag(args). Agent runtimes (Claude Code, Codex, …) always capture stdout through a pipe, so an !is_terminal trigger would have silently disabled every RTK compaction on the exact code path where the token savings matter most. Inline comment in the source spells the rationale out so a future reader doesn't re-add the trigger thinking it's harmless.

Programmatic consumers still get raw passthrough — they just have to signal intent with a real flag (--name-only, --exit-code, --check, --raw, -z, --no-color, --patch-with-raw, --patch-with-stat, --numstat, --shortstat, --dirstat, --output[=…]), which is the same convention git itself uses for "this is for a script."

cargo fmt / clippy --all-targets / test --bin rtk: 1908 passed, 0 warnings.

[YOMXXX]

Rebased this PR onto current develop and resolved conflicts in the git diff path.

Resolution notes:

• Kept develop's args_utils::restore_double_dash path for the existing clap -- restoration behavior.
• Preserved this PR's flag-driven raw passthrough for machine-friendly git diff modes, including stdout/stderr forwarding even when git returns nonzero for --exit-code.
• Preserved GNU rtk diff exit-code behavior and kept the identical-files status off stdout.
• Added an integration regression test for rtk git diff --exit-code preserving patch stdout while returning exit 1.

Local verification completed:

• cargo +1.93.0 fmt --check
• git diff --check upstream/develop...HEAD
• cargo +1.93.0 test git_diff_exit_code_preserves_stdout_on_nonzero --test git_diff_contract_test -- --nocapture
• cargo +1.93.0 test has_machine_friendly_diff_flag -- --nocapture
• cargo +1.93.0 test render_ -- --nocapture
• cargo +1.93.0 test run_returns_ -- --nocapture
• cargo +1.93.0 clippy --all-targets
• cargo +1.93.0 test --all -- --skip small_grep_not_worse_than_plain

[KuSh]

Requesting changes. There is a good PR in here, but it needs a rebase and a narrower scope, and its headline claim doesn't currently hold.

I reviewed the real PR head (c542ad1) fetched from pull/1981/head. The branch is conflicting with develop and roughly three months behind it.

The headline issue. The commit says "Fixes #1918" and the PR body says the patch now applies, but flagless rtk git diff is still decorated: the passthrough trigger is flag-only (is_terminal was deliberately dropped), so a bare git diff still falls into the compact path at git.rs:233-249 and emits --stat header + Changes: + truncated compact_diff. rtk git diff > p.patch; git apply therefore still fails, which is #1918's exact repro. No test covers the flagless case — the one new integration test uses --exit-code, which takes the passthrough branch.

Scope. Since this was opened, #2473 has landed the exit-2 half of what this PR does, with the same semantics. That leaves two genuinely unique contributions here that I'd like to keep: the machine-friendly flag passthrough, and moving the identical-files verdict off stdout. Narrowing to those would make this reviewable; as a four-file PR spanning both it's competing with itself.

Details inline. Two more that don't anchor cleanly:

• --quiet is missing from the allowlist (git.rs:111). rtk git diff --quiet on a clean tree hits println!("{}", result.stdout.trim()) and emits a bare newline, so test -z "$(rtk git diff --quiet)" fails despite the flag meaning "disable all output".
• Exact string matching misses value-taking spellings. --stat=200 and --dirstat=files,10 match neither wants_stat nor the new list, so they get the Changes: decoration instead of the stat table. You handled the prefix form for --output=, so the pattern was clearly considered — it just needs applying to the rest.
• tests/git_diff_contract_test.rs:28 isn't isolated from global git config or HOME the way its sibling tests are (they set LC_ALL/HOME). A developer with diff.external set breaks it, and the spawned binary writes to their real tracking database.

Things I checked and found fine, so you don't re-litigate them: std::process::exit does flush stdout, so print! without an explicit flush is safe; decode_process_output keeps valid UTF-8 verbatim, so the byte-fidelity claim holds for text patches; and forwarding stderr alongside stdout and returning result.exit_code in the passthrough branch are correct for --stat/--no-compact.

[KuSh]

This is the most important one: the branch is based on ee9e2f8, which predates never_worse being added to diff_cmd::run.

Current develop (0de9610) has let shown = never_worse(&raw, &rtk); in this exact block, plus the use crate::core::guard::never_worse; import. This PR head has neither, so merging as-is silently removes the guarantee that rtk diff never emits more tokens than the raw input — a cross-cutting invariant, not a local one.

Needs a rebase onto current develop with the guard reapplied. Worth doing before anything else here, since the rebase will also resolve the conflict and may change how the rest of the diff reads.

[KuSh]

The comment above says "Match GNU diff: silent on identical files", but this moves [ok] Files are identical to stderr rather than removing it. Those aren't the same thing: rtk diff a b 2>&1 | patch puts the decorative line straight back into the patch stream, which is the failure mode the PR is trying to prevent.

If the goal is GNU parity, print nothing at all on the identical path and let exit 0 carry the information. If the goal is keeping a human-readable signal, that's defensible too — but then the comment should say so rather than claiming silence.

Worth flagging that this interacts with #3469: on current develop the "identical" verdict is also emitted for byte-different files (CRLF-vs-LF, trailing newline). Whatever this PR does with the verdict should assume that distinction will soon exist.

[KuSh]

--no-color is in a list documented as "flags that change git diff output into a non-unified format", but it doesn't — it produces exactly the same unified diff without ANSI escapes.

Including it means anyone whose alias or wrapper passes --no-color (common in scripts, and some CI images set it globally) silently gets all RTK compaction disabled. That's a large, invisible behaviour change for a flag that has nothing to do with output shape.

Suggest dropping it from this list. If the intent was to make sure colour codes don't reach the parser, that belongs upstream in how the command is built rather than in the passthrough predicate.

[KuSh]

Thanks for this, and sorry it sat so long. Both files this touches were rewritten since it was opened: the decision to pass git output through raw now runs through the argument tokenizer in src/cmds/git/git.rs, and the file comparison in diff_cmd.rs was rebuilt by #3519 on 2026-09-02 and #3268 on 2026-09-08, which also added the byte level identity and invisible difference handling. develop also deliberately does not gate on is_terminal, since the hook always pipes rtk's stdout, so the TTY based passthrough would not be accepted as written. Four cases from this PR are still open on develop and are worth separate issues or narrow PRs on the current code: -z, --no-color and --output are not treated as raw shapes for git diff, a plain rtk git diff piped to git apply is still condensed (issue #1918 is still open), rtk diff on a missing file exits 1 without naming the file where diff exits 2, and the [ok] Files are identical verdict goes to stdout rather than stderr. If you want to carry any of those forward, that would be very welcome. Please comment or reopen if a part of this was missed.