Adds a status: superseded plan-frontmatter marker that scanPhasePlans excludes from both plan and summary counts, so a phase with a deliberately-unexecuted plan no longer reads incomplete forever (the plan-level analogue of #1514). Includes all-superseded completion handling and a bounded, symlink-safe frontmatter read. Fixes #2349.
This commit is contained in:
5
.changeset/daring-otters-roar.md
Normal file
5
.changeset/daring-otters-roar.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2404
|
||||
---
|
||||
**A phase with a deliberately-unexecuted (superseded) plan no longer stays stuck below 100%** — a plan reassigned or dropped mid-phase can never gain a matching SUMMARY, yet plan-scan counted it forever, so the phase read In Progress and the milestone sat below 100% permanently — the plan-level analogue of the retired-phase bug (#1514). Mark such a plan `status: superseded` in its PLAN.md frontmatter and it is now excluded from both the plan and summary counts, so the phase completes honestly (a 13-plan phase with 2 superseded reads 11/11). Plans without the marker are unchanged. (#2349)
|
||||
@@ -72,8 +72,24 @@ must_haves:
|
||||
| `autonomous` | Yes | boolean | `true` when all tasks are type `auto`. `false` when the plan contains any `checkpoint:*` task that requires human interaction. |
|
||||
| `requirements` | Yes | array of IDs | Requirement IDs from ROADMAP.md that this plan addresses. Every phase requirement ID must appear in at least one plan's `requirements` field. Empty arrays are a BLOCKER. |
|
||||
| `user_setup` | No | array of objects | External-service setup steps that Claude cannot automate (account creation, secret retrieval, dashboard configuration). When present, execute-phase generates a `USER-SETUP.md` checklist for the developer. |
|
||||
| `status` | No | `superseded` | Marks a plan that was deliberately reassigned or abandoned mid-phase and will never be executed. A `status: superseded` plan is excluded from the phase's plan and summary counts, so it never holds the phase below 100%. See [Superseded plans](#superseded-plans). Any other value (or the field's absence) has no effect on counting. |
|
||||
| `must_haves` | Yes | object | Goal-backward verification criteria. See below. |
|
||||
|
||||
### Superseded plans
|
||||
|
||||
A phase reads complete when every `*-PLAN.md` has a matching `*-SUMMARY.md`. When a plan is reassigned or dropped mid-phase — its work folded into a later plan — it will never gain a summary, and without a marker it would pin the phase below 100% forever (the plan-level analogue of a retired phase). Add `status: superseded` to that plan's frontmatter to exclude it from **both** the plan count (denominator) and the summary count (numerator):
|
||||
|
||||
```yaml
|
||||
---
|
||||
phase: 05-api
|
||||
plan: "12"
|
||||
type: execute
|
||||
status: superseded
|
||||
---
|
||||
```
|
||||
|
||||
A phase with 13 plans, two of them `superseded`, then reads `11/11 → complete` — no fabricated summary required. The match is case-insensitive. Plans without the marker are counted exactly as before.
|
||||
|
||||
---
|
||||
|
||||
## `must_haves` field
|
||||
|
||||
@@ -7,17 +7,67 @@
|
||||
* from the prior hand-written .cjs; only types are added.
|
||||
*/
|
||||
|
||||
import { existsSync, readdirSync } from 'node:fs';
|
||||
import { existsSync, readdirSync, statSync, openSync, readSync, closeSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import coreUtils = require('./core-utils.cjs');
|
||||
const { countMatchedSummaries } = coreUtils;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import frontmatterMod = require('./frontmatter.cjs');
|
||||
const { extractFrontmatter } = frontmatterMod;
|
||||
|
||||
// Excluded derivative files
|
||||
const PLAN_OUTLINE_RE = /-OUTLINE\.md$/i;
|
||||
const PLAN_PRE_BOUNCE_RE = /\.pre-bounce\.md$/i;
|
||||
const PLAN_REVIEW_RE = /-PLAN-REVIEW\.md$/i;
|
||||
|
||||
// #2349: a plan's frontmatter always sits at byte 0 and closes well before the
|
||||
// body, so only a bounded prefix is ever needed to read the `status` marker.
|
||||
// Capping the read keeps scanPhasePlans — which loops over every phase directory
|
||||
// on hot paths (state sync/validate, roadmap progress) — from slurping a
|
||||
// pathologically large committed plan file into memory just to inspect one key.
|
||||
const PLAN_FRONTMATTER_READ_CAP = 64 * 1024;
|
||||
|
||||
/**
|
||||
* #2349: a plan whose frontmatter declares `status: superseded` was deliberately
|
||||
* reassigned or never executed — its work moved to a later plan, so it can never
|
||||
* gain a matching `*-SUMMARY.md`. Like a retired phase (#1514, one level up), such
|
||||
* a plan must be excluded from BOTH the plan and summary counts; otherwise a phase
|
||||
* with a deliberately-unexecuted plan reads `completed: false` forever, pinning the
|
||||
* milestone below 100%. Reading only the frontmatter `status` key is the same seam
|
||||
* verify.cts / phase.cts already use for plan metadata; a plan without the marker is
|
||||
* counted exactly as before.
|
||||
*
|
||||
* This is the only path in scanPhasePlans that opens file *contents* (the rest is
|
||||
* filename matching), so it is hardened accordingly: `statSync().isFile()` rejects
|
||||
* anything that is not a regular file — a directory, socket, or a symlink resolving
|
||||
* to a device such as `/dev/zero` (a git-committable DoS vector; cf. #2378/#2383) —
|
||||
* BEFORE any open, and the read is bounded to a fixed prefix. Fail-safe throughout:
|
||||
* a non-regular or unreadable plan is treated as a normal (counted) plan, never
|
||||
* silently dropped.
|
||||
*/
|
||||
function isPlanSuperseded(planFullPath: string): boolean {
|
||||
let content: string;
|
||||
try {
|
||||
const st = statSync(planFullPath); // follows symlinks → resolves to the target's real type
|
||||
if (!st.isFile()) return false;
|
||||
const length = Math.min(st.size, PLAN_FRONTMATTER_READ_CAP);
|
||||
if (length === 0) return false;
|
||||
const fd = openSync(planFullPath, 'r');
|
||||
try {
|
||||
const buf = Buffer.allocUnsafe(length);
|
||||
const bytesRead = readSync(fd, buf, 0, length, 0);
|
||||
content = buf.toString('utf8', 0, bytesRead);
|
||||
} finally {
|
||||
closeSync(fd);
|
||||
}
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const status = extractFrontmatter(content)['status'];
|
||||
return typeof status === 'string' && status.trim().toLowerCase() === 'superseded';
|
||||
}
|
||||
|
||||
function isRootPlanFile(fileName: string): boolean {
|
||||
if (PLAN_OUTLINE_RE.test(fileName)) return false;
|
||||
if (PLAN_PRE_BOUNCE_RE.test(fileName)) return false;
|
||||
@@ -84,7 +134,16 @@ function scanPhasePlans(phaseDir: string): PhaseScanResult {
|
||||
} catch { /* ignore unreadable nested layout */ }
|
||||
}
|
||||
|
||||
const planFiles = rootPlanFiles.concat(nestedPlanFiles);
|
||||
const allPlanFiles = rootPlanFiles.concat(nestedPlanFiles);
|
||||
// #2349: drop plans explicitly marked `status: superseded` from the plan set
|
||||
// BEFORE counting, so they inflate neither the denominator (planCount) nor,
|
||||
// via countMatchedSummaries below, the numerator (summaryCount). Plans without
|
||||
// the marker are untouched, so behaviour is byte-for-behaviour identical for
|
||||
// every existing phase — only a phase carrying the new marker changes.
|
||||
const supersededPlanFiles = allPlanFiles.filter((f) => isPlanSuperseded(join(phaseDir, f)));
|
||||
const planFiles = supersededPlanFiles.length === 0
|
||||
? allPlanFiles
|
||||
: allPlanFiles.filter((f) => !supersededPlanFiles.includes(f));
|
||||
const summaryFiles = rootSummaryFiles.concat(nestedSummaryFiles);
|
||||
const planCount = planFiles.length;
|
||||
// Count only summaries that are the PLAN→SUMMARY partner of an existing plan
|
||||
@@ -97,7 +156,14 @@ function scanPhasePlans(phaseDir: string): PhaseScanResult {
|
||||
return {
|
||||
planCount,
|
||||
summaryCount,
|
||||
completed: planCount > 0 && summaryCount >= planCount,
|
||||
// #2349: gate completion on whether the phase had ANY plans on disk
|
||||
// (allPlanFiles), NOT on the post-exclusion planCount. A phase whose plans
|
||||
// were ALL marked superseded has planCount 0, but it is NOT an unplanned
|
||||
// empty phase — there is simply no remaining work, so it must read complete
|
||||
// (0 >= 0) rather than being pinned below 100% forever, which is the very
|
||||
// failure this fix removes. A genuinely empty phase (no plans authored)
|
||||
// still has allPlanFiles.length 0 and stays not-completed, exactly as before.
|
||||
completed: allPlanFiles.length > 0 && summaryCount >= planCount,
|
||||
hasNestedPlans,
|
||||
planFiles,
|
||||
summaryFiles,
|
||||
|
||||
@@ -1873,6 +1873,164 @@ describe('scanPhasePlans — call-site parity on mixed fixture', () => {
|
||||
assert.strictEqual(result.hasNestedPlans, true, 'plans/ dir exists with plans');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #2349: plan-level supersession. A plan whose frontmatter declares
|
||||
// `status: superseded` was deliberately reassigned / never executed and can
|
||||
// never gain a matching SUMMARY. Like a retired phase (#1514, one level up) it
|
||||
// must be excluded from BOTH the plan and summary counts, so the phase can still
|
||||
// read 100% complete instead of being pinned below it forever. A plan WITHOUT
|
||||
// the marker must be counted exactly as before (no over-exclusion).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('scanPhasePlans — superseded plans (#2349)', () => {
|
||||
const SUPERSEDED_FM =
|
||||
'---\nphase: "1"\nplan: "6"\ntype: implementation\nstatus: superseded\n---\n\n# Superseded plan\n';
|
||||
|
||||
function writePlan(dir, name, body) {
|
||||
fs.writeFileSync(path.join(dir, name), body);
|
||||
}
|
||||
|
||||
test('a plan marked status: superseded is excluded from planCount and planFiles', () => {
|
||||
const dir = phaseDir();
|
||||
touch(dir, '01-01-PLAN.md', '01-01-SUMMARY.md');
|
||||
writePlan(dir, '01-06-PLAN.md', SUPERSEDED_FM);
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 1, 'superseded plan not counted in planCount');
|
||||
assert.strictEqual(result.summaryCount, 1, 'summaryCount honest at 1');
|
||||
assert.strictEqual(result.completed, true, 'phase completes despite the summary-less superseded plan');
|
||||
assert.ok(!result.planFiles.includes('01-06-PLAN.md'), 'superseded plan absent from planFiles');
|
||||
assert.ok(result.planFiles.includes('01-01-PLAN.md'), 'live plan still present');
|
||||
});
|
||||
|
||||
test('reported case: 13 plans, 2 superseded, 11 summaries → 11/11 completed', () => {
|
||||
const dir = phaseDir();
|
||||
// 11 executed plans, each with its matching summary
|
||||
for (let i = 1; i <= 11; i++) {
|
||||
const n = String(i).padStart(2, '0');
|
||||
touch(dir, `05-${n}-PLAN.md`, `05-${n}-SUMMARY.md`);
|
||||
}
|
||||
// 2 superseded plans, no summaries (work reassigned to later plans)
|
||||
writePlan(dir, '05-12-PLAN.md', SUPERSEDED_FM);
|
||||
writePlan(dir, '05-13-PLAN.md', SUPERSEDED_FM);
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 11, 'denominator excludes the 2 superseded plans');
|
||||
assert.strictEqual(result.summaryCount, 11, 'numerator honest at 11');
|
||||
assert.strictEqual(result.completed, true, 'phase reads complete — no longer pinned below 100%');
|
||||
});
|
||||
|
||||
test('boundary: a live unsummarized plan still blocks completion (limit+1)', () => {
|
||||
const dir = phaseDir();
|
||||
touch(dir, '02-01-PLAN.md', '02-01-SUMMARY.md');
|
||||
touch(dir, '02-02-PLAN.md'); // live plan, no summary → must still block
|
||||
writePlan(dir, '02-09-PLAN.md', SUPERSEDED_FM); // superseded, excluded
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 2, 'only the superseded plan is excluded');
|
||||
assert.strictEqual(result.summaryCount, 1);
|
||||
assert.strictEqual(result.completed, false, 'a live unsummarized plan still blocks completion');
|
||||
});
|
||||
|
||||
test('no over-exclusion: a plan without the marker (or with a non-superseded status) is counted', () => {
|
||||
const dir = phaseDir();
|
||||
touch(dir, '03-01-PLAN.md'); // empty file, no frontmatter at all
|
||||
writePlan(dir, '03-02-PLAN.md', '---\nphase: "3"\nplan: "2"\nstatus: complete\n---\n\n# done\n');
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 2, 'only status: superseded is excluded; other/absent statuses count');
|
||||
assert.ok(result.planFiles.includes('03-01-PLAN.md'));
|
||||
assert.ok(result.planFiles.includes('03-02-PLAN.md'));
|
||||
});
|
||||
|
||||
test('superseded marker is case-insensitive and whitespace-tolerant', () => {
|
||||
const dir = phaseDir();
|
||||
touch(dir, '04-01-PLAN.md', '04-01-SUMMARY.md');
|
||||
writePlan(dir, '04-07-PLAN.md', '---\nphase: "4"\nplan: "7"\nstatus: Superseded \n---\n\n# x\n');
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 1, 'Superseded (mixed-case, padded) still excluded');
|
||||
assert.strictEqual(result.completed, true);
|
||||
});
|
||||
|
||||
test('nested layout: a superseded nested plan is excluded too', () => {
|
||||
const dir = phaseDir();
|
||||
const plansDir = path.join(dir, 'plans');
|
||||
fs.mkdirSync(plansDir);
|
||||
touch(plansDir, 'PLAN-01-setup.md', 'SUMMARY-01-setup.md');
|
||||
writePlan(plansDir, 'PLAN-02-dropped.md', SUPERSEDED_FM);
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 1, 'nested superseded plan excluded');
|
||||
assert.strictEqual(result.summaryCount, 1);
|
||||
assert.strictEqual(result.completed, true);
|
||||
assert.ok(!result.planFiles.includes('plans/PLAN-02-dropped.md'));
|
||||
});
|
||||
|
||||
test('fail-safe: an unreadable plan path (a directory named *-PLAN.md) is counted, not treated as superseded', () => {
|
||||
const dir = phaseDir();
|
||||
touch(dir, '06-01-PLAN.md', '06-01-SUMMARY.md');
|
||||
// A *directory* whose name matches a plan file: readdir lists it, but reading
|
||||
// it as a file throws EISDIR on every platform (root-independent, leak-safe).
|
||||
// The read-failure path must fail safe to "not superseded" (counted) — never
|
||||
// silently drop a plan whose frontmatter simply could not be read.
|
||||
fs.mkdirSync(path.join(dir, '06-02-PLAN.md'));
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 2, 'unreadable plan still counted (fail-safe, not excluded)');
|
||||
assert.strictEqual(result.completed, false, 'the unreadable plan has no summary → still blocks');
|
||||
});
|
||||
|
||||
test('all plans superseded → phase reads complete, not pinned below 100% (0 >= 0)', () => {
|
||||
const dir = phaseDir();
|
||||
// Every plan in the phase was reassigned elsewhere: nothing left to execute.
|
||||
writePlan(dir, '08-01-PLAN.md', SUPERSEDED_FM);
|
||||
writePlan(dir, '08-02-PLAN.md', SUPERSEDED_FM);
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 0, 'all plans excluded');
|
||||
assert.strictEqual(result.summaryCount, 0);
|
||||
assert.strictEqual(result.completed, true, 'a fully-superseded phase has no remaining work → complete');
|
||||
});
|
||||
|
||||
test('a genuinely empty phase (no plans authored) still reads NOT complete', () => {
|
||||
// Regression guard for the all-superseded fix: the completion guard keys off
|
||||
// whether any plans existed on disk, so a phase with zero plans is unaffected.
|
||||
const result = scanPhasePlans(phaseDir());
|
||||
assert.strictEqual(result.planCount, 0);
|
||||
assert.strictEqual(result.completed, false, 'no plans authored → not complete (unchanged)');
|
||||
});
|
||||
|
||||
test('a superseded marker is detected even when the plan body is very large', () => {
|
||||
// Frontmatter sits at byte 0; the scan reads only a bounded prefix, so a large
|
||||
// body must neither hide the marker nor force reading the whole file.
|
||||
const dir = phaseDir();
|
||||
touch(dir, '09-01-PLAN.md', '09-01-SUMMARY.md');
|
||||
const bigBody = 'x'.repeat(200 * 1024); // 200 KB body, well past the read cap
|
||||
writePlan(dir, '09-09-PLAN.md', `---\nphase: "9"\nplan: "9"\nstatus: superseded\n---\n\n${bigBody}\n`);
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, 1, 'superseded plan with a large body still excluded');
|
||||
assert.strictEqual(result.completed, true);
|
||||
});
|
||||
|
||||
test('property: K superseded plans never change the completed verdict of N summarized plans', () => {
|
||||
const fc = require('fast-check');
|
||||
fc.assert(
|
||||
fc.property(fc.integer({ min: 1, max: 8 }), fc.integer({ min: 0, max: 5 }), (n, k) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-plan-scan-prop-'));
|
||||
try {
|
||||
for (let i = 1; i <= n; i++) {
|
||||
const idx = String(i).padStart(2, '0');
|
||||
fs.writeFileSync(path.join(dir, `07-${idx}-PLAN.md`), '');
|
||||
fs.writeFileSync(path.join(dir, `07-${idx}-SUMMARY.md`), '');
|
||||
}
|
||||
for (let j = 1; j <= k; j++) {
|
||||
fs.writeFileSync(path.join(dir, `07-${String(50 + j)}-PLAN.md`), SUPERSEDED_FM);
|
||||
}
|
||||
const result = scanPhasePlans(dir);
|
||||
assert.strictEqual(result.planCount, n, 'planCount ignores the K superseded plans');
|
||||
assert.strictEqual(result.completed, true, 'N fully-summarized plans stay complete regardless of K');
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
}),
|
||||
{ numRuns: 40 },
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user