diff --git a/.changeset/happy-ibex-romp.md b/.changeset/happy-ibex-romp.md new file mode 100644 index 000000000..fd3aacaa8 --- /dev/null +++ b/.changeset/happy-ibex-romp.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3327 +--- +**/gsd-progress and /gsd-execute-plan stop counting superseded plans as outstanding work** — seven prompt-layer sites across execute-plan.md, plan-phase.md, plan-review-convergence.md and progress.md counted plans with a raw `ls *-PLAN.md | wc -l`, so a plan marked `status: superseded` was still counted as outstanding, a phase on the nested plans/ layout (#3139) reported zero plans it actually had, and loosely-named plan files were missed entirely. Every site now calls `phase find`, which gains three additive fields — `plan_count`/`summary_count` (live, superseded excluded — 'how much is left') and `plan_count_all` (physical, every plan on disk — 'what did the planner write') — so what a workflow shows and what `phase find` reports for the same phase are now the same number. This also fixes a dead route: progress.md's Route 0 resume-incomplete-phase check read `.plans`/`.summaries` arrays that its producer, roadmap.analyze, never emitted (it emits plan_count/summary_count scalars), so both counts were always 0 and the check had never fired at all — it now fires correctly. **This is a behavior change you'll notice:** plan/summary counts shown by these workflows will move — toward being correct. (#3218) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 9d4eec668..7aa6bdeb2 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -166,6 +166,29 @@ the same canonical sentinel predicate `phases list` uses — previously its own regex excluded `999` but not `0`, so a `0-*` directory could be destroyed on this irreversible path. +### `find-phase` plan/summary counts (live vs physical) + +`find-phase`'s JSON carries the existing `plans[]` / `summaries[]` arrays +**unchanged**, plus three additive scalar fields: + +| Field | Set | Answers | +|---|---|---| +| `plan_count` | live — `status: superseded` plans excluded | "how much outstanding work is left in this phase?" (same set as `plans[]`) | +| `summary_count` | live | same, for `summaries[]` | +| `plan_count_all` | physical — every canonically-named plan file on disk, superseded included | "what has the planner actually written to disk?" | + +Naming mirrors `roadmap analyze`'s existing `plan_count`/`summary_count`, and the +`_all` suffix echoes the underlying `scanPhasePlans` field it is drawn from. +**Pick by the question you're asking, not by which number looks bigger:** a +phase where every plan is `status: superseded` correctly reports `plan_count: 0` +— that is a real "nothing outstanding" answer, not a bug — while +`plan_count_all` still reports the physical count, so a check for "did the +planner produce anything at all" doesn't misread a fully-superseded phase as +untouched. + +When the phase can't be resolved, all three fields are `null`, not `0` — a +fabricated `0` would read identically to a genuinely empty phase. + ### Phase SUMMARY artifact check A phase `SUMMARY.md` asserts which files the phase created or modified. On diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index 26d297ccd..c4ea92b72 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -529,9 +529,14 @@ gsd_run query commit "" --files .planning/codebase/*.md --amend If `USER_SETUP_CREATED=true`: display `⚠️ USER SETUP REQUIRED` with path + env/config tasks at TOP. +Get plan/summary counts for the current phase from the single owner (#3218 — LIVE +counts, i.e. `status: superseded` plans excluded, matching this route's +"outstanding work" question): + ```bash -(ls -1 .planning/phases/[current-phase-dir]/*-PLAN.md 2>/dev/null || true) | wc -l -(ls -1 .planning/phases/[current-phase-dir]/*-SUMMARY.md 2>/dev/null || true) | wc -l +PHASE_COUNTS=$(gsd_run query find-phase "${PHASE}") +PLAN_COUNT=$(echo "$PHASE_COUNTS" | jq -r '.plan_count // 0') +SUMMARY_COUNT=$(echo "$PHASE_COUNTS" | jq -r '.summary_count // 0') ``` | Condition | Route | Action | diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index fcef92b84..61d1eb5cc 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -863,7 +863,12 @@ If `section_manifest` is `null` or `"chunked-planning-mode"` is in its `included **Triggered when:** Agent() returns but the return contains no recognized marker (`## PLANNING COMPLETE`, `## PHASE SPLIT RECOMMENDED`, `## ⚠ Source Audit`, `## CHECKPOINT REACHED`, `## PLANNING INCONCLUSIVE`). ```bash -DISK_PLANS=$(ls "${PHASE_DIR}"/*-PLAN.md 2>/dev/null | wc -l | tr -d ' ') +# #3218: this asks "did the planner write files to disk at all" — a +# planner-produced-nothing check, not outstanding-work counting — so it +# takes the PHYSICAL set (`plan_count_all`, status:superseded INCLUDED): a +# superseded plan is still a file the planner wrote, and this check must not +# read "nothing written" just because every plan happens to be superseded. +DISK_PLANS=$(gsd_run query find-phase "${PHASE_NUMBER}" | jq -r '.plan_count_all // 0') ``` **If `DISK_PLANS` > 0:** The planner wrote plans to disk but the Agent() return was empty or @@ -1037,7 +1042,12 @@ If thinking_partner disabled: skip this block entirely. **Triggered when:** Checker Agent() returns but the return contains neither `## VERIFICATION PASSED` nor `## ISSUES FOUND`. ```bash -DISK_PLANS=$(ls "${PHASE_DIR}"/*-PLAN.md 2>/dev/null | wc -l | tr -d ' ') +# #3218: this asks "did the planner write files to disk at all" — a +# planner-produced-nothing check, not outstanding-work counting — so it +# takes the PHYSICAL set (`plan_count_all`, status:superseded INCLUDED): a +# superseded plan is still a file the planner wrote, and this check must not +# read "nothing written" just because every plan happens to be superseded. +DISK_PLANS=$(gsd_run query find-phase "${PHASE_NUMBER}" | jq -r '.plan_count_all // 0') ``` **If `DISK_PLANS` > 0:** Plans exist on disk; the checker return was empty or truncated (the diff --git a/gsd-core/workflows/plan-review-convergence.md b/gsd-core/workflows/plan-review-convergence.md index 143e15bf0..fd84fa691 100644 --- a/gsd-core/workflows/plan-review-convergence.md +++ b/gsd-core/workflows/plan-review-convergence.md @@ -156,9 +156,12 @@ Skill(skill="gsd-plan-phase", args="{PHASE} {GSD_WS}") Run plan-phase **inline** (do NOT wrap it in Agent()). The convergence orchestrator runs at depth 0 with Agent available, so inline plan-phase can spawn gsd-planner and gsd-plan-checker at depth 1 — the one level of nesting that works on Claude Code. Wrapping plan-phase in Agent() would push it to depth 1 where the Agent tool is absent, preventing it from spawning any sub-agents. Wait until plan-phase completes and PLAN.md files are committed before continuing. -After plan-phase completes, verify plans were created: +After plan-phase completes, verify plans were created. This asks "did initial +planning write files to disk" — a planner-produced-nothing check, not +outstanding-work counting — so it takes the PHYSICAL set (`plan_count_all`, +`status: superseded` INCLUDED, #3218): ```bash -PLAN_COUNT=$(ls ${phase_dir}/${padded_phase}-*-PLAN.md 2>/dev/null | wc -l) +PLAN_COUNT=$(gsd_run query find-phase "${PHASE}" | jq -r '.plan_count_all // 0') ``` If PLAN_COUNT == 0: Error — initial planning failed. Exit. diff --git a/gsd-core/workflows/progress.md b/gsd-core/workflows/progress.md index 1ab4217f0..7728a9324 100644 --- a/gsd-core/workflows/progress.md +++ b/gsd-core/workflows/progress.md @@ -181,8 +181,14 @@ if [ -z "$ROADMAP" ]; then else for PHASE_NUM in $(echo "$ROADMAP" | jq -r '.phases[] | (.number // .phase_number)'); do PHASE_DATA=$(echo "$ROADMAP" | jq --arg n "$PHASE_NUM" '.phases[] | select((.number // .phase_number) == ($n | tonumber))') - PLAN_COUNT=$(echo "$PHASE_DATA" | jq '(.plans // []) | length') - SUMMARY_COUNT=$(echo "$PHASE_DATA" | jq '(.summaries // []) | length') + # #3218: $PHASE_DATA is a `.phases[]` entry from `roadmap.analyze`, which + # emits `plan_count`/`summary_count` SCALARS (src/roadmap.cts) — it has + # never emitted `.plans`/`.summaries` ARRAYS. Reading those absent keys + # (even with a `// []` fallback) always produced 0, permanently disabling + # this resume-incomplete-phase check. Read the scalars the producer + # actually emits. + PLAN_COUNT=$(echo "$PHASE_DATA" | jq '.plan_count // 0') + SUMMARY_COUNT=$(echo "$PHASE_DATA" | jq '.summary_count // 0') if [ "${PLAN_COUNT:-0}" -gt "${SUMMARY_COUNT:-0}" ]; then INCOMPLETE_PHASE="$PHASE_NUM" break @@ -213,11 +219,13 @@ Then exit the route step. Do NOT run Steps 1 through Routes A-F. **Step 1: Count plans, summaries, and issues in current phase** -List files in the current phase directory: +Get plan/summary counts for the current phase from the single owner (#3218 — LIVE +counts, i.e. `status: superseded` plans excluded, matching "outstanding work"): ```bash -(ls -1 .planning/phases/[current-phase-dir]/*-PLAN.md 2>/dev/null || true) | wc -l -(ls -1 .planning/phases/[current-phase-dir]/*-SUMMARY.md 2>/dev/null || true) | wc -l +PHASE_COUNTS=$(gsd_run query find-phase "${CURRENT_PHASE}") +X=$(echo "$PHASE_COUNTS" | jq -r '.plan_count // 0') +Y=$(echo "$PHASE_COUNTS" | jq -r '.summary_count // 0') (ls -1 .planning/phases/[current-phase-dir]/*-UAT.md 2>/dev/null || true) | wc -l ``` diff --git a/scripts/baselines/planning-prompt-drift-baseline.json b/scripts/baselines/planning-prompt-drift-baseline.json index ba21bda42..683f760bf 100644 --- a/scripts/baselines/planning-prompt-drift-baseline.json +++ b/scripts/baselines/planning-prompt-drift-baseline.json @@ -1,47 +1,4 @@ { - "$comment": "ADR-3180 Decision 4(e) ratchet, owned by Phase 8 (#3218). See scripts/lint-planning-prompt-drift.cjs. SHRINK-ONLY: entries are removed as sites migrate to the gsd-core CLI; new or changed entries fail lint:ci. `count` is the number of byte-identical (file, text) occurrences acknowledged at this site — a run producing fewer fails as a partial migration, more fails as an unacknowledged new copy.", - "entries": [ - { - "file": "gsd-core/workflows/execute-plan.md", - "text": "(ls -1 .planning/phases/[current-phase-dir]/*-PLAN.md 2>/dev/null || true) | wc -l", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 1 - }, - { - "file": "gsd-core/workflows/execute-plan.md", - "text": "(ls -1 .planning/phases/[current-phase-dir]/*-SUMMARY.md 2>/dev/null || true) | wc -l", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 1 - }, - { - "file": "gsd-core/workflows/plan-phase.md", - "text": "DISK_PLANS=$(ls \"${PHASE_DIR}\"/*-PLAN.md 2>/dev/null | wc -l | tr -d ' ')", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 2 - }, - { - "file": "gsd-core/workflows/plan-review-convergence.md", - "text": "PLAN_COUNT=$(ls ${phase_dir}/${padded_phase}-*-PLAN.md 2>/dev/null | wc -l)", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 1 - }, - { - "file": "gsd-core/workflows/progress.md", - "text": "(ls -1 .planning/phases/[current-phase-dir]/*-PLAN.md 2>/dev/null || true) | wc -l", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 1 - }, - { - "file": "gsd-core/workflows/progress.md", - "text": "(ls -1 .planning/phases/[current-phase-dir]/*-SUMMARY.md 2>/dev/null || true) | wc -l", - "derivation": "plan-count", - "owner_issue": "#3218", - "count": 1 - } - ] + "$comment": "ADR-3180 Decision 4(e) ratchet, owned by Phase 8 (#3218). See scripts/lint-planning-prompt-drift.cjs. SHRINK-ONLY: entries are removed as sites migrate to the gsd-core CLI; new or changed entries fail lint:ci. `count` is the number of byte-identical (file, text) occurrences acknowledged at this site — a run producing fewer fails as a partial migration, more fails as an unacknowledged new copy. Emptied by #3218: all 6 recorded sites (7 occurrences) migrated to `gsd_run query find-phase` — see gsd-core/workflows/{execute-plan,plan-phase,plan-review-convergence,progress}.md.", + "entries": [] } diff --git a/src/phase.cts b/src/phase.cts index eb227e552..352a7aad2 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -459,6 +459,13 @@ function cmdFindPhase(cwd: string, phase: string, raw: boolean): void { phase_name: null, plans: [], summaries: [], + // #3218: scalar counts alongside the arrays above. Left `null` (not `0`) + // when the phase can't be resolved at all — a fabricated `0` here would + // read identically to "phase exists with zero plans", which is a real, + // distinct answer (see the `status: superseded` case below). + plan_count: null, + summary_count: null, + plan_count_all: null, searched_directories: [] as string[], }; @@ -537,10 +544,8 @@ function cmdFindPhase(cwd: string, phase: string, raw: boolean): void { // the owner recognizes, canonical or not) rather than the live-only // `plans`, so a superseded-but-canonically-named plan is not misreported // as a naming violation. - const planNamingWarning = describeNonCanonicalPlans( - phaseFiles, - phaseScan.allPlanFiles.filter(isCanonicalPlanFile), - ); + const canonicalAllPlanFiles = phaseScan.allPlanFiles.filter(isCanonicalPlanFile); + const planNamingWarning = describeNonCanonicalPlans(phaseFiles, canonicalAllPlanFiles); const result: Record = { found: true, @@ -555,6 +560,20 @@ function cmdFindPhase(cwd: string, phase: string, raw: boolean): void { phase_name: phaseName, plans, summaries, + // #3218: scalar counts additive alongside `plans[]`/`summaries[]`, + // which stay unchanged for existing consumers. Naming mirrors + // `roadmap.analyze`'s `plan_count`/`summary_count` (live, i.e. + // status:superseded EXCLUDED — same set as `plans`/`summaries` + // above) so the two surfaces read alike. `plan_count_all` is the + // PHYSICAL count — every canonically-named plan file on disk, + // status:superseded INCLUDED, same set `planNamingWarning` above + // diffs against (`canonicalAllPlanFiles`). The `_all` suffix + // deliberately echoes `scanPhasePlans`'s own `allPlanFiles` field so + // a reader can trace the name back to its source rather than guess + // which of two similarly-named integers is the filtered one. + plan_count: plans.length, + summary_count: summaries.length, + plan_count_all: canonicalAllPlanFiles.length, }; if (planNamingWarning) result['warning'] = planNamingWarning; diff --git a/tests/emitted-drift-acks/1956-cross-artifact-fact-drift.json b/tests/emitted-drift-acks/1956-cross-artifact-fact-drift.json deleted file mode 100644 index cb041da8d..000000000 --- a/tests/emitted-drift-acks/1956-cross-artifact-fact-drift.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": 1, - "paths": { - "plan-review-convergence.md": "#1956: the plan drift guard gains a second axis — a cross-artifact fact-drift pass sitting immediately after the existing source-grounding pass, under the SAME `plan_review.source_grounding` gate. Growth is the new section only (26195 -> 30672 bytes, +4477); no existing text was rewritten and no new config key was introduced. The addition is deliberately inline rather than extracted: ADR-1610 Decision 4 names eager `@`-import relocation as proxy-gaming (it shrinks the measured file while leaving loaded context unchanged or larger), legitimate extraction is Read-at-step lazy, and this pass has no lazy-read seam — it is prose the orchestrator must already hold when it runs the guard. The file remains far inside its DEFAULT tier hard cap of 40960 bytes (tests/workflow-size-budget.test.cjs), with ~11.4 KB of headroom. Content justification: source-grounding proves a plan's cited SYMBOLS exist in source; nothing proved that the same FACT stated in two planning artifacts still agrees. Because each phase runs in a fresh context an agent typically reads one artifact and trusts it, so a stale duplicate — a phase marked complete in STATE.md but in progress in ROADMAP.md, a success criterion the plan restates with a different outcome, a term used against its CONTEXT.md definition — silently steers it wrong. The pass keys on contradicting knowledge rather than similar-looking text (the false-positive mode the issue's own research comment names), gates on a three-way conjunction, defers the axes gsd-plan-checker already owns (Dimensions 1, 7b, 9) so nothing is reported twice, and is advisory only: it never sets hardBlock and contributes to neither HIGH_COUNT nor ACTIONABLE_COUNT, so a project carrying pre-existing drift can still converge." - } -} diff --git a/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json b/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json index 8e0ec0af6..1f4de8d3e 100644 --- a/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json +++ b/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json @@ -1,7 +1,6 @@ { "version": 1, "paths": { - "diagnose-issues.md": "#2649: spawn_agents step gained a pre-dispatch worktree.base-check gate (mirrors execute-phase #683/#1369 and quick #1941). Claude Code's isolation=\"worktree\" forks from origin/HEAD, not live local HEAD; without the gate the documented GSD steady state (commit every step locally, push only on request) hit the verify-only worktree_branch_check guard's exit-42 halt mid-investigation. Growth is the base-check bash block (gsd_run query worktree.base-check --pick shouldDegrade → USE_WORKTREES=false + stderr warning) + the #2649 rationale comment. The verify-only guard stays as a backstop.", - "execute-plan.md": "#2649: Pattern A (single-plan interactive dispatch) gained the same pre-dispatch worktree.base-check gate the wave path (executor-isolation-dispatch.md) and quick.md already run — the triage found Pattern A had the identical missing-gate gap. Growth is the base-check instruction in the Pattern A description (consult shouldDegrade, auto-degrade to sequential on true, keep the verify-only guard as backstop) + the #2649 rationale." + "diagnose-issues.md": "#2649: spawn_agents step gained a pre-dispatch worktree.base-check gate (mirrors execute-phase #683/#1369 and quick #1941). Claude Code's isolation=\"worktree\" forks from origin/HEAD, not live local HEAD; without the gate the documented GSD steady state (commit every step locally, push only on request) hit the verify-only worktree_branch_check guard's exit-42 halt mid-investigation. Growth is the base-check bash block (gsd_run query worktree.base-check --pick shouldDegrade → USE_WORKTREES=false + stderr warning) + the #2649 rationale comment. The verify-only guard stays as a backstop." } } diff --git a/tests/emitted-drift-acks/2650-plan-phase-stall-detection.json b/tests/emitted-drift-acks/2650-plan-phase-stall-detection.json deleted file mode 100644 index 224026f71..000000000 --- a/tests/emitted-drift-acks/2650-plan-phase-stall-detection.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": 1, - "paths": { - "plan-phase.md": "#2650: after merging origin/next's #2993 fragmentization (which extracted the whole 'Chunked Planning Mode' section behind a lazily-loaded steps/chunked-planning-mode.md pointer), plan-phase.md's growth against the new base is no longer about restoring labels at 5 sites in one file — it is the remaining #2650 diff itself. Three of the five stall-watch spawn sites (standard planner, plan-checker, revision-loop planner respawn) still live directly in plan-phase.md; the other two (chunked outline planner, chunked per-plan planner) now live in the extracted gsd-core/workflows/plan-phase/steps/chunked-planning-mode.md, where their ORCHESTRATOR RULE lines were ported during merge resolution so tests/plan-phase-drift-guard.test.cjs (#913), which now counts labels across plan-phase.md AND every file under plan-phase/steps/*.md via readPlanPhaseCombined(), still finds all 7 required labels (5 stall-watch + 2 pre-existing researcher/pattern-mapper labels). At the 3 sites remaining in plan-phase.md itself, converting the plain blocking-wait rule to the bounded gsd_stall_watch mechanism (plus restoring run_in_background=true and adding the step 7.99 pointer to stall-detection-helpers.md) is a net growth over origin/next's own copy of the file, which has the chunked-planning-mode extraction but not the #2650 stall-detection fix. Verified still well under the ADR-857 Phase 6 PRE_PHASE6 cap (94519 bytes) after the merge. #3132: realigned lift rule from covered/backstop-as-status to resolved+verification." - } -} diff --git a/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json b/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json new file mode 100644 index 000000000..62ea585aa --- /dev/null +++ b/tests/emitted-drift-acks/3218-prompt-layer-plan-counts.json @@ -0,0 +1,17 @@ +{ + "version": 1, + "paths": { + "execute-plan.md": { + "reason": "Issue #3218 (epic #3180 phase 8): replaced a raw `ls *-PLAN.md | wc -l` re-derivation with a `gsd_run query find-phase` call plus jq extraction of the count field, routing the plan count through scanPhasePlans (ADR-3180 §7.5) instead of a glob. A few lines longer than the shell one-liner it replaces." + }, + "plan-phase.md": { + "reason": "Issue #3218 (epic #3180 phase 8): grew the most of the four files because it migrated TWO sites off `ls *-PLAN.md | wc -l` (the step 9a and step 11a filesystem-fallback DISK_PLANS checks) to `gsd_run query find-phase` + jq, and because both sites now take the PHYSICAL plan set (`plan_count_all`, status:superseded included) for their 'did the planner/checker write files to disk?' question — the old glob undercounted on the nested plans/ layout and missed loosely-named files." + }, + "plan-review-convergence.md": { + "reason": "Issue #3218 (epic #3180 phase 8): replaced a raw `ls *-PLAN.md | wc -l` re-derivation with a `gsd_run query find-phase` call plus jq extraction of the count field, routing the plan count through scanPhasePlans (ADR-3180 §7.5) so superseded plans are excluded and the review converges on the live set." + }, + "progress.md": { + "reason": "Issue #3218 (epic #3180 phase 8): replaced a raw `ls *-PLAN.md | wc -l` re-derivation with a `gsd_run query find-phase` call plus jq extraction of the count field, and additionally carries the Route-0 dead-path fix (a branch that previously reported zero plans on the nested plans/ layout is now reachable and correct)." + } + } +} diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index b1399d3bd..dbaa88600 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1820,6 +1820,196 @@ describe('phase add with project_code', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// find-phase scalar counts (#3218, phase 8 of epic #3180 — ADR-3180 §7.5) +// +// Additive to `plans[]`/`summaries[]` (matrix A9): `plan_count`/`summary_count` +// mirror `roadmap.analyze`'s naming for the LIVE set (status:superseded +// excluded); `plan_count_all` is the PHYSICAL set — every canonically-named +// plan file on disk, status:superseded included — named to echo +// `scanPhasePlans`'s own `allPlanFiles` field so a reader can trace it back to +// its source. See 40-design.md's "Per-site set" table and Amendment 1. +// ───────────────────────────────────────────────────────────────────────────── + +describe('find-phase scalar counts (#3218)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + function writePlan(phaseDir, n, { superseded = false } = {}) { + const body = superseded + ? ['---', 'status: superseded', '---', '', '# Plan (retired)', ''].join('\n') + : ['# Plan', ''].join('\n'); + fs.writeFileSync(path.join(phaseDir, `03-${n}-PLAN.md`), body); + } + + function writeSummary(phaseDir, n) { + fs.writeFileSync(path.join(phaseDir, `03-${n}-SUMMARY.md`), '# Summary\n'); + } + + test('A1: happy path — 3 plans, 2 summaries, none superseded: live 3 / 2, physical 3', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01'); + writePlan(phaseDir, '02'); + writePlan(phaseDir, '03'); + writeSummary(phaseDir, '01'); + writeSummary(phaseDir, '02'); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.plan_count, 3); + assert.strictEqual(output.summary_count, 2); + assert.strictEqual(output.plan_count_all, 3); + }); + + test('A2 (#2349 case): all 3 plans status:superseded — live 0, physical 3', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01', { superseded: true }); + writePlan(phaseDir, '02', { superseded: true }); + writePlan(phaseDir, '03', { superseded: true }); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + // 0 live plans is a REAL answer here (every plan superseded), not "the + // planner produced nothing" — that read is exactly what plan_count_all + // exists to prevent at the sites that ask the disk-existence question. + assert.strictEqual(output.plan_count, 0); + assert.strictEqual(output.summary_count, 0); + assert.strictEqual(output.plan_count_all, 3); + }); + + test('A3: 1 of 3 superseded — live 2, physical 3', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01'); + writePlan(phaseDir, '02'); + writePlan(phaseDir, '03', { superseded: true }); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.plan_count, 2); + assert.strictEqual(output.plan_count_all, 3); + }); + + test('A6: zero plans — live 0, physical 0 (boundary)', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.plan_count, 0); + assert.strictEqual(output.summary_count, 0); + assert.strictEqual(output.plan_count_all, 0); + }); + + test('A7: phase directory absent — defined verdict, no crash, counts are null (not a fabricated 0)', () => { + const result = runGsdTools('find-phase 99', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.found, false); + // null, not 0 — a fabricated 0 here would read identically to "phase + // exists with zero plans" (A2/A6), which is a real, distinct answer. + assert.strictEqual(output.plan_count, null); + assert.strictEqual(output.summary_count, null); + assert.strictEqual(output.plan_count_all, null); + }); + + // A8 (phase dir unreadable): NOT separately testable — cmdFindPhase's own + // fs.readdirSync(phaseDir) throw is caught by the SAME per-searchDir + // try/catch that produces the A7 not-found result (src/phase.cts, the loop + // around scanPhasePlans), so an unreadable phase dir and a missing one are + // indistinguishable at this seam and both land on the same `notFound` + // object this A7 test already covers (counts null, not a fabricated 0 — + // "surfaced, not silently 0"). Root-safe fs-failure injection would need + // `mock.method` on `fs.readdirSync`, but that only affects the test's own + // process, and `cmdFindPhase`'s output goes through `writeAllSync(1, ...)` — + // this file's own `capturePhaseComplete` helper (above) documents why + // intercepting fd 1 in-process is unsafe on the remote matrix, so this + // command is only exercised via the real `runGsdTools` subprocess, which + // cannot see an in-process fs mock. Same unreachable-path shape already + // recorded for the #2648 plan-coverage gate's B1 case (see the NOTE above + // `describe('phase complete plan-coverage gate (#2648)')`). + + test('A9 (regression): plans[]/summaries[] arrays are unchanged by the new scalars', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01'); + writeSummary(phaseDir, '01'); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepStrictEqual(output.plans, ['03-01-PLAN.md']); + assert.deepStrictEqual(output.summaries, ['03-01-SUMMARY.md']); + }); + + test('A10: live and physical counts are separately addressable — distinct keys, both present', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01', { superseded: true }); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.ok('plan_count' in output && 'plan_count_all' in output, 'both keys must be present'); + assert.notStrictEqual(output.plan_count, output.plan_count_all, 'must actually differ in this fixture'); + assert.strictEqual(output.plan_count, 0); + assert.strictEqual(output.plan_count_all, 1); + }); + + // ── B: parity with the other owners of the same question ────────────────── + + test('B1/B2: find-phase counts equal scanPhasePlans().planFiles/allPlanFiles length for the same phase', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01'); + writePlan(phaseDir, '02', { superseded: true }); + + const result = runGsdTools('find-phase 03', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + const planScanMod = require('../gsd-core/bin/lib/plan-scan.cjs'); + const scan = planScanMod.scanPhasePlans(phaseDir); + assert.strictEqual(output.plan_count, scan.planFiles.length, 'B1: plan_count == planFiles.length'); + assert.strictEqual(output.plan_count_all, scan.allPlanFiles.length, 'B2: plan_count_all == allPlanFiles.length'); + }); + + test('B3: find-phase plan_count agrees with roadmap.analyze plan_count for the same phase', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + ['# Roadmap', '', '- [ ] Phase 3: API', '', '### Phase 3: API', '**Goal:** Build API', '', '---', ''].join('\n'), + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api'); + fs.mkdirSync(phaseDir, { recursive: true }); + writePlan(phaseDir, '01'); + writePlan(phaseDir, '02'); + + const findResult = runGsdTools('find-phase 03', tmpDir); + assert.ok(findResult.success, `find-phase failed: ${findResult.error}`); + const findOutput = JSON.parse(findResult.output); + + const roadmapResult = runGsdTools('roadmap analyze', tmpDir); + assert.ok(roadmapResult.success, `roadmap.analyze failed: ${roadmapResult.error}`); + const roadmapOutput = JSON.parse(roadmapResult.output); + const phase3 = roadmapOutput.phases.find((p) => String(p.number) === '3' || p.number === 3); + assert.ok(phase3, 'roadmap.analyze must report phase 3'); + assert.strictEqual(findOutput.plan_count, phase3.plan_count, 'find-phase and roadmap.analyze must agree'); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // phase add-batch command (#2165) // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/plan-phase-drift-guard.test.cjs b/tests/plan-phase-drift-guard.test.cjs index f4faac3ea..22d89475f 100644 --- a/tests/plan-phase-drift-guard.test.cjs +++ b/tests/plan-phase-drift-guard.test.cjs @@ -712,8 +712,10 @@ describe('plan-phase.md — filesystem fallback (#2310)', () => { test('step 9 checks PLAN.md count on disk when planner return lacks completion marker', () => { assert.ok( - content.includes('DISK_PLANS=$(ls "${PHASE_DIR}"/*-PLAN.md'), - 'step 9a must check disk for PLAN.md files via DISK_PLANS variable' + content.includes('DISK_PLANS=$(gsd_run query find-phase') && + content.includes("jq -r '.plan_count_all // 0')"), + 'step 9a must check disk for PLAN.md files via DISK_PLANS variable ' + + '(sourced from gsd_run query find-phase\'s plan_count_all, ADR-3180 §7.5)' ); }); diff --git a/tests/planning-prompt-drift.test.cjs b/tests/planning-prompt-drift.test.cjs index e52d120db..3d9a04920 100644 --- a/tests/planning-prompt-drift.test.cjs +++ b/tests/planning-prompt-drift.test.cjs @@ -26,8 +26,10 @@ const path = require('node:path'); const drift = require('../scripts/lint-planning-prompt-drift.cjs'); const { findPromptDrift, scanRepo, loadBaseline, diffAgainstBaseline, toPosixRel, writeBaseline } = drift; const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); const REPO_ROOT = path.join(__dirname, '..'); +const DRIFT_SCRIPT = path.join(REPO_ROOT, 'scripts', 'lint-planning-prompt-drift.cjs'); // ─── POSITIVE ─────────────────────────────────────────────────────────── @@ -298,19 +300,18 @@ describe('Windows-shaped repo-relative paths are normalized to POSIX', () => { // ─── BASELINE INTEGRITY — both directions, against the real repo ───────── -test('loadBaseline on the committed baseline returns exactly 6 entries (one row per distinct (file, text) pair)', () => { - // 7 total ACKNOWLEDGED occurrences across 6 distinct pairs: plan-phase.md's - // byte-identical DISK_PLANS site fires at two different lines and is - // recorded as ONE row carrying `count: 2` (the Finding-3 fix — a - // duplicated-row baseline made migrating only one of the two sites - // invisible to the ratchet). +test('loadBaseline on the committed baseline returns ZERO entries — Phase 8 (#3218) burned the ratchet to zero', () => { + // Pre-#3218 this asserted 6 entries / 7 occurrences (the shell + // re-derivations). #3218 migrated all 7 sites to `gsd_run query find-phase` + // and, per ADR-3180 Decision 4(e), emptied the baseline rather than + // acknowledging them going stale — a stale entry left behind after its + // site migrates ALSO fails (see the "scanRepo matches the baseline + // exactly" test below), so an empty baseline is the only way this guard + // can be green on an EARNED zero rather than a baseline still covering + // sites that no longer fire. const { entries, errors } = loadBaseline(REPO_ROOT); assert.deepStrictEqual(errors, []); - assert.strictEqual(entries.length, 6); - const totalAcknowledgedOccurrences = entries.reduce((sum, e) => sum + (e.count ?? 1), 0); - assert.strictEqual(totalAcknowledgedOccurrences, 7); - const planPhaseEntry = entries.find((e) => e.file === 'gsd-core/workflows/plan-phase.md'); - assert.strictEqual(planPhaseEntry.count, 2); + assert.deepStrictEqual(entries, []); }); test('scanRepo(repoRoot) matches the baseline exactly: zero fresh AND zero stale', () => { @@ -324,3 +325,35 @@ test('scanRepo(repoRoot) matches the baseline exactly: zero fresh AND zero stale assert.deepStrictEqual(fresh, []); assert.deepStrictEqual(stale, []); }); + +// ─── E5 (test matrix): PROVE the guard, not just its pure functions, can +// still FAIL — an empty baseline that is green only because nothing +// exercises the fail path is exactly the "trusted on a green baseline it +// did not earn" failure this epic keeps recording. `main()` fixes its scan +// root to the real repo (`path.join(__dirname, '..')`), so this drives the +// actual CLI end-to-end against a real (temporary) file under a real +// SCAN_DIR — not a synthetic tree passed to the pure `scanRepo` — cleaned +// up in `t.after()` regardless of assertion outcome. ───────────────────── + +describe('CLI end-to-end: the guard fails on a deliberate unacknowledged fixture', () => { + test('a fresh, unacknowledged plan-count re-derivation exits 1 and names itself in stderr', (t) => { + const fixturePath = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'zzz-e5-drift-fixture.md'); + // helpers.cleanup() refuses any path outside the OS temp root; this + // fixture must live under a real SCAN_DIR (gsd-core/workflows/) because + // main() hardcodes its scan root to the real repo — see the file header + // above this describe block. + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- fixture lives outside the temp root helpers.cleanup() requires (see comment above) + t.after(() => fs.rmSync(fixturePath, { force: true })); + fs.writeFileSync( + fixturePath, + 'FIXTURE_COUNT=$(ls .planning/phases/zzz/*-PLAN.md 2>/dev/null | wc -l)\n', + ); + + const result = runNode([DRIFT_SCRIPT]); + assert.strictEqual(result.outcome, 'exited'); + assert.strictEqual(result.exitCode, 1); + assert.match(result.stderr, /NEW plan\/summary count re-derivation/); + assert.match(result.stderr, /gsd-core\/workflows\/zzz-e5-drift-fixture\.md/); + assert.match(result.stderr, /FIXTURE_COUNT=/); + }); +}); diff --git a/tests/policy-160-route0-resume.test.cjs b/tests/policy-160-route0-resume.test.cjs index e776664fc..bb236197f 100644 --- a/tests/policy-160-route0-resume.test.cjs +++ b/tests/policy-160-route0-resume.test.cjs @@ -420,5 +420,105 @@ describe('Route 0: resume_incomplete_phase invariant (#160)', () => { 'Route 0 in progress.md must use plans-without-summaries predicate consistent with determine_next_action Route 4' ); }); + + // ── #3218 D3: the dead-route contract test ───────────────────────────── + // + // Pre-#3218 this jq'd `.plans` / `.summaries` ARRAYS off a `.phases[]` + // entry from `roadmap.analyze`, which has NEVER emitted those keys — it + // emits `plan_count`/`summary_count` SCALARS (src/roadmap.cts). The `// + // []` fallback always fired, so PLAN_COUNT/SUMMARY_COUNT were always 0 + // and this predicate never fired at all (a permanently dead route, not + // merely a wrong number). This test pins the CONTRACT so a consumer can + // never again invent a shape the producer lacks: it reads the actual + // `.phases[]` entry the real `roadmap.analyze` CLI emits for a phase with + // outstanding work, and proves progress.md's Route 0 jq expression reads + // real, non-fallback-triggering data from it. + test('D3 contract: Route 0 reads the SAME keys roadmap.analyze actually emits (plan_count/summary_count scalars, not plans/summaries arrays)', (t) => { + const content = fs.readFileSync(progressMdPath, 'utf8'); + const route0Start = content.indexOf('Step 0: Resume-incomplete-phase'); + const route0End = content.indexOf('Step 1:', route0Start); + const route0Block = content.slice(route0Start, route0End); + + // The fix: read the scalars the producer actually emits. + assert.match( + route0Block, + /jq '\.plan_count \/\/ 0'/, + 'Route 0 must read .plan_count (the scalar roadmap.analyze emits), not .plans (an array it never emits)', + ); + assert.match( + route0Block, + /jq '\.summary_count \/\/ 0'/, + 'Route 0 must read .summary_count (the scalar roadmap.analyze emits), not .summaries (an array it never emits)', + ); + + // Negative proof: the pre-#3218 dead-route shape (`(.plans // []) | + // length`) must not have crept back in. + assert.doesNotMatch( + route0Block, + /\(\.plans \/\/ \[\]\)/, + 'Route 0 must not read the never-emitted `.plans` array', + ); + assert.doesNotMatch( + route0Block, + /\(\.summaries \/\/ \[\]\)/, + 'Route 0 must not read the never-emitted `.summaries` array', + ); + + // Contract proof against the REAL producer: build a real roadmap.analyze + // JSON via the shipped CLI for a phase with plans > summaries, then run + // progress.md's literal jq expression against a synthesized `.phases[]` + // entry from it — proving PLAN_COUNT/SUMMARY_COUNT come back non-zero + // (Route 0 CAN fire), not permanently 0 (the bug this closes). + const { execFileSync } = require('node:child_process'); + let jqAvailable = false; + try { execFileSync('jq', ['--version'], { stdio: 'ignore', timeout: 10000, killSignal: 'SIGKILL' }); jqAvailable = true; } catch { /* no jq on PATH */ } + if (!jqAvailable) { t.skip('jq not on PATH — this contract test applies progress.md\'s literal jq expression to real roadmap.analyze JSON; the source-text assertions above still validate the fix'); return; } + + const { createTempProject, cleanup } = require('./helpers.cjs'); + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + ['# Roadmap', '', '- [ ] Phase 1: Foundation', '', '### Phase 1: Foundation', '**Goal:** Setup', '', '---', ''].join('\n'), + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-PLAN.md'), '# Plan\n'); + // No SUMMARY.md — 2 plans, 0 summaries: plans > summaries. + + const toolsBin = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); + const roadmapJson = execFileSync(process.execPath, [toolsBin, 'roadmap', 'analyze'], { + cwd: tmpDir, + encoding: 'utf8', + timeout: 60000, + }); + const phase1 = JSON.parse(roadmapJson).phases.find((p) => String(p.number) === '1'); + assert.ok(phase1, 'roadmap.analyze must report phase 1'); + + // Apply progress.md's ACTUAL jq expressions (extracted verbatim above) + // to the real per-phase JSON object, exactly as `echo "$PHASE_DATA" | + // jq '...'` does in the workflow. + const planCountOut = execFileSync('jq', ['-r', '.plan_count // 0'], { + input: JSON.stringify(phase1), + encoding: 'utf8', + timeout: 10000, + killSignal: 'SIGKILL', + }).trim(); + const summaryCountOut = execFileSync('jq', ['-r', '.summary_count // 0'], { + input: JSON.stringify(phase1), + encoding: 'utf8', + timeout: 10000, + killSignal: 'SIGKILL', + }).trim(); + + assert.strictEqual(planCountOut, '2', 'PLAN_COUNT must reflect the real 2 plans on disk, not a fallback 0'); + assert.strictEqual(summaryCountOut, '0', 'SUMMARY_COUNT must reflect the real 0 summaries on disk'); + assert.ok( + Number(planCountOut) > Number(summaryCountOut), + 'Route 0 predicate (plans > summaries) must be true for this fixture — proving the route CAN fire post-fix', + ); + }); }); });