Skip to content

fix(diff): preserve POSIX exit codes - #2450

Closed
cancaries wants to merge 1 commit into
rtk-ai:developfrom
cancaries:codex/rtk-diff-exit-codes
Closed

cancaries wants to merge 1 commit into
rtk-ai:developfrom
cancaries:codex/rtk-diff-exit-codes

Conversation

@cancaries

Copy link
Copy Markdown

Fixes #2446.

Summary

  • Preserve rtk diff compact output while returning POSIX-compatible exit codes.
  • Return 0 when compared files are identical.
  • Return 1 when compared files differ.
  • Return 2 when either input file cannot be read.
  • Add focused regression tests for all three exit-code paths.

Verification

  • git diff --check

I could not run cargo test diff_cmd locally because this Windows environment does not have a Rust toolchain available.

Copilot AI review requested due to automatic review settings June 15, 2026 03:00
@CLAassistant

CLAassistant commented Jun 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the diff command to return meaningful process-style exit codes (0/1/2) instead of always returning success, and threads those exit codes back through the CLI dispatcher.

Changes:

  • Change diff_cmd::run and diff_cmd::run_stdin to return Result<i32> exit codes (0 identical, 1 different, 2 trouble).
  • Update CLI Commands::Diff handling to return the diff command’s exit code rather than always 0.
  • Add unit tests validating the new exit-code behavior for identical, different, and missing-file scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/main.rs Returns the diff subcommand’s exit code from the CLI match arm.
src/cmds/git/diff_cmd.rs Implements exit-code semantics for diff operations and adds tests for the new behavior.
Comments suppressed due to low confidence (1)

src/cmds/git/diff_cmd.rs:1

  • The exit-code values (0 and 1 here, plus 2 elsewhere) are currently “magic numbers”. Consider introducing named constants (e.g., EXIT_OK, EXIT_DIFF, EXIT_TROUBLE) or a small enum to centralize and self-document the exit-code mapping—this also reduces the chance of future inconsistencies between call sites and tests.
//! Compares two files and shows only the changed lines.

Comment thread src/cmds/git/diff_cmd.rs Outdated
Comment on lines +8 to +9
/// Ultra-condensed diff - only changed lines, no context
pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result<()> {
pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result<i32> {
Comment thread src/cmds/git/diff_cmd.rs
Comment on lines +16 to +29
let content1 = match fs::read_to_string(file1) {
Ok(content) => content,
Err(err) => {
eprintln!("rtk diff: failed to read {}: {}", file1.display(), err);
return Ok(2);
}
};
let content2 = match fs::read_to_string(file2) {
Ok(content) => content,
Err(err) => {
eprintln!("rtk diff: failed to read {}: {}", file2.display(), err);
return Ok(2);
}
};
@cancaries
cancaries force-pushed the codex/rtk-diff-exit-codes branch from 4bb84c2 to 3201982 Compare June 15, 2026 03:01
@cancaries
cancaries force-pushed the codex/rtk-diff-exit-codes branch from 3201982 to feb4fbc Compare June 17, 2026 02:54
@KuSh

KuSh commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Closing as a duplicate of #2473, which makes the same fix: a read failure on either operand returns POSIX diff's exit 2 instead of collapsing into exit 1.

Confirmed still live on 0.46.0 — rtk diff x /nope; echo $? prints 1. The other half of #2446 (differ → exit 1) is already fixed; this is what's left.

#2473 was picked because its tests exercise the read_operands helper directly. The three tests here call run(), which goes through timer.track(...) and writes into the machine's real tracking database — the problem #3758 is currently fixing. Your test_run_returns_zero_for_identical_files / ..._one_for_different_files pair is good coverage of the full contract, though; it's the kind of thing that belongs in tests/guard_integration_test.rs as a real-process test, which nothing currently has. If you'd like to submit that separately it would fill a genuine gap.

@KuSh KuSh closed this Aug 30, 2026
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.

rtk diff exits 0 when files differ, breaking diff conditionals

4 participants