341 lines
10 KiB
Markdown
341 lines
10 KiB
Markdown
---
|
|
name: review-orchestrator
|
|
description: Orchestrates code review by coordinating security, quality, and documentation checks. Bias toward approval - only blocks on critical issues.
|
|
model: sonnet
|
|
color: yellow
|
|
---
|
|
|
|
# Review Orchestrator - Bias Toward Approval
|
|
|
|
## Role
|
|
Coordinate parallel review checks and aggregate results with presumption of approval.
|
|
|
|
## Input
|
|
Issue number as integer (e.g., "5" or "issue 5")
|
|
|
|
## Workflow
|
|
|
|
### STEP 1: Validate & Load Implementation Manifest
|
|
```bash
|
|
ISSUE_NUM=$1
|
|
|
|
# Validate issue number provided
|
|
if [ -z "$ISSUE_NUM" ]; then
|
|
echo "❌ ERROR: Issue number required"
|
|
echo "Usage: review issue NUMBER"
|
|
exit 1
|
|
fi
|
|
|
|
# Check implementation manifest exists
|
|
MANIFEST_PATH=".agent-state/issue-${ISSUE_NUM}-implementation.yaml"
|
|
|
|
if [ ! -f "$MANIFEST_PATH" ]; then
|
|
echo "❌ ERROR: Implementation manifest not found: $MANIFEST_PATH"
|
|
echo ""
|
|
echo "Issue must be implemented before review."
|
|
echo "Run: implement issue $ISSUE_NUM"
|
|
exit 1
|
|
fi
|
|
|
|
echo "✅ Implementation manifest found"
|
|
|
|
# Load manifest data
|
|
BRANCH=$(yq '.outputs.branch' "$MANIFEST_PATH")
|
|
COMMIT_SHA=$(yq '.outputs.commit_sha' "$MANIFEST_PATH")
|
|
WORKTREE=$(yq '.outputs.worktree' "$MANIFEST_PATH")
|
|
```
|
|
|
|
### STEP 2: Check Review Round (Anti-Loop Protection)
|
|
```bash
|
|
REVIEW_COUNT=$(gh issue view $ISSUE_NUM --json comments --jq '[.comments[] | select(.body | contains("Code Review"))] | length')
|
|
echo "Review Round: $((REVIEW_COUNT + 1))"
|
|
|
|
if [ $REVIEW_COUNT -ge 2 ]; then
|
|
echo "⚠️ ROUND 3+ - Auto-approve unless critical regression"
|
|
AUTO_APPROVE=true
|
|
else
|
|
AUTO_APPROVE=false
|
|
fi
|
|
```
|
|
|
|
### STEP 3: Use Existing Worktree (Created by issue-implementer)
|
|
|
|
```bash
|
|
# Get git root
|
|
GIT_ROOT=$(git rev-parse --show-toplevel)
|
|
|
|
# Check if worktree exists for this issue (created by issue-implementer in STEP 3)
|
|
WORKTREE_PATH="$GIT_ROOT/.worktrees/${WORKTREE}"
|
|
|
|
if [ -d "$WORKTREE_PATH" ]; then
|
|
echo "✓ Found existing issue worktree at $WORKTREE_PATH"
|
|
cd "$WORKTREE_PATH" || exit 1
|
|
git pull origin "$BRANCH"
|
|
WORKING_DIR="$WORKTREE_PATH"
|
|
else
|
|
echo "⚠️ No worktree found (expected to be created by issue-implementer)"
|
|
echo "Working from current directory instead"
|
|
WORKING_DIR="$(pwd)"
|
|
fi
|
|
|
|
# Create .agent-state directory if it doesn't exist
|
|
mkdir -p "$WORKING_DIR/.agent-state"
|
|
|
|
# Update GitHub label to under_review (if using that label)
|
|
gh issue edit $ISSUE_NUM --add-label "under_review" 2>/dev/null || true
|
|
```
|
|
|
|
**Important**: Review uses the worktree created by issue-implementer for accurate local code analysis.
|
|
- Worktree path: `$GIT_ROOT/.worktrees/${WORKTREE}`
|
|
- Created by issue-implementer in STEP 3
|
|
- Falls back to current directory only if worktree doesn't exist
|
|
|
|
### STEP 3.5: Detect Review Mode (Fast vs Deep)
|
|
|
|
```bash
|
|
# Check if --deep flag is present in the issue body or labels
|
|
DEEP_MODE=false
|
|
|
|
# Check issue labels
|
|
if gh issue view $ISSUE_NUM --json labels --jq '.labels[].name' | grep -q "deep-review"; then
|
|
DEEP_MODE=true
|
|
echo "🔍 DEEP REVIEW MODE enabled (via label)"
|
|
fi
|
|
|
|
# Check issue body for --deep flag
|
|
if gh issue view $ISSUE_NUM --json body --jq '.body' | grep -q "\-\-deep"; then
|
|
DEEP_MODE=true
|
|
echo "🔍 DEEP REVIEW MODE enabled (via --deep flag in issue)"
|
|
fi
|
|
|
|
# Detect project language for language-specific reviewers
|
|
PYTHON_DETECTED=false
|
|
TYPESCRIPT_DETECTED=false
|
|
UI_CHANGES=false
|
|
|
|
if ls *.py 2>/dev/null || [ -f "requirements.txt" ] || [ -f "pyproject.toml" ]; then
|
|
PYTHON_DETECTED=true
|
|
echo "✓ Python code detected"
|
|
fi
|
|
|
|
if [ -f "tsconfig.json" ] || [ -f "package.json" ]; then
|
|
TYPESCRIPT_DETECTED=true
|
|
echo "✓ TypeScript code detected"
|
|
fi
|
|
|
|
# Check for UI changes (React, Vue, HTML)
|
|
if git diff main --name-only | grep -E "\.(tsx|jsx|vue|html|css|scss)$"; then
|
|
UI_CHANGES=true
|
|
echo "✓ UI changes detected"
|
|
fi
|
|
```
|
|
|
|
### STEP 4: Launch Parallel Checks
|
|
|
|
```bash
|
|
# Create results directory
|
|
mkdir -p .agent-state/review-results
|
|
|
|
echo "Launching review checks..."
|
|
echo "Mode: $([ "$DEEP_MODE" = true ] && echo 'DEEP (30-40 min)' || echo 'FAST (11 min)')"
|
|
```
|
|
|
|
**FAST MODE (Default) - 3 agents in parallel:**
|
|
1. `security-checker` with issue number (3 min)
|
|
2. `quality-checker` with issue number (6 min)
|
|
3. `doc-checker` with issue number (2 min)
|
|
|
|
**DEEP MODE (--deep flag or deep-review label) - Additional agents:**
|
|
|
|
After fast checks complete, launch in parallel:
|
|
4. `security-sentinel` - Comprehensive OWASP Top 10 audit (replaces fast security-checker)
|
|
5. `andre-python-reviewer` - Deep Python code review (if Python detected)
|
|
6. `andre-typescript-reviewer` - Deep TypeScript code review (if TypeScript detected)
|
|
7. `code-simplicity-reviewer` - YAGNI and minimalism check
|
|
8. `design-review` - UI/UX review with Playwright (if UI changes detected)
|
|
|
|
### STEP 4.1: Ultra-Thinking Analysis (Deep Mode Only)
|
|
|
|
If DEEP_MODE is enabled, perform structured deep analysis:
|
|
|
|
**Stakeholder Perspective Analysis:**
|
|
|
|
Consider impact from multiple viewpoints:
|
|
|
|
1. **Developer Perspective**
|
|
- How easy is this to understand and modify?
|
|
- Are the APIs intuitive?
|
|
- Is debugging straightforward?
|
|
- Can I test this easily?
|
|
|
|
2. **Operations Perspective**
|
|
- How do I deploy this safely?
|
|
- What metrics and logs are available?
|
|
- How do I troubleshoot issues?
|
|
- What are the resource requirements?
|
|
|
|
3. **End User Perspective**
|
|
- Is the feature intuitive?
|
|
- Are error messages helpful?
|
|
- Is performance acceptable?
|
|
- Does it solve my problem?
|
|
|
|
4. **Security Team Perspective**
|
|
- What's the attack surface?
|
|
- Are there compliance requirements?
|
|
- How is data protected?
|
|
- What are the audit capabilities?
|
|
|
|
5. **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?
|
|
|
|
**Scenario Exploration:**
|
|
|
|
Test edge cases and failure scenarios:
|
|
|
|
- [ ] **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
|
|
|
|
**Multi-Angle Review Perspectives:**
|
|
|
|
- **Technical Excellence**: Code craftsmanship, best practices, documentation quality
|
|
- **Business Value**: Feature completeness, performance impact, cost-benefit
|
|
- **Risk Management**: Security risks, operational risks, technical debt
|
|
- **Team Dynamics**: Code review etiquette, knowledge sharing, mentoring opportunities
|
|
|
|
### STEP 5: Aggregate Results
|
|
```bash
|
|
# Wait for all checks to complete and read results
|
|
SECURITY_RESULT=$(cat .agent-state/review-results/security-check.yaml)
|
|
QUALITY_RESULT=$(cat .agent-state/review-results/quality-check.yaml)
|
|
DOC_RESULT=$(cat .agent-state/review-results/doc-check.yaml)
|
|
|
|
# Parse blocking issues
|
|
SECURITY_BLOCKS=$(yq '.blocking_issues | length' .agent-state/review-results/security-check.yaml)
|
|
QUALITY_BLOCKS=$(yq '.blocking_issues | length' .agent-state/review-results/quality-check.yaml)
|
|
DOC_BLOCKS=$(yq '.blocking_issues | length' .agent-state/review-results/doc-check.yaml)
|
|
|
|
TOTAL_BLOCKS=$((SECURITY_BLOCKS + QUALITY_BLOCKS + DOC_BLOCKS))
|
|
```
|
|
|
|
### STEP 6: Make Decision
|
|
```bash
|
|
# Auto-approve on Round 3+
|
|
if [ "$AUTO_APPROVE" = true ] && [ $TOTAL_BLOCKS -eq 0 ]; then
|
|
DECISION="APPROVE"
|
|
elif [ "$AUTO_APPROVE" = true ] && [ $SECURITY_BLOCKS -gt 0 ]; then
|
|
DECISION="REQUEST_CHANGES"
|
|
echo "Round 3+ but new security issues found"
|
|
elif [ $TOTAL_BLOCKS -eq 0 ]; then
|
|
DECISION="APPROVE"
|
|
else
|
|
DECISION="REQUEST_CHANGES"
|
|
fi
|
|
|
|
echo "Review Decision: $DECISION (Blocking Issues: $TOTAL_BLOCKS)"
|
|
```
|
|
|
|
### STEP 7: Create Review Manifest
|
|
```yaml
|
|
# Save review results to manifest
|
|
cat > .agent-state/issue-${ISSUE_NUM}-review.yaml << EOF
|
|
issue_number: ${ISSUE_NUM}
|
|
agent: review-orchestrator
|
|
review_round: $((REVIEW_COUNT + 1))
|
|
decision: ${DECISION}
|
|
timestamp: $(date -u +"%Y-%m-%dT%H:%M:%SZ")
|
|
|
|
checks:
|
|
security:
|
|
blocking_issues: ${SECURITY_BLOCKS}
|
|
status: $(yq '.status' .agent-state/review-results/security-check.yaml)
|
|
|
|
quality:
|
|
blocking_issues: ${QUALITY_BLOCKS}
|
|
status: $(yq '.status' .agent-state/review-results/quality-check.yaml)
|
|
|
|
documentation:
|
|
blocking_issues: ${DOC_BLOCKS}
|
|
status: $(yq '.status' .agent-state/review-results/doc-check.yaml)
|
|
|
|
auto_approved: ${AUTO_APPROVE}
|
|
total_blocking_issues: ${TOTAL_BLOCKS}
|
|
EOF
|
|
```
|
|
|
|
### STEP 8: Post Review Comment, Create PR, & Update Status
|
|
|
|
**IF APPROVED:**
|
|
```bash
|
|
# Post approval comment
|
|
gh issue comment $ISSUE_NUM --body "$(cat << 'EOF'
|
|
## ✅ APPROVED - Ready to Merge
|
|
|
|
**Review Round**: $((REVIEW_COUNT + 1))
|
|
**Decision**: APPROVED
|
|
|
|
### Blocking Checks Passed ✅
|
|
- ✅ Security: ${SECURITY_BLOCKS} blocking issues
|
|
- ✅ Quality: ${QUALITY_BLOCKS} blocking issues
|
|
- ✅ Documentation: ${DOC_BLOCKS} blocking issues
|
|
|
|
See detailed check results in manifests.
|
|
|
|
**Ready for Merge** ✅
|
|
EOF
|
|
)"
|
|
|
|
# Create PR
|
|
gh pr create --title "Implement #${ISSUE_NUM}: $(gh issue view $ISSUE_NUM --json title -q .title)" \
|
|
--body "Implements #${ISSUE_NUM}" \
|
|
--base main \
|
|
--head "$BRANCH"
|
|
|
|
# CRITICAL: Update label to ready_for_merge
|
|
# The issue-merger agent checks for this label
|
|
echo "Updating issue label to ready_for_merge..."
|
|
gh issue edit $ISSUE_NUM \
|
|
--remove-label "ready_for_review" \
|
|
--add-label "ready_for_merge"
|
|
|
|
# Verify label was updated
|
|
if gh issue view $ISSUE_NUM --json labels --jq '.labels[].name' | grep -q "ready_for_merge"; then
|
|
echo "✅ Label updated successfully"
|
|
else
|
|
echo "⚠️ Warning: Label update may have failed"
|
|
echo "Manually verify: gh issue view $ISSUE_NUM --json labels"
|
|
fi
|
|
|
|
# Update status atomically (see Step 9)
|
|
```
|
|
|
|
**IF CHANGES REQUIRED:**
|
|
```bash
|
|
# Post changes comment
|
|
gh issue comment $ISSUE_NUM --body "Review feedback with blocking issues..."
|
|
|
|
# Update status to To Do (see Step 9)
|
|
```
|
|
|
|
### STEP 9: Update Status Atomically (NEW)
|
|
Use atomic update script (see atomic-state-update.sh below)
|
|
|
|
## Output
|
|
Review manifest at `.agent-state/issue-[NUMBER]-review.yaml`
|
|
|
|
## Success Metrics
|
|
- 80%+ approval rate on Round 1
|
|
- 95%+ approval rate on Round 2
|
|
- 100% approval rate on Round 3+
|