chore(#1671): fragmentize plan-phase.md and repair flag forwarding to the init bundle — Phase 6.2 (#3019)
* chore(#2993): fragmentize plan-phase.md onto the fragment model Epic #1671 Phase 6.2. plan-phase.md is the largest workflow in the repo and carried zero markers; it was deferred out of the Phase 3 pilot for two reasons, both now dead. The 36-byte PRE_PHASE6 headroom was never the blocker it looked like — fragmentizing is net-negative on host source, so the trim is what creates the room. The --mvp interleaving was resolved by measurement in #2992 and no sub-line mechanism is built. - widen WHEN_VOCABULARY 14 -> 19 via a second coordinated ADR-1671 amendment: flag:--ingest, flag:--prd, flag:--research-phase, flag:--reviews, state:chunked-mode - state:chunked-mode is `--chunked` OR config workflow.plan_chunked, and that disjunction is resolved in the FACT, never in the grammar, so a compound condition never becomes an operator - parse the new flags on the plan-phase route; extract six gated bodies to gsd-core/workflows/plan-phase/steps/ behind manifest-gated stubs - prd-express-path.md was already extracted but read unconditionally; its wrapper is now gated, so the existing extraction finally pays off plan-phase.md 94,483 -> 87,575 bytes (cap 94,519): headroom goes from 36 bytes to 6,944. Also closes a surfaced docs gap: five real plan-phase flags (--chunked, --skip-ui, --bounce, --skip-bounce, --granularity) were documented in neither the argument-hint nor help. Making --chunked load-bearing without fixing its siblings would leave the defect class half-open. Refs #2993 * fix(#2993): forward flags to the init bundle so section gating actually fires Blocker found by the correctness review, confirmed directly, and missed by both the isolated reviewer and every test in this branch. Neither workflow forwarded its flags to the init CLI: plan-phase.md:71 INIT=$(gsd_run query init.plan-phase "$PHASE" $GRAN_PARAM) execute-phase.md:84 INIT=$(gsd_run query init.execute-phase "${PHASE_ARG}") So every flag: atom was permanently false in production and its section permanently excluded. For plan-phase that made the PRD express path UNREACHABLE — a regression, since it was an unconditional read before. For execute-phase this is PRE-EXISTING: #2932 shipped `flag:--wave` gating that has never once been true, so `--wave` silently dropped its own wave-filtering guidance. Fixed here under the no-defer rule. Why every test missed it: they drive the init CLI directly with flags, which works. Production goes through the workflow's bash line, which did not pass them — the exact "assert against the shape production uses" trap this branch's own test matrix warns about. - parse and forward --prd/--ingest/--research-phase/--reviews/--chunked (plan-phase) and --wave (execute-phase), using the anchored regex idiom the neighbouring GRAN_PARAM line already uses - add a regression guard DERIVED FROM THE MANIFEST: for every flag:--X section, the owning workflow's init line must forward --X. It fails against the pre-fix files and covers any future atom, rather than spot-checking today's six. Verified through the workflow shape, not the CLI shape: `3 --prd spec.md` now yields ["prd-express-gate"] (was []), `2 --wave 2` yields ["partial-wave"] (was []). Refs #2993 * test(#2993): acknowledge the execute-phase ripple and regenerate install-tree fixtures Remote matrix was red with 46 unique failures, identical on both lanes. Both causes are mechanical consequences of changing shipped workflow content, and neither is visible to any local gate. - emitted-attribution: execute-phase.md grew 163 bytes from the WAVE_PARAM forwarding fix and was unacknowledged, while the ack fragment named plan-phase.md, which SHRANK and therefore needed no ack at all — a stale entry is itself a failure. The reason now names the real ripple. The entry had to merge into the existing 2930 fragment: the ack linter does unconditional cross-fragment duplicate-key detection with no spent/live exception, so a second fragment declaring execute-phase.md collides even when the first is already merged and inert. Resolved per the linter's own guidance and that file's precedent of appending successive ripple reasons to one entry. - golden-install-tree: tests/fixtures/install-tree/*.json are committed and deliberately excluded from the ADR-2719 attribution cutover, so they must be regenerated when shipped tree content changes. Regenerated after build:lib per the ordering landmine. 19 runtimes each gained exactly the six new plan-phase step files; zero paths removed, which is the absolute failure shape those fixtures exist to catch. Refs #2993 * fix(#2993): restore the launcher preamble in an extracted step and follow moved content in its drift guards Second red run: 26 unique failures, identical on both lanes, in two classes. RUNTIME BUG (runtime-launcher-parity, 7 failures) — chunked-planning-mode.md calls gsd_run but carried no canonical launcher preamble, which is what DEFINES gsd_run(). On any non-Claude runtime that step would fail outright. The preamble is now copied verbatim from the canonical source of truth, gsd-core/workflows/_runtime-launcher.snippet.sh, and the fence dedented to column 0 to match the prd-express-path.md sibling (a list-continuation indent breaks the byte-equal preamble match). prd-express-path.md already had a correct one. This is the same defect #2932 hit when it extracted steps; the parity test caught a real bug, not a stale assertion. DRIFT GUARDS (plan-phase-drift-guard, issue-2762-plan-reviews-chunked, skill-frontmatter-contract) — these assert plan-phase.md contains content this branch moved into step files. Retargeted at where the content now lives, with the asserted property unchanged; the ALL-RUNTIMES label COUNT test now reads host + every step file so the count is preserved across the split rather than reduced. Each retargeted guard was verified to still fail when its step file is stripped, so none was weakened into vacuity. No emitted-drift ack was needed: currentSizes() enumerates gsd-core/workflows/*.md non-recursively, so files under plan-phase/steps/ are never in the size ratchet's scope. Refs #2993 * chore(#2993): backfill changeset pr number to 3019 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -3613,22 +3613,35 @@ describe('init section manifest', () => {
|
||||
// execute-phase-only to per-workflow (`buildSectionManifestField` now wires
|
||||
// into `cmdInitPlanPhase`/`cmdInitNewProject`/`cmdInitNewMilestone`/
|
||||
// `cmdInitQuick`/`cmdInitProgress` too — src/init.cts:740/787/839/885/1810).
|
||||
// The OLD assertion here ("section_manifest must be execute-phase-only")
|
||||
// is stale: plan-phase's field is now PRESENT, but `null` — the shipped
|
||||
// gsd-core/workflows/section-manifest.json still keys only `execute-phase`
|
||||
// (design row C3: a workflow absent from the artifact's `workflows` map
|
||||
// degrades to `null`, never an absent field). `resume` has no dedicated
|
||||
// `cmdInit*` manifest wiring at all (design's withheld-atom survey), so its
|
||||
// field truly remains absent — that half of the guard is unchanged.
|
||||
// #2993 (epic #1671 Phase 6.2) makes the OLD assertion here doubly stale:
|
||||
// plan-phase's field is no longer `null` at all — the shipped
|
||||
// gsd-core/workflows/section-manifest.json now keys `plan-phase` too, with
|
||||
// 6 real sections (row C1). A no-flag invocation excludes every one of
|
||||
// them (none of the 5 governing flags/state were supplied). `resume` has
|
||||
// no dedicated `cmdInit*` manifest wiring at all (design's withheld-atom
|
||||
// survey), so its field truly remains absent — that half of the guard is
|
||||
// unchanged.
|
||||
|
||||
describe('init dispatch: other subcommands unaffected (#2932/#2992 row 59, CRITICAL radius guard)', () => {
|
||||
test('planPhaseEmitsANullManifestNotAnAbsentField (row C3/C16 — absence is not empty)', (t) => {
|
||||
describe('init dispatch: other subcommands unaffected (#2932/#2992/#2993 row 59, CRITICAL radius guard)', () => {
|
||||
test('planPhaseEmitsARealComputedManifestNotNull (#2993 — the shipped artifact now has a plan-phase key)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-e59-');
|
||||
|
||||
const planPhase = parseOkJson(runSectionManifestCli(['init.plan-phase', '1'], dir), 'plan-phase');
|
||||
assert.equal(planPhase.phase_found, true);
|
||||
assert.ok('section_manifest' in planPhase, 'section_manifest field must be present (computed, degraded to null) for plan-phase');
|
||||
assert.equal(planPhase.section_manifest, null, 'plan-phase has no key in the shipped artifact, so it must degrade to null, never {included:[]}');
|
||||
assert.ok('section_manifest' in planPhase, 'section_manifest field must be present for plan-phase');
|
||||
assert.deepStrictEqual(planPhase.section_manifest, {
|
||||
workflow: 'plan-phase',
|
||||
included: [],
|
||||
excluded: [
|
||||
'reviews-prerequisite',
|
||||
'prd-express-gate',
|
||||
'adr-ingest-express-path',
|
||||
'research-only-modifiers',
|
||||
'research-only-early-exit',
|
||||
'chunked-planning-mode',
|
||||
],
|
||||
read: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('resumeNeverEmitsASectionManifestField', (t) => {
|
||||
@@ -3638,6 +3651,221 @@ describe('init section manifest', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── C6 (#2993): plan-phase's section_manifest degrades to null under a
|
||||
// missing/malformed manifest artifact, exactly like execute-phase's own
|
||||
// E57/E58 rows — the "safe superset" contract (all 6 sections read) is
|
||||
// documented per-section in plan-phase.md's own stub prose ("If
|
||||
// `section_manifest` is `null` or `"<id>"` is in its `included` list:
|
||||
// read ... Otherwise skip"), verified for each of the 6 ids below; this
|
||||
// test proves the JS-side half — that plan-phase really does receive
|
||||
// `null`, never a stale/partial selection, under a degraded artifact.
|
||||
|
||||
describe('init plan-phase: section_manifest degrades to null (#2993 row C6)', () => {
|
||||
test('degradesToNullManifestWhenArtifactMissing', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-c6-missing-');
|
||||
const missingPath = path.join(dir, 'does-not-exist-section-manifest.json');
|
||||
const result = runSectionManifestCli(['init.plan-phase', '1'], dir, { GSD_SECTION_MANIFEST: missingPath });
|
||||
assert.equal(result.status, 0, `expected exit 0 despite missing manifest artifact, got ${result.status} (stderr: ${result.stderr})`);
|
||||
assertNoStackTrace(result.stderr, 'plan-phase-manifest-missing');
|
||||
const body = JSON.parse(result.stdout);
|
||||
assert.equal(body.section_manifest, null);
|
||||
});
|
||||
|
||||
test('degradesToNullManifestWhenArtifactMalformed', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-c6-malformed-');
|
||||
const badPath = path.join(dir, 'bad-section-manifest.json');
|
||||
fs.writeFileSync(badPath, '{ this is not valid json');
|
||||
const result = runSectionManifestCli(['init.plan-phase', '1'], dir, { GSD_SECTION_MANIFEST: badPath });
|
||||
assert.equal(result.status, 0, `expected exit 0 despite malformed manifest artifact, got ${result.status} (stderr: ${result.stderr})`);
|
||||
assertNoStackTrace(result.stderr, 'plan-phase-manifest-malformed');
|
||||
const body = JSON.parse(result.stdout);
|
||||
assert.equal(body.section_manifest, null);
|
||||
});
|
||||
|
||||
test('everyPlanPhaseSectionsHostStubDocumentsTheNullSafeSuperset (doc-level half of row C6)', () => {
|
||||
const planPhasePath = path.join(GSD_ROOT, 'workflows', 'plan-phase.md');
|
||||
const content = fs.readFileSync(planPhasePath, 'utf-8');
|
||||
const ids = [
|
||||
'reviews-prerequisite',
|
||||
'prd-express-gate',
|
||||
'adr-ingest-express-path',
|
||||
'research-only-modifiers',
|
||||
'research-only-early-exit',
|
||||
'chunked-planning-mode',
|
||||
];
|
||||
for (const id of ids) {
|
||||
const expectedGate = `If \`section_manifest\` is \`null\` or \`"${id}"\` is in its \`included\` list:`;
|
||||
assert.ok(
|
||||
content.includes(expectedGate),
|
||||
`plan-phase.md must document the null-safe-superset gate for "${id}": expected to find "${expectedGate}"`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── #2993 (epic #1671 Phase 6.2) rows B1-B11: plan-phase facts assembly,
|
||||
// driven through the REAL CLI (prod-shape — matrix section B header) ─────
|
||||
|
||||
describe('init plan-phase: new flag facts assembly (#2993 rows B1-B6)', () => {
|
||||
function runPlanPhase(phaseArgs, cwd, env = {}) {
|
||||
return runSectionManifestCli(['init.plan-phase', ...phaseArgs], cwd, env);
|
||||
}
|
||||
|
||||
test('reviewsFlagPresentIncludesOnlyReviewsPrerequisite (row B1)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b1-present-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--reviews'], dir), 'b1-present');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['reviews-prerequisite']);
|
||||
});
|
||||
|
||||
test('reviewsFlagAbsentExcludesReviewsPrerequisite (row B1)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b1-absent-');
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b1-absent');
|
||||
assert.ok(!body.section_manifest.included.includes('reviews-prerequisite'));
|
||||
assert.ok(body.section_manifest.excluded.includes('reviews-prerequisite'));
|
||||
});
|
||||
|
||||
test('prdFlagWithValuePresentIncludesOnlyPrdExpressGate (row B2)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b2-present-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--prd', 'some-prd.md'], dir), 'b2-present');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['prd-express-gate']);
|
||||
});
|
||||
|
||||
test('prdFlagAbsentExcludesPrdExpressGate (row B2 — value flag absence is null)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b2-absent-');
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b2-absent');
|
||||
assert.ok(!body.section_manifest.included.includes('prd-express-gate'));
|
||||
assert.ok(body.section_manifest.excluded.includes('prd-express-gate'));
|
||||
});
|
||||
|
||||
test('ingestFlagWithValuePresentIncludesOnlyAdrIngestExpressPath (row B3)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b3-present-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--ingest', 'some-adr.md'], dir), 'b3-present');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['adr-ingest-express-path']);
|
||||
});
|
||||
|
||||
test('ingestFlagAbsentExcludesAdrIngestExpressPath (row B3)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b3-absent-');
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b3-absent');
|
||||
assert.ok(!body.section_manifest.included.includes('adr-ingest-express-path'));
|
||||
assert.ok(body.section_manifest.excluded.includes('adr-ingest-express-path'));
|
||||
});
|
||||
|
||||
test('researchPhaseFlagWithValuePresentIncludesBothResearchOnlySections (row B4)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b4-present-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--research-phase', '3'], dir), 'b4-present');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['research-only-modifiers', 'research-only-early-exit']);
|
||||
});
|
||||
|
||||
test('researchPhaseFlagAbsentExcludesBothResearchOnlySections (row B4)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b4-absent-');
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b4-absent');
|
||||
assert.ok(!body.section_manifest.included.includes('research-only-modifiers'));
|
||||
assert.ok(!body.section_manifest.included.includes('research-only-early-exit'));
|
||||
});
|
||||
|
||||
test('emptyPrdValueIsFalsyAndTreatedAsAbsent (row B5 — no spurious inclusion)', (t) => {
|
||||
// `--prd` immediately followed by another flag token (no value token
|
||||
// present at all) resolves to `null` in parseNamedArgs — the "empty
|
||||
// value" shape for a value flag. Combined with a second, independently
|
||||
// gated flag to prove ONLY the second flag's section activates.
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b5-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--prd', '--reviews'], dir), 'b5');
|
||||
assert.ok(!body.section_manifest.included.includes('prd-express-gate'), '--prd with no value must never spuriously include prd-express-gate');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['reviews-prerequisite']);
|
||||
});
|
||||
|
||||
test('chunkedFlagPresentIncludesChunkedPlanningMode (row B6)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b6-');
|
||||
const body = parseOkJson(runPlanPhase(['1', '--chunked'], dir), 'b6');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['chunked-planning-mode']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('init plan-phase: state:chunked-mode disjunction — flag OR config (#2993 rows B7-B11)', () => {
|
||||
function runPlanPhase(phaseArgs, cwd, env = {}) {
|
||||
return runSectionManifestCli(['init.plan-phase', ...phaseArgs], cwd, env);
|
||||
}
|
||||
|
||||
function writeConfig(dir, workflowConfig) {
|
||||
fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify({ workflow: workflowConfig }));
|
||||
}
|
||||
|
||||
test('chunkedFlagAbsentConfigTrueIncludesChunkedPlanningMode (row B7 — config arm of the disjunction)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b7-');
|
||||
writeConfig(dir, { plan_chunked: true });
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b7');
|
||||
assert.deepStrictEqual(body.section_manifest.included, ['chunked-planning-mode']);
|
||||
});
|
||||
|
||||
test('chunkedFlagAbsentConfigFalseExcludesChunkedPlanningMode (row B8)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b8-false-');
|
||||
writeConfig(dir, { plan_chunked: false });
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b8-false');
|
||||
assert.ok(!body.section_manifest.included.includes('chunked-planning-mode'));
|
||||
});
|
||||
|
||||
test('chunkedFlagAbsentConfigAbsentExcludesChunkedPlanningMode (row B8)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b8-absent-');
|
||||
writeConfig(dir, {});
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b8-absent');
|
||||
assert.ok(!body.section_manifest.included.includes('chunked-planning-mode'));
|
||||
});
|
||||
|
||||
test('chunkedFlagAbsentConfigFileMissingExcludesChunkedPlanningMode (row B8)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b8-nofile-');
|
||||
assert.equal(fs.existsSync(path.join(dir, '.planning', 'config.json')), false, 'sanity: no config.json');
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b8-nofile');
|
||||
assert.ok(!body.section_manifest.included.includes('chunked-planning-mode'));
|
||||
});
|
||||
|
||||
test('configStringTrueDegradesToFalse (row B9 — strict === true, never coerced)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b9-');
|
||||
writeConfig(dir, { plan_chunked: 'true' });
|
||||
const body = parseOkJson(runPlanPhase(['1'], dir), 'b9');
|
||||
assert.ok(
|
||||
!body.section_manifest.included.includes('chunked-planning-mode'),
|
||||
'a string "true" config value must never coerce to boolean true',
|
||||
);
|
||||
});
|
||||
|
||||
test('configReadThrowsDegradesToFalseBoundedNeverPropagates (row B10)', (t) => {
|
||||
// `readConfigJsonBoolean` (src/init.cts) is private and reads
|
||||
// `.planning/config.json` via `fs.readFileSync`, so its `catch` clause
|
||||
// cannot be exercised via an in-process fs monkeypatch across the
|
||||
// spawned-CLI process boundary this suite otherwise drives (the whole
|
||||
// matrix section is prod-shape: "drive the real CLI"). A directory at
|
||||
// the config path forces a REAL, deterministic, cross-platform fs
|
||||
// fault (EISDIR-class on every OS `fs.readFileSync` targets) through
|
||||
// the real CLI — never a chmod/permission trick — landing on the same
|
||||
// bounded, non-throwing degrade path a monkeypatched throw would.
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b10-');
|
||||
// sanity: seedSinglePhaseProject never writes a config.json, so this is
|
||||
// the fixture's own natural state, not a removal.
|
||||
assert.equal(fs.existsSync(path.join(dir, '.planning', 'config.json')), false);
|
||||
fs.mkdirSync(path.join(dir, '.planning', 'config.json'));
|
||||
const result = runPlanPhase(['1'], dir);
|
||||
assert.equal(result.status, 0, `expected exit 0 despite an unreadable config.json, got ${result.status} (stderr: ${result.stderr})`);
|
||||
assertNoStackTrace(result.stderr, 'config-read-throws');
|
||||
const body = JSON.parse(result.stdout);
|
||||
assert.ok(!body.section_manifest.included.includes('chunked-planning-mode'), 'a config read fault must degrade chunkedMode to false, never throw or propagate');
|
||||
});
|
||||
|
||||
test('nonObjectConfigJsonDegradesToFalse (row B11)', (t) => {
|
||||
const dir = seedSinglePhaseProject(t, 'gsd-b11-');
|
||||
for (const hostileJson of ['0', '"s"', '[]', 'null', 'true']) {
|
||||
fs.writeFileSync(path.join(dir, '.planning', 'config.json'), hostileJson);
|
||||
const result = runPlanPhase(['1'], dir);
|
||||
assert.equal(result.status, 0, `hostile config JSON ${hostileJson}: expected exit 0 (stderr: ${result.stderr})`);
|
||||
assertNoStackTrace(result.stderr, `hostile-config-json:${hostileJson}`);
|
||||
const body = JSON.parse(result.stdout);
|
||||
assert.ok(
|
||||
!body.section_manifest.included.includes('chunked-planning-mode'),
|
||||
`hostile config JSON ${hostileJson} must degrade chunkedMode to false`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── Row 62: stub <execution_context> @-refs still resolve (ADR-0002) ────
|
||||
|
||||
describe('commands/gsd/execute-phase.md: <execution_context> @-refs resolve (#2932 row 62)', () => {
|
||||
|
||||
Reference in New Issue
Block a user