diff --git a/.claude/commands/workflow/review-fix.md b/.claude/commands/workflow/review-fix.md new file mode 100644 index 00000000..79fae007 --- /dev/null +++ b/.claude/commands/workflow/review-fix.md @@ -0,0 +1,429 @@ +--- +name: review-fix +description: Automated fixing of code review findings with AI-powered planning and coordinated execution. Uses intelligent grouping, multi-stage timeline coordination, and test-driven verification. +argument-hint: " [--resume] [--max-iterations=N]" +allowed-tools: SlashCommand(*), TodoWrite(*), Read(*), Bash(*), Task(*), Edit(*), Write(*) +--- + +# Workflow Review-Fix Command + +## Quick Start + +```bash +# Fix from exported findings file +/workflow:review-fix .workflow/.reviews/session-WFS-123/fix-export-1706184622000.json + +# Fix from review directory (auto-discovers latest export) +/workflow:review-fix .workflow/.reviews/session-WFS-123/ + +# Resume interrupted fix session +/workflow:review-fix --resume + +# Custom max retry attempts per finding +/workflow:review-fix .workflow/.reviews/session-WFS-123/ --max-iterations=5 +``` + +**Fix Source**: Exported findings from review cycle dashboard +**Output Directory**: `{review-dir}/fixes/{fix-session-id}/` (fixed location) +**Default Max Iterations**: 3 (per finding, adjustable) +**CLI Tools**: @cli-planning-agent (planning), @cli-execute-agent (fixing) + +## What & Why + +### Core Concept +Automated fix orchestrator with **two-phase architecture**: AI-powered planning followed by coordinated parallel/serial execution. Generates fix timeline with intelligent grouping and dependency analysis, then executes fixes with conservative test verification. + +**Fix Process**: +- **Planning Phase**: AI analyzes findings, generates fix plan with grouping and execution strategy +- **Execution Phase**: Main orchestrator coordinates agents per timeline stages +- **No rigid structure**: Adapts to task requirements, not bound to fixed JSON format + +**vs Manual Fixing**: +- **Manual**: Developer reviews findings one-by-one, fixes sequentially +- **Automated**: AI groups related issues, executes in optimal parallel/serial order with automatic test verification + +### Value Proposition +1. **Intelligent Planning**: AI-powered analysis identifies optimal grouping and execution strategy +2. **Multi-stage Coordination**: Supports complex parallel + serial execution with dependency management +3. **Conservative Safety**: Mandatory test verification with automatic rollback on failure +4. **Real-time Visibility**: Dashboard shows planning progress, stage timeline, and active agents +5. **Resume Support**: Checkpoint-based recovery for interrupted sessions + +### Orchestrator Boundary (CRITICAL) +- **ONLY command** for automated review finding fixes +- Manages: Planning phase coordination, stage-based execution, agent scheduling, progress tracking +- Delegates: Fix planning to @cli-planning-agent, fix execution to @cli-execute-agent + + +### Execution Flow + +``` +1. Discovery & Initialization + └─ Validate export file, create fix session structure, initialize state files → Update dashboard + +2. Phase 2: Planning (@cli-planning-agent): + ├─ Analyze findings for patterns and dependencies + ├─ Group by file + dimension + root cause similarity + ├─ Determine execution strategy (parallel/serial/hybrid) + ├─ Generate fix timeline with stages + └─ Output: fix-plan.json → Update dashboard to execution phase + +3. Phase 3: Execution (Stage-based): + For each timeline stage: + ├─ Load groups for this stage + ├─ If parallel: Launch all group agents simultaneously + ├─ If serial: Execute groups sequentially + ├─ Each agent: + │ ├─ Analyze code context + │ ├─ Apply fix per strategy + │ ├─ Run affected tests + │ ├─ On test failure: Rollback, retry up to max_iterations + │ └─ On success: Commit, write completion JSON + ├─ Update fix-progress.json after each finding + └─ Advance to next stage + +4. Phase 4: Completion + └─ Aggregate results → Generate fix-summary.md → Update history → Output summary +``` + +### Agent Roles + +| Agent | Responsibility | +|-------|---------------| +| **Orchestrator** | Input validation, session management, planning coordination, stage-based execution scheduling, progress tracking, aggregation | +| **@cli-planning-agent** | Findings analysis, intelligent grouping (file+dimension+root cause), execution strategy determination (parallel/serial/hybrid), timeline generation with dependency mapping | +| **@cli-execute-agent** | Fix execution per group, code context analysis, Edit tool operations, test verification, git rollback on failure, completion JSON generation | + +## Enhanced Features + +### 1. Two-Phase Architecture + +**Phase Separation**: + +| Phase | Agent | Output | Purpose | +|-------|-------|--------|---------| +| **Planning** | @cli-planning-agent | fix-plan.json | Analyze findings, group intelligently, determine optimal execution strategy | +| **Execution** | @cli-execute-agent | completions/*.json | Execute fixes per plan with test verification and rollback | + +**Benefits**: +- Clear separation of concerns (analysis vs execution) +- Reusable plans (can re-execute without re-planning) +- Better error isolation (planning failures vs execution failures) + +### 2. Intelligent Grouping Strategy + +**Three-Level Grouping**: + +```javascript +// Level 1: Primary grouping by file + dimension +{file: "auth.ts", dimension: "security"} → Group A +{file: "auth.ts", dimension: "quality"} → Group B +{file: "query-builder.ts", dimension: "security"} → Group C + +// Level 2: Secondary grouping by root cause similarity +Group A findings → Semantic similarity analysis (threshold 0.7) + → Sub-group A1: "missing-input-validation" (findings 1, 2) + → Sub-group A2: "insecure-crypto" (finding 3) + +// Level 3: Dependency analysis +Sub-group A1 creates validation utilities +Sub-group C4 depends on those utilities +→ A1 must execute before C4 (serial stage dependency) +``` + +**Similarity Computation**: +- Combine: `description + recommendation + category` +- Vectorize: TF-IDF or LLM embedding +- Cluster: Greedy algorithm with cosine similarity > 0.7 + +### 3. Execution Strategy Determination + +**Strategy Types**: + +| Strategy | When to Use | Stage Structure | +|----------|-------------|-----------------| +| **Parallel** | All groups independent, different files | Single stage, all groups in parallel | +| **Serial** | Strong dependencies, shared resources | Multiple stages, one group per stage | +| **Hybrid** | Mixed dependencies | Multiple stages, parallel within stages | + +**Dependency Detection**: +- Shared file modifications +- Utility creation + usage patterns +- Test dependency chains +- Risk level clustering (high-risk groups isolated) + +### 4. Conservative Test Verification + +**Test Strategy** (per fix): + +```javascript +// 1. Identify affected tests +const testPattern = identifyTestPattern(finding.file); +// e.g., "tests/auth/**/*.test.*" for src/auth/service.ts + +// 2. Run tests +const result = await runTests(testPattern); + +// 3. Evaluate +if (result.passRate < 100%) { + // Rollback + await gitCheckout(finding.file); + + // Retry with failure context + if (attempts < maxIterations) { + const fixContext = analyzeFailure(result.stderr); + regenerateFix(finding, fixContext); + retry(); + } else { + markFailed(finding.id); + } +} else { + // Commit + await gitCommit(`Fix: ${finding.title} [${finding.id}]`); + markFixed(finding.id); +} +``` + +**Pass Criteria**: 100% test pass rate (no partial fixes) + +## Core Responsibilities + +### Orchestrator + +**Phase 1: Discovery & Initialization** +- Input validation: Check export file exists and is valid JSON +- Auto-discovery: If review-dir provided, find latest `*-fix-export.json` +- Session creation: Generate fix-session-id (`fix-{timestamp}`) +- Directory structure: Create `{review-dir}/fixes/{fix-session-id}/` with subdirectories +- State files: Initialize fix-state.json, fix-progress.json, active-fix-session.json +- TodoWrite initialization: Set up 4-phase tracking + +**Phase 2: Planning Coordination** +- Launch @cli-planning-agent with findings data and project context +- Monitor planning progress (dashboard shows "Planning fixes..." indicator) +- Validate fix-plan.json output (schema conformance) +- Load plan into memory for execution phase +- Update fix-state.json with plan_file reference +- TodoWrite update: Mark planning complete, start execution + +**Phase 3: Execution Orchestration** +- Load fix-plan.json timeline stages +- For each stage: + - If parallel mode: Launch all group agents via `Promise.all()` + - If serial mode: Execute groups sequentially with `await` + - Assign agent IDs (agents update their fix-progress-{N}.json) +- Handle agent failures gracefully (mark group as failed, continue) +- Advance to next stage only when current stage complete +- Dashboard polls and aggregates fix-progress-{N}.json files for display + +**Phase 4: Completion & Aggregation** +- Collect final status from all fix-progress-{N}.json files +- Generate fix-summary.md with timeline and results +- Update fix-history.json with new session entry +- Remove active-fix-session.json +- TodoWrite completion: Mark all phases done +- Output summary to user with dashboard link + +### Planning Agent (@cli-planning-agent) + +**Role**: Analyze findings, create execution strategy, initialize progress tracking + +**Orchestrator Provides**: +- Review findings data (id, title, severity, file, description, recommendations) +- Project context (structure, test framework, git status) +- Output directory and template paths + +**Agent Outputs**: +- fix-plan.json (groups, timeline, execution strategy) +- fix-progress-{N}.json (one per group, initial state) + +### Execution Agent (@cli-execute-agent) + +**Role**: Execute fixes for assigned group, update progress in real-time + +**Orchestrator Provides**: +- Group assignment (from fix-plan.json) +- Fix strategy and risk assessment (from fix-plan.json) +- Progress file path (fix-progress-{N}.json) + +**Agent Responsibilities**: +- Read/update assigned progress file +- Apply fixes with flow control tracking +- Run tests and verify +- Commit successful fixes to git + +## Reference + +### CLI Tool Configuration + +**Planning Agent**: +```javascript +{ + subagent_type: "cli-planning-agent", + timeout: 300000, // 5 minutes for planning + description: "Generate fix plan for N code review findings" +} +``` + +**Execution Agent**: +```javascript +{ + subagent_type: "cli-execute-agent", + timeout: 3600000, // 60 minutes per group (adjustable) + description: "Fix M issues: {group_name}" +} +``` + +### Output File Structure + +``` +.workflow/.reviews/{review-id}/ +├── fix-export-{timestamp}.json # Exported findings (input) +└── fixes/{fix-session-id}/ + ├── fix-plan.json # Planning agent output (execution plan) + ├── fix-progress-1.json # Group 1 progress (planning agent init → agent updates) + ├── fix-progress-2.json # Group 2 progress (planning agent init → agent updates) + ├── fix-progress-3.json # Group 3 progress (planning agent init → agent updates) + ├── fix-summary.md # Final report (orchestrator generates) + ├── active-fix-session.json # Active session marker + └── fix-history.json # All sessions history +``` + +**File Producers**: +- **Planning Agent**: `fix-plan.json`, all `fix-progress-*.json` (initial state) +- **Execution Agents**: Update assigned `fix-progress-{N}.json` in real-time +- **Dashboard**: Reads `fix-plan.json` + all `fix-progress-*.json`, aggregates in-memory every 3 seconds + +### Agent Output Files + +**fix-plan.json**: +- **Generated by**: @cli-planning-agent +- **Template**: `~/.claude/workflows/cli-templates/fix-plan-template.json` +- **Purpose**: Execution strategy, group definitions, timeline stages +- **Orchestrator uses**: Load groups, determine execution order, assign agents + +**fix-progress-{N}.json** (one per group): +- **Generated by**: @cli-planning-agent (initial) → @cli-execute-agent (updates) +- **Template**: `~/.claude/workflows/cli-templates/fix-progress-template.json` +- **Purpose**: Track individual group progress with flow control steps +- **Consumed by**: Dashboard (reads all, aggregates in-memory) + +### Agent Invocation Template + +**Planning Agent**: +```javascript +Task({ + subagent_type: "cli-planning-agent", + description: `Generate fix plan and initialize progress files for ${findings.length} findings`, + prompt: ` + Analyze ${findings.length} code review findings and generate: + - fix-plan.json (execution strategy, groups, timeline) + - fix-progress-{N}.json for each group (initial state) + + Input: ${JSON.stringify(findings, null, 2)} + Templates: @~/.claude/workflows/cli-templates/fix-plan-template.json + @~/.claude/workflows/cli-templates/fix-progress-template.json + Output Dir: ${sessionDir} + ` +}) +``` + +**Execution Agent** (per group): +```javascript +Task({ + subagent_type: "cli-execute-agent", + description: `Fix ${group.findings.length} issues: ${group.group_name}`, + prompt: ` + Execute fixes for findings in ${group.progress_file}. + Read initial state, apply fixes, run tests, commit changes. + Update progress file in real-time with flow_control steps. + + Group: ${group.group_id} (${group.group_name}) + Progress File: ${group.progress_file} + Fix Strategy: ${JSON.stringify(group.fix_strategy, null, 2)} + ` +}) +``` + +### Completion Conditions + +**Success Criteria**: +- ✅ All findings processed (fixed or failed) +- ✅ Success rate ≥ 70% (configurable threshold) +- ✅ All fixed code has passing tests +- ✅ All changes committed to git +- ✅ All fix-progress-{N}.json files have status = "completed" +- ✅ fix-summary.md generated +- ✅ fix-history.json updated + +**Trigger Next Phase**: +- Planning complete → Start execution +- Stage complete → Advance to next stage +- All stages complete → Generate completion report + +### Error Handling + +**Planning Failures**: +- Invalid template → Abort with error message +- Insufficient findings data → Request complete export +- Planning timeout → Retry once, then fail gracefully + +**Execution Failures**: +- Agent crash → Mark group as failed, continue with other groups +- Test command not found → Skip test verification, warn user +- Git operations fail → Abort with error, preserve state + +**Rollback Scenarios**: +- Test failure after fix → Automatic `git checkout` rollback +- Max iterations reached → Leave file unchanged, mark as failed +- Unrecoverable error → Rollback entire group, save checkpoint + +### TodoWrite Structure + +**Initialization**: +```javascript +TodoWrite({ + todos: [ + {content: "Phase 1: Discovery & Initialization", status: "completed"}, + {content: "Phase 2: Planning", status: "in_progress"}, + {content: "Phase 3: Execution", status: "pending"}, + {content: "Phase 4: Completion", status: "pending"} + ] +}); +``` + +**During Execution**: +```javascript +TodoWrite({ + todos: [ + {content: "Phase 1: Discovery & Initialization", status: "completed"}, + {content: "Phase 2: Planning", status: "completed"}, + {content: "Phase 3: Execution", status: "in_progress"}, + {content: " → Stage 1: Parallel execution (3 groups)", status: "completed"}, + {content: " • Group G1: Auth validation (2 findings)", status: "completed"}, + {content: " • Group G2: Query security (3 findings)", status: "completed"}, + {content: " • Group G3: Config quality (1 finding)", status: "completed"}, + {content: " → Stage 2: Serial execution (1 group)", status: "in_progress"}, + {content: " • Group G4: Dependent fixes (2 findings)", status: "in_progress"}, + {content: "Phase 4: Completion", status: "pending"} + ] +}); +``` + +**Update Rules**: +- Add stage items dynamically based on fix-plan.json timeline +- Add group items per stage +- Mark completed immediately after each group finishes +- Update parent phase status when all child items complete + +## Best Practices + +1. **Trust AI Planning**: Planning agent's grouping and execution strategy are based on dependency analysis +2. **Conservative Approach**: Test verification is mandatory - no fixes kept without passing tests +3. **Parallel Efficiency**: Default 3 concurrent agents balances speed and resource usage +4. **Monitor Dashboard**: Real-time stage timeline and agent status provide execution visibility +5. **Resume Support**: Fix sessions can resume from checkpoints after interruption +6. **Manual Review**: Always review failed fixes manually - may require architectural changes +7. **Incremental Fixing**: Start with small batches (5-10 findings) before large-scale fixes + + diff --git a/.claude/templates/review-cycle-dashboard.html b/.claude/templates/review-cycle-dashboard.html index 46b8fbd3..c5771a52 100644 --- a/.claude/templates/review-cycle-dashboard.html +++ b/.claude/templates/review-cycle-dashboard.html @@ -127,6 +127,412 @@ border-color: var(--accent-color); } + .export-btn-fix { + background-color: var(--success-color); + color: white; + border-color: var(--success-color); + } + + .export-btn-fix:hover { + background-color: #38a169; + } + + .export-btn-fix:disabled { + opacity: 0.5; + cursor: not-allowed; + } + + /* Selection Controls */ + .selection-controls { + display: flex; + gap: 10px; + align-items: center; + font-size: 0.9rem; + } + + .selection-counter { + color: var(--text-secondary); + font-weight: 500; + } + + .selection-btn { + padding: 6px 12px; + font-size: 0.85rem; + } + + /* Checkbox Styles */ + .finding-checkbox { + width: 20px; + height: 20px; + cursor: pointer; + accent-color: var(--accent-color); + } + + /* Fix Status Badges */ + .fix-status-badge { + display: inline-block; + padding: 4px 10px; + border-radius: 12px; + font-size: 0.75rem; + font-weight: 600; + text-transform: uppercase; + margin-top: 8px; + } + + .fix-status-badge.status-pending { + background-color: transparent; + color: var(--text-secondary); + border: 1px solid var(--border-color); + } + + .fix-status-badge.status-in-progress { + background-color: var(--accent-color); + color: white; + animation: pulse 2s infinite; + } + + .fix-status-badge.status-fixed { + background-color: var(--success-color); + color: white; + } + + .fix-status-badge.status-failed { + background-color: var(--danger-color); + color: white; + } + + @keyframes pulse { + 0%, 100% { opacity: 1; } + 50% { opacity: 0.7; } + } + + /* Fix Progress Section */ + .fix-progress-section { + background-color: var(--bg-card); + padding: 20px; + border-radius: 8px; + box-shadow: var(--shadow); + margin-bottom: 20px; + display: none; /* Hidden by default */ + } + + .fix-progress-section.active { + display: block; + } + + .fix-progress-header { + display: flex; + justify-content: space-between; + align-items: center; + margin-bottom: 15px; + } + + .fix-progress-bar { + height: 8px; + background-color: var(--bg-primary); + border-radius: 4px; + overflow: hidden; + margin-bottom: 15px; + } + + .fix-progress-bar-fill { + height: 100%; + background-color: var(--success-color); + transition: width 0.3s; + } + + .active-agents-list { + margin-top: 15px; + border-top: 1px solid var(--border-color); + padding-top: 15px; + } + + .active-agent-item { + padding: 10px; + background-color: var(--bg-primary); + border-radius: 6px; + margin-bottom: 8px; + font-size: 0.9rem; + } + + .active-agent-item .agent-task { + color: var(--text-primary); + font-size: 0.9rem; + margin-top: 4px; + } + + .active-agent-item .agent-file { + color: var(--text-secondary); + font-size: 0.85rem; + margin-top: 4px; + } + + .agent-status-badge { + display: inline-block; + padding: 2px 8px; + margin-left: 8px; + font-size: 0.75rem; + background-color: var(--accent-color); + color: white; + border-radius: 10px; + font-weight: 500; + } + + .group-active-items { + margin-top: 8px; + padding: 6px 8px; + background-color: rgba(59, 130, 246, 0.1); + border-left: 3px solid var(--accent-color); + border-radius: 4px; + font-size: 0.85rem; + color: var(--text-primary); + word-wrap: break-word; + } + + /* Flow Control Steps */ + .flow-control-container { + margin-top: 12px; + padding: 10px; + background-color: rgba(139, 92, 246, 0.05); + border-radius: 6px; + border: 1px solid rgba(139, 92, 246, 0.2); + } + + .flow-control-header { + font-size: 0.8rem; + font-weight: 600; + color: var(--text-secondary); + margin-bottom: 8px; + text-transform: uppercase; + letter-spacing: 0.5px; + } + + .flow-steps { + display: flex; + flex-direction: column; + gap: 6px; + } + + .flow-step { + display: flex; + align-items: center; + gap: 8px; + padding: 6px 10px; + border-radius: 4px; + font-size: 0.85rem; + transition: all 0.2s; + } + + .flow-step-completed { + background-color: rgba(34, 197, 94, 0.1); + color: #22c55e; + } + + .flow-step-in-progress { + background-color: rgba(59, 130, 246, 0.1); + color: var(--accent-color); + animation: pulse 2s ease-in-out infinite; + } + + .flow-step-failed { + background-color: rgba(239, 68, 68, 0.1); + color: #ef4444; + } + + .flow-step-pending { + background-color: rgba(156, 163, 175, 0.1); + color: var(--text-secondary); + } + + .flow-step-icon { + font-size: 1rem; + } + + .flow-step-name { + font-weight: 500; + } + + @keyframes pulse { + 0%, 100% { opacity: 1; } + 50% { opacity: 0.6; } + } + + .collapse-btn { + padding: 6px 12px; + font-size: 0.85rem; + cursor: pointer; + } + + /* History Timeline */ + .history-drawer { + position: fixed; + right: -600px; + top: 0; + width: 600px; + height: 100vh; + background-color: var(--bg-secondary); + box-shadow: var(--shadow-lg); + transition: right 0.3s; + z-index: 1001; + overflow-y: auto; + } + + .history-drawer.active { + right: 0; + } + + .history-timeline { + padding: 20px; + } + + .history-item { + padding: 15px; + background-color: var(--bg-card); + border-radius: 6px; + margin-bottom: 15px; + border-left: 4px solid var(--accent-color); + } + + .history-item.status-fixed { + border-left-color: var(--success-color); + } + + .history-item.status-failed { + border-left-color: var(--danger-color); + } + + .history-item-header { + display: flex; + justify-content: space-between; + align-items: center; + margin-bottom: 10px; + } + + .history-item-title { + font-weight: 600; + color: var(--text-primary); + } + + .history-item-timestamp { + font-size: 0.85rem; + color: var(--text-secondary); + } + + .history-item-meta { + font-size: 0.85rem; + color: var(--text-secondary); + margin-top: 8px; + } + + .history-item-tests { + margin-top: 10px; + padding: 10px; + background-color: var(--bg-primary); + border-radius: 4px; + font-size: 0.85rem; + } + + /* Stage Timeline */ + .stage-timeline { + display: flex; + gap: 10px; + margin: 20px 0; + padding: 15px; + background-color: var(--bg-secondary); + border-radius: 8px; + overflow-x: auto; + } + + .stage-item { + flex: 1; + min-width: 150px; + padding: 12px; + background-color: var(--bg-card); + border-radius: 6px; + border: 2px solid var(--border-color); + text-align: center; + transition: all 0.3s ease; + } + + .stage-item.pending { + opacity: 0.6; + border-color: var(--border-color); + } + + .stage-item.in-progress { + border-color: var(--accent-color); + box-shadow: 0 0 10px rgba(59, 130, 246, 0.3); + } + + .stage-item.completed { + border-color: var(--success-color); + background-color: rgba(34, 197, 94, 0.05); + } + + .stage-number { + font-size: 0.85rem; + font-weight: 600; + color: var(--text-secondary); + margin-bottom: 5px; + } + + .stage-mode { + font-size: 0.9rem; + font-weight: 600; + margin-bottom: 5px; + } + + .stage-groups { + font-size: 0.75rem; + color: var(--text-secondary); + } + + /* Active Groups List */ + .active-groups-list { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(300px, 1fr)); + gap: 15px; + margin: 20px 0; + } + + .active-group-card { + background-color: var(--bg-card); + border-radius: 8px; + padding: 15px; + border-left: 4px solid var(--accent-color); + } + + .group-header { + display: flex; + justify-content: space-between; + align-items: center; + margin-bottom: 10px; + } + + .group-id { + font-weight: 600; + font-size: 0.95rem; + } + + .group-status { + font-size: 0.75rem; + padding: 4px 8px; + border-radius: 12px; + background-color: var(--accent-color); + color: white; + } + + .group-name { + font-size: 0.9rem; + color: var(--text-secondary); + margin-bottom: 8px; + } + + .group-findings { + font-size: 0.85rem; + color: var(--text-primary); + } + /* Severity Summary Cards */ .severity-grid { display: grid; @@ -616,10 +1022,62 @@ +
+ 0 findings selected + + +
+ + + +
+
+

Fix Progress

+ PLANNING +
+ + +
+
+ + + + + + + + + +
+
@@ -703,6 +1161,18 @@
+ +
+
+
+
📜 Fix History
+ +
+
+ +
+
+