fix(#1455): resolve project-code-prefixed roadmap headings

* fix: remove hardcoded phase project-code prefix cap Centralize project-code prefix stripping/matching and replace fixed {1,6} caps so long codes (for example MANIFOLD-117) resolve across phase, roadmap parser, roadmap upgrade, and validate flows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on prefix parsing - allow project_code prefixes with digits and underscores while preserving milestone parsing - use shared optional project-code prefix source in phase dir parsing - extend regression coverage for APP1/APP_1 prefixed phases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: resolve review nit in phase-id test comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve project-code-prefixed roadmap headings Make getRoadmapPhaseInternal recover from drifted project-code-prefixed ROADMAP headings while preserving canonical bare-heading preference. Add init.phase-op and parser regressions for #1455 and guard gsd-roadmapper against emitting project_code in headings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: add changeset for project-code phase fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: update roadmapper agent size baseline Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(#1455): tighten project-code prefix regex to [A-Z] start; add boundary test; document source order Resolves blockers from review: - Regex changed from [A-Z_][A-Z0-9_]* to [A-Z][A-Z0-9_]* so leading underscores (_FOO-7, _-7) are never misread as project-code prefixes; adds boundary test asserting both do NOT strip. - Adds comment to roadmapPhaseLookupSources explaining why 3 sources are needed (order-dependent canonical-heading preference). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Solvely-Colin <211764741+Solvely-Colin@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-20 18:45:41 -04:00
parent 3091ce007e
commit 2dedbdd11c
13 changed files with 237 additions and 29 deletions
--- a/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs
+++ b/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs
@@ -103,6 +103,9 @@ describe('Drift item W005 — phaseDirNameRe: 999.X-name dirs must not trigger W
    assert.ok(re.test('01-setup'), 'should accept 01-setup');
    assert.ok(re.test('999-longphase'), 'should accept 999-longphase (3-digit prefix)');
    assert.ok(re.test('999.1-foo'), 'should accept 999.1-foo (sub-phase)');
+    assert.ok(re.test('MANIFOLD-999.1-foo'), 'should accept long project-code prefixes');
+    assert.ok(re.test('APP1-999.1-foo'), 'should accept numeric characters in project-code prefixes');
+    assert.ok(re.test('APP_1-999.1-foo'), 'should accept underscore characters in project-code prefixes');
    assert.ok(!re.test('1-shortname'), 'should reject single-digit prefix');
  });
 });
@@ -192,6 +195,9 @@ describe('Drift item W006-archived — MILESTONE_ARCHIVE_DIR_RE and PHASE_TOKEN_
    assert.strictEqual(re.exec('03B-feature')?.[1], '03B');
    assert.strictEqual(re.exec('999.1-foo')?.[1], '999.1');
    assert.strictEqual(re.exec('CK-64-auth')?.[1], '64');
+    assert.strictEqual(re.exec('MANIFOLD-64-auth')?.[1], '64');
+    assert.strictEqual(re.exec('APP1-64-auth')?.[1], '64');
+    assert.strictEqual(re.exec('APP_1-64-auth')?.[1], '64');
  });
 });

--- a/tests/agent-size-baseline.json
+++ b/tests/agent-size-baseline.json
@@ -26,7 +26,7 @@
  "gsd-planner.md": 49306,
  "gsd-project-researcher.md": 22014,
  "gsd-research-synthesizer.md": 13653,
-  "gsd-roadmapper.md": 21781,
+  "gsd-roadmapper.md": 22183,
  "gsd-security-auditor.md": 6226,
  "gsd-ui-auditor.md": 17159,
  "gsd-ui-checker.md": 11088,
--- a/tests/init.test.cjs
+++ b/tests/init.test.cjs
@@ -981,6 +981,40 @@ describe('cmdInitPhaseOp fallback', () => {
    assert.strictEqual(output.has_plans, false);
  });

