fix(#1159): phase complete ignores historical/deferred requirement metadata (#1197)

* fix(#1159): phase complete ignores historical/deferred requirement metadata

Defect A: `phase complete` emitted a false "has unresolved gaps" warning
when a VERIFICATION.md file's frontmatter contained `status: passed` but
the body contained `previous_status: gaps_found`. The full-text regex
`/status: gaps_found/` matched the substring inside `previous_status:
gaps_found`, producing a spurious warning. Fixed by reading only the
frontmatter `status` key via `extractFrontmatter` in both `phase.cts`
(phase complete warning check) and `commands.cts` (determinePhaseStatus).

Defect B: requirement IDs under explicitly deferred/backlog/future/v2
section headings in REQUIREMENTS.md were flagged as missing from the
Traceability table. Fixed by splitting the body into markdown sections,
detecting deferred-intent headings via a keyword regex, and skipping
those sections when collecting IDs to check against the table.

Both fixes include boundary tests (false positive suppressed; genuine
gap/missing warnings still fire) in 4-phase-complete-cjs-regression.test.cjs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#1159): address adversarial-review findings (subheading + case sensitivity)

- Defect B: Replace section-split approach with line-by-line depth-tracking
  to correctly propagate deferred status to sub-headings and ignore headings
  inside fenced code blocks. Previously `## Future Backlog` / `### Sub` would
  leak sub-heading IDs (split created a new non-deferred section per heading).
- Defect A/commands: restore case-insensitive status matching via toLowerCase()
  to match the prior /status:\s*passed/i regex semantics.
- Add test #1159-B-4 covering the nested-subheading case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(changeset): backfill PR number for #1159 fix

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-14 00:00:37 -04:00
committed by GitHub
parent 5fa4dcd78c
commit be132445aa
4 changed files with 450 additions and 10 deletions

View File

@@ -0,0 +1,6 @@
---
type: Fixed
pr: 1197
---
**`phase complete` no longer emits false warnings from historical verification metadata or deferred requirement IDs** — two distinct false-positive warning bugs: (A) the verification-status check used a full-text regex that matched `previous_status: gaps_found` in the file body, triggering an "unresolved gaps" warning even when the current frontmatter `status: passed`; the check now reads only the frontmatter `status` key via `extractFrontmatter`. (B) requirement IDs under explicitly deferred/backlog/future/v2 section headings in `REQUIREMENTS.md` were flagged as missing from the Traceability table; the check now skips any section whose heading matches those terms. (#1197)

View File

@@ -110,9 +110,17 @@ function determinePhaseStatus(plans: number, summaries: number, phaseDir: string
const verificationFile = files.find(f => f === 'VERIFICATION.md' || f.endsWith('-VERIFICATION.md'));
if (verificationFile) {
const content = platformReadSync(path.join(phaseDir, verificationFile)) || '';
if (/status:\s*passed/i.test(content)) return 'Complete';
if (/status:\s*human_needed/i.test(content)) return 'Needs Review';
if (/status:\s*gaps_found/i.test(content)) return 'Executed';
// #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false
// matches from historical body metadata such as `previous_status: gaps_found`.
// Full-text regexes like /status:\s*gaps_found/ match the substring inside
// `previous_status: gaps_found`, producing incorrect phase status labels.
const fm = extractFrontmatter(content) as Record<string, unknown>;
// Normalise to lower-case to preserve the prior case-insensitive behaviour
// while reading only the frontmatter `status` key (not the full body text).
const fmStatus = typeof fm['status'] === 'string' ? fm['status'].trim().toLowerCase() : '';
if (fmStatus === 'passed') return 'Complete';
if (fmStatus === 'human_needed') return 'Needs Review';
if (fmStatus === 'gaps_found') return 'Executed';
// Verification exists but unrecognized status — treat as executed
return 'Executed';
}

View File

@@ -1373,8 +1373,16 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
(f) => f.includes('-VERIFICATION') && f.endsWith('.md'),
)) {
const content = fs.readFileSync(path.join(phaseFullDir, file), 'utf-8');
if (/status: human_needed/.test(content)) warnings.push(`${file}: needs human verification`);
if (/status: gaps_found/.test(content)) warnings.push(`${file}: has unresolved gaps`);
// #1159 (Defect A): read ONLY the frontmatter `status` key to avoid false positives
// from historical metadata in the file body (e.g. `previous_status: gaps_found`).
// A full-text regex like /status: gaps_found/ matches the substring inside
// `previous_status: gaps_found`, producing spurious warnings even when the
// current frontmatter status is `passed`.
const verFm = extractFrontmatter(content) as Record<string, unknown>;
// Normalise to lower-case so `status: Passed` (title-case) is not missed.
const verStatus = typeof verFm['status'] === 'string' ? verFm['status'].trim().toLowerCase() : '';
if (verStatus === 'human_needed') warnings.push(`${file}: needs human verification`);
if (verStatus === 'gaps_found') warnings.push(`${file}: has unresolved gaps`);
}
} catch {
/* intentionally empty */
@@ -1495,12 +1503,58 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
}
}
// #1159 (Defect B): collect requirement IDs only from ACTIVE sections.
// Requirements under headings whose text contains "deferred", "backlog",
// "future", or "v2" (case-insensitive) are explicitly out of current scope
// and must not be flagged as missing from the Traceability table.
//
// Strategy: walk lines, track heading depth, and toggle a "deferred" flag
// when a heading matching the pattern is encountered. A sub-heading (higher
// depth) that is ITSELF in a deferred parent remains deferred unless it
// opens a same-or-shallower heading that does NOT match the pattern.
// Lines inside fenced code blocks (``` or ~~~) are treated as content, not
// headings, to avoid false deferred-section detection from code examples.
const DEFERRED_HEADING_RE = /\b(?:deferred|backlog|future|v\d+)\b/i;
const bodyReqIds: string[] = [];
const bodyReqPattern = /\*\*([A-Z][A-Z0-9]*-\d+)\*\*/g;
let bodyMatch: RegExpExecArray | null;
while ((bodyMatch = bodyReqPattern.exec(reqContent)) !== null) {
const id = bodyMatch[1];
if (!bodyReqIds.includes(id)) bodyReqIds.push(id);
// deferredDepth: the heading level that opened the current deferred block,
// or 0 when we are in an active section.
let deferredDepth = 0;
let inFence = false;
for (const line of reqContent.split(/\r?\n/)) {
// Track fenced code blocks (``` or ~~~).
if (/^\s*(?:```|~~~)/.test(line)) {
inFence = !inFence;
continue;
}
if (inFence) continue; // ignore content inside a code fence
const headingM = line.match(/^(#{1,6})\s+(.*)/);
if (headingM) {
const depth = headingM[1].length;
const text = headingM[2];
if (deferredDepth > 0 && depth > deferredDepth) {
// Sub-heading inside a deferred block: stays deferred regardless of name.
continue;
}
// Heading at same level or shallower than current deferred opener,
// or no active deferred block yet.
if (DEFERRED_HEADING_RE.test(text)) {
deferredDepth = depth; // enter a deferred block
} else {
deferredDepth = 0; // back in an active section
}
continue;
}
if (deferredDepth > 0) continue; // skip content in deferred sections
// Collect bold REQ-ID patterns from active-section lines.
const reqPat = /\*\*([A-Z][A-Z0-9]*-\d+)\*\*/g;
let bodyMatch: RegExpExecArray | null;
while ((bodyMatch = reqPat.exec(line)) !== null) {
const id = bodyMatch[1];
if (!bodyReqIds.includes(id)) bodyReqIds.push(id);
}
}
const traceabilityHeadingMatch = reqContent.match(/^#{1,6}\s+Traceability\b/im);

View File

@@ -717,3 +717,375 @@ describe('issue #4 (CJS): cmdPhaseComplete — progress percent clamp', () => {
);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Regressions: issue #1159 — Defect A
// VERIFICATION.md with `previous_status: gaps_found` in the body but
// `status: passed` in frontmatter must NOT emit a "has unresolved gaps" warning.
// The bug: /status: gaps_found/.test(fullContent) matches the substring inside
// `previous_status: gaps_found`, causing a false positive.
// ─────────────────────────────────────────────────────────────────────────────
/**
* Build a minimal project fixture with a VERIFICATION.md file whose
* frontmatter status is `verFmStatus` and whose body contains `previous_status: gaps_found`.
* Phase 01 has a plan+summary; Phase 02 exists for next-phase detection.
*/
function createVerificationFixture(verFmStatus) {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1159-verif-'));
const planDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planDir, 'phases');
const phase01Dir = path.join(phasesDir, '01-foundation');
fs.mkdirSync(phase01Dir, { recursive: true });
fs.mkdirSync(path.join(phasesDir, '02-api'), { recursive: true });
fs.writeFileSync(path.join(planDir, 'ROADMAP.md'), [
'# Roadmap',
'',
'- [ ] Phase 01: Foundation',
'- [ ] Phase 02: API',
'',
'### Phase 01: Foundation',
'**Goal:** Build the foundation',
'**Plans:** 1 plans',
'',
'## Progress',
'',
'| Phase | Plans Complete | Status | Completed |',
'|-------|----------------|--------|-----------|',
'| 01. Foundation | 0/1 | Not started | - |',
'| 02. API | 0/1 | Not started | - |',
'',
].join('\n'));
fs.writeFileSync(path.join(planDir, 'STATE.md'), [
'# State',
'',
'**Current Phase:** 01',
'**Current Phase Name:** Foundation',
'**Status:** In progress',
'**Completed Phases:** 0',
'**Total Phases:** 2',
'**Progress:** 0%',
'',
].join('\n'));
// No REQUIREMENTS.md intentionally (not needed for this defect check)
fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan 1\nDo the work.\n');
fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary 1\nDone.\n');
// The VERIFICATION.md has the CURRENT status in frontmatter but historical
// `previous_status: gaps_found` in the body — this is the false-positive trigger.
fs.writeFileSync(path.join(phase01Dir, '01-VERIFICATION.md'), [
'---',
`status: ${verFmStatus}`,
'phase: "01"',
'---',
'',
'# Verification',
'',
'<!-- Historical context from previous run -->',
'previous_status: gaps_found',
'',
'## Summary',
'All checks passed on re-run.',
'',
].join('\n'));
return tmpDir;
}
describe('issue #1159 (Defect A): VERIFICATION.md historical metadata must not trigger gap warning', () => {
let tmpDir;
afterEach(() => {
cleanup(tmpDir);
});
test(
'#1159-A-1: status:passed + previous_status:gaps_found in body → NO "has unresolved gaps" warning',
() => {
tmpDir = createVerificationFixture('passed');
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
// The output is JSON; parse and check warnings array
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const gapWarnings = warnings.filter((w) => /unresolved gaps/i.test(w));
assert.equal(
gapWarnings.length,
0,
`#1159-A-1 FAILED: got false gap warning(s) when frontmatter status=passed.\n` +
`Warnings: ${JSON.stringify(warnings)}\n` +
`(The regex /status: gaps_found/ matched 'previous_status: gaps_found' in the body.)`,
);
},
);
test(
'#1159-A-2 (boundary): status:gaps_found in frontmatter → DOES emit "has unresolved gaps" warning',
() => {
tmpDir = createVerificationFixture('gaps_found');
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const gapWarnings = warnings.filter((w) => /unresolved gaps/i.test(w));
assert.ok(
gapWarnings.length > 0,
`#1159-A-2 FAILED: expected a gap warning when frontmatter status=gaps_found but got none.\n` +
`Warnings: ${JSON.stringify(warnings)}`,
);
},
);
test(
'#1159-A-3 (boundary): status:human_needed in frontmatter → DOES emit "needs human verification" warning',
() => {
tmpDir = createVerificationFixture('human_needed');
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const humanWarnings = warnings.filter((w) => /human verification/i.test(w));
assert.ok(
humanWarnings.length > 0,
`#1159-A-3 FAILED: expected human-verification warning when frontmatter status=human_needed.\n` +
`Warnings: ${JSON.stringify(warnings)}`,
);
},
);
});
// ─────────────────────────────────────────────────────────────────────────────
// Regressions: issue #1159 — Defect B
// Requirement IDs (e.g. FILE-001) that appear under explicitly deferred/future/v2
// sections in REQUIREMENTS.md must NOT be flagged as "missing from Traceability".
// ─────────────────────────────────────────────────────────────────────────────
/**
* Build a minimal project fixture with a REQUIREMENTS.md that has:
* - An active requirement ACTIVE-001 that IS in the Traceability table
* - A deferred requirement DEFER-001 under a "Deferred v2 Requirements" heading
* that is NOT in the Traceability table (correctly out of scope)
* - Optionally a truly-missing active requirement MISSING-001 (not in table)
*/
function createDeferredReqFixture({ includeMissingActive = false } = {}) {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1159-deferred-'));
const planDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planDir, 'phases');
const phase01Dir = path.join(phasesDir, '01-foundation');
fs.mkdirSync(phase01Dir, { recursive: true });
fs.mkdirSync(path.join(phasesDir, '02-api'), { recursive: true });
const missingActiveLines = includeMissingActive
? ['', '- **MISSING-001** Active req not in traceability table.']
: [];
fs.writeFileSync(path.join(planDir, 'REQUIREMENTS.md'), [
'# Requirements',
'',
'## Functional Requirements',
'',
'- **ACTIVE-001** Core feature must work.',
...missingActiveLines,
'',
'## Deferred v2 Requirements',
'',
'- **DEFER-001** Nice-to-have for v2, explicitly out of scope.',
'',
'## Future Backlog',
'',
'- **FUTURE-001** Consider for next major release.',
'',
'## Traceability',
'',
'| Requirement | Phase | Status |',
'|-------------|-------|--------|',
'| ACTIVE-001 | Phase 01 | Pending |',
'',
].join('\n'));
// Roadmap references ACTIVE-001 for phase 01
fs.writeFileSync(path.join(planDir, 'ROADMAP.md'), [
'# Roadmap',
'',
'- [ ] Phase 01: Foundation',
'- [ ] Phase 02: API',
'',
'### Phase 01: Foundation',
'**Goal:** Build the foundation',
'**Requirements:** ACTIVE-001',
'**Plans:** 1 plans',
'',
'## Progress',
'',
'| Phase | Plans Complete | Status | Completed |',
'|-------|----------------|--------|-----------|',
'| 01. Foundation | 0/1 | Not started | - |',
'| 02. API | 0/1 | Not started | - |',
'',
].join('\n'));
fs.writeFileSync(path.join(planDir, 'STATE.md'), [
'# State',
'',
'**Current Phase:** 01',
'**Current Phase Name:** Foundation',
'**Status:** In progress',
'**Completed Phases:** 0',
'**Total Phases:** 2',
'**Progress:** 0%',
'',
].join('\n'));
fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan 1\nDo the work.\n');
fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary 1\nDone.\n');
return tmpDir;
}
describe('issue #1159 (Defect B): deferred/future requirement IDs must not trigger traceability warning', () => {
let tmpDir;
afterEach(() => {
cleanup(tmpDir);
});
test(
'#1159-B-1: IDs under "Deferred v2 Requirements" and "Future Backlog" sections → NO traceability warning',
() => {
tmpDir = createDeferredReqFixture({ includeMissingActive: false });
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const traceWarnings = warnings.filter((w) => /Traceability/i.test(w));
assert.equal(
traceWarnings.length,
0,
`#1159-B-1 FAILED: got false traceability warning(s) for deferred/future IDs.\n` +
`Warnings: ${JSON.stringify(warnings)}\n` +
`(DEFER-001 and FUTURE-001 are under deferred/future sections and must be ignored.)`,
);
},
);
test(
'#1159-B-2 (boundary): truly-missing ACTIVE ID (not in table, not in deferred section) → DOES warn',
() => {
tmpDir = createDeferredReqFixture({ includeMissingActive: true });
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const traceWarnings = warnings.filter((w) => /Traceability/i.test(w));
assert.ok(
traceWarnings.length > 0,
`#1159-B-2 FAILED: expected traceability warning for MISSING-001 (active, not in table) but got none.\n` +
`Warnings: ${JSON.stringify(warnings)}`,
);
// Verify MISSING-001 is specifically mentioned
const mentionsMissing = traceWarnings.some((w) => w.includes('MISSING-001'));
assert.ok(
mentionsMissing,
`#1159-B-2 FAILED: warning exists but MISSING-001 not mentioned.\n` +
`Traceability warnings: ${JSON.stringify(traceWarnings)}`,
);
},
);
test(
'#1159-B-3 (boundary): deferred IDs must not contaminate warning even when active ID is also missing',
() => {
tmpDir = createDeferredReqFixture({ includeMissingActive: true });
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const traceWarnings = warnings.filter((w) => /Traceability/i.test(w));
// DEFER-001 and FUTURE-001 must NOT appear in the traceability warnings
const mentionsDefer = traceWarnings.some((w) => w.includes('DEFER-001') || w.includes('FUTURE-001'));
assert.ok(
!mentionsDefer,
`#1159-B-3 FAILED: deferred IDs (DEFER-001/FUTURE-001) appeared in traceability warning.\n` +
`Traceability warnings: ${JSON.stringify(traceWarnings)}`,
);
},
);
test(
'#1159-B-4 (subheading): IDs under sub-headings of a deferred section are also suppressed',
() => {
// Codex adversarial finding: splitting on EVERY heading failed to propagate
// deferred status to sub-headings (e.g. "## Future Backlog" → "### Sub").
// The fix uses heading-depth tracking so sub-headings inherit deferred state.
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1159-subhead-'));
const planDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planDir, 'phases');
const phase01Dir = path.join(phasesDir, '01-foundation');
fs.mkdirSync(phase01Dir, { recursive: true });
fs.mkdirSync(path.join(phasesDir, '02-api'), { recursive: true });
fs.writeFileSync(path.join(planDir, 'REQUIREMENTS.md'), [
'# Requirements',
'',
'## Functional Requirements',
'',
'- **ACTIVE-001** Core feature.',
'',
'## Future Backlog',
'',
'### Sub-category A',
'',
'- **SUB-001** This is under a sub-heading of a deferred section.',
'',
'## Traceability',
'',
'| Requirement | Phase | Status |',
'|-------------|-------|--------|',
'| ACTIVE-001 | Phase 01 | Pending |',
'',
].join('\n'));
fs.writeFileSync(path.join(planDir, 'ROADMAP.md'), [
'# Roadmap',
'',
'- [ ] Phase 01: Foundation',
'- [ ] Phase 02: API',
'',
'### Phase 01: Foundation',
'**Goal:** Build the foundation',
'**Requirements:** ACTIVE-001',
'**Plans:** 1 plans',
'',
'## Progress',
'',
'| Phase | Plans Complete | Status | Completed |',
'|-------|----------------|--------|-----------|',
'| 01. Foundation | 0/1 | Not started | - |',
'| 02. API | 0/1 | Not started | - |',
'',
].join('\n'));
fs.writeFileSync(path.join(planDir, 'STATE.md'), [
'# State',
'',
'**Current Phase:** 01',
'**Completed Phases:** 0',
'**Total Phases:** 2',
'**Progress:** 0%',
'',
].join('\n'));
fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan 1\n');
fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary 1\n');
const { output } = runGsdTools(['phase', 'complete', '1'], tmpDir);
const parsed = JSON.parse(output);
const warnings = parsed.warnings || [];
const traceWarnings = warnings.filter((w) => /Traceability/i.test(w));
const mentionsSub = traceWarnings.some((w) => w.includes('SUB-001'));
assert.ok(
!mentionsSub,
`#1159-B-4 FAILED: SUB-001 (under sub-heading of deferred section) appeared in warning.\n` +
`Traceability warnings: ${JSON.stringify(traceWarnings)}`,
);
},
);
});