chore(#2143): withSection bounded-mutation seam + phase.cts migration — Phase 2 (#2250)

* chore(#2143): bounded-mutation seam (withSection/withPhaseSection) + phase.cts migration — Phase 2

Phase 2 of epic #2143 (ADR-2143 §4): add a bounded-mutation primitive so a
per-phase ROADMAP edit is structurally confined to that phase's own section,
and migrate the phase-scoped mutation sites in `phase.cts` onto it.

- `src/markdown-sectionizer.cts`: `withSection(content, target, edit, opts?)` —
  resolves a section via `collectSection` and applies `edit` to ONLY that
  section's body, re-serialising via `replaceSection`. The edit callback sees
  only the section body, so any regex it runs is physically confined.
- `src/roadmap-parser.cts`: `withPhaseSection(content, phaseId, edit)` —
  resolves a phase's `### Phase N` detail-section heading via the #2121
  phase-id source and delegates to `withSection`. Heading match is anchored to
  the heading start (a sibling phase whose title mentions the number is not
  hijacked) and bounds at the next ATX heading of any level (`levelBounded:false`).
- `src/phase.cts`: `mutateMilestonePhase`'s plan-count and per-plan-checkbox
  writes now route through `withPhaseSection` — structurally retiring the
  #2130 / #2067 / #2080 boundary-crossing class for these sites. The phase-LIST
  checkbox is intentionally left milestone-slice-scoped (it lives outside any
  `### Phase N` detail section). Cross-phase renumbering is untouched.
- Property test (fast-check): editing phase k leaves every sibling section
  byte-identical; regression tests for title-collision + mixed heading depth.

Behaviour-preserving (verified by old-vs-new differential runs on real
fixtures). Extend-never-mutate (ADR-2143 §2). Registration: CONTEXT.md +
docs/INVENTORY.md export lists.

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

* chore(#2243): backfill changeset PR number (#2250)

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-07-13 20:39:22 -04:00
committed by GitHub
parent d49ac81306
commit efd04716da
8 changed files with 500 additions and 23 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2250
---
**ROADMAP phase edits can no longer escape their section** — completing a phase updated its plan count and per-plan checkboxes with whole-document regexes that could bleed into a neighbouring phase; those per-phase writes are now structurally bounded to the phase own section via a new `withSection` / `withPhaseSection` seam (#2130, #2067, #2080). (#2250)

View File

@@ -140,13 +140,13 @@ Primary installer for all runtimes. Single production file: `bin/install.js` (ge
Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`).
### Markdown Sectionizer
Canonical markdown-structure parsing seam (`gsd-core/bin/lib/markdown-sectionizer.cjs`, generated from `src/markdown-sectionizer.cts`). Pure functions, Node built-ins only. Exports: `stripFencedCode(content) → { text, unterminatedFence }` (CommonMark-correct state machine, CRLF-safe, signals unterminated fences); `tokenizeHeadings(content) → HeadingToken[]` (ATX headings outside fenced blocks, `{ level, text, line, offset }`); `collectSections(content, stopPredicate) → Section[]` (line-by-line section collection driven by a heading predicate); `collectSection(content, headingPredicate, { levelBounded, stripFences }) → Section | null` (single named section with level-bounded stop); `iterateBullets(sectionText) → BulletItem[]` (dash/checkbox/numbered markers with indented continuation); `extractTaggedBlocks(content, tagName) → string[]` (inner text of every `<tagName>…</tagName>` block in document order, tagName regex-escaped, caller decides fence-stripping — generalises `decisions.cts`'s bespoke extractor for T1); `replaceSection(content, section, newBody) → string` (pure character-offset splice using `Section.bodyStart`/`bodyEnd` for read-modify-write callers — eliminates T6 `state.cts`'s 7× inline `content.replace` pattern). `Section` carries `bodyStart`/`bodyEnd` offsets for `replaceSection`. ADR-1372 (epic #1372) establishes this seam and a tiered migration plan (T0–T7) to retire the 8+ ad-hoc markdown parsers and ~20 inline section-collects across `src/*.cts`. New `src/*.cts` modules must import this seam instead of hand-rolling fence strippers or heading-regex section walks (enforced by the `no-adhoc-markdown-parsing` ESLint rule landing in tier T7).
Canonical markdown-structure parsing seam (`gsd-core/bin/lib/markdown-sectionizer.cjs`, generated from `src/markdown-sectionizer.cts`). Pure functions, Node built-ins only. Exports: `stripFencedCode(content) → { text, unterminatedFence }` (CommonMark-correct state machine, CRLF-safe, signals unterminated fences); `tokenizeHeadings(content) → HeadingToken[]` (ATX headings outside fenced blocks, `{ level, text, line, offset }`); `collectSections(content, stopPredicate) → Section[]` (line-by-line section collection driven by a heading predicate); `collectSection(content, headingPredicate, { levelBounded, stripFences }) → Section | null` (single named section with level-bounded stop); `iterateBullets(sectionText) → BulletItem[]` (dash/checkbox/numbered markers with indented continuation); `extractTaggedBlocks(content, tagName) → string[]` (inner text of every `<tagName>…</tagName>` block in document order, tagName regex-escaped, caller decides fence-stripping — generalises `decisions.cts`'s bespoke extractor for T1); `replaceSection(content, section, newBody) → string` (pure character-offset splice using `Section.bodyStart`/`bodyEnd` for read-modify-write callers — eliminates T6 `state.cts`'s 7× inline `content.replace` pattern); `withSection(content, target, edit) → string` (resolve the section whose heading matches `target` — exact heading text or a `HeadingToken` predicate — and run `edit(body)` against ONLY that section's body before splicing the result back; bounded no-op when no heading matches or `edit` returns the same/non-string body; ADR-2143 §4 structurally retires the #2130/#2067/#2080 boundary-crossing class by confining any regex the caller runs to the matched section). `Section` carries `bodyStart`/`bodyEnd` offsets for `replaceSection`. ADR-1372 (epic #1372) establishes this seam and a tiered migration plan (T0–T7) to retire the 8+ ad-hoc markdown parsers and ~20 inline section-collects across `src/*.cts`. New `src/*.cts` modules must import this seam instead of hand-rolling fence strippers or heading-regex section walks (enforced by the `no-adhoc-markdown-parsing` ESLint rule landing in tier T7).
### Markdown Table Model
Canonical GFM table parsing + schema registry seam (`gsd-core/bin/lib/markdown-table.cjs`, generated from `src/markdown-table.cts`; ADR-2143, epic #2143). Pure functions, Node built-ins only, string-in/value-out, no I/O. Exports: `parseMarkdownTable(sectionText) → Result<MarkdownTable>` (parses the first GFM pipe table found; typed `{ok:false,reason}` parse errors for no-table, missing/misaligned delimiter row, and ragged data rows — never silently drops or coerces a malformed row); `MarkdownTable` (`{columns: string[], rows: Record<string,string>[]}`, rows addressed by column name, not position); `Result<T>` (`{ok:true,value}\|{ok:false,reason}` — deliberately distinct from command-routing-hub's dispatch `Result` `{ok,data\|kind}`; the two never mix); `TABLE_SCHEMAS` (`Record<string, CanonicalTableVariant[]>` — the canonical column-header variants for every GFM table GSD parses or generates: `RoadmapProgress` flat/milestone-grouped, `RequirementsTraceability`, `QuickTasks` no-status/with-status, `Security` trust-boundaries/threat-register/accepted-risks/audit-trail); `matchTableSchema(columns) → {id,label}\|null` (resolves a parsed header back to its canonical schema by exact column-name/order match). This registry is the single source of truth for ROADMAP/STATE/SECURITY canonical tables — a parity test (`tests/markdown-table.test.cjs`) asserts every variant's header appears verbatim in the template/workflow file that generates it, so the registry and templates can never silently drift (ADR-2143 §3 Generative-Fix-Divergence guard). `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` is the first consumer: it locates the Progress section via the Markdown Sectionizer's `collectSection` and reads cells by column NAME through this seam, fixing #2137 (the prior position-anchored regex assumed `Status` was always the 3rd cell, which broke for the 5-column milestone-grouped `Milestone` variant).
### Roadmap Parser Module
Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`).
Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`, `withPhaseSection`). `withPhaseSection(content, phaseId, edit)` resolves a phase's `### Phase N` detail-section heading via the #2121 phase-id source (`phaseMarkdownRegexSource`) and delegates to the markdown-sectionizer seam's `withSection`, so a per-phase ROADMAP edit is bounded to that phase's own section (ADR-2143 §4). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`, `markdown-sectionizer`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`).
### Core Utilities Module
Module owning the shared low-level utility primitives extracted from Core: POSIX path normalization (`toPosixPath`), filesystem scanning (`detectSubRepos`, `readSubdirectories`, `getPhaseFileStats`, `pathExistsInternal`), and small pure helpers (`generateSlugInternal`, `extractOneLinerFromBody`, `filterPlanFiles`, `filterSummaryFiles`, `extractCanonicalPlanId`, `timeAgo`). Depends only on Node built-ins and already-leafed modules (`phase-id` for `comparePhaseNum`, `planning-workspace` for `findContextMdIn`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2c (#877) as the shared leaf that unblocks the phase-locator fs-search extraction (2d); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/core-utils.cjs` (generated from `src/core-utils.cts`).

View File

@@ -465,7 +465,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
| `legacy-cleanup.cjs` | Detect and remove leftover get-shit-done-cc artifacts; exports `planLegacyCleanup` (pure scan) and `applyLegacyCleanup` (thin IO applier) that root out stale files from the old package across every GSD-managed runtime config directory (#607) |
| `loop-host-contract.cjs` | Generated Loop Host Contract — 12 loop points, per-step agent roles, and core artifacts for the five-step pipeline (discuss/plan/execute/verify/ship); emitted by `scripts/gen-loop-host-contract.cjs --write` (ADR-894 §3); consumed by `gen-capability-registry.cjs` |
| `loop-resolver.cjs` | Loop Extension Point resolver — ADR-857 phase 3c/6 registry-consuming query; given a canonical loop point, filters `byLoopPoint` by resolved Capability State plus config activation (`when` key traversal with prototype-pollution guard), returns `{ point, activeHooks, rendered }` envelope; `resolveLoopHooks` and `renderLoopHooks` are pure (no I/O); command surface: `gsd-tools loop render-hooks <point> [--config-dir <path>]` |
| `markdown-sectionizer.cjs` | Canonical markdown-structure parsing seam (ADR-1372, epic #1372) — pure, Node built-ins only; exports `stripFencedCode` (CommonMark-correct fence stripper, CRLF-safe), `tokenizeHeadings` (ATX headings outside fenced blocks), `collectSections`/`collectSection` (line-by-line section collection with `bodyStart`/`bodyEnd` offsets), `iterateBullets` (dash/checkbox/numbered markers), `extractTaggedBlocks` (inner text of `<tag>…</tag>` blocks, caller decides fence-stripping), and `replaceSection` (pure character-offset body splice for read-modify-write callers); foundation for T0–T7 migration tiers retiring 8+ ad-hoc parsers |
| `markdown-sectionizer.cjs` | Canonical markdown-structure parsing seam (ADR-1372, epic #1372) — pure, Node built-ins only; exports `stripFencedCode` (CommonMark-correct fence stripper, CRLF-safe), `tokenizeHeadings` (ATX headings outside fenced blocks), `collectSections`/`collectSection` (line-by-line section collection with `bodyStart`/`bodyEnd` offsets), `iterateBullets` (dash/checkbox/numbered markers), `extractTaggedBlocks` (inner text of `<tag>…</tag>` blocks, caller decides fence-stripping), `replaceSection` (pure character-offset body splice for read-modify-write callers), and `withSection` (resolve a section by heading/predicate and run an edit callback against ONLY its body, splicing the result back — ADR-2143 §4 bounded mutation); foundation for T0–T7 migration tiers retiring 8+ ad-hoc parsers |
| `markdown-table.cjs` | Canonical GFM table model + `TABLE_SCHEMAS` registry seam (ADR-2143, epic #2143) — pure, Node built-ins only; exports `parseMarkdownTable(sectionText) → Result<MarkdownTable>` (parses the first GFM pipe table, typed parse errors for ragged/malformed rows rather than silent coercion), `MarkdownTable` (`{columns, rows}`, rows addressed by column name), `Result<T>` (`{ok:true,value}\|{ok:false,reason}` — distinct from command-routing-hub's dispatch `Result`), `TABLE_SCHEMAS` (canonical column-header variants for `RoadmapProgress`/`RequirementsTraceability`/`QuickTasks`/`Security` tables), and `matchTableSchema(columns) → {id,label}\|null` (resolves parsed headers back to a canonical schema); consumed by `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` (fixes #2137, the 5-column milestone-grouped Progress table) |
| `milestone.cjs` | Milestone archival, requirements marking |
| `model-catalog.cjs` | CJS adapter over the shared model catalog JSON; exports canonical runtime tier defaults, agent profile maps, alias maps, and routing metadata for all CLI consumers |

View File

@@ -62,6 +62,13 @@ export interface Section {
bodyEnd: number;
}
/** Options shared by `collectSection` and `withSection` (see `collectSection`'s doc comment). */
export interface CollectSectionOptions {
levelBounded?: boolean;
stopAtLevel?: number;
stripFences?: boolean;
}
/** Recognised bullet markers. */
export type BulletMarker = 'dash' | 'checkbox-unchecked' | 'checkbox-checked' | 'numbered';
@@ -340,7 +347,7 @@ export function collectSections(
export function collectSection(
content: string,
headingPredicate: (heading: HeadingToken) => boolean,
opts: { levelBounded?: boolean; stopAtLevel?: number; stripFences?: boolean } = {},
opts: CollectSectionOptions = {},
): Section | null {
if (typeof content !== 'string' || content.length === 0) return null;
@@ -618,5 +625,47 @@ export function replaceSection(content: string, section: Section, newBody: strin
return content.slice(0, section.bodyStart) + newBody + content.slice(section.bodyEnd);
}
// ─── withSection ──────────────────────────────────────────────────────────────
/**
* Locate the section whose heading matches `target`, run `edit` against ONLY
* that section's body, and splice the result back into `content`.
*
* `target` is either an exact (trimmed) heading-text match or a predicate
* function over `HeadingToken`. `edit` receives ONLY the section body — so any
* regex it runs is physically confined to that section — an edit cannot cross
* a section boundary (ADR-2143 §4, structurally retires the #2130/#2067/#2080
* boundary-crossing class, where a hand-rolled regex escaped its intended
* section and mutated a sibling/shipped/backticked-literal occurrence instead).
*
* Bounded no-op behaviour (Phase 3 of ADR-2143 adds fail-loud diagnostics on
* top of this):
* - No heading matches `target` → `content` is returned unchanged.
* - `edit` returns a non-string, or returns the same string it was given →
* `content` is returned unchanged (no-op splice avoided).
*
* `opts` is forwarded verbatim to `collectSection` (see its doc comment for
* `levelBounded` / `stopAtLevel` / `stripFences` semantics) — it lets a caller
* whose heading levels are non-uniform (e.g. a mix of `###`/`####` phase
* headings) choose the correct section-end rule instead of relying on the
* `levelBounded: true` default.
*/
export function withSection(
content: string,
target: string | ((h: HeadingToken) => boolean),
edit: (body: string) => string,
opts: CollectSectionOptions = {},
): string {
if (typeof content !== 'string') return content;
const predicate = typeof target === 'function'
? target
: (h: HeadingToken) => h.text.trim() === target.trim();
const section = collectSection(content, predicate, opts);
if (!section) return content; // bounded no-op on miss (Phase 3 adds fail-loud)
const newBody = edit(section.body);
if (typeof newBody !== 'string' || newBody === section.body) return content;
return replaceSection(content, section, newBody);
}
// Consumers: require('../gsd-core/bin/lib/markdown-sectionizer.cjs')
// Named CJS exports are the canonical surface (ADR-457 .cts → .cjs build-at-publish).

View File

@@ -44,7 +44,7 @@ import phaseLocatorMod = require('./phase-locator.cjs');
const { findPhaseInternal, getArchivedPhaseDirs } = phaseLocatorMod;
// eslint-disable-next-line @typescript-eslint/no-require-imports -- roadmap-parser.cjs is an export= CommonJS module
import roadmapParserMod = require('./roadmap-parser.cjs');
const { stripShippedMilestones, extractCurrentMilestone, getMilestonePhaseFilter, currentMilestoneRawRanges } = roadmapParserMod;
const { stripShippedMilestones, extractCurrentMilestone, getMilestonePhaseFilter, currentMilestoneRawRanges, withPhaseSection } = roadmapParserMod;
// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module
import planningWorkspace = require('./planning-workspace.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module
@@ -1479,6 +1479,12 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
// inline / backticked prose literal cannot match. Milestone-scoped below
// (mutateMilestonePhase) so a Backlog entry or a same-numbered shipped-
// milestone phase cannot be flipped either.
// ADR-2143 §4 note: this is the phase-LIST checkbox — it lives in the
// milestone's `- [ ] Phase N: …` checklist, OUTSIDE any `### Phase N`
// detail section, so there is no section for withPhaseSection to bind
// to. Left as a milestone-slice-scoped regex (not migrated to the
// sectionizer seam); see planCountBodyPattern below for the sites that
// WERE migrated.
const checkboxPattern = new RegExp(
`^[ \\t]*(-\\s*\\[)[ ](\\]\\s*(?:\\*\\*)?\\s*Phase\\s+${phaseEscaped}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s][^\\n]*)`,
'im',
@@ -1518,10 +1524,14 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
roadmapContent = roadmapContent.replace(tableRowPattern, updateProgressRow);
}
const planCountPattern = new RegExp(
`(#{2,4}\\s*Phase\\s+${phaseEscaped}(?:(?!\\n#{1,4}\\s)[\\s\\S])*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`,
'i',
);
// ADR-2143 §4: the plan-count write is now routed through
// withPhaseSection (see mutateMilestonePhase below), which hands this
// pattern ONLY phase N's own detail-section body — so the pattern no
// longer needs its own `#{2,4}\s*Phase\s+N` anchor + skip-ahead-past-
// interior-headings lookahead; the section boundary itself confines
// the match (the #2067/#2200 boundary-crossing class is now
// structurally impossible for this site rather than regex-enforced).
const planCountBodyPattern = /(\*\*Plans:\*\*\s*)[^\n]+/i;
const phaseInfoSummaries = phaseInfo['summaries'] as string[];
@@ -1534,17 +1544,25 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
const mutateMilestonePhase = (slice: string): string => {
let s = slice;
s = s.replace(checkboxPattern, `$1x$2 (completed ${today})`);
s = s.replace(planCountPattern, `$1${summaryCount}/${planCount} plans complete`);
for (const summaryFile of phaseInfoSummaries) {
const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', '');
if (!planId) continue;
const planEscaped = escapeRegex(planId);
const planCheckboxPattern = new RegExp(
`(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`,
'i',
);
s = s.replace(planCheckboxPattern, '$1x$2');
}
// ADR-2143 §4: the plan-count write and the per-plan checkbox flips
// are both scoped to phase N's OWN detail section via
// withPhaseSection — the edit callback below only ever sees that
// section's body, so neither regex can escape into a sibling
// phase's section, a shipped milestone, or a Backlog entry.
s = withPhaseSection(s, phaseNum, (body) => {
let b = body.replace(planCountBodyPattern, `$1${summaryCount}/${planCount} plans complete`);
for (const summaryFile of phaseInfoSummaries) {
const planId = summaryFile.replace('-SUMMARY.md', '').replace('SUMMARY.md', '');
if (!planId) continue;
const planEscaped = escapeRegex(planId);
const planCheckboxPattern = new RegExp(
`(-\\s*\\[) (\\]\\s*(?:\\*\\*)?${planEscaped}(?:\\*\\*)?)`,
'i',
);
b = b.replace(planCheckboxPattern, '$1x$2');
}
return b;
});
return s;
};

View File

@@ -13,6 +13,7 @@
* - ./phase-id.cjs (escapeRegex, phaseMarkdownRegexSource)
* - ./planning-workspace.cjs (planningDir)
* - ./shell-command-projection.cjs (platformReadSync)
* - ./markdown-sectionizer.cjs (tokenizeHeadings, stripTaggedBlocks, withSection)
*/
import fs from 'node:fs';
@@ -32,7 +33,8 @@ const {
import planningWorkspace = require('./planning-workspace.cjs');
const { planningDir } = planningWorkspace;
import { platformReadSync } from './shell-command-projection.cjs';
import { tokenizeHeadings, stripTaggedBlocks } from './markdown-sectionizer.cjs';
import { tokenizeHeadings, stripTaggedBlocks, withSection } from './markdown-sectionizer.cjs';
import type { HeadingToken } from './markdown-sectionizer.cjs';
// ─── Roadmap milestone scoping ───────────────────────────────────────────────
@@ -200,6 +202,49 @@ function replaceInCurrentMilestone(content: string, pattern: RegExp, replacement
return before + after.replace(pattern, replacement);
}
/**
* Resolve a single phase's detail-section heading (`### Phase N: …`, any level
* 1–6, via the #2121 phase-id source) and run `edit` against ONLY that
* section's body. Delegates to `withSection` (markdown-sectionizer.cjs), so a
* per-phase ROADMAP edit is structurally bounded to that phase's own section —
* it cannot escape into a sibling phase, a shipped-milestone `<details>` block,
* or a backticked prose literal (ADR-2143 §4).
*
* `content` is expected to already be scoped to the current milestone's raw
* range(s) by the caller (see `currentMilestoneRawRanges`) — `withPhaseSection`
* composes with that milestone-level scoping rather than replacing it.
*
* The matched phase number must be delimited by whitespace, a colon, an
* open-paren tag, or end-of-heading — never a bare `\b`. A trailing `\b` sits
* between the last digit and a following `.` or letter, so it would let a
* query for phase `1` prefix-match a decimal sub-phase heading like
* `### Phase 1.1: Sub` or a distinct suffixed phase like `### Phase 1A: …`.
*
* The phase token must additionally anchor to the START of the heading text
* (after an optional leading `[tag]`, mirroring `findRoadmapPhaseInContent`
* below) — never merely appear anywhere in it. Without this anchor, a query
* for phase `1` would match a SIBLING phase whose own TITLE happens to
* mention "Phase 1" (e.g. `### Phase 3: Migrate off Phase 1 legacy pipeline`),
* and — because `collectSection` picks the first matching heading in document
* order — that sibling would be hijacked instead of the real Phase 1 section.
*
* The section body is bounded by `{ levelBounded: false }`: it ends at the
* next ATX heading of ANY level, not merely a heading at or above the phase
* heading's own level. Real ROADMAPs are not guaranteed to use a uniform
* phase-heading level, so a level-bounded stop could fold a deeper sibling
* heading (e.g. a `####` phase following a `###` phase) into this phase's
* body and let `edit` reach into it.
*/
function withPhaseSection(
content: string,
phaseId: unknown,
edit: (body: string) => string,
): string {
const src = phaseMarkdownRegexSource(phaseId);
const headingRe = new RegExp(`^\\s*(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+${src}(?=[\\s:(]|$)`, 'i');
return withSection(content, (h: HeadingToken) => headingRe.test(h.text), edit, { levelBounded: false });
}
// ─── Roadmap phase lookup ─────────────────────────────────────────────────────
// #2199: a bullet/checkbox phase entry, e.g. `- [ ] **Phase 36 — Authentication**`
@@ -650,4 +695,5 @@ export = {
getMilestoneInfo,
getMilestonePhaseFilter,
currentMilestoneRawRanges,
withPhaseSection,
};

View File

@@ -5,7 +5,7 @@
*
* Module: gsd-core/bin/lib/markdown-sectionizer.cjs
* Exports: stripFencedCode, tokenizeHeadings, collectSections, collectSection,
* iterateBullets, extractTaggedBlocks, replaceSection
* iterateBullets, extractTaggedBlocks, replaceSection, withSection
*
* Covers the parser QA matrix from CONTRIBUTING.md §'Parser and project-file inputs':
* - LF vs CRLF line endings
@@ -16,7 +16,9 @@
* - All three bullet markers (dash/checkbox/numbered) + indented continuation lines
* - Empty/whitespace/non-string input
*
* Includes a fast-check property test (stripFencedCode idempotence invariant).
* Includes fast-check property tests: stripFencedCode idempotence invariant,
* and withSection's ADR-2143 §4 bounded-mutation guarantee (an edit confined to
* one section cannot alter any other section, even with a greedy regex).
* The parity guard for the T0-era tracked duplication (stripFencedCode vs
* uat-predicate _stripFencedBlocks) was removed in T5: uat-predicate now imports
* the seam directly, so the guard would compare the seam to itself.
@@ -35,6 +37,7 @@ const {
extractTaggedBlocks,
stripTaggedBlocks,
replaceSection,
withSection,
} = require('../gsd-core/bin/lib/markdown-sectionizer.cjs');
// ─── stripFencedCode ──────────────────────────────────────────────────────────
@@ -1053,6 +1056,210 @@ describe('extractTaggedBlocks: nested same-name tag behavior (#2128 stop-at-next
});
});
// ─── withSection ───────────────────────────────────────────────────────────────
describe('withSection', () => {
test('edits only the targeted section, leaving surrounding sections untouched', () => {
const content = '## Intro\nIntro body.\n## Name\nOld name body.\n## Footer\nFooter body.\n';
const result = withSection(content, 'Name', (body) => body.replace('Old name body.', 'New name body.'));
assert.ok(result.includes('## Intro\nIntro body.'), 'Intro section preserved verbatim');
assert.ok(result.includes('## Name\nNew name body.'), 'Name section updated');
assert.ok(!result.includes('Old name body.'), 'old body removed');
assert.ok(result.includes('## Footer\nFooter body.'), 'Footer section preserved verbatim');
});
test('a section not present in content leaves content unchanged (bounded no-op)', () => {
const content = '## A\nBody A\n## B\nBody B\n';
const result = withSection(content, 'Nonexistent', (body) => body + ' MUTATED');
assert.equal(result, content, 'no matching heading -> content returned unchanged');
});
test('predicate form: target may be a HeadingToken predicate function', () => {
const content = '## Alpha\nAlpha body\n## Beta\nBeta body\n';
const result = withSection(content, (h) => h.level === 2 && h.text === 'Beta', (body) => body.toUpperCase());
assert.ok(result.includes('## Beta\nBETA BODY'), 'predicate-matched section edited');
assert.ok(result.includes('## Alpha\nAlpha body'), 'non-matched section untouched');
});
test('edit returning the identical body is a no-op (no splice performed)', () => {
const content = '## A\nBody A\n## B\nBody B\n';
const result = withSection(content, 'A', (body) => body);
assert.equal(result, content, 'identical body -> no-op');
});
test('edit returning a non-string is a no-op (defensive)', () => {
const content = '## A\nBody A\n## B\nBody B\n';
// Deliberately malformed edit callback (returns a number, not a string).
const result = withSection(content, 'A', () => 42);
assert.equal(result, content, 'non-string return -> no-op');
});
test('non-string content is returned unchanged', () => {
assert.equal(withSection(null, 'A', (b) => b + 'x'), null);
assert.equal(withSection(undefined, 'A', (b) => b + 'x'), undefined);
});
test('ADR-2143 §4: a greedy regex inside the edit callback cannot cross a section boundary', () => {
// Build a 3-section document; run a maximally-greedy regex (`[\s\S]*`) inside
// the edit callback for section 2. The callback only ever sees section 2's
// body, so sections 1 and 3 must remain byte-identical.
const content = [
'## Section One',
'alpha content line 1',
'alpha content line 2',
'## Section Two',
'beta content to be replaced',
'## Section Three',
'gamma content line 1',
'gamma content line 2',
].join('\n') + '\n';
const before = collectSection(content, (h) => h.text === 'Section One');
const afterSectionThree = collectSection(content, (h) => h.text === 'Section Three');
assert.ok(before !== null && afterSectionThree !== null);
const result = withSection(content, 'Section Two', (body) => body.replace(/[\s\S]*/, 'REPLACED ENTIRELY'));
const resultOne = collectSection(result, (h) => h.text === 'Section One');
const resultThree = collectSection(result, (h) => h.text === 'Section Three');
assert.equal(resultOne.body, before.body, 'Section One byte-identical after greedy edit on Section Two');
assert.equal(resultThree.body, afterSectionThree.body, 'Section Three byte-identical after greedy edit on Section Two');
assert.ok(result.includes('## Section Two\nREPLACED ENTIRELY'), 'Section Two was replaced as intended');
});
});
describe('withSection: property-based tests', () => {
// Anchored phase-heading predicate mirroring roadmap-parser.cjs's
// withPhaseSection fix: the phase token must sit at the START of the
// heading text, so a section whose TITLE merely mentions another phase
// number is never matched by that other phase's query.
const anchoredPhasePredicate = (k) => {
const re = new RegExp(`^Phase\\s+${k}(?=[\\s:(]|$)`, 'i');
return (h) => re.test(h.text);
};
test('property (ADR-2143 §4): editing phase k never alters any sibling section j≠k', () => {
// Model a ROADMAP-like document with N `## Phase k` sections (k=1..N), each
// with a distinct, generated body. Pick a random k, run withSection to append
// ' EDITED' to that section's body, and assert every OTHER section (j≠k) is
// byte-identical in the output — the bounded-mutation guarantee this seam
// exists to provide (structurally retires the #2130/#2067/#2080 boundary-
// crossing class).
//
// Also exercises Blocker 1 (title-collision hijack): each section's heading
// TITLE may be decorated with a reference to a DIFFERENT phase number
// (e.g. `Phase 3: legacy Phase 1 notes`), and the predicate above (anchored
// to the start of the heading text) must still resolve to the section whose
// OWN number matches, never a differently-numbered section whose title
// happens to mention the queried number.
//
// Heading level is kept UNIFORM (`##`) across sections deliberately: mixing
// random heading levels would make "which section is k" ambiguous for this
// property (a shallower section can syntactically nest a deeper one) — see
// the separate explicit mixed-level test below instead.
fc.assert(
fc.property(
fc.integer({ min: 2, max: 6 }).chain((n) =>
fc.record({
n: fc.constant(n),
bodies: fc.array(
fc.string({ minLength: 1, maxLength: 40 }).filter((s) => !/[\r\n]/.test(s) && s.trim().length > 0),
{ minLength: n, maxLength: n },
),
targetIdx: fc.integer({ min: 0, max: n - 1 }),
// For each section, optionally reference a DIFFERENT phase number in
// its own heading title (title-collision decoy). `undefined`/self
// means "no decoy for this section".
titleDecoys: fc.array(
fc.option(fc.integer({ min: 1, max: 6 }), { nil: undefined }),
{ minLength: n, maxLength: n },
),
}),
),
({ n, bodies, targetIdx, titleDecoys }) => {
const lines = [];
for (let k = 1; k <= n; k++) {
const decoy = titleDecoys[k - 1];
const heading = decoy !== undefined && decoy !== k
? `Phase ${k}: legacy Phase ${decoy} notes`
: `Phase ${k}`;
lines.push(`## ${heading}`);
lines.push(`body-${k}: ${bodies[k - 1]}`);
}
const doc = lines.join('\n') + '\n';
const targetPhase = targetIdx + 1;
// Snapshot every section's body BEFORE the edit.
const before = [];
for (let k = 1; k <= n; k++) {
const s = collectSection(doc, anchoredPhasePredicate(k));
assert.ok(s !== null, `Phase ${k} section must be found before edit`);
before.push(s.body);
}
const result = withSection(
doc,
anchoredPhasePredicate(targetPhase),
(body) => body + ' EDITED',
);
for (let k = 1; k <= n; k++) {
const s = collectSection(result, anchoredPhasePredicate(k));
assert.ok(s !== null, `Phase ${k} section must still be found after edit`);
if (k === targetPhase) {
assert.equal(s.body, before[k - 1] + ' EDITED', `Phase ${targetPhase} (the target) must be edited`);
} else {
assert.equal(s.body, before[k - 1], `Phase ${k} (j≠k) must be byte-identical after editing Phase ${targetPhase}`);
}
}
},
),
);
});
test('mixed heading levels + title collision: withSection({levelBounded:false}) does not fold a deeper heading into the target section, and the anchored predicate is not hijacked by a title mentioning another phase', () => {
// Phase 3 (appearing FIRST) has a title that mentions "Phase 1" — under the
// OLD unanchored regex this would be matched first (document order) by a
// query for phase 1 (Blocker 1). Phase 1 is followed by a DEEPER heading
// (`#### Phase 2`, level 4 vs Phase 1's level 3) — under the default
// `levelBounded: true` this would nest Phase 2 inside Phase 1's section
// and let an edit on Phase 1 reach into it (Blocker 2).
const content = [
'### Phase 3: Migrate off Phase 1 pipeline',
'gamma body line',
'### Phase 1: Foundation',
'alpha body line',
'#### Phase 2: API (deeper level, nested syntactically under Phase 1)',
'**Plans:** 1 plans',
].join('\n') + '\n';
const before2 = collectSection(content, anchoredPhasePredicate(2));
assert.ok(before2 !== null, 'Phase 2 section must be found before edit');
const result = withSection(
content,
anchoredPhasePredicate(1),
(body) => body + ' EDITED',
{ levelBounded: false },
);
assert.ok(
result.includes('### Phase 3: Migrate off Phase 1 pipeline\ngamma body line'),
'Phase 3 (title mentions "Phase 1") is byte-identical — not hijacked by the anchored Phase-1 query',
);
assert.ok(!result.includes('gamma body line EDITED'), 'the edit did not land in Phase 3');
const after2 = collectSection(result, anchoredPhasePredicate(2));
assert.equal(
after2.body,
before2.body,
'Phase 2 (deeper level than Phase 1) stays byte-identical — levelBounded:false stopped Phase 1 at the next heading of ANY level',
);
assert.ok(result.includes('alpha body line EDITED'), "Phase 1's own body was correctly edited");
});
});
// Parity guard removed in T5 (ADR-1372): uat-predicate now imports stripFencedCode
// from the seam directly, so comparing the seam to itself is tautological.
// The seam's stripFencedCode correctness is already covered by the tests above.

View File

@@ -32,6 +32,7 @@ const {
getRoadmapPhaseInternal,
getMilestoneInfo,
getMilestonePhaseFilter,
withPhaseSection,
} = roadmapParser;
// ─── helpers ─────────────────────────────────────────────────────────────────
@@ -732,6 +733,157 @@ describe('roadmap-parser: getMilestonePhaseFilter', () => {
});
});
// ─── withPhaseSection (ADR-2143 §4 — bounded mutation) ────────────────────────
describe('roadmap-parser: withPhaseSection', () => {
test('mutating phase k leaves phase j (j≠k) byte-identical', () => {
const content = [
'# Roadmap',
'',
'### Phase 1: Foundation',
'**Goal:** Setup',
'**Plans:** 1 plans',
'',
'### Phase 2: API',
'**Goal:** Build API',
'**Plans:** 1 plans',
'',
'### Phase 3: Polish',
'**Goal:** Harden',
'**Plans:** 1 plans',
'',
].join('\n');
const result = withPhaseSection(content, '2', (body) =>
body.replace(/(\*\*Plans:\*\*\s*)[^\n]+/i, '$11/1 plans complete'),
);
assert.ok(
result.includes('### Phase 2: API\n**Goal:** Build API\n**Plans:** 1/1 plans complete'),
'phase 2 (the target) is updated',
);
assert.ok(
result.includes('### Phase 1: Foundation\n**Goal:** Setup\n**Plans:** 1 plans'),
'phase 1 (j≠k) is byte-identical',
);
assert.ok(
result.includes('### Phase 3: Polish\n**Goal:** Harden\n**Plans:** 1 plans'),
'phase 3 (j≠k) is byte-identical',
);
});
test('a greedy edit callback cannot escape phase N\'s own section', () => {
const content = [
'### Phase 1: Alpha',
'alpha body',
'### Phase 2: Beta',
'beta body',
'### Phase 3: Gamma',
'gamma body',
].join('\n') + '\n';
const result = withPhaseSection(content, '2', (body) => body.replace(/[\s\S]*/, 'REPLACED'));
assert.ok(result.includes('### Phase 1: Alpha\nalpha body'), 'Phase 1 untouched by a greedy regex targeting Phase 2');
assert.ok(result.includes('### Phase 3: Gamma\ngamma body'), 'Phase 3 untouched by a greedy regex targeting Phase 2');
assert.ok(result.includes('### Phase 2: Beta\nREPLACED'), 'Phase 2 was the intended target');
});
test('no matching phase heading -> content unchanged (bounded no-op)', () => {
const content = '### Phase 1: Foundation\n**Plans:** 1 plans\n';
const result = withPhaseSection(content, '99', (body) => body + ' MUTATED');
assert.equal(result, content, 'no Phase 99 heading -> unchanged');
});
test('resolves the phase heading via the #2121 phase-id source (zero-padding tolerant)', () => {
const content = [
'### Phase 02: Padded',
'**Plans:** 1 plans',
'### Phase 3: Next',
'**Plans:** 1 plans',
].join('\n') + '\n';
// Query with the un-padded form ("2") — must still resolve "Phase 02".
const result = withPhaseSection(content, '2', (body) => body.replace('1 plans', '1/1 plans complete'));
assert.ok(result.includes('### Phase 02: Padded\n**Plans:** 1/1 plans complete'), 'un-padded query resolves padded heading');
assert.ok(result.includes('### Phase 3: Next\n**Plans:** 1 plans'), 'Phase 3 untouched');
});
test('a query for phase "1" does not prefix-match a decimal sub-phase heading "Phase 1.1"', () => {
// Sub-phase appears BEFORE the parent phase in the document, so a bare
// `\b`-terminated regex (which would match "1" as a prefix of "1.1")
// could resolve the wrong (first-encountered) section.
const content = [
'### Phase 1.1: Sub',
'sub body',
'### Phase 1: Base',
'base body',
].join('\n') + '\n';
const result = withPhaseSection(content, '1', (body) => body + ' EDITED');
assert.ok(result.includes('### Phase 1.1: Sub\nsub body\n'), 'Phase 1.1 body is byte-identical (untouched)');
assert.ok(!result.includes('sub body EDITED'), 'the edit did not land in Phase 1.1');
assert.ok(result.includes('### Phase 1: Base\nbase body EDITED'), "Phase 1's own body received the edit");
const subResult = withPhaseSection(content, '1.1', (body) => body + ' EDITED');
assert.ok(subResult.includes('### Phase 1.1: Sub\nsub body EDITED'), "Phase 1.1's own body received the edit");
assert.ok(subResult.includes('### Phase 1: Base\nbase body\n'), 'Phase 1 body is byte-identical (untouched)');
});
test('Blocker 1 regression: a query for phase "1" is not hijacked by a sibling phase whose TITLE mentions "Phase 1"', () => {
// Phase 3's own title mentions "Phase 1" ("Migrate off Phase 1 pipeline")
// and appears BEFORE the real Phase 1 heading in document order. Under the
// OLD unanchored regex (`(?:^|\s)Phase\s+1(?=[\s:(]|$)`), that substring
// inside Phase 3's heading text would match first — and because
// `collectSection` picks the FIRST matching heading, `withPhaseSection`
// would edit Phase 3's section instead of Phase 1's.
const content = [
'### Phase 3: Migrate off Phase 1 pipeline',
'**Plans:** 1 plans',
'### Phase 1: Foundation',
'**Plans:** 1 plans',
].join('\n') + '\n';
const result = withPhaseSection(content, '1', (body) =>
body.replace(/(\*\*Plans:\*\*\s*)[^\n]+/, '$1DONE'),
);
assert.ok(
result.includes('### Phase 1: Foundation\n**Plans:** DONE'),
"Phase 1's own Plans line is edited",
);
assert.ok(
result.includes('### Phase 3: Migrate off Phase 1 pipeline\n**Plans:** 1 plans'),
'Phase 3 (title mentions "Phase 1") is byte-identical — not hijacked',
);
});
test('Blocker 2 regression: a following DEEPER heading is not folded into phase 1\'s section body', () => {
// Phase 1 is `###` (level 3); the very next heading, `#### Phase 2: API`
// (level 4), is DEEPER than Phase 1. Under the default `levelBounded: true`
// stop rule, a deeper heading does not terminate the section (it only stops
// at a heading whose level <= the target's own level), so Phase 2's whole
// section — including its `**Plans:**` line — would be folded into Phase
// 1's body and reachable by `edit`.
const content = [
'### Phase 1: Foundation',
'#### Phase 2: API',
'**Plans:** 1 plans',
'### Phase 3: Polish',
'**Plans:** 1 plans',
].join('\n') + '\n';
const phase2Snippet = '#### Phase 2: API\n**Plans:** 1 plans';
assert.ok(content.includes(phase2Snippet), 'sanity: fixture contains the expected Phase 2 snippet');
const result = withPhaseSection(content, '1', (body) => `${body}[EDITED]`);
assert.ok(
result.includes(phase2Snippet),
"Phase 2's heading + Plans line stay contiguous and byte-identical — Phase 1's edit did not reach into it",
);
assert.ok(!result.includes('1 plans[EDITED]'), "the edit did not land inside Phase 2's Plans line");
});
});
// ────────────────────────────────────────────────────────────────────────
// Folded from tests/bug-2554-decimal-phase-filter.test.cjs — consolidation epic #1969 (B3 #1972)