fix(#2978): add structural validation to roadmap validate (#3092)

* test(#2978): roadmap validate must perform structural validation

Failing-first: roadmap validate returns {"warnings":[]} (exit 0) for every
input — empty file, garbage, missing file, truncated frontmatter — because it
performs no structural validation and its one opt-in check (W021 milestone-
prefix) is off by default. Six cases: empty, garbage, missing, truncated
frontmatter, well-formed (no false positive), BOM-prefixed (not corruption).

* fix(#2978): add structural validation to roadmap validate

roadmap validate returned {"warnings":[]} (exit 0) for every input — empty
file, garbage, missing file, truncated frontmatter — because it performed no
structural validation and its one opt-in check (W021 milestone-prefix) is
off by default. A verb named validate that cannot produce a negative result
provides false assurance.

Add four structural checks, each producing a coded warning {code, message}:
- V001: file missing/unreadable (was silent success)
- V002: empty/whitespace-only
- V003: malformed frontmatter (unterminated --- fence; BOM-tolerant per #3057)
- V004: no recognizable phase entries (no ### Phase N: heading)
Keep the existing W021 milestone-prefix check as-is. Exit non-zero via
ExitError(1) when warnings are non-empty, per the documented contract
('exits non-zero on any error or warning'). Well-formed roadmaps (incl.
BOM-prefixed, CRLF) still validate cleanly with warnings: [] and exit 0.

* test(#2978): update W021 tests for non-zero exit on warnings

Two existing W021 tests asserted roadmap validate exits 0 even with warnings
('roadmap validate should exit 0 even with warnings') — that was the bug.
#2978 made validate exit non-zero on any warning (per its documented
contract). Updated both mismatch-case tests to expect success===false and
parse the JSON output from the failure path (stdout is written before the
ExitError throw).

* chore(#2978): add changeset fragment

* chore(#2978): backfill changeset PR number 3092

---------

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

.changeset/silly-moles-rally.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3092
+---
+**`roadmap validate` now performs real structural validation** — it previously returned `{"warnings":[]}` (exit 0) for every input including empty files, garbage text, and missing files, providing false assurance. It now checks file existence/readability, emptiness, frontmatter well-formedness, and the presence of at least one phase entry, exiting non-zero on any warning (per its documented contract). The existing opt-in milestone-prefix consistency check is preserved. (#2978)

src/roadmap-command-router.cts

@@ -21,6 +21,9 @@ const { planningDir } = planningWorkspace;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import configLoaderMod = require('./config-loader.cjs');
 const { loadConfig } = configLoaderMod;
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import cliExitMod = require('./cli-exit.cjs');
+const { ExitError } = cliExitMod;
 
 // ─── Types ────────────────────────────────────────────────────────────────────
 
@@ -143,11 +146,43 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
       'annotate-dependencies': () => roadmap.cmdRoadmapAnnotateDependencies(cwd, args[2], raw),
       'validate': () => {
         const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md');
-        let roadmapContent = '';
+        const warnings: Array<{ code: string; message: string }> = [];
+
+        // #2978: structural validation. A verb named "validate" that cannot
+        // produce a negative result provides false assurance. Before the
+        // opt-in milestone-prefix check, verify the file is structurally a
+        // roadmap at all.
+        let roadmapContent: string;
         try {
           roadmapContent = fs.readFileSync(roadmapPath, 'utf8');
         } catch {
-          // ROADMAP.md missing — return empty warnings
+          // ROADMAP.md missing — not silent success.
+          warnings.push({ code: 'V001', message: 'ROADMAP.md not found or unreadable' });
+          const result = { warnings };
+          process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
+          throw new ExitError(1);
+        }
+
+        // Empty or whitespace-only.
+        if (roadmapContent.trim() === '') {
+          warnings.push({ code: 'V002', message: 'ROADMAP.md is empty' });
+        }
+
+        // Malformed frontmatter — a `---` opener with no matching closer.
+        // Tolerate a leading BOM (#3057) before the fence.
+        const contentAfterBom = roadmapContent.replace(/^\uFEFF/, '');
+        if (contentAfterBom.startsWith('---')) {
+          const closeMatch = contentAfterBom.slice(3).match(/\r?\n---\s*(\r?\n|$)/);
+          if (!closeMatch) {
+            warnings.push({ code: 'V003', message: 'ROADMAP.md frontmatter is malformed (unterminated --- fence)' });
+          }
+        }
+
+        // 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);
+        if (!hasPhaseEntry && !warnings.some((w) => w.code === 'V002')) {
+          warnings.push({ code: 'V004', message: 'ROADMAP.md contains no recognizable phase entries (no "### Phase N:" headings)' });
         }
 
         // W021 only fires when phase_id_convention is explicitly 'milestone-prefixed'.
@@ -173,13 +208,17 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
             }
           }
         }
-        const warnings = (convention === 'milestone-prefixed')
-          ? checkW021(roadmapContent)
-          : [];
+        if (convention === 'milestone-prefixed') {
+          warnings.push(...checkW021(roadmapContent));
+        }
 
         const result = { warnings };
-        if (raw) process.stdout.write(JSON.stringify(result));
-        else process.stdout.write(JSON.stringify(result, null, 2));
+        process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
+        // #2978: exit non-zero on any warning, per the documented contract
+        // ("exits non-zero on any error or warning").
+        if (warnings.length > 0) {
+          throw new ExitError(1);
+        }
       },
       'upgrade': () => {
         const dryRun = !args.includes('--apply');

tests/milestone-prefixed-convention.test.cjs

@@ -89,7 +89,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
     ]);
 
     const result = runGsdTools(['roadmap', 'validate'], tmpDir);
-    assert.ok(result.success, `roadmap validate should exit 0 even with warnings: ${result.error}`);
+    // #2978: validate now exits non-zero on any warning (per its documented contract).
+    assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
 
     const out = JSON.parse(result.output);
     assert.ok(Array.isArray(out.warnings), 'output.warnings should be an array');
@@ -219,7 +220,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
     ]);
 
     const result = runGsdTools(['roadmap', 'validate'], tmpDir);
-    assert.ok(result.success, `roadmap validate failed: ${result.error}`);
+    // #2978: validate now exits non-zero on any warning (per its documented contract).
+    assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
 
     const out = JSON.parse(result.output);
     const w021 = (out.warnings || []).filter(w => w.code === 'W021');

tests/roadmap.test.cjs

@@ -3612,3 +3612,79 @@ describe('bug #1103 — annotate-dependencies preserves newline before Plans: he
 });
   });
 }
