Feat/pr scan agent by shubhadapaithankar · Pull Request #4726 · Azure/ARO-RP

shubhadapaithankar · 2026-03-30T20:01:15Z

PR Scan Agent for ARO-RP

Adds a reusable PR scan workflow with Claude Code agent for systematic code review of pull requests.

Overview

This PR introduces a PR scan agent that performs production-safety-focused code review across 7 categories: correctness, API compatibility, security, reliability, tests, observability, and performance. The agent understands ARO-RP specific patterns (VMSize types,
async mutations, multi-module builds, admin API patterns) and complements CI checks by catching logic issues, breaking changes, and architectural concerns that automated tests miss.

What's Included

New Files

.claude/agents/pr-scan.md (240 lines) - Agent definition with 7-category review checklist and ARO-RP specific checks
.claude/agents/README.md (62 lines) - Agent directory index
docs/agent-guides/pr-scan-agent.md (473 lines) - Complete usage guide with examples, troubleshooting, and FAQ
hack/pr-scan.sh (232 lines) - Context gathering script with auto-detection, multiple modes, gh CLI integration

Modified Files

Makefile - Added pr-scan target with auto-detection support

Total: 5 files, 1,087 lines added

Key Features

🚀 Zero-Config Auto-Detection

# No PR number needed - just run from your feature branch 
make pr-scan                                                                                                                                                                                                                                                               
make pr-scan MODE=quick                                   
                                                                                                                                                                                                                                                                           
Automatically detects your current branch and scans against master. Perfect for quick pre-commit checks.                                                                                                                                                                   
                                                                                                                                                                                                                                                                           
🎯 Four Review Modes                                                                                                                                                                                                                                                       
                                                          
┌──────────┬────────────────────────────────┬───────────────────────────────────────┬────────────────────────────────────┐                                                                                                                                                 
│   Mode   │            Use Case            │            What It Checks             │               Output               │
├──────────┼────────────────────────────────┼───────────────────────────────────────┼────────────────────────────────────┤                                                                                                                                                 
│ full     │ Comprehensive pre-merge review │ All 7 categories                      │ All severity levels                │
├──────────┼────────────────────────────────┼───────────────────────────────────────┼────────────────────────────────────┤                                                                                                                                                 
│ quick    │ Fast critical issues scan      │ All 7 categories                      │ Blocker/High only                  │                                                                                                                                                 
├──────────┼────────────────────────────────┼───────────────────────────────────────┼────────────────────────────────────┤                                                                                                                                                 
│ security │ Security-focused review        │ Authz, secrets, injection, crypto     │ Security findings (all severities) │                                                                                                                                                 
├──────────┼────────────────────────────────┼───────────────────────────────────────┼────────────────────────────────────┤                                                                                                                                                 
│ pipeline │ CI/CD changes only             │ YAML syntax, scripts, secrets in logs │ Pipeline findings (all severities) │
└──────────┴────────────────────────────────┴───────────────────────────────────────┴────────────────────────────────────┘                                                                                                                                                 
                                                          
📋 Structured Output Format                                                                                                                                                                                                                                                
                                                          
Every scan produces consistent output:                                                                                                                                                                                                                                     
- Summary (3-5 bullets) - Changes summary, risk level, recommendation
- Findings by severity (Blocker/High/Medium/Low/Nit)                                                                                                                                                                                                                       
  - File:line or symbol reference                         
  - Why it matters (customer impact, production risk)                                                                                                                                                                                                                      
  - Concrete fix suggestion with code examples                                                                                                                                                                                                                             
- Questions for Author - Unclear assumptions that need validation                                                                                                                                                                                                          
- Not Reviewed - Explicit scope limitations (generated files, binaries, external deps)                                                                                                                                                                                     
                                                                                                                                                                                                                                                                           
🔧 Multiple Invocation Methods                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                                           
# 1. Via Makefile (recommended)                                                                                                                                                                                                                                            
make pr-scan                    # Auto: scan current branch                                                                                                                                                                                                                
make pr-scan PR=1234            # Scan specific PR                                                                                                                                                                                                                         
make pr-scan BRANCH=fix/bug     # Scan specific branch                                                                                                                                                                                                                     
make pr-scan MODE=security      # With mode                                                                                                                                                                                                                                
make pr-scan PR=1234 BASE=main MODE=quick  # All options                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                           
# 2. Via script directly                                                                                                                                                                                                                                                   
./hack/pr-scan.sh               # Auto: current branch                                                                                                                                                                                                                     
./hack/pr-scan.sh --pr 1234 --mode security                                                                                                                                                                                                                                
./hack/pr-scan.sh --branch fix/bug --base master                                                                                                                                                                                                                           
./hack/pr-scan.sh --list-open   # List open PRs (requires gh CLI)                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                           
# 3. Via Claude Code chat                                                                                                                                                                                                                                                  
"Scan PR 1234"                                                                                                                                                                                                                                                             
"Review my current branch in quick mode"                                                                                                                                                                                                                                   
"Check branch fix/feature for security issues"                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                                           
✅ ARO-RP Specific Checks                                                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                           
The agent understands production constraints and catches ARO-specific issues:                                                                                                                                                                                              
                                                          
