chore(#2143): prohibition-with-teeth + migrate remaining ad-hoc table sites — Phase 4 (final) (#2253)
* chore(#2143): prohibition-with-teeth + migrate remaining table sites — Phase 4 Phase 4 of epic #2143 (ADR-2143 §7). Completes the markdown table/mutation consolidation by (a) giving the ad-hoc-parsing prohibition teeth and (b) migrating the last ad-hoc table sites onto the shared seam. - src/markdown-table.cts: new formatting-preserving `updateTableCell` primitive (self-contained, ragged-row-tolerant header/delimiter/cell-range scan; splices only the target cell's raw span, preserving all other bytes incl. padding/CRLF; no-op-preserves-padding when a transformer returns the current value). Exports splitTableRow/isDelimiterRow/findTableStartOffset for tolerant reuse. - eslint-rules/no-adhoc-markdown-parsing.cjs: TABLE-REGEX detector extended to `new RegExp(<literal|static-template>)`; new `.replace()`-mutation detector for roadmap/state/content receivers with a table/section-shaped pattern. - scripts/lint-table-schema-drift.cjs (wired into lint:ci): fails if a TABLE_SCHEMA header drifts from its authored table; tests import its logic (single source). - Migrated onto the seam (behaviour-preserving vs pre-Phase-4 HEAD, verified byte-diff old-vs-new): roadmap.cts cmdRoadmapUpdatePlanProgress, phase.cts cmdPhaseComplete + traceability, milestone.cts cmdRequirementsMarkComplete, uat.cts read path, state.cts metrics/decisions/By-Phase. - Incidental correctness gains from the migration: a decoy table can no longer swallow a phase-progress update (## Progress scoping); a ragged neighbouring row no longer silently aborts an edit; completing integer phase N no longer touches a decimal sub-phase N.x row; record-metric no longer drops trailing section content or duplicates the ## Performance Metrics section. - Kept justified allow-adhoc-markdown markers only where genuinely not a table (security.cts <|role|> token) or a loose non-GFM section (uat human-verify). Two orthogonal isolated reviews (correctness/adversarial + security) passed; correctness found 4 behaviour regressions in the first migration pass, all fixed and re-verified byte-identical-or-better vs OLD. Surfaced for maintainer (pre-existing, ambiguous domain logic, NOT changed here): templates/state.md places a By-Phase table under ## Performance Metrics while cmdStateRecordMetric assumes a Plan|Duration|Tasks|Files table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): match traceability row by first-cell value, not Requirement header Phase 4's migration matched the REQUIREMENTS.md traceability row by a column literally named `Requirement` (`row['Requirement']`), but real tables head that column `REQ-ID`. The by-name lookup found nothing, so `phase complete` and `requirements mark-complete` left the Status cell `Pending` (regressed #2769 / #2203, caught by gsd-test — 8 failures, both node 22/24). - src/phase.cts, src/milestone.cts: match the row by its FIRST cell's value (the requirement-ID column) regardless of that column's HEADER name, via `Object.values(row)[0]` (updateTableCell builds the record in header order). This mirrors OLD's first-cell `\|\s*<id>\s*\|` anchor, restoring header-name independence while keeping the seam. - src/milestone.cts hasTable: broadened from `Requirement`-only to also recognize `Requirement ID` / `REQ-ID` / `REQ ID` headers, kept in sync with the now-positional rowMatch/hasRow so a REQ-ID-headed table participates in the ADR-2143 §6 write-set and the #2140 table_unmatched drift check (it was silently omitted before — a checkbox-only partial reconcile against a REQ-ID table could report as fully reconciled). The `Requirement`-headed path is byte-identical to OLD. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#2143): replace stale structural milestone guards with behavioural suite The `milestone.cjs regex global state fix` block was a source-structure guard (allow-test-rule: structural-regression-guard) — it readFileSync'd the compiled milestone.cjs and asserted removed regex idioms (`tablePattern.test`, `afterTable !== reqContent`, `doneTable = new RegExp(...)`). Phase 4's migration deleted those regexes (table update is now updateTableCell), making the assertions obsolete. Per the Test Cleanup rule, replace them in-PR with a behavioural suite driving the compiled CLI: - multi-ID mark-complete flips all IDs (guards the lastIndex/global-state class), - Pending->Complete flip under both `REQ-ID` and `Requirement` headers (#2769), - idempotent already_complete detection with no corruption, - REQ-ID-headed table participates in write_set (traceability entry, applied), - REQ-ID-headed table trips #2140 table_unmatched drift on a missing row. Pruned the now-nonexistent structural-regression-guard entry from the lint-allow-test-rule-refs allowlist (the source-text-is-the-product entry for the same file remains valid). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): backfill PR number 2253 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): record-metric targets its own metrics table, not By-Phase velocity `state record-metric` appended its per-plan row (`| Phase 1 P1 | 5min | 3 tasks | 4 files |`) into the FIRST table under `## Performance Metrics` — which on a real template-derived STATE.md is the By-Phase velocity table `| Phase | Plans | Total | Avg/Plan |`, polluting it on EVERY plan completion (execute-plan.md:414 is a per-plan call). The command's own metrics table is `| Plan | Duration | Tasks | Files |`, which the template does not ship, so the row never reached it; the scaffold branch also emitted a wrong `| Phase | Plan | Duration | Notes |` header matching neither the row nor the canonical table. Pre-existing (predates Phase 4); surfaced while migrating this site and fixed here per no-defer, on the user's explicit go-ahead. - src/state.cts cmdStateRecordMetric: locate the metrics table by its own header shape (`Plan|Duration|Tasks|Files`, via splitTableRow/isDelimiterRow) rather than "first table in the section". When the section exists but has no metrics table (only the By-Phase table), self-heal by appending a fresh **Per-Plan Metrics:** table to the END of the section body — By-Phase table, Recent Trend and footer preserved verbatim, no duplicate `## Performance Metrics` heading, created stays false. Absent-section scaffold header corrected to the canonical `| Plan | Duration | Tasks | Files |`. Ragged-tolerance + None-yet preserved. - Not touching templates/state.md (golden-install-parity hashed) — record-metric self-creates the table on first use instead. Failing-first regression test (tests/state.test.cjs) demonstrates the By-Phase pollution on the pre-fix build, then green after. Verified: no pollution, self- heal idempotency, both-tables isolation, content/heading preservation, flags, None-yet, corrected scaffold header (23-check adversarial harness + all existing record-metric scenarios). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(#2143): deleteSection seam primitive (level-bounded whole-section removal) ADR-2143 §4 shipped withSection/collectSection (replace a section BODY) but no way to DELETE a section (heading + body). Phase 4 suppressed the phase-remove section delete instead of building it. deleteSection(content, predicate, opts) locates the section via the collectSection machinery and splices out from the heading's start offset to the next same-or-higher-level heading — so a level-3 `### Phase N` delete stops at a following level-2 `## Progress`, never past it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase remove no longer deletes ## Progress on last-phase removal updateRoadmapAfterPhaseRemoval deleted a `### Phase N` detail section with a greedy raw regex whose lazy scan, on the LAST phase, ran to EOF and destroyed the following `## Progress` heading and its entire tracking table — silent data loss, uncovered by tests (removal tests only exercised a middle phase). Migrated onto the new deleteSection seam (level-bounded, stops at `## Progress`); dropped the allow-adhoc-markdown SECTION-DELETION suppression. Failing-first regression (tests/phase.test.cjs) removes the LAST phase and asserts the ## Progress heading + table survive; middle-phase removal is byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(#2143): deleteTableRow seam primitive (row removal, ragged-tolerant) Sibling of updateTableCell: locates the first GFM table, matches a DATA row by predicate (ragged-tolerant record build, header order), and splices out that row's whole line preserving every other byte. Returns {ok:false,reason} on no table / no match. Enables migrating the phase-remove Progress-table row delete off its ad-hoc regex (ADR-2143 §7 — the "future row-delete seam" Phase 4 punted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase remove deletes the Progress row via deleteTableRow The Progress-table row delete used a whole-document regex with two defects: (a) `\.?\s` required whitespace after the phase number, so a COMPACT row `|2|Beta|` was never deleted (stale row left behind); (b) unscoped — it could strike a row in a different table (e.g. an earlier `| Phase | Requirements |` table). Migrated onto deleteTableRow, scoped to the `## Progress` section (mirrors deriveProgressFromRoadmap), matching the row by first-cell phase number (integer zero-pad-insensitive; decimal exact; removing `2` never touches `2.5`). Both allow-adhoc-markdown suppressions removed. New behavioural tests: compact unpadded row deleted; padded byte-parity on the surviving rows (their ordinal correctly renumbers via the pre-existing renumber block). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): deleteTableRow leaves no dangling newline on last EOL-less row Deleting the final row of a table with no trailing EOL sliced from the row's start to end-of-string, stranding the newline that terminated the previous line. Back rowStart over the preceding \r?\n in that branch so the table ends cleanly. (Caught by the primitive's own unit test on gsd-test; local scenario checks missed the no-trailing-EOL edge.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): migrate read-only section-collects onto collectSection Six hand-rolled `## Section` read-extract regexes replaced by the collectSection seam (behaviour-preserving; extracted bodies feed the same downstream parsers): state.cts matchSessionSection (## Session / ## Session Continuity) + ## Blockers, smart-entry.cts ## Blockers, audit.cts ## Current Focus + ## Open Questions. Removes 6 allow-adhoc-markdown "pending #1372" suppressions. Incidental fix: the old Session regex `## Session[ \t]*\n` silently failed on a CRLF `## Session\r\n` heading (Windows STATE.md), nulling all session fields; collectSection is CRLF-safe, so session state now resolves on Windows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): fence-safe state-transition section writes + dedup stripFrontmatter - milestoneCompleteCore's `## Current Position` and `## Operator Next Steps` section resets used fence-blind raw regexes that a fenced `##` inside the body could truncate/mis-target (#2130/#2067/#2080 class). Migrated onto a fence-aware tokenizeHeadings-based helper (resetSectionVerbatim) that is byte-identical to the old output on the canonical path (9/9 fixtures) and correctly ignores a fenced fake heading (proven robustness gain). - mutateCurrentPositionFirstTime: hand-rolled locate+splice → collectSection + replaceSection (byte-parity). - stripFrontmatter was inlined byte-identically in state.cts AND state-transition.cts; hoisted the single canonical copy into frontmatter.cts (both call sites now import it) + unit tests — eliminates the divergence risk per CLAUDE.md "Generative Fix Divergence". Removes 3 allow-adhoc-markdown / #1372 markers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): name-address By-Phase sum + uat parse, eslint recall hole, catches - state.cts By-Phase "Total plans completed" sum: positional 2nd-cell regex → name-addressed splitTableRow read (correct on a reordered header, where the old code silently summed the wrong column). Marker removed. - uat.cts parseVerificationItems: loose pipe regex → splitTableRow within the existing table/numbered/bullet union scan (item list byte-identical; does NOT reintroduce the reverted strict-parseMarkdownTable item-drop). Marker removed. - eslint no-adhoc-markdown-parsing: close the `new RegExp(identifier)` recall hole — resolve a const-declared table-shaped regex identifier (mirrors the .replace() detector) + RuleTester cases; param/call args stay out (boundary). - commands.cts: delete a lying comment that claimed the scaffold date "stays on raw UTC / deferred" — #2136 already moved it to realClock.localToday(). - Empty catches (classified, not blind-swept): removed 4 dead try/catch; fixed 3 error-hiding (phase-insert decimal-dir I/O collision now fails loud; phase-remove rename partial-failure surfaced; milestone-archive true count via finally); left best-effort swallows with justification comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): extractFencedBlock seam + migrate api-coverage named fence parseCoverageMatrix extracted its ```coverage fenced block with an ad-hoc regex (the last real allow-adhoc-markdown suppression). Added extractFencedBlock to the markdown-sectionizer seam (reuses stripFencedCode's CommonMark fence engine — info-string match, ~~~/backtick, nesting, indent) and migrated onto it; byte- parity on the parsed CoverageMatrix across 8 fixtures. Only security.cts:367 (a genuine `<|role|>` protocol-token false-positive, not a GFM table) remains marked in src/ — the "prohibition with teeth" goal (nothing grandfathered but a true FP) is met. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): By-Phase row insert is name-addressed (insertTableRow seam) updatePerformanceMetricsSection's INSERT-new-row branch located the By-Phase table with a canonical-column-order-only regex + a hardcoded positional row literal, so on a reordered header it silently inserted nothing — inconsistent with the now name-addressed UPDATE and SUM halves of the same function. Added insertTableRow (markdown-table seam sibling of updateTableCell/deleteTableRow: name-addressed, header-order-agnostic, EOL-preserving) and migrated the branch onto it, mapping By-Phase values by column NAME. Canonical-order output is byte-identical; a reordered header now inserts a correctly-mapped row; a pre-existing CRLF mixed-EOL splice glitch is incidentally fixed. Retired the now-dead byPhaseTablePattern const. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): phase-list checkbox flip via updateBullet seam Added updateBullet (markdown-sectionizer): a fence-aware, offset-tracked single-bullet write primitive (GFM 1–4-space marker tolerance) — the write counterpart to read-only iterateBullets. Migrated mutateMilestonePhase's phase-list checkbox flip (`- [ ] Phase N …` → `- [x] … (completed <date>)`) off its whole-slice regex onto it, same milestone-slice scope + clock seam. Byte-identical across simple / idempotent / metachar-title / double-space / CRLF scenarios. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): scope the Progress-ordinal renumber to ## Progress via seam phase remove's integer-renumber decremented Progress-table phase ordinals with a whole-document `content.replace(/(\|\s*)(\d+)(\.\s)/g, …)` — unscoped, so it also rewrote any `| N. …` cell in an unrelated/decoy table (same class as the batch-2 row-delete scoping bug). Migrated onto updateTableCell, scoped to the ## Progress section, decrementing each affected row's leading phase ordinal by column name. Byte-identical on canonical Progress tables + multi-row + decimal-sibling cases; a decoy `| 3. … |` row before ## Progress is now correctly left untouched. The sibling heading / checkbox-bullet / PLAN.md-filename / Depends-on-prose renumbers are not GFM-table mutations (outside ADR-2143's table/section mandate) — left as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2143): review fixes — scope traceability write, restore Current Position H3-stop Adversarial review of the remediation (BLOCK verdict) — all 9 findings fixed: - F1 (BLOCKER): requirements mark-complete / phase complete flipped the checkbox but NOT the traceability row on the shipped template, because updateTableCell bound to the FIRST table (## Out of Scope, no Status column) instead of the ## Traceability table — the #2140 silent-divergence class, re-introduced by the seam migration and missed by tests (fixtures had Traceability first). Scoped the write + hasRow probe to the ## Traceability section slice (updateTraceability Cell helper) in milestone.cts + phase.cts. Failing-first tests on the Out-of-Scope-before-Traceability layout; the #2769 first-cell match preserved. - F2 (MAJOR): mutateCurrentPositionFirstTime restored to locateCurrentPosition (STOP_H2_PLUS) — collectSection's default H2-stop swallowed a level-3 subsection and the field regexes clobbered it (#2130 class). - F3/F8: Progress-ordinal renumber re-escapes via escapeCell + keys padding recovery by row index (was de-escaping `\|` and losing padding on dup values). - F4: insertTableRow escapes cell values internally. - F5: updateBullet accepts a tab after the marker (`[ \t]{1,4}`). - F7: resetSectionVerbatim consumes CRLF blank lines (byte-parity on CRLF). - F6/F9: corrected two misleading comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): data-loss + CRLF-session user-facing fixes (#2253) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#2143): de-flake the G10 windsurf ReDoS-guard wall-clock assertion The G10 test asserted `elapsedMs < 1000` for a 200k-char payload — a wall-clock assertion (CLAUDE.md: never assert on wall-clock time) that flaked on a loaded node24 bench at ~1.1s. It was redundant: runHook's spawnSync `timeout: 10000` already SIGKILLs a catastrophic-backtracking hook, so the exit-0 assertion is the real ReDoS guard. Removed the timing assertion; kept exit-0 + documented the subprocess-timeout mechanism. Surfaced (not caused) by this branch's gsd-test runs loading the bench; unrelated to the markdown-parsing changes but fixed in place per the no-flaky-tests rule. 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
452
src/state.cts
452
src/state.cts
@@ -27,7 +27,7 @@ const { planningDir, planningPaths } = planningWorkspace;
|
||||
import { realClock } from './clock.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import frontmatter = require('./frontmatter.cjs');
|
||||
const { extractFrontmatter, reconstructFrontmatter } = frontmatter;
|
||||
const { extractFrontmatter, reconstructFrontmatter, stripFrontmatter } = frontmatter;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import scanPhasePlans = require('./plan-scan.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
@@ -46,7 +46,9 @@ import {
|
||||
KNOWN_TEMPLATE_DEFAULTS,
|
||||
stateReplaceFieldIfTemplate,
|
||||
} from './state-document.cjs';
|
||||
import { tokenizeHeadings } from './markdown-sectionizer.cjs';
|
||||
import { tokenizeHeadings, collectSection, replaceSection } from './markdown-sectionizer.cjs';
|
||||
import type { HeadingToken } from './markdown-sectionizer.cjs';
|
||||
import { parseMarkdownTable, updateTableCell, deleteTableRow, insertTableRow, splitTableRow, isDelimiterRow } from './markdown-table.cjs';
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -274,8 +276,11 @@ function _stateLockBodyPid(lockPath: string): number | null {
|
||||
// Monotonic sequence for unique stale-steal rename targets (no crypto dependency).
|
||||
let _stateStealSeq = 0;
|
||||
|
||||
// Hoisted to module scope — compiled once, not per call (#320). Stateless (/i, used with .match).
|
||||
const byPhaseTablePattern = /(\|\s*Phase\s*\|\s*Plans\s*\|\s*Total\s*\|\s*Avg\/Plan\s*\|[ \t]*\r?\n\|(?:[- :\t]+\|)+[ \t]*\r?\n)((?:[ \t]*\|[^\n]*\n)*)(?=\r?\n|$)/i;
|
||||
// The `byPhaseTablePattern` regex hoisted here for #320 (canonical-column-
|
||||
// ORDER-only By-Phase table match) is retired (#2245 audit): its last caller
|
||||
// — updatePerformanceMetricsSection's row-INSERT branch — now locates the
|
||||
// table via findTableStartOffset/insertTableRow, name-addressed and
|
||||
// header-order-agnostic like the update/sum halves of the same function.
|
||||
|
||||
// ─── ADR-1372 T6: seam-based section splice helper ───────────────────────────
|
||||
|
||||
@@ -540,33 +545,133 @@ function cmdStateRecordMetric(cwd: string, options: StateRecordMetricOptions, ra
|
||||
let _recorded = false;
|
||||
let created = false;
|
||||
readModifyWriteStateMd(statePath, (content) => {
|
||||
// Find Performance Metrics section and its table
|
||||
const metricsPattern = /(##\s*Performance Metrics[\s\S]*?\n\|[^\n]+\n\|[-|\s]+\n)([\s\S]*?)(?=\n##|\n$|$)/i; // allow-adhoc-markdown: metrics-table write-path section-collect in state.cts; pending collectSection migration #1372
|
||||
const metricsMatch = content.match(metricsPattern);
|
||||
|
||||
const newRow = `| Phase ${phase} P${plan} | ${duration} | ${tasks || '-'} tasks | ${files || '-'} files |`;
|
||||
|
||||
if (metricsMatch) {
|
||||
let tableBody = metricsMatch[2].trimEnd();
|
||||
// Find the "## Performance Metrics" section via the markdown-sectionizer
|
||||
// seam (ADR-2143 §7) — supersedes the prior hand-rolled section+table
|
||||
// regex.
|
||||
const metricsSection = collectSection(content, (h) => /^performance metrics$/i.test(h.text.trim()));
|
||||
|
||||
if (tableBody.trim() === '' || tableBody.includes('None yet')) {
|
||||
tableBody = newRow;
|
||||
} else {
|
||||
tableBody = tableBody + '\n' + newRow;
|
||||
const eol = metricsSection && /\r\n/.test(metricsSection.body) ? '\r\n' : '\n';
|
||||
const lines = metricsSection ? metricsSection.body.split(/\r?\n/) : [];
|
||||
|
||||
// Locate THIS command's OWN metrics table by its HEADER shape, using the
|
||||
// exact same splitTableRow/isDelimiterRow header/delimiter-shape checks
|
||||
// `parseMarkdownTable` uses. A live "## Performance Metrics" section
|
||||
// (gsd-core/templates/state.md:39-56) also carries the "By Phase"
|
||||
// velocity table (`| Phase | Plans | Total | Avg/Plan |`) — the prior
|
||||
// "first table in the section" targeting spliced every per-plan row into
|
||||
// THAT table instead, polluting it on EVERY plan completion
|
||||
// (execute-plan.md:414 calls record-metric per-plan) (#2245/#2143).
|
||||
// Matching the header cells to this command's own canonical
|
||||
// `Plan | Duration | Tasks | Files` shape (case-insensitive/trimmed)
|
||||
// finds the right table regardless of what else shares the section, and
|
||||
// deliberately does NOT require `parseMarkdownTable(...).ok` (which
|
||||
// additionally requires every DATA row's cell count to match the
|
||||
// header) — a single ragged sibling row (a hand-edited stray/extra pipe)
|
||||
// must not blind this scan (#2245 Blocker 2 parity with the other
|
||||
// Phase-4 ragged-tolerance fixes: updateTableCell / findTableStartOffset).
|
||||
const METRICS_HEADER = ['plan', 'duration', 'tasks', 'files'];
|
||||
let headerIdx = -1;
|
||||
for (let i = 0; i < lines.length - 1; i++) {
|
||||
const trimmed = lines[i].trim();
|
||||
if (!trimmed.startsWith('|') || trimmed.indexOf('|', 1) === -1) continue;
|
||||
const delimiterLine = lines[i + 1];
|
||||
if (delimiterLine === undefined || !delimiterLine.trim().startsWith('|')) continue;
|
||||
const headerCells = splitTableRow(lines[i]);
|
||||
const delimiterCells = splitTableRow(delimiterLine);
|
||||
if (!isDelimiterRow(delimiterCells) || delimiterCells.length !== headerCells.length) continue;
|
||||
const normalized = headerCells.map((cell) => cell.trim().toLowerCase());
|
||||
const isMetricsHeader = normalized.length === METRICS_HEADER.length
|
||||
&& normalized.every((cell, idx) => cell === METRICS_HEADER[idx]);
|
||||
if (isMetricsHeader) { headerIdx = i; break; }
|
||||
}
|
||||
const hasTable = headerIdx !== -1;
|
||||
|
||||
if (metricsSection && hasTable) {
|
||||
const delimiterIdx = headerIdx + 1;
|
||||
const prefixLines = lines.slice(0, delimiterIdx + 1);
|
||||
|
||||
// Ragged-tolerant row scan: every consecutive `|`-prefixed line
|
||||
// following the delimiter counts as an existing row REGARDLESS of its
|
||||
// cell count matching the header — a ragged sibling row must never
|
||||
// blind this scan to the table's true last row (unlike
|
||||
// `parsedTable.value.rows.length`, which this replaces). Anchored to
|
||||
// the METRICS table's OWN header/delimiter (`headerIdx` above), never
|
||||
// the section's first table (#2245/#2143).
|
||||
let lastRowIdx = delimiterIdx;
|
||||
for (let i = delimiterIdx + 1; i < lines.length; i++) {
|
||||
if (!lines[i].trim().startsWith('|')) break;
|
||||
lastRowIdx = i;
|
||||
}
|
||||
const rowCount = lastRowIdx - delimiterIdx;
|
||||
|
||||
_recorded = true;
|
||||
return content.replace(metricsPattern, (_match, header: string) => `${header}${tableBody}\n`);
|
||||
|
||||
let newBody: string;
|
||||
if (rowCount > 0) {
|
||||
// Splice the new row immediately after the table's LAST existing data
|
||||
// row — every other byte of the section, INCLUDING any trailing prose
|
||||
// that follows the table (e.g. the default template's "**Recent
|
||||
// Trend:**" subsection + "*Updated after each plan completion*"
|
||||
// footer), is preserved verbatim. The prior implementation truncated
|
||||
// the section body to header+delimiter+rows+newRow, silently dropping
|
||||
// everything that followed the table on a live STATE.md (#2245
|
||||
// Blocker 1 — a per-plan path, run after every plan execution).
|
||||
// `lastRowIdx` (computed above by the ragged-tolerant scan) already
|
||||
// equals `delimiterIdx + rowCount` by construction.
|
||||
const before = lines.slice(0, lastRowIdx + 1);
|
||||
const after = lines.slice(lastRowIdx + 1);
|
||||
newBody = [...before, newRow, ...after].join(eol);
|
||||
} else {
|
||||
// No existing data rows (e.g. a "None yet" placeholder line instead of
|
||||
// a real row) — replace the placeholder/table-body remainder with the
|
||||
// new row, matching the section's prior (verified) collapse-to-
|
||||
// first-row behavior for an otherwise-empty table.
|
||||
// No trailing eol here: replaceSection's `content.slice(bodyEnd)`
|
||||
// already supplies the newline(s) that followed the (trimEnd()-ed)
|
||||
// section body.
|
||||
newBody = prefixLines.join(eol) + eol + newRow;
|
||||
}
|
||||
|
||||
return replaceSection(content, metricsSection, newBody);
|
||||
}
|
||||
|
||||
// Section absent — DWIM: auto-create canonical ## Performance Metrics scaffold,
|
||||
// then append the row. Matches state begin-phase / advance-plan DWIM behavior.
|
||||
if (metricsSection) {
|
||||
// Section EXISTS but carries no metrics table of its own — e.g. a live
|
||||
// STATE.md whose "## Performance Metrics" section holds only the
|
||||
// By-Phase velocity table (gsd-core/templates/state.md:48). Self-heal
|
||||
// by appending a fresh Per-Plan Metrics table to the END of the
|
||||
// section body — every existing byte (By-Phase table, Recent Trend,
|
||||
// footer) is preserved verbatim, and no second "## Performance
|
||||
// Metrics" heading is introduced. The section already existed, so
|
||||
// `created` stays false (#2245/#2143).
|
||||
_recorded = true;
|
||||
const newBody = metricsSection.body
|
||||
+ eol + '**Per-Plan Metrics:**'
|
||||
+ eol + eol
|
||||
+ '| Plan | Duration | Tasks | Files |'
|
||||
+ eol
|
||||
+ '|------|----------|-------|-------|'
|
||||
+ eol
|
||||
+ newRow
|
||||
+ eol;
|
||||
return replaceSection(content, metricsSection, newBody);
|
||||
}
|
||||
|
||||
// Section absent (or malformed) — DWIM: auto-create canonical
|
||||
// ## Performance Metrics scaffold, then append the row. Matches state
|
||||
// begin-phase / advance-plan DWIM behavior. Header corrected to this
|
||||
// command's own canonical shape (`Plan | Duration | Tasks | Files`) —
|
||||
// the prior scaffold's `| Phase | Plan | Duration | Notes |` header
|
||||
// matched neither the appended row's shape nor the canonical table
|
||||
// above (#2245/#2143).
|
||||
const scaffold = [
|
||||
'',
|
||||
'## Performance Metrics',
|
||||
'',
|
||||
'| Phase | Plan | Duration | Notes |',
|
||||
'|-------|------|----------|-------|',
|
||||
'| Plan | Duration | Tasks | Files |',
|
||||
'|------|----------|-------|-------|',
|
||||
newRow,
|
||||
'',
|
||||
].join('\n');
|
||||
@@ -1120,15 +1225,22 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
|
||||
* Match the session section body from a STATE.md body. #1101: recognise the
|
||||
* bootstrap `## Session Continuity` heading but PREFER the normalized `## Session`
|
||||
* block when both exist (legacy duplicate files), so the reader agrees with the
|
||||
* writer (which updates `## Session` first). `(?:^|\n)` line-anchors (kept out of
|
||||
* `/m` so `$` stays end-of-string for the `(?=\n##|$)` section boundary), which
|
||||
* excludes an h3 `### Session Continuity`; the trailing-` Archive` boundary still
|
||||
* excludes `## Session Continuity Archive` (preserving the #2444 scoping).
|
||||
* Returns the match whose group 1 is the section body, or null.
|
||||
* writer (which updates `## Session` first). Level-2-exact heading match
|
||||
* (excludes an h3 `### Session Continuity`); the exact `'session continuity'`
|
||||
* text match still excludes `## Session Continuity Archive` (preserving the
|
||||
* #2444 scoping). Migrated onto the `collectSection` seam (#2143 audit,
|
||||
* epic #2143): CRLF-safe — the prior hand-rolled `[ \t]*\n` regex silently
|
||||
* failed to match a CRLF `## Session\r\n` heading line (the `\r` broke the
|
||||
* `[ \t]*\n` boundary); `tokenizeHeadings` strips the trailing `\r` before
|
||||
* heading-text extraction, so this now matches CRLF headings too.
|
||||
* Returns the section body, or null.
|
||||
*/
|
||||
function matchSessionSection(body: string): RegExpMatchArray | null {
|
||||
return body.match(/(?:^|\n)##[ \t]*Session[ \t]*\n([\s\S]*?)(?=\n##|$)/i) // allow-adhoc-markdown: read-only session-section extract in state.cts; pending collectSection migration #1372
|
||||
|| body.match(/(?:^|\n)##[ \t]*Session Continuity[ \t]*\n([\s\S]*?)(?=\n##|$)/i); // allow-adhoc-markdown: read-only session-continuity section extract in state.cts; pending collectSection migration #1372
|
||||
function matchSessionSection(body: string): string | null {
|
||||
const isSession = (h: HeadingToken): boolean => h.level === 2 && h.text.trim().toLowerCase() === 'session';
|
||||
const isSessionContinuity = (h: HeadingToken): boolean => h.level === 2 && h.text.trim().toLowerCase() === 'session continuity';
|
||||
const section = collectSection(body, isSession, { levelBounded: true })
|
||||
?? collectSection(body, isSessionContinuity, { levelBounded: true });
|
||||
return section ? section.body : null;
|
||||
}
|
||||
|
||||
function parseProsePhaseField(value: string | null): { phase: string | null; name: string | null } {
|
||||
@@ -1200,14 +1312,15 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
|
||||
const totalPlansInPhase = totalPlansRaw ? parseInt(totalPlansRaw, 10) : null;
|
||||
const progressPercent = progressRaw ? parseInt(progressRaw.replace('%', ''), 10) : null;
|
||||
|
||||
// Extract decisions table
|
||||
// Extract decisions table — via the markdown-sectionizer/markdown-table
|
||||
// seams (ADR-2143 §7), cells addressed by column NAME rather than a
|
||||
// hand-rolled section+table regex.
|
||||
const decisions: Array<{ phase: string; summary: string; rationale: string }> = [];
|
||||
const decisionsMatch = body.match(/##\s*Decisions Made[\s\S]*?\n\|[^\n]+\n\|[-|\s]+\n([\s\S]*?)(?=\n##|\n$|$)/i); // allow-adhoc-markdown: read-only decisions-table section-collect in state.cts; pending collectSection migration #1372
|
||||
if (decisionsMatch) {
|
||||
const tableBody = decisionsMatch[1];
|
||||
const rows = tableBody.trim().split('\n').filter(r => r.includes('|'));
|
||||
for (const row of rows) {
|
||||
const cells = row.split('|').map(c => c.trim()).filter(Boolean);
|
||||
const decisionsSection = collectSection(body, (h) => /^decisions made$/i.test(h.text.trim()));
|
||||
const decisionsTable = decisionsSection ? parseMarkdownTable(decisionsSection.body) : null;
|
||||
if (decisionsTable && decisionsTable.ok) {
|
||||
for (const row of decisionsTable.value.rows) {
|
||||
const cells = decisionsTable.value.columns.map((c) => (row[c] ?? '').trim()).filter(Boolean);
|
||||
if (cells.length >= 3) {
|
||||
decisions.push({
|
||||
phase: cells[0],
|
||||
@@ -1220,10 +1333,9 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
|
||||
|
||||
// Extract blockers list
|
||||
const blockers: string[] = [];
|
||||
const blockersMatch = body.match(/##\s*Blockers\s*\n([\s\S]*?)(?=\n##|$)/i); // allow-adhoc-markdown: read-only blockers section-collect in state.cts; pending collectSection migration #1372
|
||||
if (blockersMatch) {
|
||||
const blockersSection = blockersMatch[1];
|
||||
const items = blockersSection.match(/^-\s+(.+)$/gm) || [];
|
||||
const blockersSection = collectSection(body, (h) => h.level === 2 && h.text.trim().toLowerCase() === 'blockers', { levelBounded: true });
|
||||
if (blockersSection) {
|
||||
const items = blockersSection.body.match(/^-\s+(.+)$/gm) || [];
|
||||
for (const item of items) {
|
||||
blockers.push(item.replace(/^-\s+/, '').trim());
|
||||
}
|
||||
@@ -1239,8 +1351,8 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
|
||||
// #1101: prefer the canonical `## Session` block, falling back to the bootstrap
|
||||
// `## Session Continuity` heading. See matchSessionSection for the anchoring.
|
||||
const sessionMatch = matchSessionSection(body);
|
||||
if (sessionMatch) {
|
||||
const sessionSection = sessionMatch[1];
|
||||
if (sessionMatch !== null) {
|
||||
const sessionSection = sessionMatch;
|
||||
// Accept both `**Last Date:**` (canonical template form) and `**Last session:**`
|
||||
// (the form written by the DWIM auto-create / normalize path added for #944).
|
||||
const lastDateMatch = sessionSection.match(/\*\*Last Date:\*\*\s*(.+)/i)
|
||||
@@ -1360,18 +1472,20 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re
|
||||
// #1101: prefer the canonical `## Session` block, falling back to the bootstrap
|
||||
// `## Session Continuity` heading. See matchSessionSection for the anchoring.
|
||||
const sessionSectionMatch = matchSessionSection(bodyContent);
|
||||
const sessionBodyScope = sessionSectionMatch ? sessionSectionMatch[1] : bodyContent;
|
||||
const sessionBodyScope = sessionSectionMatch ?? bodyContent;
|
||||
const stoppedAt = stateExtractField(sessionBodyScope, 'Stopped At') || stateExtractField(sessionBodyScope, 'Stopped at');
|
||||
const pausedAt = stateExtractField(bodyContent, 'Paused At');
|
||||
|
||||
let milestone: string | null = null;
|
||||
let milestoneName: string | null = null;
|
||||
if (cwd) {
|
||||
try {
|
||||
const info = getMilestoneInfo(cwd);
|
||||
milestone = info.version;
|
||||
milestoneName = info.name;
|
||||
} catch { /* intentionally empty */ }
|
||||
// DEAD catch removed (#2245 audit): getMilestoneInfo has its own outer
|
||||
// try/catch (roadmap-parser.cts) that already swallows every internal
|
||||
// failure and always returns a MilestoneInfo — it never throws, so this
|
||||
// wrapper could never be triggered.
|
||||
const info = getMilestoneInfo(cwd);
|
||||
milestone = info.version;
|
||||
milestoneName = info.name;
|
||||
}
|
||||
|
||||
let totalPhases: number | null = totalPhasesRaw ? parseInt(totalPhasesRaw, 10) : null;
|
||||
@@ -1508,6 +1622,13 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re
|
||||
completedPlans = cached.completedPlans;
|
||||
milestoneUnbounded = cached.milestoneBounded === false;
|
||||
}
|
||||
/* best-effort (#2245 audit): this is a READ path building STATE.md's
|
||||
* display frontmatter. The real throw source is fs.readdirSync(phasesDir)
|
||||
* a few lines up — an inaccessible/racily-removed phases dir must not
|
||||
* crash `state show`; on failure this simply keeps whatever
|
||||
* frontmatter-derived totals/completedPhases/etc. were already set
|
||||
* above, a graceful degrade rather than a corrupted write (nothing is
|
||||
* persisted from this block). */
|
||||
} catch { /* intentionally empty */ }
|
||||
}
|
||||
|
||||
@@ -1552,20 +1673,6 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re
|
||||
return fm;
|
||||
}
|
||||
|
||||
function stripFrontmatter(content: string): string {
|
||||
// Strip ALL frontmatter blocks at the start of the file.
|
||||
// Handles CRLF line endings and multiple stacked blocks (corruption recovery).
|
||||
// Greedy: keeps stripping ---...--- blocks separated by optional whitespace.
|
||||
let result = content;
|
||||
|
||||
while (true) {
|
||||
const stripped = result.replace(/^\s*---\r?\n[\s\S]*?\r?\n---\s*/, '');
|
||||
if (stripped === result) break;
|
||||
result = stripped;
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
function syncStateFrontmatter(content: string, cwd: string | undefined): string {
|
||||
// Read existing frontmatter BEFORE stripping — it may contain values
|
||||
// that the body no longer has (e.g., Status field removed by an agent).
|
||||
@@ -1954,7 +2061,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
|
||||
// A stale "Stopped at:" in a non-Session section (e.g. Session Continuity
|
||||
// Archive prose) must not interfere with the delta comparison.
|
||||
const preSessionMatch = matchSessionSection(preBody);
|
||||
const preSessionScope = preSessionMatch ? preSessionMatch[1] : preBody;
|
||||
const preSessionScope = preSessionMatch ?? preBody;
|
||||
const preBodyStoppedAt = stateExtractField(preSessionScope, 'Stopped At') || stateExtractField(preSessionScope, 'Stopped at');
|
||||
|
||||
// ADR-1769 Phase 6 / #1743 / #1695: snapshot the body source for the curated
|
||||
@@ -1988,7 +2095,7 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
|
||||
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
|
||||
// consistent with the pre-transform snapshot above and buildStateFrontmatter.
|
||||
const postSessionMatch = matchSessionSection(postBody);
|
||||
const postSessionScope = postSessionMatch ? postSessionMatch[1] : postBody;
|
||||
const postSessionScope = postSessionMatch ?? postBody;
|
||||
const postBodyStoppedAt = stateExtractField(postSessionScope, 'Stopped At') || stateExtractField(postSessionScope, 'Stopped at');
|
||||
// ADR-1769 Phase 6 / #1695: post-transform body Phase source for the
|
||||
// current_phase_name delta comparison.
|
||||
@@ -2157,6 +2264,38 @@ function cmdSignalResume(cwd: string, raw: boolean): void {
|
||||
|
||||
// ─── Gate Functions (STATE.md consistency enforcement) ────────────────────────
|
||||
|
||||
/**
|
||||
* Find the character offset where the FIRST GFM table whose header is a
|
||||
* superset of `required` column names begins (order-independent; extra
|
||||
* columns tolerated) — the position-aware counterpart to markdown-table's
|
||||
* `findTableWithColumns`, used to scope `updateTableCell` (which always
|
||||
* operates on "the first table in its input") to the RIGHT table when an
|
||||
* unrelated earlier table (that doesn't itself name every required column)
|
||||
* may precede it in the same document. Returns `null` when no such table is
|
||||
* found. Never trips the table-regex fingerprint (no `[^|]` cell-capture
|
||||
* class) and never throws.
|
||||
*
|
||||
* Ragged-tolerant (#2245 Blocker 2): accepts the offset the moment a HEADER
|
||||
* line names every required column — it deliberately does NOT additionally
|
||||
* require `parseMarkdownTable(text.slice(m.index)).ok`, which validates every
|
||||
* DATA row's cell count. A ragged sibling row anywhere in the table used to
|
||||
* make that whole-table parse fail, so the offset came back `null` and the
|
||||
* caller's `updateTableCell` calls (which scope to this offset) never even
|
||||
* ran against an otherwise-perfectly-findable row.
|
||||
*/
|
||||
function findTableStartOffset(text: string, required: string[]): number | null {
|
||||
const lineRe = /^[ \t]*\|.*\|[ \t]*$/gm;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = lineRe.exec(text)) !== null) {
|
||||
const trimmed = m[0].trim();
|
||||
const cols = trimmed.replace(/^\|/, '').replace(/\|$/, '').split(/(?<!\\)\|/).map((c) => c.trim());
|
||||
if (required.every((rq) => cols.includes(rq))) {
|
||||
return m.index;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Update the ## Performance Metrics section in STATE.md content.
|
||||
* Increments Velocity totals and upserts a By Phase table row.
|
||||
@@ -2168,46 +2307,125 @@ function updatePerformanceMetricsSection(content: string, cwd: string, phaseNum:
|
||||
// the same phase again upserts the same row, so the column sum is stable. The previous
|
||||
// blind-add (prevTotal + summaryCount) re-read the cumulative total each call and
|
||||
// double-counted on every re-run. (#1582)
|
||||
const byPhaseMatch = content.match(byPhaseTablePattern);
|
||||
if (byPhaseMatch) {
|
||||
let tableBody = byPhaseMatch[2].trim();
|
||||
//
|
||||
// Located by column NAME via the markdown-table seam (ADR-2143 §7) —
|
||||
// supersedes the prior module-level byPhaseTablePattern regex for the
|
||||
// existence/lookup half of this logic.
|
||||
const byPhaseCols = ['Phase', 'Plans', 'Total', 'Avg/Plan'];
|
||||
// Ragged-tolerant (#2245 Blocker 2): scope to the table's start offset
|
||||
// (findTableStartOffset — itself now ragged-tolerant, see above) rather
|
||||
// than gating existence/lookup on findTableWithColumns, which requires the
|
||||
// WHOLE table to parse — a ragged row for a DIFFERENT phase used to
|
||||
// silently no-op every phase's upsert.
|
||||
const tableStart = findTableStartOffset(content, byPhaseCols);
|
||||
if (tableStart !== null) {
|
||||
// Match the existing row for this phase, tolerating leading-zero padding in either
|
||||
// direction (#1659): canonicalize a numeric phase to its integer form so a seeded
|
||||
// "| 05 |" row is upserted (not duplicated) by `phase complete 5`, and vice-versa.
|
||||
const phaseNumStr = String(phaseNum);
|
||||
const canonCell = /^\d+$/.test(phaseNumStr) ? `0*${Number(phaseNumStr)}` : escapeRegex(phaseNumStr);
|
||||
const phaseRowPattern = new RegExp(`^\\|\\s*${canonCell}\\s*\\|.*$`, 'm');
|
||||
const newRow = `| ${phaseNum} | ${summaryCount} | - | - |`;
|
||||
const phaseCellRe = new RegExp(`^${canonCell}$`, 'i');
|
||||
const rowMatch = (row: Record<string, string>): boolean => phaseCellRe.test((row['Phase'] ?? '').trim());
|
||||
|
||||
if (phaseRowPattern.test(tableBody)) {
|
||||
// Update existing row
|
||||
tableBody = tableBody.replace(phaseRowPattern, newRow);
|
||||
const before = content.slice(0, tableStart);
|
||||
let tableText = content.slice(tableStart);
|
||||
|
||||
// Ragged-tolerant existence probe: a no-op updateTableCell write on the
|
||||
// identifying "Phase" column (its own tolerant row scan) decides whether
|
||||
// this phase's row already exists, without requiring every OTHER row in
|
||||
// the table to also parse cleanly.
|
||||
let rowExists = false;
|
||||
const existsProbe = updateTableCell(tableText, rowMatch, 'Phase', (current) => {
|
||||
rowExists = true;
|
||||
return current;
|
||||
});
|
||||
void existsProbe;
|
||||
|
||||
if (rowExists) {
|
||||
// Update existing row — one updateTableCell call per column (Phase
|
||||
// itself may also change shape, e.g. "05" -> "5" per #1659).
|
||||
const phaseResult = updateTableCell(tableText, rowMatch, 'Phase', ` ${phaseNum} `);
|
||||
if (phaseResult.ok) tableText = phaseResult.value;
|
||||
const plansResult = updateTableCell(tableText, rowMatch, 'Plans', ` ${summaryCount} `);
|
||||
if (plansResult.ok) tableText = plansResult.value;
|
||||
const totalResult = updateTableCell(tableText, rowMatch, 'Total', ' - ');
|
||||
if (totalResult.ok) tableText = totalResult.value;
|
||||
const avgResult = updateTableCell(tableText, rowMatch, 'Avg/Plan', ' - ');
|
||||
if (avgResult.ok) tableText = avgResult.value;
|
||||
|
||||
content = before + tableText;
|
||||
} else {
|
||||
// Remove placeholder row and add new row
|
||||
tableBody = tableBody.replace(/^\|\s*-\s*\|\s*-\s*\|\s*-\s*\|\s*-\s*\|$/m, '').trim();
|
||||
tableBody = tableBody ? tableBody + '\n' + newRow : newRow;
|
||||
}
|
||||
// Row doesn't exist — INSERT a new row. Row insertion (unlike a cell
|
||||
// update) is outside updateTableCell's scope (ADR-2143 §7 Phase 4);
|
||||
// `insertTableRow` (markdown-table.cjs) is its name-addressed,
|
||||
// header-order-agnostic sibling (#2245 audit: this used to locate the
|
||||
// table via `byPhaseTablePattern`, a canonical-column-ORDER-only regex,
|
||||
// and build the row as a hardcoded positional literal — so a reordered/
|
||||
// superset By-Phase header, already tolerated above by
|
||||
// findTableStartOffset and read by-NAME in the update/sum halves,
|
||||
// silently inserted NOTHING).
|
||||
//
|
||||
// Drop a lone all-placeholder row first (e.g. the freshly-scaffolded
|
||||
// "| - | - | - | - |" seed row) — same convention the prior
|
||||
// canonical-order path used, generalized to any column order/count:
|
||||
// a row whose every PRESENT cell is "-" is the placeholder.
|
||||
const placeholderRow = (row: Record<string, string>): boolean =>
|
||||
Object.values(row).every((cell) => cell.trim() === '-');
|
||||
const withoutPlaceholder = deleteTableRow(tableText, placeholderRow);
|
||||
if (withoutPlaceholder.ok) tableText = withoutPlaceholder.value;
|
||||
|
||||
content = content.replace(byPhaseTablePattern, (_match, tableHeader: string) => `${tableHeader}${tableBody}\n`);
|
||||
// Map the By-Phase values onto the table's ACTUAL header columns by
|
||||
// NAME — an unrecognized column (a superset header) falls back to "-",
|
||||
// insertTableRow's default.
|
||||
const valueFor = (col: string): string | undefined => {
|
||||
if (col === 'Phase') return String(phaseNum);
|
||||
if (col === 'Plans') return String(summaryCount);
|
||||
if (col === 'Total' || col === 'Avg/Plan') return '-';
|
||||
return undefined;
|
||||
};
|
||||
const insertResult = insertTableRow(tableText, valueFor);
|
||||
if (insertResult.ok) tableText = insertResult.value;
|
||||
|
||||
content = before + tableText;
|
||||
}
|
||||
}
|
||||
|
||||
// Velocity: Total plans completed — DERIVED as the sum of the By-Phase Plans column
|
||||
// (the second cell) across all data rows. Idempotent by construction (re-running phase
|
||||
// complete upserts the same row → same sum) and self-healing (a hand-edited inflated
|
||||
// total is corrected to the true sum on the next completion). When the By-Phase table
|
||||
// is absent, leave the velocity total unchanged rather than guess. (#1582)
|
||||
// across all data rows. Idempotent by construction (re-running phase complete upserts
|
||||
// the same row → same sum) and self-healing (a hand-edited inflated total is corrected
|
||||
// to the true sum on the next completion). When the By-Phase table is absent, leave the
|
||||
// velocity total unchanged rather than guess. (#1582)
|
||||
//
|
||||
// Ragged-tolerant AND name-addressed (#2245 audit): each data row is split via
|
||||
// `splitTableRow` and its "Plans" cell located by the HEADER's own column
|
||||
// order (not a fixed ordinal), so a reordered/superset By-Phase header is
|
||||
// summed correctly instead of silently reading the wrong cell. A row that's
|
||||
// too short to physically contain the "Plans" column is skipped, not
|
||||
// treated as an error — mirrors updateTableCell's ragged-row tolerance
|
||||
// (a hand-edited/ragged row for one phase must not blank out the derived
|
||||
// total for every phase). Still scoped via findTableStartOffset so the RIGHT
|
||||
// table is summed when an earlier unrelated table also has a "Phase"
|
||||
// column (#2012).
|
||||
if (/Total plans completed:\s*(\d+|\[N\])/.test(content)) {
|
||||
const tableForSum = content.match(byPhaseTablePattern);
|
||||
if (tableForSum) {
|
||||
const sumTableStart = findTableStartOffset(content, byPhaseCols);
|
||||
if (sumTableStart !== null) {
|
||||
const tableLines = content.slice(sumTableStart).split(/\r?\n/);
|
||||
const headerCells = splitTableRow(tableLines[0] ?? '');
|
||||
const plansIdx = headerCells.indexOf('Plans');
|
||||
let sum = 0;
|
||||
for (const row of tableForSum[2].split(/\r?\n/)) {
|
||||
// Data rows look like `| <phase> | <plans> | … |`, optionally indented (the
|
||||
// byPhaseTablePattern data-row capture allows `[ \t]*` leading whitespace, so the
|
||||
// sum must too or hand-edited/legacy indented rows are silently skipped — #1582
|
||||
// codex review). Header (`| Phase | Plans | …`) and separator (`| --- | --- | …`)
|
||||
// rows have a non-numeric second cell and are skipped; non-numeric cells → 0.
|
||||
const cellMatch = row.match(/^\s*\|\s*[^|]+\s*\|\s*(\d+)\s*\|/);
|
||||
if (cellMatch) sum += parseInt(cellMatch[1], 10);
|
||||
if (plansIdx !== -1) {
|
||||
// The delimiter row is skipped by NAME (isDelimiterRow), not by a
|
||||
// hardcoded "always line index 1" assumption, so this stays
|
||||
// self-consistent with the ragged-tolerant read below.
|
||||
const delimiterCells = splitTableRow(tableLines[1] ?? '');
|
||||
const dataStart = isDelimiterRow(delimiterCells) ? 2 : 1;
|
||||
for (const row of tableLines.slice(dataStart)) {
|
||||
if (!row.trim().startsWith('|')) break;
|
||||
const cells = splitTableRow(row);
|
||||
if (plansIdx < cells.length && /^\d+$/.test(cells[plansIdx])) {
|
||||
sum += parseInt(cells[plansIdx], 10);
|
||||
}
|
||||
}
|
||||
}
|
||||
content = content.replace(
|
||||
/Total plans completed:\s*(\d+|\[N\])/,
|
||||
@@ -2344,7 +2562,10 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
|
||||
warnings.push(`Status drift: STATE.md says "${status}" but ${vf} shows verification passed — phase may be complete`);
|
||||
drift['verification_status'] = { state_status: status, verification: 'passed' };
|
||||
}
|
||||
} catch { /* intentionally empty */ }
|
||||
} catch { /* best-effort (#2245 audit): cmdStateValidate is a diagnostic
|
||||
* warnings scan across N VERIFICATION.md files — one unreadable file
|
||||
* (permission/race) must not abort the scan of the rest; it's simply
|
||||
* excluded from drift detection. */ }
|
||||
}
|
||||
|
||||
// Check if all plans have summaries but status still says executing
|
||||
@@ -2355,7 +2576,13 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch { /* intentionally empty */ }
|
||||
} catch { /* best-effort (#2245 audit): cmdStateValidate is a read-only
|
||||
* diagnostic scan of the current phase's directory (readdirSync +
|
||||
* scanPhasePlans). A disk-scan failure here means drift detection for
|
||||
* this phase is skipped for this run, degrading to "no warnings from
|
||||
* that scan" rather than crashing the validate command — the same
|
||||
* degrade-on-scan-failure pattern buildStateFrontmatter's own disk scan
|
||||
* already uses. */ }
|
||||
}
|
||||
|
||||
const valid = warnings.length === 0;
|
||||
@@ -2451,29 +2678,30 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b
|
||||
|
||||
// Determine total phases from ROADMAP (may be larger than realized disk dirs).
|
||||
// Mirrors the logic in buildStateFrontmatter so both report consistent percents (#3242 Bug B).
|
||||
// DEAD catch removed (#2245 audit): every operation in this block is a regex
|
||||
// exec/test over an already-read string plus pure Set/Math ops — none of
|
||||
// which can throw — so the try/catch could never be triggered.
|
||||
let syncTotalPhases: number | null = null;
|
||||
try {
|
||||
let roadmapPhaseCount = 0;
|
||||
if (syncRoadmapScope !== null) {
|
||||
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
|
||||
const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = phaseHeadingPattern.exec(syncRoadmapScope)) !== null) {
|
||||
// Only count tokens that contain at least one digit — excludes
|
||||
// pure-word section headings (Overview, Details) while keeping
|
||||
// numeric phases (01, 05.1) and project-code IDs (PROJ-42).
|
||||
if (!/\d/.test(m[1])) continue;
|
||||
// #1514: retired/folded phases are struck through; exclude from total.
|
||||
if (syncRetiredPhaseNums.has(phaseKeyFromToken(m[1]))) continue;
|
||||
roadmapPhaseCount++;
|
||||
}
|
||||
let roadmapPhaseCount = 0;
|
||||
if (syncRoadmapScope !== null) {
|
||||
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
|
||||
const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = phaseHeadingPattern.exec(syncRoadmapScope)) !== null) {
|
||||
// Only count tokens that contain at least one digit — excludes
|
||||
// pure-word section headings (Overview, Details) while keeping
|
||||
// numeric phases (01, 05.1) and project-code IDs (PROJ-42).
|
||||
if (!/\d/.test(m[1])) continue;
|
||||
// #1514: retired/folded phases are struck through; exclude from total.
|
||||
if (syncRetiredPhaseNums.has(phaseKeyFromToken(m[1]))) continue;
|
||||
roadmapPhaseCount++;
|
||||
}
|
||||
if (roadmapPhaseCount > 0) {
|
||||
syncTotalPhases = Math.max(entries.length, roadmapPhaseCount);
|
||||
} else {
|
||||
syncTotalPhases = entries.length;
|
||||
}
|
||||
} catch { /* intentionally empty */ }
|
||||
}
|
||||
if (roadmapPhaseCount > 0) {
|
||||
syncTotalPhases = Math.max(entries.length, roadmapPhaseCount);
|
||||
} else {
|
||||
syncTotalPhases = entries.length;
|
||||
}
|
||||
|
||||
// ADR-1769 Phase 7: the body writes (Total Plans in Phase, Progress bar, Last
|
||||
// Activity) are the pure `syncCore` in src/state-transition.cts.
|
||||
|
||||
Reference in New Issue
Block a user