feat(commands): add /pr-review command for batch PR review with worktrees#225
Conversation
…rees Add Claude command for processing multiple PR review comments in parallel: - .claude/commands/pr-review.md: Main command documentation with usage examples - .claude/commands/batch-pr-review.md: Detailed workflow specification - scripts/Invoke-BatchPRReview.ps1: PowerShell helper for worktree management Features: - Process single or multiple PRs (comma-separated or all-open) - Optional parallel execution with git worktrees for isolation - Automatic cleanup: commit uncommitted changes, push, remove worktrees - Progress monitoring and summary table output - Error recovery for common failure scenarios Usage: /pr-review 194 # Single PR /pr-review 53,141,143 --parallel # Multiple PRs with worktrees /pr-review all-open --parallel # All open PRs needing review Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new PowerShell tool and tests to manage batch PR reviews via per-PR git worktrees, plus a pr-review command workflow doc, session/handoff and slash-command reference docs; removes a generated PR-225 comments map file. (36 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script as Invoke-BatchPRReview.ps1
participant GH as GitHub CLI (gh)
participant Git as git
User->>Script: run with PRNumbers + Operation
Script->>GH: gh pr view --json headRefName (Get-PRBranch)
GH-->>Script: branch name or error
alt branch resolved
Script->>Git: git fetch && git worktree add <path> <branch> (New-PRWorktree)
Git-->>Script: created / error
else branch unresolved
Script-->>User: error + guidance
end
Script->>Git: git status / rev-parse / log (Get-WorktreeStatus)
Git-->>Script: status details
alt uncommitted or unpushed
Script->>Git: git add && git commit && git push (Push-WorktreeChanges)
Git-->>Script: success / failure
end
alt Cleanup requested
Script->>Git: check upstream/existence
alt safe or Force
Script->>Git: git worktree remove <path> (Remove-PRWorktree)
Git-->>Script: removed
else abort
Script-->>User: abort cleanup
end
end
Script-->>User: operation summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
Spec-to-Implementation ValidationWarning No spec references found This PR does not reference any specifications (REQ-, DESIGN-, TASK-*, or linked issues). How to add spec referencesAdd spec references to your PR description to enable traceability:
Spec Requirement by PR Type:
See PR template for full guidance. Powered by AI Spec Validator - Validation skipped - no specs found |
AI Quality Gate ReviewCaution ❌ Final Verdict: CRITICAL_FAIL WalkthroughThis PR was reviewed by six AI agents in parallel, analyzing different aspects of the changes:
Review Summary
Architect Review DetailsDesign Quality Assessment
Overall Design Score: 4/5 Architectural Concerns
Breaking Change Assessment
Technical Debt Analysis
The implementation is clean. Minor debt: documentation references Python pseudocode that will never execute (lines 24-28, 60-75 in pr-review.md). This creates confusion about whether Claude interprets these as executable. ADR Assessment
Alignment Check
Recommendations
VerdictAnalyst Review DetailsNow let me examine the existing patterns for Claude commands and PowerShell scripts in this repository to understand consistency requirements. Now I have enough context. Let me analyze this PR: PR Review Analysis: feat(commands): add /pr-review command for batch PR review with worktreesCode Quality Score
Overall: 3.5/5 Impact Assessment
Findings
Recommendations
VerdictRoadmap Review DetailsStrategic Alignment Assessment
Feature Completeness
Impact Analysis
Concerns
Recommendations
VerdictDevOps Review DetailsPipeline Impact Assessment
CI/CD Quality Checks
Findings
Template Assessment
Automation Opportunities
Recommendations
QA Review DetailsNow I have the full context. Let me analyze the PR changes and produce a QA review. QA Review: PR Review Command with Batch WorktreesTest Coverage Assessment
Quality Concerns
Regression Risk Assessment
Edge Cases NOT Tested
Functions Requiring Tests
Required Actions Before Merge
Security Review DetailsSecurity Review: PR Review Command with Batch Worktree SupportFindings
Analysis DetailsCommand Injection Risk (Medium)Location: git worktree add $worktreePath $branch 2>&1The
Risk Score: 4/10 (Low likelihood due to GitHub API as data source, Medium impact if exploited) Force Push Behavior (Low)The Path Disclosure (Low)Internal file system paths appear in user-facing output. This is acceptable for CLI tools but could reveal directory structure to logs. Positive Security Patterns
Recommendations
# Add validation in Get-PRBranch
if ($prInfo.headRefName -match '[;&|`$]') {
Write-Error "Invalid branch name contains shell metacharacters"
return $null
}
VerdictRun Details
Powered by AI Quality Gate - View Workflow |
There was a problem hiding this comment.
Code Review
This pull request introduces a new /pr-review command for batch processing of pull request reviews using git worktrees, along with supporting documentation and a PowerShell script for worktree management. The changes are well-structured and the documentation is clear.
My review focuses on adherence to the repository's comprehensive style guide, particularly for PowerShell scripting and Markdown standards. I've identified several areas for improvement:
- PowerShell Scripting: The script
Invoke-BatchPRReview.ps1has some style guide violations, including the use of non-approved verbs, improper use ofWrite-Host, and a lack of-WhatIfsupport for state-changing operations. More critically, there is a potential path traversal vulnerability that needs to be addressed. - Markdown Documentation: The new command documentation files are missing language identifiers on some code blocks, which is required by the style guide.
I've provided specific suggestions to address these points, which will improve the script's safety, maintainability, and compliance with repository standards.
I am having trouble creating individual review comments. Click here to see my feedback.
scripts/Invoke-BatchPRReview.ps1 (31)
This script performs state-modifying operations like creating/removing worktrees and pushing commits, but it doesn't support the -WhatIf common parameter. This is a crucial safety feature in PowerShell to allow users to preview changes before they are made. According to the repository style guide, scripts that modify state should support -WhatIf.
[CmdletBinding(SupportsShouldProcess = $true)]
References
- The style guide (line 128) states: "Support -WhatIf and -Verbose parameters for scripts that modify state." (link)
scripts/Invoke-BatchPRReview.ps1 (40-41)
The $WorktreeRoot parameter is used to construct file paths without proper validation. This could lead to a path traversal vulnerability (CWE-22) if a user provides a malicious path like ../../... The style guide requires validation of all external inputs.
[ValidateScript({ Test-Path -Path $_ -PathType Container })]
[string]$WorktreeRoot,
.claude/commands/batch-pr-review.md (7)
This code block is missing a language identifier, which prevents syntax highlighting and can make the code harder to read. Per the style guide, all fenced code blocks should have a language identifier.
```bash
References
- The style guide requires all fenced code blocks to have a language identifier for syntax highlighting. (link)
.claude/commands/batch-pr-review.md (94)
This code block, which illustrates a directory structure, is missing a language identifier. This can lead to inconsistent rendering. The style guide requires language identifiers for all fenced code blocks.
```text
References
- The style guide requires all fenced code blocks to have a language identifier for syntax highlighting. (link)
.claude/commands/pr-review.md (7)
This code block is missing a language identifier, which prevents syntax highlighting and can make the code harder to read. The style guide requires language identifiers for all fenced code blocks.
```bash
References
- The style guide requires all fenced code blocks to have a language identifier for syntax highlighting. (link)
scripts/Invoke-BatchPRReview.ps1 (79)
This script uses Write-Host for status messages. The style guide recommends using Write-Verbose for progress/debug information and Write-Warning for non-terminating issues. This allows users to control script verbosity. This specific message seems like a warning. This feedback applies to other Write-Host calls in the script as well; they should be converted to Write-Verbose for progress messages.
Write-Warning "Worktree already exists: $worktreePath"
References
- The style guide (lines 70-86) specifies using
Write-Error,Write-Warning, andWrite-Verbosefor script output, and discouragesWrite-Hostfor debugging or status messages. (link)
scripts/Invoke-BatchPRReview.ps1 (173)
The function Push-WorktreeChanges uses the verb Push, which is not on the approved list in the repository's PowerShell style guide. To maintain consistency, it should be renamed.
function Invoke-WorktreeSync {
References
- The style guide (lines 18-28) provides a list of approved PowerShell verbs.
Pushis not on this list. (link)
There was a problem hiding this comment.
Pull request overview
This PR adds a new /pr-review Claude command for processing multiple PR review comments in parallel using git worktrees for isolation. The feature enables batch processing of PRs with automatic cleanup and progress monitoring.
- Adds PowerShell helper script for git worktree management (setup, status, cleanup)
- Provides two command documentation files explaining single/batch PR review workflows
- Implements parallel execution support using git worktrees for isolated PR processing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
scripts/Invoke-BatchPRReview.ps1 |
PowerShell helper script that manages git worktrees for parallel PR review operations, including creation, status monitoring, and cleanup with automatic commit/push |
.claude/commands/pr-review.md |
Main command documentation defining the /pr-review command with usage examples for single and batch PR processing, including sequential and parallel execution modes |
.claude/commands/batch-pr-review.md |
Detailed workflow specification for the /batch-pr-review command covering setup, parallel execution, monitoring, cleanup, and error handling phases |
…uirements) Add 5 BLOCKING constraints for parallel worktree execution: 1. Worktree Isolation - ALL changes contained within assigned worktree 2. Working Directory - MUST cd into worktree before file operations 3. Path Validation - paths relative to worktree root only 4. Git Operations - execute from within worktree directory 5. Verification Gate - check main repo for unexpected changes before cleanup Violation of these constraints causes work leakage between PRs and merge conflicts.
- Clarify pr-comment-responder skill is optional (comment 2638201580, 2638201614) - Improve warning message with actionable guidance (comment 2638201588) - Add $LASTEXITCODE checks after git operations (comment 2638201593) - Use default push behavior instead of hardcoded 'origin' (comment 2638201619) - Add language specifier to usage code block (markdownlint MD040) Comment 2638201604 dismissed as false positive - commit message already contains "session" as shown in line 193. Comment 2638201612 (missing tests) deferred to follow-up issue. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document all 7 Copilot review comments and their resolutions: - 4 fixed (documentation, warning message, exit codes, remote) - 1 deferred (test coverage) - 1 dismissed as false positive (commit message format) - 1 marked as duplicate Generated with [Claude Code](https://claude.com/claude-code)
Resolves QA issues from CI run 20418510304:
**QA-1: LASTEXITCODE check in Get-PRBranch**
- Changed from suppressing stderr (2>$null) to capturing output (2>&1)
- Added explicit $LASTEXITCODE check before processing JSON
- Added secondary validation for missing headRefName
**QA-2: Upstream check in Get-WorktreeStatus**
- Added `git rev-parse --abbrev-ref '@{u}'` before git log "@{u}.."
- Prevents error when branch has no upstream configured
**QA-3: Pester test coverage**
- Created scripts/tests/Invoke-BatchPRReview.Tests.ps1
- 23 tests covering: parameter validation, error handling verification,
function definitions, code quality, and integration
**Additional fix: PowerShell variable syntax**
- Changed "PR #$PRNumber:" to "PR #${PRNumber}:" throughout
- Fixes parse error where colon was interpreted as drive reference
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.claude/commands/pr-review.md (1)
99-101: Documentation showsgit push origin {branch}but script usesgit push.This was flagged in a past review. The script correctly uses
git pushwithout hardcoded remote. Documentation example at line 100 still shows the old pattern.Fix documentation to match implementation
- git push origin {branch} + git push
🧹 Nitpick comments (2)
scripts/tests/Invoke-BatchPRReview.Tests.ps1 (1)
13-29: Missing AfterAll block for final cleanup.Per coding guidelines, Pester tests should use AfterAll to clean temp directories after all tests complete. AfterEach handles per-test cleanup, but if a test throws before AfterEach runs, artifacts may persist.
Add AfterAll block
BeforeAll { $Script:ScriptPath = Join-Path $PSScriptRoot "..\Invoke-BatchPRReview.ps1" $Script:TestDir = Join-Path $TestDrive "worktree-test" } + +AfterAll { + if (Test-Path $Script:TestDir) { + Remove-Item -Path $Script:TestDir -Recurse -Force -ErrorAction SilentlyContinue + } +}scripts/Invoke-BatchPRReview.ps1 (1)
93-96: Write-Error with ErrorActionPreference='Stop' terminates before return.Line 94 uses
Write-Errorwhich terminates execution when$ErrorActionPreference = 'Stop'. Line 95'sreturn $falsenever runs. This isn't critical since the script stops on error, but the return statement is dead code.Consider using Write-Warning and returning false for graceful continuation, or remove the unreachable return:
Option: Use Write-Warning for graceful failure
if ($LASTEXITCODE -ne 0) { - Write-Error "Failed to create worktree for PR #$PRNumber" + Write-Warning "Failed to create worktree for PR #$PRNumber" return $false }
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.agents/pr-comments/PR-225/comments.md.claude/commands/pr-review.mdscripts/Invoke-BatchPRReview.ps1scripts/tests/Invoke-BatchPRReview.Tests.ps1
🧰 Additional context used
📓 Path-based instructions (42)
**/.agents/**/*.md
📄 CodeRabbit inference engine (.agents/governance/interview-response-template.md)
Primary deliverables from agents should be saved to
.agents/[category]/[pattern].mdwith naming convention[PREFIX]-NNN-[description].mdSingle-source agent files should use frontmatter markers to delineate platform-specific sections for VS Code and Copilot CLI variants
Maintain artifact synchronization markers in tracking files (.md) with status indicators ([COMPLETE], [RESOLVED], [VERIFIED]) and timestamps to document completion and verification of work
Files:
.agents/pr-comments/PR-225/comments.md
.agents/**/*.{md,yml,yaml,json}
📄 CodeRabbit inference engine (.agents/critique/001-agent-templating-critique.md)
For agent platform files, evaluate whether near-identical variants (99%+ overlap) can be consolidated with conditional configuration rather than maintaining separate files
Files:
.agents/pr-comments/PR-225/comments.md
.agents/**/*.md
📄 CodeRabbit inference engine (.agents/retrospective/pr43-coderabbit-root-cause-analysis.md)
.agents/**/*.md: Use PREFIX-NNN naming convention (e.g., EPIC-001, CRITIQUE-001) for sequenced artifacts and type-prefixed naming (e.g., prd-, tasks-) for non-sequenced artifacts
Normalize all file paths in markdown documents to be repository-relative before committing, removing absolute machine-specific paths
.agents/**/*.md: Session logs and documentation must include Phase checklist verification (Phase 1-3 protocol compliance including agent activation, instruction reading, handoff file updates, and session logging)
Session logs must document Session ID, date, agent name, and branch information in a standardized header formatAll artifact files in .agents/ must be in Markdown format
Document analysis recommendations with specific rationale when adding new governance documents like PROJECT-CONSTRAINTS.md
Maintain debugging skills documentation in
.agents/directoryDocument implementation notes explaining deviations from user prompts or decisions made during development (e.g., using plural form for directory names)
Run markdown lint on all generated artifacts before completing a session
Files:
.agents/pr-comments/PR-225/comments.md
**/.agents/pr-comments/**/*.md
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-20-pr-147-comment-2637248710-failure.md)
**/.agents/pr-comments/**/*.md: Update artifact status atomically with fix application before API calls - immediately update tasks.md and comments.md status fields when implementing a fix, before posting follow-up comments or resolving threads
Check artifact state matches API state using diff before resolution - verify that artifact status (tasks.md, comments.md) matches intended GitHub API state changes before calling resolveThread mutation or closing threads
Synchronize artifact state with every state change, not deferred to session end - update corresponding tracking artifacts immediately after any work state change (fix applied, comment posted, thread resolved), treating artifacts as real-time source of truth
Files:
.agents/pr-comments/PR-225/comments.md
.agents/**
⚙️ CodeRabbit configuration file
Agent configuration files. Only flag security issues or broken cross-references. Ignore style, formatting, and structure.
Files:
.agents/pr-comments/PR-225/comments.md
scripts/**/*.ps1
📄 CodeRabbit inference engine (.agents/security/infrastructure-file-patterns.md)
PowerShell scripts in scripts directory (
scripts/**/*.ps1) should trigger security agent review due to high security implicationsValidation scripts belong in
scripts/directory; may duplicate to.agents/utilities/for agent accessAll scripts must avoid credential handling and should not store or process sensitive authentication information
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-15-cva-install-scripts.md)
Extract environment variations to .psd1 data files, keeping logic generic rather than hardcoding configuration in scripts
Convert PathInfo objects to string representations when passing to string-typed parameters to prevent type mismatch errors
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
scripts/**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-15-documentation-gap.md)
Create module-specific README documentation for PowerShell scripts and modules, including parameter documentation and usage examples
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,psd1}?(@(test|spec))
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-15-install-scripts-session.md)
Use BeforeAll blocks for all variable initialization in Pester 5.x tests; avoid variable assignments outside BeforeAll during Discovery phase
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.ps1
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-15-instruction-files-gap.md)
User instruction files should be excluded from agent file copying during installation to prevent them from being treated as agent files
Use
.Pathproperty to extract string value from PathInfo objects returned by Resolve-Path in PowerShell (e.g.,(Resolve-Path $Path).Pathinstead ofResolve-Path $Path)PowerShell scripts should normalize output line endings (convert CRLF to LF) when output is intended to be processed by shell commands like grep to ensure cross-platform compatibility
Use
-cmatchinstead of-matchwhen pattern requires case-sensitive matching in PowerShell validation scripts (e.g., EPIC vs epic for naming conventions)
**/*.ps1: In PowerShell script scope, use exit with explicit exit codes instead of return; return exits with code 0 regardless of boolean value
Document the bash-PowerShell exit code contract explicitly in PowerShell script comments: exit code 0 means success, non-zero means failureWhen combining ShouldProcess with PassThru in PowerShell cmdlets: always provide explicit return value in else branch when ShouldProcess returns false
Before executing any PowerShell generation script, audit its code for alignment with known user requirements and identify dead/unused functions
**/*.ps1: Create regression tests for PowerShell scripts covering wildcard character detection, path resolution correctness, and edge cases when fixing path handling logic
In PowerShell, validate wildcard detection logic by distinguishing between literal wildcard characters (?and*) vs. actual wildcard patterns, using the-likeoperator correctly with character class escaping (e.g.,*[?]*instead of*?*)
**/*.ps1: Before implementing features search for pre-existing test coverage
When tests pre-exist run them first to understand feature expectations
Use deep cloning to preserve source data integrity when transforming configurations
Use regex with exact match anchors (^...$) to prevent partial matches in configuration transformations
Up...
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/{install,*.ps1,*.json}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-15-instruction-files-gap.md)
Verify that all files referenced in installer configuration (InstructionsFile, SourceDir, etc.) exist in their expected locations before release
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
{install.ps1,build/**/*.{ps1,sh},scripts/**/*.{ps1,sh}}
📄 CodeRabbit inference engine (.agents/roadmap/epic-agent-consolidation.md)
Build script must generate platform-specific YAML frontmatter for VS Code and Copilot CLI variants at build time
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/scripts/tests/**/*.ps1
📄 CodeRabbit inference engine (.agents/planning/PRD-visual-studio-install-support.md)
Add unit tests in 'scripts/tests/' directory for new VisualStudio configuration and installation functions
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.Tests.ps1
📄 CodeRabbit inference engine (.agents/qa/047-test-pollution-fix-verification.md)
**/*.Tests.ps1: AddBeforeEachcleanup block to Pester test contexts to prevent test pollution between tests
UseBeforeAllblock in Pester tests to create isolated temp directories for test execution
UseAfterAllblock in Pester tests to clean up and remove temp directories after all tests complete
Clean temp test directory usingGet-ChildItem -Recurse | Remove-Item -Forcepattern with-ErrorAction SilentlyContinueto safely remove test artifacts
Verify test isolation through file count assertions in Pester tests to detect if previous test files remainPre-compute collections before Pester hashtable initialization; pipeline operators inside hashtable index expressions are not supported
**/*.Tests.ps1: Include $LASTEXITCODE assertion tests in Pester test suites for PowerShell scripts invoked from bash hooks to validate exit code contracts
Verify that Pester tests for PowerShell scripts check both internal logic and external exit code behavior when scripts are executed from bash contexts
**/*.Tests.ps1: PowerShell cmdlets with 2+ switch parameters require combination testing: n parameters = n individual + C(n,2) pair tests minimum
Integration tests must include first-time setup scenario where config files don't exist yet
Organize PowerShell Pester test files with contexts for: Basic Functionality, Error Handling, Parameter Combinations, and Edge Cases
**/*.Tests.ps1: Use Pester test isolation pattern with BeforeAll, AfterAll, and BeforeEach blocks when creating file system-based tests in PowerShell
Achieve 100% branch coverage and 80%+ edge case coverage for wildcard detection and path resolution tests, with execution time under 5 seconds
For file system-based tests in PowerShell, use temp directory isolation with proper cleanup in BeforeAll/AfterAll blocks to prevent test pollution and ensure test repeatabilityPester test files must follow the AAA (Arrange-Act-Assert) pattern and use mocking for dependencies with behavior verification as docum...
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.{md,json,yml,yaml,ps1,sh,bash,toml,ini,cfg,config,txt}
📄 CodeRabbit inference engine (.agents/retrospective/phase1-remediation-pr43.md)
Use relative paths only in documentation and configuration files; avoid absolute paths (e.g., C:\ on Windows) to prevent environment contamination and maintain cross-platform portability
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/tests/**/*.Tests.ps1
📄 CodeRabbit inference engine (.agents/retrospective/pr-feedback-remediation.md)
Create Pester tests following 5.x structure with test cases covering Pattern Detection, File Filtering, Exit Code Behavior, Reporting, and Edge Cases for validation scripts
Create Pester tests for error handling paths in skill scripts (e.g.,
skill-*.ps1) to verify graceful failure handling
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.agents/steering/powershell-patterns.md)
**/*.{ps1,psm1}: Use approved verbs for PowerShell functions (Get-, Set-, New-, Remove-, etc.)
Implement parameter conventions and validation in PowerShell functions
Use proper error handling with ErrorActionPreference, try/catch blocks in PowerShell
Follow PowerShell pipeline usage and best practices for function design
Use comment-based help standards in PowerShell functions
Implement advanced function templates with CmdletBinding attribute
Use parameter validation attributes in PowerShell functions
Handle pipeline input properly in PowerShell cmdlets
Implement progress reporting for long-running PowerShell operations
Use proper configuration management patterns in PowerShell
Avoid using aliases in PowerShell scripts
Never suppress errors without proper error handling in PowerShell
Avoid hardcoded paths in PowerShell scriptsUse
.claude/skills/github/scripts for GitHub operations. NEVER use rawghcommands directly.
**/*.{ps1,psm1}: All PowerShell scripts (.ps1, .psm1) must include corresponding Pester test files (.Tests.ps1) with test coverage
PowerShell modules should use type safety, object pipelines, and structured error handling rather than string-based output
**/*.{ps1,psm1}: Use PowerShell regex pattern^[a-zA-Z0-9][a-zA-Z0-9 _\-\.]{0,48}[a-zA-Z0-9]?$for GitHub label validation to allow spaces in labels
All GitHub label validation must reject empty strings, accept single characters only if alphanumeric, reject newlines and tabs, enforce maximum 50 character limit per GitHub's specifications, and allow spaces in label names
Document all regex edge cases including empty string rejection, single character handling, newline/tab rejection, and character limit rationale in PowerShell code commentsMUST use PowerShell for all scripting (.ps1, .psm1)
**/*.{ps1,psm1}: Sanitize all values written to $env:GITHUB_OUTPUT by escaping newlines and special characters to prevent injection of arbitrary output variables
Quote label names in gh commands or...
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,yml,yaml,md}
📄 CodeRabbit inference engine (.agents/analysis/003-session-protocol-skill-gate.md)
Validate skill availability before implementing GitHub operations - Check if
.claude/skills/github/directory exists and contains the required capability; list available GitHub skill scripts; read theskill-usage-mandatorymemory; document available skills in session log; use existing skills if they exist rather than writing inline code
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,yml,yaml}
📄 CodeRabbit inference engine (.agents/analysis/003-session-protocol-skill-gate.md)
NEVER use raw
ghcommands (likegh pr view,gh issue create,gh api) when a Claude skill exists for that functionality - Always use the tested skill scripts from.claude/skills/github/instead of writing inline commands
**/*.{ps1,yml,yaml}: Use atomicgh label create $label --forceoperation instead of separate check + create steps to prevent TOCTOU race conditions
Implement all 4 mandatory Phase 1 security hardening conditions (label creation atomicity, auth check always-run, GITHUB_OUTPUT sanitization, debug file path randomization) before merge approval
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/{ai-issue-triage.yml,*.ps1}
📄 CodeRabbit inference engine (.agents/analysis/004-pr-60-gap-coverage-validation.md)
Add explicit exit code checks for npm and GitHub CLI commands instead of using
|| trueerror suppression patterns
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{yml,yaml,ps1}
📄 CodeRabbit inference engine (.agents/analysis/004-pr-60-gap-coverage-validation.md)
Generate unique temporary directories for each workflow run using timestamped or UUID-based naming to prevent directory collision
Ensure authentication checks always run and are not skipped by conditional logic in diagnostics mode - remove or restructure conditions that bypass authentication
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{Tests.ps1,yml}
📄 CodeRabbit inference engine (.agents/critique/004-pr-60-remediation-final-validation.md)
All test cases must pass before PR merge; implement blocking acceptance gate for 7 mandatory Pester injection tests (5 for labels, 2 for milestone) as required criteria
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.{ps1,psm1,yml,yaml}
📄 CodeRabbit inference engine (.agents/planning/PR-60/006-agent-validation-sign-offs.md)
Align all PowerShell implementations with ADR-005 (PowerShell-only requirement) and ADR-006 (thin workflows principle) to maintain architecture coherence
Add rate limiting for label operations to prevent DoS of GitHub API quota by capping the maximum number of labels per issue
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{Tests.ps1,yml,yaml}
📄 CodeRabbit inference engine (.agents/planning/PR-60/006-agent-validation-sign-offs.md)
Establish daily test execution gate requiring all Pester tests to PASS with exit code 0 before proceeding to next implementation phase
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
{**/*.yml,**/*.ps1}
📄 CodeRabbit inference engine (.agents/planning/PR-60/007-phase-1-detailed-schedule.md)
Verify exit code after each critical command (gh issue edit, npm install) in PowerShell workflows and scripts
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
{**/*.ps1,**/*.yml}
📄 CodeRabbit inference engine (.agents/planning/PR-60/007-phase-1-detailed-schedule.md)
Remove or justify all '|| true' patterns in PowerShell scripts and workflows to prevent silent failures
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
{**/*.psm1,**/*.ps1}
📄 CodeRabbit inference engine (.agents/planning/PR-60/007-phase-1-detailed-schedule.md)
Replace 'exit 1' with 'throw' statements in PowerShell modules and scripts for proper exception handling
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.Tests.{ps1,sh}
📄 CodeRabbit inference engine (.agents/planning/pr-60-implementation-plan.md)
Add test cases for security vulnerabilities including code injection prevention scenarios, malformed input handling, and edge cases in critical paths
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.{md,js,ts,tsx,jsx,json,yaml,yml,sh,ps1}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-18-serena-memory-reference-migration.md)
Search entire codebase for pattern before migration to identify all references
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,psm1,sh,bash}
📄 CodeRabbit inference engine (.agents/security/SR-PR60-implementation-review.md)
Use unique, secured temporary file paths with random identifiers and cleanup logic instead of hardcoded predictable paths like /tmp/categorize-output.txt
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*Tests.ps1
📄 CodeRabbit inference engine (.agents/sessions/2025-12-18-session-28-pr-60-qa-strategy.md)
Use Mock with endpoint pattern matching and mode switching (Success, ApiError, NotFound, Unauthenticated) for realistic gh CLI API mocking across test files
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
scripts/tests/*.Tests.ps1
📄 CodeRabbit inference engine (scripts/AGENTS.md)
All PowerShell test files must use Pester framework and cover the corresponding script/module functions
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1
**/*.{js,ts,ps1,py,json,yaml,yml,md}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-19-skill-extraction-summary.md)
Use identical syntax for all instances when migrating patterns to maintain consistency
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*{.ps1,github,reaction}*
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-20-pr-94-acknowledgment-failure.md)
PowerShell script failure requires immediate gh CLI fallback attempt (dual-path tooling for GitHub operations)
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,js,ts,tsx,jsx,py}
📄 CodeRabbit inference engine (.agents/sessions/2025-12-20-session-43-qa-validation-pr147.md)
Use regex patterns with single-line mode flag
(?s)for multi-line matching in YAML comment extraction
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.@(js|ts|py|go|java|rb|php|ps1|regex)
📄 CodeRabbit inference engine (.agents/critique/047-pr-212-retrospective-critique.md)
Use
([pattern])?instead of[pattern]?for optional trailing groups in regex to ensure correct optional behavior and prevent special character bypass
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,js,ts,py}
📄 CodeRabbit inference engine (.agents/retrospective/2025-12-20-pr-212-comment-response.md)
Use atomic optional groups
([pattern])?$instead of[pattern]?$for optional trailing characters in regex validation to prevent special character bypass
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,ts,tsx,js}
📄 CodeRabbit inference engine (.agents/sessions/2025-12-20-session-01.md)
Verify regex validation patterns in security remediation code
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,sh,bash}
📄 CodeRabbit inference engine (.agents/sessions/2025-12-21-session-53-pr212-security-continuation.md)
Transform validation error messages into 5-word activation prompts that trigger correct behavior
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,psm1,psd1,yml,yaml,sh}
📄 CodeRabbit inference engine (.agents/HANDOFF.md)
Use PowerShell exclusively for implementation in GitHub Actions workflows, scripts, and automation; avoid bash unless absolutely necessary for CI/CD platform compatibility
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
**/*.{ps1,psm1,yml}
📄 CodeRabbit inference engine (.agents/sessions/2025-12-21-session-57-pr-222-review-response.md)
Include the
-Forceflag when usingImport-Moduleto ensure module reload and prevent stale module state
Files:
scripts/tests/Invoke-BatchPRReview.Tests.ps1scripts/Invoke-BatchPRReview.ps1
scripts/*.ps1
📄 CodeRabbit inference engine (.agents/sessions/2025-12-18-session-21-check-skill-exists.md)
PowerShell scripts must support -Operation, -Action, and -ListAvailable parameters with ValidateSet constraint for valid operations
Files:
scripts/Invoke-BatchPRReview.ps1
🔍 Remote MCP
Since I'm having difficulty locating the exact repository, let me use DeepWiki to explore common repository patterns related to Claude commands and PR review automation. However, I note that the context provided contains comprehensive summaries already. Let me search for relevant best practices and patterns related to the technologies used:Excellent! Now I have comprehensive context about git worktree best practices and PowerShell error handling. Let me compile the findings.
Summary of Relevant Context for PR #225 Review
Based on my research, here is additional context relevant to reviewing this PR:
Git Worktree Best Practices and Architecture
Git worktree lets you attach extra working directories that share the same .git store, each checked out at a different branch or commit. A branch can be checked out in only one worktree at a time. The PR's implementation aligns with this constraint by using structured worktree management functions.
Best practice involves organizing worktrees in a structured hierarchy like worktrees/feature/, worktrees/bugfix/, worktrees/review/ for different purposes. The PR's approach of per-PR worktrees follows this pattern.
Key isolation benefits per worktree design:
- Each worktree keeps its own working files like node_modules, builds, and caches. The repository data like commits, branches, tags and stashes, is shared across them all.
PowerShell Error Handling - Critical for this PR
The PR extensively implements error checking via $LASTEXITCODE. This is correct approach for external commands:
To trap exit code from external processes, utilize the $LastExitCode PowerShell variable. When the launched process exits, PowerShell will write the exit code directly to $LastExitCode. In most cases an exit code of 0 means success, and 1 or greater indicates a failure.
Critical caveat for git commands: When you need to call an external executable, most of the time you'll get the best results by checking $LASTEXITCODE for status information. However, $LASTEXITCODE is not set if you pipe the output of the native command to a cmdlet. This is important if the PR's script uses piped commands.
PowerShell-specific behavior to note: When you redirect stderr to stdout using 2>&1, in PowerShell this merge changes how stderr is handled for native commands: Native stderr gets converted into PowerShell ErrorRecord objects.
Batch PR Review Workflow Patterns
A common pattern with GitHub CLI is creating PR-specific worktrees: branch=$(gh pr view "$pr" --json headRefName -q .headRefName); git fetch "$remote" "$branch"; git worktree add "../$branch" "$branch"
If there are uncommitted changes in a particular folder, Git will not allow you to remove it by default. You can override this restriction by using the --force option. The PR's Remove-PRWorktree function correctly implements force handling.
Review Quality Considerations for PR #225
Based on best practices, key review areas should include:
- Worktree isolation enforcement - Verify the PR implements safeguards against concurrent branch checkouts across worktrees
- LASTEXITCODE checks - Confirm all git operations (add, commit, push) have explicit exit code validation immediately after the command
- Error recovery paths - Test cleanup behavior when errors occur mid-operation
- Upstream branch validation - The PR mentions checking upstream existence before querying status, which aligns with avoiding detached HEAD state issues
- Test coverage - The 23 Pester tests should cover edge cases like malformed PR numbers, missing branches, uncommitted changes
[::web_search::]
🔇 Additional comments (12)
scripts/tests/Invoke-BatchPRReview.Tests.ps1 (3)
63-111: Structural verification approach is acceptable.These tests verify implementation patterns via regex rather than behavior. This is a pragmatic approach for ensuring error handling code exists. No issues here.
113-125: Tests cover mandatory parameter validation correctly.
178-203: Integration test properly uses try/finally and skip condition.Good pattern: conditional skip for git availability, proper cleanup with try/finally. Test validates graceful handling of non-existent PRs.
.claude/commands/pr-review.md (2)
176-243: Constraints section documents isolation requirements well.The five blocking constraints cover worktree isolation, working directory, path validation, git operations, and verification. Clear guidance for parallel execution safety.
245-256: Prerequisites and related commands documented correctly.Past feedback addressed: pr-comment-responder marked as optional with separate installation guidance.
.agents/pr-comments/PR-225/comments.md (1)
1-158: PR comment tracking artifact is well-structured.Documents all review comments with status tracking, resolutions, and QA findings. No security issues or broken references.
scripts/Invoke-BatchPRReview.ps1 (6)
59-75: Get-PRBranch error handling looks good.LASTEXITCODE check after gh command with early return prevents JSON parse errors on failure. Warning message includes actionable troubleshooting.
102-145: Get-WorktreeStatus handles upstream detection correctly.Proper try/finally for Push-Location cleanup. Upstream check before
@{u}reference prevents errors on branches without tracking. Clean implementation.
147-184: Remove-PRWorktree has good safety checks.Idempotent behavior (returns true if already removed). Safety checks for uncommitted/unpushed changes before removal. Same dead code issue at line 179 as noted in New-PRWorktree.
186-233: Push-WorktreeChanges implements robust error handling.LASTEXITCODE checks after each git operation. Uses
${PRNumber}syntax correctly. try/catch/finally ensures cleanup. Git push without hardcoded remote. Addresses all past feedback.
255-271: Cleanup operation handles push-before-remove correctly.Pushes changes before removing worktrees. Out-Null suppresses boolean returns, not errors (errors terminate due to ErrorActionPreference). Shows remaining worktrees after cleanup.
31-45: Parameter design prevents injection risks.PR numbers typed as
int[]- blocks command injection. ValidateSet constrains Operation. WorktreeRoot used only with Join-Path, never directly in shell commands.
- Add protocol reference at top of document - Add Step 0: Session Initialization (BLOCKING) with all MUST requirements - Serena initialization - Context retrieval (HANDOFF.md) - Skill validation - Session log creation - Add Step 8: Session End Protocol (BLOCKING) - Documentation update - Quality checks (markdown lint) - QA validation - Git operations - Retrospective (SHOULD) Ensures pr-review command follows RFC 2119 session requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Add a Claude command for processing multiple PR review comments in parallel using git worktrees for isolation.
Related Issues:
Features
Files Added
.claude/commands/pr-review.mdscripts/Invoke-BatchPRReview.ps1Usage Examples
PowerShell Helper
Test Plan
Generated with Claude Code