diff --git a/src/lib/AgentManager.integration.test.ts b/src/lib/AgentManager.integration.test.ts index c1482042..114abb79 100644 --- a/src/lib/AgentManager.integration.test.ts +++ b/src/lib/AgentManager.integration.test.ts @@ -26,6 +26,7 @@ const ALL_AGENT_NAMES = [ 'iloom-issue-complexity-evaluator', 'iloom-issue-enhancer', 'iloom-issue-implementer', + 'iloom-issue-phase-verifier', 'iloom-issue-planner', 'iloom-wave-verifier', ] diff --git a/src/lib/DockerManager.test.ts b/src/lib/DockerManager.test.ts index 2d9c5502..b1b69e3c 100644 --- a/src/lib/DockerManager.test.ts +++ b/src/lib/DockerManager.test.ts @@ -505,7 +505,7 @@ describe('DockerManager', () => { expect(result).toBe(true) // Should have used docker rm -f (atomic force-remove) - expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-123'], { reject: false }) + expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-123'], { reject: false, timeout: 10000 }) }) it('should not throw if container does not exist', async () => { @@ -534,7 +534,7 @@ describe('DockerManager', () => { const result = await DockerManager.stopAndRemoveContainer('iloom-dev-stopped') expect(result).toBe(false) - expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-stopped'], { reject: false }) + expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-stopped'], { reject: false, timeout: 10000 }) }) }) @@ -551,7 +551,7 @@ describe('DockerManager', () => { expect(execa).toHaveBeenCalledWith( 'docker', ['ps', '--filter', 'name=^iloom-dev-123$', '--format', '{{.Names}}'], - { reject: false } + { reject: false, timeout: 5000 } ) }) diff --git a/src/lib/DockerManager.ts b/src/lib/DockerManager.ts index a4482601..ddd2fedf 100644 --- a/src/lib/DockerManager.ts +++ b/src/lib/DockerManager.ts @@ -275,14 +275,14 @@ export class DockerManager { if (!isRunning) { logger.debug(`No running container found with name "${containerName}"`) // Still try to remove in case container exists but is stopped - await execa('docker', ['rm', '-f', containerName], { reject: false }) + await execa('docker', ['rm', '-f', containerName], { reject: false, timeout: 10000 }) return false } logger.info(`Removing Docker container "${containerName}"...`) // Atomic force-remove: handles running and stopped containers - await execa('docker', ['rm', '-f', containerName], { reject: false }) + await execa('docker', ['rm', '-f', containerName], { reject: false, timeout: 10000 }) logger.success(`Docker container "${containerName}" removed`) return true @@ -296,14 +296,17 @@ export class DockerManager { */ static async isContainerRunning(containerName: string): Promise { try { + logger.debug(`Running: docker ps --filter name=^${containerName}$ (timeout: 5s)`) const result = await execa('docker', [ 'ps', '--filter', `name=^${containerName}$`, '--format', '{{.Names}}', - ], { reject: false }) + ], { reject: false, timeout: 5000 }) + logger.debug(`docker ps exited ${result.exitCode}, stdout: "${result.stdout.trim()}"`) return result.exitCode === 0 && result.stdout.trim() === containerName - } catch { + } catch (error) { + logger.debug(`docker ps failed: ${error instanceof Error ? error.message : String(error)}`) return false } } diff --git a/src/lib/ResourceCleanup.ts b/src/lib/ResourceCleanup.ts index 2fc34d1a..6414b606 100644 --- a/src/lib/ResourceCleanup.ts +++ b/src/lib/ResourceCleanup.ts @@ -590,7 +590,9 @@ export class ResourceCleanup { // Try Docker container cleanup first if identifier provided if (dockerIdentifier !== undefined) { const containerName = DockerManager.buildContainerName(dockerIdentifier) + getLogger().debug(`Checking Docker container: ${containerName}`) const isRunning = await DockerManager.isContainerRunning(containerName) + getLogger().debug(`Docker container ${containerName} running: ${isRunning}`) if (isRunning) { getLogger().info(`Terminating Docker container: ${containerName}`) await DockerManager.stopAndRemoveContainer(containerName) @@ -600,7 +602,9 @@ export class ResourceCleanup { } // Existing process-based detection + getLogger().debug(`Detecting dev server process on port ${port}`) const processInfo = await this.processManager.detectDevServer(port) + getLogger().debug(`Dev server detection result: ${processInfo ? `${processInfo.name} (PID: ${processInfo.pid})` : 'none'}`) if (!processInfo) { getLogger().debug(`No process found on port ${port}`) @@ -619,6 +623,7 @@ export class ResourceCleanup { await this.processManager.terminateProcess(processInfo.pid) // Verify termination + getLogger().debug(`Verifying port ${port} is free`) const isFree = await this.processManager.verifyPortFree(port) if (!isFree) { throw new Error(`Dev server may still be running on port ${port}`) diff --git a/templates/agents/CLAUDE.md b/templates/agents/CLAUDE.md index 27e09106..5e78d7b3 100644 --- a/templates/agents/CLAUDE.md +++ b/templates/agents/CLAUDE.md @@ -9,11 +9,12 @@ This directory contains phase agent definitions as Markdown files with YAML fron Agents execute in a fixed sequence during issue workflows. Each phase has a single responsibility: ``` -complexity-evaluator → analyzer (or analyze-and-plan for SIMPLE) → planner → implementer → code-reviewer +complexity-evaluator → analyzer (or analyze-and-plan for SIMPLE) → planner → implementer → phase-verifier + code-reviewer ``` - **SIMPLE tasks** (< 5 files, < 200 LOC): Use `analyze-and-plan` which combines analysis + planning into one step, then `implementer` - **COMPLEX tasks**: Full pipeline: `analyzer` → `planner` → `implementer` +- **Phase verification**: `phase-verifier` runs after each implementation phase to verify must-haves (both modes) - **Code review**: `code-reviewer` runs after implementation (swarm mode only; in regular mode, review is optional) - **Wave verification**: `wave-verifier` is an epic-level agent that verifies a wave of completed children @@ -26,6 +27,7 @@ complexity-evaluator → analyzer (or analyze-and-plan for SIMPLE) → planner | `iloom-issue-analyze-and-plan.md` | Analysis + Planning | opus | Regular + Swarm | Combined phase for SIMPLE tasks | | `iloom-issue-planner.md` | Planning | opus | Regular + Swarm | Detailed implementation plan (COMPLEX only) | | `iloom-issue-implementer.md` | Implementation | opus | Regular + Swarm | Execute the plan, run typecheck/lint/tests | +| `iloom-issue-phase-verifier.md` | Verification | inherit | Regular + Swarm | Verify phase must-haves, GO/NO-GO verdict | | `iloom-code-reviewer.md` | Review | opus | Swarm | Autonomous code review, no human gates | | `iloom-issue-enhancer.md` | Enhancement | — | Regular | Enhance issue descriptions with context | | `iloom-wave-verifier.md` | Verification | — | Swarm (epic) | Verify a wave of completed children | diff --git a/templates/agents/iloom-issue-analyze-and-plan.md b/templates/agents/iloom-issue-analyze-and-plan.md index 6e9eef5d..59106af6 100644 --- a/templates/agents/iloom-issue-analyze-and-plan.md +++ b/templates/agents/iloom-issue-analyze-and-plan.md @@ -533,20 +533,48 @@ If structure is >5 lines: ### Detailed Execution Order -**NOTE:** These steps are executed in a SINGLE implementation run. The implementer follows them sequentially - do NOT create separate agent invocations for each step. +**NOTE:** These steps are executed in a SINGLE implementation run. The implementer follows them sequentially - do NOT create separate agent invocations for each step. The orchestrator runs the phase-verifier against all must-haves after the implementation run completes. + + +### Phase 1: [Foundation] +**Must-haves (verified after this phase):** +- [Concrete, independently verifiable invariant — e.g. "`UserService.getById(id: string): Promise` exists and is exported from src/services/user.ts"] +- [Behavior preserved — e.g. "all existing tests still pass"] +- [Scope boundary — e.g. "no files outside src/services/ and tests/ modified"] +- [Negative invariant — e.g. "no new dependency added", "the public API surface is unchanged"] 1. **[Step Name]** - Files: `/path/to/file1.ts`, `/path/to/file2.ts` - [Action with file:line reference] → Verify: [Expected outcome] +### Phase 2: [Integration] +**Must-haves (verified after this phase):** +- [Concrete, independently verifiable invariant for this phase] +- [Behavior preserved — e.g. "all existing tests still pass"] +- [Scope boundary — e.g. "no files outside src/routes/ and tests/ modified"] +- [Negative invariant — e.g. "no breaking changes to the public API"] + 2. **[Step Name]** - Files: `/path/to/file3.ts` - [Action with file:line reference] → Verify: [Expected outcome] -[Continue - keep brief, one line per step...] +[Continue - keep brief, one line per step. EVERY phase MUST include a "Must-haves" block.] **NOTE:** Follow the project's development workflow as specified in CLAUDE.md. +### Phase Must-Haves / Invariants + +**Every phase in the Detailed Execution Order MUST include a "Must-haves" list** — 3-7 concrete, independently verifiable statements that must be true after the phase completes. A `phase-verifier` agent checks these against the actual worktree after each implementation phase, so write them to be checkable from the code and test results alone, without access to your reasoning. + +Good must-haves: +- **Verifiable from the diff/worktree**: "Function `getById(id: string): Promise` exists in `src/services/user.ts` and is exported" — not "the service layer is clean" +- **Behavior preserved**: "All existing tests in `tests/auth/` still pass", "the CLI's `--dry-run` flag output is unchanged" +- **Contracts honored**: the exact signatures/types other phases implement against +- **Scope boundaries**: "No files outside `src/services/` and `tests/services/` are modified" +- **Negative invariants**: things that must NOT happen ("no new dependency added", "the public API surface is unchanged") + +**Must-haves MUST include "must NOT happen" criteria where applicable** — especially for performance and interaction properties (e.g. "a transform-only edit must not trigger pixel re-extraction"). Positive-only criteria let an implementation satisfy the letter of the plan while violating the feature's purpose; verifiers can only check what's written down, so if the forbidden behavior isn't a must-have, nothing in the pipeline will catch it. + ### Dependencies and Configuration - [Package name@version] - [Purpose] @@ -769,6 +797,7 @@ Before submitting your combined analysis and plan, verify (DO NOT print this che - **STAY SCOPED**: Only address what's in the issue - **ONE-SENTENCE RULE**: Applied throughout Section 2 - **NO AI SLOP**: No time estimates, rollback plans, or redundant sections +- **EVERY PHASE GETS MUST-HAVES** — the phase-verifier agent depends on them; a phase without verifiable invariants cannot be verified ## Success Criteria diff --git a/templates/agents/iloom-issue-phase-verifier.md b/templates/agents/iloom-issue-phase-verifier.md new file mode 100644 index 00000000..32ec025b --- /dev/null +++ b/templates/agents/iloom-issue-phase-verifier.md @@ -0,0 +1,104 @@ +--- +name: iloom-issue-phase-verifier +description: >- + Use this agent after an implementation phase completes to independently verify that the + phase's must-haves/invariants from the plan are actually met in the worktree. The agent + gathers its own evidence (reads files, runs targeted tests) and returns a GO/NO-GO verdict + per must-have. It reports violations but never fixes them. + + Provide context under these headers: `## Phase Must-Haves` (the invariant list from the + plan), `## Worktree` (path and branch), and optionally `## Pre-gathered Diff`, + `## Implementer Report`, and `## Commit Range`. + + Examples: + + Context: Orchestrator wants to verify that phase 2 work meets its must-haves + user: "Verify Phase 2 of the implementation against its must-haves" + assistant: "I'll check all must-have criteria from that phase against the worktree and report results." + + The orchestrator needs phase verification after a completed phase, so use the iloom-issue-phase-verifier agent. + + + + Context: Pipeline wants to gate the next phase on verification passing + user: "Run phase verification for the compositor phase before proceeding to Phase 3" + assistant: "I'll verify all must-haves for the specified phase, and return a structured GO/NO-GO report." + + Phase gating requires verification of completed work, so use the iloom-issue-phase-verifier agent. + + +model: inherit +color: orange +--- + +You are an independent verification agent. After an implementer agent finishes a phase, you check — with your own evidence — that every must-have/invariant the plan defined for that phase is actually true in the worktree. You are the safeguard against plausible-but-wrong implementation reports. + +## Core Principles + +1. **Trust nothing you didn't verify yourself.** The `## Implementer Report` section tells you what the implementer *claims*; your job is to confirm or refute each claim against the actual code. Never mark a must-have as met because the report says so. +2. **Evidence or it didn't happen.** Every verdict cites concrete evidence: a file:line you read, a command you ran and its output, a test result. "Looks correct" is not a verdict. +3. **Report, never fix.** You make no edits. If something is wrong, you document it precisely enough that a fix agent can act on it without re-investigating. + +## Workflow + +### Step 1: Parse the inputs + +- `## Phase Must-Haves` — the checklist. Each item gets its own verdict. +- `## Worktree` — path and branch. All file reads and commands run here (`git -C `, `cd ` for builds/tests). +- `## Pre-gathered Diff` — the phase's changes. Use it to spot scope violations and to target your reading. +- `## Implementer Report` — claims to verify, not facts. +- `## Commit Range` — optional. If provided, contains `Previous commit: ` and `Phase commit: `. Use these to pull your own diff if `## Pre-gathered Diff` is absent or if you need to verify scope against the actual commit history: `git -C diff ..`. + +If the must-haves list is missing, say so and verify against the phase's step definitions instead — but flag that the plan lacked invariants. + +### Step 2: Verify each must-have + +For each item, choose the cheapest check that is actually conclusive: + +- **Existence/signature claims** ("function X exists with signature Y"): Read the file at the relevant location. Confirm the exact name, signature, and export. +- **Behavior-preserved claims** ("existing tests still pass"): Run the relevant test command in the worktree. Prefer targeted test invocations over full suites when the suite is slow; run the full suite if the must-have names it. +- **Scope boundaries** ("no files outside X modified"): Check the diff's file list (`git -C diff --name-only ` plus untracked files). +- **Negative invariants** ("no new dependency"): Check the relevant manifests/lockfiles in the diff. +- **Contract claims** (cross-phase interfaces): Read both sides if both exist; if the counterpart phase hasn't run yet, verify this side matches the contract exactly as written in the plan. + +Also perform two standing checks regardless of the list: + +- **Scope check**: Every modified/created file is within the phase's declared file scope. +- **Completeness check**: Every step in the phase's plan section has corresponding changes in the diff. + +### Step 3: Verdict + +Output format: + +```markdown +# Phase Verification: [GO / NO-GO] + +## Must-Have Verdicts + +| # | Must-have | Verdict | Evidence | +|---|-----------|---------|----------| +| 1 | [short restatement] | MET / VIOLATED / UNVERIFIABLE | [file:line, command + result] | + +## Violations (if any) + +### [Must-have #N]: [title] +- **What was expected:** [from the plan] +- **What is actually there:** [evidence, file:line] +- **Suggested fix direction:** [one sentence — direction only, you do not implement] + +## Scope & Completeness +- Files changed outside phase scope: [list or "none"] +- Plan steps with no corresponding changes: [list or "none"] + +## Notes (optional) +[Anything observed that isn't a violation but the caller should know — e.g. an UNVERIFIABLE must-have and why] +``` + +**GO** requires every must-have MET and no scope violations. Anything VIOLATED means **NO-GO**. UNVERIFIABLE items don't automatically fail the phase — explain why they couldn't be checked and let the caller decide, but never silently count them as met. + +## Constraints + +- **Read-only**: You never edit files. Running builds and tests is fine; editing source is not. +- Keep the verdict table tight — one row per must-have, evidence in a few words with file:line. Detail goes in the Violations section only for items that failed. +- Time-box heavy commands: if the full test suite takes more than a few minutes and no must-have explicitly requires it, run the targeted subset and note the narrowing in the evidence column. +- Do not post comments to issue trackers or manage loom state — the caller (orchestrator or pipeline) handles all external interactions. diff --git a/templates/agents/iloom-issue-planner.md b/templates/agents/iloom-issue-planner.md index 388be10c..156852c5 100644 --- a/templates/agents/iloom-issue-planner.md +++ b/templates/agents/iloom-issue-planner.md @@ -451,6 +451,12 @@ If structure is >5 lines: Provide execution steps concisely. Group steps by parallel execution phase — steps within the same phase run concurrently, phases run sequentially: ### Phase 1 (parallel): [Foundation] +**Must-haves (verified after this phase):** +- [Concrete, independently verifiable invariant — e.g. "`UserService.getById(id: string): Promise` exists and is exported from src/services/user.ts"] +- [Behavior preserved — e.g. "all existing tests still pass"] +- [Scope boundary — e.g. "no files outside src/services/ and tests/ modified"] +- [Negative invariant — e.g. "no new dependency added", "the public API surface is unchanged"] + #### Step 1a: [Step Name] **Files:** [List all files this step touches] **Contract:** [If this step produces or consumes a shared interface/type, specify it here] @@ -462,14 +468,33 @@ Provide execution steps concisely. Group steps by parallel execution phase — s 1. [Action with file:line reference] → Verify: [Expected outcome] ### Phase 2 (sequential): [Integration] +**Must-haves (verified after this phase):** +- [Concrete, independently verifiable invariant for this phase] +- [Behavior preserved — e.g. "all existing tests still pass"] +- [Scope boundary — e.g. "no files outside src/routes/ and tests/ modified"] +- [Negative invariant — e.g. "no breaking changes to the public API"] + #### Step 2: [Step Name] **Files:** [List all files this step touches] 1. [Action with file:line reference] → Verify: [Expected outcome] -[Continue for all phases — maximize steps per parallel phase, minimize the number of phases...] +[Continue for all phases — maximize steps per parallel phase, minimize the number of phases. EVERY phase MUST include a "Must-haves" block.] **NOTE:** Follow the project's development workflow as specified in CLAUDE.md (e.g., TDD, test-after, or other approaches). +### Phase Must-Haves / Invariants + +**Every phase in the Detailed Execution Order MUST include a "Must-haves" list** — 3-7 concrete, independently verifiable statements that must be true after the phase completes. A `phase-verifier` agent checks these against the actual worktree after each implementation phase, so write them to be checkable from the code and test results alone, without access to your reasoning. + +Good must-haves: +- **Verifiable from the diff/worktree**: "Function `getById(id: string): Promise` exists in `src/services/user.ts` and is exported" — not "the service layer is clean" +- **Behavior preserved**: "All existing tests in `tests/auth/` still pass", "the CLI's `--dry-run` flag output is unchanged" +- **Contracts honored**: the exact signatures/types other phases implement against +- **Scope boundaries**: "No files outside `src/services/` and `tests/services/` are modified" +- **Negative invariants**: things that must NOT happen ("no new dependency added", "the public API surface is unchanged") + +**Must-haves MUST include "must NOT happen" criteria where applicable** — especially for performance and interaction properties (e.g. "a transform-only edit must not trigger pixel re-extraction"). Positive-only criteria let an implementation satisfy the letter of the plan while violating the feature's purpose; verifiers can only check what's written down, so if the forbidden behavior isn't a must-have, nothing in the pipeline will catch it. + ## Execution Plan This section tells the orchestrator EXACTLY how to execute the implementation steps. The orchestrator will parse this and follow the instructions — spawning multiple agents for parallel steps, waiting for completion, then continuing. @@ -562,6 +587,7 @@ This section tells the orchestrator EXACTLY how to execute the implementation st - **NO ENHANCEMENTS** - stick strictly to stated requirements - **QUESTION NECESSITY** - if requirements describe writing values that already match the system's built-in defaults, the write may be unnecessary. Plan for the actual need, not the literal phrasing of the issue. - **NO CROSS-COMPONENT FABRICATION** - when your plan includes a specific value (score range, threshold, enum, format) from another component's source, look it up and cite file:line. If you cannot find it, describe the intent (e.g., "the reviewer's critical category") instead of a plausible-sounding number. Log a recap assumption. Fabricating values is a hard error. +- **EVERY PHASE GETS MUST-HAVES** — the phase-verifier agent depends on them; a phase without verifiable invariants cannot be verified ## Workflow diff --git a/templates/prompts/issue-prompt.txt b/templates/prompts/issue-prompt.txt index 64b1fdc2..96c96272 100644 --- a/templates/prompts/issue-prompt.txt +++ b/templates/prompts/issue-prompt.txt @@ -13,6 +13,7 @@ Each phase agent is available as a skill in your .claude/skills/ directory. - Planning: /iloom-swarm-issue-planner "Create implementation plan..." - Analyze+Plan: /iloom-swarm-issue-analyze-and-plan "Analyze and plan..." - Implementation: /iloom-swarm-issue-implementer "Implement the plan..." +- Phase Verification: /iloom-swarm-issue-phase-verifier "Verify phase N..." - Code Review: /iloom-swarm-code-reviewer "Run code review." - Artifact Review: /iloom-swarm-artifact-reviewer "Review this artifact..." @@ -369,6 +370,7 @@ The following skills are pre-rendered in your .claude/skills/ directory: - /iloom-swarm-issue-analyze-and-plan - /iloom-swarm-issue-planner - /iloom-swarm-issue-implementer +- /iloom-swarm-issue-phase-verifier - /iloom-swarm-code-reviewer - /iloom-swarm-artifact-reviewer @@ -418,7 +420,16 @@ Always prepend: `"Your working directory is . cd there before doi {{#if ARTIFACT_REVIEW_ENABLED}}{{#if IMPLEMENTER_REVIEW_ENABLED}} 7a. Run artifact review on implementation output via /iloom-swarm-artifact-reviewer skill {{/if}}{{/if}} -8. Set state to `code_review` (pass `worktreePath: ""`), run code review via /iloom-swarm-code-reviewer skill +7b. COMPLEX only: After each phase completes, commit the phase and run per-phase verification: + - Commit: `git add -A && git commit -m "Phase N: "` + - Run /iloom-swarm-issue-phase-verifier and /iloom-swarm-code-reviewer sequentially for the phase + - If verifier NO-GO or critical findings: run fix implementer, commit fix, re-verify (max 2 attempts, then fail) + - If verifier GO and no critical findings: proceed to next phase +7c. SIMPLE only: After implementation completes, commit and run single verification pass: + - Commit: `git add -A && git commit -m "Implementation: "` + - Run /iloom-swarm-issue-phase-verifier (if must-haves exist) and /iloom-swarm-code-reviewer sequentially + - If verifier NO-GO or critical findings: run fix implementer, commit fix, re-verify (max 2 attempts, then fail) +8. Set state to `code_review` (pass `worktreePath: ""`). If per-phase code review was already done (COMPLEX workflow), skip standalone code review. Otherwise run code review via /iloom-swarm-code-reviewer skill. 9. If critical/high issues found, auto-fix and log a `fix` recap entry for each finding addressed 10. Stage and commit all changes: `git add -A && git commit -m "feat(issue-): [descriptive summary of changes]"` — the commit message must summarize what was implemented (not just "fixes #NNN") 11. Set state to `done` (pass `worktreePath: ""`), report structured result to orchestrator @@ -468,7 +479,16 @@ You are orchestrating a set of agents through a development process, with human {{#if ARTIFACT_REVIEW_ENABLED}}{{#if IMPLEMENTER_REVIEW_ENABLED}} 16a. Run artifact review on implementation output using @agent-iloom-artifact-reviewer (run_in_background: true) {{/if}}{{/if}} -17. Run code review using @agent-iloom-code-reviewer (foreground -- needs MCP tool access) +16b. COMPLEX only: After each phase completes, commit the phase and run per-phase verification: + - Commit: `git add -A && git commit -m "Phase N: "` + - Run @agent-iloom-issue-phase-verifier (run_in_background: true) and @agent-iloom-code-reviewer (run_in_background: true) in parallel for the phase + - If verifier NO-GO or critical findings: run fix implementer, commit fix, re-verify (max 2 attempts, then ask user) + - If verifier GO and no critical findings: proceed to next phase +16c. SIMPLE only: After implementation completes, commit and run single verification pass: + - Commit: `git add -A && git commit -m "Implementation: "` + - Run @agent-iloom-issue-phase-verifier (run_in_background: true, if must-haves exist) and @agent-iloom-code-reviewer (run_in_background: true) in parallel + - If verifier NO-GO or critical findings: run fix implementer, commit fix, re-verify (max 2 attempts, then ask user) +17. Run code review using @agent-iloom-code-reviewer (foreground -- needs MCP tool access). NOTE: If per-phase code review was already done (COMPLEX workflow), skip this step. {{#if DRAFT_PR_MODE}} {{#if AUTO_COMMIT_PUSH}} 18. Auto-commit and push changes to draft PR (STEP 5.5) @@ -1122,9 +1142,10 @@ Only execute if workflow plan determined NEEDS_IMPLEMENTATION: 3. **Execute implementation:** - **FOR COMPLEX workflows with Execution Plan (multiple steps):** + **FOR COMPLEX workflows with Execution Plan (multiple phases with steps):** - a. Update progress comment with step checklist from Execution Plan + a. Update progress comment with step checklist from Execution Plan. + Record the current commit hash as `prev_phase_hash` (the baseline for diffing the first phase). b. Execute according to the Execution Plan - EACH STEP GETS ITS OWN IMPLEMENTER AGENT: @@ -1154,6 +1175,51 @@ Only execute if workflow plan determined NEEDS_IMPLEMENTATION: - Update progress comment for all completed steps {{/if}} + c. **Phase commit**: After all steps in the current phase pass post-wave validation: + ```bash + git add -A && git commit -m "Phase N: " + ``` + Record the commit hash as `phase_hash`. + + d. **Phase verification** (run verifier and reviewer): + +{{#if SWARM_MODE}} + Execute BOTH agents sequentially (skills run inline): + + 1. Run /iloom-swarm-issue-phase-verifier skill with prompt: + "Verify Phase N. ## Phase Must-Haves\n[paste Phase N's must-haves from the plan]\n## Worktree\n- Path: \n- Branch: \n## Commit Range\n- Previous: \n- Phase: \n## Implementer Report\n[implementer's summary output]" + + 2. Run /iloom-swarm-code-reviewer skill with prompt: + "Review Phase N changes only (flag issues in THESE changes, not missing functionality for later phases). ## Pre-gathered Diff\n[output of git diff ..]" +{{else}} + Launch BOTH agents simultaneously in ONE message: + + 1. @agent-iloom-issue-phase-verifier (run_in_background: true) with prompt: + "Verify Phase N. ## Phase Must-Haves\n[paste Phase N's must-haves from the plan]\n## Worktree\n- Path: \n- Branch: \n## Commit Range\n- Previous: \n- Phase: \n## Implementer Report\n[implementer's summary output]" + + 2. @agent-iloom-code-reviewer (run_in_background: true) with prompt: + "Review Phase N changes only (flag issues in THESE changes, not missing functionality for later phases). ## Pre-gathered Diff\n[output of git diff ..]" +{{/if}} + + e. **Gate on results**: + - Verifier GO + no critical review findings → update progress comment, set `prev_phase_hash = phase_hash`, proceed to next phase + - Verifier NO-GO or critical findings → **fix round**: +{{#if SWARM_MODE}} + 1. Execute fresh implementer via /iloom-swarm-issue-implementer skill with the specific violations/findings + 2. After fix completes: `git commit -am "Fix Phase N: "` + 3. Update `phase_hash` to the new commit hash + 4. Re-run verification (phase-verifier + code-reviewer, same prompts with updated commit range) + 5. If second verification also fails: report failure and stop workflow +{{else}} + 1. Launch fresh @agent-iloom-issue-implementer (run_in_background: true) with the specific violations/findings + 2. After fix completes: `git commit -am "Fix Phase N: "` + 3. Update `phase_hash` to the new commit hash + 4. Re-run verification (phase-verifier + code-reviewer in parallel, same prompts with updated commit range) + 5. If second verification also fails: stop and ask user how to proceed via AskUserQuestion tool +{{/if}} + + f. Repeat steps b–e for each subsequent phase in the Execution Plan. + **FOR SIMPLE/TRIVIAL workflows (single implementation run):** {{#if SWARM_MODE}} @@ -1164,6 +1230,48 @@ Only execute if workflow plan determined NEEDS_IMPLEMENTATION: - Wait for completion {{/if}} + **SIMPLE/TRIVIAL post-implementation verification:** + + After implementation completes: + + a. **Commit**: `git add -A && git commit -m "Implementation: "`. Record the commit hash as `impl_hash` and the previous commit as `pre_impl_hash`. + + b. **Run verification** (if must-haves exist in the plan): +{{#if SWARM_MODE}} + Execute agents sequentially: + + 1. Run /iloom-swarm-issue-phase-verifier skill with prompt: + "Verify implementation. ## Phase Must-Haves\n[paste all must-haves from the plan]\n## Worktree\n- Path: \n- Branch: \n## Commit Range\n- Previous: \n- Implementation: \n## Implementer Report\n[implementer's summary output]" + + 2. Run /iloom-swarm-code-reviewer skill with prompt: + "Review implementation changes. ## Pre-gathered Diff\n[output of git diff ..]" +{{else}} + Launch BOTH agents simultaneously in ONE message: + + 1. @agent-iloom-issue-phase-verifier (run_in_background: true) with prompt: + "Verify implementation. ## Phase Must-Haves\n[paste all must-haves from the plan]\n## Worktree\n- Path: \n- Branch: \n## Commit Range\n- Previous: \n- Implementation: \n## Implementer Report\n[implementer's summary output]" + + 2. @agent-iloom-code-reviewer (run_in_background: true) with prompt: + "Review implementation changes. ## Pre-gathered Diff\n[output of git diff ..]" +{{/if}} + + c. **Gate on results** (same logic as COMPLEX): + - Verifier GO + no critical review findings → proceed to finalize + - Verifier NO-GO or critical findings → **fix round**: +{{#if SWARM_MODE}} + 1. Execute fresh implementer via /iloom-swarm-issue-implementer skill with the specific violations/findings + 2. After fix completes: `git commit -am "Fix: "` + 3. Re-run verification (same prompts with updated commit range) + 4. If second verification also fails: report failure and stop workflow +{{else}} + 1. Launch fresh @agent-iloom-issue-implementer (run_in_background: true) with the specific violations/findings + 2. After fix completes: `git commit -am "Fix: "` + 3. Re-run verification (phase-verifier + code-reviewer in parallel, same prompts with updated commit range) + 4. If second verification also fails: stop and ask user how to proceed via AskUserQuestion tool +{{/if}} + + d. If no must-haves exist in the plan, skip verification and proceed to finalize. + 4. **Finalize the progress comment:** - Update the progress comment to this format: ```markdown @@ -1252,6 +1360,8 @@ If workflow plan determined SKIP_IMPLEMENTATION: ## STEP 5 - Review Phase +**NOTE:** If you executed a COMPLEX multi-phase plan in STEP 4 with per-phase verification and code review, the code review is already complete. Skip this step and proceed to the next step. Similarly, if you executed a SIMPLE workflow in STEP 4 and ran the post-implementation verification pass (which includes code review), skip this step. + This section is about reviewing code changes for quality, security, and compliance issues. {{#if SWARM_MODE}}