From 1c073c1c819c32dc82d54ec8275dffa514f4c3a3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 12 Jun 2026 15:55:01 -0400 Subject: [PATCH] fix(#1107): progress consults verification.status before reporting a phase complete (#1116) /gsd-progress derived phase completeness from plan/summary counts only and never consulted the verification.status query (the #651 seam), so a phase whose VERIFICATION.md ended human_needed or gaps_found was reported complete and routing skipped to the next phase. Add Step 1.7 (consult verification.status for the current phase) and routing rows that send gaps_found to plan-phase --gaps (Route V.gaps) and human_needed to verify-work (Route V.human) before the generic complete row. passed/missing/unknown still route as complete so unverified phases are not falsely blocked. Closes #1107 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 --- ...07-progress-verification-status-routing.md | 5 ++ gsd-core/workflows/progress.md | 59 +++++++++++++- tests/progress-forensic.test.cjs | 79 +++++++++++++++++++ tests/workflow-size-baseline.json | 2 +- 4 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 .changeset/1107-progress-verification-status-routing.md diff --git a/.changeset/1107-progress-verification-status-routing.md b/.changeset/1107-progress-verification-status-routing.md new file mode 100644 index 000000000..0b844027b --- /dev/null +++ b/.changeset/1107-progress-verification-status-routing.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1116 +--- +**`/gsd-progress` no longer reports a phase as complete (and routes to the next phase) when its verification ended `human_needed` or `gaps_found`** — routing derived completeness from plan/summary counts only and never consulted the `verification.status` query (the seam built in #651). A new Step 1.7 consults it for the current phase, and the routing table sends `gaps_found` to `/gsd:plan-phase {phase} --gaps` (Route V.gaps) and `human_needed` to `/gsd:verify-work {phase}` (Route V.human) before the generic complete row. `passed`, `missing` (unverified), and `unknown` still route as complete, so unverified phases are not falsely blocked. (#1107) diff --git a/gsd-core/workflows/progress.md b/gsd-core/workflows/progress.md index 521e573ef..735911b98 100644 --- a/gsd-core/workflows/progress.md +++ b/gsd-core/workflows/progress.md @@ -271,6 +271,19 @@ Resume testing: `/gsd:verify-work {phase} ${GSD_WS}` — retest specific phase This is a WARNING, not a blocker — routing proceeds normally. The debt is visible so the user can make an informed choice. +**Step 1.7: Check verification status for the current phase** + +A phase whose verification ended `gaps_found` or `human_needed` is NOT complete, even when every PLAN.md has a matching SUMMARY.md. The count-based status (`roadmap.analyze`) only sees plans/summaries, so without this check such a phase is reported complete and routing skips straight to the next phase. When the phase appears count-complete (`summaries = plans AND plans > 0`), consult the verification report (the same `verification.status` gate `ship` and `execute-phase` use, from #651): + +```bash +PHASE_DIR=".planning/phases/[current-phase-dir]" +VERIFICATION=$(gsd_run query verification.status "${PHASE_DIR}" 2>/dev/null) +VERIFICATION_STATUS=$(printf '%s' "$VERIFICATION" | jq -r '.status' 2>/dev/null || echo "") +VERIFICATION_NEXT_ACTION=$(printf '%s' "$VERIFICATION" | jq -r '.next_action' 2>/dev/null || echo "") +``` + +Track: `verification_status` — the `.status` field (`passed | gaps_found | human_needed | missing | unknown`). The query already handles a missing VERIFICATION.md (returns `missing`) and unexpected values, so no per-status file probing is needed. `passed`, `missing` (not yet verified), and `unknown` route as complete (Step 3) — `missing` with an advisory that the phase is unverified; `gaps_found` and `human_needed` route back to close the verification debt (Step 2). + **Step 2: Route based on counts** | Condition | Meaning | Action | @@ -278,9 +291,13 @@ This is a WARNING, not a blocker — routing proceeds normally. The debt is visi | uat_partial > 0 | UAT testing incomplete | Go to **Route E.2** | | uat_with_gaps > 0 | UAT gaps need fix plans | Go to **Route E** | | summaries < plans | Unexecuted plans exist | Go to **Route A** | -| summaries = plans AND plans > 0 | Phase complete | Go to Step 3 | +| summaries = plans AND plans > 0 AND verification_status = gaps_found | Phase executed; verification found gaps | Go to **Route V.gaps** | +| summaries = plans AND plans > 0 AND verification_status = human_needed | Phase executed; awaiting human verification | Go to **Route V.human** | +| summaries = plans AND plans > 0 | Phase complete (verification passed, missing, or n/a) | Go to Step 3 | | plans = 0 | Phase not yet planned | Go to **Route B** | +Rows are evaluated top to bottom; the first matching row wins. The two `verification_status` rows must precede the general `summaries = plans` row so a non-`passed` verification is not reported as complete. + --- **Route A: Unexecuted plan exists** @@ -431,6 +448,46 @@ UAT.md exists with `status: partial` — testing session ended before all items --- +**Route V.gaps: verification found gaps (gaps_found)** + +VERIFICATION.md exists with `status: gaps_found` — verification identified gaps that need fix plans. The phase is NOT complete. + +``` +--- + +## ⚠ Verification Gaps Found + +**{phase_num}-VERIFICATION.md** reports `gaps_found`. ${VERIFICATION_NEXT_ACTION} + +`/clear` then: + +`/gsd:plan-phase {phase} --gaps ${GSD_WS}` + +--- +``` + +--- + +**Route V.human: human verification required (human_needed)** + +VERIFICATION.md exists with `status: human_needed` — automated checks passed but manual verification items remain. The phase is NOT complete until they are resolved. + +``` +--- + +## Human Verification Required + +**{phase_num}-VERIFICATION.md** reports `human_needed`. ${VERIFICATION_NEXT_ACTION} + +`/clear` then: + +`/gsd:verify-work {phase} ${GSD_WS}` — resume human verification + +--- +``` + +--- + **Step 3: Check milestone status (only when phase complete)** Read ROADMAP.md and identify: diff --git a/tests/progress-forensic.test.cjs b/tests/progress-forensic.test.cjs index 106760f18..97f2d3c0f 100644 --- a/tests/progress-forensic.test.cjs +++ b/tests/progress-forensic.test.cjs @@ -133,3 +133,82 @@ describe('#2189: progress --forensic flag', () => { ); }); }); + +/** + * Regression — issue #1107 + * + * /gsd-progress reported a phase as complete and routed to the next phase even + * when its VERIFICATION.md ended `human_needed` / `gaps_found`, because routing + * derived completeness from plan/summary counts only and never consulted the + * `verification.status` query (built in #651). The fix adds a Step 1.7 consult + * and routing rows that send non-`passed` phases back to close the debt. + */ +describe('#1107: progress routing consults verification.status before reporting complete', () => { + function readWorkflow() { + return fs.readFileSync( + path.join(__dirname, '..', 'gsd-core', 'workflows', 'progress.md'), 'utf8' + ); + } + + test('workflow consults verification.status for the current phase', () => { + const workflow = readWorkflow(); + assert.ok( + workflow.includes('verification.status'), + 'progress workflow must query verification.status (the #651 seam)' + ); + assert.ok( + workflow.includes('verification_status'), + 'progress workflow must track a verification_status value for routing' + ); + }); + + test('routing table has gaps_found and human_needed rows BEFORE the generic complete row', () => { + const workflow = readWorkflow(); + const gapsIdx = workflow.indexOf('verification_status = gaps_found'); + const humanIdx = workflow.indexOf('verification_status = human_needed'); + const completeIdx = workflow.indexOf('Phase complete (verification passed'); + assert.ok(gapsIdx > -1, 'routing table must have a gaps_found row'); + assert.ok(humanIdx > -1, 'routing table must have a human_needed row'); + assert.ok(completeIdx > -1, 'routing table must keep a generic complete row'); + assert.ok( + gapsIdx < completeIdx && humanIdx < completeIdx, + 'verification rows must precede the generic "summaries = plans" complete row (first-match-wins)' + ); + }); + + test('gaps_found routes to plan-phase --gaps (Route V.gaps)', () => { + const workflow = readWorkflow(); + // Anchor on the definition heading (`**Route V.gaps:`), not the routing-table + // reference (`Go to **Route V.gaps**`). + assert.ok(workflow.includes('**Route V.gaps:'), 'must define a Route V.gaps section'); + const route = workflow.slice( + workflow.indexOf('**Route V.gaps:'), + workflow.indexOf('**Route V.human:') + ); + assert.ok( + route.includes('--gaps') && route.includes('plan-phase'), + 'Route V.gaps must route to /gsd:plan-phase {phase} --gaps' + ); + }); + + test('human_needed routes to verify-work (Route V.human)', () => { + const workflow = readWorkflow(); + assert.ok(workflow.includes('**Route V.human:'), 'must define a Route V.human section'); + const route = workflow.slice( + workflow.indexOf('**Route V.human:'), + workflow.indexOf('**Step 3', workflow.indexOf('**Route V.human:')) + ); + assert.ok( + route.includes('verify-work'), + 'Route V.human must route to /gsd:verify-work {phase}' + ); + }); + + test('missing/passed verification still routes as complete (no false blocker)', () => { + const workflow = readWorkflow(); + assert.ok( + workflow.includes('Phase complete (verification passed, missing, or n/a)'), + 'the generic complete row must still cover passed/missing/unknown so unverified phases are not falsely blocked' + ); + }); +}); diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index 3acfb3825..40bbd0cc6 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -56,7 +56,7 @@ "plant-seed.md": 11741, "pr-branch.md": 4994, "profile-user.md": 20457, - "progress.md": 26647, + "progress.md": 29387, "quick.md": 46213, "reapply-patches.md": 20393, "remove-phase.md": 8469,