diff --git a/.changeset/daring-wolves-frolic.md b/.changeset/daring-wolves-frolic.md new file mode 100644 index 000000000..6da13e02b --- /dev/null +++ b/.changeset/daring-wolves-frolic.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3038 +--- +**A halted plan no longer leaves its dependents on the runnable work list** — when a plan reaches a designed stop and its SUMMARY records `status: halted`, plans that depend on it (directly or transitively) are now reported as blocked, with the halted plan(s) named, instead of being offered to the executor as ordinary incomplete work. (#2830) diff --git a/.gitignore b/.gitignore index 9c1928d03..22fbe9bec 100644 --- a/.gitignore +++ b/.gitignore @@ -209,6 +209,7 @@ build/ /gsd-core/bin/lib/capability-activation.cjs /gsd-core/bin/lib/federated-config.cjs /gsd-core/bin/lib/phase-locator.cjs +/gsd-core/bin/lib/plan-dependency-graph.cjs /gsd-core/bin/lib/phase-estimation.cjs /gsd-core/bin/lib/estimate-cli.cjs /gsd-core/bin/lib/roadmap-parser.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 2ec6f4c5c..4fc50e636 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -27,7 +27,10 @@ Module owning phase-effort estimation and its calibration against measured reali Module owning the canonical phase-verification status projection shared by phase transition, progress, manager, autonomous, and closeout readiness paths. `readVerificationStatus(phaseDir, opts?)` reads the first `*-VERIFICATION.md` frontmatter `status`, maps it through `VERIFICATION_ROUTING_TABLE`, and fail-closes — only `{passed}` satisfies the canonical gate; `missing`/`unknown`/`gaps_found`/`human_needed`/`stale` all route away from "complete" (#1522). `findStaleVerificationSummary` flags a SUMMARY newer than the VERIFICATION file (status `stale`). Both honor a no-throw, degrade-to-safe contract (any FS error → `missing` / not-stale) and an injectable `opts.fs` seam. Source of truth: `gsd-core/bin/lib/verification.cjs` (generated from `src/verification.cts`). ### Phase Locator Module -Module owning phase-directory search and location: active-phase discovery against the `.planning/phases/` tree (`searchPhaseInDir`, `findPhaseInternal`) and archived-phase-dir enumeration (`getArchivedPhaseDirs`), matching phase ids/tokens against the filesystem. Depends only on leaf modules (`phase-id` for token/name matching, `core-utils` for fs-scan/path helpers, `planning-workspace` for `planningDir`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2d (#881); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-locator.cjs` (generated from `src/phase-locator.cts`). +Module owning phase-directory search and location: active-phase discovery against the `.planning/phases/` tree (`searchPhaseInDir`, `findPhaseInternal`) and archived-phase-dir enumeration (`getArchivedPhaseDirs`), matching phase ids/tokens against the filesystem. Depends only on leaf modules (`phase-id` for token/name matching, `core-utils` for fs-scan/path helpers, `planning-workspace` for `planningDir`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2d (#881); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-locator.cjs` (generated from `src/phase-locator.cts`). Since #2830, `searchPhaseInDir` also parses each plan's `depends_on` and each completed plan's SUMMARY `status` and calls Plan Dependency Graph Module's `computeHaltPropagation` to populate `halted_plans`/`blocked_by`/`runnable_plans` — additive fields; `incomplete_plans` keeps its pre-#2830 meaning unchanged. + +### Plan Dependency Graph Module +Module owning the single halt-propagation engine over a plan's `depends_on` DAG (#2830). **Domain term: _halted_** — a plan that reached a designed stop (a gate failure, a spike concluding without expanding, or any other intentional non-completion) and wrote a SUMMARY recording that fact via `status: halted` in its frontmatter, as opposed to `status: complete` (ordinary finish) or no SUMMARY at all (not yet attempted). **Domain term: _blocked_** — a plan whose `depends_on` chain reaches a halted plan, directly or transitively; distinct from merely _incomplete_ (no SUMMARY yet) — an ordinary in-progress/not-yet-started dependency does not block. `computeHaltPropagation(nodes: {id, resolvedDependsOn, halted}[])` performs exactly one Kahn's-algorithm topological pass and returns `{order, visited, blockedBy}`, where `blockedBy` maps a plan id to the de-duplicated set of halted plan ids transitively upstream of it (diamond-safe, any depth). This is the SHARED engine both of the two independent "which plans are incomplete" readers call — `phase.cts`'s wave-grouping (`cmdPhasePlanIndex`) and `phase-locator.cts`'s phase-location primitive (`searchPhaseInDir`) — so the two-implementation divergence that caused #2830 (one parsed `depends_on` for waves only, the other never parsed it at all) cannot recur: each caller resolves its own raw `depends_on` tokens to canonical ids before calling in, but the graph traversal itself exists in exactly one place. Pure — no I/O, no config; each caller does its own file reads (a plan's frontmatter, a completed plan's SUMMARY `status`) and fails open (treats an unreadable/malformed file as "not halted"/"no deps") rather than throwing. Source of truth: `gsd-core/bin/lib/plan-dependency-graph.cjs` (generated from `src/plan-dependency-graph.cts`). ### Dispatch Policy Module Module owning dispatch error mapping, fallback policy, timeout classification, and CLI exit mapping contract. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 0bad27cc4..c95e57702 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -410,6 +410,7 @@ "phase-locator.cjs", "phase.cjs", "phases-command-router.cjs", + "plan-dependency-graph.cjs", "plan-drift-guard.cjs", "plan-scan.cjs", "planning-workspace.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 6aa63dc14..a9354dec5 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -499,6 +499,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `phase-locator.cjs` | Phase-directory search/location — active + archived phase-dir discovery, phase-id matching against the filesystem (extracted from `core.cjs`, ADR-857) | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | | `phases-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phases` | +| `plan-dependency-graph.cjs` | Shared halt-propagation over a plan's `depends_on` DAG — the single topological-order + halt-propagation engine used by both `phase.cjs`'s wave-grouping and `phase-locator.cjs`'s phase-location primitive, so the two can never diverge on which plans a halted plan blocks (#2830) | | `plan-scan.cjs` | Canonical phase-plan scanner for detecting plan and summary files in flat and nested layouts (k014) | | `planning-workspace.cjs` | Planning path/workstream seam (`planningDir`, `planningPaths`, active-workstream routing, `.planning/.lock` orchestration) | | `project-root.cjs` | Resolves a project root from a starting directory using four heuristics (own `.planning/` guard, `sub_repos` config, `multiRepo` flag, `.git` heuristic) | diff --git a/eslint.config.mjs b/eslint.config.mjs index a22dad316..719797c76 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -170,6 +170,7 @@ export default tseslint.config( 'gsd-core/bin/lib/normalize-test-command.cjs', 'gsd-core/bin/lib/config-loader.cjs', 'gsd-core/bin/lib/phase-locator.cjs', + 'gsd-core/bin/lib/plan-dependency-graph.cjs', 'gsd-core/bin/lib/roadmap-parser.cjs', 'gsd-core/bin/lib/drift.cjs', 'gsd-core/bin/lib/cjs-command-router-adapter.cjs', diff --git a/gsd-core/references/artifact-types.md b/gsd-core/references/artifact-types.md index b2cbc0183..ed4e833a9 100644 --- a/gsd-core/references/artifact-types.md +++ b/gsd-core/references/artifact-types.md @@ -43,6 +43,13 @@ reads is inert — the consumption mechanism is what gives an artifact meaning. - **Lifecycle**: Created at plan completion → Read by subsequent plans in same phase - **Location**: `.planning/phases/XX-name/XX-YY-SUMMARY.md` - **Consumed by**: Orchestrator (progress), planner (context for future plans), `milestone-summary` +- **`status:`** — `complete` (default) or **`halted`**. `halted` records a *designed stop*: + the plan ran and answered its question, but the answer means the work it was gating cannot + proceed (a spike that returns "no", for example). It is a success, not a failure — the plan + did its job. Marking a SUMMARY `halted` propagates transitively over `depends_on`: every + plan that depends on it, directly or through a chain, is reported as **blocked** rather than + offered to the executor as ordinary incomplete work, and is named with its cause. Any other + value — including no `status:` field at all — reads as complete. (#2830) ### HANDOFF.json / .continue-here.md - **Shape**: Structured pause state (JSON machine-readable + Markdown human-readable) diff --git a/gsd-core/templates/summary-complex.md b/gsd-core/templates/summary-complex.md index 250a38cfc..a4349d3a9 100644 --- a/gsd-core/templates/summary-complex.md +++ b/gsd-core/templates/summary-complex.md @@ -28,6 +28,8 @@ completed: YYYY-MM-DD status: complete --- +**Status (#2830):** `status: complete` is the default — the plan finished. Use `status: halted` instead when the plan reached a designed stop (a gate failure, a spike concluding without expanding into the full build, or any other intentional non-completion) and intentionally left tasks unfinished. + # Phase [X]: [Name] Summary (Complex) **[Substantive one-liner describing outcome]** diff --git a/gsd-core/templates/summary-minimal.md b/gsd-core/templates/summary-minimal.md index 4cd8ae6f3..047465aee 100644 --- a/gsd-core/templates/summary-minimal.md +++ b/gsd-core/templates/summary-minimal.md @@ -25,6 +25,8 @@ completed: YYYY-MM-DD status: complete --- +**Status (#2830):** `status: complete` is the default — the plan finished. Use `status: halted` instead when the plan reached a designed stop (a gate failure, a spike concluding without expanding into the full build, or any other intentional non-completion) and intentionally left tasks unfinished. + # Phase [X]: [Name] Summary (Minimal) **[Substantive one-liner describing outcome]** diff --git a/gsd-core/templates/summary-standard.md b/gsd-core/templates/summary-standard.md index 5ef26eb5b..5a6e97d37 100644 --- a/gsd-core/templates/summary-standard.md +++ b/gsd-core/templates/summary-standard.md @@ -27,6 +27,8 @@ completed: YYYY-MM-DD status: complete --- +**Status (#2830):** `status: complete` is the default — the plan finished. Use `status: halted` instead when the plan reached a designed stop (a gate failure, a spike concluding without expanding into the full build, or any other intentional non-completion) and intentionally left tasks unfinished. + # Phase [X]: [Name] Summary **[Substantive one-liner describing outcome]** diff --git a/gsd-core/templates/summary.md b/gsd-core/templates/summary.md index 11a235904..948a4f405 100644 --- a/gsd-core/templates/summary.md +++ b/gsd-core/templates/summary.md @@ -171,6 +171,8 @@ None - no external service configuration required. **Patterns:** Established conventions future phases should maintain. **Population:** Frontmatter is populated during summary creation in execute-plan.md. See `` for field-by-field guidance. + +**Status (#2830):** `status: complete` is the default — the plan finished. Use `status: halted` instead when the plan reached a designed stop (a gate failure, a spike concluding without expanding into the full build, or any other intentional non-completion) and intentionally left tasks unfinished. `halted` is machine-read: any plan whose `depends_on` (directly or transitively) names a halted plan is reported as blocked, not offered to the executor, until the halt is resolved and re-summarized as `complete`. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index dec6d3c64..878f200ef 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -334,9 +334,9 @@ Load plan inventory with wave grouping in one call: PLAN_INDEX=$(gsd_run query phase-plan-index "${PHASE_NUMBER}") ``` -Parse JSON for: `phase`, `plans[]` (each with `id`, `wave`, `autonomous`, `objective`, `files_modified`, `task_count`, `has_summary`), `waves` (map of wave number → plan IDs), `incomplete`, `has_checkpoints`. +Parse JSON for: `phase`, `plans[]` (each with `id`, `wave`, `autonomous`, `objective`, `files_modified`, `task_count`, `has_summary`, `halted`, `blocked_by`), `waves` (map of wave number → plan IDs), `incomplete`, `runnable`, `has_checkpoints`. -**Filtering:** Skip plans where `has_summary: true`. If `--gaps-only`: also skip non-gap_closure plans. If `WAVE_FILTER` is set: also skip plans whose `wave` does not equal `WAVE_FILTER`. +**Filtering:** Skip plans where `has_summary: true`. Additionally skip any plan whose `blocked_by` array is non-empty (#2830) — it depends, directly or transitively, on a plan that halted at a designed stop rather than completing — and report it by name: "Skipping {plan.id}: blocked by halted {blocked_by.join(', ')}". Never silently drop a blocked plan from the report; it must appear by name with its reason, not merely vanish from the executable list. This rule is additive to the `has_summary` skip, not a replacement for it. If `--gaps-only`: also skip non-gap_closure plans. If `WAVE_FILTER` is set: also skip plans whose `wave` does not equal `WAVE_FILTER`. **Wave safety check:** If `WAVE_FILTER` is set and there are still incomplete plans in any lower wave that match the current execution mode, STOP and tell the user to finish earlier waves first. Do not let Wave 2+ execute while prerequisite earlier-wave plans remain incomplete. diff --git a/src/init.cts b/src/init.cts index e64c024d0..8eb16387f 100644 --- a/src/init.cts +++ b/src/init.cts @@ -824,6 +824,9 @@ function cmdInitExecutePhase( plans: [], summaries: [], incomplete_plans: [], + halted_plans: [], + blocked_by: {}, + runnable_plans: [], has_research: false, has_context: false, has_verification: false, @@ -869,6 +872,16 @@ function cmdInitExecutePhase( plan_count: (phaseInfo?.['plans'] as unknown[] | undefined)?.length || 0, incomplete_count: (phaseInfo?.['incomplete_plans'] as unknown[] | undefined)?.length || 0, + // #2830: the halt-aware view, forwarded from the shared computation in + // phase-locator. Additive — `incomplete_plans`/`incomplete_count` above keep + // their exact name, type and semantics. Without this passthrough the shared + // truth is computed and then dropped at this consumer, which is the path the + // issue reports as regressed. + halted_plans: phaseInfo?.['halted_plans'] || [], + blocked_by: phaseInfo?.['blocked_by'] || {}, + runnable_plans: phaseInfo?.['runnable_plans'] || [], + runnable_count: (phaseInfo?.['runnable_plans'] as unknown[] | undefined)?.length || 0, + branch_name: config.branching_strategy === 'phase' && phaseInfo ? (config.phase_branch_template as string) diff --git a/src/phase-locator.cts b/src/phase-locator.cts index 11213dad6..d2c4db1ea 100644 --- a/src/phase-locator.cts +++ b/src/phase-locator.cts @@ -27,6 +27,12 @@ const { readSubdirectories, getPhaseFileStats, extractCanonicalPlanId, toPosixPa // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { planningDir } = planningWorkspace; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import frontmatterModule = require('./frontmatter.cjs'); +const { extractFrontmatter } = frontmatterModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planDependencyGraphModule = require('./plan-dependency-graph.cjs'); +const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted } = planDependencyGraphModule; // ─── Phase search types ─────────────────────────────────────────────────────── @@ -45,6 +51,44 @@ interface PhaseSearchResult { has_reviews: boolean; archived?: string; ambiguous_matches?: string[]; + /** + * #2830: plan filenames (from `plans`) whose own SUMMARY declares + * `status: halted` — a designed stop, not an ordinary completion. + */ + halted_plans: string[]; + /** + * #2830: plan filename -> the halted plan id(s) (canonical, e.g. "01-02") + * transitively blocking it, for every entry in `incomplete_plans` that is + * blocked by an upstream halt. A plan filename absent from this map is not + * blocked (either not incomplete, or incomplete with no halted upstream). + */ + blocked_by: Record; + /** + * #2830: the runnable-only view — `incomplete_plans` filtered to exclude + * anything present as a key in `blocked_by`. `incomplete_plans` itself + * keeps its pre-#2830 meaning ("no matching SUMMARY yet") unchanged. + */ + runnable_plans: string[]; +} + +/** + * #2830: parse a plan file's `depends_on` frontmatter. Returns [] — never + * throws — on a missing/unreadable/malformed plan or absent field, matching + * this primitive's existing fail-safe posture (a plan directory this + * primitive can otherwise read must never throw here). + */ +function parsePlanDependsOn(phaseDir: string, planFile: string): string[] { + try { + const planPath = path.join(phaseDir, planFile); + const content = fs.readFileSync(planPath, 'utf-8'); + const fm = extractFrontmatter(content, planPath); + const fmDeps = fm['depends_on']; + if (Array.isArray(fmDeps)) return fmDeps.map(String); + if (typeof fmDeps === 'string' && fmDeps.trim() !== '') return [fmDeps]; + return []; + } catch { + return []; + } } interface ArchivedPhaseDir { @@ -117,6 +161,9 @@ function searchPhaseInDir(baseDir: string, relBase: string, normalized: string): has_verification: false, has_reviews: false, ambiguous_matches: matches, + halted_plans: [], + blocked_by: {}, + runnable_plans: [], }; } @@ -144,6 +191,54 @@ function searchPhaseInDir(baseDir: string, relBase: string, normalized: string): return !completedPlanIds.has(planId) && !completedPlanIds.has(canonical); }); + // #2830: reverse lookup from a completed plan's id (exact or canonical) to + // its actual summary filename. Shared builder (also used by phase.cts's + // cmdPhasePlanIndex) so the two can never disagree about which summary + // belongs to which plan. + const summaryFileByPlanId = buildSummaryFileIndex(summaries, extractCanonicalPlanId); + + // #2830: this primitive previously never parsed depends_on at all — see + // src/plan-dependency-graph.cts's file header. Build the same + // PlanHaltNode[] shape phase.cts's cmdPhasePlanIndex builds (id resolution + // mirrors its planMap/canonicalToId pattern) and hand it to the ONE + // shared halt-propagation traversal so this reader and the wave-grouping + // reader can never diverge on the halt rule again. + const planIds = plans.map(p => p.replace('-PLAN.md', '').replace('PLAN.md', '')); + const planIdByLower = new Map(planIds.map(id => [id.toLowerCase(), id])); + const canonicalToPlanId = new Map( + plans.map((p, i) => [extractCanonicalPlanId(p).toLowerCase(), planIds[i]]), + ); + + const haltNodes = plans.map((p, i) => { + const planId = planIds[i]; + const canonical = extractCanonicalPlanId(p); + const summaryFile = summaryFileByPlanId.get(planId) ?? summaryFileByPlanId.get(canonical); + const halted = summaryFile !== undefined && isSummaryFileHalted(path.join(phaseDir, summaryFile)); + const resolvedDependsOn = parsePlanDependsOn(phaseDir, p) + .map((dep) => { + const lower = dep.toLowerCase(); + return planIdByLower.get(lower) ?? canonicalToPlanId.get(lower) ?? null; + }) + .filter((id): id is string => id !== null); + return { id: planId, resolvedDependsOn, halted }; + }); + const { blockedBy } = computeHaltPropagation(haltNodes); + + const haltedPlans = plans.filter((_, i) => haltNodes[i].halted); + const incompletePlanSet = new Set(incompletePlans); + const blockedByFiles: Record = {}; + const runnablePlans: string[] = []; + for (let i = 0; i < plans.length; i++) { + const p = plans[i]; + if (!incompletePlanSet.has(p)) continue; + const causes = blockedBy.get(planIds[i]) ?? []; + if (causes.length > 0) { + blockedByFiles[p] = causes; + } else { + runnablePlans.push(p); + } + } + return { found: true, directory: toPosixPath(path.join(relBase, match)), @@ -157,6 +252,9 @@ function searchPhaseInDir(baseDir: string, relBase: string, normalized: string): has_context: hasContext, has_verification: hasVerification, has_reviews: hasReviews, + halted_plans: haltedPlans, + blocked_by: blockedByFiles, + runnable_plans: runnablePlans, }; } catch { return null; diff --git a/src/phase.cts b/src/phase.cts index 2bb5a716b..46da8808f 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -68,6 +68,9 @@ import verificationMod = require('./verification.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- verify.cjs is an export= CommonJS module import verifyMod = require('./verify.cjs'); const { readVerificationStatus } = verificationMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-dependency-graph.cjs is an export= CommonJS module +import planDependencyGraphMod = require('./plan-dependency-graph.cjs'); +const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted } = planDependencyGraphMod; const { planningDir, withPlanningLock, listAvailableWorkstreams, getActiveWorkstream } = planningWorkspace; @@ -511,16 +514,40 @@ interface RawPlan { filesModified: string[]; taskCount: number; hasSummary: boolean; + /** #2830: true iff this plan's own SUMMARY declares `status: halted` (a designed stop). */ + halted: boolean; +} + +/** + * Resolve a raw `depends_on` token to the `RawPlan.id` it refers to + * (case-folded exact match, falling back to canonical-id matching). Returns + * `null` when the token does not resolve to any plan in this phase (a typo + * or a cross-phase reference) — every call site treats that as "ignore this + * edge", never a throw. Shared by `computeDependencyLevels`'s DAG-edge + * resolution, the `depends_on` display mapping, and (#2830) the + * halt-propagation node resolution, so the three can never disagree about + * which token resolves to which plan. + */ +function resolveDependencyId( + dep: string, + planMap: Map, + canonicalToId: Map, +): string | null { + const lower = dep.toLowerCase(); + return planMap.has(lower) ? (planMap.get(lower) as RawPlan).id : (canonicalToId.get(lower) ?? null); } // O(V + E). Assigns each in-phase plan its longest-path topological level over the -// in-phase dependsOn DAG (Kahn's algorithm). Returns { level: Map, visited: number }. -// visited < rawPlans.length signals a dependency cycle. +// in-phase dependsOn DAG (Kahn's algorithm). Returns { level: Map, visited: number, +// order: string[] }. visited < rawPlans.length signals a dependency cycle. `order` (#2830) is +// the exact dequeue order this pass already produces — a valid topological order — passed to +// computeHaltPropagation as `precomputedOrder` so halt propagation does not re-run Kahn's +// algorithm a second time over the same graph. function computeDependencyLevels( rawPlans: RawPlan[], planMap: Map, canonicalToId: Map, -): { level: Map; visited: number } { +): { level: Map; visited: number; order: string[] } { const level = new Map(); const inDeg = new Map(); const adj = new Map(); @@ -529,10 +556,7 @@ function computeDependencyLevels( if (!inDeg.has(p.id)) inDeg.set(p.id, 0); if (!adj.has(p.id)) adj.set(p.id, []); for (const dep of p.dependsOn) { - const depLower = dep.toLowerCase(); - const resolvedDep = planMap.has(depLower) - ? (planMap.get(depLower) as RawPlan).id - : canonicalToId.get(depLower); + const resolvedDep = resolveDependencyId(dep, planMap, canonicalToId); if (!resolvedDep) continue; if (!adj.has(resolvedDep)) adj.set(resolvedDep, []); (adj.get(resolvedDep) as string[]).push(p.id); @@ -568,7 +592,7 @@ function computeDependencyLevels( } } - return { level, visited }; + return { level, visited, order: queue }; } function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { @@ -598,7 +622,7 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { if (!phaseDir) { output( - { phase: normalized, error: 'Phase not found', plans: [], waves: {}, incomplete: [], has_checkpoints: false }, + { phase: normalized, error: 'Phase not found', plans: [], waves: {}, incomplete: [], runnable: [], has_checkpoints: false }, raw, ); return; @@ -617,6 +641,12 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { return canonical === exact ? [exact] : [exact, canonical]; }), ); + // #2830: reverse lookup from a completed plan's id (exact or canonical) to + // the actual summary filename, so a plan's own SUMMARY frontmatter can be + // read for its `status`. Shared builder (also used by phase-locator.cts's + // searchPhaseInDir) so the two can never disagree about which summary + // belongs to which plan. + const summaryFileByPlanId = buildSummaryFileIndex(summaryFiles, extractCanonicalPlanId); // ── Pass 1: parse each plan file ───────────────────────────────────────── @@ -660,6 +690,16 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { const hasSummary = completedPlanIds.has(planId) || completedPlanIds.has(extractCanonicalPlanId(planFile)); + // #2830: a plan can have a SUMMARY (hasSummary=true) and still be halted — + // a designed stop still writes a completion record, just one whose status + // says "halted" rather than "complete". Only look up the summary file + // when one exists; there is nothing to read otherwise. + const summaryFile = + summaryFileByPlanId.get(planId) ?? summaryFileByPlanId.get(extractCanonicalPlanId(planFile)); + const halted = hasSummary && summaryFile !== undefined + ? isSummaryFileHalted(path.join(phaseDir, summaryFile)) + : false; + rawPlans.push({ id: planId, declaredWave, @@ -669,6 +709,7 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { filesModified, taskCount, hasSummary, + halted, }); } @@ -692,7 +733,7 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { rawPlans.map((p) => [extractCanonicalPlanId(p.id).toLowerCase(), p.id]), ); - const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId); + const { level, visited, order } = computeDependencyLevels(rawPlans, planMap, canonicalToId); if (visited < rawPlans.length) { const cycleNodes = rawPlans.filter((p) => !level.has(p.id)).map((p) => p.id); @@ -702,6 +743,20 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { return; } + // #2830: single shared halt-propagation pass, reusing the SAME id + // resolution (planMap/canonicalToId) AND the SAME topological order + // (`order`, computeDependencyLevels's own Kahn's-algorithm dequeue + // sequence) — passed as `precomputedOrder` so computeHaltPropagation does + // NOT run Kahn's algorithm a second time over this graph. + const haltNodes = rawPlans.map((p) => ({ + id: p.id, + resolvedDependsOn: p.dependsOn + .map((dep) => resolveDependencyId(String(dep), planMap, canonicalToId)) + .filter((id): id is string => id !== null), + halted: p.halted, + })); + const { blockedBy } = computeHaltPropagation(haltNodes, order); + // ── Pass 3: determine lowest bucket key and build output ───────────────── const anyWaveZero = rawPlans.some((p) => p.declaredWave === 0); @@ -710,6 +765,7 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { const plans: Record[] = []; const waves: Record = {}; const incomplete: string[] = []; + const runnable: string[] = []; let hasCheckpoints = false; const warnings: string[] = []; @@ -717,8 +773,15 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { if (!rawPlan.autonomous) { hasCheckpoints = true; } + const blockedByIds = blockedBy.get(rawPlan.id) ?? []; if (!rawPlan.hasSummary) { incomplete.push(rawPlan.id); + // #2830: the runnable-only view — incomplete AND not transitively + // blocked by a halted upstream plan. Additive alongside `incomplete`, + // which keeps its existing "no SUMMARY yet" meaning unchanged. + if (blockedByIds.length === 0) { + runnable.push(rawPlan.id); + } } const computedWave = (level.get(rawPlan.id) ?? 0) + levelOffset; @@ -732,6 +795,13 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { const plan: Record = { id: rawPlan.id, wave: effectiveWave, + // DELIBERATELY not `resolveDependencyId`: the emitted field is a DISPLAY + // mapping, not the DAG resolution. It rewrites a dep only when it names a + // plan directly (planMap) and otherwise passes it through verbatim — a + // short canonical prefix like `24-01` stays `24-01` rather than becoming + // `24-01-auth-hardening`. #3785 pins that contract. Full resolution via + // canonicalToId is used for the wave DAG and #2830 halt propagation only; + // routing this line through it too silently changed the output shape. depends_on: rawPlan.dependsOn.map((dep) => { const lower = String(dep).toLowerCase(); return planMap.has(lower) ? (planMap.get(lower) as RawPlan).id : dep; @@ -741,6 +811,11 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { files_modified: rawPlan.filesModified, task_count: rawPlan.taskCount, has_summary: rawPlan.hasSummary, + // #2830: additive fields — halted is this plan's OWN status; blocked_by + // names the halted plan(s) transitively upstream of it (empty when not + // blocked). Neither mutates has_summary/incomplete's existing meaning. + halted: rawPlan.halted, + blocked_by: blockedByIds, }; plans.push(plan); @@ -757,6 +832,7 @@ function cmdPhasePlanIndex(cwd: string, phase: string, raw: boolean): void { plans, waves, incomplete, + runnable, has_checkpoints: hasCheckpoints, }; if (planNamingWarning) result['warning'] = planNamingWarning; diff --git a/src/plan-dependency-graph.cts b/src/plan-dependency-graph.cts new file mode 100644 index 000000000..a209d348f --- /dev/null +++ b/src/plan-dependency-graph.cts @@ -0,0 +1,256 @@ +/** + * Plan Dependency Graph — shared halt-propagation over a plan's depends_on DAG (#2830). + * + * Two independent "which plans are incomplete" readers exist in this codebase: + * phase.cts's wave-grouping (`cmdPhasePlanIndex`) and phase-locator.cts's + * phase-location primitive (`searchPhaseInDir`, consumed by ~50 symbols across + * five command routers). Before #2830, only the former parsed `depends_on` — + * and even it used the DAG only for topological wave assignment, never to + * propagate a halted plan's block onto its dependents. The latter never parsed + * `depends_on` at all; it derived completion from summary-file presence only. + * A plan that reaches a designed stop still writes a SUMMARY (recording the + * halt in prose only — no prior structured status existed for it), so both + * readers saw it as ordinary "complete" and reported its dependents as + * ordinary incomplete work, available to spawn against. + * + * This module is the SINGLE topological-order + halt-propagation engine both + * readers call, so the two can never re-diverge on this rule again. Each + * caller resolves its own raw `depends_on` tokens to canonical plan ids + * (case-fold + `extractCanonicalPlanId` fallback — the same resolution + * already performed by phase.cts's `computeDependencyLevels`) before + * building `PlanHaltNode[]`; this module owns the graph traversal (exactly + * one pass — Kahn's algorithm, or none at all when the caller already has a + * valid topological order, see `computeHaltPropagation`'s `precomputedOrder` + * parameter) plus the two small pure helpers below (`isHaltedStatus`, + * `buildSummaryFileIndex`) that both callers would otherwise duplicate + * identically — the exact "two implementations, one drifts" failure mode + * this fix exists to close. Each caller still owns its own file I/O (the + * actual `fs.readFileSync` + `extractFrontmatter` calls); only the + * interpretive logic is centralized here — except the SUMMARY-file + * read+extract wrapper itself (`isSummaryFileHalted`), which both callers + * previously duplicated near-identically and which is now centralized here + * too, for the same reason. + */ + +import fs from 'node:fs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module +import frontmatterMod = require('./frontmatter.cjs'); +const { extractFrontmatter } = frontmatterMod; + +/** + * The one place "does this SUMMARY status value mean halted" is decided. + * Case-insensitive, trims whitespace. Both `phase.cts`'s `cmdPhasePlanIndex` + * and `phase-locator.cts`'s `searchPhaseInDir` call this after reading a + * completed plan's SUMMARY frontmatter `status` field, so the definition of + * "halted" cannot drift between the two readers. + */ +function isHaltedStatus(status: unknown): boolean { + if (typeof status !== 'string') return false; + // #2830 review (defect 2): strip an unquoted trailing YAML comment (a run + // of whitespace followed by `#` and the rest of the line) before + // trimming/lowercasing. YAML scalars don't need quoting to carry an inline + // comment (`status: halted # designed stop`), but `extractFrontmatter` + // does not strip one — and all four summary templates literally show that + // spelling as guidance on the value line. Without this, an executor that + // mimics the template's own presentation would write a halt that silently + // reads back as not-halted. A `#` with no preceding whitespace is NOT a + // YAML comment start, so `halted#nospace` intentionally still fails to match. + const withoutTrailingComment = status.replace(/\s+#.*$/, ''); + return withoutTrailingComment.trim().toLowerCase() === 'halted'; +} + +/** + * Read a plan's SUMMARY file and report whether it declares `status: halted` + * (a designed stop, not an ordinary completion). Returns false — never + * throws — on a missing/unreadable/malformed SUMMARY, so an unreadable file + * degrades to the pre-#2830 behavior ("has a SUMMARY = complete") rather + * than breaking either caller. + * + * Two callers share this wrapper: `phase.cts`'s `cmdPhasePlanIndex` and + * `phase-locator.cts`'s `searchPhaseInDir` (the phase-location primitive + * consumed by ~50 symbols across five command routers). Both previously + * carried a near-identical local copy (read file -> `extractFrontmatter` -> + * `isHaltedStatus` -> swallow errors) that this module's own header comment + * calls out as the exact "two implementations, one drifts" failure mode it + * exists to prevent — centralizing the read+extract wrapper here, not just + * the `isHaltedStatus` predicate, closes that gap. + * + * Takes a single resolved `summaryPath` (not a `dir` + `filename` pair) — + * `phase-locator.cts`'s prior local copy took the two parts separately and + * `path.join`'d them internally; that caller now does the join itself + * before calling in, so both callers share one signature. + * + * @param summaryPath - absolute or relative path to a `*-SUMMARY.md` file. + */ +function isSummaryFileHalted(summaryPath: string): boolean { + try { + const content = fs.readFileSync(summaryPath, 'utf-8'); + const fm = extractFrontmatter(content, summaryPath); + return isHaltedStatus(fm['status']); + } catch { + return false; + } +} + +/** + * Build a planId -> summary-filename lookup from a phase's summary file + * list, keyed by both the exact SUMMARY-file-derived id and its canonical + * form (mirrors the `completedPlanIds` construction each caller already + * performs for its own SUMMARY-presence check — same `summaryFiles` list, + * same exact/canonical key pair — so the two can never disagree about which + * summary file belongs to which plan id). `extractCanonicalPlanId` is + * supplied by the caller (each module owns its own resolution helper). + */ +function buildSummaryFileIndex( + summaryFiles: string[], + extractCanonicalPlanId: (filename: string) => string, +): Map { + const index = new Map(); + for (const s of summaryFiles) { + const exact = s.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); + const canonical = extractCanonicalPlanId(s); + index.set(exact, s); + if (canonical !== exact) index.set(canonical, s); + } + return index; +} + +interface PlanHaltNode { + /** Canonical plan id, already resolved — matches another node's `id` for a dependency edge to count. */ + id: string; + /** Dependency ids, already resolved to `id` values present in this node list. Unresolved/cross-phase deps must be filtered out by the caller before this call. */ + resolvedDependsOn: string[]; + /** True iff this plan's own completion record (SUMMARY) declares `status: halted`. */ + halted: boolean; +} + +interface HaltPropagationResult { + /** + * Topological order (Kahn's algorithm), dependencies before dependents. + * A length shorter than the input `nodes.length` signals a dependency + * cycle among the unlisted ids — mirrors `computeDependencyLevels`'s + * `visited` counter contract. + */ + order: string[]; + visited: number; + /** + * planId -> de-duplicated list of halted plan ids that transitively block + * it (direct or via any number of intermediate dependents). No entry means + * not blocked. A halted plan's own id is never a key in its own value — a + * halted plan is halted, not blocked by itself. + */ + blockedBy: Map; +} + +/** + * Computes halt-propagation over a plan dependency DAG. + * + * `precomputedOrder`: when the caller ALREADY has a valid topological order + * for these exact node ids (e.g. phase.cts's `cmdPhasePlanIndex`, which runs + * `computeDependencyLevels`'s Kahn's-algorithm pass for wave assignment + * before ever calling this function), pass it here and this function skips + * running Kahn's algorithm a second time — the halt-propagation forward pass + * below only needs A valid topological order, not to derive one itself. + * Omit it (as phase-locator.cts's `searchPhaseInDir` does — it has no prior + * traversal of this DAG) and this function derives the order itself; either + * way, exactly one Kahn's-algorithm pass runs per caller, never two. + * + * Diamond-safe (a plan blocked via two different halted ancestors gets both + * in its `blockedBy` list, deduplicated) and transitive-safe (a dependent of + * a dependent of a halted plan is blocked, at any depth). + */ +function computeHaltPropagation(nodes: PlanHaltNode[], precomputedOrder?: string[]): HaltPropagationResult { + const byId = new Map(nodes.map((n) => [n.id, n])); + + let order: string[]; + let visited: number; + + if (precomputedOrder) { + // Caller already ran Kahn's algorithm over this exact node set (same ids) + // — trust its order and skip re-deriving one. `visited` mirrors the same + // "count of nodes reachable in valid topological order" contract a fresh + // derivation would produce. + order = precomputedOrder; + visited = precomputedOrder.length; + } else { + const inDeg = new Map(); + const adj = new Map(); // dependency id -> dependent ids + + for (const n of nodes) { + if (!inDeg.has(n.id)) inDeg.set(n.id, 0); + if (!adj.has(n.id)) adj.set(n.id, []); + for (const depId of n.resolvedDependsOn) { + if (!byId.has(depId)) continue; // fail-safe: caller should have filtered these already + if (!adj.has(depId)) adj.set(depId, []); + (adj.get(depId) as string[]).push(n.id); + inDeg.set(n.id, (inDeg.get(n.id) ?? 0) + 1); + } + } + + const queue: string[] = []; + for (const n of nodes) { + if ((inDeg.get(n.id) ?? 0) === 0) queue.push(n.id); + } + + // Dequeue by head index, not Array.shift() — O(1) amortized, same + // rationale as computeDependencyLevels (#307). + let head = 0; + let v = 0; + while (head < queue.length) { + const cur = queue[head++]; + v++; + for (const dep of adj.get(cur) ?? []) { + inDeg.set(dep, (inDeg.get(dep) as number) - 1); + if (inDeg.get(dep) === 0) queue.push(dep); + } + } + order = queue; + visited = v; + } + + // `order` is a valid topological order for every visited node: for edge + // dep -> dependent, dep appears before dependent. A single forward pass + // over it — using each node's own already-resolved dependsOn ids — + // computes blockedBy without any further graph traversal. + const blockedBy = new Map(); + for (const id of order) { + const node = byId.get(id); + if (!node) continue; + const causes = new Set(); + for (const depId of node.resolvedDependsOn) { + const depNode = byId.get(depId); + if (!depNode) continue; + if (depNode.halted) causes.add(depId); + const depCauses = blockedBy.get(depId); + if (depCauses) { + for (const c of depCauses) causes.add(c); + } + } + if (causes.size > 0) blockedBy.set(id, Array.from(causes)); + } + + // #2830 review (defect 1): a node involved in a depends_on cycle (or + // downstream of one) never reaches indegree 0, so it never appears in + // `order` and the forward pass above never visits it — it would otherwise + // end up absent from BOTH `blockedBy` and any cycle diagnostic, i.e. + // reported as ordinary runnable. A node whose position in the topological + // order is undecidable cannot be shown to be safe to run: silently + // dropping it is the exact silent-disappearance failure #2830 exists to + // prevent. Fail closed — give every such non-halted node an explicit, + // non-empty `blockedBy` entry so no consumer (present or future) can + // re-admit it as runnable merely by checking "absent from blockedBy". + // (A node that is itself halted is not "blocked" — it IS the blocker — + // so it is left out here exactly as the normal forward pass leaves it out.) + if (order.length < nodes.length) { + const orderSet = new Set(order); + for (const n of nodes) { + if (orderSet.has(n.id) || n.halted) continue; + const causes = Array.from(new Set(n.resolvedDependsOn.filter((depId) => byId.has(depId)))).sort(); + blockedBy.set(n.id, causes.length > 0 ? causes : [n.id]); + } + } + + return { order, visited, blockedBy }; +} + +export = { computeHaltPropagation, isHaltedStatus, buildSummaryFileIndex, isSummaryFileHalted }; diff --git a/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json b/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json index 968e9111d..056751ee0 100644 --- a/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json +++ b/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json @@ -1,6 +1,6 @@ { "version": 1, "paths": { - "execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5. #2639: handle_branching now warns when local is ahead of origin (+376 B condensed one-line WARNING + rev-list --count check). #2993 (epic #1671 Phase 6.2): fixes the sibling gap #2932 shipped — `flag:--wave` section gating was added to the WHEN_VOCABULARY and to init.execute-phase's section manifest, but execute-phase.md never actually parsed `--wave` out of `$ARGUMENTS` or forwarded it on the `gsd_run query init.execute-phase` line, so the flag could never fire. This diff adds a WAVE_PARAM extraction (`--wave ` via BASH_REMATCH) and appends it to the init call, growing the file 163 bytes (89,507 -> 89,670)." + "execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5. #2639: handle_branching now warns when local is ahead of origin (+376 B condensed one-line WARNING + rev-list --count check). #2993 (epic #1671 Phase 6.2): fixes the sibling gap #2932 shipped — `flag:--wave` section gating was added to the WHEN_VOCABULARY and to init.execute-phase's section manifest, but execute-phase.md never actually parsed `--wave` out of `$ARGUMENTS` or forwarded it on the `gsd_run query init.execute-phase` line, so the flag could never fire. This diff adds a WAVE_PARAM extraction (`--wave ` via BASH_REMATCH) and appends it to the init call, growing the file 163 bytes (89,507 -> 89,670). #2830: the discover_and_group_plans step gained the halt-aware skip rule — plans whose blocked_by is non-empty are skipped in addition to the existing has_summary skip, and each is reported BY NAME with its cause (\"Skipping {id}: blocked by halted {…}\") rather than silently vanishing from the executable list. Growth is the added rule plus the blocked_by/runnable fields in the documented phase-plan-index parse contract, growing the file 518 bytes (89,670 -> 90,188); no other content changed." } } diff --git a/tests/fix-2830-halted-plan-dependents.test.cjs b/tests/fix-2830-halted-plan-dependents.test.cjs new file mode 100644 index 000000000..ae2c35454 --- /dev/null +++ b/tests/fix-2830-halted-plan-dependents.test.cjs @@ -0,0 +1,764 @@ +/** + * Regression tests for #2830: a halted plan leaves its dependents on the + * runnable work list. + * + * Two independent "which plans are incomplete" readers exist: + * - phase.cts's cmdPhasePlanIndex (`gsd-tools phase-plan-index`) — parses + * depends_on for wave assignment, but (pre-fix) never propagates a halt. + * - phase-locator.cts's searchPhaseInDir/findPhaseInternal — the + * phase-location primitive consumed by ~50 symbols across 5 command + * routers; (pre-fix) never parsed depends_on at all. + * + * Both must now report a direct or transitive dependent of a halted plan as + * blocked — never offered as ordinary runnable work — while leaving the + * pre-existing `incomplete`/`incomplete_plans` fields byte-identical. + * + * Every assertion below is behavioral (structured JSON from the real CLI / + * the real compiled module) — no source-grep or raw-text matching, so no + * `allow-test-rule` exemption is needed anywhere in this file. + */ + +'use strict'; + +const { test, describe, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const fc = require('./helpers/fast-check-setup.cjs'); + +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); +const planDependencyGraph = require('../gsd-core/bin/lib/plan-dependency-graph.cjs'); + +// ─── Fixture builder ────────────────────────────────────────────────────── +// +// Builds a phase directory with: +// 01-01 — halted spike (SUMMARY status: halted) +// 01-02 — depends_on 01-01 (direct dependent) +// 01-03 — depends_on 01-02 (transitive dependent, 2 hops) +// 01-04 — decoupled (no depends_on) — the negative case +// Callers can pass extra plan/summary writers for diamond/boundary variants. + +function writePlan(phaseDir, filename, frontmatterLines, taskLine = 'Work') { + fs.writeFileSync( + path.join(phaseDir, filename), + [ + '---', + ...frontmatterLines, + '---', + '', + `# ${filename}`, + '', + `${filename}`, + '', + taskLine, + ].join('\n'), + ); +} + +function writeSummary(phaseDir, filename, status = 'complete') { + fs.writeFileSync( + path.join(phaseDir, filename), + ['---', 'phase: 01-alpha', 'plan: 01', `status: ${status}`, 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\n'), + ); +} + +function buildBaseFixture(tmpDir) { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '01-01-PLAN.md', ['wave: 1', 'objective: Halted spike', 'autonomous: true']); + writeSummary(phaseDir, '01-01-SUMMARY.md', 'halted'); + + writePlan(phaseDir, '01-02-PLAN.md', [ + 'wave: 2', 'objective: Direct dependent', 'autonomous: true', 'depends_on:', ' - 01-01', + ]); + + writePlan(phaseDir, '01-03-PLAN.md', [ + 'wave: 3', 'objective: Transitive dependent', 'autonomous: true', 'depends_on:', ' - 01-02', + ]); + + writePlan(phaseDir, '01-04-PLAN.md', ['wave: 1', 'objective: Decoupled plan', 'autonomous: true']); + + return phaseDir; +} + +// ─── phase-plan-index (cmdPhasePlanIndex, src/phase.cts) ────────────────── + +describe('phase-plan-index: halt propagation (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('direct dependent of a halted plan is blocked, not runnable', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); + const data = JSON.parse(result.output); + + const p02 = data.plans.find((p) => p.id === '01-02'); + assert.ok(p02, '01-02 should be present'); + assert.deepEqual(p02.blocked_by, ['01-01'], '01-02 should be blocked by the halted 01-01'); + assert.strictEqual(p02.has_summary, false); + assert.ok(!data.runnable.includes('01-02'), '01-02 must NOT be in the runnable view'); + }); + + test('transitive dependent (2 hops) is blocked via chain', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p03 = data.plans.find((p) => p.id === '01-03'); + assert.ok(p03, '01-03 should be present'); + assert.deepEqual(p03.blocked_by, ['01-01'], '01-03 should be transitively blocked by 01-01'); + assert.ok(!data.runnable.includes('01-03'), '01-03 must NOT be in the runnable view'); + }); + + test('transitive dependent at 3 hops stays blocked', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = buildBaseFixture(tmpDir); + writePlan(phaseDir, '01-05-PLAN.md', [ + 'wave: 4', 'objective: 3-hop dependent', 'autonomous: true', 'depends_on:', ' - 01-03', + ]); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p05 = data.plans.find((p) => p.id === '01-05'); + assert.ok(p05, '01-05 should be present'); + assert.deepEqual(p05.blocked_by, ['01-01'], '01-05 should stay blocked at 3 hops'); + assert.ok(!data.runnable.includes('01-05')); + }); + + test('diamond dependency is blocked by both halted ancestors', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); + writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); + writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-03-PLAN.md', [ + 'wave: 2', 'objective: Diamond join', 'autonomous: true', + 'depends_on:', ' - 02-01', ' - 02-02', + ]); + + const result = runGsdTools(['phase-plan-index', '2', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); + const data = JSON.parse(result.output); + + const p03 = data.plans.find((p) => p.id === '02-03'); + assert.ok(p03, '02-03 should be present'); + assert.deepEqual( + [...p03.blocked_by].sort(), + ['02-01', '02-02'], + '02-03 should be blocked by BOTH halted ancestors, deduplicated', + ); + }); + + test('unrelated decoupled plan stays runnable', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p04 = data.plans.find((p) => p.id === '01-04'); + assert.ok(p04, '01-04 should be present'); + assert.deepEqual(p04.blocked_by, [], '01-04 has no depends_on, so it must not be blocked'); + assert.ok(data.runnable.includes('01-04'), '01-04 (decoupled) must stay in the runnable view'); + }); + + test('incomplete field stays byte-identical when blocked plans are present', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + // Pre-#2830 semantics: incomplete = every plan without a matching SUMMARY, + // blocked or not. 01-01 has a SUMMARY (halted, but still a SUMMARY) so it + // is excluded; 01-02/01-03/01-04 have none, so all three are included — + // exactly as they would be with no halt-awareness at all. + assert.deepEqual( + [...data.incomplete].sort(), + ['01-02', '01-03', '01-04'], + 'incomplete must list every no-SUMMARY plan regardless of blocked status', + ); + }); + + test('dependency on an ordinary incomplete (non-halted) plan is not "blocked"', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-ordinary'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // 03-01 has NO summary at all (ordinary incomplete, not halted). + writePlan(phaseDir, '03-01-PLAN.md', ['wave: 1', 'objective: Ordinary unfinished plan', 'autonomous: true']); + writePlan(phaseDir, '03-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on ordinary incomplete plan', 'autonomous: true', + 'depends_on:', ' - 03-01', + ]); + + const result = runGsdTools(['phase-plan-index', '3', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '03-01'); + const p02 = data.plans.find((p) => p.id === '03-02'); + assert.deepEqual(p01.blocked_by, [], '03-01 (no summary) is not itself halted or blocked'); + assert.deepEqual(p02.blocked_by, [], '03-02 must NOT be "blocked" by an ordinary (non-halted) dependency'); + assert.ok(data.runnable.includes('03-02'), '03-02 stays runnable — only a halted upstream blocks'); + }); + + test('unresolved depends_on id is ignored, not blocked', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '04-unresolved'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '04-01-PLAN.md', [ + 'wave: 1', 'objective: References a nonexistent plan', 'autonomous: true', + 'depends_on:', ' - 99-99', + ]); + + const result = runGsdTools(['phase-plan-index', '4', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should not throw on an unresolved dependency: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '04-01'); + assert.deepEqual(p01.blocked_by, [], 'an unresolved depends_on id must not produce a spurious block'); + assert.ok(data.runnable.includes('04-01')); + }); + + test('malformed (unterminated) SUMMARY frontmatter fails open to not-halted', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-malformed'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '05-01-PLAN.md', ['wave: 1', 'objective: Has a malformed summary', 'autonomous: true']); + writeSummary(phaseDir, '05-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '05-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on 05-01', 'autonomous: true', 'depends_on:', ' - 05-01', + ]); + + // extractFrontmatter must fail safe on an unterminated frontmatter block + // (no closing '---'): isSummaryHalted's try/catch wraps BOTH the + // fs.readFileSync call and the extractFrontmatter call, so this exercises + // the identical fail-open catch site a genuine fs read error would hit — + // see the separate real-fs-fault-injection test below for the read-error + // half of that same catch block. + fs.writeFileSync( + path.join(phaseDir, '05-01-SUMMARY.md'), + '---\nphase: 05-malformed\nplan: 01\nstatus: halted\n', // no closing '---' + ); + + const result = runGsdTools(['phase-plan-index', '5', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index must not throw on malformed frontmatter: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '05-01'); + const p02 = data.plans.find((p) => p.id === '05-02'); + assert.strictEqual(p01.halted, false, 'unterminated frontmatter must fail open to not-halted'); + assert.deepEqual(p02.blocked_by, [], 'dependent of a fail-open-not-halted plan must not be blocked'); + }); + + test('CRLF SUMMARY frontmatter still detects status: halted', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '06-crlf'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '06-01-PLAN.md', ['wave: 1', 'objective: Halted with CRLF summary', 'autonomous: true']); + fs.writeFileSync( + path.join(phaseDir, '06-01-SUMMARY.md'), + ['---', 'phase: 06-crlf', 'plan: 01', 'status: halted', 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\r\n'), + ); + writePlan(phaseDir, '06-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on CRLF-summarized halt', 'autonomous: true', 'depends_on:', ' - 06-01', + ]); + + const result = runGsdTools(['phase-plan-index', '6', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed on CRLF frontmatter: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '06-01'); + const p02 = data.plans.find((p) => p.id === '06-02'); + assert.strictEqual(p01.halted, true, 'CRLF SUMMARY frontmatter must still parse status: halted'); + assert.deepEqual(p02.blocked_by, ['06-01'], 'dependent must be blocked even when the halt was recorded with CRLF newlines'); + }); + + test('status complete does not block dependents', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '07-complete'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '07-01-PLAN.md', ['wave: 1', 'objective: Ordinary completion', 'autonomous: true']); + writeSummary(phaseDir, '07-01-SUMMARY.md', 'complete'); + writePlan(phaseDir, '07-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on completed plan', 'autonomous: true', 'depends_on:', ' - 07-01', + ]); + + const result = runGsdTools(['phase-plan-index', '7', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '07-01'); + const p02 = data.plans.find((p) => p.id === '07-02'); + assert.strictEqual(p01.halted, false); + assert.deepEqual(p02.blocked_by, []); + assert.ok(data.runnable.includes('07-02')); + }); + + test('dependency cycle detection is unaffected by halt propagation', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cycle'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '08-01-PLAN.md', [ + 'wave: 1', 'objective: Cycle A', 'autonomous: true', 'depends_on:', ' - 08-02', + ]); + writePlan(phaseDir, '08-02-PLAN.md', [ + 'wave: 1', 'objective: Cycle B', 'autonomous: true', 'depends_on:', ' - 08-01', + ]); + + const result = runGsdTools(['phase-plan-index', '8', '--raw'], tmpDir); + // CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" bans + // regex-matching a child process's human-readable stderr/reason prose — + // assert on the typed failure signal (`result.success`) instead. To keep + // this test specific to the CYCLE (not "failed for any reason"), pair it + // with a differential fixture: the identical dependency shape with the + // cycle edge removed must succeed, isolating the cycle as the cause of + // the failure above without parsing error prose. + assert.strictEqual(result.success, false, 'a dependency cycle must still fail the command'); + + const acyclicDir = path.join(tmpDir, '.planning', 'phases', '09-nocycle'); + fs.mkdirSync(acyclicDir, { recursive: true }); + writePlan(acyclicDir, '09-01-PLAN.md', ['wave: 1', 'objective: No cycle A', 'autonomous: true']); + writePlan(acyclicDir, '09-02-PLAN.md', [ + 'wave: 1', 'objective: No cycle B', 'autonomous: true', 'depends_on:', ' - 09-01', + ]); + const acyclicResult = runGsdTools(['phase-plan-index', '9', '--raw'], tmpDir); + assert.strictEqual( + acyclicResult.success, + true, + `the identical dependency shape without the cycle edge must succeed, isolating the cycle as the cause of the failure above: ${acyclicResult.error}`, + ); + }); +}); + +// ─── findPhaseInternal / searchPhaseInDir (src/phase-locator.cts) ───────── + +describe('findPhaseInternal: halt propagation (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('direct dependent of a halted plan is blocked', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(result, 'expected a result'); + assert.deepEqual(result.blocked_by['01-02-PLAN.md'], ['01-01'], '01-02 should be blocked by halted 01-01'); + assert.ok(!result.runnable_plans.includes('01-02-PLAN.md')); + }); + + test('transitive dependent is blocked via chain', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual(result.blocked_by['01-03-PLAN.md'], ['01-01'], '01-03 should be transitively blocked'); + assert.ok(!result.runnable_plans.includes('01-03-PLAN.md')); + }); + + test('diamond dependency blocked by both halted ancestors', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); + writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); + writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-03-PLAN.md', [ + 'wave: 2', 'objective: Diamond join', 'autonomous: true', + 'depends_on:', ' - 02-01', ' - 02-02', + ]); + + const result = phaseLocator.findPhaseInternal(tmpDir, '2'); + assert.ok(result, 'expected a result'); + assert.deepEqual( + [...result.blocked_by['02-03-PLAN.md']].sort(), + ['02-01', '02-02'], + 'diamond join should be blocked by both halted ancestors, deduplicated', + ); + }); + + test('unrelated decoupled plan stays runnable', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(result.runnable_plans.includes('01-04-PLAN.md'), '01-04 (decoupled) must stay runnable'); + assert.strictEqual(result.blocked_by['01-04-PLAN.md'], undefined); + }); + + test('incomplete_plans stays byte-identical when blocked plans are present', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual( + [...result.incomplete_plans].sort(), + ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], + 'incomplete_plans must list every no-SUMMARY plan regardless of blocked status', + ); + }); + + test('halted_plans reports the halted plan itself by filename', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual(result.halted_plans, ['01-01-PLAN.md']); + }); +}); + +// ─── Parity: the two implementations must agree ─────────────────────────── + +describe('parity: phase-plan-index and findPhaseInternal agree on blocking (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('same fixture yields the same blocked-plan set and cause set from both readers', () => { + tmpDir = createTempProject('gsd-2830-parity-'); + buildBaseFixture(tmpDir); + + const cliResult = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + assert.ok(cliResult.success, `phase-plan-index should succeed: ${cliResult.error}`); + const cliData = JSON.parse(cliResult.output); + + const locatorResult = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(locatorResult, 'findPhaseInternal should return a result'); + + // Build { planId -> sorted cause list } from each reader and require them + // to be structurally identical (id-mapped, since one reader keys by bare + // id and the other by filename). + const cliBlocked = {}; + for (const plan of cliData.plans) { + if (plan.blocked_by.length > 0) cliBlocked[plan.id] = [...plan.blocked_by].sort(); + } + + const locatorBlocked = {}; + for (const [filename, causes] of Object.entries(locatorResult.blocked_by)) { + const planId = filename.replace(/-PLAN\.md$/i, '').replace(/^PLAN\.md$/i, ''); + locatorBlocked[planId] = [...causes].sort(); + } + + assert.deepEqual( + locatorBlocked, + cliBlocked, + 'phase-plan-index and findPhaseInternal must report the exact same blocked-plan-id -> cause-set mapping', + ); + }); +}); + +// ─── computeHaltPropagation — direct module tests + fast-check property ─── + +describe('computeHaltPropagation: graph invariants (#2830)', () => { + test('a node with no depends_on and not halted is never blocked', () => { + const { blockedBy } = planDependencyGraph.computeHaltPropagation([ + { id: 'A', resolvedDependsOn: [], halted: false }, + ]); + assert.strictEqual(blockedBy.get('A'), undefined); + }); + + test('a halted node is never blocked by itself', () => { + const { blockedBy } = planDependencyGraph.computeHaltPropagation([ + { id: 'A', resolvedDependsOn: [], halted: true }, + ]); + assert.strictEqual(blockedBy.get('A'), undefined, 'a halted plan is halted, not "blocked"'); + }); + + test('precomputedOrder (the phase.cts call shape) yields the same result as self-derived order', () => { + // phase.cts's cmdPhasePlanIndex passes computeDependencyLevels's own + // topological order in as `precomputedOrder` so this function does not + // re-run Kahn's algorithm. Assert both call shapes agree. + const nodes = [ + { id: 'A', resolvedDependsOn: [], halted: true }, + { id: 'B', resolvedDependsOn: ['A'], halted: false }, + { id: 'C', resolvedDependsOn: ['B'], halted: false }, + ]; + const selfDerived = planDependencyGraph.computeHaltPropagation(nodes); + const withPrecomputed = planDependencyGraph.computeHaltPropagation(nodes, ['A', 'B', 'C']); + assert.deepEqual([...withPrecomputed.blockedBy.entries()], [...selfDerived.blockedBy.entries()]); + assert.deepEqual(withPrecomputed.order, ['A', 'B', 'C']); + assert.strictEqual(withPrecomputed.visited, 3); + }); + + // Generate a random DAG over N nodes: edges only point from a lower index + // to a higher index (guarantees acyclicity by construction, independent of + // the code under test), with a random halted flag per node. + // + // Acyclic BY CONSTRUCTION: `to` is always drawn strictly above `from`, so + // no `.filter()` is involved. A filter here is not merely slower — with + // n === 1 the predicate `from < to` is unsatisfiable and fast-check retries + // generation forever, which hung the whole suite (the runner sets + // --test-timeout=0, so it never dies). Hoisted to describe scope so the + // regression test below can sample the identical arbitrary. + const dagArb = fc.integer({ min: 1, max: 12 }).chain((n) => { + const ids = Array.from({ length: n }, (_, i) => `N${i}`); + const edgeArb = n < 2 + ? fc.constant([]) + : fc.array( + fc.integer({ min: 0, max: n - 2 }).chain((from) => + fc.integer({ min: from + 1, max: n - 1 }).map((to) => ({ from, to }))), + { maxLength: n * 2 }, + ); + const haltedArb = fc.array(fc.boolean(), { minLength: n, maxLength: n }); + return fc.record({ ids: fc.constant(ids), edges: edgeArb, halted: haltedArb }); + }); + + test('fast-check — blocked set matches reachability from halted nodes', () => { + fc.assert( + fc.property(dagArb, ({ ids, edges, halted }) => { + const dependsOn = new Map(ids.map((id) => [id, []])); + for (const { from, to } of edges) { + dependsOn.get(ids[from]).push(ids[to]); + } + const nodes = ids.map((id, i) => ({ + id, + resolvedDependsOn: dependsOn.get(id), + halted: halted[i], + })); + + const { blockedBy } = planDependencyGraph.computeHaltPropagation(nodes); + + // Reference model: reachability via depends_on edges from a halted node. + const haltedSet = new Set(nodes.filter((n) => n.halted).map((n) => n.id)); + const dependsOnMap = new Map(nodes.map((n) => [n.id, n.resolvedDependsOn])); + function reachableHaltedCauses(id, seen = new Set()) { + const causes = new Set(); + for (const dep of dependsOnMap.get(id) ?? []) { + if (seen.has(dep)) continue; + seen.add(dep); + if (haltedSet.has(dep)) causes.add(dep); + for (const c of reachableHaltedCauses(dep, seen)) causes.add(c); + } + return causes; + } + + for (const n of nodes) { + const expected = [...reachableHaltedCauses(n.id)].sort(); + const actual = [...(blockedBy.get(n.id) ?? [])].sort(); + assert.deepEqual( + actual, + expected, + `node ${n.id}: computeHaltPropagation blockedBy must equal halted-reachability`, + ); + } + }), + { numRuns: 50 }, + ); + }); + + test('the DAG generator terminates on the degenerate single-node case (regression: unsatisfiable filter hung the suite)', () => { + // Bounded, non-hanging sample: if edgeArb regresses to a `.filter(from < to)` + // over a forced-equal {from, to} pair (n === 1), fast-check would retry + // generation forever and this assertion would never run. A small, + // explicit numRuns/seed keeps the check itself deterministic and fast. + const samples = fc.sample(dagArb, { numRuns: 20, seed: 7 }); + assert.ok(samples.length === 20, 'fc.sample must return the requested number of samples without hanging'); + const singleNodeSamples = samples.filter(({ ids }) => ids.length === 1); + assert.ok(singleNodeSamples.length > 0, 'the sample must include at least one degenerate single-node case'); + for (const { edges } of singleNodeSamples) { + assert.deepEqual(edges, [], 'the single-node case must yield an empty edge list, not an unsatisfiable filter'); + } + }); +}); + +// ─── init execute-phase (cmdInitExecutePhase, src/init.cts) ─────────────── +// +// #2830 names `init execute-phase` as the exact regressed consumer: the +// locator already computed halted_plans/blocked_by/runnable_plans, but the +// command built its output by explicit field enumeration, silently dropping +// all three. This drives the real CLI end to end (not the locator directly) +// to prove the passthrough, modeled on the "init execute-phase JSON output" +// fixture shape in tests/tdd-mode.test.cjs (ROADMAP.md + a phase directory +// resolvable by number). + +describe('init execute-phase: halt propagation passthrough (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('halted_plans, blocked_by, runnable_plans are forwarded; incomplete fields stay byte-identical', () => { + tmpDir = createTempProject('gsd-2830-init-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['init', 'execute-phase', '1', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + + // Guard: if the phase was not resolved, every assertion below is vacuous. + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + json.halted_plans.includes('01-01-PLAN.md'), + 'halted_plans must include the halted plan', + ); + + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '01-02-PLAN.md'), + 'blocked_by must have an entry for the direct dependent', + ); + assert.deepEqual( + json.blocked_by['01-02-PLAN.md'], + ['01-01'], + "blocked_by['01-02-PLAN.md'] must name 01-01 as the blocking cause", + ); + + assert.ok( + !json.runnable_plans.includes('01-02-PLAN.md'), + 'the dependent must NOT be offered as runnable work', + ); + assert.ok( + json.runnable_plans.includes('01-04-PLAN.md'), + 'the decoupled plan (no dependency on the halted plan) must stay runnable', + ); + + // Back-compat: incomplete_plans/incomplete_count must be exactly what they + // were pre-#2830 — every no-SUMMARY plan, blocked or not, still counted. + assert.deepEqual( + [...json.incomplete_plans].sort(), + ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], + 'incomplete_plans must be unchanged by halt-awareness', + ); + assert.strictEqual( + json.incomplete_count, + 3, + 'incomplete_count must be unchanged by halt-awareness', + ); + }); +}); + +// ─── #2830 review findings ───────────────────────────────────────────────── +// +// Adversarial review of the #2830 fix found two confirmed defects: +// 1. (BLOCKER) computeHaltPropagation's Kahn pass silently drops cycle +// participants (and anything downstream of them) from BOTH `order` and +// `blockedBy` — phase.cts hard-fails on a cycle before this ever +// matters, but phase-locator.cts (consumed by `init execute-phase`) +// does not pre-check, so a plan directly depends_on-ing a halted plan +// inside a cycle was reported as ordinary runnable. +// 2. (MAJOR) the summary templates presented `status: halted` as a +// trailing `#`-comment on the value line, but `extractFrontmatter` +// does not strip trailing YAML comments — an executor mimicking the +// template's own presentation wrote a halt that silently read back as +// not-halted. + +describe('#2830 review findings', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('defect 1: cycle repro — 08-02 depends_on [halted 08-01, 08-03]; 08-03 depends_on [08-02]', () => { + tmpDir = createTempProject('gsd-2830-review-cycle-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cyclehalt'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '08-01-PLAN.md', ['wave: 1', 'objective: Halted upstream', 'autonomous: true']); + writeSummary(phaseDir, '08-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '08-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on halted + cyclic peer', 'autonomous: true', + 'depends_on:', ' - 08-01', ' - 08-03', + ]); + writePlan(phaseDir, '08-03-PLAN.md', [ + 'wave: 2', 'objective: Cyclic peer of 08-02', 'autonomous: true', 'depends_on:', ' - 08-02', + ]); + + const result = runGsdTools(['init', 'execute-phase', '8', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + !json.runnable_plans.includes('08-02-PLAN.md'), + 'a plan that directly depends_on a halted plan must never be offered as runnable, cycle or not', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '08-02-PLAN.md'), + 'a plan must never silently vanish from both runnable_plans and blocked_by', + ); + assert.ok( + Array.isArray(json.blocked_by['08-02-PLAN.md']) && json.blocked_by['08-02-PLAN.md'].length > 0, + 'blocked_by entry must be non-empty, not a vacuous placeholder', + ); + }); + + test('defect 1: self-dependency (A depends_on A, nothing halted) must not be silently runnable', () => { + tmpDir = createTempProject('gsd-2830-review-self-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '09-selfdep'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '09-01-PLAN.md', [ + 'wave: 1', 'objective: Depends on itself', 'autonomous: true', 'depends_on:', ' - 09-01', + ]); + + const result = runGsdTools(['init', 'execute-phase', '9', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + !json.runnable_plans.includes('09-01-PLAN.md'), + 'a self-dependent plan must not be silently offered as runnable', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '09-01-PLAN.md'), + 'a self-dependent plan must never silently vanish from both runnable_plans and blocked_by', + ); + }); + + test('defect 2: SUMMARY status with an inline YAML comment still blocks the dependent', () => { + tmpDir = createTempProject('gsd-2830-review-comment-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '10-inlinecomment'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '10-01-PLAN.md', ['wave: 1', 'objective: Halted, recorded with an inline comment', 'autonomous: true']); + writeSummary(phaseDir, '10-01-SUMMARY.md', 'halted # designed stop'); + writePlan(phaseDir, '10-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on the inline-commented halt', 'autonomous: true', 'depends_on:', ' - 10-01', + ]); + + const result = runGsdTools(['init', 'execute-phase', '10', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + json.halted_plans.includes('10-01-PLAN.md'), + 'status: halted # designed stop must still be recognized as halted', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '10-02-PLAN.md'), + 'the dependent of an inline-commented halt must be blocked', + ); + assert.ok( + !json.runnable_plans.includes('10-02-PLAN.md'), + 'the dependent of an inline-commented halt must not be offered as runnable', + ); + }); + + test('defect 2: isHaltedStatus strips an unquoted trailing YAML comment before comparing', () => { + const { isHaltedStatus } = planDependencyGraph; + assert.strictEqual(isHaltedStatus('halted'), true); + assert.strictEqual(isHaltedStatus('Halted'), true); + assert.strictEqual(isHaltedStatus('halted '), true); + assert.strictEqual(isHaltedStatus('halted # designed stop'), true); + assert.strictEqual(isHaltedStatus('halted#nospace'), false, 'a `#` with no preceding whitespace is not a YAML comment'); + assert.strictEqual(isHaltedStatus('complete'), false); + assert.strictEqual(isHaltedStatus('complete # done'), false); + assert.strictEqual(isHaltedStatus(''), false); + assert.strictEqual(isHaltedStatus(undefined), false); + }); +});