- VMSize type confusion - Three distinct VMSize types (api.VMSize, vms.VMSize, pkg/api/v*/VMSize) that aren't interchangeable                                                                                                                                              
- Package deployment context - Code running in RP control plane vs customer cluster vs CI
- Admin API underscore pattern - HTTP parsing decoupled from business logic (postAdminFoo → _postAdminFoo)                                                                                                                                                                 
- Frontend-backend async pattern - PUT/DELETE must write to CosmosDB, not execute directly (architecture invariant)                                                                                                                                                        
- Multi-module builds - Root + pkg/api test coverage verification                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                           
Usage Examples                                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                                           
Example 1: Quick Pre-Commit Check                                                                                                                                                                                                                                          
                                                          
git checkout -b feat/add-vm-resize-validation                                                                                                                                                                                                                              
# ... make changes ...                                                                                                                                                                                                                                                     
make pr-scan MODE=quick                                                                                                                                                                                                                                                    
# Output: "1 High finding: missing nil check in validation logic"                                                                                                                                                                                                          
# Fix issue, commit                                                                                                                                                                                                                                                        
                                                                                                                                                                                                                                                                           
Example 2: Security Review for Auth Changes                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                           
make pr-scan BRANCH=feat/workload-identity MODE=security                                                                                                                                                                                                                   
# Agent focuses on authz checks, secret handling, input validation                                                                                                                                                                                                         
# Output identifies missing RBAC check in new admin endpoint                                                                                                                                                                                                               
                                                                                                                                                                                                                                                                           
Example 3: Pipeline Changes Review                                                                                                                                                                                                                                         
                                                                                                                                                                                                                                                                           
make pr-scan PR=1234 MODE=pipeline                                                                                                                                                                                                                                         
# Reviews only .pipelines/*.yml, .github/workflows/*.yml, CI scripts                                                                                                                                                                                                       
# Output finds grep -Po portability issue, suggests awk alternative                                                                                                                                                                                                        
                                                                                                                                                                                                                                                                           
Real-World Validation                                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                           
Tested on 5 diverse open PRs from this repository:                                                                                                                                                                                                                         
                                                          
┌────────────────────────────────────────┬────────────┬────────────────────────────────────────────────────────────────────────┐                                                                                                                                           
│                PR Type                 │ Agent Mode │                              Key Findings                              │
├────────────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────┤                                                                                                                                           
│ Monitor goroutine leak fix             │ Quick      │ ✅ Clean implementation, excellent test coverage                       │
├────────────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────┤                                                                                                                                           
│ ARM template expression limit          │ Security   │ 🟡 1 Medium: needs edge case validation                                │                                                                                                                                           
├────────────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────┤                                                                                                                                           
│ VM SKU iterator refactoring (16 files) │ Full       │ 🟡 1 High: early-exit optimization concern; test readability trade-off │                                                                                                                                           
├────────────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────┤                                                                                                                                           
│ VM resize automatic recovery           │ Quick      │ 🔴 1 Blocker: 30min timeout operation in HTTP handler                  │
├────────────────────────────────────────┼────────────┼────────────────────────────────────────────────────────────────────────┤                                                                                                                                           
│ Quota validation per-VM                │ Security   │ 🔴 2 High: weak VM name filtering could allow quota bypass             │
└────────────────────────────────────────┴────────────┴────────────────────────────────────────────────────────────────────────┘                                                                                                                                           
                                                          
Real issues caught:                                                                                                                                                                                                                                                        
- ✅ Production-impacting blocker (HTTP timeout causing Geneva Action failures)
- ✅ Security vulnerabilities (quota bypass via VM name filtering)                                                                                                                                                                                                         
- ✅ Reliability issues (missing error handling in retry logic)   
- ✅ Architectural concerns (memory vs readability trade-offs)                                                                                                                                                                                                             
- ✅ Concrete fix suggestions with code examples                                                                                                                                                                                                                           
                                                                                                                                                                                                                                                                           
