Skip to content

fix: report a missing gh or git binary as a clean error instead of a FileNotFoundError traceback - #155

Merged
2 commits merged into
mainfrom
fix/missing-cli-tools-error
Sep 27, 2026
Merged

2 commits merged into
mainfrom
fix/missing-cli-tools-error

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Summary

When gh (or git) is not on PATH, subprocess.run raises FileNotFoundError, which no caller expected: check_gh_auth (typed -> bool) raised instead of returning False, get_pr_state (catching only GitOperationError/JSONDecodeError) killed merge mid-loop, and split showed a raw traceback. Worse, once the wrappers convert that into a GitOperationError, clean and the re-split prompt swallow it per-PR as a warning, report "Cleanup complete" and delete the plan file while every PR and branch is left in place.

Now:

  • _run_gh and a shared _git helper (also used by a new run_git_raw for extract_diff / _get_base_file_content, which had their own subprocess.run) raise GitOperationError("'gh' is not installed or not on PATH")
  • require_tools pre-checks run up front in split (git, plus gh unless --dry-run; the PR-number/user:branch path always needs gh), execute, merge (which would otherwise skip every PR as a "fetch error" and exit 0), and — via a shared _require_cleanup_tools — before the confirm prompt in both clean and the re-split path, so a missing binary is reported instead of turning into a bogus "authentication failed" or a successful-looking cleanup

status intentionally still degrades to the cached state (read-only).

Test plan

  • TestMissingCliTools (wrappers), TestRequireTools, TestMissingCliToolsAreReportedFirst (split, dry-run of a local branch needs only git, dry-run of a PR number needs gh, execute, merge, clean keeps the plan, re-split cleanup in dry-run) — all fail on main
  • Review re-ran four PATH=/nonexistent repros (clean, split fix: clean deletes remote branches even when the local branch is gone and keeps the plan on failure #123 --dry-run, merge, split --dry-run re-split): clean error, exit 1, plan retained
  • 454 tests pass, ruff clean
  • Local review: 5/5 (after four fix→re-review rounds)

@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 85718b06-5f2d-43f3-97a6-318ed4c936e4

📥 Commits

Reviewing files that changed from the base of the PR and between 671a081 and 98d2004.

📒 Files selected for processing (10)
  • pr_split/cli.py
  • pr_split/diff_ops/parser.py
  • pr_split/diff_ops/reconstructor.py
  • pr_split/exceptions.py
  • pr_split/git_ops/branches.py
  • pr_split/git_ops/prs.py
  • tests/conftest.py
  • tests/test_cli_coverage.py
  • tests/test_git_branches.py
  • tests/test_git_prs_coverage.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Fold the FileNotFoundError handling into main's locale-pinned git
helpers; keep main's bytes-mode extract_diff/_get_base_file_content
(CRLF-safe) and catch a missing git there directly instead of routing
them through a text-mode run_git_raw. Run the tool pre-check before
main's new git/gh calls (remote-branch adoption, commit_exists, base-merged
lookup), and stub require_tools in conftest so tests do not depend on gh
being installed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
@vitali87 vitali87 closed this pull request by merging all changes into main in 8ca9e3b Sep 27, 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.

2 participants