fix(next-task): enforce strict review loop exit conditions (#235)

CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`/next-task` review loop exit conditions** — The Phase 9 review loop now continues iterating until all issues are resolved or a stall is detected (MAX_STALLS reduced from 2 to 1: two consecutive identical-hash iterations = stall). The `orchestrate-review` skill now uses `completePhase()` instead of `updateFlow()` to properly advance workflow state. Added `pre-review-gates` and `docs-update` to the `PHASES` array and `RESULT_FIELD_MAP` in `workflow-state.js`, ensuring these phases can be tracked and resumed correctly. Fixes issue #235.
+
 - **`/debate` command inline orchestration** — The `/debate` command now manages the full debate workflow directly (parse → resolve → execute → verdict), following the `/consult` pattern. The `debate-orchestrator` agent is now the programmatic entry point for other agents/workflows that need to spawn a debate via `Task()`. Fixes issue #231.
 
 ## [5.1.0] - 2026-02-18

__tests__/workflow-state.test.js

@@ -50,6 +50,14 @@ describe('workflow-state', () => {
       expect(PHASES).toContain('complete');
       expect(PHASES.indexOf('policy-selection')).toBeLessThan(PHASES.indexOf('complete'));
     });
+
+    test('new phases are in correct order relative to neighbors', () => {
+      expect(PHASES.indexOf('implementation')).toBeLessThan(PHASES.indexOf('pre-review-gates'));
+      expect(PHASES.indexOf('pre-review-gates')).toBeLessThan(PHASES.indexOf('review-loop'));
+      expect(PHASES.indexOf('review-loop')).toBeLessThan(PHASES.indexOf('delivery-validation'));
+      expect(PHASES.indexOf('delivery-validation')).toBeLessThan(PHASES.indexOf('docs-update'));
+      expect(PHASES.indexOf('docs-update')).toBeLessThan(PHASES.indexOf('shipping'));
+    });
   });
 
   describe('tasks.json operations', () => {
@@ -205,6 +213,124 @@ describe('workflow-state', () => {
     });
   });
 
