diff --git a/.changeset/merry-lynx-climb.md b/.changeset/merry-lynx-climb.md new file mode 100644 index 000000000..444afe060 --- /dev/null +++ b/.changeset/merry-lynx-climb.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4826 +--- +**Roadmap queries read past hard-wrapped Goal/Requirements fields** — the roadmapper soft-wraps long fields at ~85 chars, but five single-line field regexes truncated every wrapped Goal/Requirements at the first line: plan-phase's phase_req_ids silently dropped continuation-line REQ IDs (escaping the Requirements Coverage Gate), get-phase/analyze cut the goal mid-sentence, and phase complete left later REQs Pending with empty warnings. A shared multiline extractor now reads wrapped fields to the next field label. (#4731) diff --git a/src/init.cts b/src/init.cts index 890eed065..902be1e2d 100644 --- a/src/init.cts +++ b/src/init.cts @@ -141,7 +141,6 @@ const { resolveCapabilityRuntimeState } = capabilityStateMod; void stripShippedMilestones; // Accept all bold/colon variants of the Requirements header (#2769) -const REQUIREMENTS_HEADER_RE = /^\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]*)$/m; // #2056/#2104: isForeignPrefixedPhaseQuery is imported from phase-id.cts // (the canonical predicate). parsePhasePrefix is no longer needed locally. @@ -939,9 +938,14 @@ function cmdInitExecutePhase( has_reviews: false, }; }); - const reqMatch = (roadmapPhase?.['section'] as string | undefined)?.match(REQUIREMENTS_HEADER_RE); - const reqExtracted = reqMatch - ? reqMatch[1].replace(/[\[\]]/g, '').split(',').map((s) => s.trim()).filter(Boolean).join(', ') + // #4731: multiline-aware — the Requirements field may hard-wrap, so the + // value is extracted past the line break before the ID scan. + const phaseSection = roadmapPhase?.['section'] as string | undefined; + const reqLine = phaseSection + ? roadmapParser.extractPhaseFieldMultiline(phaseSection, 'Requirements') + : null; + const reqExtracted = reqLine + ? reqLine.replace(/[\[\]]/g, '').split(',').map((s) => s.trim()).filter(Boolean).join(', ') : null; const phase_req_ids = reqExtracted && reqExtracted !== 'TBD' ? reqExtracted : null; @@ -1109,9 +1113,14 @@ function cmdInitPlanPhase( has_reviews: false, }; }); - const reqMatch = (roadmapPhase?.['section'] as string | undefined)?.match(REQUIREMENTS_HEADER_RE); - const reqExtracted = reqMatch - ? reqMatch[1].replace(/[\[\]]/g, '').split(',').map((s) => s.trim()).filter(Boolean).join(', ') + // #4731: multiline-aware — the Requirements field may hard-wrap, so the + // value is extracted past the line break before the ID scan. + const phaseSection = roadmapPhase?.['section'] as string | undefined; + const reqLine = phaseSection + ? roadmapParser.extractPhaseFieldMultiline(phaseSection, 'Requirements') + : null; + const reqExtracted = reqLine + ? reqLine.replace(/[\[\]]/g, '').split(',').map((s) => s.trim()).filter(Boolean).join(', ') : null; const phase_req_ids = reqExtracted && reqExtracted !== 'TBD' ? reqExtracted : null; @@ -2841,8 +2850,7 @@ function cmdInitManager(cwd: string, raw: boolean): void { : content.length; const section = content.slice(sectionStart, sectionEnd); - const goalMatch = section.match(/\*\*Goal(?::\*\*|\*\*:)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; + const goal = roadmapParser.extractPhaseFieldMultiline(section, 'Goal'); const dependsMatch = section.match(/\*\*Depends on(?::\*\*|\*\*:)\s*([^\n]+)/i); const depends_on = dependsMatch ? dependsMatch[1].trim() : null; diff --git a/src/phase.cts b/src/phase.cts index 0325fadc0..81661588a 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -3896,16 +3896,22 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { ); const sectionText = phaseSectionMatch ? phaseSectionMatch[1] : ''; - const reqMatch = sectionText.match( - /\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]+)/i, - ); + // #4731: multiline-aware — hard-wrapped Requirements read past the + // line break before the ID scan. The shared extractor also stops at + // headings and table rows, so a Requirements field followed by the + // Traceability table cannot bleed other phases' REQ-IDs into the + // citation scan (isolated-review MEDIUM on the inline lookahead, + // whose lazy capture swallowed everything to section end). + const reqLine = sectionText + ? roadmapParserMod.extractPhaseFieldMultiline(sectionText, 'Requirements') + : null; const originalReqContent = fs.readFileSync(reqPath, 'utf-8'); let reqContent = originalReqContent; // #2316: `citedReqIds` — the REQ-IDs ROADMAP's own **Requirements:** // line for this phase actually cites — is hoisted out of the - // `if (reqMatch)` block (previously scoped only inside it) so the + // `if (reqLine)` block (previously scoped only inside it) so the // ghost-ID cross-check below (~#2316-1) can consult it. `TBD` is the // literal placeholder `phase.add`/`-batch`/`-insert` seed // (`**Requirements**: TBD`, src/phase.cts:833,920,1078) — never a @@ -3918,18 +3924,18 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // `else`, discarding this fact silently instead of surfacing it. const traceabilityWriteMisses: string[] = []; - if (reqMatch) { + if (reqLine) { // #2334 HIGH 3 + #3697: selection and under-selection detection both // live in `analyzeRequirementsLine` (module scope, above), extracted in // round 3 so the parser is directly testable — a closure in here is // reachable only by spawning the CLI, which no fast-check property test // can do. `citedReqIds` is byte-identical to the expression that stood // here; nothing about what phase-complete MARKS has changed. - const reqLineAnalysis = analyzeRequirementsLine(reqMatch[1]); + const reqLineAnalysis = analyzeRequirementsLine(reqLine); citedReqIds = reqLineAnalysis.citedReqIds; const reqLineWarning = formatRequirementsLineWarning( phaseNum, - reqMatch[1], + reqLine, reqLineAnalysis, ); if (reqLineWarning) { diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 77d7d3268..b0ae73e1e 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -2255,5 +2255,45 @@ export = { // #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. + extractPhaseFieldMultiline, hasPhaseEntries, }; + + +/** + * #4731 — extract a bold-labeled phase field (Goal, Requirements, ...) reading + * past hard-wrapped continuation lines. The roadmapper soft-wraps long fields + * at ~85 chars, so a single-line capture silently truncated every wrapped + * Goal/Requirements. Stops at: another `**Field**` label line, a blank line, + * or any heading. Wrapped lines are trimmed and joined with single spaces; a + * single-line field is returned unchanged (trimmed). Returns null when the + * label is absent. + */ +function extractPhaseFieldMultiline(section: string, label: string): string | null { + // #2769 label shapes: `**X:**`, `**X**:`, and the spaced `**X** :`. + const labelRe = new RegExp( + '\\*\\*' + label + '(?::\\*\\*|\\*\\*\\s*:?)\\s*([^\\n]+)', + 'i', + ); + const match = section.match(labelRe); + if (!match) return null; + const startIdx = match.index ?? 0; + const after = section.slice(startIdx + match[0].length); + const firstLine = match[1].trim(); + const contLines = []; + const lines = after.split('\n'); + for (let li = 0; li < lines.length; li++) { + const raw = lines[li]; + // The split's first element is the remainder of the captured first line's + // own newline — an empty leading element is the line break, not a blank + // continuation line. + if (li === 0 && !raw.trim()) continue; + if (!raw.trim()) break; + if (/^\s*\*\*[A-Z][A-Za-z ]*:?(\*\*)?:?\s/.test(raw)) break; + if (/^\s*#{1,4}\s/.test(raw)) break; + if (/^\s*\|/.test(raw)) break; + contLines.push(raw.trim()); + } + return [firstLine, ...contLines].join(' ').trim() || null; +} + diff --git a/src/roadmap.cts b/src/roadmap.cts index b302f0d85..81bc48c28 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -231,9 +231,9 @@ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: s const section = content.slice(headerIndex, sectionEnd).trim(); - // Extract goal if present (supports both **Goal:** and **Goal**: formats) - const goalMatch = section.match(/\*\*Goal(?::\*\*|\*\*:)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; + // Extract goal if present (supports both **Goal:** and **Goal**: formats). + // #4731: multiline-aware — hard-wrapped Goals read past the line break. + const goal = roadmapParserModule.extractPhaseFieldMultiline(section, 'Goal'); // Mode: vertical-MVP slice mode flag. Lowercased + trimmed for canonical // comparison; unrecognized values are preserved verbatim for forward-compat. @@ -509,8 +509,7 @@ function collectAnalyzePhases( const sectionEnd = nextHeader ? sectionStart + nextHeader.index! : content.length; const section = content.slice(sectionStart, sectionEnd); - const goalMatch = section.match(/\*\*Goal(?::\*\*|\*\*:)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; + const goal = roadmapParserModule.extractPhaseFieldMultiline(section, 'Goal'); const modeMatch = section.match(/\*\*Mode(?::\*\*|\*\*:)\s*([^\n]+)/i); const mode = modeMatch ? modeMatch[1].trim().toLowerCase() : null; @@ -1645,6 +1644,7 @@ function cmdRoadmapAnnotateDependencies(cwd: string, phaseNum: string | null | u }, raw, updated ? `annotated ${waves.length} wave(s), ${crossCuttingTruths.length} constraint(s)` : 'skipped (already annotated or no plan list)'); } + export = { cmdRoadmapGetPhase, getRoadmapPhaseWithFallback, diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 8b934f065..0a549d581 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -5275,3 +5275,68 @@ describe('#4040 partial-init completeness fields', () => { 'new-project gate must resume a partial bootstrap instead of erroring'); }); }); + +// ── #4731 — hard-wrapped Goal/Requirements fields read past the line break ─── +// The roadmapper soft-wraps long fields at ~85 chars; the five single-line +// field regexes truncated every wrapped Goal/Requirements at the first line: +// plan-phase's phase_req_ids silently dropped the IDs on continuation lines +// (silently escaping the Requirements Coverage Gate) and get-phase/analyze +// cut the goal mid-sentence. +describe('init plan-phase — wrapped Goal/Requirements fields (#4731)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempDir('gsd-4731-'); + fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), + ['# Requirements', '', '- [ ] **REQ-01**: thing', '- [ ] **REQ-11**: thing'].join('\n'), + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '## Phases', + '', + '- [ ] **Phase 1: Demo** - Goal', + '', + '## Phase Details', + '', + '### Phase 1: Demo', + '', + '**Goal:** Deliver a small demo feature that exercises the planning pipeline end to end with a', + 'goal sentence long enough to wrap onto a second line', + '**Requirements**: REQ-01, REQ-02, REQ-03, REQ-04, REQ-05, REQ-06, REQ-07, REQ-08, REQ-09,', + 'REQ-10, REQ-11', + '**Plans**: 1 plans', + '', + ].join('\n'), + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-demo'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '---\nphase: 01-demo\nplan: 01\n---\n# Plan'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '---\nphase: 01-demo\nplan: 01\n---\n# Summary'); + }); + + afterEach(() => cleanup(tmpDir)); + + test('wrapped Requirements yield all eleven IDs (#4731)', () => { + const result = runGsdTools('init plan-phase 1 --pick phase_req_ids', tmpDir); + assert.equal( + result.output.trim(), + 'REQ-01, REQ-02, REQ-03, REQ-04, REQ-05, REQ-06, REQ-07, REQ-08, REQ-09, REQ-10, REQ-11', + 'continuation-line REQ-10/REQ-11 must not be silently dropped', + ); + }); + + test('wrapped Goal is returned in full (#4731)', () => { + const result = runGsdTools('query roadmap.get-phase 1 --pick goal', tmpDir); + assert.equal( + result.output.trim(), + 'Deliver a small demo feature that exercises the planning pipeline end to end with a goal sentence long enough to wrap onto a second line', + 'the goal must read past the hard wrap', + ); + }); +}); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index ad1014c16..bcb6da0c7 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -12385,6 +12385,140 @@ describe('issue #2334: ghost-REQ-ID classification must probe write surfaces, no ); }); +// ── #4731 review follow-up — the phase-complete citation scan must stop at the ─ +// section's own table. The inline lookahead the first cut used stopped only at +// the next `**Bold**` label or end-of-section, so a Requirements field followed +// by a table bled every ID-shaped cell after it into the citation scan — and +// phase complete then ticked other phases' checkboxes and flipped their +// Traceability rows. The shared extractor stops at headings, table rows, and +// blank lines; these pins hold that boundary. +describe('#4731 review follow-up: Requirements citation scan stops at the section table', () => { + function build4731TableBleedFixture() { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4731-table-bleed-')); + const planDir = path.join(tmpDir, '.planning'); + const phase1Dir = path.join(planDir, 'phases', '01-preset'); + const phase2Dir = path.join(planDir, 'phases', '02-next'); + fs.mkdirSync(phase1Dir, { recursive: true }); + fs.mkdirSync(phase2Dir, { recursive: true }); + + // REQ-99 belongs to phase 02 — it sits in phase 01's deliverables table and + // is still Pending in REQUIREMENTS.md. Phase 01's own IDs are hard-wrapped + // across three lines (the #4731 shape). + fs.writeFileSync(path.join(planDir, 'REQUIREMENTS.md'), [ + '# Requirements', + '', + '## Active', + '', + '- [ ] **REQ-11**: first wrapped requirement', + '- [ ] **REQ-12**: second wrapped requirement', + '- [ ] **REQ-13**: third wrapped requirement', + '- [ ] **REQ-99**: phase 02 requirement, still pending', + '', + '## Traceability', + '', + '| Requirement | Phase | Status |', + '|-------------|-------|--------|', + '| REQ-11 | Phase 1 | Pending |', + '| REQ-12 | Phase 1 | Pending |', + '| REQ-13 | Phase 1 | Pending |', + '| REQ-99 | Phase 2 | Pending |', + '', + ].join('\n')); + + fs.writeFileSync(path.join(planDir, 'ROADMAP.md'), [ + '# Roadmap', + '', + '- [ ] Phase 01: Preset', + '- [ ] Phase 02: Next', + '', + '### Phase 01: Preset', + '**Goal:** Build preset', + '**Requirements**: REQ-11,', + 'REQ-12,', + 'REQ-13', + '', + '| Deliverable | Requirement |', + '|-------------|-------------|', + '| Widget | REQ-99 |', + '', + '**Plans:** 1 plans', + '', + '### Phase 02: Next', + '**Goal:** whatever', + '**Requirements:** REQ-99', + '**Plans:** 1 plans', + '', + '## Progress', + '', + '| Phase | Plans Complete | Status | Completed |', + '|-------|----------------|--------|-----------|', + '| 01. Preset | 0/1 | Not started | - |', + '| 02. Next | 0/1 | Not started | - |', + '', + ].join('\n')); + + fs.writeFileSync(path.join(planDir, 'STATE.md'), [ + '---', 'milestone: v1.3', '---', + '# State', + '', + '**Current Phase:** 01', + '**Completed Phases:** 0', + '**Total Phases:** 2', + '**Progress:** 0%', + '', + ].join('\n')); + + for (const [dir, n] of [[phase1Dir, '01'], [phase2Dir, '02']]) { + fs.writeFileSync(path.join(dir, `${n}-01-PLAN.md`), '# Plan\nDo the work.\n'); + fs.writeFileSync(path.join(dir, `${n}-01-SUMMARY.md`), '# Summary\nDone.\n'); + } + + return tmpDir; + } + + test( + '#4731-followup: a wrapped Requirements field followed by a table cites ONLY the wrapped IDs — the table\'s REQ-99 stays Pending', + () => { + const tmpDir = build4731TableBleedFixture(); + try { + const { output } = runVerifiedPhaseComplete(['phase', 'complete', '1'], tmpDir); + const parsed = JSON.parse(output); + const warnings = parsed.warnings || []; + const reqContent = fs.readFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), 'utf-8'); + for (const id of ['REQ-11', 'REQ-12', 'REQ-13']) { + assert.ok( + new RegExp(`-\\s*\\[x\\]\\s*\\*\\*${id}\\*\\*`, 'i').test(reqContent), + `#4731-followup FAILED (fixture invariant): ${id} is cited by the wrapped Requirements field and must be ticked.\n${reqContent}`, + ); + } + // The discriminator: REQ-99 appears AFTER the field (in the section's + // deliverables table) — the scan must have stopped at the table. + assert.ok( + /-\s*\[\s\]\s*\*\*REQ-99\*\*/i.test(reqContent), + `#4731-followup FAILED: REQ-99 lives in the table AFTER the Requirements field (it belongs to phase 02) ` + + `and must NOT have been ticked by phase 01's completion scan.\n${reqContent}`, + ); + const req99Row = reqContent.split(/\r?\n/) + .filter((l) => l.trim().startsWith('|')) + .map((l) => splitTableRow(l)) + .find((cells) => cells[0] && cells[0].trim().toLowerCase() === 'req-99'); + assert.ok( + req99Row && /^Pending$/i.test(req99Row[req99Row.length - 1].trim()), + `#4731-followup FAILED: REQ-99's Traceability row must stay Pending — the citation scan bled into the table.\n${reqContent}`, + ); + assert.ok( + !warnings.some((w) => /not registered anywhere/i.test(w) && /REQ-99/i.test(w)), + `#4731-followup FAILED: REQ-99 must not surface as a ghost — it is registered; the scan just must not reach it, ` + + `got: ${JSON.stringify(warnings)}`, + ); + assert.strictEqual(parsed.requirements_updated, true, "#4731-followup FAILED (fixture invariant): the phase's own IDs must have been written"); + } finally { + cleanup(tmpDir); + } + }, + ); +}); + // ───────────────────────────────────────────────────────────────────────────── // Regressions: issue #3697 — the `**Requirements**:` tokenizer UNDER-selects // silently. #2339 fixed OVER-selection (the shape filter) and added the