From 71e00d426ec4617c79ee2e1ca09bf747b7d6c58f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 19 Aug 2026 21:55:18 -0400 Subject: [PATCH] fix(#3639): dir-aware sentinel recognition for the disk-side guards (#3698) * test(#3639): pin bracket sentinel recognition in disk-side guards * fix(#3639): dir-aware sentinel recognition for the disk-side guards * chore(#3639): add changeset * fix(#3639): disclose the digit-continuation residual, join phases-clear, load-bearing over-suppression guard * test(#3639): match the token form W007 reports * chore(#3639): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/witty-ibex-frolic.md | 5 ++ src/health-diagnostic-rules/consistency.cts | 10 +++- .../roadmap-disk-consistency.cts | 9 ++- src/milestone.cts | 8 ++- src/phase-id.cts | 34 ++++++++++++ tests/adr-612-bracket-grammar.test.cjs | 40 ++++++++++++++ .../roadmap-disk-consistency.test.cjs | 55 +++++++++++++++++++ 7 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 .changeset/witty-ibex-frolic.md diff --git a/.changeset/witty-ibex-frolic.md b/.changeset/witty-ibex-frolic.md new file mode 100644 index 000000000..ea6cfe76c --- /dev/null +++ b/.changeset/witty-ibex-frolic.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3698 +--- +**Bracket-convention icebox and pre-milestone directories no longer produce spurious health warnings** — the disk-side guards could not see bracket sentinel-ness (it lives in the milestone portion of `GSD.999-07-icebox`), so icebox dirs false-fired as roadmap orphans. A dir-aware sentinel recognizer now excludes them exactly like their legacy twins. (#3639) diff --git a/src/health-diagnostic-rules/consistency.cts b/src/health-diagnostic-rules/consistency.cts index fd879d5e6..07e0adc42 100644 --- a/src/health-diagnostic-rules/consistency.cts +++ b/src/health-diagnostic-rules/consistency.cts @@ -43,7 +43,7 @@ type Rule = healthDiagnosticMod.Rule; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('../phase-id.cjs'); -const { isSentinelPhaseId } = phaseIdMod; +const { isSentinelPhaseDir } = phaseIdMod; // ─── C001 — gap in disk phase numbering (integer sequence) ──────────────── // (verify.cts:1504-1519) @@ -57,8 +57,12 @@ function checkC001(snapshot: PlanningSnapshot): Diagnostic[] { const integerPhases = snapshot.allPhaseDirNames.value // #3225: exclude sentinel phase ids (999.x/0.x) — never part of the - // sequential numbering, mirrors verify.cts:1510 verbatim. - .filter((p) => !p.includes('.') && !isSentinelPhaseId(p)) + // sequential numbering, mirrors verify.cts:1510 verbatim. The dot filter + // already drops every code-prefixed (bracket) name before the sentinel + // test, so the dir-aware recognizer below is defense-in-depth for the + // dotless legacy forms — kept so this guard cannot regress if the dot + // filter is ever loosened (#3639). + .filter((p) => !p.includes('.') && !isSentinelPhaseDir(p)) .map((p) => parseInt(p, 10)) .filter((n) => !Number.isNaN(n)) .sort((a, b) => a - b); diff --git a/src/health-diagnostic-rules/roadmap-disk-consistency.cts b/src/health-diagnostic-rules/roadmap-disk-consistency.cts index 5b92de5bd..e109bd6c3 100644 --- a/src/health-diagnostic-rules/roadmap-disk-consistency.cts +++ b/src/health-diagnostic-rules/roadmap-disk-consistency.cts @@ -113,7 +113,7 @@ const { SCOPE } = planningScopeMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('../phase-id.cjs'); -const { matchPhaseDirs, normalizePhaseName, extractPhaseToken, isSentinelPhaseId } = phaseIdMod; +const { matchPhaseDirs, normalizePhaseName, extractPhaseToken, isSentinelPhaseId, isSentinelPhaseDir } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import validateMod = require('../validate.cjs'); @@ -248,8 +248,11 @@ function checkW007(snapshot: PlanningSnapshot): Diagnostic[] { // `verify.cts:1373-1397`) — same token, relocated read, not reinvented. const token = extractPhaseToken(dirName); // #3225: a sentinel dir on disk (999-interim, 0-drafts) is defined as - // never-on-roadmap; it must not trigger W007. - if (isSentinelPhaseId(token)) continue; + // never-on-roadmap; it must not trigger W007. #3639: judged on the DIR + // NAME via the dir-aware recognizer — the extracted token is + // milestone-stripped, so a bracket sentinel (GSD.999-07-icebox) was + // invisible to the id predicate and false-fired as an orphan. + if (isSentinelPhaseDir(dirName)) continue; if (claimedDirs.has(dirName)) continue; diagnostics.push({ code: 'W007', diff --git a/src/milestone.cts b/src/milestone.cts index 2b97f3cd2..67bb700cc 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -30,7 +30,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, matchPhaseDirs, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, matchPhaseDirs, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId, isSentinelPhaseDir } = phaseIdMod; import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); @@ -1198,7 +1198,11 @@ function cmdPhasesClear(cwd: string, raw: boolean, args: string[]): void { // divergence meant a `0-*` directory `roadmap analyze` preserves as a // sentinel was DELETED here. Routed through the canonical predicate so // every reader of "is this a sentinel phase" agrees by construction. - const dirs = entries.filter((e) => e.isDirectory() && !isSentinelPhaseId(e.name)); + // #3639: the DIR-AWARE recognizer — the convention-less id predicate + // never saw bracket sentinel dirs (GSD.999-07-icebox), so they were + // counted for deletion here while the disk guards (post-#3639) preserve + // them; the destructive path must not be the one blind reader left. + const dirs = entries.filter((e) => e.isDirectory() && !isSentinelPhaseDir(e.name)); if (dirs.length > 0 && !confirm) { error( diff --git a/src/phase-id.cts b/src/phase-id.cts index 083133662..b695d070c 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -414,6 +414,39 @@ function isSentinelPhaseId(phaseId: unknown, convention?: string): boolean { return SENTINEL_RANGES.includes(parseInt(legacy[1], 10)); } +/** + * Disk-side sentinel recognizer (#3639): is this on-disk PHASE DIRECTORY a + * sentinel (never-on-roadmap by convention)? + * + * The disk-side guards (C001 gap numbering, W007 orphan dirs) see raw + * directory names and do not know the repo's naming convention — and neither + * convention-blind route could recognize a bracket sentinel: `isSentinelPhaseId` + * without the convention argument reads only the legacy leading int, while + * `extractPhaseToken(dirName)` (convention-aware or not) strips the MILESTONE + * and returns the bare phase token — bracket sentinel-ness lives in the + * milestone portion (`GSD.999-07-icebox` is icebox because of the 999, not + * the 07). This helper reads the milestone directly off the dir name. + * + * The bracket branch requires the FULL bracket dir shape — code prefix, dot, + * milestone digits, hyphen, PHASE DIGITS — so a #1324 letter-prefixed real + * dir with a LETTER slug (`P0.0-foundation`) never matches it (ADR-2121 + * indistinguishability, same gate as extractPhaseToken below). DISCLOSED + * RESIDUAL (#3639 review): the #1324 family also has digit continuations + * (`P0.0-1-foundation`, `P0.3-2` are real shapes per derivePhaseTokenSegments), + * and `{code}.{0|999}-{digit}...` is string-indistinguishable from a bracket + * sentinel dir — no convention-free discriminator exists (ADR-2121). Such a + * dir reads as sentinel here, which at the disk-guard call sites suppresses + * a warning (conservative for a linter) rather than deleting data. The + * digit-continuation family with NON-sentinel first decimals (`P0.3-2`) + * reads milestone 3 — ordinary — exactly as the convention-gated id + * predicate does. Everything else falls to the legacy leading-int rule. + */ +function isSentinelPhaseDir(dirName: string): boolean { + const bracketDir = dirName.match(/^[A-Z][A-Z0-9_]*\.(\d+)-\d/); // milestone digits + hyphen + phase DIGITS + if (bracketDir) return SENTINEL_RANGES.includes(parseInt(bracketDir[1], 10)); + return isSentinelPhaseId(dirName); +} + /** * Render a regex source fragment matching a phase number against ROADMAP/STATE * prose regardless of zero-padding on either side. @@ -1198,6 +1231,7 @@ export = { toDir, SENTINEL_RANGES, isSentinelPhaseId, + isSentinelPhaseDir, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, comparePhaseNum, diff --git a/tests/adr-612-bracket-grammar.test.cjs b/tests/adr-612-bracket-grammar.test.cjs index 04d1b3ff6..5eb8f1774 100644 --- a/tests/adr-612-bracket-grammar.test.cjs +++ b/tests/adr-612-bracket-grammar.test.cjs @@ -260,6 +260,46 @@ describe('bracket grammar: sentinel guard', () => { assert.strictEqual(core.isSentinelPhaseId('feature-branch'), false); }); + test('#3639: isSentinelPhaseDir reads the MILESTONE off a bracket dir name, convention-less', () => { + // The disk-side guards see raw DIRECTORY names and do not know the repo's + // convention. isSentinelPhaseDir recognizes the full bracket dir shape + // (code prefix, dot, MILESTONE digits, hyphen, PHASE digits) so sentinel + // milestone dirs are recognized without a convention argument. + assert.strictEqual(core.isSentinelPhaseDir('GSD.999-07-icebox'), true); + assert.strictEqual(core.isSentinelPhaseDir('GSD.00-01-backlog'), true); + assert.strictEqual(core.isSentinelPhaseDir('GSD.000-01-padded'), true); + // Ordinary bracket milestones: not sentinels. + assert.strictEqual(core.isSentinelPhaseDir('GSD.03-01-auth'), false); + assert.strictEqual(core.isSentinelPhaseDir('GSD.998-01-near-icebox'), false); + // Legacy sentinel dirs keep their recognition through the same helper. + assert.strictEqual(core.isSentinelPhaseDir('999-interim'), true); + assert.strictEqual(core.isSentinelPhaseDir('0-drafts'), true); + assert.strictEqual(core.isSentinelPhaseDir('M-999-07-icebox'), true); + }); + + test('#3639: isSentinelPhaseDir never re-reads #1324 letter-prefixed REAL dirs as bracket sentinels', () => { + // ADR-2121 indistinguishability: the bracket branch requires PHASE DIGITS + // after the milestone hyphen — `P0.0-foundation` (letters after the + // hyphen) never matches it, and the legacy branch already returns false + // for these (pinned in the suite above). + assert.strictEqual(core.isSentinelPhaseDir('P0.0-foundation'), false); + assert.strictEqual(core.isSentinelPhaseDir('P0.999-x'), false); + assert.strictEqual(core.isSentinelPhaseDir('P0.3-2'), false); + }); + + test('#3639 disclosed residual: #1324 digit-continuation dirs with a SENTINEL-valued first decimal read as bracket sentinels', () => { + // ADR-2121: `{code}.{0|999}-{digit}...` is string-indistinguishable from + // a bracket sentinel dir — no convention-free discriminator exists. The + // dir-aware recognizer takes the sentinel reading (conservative at the + // disk-guard sites: a suppressed warning, never deleted data), while the + // NON-sentinel first decimals stay ordinary. Pinned so a change to either + // reading is a deliberate act, not drift. + assert.strictEqual(core.isSentinelPhaseDir('P0.0-1-foundation'), true); + assert.strictEqual(core.isSentinelPhaseDir('P0.999-2-x'), true); + assert.strictEqual(core.isSentinelPhaseDir('P0.3-2'), false); + assert.strictEqual(core.isSentinelPhaseDir('P0.1-2-x'), false); + }); + test('the bracket sentinel path is OFF by default: a convention-less #1324 dir is not a sentinel', () => { // `P0.0-foundation` is a real #1324 letter-prefixed phase, NOT milestone-0 // sentinel. Auto-detecting the `P0`/`.0` prefix would be a false positive diff --git a/tests/health-diagnostic-rules/roadmap-disk-consistency.test.cjs b/tests/health-diagnostic-rules/roadmap-disk-consistency.test.cjs index 932b78b56..081b10779 100644 --- a/tests/health-diagnostic-rules/roadmap-disk-consistency.test.cjs +++ b/tests/health-diagnostic-rules/roadmap-disk-consistency.test.cjs @@ -224,6 +224,61 @@ describe('W007 — disk dir with no ROADMAP entry', () => { assert.deepEqual(ruleFor('W007').check(snapshot), []); }); + test('#3639: does not fire for a BRACKET icebox directory (GSD.999-07-icebox) even with no roadmap entry', (t) => { + // Bracket sentinel-ness lives in the MILESTONE portion ({CODE}.999-{PP}); + // the convention-less predicate read only the phase token and never saw it. + const cwd = createTempDir('gsd-3639-w007-bracket-ice-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['## GSD.03 Current 🚧', '', '### Phase 3.1: Foo'].join('\n')); + makePhaseDir(cwd, 'GSD.03-01-foo'); + makePhaseDir(cwd, 'GSD.999-07-icebox'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W007').check(snapshot); + assert.deepEqual( + diagnostics.filter((d) => d.message.includes('999-07-icebox')), + [], + `a bracket icebox dir is never-on-roadmap by convention — no orphan warning. Got: ${JSON.stringify(diagnostics)}`, + ); + }); + + test('#3639: does not fire for a BRACKET pre-milestone directory (GSD.00-01-backlog)', (t) => { + const cwd = createTempDir('gsd-3639-w007-bracket-pre-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['## GSD.03 Current 🚧', '', '### Phase 3.1: Foo'].join('\n')); + makePhaseDir(cwd, 'GSD.03-01-foo'); + makePhaseDir(cwd, 'GSD.00-01-backlog'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W007').check(snapshot); + assert.deepEqual( + diagnostics.filter((d) => d.message.includes('00-01-backlog')), + [], + `a bracket pre-milestone dir is never-on-roadmap by convention. Got: ${JSON.stringify(diagnostics)}`, + ); + }); + + test('#3639 over-suppression guard: an UNCLAIMED ordinary #1324 digit-continuation dir still fires W007', (t) => { + // The disclosed residual (#3639 review): #1324 dirs with a SENTINEL-valued + // first decimal (`P0.0-1-x`) are indistinguishable from bracket sentinels + // and read as sentinel (suppressed). This row pins that the family is not + // BLANKET-suppressed: a NON-sentinel first decimal (`P0.1-2-x`), unclaimed, + // must still fire — otherwise the recognizer would silently swallow every + // letter-prefixed real dir. + const cwd = createTempDir('gsd-3639-w007-over-sup-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['## v1.0 Current 🚧', '', '### Phase 1: Foo'].join('\n')); + makePhaseDir(cwd, '01-foo'); + makePhaseDir(cwd, 'P0.1-2-x'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W007').check(snapshot); + assert.ok( + diagnostics.some((d) => d.message.includes('P0.1-2')), + `an unclaimed ordinary #1324 digit-continuation dir must still fire W007 (the message names the extracted token, not the full dir). Got: ${JSON.stringify(diagnostics)}`, + ); + }); + test('boundary: zero declared phases and zero phase directories produces zero findings', (t) => { const cwd = createTempDir('gsd-3309-w007-4-'); t.after(() => cleanup(cwd));