fix(pr): handle --count gracefully in repositories with few commits - #197
fix(pr): handle --count gracefully in repositories with few commits#197rudra-chothe wants to merge 10 commits into
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@rudra-chothe is attempting to deploy a commit to the Gunjan Ghate's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 58 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR strengthens CLI validation and safety through remote URL verification, improves git staging to handle deletions and dotfiles, adds a detached-HEAD guard in the commit flow, refactors split-command AI flag handling for consistency, and includes test coverage for the new validators and staging behavior. ChangesCLI Validation, Staging, and Safety Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Readme.md (1)
319-319:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate staging step in the architecture diagram to match current implementation.
The diagram still says
git add ., but current behavior stages with-A(including deletions and dotfiles). This is now stale and can mislead contributors validating staging flow.Suggested doc fix
- DiffCheck -- No --> StageAll[Prompt & git add .] + DiffCheck -- No --> StageAll[Prompt & git add -A]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Readme.md` at line 319, The diagram label still shows "git add ." but the implementation stages with "git add -A"; update the architecture diagram text (e.g., the StageAll node that currently reads StageAll[Prompt & git add .]) to use "git add -A" (or "git add --all") so the diagram matches actual behavior and clearly indicates deletions and dotfiles are included.cli/index.js (1)
793-804:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim the interactive
remoteUrlbefore persisting it.Line 798 validates a trimmed value, but Line 803 uses the raw prompt value. Inputs like
' https://github.com/org/repo.git 'pass validation yet can be stored/used with whitespace.Suggested patch
- const { remoteUrl } = await inquirer.prompt([ + const { remoteUrl } = await inquirer.prompt([ { type: 'input', name: 'remoteUrl', message: 'Enter remote origin URL (e.g. https://github.com/user/repo.git):', validate: validateRemoteUrl } ]); + const normalizedRemoteUrl = remoteUrl.trim(); try { - await git.remote(['add', 'origin', remoteUrl]); - console.log(chalk.green(`✅ Remote origin set to ${remoteUrl}`)); + await git.remote(['add', 'origin', normalizedRemoteUrl]); + console.log(chalk.green(`✅ Remote origin set to ${normalizedRemoteUrl}`)); return true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/index.js` around lines 793 - 804, Prompted remoteUrl is validated on a trimmed value but the raw prompt result is used as-is; after receiving the prompt result (variable remoteUrl) trim it before using or persisting it (i.e., pass remoteUrl.trim() to git.remote(['add','origin', ...]) and to the console.log message) so stored/used remotes never contain leading/trailing whitespace; reference variables/functions: remoteUrl, validateRemoteUrl, git.remote(['add','origin', ...]).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/helpers/gitUtils.js`:
- Around line 27-33: The SSH validation is too restrictive: update sshScpPattern
and sshUrlPattern so they accept any user (not only "git") and match the
advertised formats "user@host:path" and "ssh://[user@]host[:port]/path";
specifically, modify sshScpPattern (currently named sshScpPattern) to allow a
generic username before the @ (e.g., alphanumeric plus common username chars)
and modify sshUrlPattern (currently named sshUrlPattern) to make the "user@"
portion optional (allowing ssh://host... and ssh://user@host...) while keeping
the existing host, optional port and path rules, then keep the same returned
error string.
In `@cli/index.js`:
- Around line 936-945: The code forces 'main' when opts.branch == false or
!hasCommits which can repoint main; instead use the repository's current branch
name from branchInfo (e.g., branchInfo.current or branchInfo.name) and check
that out rather than force-creating 'main'. Set branchName = branchInfo.current
|| 'main' and use a normal checkout of branchName (not git checkout -B main) so
you commit on the current branch; if branchInfo has no current name, fall back
to creating a new safe branch name (e.g., gg-work/${Date.now()}) or 'main' only
as last resort.
In `@cli/tests/stageAllFiles.test.js`:
- Around line 47-54: The test pre-stages the README deletion with execSync('git
rm --cached README.md'), so remove that pre-staging and let stageAllFiles() be
the only thing that stages deletions; specifically edit the test in
stageAllFiles.test.js to delete the execSync('git rm --cached README.md', {
stdio: 'ignore' }) line (or replace it with a command that only removes the
working file but does not stage, e.g., ensure the file is removed via
rm(join(process.cwd(), 'README.md'), { force: true }) before calling
stageAllFiles()), so that the call to stageAllFiles() is what's being tested for
staging deletions.
---
Outside diff comments:
In `@cli/index.js`:
- Around line 793-804: Prompted remoteUrl is validated on a trimmed value but
the raw prompt result is used as-is; after receiving the prompt result (variable
remoteUrl) trim it before using or persisting it (i.e., pass remoteUrl.trim() to
git.remote(['add','origin', ...]) and to the console.log message) so stored/used
remotes never contain leading/trailing whitespace; reference
variables/functions: remoteUrl, validateRemoteUrl, git.remote(['add','origin',
...]).
In `@Readme.md`:
- Line 319: The diagram label still shows "git add ." but the implementation
stages with "git add -A"; update the architecture diagram text (e.g., the
StageAll node that currently reads StageAll[Prompt & git add .]) to use "git add
-A" (or "git add --all") so the diagram matches actual behavior and clearly
indicates deletions and dotfiles are included.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb7c9fa3-60bf-44b6-9fe3-d5281e010c23
📒 Files selected for processing (7)
Readme.mdcli/commands/split.jscli/helpers/gitUtils.jscli/index.jscli/providers/gemini.jscli/tests/stageAllFiles.test.jscli/tests/validation.test.js
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@rudra-chothe looks good |
|
@rudra-chothe do tell where are the main code implementation exists apart from Docs and test files |
the main code changes are in |
2 participants
📝 Description
Fixes #165
The
gg prcommand usedHEAD~${count}..HEADto collect modified files. In repositories with fewer commits than the requested--count, this Gitrange is invalid and causes the command to fail — common in new or beginner test repositories.
This PR:
--countto the number of commits available in the repositorycli/tests/prCommitRange.test.js🚀 Type of Change
🧪 How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
npm test(fromcli/)fatal: ambiguous argument 'HEAD~5..HEAD'Requested 5 commits, but only 1 exist. Using all available commits.README.mdin Files ModifiedprCommitRange.test.js📸 Screenshots / Logs
Click to expand logs
```bash Requested 5 commits, but only 1 exist. Using all available commits. Analyzing last 1 commit... Generated PR Description: ## Summary This PR includes updates from recent commits. ## Changes Made ### Other Changes - docs: initial commit ## Files Modified - README.md ```Summary by CodeRabbit
Release Notes
Documentation
New Features
--no-branchto override).Bug Fixes
Tests