+
+// ─────────────────────────────────────────────────────────────────────────────
+// bug #2978: roadmap validate returns {"warnings":[]} for every input
+// ─────────────────────────────────────────────────────────────────────────────
+
+describe('bug #2978: roadmap validate performs structural validation', () => {
+  test('empty (zero-byte) ROADMAP.md → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-empty-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'empty file must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(Array.isArray(payload.warnings) && payload.warnings.length > 0,
+        `empty file must produce non-empty warnings; got: ${JSON.stringify(payload)}`);
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('garbage/non-roadmap text → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-garbage-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'not a roadmap at all');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'garbage must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'garbage must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('missing ROADMAP.md → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-missing-');
+    try {
+      // createTempProject creates .planning/phases but no ROADMAP.md — don't write one
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'missing file must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'missing file must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('truncated frontmatter (unterminated ---) → non-empty warnings', () => {
+    const tmpDir = createTempProject('gsd-2978-trunc-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '---\nmilestone: v1.0\n### Phase 1: Setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'truncated frontmatter must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'truncated frontmatter must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('well-formed roadmap → warnings: [], exit 0 (no false positive)', () => {
+    const tmpDir = createTempProject('gsd-2978-good-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '# Roadmap\n\n## v1.0\n\n### Phase 1: Foundation\n**Goal:** setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.ok(result.success, `well-formed roadmap must exit 0; got: ${result.error}`);
+      const payload = JSON.parse(result.output);
+      assert.deepStrictEqual(payload.warnings, [], 'well-formed roadmap must have no warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('BOM-prefixed well-formed roadmap → warnings: [], exit 0 (not corruption)', () => {
+    const tmpDir = createTempProject('gsd-2978-bom-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '\uFEFF# Roadmap\n\n### Phase 1: Foundation\n**Goal:** setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.ok(result.success, `BOM-prefixed roadmap must exit 0; got: ${result.error}`);
+      const payload = JSON.parse(result.output);
+      assert.deepStrictEqual(payload.warnings, [], 'BOM is not corruption');
+    } finally { cleanup(tmpDir); }
+  });
+});