golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#1156): support table-format STATE.md and insert missing roadmap plan rows (#1172)

Browse Source 
* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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-</details>) 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 <noreply@anthropic.com>

* fix(#1163): scope missing-plan detection to active milestone; preserve authored state fields in table format

- roadmap.cts: compute activeRegion (post-</details> slice) once and use it
  for BOTH missingPlans detection and row insertion, so archived <details>
  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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* docs(#1156): add changeset fragment for table-format state + roadmap insert fix

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-13 21:56:18 -04:00
committed by GitHub
parent 4db185da74
commit 3827054954
6 changed files with 1185 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

94
src/roadmap.cts
View File

@@ -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-</details>) 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 <details>
// 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('</details>');
const activeRegion = lastDetailsClose === -1
? roadmapContent
: roadmapContent.slice(lastDetailsClose + '</details>'.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 <details> 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-</details>) 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 </details> blocks.
const regionA = scopedReplace(activeRegion, insertRowsPatternA);
withRows = regionA !== activeRegion ? regionA : scopedReplace(activeRegion, insertRowsPatternB);
} else {
const beforeDetails = roadmapContent.slice(0, lastDetailsClose + '</details>'.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({

54
src/state-document.cts
View File

@@ -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;
}

84
src/state.cts
View File

@@ -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}`);