+  test('fallback resolves drifted project-code-prefixed roadmap heading by bare number (#1455)', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'ROADMAP.md'),
+      '# Roadmap\n\n### Phase MANIFOLD-117: Prefixed Heading\n**Goal:** Build prefixed phase\n**Plans:** TBD\n'
+    );
+
+    const result = runGsdTools('init phase-op 117', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const output = JSON.parse(result.output);
+    assert.strictEqual(output.phase_found, true);
+    assert.strictEqual(output.phase_dir, null);
+    assert.strictEqual(output.phase_number, '117');
+    assert.strictEqual(output.phase_name, 'Prefixed Heading');
+    assert.strictEqual(output.phase_slug, 'prefixed-heading');
+  });
+
+  test('fallback resolves drifted project-code-prefixed roadmap heading by prefixed ID (#1455)', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'ROADMAP.md'),
+      '# Roadmap\n\n### Phase MANIFOLD-117: Prefixed Heading\n**Goal:** Build prefixed phase\n**Plans:** TBD\n'
+    );
+
+    const result = runGsdTools('init phase-op MANIFOLD-117', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const output = JSON.parse(result.output);
+    assert.strictEqual(output.phase_found, true);
+    assert.strictEqual(output.phase_dir, null);
+    assert.strictEqual(output.phase_number, 'MANIFOLD-117');
+    assert.strictEqual(output.phase_name, 'Prefixed Heading');
+    assert.strictEqual(output.phase_slug, 'prefixed-heading');
+  });
+
  test('prefers current milestone roadmap entry over archived phase with same number', () => {
    const archiveDir = path.join(
      tmpDir,
--- a/tests/phase-id.test.cjs
+++ b/tests/phase-id.test.cjs
@@ -85,6 +85,16 @@ describe('normalizePhaseName', () => {
    assert.strictEqual(phaseId.normalizePhaseName('CK-01'), '01');
    assert.strictEqual(phaseId.normalizePhaseName('PROJ-3'), '03');
    assert.strictEqual(phaseId.normalizePhaseName('AB-12'), '12');
+    assert.strictEqual(phaseId.normalizePhaseName('MANIFOLD-7'), '07');
+    assert.strictEqual(phaseId.normalizePhaseName('APP1-7'), '07');
+    assert.strictEqual(phaseId.normalizePhaseName('APP_1-7'), '07');
+  });
+
+  test('does not strip leading-underscore pseudo-prefix (#1455)', () => {
+    // Valid project_code values must start with [A-Z]; leading underscores
+    // (_FOO-7, _-7) are not valid codes and must not be stripped.
+    assert.strictEqual(phaseId.normalizePhaseName('_FOO-7'), '_FOO-7');
+    assert.strictEqual(phaseId.normalizePhaseName('_-7'), '_-7');
  });

  test('handles letter suffix (preserves original case per #1962)', () => {
@@ -104,10 +114,10 @@ describe('normalizePhaseName', () => {
  });

  test('custom phase IDs: project_code prefix is stripped, then numeric part is normalized', () => {
-    // The regex /^[A-Z]{1,6}-(?=\d)/ matches 'PROJ-' and strips it, leaving '42'
-    // which is then normalized to '42' (no leading zero needed for 2+ digits)
+    // The project-code prefix is stripped, leaving a numeric token that normalizes to '42' (no leading zero needed for 2+ digits).
    assert.strictEqual(phaseId.normalizePhaseName('PROJ-42'), '42');
    assert.strictEqual(phaseId.normalizePhaseName('AUTH-101'), '101');
+    assert.strictEqual(phaseId.normalizePhaseName('MANIFOLD-117'), '117');
  });

  test('custom phase IDs with non-numeric remainder pass through as-is', () => {
@@ -158,6 +168,9 @@ describe('comparePhaseNum', () => {
  test('strips project_code prefix before comparing', () => {
    assert.strictEqual(phaseId.comparePhaseNum('CK-01', '01'), 0);
    assert.ok(phaseId.comparePhaseNum('CK-01', 'CK-02') < 0);
+    assert.strictEqual(phaseId.comparePhaseNum('MANIFOLD-117', '117'), 0);
+    assert.strictEqual(phaseId.comparePhaseNum('APP1-117', '117'), 0);
+    assert.strictEqual(phaseId.comparePhaseNum('APP_1-117', '117'), 0);
  });

  test('handles non-parseable phase IDs via localeCompare fallback', () => {
@@ -183,6 +196,9 @@ describe('extractPhaseToken', () => {
  test('extracts token with project_code prefix', () => {
    assert.strictEqual(phaseId.extractPhaseToken('CK-01-some-phase'), 'CK-01');
    assert.strictEqual(phaseId.extractPhaseToken('PROJ-12-feature'), 'PROJ-12');
+    assert.strictEqual(phaseId.extractPhaseToken('MANIFOLD-117-feature'), 'MANIFOLD-117');
+    assert.strictEqual(phaseId.extractPhaseToken('APP1-117-feature'), 'APP1-117');
+    assert.strictEqual(phaseId.extractPhaseToken('APP_1-117-feature'), 'APP_1-117');
  });

  test('extracts glued letter-prefix phase tokens (#1324)', () => {
@@ -215,6 +231,9 @@ describe('phaseTokenMatches', () => {
  test('matches with project_code prefix stripped', () => {
    assert.ok(phaseId.phaseTokenMatches('CK-01-phase', '01'));
    assert.ok(phaseId.phaseTokenMatches('PROJ-12-feature', '12'));
+    assert.ok(phaseId.phaseTokenMatches('MANIFOLD-117-feature', '117'));
+    assert.ok(phaseId.phaseTokenMatches('APP1-117-feature', '117'));
+    assert.ok(phaseId.phaseTokenMatches('APP_1-117-feature', '117'));
  });

  test('matches glued letter-prefix phase dirs (#1324)', () => {
@@ -281,6 +300,7 @@ describe('phaseMarkdownRegexSource', () => {
    const withPrefix = phaseId.phaseMarkdownRegexSource('CK-01');
    const withoutPrefix = phaseId.phaseMarkdownRegexSource('01');
    assert.strictEqual(withPrefix, withoutPrefix);
+    assert.strictEqual(phaseId.phaseMarkdownRegexSource('MANIFOLD-117'), phaseId.phaseMarkdownRegexSource('117'));
  });

  test('falls back to escaped literal for unparseable input', () => {
@@ -307,6 +327,9 @@ describe('phaseMarkdownRegexSourceExact', () => {
    assert.strictEqual(result, 'PROJ-42');
    // The result is a valid regex source
    assert.doesNotThrow(() => new RegExp(result));
+    assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('MANIFOLD-117'), 'MANIFOLD-117');
+    assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('APP1-117'), 'APP1-117');
+    assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('APP_1-117'), 'APP_1-117');
  });

  test('returns null for non-prefixed IDs', () => {
@@ -350,6 +373,9 @@ describe('getMilestoneFromPhaseId', () => {

  test('strips project_code prefix before parsing', () => {
    assert.strictEqual(phaseId.getMilestoneFromPhaseId('CK-2-01'), 'v2.0');
+    assert.strictEqual(phaseId.getMilestoneFromPhaseId('MANIFOLD-2-01'), 'v2.0');
+    assert.strictEqual(phaseId.getMilestoneFromPhaseId('APP1-2-01'), 'v2.0');
+    assert.strictEqual(phaseId.getMilestoneFromPhaseId('APP_1-2-01'), 'v2.0');
  });

  test('coerces non-string values', () => {
@@ -384,6 +410,9 @@ describe('getPhaseDirFromPhaseId', () => {
  test('strips project_code from phaseId before parsing', () => {
    const result = phaseId.getPhaseDirFromPhaseId('CK-1-2', null, null);
    assert.strictEqual(result, '01-02');
+    assert.strictEqual(phaseId.getPhaseDirFromPhaseId('MANIFOLD-1-2', null, null), '01-02');
+    assert.strictEqual(phaseId.getPhaseDirFromPhaseId('APP1-1-2', null, null), '01-02');
+    assert.strictEqual(phaseId.getPhaseDirFromPhaseId('APP_1-1-2', null, null), '01-02');
  });

  test('handles deep decomposition IDs (M-N-N)', () => {
--- a/tests/roadmap-parser.test.cjs
+++ b/tests/roadmap-parser.test.cjs
@@ -250,6 +250,52 @@ describe('roadmap-parser: getRoadmapPhaseInternal', () => {
    assert.strictEqual(result.goal, 'Set up infrastructure');
  });

+  test('finds drifted project-code-prefixed headings by bare number (#1455)', () => {
+    writeRoadmap(tmpDir, [
+      '## v1.0: Current',
+      '### Phase MANIFOLD-117: Prefixed Heading',
+      '**Goal:** Recover from roadmapper heading drift',
+    ].join('\n'));
+
+    const result = getRoadmapPhaseInternal(tmpDir, '117');
+    assert.ok(result !== null, 'bare number lookup should tolerate a prefixed heading');
+    assert.strictEqual(result.found, true);
+    assert.strictEqual(result.phase_number, '117');
+    assert.strictEqual(result.phase_name, 'Prefixed Heading');
+    assert.strictEqual(result.goal, 'Recover from roadmapper heading drift');
+  });
+
+  test('finds drifted project-code-prefixed headings by prefixed query (#1455)', () => {
+    writeRoadmap(tmpDir, [
+      '## v1.0: Current',
+      '### Phase MANIFOLD-117: Prefixed Heading',
+      '**Goal:** Exact prefixed lookup works on init resolver',
+    ].join('\n'));
+
+    const result = getRoadmapPhaseInternal(tmpDir, 'MANIFOLD-117');
+    assert.ok(result !== null, 'prefixed lookup should resolve the matching prefixed heading');
+    assert.strictEqual(result.found, true);
+    assert.strictEqual(result.phase_number, 'MANIFOLD-117');
+    assert.strictEqual(result.phase_name, 'Prefixed Heading');
+    assert.strictEqual(result.goal, 'Exact prefixed lookup works on init resolver');
+  });
+
+  test('prefers canonical bare heading before prefixed drift fallback (#1455)', () => {
+    writeRoadmap(tmpDir, [
+      '## v1.0: Current',
+      '### Phase MANIFOLD-117: Prefixed Heading',
+      '**Goal:** Drift fallback',
+      '',
+      '### Phase 117: Bare Heading',
+      '**Goal:** Canonical bare',
+    ].join('\n'));
+
+    const result = getRoadmapPhaseInternal(tmpDir, '117');
+    assert.ok(result !== null, 'bare lookup should resolve');
+    assert.strictEqual(result.phase_name, 'Bare Heading');
+    assert.strictEqual(result.goal, 'Canonical bare');
+  });
+
  test('returns null for missing phase number', () => {
    writeRoadmap(tmpDir, '### Phase 1: Foo\n**Goal:** bar\n');
    const result = getRoadmapPhaseInternal(tmpDir, '99');
--- a/tests/roadmapper-granularity.test.cjs
+++ b/tests/roadmapper-granularity.test.cjs
@@ -114,4 +114,19 @@ describe('gsd-roadmapper phase_id_convention support (#1205)', () => {
      'phase_identification block must document that sequential is the default/fallback'
    );
  });
+
+  test('phase headings and checklists must not include project_code (#1455)', () => {
+    const phaseIdentification = extractBlock(content, 'phase_identification');
+    const outputFormats = extractBlock(content, 'output_formats');
+    const combined = `${phaseIdentification}\n${outputFormats}`;
+
+    assert.ok(
+      combined.includes('project_code'),
+      'roadmapper instructions must explicitly mention project_code'
+    );
+    assert.ok(
+      /project_code[\s\S]{0,120}Never include|Do not include `project_code`/.test(combined),
+      'roadmapper must state that project_code is not part of phase headings/checklists'
+    );
+  });
 });