/vt-d-wf-review¶
Perform exhaustive code reviews using multi-agent analysis, ultra-thinking, and worktrees
Plugin: vt-product-dev
Usage: /vt-d-wf-review [PR number, GitHub URL, branch name, or latest]
Review Command¶
Introduction¶
Prerequisites¶
gh) installed and authenticated
- Clean main/master branch
- Proper permissions to create worktrees and access the repository
- For document reviews: Path to a markdown file or document
Main Tasks¶
1. Determine Review Target & Setup (ALWAYS FIRST)¶
Immediate Actions:¶
- [ ] Determine review type: PR number (numeric), GitHub URL, file path (.md), or empty (current branch)
- [ ] Check current git branch
- [ ] If ALREADY on the target branch (PR branch, requested branch name, or the branch already checked out for review) → proceed with analysis on current branch
- [ ] If DIFFERENT branch than the review target → offer to use worktree: "Use git-worktree skill for isolated Call
skill: git-worktreewith branch name - [ ] Fetch PR metadata using
gh pr view --jsonfor title, body, files, linked issues - [ ] Set up language-specific analysis tools
- [ ] Prepare security scanning environment
- [ ] Make sure we are on the branch we are reviewing. Use gh pr checkout to switch to the branch or manually checkout the branch.
Ensure that the code is ready for analysis (either in worktree or on current branch). ONLY then proceed to the next step.
Parallel Agents to review the PR:¶
Run ALL or most of these agents at the same time.
IMPORTANT: Conditional dispatch — Before invoking each agent, check if its definition file exists in the agents directory. If the agent file does not exist, skip it with a note in the review output: "Skipped [agent-name]: agent definition not found." This prevents errors from missing agents and is forward-compatible (new agents are picked up automatically when added).
Always available (core review agents):
- Task angular-reviewer(PR content) — if
angular.jsonexists - Task nestjs-reviewer(PR content) — if
nest-cli.jsonexists - Task pattern-recognition-specialist(PR content)
- Task architecture-strategist(PR content)
- Task security-sentinel(PR content)
- Task performance-oracle(PR content)
- Task data-integrity-guardian(PR content)
- Task agent-native-reviewer(PR content) - Verify new features are agent-accessible
Conditional — invoke only if agent file exists:
- Task code-philosopher(PR content) — if agents/review/code-philosopher.md exists
- Task devops-harmony-analyst(PR content) — if agents/review/devops-harmony-analyst.md exists
- Task dependency-detective(PR content) — if agents/review/dependency-detective.md exists
- Task git-history-analyzer(PR content) — if agents/review/git-history-analyzer.md exists
- If turbo is used: Task rails-turbo-expert(PR content) — if agents/review/rails-turbo-expert.md exists
Conditional Agents (Run if applicable):¶
These agents are run ONLY when the PR matches specific criteria. Check the PR files list to determine if they apply:
If PR contains database migrations (db/migrate/*.rb files) or data backfills:
- Task data-migration-expert(PR content) — if agents/review/data-migration-expert.md exists; validates ID mappings match production, checks for swapped values, verifies rollback safety
- Task deployment-verification-agent(PR content) - Creates Go/No-Go deployment checklist with SQL verification queries
When to run migration agents:
- PR includes files matching db/migrate/*.rb
- PR modifies columns that store IDs, enums, or mappings
- PR includes data backfill scripts or rake tasks
- PR changes how data is read/written (e.g., changing from FK to string column)
- PR title/body mentions: migration, backfill, data transformation, ID mapping
What these agents check:
- data-migration-expert: Verifies hard-coded mappings match production reality (prevents swapped IDs), checks for orphaned associations, validates dual-write patterns
- deployment-verification-agent: Produces executable pre/post-deploy checklists with SQL queries, rollback procedures, and monitoring plans
4. Ultra-Thinking Deep Dive Phases¶
Phase 3: Stakeholder Perspective Analysis¶
-
Developer Perspective
-
How easy is this to understand and modify?
- Are the APIs intuitive?
- Is debugging straightforward?
-
Can I test this easily?
-
Operations Perspective
-
How do I deploy this safely?
- What metrics and logs are available?
- How do I troubleshoot issues?
-
What are the resource requirements?
-
End User Perspective
-
Is the feature intuitive?
- Are error messages helpful?
- Is performance acceptable?
-
Does it solve my problem?
-
Security Team Perspective
-
What's the attack surface?
- Are there compliance requirements?
- How is data protected?
-
What are the audit capabilities?
-
Business Perspective
- What's the ROI?
- Are there legal/compliance risks?
- How does this affect time-to-market?
- What's the total cost of ownership?
Phase 4: Scenario Exploration¶
- [ ] Happy Path: Normal operation with valid inputs
- [ ] Invalid Inputs: Null, empty, malformed data
- [ ] Boundary Conditions: Min/max values, empty collections
- [ ] Concurrent Access: Race conditions, deadlocks
- [ ] Scale Testing: 10x, 100x, 1000x normal load
- [ ] Network Issues: Timeouts, partial failures
- [ ] Resource Exhaustion: Memory, disk, connections
- [ ] Security Attacks: Injection, overflow, DoS
- [ ] Data Corruption: Partial writes, inconsistency
- [ ] Cascading Failures: Downstream service issues
6. Multi-Angle Review Perspectives¶
Technical Excellence Angle¶
- Code craftsmanship evaluation
- Engineering best practices
- Technical documentation quality
- Tooling and automation assessment
Business Value Angle¶
- Feature completeness validation
- Performance impact on users
- Cost-benefit analysis
- Time-to-market considerations
Risk Management Angle¶
- Security risk assessment
- Operational risk evaluation
- Compliance risk verification
- Technical debt accumulation
Team Dynamics Angle¶
- Code review etiquette
- Knowledge sharing effectiveness
- Collaboration patterns
- Mentoring opportunities
4. Simplification and Minimalism Review¶
Run the Task code-simplicity-reviewer() to see if we can simplify the code.
5. Findings Synthesis and Todo Creation Using file-todos Skill¶
Step 1: Synthesize All Findings¶
- [ ] Collect findings from all parallel agents
- [ ] Categorize by type: security, performance, architecture, quality, etc.
- [ ] Assign severity levels: 🔴 CRITICAL (P1), 🟡 IMPORTANT (P2), 🔵 NICE-TO-HAVE (P3)
- [ ] Remove duplicate or overlapping findings
- [ ] Estimate effort for each finding (Small/Medium/Large)
Step 2: Create Todo Files Using file-todos Skill¶
Implementation Options:
Option A: Direct File Creation (Fast)
- Create todo files directly using Write tool
- All findings in parallel for speed
- Use standard template from
plugins/vt-base/assets/todo-template.md - Follow naming convention:
{issue_id}-pending-{priority}-{description}.md
Option B: Sub-Agents in Parallel (Recommended for Scale) For large PRs with 15+ findings, use sub-agents to create finding files in parallel:
# Launch multiple finding-creator agents in parallel
Task() - Create todos for first finding
Task() - Create todos for second finding
Task() - Create todos for third finding
etc. for each finding.
Sub-agents can:
- Process multiple findings simultaneously
- Write detailed todo files with all sections filled
- Organize findings by severity
- Create comprehensive Proposed Solutions
- Add acceptance criteria and work logs
- Complete much faster than sequential processing
Execution Strategy:
- Synthesize all findings into categories (P1/P2/P3)
- Group findings by severity
- Launch 3 parallel sub-agents (one per severity level)
- Each sub-agent creates its batch of todos using the file-todos skill
- Consolidate results and present summary
Process (Using file-todos Skill):
-
For each finding:
-
Determine severity (P1/P2/P3)
- Write detailed Problem Statement and Findings
- Create 2-3 Proposed Solutions with pros/cons/effort/risk
- Estimate effort (Small/Medium/Large)
-
Add acceptance criteria and work log
-
Use file-todos skill for structured todo management:
The skill provides:
- Template location:
plugins/vt-base/assets/todo-template.md - Naming convention:
{issue_id}-{status}-{priority}-{description}.md - YAML frontmatter structure: status, priority, issue_id, tags, dependencies
-
All required sections: Problem Statement, Findings, Solutions, etc.
-
Create todo files in parallel:
- Examples:
001-pending-p1-path-traversal-vulnerability.md
002-pending-p1-api-response-validation.md
003-pending-p2-concurrency-limit.md
004-pending-p3-unused-parameter.md
- Follow template structure:
plugins/vt-base/assets/todo-template.md
Todo File Structure (from template):
Each todo must include:
- YAML frontmatter: status, priority, issue_id, tags, dependencies
- Problem Statement: What's broken/missing, why it matters
- Findings: Discoveries from agents with evidence/location
- Proposed Solutions: 2-3 options, each with pros/cons/effort/risk
- Recommended Action: (Filled during triage, leave blank initially)
- Technical Details: Affected files, components, database changes
- Acceptance Criteria: Testable checklist items
- Work Log: Dated record with actions and learnings
- Resources: Links to PR, issues, documentation, similar patterns
File naming convention:
{issue_id}-{status}-{priority}-{description}.md
Examples:
- 001-pending-p1-security-vulnerability.md
- 002-pending-p2-performance-optimization.md
- 003-pending-p3-code-cleanup.md
Status values:
pending- New findings, needs triage/decisionready- Approved by manager, ready to workcomplete- Work finished
Priority values:
p1- Critical (blocks merge, security/data issues)p2- Important (should fix, architectural/performance)p3- Nice-to-have (enhancements, cleanup)
Tagging: Always add code-review tag, plus: security, performance, architecture, rails, quality, etc.
Step 3: Summary Report¶
After creating all todo files, present comprehensive summary:
## ✅ Code Review Complete
**Review Target:** PR #XXXX - [PR Title] **Branch:** [branch-name]
### Findings Summary:
- **Total Findings:** [X]
- **🔴 CRITICAL (P1):** [count] - BLOCKS MERGE
- **🟡 IMPORTANT (P2):** [count] - Should Fix
- **🔵 NICE-TO-HAVE (P3):** [count] - Enhancements
### Created Todo Files:
**P1 - Critical (BLOCKS MERGE):**
- `001-pending-p1-{finding}.md` - {description}
- `002-pending-p1-{finding}.md` - {description}
**P2 - Important:**
- `003-pending-p2-{finding}.md` - {description}
- `004-pending-p2-{finding}.md` - {description}
**P3 - Nice-to-Have:**
- `005-pending-p3-{finding}.md` - {description}
### Review Agents Used:
- angular-reviewer (if angular.json exists)
- nestjs-reviewer (if nest-cli.json exists)
- security-sentinel
- performance-oracle
- architecture-strategist
- agent-native-reviewer
- [other agents]
### Next Steps:
1. **Address P1 Findings**: CRITICAL - must be fixed before merge
- Review each P1 todo in detail
- Implement fixes or request exemption
- Verify fixes before merging PR
2. **Triage All Todos**:
```bash
ls todos/*-pending-*.md # View all pending todos
/vt-d-triage # Use slash command for interactive triage
```
- Work on Approved Todos:
- Track Progress:
- Rename file when status changes: pending → ready → complete
- Update Work Log as you work
- Commit todos:
git add todos/ && git commit -m "refactor: add code review findings"
Severity Breakdown:¶
🔴 P1 (Critical - Blocks Merge):
- Security vulnerabilities
- Data corruption risks
- Breaking changes
- Critical architectural issues
🟡 P2 (Important - Should Fix):
- Performance issues
- Significant architectural concerns
- Major code quality problems
- Reliability issues
🔵 P3 (Nice-to-Have):
- Minor improvements
- Code cleanup
- Optimization opportunities
- Documentation updates
### 7. End-to-End Testing (Required for Web Projects)
<detect_project_type>
**First, detect the project type from PR files:**
| Indicator | Project Type |
|-----------|--------------|
| `*.xcodeproj`, `*.xcworkspace`, `Package.swift` (iOS) | iOS/macOS |
| `package.json` (react/vue/svelte/angular/next/nuxt/remix), `app/views/*`, `*.html.*`, `playwright.config.*`, `vite.config.*` | Web |
| Both iOS files AND web files | Hybrid (test both) |
| None of the above (pure API, CLI, library) | Non-web (skip browser tests) |
</detect_project_type>
<offer_testing>
After presenting the Summary Report, **automatically determine testing requirements** based on project type:
**For Web Projects (browser testing REQUIRED — no user choice):**
```markdown
**Web project detected — running browser tests automatically.**
Browser testing is mandatory for all web UI changes. Dispatching Playwright tests now...
Automatically dispatch browser testing without asking. Do NOT offer "skip" for web projects.
For Non-Web Projects (backend API, CLI, library):
**No browser testing needed** (non-web project detected).
This appears to be a backend/CLI/library project with no web UI.
For iOS Projects:
**"Want to run Xcode simulator tests on the app?"**
1. Yes - run `/xcode-test`
2. No - skip (provide justification)
For Hybrid Projects (e.g., Rails + Hotwire Native):
**Web browser tests will run automatically** (web project detected).
**Additionally, want to run iOS simulator tests?**
1. Yes - also run `/xcode-test`
2. No - web tests only
IMPORTANT: Browser testing via Playwright is optional ONLY for: - Backend API changes with no UI modifications - CLI tools and libraries - Infrastructure/deployment changes
For ALL changes touching web UI, browser testing runs automatically and must pass.
If User Accepts Web Testing:¶
Spawn a subagent to run browser tests (preserves main context):
Task general-purpose("Run /test-browser for PR #[number]. Test all affected pages, check for console errors, handle failures by creating todos and fixing.")
The subagent will: 1. Identify pages affected by the PR 2. Navigate to each page and capture snapshots (using Playwright MCP or agent-browser CLI) 3. Check for console errors 4. Test critical interactions 5. Pause for human verification on OAuth/email/payment flows 6. Create P1 todos for any failures 7. Fix and retry until all tests pass
Standalone: /test-browser [PR number]
If User Accepts iOS Testing:¶
Spawn a subagent to run Xcode tests (preserves main context):
Task general-purpose("Run /xcode-test for scheme [name]. Build for simulator, install, launch, take screenshots, check for crashes.")
The subagent will: 1. Verify XcodeBuildMCP is installed 2. Discover project and schemes 3. Build for iOS Simulator 4. Install and launch app 5. Take screenshots of key screens 6. Capture console logs for errors 7. Pause for human verification (Sign in with Apple, push, IAP) 8. Create P1 todos for any failures 9. Fix and retry until all tests pass
Standalone: /xcode-test [scheme]
8. Codify Decision Gate¶
After code review is complete and todos are created, present the Codify decision:
✅ Code Review Complete
**What should we codify from this work?**
1. **Document solution** - Run `/vt-c-compound` (if non-trivial problem solved)
2. **Extract skill** - Create new learning skill from patterns discovered
3. **Update existing skill** - Add patterns to existing skill references
4. **Promote pattern** - Add to Required Reading (critical patterns)
5. **Skip** - No codification needed
6. **Multiple** - Run multiple codification steps
**Decision Logic Hints:**
- If PR contains bug fixes → Consider Option 1
- If PR introduces new domain knowledge → Consider Option 2
- If PR refines existing patterns → Consider Option 3
- If P1 findings revealed critical pattern → Consider Option 4
Handle User Response:
Option 1: Document solution
# Launch compound-docs skill with PR context
Task general-purpose("Document the solution from PR #{pr_number} using /vt-c-compound. Extract the problem, investigation steps, root cause, and solution from the PR changes and review findings.")
Option 2: Extract skill
# Launch skill-extractor agent
Task skill-extractor("Analyze PR #{pr_number} for skill extraction opportunities. Check if patterns should become a new learning skill.")
Option 3: Update existing skill
- Prompt: "Which skill? (provide name or describe domain)"
- Search for matching skill in
skills/ - Add relevant patterns to skill's
references/directory - Confirm: "✓ Updated {skill-name} with patterns from PR #{pr_number}"
Option 4: Promote pattern
- Extract critical pattern from PR findings
- Add to
docs/solutions/patterns/cora-critical-patterns.md - Use format: ❌ WRONG vs ✅ CORRECT
- Log to metrics/pattern-promotions.json with
auto_promoted: false - Confirm: "✓ Added to Required Reading as Pattern #{number}"
Option 5: Skip
- Return to main workflow
- No codification needed for this PR
Option 6: Multiple
- Execute multiple options selected by user
- Run in sequence
Important: P1 Findings Block Merge¶
Any 🔴 P1 (CRITICAL) findings must be addressed before merging the PR. Present these prominently and ensure they're resolved before accepting the PR. ```