From 3827054954618e87ff33b16c886c6b9dd78f9552 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 13 Jun 2026 21:56:18 -0400 Subject: [PATCH] fix(#1156): support table-format STATE.md and insert missing roadmap plan rows (#1172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#1162): support table-format STATE.md in state field read/replace Extend stateExtractField and stateReplaceField in state-document.cts to detect and operate on pipe-table rows (| Field | value |). The separator row | --- | --- | is excluded from matching. updateCurrentPositionFields in state.cts now falls through to stateReplaceField for table-format Current Position sections when the inline Status:/Last activity: patterns do not match. Co-Authored-By: Claude Opus 4.8 * fix(#1163): insert missing plan checklist rows in roadmap update-plan-progress cmdRoadmapUpdatePlanProgress now inserts `- [ ] NN-XX-PLAN.md` checkbox rows under the phase Plans: line when no per-plan checkbox rows exist yet (fresh template). Rows are sorted ascending and any already-summarised plans are immediately marked [x]. The planCountPattern is extended to also match plain `Plans:` (in addition to bold `**Plans:**`) so plan counts are updated in both template variants. The existing-rows check covers both top-level and indented checkbox forms to preserve idempotency. Co-Authored-By: Claude Opus 4.8 * fix(#1163): fill partial plan-row gaps and scope insertion to active milestone - Finding 1 (HIGH): replace all-or-nothing rowsAlreadyPresent guard with per-file set-difference so missing rows are inserted even when SOME plan rows already exist - Finding 2 (MEDIUM): extend planCountPattern to recognise **Plans**: (canonical template form — bold word + outer colon) alongside **Plans:** and plain Plans:; use two-pattern fallback for row insertion to anchor under Plans: checklist header rather than the **Plans**: summary line - Finding 3 (MEDIUM): scope row insertion to the active (post-) milestone region so duplicate phase headings in archived sections never receive new rows - Finding 4 (LOW): rename misleading "pipe-like content" test to honestly describe what it tests (multi-row table isolation); add out-of-scope escaped-pipe comment - Remove now-unused anyCheckboxMatched variable (lint clean) - Add 5 adversarial regression tests (pre-fix failures confirmed) Co-Authored-By: Claude Opus 4.8 * fix(#1163): scope missing-plan detection to active milestone; preserve authored state fields in table format - roadmap.cts: compute activeRegion (post- slice) once and use it for BOTH missingPlans detection and row insertion, so archived
rows no longer suppress active-section inserts (Finding 1 code-review) - roadmap.cts: change (Plans:) inner capture to non-capturing (?:Plans:) in insertRowsPatternA to prevent group-numbering shift (Finding 3) - state.cts: mirror inline-branch preserve-authored guards onto table-format branches in updateCurrentPositionFields — Status table branch checks isInList/matchesPattern before replacing; Last Activity table branch checks isDateShape/inList, preserving executor-authored narrative prose (Finding 2 code-review) - tests: add three failing-first regression tests confirming each finding Co-Authored-By: Claude Opus 4.8 * test(#1162,#1163): fold table-format + plan-row regressions into owning module tests Move bug-1162 cases into tests/state.test.cjs and bug-1163 cases into tests/roadmap.test.cjs under named regression describe blocks; delete the standalone bug-NNNN files and prune their allowlist entries. Co-Authored-By: Claude Opus 4.8 * docs(#1156): add changeset fragment for table-format state + roadmap insert fix Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/proud-foxes-travel.md | 5 + src/roadmap.cts | 94 +++++- src/state-document.cts | 54 ++- src/state.cts | 84 +++-- tests/roadmap.test.cjs | 549 +++++++++++++++++++++++++++++++ tests/state.test.cjs | 427 +++++++++++++++++++++++- 6 files changed, 1185 insertions(+), 28 deletions(-) create mode 100644 .changeset/proud-foxes-travel.md diff --git a/.changeset/proud-foxes-travel.md b/.changeset/proud-foxes-travel.md new file mode 100644 index 000000000..44d83abb6 --- /dev/null +++ b/.changeset/proud-foxes-travel.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1172 +--- +**`state update` and `roadmap update-plan-progress` now handle current Markdown artifact shapes** — state field read/replace works on table-format `STATE.md` (`| Status | … |`), and `roadmap update-plan-progress` inserts missing per-plan checklist rows (filling partial gaps), tolerates `Plans:`/`**Plans:**`/`**Plans**:`, and scopes changes to the active milestone. diff --git a/src/roadmap.cts b/src/roadmap.cts index 9a2fd744a..5d208df4f 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -513,9 +513,13 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und return '|' + cells.join('|') + '|'; }); - // Update plan count in phase detail section + // Update plan count in phase detail section. + // Three recognised forms (all tolerated; canonical template uses the first): + // `**Plans**: N plans` — bold word + outer colon (gsd-core/templates/roadmap.md) + // `**Plans:** N plans` — bold "Plans:" (colon inside bold) + // `Plans: N plans` — plain text header const planCountPattern = new RegExp( - `(#{2,4}\\s*Phase\\s+${phasePattern}(?=[:\\s])[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, + `(#{2,4}\\s*Phase\\s+${phasePattern}(?=[:\\s])[\\s\\S]*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*|(?:^|\\n)Plans:)\\s*)[^\\n]+`, 'i' ); const planCountText = isComplete @@ -544,6 +548,92 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und roadmapContent = roadmapContent.replace(planCheckboxPattern, '$1x$2'); } + // Compute the active (post-
) region offset ONCE. Both the + // missing-plan DETECTION and the row INSERTION must use the same active + // region string so that a plan row that exists only in an archived
+ // block is not counted as "already present" in the active milestone section. + // (Finding 1 code-review: detection was previously running against the full + // roadmapContent, causing archived rows to suppress active-section inserts.) + const lastDetailsClose = roadmapContent.lastIndexOf('
'); + const activeRegion = lastDetailsClose === -1 + ? roadmapContent + : roadmapContent.slice(lastDetailsClose + ''.length); + + // Compute which plan files are MISSING a checkbox row in the ACTIVE region. + // This handles three cases: + // (a) Fresh template — no rows at all: all plans are missing. + // (b) Partial gap — some rows exist, others don't: only the absent ones. + // (c) All rows present — nothing to insert (idempotent). + // + // Detection is scoped to the active region so a plan that appears in an + // archived
block is still correctly detected as missing from the + // active milestone section. + const missingPlans = phaseInfo!.plans.filter((planFile) => { + const planEscaped = escapeRegex(planFile); + return !new RegExp(`-\\s*\\[[x ]\\]\\s*(?:\\*\\*)?${planEscaped}`, 'i').test(activeRegion); + }); + + if (missingPlans.length > 0) { + // Insert missing plan checklist rows (#1163). We prefer to anchor to the + // bare `Plans:` checklist header (canonical template form) and fall back to + // the bold `**Plans**:`/`**Plans:**` summary line only when no bare header + // is present. Using two separate patterns avoids the lazy-quantifier trap + // where a single alternation would stop at the first matching alternative + // (the bold summary) before reaching the checklist header. + // + // Canonical template (gsd-core/templates/roadmap.md) uses BOTH lines: + // **Plans**: N plans ← summary (colon outside bold) + // Plans: ← checklist header (PREFERRED insertion anchor) + // Rows must land after `Plans:`, not between the summary and the header. + // + // Pattern A: anchor to bare `Plans:` header (preferred). + // Pattern B: fallback to bold summary when no bare header exists. + const insertRowsPatternA = new RegExp( + `(#{2,4}\\s*Phase\\s+${phasePattern}(?=[:\\s])[\\s\\S]*?(?:^|\\n)(?:Plans:)[^\\n]*)`, + 'i' + ); + const insertRowsPatternB = new RegExp( + `(#{2,4}\\s*Phase\\s+${phasePattern}(?=[:\\s])[\\s\\S]*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*)[^\\n]*)`, + 'i' + ); + + const sortedMissing = [...missingPlans].sort(); + const newRows = sortedMissing.map(p => `- [ ] ${p}`).join('\n'); + const inserter = (match: string) => `${match}\n${newRows}`; + + // Scope insertion to the active (post-
) milestone region to + // prevent duplicate phase headings in archived sections from receiving rows. + // replaceInCurrentMilestone only accepts a string replacement, so we + // perform the scoped replace manually here (same strategy as that helper). + // Note: lastDetailsClose was computed above (shared with detection). + const scopedReplace = (src: string, pat: RegExp) => src.replace(pat, inserter); + let withRows: string; + if (lastDetailsClose === -1) { + // activeRegion === roadmapContent when there are no blocks. + const regionA = scopedReplace(activeRegion, insertRowsPatternA); + withRows = regionA !== activeRegion ? regionA : scopedReplace(activeRegion, insertRowsPatternB); + } else { + const beforeDetails = roadmapContent.slice(0, lastDetailsClose + ''.length); + const regionA = scopedReplace(activeRegion, insertRowsPatternA); + const afterWithRows = regionA !== activeRegion ? regionA : scopedReplace(activeRegion, insertRowsPatternB); + withRows = beforeDetails + afterWithRows; + } + if (withRows !== roadmapContent) { + roadmapContent = withRows; + // Mark any newly-inserted rows that already have summaries as complete + for (const summaryFile of phaseInfo!.summaries) { + const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', ''); + if (!planId) continue; + const planEscaped = escapeRegex(planId); + const planCheckboxPattern = new RegExp( + `(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`, + 'i' + ); + roadmapContent = roadmapContent.replace(planCheckboxPattern, '$1x$2'); + } + } + } + platformWriteSync(roadmapPath, roadmapContent); }); output({ diff --git a/src/state-document.cts b/src/state-document.cts index e8c7423b6..cb9accd20 100644 --- a/src/state-document.cts +++ b/src/state-document.cts @@ -32,27 +32,79 @@ function existingProgressExceedsDerived(existingProgress: ProgressRecord, derive return existing !== null && derived !== null && existing > derived; } +/** + * Return true if a pipe-table row's first cell is a separator cell (`---` + * variants) rather than a field name. Prevents the separator row + * `| --- | --- |` from being treated as a field named "---". + */ +function isTableSeparatorRow(firstCell: string): boolean { + // A separator cell contains only dashes, colons (alignment hints), and whitespace. + return /^[\s\-:]+$/.test(firstCell.trim()); +} + +/** + * Build a regex that matches a pipe-table row `| FieldName | value |` for the + * given (already-escaped) field name. The match is case-insensitive and + * tolerates variable amounts of whitespace around the cell contents. + * + * Capture group 1: leading pipe + whitespace before the field cell + * Capture group 2: the field name cell text (trimmed) + * Capture group 3: whitespace between field cell and separator pipe + * Capture group 4: the value cell text (trimmed) + * Capture group 5: trailing whitespace + closing pipe(s) + * + * We use a single-line match (`m` flag so ^ anchors work on each line) to + * avoid cross-row replacement. + */ +function tableRowPattern(escapedFieldName: string): RegExp { + return new RegExp( + `^(\\|[ \\t]*)(${escapedFieldName})([ \\t]*\\|[ \\t]*)([^|\\n]*?)([ \\t]*\\|[ \\t]*)$`, + 'im', + ); +} + export function stateExtractField(content: string, fieldName: string): string | null { const escaped = escapeRegex(fieldName); + // Bold inline format: **FieldName:** value const boldPattern = new RegExp(`\\*\\*${escaped}:\\*\\*[ \\t]*(.+)`, 'i'); const boldMatch = content.match(boldPattern); if (boldMatch) return boldMatch[1].trim(); + // Plain line-start format: FieldName: value const plainPattern = new RegExp(`^${escaped}:[ \\t]*(.+)`, 'im'); const plainMatch = content.match(plainPattern); - return plainMatch ? plainMatch[1].trim() : null; + if (plainMatch) + return plainMatch[1].trim(); + // Pipe-table format: | FieldName | value | + // (Separator rows such as `| --- | --- |` are excluded.) + const tableMatch = content.match(tableRowPattern(escaped)); + if (tableMatch && !isTableSeparatorRow(tableMatch[2])) + return tableMatch[4].trim(); + return null; } export function stateReplaceField(content: string, fieldName: string, newValue: string): string | null { const escaped = escapeRegex(fieldName); + // Bold inline format: **FieldName:** value const boldPattern = new RegExp(`(\\*\\*${escaped}:\\*\\*\\s*)(.*)`, 'i'); if (boldPattern.test(content)) { return content.replace(boldPattern, (_match, prefix: string) => `${prefix}${newValue}`); } + // Plain line-start format: FieldName: value const plainPattern = new RegExp(`(^${escaped}:\\s*)(.*)`, 'im'); if (plainPattern.test(content)) { return content.replace(plainPattern, (_match, prefix: string) => `${prefix}${newValue}`); } + // Pipe-table format: | FieldName | value | + // Preserve the surrounding pipe/whitespace structure; only swap the value cell. + const tblPat = tableRowPattern(escaped); + const tblMatch = content.match(tblPat); + if (tblMatch && !isTableSeparatorRow(tblMatch[2])) { + // Reconstruct the row, preserving the original surrounding whitespace/pipes. + return content.replace(tblPat, (_m, leadPipe: string, fieldCell: string, midPipe: string, _oldVal: string, trailPipe: string) => + `${leadPipe}${fieldCell}${midPipe}${newValue}${trailPipe}`, + ); + } return null; } diff --git a/src/state.cts b/src/state.cts index 5dbfc20eb..f45f6ea9f 100644 --- a/src/state.cts +++ b/src/state.cts @@ -351,34 +351,70 @@ function updateCurrentPositionFields(content: string, fields: { status?: string; const statusDefaults = KNOWN_TEMPLATE_DEFAULTS['Status']; const lastActivityDefaults = KNOWN_TEMPLATE_DEFAULTS['Last Activity']; - if (fields.status && /^Status:/m.test(posBody)) { - // Only replace when the existing Current Position Status is a known template default. - const existingStatusMatch = posBody.match(/^Status:\s*(.+)$/m); - const existingStatus = existingStatusMatch ? existingStatusMatch[1].trim() : null; - const isInList = existingStatus && statusDefaults.some(d => d.toLowerCase() === existingStatus.toLowerCase()); - const matchesPattern = existingStatus && KNOWN_STATUS_PATTERNS.some(p => p.test(existingStatus)); - const isDefault = !existingStatus || isInList || matchesPattern; - if (isDefault) { - posBody = posBody.replace(/^Status:.*$/m, `Status: ${fields.status}`); + if (fields.status) { + if (/^Status:/m.test(posBody)) { + // Inline format: Status: value — only replace when the existing value is a + // known template default (Knuth invariant: preserve executor-authored values). + const existingStatusMatch = posBody.match(/^Status:\s*(.+)$/m); + const existingStatus = existingStatusMatch ? existingStatusMatch[1].trim() : null; + const isInList = existingStatus && statusDefaults.some(d => d.toLowerCase() === existingStatus.toLowerCase()); + const matchesPattern = existingStatus && KNOWN_STATUS_PATTERNS.some(p => p.test(existingStatus)); + const isDefault = !existingStatus || isInList || matchesPattern; + if (isDefault) { + posBody = posBody.replace(/^Status:.*$/m, `Status: ${fields.status}`); + } + } else { + // Table format: | Status | value | — apply the same preserve-authored guard + // as the inline branch: only overwrite a known template default. + // (Finding 2 code-review: the table branch was unconditional before this fix.) + const existingStatus = stateExtractField(posBody, 'Status'); + const isInList = existingStatus && statusDefaults.some(d => d.toLowerCase() === existingStatus.toLowerCase()); + const matchesPattern = existingStatus && KNOWN_STATUS_PATTERNS.some(p => p.test(existingStatus)); + const isDefault = !existingStatus || isInList || matchesPattern; + if (isDefault) { + const replaced = stateReplaceField(posBody, 'Status', fields.status); + if (replaced !== null) posBody = replaced; + } } } - if (fields.lastActivity && /^Last activity:/im.test(posBody)) { - // Only replace when the existing Current Position Last activity is a known template - // default (a bare ISO date). Executor-authored narrative prose is preserved. - const existingActivityMatch = posBody.match(/^Last activity:\s*(.+)$/im); - const existingActivity = existingActivityMatch ? existingActivityMatch[1].trim() : null; - // A bare ISO date (YYYY-MM-DD with nothing after) is handler-generated. - // A date with a narrative suffix (e.g. "2026-02-15 -- blocked by infra...") - // was authored by the executor and must be preserved. - const isDateShape = existingActivity && /^\d{4}-\d{2}-\d{2}$/.test(existingActivity); - const inList = existingActivity && lastActivityDefaults.some(d => d.toLowerCase() === existingActivity.toLowerCase()); - const isDefault = !existingActivity || isDateShape || inList; - if (isDefault) { - posBody = posBody.replace(/^Last activity:.*$/im, `Last activity: ${fields.lastActivity}`); + if (fields.lastActivity) { + if (/^Last activity:/im.test(posBody)) { + // Inline format — only replace when the existing value is a known template + // default (a bare ISO date). Executor-authored narrative prose is preserved. + const existingActivityMatch = posBody.match(/^Last activity:\s*(.+)$/im); + const existingActivity = existingActivityMatch ? existingActivityMatch[1].trim() : null; + // A bare ISO date (YYYY-MM-DD with nothing after) is handler-generated. + // A date with a narrative suffix (e.g. "2026-02-15 -- blocked by infra...") + // was authored by the executor and must be preserved. + const isDateShape = existingActivity && /^\d{4}-\d{2}-\d{2}$/.test(existingActivity); + const inList = existingActivity && lastActivityDefaults.some(d => d.toLowerCase() === existingActivity.toLowerCase()); + const isDefault = !existingActivity || isDateShape || inList; + if (isDefault) { + posBody = posBody.replace(/^Last activity:.*$/im, `Last activity: ${fields.lastActivity}`); + } + } else { + // Table format — apply the same preserve-authored guard as the inline branch: + // only overwrite a bare ISO date or a known default; preserve narrative prose. + // (Finding 2 code-review: the table branch was unconditional before this fix.) + const existingActivity = stateExtractField(posBody, 'Last Activity') + ?? stateExtractField(posBody, 'Last activity'); + const isDateShape = existingActivity && /^\d{4}-\d{2}-\d{2}$/.test(existingActivity); + const inList = existingActivity && lastActivityDefaults.some(d => d.toLowerCase() === existingActivity.toLowerCase()); + const isDefault = !existingActivity || isDateShape || inList; + if (isDefault) { + const replaced = stateReplaceField(posBody, 'Last Activity', fields.lastActivity) + ?? stateReplaceField(posBody, 'Last activity', fields.lastActivity); + if (replaced !== null) posBody = replaced; + } } } - if (fields.plan && /^Plan:/m.test(posBody)) { - posBody = posBody.replace(/^Plan:.*$/m, `Plan: ${fields.plan}`); + if (fields.plan) { + if (/^Plan:/m.test(posBody)) { + posBody = posBody.replace(/^Plan:.*$/m, `Plan: ${fields.plan}`); + } else { + const replaced = stateReplaceField(posBody, 'Plan', fields.plan); + if (replaced !== null) posBody = replaced; + } } return content.replace(posPattern, () => `${posMatch[1]}${posBody}`); diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 399b60a63..af4a60275 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -893,3 +893,552 @@ describe('roadmap update-plan-progress command', () => { // ───────────────────────────────────────────────────────────────────────────── // phase add command // ───────────────────────────────────────────────────────────────────────────── + +// ───────────────────────────────────────────────────────────────────────────── +// regressions: insert missing plan rows (#1163) +// ───────────────────────────────────────────────────────────────────────────── + +// Phase numbers are zero-padded by normalizePhaseName so the .planning/phases/ +// directory must use the padded form (e.g. "05-test-phase" for phase 5). +const PHASE_NUM_1163 = '5'; // what we pass on the command line +const PHASE_DIR_SLUG_1163 = '05'; // what ends up on disk after normalization + +/** + * ROADMAP.md with a phase-5 detail section that uses bold **Plans:** and has + * NO per-plan checkbox rows yet. + */ +function buildRoadmapBoldPlans(phaseNum = PHASE_NUM_1163, planCount = '0/3 plans executed') { + return [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + '| Phase | Plans | Status | Completed |', + '| --- | --- | --- | --- |', + `| Phase ${phaseNum}: test phase | 0/3 | Planned | |`, + '', + `### Phase ${phaseNum}: test phase`, + '', + 'Goal: build something', + '', + `**Plans:** ${planCount}`, + '', + '(No individual plan rows yet — template freshly generated)', + '', + ].join('\n'); +} + +/** + * ROADMAP.md using the CANONICAL template shape: + * **Plans**: N plans ← bold summary metadata line + * (blank line) + * Plans: ← checklist header + * - [ ] NN-XX-PLAN.md ← per-plan checkboxes + * + * The template (gsd-core/templates/roadmap.md) always uses this two-line form. + * `**Plans**:` has the colon OUTSIDE the bold markers. + */ +function buildRoadmapCanonicalTemplate(phaseNum = PHASE_NUM_1163, existingRows = []) { + const rowLines = existingRows.length > 0 + ? ['Plans:', ...existingRows.map(r => `- [ ] ${r}`), ''] + : ['Plans:', '']; + return [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + '| Phase | Plans | Status | Completed |', + '| --- | --- | --- | --- |', + `| Phase ${phaseNum}: test phase | 0/3 | Planned | |`, + '', + `### Phase ${phaseNum}: test phase`, + '', + '**Goal**: build something', + `**Plans**: 0/3 plans`, + '', + ...rowLines, + ].join('\n'); +} + +/** + * ROADMAP.md with a duplicate phase heading in an archived
section + * plus the same phase as the ACTIVE milestone section. + */ +function buildRoadmapWithArchivedDuplicate(phaseNum = PHASE_NUM_1163) { + return [ + '# ROADMAP', + '', + '
', + 'v0.9 — shipped', + '', + `### Phase ${phaseNum}: test phase`, + '', + '**Plans**: 2/2 plans complete', + '', + 'Plans:', + `- [x] ${phaseNum}-01-PLAN.md`, + `- [x] ${phaseNum}-02-PLAN.md`, + '', + '
', + '', + '## Milestone v1.0', + '', + `### Phase ${phaseNum}: test phase`, + '', + '**Plans**: 0/3 plans', + '', + 'Plans:', + '', + ].join('\n'); +} + +/** + * ROADMAP.md with a phase-5 detail section that uses plain `Plans:` (not bold) + * and NO per-plan checkbox rows yet. + */ +function buildRoadmapPlainPlans(phaseNum = PHASE_NUM_1163) { + return [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + '| Phase | Plans | Status | Completed |', + '| --- | --- | --- | --- |', + `| Phase ${phaseNum}: test phase | 0/3 | Planned | |`, + '', + `### Phase ${phaseNum}: test phase`, + '', + 'Goal: build something', + '', + `Plans: 0/3 plans executed`, + '', + '(No individual plan rows yet)', + '', + ].join('\n'); +} + +/** + * Create plan files for a given phase in the .planning/phases tree. + * Uses the normalized (zero-padded) directory name so findPhaseInternal can + * locate the phase. Returns the phase directory path. + */ +function createPhaseWithPlans(tmpDir, phaseNum, planNames) { + // normalizePhaseName('5') → '05', so use zero-padded slug on disk + const paddedNum = String(phaseNum).padStart(2, '0'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${paddedNum}-test-phase`); + fs.mkdirSync(phaseDir, { recursive: true }); + for (const name of planNames) { + fs.writeFileSync(path.join(phaseDir, name), `# ${name}\n`); + } + return phaseDir; +} + +describe('regressions: insert missing plan rows (#1163)', () => { + let tmpDir; + let roadmapPath; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1163-'); + roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ── Bold **Plans:** — insert missing rows ──────────────────────────────── + + test('inserts plan checkbox rows under bold **Plans:** when none exist', () => { + fs.writeFileSync(roadmapPath, buildRoadmapBoldPlans('5')); + createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + '5-03-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'expected updated:true'); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + assert.ok(written.includes('- [ ] 5-01-PLAN.md'), '5-01-PLAN.md row not inserted'); + assert.ok(written.includes('- [ ] 5-02-PLAN.md'), '5-02-PLAN.md row not inserted'); + assert.ok(written.includes('- [ ] 5-03-PLAN.md'), '5-03-PLAN.md row not inserted'); + }); + + // ── Plain Plans: — insert missing rows ────────────────────────────────── + + test('inserts plan checkbox rows under plain Plans: when none exist', () => { + fs.writeFileSync(roadmapPath, buildRoadmapPlainPlans('5')); + createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'expected updated:true'); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + assert.ok(written.includes('- [ ] 5-01-PLAN.md'), '5-01-PLAN.md row not inserted (plain Plans:)'); + assert.ok(written.includes('- [ ] 5-02-PLAN.md'), '5-02-PLAN.md row not inserted (plain Plans:)'); + }); + + // ── Plan count update with plain Plans: ───────────────────────────────── + + test('plan count is updated when section uses plain Plans: (not bold)', () => { + fs.writeFileSync(roadmapPath, buildRoadmapPlainPlans('5')); + // createPhaseWithPlans returns the padded dir path (05-test-phase) + const phaseDir = createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + ]); + // Create a summary for plan 1 so it's not 0/2 + fs.writeFileSync(path.join(phaseDir, '5-01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + // Plan count should reflect 1 completed out of 2 + assert.ok( + written.match(/Plans:\s*1\/2 plans executed/), + 'Plain Plans: count not updated. ROADMAP.md:\n' + written, + ); + }); + + // ── Bold **Plans:** count update ───────────────────────────────────────── + + test('plan count is updated when section uses bold **Plans:**', () => { + fs.writeFileSync(roadmapPath, buildRoadmapBoldPlans('5', '0/3 plans executed')); + const phaseDir = createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + '5-03-PLAN.md', + ]); + // Complete 1 plan + fs.writeFileSync(path.join(phaseDir, '5-01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + assert.ok( + written.includes('**Plans:** 1/3 plans executed'), + '**Plans:** count not updated. ROADMAP.md:\n' + written, + ); + }); + + // ── Existing rows are checked off, not duplicated ─────────────────────── + + test('existing plan rows are marked complete, not duplicated when SUMMARY exists', () => { + const roadmapWithRows = [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + '### Phase 5: test phase', + '', + '**Plans:** 0/2 plans executed', + '', + '- [ ] 5-01-PLAN.md', + '- [ ] 5-02-PLAN.md', + '', + ].join('\n'); + fs.writeFileSync(roadmapPath, roadmapWithRows); + + const phaseDir = createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + ]); + fs.writeFileSync(path.join(phaseDir, '5-01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + + // 5-01 should be checked, 5-02 should remain unchecked + assert.ok(written.includes('- [x] 5-01-PLAN.md'), '5-01-PLAN.md not marked complete'); + assert.ok(written.includes('- [ ] 5-02-PLAN.md'), '5-02-PLAN.md incorrectly marked complete'); + + // Only two rows — no duplicates + const matches = (written.match(/- \[.\] 5-\d+-PLAN\.md/g) || []); + assert.equal(matches.length, 2, `Expected 2 checkbox rows, got ${matches.length}:\n${written}`); + }); + + // ── Inserted rows are sorted ───────────────────────────────────────────── + + test('inserted plan rows are sorted in ascending order', () => { + fs.writeFileSync(roadmapPath, buildRoadmapBoldPlans('5')); + createPhaseWithPlans(tmpDir, '5', [ + '5-03-PLAN.md', + '5-01-PLAN.md', + '5-02-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + const rowPositions = [ + written.indexOf('5-01-PLAN.md'), + written.indexOf('5-02-PLAN.md'), + written.indexOf('5-03-PLAN.md'), + ]; + assert.ok( + rowPositions[0] < rowPositions[1] && rowPositions[1] < rowPositions[2], + 'Inserted plan rows are not in ascending order. ROADMAP.md:\n' + written, + ); + }); + + // ── No plans found — command returns updated:false without inserting ───── + + test('returns updated:false gracefully when phase has no plan files', () => { + fs.writeFileSync(roadmapPath, buildRoadmapBoldPlans('5')); + // Create phase dir (padded slug) but no plan files + const phaseDir = path.join(tmpDir, '.planning', 'phases', `${PHASE_DIR_SLUG_1163}-test-phase`); + fs.mkdirSync(phaseDir, { recursive: true }); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, false, 'Expected updated:false when no plans exist'); + }); + + // ── Adversarial: CRLF in ROADMAP.md ────────────────────────────────────── + + test('CRLF line endings in ROADMAP.md are handled without corruption', () => { + const content = buildRoadmapBoldPlans('5').replace(/\n/g, '\r\n'); + fs.writeFileSync(roadmapPath, content); + createPhaseWithPlans(tmpDir, '5', ['5-01-PLAN.md', '5-02-PLAN.md']); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'CRLF ROADMAP not handled'); + }); +}); + +describe('regressions: insert missing plan rows (#1163) — adversarial: partial gaps, canonical template, scoped insertion', () => { + let tmpDir; + let roadmapPath; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1163-adv-'); + roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ── Finding 1: partial-row gaps ────────────────────────────────────────── + + test('(Finding 1) inserts missing rows when SOME plan rows already exist', () => { + // Phase has 5-01, 5-02, 5-03 on disk. + // ROADMAP already has a row for 5-01 only. + // Expected: 5-02 and 5-03 rows are inserted; 5-01 is NOT duplicated. + const roadmapContent = [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + `### Phase 5: test phase`, + '', + '**Plans:** 0/3 plans executed', + '', + 'Plans:', + '- [ ] 5-01-PLAN.md', + '', + ].join('\n'); + fs.writeFileSync(roadmapPath, roadmapContent); + + createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + '5-03-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + + // All three plans must have rows + assert.ok(written.includes('5-01-PLAN.md'), '5-01-PLAN.md row missing'); + assert.ok(written.includes('5-02-PLAN.md'), '5-02-PLAN.md row was not inserted for partial gap'); + assert.ok(written.includes('5-03-PLAN.md'), '5-03-PLAN.md row was not inserted for partial gap'); + + // No duplicates: exactly one checkbox row per plan + const rows01 = (written.match(/- \[.\] 5-01-PLAN\.md/g) || []); + const rows02 = (written.match(/- \[.\] 5-02-PLAN\.md/g) || []); + const rows03 = (written.match(/- \[.\] 5-03-PLAN\.md/g) || []); + assert.equal(rows01.length, 1, `5-01-PLAN.md duplicated (${rows01.length} times)`); + assert.equal(rows02.length, 1, `5-02-PLAN.md duplicated (${rows02.length} times)`); + assert.equal(rows03.length, 1, `5-03-PLAN.md duplicated (${rows03.length} times)`); + }); + + test('(Finding 1) running twice (idempotent) does not duplicate partially-inserted rows', () => { + const roadmapContent = [ + '# ROADMAP', + '', + '## Milestone v1.0', + '', + `### Phase 5: test phase`, + '', + '**Plans:** 0/2 plans executed', + '', + 'Plans:', + '- [ ] 5-01-PLAN.md', + '', + ].join('\n'); + fs.writeFileSync(roadmapPath, roadmapContent); + createPhaseWithPlans(tmpDir, '5', ['5-01-PLAN.md', '5-02-PLAN.md']); + + // Run once to insert 5-02 + runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + // Run again — should be a no-op + runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + const rows01 = (written.match(/- \[.\] 5-01-PLAN\.md/g) || []); + const rows02 = (written.match(/- \[.\] 5-02-PLAN\.md/g) || []); + assert.equal(rows01.length, 1, `5-01-PLAN.md duplicated after two runs (${rows01.length} times)`); + assert.equal(rows02.length, 1, `5-02-PLAN.md duplicated after two runs (${rows02.length} times)`); + }); + + // ── Finding 2: canonical template shape ───────────────────────────────── + // The canonical template (gsd-core/templates/roadmap.md) uses: + // **Plans**: N plans ← summary line (bold word, outer colon) + // (blank line) + // Plans: ← checklist header + // - [ ] NN-XX rows + // + // NOTE: `**Plans**:` differs from `**Plans:**` (colon placement): + // **Plans**: → bold "Plans" + outer colon (CANONICAL) + // **Plans:** → bold "Plans:" (previously assumed form) + // Rows must be inserted under the `Plans:` checklist header, not after the + // `**Plans**:` summary line. + + test('(Finding 2) canonical template: inserts rows under Plans: checklist header, not after **Plans**: summary', () => { + // Canonical form: **Plans**: summary + blank + Plans: checklist header + no rows yet + fs.writeFileSync(roadmapPath, buildRoadmapCanonicalTemplate('5', [])); + createPhaseWithPlans(tmpDir, '5', ['5-01-PLAN.md', '5-02-PLAN.md']); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + assert.ok(written.includes('- [ ] 5-01-PLAN.md'), '5-01-PLAN.md not inserted under Plans:'); + assert.ok(written.includes('- [ ] 5-02-PLAN.md'), '5-02-PLAN.md not inserted under Plans:'); + + // Rows must appear AFTER the `Plans:` line, not between `**Plans**:` and `Plans:` + const plansHeaderIdx = written.indexOf('\nPlans:\n'); + const boldPlansIdx = written.indexOf('**Plans**:'); + const row01Idx = written.indexOf('- [ ] 5-01-PLAN.md'); + assert.ok(boldPlansIdx !== -1, '**Plans**: summary line is missing from output'); + assert.ok(plansHeaderIdx !== -1, 'Plans: checklist header is missing from output'); + assert.ok(row01Idx > plansHeaderIdx, 'row 5-01 appears before Plans: checklist header'); + }); + + test('(Finding 2) canonical template: plan count updated on **Plans**: summary line', () => { + // **Plans**: uses bold word + outer colon — the count update regex must handle it + fs.writeFileSync(roadmapPath, buildRoadmapCanonicalTemplate('5', [])); + const phaseDir = createPhaseWithPlans(tmpDir, '5', ['5-01-PLAN.md', '5-02-PLAN.md']); + fs.writeFileSync(path.join(phaseDir, '5-01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + // The **Plans**: summary line should be updated to reflect 1/2 + assert.ok( + written.match(/\*\*Plans\*\*:\s*1\/2 plans executed/), + '**Plans**: count not updated for canonical template shape.\nROADMAP.md:\n' + written, + ); + }); + + // ── Finding 3: insertion scoped to active milestone ────────────────────── + + test('(Finding 3) rows are inserted only in the active milestone, not in archived
', () => { + // ROADMAP has duplicate phase heading: one inside
(archived) and + // one in the active section. Rows must land ONLY in the active section. + fs.writeFileSync(roadmapPath, buildRoadmapWithArchivedDuplicate('5')); + createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + '5-03-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + + // The archived section already has rows for 5-01 and 5-02 only (checked off). + // The active section should get 5-03 row (and ideally 5-01/5-02 too if they + // were missing from the active section — the active section's Plans: was empty). + // Key assertion: no NEW rows were inserted into the archived
block. + + const detailsStart = written.indexOf('
'); + const detailsEnd = written.indexOf('
'); + const archivedSection = written.slice(detailsStart, detailsEnd + '
'.length); + + // The archived section should still have exactly 2 rows (5-01 and 5-02) + const archivedRows = (archivedSection.match(/- \[.\] 5-\d+-PLAN\.md/g) || []); + assert.equal(archivedRows.length, 2, `Archived section row count changed — rows were inserted into archived section:\n${archivedSection}`); + + // The active milestone section (after
) should have the new rows + const activeSection = written.slice(detailsEnd + '
'.length); + assert.ok(activeSection.includes('5-03-PLAN.md'), '5-03 row not inserted in active milestone section'); + }); + + // ── Finding 1 (code-review round 2): detection scoped to active region ─── + // When an archived
block contains checkbox rows for the SAME plan + // files as the current phase, the missingPlans filter must detect those plans + // as MISSING from the ACTIVE section and insert them there — not skip them + // because they appear anywhere in the full file content. + + test('(Finding 1 code-review) inserts ALL missing rows in active section even when archived section has same plans', () => { + // Archived section has 5-01 and 5-02 (checked off for v0.9). + // Active section has NO checkbox rows yet (empty Plans: block). + // Phase 5 on disk has 5-01, 5-02, 5-03. + // Expected: active section gets rows for ALL THREE plans (5-01, 5-02, 5-03). + // Bug (pre-fix): detection runs against full content → 5-01 and 5-02 found in + // archived block → missingPlans = [5-03 only] → only 5-03 inserted. + fs.writeFileSync(roadmapPath, buildRoadmapWithArchivedDuplicate('5')); + createPhaseWithPlans(tmpDir, '5', [ + '5-01-PLAN.md', + '5-02-PLAN.md', + '5-03-PLAN.md', + ]); + + const result = runGsdTools(['roadmap', 'update-plan-progress', '5'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(roadmapPath, 'utf-8'); + const detailsEnd = written.indexOf('
'); + const activeSection = written.slice(detailsEnd + '
'.length); + + // All three plans must be present in the active section + assert.ok(activeSection.includes('5-01-PLAN.md'), '5-01-PLAN.md not inserted in active section (detection not scoped to active region)'); + assert.ok(activeSection.includes('5-02-PLAN.md'), '5-02-PLAN.md not inserted in active section (detection not scoped to active region)'); + assert.ok(activeSection.includes('5-03-PLAN.md'), '5-03-PLAN.md not inserted in active section'); + + // Archived section must remain untouched (still exactly 2 rows) + const archivedSection = written.slice(written.indexOf('
'), detailsEnd + '
'.length); + const archivedRows = (archivedSection.match(/- \[.\] 5-\d+-PLAN\.md/g) || []); + assert.equal(archivedRows.length, 2, `Archived section row count changed:\n${archivedSection}`); + }); +}); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 38eb29d9a..5cc6d570c 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -10,7 +10,7 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { runGsdTools, cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); const { createFixture } = require('./fixtures/index.cjs'); describe('state-snapshot command', () => { @@ -3230,3 +3230,428 @@ describe('state add-roadmap-evolution (bug #1140)', () => { assert.ok(!/\bat .*\(.*:\d+:\d+\)/.test(result.error || ''), 'no stack trace'); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// regressions: table-format STATE.md (#1162) +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Minimal STATE.md that uses a pipe-table for the Current Position section. + * This is the format that triggered the "Field not found" silent failure. + */ +function buildTableFormatState(opts) { + const { + status = 'Ready to plan', + phase = '3', + planCount = '4', + lastActivity = '2026-01-01', + } = opts || {}; + + return [ + '---', + 'gsd_state_version: 1.0', + 'status: planning', + '---', + '', + '# GSD State', + '', + '## Current Position', + '', + '| Field | Value |', + '| --- | --- |', + `| Status | ${status} |`, + `| Phase | ${phase} |`, + `| Total Plans in Phase | ${planCount} |`, + `| Last Activity | ${lastActivity} |`, + '', + '## Accumulated Context', + '', + 'Some context here.', + '', + ].join('\n'); +} + +/** + * STATE.md that uses bold inline format (the existing working format). + * Included as a control case to confirm we did not break bold-field support. + */ +function buildBoldFormatState(opts) { + const { + status = 'Ready to plan', + phase = '3', + } = opts || {}; + + return [ + '---', + 'gsd_state_version: 1.0', + 'status: planning', + '---', + '', + '# GSD State', + '', + '## Current Position', + '', + `**Status:** ${status}`, + `**Phase:** ${phase}`, + '', + ].join('\n'); +} + +describe('regressions: table-format STATE.md (#1162)', () => { + let tmpDir; + let statePath; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1162-'); + statePath = path.join(tmpDir, '.planning', 'STATE.md'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ── Happy path: table-format field replacement ────────────────────────── + + test('state update rewrites table-cell Status value', () => { + fs.writeFileSync(statePath, buildTableFormatState({ status: 'Ready to plan' })); + + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], tmpDir); + + // Command must report success + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'expected updated:true but got: ' + JSON.stringify(parsed)); + + // The table cell must be rewritten on disk + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('| Status | Ready to execute |'), + 'Table cell not rewritten. STATE.md content:\n' + written, + ); + // Original value must be gone + assert.ok( + !written.includes('| Status | Ready to plan |'), + 'Old table cell value still present in STATE.md', + ); + }); + + test('state update rewrites table-cell value for arbitrary field', () => { + fs.writeFileSync(statePath, buildTableFormatState({ lastActivity: '2026-01-01' })); + + const result = runGsdTools(['state', 'update', 'Last Activity', '2026-06-13'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'expected updated:true'); + + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('| Last Activity | 2026-06-13 |'), + 'Last Activity table cell not rewritten. Content:\n' + written, + ); + }); + + test('state update is case-insensitive for table field names', () => { + // Table may have lowercase "status" in the first cell + const content = [ + '---', + 'gsd_state_version: 1.0', + '---', + '', + '## Current Position', + '', + '| Field | Value |', + '| --- | --- |', + '| status | Ready to plan |', + '', + ].join('\n'); + fs.writeFileSync(statePath, content); + + const result = runGsdTools(['state', 'update', 'status', 'Ready to execute'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'case-insensitive table match failed'); + }); + + // ── Negative: separator row must NOT be treated as a field ─────────────── + + test('separator row | --- | --- | is not matched as a field', () => { + // The field name "---" is rejected by the field-name validator before + // stateReplaceField is even called. The command exits with a non-zero + // status and a plain-text error, NOT a JSON { updated: false } result. + // The key invariant is that the file is never corrupted. + const originalContent = buildTableFormatState(); + fs.writeFileSync(statePath, originalContent); + + const result = runGsdTools(['state', 'update', '---', 'injected'], tmpDir); + + // The validator rejects '---' as an invalid field name — command must fail + // OR, if somehow the command succeeds, updated must be false. + if (result.success) { + // Unlikely path — if the validator is relaxed in future, still must not update. + let parsed; + try { parsed = JSON.parse(result.output); } catch { parsed = null; } + if (parsed) { + assert.equal(parsed.updated, false, 'separator row incorrectly matched as a field'); + } + } + // Either way: the file must be untouched (no 'injected' value written) + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok(!written.includes('injected'), 'separator row replacement leaked into file'); + }); + + // ── Regression: bold-format still works after the fix ──────────────────── + + test('state update bold-format still works after table support added', () => { + fs.writeFileSync(statePath, buildBoldFormatState({ status: 'Ready to plan' })); + + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'bold-format update broken after fix'); + + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('**Status:** Ready to execute'), + 'Bold-format field not rewritten. Content:\n' + written, + ); + }); + + // ── updateCurrentPositionFields table support ───────────────────────────── + + test('state planned-phase updates table-cell Status via updateCurrentPositionFields', () => { + // cmdStatePlannedPhase uses updateCurrentPositionFields internally; + // verify it also handles the table format. + const content = [ + '---', + 'gsd_state_version: 1.0', + 'status: planning', + '---', + '', + '# GSD State', + '', + '**Status:** Ready to plan', + '**Total Plans in Phase:** 0', + '**Last Activity:** 2026-01-01', + '**Last Activity Description:** initial', + '', + '## Current Position', + '', + '| Field | Value |', + '| --- | --- |', + '| Status | Ready to plan |', + '| Last Activity | 2026-01-01 |', + '', + ].join('\n'); + fs.writeFileSync(statePath, content); + + // Create a minimal phase dir so planned-phase can count plans + const phaseDir = path.join(tmpDir, '.planning', 'phases', '1-test-phase'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '1-01-PLAN.md'), '# Plan 1'); + + const result = runGsdTools(['state', 'planned-phase', '1', '--plan-count', '1'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(statePath, 'utf-8'); + // The Current Position table cell should now be "Ready to execute" + assert.ok( + written.includes('| Status | Ready to execute |'), + 'planned-phase did not update table-cell Status. Content:\n' + written, + ); + }); + + // ── Adversarial / edge cases ────────────────────────────────────────────── + + test('table field with extra whitespace in cells is handled', () => { + const content = [ + '---', + 'gsd_state_version: 1.0', + '---', + '', + '## Current Position', + '', + '| Status | Ready to plan |', + '', + ].join('\n'); + fs.writeFileSync(statePath, content); + + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'extra-whitespace table cell not matched'); + }); + + test('updating one row in a multi-row table does not corrupt adjacent rows', () => { + // Regression: updating `Status` must leave the `Phase` row untouched. + // NOTE: values containing literal '|' (e.g., "blocked | waiting") are NOT + // supported — the current value regex [^|\n]*? stops at the first pipe. + // Escaped-pipe values are out of scope for single-token status fields. + const content = [ + '---', + 'gsd_state_version: 1.0', + '---', + '', + '## Current Position', + '', + '| Status | Ready to plan |', + '| Phase | 3 |', + '', + ].join('\n'); + fs.writeFileSync(statePath, content); + + // Normal replacement — verify Phase row is untouched + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok(written.includes('| Phase | 3 |'), 'Phase row was corrupted during Status update'); + }); + + test('CRLF line endings in table format are handled', () => { + const content = buildTableFormatState({ status: 'Ready to plan' }).replace(/\n/g, '\r\n'); + fs.writeFileSync(statePath, content); + + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, true, 'CRLF table format not handled'); + }); + + test('missing STATE.md returns updated:false gracefully', () => { + // No STATE.md written — verify the command does not throw + const missingDir = createTempProject('gsd-1162-missing-'); + try { + const result = runGsdTools(['state', 'update', 'Status', 'Ready to execute'], missingDir); + const parsed = JSON.parse(result.output); + assert.equal(parsed.updated, false, 'missing STATE.md should return updated:false'); + } finally { + cleanup(missingDir); + } + }); +}); + +describe('regressions: table-format STATE.md (#1162) — updateCurrentPositionFields preserve-authored invariants', () => { + let tmpDir; + let statePath; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1162-f2-'); + statePath = path.join(tmpDir, '.planning', 'STATE.md'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // Helper: a STATE.md with table-format Current Position section. + // We use planned-phase to exercise updateCurrentPositionFields indirectly, + // because that is the call-site that writes Status/Last Activity. + function buildMixedFormatState(opts) { + const { + status = 'Ready to plan', + lastActivity = '2026-01-01', + } = opts || {}; + return [ + '---', + 'gsd_state_version: 1.0', + 'status: planning', + '---', + '', + '# GSD State', + '', + '**Status:** Ready to plan', + '**Total Plans in Phase:** 0', + `**Last Activity:** ${lastActivity}`, + '**Last Activity Description:** initial', + '', + '## Current Position', + '', + '| Field | Value |', + '| --- | --- |', + `| Status | ${status} |`, + `| Last Activity | ${lastActivity} |`, + '', + ].join('\n'); + } + + // (a) Custom Status in table format must NOT be overwritten by planned-phase. + test('(Finding 2a) custom Status in table format is preserved by updateCurrentPositionFields', () => { + // "Blocked: waiting on infra" is executor-authored — not in KNOWN_TEMPLATE_DEFAULTS. + // planned-phase calls updateCurrentPositionFields with status="Ready to execute". + // The table-format branch must honour the same guard as the inline branch: + // only overwrite when the existing value is a known template default. + const content = buildMixedFormatState({ status: 'Blocked: waiting on infra', lastActivity: '2026-01-01' }); + fs.writeFileSync(statePath, content); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '2-test-phase'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '2-01-PLAN.md'), '# Plan\n'); + + const result = runGsdTools(['state', 'planned-phase', '2', '--plan-count', '1'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('| Status | Blocked: waiting on infra |'), + 'Custom Status was overwritten by updateCurrentPositionFields table branch.\nContent:\n' + written, + ); + assert.ok( + !written.includes('| Status | Ready to execute |'), + 'Custom Status replaced with Ready to execute in table branch.\nContent:\n' + written, + ); + }); + + // (b) Narrative Last Activity in table format must NOT be overwritten. + test('(Finding 2b) narrative Last Activity in table format is preserved', () => { + // "2026-02-15 -- blocked" has trailing prose — executor-authored. + // planned-phase calls updateCurrentPositionFields with today's ISO date. + // Must be preserved. + const content = buildMixedFormatState({ status: 'Ready to plan', lastActivity: '2026-02-15 -- blocked' }); + fs.writeFileSync(statePath, content); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '2-test-phase'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '2-01-PLAN.md'), '# Plan\n'); + + const result = runGsdTools(['state', 'planned-phase', '2', '--plan-count', '1'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('| Last Activity | 2026-02-15 -- blocked |'), + 'Narrative Last Activity was overwritten in table branch.\nContent:\n' + written, + ); + }); + + // (c) Known-default Status and bare-date Last Activity ARE updated. + test('(Finding 2c) known-default Status and bare-date Last Activity ARE updated in table format', () => { + // "Ready to plan" is a known default; "2026-01-01" is a bare ISO date. + // Both should be replaced by planned-phase. + const content = buildMixedFormatState({ status: 'Ready to plan', lastActivity: '2026-01-01' }); + fs.writeFileSync(statePath, content); + + const phaseDir = path.join(tmpDir, '.planning', 'phases', '2-test-phase'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '2-01-PLAN.md'), '# Plan\n'); + + const result = runGsdTools(['state', 'planned-phase', '2', '--plan-count', '1'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const written = fs.readFileSync(statePath, 'utf-8'); + assert.ok( + written.includes('| Status | Ready to execute |'), + 'Known-default Status not updated in table branch.\nContent:\n' + written, + ); + // Last Activity should be today's date (not 2026-01-01) + assert.ok( + !written.includes('| Last Activity | 2026-01-01 |'), + 'Bare-date Last Activity was NOT updated in table branch.\nContent:\n' + written, + ); + }); +});