fix(#3641): make v005/v004 see bracket-convention phase entries (#3723)

* test(#3641): failing-first bracket-window validate rows

* fix(#3641): thread phase convention into hasphaseentries for v004/v005

* test(#3641): review rows - digit-anchor, decoy, probe parity, t.after

* fix(#3641): digit-anchor bracket entry token; thread probe scope axis

* fix(#3641): align frontmatter bound with probe; changeset

* chore(#3641): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>

src/roadmap-command-router.cts

@@ -26,7 +26,7 @@ import cliExitMod = require('./cli-exit.cjs');
 const { ExitError } = cliExitMod;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import roadmapParserMod = require('./roadmap-parser.cjs');
-const { extractCurrentMilestoneScoped } = roadmapParserMod;
+const { extractCurrentMilestoneScoped, hasPhaseEntries } = roadmapParserMod;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import planningScopeMod = require('./planning-scope.cjs');
 const { SCOPE } = planningScopeMod;
@@ -188,11 +188,45 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
           }
         }
 
-        // No recognizable phase structure — at least one `### Phase N:` heading.
-        // Mirrors the phase-heading pattern used across roadmap-parser.cts.
-        const hasPhaseEntry = /^#{2,4}\s*Phase\s+\S/im.test(roadmapContent);
+        // #3641: resolve phase_id_convention ONCE, ahead of every consumer in
+        // this validate pass — V004's entry check and V005's scope classifier
+        // below, and the W021 milestone-prefix check after them.
+        // Authoritative source: .planning/config.json (set by the upgrade
+        // command). Fallback: ROADMAP.md frontmatter (for projects that set
+        // the field there directly).
+        let convention: string | undefined | null;
+        try {
+          const cfg = loadConfig(cwd);
+          convention = cfg['phase_id_convention'] as string | undefined | null;
+        } catch {
+          convention = undefined;
+        }
+        if (convention === undefined || convention === null) {
+          // Fallback: read from ROADMAP.md frontmatter. Bounded to match
+          // cmdRoadmapMilestoneScope's copy exactly (#3641 review NEW-1: an
+          // unbounded capture here read past 4KB frontmatters the probe's
+          // bounded copy could not, diverging validate from the probe).
+          const fmMatch = roadmapContent.match(/^---\r?\n([\s\S]{0,4000}?)\r?\n---/);
+          if (fmMatch) {
+            const kvMatch = fmMatch[1].match(/^phase_id_convention:\s*(.*)$/m);
+            if (kvMatch) {
+              const val = kvMatch[1].trim();
+              if (val !== 'null' && val !== '') {
+                convention = val.replace(/^["']|["']$/g, '');
+              }
+            }
+          }
+        }
+
+        // No recognizable phase structure. #3641: routed through the
+        // roadmap-parser owner (`hasPhaseEntries` — headings, #2199 bullets,
+        // #3577 table rows) instead of a private inline heading regex, so the
+        // document-level check and the V005 scope axis below can never
+        // disagree about what a phase entry is — and so a bracket-convention
+        // project's `### [GSD.04] 01:` entries are entries here too.
+        const hasPhaseEntry = hasPhaseEntries(roadmapContent, convention);
         if (!hasPhaseEntry && !warnings.some((w) => w.code === 'V002')) {
-          warnings.push({ code: 'V004', message: 'ROADMAP.md contains no recognizable phase entries (no "### Phase N:" headings)' });
+          warnings.push({ code: 'V004', message: 'ROADMAP.md contains no recognizable phase entries (no phase headings, bullet entries, or table rows)' });
         }
 
         // #3263: a whole-document phase check (V004 above) is satisfied by a
@@ -207,13 +241,15 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
         // an unscoped/unreadable window is a different failure mode with a
         // different remediation, deliberately not warned here.
         try {
-          const scoped = extractCurrentMilestoneScoped(contentAfterBom, cwd);
+          // #3641: thread the resolved convention so the scope axis's
+          // hasPhaseEntries comparison recognizes bracket phase entries.
+          const scoped = extractCurrentMilestoneScoped(contentAfterBom, cwd, undefined, convention);
           if (scoped.scope === SCOPE.TRUNCATED) {
             warnings.push({
               code: 'V005',
               message:
                 'Active milestone window is truncated: phase entries exist in ROADMAP.md but are excluded from the ' +
-                'active milestone\'s resolved window (check for a heading between the milestone heading and its "### Phase N:" sections)',
+                'active milestone\'s resolved window (check for a heading between the milestone heading and its phase-entry sections)',
             });
           }
         } catch {
@@ -221,29 +257,8 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
           // structural warnings already collected above.
         }
 
-        // W021 only fires when phase_id_convention is explicitly 'milestone-prefixed'.
-        // Authoritative source: .planning/config.json (set by the upgrade command).
-        // Fallback: ROADMAP.md frontmatter (for projects that set the field there directly).
-        let convention: string | undefined | null;
-        try {
-          const cfg = loadConfig(cwd);
-          convention = cfg['phase_id_convention'] as string | undefined | null;
-        } catch {
-          convention = undefined;
-        }
-        if (convention === undefined || convention === null) {
-          // Fallback: read from ROADMAP.md frontmatter
-          const fmMatch = roadmapContent.match(/^---\r?\n([\s\S]+?)\r?\n---/);
-          if (fmMatch) {
-            const kvMatch = fmMatch[1].match(/^phase_id_convention:\s*(.*)$/m);
-            if (kvMatch) {
-              const val = kvMatch[1].trim();
-              if (val !== 'null' && val !== '') {
-                convention = val.replace(/^["']|["']$/g, '');
-              }
-            }
-          }
-        }
+        // W021 only fires when phase_id_convention is explicitly
+        // 'milestone-prefixed' — the same hoisted resolution above (#3641).
         if (convention === 'milestone-prefixed') {
           warnings.push(...checkW021(roadmapContent));
         }

src/roadmap-parser.cts

@@ -32,6 +32,10 @@ const {
   roadmapPhaseLookupSources,
   extractPhaseToken,
   isSentinelPhaseId,
+  // #3641: the single-owner heading-intro and digit-token grammar sources —
+  // see BRACKET_PHASE_ENTRY_HEADING_RE below.
+  PHASE_HEADING_PREFIX_SRC,
+  PHASE_NUMBER_TOKEN_SOURCE,
 } = phaseIdModule;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import planningWorkspace = require('./planning-workspace.cjs');
@@ -412,12 +416,40 @@ function sliceMilestoneWindow(content: string, version: string): string | null {
  * exist" — a window containing only sentinel phases still reached the
  * region and must read COMPLETE, not TRUNCATED.
  */
-function hasPhaseEntries(markdown: string): boolean {
+// #3641: the bracket-convention phase-ENTRY heading shape — ADR-612 Decision
+// 1's own discriminator: a phase heading is a bracket followed by a
+// DIGIT-then-colon (`### [GSD.04] 01: Name`); a bracket followed by a NAME
+// is a milestone heading and must never count. Every fragment interpolates a
+// single-owner export from phase-id.cts — the heading intro
+// (PHASE_HEADING_PREFIX_SRC: a `[...]` bracket optionally followed by a
+// `Phase ` label, or a bare `Phase ` label), the digit-bearing token
+// (PHASE_NUMBER_TOKEN_SOURCE, which also covers the dotted sub-phase form
+// `[GSD.02] 05.03:`), and the optional pre-colon tag
+// (OPTIONAL_PHASE_TAG_SOURCE) — never a re-typed grammar. Tested IN
+// ADDITION to the legacy pattern below, so bracket mode is a strict
+// superset: mid-migration legacy-labeled headings (`Phase AUTH-101:`-style
+// custom ids included) keep their existing recognition. Review finding: an
+// earlier single-alternative form with a `[\\w]` token admitted
+// `[bracket] Word:` shapes — a colon-bearing MILESTONE heading inside the
+// window read as an entry (defeating V005 outright for that spelling) and a
+// decoy `### [GSD.04] Notes:` outside the window manufactured a false V005
+// while suppressing the correct V004. The digit anchor forecloses both.
+const BRACKET_PHASE_ENTRY_HEADING_RE = new RegExp(
+  `^${PHASE_HEADING_PREFIX_SRC}${PHASE_NUMBER_TOKEN_SOURCE}${OPTIONAL_PHASE_TAG_SOURCE}\\s*:`,
+  'i',
+);
+
+function hasPhaseEntries(markdown: string, phaseIdConvention?: string | null): boolean {
   // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
+  // #3641: the widened grammar engages ONLY when the resolved convention is
+  // 'bracket' — a project that has not opted in runs the legacy pattern
+  // alone, byte-identically.
   const phaseHeadingPattern = /^(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/i;
+  const bracketMode = phaseIdConvention === 'bracket';
   for (const h of tokenizeHeadings(markdown)) {
     if (h.level < 2 || h.level > 4) continue;
     if (phaseHeadingPattern.test(h.text)) return true;
+    if (bracketMode && BRACKET_PHASE_ENTRY_HEADING_RE.test(h.text)) return true;
   }
   // #3184 review finding: the bullet fallback must be fence-aware too, or a
   // FENCED markdown EXAMPLE of the `- [ ] **Phase N — Name**` syntax (e.g. a
@@ -598,8 +630,18 @@ function classifyMilestoneWindow(input: {
  * symbols, 20 direct callers) means its signature and return type do not
  * change. This is the real owner; `extractCurrentMilestone` becomes a
  * one-line wrapper returning `.value` so every existing caller is untouched.
+ *
+ * @param phaseIdConvention - #3641: the RESOLVED `phase_id_convention`
+ *   config value, threaded to `hasPhaseEntries` so the scope axis's row-8
+ *   comparison recognizes bracket-convention phase entries
+ *   (`### [GSD.04] 01: Name`). Optional: absent, or any value other than
+ *   `'bracket'`, compiles the legacy entry grammar byte-identically — the
+ *   widening engages only for a project that resolved the convention
+ *   explicitly. `extractCurrentMilestone`'s wrapper deliberately does NOT
+ *   expose it (its 20 callers are not the scope-axis consumers; V005's
+ *   router site and `getMilestonePhaseFilter` resolve and thread it).
  */
-function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: string | null): { value: string; scope: Scope } {
+function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: string | null, phaseIdConvention?: string | null): { value: string; scope: Scope } {
   if (!cwd) {
     // Row 1: a deliberate unscoped read (no cwd supplied) is a real answer —
     // the caller asked for no scoping, so whole-document is COMPLETE.
@@ -640,13 +682,13 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin
         versionResolved,
         hasVersionedMilestones: versionedMilestonesPresent,
         headingFound: false,
-        windowHasPhaseEntries: hasPhaseEntries(value),
-        documentHasPhaseEntries: hasPhaseEntries(value),
+        windowHasPhaseEntries: hasPhaseEntries(value, phaseIdConvention),
+        documentHasPhaseEntries: hasPhaseEntries(value, phaseIdConvention),
       }),
     };
   }
 
-  const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(content));
+  const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(content), phaseIdConvention);
   const summaryPattern = new RegExp(
     `<summary[^>]*>([^<]*${escapeRegex(version)}[^<]*)<\\/summary>`,
     'i'
@@ -680,7 +722,7 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin
             versionResolved,
             hasVersionedMilestones: versionedMilestonesPresent,
             headingFound: true,
-            windowHasPhaseEntries: hasPhaseEntries(value),
+            windowHasPhaseEntries: hasPhaseEntries(value, phaseIdConvention),
             documentHasPhaseEntries,
           }),
         };
@@ -694,7 +736,7 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin
         versionResolved,
         hasVersionedMilestones: versionedMilestonesPresent,
         headingFound: false,
-        windowHasPhaseEntries: hasPhaseEntries(value),
+        windowHasPhaseEntries: hasPhaseEntries(value, phaseIdConvention),
         documentHasPhaseEntries,
       }),
     };
@@ -787,7 +829,7 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin
       versionResolved,
       hasVersionedMilestones: versionedMilestonesPresent,
       headingFound: true,
-      windowHasPhaseEntries: hasPhaseEntries(value),
+      windowHasPhaseEntries: hasPhaseEntries(value, phaseIdConvention),
       documentHasPhaseEntries,
     }),
   };
@@ -1380,7 +1422,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
     const roadmapPath = path.join(planningDir(cwd, ws), 'ROADMAP.md');
     const roadmapContent = platformReadSync(roadmapPath);
     if (roadmapContent === null) throw new Error('missing');
-    const scopedResult = extractCurrentMilestoneScoped(roadmapContent, cwd, ws);
+    const scopedResult = extractCurrentMilestoneScoped(roadmapContent, cwd, ws, phaseIdConvention);
     let roadmap = scopedResult.value;
     // Default: the filter's window IS extractCurrentMilestoneScoped's own
     // window (reused verbatim, not re-derived — ADR-3180 Decision 4c).
@@ -1417,7 +1459,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
       // via `storedMilestone`).
       const sliced = sliceMilestoneWindow(roadmapContent, versionOverride);
 
-      const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(roadmapContent));
+      const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(roadmapContent), phaseIdConvention);
 
       if (sliced !== null) {
         versionScoped = true;
@@ -1441,7 +1483,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
         versionResolved: true,
         hasVersionedMilestones: hasVersionedMilestonesGlobal,
         headingFound: sliced !== null,
-        windowHasPhaseEntries: hasPhaseEntries(roadmap),
+        windowHasPhaseEntries: hasPhaseEntries(roadmap, phaseIdConvention),
         documentHasPhaseEntries,
       });
     }