How It Works                                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                                                           
1. Context gathering: hack/pr-scan.sh fetches PR metadata, diff, commits, changed files                                                                                                                                                                                    
2. Mode-specific guidance: Script outputs mode hints (e.g., "quick: focus on Blocker/High only")                                                                                                                                                                           
3. Agent analysis: Claude Code reads .claude/agents/pr-scan.md and applies 7-category checklist                                                                                                                                                                            
4. Structured output: Agent formats findings with severity, file:line, why it matters, fix suggestions                                                                                                                                                                     
                                                                                                                                                                                                                                                                           
The script is read-only (uses git diff, git log, optional gh pr view) and never modifies code or pushes changes.                                                                                                                                                           
                                                                                                                                                                                                                                                                           
Requirements                                                                                                                                                                                                                                                               
                                                          
- Claude Code: CLI, desktop app, or VS Code extension                                                                                                                                                                                                                      
- Git: Repository access (already required for development)
- Optional: gh CLI for PR metadata (script gracefully falls back to git-only mode without it)                                                                                                                                                                              
                                                                                                                                                                                                                                                                           
Documentation                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                           
Complete documentation in docs/agent-guides/pr-scan-agent.md:                                                                                                                                                                                                              
- 🚀 Quick start guide (3 invocation methods)
- 📖 Review modes explained                                                                                                                                                                                                                                                
- 🔧 Input formats and examples                           
- ❓ Troubleshooting common issues (6 scenarios)                                                                                                                                                                                                                           
- 📊 Comparison with CI checks                                                                                                                                                                                                                                             
- 💡 Tips for best results                                                                                                                                                                                                                                                 
- ❓ FAQ                                                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                           
Design Decisions                                                                                                                                                                                                                                                           
                                                                                                                                                                                                                                                                           
Why Auto-Detection?                                                                                                                                                                                                                                                        
                                                                                                                                                                                                                                                                           
Reduces friction for quick checks. Most common use case: "scan my current work" before pushing.                                                                                                                                                                            
                                                          
Why Multiple Modes?                                                                                                                                                                                                                                                        
                                                          
Different contexts need different depth:                                                                                                                                                                                                                                   
- Quick: Pre-push sanity check (2-3 min)                  
- Full: Comprehensive pre-merge review (5-10 min)                                                                                                                                                                                                                          
- Security: Auth/secrets changes (deep dive)              
- Pipeline: CI changes (targeted scope)                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                                                           
Why Read-Only?                                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                                           
Agent is a review tool, not an auto-fixer. Catches issues humans should decide how to fix. Never modifies code, bypasses security checks, or commits secrets.                                                                                                              
                                                                                                                                                                                                                                                                           
Why Structured Output?                                                                                                                                                                                                                                                     
                                                          
