feat: add --pr option to batch commands for git worktree support#5
feat: add --pr option to batch commands for git worktree support#5
Conversation
Batch commands (mark, reply, resolve, react) now accept --pr <url> and --owner/--repo/--number options for explicit PR specification instead of relying solely on auto-detection via `gh pr view`. This enables usage from git worktrees where auto-detection may fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Comment |
Add comprehensive CLAUDE.md file with: - Project overview and architecture documentation - Development commands and testing guidelines - Code style conventions and best practices - Git workflow with worktrees support - Documentation update planning rules Update .gitignore: - Add .worktrees/ directory for feature development isolation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 16-22: The PR number parsing currently uses parseInt which yields
NaN for invalid input and later gets treated as falsy in parsePRInfo, causing
silent fallback; update addPROptions to use a custom parser for the --number
option that converts the input to a number, verifies Number.isInteger and not
NaN, and throws a clear error (so the CLI fails fast) when validation fails, and
also add a defensive check in parsePRInfo to explicitly detect NaN/invalid
numbers from the parsed options and throw or surface an error instead of
treating it as 0; refer to addPROptions and parsePRInfo to locate the changes.
🧹 Nitpick comments (1)
CLAUDE.md (1)
199-203: Tighten phrasing: “CLI interface” → “CLI”.Minor redundancy tweak for readability.
✏️ Proposed wording tweak
-1. **Changes in API or CLI interface** - require README.md update: +1. **Changes in API or CLI** - require README.md update:
| function addPROptions(cmd: Command): Command { | ||
| return cmd | ||
| .option('--pr <url>', 'PR URL (auto-detects from current repo if omitted)') | ||
| .option('--owner <owner>', 'Repository owner') | ||
| .option('--repo <repo>', 'Repository name') | ||
| .option('--number <number>', 'PR number', parseInt); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and verify it exists
find . -name "cli.ts" -type fRepository: fixcik/gh-pr-threads
Length of output: 76
🏁 Script executed:
# Read the addPROptions function and surrounding context
cat -n src/cli.ts | head -50Repository: fixcik/gh-pr-threads
Length of output: 2644
🏁 Script executed:
# Search for where --number option is used or parsed downstream
rg "options\.number|prOptions" src/Repository: fixcik/gh-pr-threads
Length of output: 2089
🏁 Script executed:
# Check how Commander.js handles NaN values in option coercion
rg "parseInt" src/Repository: fixcik/gh-pr-threads
Length of output: 655
🏁 Script executed:
# Check parsePRInfo function to see how options.number is handled
cat -n src/utils/pr.tsRepository: fixcik/gh-pr-threads
Length of output: 2697
🏁 Script executed:
# Check prepareBatchCommandContext in shared.ts
cat -n src/commands/shared.ts | head -80Repository: fixcik/gh-pr-threads
Length of output: 2797
Validate --number to prevent silent fallback to auto-detect on invalid input.
Lines 21 and 44 use parseInt, which returns NaN for non-numeric input. In parsePRInfo (src/utils/pr.ts:64), NaN is falsy and becomes 0, causing the condition on line 66 to fail and silently fall back to detectCurrentPR(). This means an invalid PR number is ignored without warning, potentially targeting the wrong PR for modifications.
Suggested fix (fail fast on invalid PR numbers)
+function parsePrNumber(value: string): number {
+ const n = Number.parseInt(value, 10);
+ if (!Number.isInteger(n) || n <= 0) {
+ throw new Error('PR number must be a positive integer');
+ }
+ return n;
+}
+
function addPROptions(cmd: Command): Command {
return cmd
.option('--pr <url>', 'PR URL (auto-detects from current repo if omitted)')
.option('--owner <owner>', 'Repository owner')
.option('--repo <repo>', 'Repository name')
- .option('--number <number>', 'PR number', parseInt);
+ .option('--number <number>', 'PR number', parsePrNumber);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function addPROptions(cmd: Command): Command { | |
| return cmd | |
| .option('--pr <url>', 'PR URL (auto-detects from current repo if omitted)') | |
| .option('--owner <owner>', 'Repository owner') | |
| .option('--repo <repo>', 'Repository name') | |
| .option('--number <number>', 'PR number', parseInt); | |
| } | |
| function parsePrNumber(value: string): number { | |
| const n = Number.parseInt(value, 10); | |
| if (!Number.isInteger(n) || n <= 0) { | |
| throw new Error('PR number must be a positive integer'); | |
| } | |
| return n; | |
| } | |
| function addPROptions(cmd: Command): Command { | |
| return cmd | |
| .option('--pr <url>', 'PR URL (auto-detects from current repo if omitted)') | |
| .option('--owner <owner>', 'Repository owner') | |
| .option('--repo <repo>', 'Repository name') | |
| .option('--number <number>', 'PR number', parsePrNumber); | |
| } |
🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 16 - 22, The PR number parsing currently uses
parseInt which yields NaN for invalid input and later gets treated as falsy in
parsePRInfo, causing silent fallback; update addPROptions to use a custom parser
for the --number option that converts the input to a number, verifies
Number.isInteger and not NaN, and throws a clear error (so the CLI fails fast)
when validation fails, and also add a defensive check in parsePRInfo to
explicitly detect NaN/invalid numbers from the parsed options and throw or
surface an error instead of treating it as 0; refer to addPROptions and
parsePRInfo to locate the changes.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Instead of silently falling back to auto-detect when --number receives non-numeric input (parseInt returns NaN → treated as falsy), now exits with a clear error message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
✅ Created PR with unit tests: #6 |
1 participant
Summary
--pr <url>and--owner/--repo/--numberoptions to all batch commands (mark,reply,resolve,react)gh pr viewmay failaddPROptions()CLI helper to avoid duplication across 4 commandsTest plan
npm run checkpasses (typecheck + lint + 155 tests)npm run buildsucceedsresolve --help/mark --helpshow--pr,--owner,--repo,--numberoptionsgh-pr-threads resolve <id> --pr <url>from a non-repo directorySummary by CodeRabbit
New Features
Documentation
Chores