@@ -1662,4 +1704,8 @@ export = {
   scanMilestonePhaseIds,
   collectTablePhaseRows,
   findMilestoneScopeHeadingLines,
+  // #3641: the scope axis's phase-ENTRY predicate, exported so roadmap
+  // validate's V004 document-level check routes through the same single
+  // owner (and its convention gate) instead of a private inline copy.
+  hasPhaseEntries,
 };

src/roadmap.cts

@@ -34,6 +34,11 @@ import { platformWriteSync } from './shell-command-projection.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import planningWorkspace = require('./planning-workspace.cjs');
 const { planningPaths, withPlanningLock, findContextMdIn } = planningWorkspace;
+// #3641: milestone-scope's convention resolution reads the project config
+// (no cycle — config-loader does not import this module).
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import configLoaderForScope = require('./config-loader.cjs');
+const { loadConfig: loadConfigForScope } = configLoaderForScope;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import scanPhasePlans = require('./plan-scan.cjs');
 // eslint-disable-next-line @typescript-eslint/no-require-imports
@@ -707,7 +712,35 @@ function cmdRoadmapMilestoneScope(cwd: string, raw: boolean): void {
   }
 
   const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
-  const { value: window, scope } = extractCurrentMilestoneScoped(rawContent, cwd);
+  // #3641: resolve phase_id_convention and thread it into the scope axis, so
+  // this probe and `roadmap validate`'s V005 answer the SAME question the
+  // SAME way for a bracket-convention project — a window the classifier
+  // calls TRUNCATED in validate must never read COMPLETE here (the #3262
+  // capture/compare guard consumes this scope). Resolution mirrors the
+  // validate router's: .planning/config.json first, ROADMAP.md frontmatter
+  // as fallback.
+  let phaseIdConvention: string | undefined | null;
+  try {
+    const cfg = loadConfigForScope(cwd);
+    phaseIdConvention = cfg['phase_id_convention'] as string | undefined | null;
+  } catch {
+    phaseIdConvention = undefined;
+  }
+  if (phaseIdConvention === undefined || phaseIdConvention === null) {
+    // Bounded per local/no-unbounded-quantifier (#2128): frontmatter is a
+    // short header block — 4KB is orders of magnitude beyond any real one.
+    const fmMatch = rawContent.match(/^---\r?\n([\s\S]{0,4000}?)\r?\n---/);
+    if (fmMatch) {
+      const kvMatch = fmMatch[1].match(/^phase_id_convention:\s*(.*)$/m);
+      if (kvMatch) {
+        const val = kvMatch[1].trim();
+        if (val !== 'null' && val !== '') {
+          phaseIdConvention = val.replace(/^["']|["']$/g, '');
+        }
+      }
+    }
+  }
+  const { value: window, scope } = extractCurrentMilestoneScoped(rawContent, cwd, undefined, phaseIdConvention);
   // Document order (Set insertion order) — deterministic for a given document.
   const phases = [...scanMilestonePhaseIds(window)];
   output({ scope, phases, phase_count: phases.length }, raw, undefined);