Consistent format makes findings actionable:                                                                                                                                                                                                                               
- Severity helps prioritize (Blocker = don't merge)       
- File:line enables quick navigation                                                                                                                                                                                                                                       
- "Why it matters" explains production impact             
- Fix suggestions provide concrete next steps                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                           
Benefits                                                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                           
For Developers                                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                                                           
- ✅ Catch issues before pushing (auto-detection + quick mode)                                                                                                                                                                                                             
- ✅ Learn ARO-RP patterns (agent explains "why" for each finding)
- ✅ Reduce review round-trips (fix issues proactively)                                                                                                                                                                                                                    
                                                                                                                                                                                                                                                                           
For Reviewers                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                           
- ✅ First-pass filter (agent flags areas needing human attention)                                                                                                                                                                                                         
- ✅ Focus on architecture/design (agent handles common patterns)
- ✅ Consistent review quality (same checklist every time)                                                                                                                                                                                                                 
                                                                                                                                                                                                                                                                           
For the Team                                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                                                           
- ✅ Complements CI (catches logic issues CI can't detect)                                                                                                                                                                                                                 
- ✅ Captures tribal knowledge (ARO-specific patterns documented in agent)
- ✅ Scales review capacity (agent handles routine checks 24/7)                                                                                                                                                                                                            
                                                                                                                                                                                                                                                                           
                                                                                                                                      
                                                          
Agent complements CI - it catches issues that pass all automated checks.                                                                                                                                                                                                   
                                                          
Future Enhancements                                                                                                                                                                                                                                                        
                                                          
Possible extensions (not in this PR):                                                                                                                                                                                                                                      
- Additional modes: performance (profiling focus), observability (metrics/logging)
- Integration with GitHub PR comments (automated suggestions)                                                                                                                                                                                                              
- Custom rules for team-specific patterns                    
- Metrics tracking (findings over time, pattern frequency)                                                                                                                                                                                                                 
                                                                                                                                                                                                                                                                           
Notes                                                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                           
- No hardcoded PR numbers: All examples use placeholders (1234, 5678) for generality                                                                                                                                                                                       
- Works without gh CLI: Gracefully degrades to git-only mode                                                                                                                                                                                                               
- Extensible: Easy to add new modes or ARO-specific checks                                                                                                                                                                                                                 
- Safe: Never modifies code, pushes changes, or bypasses security checks                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                           
Testing the Agent                                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                           
Try it on this PR:                                                                                                                                                                                                                                                         
# Once PR is merged and you're on master                  
git checkout -b test/pr-scan-agent                                                                                                                                                                                                                                         
# Make a small change                                                                                                                                                                                                                                                      
make pr-scan MODE=quick                                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                                                           
Or test on any open PR:                                                                                                                                                                                                                                                    
make pr-scan PR=<number>

E2E Collect must-gather runs go build ./hack/db but the pipeline never installed Go, relying on 1es-aro-ci-pool agents having go on PATH. Add GoTool@0 (1.25.3, matching go.mod) after checkout for CSP and MIWI jobs. Made-with: Cursor

- Add template-install-go.yml: grep version from go.mod under ARO_CHECKOUT_PATH, then GoTool@0 with goDownloadUrl (vars.yml) for MS golang endpoint. - Wire both E2E jobs via template; extend vars.yml with GO_TOOL_DOWNLOAD_URL. Made-with: Cursor

Implement reusable PR scan workflow with Claude Code agent for systematic code review of pull requests. New files: - .claude/agents/pr-scan.md: Agent definition with 7-category review checklist (correctness, API compatibility, security, reliability, tests, observability, performance) - .claude/agents/README.md: Agent directory index - docs/agent-guides/pr-scan-agent.md: Complete usage guide with examples, troubleshooting, and integration patterns - hack/pr-scan.sh: Context gathering script supporting --pr, --branch, --base, --mode, and --list-open options Modified files: - Makefile: Added pr-scan target forwarding to hack/pr-scan.sh Features: - Four review modes: full, quick, security, pipeline - Works with or without gh CLI (graceful fallback) - Structured output: Summary, Findings (Blocker/High/Medium/Low), Questions for Author, Not Reviewed scope - ARO-RP specific checks: VMSize types, async mutations, multi-module builds, admin API patterns - Complements CI by catching logic issues, breaking changes, and architectural concerns Usage: make pr-scan PR=1234 make pr-scan BRANCH=fix/feature MODE=quick ./hack/pr-scan.sh --list-open Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

When no PR= or BRANCH= is specified, automatically scan the current git branch against master. This makes it easier to quickly check work-in-progress without specifying branch names. Usage: make pr-scan # Auto: scan current branch make pr-scan MODE=quick # Auto with mode ./hack/pr-scan.sh # Same via script Changes: - hack/pr-scan.sh: Add --auto flag and auto-detection logic - Makefile: Default to --auto when no PR/BRANCH specified - docs: Updated all examples to show auto-detection first Auto mode detects current branch via 'git rev-parse --abbrev-ref HEAD' and prevents scanning master/main directly (must be on feature branch).

swiencki · 2026-03-30T22:13:33Z

Heads up, there's significant overlap with Azure/ARO-HCP#4638, which adds an in-repo PR reviewer agent with domain routing, review fixtures, and validation. That effort has an architecture ADR behind it
(openshift-online/architecture#71). Worth coordinating so the two efforts don't diverge.

shubhadapaithankar and others added 4 commits March 23, 2026 14:26

ci: install Go on E2E jobs for must-gather db build

eea46a2

E2E Collect must-gather runs go build ./hack/db but the pipeline never installed Go, relying on 1es-aro-ci-pool agents having go on PATH. Add GoTool@0 (1.25.3, matching go.mod) after checkout for CSP and MIWI jobs. Made-with: Cursor

shubhadapaithankar requested review from alcasim, bennerv, cadenmarchese, hawkowl, hlipsig, jharrington22, kevinobriendotca, kimorris27, mociarain, mrWinston, rogbas, sankur-codes, tiguelu, tsatam, tuxerrante, ventifus, wanghaoran1988 and yjst2012 as code owners March 30, 2026 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/pr scan agent#4726

Feat/pr scan agent#4726
shubhadapaithankar wants to merge 4 commits intomasterfrom
feat/pr-scan-agent

shubhadapaithankar commented Mar 30, 2026

Uh oh!

swiencki commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shubhadapaithankar commented Mar 30, 2026

PR Scan Agent for ARO-RP

Overview

What's Included

New Files

Modified Files

Key Features

🚀 Zero-Config Auto-Detection

Uh oh!

swiencki commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants