Skip to content

test(git): cover exit-code propagation for git stash list/show failure - #2500

Merged
aeppling merged 1 commit into
rtk-ai:developfrom
hgunduzoglu:fix/git-stash-list-show-exit-code
Jun 26, 2026
Merged

aeppling merged 1 commit into
rtk-ai:developfrom
hgunduzoglu:fix/git-stash-list-show-exit-code

Conversation

@hgunduzoglu

Copy link
Copy Markdown
Contributor

Part of #2497 (3/3).

Problem

run_stash's list and show branches printed output and returned Ok(0) without checking result.success(). A failed git stash list (e.g. outside a repo) was reported as No stashes + exit 0, and git stash show failures as Empty stash + exit 0. The apply/pop/push branches already guard on success.

$ cd /non-git-dir
$ git stash list        # native → exit 128
$ rtk git stash list    # before
No stashes              # exit 0  ✗

Fix

Add if !result.success() guards to both branches: surface git's stderr (FAILED: git stash list/show header) and return the real exit code.

Behavior now matches native git exactly:

scenario native rtk after
stash list, no stashes (valid repo) exit 0 No stashes, exit 0 ✓ (unchanged)
stash list, non-git dir exit 128 FAILED… exit 128 ✓
stash show, no stashes exit 1 (No stash entries found) surfaces it, exit 1 ✓
stash show, non-git dir exit 128 FAILED… exit 128 ✓

Note git stash list exits 0 when there are genuinely no stashes, so the friendly No stashes success path is preserved — only real failures now propagate.

Testing

  • New tests test_run_stash_list_propagates_failure and test_run_stash_show_propagates_failure (non-git temp dir via git -C, assert non-zero exit; != 0 so cross-platform).
  • cargo fmt --all --check && cargo clippy --all-targets && cargo test — clean, 2201 passed.
  • Manual verification of all four scenarios above against native git.

Completes the #2497 series (run_status #2498, run_worktree #2499). Same masking-failure class as #1581 / #1535 / #2446 / #2494.

develop already propagates the exit code for failed git stash list/show
(via the empty-stdout guard). Add regression tests so the masking-failure
behavior (rtk-ai#2497) can't creep back.
@hgunduzoglu
hgunduzoglu force-pushed the fix/git-stash-list-show-exit-code branch from c2de29d to c04dec1 Compare June 26, 2026 10:16
@hgunduzoglu hgunduzoglu changed the title fix(git): propagate exit code on git stash list/show failure test(git): cover exit-code propagation for git stash list/show failure Jun 26, 2026
@hgunduzoglu

Copy link
Copy Markdown
Contributor Author

Rebased onto current develop and reworked this. While it was open, develop independently fixed the stash list/show masking (the empty-stdout guard now surfaces stderr and returns result.exit_code, plus the new never_worse guard) — so my implementation change was superseded and conflicted.

Rather than duplicate that, I've reset this branch to develop and reduced the PR to just the regression tests for git stash list/show failure propagation, which develop doesn't have yet. They run the handler against a non-git dir via git -C and assert a non-zero exit, so the masking-failure behavior (#2497) can't creep back. cargo fmt/clippy/test green; conflict resolved.

(run_status #2498 merged, run_worktree #2499 LGTM — thanks!)

Comment thread src/cmds/git/git.rs Outdated
let result = exec_capture(&mut cmd).context("Failed to run git stash list")?;

if !result.success() {
eprintln!("FAILED: git stash list");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as for 2499 review

Comment thread src/cmds/git/git.rs Outdated
let result = exec_capture(&mut cmd).context("Failed to run git stash show")?;

if !result.success() {
eprintln!("FAILED: git stash show");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

@aeppling
aeppling merged commit 23aae98 into rtk-ai:develop Jun 26, 2026
21 of 22 checks passed
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.

2 participants