fix(#38): replace misleading approved checkpoint in execute-phase human_needed branch (#560)

Fixes #38

Removes the `"approved" → continue` ack-and-advance shortcut from the `human_needed` verification path in execute-phase. The phase now stays pending until `/gsd:verify-work` completes the UAT and triggers its auto-transition — enforcing the invariant that ROADMAP advances only after a completed verification record.

Also fixes: UAT file format mismatch (`status: testing` + correct `## Current Test` key shape), filename alignment (`{phase_num}-UAT.md`), explicit ack handler covering legacy `approved` keyword, stale `HUMAN-UAT.md` references in agent/reference files.

.changeset/happy-herons-snooze.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 560
+---
+execute-phase no longer accepts 'approved' at the human_needed checkpoint as a substitute for actual verification — the phase stays pending until /gsd:verify-work completes the UAT.

agents/gsd-verifier.md

@@ -544,7 +544,7 @@ done
 </verify>
 ```
 
-Merge those harvested items into the same human verification list as your own analysis. Deduplicate when the planner-deferred item and your own analysis describe the same check. The downstream `human_needed` → HUMAN-UAT.md path in `workflows/execute-phase.md` is the single sink — no separate file is created.
+Merge those harvested items into the same human verification list as your own analysis. Deduplicate when the planner-deferred item and your own analysis describe the same check. The downstream `human_needed` → `{phase_num}-UAT.md` path in `workflows/execute-phase.md` is the single sink — no separate file is created.
 
 **Format:**
 

get-shit-done/references/checkpoints.md

@@ -18,7 +18,7 @@ Plans execute autonomously. Checkpoints formalize interaction points where human
 
 **When:** Claude completed automated work, human confirms it works correctly.
 
-> **Default mode (#3309): `workflow.human_verify_mode = end-of-phase`.** New projects do NOT halt mid-flight at `checkpoint:human-verify`. The planner suppresses those task emissions and embeds the verification details into the relevant `auto` task's `<verify><human-check>` block; the verifier harvests every `<verify><human-check>` at end-of-phase (Step 8) and consolidates them into the existing `human_needed` → HUMAN-UAT.md flow in `workflows/execute-phase.md`. The user reviews everything in one batch.
+> **Default mode (#3309): `workflow.human_verify_mode = end-of-phase`.** New projects do NOT halt mid-flight at `checkpoint:human-verify`. The planner suppresses those task emissions and embeds the verification details into the relevant `auto` task's `<verify><human-check>` block; the verifier harvests every `<verify><human-check>` at end-of-phase (Step 8) and consolidates them into the existing `human_needed` → `{phase_num}-UAT.md` flow in `workflows/execute-phase.md`. The user reviews everything in one batch.
 >
 > **Why this is the default:** every mid-flight halt costs a full executor cold-start (CLAUDE.md, MEMORY.md, STATE.md, plan re-read on respawn) because subagent context is discarded across the pause. A plan with N human-verify checkpoints pays the cold-start cost N+1 times — measured at "tens of thousands of tokens" per round-trip on real projects.
 >

get-shit-done/references/planner-human-verify-mode.md

@@ -27,7 +27,7 @@ Instead, fold each would-be verification step into the relevant `auto` task usin
 </task>
 ```
 
-The verifier (Step 8) harvests every `<verify><human-check>` block at end-of-phase and consolidates them into the existing `human_needed` → HUMAN-UAT.md path in `workflows/execute-phase.md`. The user reviews everything in one batch instead of paying a cold-start cost per item.
+The verifier (Step 8) harvests every `<verify><human-check>` block at end-of-phase and consolidates them into the existing `human_needed` → `{phase_num}-UAT.md` path in `workflows/execute-phase.md`. The user reviews everything in one batch instead of paying a cold-start cost per item.
 
 ### `mid-flight` (opt-back-in — pre-#3309 behavior)
 

get-shit-done/workflows/execute-phase.md

@@ -1406,11 +1406,11 @@ grep "^status:" "$PHASE_DIR"/*-VERIFICATION.md | cut -d: -f2 | tr -d ' '
 
 **Step A: Persist human verification items as UAT file.**
 
-Create `{phase_dir}/{phase_num}-HUMAN-UAT.md` using UAT template format:
+Create `{phase_dir}/{phase_num}-UAT.md` using UAT template format:
 
 ```markdown
 ---
-status: partial
+status: testing
 phase: {phase_num}-{phase_name}
 source: [{phase_num}-VERIFICATION.md]
 started: [now ISO]
@@ -1419,7 +1419,11 @@ updated: [now ISO]
 
 ## Current Test
 
-[awaiting human testing]
+number: 1
+name: {first human_verification item description}
+expected: |
+  {expected behavior from VERIFICATION.md}
+awaiting: user response
 
 ## Tests
 
@@ -1443,26 +1447,32 @@ blocked: 0
 
 Commit the file:
 ```bash
-gsd_run query commit "test({phase_num}): persist human verification items as UAT" --files "{phase_dir}/{phase_num}-HUMAN-UAT.md"
+gsd_run query commit "test({phase_num}): persist human verification items as UAT" --files "{phase_dir}/{phase_num}-UAT.md"
 ```
 
 **Step B: Present to user:**
 
 ```
-## ✓ Phase {X}: {Name} — Human Verification Required
+## ◷ Phase {X}: {Name} — Human Verification Needed
 
-All automated checks passed. {N} items need human testing:
+All automated checks passed. {N} item(s) require human testing before this phase can be marked complete:
 
 {From VERIFICATION.md human_verification section}
 
-Items saved to `{phase_num}-HUMAN-UAT.md` — they will appear in `/gsd:progress` and `/gsd:audit-uat`.
+Tests saved to `{phase_num}-UAT.md`.
 
-"approved" → continue | Report issues → gap closure
+When ready to run the tests:
+
+`/gsd:verify-work {X} ${GSD_WS}`
+
+Verify-work will walk you through each item and mark the phase complete when all tests pass.
 ```
 
-**If user says "approved":** Proceed to `update_roadmap`. The HUMAN-UAT.md file persists with `status: partial` and will surface in future progress checks until the user runs `/gsd:verify-work` on it.
+**Do NOT advance the phase from this branch.** Phase completion is handled by verify-work's auto-transition after UAT passes.
 
-**If user reports issues:** Proceed to gap closure as currently implemented.
+**If user acknowledges without reporting issues (including "ok", "noted", "ack", "got it", "approved", "done", "yes", "pass", or similar):** Stop. The phase remains pending. No further orchestrator action — wait for the user to run `/gsd:verify-work`.
+
+**If user reports issues now (before running verify-work):** Proceed to gap closure as currently implemented.
 
 **If gaps_found:**
 ```

tests/fix-3722-execute-phase-human-needed-checkpoint.test.cjs

@@ -0,0 +1,128 @@
+// allow-test-rule: source-text-is-the-product
+// execute-phase.md IS the runtime contract loaded by the orchestrator.
+// Asserting that the "ack-and-advance" path is absent is the only way to verify
+// the state machine lie (issue #38) cannot regress at runtime.
+'use strict';
+
+/**
+ * execute-phase.md human_needed branch — issue #38 / fix #3722
+ *
+ * The old design offered '"approved" → continue' as a shortcut that advanced
+ * ROADMAP.md without completing human verification. This is a state machine lie:
+ * the phase appears complete in the project record while HUMAN-UAT.md items
+ * remain unresolved.
+ *
+ * The correct design:
+ *   - human_needed branch creates a {phase_num}-UAT.md file (not {phase_num}-HUMAN-UAT.md)
+ *   - directs the user to /gsd:verify-work to complete verification
+ *   - does NOT call update_roadmap directly (phase completion goes through verify-work)
+ *   - does NOT offer "approved" → continue as a bypass
+ */
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const EXECUTE_PHASE = path.join(
+  __dirname,
+  '..',
+  'get-shit-done',
+  'workflows',
+  'execute-phase.md'
+);
+
+describe('execute-phase.md human_needed branch — issue #38', () => {
+  let content;
+
+  // Read once; all tests share the string.
+  test('workflow file is readable', () => {
+    content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    assert.ok(content.length > 0, 'execute-phase.md must be non-empty');
+  });
+
+  test('human_needed section exists', () => {
+    if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    assert.ok(
+      content.includes('human_needed'),
+      'execute-phase.md must contain a human_needed branch'
+    );
+  });
+
+  test('"approved" → continue bypass is absent from human_needed branch', () => {
+    if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    // The old prompt offered '"approved" → continue' as a shortcut that advanced
+    // ROADMAP.md without completing verification. That path must not exist.
+    assert.ok(
+      !content.includes('"approved" → continue'),
+      'human_needed branch must not offer "approved" → continue: it marks the phase complete without verification (issue #38)'
+    );
+  });
+
+  test('human_needed branch does NOT call update_roadmap directly', () => {
+    if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    // Locate the human_needed section and check that update_roadmap does not
+    // appear before the gaps_found section (i.e. it is not reachable from human_needed).
+    const humanNeededIdx = content.indexOf('**If human_needed:**');
+    const gapsFoundIdx = content.indexOf('**If gaps_found:**');
+    const updateRoadmapIdx = content.indexOf('update_roadmap', humanNeededIdx);
+
+    assert.ok(humanNeededIdx !== -1, '**If human_needed:** section must exist');
+    assert.ok(gapsFoundIdx !== -1, '**If gaps_found:** section must exist');
+    assert.ok(
+      humanNeededIdx < gapsFoundIdx,
+      'human_needed section must appear before gaps_found section'
+    );
+
+    // update_roadmap must not appear between human_needed and gaps_found sections
+    const updateRoadmapBetween =
+      updateRoadmapIdx !== -1 &&
+      updateRoadmapIdx > humanNeededIdx &&
+      updateRoadmapIdx < gapsFoundIdx;
+
+    assert.ok(
+      !updateRoadmapBetween,
+      'update_roadmap must not be reachable directly from the human_needed branch — phase completion must go through verify-work (issue #38)'
+    );
+  });
+
+  test('human_needed branch directs user to /gsd:verify-work', () => {
+    if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    const humanNeededIdx = content.indexOf('**If human_needed:**');
+    const gapsFoundIdx = content.indexOf('**If gaps_found:**');
+    assert.ok(humanNeededIdx !== -1, '**If human_needed:** section must exist');
+
+    const humanNeededSection = content.slice(
+      humanNeededIdx,
+      gapsFoundIdx !== -1 ? gapsFoundIdx : undefined
+    );
+    assert.ok(
+      humanNeededSection.includes('verify-work'),
+      'human_needed branch must direct the user to /gsd:verify-work to complete verification'
+    );
+  });
+
+  test('human_needed branch creates {phase_num}-UAT.md (not HUMAN-UAT.md)', () => {
+    if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
+    const humanNeededIdx = content.indexOf('**If human_needed:**');
+    const gapsFoundIdx = content.indexOf('**If gaps_found:**');
+    assert.ok(humanNeededIdx !== -1, '**If human_needed:** section must exist');
+
+    const humanNeededSection = content.slice(
+      humanNeededIdx,
+      gapsFoundIdx !== -1 ? gapsFoundIdx : undefined
+    );
+
+    // The file should be named {phase_num}-UAT.md so verify-work's glob picks it up
+    assert.ok(
+      humanNeededSection.includes('-UAT.md'),
+      'human_needed branch must create a {phase_num}-UAT.md file for verify-work to resume'
+    );
+
+    // HUMAN-UAT.md causes a naming mismatch with verify-work's create_uat_file step
+    assert.ok(
+      !humanNeededSection.includes('HUMAN-UAT.md'),
+      'human_needed branch must NOT create HUMAN-UAT.md — use {phase_num}-UAT.md to align with verify-work\'s resume path (issue #38 edge case 3)'
+    );
+  });
+});