+  describe('new phases: pre-review-gates and docs-update', () => {
+    beforeEach(() => {
+      createFlow(
+        { id: '1', title: 'Test', source: 'manual' },
+        { stoppingPoint: 'merged' },
+        testDir
+      );
+    });
+
+    test('PHASES contains pre-review-gates and docs-update', () => {
+      expect(PHASES).toContain('pre-review-gates');
+      expect(PHASES).toContain('docs-update');
+    });
+
+    test('isValidPhase accepts pre-review-gates and docs-update', () => {
+      expect(isValidPhase('pre-review-gates')).toBe(true);
+      expect(isValidPhase('docs-update')).toBe(true);
+    });
+
+    test('completePhase from implementation advances to pre-review-gates', () => {
+      setPhase('implementation', testDir);
+      completePhase(null, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('pre-review-gates');
+      expect(flow.status).toBe('in_progress');
+    });
+
+    test('completePhase from pre-review-gates advances to review-loop', () => {
+      setPhase('pre-review-gates', testDir);
+      completePhase({ passed: true }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('review-loop');
+      expect(flow.status).toBe('in_progress');
+      expect(flow.preReviewResult).toEqual({ passed: true });
+    });
+
+    test('completePhase from review-loop stores reviewResult and advances to delivery-validation', () => {
+      setPhase('review-loop', testDir);
+      completePhase({ approved: true, iterations: 2 }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('delivery-validation');
+      expect(flow.status).toBe('in_progress');
+      expect(flow.reviewResult).toEqual({ approved: true, iterations: 2 });
+    });
+
+    test('completePhase from review-loop stores blocked result (stall-detected)', () => {
+      setPhase('review-loop', testDir);
+      completePhase({ approved: false, blocked: true, reason: 'stall-detected', remaining: { critical: 1 } }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('delivery-validation');
+      expect(flow.reviewResult.approved).toBe(false);
+      expect(flow.reviewResult.reason).toBe('stall-detected');
+    });
+
+    test('completePhase from review-loop stores blocked result (iteration-limit)', () => {
+      setPhase('review-loop', testDir);
+      completePhase({ approved: false, blocked: true, reason: 'iteration-limit', remaining: { critical: 0, high: 2 } }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('delivery-validation');
+      expect(flow.reviewResult.reason).toBe('iteration-limit');
+      expect(flow.reviewResult.remaining.high).toBe(2);
+    });
+
+    test('completePhase stores falsy result (result !== null fix)', () => {
+      setPhase('pre-review-gates', testDir);
+      completePhase({ passed: false, reason: 'lint-failure' }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.preReviewResult).toBeDefined();
+      expect(flow.preReviewResult.passed).toBe(false);
+    });
+
+    test('completePhase from shipping advances to complete', () => {
+      setPhase('shipping', testDir);
+      completePhase(null, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('complete');
+      expect(flow.status).toBe('completed');
+    });
+
+    test('completePhase returns null for unknown current phase', () => {
+      setPhase('review-loop', testDir);
+      // Manually corrupt the phase to an unknown value
+      const currentFlow = readFlow(testDir);
+      currentFlow.phase = 'nonexistent-phase';
+      writeFlow(currentFlow, testDir);
+
+      const result = completePhase(null, testDir);
+      expect(result).toBeNull();
+    });
+
+    test('completePhase from delivery-validation advances to docs-update', () => {
+      setPhase('delivery-validation', testDir);
+      completePhase({ passed: true }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('docs-update');
+      expect(flow.status).toBe('in_progress');
+      expect(flow.deliveryResult).toEqual({ passed: true });
+    });
+
+    test('completePhase from docs-update advances to shipping', () => {
+      setPhase('docs-update', testDir);
+      completePhase({ docsUpdated: true }, testDir);
+
+      const flow = readFlow(testDir);
+      expect(flow.phase).toBe('shipping');
+      expect(flow.status).toBe('in_progress');
+      expect(flow.docsResult).toEqual({ docsUpdated: true });
+    });
+  });
+
   describe('convenience functions', () => {
     test('getFlowSummary returns summary object', () => {
       createFlow(

adapters/codex/skills/next-task/SKILL.md

@@ -17,7 +17,7 @@ This workflow exists because each step serves a purpose. Taking shortcuts defeat
 | Step | Purpose | What Happens If Skipped |
 |------|---------|------------------------|
 | Worktree creation | Parallel task isolation | Conflicts, lost work |
-| Review loop (3 iterations) | Catches bugs humans miss | Bugs ship to production |
+| Review loop (5 iterations, stall-safe) | Catches bugs humans miss | Bugs ship to production |
 | 3-minute CI wait | Auto-reviewers need time | Miss critical feedback |
 | Address all PR comments | Quality gate | Merge blocked, trust lost |
 
@@ -338,6 +338,9 @@ For each fix:
 Use Edit tool to apply. Commit message: "fix: clean up AI slop"`
   });
 }
+
+const gatesPassed = (deslop.fixes?.length || 0) === 0;
+workflowState.completePhase({ passed: gatesPassed, deslopFixes: deslop.fixes?.length || 0, coverageResult });
 ```
 </phase-8>
 
@@ -346,6 +349,10 @@ Use Edit tool to apply. Commit message: "fix: clean up AI slop"`
 
 **Blocking gate** - Must run iterations before delivery validation.
 
+```javascript
+workflowState.startPhase('review-loop');
+```
+
 **CRITICAL**: You MUST spawn multiple parallel reviewer agents. Do NOT use a single generic reviewer.
 
 ### Step 1: Get Changed Files
@@ -415,13 +422,13 @@ For each finding, use Edit tool to apply the suggested fix. Commit after each ba
 
 Repeat steps 3-5 until:
 - `openCount === 0` (all issues resolved) -> approved
-- 3+ iterations with only medium/low issues -> orchestrator may override
-- 5 iterations reached -> blocked
+- Same findings hash for 2 consecutive iterations (stall detected) -> blocked
+- 5 iterations reached (hard limit) -> blocked
 
 ### Review Iteration Rules
 - MUST run at least 1 full iteration with ALL 4 core reviewers
 - Do NOT use a single generic reviewer - spawn all specialists in parallel
-- Orchestrator may override after 3+ iterations if only medium/low issues remain
+- MUST continue while `openCount > 0`. Only stop on: openCount===0, stall detection, or 5-iteration hard limit
 - Do not skip directly to delivery validation
 - Do not claim "review passed" without spawning the reviewer agents
 
@@ -436,6 +443,11 @@ After review loop completes, output:
 - Findings resolved: X critical, Y high, Z medium
 - Status: approved | blocked
 ```
+
+Then advance the workflow state:
+```javascript
+workflowState.completePhase({ approved, iterations, remaining });
+```
 </phase-9>
 
 <phase-10>

adapters/opencode/agents/ci-monitor.md

@@ -92,6 +92,7 @@ Agent MUST exit monitoring loop when ANY of these occur:
 4. ci-fixer reports unfixable issue - ESCALATE
 
 - Phase: ci-wait
+- Call `workflowState.completePhase(result)` to advance workflow state
 
 
 ## Output Format

adapters/opencode/agents/delivery-validator.md

@@ -74,13 +74,8 @@ delivery-validator (YOU ARE HERE)
 
 ## State Updates
 
-```javascript
-// On success
-workflowState.completePhase({ approved: true, checks });
+- Call `workflowState.completePhase(result)` to advance workflow state
 
-// On failure
-workflowState.failPhase('Validation failed', { failedChecks, fixInstructions });
-```
 
 ## Output Format
 

adapters/opencode/agents/exploration-agent.md

@@ -117,6 +117,7 @@ git diff HEAD~20 -- ${RELEVANT_DIRS} --stat
 ## Phase 10: Update State
 
 - Phase: exploration
+- Call `workflowState.completePhase(result)` to advance workflow state
 
 
 ## Output Format

adapters/opencode/agents/implementation-agent.md

@@ -133,7 +133,8 @@ Make atomic, meaningful commits:
 
 After implementation completes:
 
-*(JavaScript reference - not executable in OpenCode)*
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 
 ## [CRITICAL] WORKFLOW GATES - READ CAREFULLY
 

adapters/opencode/commands/next-task.md

@@ -50,7 +50,7 @@ This workflow exists because each step serves a purpose. Taking shortcuts defeat
 | Step | Purpose | What Happens If Skipped |
 |------|---------|------------------------|
 | Worktree creation | Parallel task isolation | Conflicts, lost work |
-| Review loop (3 iterations) | Catches bugs humans miss | Bugs ship to production |
+| Review loop (5 iterations, stall-safe) | Catches bugs humans miss | Bugs ship to production |
 | 3-minute CI wait | Auto-reviewers need time | Miss critical feedback |
 | Address all PR comments | Quality gate | Merge blocked, trust lost |
 
@@ -236,11 +236,9 @@ Spawn: `worktree-manager` (haiku)
 
 **Last human interaction point.** Present plan via EnterPlanMode/ExitPlanMode.
 
-```javascript
-EnterPlanMode();
-// User reviews and approves via ExitPlanMode
-workflowState.completePhase({ planApproved: true, plan });
-```
+- Use EnterPlanMode for user approval
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 </phase-6>
 
 <phase-7>
@@ -260,6 +258,7 @@ workflowState.completePhase({ planApproved: true, plan });
 - Invoke `@deslop-agent` agent
 - Invoke `@test-coverage-checker` agent
 - Phase: pre-review-gates
+- Call `workflowState.completePhase(result)` to advance workflow state
 
 </phase-8>
 
@@ -268,6 +267,10 @@ workflowState.completePhase({ planApproved: true, plan });
 
 **Blocking gate** - Must run iterations before delivery validation.
 
+```javascript
+workflowState.startPhase('review-loop');
+```
+
 **CRITICAL**: You MUST spawn multiple parallel reviewer agents. Do NOT use a single generic reviewer.
 
 ### Step 1: Get Changed Files
@@ -307,13 +310,13 @@ For each finding, use Edit tool to apply the suggested fix. Commit after each ba
 
 Repeat steps 3-5 until:
 - `openCount === 0` (all issues resolved) -> approved
-- 3+ iterations with only medium/low issues -> orchestrator may override
-- 5 iterations reached -> blocked
+- Same findings hash for 2 consecutive iterations (stall detected) -> blocked
+- 5 iterations reached (hard limit) -> blocked
 
 ### Review Iteration Rules
 - MUST run at least 1 full iteration with ALL 4 core reviewers
 - Do NOT use a single generic reviewer - spawn all specialists in parallel
-- Orchestrator may override after 3+ iterations if only medium/low issues remain
+- MUST continue while `openCount > 0`. Only stop on: openCount===0, stall detection, or 5-iteration hard limit
 - Do not skip directly to delivery validation
 - Do not claim "review passed" without spawning the reviewer agents
 
@@ -328,6 +331,10 @@ After review loop completes, output:
 - Findings resolved: X critical, Y high, Z medium
 - Status: approved | blocked
 ```
+
+Then advance the workflow state:
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 </phase-9>
 
 <phase-10>
@@ -349,6 +356,7 @@ Uses the unified sync-docs agent from the sync-docs plugin with `before-pr` scop
 
 - Invoke `@sync-docs-agent` agent
 - Phase: docs-update
+- Call `workflowState.completePhase(result)` to advance workflow state
 
 </phase-11>
 

adapters/opencode/commands/ship.md

@@ -78,7 +78,8 @@ Parse from $ARGUMENTS:
 
 ## State Integration
 
-*(JavaScript reference - not executable in OpenCode)*
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 
 ## Phase 1: Pre-flight Checks
 

adapters/opencode/skills/discover-tasks/SKILL.md

@@ -66,7 +66,8 @@ done
 
 ### Phase 5: Update State
 
-*(JavaScript reference - not executable in OpenCode)*
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 
 ### Phase 6: Post Comment (GitHub only)
 

adapters/opencode/skills/orchestrate-review/SKILL.md

@@ -1,5 +1,6 @@
 ---
 name: orchestrate-review
+version: 5.1.0
 description: "Use when user asks to \"deep review the code\", \"thorough code review\", \"multi-pass review\", or when orchestrating the Phase 9 review loop. Provides review pass definitions (code quality, security, performance, test coverage), signal detection patterns, and iteration algorithms."
 metadata:
   short-description: "Multi-pass code review orchestration"
@@ -63,6 +64,8 @@ if (signals.hasDevops) passes.push({ id: 'devops', role: 'devops reviewer',
 
 - Invoke `@general-purpose` agent
 - Invoke `@deslop-agent` agent
+- Use AskUserQuestion tool for user input
+- Call `workflowState.completePhase(result)` to advance workflow state
 
 
 ## Review Queue

adapters/opencode/skills/validate-delivery/SKILL.md

@@ -68,15 +68,8 @@ AFTER=$(npm test 2>&1 | grep -oE '[0-9]+ passing' | grep -oE '[0-9]+')
 
 ### If All Pass
 
-```javascript
-workflowState.completePhase({
-  approved: true,
-  checks,
-  summary: 'All validation checks passed'
-});
-
-return { approved: true, checks };
-```
+- Call `workflowState.completePhase(result)` to advance workflow state
+
 
 ### If Any Fail