* test(#2830): add failing regression tests for halted-plan dependent blocking Add tests/fix-2830-halted-plan-dependents.test.cjs covering direct, transitive (2 and 3 hop), and diamond dependents of a halted plan across both independent "which plans are incomplete" readers (phase-plan-index's cmdPhasePlanIndex and findPhaseInternal/searchPhaseInDir), the negative case (an unrelated decoupled plan stays runnable), and a parity check that the two readers agree. Uses only modules that already exist at this commit (gsd-tools.cjs via subprocess, the pre-existing phase-locator.cjs) so the test file loads and runs cleanly on a fresh clone of this exact commit. These fail against current behavior: neither reader has any concept of a halted plan or a blocked_by/runnable view yet. * fix(#2830): a halted plan no longer leaves its dependents on the runnable work list A plan that reaches a designed stop still writes a SUMMARY, so both "which plans are incomplete" readers saw it as an ordinary completion and reported its dependents as ordinary runnable work — never checking whether an upstream plan had halted rather than finished. - New `status: halted` frontmatter value, documented in all four SUMMARY templates alongside the existing `status: complete`. - New shared src/plan-dependency-graph.cts: a single computeHaltPropagation pass that both phase.cts's cmdPhasePlanIndex (wave-grouping) and phase-locator.cts's searchPhaseInDir (the phase-location primitive, ~50 dependent symbols across 5 command routers) now call, so the two-implementation divergence that caused this bug cannot recur. It accepts an optional precomputedOrder so cmdPhasePlanIndex — which already runs Kahn's algorithm in computeDependencyLevels for wave assignment — passes that order straight through instead of a second traversal; searchPhaseInDir (no prior traversal) lets the module derive its own. The two small duplicated predicates each reader would otherwise carry (is this status "halted"?, which summary file matches which plan id?) are centralized in the same module as isHaltedStatus/buildSummaryFileIndex. - Additive fields only: `halted`/`blocked_by`/`runnable` on cmdPhasePlanIndex's plans[] and top level, `halted_plans`/`blocked_by`/ `runnable_plans` on searchPhaseInDir's result. The pre-existing `incomplete`/`incomplete_plans` fields are unchanged in meaning and membership. - execute-phase.md's discover_and_group_plans step now also skips any plan whose `blocked_by` is non-empty, reporting it by name with its blocking chain, in addition to (not instead of) the existing has_summary skip rule. Extends tests/fix-2830-halted-plan-dependents.test.cjs (introduced in the prior commit) with direct unit coverage of computeHaltPropagation (including the precomputedOrder call shape) and a fast-check property test — both only possible once this commit's new module exists. Closes #2830 * fix(#2830): surface the halt-aware view from init execute-phase The adopted work made phase-locator compute halted_plans / blocked_by / runnable_plans, but cmdInitExecutePhase builds its output by explicitly enumerating fields, so all three were computed and then silently dropped at the exact consumer the issue names as regressed. Forwards them additively -- incomplete_plans and incomplete_count keep their name, type and semantics byte-for-byte -- and adds the same three empty defaults to the roadmap-only fallback so the shape is consistent in both branches. Covered by a new test that drives the real CLI end to end rather than the locator function, since the locator already worked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#2830): fail closed on dependency cycles and stop the templates inviting the defect Three review findings, all fixed: - BLOCKER (isolated adversarial). Cycle participants never reach indegree 0 in the Kahn pass, so they were excluded from the topological order, never visited by the forward pass, and vanished from blocked_by entirely -- i.e. reported as runnable. The wave-grouping reader hard-fails on a cycle so it never hit this, but the phase-location reader does not, so init execute-phase offered a plan depending directly on a halted plan. Reproduced, then fixed in the shared engine so every consumer is safe regardless of pre-checks: a node absent from the order is now blocked with a deterministic, non-empty named cause. A plan silently missing from both blocked_by and runnable is the exact disappearance this issue exists to prevent. - MAJOR (isolated adversarial). All four summary templates showed the field as an inline comment on the value line. Frontmatter parsing does not strip trailing comments, so an executor copying the templates' own presentation wrote a halt that parsed as a non-halted string, silently reproducing the original bug. Guidance moved off the value line, and the halt predicate now tolerates an unquoted trailing comment. - HARD standards violation. A test regex-matched child-process stderr prose for /cycle/i, which CONTRIBUTING bans. Replaced with the structured failure signal plus a differential assertion (same fixture without the cycle edge must succeed), so it stays cycle-specific without matching prose. Also folds the duplicated read-summary-and-check-halted wrapper out of both readers into the shared module -- centralizing only the predicate left the exact two-copies-that-drift pattern the module exists to prevent -- and commits the artifact-types documentation for the new status value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(#2830): stop the property generator hanging the whole suite The remote runner did not fail -- it hung. Two containers sat in this file for 31+ minutes, and an earlier attempt ran 9 hours before I killed it. The runner passes --test-timeout=0, so nothing ever reaps it: this would have hung CI indefinitely, not reported a failure. Root cause: the DAG generator built edges by rejection -- from: fc.integer({ min: 0, max: n - 1 }) to: fc.integer({ min: 0, max: n - 1 }) .filter(({ from, to }) => from < to) With n === 1 both integers are forced to 0, so the predicate is unsatisfiable and fast-check retries value generation forever. n is drawn from 1..12 and fast-check biases toward boundary values, so n === 1 is reached almost at once. This also explains why the failing-first run completed normally while the fixed run hung: before the fix the graph module did not exist, so the property test threw on import and never reached generation. It only starts hanging once the code under test works. Generates the DAG by construction instead -- `to` is drawn strictly above `from`, with the degenerate single-node case short-circuited to an empty edge list -- so no rejection sampling is involved. Switches the import to the shared fast-check setup so the seed and run count are pinned per CONTRIBUTING, and adds a bounded regression guard that samples the arbitrary directly, so a future reintroduction fails loudly instead of hanging. Verified: the file now completes in 2 seconds, 29 tests started and 29 finished, zero failures, against an indefinite hang before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#2830): restore the depends_on display contract and acknowledge the workflow growth Full-suite run surfaced two things the focused harnesses could not. 1. Regression of a pinned pre-existing contract (#3785). A refactor routed the EMITTED depends_on field through the new dependency resolver, which also consults the canonical-prefix map. The original consulted the plan map only, so a short canonical prefix passed through verbatim -- '24-01' stayed '24-01' rather than becoming '24-01-auth-hardening'. The emitted field is a DISPLAY mapping, not the DAG resolution, and #3785 pins that. Reverted with a comment recording why it must not use the resolver; full resolution is still used for the wave DAG and halt propagation, which is what needs it. 2. The workflow file grew 518 bytes without an acknowledgment, from the halt-aware skip rule and the widened parse contract. Acknowledged. Note on where the acknowledgment landed: the guidance is to add a NEW fragment, but execute-phase.md is already named by an existing fragment and the linter hard-fails when two ack sources name the same path. Appending to the owning fragment, following its own established multi-PR pattern, was the only lint-clean option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#2830): backfill changeset pr number Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/daring-wolves-frolic.md
Normal file
5
.changeset/daring-wolves-frolic.md
Normal file
@@ -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)
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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) |
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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]**
|
||||
|
||||
@@ -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]**
|
||||
|
||||
@@ -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]**
|
||||
|
||||
@@ -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 `<step name="create_summary">` 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`.
|
||||
</frontmatter_guidance>
|
||||
|
||||
<coverage_guidance>
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
13
src/init.cts
13
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)
|
||||
|
||||
@@ -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<string, string[]>;
|
||||
/**
|
||||
* #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<string, string[]> = {};
|
||||
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;
|
||||
|
||||
@@ -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<string, RawPlan>,
|
||||
canonicalToId: Map<string, string>,
|
||||
): 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<id,number>, visited: number }.
|
||||
// visited < rawPlans.length signals a dependency cycle.
|
||||
// in-phase dependsOn DAG (Kahn's algorithm). Returns { level: Map<id,number>, 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<string, RawPlan>,
|
||||
canonicalToId: Map<string, string>,
|
||||
): { level: Map<string, number>; visited: number } {
|
||||
): { level: Map<string, number>; visited: number; order: string[] } {
|
||||
const level = new Map<string, number>();
|
||||
const inDeg = new Map<string, number>();
|
||||
const adj = new Map<string, string[]>();
|
||||
@@ -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<string, unknown>[] = [];
|
||||
const waves: Record<string, string[]> = {};
|
||||
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<string, unknown> = {
|
||||
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;
|
||||
|
||||
256
src/plan-dependency-graph.cts
Normal file
256
src/plan-dependency-graph.cts
Normal file
@@ -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<string, string> {
|
||||
const index = new Map<string, string>();
|
||||
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<string, string[]>;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<string, number>();
|
||||
const adj = new Map<string, string[]>(); // 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<string, string[]>();
|
||||
for (const id of order) {
|
||||
const node = byId.get(id);
|
||||
if (!node) continue;
|
||||
const causes = new Set<string>();
|
||||
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 };
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `<!-- gsd:section -->` 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 <value>` 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 `<!-- gsd:section -->` 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 <value>` 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."
|
||||
}
|
||||
}
|
||||
|
||||
764
tests/fix-2830-halted-plan-dependents.test.cjs
Normal file
764
tests/fix-2830-halted-plan-dependents.test.cjs
Normal file
@@ -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 = '<task>Work</task>') {
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, filename),
|
||||
[
|
||||
'---',
|
||||
...frontmatterLines,
|
||||
'---',
|
||||
'',
|
||||
`# ${filename}`,
|
||||
'',
|
||||
`<objective>${filename}</objective>`,
|
||||
'',
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user