From cd79a8f49f3a4b8a8eda4ae4c0f4cfa00b29b053 Mon Sep 17 00:00:00 2001
From: Cali93 <32299095+Cali93@users.noreply.github.com>
Date: Fri, 16 Jan 2026 16:48:22 -0300
Subject: [PATCH 1/6] feat(multi-agent-review): spin sub-agents to review
specific aspects
---
claude/auto-review/action.yml | 188 ++++++------------
claude/auto-review/agents/review-bugs.md | 97 +++++++++
claude/auto-review/agents/review-patterns.md | 135 +++++++++++++
claude/auto-review/agents/review-security.md | 111 +++++++++++
.../extract-findings-from-comment.test.js | 47 ++++-
.../claude-comments/multi-agent-output.md | 53 +++++
.../scripts/extract-findings-from-comment.js | 8 +
7 files changed, 513 insertions(+), 126 deletions(-)
create mode 100644 claude/auto-review/agents/review-bugs.md
create mode 100644 claude/auto-review/agents/review-patterns.md
create mode 100644 claude/auto-review/agents/review-security.md
create mode 100644 claude/auto-review/scripts/__tests__/fixtures/claude-comments/multi-agent-output.md
diff --git a/claude/auto-review/action.yml b/claude/auto-review/action.yml
index 2da190a..7d8d9e6 100644
--- a/claude/auto-review/action.yml
+++ b/claude/auto-review/action.yml
@@ -100,145 +100,83 @@ runs:
---
- ## REVIEW FOCUS AREAS
+ ## MULTI-AGENT REVIEW
- Analyze code changes for issues in these areas:
- - **Code quality and best practices** for the technologies used in this project
- - **Potential bugs or issues** especially in critical code paths and async operations
- - **Performance considerations** for both frontend and backend code
- - **Security implications** particularly for authentication, API endpoints, and data handling
- - **Test coverage** and quality of test implementations
- - **Documentation updates** if needed for significant changes
- - **Type safety** and proper usage of type systems
- - **Error handling** and edge cases
- - **Code maintainability** and readability
+ You will conduct a comprehensive code review using THREE specialized subagents running IN PARALLEL.
- ---
+ ### Step 1: Spawn All Review Agents Simultaneously
- ## AUTOMATED CHECKS
+ Use the Task tool to launch ALL THREE agents in a SINGLE message with multiple tool calls. This is critical for parallel execution.
- ### External Domain URL Detection
+ **CRITICAL: Use subagent_type=\"general-purpose\" for all agents** and include the specialized instructions in each prompt.
- Scan all changed files for URLs matching the pattern 'https?://(?:www\.)?([a-zA-Z0-9.-]+\.[a-zA-Z]{2,})'. **ONLY report if URLs are found** pointing to domains other than the approved company domains (reown.com, walletconnect.com, walletconnect.org). **If no external domain URLs are detected, do not mention this check at all.**
+ **Agents to spawn (all with subagent_type=\"general-purpose\"):**
- If external domain URLs are found, report them using this exact format:
+ 1. **Bug Review Agent** - Include in prompt:
+ - Focus: Logic errors, null handling, race conditions, error handling, resource leaks, type mismatches
+ - Severity: CRITICAL (crashes/data corruption), HIGH (production bugs), MEDIUM (edge cases), LOW (minor)
+ - ID prefix: bug-
+ - Output format: \\\"#### Issue N: description\\\" with **ID:** bug-{file-slug}-{semantic-slug}-{hash}
-
- 🔒 **External Domain URL Detected** (Non-blocking)
- **URL:** [detected_url]
- **File:** [file_path:line_number]
+ 2. **Security Review Agent** - Include in prompt:
+ - Focus: Injection, auth/authz, data exposure, crypto weaknesses, SSRF, path traversal, input validation
+ - Severity: CRITICAL (RCE/auth bypass), HIGH (immediate fix), MEDIUM (limited exploit), LOW (improvement)
+ - ID prefix: sec-
+ - Include **Exploit Scenario:** for each finding
+ - Output format: \\\"#### Issue N: description\\\" with **ID:** sec-{file-slug}-{semantic-slug}-{hash}
- This change introduces URLs pointing to external domains. Please verify that these external dependencies are intentional and review for potential security, privacy, or compliance implications. Approved company domains are: reown.com, walletconnect.com, walletconnect.org
-
+ 3. **Patterns Review Agent** - Include in prompt:
+ - Focus: Anti-patterns, code smells, performance (N+1, memory leaks), missing abstractions
+ - Domain checks: External URLs (only flag non-reown.com/walletconnect.com/walletconnect.org), Cache-Control, GitHub Actions pull_request_target security, WalletConnect Pay architecture (cross-service DB, idempotency, resilience, saga compensation, trace context)
+ - Severity: HIGH (significant issue), MEDIUM (should address), LOW (improvement)
+ - ID prefix: pat-
+ - Output format: \\\"#### Issue N: description\\\" with **ID:** pat-{file-slug}-{semantic-slug}-{hash}
- ### Static Resource Cache-Control Validation
+ **For EACH agent prompt, include:**
+ - This preamble: \\\"You are a code reviewer. Provide actionable feedback on code changes. **Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.\\\"
+ - PR number and repository
+ - List of changed files
+ - The specific focus areas and severity levels for that agent type
+ - The ID format with the appropriate prefix (bug-, sec-, pat-)
+ - \\\"If no issues found, state: No [type] issues found.\\\"
+ - \\\"Wrap all issues in a collapsed \`\` block.\\\"
- Scan all changed files for static immutable resources (fonts, images, CSS, JS, media files) and their Cache-Control header configurations. **ONLY report if issues are found.** **If no cache-control issues are detected, do not mention this check at all.**
+ **CRITICAL - Before flagging anything, agents MUST:**
+ - **Be certain** - Don't flag something as a bug if unsure. Investigate first.
+ - **Don't invent hypothetical problems** - If an edge case matters, explain the realistic scenario where it occurs.
+ - **Only review the changes** - Don't flag pre-existing code that wasn't modified in this PR.
+ - **Communicate severity honestly** - Don't overstate. A minor issue is not HIGH severity.
- Flag any issues using these guidelines:
+ ### Step 2: Consolidate Findings
- 1. **Identify static resources**: Look for URLs or file references to static assets including:
- - Font files: .woff, .woff2, .ttf, .otf, .eot
- - Images: .jpg, .jpeg, .png, .gif, .svg, .webp, .ico
- - Stylesheets: .css
- - Scripts: .js (when served as static files)
- - Media: .mp4, .webm, .mp3, .wav
+ After ALL agents complete, consolidate their findings:
+ 1. Collect all issues from each agent's response
+ 2. **Deduplicate**: If multiple agents report the same issue (same file + same/adjacent line + similar description):
+ - Keep the finding with higher severity
+ - Merge unique context from duplicates
+ - Preserve the ID from the more specific agent (sec- > bug- > pat-)
+ 3. Sort final list by severity: CRITICAL > HIGH > MEDIUM > LOW
- 2. **Check Cache-Control headers**: Examine if Cache-Control headers are explicitly set for these resources. Flag in these cases:
- - **Explicit headers with insufficient max-age**: If Cache-Control is set with max-age < 31536000 (1 year) for static immutable resources
- - **Missing or implicit headers**: If Cache-Control is not explicitly set in the code and may rely on server defaults
+ ### Step 3: Output Consolidated Review
- 3. **Report format for insufficient cache-control**:
-
- ⚠️ **Static Resource Cache-Control Issue**
- **Resource:** [URL or file path]
- **File:** [file_path:line_number]
- **Current Cache-Control:** [value or \"Not explicitly set (implicit)\"]
- **Recommendation:** Static immutable resources should have \"Cache-Control: public, max-age=31536000, immutable\" (1 year)
- **Rationale:** Improves performance by allowing browsers to cache static resources for extended periods. Use cache-busting techniques (versioned URLs/filenames) for updates.
-
+ Produce a SINGLE consolidated summary containing all unique issues in the standard collapsed \`\` format. Do NOT include separate sections per agent - merge everything into one unified list.
+
+ ---
+
+ ## REVIEW FOCUS AREAS (Reference for Agents)
+
+ Analyze code changes for issues in these areas:
+ - **Code quality and best practices** for the technologies used in this project
+ - **Potential bugs or issues** especially in critical code paths and async operations
+ - **Performance considerations** for both frontend and backend code
+ - **Security implications** particularly for authentication, API endpoints, and data handling
+ - **Test coverage** and quality of test implementations
+ - **Documentation updates** if needed for significant changes
+ - **Type safety** and proper usage of type systems
+ - **Error handling** and edge cases
+ - **Code maintainability** and readability
- 4. **When to flag**:
- - Flag: Static resource URLs being added/modified without proper cache headers
- - Flag: Cache-Control headers set to less than 1 year for static immutable resources
- - Flag: Static resources where caching configuration is not visible in the changed code (ask reviewer to verify server/CDN configuration)
- - Don't flag: Dynamic/API endpoints or resources that change frequently
- - Don't flag: Resources already configured with max-age >= 31536000
-
- ### GitHub Actions Workflow Security (Supply Chain Attack Prevention)
-
- Scan all changed .github/workflows/*.yml or .github/workflows/*.yaml files for dangerous patterns that could enable supply chain attacks. **ONLY report if issues are found.** **If no workflow security issues are detected, do not mention this check at all.**
-
- Flag using these guidelines:
-
- 1. **CRITICAL: pull_request_target with PR head checkout** - Flag ANY workflow that:
- - Uses on: pull_request_target trigger AND
- - Checks out the PR head using patterns like:
- - ref: github.event.pull_request.head.sha
- - ref: github.event.pull_request.head.ref
- - ref: refs/pull/github.event.pull_request.number/merge
- - Any checkout that references the pull request head rather than base
- - **Severity: CRITICAL** - This combination allows attackers to run arbitrary code by modifying repository scripts in their PR
-
- 2. **HIGH: pull_request_target with script execution** - Flag workflows that:
- - Use on: pull_request_target AND
- - Execute repository scripts (.js, .sh, .py, .ts, etc.) after checkout
- - **Severity: HIGH** - Could be exploited if checkout configuration changes
-
- 3. **MEDIUM: pull_request_target usage** - Flag ANY use of pull_request_target as requiring security review:
- - This trigger runs with repository secrets access
- - External contributors can trigger it via PRs
- - **Severity: MEDIUM** - Requires careful review even without obvious vulnerabilities
-
- Report format:
- 🚨 **GitHub Actions Security Risk** (Blocking)
- **Severity:** [CRITICAL/HIGH/MEDIUM]
- **File:** [file_path:line_number]
- **Pattern:** [description of the dangerous pattern found]
- **Risk:** [specific attack vector]
- **Recommendation:** [specific fix - e.g., use pull_request trigger instead, or avoid checking out PR head, or use workflow_run pattern for external PRs]
-
- ### WalletConnect Pay Architecture Compliance
-
- Scan changed files for SOA and reliability pattern violations. **ONLY report if violations found.** **If no violations detected, do not mention this check.**
-
- 1. **CRITICAL: Cross-Service Database Access**
- - Imports of other services' DB models/DAOs/repositories
- - SQL queries to tables outside service boundary
- - Shared DB connections across services
- **Report:** 🚨 **Architecture Violation: Cross-Service DB Access** (Blocking) - Services must expose data via APIs, not direct DB access
-
- 2. **HIGH: Missing Idempotency Key Validation**
- - POST/PUT/PATCH/DELETE handlers without idempotency key extraction
- - Payment/wallet/transaction mutations lacking idempotency store checks
- - No duplicate request detection or payload fingerprint validation
- **Report:** ⚠️ **Missing Idempotency Protection** - Recommend extracting idempotency key from request headers, checking against a store for duplicates, and returning cached response if already processed
-
- 3. **HIGH: External Calls Without Timeout/Retry**
- - fetch()/axios/http.request() without explicit timeout
- - External API calls without retry + exponential backoff
- - Critical dependencies without circuit breaker
- **Report:** ⚠️ **Missing Resilience Patterns** - Add timeout, retry with backoff, circuit breaker for cascading failure prevention
-
- 4. **HIGH: Non-Idempotent Event Consumers**
- - Message queue consumers (SQS/SNS/Kafka) without message ID deduplication
- - Event handlers mutating state without checking if already processed
- - Webhook handlers lacking replay protection
- **Report:** ⚠️ **Non-Idempotent Event Consumer** - Recommend checking event/message ID against processed events store before performing mutations to prevent duplicate processing
-
- 5. **MEDIUM: Missing Compensating Actions**
- - Sequential cross-service calls without rollback logic
- - Multi-step workflows (hold → settle → notify) without saga pattern
- - Try-catch blocks not emitting compensating events on failure
- **Report:** ⚠️ **Missing Saga Compensation** - Recommend adding catch blocks that release holds and emit compensating/reversal events on failure
-
- 6. **MEDIUM: State Transitions Without Trace Context**
- - Payment/wallet state changes without structured logs + correlation IDs
- - Cross-service calls missing trace ID propagation (X-Trace-Id header)
- - State events not linked to originating operation
- **Report:** ⚠️ **Missing Trace Context** - Recommend adding structured logging with traceId and correlationId for state changes, and propagating trace headers across service calls
-
- **Scope:** Flag new code or modifications introducing anti-patterns in payment/wallet/transaction operations. Ignore read-only ops, utilities, tests."
+ **Note:** Domain-specific checks (External URLs, Cache-Control, GitHub Actions Security, WalletConnect Pay Architecture) are handled by the review-patterns agent."
# Add project context
@@ -410,7 +348,7 @@ runs:
anthropic_api_key: ${{ inputs.anthropic_api_key }}
prompt: ${{ env.REVIEW_PROMPT }}
track_progress: true
- claude_args: --model ${{ inputs.model }}
+ claude_args: --model ${{ inputs.model }} --allowedTools "Read,Glob,Grep,Task,WebFetch"
allowed_bots: devin-ai-integration[bot]
- name: Extract findings from Claude's comment
diff --git a/claude/auto-review/agents/review-bugs.md b/claude/auto-review/agents/review-bugs.md
new file mode 100644
index 0000000..ca57930
--- /dev/null
+++ b/claude/auto-review/agents/review-bugs.md
@@ -0,0 +1,97 @@
+---
+name: review-bugs
+description: Reviews code for bugs, logic errors, race conditions, and runtime issues. Use when analyzing PR changes for functional correctness.
+tools: Read, Glob, Grep
+model: sonnet
+---
+
+You are a code reviewer. Provide actionable feedback on code changes.
+
+**Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.
+
+Your specialization: finding bugs and functional issues.
+
+## Your Focus Areas
+
+Analyze the PR changes for:
+
+1. **Logic Errors**
+ - Incorrect conditionals or boolean logic
+ - Off-by-one errors in loops or array access
+ - Wrong comparison operators (< vs <=, == vs ===)
+ - Inverted conditions or negation mistakes
+
+2. **Null/Undefined Handling**
+ - Missing null checks before dereferencing
+ - Optional chaining gaps
+ - Unhandled undefined returns from functions
+ - Incorrect default values
+
+3. **Race Conditions & Concurrency**
+ - Shared state modifications without synchronization
+ - Missing await on async operations
+ - Promise handling issues (unhandled rejections)
+ - State updates that could be overwritten
+
+4. **Error Handling**
+ - Missing try-catch blocks around fallible operations
+ - Swallowed errors (empty catch blocks)
+ - Incorrect error propagation
+ - Resource cleanup in error paths
+
+5. **Resource Leaks**
+ - Unclosed file handles, connections, or streams
+ - Missing cleanup in finally blocks
+ - Event listeners not removed
+ - Timers/intervals not cleared
+
+6. **Type Mismatches**
+ - Implicit type coercion issues
+ - Incorrect type assertions
+ - Function signature mismatches
+ - Incompatible type assignments
+
+## Review Process
+
+1. Read the full file content for each changed file to understand context
+2. Focus on the changed lines but consider how they interact with surrounding code
+3. Look for edge cases the code doesn't handle
+4. Verify error paths are properly handled
+
+## Before Flagging Anything
+
+- **Be certain** - Don't flag something as a bug if you're unsure. Investigate first.
+- **Don't invent hypothetical problems** - If an edge case matters, explain the realistic scenario where it occurs.
+- **Only review the changes** - Don't flag pre-existing code that wasn't modified in this PR.
+- **Communicate severity honestly** - Don't overstate. A minor issue is not HIGH severity.
+
+## Severity Levels
+
+- **CRITICAL**: Will cause crashes, data corruption, or major functionality breakage
+- **HIGH**: Likely to cause bugs in production under normal usage
+- **MEDIUM**: Could cause issues in edge cases or specific conditions
+- **LOW**: Minor issues that are unlikely to cause problems
+
+## Output Format
+
+Report each issue using this exact format:
+
+```
+#### Issue N: Brief description
+**ID:** bug-{file-slug}-{semantic-slug}-{hash}
+**File:** path/to/file.ext:lineNumber
+**Severity:** CRITICAL|HIGH|MEDIUM|LOW
+**Category:** bug
+**Context:** Detailed explanation of the bug and why it's problematic.
+**Recommendation:** Specific fix with code snippet if applicable.
+```
+
+**ID Format Rules:**
+- Always prefix with `bug-`
+- file-slug: filename without extension, first 12 chars, lowercase
+- semantic-slug: 2-4 key terms from issue description
+- hash: first 4 chars of SHA256(file_path + description)
+
+If no bugs are found, state: "No bug issues found."
+
+Wrap all issues in a collapsed `` block with summary showing the count.
diff --git a/claude/auto-review/agents/review-patterns.md b/claude/auto-review/agents/review-patterns.md
new file mode 100644
index 0000000..fc81def
--- /dev/null
+++ b/claude/auto-review/agents/review-patterns.md
@@ -0,0 +1,135 @@
+---
+name: review-patterns
+description: Reviews code for architectural patterns, maintainability, performance, and domain-specific compliance. Use when analyzing PR changes for code quality.
+tools: Read, Glob, Grep
+model: sonnet
+---
+
+You are a code reviewer. Provide actionable feedback on code changes.
+
+**Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.
+
+Your specialization: code quality, architecture, and domain-specific compliance.
+
+## Your Focus Areas
+
+### Code Quality & Architecture
+
+1. **Anti-Patterns**
+ - God objects/classes with too many responsibilities
+ - Spaghetti code with tangled dependencies
+ - Copy-paste code (DRY violations)
+ - Magic numbers/strings without constants
+ - Deep nesting (arrow anti-pattern)
+
+2. **Code Smells**
+ - Long methods/functions (> 50 lines)
+ - Long parameter lists (> 4 parameters)
+ - Feature envy (method uses other object's data more than its own)
+ - Inappropriate intimacy (classes too coupled)
+ - Dead code
+
+3. **Performance Issues**
+ - N+1 query patterns
+ - Missing database indexes for queried fields
+ - Unnecessary re-renders in React components
+ - Memory leaks (unbounded caches, retained references)
+ - Blocking operations in async contexts
+ - Inefficient algorithms (O(n^2) when O(n) is possible)
+
+4. **Missing Abstractions**
+ - Repeated logic that should be extracted
+ - Missing interfaces/protocols for polymorphism
+ - Hardcoded values that should be configurable
+ - Missing error types for domain errors
+
+### Domain-Specific Checks
+
+5. **External Domain URL Detection**
+ - Scan changed files for URLs (https?://...)
+ - Flag URLs pointing to domains OTHER than: reown.com, walletconnect.com, walletconnect.org
+ - Report format:
+ ```
+ **External Domain URL Detected** (Non-blocking)
+ **URL:** [detected_url]
+ **File:** [file_path:line_number]
+ Verify external dependencies are intentional.
+ ```
+
+6. **Static Resource Cache-Control Validation**
+ - Check static resources (.woff, .woff2, .ttf, .jpg, .png, .svg, .css, .js) for Cache-Control headers
+ - Flag if max-age < 31536000 (1 year) for immutable resources
+ - Report format:
+ ```
+ **Static Resource Cache-Control Issue**
+ **Resource:** [URL or file path]
+ **File:** [file_path:line_number]
+ **Recommendation:** Use "Cache-Control: public, max-age=31536000, immutable"
+ ```
+
+7. **GitHub Actions Workflow Security**
+ - **CRITICAL**: `pull_request_target` with PR head checkout (ref: github.event.pull_request.head.*)
+ - **HIGH**: `pull_request_target` executing repository scripts after checkout
+ - **MEDIUM**: Any `pull_request_target` usage requires security review
+ - Report format:
+ ```
+ **GitHub Actions Security Risk** (Blocking)
+ **Severity:** [CRITICAL/HIGH/MEDIUM]
+ **File:** [file_path:line_number]
+ **Pattern:** [description of dangerous pattern]
+ **Risk:** [specific attack vector]
+ ```
+
+8. **WalletConnect Pay Architecture Compliance**
+ - **CRITICAL**: Cross-service database access (imports of other services' DB models)
+ - **HIGH**: Missing idempotency key validation in mutation handlers
+ - **HIGH**: External calls without timeout/retry/circuit breaker
+ - **HIGH**: Non-idempotent event consumers (SQS/SNS/Kafka without dedup)
+ - **MEDIUM**: Missing compensating actions in multi-step workflows
+ - **MEDIUM**: State transitions without trace context (traceId/correlationId)
+
+## Review Process
+
+1. Read the full file content for each changed file to understand context
+2. Identify patterns and anti-patterns in the code structure
+3. Check for performance implications of the changes
+4. Verify domain-specific compliance rules
+5. Consider maintainability impact
+
+## Before Flagging Anything
+
+- **Be certain** - Don't flag something as an anti-pattern if you're unsure. Investigate first.
+- **Don't invent hypothetical problems** - If a pattern issue matters, explain the realistic impact.
+- **Only review the changes** - Don't flag pre-existing code that wasn't modified in this PR.
+- **Don't be a zealot about style** - Some "violations" are acceptable when they're the simplest option.
+- **Communicate severity honestly** - Don't overstate. A minor style issue is not HIGH severity.
+
+## Severity Levels
+
+- **HIGH**: Significant maintainability or performance issue
+- **MEDIUM**: Code quality issue that should be addressed
+- **LOW**: Minor improvement opportunity
+
+## Output Format
+
+Report each issue using this exact format:
+
+```
+#### Issue N: Brief description
+**ID:** pat-{file-slug}-{semantic-slug}-{hash}
+**File:** path/to/file.ext:lineNumber
+**Severity:** HIGH|MEDIUM|LOW
+**Category:** patterns|performance|architecture|domain-compliance
+**Context:** Detailed explanation of the issue and its impact.
+**Recommendation:** Specific improvement with code snippet if applicable.
+```
+
+**ID Format Rules:**
+- Always prefix with `pat-`
+- file-slug: filename without extension, first 12 chars, lowercase
+- semantic-slug: 2-4 key terms from issue description
+- hash: first 4 chars of SHA256(file_path + description)
+
+If no pattern issues are found, state: "No pattern issues found."
+
+Wrap all issues in a collapsed `` block with summary showing the count.
diff --git a/claude/auto-review/agents/review-security.md b/claude/auto-review/agents/review-security.md
new file mode 100644
index 0000000..0822e6c
--- /dev/null
+++ b/claude/auto-review/agents/review-security.md
@@ -0,0 +1,111 @@
+---
+name: review-security
+description: Reviews code for security vulnerabilities, authentication issues, and data protection. Use when analyzing PR changes for security concerns.
+tools: Read, Glob, Grep
+model: sonnet
+---
+
+You are a code reviewer. Provide actionable feedback on code changes.
+
+**Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.
+
+Your specialization: finding security vulnerabilities.
+
+## Your Focus Areas
+
+Analyze the PR changes for:
+
+1. **Injection Vulnerabilities**
+ - SQL injection (string concatenation in queries)
+ - Command injection (shell command construction)
+ - NoSQL injection (MongoDB query construction)
+ - Template injection (user input in templates)
+ - LDAP injection
+
+2. **Authentication & Authorization**
+ - Missing authentication checks
+ - Broken access control (IDOR vulnerabilities)
+ - Insecure session management
+ - Hardcoded credentials or API keys
+ - Weak password handling
+ - Missing CSRF protection
+
+3. **Data Exposure**
+ - Sensitive data in logs
+ - PII exposure in responses
+ - Credentials in error messages
+ - Secrets in version control
+ - Insecure data storage
+
+4. **Cryptographic Weaknesses**
+ - Weak hashing algorithms (MD5, SHA1 for passwords)
+ - Insecure random number generation
+ - Hardcoded cryptographic keys
+ - Missing encryption for sensitive data
+ - Improper certificate validation
+
+5. **Network Security**
+ - SSRF vulnerabilities (user-controlled URLs)
+ - Open redirects
+ - Missing TLS enforcement
+ - Insecure CORS configuration
+ - Unsafe deserialization
+
+6. **Path Traversal & File Handling**
+ - Path traversal in file operations
+ - Arbitrary file read/write
+ - Unsafe file uploads
+ - Symlink attacks
+
+7. **Input Validation**
+ - Missing input sanitization
+ - Prototype pollution
+ - ReDoS vulnerabilities (unsafe regex)
+ - Integer overflow/underflow
+
+## Review Process
+
+1. Read the full file content for each changed file to understand context
+2. Trace data flow from untrusted sources to sensitive sinks
+3. Check authorization boundaries at all entry points
+4. Verify cryptographic implementations follow best practices
+5. Look for OWASP Top 10 vulnerabilities
+
+## Before Flagging Anything
+
+- **Be certain** - Don't flag something as a vulnerability if you're unsure. Investigate first.
+- **Don't invent hypothetical problems** - If an attack vector matters, explain the realistic scenario where it's exploitable.
+- **Only review the changes** - Don't flag pre-existing code that wasn't modified in this PR.
+- **Communicate severity honestly** - Don't overstate. A theoretical issue with no clear exploit path is not CRITICAL.
+
+## Severity Levels
+
+- **CRITICAL**: Exploitable vulnerability allowing RCE, auth bypass, or full data breach
+- **HIGH**: Significant vulnerability requiring immediate remediation
+- **MEDIUM**: Security weakness with limited exploitability
+- **LOW**: Minor security improvement opportunity
+
+## Output Format
+
+Report each issue using this exact format:
+
+```
+#### Issue N: Brief description
+**ID:** sec-{file-slug}-{semantic-slug}-{hash}
+**File:** path/to/file.ext:lineNumber
+**Severity:** CRITICAL|HIGH|MEDIUM|LOW
+**Category:** security
+**Context:** Detailed explanation of the vulnerability and attack vector.
+**Exploit Scenario:** How an attacker could exploit this vulnerability.
+**Recommendation:** Specific fix with code snippet if applicable.
+```
+
+**ID Format Rules:**
+- Always prefix with `sec-`
+- file-slug: filename without extension, first 12 chars, lowercase
+- semantic-slug: 2-4 key terms from issue description
+- hash: first 4 chars of SHA256(file_path + description)
+
+If no security issues are found, state: "No security issues found."
+
+Wrap all issues in a collapsed `` block with summary showing the count.
diff --git a/claude/auto-review/scripts/__tests__/extract-findings-from-comment.test.js b/claude/auto-review/scripts/__tests__/extract-findings-from-comment.test.js
index ba899a6..566c5f9 100644
--- a/claude/auto-review/scripts/__tests__/extract-findings-from-comment.test.js
+++ b/claude/auto-review/scripts/__tests__/extract-findings-from-comment.test.js
@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
import fs from 'fs';
import path from 'path';
import { fileURLToPath } from 'url';
@@ -222,6 +222,51 @@ This is the issue with no file specified.`;
expect(findings[0].file).toBeUndefined();
expect(findings[0].description).toBe('Missing file info');
});
+
+ it('should parse multi-agent output and extract agent from ID prefix', () => {
+ const comment = fs.readFileSync(
+ path.join(fixturesDir, 'claude-comments', 'multi-agent-output.md'),
+ 'utf8'
+ );
+
+ const findings = parseClaudeComment(comment);
+
+ expect(findings).toHaveLength(4);
+
+ // Security agent issue (sec- prefix)
+ expect(findings[0].id).toBe('sec-users-sql-injection-f3a2');
+ expect(findings[0].agent).toBe('review-security');
+ expect(findings[0].description).toBe('SQL injection vulnerability in user query');
+
+ // Bug agent issue (bug- prefix)
+ expect(findings[1].id).toBe('bug-cache-race-condition-a1b2');
+ expect(findings[1].agent).toBe('review-bugs');
+ expect(findings[1].description).toBe('Race condition in cache update');
+
+ // Patterns agent issue (pat- prefix)
+ expect(findings[2].id).toBe('pat-userservi-n-plus-one-c3d4');
+ expect(findings[2].agent).toBe('review-patterns');
+ expect(findings[2].description).toBe('N+1 query pattern in user fetch');
+
+ // Another bug agent issue
+ expect(findings[3].id).toBe('bug-auth-missing-validation-8b91');
+ expect(findings[3].agent).toBe('review-bugs');
+ });
+
+ it('should not set agent for IDs without recognized prefix', () => {
+ const comment = `#### Issue 1: Test issue
+**ID:** users-sql-injection-f3a2
+**File:** test.ts:1
+**Severity:** HIGH
+
+Problem.`;
+
+ const findings = parseClaudeComment(comment);
+
+ expect(findings).toHaveLength(1);
+ expect(findings[0].id).toBe('users-sql-injection-f3a2');
+ expect(findings[0].agent).toBeUndefined();
+ });
});
describe('getLatestClaudeComment', () => {
diff --git a/claude/auto-review/scripts/__tests__/fixtures/claude-comments/multi-agent-output.md b/claude/auto-review/scripts/__tests__/fixtures/claude-comments/multi-agent-output.md
new file mode 100644
index 0000000..effe711
--- /dev/null
+++ b/claude/auto-review/scripts/__tests__/fixtures/claude-comments/multi-agent-output.md
@@ -0,0 +1,53 @@
+I've conducted a comprehensive review using specialized agents for bugs, security, and patterns. Here are the consolidated findings:
+
+
+Found 4 issue(s)
+
+#### Issue 1: SQL injection vulnerability in user query
+**ID:** sec-users-sql-injection-f3a2
+**File:** src/database/users.ts:45
+**Severity:** HIGH
+**Category:** security
+**Context:** The user query is constructed using string concatenation, allowing SQL injection attacks.
+**Exploit Scenario:** An attacker could inject malicious SQL via the userId parameter to extract or modify database contents.
+**Recommendation:** Use parameterized queries:
+```typescript
+const result = await db.query('SELECT * FROM users WHERE id = $1', [userId]);
+```
+
+#### Issue 2: Race condition in cache update
+**ID:** bug-cache-race-condition-a1b2
+**File:** src/services/cache.ts:89
+**Severity:** HIGH
+**Category:** bug
+**Context:** The cache read and write operations are not atomic, allowing race conditions when multiple requests update the same key concurrently.
+**Recommendation:** Use atomic operations or implement locking:
+```typescript
+await cache.setNX(key, value, { EX: ttl });
+```
+
+#### Issue 3: N+1 query pattern in user fetch
+**ID:** pat-userservi-n-plus-one-c3d4
+**File:** src/services/userService.ts:156
+**Severity:** MEDIUM
+**Category:** performance
+**Context:** The code fetches users in a loop, causing N+1 database queries instead of a single batch query.
+**Recommendation:** Use batch fetching:
+```typescript
+const users = await db.query('SELECT * FROM users WHERE id = ANY($1)', [userIds]);
+```
+
+#### Issue 4: Missing input validation
+**ID:** bug-auth-missing-validation-8b91
+**File:** src/auth/login.ts:23
+**Severity:** MEDIUM
+**Category:** bug
+**Context:** The login function doesn't validate email format before processing, which could lead to unexpected behavior.
+**Recommendation:** Add email validation:
+```typescript
+if (!isValidEmail(email)) {
+ throw new Error('Invalid email format');
+}
+```
+
+
diff --git a/claude/auto-review/scripts/extract-findings-from-comment.js b/claude/auto-review/scripts/extract-findings-from-comment.js
index 3b5e310..23ee400 100644
--- a/claude/auto-review/scripts/extract-findings-from-comment.js
+++ b/claude/auto-review/scripts/extract-findings-from-comment.js
@@ -53,9 +53,17 @@ export function parseClaudeComment(commentBody) {
// Extract ID if present
// Format: **ID:** file-slug-semantic-slug-hash
+ // Multi-agent format: **ID:** {agent-prefix}-file-slug-semantic-slug-hash (bug-, sec-, pat-)
const idMatch = content.match(/\*\*ID:\*\*\s+([a-z0-9\-]+)/i);
if (idMatch) {
finding.id = idMatch[1].trim().toLowerCase();
+
+ // Extract agent from ID prefix (bug-, sec-, pat-)
+ const agentPrefixMatch = finding.id.match(/^(bug|sec|pat)-/);
+ if (agentPrefixMatch) {
+ const agentMap = { 'bug': 'review-bugs', 'sec': 'review-security', 'pat': 'review-patterns' };
+ finding.agent = agentMap[agentPrefixMatch[1]];
+ }
}
// Extract file path and line number
From c5ced018e568231cd75c7dfa1592af160cc907fe Mon Sep 17 00:00:00 2001
From: Cali93 <32299095+Cali93@users.noreply.github.com>
Date: Tue, 20 Jan 2026 09:54:13 -0300
Subject: [PATCH 2/6] chore(agents): reference agents in action.yml
---
claude/auto-review/action.yml | 46 +++++++-------------
claude/auto-review/agents/review-bugs.md | 7 +--
claude/auto-review/agents/review-patterns.md | 7 +--
claude/auto-review/agents/review-security.md | 7 +--
4 files changed, 18 insertions(+), 49 deletions(-)
diff --git a/claude/auto-review/action.yml b/claude/auto-review/action.yml
index 7d8d9e6..679d484 100644
--- a/claude/auto-review/action.yml
+++ b/claude/auto-review/action.yml
@@ -104,48 +104,32 @@ runs:
You will conduct a comprehensive code review using THREE specialized subagents running IN PARALLEL.
+ **Agent specification files location:** ${{ github.action_path }}/agents/
+
### Step 1: Spawn All Review Agents Simultaneously
Use the Task tool to launch ALL THREE agents in a SINGLE message with multiple tool calls. This is critical for parallel execution.
- **CRITICAL: Use subagent_type=\"general-purpose\" for all agents** and include the specialized instructions in each prompt.
+ **CRITICAL: Use subagent_type=\"general-purpose\" for all agents.**
+
+ **For EACH agent prompt, include:**
+ 1. Instruction to read their specification file: \\\"First, read your agent specification file at [path] and follow its instructions.\\\"
+ 2. PR number and repository
+ 3. List of changed files to review
- **Agents to spawn (all with subagent_type=\"general-purpose\"):**
+ **Agents to spawn:**
- 1. **Bug Review Agent** - Include in prompt:
- - Focus: Logic errors, null handling, race conditions, error handling, resource leaks, type mismatches
- - Severity: CRITICAL (crashes/data corruption), HIGH (production bugs), MEDIUM (edge cases), LOW (minor)
+ 1. **Bug Review Agent**
+ - Spec file: ${{ github.action_path }}/agents/review-bugs.md
- ID prefix: bug-
- - Output format: \\\"#### Issue N: description\\\" with **ID:** bug-{file-slug}-{semantic-slug}-{hash}
- 2. **Security Review Agent** - Include in prompt:
- - Focus: Injection, auth/authz, data exposure, crypto weaknesses, SSRF, path traversal, input validation
- - Severity: CRITICAL (RCE/auth bypass), HIGH (immediate fix), MEDIUM (limited exploit), LOW (improvement)
+ 2. **Security Review Agent**
+ - Spec file: ${{ github.action_path }}/agents/review-security.md
- ID prefix: sec-
- - Include **Exploit Scenario:** for each finding
- - Output format: \\\"#### Issue N: description\\\" with **ID:** sec-{file-slug}-{semantic-slug}-{hash}
- 3. **Patterns Review Agent** - Include in prompt:
- - Focus: Anti-patterns, code smells, performance (N+1, memory leaks), missing abstractions
- - Domain checks: External URLs (only flag non-reown.com/walletconnect.com/walletconnect.org), Cache-Control, GitHub Actions pull_request_target security, WalletConnect Pay architecture (cross-service DB, idempotency, resilience, saga compensation, trace context)
- - Severity: HIGH (significant issue), MEDIUM (should address), LOW (improvement)
+ 3. **Patterns Review Agent**
+ - Spec file: ${{ github.action_path }}/agents/review-patterns.md
- ID prefix: pat-
- - Output format: \\\"#### Issue N: description\\\" with **ID:** pat-{file-slug}-{semantic-slug}-{hash}
-
- **For EACH agent prompt, include:**
- - This preamble: \\\"You are a code reviewer. Provide actionable feedback on code changes. **Diffs alone are not enough.** Read the full file(s) being modified to understand context. Code that looks wrong in isolation may be correct given surrounding logic.\\\"
- - PR number and repository
- - List of changed files
- - The specific focus areas and severity levels for that agent type
- - The ID format with the appropriate prefix (bug-, sec-, pat-)
- - \\\"If no issues found, state: No [type] issues found.\\\"
- - \\\"Wrap all issues in a collapsed \`\` block.\\\"
-
- **CRITICAL - Before flagging anything, agents MUST:**
- - **Be certain** - Don't flag something as a bug if unsure. Investigate first.
- - **Don't invent hypothetical problems** - If an edge case matters, explain the realistic scenario where it occurs.
- - **Only review the changes** - Don't flag pre-existing code that wasn't modified in this PR.
- - **Communicate severity honestly** - Don't overstate. A minor issue is not HIGH severity.
### Step 2: Consolidate Findings
diff --git a/claude/auto-review/agents/review-bugs.md b/claude/auto-review/agents/review-bugs.md
index ca57930..f51d2f0 100644
--- a/claude/auto-review/agents/review-bugs.md
+++ b/claude/auto-review/agents/review-bugs.md
@@ -1,9 +1,4 @@
----
-name: review-bugs
-description: Reviews code for bugs, logic errors, race conditions, and runtime issues. Use when analyzing PR changes for functional correctness.
-tools: Read, Glob, Grep
-model: sonnet
----
+# Bug Review Agent
You are a code reviewer. Provide actionable feedback on code changes.
diff --git a/claude/auto-review/agents/review-patterns.md b/claude/auto-review/agents/review-patterns.md
index fc81def..77fb233 100644
--- a/claude/auto-review/agents/review-patterns.md
+++ b/claude/auto-review/agents/review-patterns.md
@@ -1,9 +1,4 @@
----
-name: review-patterns
-description: Reviews code for architectural patterns, maintainability, performance, and domain-specific compliance. Use when analyzing PR changes for code quality.
-tools: Read, Glob, Grep
-model: sonnet
----
+# Patterns Review Agent
You are a code reviewer. Provide actionable feedback on code changes.
diff --git a/claude/auto-review/agents/review-security.md b/claude/auto-review/agents/review-security.md
index 0822e6c..61b6db2 100644
--- a/claude/auto-review/agents/review-security.md
+++ b/claude/auto-review/agents/review-security.md
@@ -1,9 +1,4 @@
----
-name: review-security
-description: Reviews code for security vulnerabilities, authentication issues, and data protection. Use when analyzing PR changes for security concerns.
-tools: Read, Glob, Grep
-model: sonnet
----
+# Security Review Agent
You are a code reviewer. Provide actionable feedback on code changes.
From 2311ae2a807d43682d60f193a3bb345bee9f4ea7 Mon Sep 17 00:00:00 2001
From: Ben Kremer
Date: Fri, 23 Jan 2026 14:37:17 +0700
Subject: [PATCH 3/6] fix(auto-review): remove duplicative ID prefix specs from
action.yml
Each agent's spec file already defines its own ID format (bug-, sec-, pat-).
Removes redundant prefix specs from agent list that weren't being followed.
Addresses review feedback from @bkrem on PR #64.
---
claude/auto-review/action.yml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/claude/auto-review/action.yml b/claude/auto-review/action.yml
index 074a6c2..7416371 100644
--- a/claude/auto-review/action.yml
+++ b/claude/auto-review/action.yml
@@ -98,9 +98,9 @@ runs:
2. PR number, repository, list of changed files
**Agents:**
- 1. **Bug Agent** - Spec: ${{ github.action_path }}/agents/review-bugs.md - ID prefix: bug-
- 2. **Security Agent** - Spec: ${{ github.action_path }}/agents/review-security.md - ID prefix: sec-
- 3. **Patterns Agent** - Spec: ${{ github.action_path }}/agents/review-patterns.md - ID prefix: pat-
+ 1. **Bug Agent** - Spec: ${{ github.action_path }}/agents/review-bugs.md
+ 2. **Security Agent** - Spec: ${{ github.action_path }}/agents/review-security.md
+ 3. **Patterns Agent** - Spec: ${{ github.action_path }}/agents/review-patterns.md
### Step 2: Consolidate Findings
From f5e581b56fa07a30a7387dee38b4ebae962435fa Mon Sep 17 00:00:00 2001
From: Ben Kremer
Date: Fri, 23 Jan 2026 15:38:23 +0700
Subject: [PATCH 4/6] chore(agents): align issue format with main action.yml
spec
- Use consistent ID template placeholders ({filename}-{2-4-key-terms}-{hash})
- Change severity/category separator from | to / to match main prompt
- Rename "ID Format" to "ID Generation" for consistency
- Update recommendation wording to match main action.yml
- Add CRITICAL severity to patterns agent for domain-specific checks
- Remove redundant details block instruction (handled by orchestrator)
---
claude/auto-review/agents/review-bugs.md | 10 ++++------
claude/auto-review/agents/review-patterns.md | 13 ++++++-------
claude/auto-review/agents/review-security.md | 10 ++++------
3 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/claude/auto-review/agents/review-bugs.md b/claude/auto-review/agents/review-bugs.md
index f2a398d..0ff4847 100644
--- a/claude/auto-review/agents/review-bugs.md
+++ b/claude/auto-review/agents/review-bugs.md
@@ -73,9 +73,9 @@ Report each issue using this exact format:
```
#### Issue N: Brief description
-**ID:** bug-{file-slug}-{semantic-slug}-{hash}
+**ID:** bug-{filename}-{2-4-key-terms}-{hash}
**File:** path/to/file.ext:lineNumber
-**Severity:** CRITICAL|HIGH|MEDIUM|LOW
+**Severity:** CRITICAL/HIGH/MEDIUM/LOW
**Category:** bug
**Context:**
@@ -84,12 +84,10 @@ Report each issue using this exact format:
- **Impact:** Potential consequences (crash, data corruption, etc.)
- **Trigger:** Under what conditions this bug manifests
-**Recommendation:** Specific fix with code snippet (1-10 lines).
+**Recommendation:** Fix with minimal code snippet (1-10 lines).
```
-**ID Format:** bug-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
+**ID Generation:** bug-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
Example: bug-cache-race-condition-a1b2
If no bugs found: "No bug issues found."
-
-Wrap all issues in collapsed `` block.
diff --git a/claude/auto-review/agents/review-patterns.md b/claude/auto-review/agents/review-patterns.md
index cc49c24..cf3f97d 100644
--- a/claude/auto-review/agents/review-patterns.md
+++ b/claude/auto-review/agents/review-patterns.md
@@ -83,6 +83,7 @@ Your specialization: code quality, architecture, and domain-specific compliance.
## Severity Levels
+- **CRITICAL**: Severe issue (e.g., workflow security vulnerabilities, cross-service DB access)
- **HIGH**: Significant maintainability or performance issue
- **MEDIUM**: Code quality issue that should be addressed
- **LOW**: Minor improvement opportunity
@@ -93,10 +94,10 @@ Report each issue using this exact format:
```
#### Issue N: Brief description
-**ID:** pat-{file-slug}-{semantic-slug}-{hash}
+**ID:** pat-{filename}-{2-4-key-terms}-{hash}
**File:** path/to/file.ext:lineNumber
-**Severity:** HIGH|MEDIUM|LOW
-**Category:** patterns|performance|architecture|domain-compliance
+**Severity:** CRITICAL/HIGH/MEDIUM/LOW
+**Category:** patterns/performance/architecture/domain-compliance
**Context:**
- **Pattern:** What the problematic code pattern is
@@ -104,12 +105,10 @@ Report each issue using this exact format:
- **Impact:** Potential consequences (performance, maintainability, etc.)
- **Trigger:** Under what conditions this becomes problematic
-**Recommendation:** Specific fix with code snippet (1-10 lines).
+**Recommendation:** Fix with minimal code snippet (1-10 lines).
```
-**ID Format:** pat-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
+**ID Generation:** pat-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
Example: pat-userservi-n-plus-one-c3d4
If no pattern issues found: "No pattern issues found."
-
-Wrap all issues in collapsed `` block.
diff --git a/claude/auto-review/agents/review-security.md b/claude/auto-review/agents/review-security.md
index 72701ed..3a274ea 100644
--- a/claude/auto-review/agents/review-security.md
+++ b/claude/auto-review/agents/review-security.md
@@ -86,9 +86,9 @@ Report each issue using this exact format:
```
#### Issue N: Brief description
-**ID:** sec-{file-slug}-{semantic-slug}-{hash}
+**ID:** sec-{filename}-{2-4-key-terms}-{hash}
**File:** path/to/file.ext:lineNumber
-**Severity:** CRITICAL|HIGH|MEDIUM|LOW
+**Severity:** CRITICAL/HIGH/MEDIUM/LOW
**Category:** security
**Context:**
@@ -97,12 +97,10 @@ Report each issue using this exact format:
- **Impact:** Potential consequences (RCE, data breach, auth bypass, etc.)
- **Trigger:** Under what conditions this becomes exploitable
-**Recommendation:** Specific fix with code snippet (1-10 lines).
+**Recommendation:** Fix with minimal code snippet (1-10 lines).
```
-**ID Format:** sec-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
+**ID Generation:** sec-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}
Example: sec-users-sql-injection-f3a2
If no security issues found: "No security issues found."
-
-Wrap all issues in collapsed `` block.
From 95e8d3a7aca4a2792a249302a05fa892568ce4fb Mon Sep 17 00:00:00 2001
From: Ben Kremer
Date: Fri, 23 Jan 2026 15:49:46 +0700
Subject: [PATCH 5/6] feat(auto-review): add selective subagent spawning
heuristic
Implement heuristic to selectively spawn review agents based on PR
characteristics, avoiding unnecessary agent invocation for simple PRs.
Key changes:
- Add determine-agents.js with file categorization and keyword analysis
- Skip review entirely for docs-only, rename-only, and empty PRs
- Spawn security agent for workflow/auth/SQL/infra/deps files or keywords
- Spawn patterns agent for large files (>300 lines) or >5 code files
- Always spawn all agents for large PRs (>500 lines or >15 files)
- Add force_all_agents input and full-review/skip-review label overrides
- Add comprehensive test suite (47 tests)
Heuristic behavior:
- Docs/rename/empty PR: Skip review entirely
- Lockfile-only: Security agent only
- Test-only: Bug agent only
- Large PR: All 3 agents
- Small code PR: Bug agent only (+ security/patterns if triggered)
---
claude/auto-review/action.yml | 126 +++-
.../__tests__/determine-agents.test.js | 602 ++++++++++++++++++
.../auto-review/scripts/determine-agents.js | 396 ++++++++++++
3 files changed, 1112 insertions(+), 12 deletions(-)
create mode 100644 claude/auto-review/scripts/__tests__/determine-agents.test.js
create mode 100644 claude/auto-review/scripts/determine-agents.js
diff --git a/claude/auto-review/action.yml b/claude/auto-review/action.yml
index 7416371..ac6e86a 100644
--- a/claude/auto-review/action.yml
+++ b/claude/auto-review/action.yml
@@ -23,10 +23,57 @@ inputs:
description: "Post inline PR comments generated from findings.json"
required: false
default: "true"
+ force_all_agents:
+ description: "Force all review agents regardless of heuristic (overrides selective spawning)"
+ required: false
+ default: "false"
runs:
using: "composite"
steps:
+ - name: Determine agents to spawn
+ shell: bash
+ env:
+ GH_TOKEN: ${{ github.token }}
+ FORCE_ALL_AGENTS: ${{ inputs.force_all_agents }}
+ run: |
+ SCRIPT_PATH="${{ github.action_path }}/scripts/determine-agents.js"
+
+ if [[ ! -f "$SCRIPT_PATH" ]]; then
+ echo "::warning::Determine agents script not found, spawning all agents"
+ echo "AGENTS_TO_RUN=bug,security,patterns" >> $GITHUB_ENV
+ echo "AGENT_REASON=Script not found (fallback)" >> $GITHUB_ENV
+ exit 0
+ fi
+
+ RESULT=$(node "$SCRIPT_PATH")
+
+ AGENTS=$(echo "$RESULT" | jq -r '.agents | join(",")')
+ REASON=$(echo "$RESULT" | jq -r '.reason')
+ SKIPPED=$(echo "$RESULT" | jq -r '.skipped | join(",")')
+
+ echo "AGENTS_TO_RUN=$AGENTS" >> $GITHUB_ENV
+ echo "AGENT_REASON=$REASON" >> $GITHUB_ENV
+ echo "AGENTS_SKIPPED=$SKIPPED" >> $GITHUB_ENV
+
+ # Set skip flag if no agents to spawn
+ if [[ -z "$AGENTS" ]]; then
+ echo "SKIP_REVIEW=true" >> $GITHUB_ENV
+ else
+ echo "SKIP_REVIEW=false" >> $GITHUB_ENV
+ fi
+
+ echo "::group::Agent Selection"
+ echo "Agents to spawn: ${AGENTS:-none}"
+ echo "Reason: $REASON"
+ if [[ -n "$SKIPPED" ]]; then
+ echo "Skipped agents: $SKIPPED"
+ fi
+ if [[ -z "$AGENTS" ]]; then
+ echo "Review will be skipped"
+ fi
+ echo "::endgroup::"
+
- name: Set up review prompt
shell: bash
run: |
@@ -83,24 +130,62 @@ runs:
---
- ## MULTI-AGENT REVIEW
+ ## MULTI-AGENT REVIEW"
+
+ # Check if we have any agents to spawn
+ if [[ -z "$AGENTS_TO_RUN" ]]; then
+ # No agents to spawn - skip review
+ PROMPT="$PROMPT
+
+ **Review skipped:** $AGENT_REASON
+
+ Respond with: \\\"✅ No code review needed - $AGENT_REASON\\\""
+ else
+ # Build agent list dynamically
+ AGENT_COUNT=0
+ AGENT_LIST=""
+
+ if [[ "$AGENTS_TO_RUN" == *"bug"* ]]; then
+ AGENT_COUNT=$((AGENT_COUNT + 1))
+ AGENT_LIST="$AGENT_LIST
+ $AGENT_COUNT. **Bug Agent** - Spec: ${{ github.action_path }}/agents/review-bugs.md"
+ fi
+
+ if [[ "$AGENTS_TO_RUN" == *"security"* ]]; then
+ AGENT_COUNT=$((AGENT_COUNT + 1))
+ AGENT_LIST="$AGENT_LIST
+ $AGENT_COUNT. **Security Agent** - Spec: ${{ github.action_path }}/agents/review-security.md"
+ fi
+
+ if [[ "$AGENTS_TO_RUN" == *"patterns"* ]]; then
+ AGENT_COUNT=$((AGENT_COUNT + 1))
+ AGENT_LIST="$AGENT_LIST
+ $AGENT_COUNT. **Patterns Agent** - Spec: ${{ github.action_path }}/agents/review-patterns.md"
+ fi
+
+ if [[ $AGENT_COUNT -eq 1 ]]; then
+ AGENT_WORD="subagent"
+ else
+ AGENT_WORD="subagents"
+ fi
+
+ PROMPT="$PROMPT
+
+ Based on PR analysis: $AGENT_REASON
- Conduct review using THREE specialized subagents IN PARALLEL.
+ Conduct review using $AGENT_COUNT specialized $AGENT_WORD IN PARALLEL.
**Agent specs:** ${{ github.action_path }}/agents/
- ### Step 1: Spawn All Agents Simultaneously
+ ### Step 1: Spawn Agents Simultaneously
- Use Task tool to launch ALL THREE agents in a SINGLE message. Use subagent_type=\\\"general-purpose\\\".
+ Use Task tool to launch agents in a SINGLE message. Use subagent_type=\\\"general-purpose\\\".
**For EACH agent prompt include:**
1. \\\"Read your spec file at [path] and follow its instructions.\\\"
2. PR number, repository, list of changed files
- **Agents:**
- 1. **Bug Agent** - Spec: ${{ github.action_path }}/agents/review-bugs.md
- 2. **Security Agent** - Spec: ${{ github.action_path }}/agents/review-security.md
- 3. **Patterns Agent** - Spec: ${{ github.action_path }}/agents/review-patterns.md
+ **Agents:**$AGENT_LIST
### Step 2: Consolidate Findings
@@ -111,7 +196,8 @@ runs:
### Step 3: Output
- Produce SINGLE consolidated summary in collapsed \`\` format. No separate agent sections."
+ Produce SINGLE consolidated summary in collapsed \\\`\\\` format. No separate agent sections."
+ fi
# Add project context
@@ -197,7 +283,23 @@ runs:
echo "::warning::The 'timeout_minutes' input is deprecated and has no effect in claude-code-action@v1. Please use job-level 'timeout-minutes' instead. See: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes"
fi
+ - name: Skip review notification
+ if: env.SKIP_REVIEW == 'true'
+ shell: bash
+ env:
+ GH_TOKEN: ${{ github.token }}
+ run: |
+ echo "::notice::Skipping review: $AGENT_REASON"
+
+ # Post a comment to the PR
+ gh pr comment "${{ github.event.pull_request.number }}" \
+ --repo "${{ github.repository }}" \
+ --body "✅ **Auto Review Skipped**: $AGENT_REASON
+
+ _No code review needed for this PR. Add the \`full-review\` label to force a full review._"
+
- name: Automatic PR Review
+ if: env.SKIP_REVIEW != 'true'
uses: anthropics/claude-code-action@v1
with:
anthropic_api_key: ${{ inputs.anthropic_api_key }}
@@ -207,7 +309,7 @@ runs:
allowed_bots: devin-ai-integration[bot]
- name: Extract findings from Claude's comment
- if: inputs.comment_pr_findings == 'true'
+ if: inputs.comment_pr_findings == 'true' && env.SKIP_REVIEW != 'true'
shell: bash
env:
GH_TOKEN: ${{ github.token }}
@@ -227,7 +329,7 @@ runs:
node "$SCRIPT_PATH"
- name: Ensure GitHub CLI and jq for inline comments
- if: inputs.comment_pr_findings == 'true'
+ if: inputs.comment_pr_findings == 'true' && env.SKIP_REVIEW != 'true'
shell: bash
run: |
# Install GitHub CLI if needed
@@ -261,7 +363,7 @@ runs:
fi
- name: Post inline findings comments
- if: inputs.comment_pr_findings == 'true'
+ if: inputs.comment_pr_findings == 'true' && env.SKIP_REVIEW != 'true'
shell: bash
env:
GH_TOKEN: ${{ github.token }}
diff --git a/claude/auto-review/scripts/__tests__/determine-agents.test.js b/claude/auto-review/scripts/__tests__/determine-agents.test.js
new file mode 100644
index 0000000..1260586
--- /dev/null
+++ b/claude/auto-review/scripts/__tests__/determine-agents.test.js
@@ -0,0 +1,602 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import {
+ determineAgents,
+ categorizeFiles,
+ fetchPrFiles,
+ fetchPrLabels,
+} from "../determine-agents.js";
+import { ghApi } from "../lib/github-utils.js";
+
+// Mock ghApi
+vi.mock("../lib/github-utils.js", async () => {
+ const actual = await vi.importActual("../lib/github-utils.js");
+ return {
+ ...actual,
+ ghApi: vi.fn(),
+ };
+});
+
+describe("categorizeFiles", () => {
+ it("should count files and lines correctly", () => {
+ const files = [
+ { filename: "src/app.ts", additions: 10, deletions: 5 },
+ { filename: "src/utils.ts", additions: 20, deletions: 10 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.totalFiles).toBe(2);
+ expect(stats.totalLines).toBe(45);
+ expect(stats.maxSingleFileLines).toBe(30);
+ });
+
+ it("should identify docs-only PRs", () => {
+ const files = [
+ { filename: "README.md", additions: 10, deletions: 0 },
+ { filename: "docs/guide.rst", additions: 5, deletions: 2 },
+ { filename: "CHANGELOG.txt", additions: 3, deletions: 1 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.docsOnly).toBe(true);
+ expect(stats.testOnly).toBe(false);
+ });
+
+ it("should identify test-only PRs", () => {
+ const files = [
+ { filename: "src/__tests__/app.test.ts", additions: 50, deletions: 10 },
+ { filename: "tests/utils.spec.js", additions: 30, deletions: 5 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.testOnly).toBe(true);
+ expect(stats.docsOnly).toBe(false);
+ });
+
+ it("should identify rename-only PRs", () => {
+ const files = [
+ { filename: "src/newName.ts", status: "renamed", changes: 0, additions: 0, deletions: 0 },
+ { filename: "src/another.ts", status: "renamed", changes: 0, additions: 0, deletions: 0 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.renameOnly).toBe(true);
+ });
+
+ it("should identify lockfile-only PRs", () => {
+ const files = [
+ { filename: "package-lock.json", additions: 100, deletions: 50 },
+ { filename: "yarn.lock", additions: 200, deletions: 100 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.lockfileOnly).toBe(true);
+ });
+
+ it("should detect workflow files", () => {
+ const files = [
+ { filename: ".github/workflows/ci.yml", additions: 10, deletions: 0 },
+ { filename: "src/app.ts", additions: 5, deletions: 2 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.hasWorkflowFiles).toBe(true);
+ });
+
+ it("should detect auth-related files", () => {
+ const files = [
+ { filename: "src/auth/login.ts", additions: 20, deletions: 5 },
+ { filename: "src/session.ts", additions: 10, deletions: 3 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.hasAuthFiles).toBe(true);
+ });
+
+ it("should detect SQL/migration files", () => {
+ const files = [
+ { filename: "db/migrations/001_create_users.sql", additions: 30, deletions: 0 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.hasSqlFiles).toBe(true);
+ });
+
+ it("should detect infrastructure files", () => {
+ const files = [
+ { filename: "Dockerfile", additions: 15, deletions: 5 },
+ { filename: "terraform/main.tf", additions: 50, deletions: 10 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.hasInfraFiles).toBe(true);
+ });
+
+ it("should detect dependency files", () => {
+ const files = [
+ { filename: "package.json", additions: 3, deletions: 1 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.hasDepFiles).toBe(true);
+ });
+
+ it("should find security keywords in patches", () => {
+ const files = [
+ {
+ filename: "src/api.ts",
+ additions: 10,
+ deletions: 5,
+ patch: `+const token = process.env.API_TOKEN;\n+const result = await fetch(url);`,
+ },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.securityKeywords).toContain("token");
+ expect(stats.securityKeywords).toContain("fetch");
+ });
+
+ it("should find patterns keywords in patches", () => {
+ const files = [
+ {
+ filename: "src/component.tsx",
+ additions: 20,
+ deletions: 5,
+ patch: `+useEffect(() => {\n+ // Connect to walletconnect.com\n+}, []);`,
+ },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.patternsKeywords).toContain("useEffect");
+ expect(stats.patternsKeywords).toContain("walletconnect\\.com");
+ });
+
+ it("should exclude removed files from code file count", () => {
+ const files = [
+ { filename: "src/old.ts", status: "removed", additions: 0, deletions: 50 },
+ { filename: "src/new.ts", status: "added", additions: 30, deletions: 0 },
+ ];
+
+ const stats = categorizeFiles(files);
+
+ expect(stats.codeFiles).toBe(1);
+ });
+});
+
+describe("determineAgents", () => {
+ describe("label overrides", () => {
+ it("should skip all agents with skip-review label", () => {
+ const files = [{ filename: "src/app.ts", additions: 100, deletions: 50 }];
+
+ const result = determineAgents(files, { labels: ["skip-review"] });
+
+ expect(result.agents).toEqual([]);
+ expect(result.skipped).toEqual(["bug", "security", "patterns"]);
+ expect(result.reason).toContain("skip-review");
+ });
+
+ it("should spawn all agents with full-review label", () => {
+ const files = [{ filename: "README.md", additions: 5, deletions: 0 }];
+
+ const result = determineAgents(files, { labels: ["full-review"] });
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ expect(result.skipped).toEqual([]);
+ expect(result.reason).toContain("full-review");
+ });
+
+ it("should be case-insensitive for labels", () => {
+ const files = [{ filename: "src/app.ts", additions: 10, deletions: 5 }];
+
+ const result = determineAgents(files, { labels: ["FULL-REVIEW"] });
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ });
+ });
+
+ describe("forceAllAgents flag", () => {
+ it("should spawn all agents when forceAllAgents is true", () => {
+ const files = [{ filename: "README.md", additions: 5, deletions: 0 }];
+
+ const result = determineAgents(files, { forceAllAgents: true });
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ expect(result.reason).toContain("Force all agents");
+ });
+
+ it("should override skip-review label when forceAllAgents is true", () => {
+ const files = [{ filename: "src/app.ts", additions: 10, deletions: 5 }];
+
+ const result = determineAgents(files, {
+ labels: ["skip-review"],
+ forceAllAgents: true,
+ });
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ });
+ });
+
+ describe("edge cases", () => {
+ it("should skip all agents for docs-only changes", () => {
+ const files = [
+ { filename: "README.md", additions: 10, deletions: 5 },
+ { filename: "docs/guide.md", additions: 20, deletions: 10 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual([]);
+ expect(result.reason).toContain("Docs-only");
+ });
+
+ it("should skip all agents for rename-only changes", () => {
+ const files = [
+ { filename: "src/newName.ts", status: "renamed", changes: 0, additions: 0, deletions: 0 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual([]);
+ expect(result.reason).toContain("Rename-only");
+ });
+
+ it("should only spawn security for lockfile-only changes", () => {
+ const files = [
+ { filename: "package-lock.json", additions: 500, deletions: 200 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["security"]);
+ expect(result.reason).toContain("Lockfile-only");
+ expect(result.skipped).toContain("bug");
+ expect(result.skipped).toContain("patterns");
+ });
+
+ it("should only spawn bug for test-only changes", () => {
+ const files = [
+ { filename: "src/__tests__/app.test.ts", additions: 50, deletions: 10 },
+ { filename: "tests/utils.spec.js", additions: 30, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug"]);
+ expect(result.reason).toContain("Test-only");
+ expect(result.skipped).toContain("security");
+ expect(result.skipped).toContain("patterns");
+ });
+ });
+
+ describe("large PRs", () => {
+ it("should spawn all agents for PRs with >500 lines", () => {
+ const files = [
+ { filename: "src/big.ts", additions: 300, deletions: 250 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ expect(result.reason).toContain("Large PR");
+ expect(result.reason).toContain("550 lines");
+ });
+
+ it("should spawn all agents for PRs with >15 files", () => {
+ const files = Array.from({ length: 16 }, (_, i) => ({
+ filename: `src/file${i}.ts`,
+ additions: 5,
+ deletions: 2,
+ }));
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ expect(result.reason).toContain("Large PR");
+ expect(result.reason).toContain("16 files");
+ });
+ });
+
+ describe("security agent triggers", () => {
+ it("should spawn security for workflow files", () => {
+ const files = [
+ { filename: ".github/workflows/ci.yml", additions: 10, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("workflow files");
+ });
+
+ it("should spawn security for auth-related files", () => {
+ const files = [
+ { filename: "src/auth/login.ts", additions: 20, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("auth files");
+ });
+
+ it("should spawn security for SQL files", () => {
+ const files = [
+ { filename: "db/migrations/001.sql", additions: 30, deletions: 0 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("SQL/migration files");
+ });
+
+ it("should spawn security for infrastructure files", () => {
+ const files = [
+ { filename: "Dockerfile", additions: 15, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("infrastructure files");
+ });
+
+ it("should spawn security for dependency files", () => {
+ const files = [
+ { filename: "package.json", additions: 3, deletions: 1 },
+ { filename: "src/app.ts", additions: 10, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("dependency files");
+ });
+
+ it("should spawn security when security keywords found in patch", () => {
+ const files = [
+ {
+ filename: "src/api.ts",
+ additions: 10,
+ deletions: 5,
+ patch: `+const secret = getSecret();\n+const token = generateToken();`,
+ },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("security");
+ expect(result.reason).toContain("security keywords");
+ });
+ });
+
+ describe("patterns agent triggers", () => {
+ it("should spawn patterns for workflow files", () => {
+ const files = [
+ { filename: ".github/workflows/ci.yml", additions: 10, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("patterns");
+ expect(result.reason).toContain("workflow files");
+ });
+
+ it("should spawn patterns for large single files (>300 lines)", () => {
+ const files = [
+ { filename: "src/bigfile.ts", additions: 200, deletions: 150 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("patterns");
+ expect(result.reason).toContain("large file");
+ expect(result.reason).toContain("350 lines");
+ });
+
+ it("should spawn patterns for >5 code files", () => {
+ const files = Array.from({ length: 6 }, (_, i) => ({
+ filename: `src/file${i}.ts`,
+ additions: 10,
+ deletions: 5,
+ }));
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("patterns");
+ expect(result.reason).toContain("6 code files");
+ });
+
+ it("should spawn patterns when patterns keywords found", () => {
+ const files = [
+ {
+ filename: "src/component.tsx",
+ additions: 10,
+ deletions: 5,
+ patch: `+useEffect(() => { console.log('mounted'); }, []);`,
+ },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("patterns");
+ expect(result.reason).toContain("patterns keywords");
+ });
+ });
+
+ describe("small code PRs", () => {
+ it("should only spawn bug for trivial code changes", () => {
+ const files = [
+ { filename: "src/app.ts", additions: 5, deletions: 2 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug"]);
+ expect(result.skipped).toContain("security");
+ expect(result.skipped).toContain("patterns");
+ });
+
+ it("should include bug but not patterns for medium PR without triggers", () => {
+ const files = [
+ { filename: "src/app.ts", additions: 50, deletions: 20 },
+ { filename: "src/utils.ts", additions: 30, deletions: 10 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("bug");
+ expect(result.skipped).toContain("patterns");
+ });
+ });
+
+ describe("mixed file types", () => {
+ it("should apply heuristic to code files when mixed with docs", () => {
+ const files = [
+ { filename: "README.md", additions: 50, deletions: 20 },
+ { filename: "src/auth.ts", additions: 15, deletions: 5 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toContain("bug");
+ expect(result.agents).toContain("security");
+ expect(result.docsOnly).toBeUndefined(); // Mixed, not docs-only
+ });
+
+ it("should detect both security and patterns triggers", () => {
+ const files = [
+ { filename: ".github/workflows/ci.yml", additions: 10, deletions: 0 },
+ { filename: "src/auth/login.ts", additions: 200, deletions: 150 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug", "security", "patterns"]);
+ });
+ });
+
+ describe("empty or minimal PRs", () => {
+ it("should handle empty file list", () => {
+ const result = determineAgents([]);
+
+ expect(result.agents).toEqual([]);
+ expect(result.reason).toContain("Empty PR");
+ expect(result.skipped).toEqual(["bug", "security", "patterns"]);
+ });
+
+ it("should handle files with zero changes", () => {
+ const files = [
+ { filename: "src/app.ts", additions: 0, deletions: 0 },
+ ];
+
+ const result = determineAgents(files);
+
+ expect(result.agents).toEqual(["bug"]);
+ });
+ });
+});
+
+describe("fetchPrFiles", () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ });
+
+ const mockContext = {
+ repo: { owner: "walletconnect", repo: "test-repo" },
+ issue: { number: 123 },
+ };
+
+ it("should fetch files from GitHub API", async () => {
+ const mockFiles = [
+ { filename: "src/app.ts", additions: 10, deletions: 5 },
+ ];
+ ghApi.mockReturnValue(mockFiles);
+
+ const files = await fetchPrFiles(mockContext);
+
+ expect(ghApi).toHaveBeenCalledWith(
+ "/repos/walletconnect/test-repo/pulls/123/files"
+ );
+ expect(files).toEqual(mockFiles);
+ });
+
+ it("should return empty array on error", async () => {
+ ghApi.mockImplementation(() => {
+ throw new Error("API error");
+ });
+
+ const files = await fetchPrFiles(mockContext);
+
+ expect(files).toEqual([]);
+ });
+
+ it("should return empty array when API returns null", async () => {
+ ghApi.mockReturnValue(null);
+
+ const files = await fetchPrFiles(mockContext);
+
+ expect(files).toEqual([]);
+ });
+});
+
+describe("fetchPrLabels", () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ });
+
+ const mockContext = {
+ repo: { owner: "walletconnect", repo: "test-repo" },
+ issue: { number: 123 },
+ };
+
+ it("should fetch labels from GitHub API", async () => {
+ ghApi.mockReturnValue({
+ labels: [{ name: "bug" }, { name: "full-review" }],
+ });
+
+ const labels = await fetchPrLabels(mockContext);
+
+ expect(ghApi).toHaveBeenCalledWith(
+ "/repos/walletconnect/test-repo/pulls/123"
+ );
+ expect(labels).toEqual(["bug", "full-review"]);
+ });
+
+ it("should return empty array on error", async () => {
+ ghApi.mockImplementation(() => {
+ throw new Error("API error");
+ });
+
+ const labels = await fetchPrLabels(mockContext);
+
+ expect(labels).toEqual([]);
+ });
+
+ it("should handle PR with no labels", async () => {
+ ghApi.mockReturnValue({ labels: [] });
+
+ const labels = await fetchPrLabels(mockContext);
+
+ expect(labels).toEqual([]);
+ });
+
+ it("should handle missing labels field", async () => {
+ ghApi.mockReturnValue({});
+
+ const labels = await fetchPrLabels(mockContext);
+
+ expect(labels).toEqual([]);
+ });
+});
diff --git a/claude/auto-review/scripts/determine-agents.js b/claude/auto-review/scripts/determine-agents.js
new file mode 100644
index 0000000..63eed6f
--- /dev/null
+++ b/claude/auto-review/scripts/determine-agents.js
@@ -0,0 +1,396 @@
+#!/usr/bin/env node
+
+/**
+ * Determines which review subagents to spawn based on PR characteristics.
+ * Implements a conservative heuristic that errs on the side of spawning agents.
+ */
+
+import { ghApi, loadGitHubContext, createLogger } from "./lib/github-utils.js";
+
+const logger = createLogger("determine-agents");
+
+// All available agents
+const ALL_AGENTS = ["bug", "security", "patterns"];
+
+// File pattern matchers
+const PATTERNS = {
+ docs: /\.(md|txt|rst|mdx)$/i,
+ tests: /\.(test|spec)\.[jt]sx?$|__tests__\//i,
+ workflows: /\.github\/workflows\/.*\.ya?ml$/,
+ auth: /auth|login|session/i,
+ sql: /\.sql$|\/migrations\//i,
+ secrets: /\.env|secrets?|config\//i,
+ infra: /Dockerfile|\.tf$|\.tfvars$/,
+ lockfiles: /\.lock$|package-lock\.json$|yarn\.lock$|pnpm-lock\.yaml$|go\.sum$/,
+ deps: /package\.json$|go\.mod$|requirements\.txt$|Gemfile$/,
+};
+
+// Keywords that trigger security agent
+const SECURITY_KEYWORDS = [
+ "password",
+ "secret",
+ "token",
+ "api_key",
+ "apikey",
+ "credential",
+ "jwt",
+ "bearer",
+ "crypto",
+ "hash",
+ "encrypt",
+ "decrypt",
+ "md5",
+ "sha1",
+ "exec",
+ "spawn",
+ "shell",
+ "eval",
+ "query",
+ "sql",
+ "fetch",
+ "axios",
+ "http",
+ "redirect",
+ "cors",
+ "readFile",
+ "writeFile",
+ "fs\\.",
+ "path\\.join",
+];
+
+// Keywords that trigger patterns agent
+const PATTERNS_KEYWORDS = [
+ "walletconnect\\.com",
+ "reown\\.com",
+ "Cache-Control",
+ "max-age",
+ "useEffect",
+ "useMemo",
+ "useCallback",
+];
+
+/**
+ * Check if a file matches a pattern category
+ * @param {string} filename
+ * @param {string} category - Key from PATTERNS
+ * @returns {boolean}
+ */
+function matchesPattern(filename, category) {
+ return PATTERNS[category]?.test(filename) ?? false;
+}
+
+/**
+ * Check if patch content contains any keywords from a list
+ * @param {string} patch - Diff patch content
+ * @param {string[]} keywords
+ * @returns {string[]} - Matching keywords found
+ */
+function findKeywords(patch, keywords) {
+ if (!patch) return [];
+ const found = [];
+ for (const keyword of keywords) {
+ const regex = new RegExp(keyword, "i");
+ if (regex.test(patch)) {
+ found.push(keyword);
+ }
+ }
+ return found;
+}
+
+/**
+ * Categorize PR files and compute statistics
+ * @param {Array<{filename: string, status: string, additions: number, deletions: number, changes: number, patch?: string}>} files
+ * @returns {Object}
+ */
+export function categorizeFiles(files) {
+ const codeFiles = files.filter(
+ (f) => !matchesPattern(f.filename, "docs") && f.status !== "removed"
+ );
+
+ const stats = {
+ totalFiles: files.length,
+ codeFiles: codeFiles.length,
+ totalLines: files.reduce((sum, f) => sum + (f.additions || 0) + (f.deletions || 0), 0),
+ maxSingleFileLines: Math.max(
+ 0,
+ ...files.map((f) => (f.additions || 0) + (f.deletions || 0))
+ ),
+
+ // Edge case flags
+ isEmpty: files.length === 0,
+ docsOnly: files.length > 0 && files.every((f) => matchesPattern(f.filename, "docs")),
+ testOnly:
+ codeFiles.length > 0 &&
+ codeFiles.every((f) => matchesPattern(f.filename, "tests")),
+ renameOnly:
+ files.length > 0 &&
+ files.every((f) => f.status === "renamed" && (f.changes || 0) === 0),
+ lockfileOnly:
+ files.length > 0 && files.every((f) => matchesPattern(f.filename, "lockfiles")),
+
+ // Signal flags
+ hasWorkflowFiles: files.some((f) => matchesPattern(f.filename, "workflows")),
+ hasAuthFiles: files.some((f) => matchesPattern(f.filename, "auth")),
+ hasSqlFiles: files.some((f) => matchesPattern(f.filename, "sql")),
+ hasSecretFiles: files.some((f) => matchesPattern(f.filename, "secrets")),
+ hasInfraFiles: files.some((f) => matchesPattern(f.filename, "infra")),
+ hasDepFiles: files.some((f) => matchesPattern(f.filename, "deps")),
+ };
+
+ // Aggregate patches for keyword search
+ const allPatches = files
+ .map((f) => f.patch || "")
+ .filter(Boolean)
+ .join("\n");
+
+ stats.securityKeywords = findKeywords(allPatches, SECURITY_KEYWORDS);
+ stats.patternsKeywords = findKeywords(allPatches, PATTERNS_KEYWORDS);
+
+ return stats;
+}
+
+/**
+ * Determine which agents to spawn based on PR characteristics
+ * @param {Array<{filename: string, status: string, additions: number, deletions: number, changes: number, patch?: string}>} prFiles
+ * @param {{labels?: string[], forceAllAgents?: boolean}} metadata
+ * @returns {{agents: string[], reason: string, skipped: string[]}}
+ */
+export function determineAgents(prFiles, metadata = {}) {
+ const { labels = [], forceAllAgents = false } = metadata;
+
+ // Force all agents if requested
+ if (forceAllAgents) {
+ return {
+ agents: [...ALL_AGENTS],
+ reason: "Force all agents flag set",
+ skipped: [],
+ };
+ }
+
+ // Check override labels first
+ const labelSet = new Set(labels.map((l) => l.toLowerCase()));
+
+ if (labelSet.has("skip-review")) {
+ return {
+ agents: [],
+ reason: "skip-review label present",
+ skipped: [...ALL_AGENTS],
+ };
+ }
+
+ if (labelSet.has("full-review")) {
+ return {
+ agents: [...ALL_AGENTS],
+ reason: "full-review label override",
+ skipped: [],
+ };
+ }
+
+ // Categorize files
+ const stats = categorizeFiles(prFiles);
+
+ logger.log(
+ `Analyzing PR (${stats.totalFiles} files, ${stats.totalLines} lines changed)`
+ );
+ logger.log(
+ `File signals: hasWorkflowFiles=${stats.hasWorkflowFiles}, hasAuthFiles=${stats.hasAuthFiles}, hasInfraFiles=${stats.hasInfraFiles}`
+ );
+ logger.log(
+ `Content signals: securityKeywords=[${stats.securityKeywords.slice(0, 5).join(", ")}${stats.securityKeywords.length > 5 ? "..." : ""}]`
+ );
+
+ // Edge cases - no code to review
+ if (stats.isEmpty) {
+ return {
+ agents: [],
+ reason: "Empty PR (no files)",
+ skipped: [...ALL_AGENTS],
+ };
+ }
+
+ if (stats.docsOnly) {
+ return {
+ agents: [],
+ reason: "Docs-only change",
+ skipped: [...ALL_AGENTS],
+ };
+ }
+
+ if (stats.renameOnly) {
+ return {
+ agents: [],
+ reason: "Rename-only change (no content modifications)",
+ skipped: [...ALL_AGENTS],
+ };
+ }
+
+ // Edge cases - limited review
+ if (stats.lockfileOnly) {
+ return {
+ agents: ["security"],
+ reason: "Lockfile-only change (supply chain review)",
+ skipped: ["bug", "patterns"],
+ };
+ }
+
+ if (stats.testOnly) {
+ return {
+ agents: ["bug"],
+ reason: "Test-only change",
+ skipped: ["security", "patterns"],
+ };
+ }
+
+ // Large PR → all agents
+ if (stats.totalLines > 500 || stats.totalFiles > 15) {
+ return {
+ agents: [...ALL_AGENTS],
+ reason: `Large PR (${stats.totalFiles} files, ${stats.totalLines} lines)`,
+ skipped: [],
+ };
+ }
+
+ // Build agent list based on signals
+ const agents = ["bug"]; // Always include bug for code changes
+ const reasons = ["Code changes present"];
+ const skipped = [];
+
+ // Security agent triggers
+ const needsSecurity =
+ stats.hasWorkflowFiles ||
+ stats.hasAuthFiles ||
+ stats.hasSqlFiles ||
+ stats.hasSecretFiles ||
+ stats.hasInfraFiles ||
+ stats.hasDepFiles ||
+ stats.securityKeywords.length > 0;
+
+ if (needsSecurity) {
+ agents.push("security");
+ const secReasons = [];
+ if (stats.hasWorkflowFiles) secReasons.push("workflow files");
+ if (stats.hasAuthFiles) secReasons.push("auth files");
+ if (stats.hasSqlFiles) secReasons.push("SQL/migration files");
+ if (stats.hasSecretFiles) secReasons.push("config/secret files");
+ if (stats.hasInfraFiles) secReasons.push("infrastructure files");
+ if (stats.hasDepFiles) secReasons.push("dependency files");
+ if (stats.securityKeywords.length > 0) secReasons.push("security keywords");
+ reasons.push(`Security: ${secReasons.join(", ")}`);
+ } else {
+ skipped.push("security");
+ }
+
+ // Patterns agent triggers (conservative: >300 lines single file OR >5 files OR workflow files)
+ const needsPatterns =
+ stats.hasWorkflowFiles ||
+ stats.maxSingleFileLines > 300 ||
+ stats.codeFiles > 5 ||
+ stats.patternsKeywords.length > 0;
+
+ if (needsPatterns) {
+ agents.push("patterns");
+ const patReasons = [];
+ if (stats.hasWorkflowFiles) patReasons.push("workflow files");
+ if (stats.maxSingleFileLines > 300)
+ patReasons.push(`large file (${stats.maxSingleFileLines} lines)`);
+ if (stats.codeFiles > 5) patReasons.push(`${stats.codeFiles} code files`);
+ if (stats.patternsKeywords.length > 0) patReasons.push("patterns keywords");
+ reasons.push(`Patterns: ${patReasons.join(", ")}`);
+ } else {
+ skipped.push("patterns");
+ }
+
+ const result = {
+ agents,
+ reason: reasons.join("; "),
+ skipped,
+ };
+
+ logger.log(`Decision: Spawning [${agents.join(", ")}]`);
+ logger.log(`Reason: ${result.reason}`);
+ if (skipped.length > 0) {
+ logger.log(`Skipped: [${skipped.join(", ")}]`);
+ }
+
+ return result;
+}
+
+/**
+ * Fetch PR files from GitHub API
+ * @param {{repo: {owner: string, repo: string}, issue: {number: number}}} context
+ * @returns {Promise}
+ */
+export async function fetchPrFiles(context) {
+ const { owner, repo } = context.repo;
+ const prNumber = context.issue.number;
+ const endpoint = `/repos/${owner}/${repo}/pulls/${prNumber}/files`;
+
+ try {
+ const files = ghApi(endpoint);
+ return files || [];
+ } catch (error) {
+ logger.error(`Failed to fetch PR files: ${error.message}`);
+ return [];
+ }
+}
+
+/**
+ * Fetch PR labels from GitHub API
+ * @param {{repo: {owner: string, repo: string}, issue: {number: number}}} context
+ * @returns {Promise}
+ */
+export async function fetchPrLabels(context) {
+ const { owner, repo } = context.repo;
+ const prNumber = context.issue.number;
+ const endpoint = `/repos/${owner}/${repo}/pulls/${prNumber}`;
+
+ try {
+ const pr = ghApi(endpoint);
+ return (pr?.labels || []).map((l) => l.name);
+ } catch (error) {
+ logger.error(`Failed to fetch PR labels: ${error.message}`);
+ return [];
+ }
+}
+
+/**
+ * Main entry point when run as script
+ */
+async function main() {
+ const context = loadGitHubContext();
+
+ if (!context.issue.number) {
+ logger.error("No PR number found in context");
+ // Return all agents as fallback
+ console.log(JSON.stringify({ agents: ALL_AGENTS, reason: "No PR context", skipped: [] }));
+ process.exit(0);
+ }
+
+ // Check for force flag from environment
+ const forceAllAgents = process.env.FORCE_ALL_AGENTS === "true";
+
+ const [files, labels] = await Promise.all([
+ fetchPrFiles(context),
+ fetchPrLabels(context),
+ ]);
+
+ const result = determineAgents(files, { labels, forceAllAgents });
+
+ // Output as JSON for consumption by action.yml
+ console.log(JSON.stringify(result));
+}
+
+// Execution guard for direct invocation
+if (
+ import.meta.url === `file://${process.argv[1]}` ||
+ process.argv[1]?.endsWith("determine-agents.js")
+) {
+ main().catch((error) => {
+ logger.error(`Fatal error: ${error.message}`);
+ // Return all agents as fallback on error
+ console.log(
+ JSON.stringify({ agents: ALL_AGENTS, reason: "Error fallback", skipped: [] })
+ );
+ process.exit(0);
+ });
+}
From 21f0a9e4ba531bf898ae33afc25e9519d22c5d3a Mon Sep 17 00:00:00 2001
From: Ben Kremer
Date: Mon, 16 Feb 2026 15:36:15 +0700
Subject: [PATCH 6/6] feat(agents): expand agent scope to include best practice
recommendations
Add scope directive and best practices section to each review agent,
instructing them to proactively recommend missing best practices within
their domain (bugs, security, patterns) even when no explicit issue exists.
Best practice recommendations use LOW severity.
---
claude/auto-review/agents/review-bugs.md | 13 +++++++++++++
claude/auto-review/agents/review-patterns.md | 14 ++++++++++++++
claude/auto-review/agents/review-security.md | 14 ++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/claude/auto-review/agents/review-bugs.md b/claude/auto-review/agents/review-bugs.md
index 0ff4847..334eb24 100644
--- a/claude/auto-review/agents/review-bugs.md
+++ b/claude/auto-review/agents/review-bugs.md
@@ -6,6 +6,8 @@ You are a code reviewer. Provide actionable feedback on code changes.
Your specialization: finding bugs and functional issues.
+**Scope:** Your review is not limited strictly to the focus areas below. Within the bug/correctness domain, also proactively recommend missing best practices - even when no explicit bug exists.
+
## Your Focus Areas
Analyze the PR changes for:
@@ -46,6 +48,17 @@ Analyze the PR changes for:
- Function signature mismatches
- Incompatible type assignments
+## Best Practices to Recommend
+
+Beyond finding bugs, suggest improvements when you notice:
+- Missing defensive coding patterns (guard clauses, assertions)
+- Opportunities for better error handling structure
+- Input validation that could prevent future bugs
+- Null safety improvements (optional chaining, nullish coalescing)
+- Missing type narrowing or runtime checks
+
+Use **LOW** severity for best practice recommendations (vs actual bugs).
+
## Review Process
1. Read the full file content for each changed file to understand context
diff --git a/claude/auto-review/agents/review-patterns.md b/claude/auto-review/agents/review-patterns.md
index cf3f97d..68842a0 100644
--- a/claude/auto-review/agents/review-patterns.md
+++ b/claude/auto-review/agents/review-patterns.md
@@ -6,6 +6,8 @@ You are a code reviewer. Provide actionable feedback on code changes.
Your specialization: code quality, architecture, and domain-specific compliance.
+**Scope:** Your review is not limited strictly to the focus areas below. Within the code quality domain, also proactively recommend missing best practices - even when no explicit anti-pattern exists.
+
## Your Focus Areas
### Code Quality & Architecture
@@ -65,6 +67,18 @@ Your specialization: code quality, architecture, and domain-specific compliance.
- **MEDIUM:** Multi-step workflows without saga compensation → Add rollback/compensating events
- **MEDIUM:** State transitions without trace context → Add traceId/correlationId logging
+## Best Practices to Recommend
+
+Beyond finding anti-patterns, suggest improvements when you notice:
+- Opportunities for better code organization or structure
+- Missing abstractions that would improve maintainability
+- Testability improvements (dependency injection, pure functions)
+- Documentation needs for complex logic
+- Consistency improvements with existing codebase patterns
+- Opportunities for better naming or self-documenting code
+
+Use **LOW** severity for best practice recommendations (vs actual issues).
+
## Review Process
1. Read the full file content for each changed file to understand context
diff --git a/claude/auto-review/agents/review-security.md b/claude/auto-review/agents/review-security.md
index 3a274ea..70ae30b 100644
--- a/claude/auto-review/agents/review-security.md
+++ b/claude/auto-review/agents/review-security.md
@@ -6,6 +6,8 @@ You are a code reviewer. Provide actionable feedback on code changes.
Your specialization: finding security vulnerabilities.
+**Scope:** Your review is not limited strictly to the focus areas below. Within the security domain, also proactively recommend missing best practices and security hardening - even when no explicit vulnerability exists.
+
## Your Focus Areas
Analyze the PR changes for:
@@ -58,6 +60,18 @@ Analyze the PR changes for:
- ReDoS vulnerabilities (unsafe regex)
- Integer overflow/underflow
+## Best Practices to Recommend
+
+Beyond finding vulnerabilities, suggest security hardening when you notice:
+- Missing input validation/sanitization (even if not currently exploitable)
+- Opportunities for defense-in-depth (multiple security layers)
+- Missing security headers or secure defaults
+- Logging improvements for security audit trails
+- Rate limiting or abuse prevention opportunities
+- Secure coding patterns that could prevent future vulnerabilities
+
+Use **LOW** severity for best practice recommendations (vs actual vulnerabilities).
+
## Review Process
1. Read the full file content for each changed file to understand context