* chore(#2143): fail-loud Result + per-surface write-set contract — Phase 3 Phase 3 of epic #2143 (ADR-2143 §5/§6). The three target bugs (#2140, #2112, #2118) were already fixed tactically on next; this introduces the reusable structural contracts and rewires the primary #2140 site onto them. - src/write-set.cts (new): the parse `Result<T> = {ok,value|reason}` (§5) and the per-surface write-set (`WriteOutcome {surface, applied, requirement?}`, `WriteSet`, `writeSetComplete`) (§6). markdown-table.cts now imports + re-exports `Result` from here (single source; distinct from command-routing-hub's Result). - requirements mark-complete (src/milestone.cts): returns a PER-REQUIREMENT, per-surface write-set; `write_set_complete` is true only if every surface of every requirement applied — structurally forbidding the #2140 OR-into-one-flag masking, including across a multi-ID batch (adversarial-review regression). Pre-existing output fields unchanged (behaviour-preserving; #2140 already fixed). - deriveProgressFromRoadmap (src/phase-lifecycle.cts): removed the vestigial null-swallowing try/catch (findTableWithColumns never throws) — ADR §5 no-swallow; RoadmapProgress return contract unchanged. - commit --files (#2112) and milestone complete --dry-run (#2118) left as-is (single-surface commit / pre-mutation preview — not genuine multi-surface writes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#2244): backfill changeset PR number (#2251) 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
5
.changeset/graceful-koalas-greet.md
Normal file
5
.changeset/graceful-koalas-greet.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Changed
|
||||
pr: 2251
|
||||
---
|
||||
**`requirements mark-complete` reports a per-surface write-set** — the command now returns a per-requirement `write_set` (checkbox + traceability surfaces) and a `write_set_complete` that is true only when every surface of every requirement applied, so a partial (checkbox-only) reconcile can no longer masquerade as full success even inside a multi-ID batch. Introduces the reusable ADR-2143 §5/§6 `Result` / `WriteSet` contract. (#2251)
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -92,6 +92,7 @@ build/
|
||||
/gsd-core/bin/lib/capability-lock.cjs
|
||||
/gsd-core/bin/lib/markdown-sectionizer.cjs
|
||||
/gsd-core/bin/lib/markdown-table.cjs
|
||||
/gsd-core/bin/lib/write-set.cjs
|
||||
/gsd-core/bin/lib/resolution.cjs
|
||||
/gsd-core/bin/lib/research-store.cjs
|
||||
/gsd-core/bin/lib/research-provider.cjs
|
||||
|
||||
@@ -143,7 +143,10 @@ Module owning the tool's CLI I/O primitives: `output()` result emission (with la
|
||||
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).
|
||||
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}` — re-exported from the Write-Set Module, the ADR-2143 §5 single source of truth for this shape, so existing importers of `Result` from `markdown-table.cjs` are unaffected; 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).
|
||||
|
||||
### Write-Set Module
|
||||
Shared fail-loud `Result<T>` and per-surface write-set contracts (`gsd-core/bin/lib/write-set.cjs`, generated from `src/write-set.cts`; ADR-2143 §5/§6, epic #2143). Pure, Node built-ins only, no I/O. Exports: `Result<T>` (`{ok:true,value}\|{ok:false,reason}` — ADR-2143 §5 fail-loud parse shape, never a bare `null` a caller can mistake for "empty but fine"; the single source of truth `markdown-table.cjs` re-exports so its existing importers are unaffected; deliberately distinct from command-routing-hub's dispatch `Result` `{ok,data\|kind}`); `WriteOutcome` (`{surface: string, applied: boolean}` — one surface's outcome within a multi-surface write); `WriteSet` (`WriteOutcome[]`); `writeSetComplete(ws) → boolean` (true only when the set is non-empty AND every surface applied — ADR-2143 §6's "no OR-into-one-flag" rule: a command that mutates more than one surface must not collapse independent surface outcomes into a single boolean, the anti-pattern that let a checkbox-only partial write (#2140) report full success). `milestone.cts`'s `requirements mark-complete` handler is the first consumer: it reports a `write_set` (`checkbox`/`traceability` surfaces) and `write_set_complete` alongside its existing `updated`/`marked_complete`/`already_complete`/`not_found`/`table_unmatched` fields, which remain computed exactly as before — the write-set is additive, structured ADR-2143 documentation of the same per-surface facts #2140's tactical fix already exposed via `table_unmatched`.
|
||||
|
||||
### 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`, `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`).
|
||||
|
||||
@@ -447,7 +447,8 @@
|
||||
"workstream-name-policy.cjs",
|
||||
"workstream.cjs",
|
||||
"worktree-base-ref.cjs",
|
||||
"worktree-safety.cjs"
|
||||
"worktree-safety.cjs",
|
||||
"write-set.cjs"
|
||||
],
|
||||
"hooks": [
|
||||
"gsd-check-update-worker.js",
|
||||
|
||||
@@ -533,6 +533,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
| `workstream.cjs` | Workstream CRUD, migration, session-scoped active pointer |
|
||||
| `worktree-base-ref.cjs` | Worktree base-ref drift detection and degrade decision (`evaluateWorktreeBaseDegrade`) plus no-clobber `worktree.baseRef` settings management for the `base-check`/`set-baseref` subcommands (#683) |
|
||||
| `worktree-safety.cjs` | Worktree-root resolution and non-destructive prune policy decisions; owns W017 health-check logic |
|
||||
| `write-set.cjs` | Shared fail-loud `Result<T>` (`{ok:true,value}\|{ok:false,reason}`) and per-surface write-set contracts (ADR-2143, epic #2143) — `WriteOutcome` (`{surface,applied}`), `WriteSet` (`WriteOutcome[]`), and `writeSetComplete(ws)` (true only when the set is non-empty AND every surface applied, never an OR-into-one-flag); `markdown-table.cjs` re-exports `Result` from here so existing importers are unaffected; consumed by `milestone.cts`'s `requirements mark-complete` handler to report a structured per-surface (`checkbox`/`traceability`) write-set alongside its existing fields (fixes the structural half of #2140) |
|
||||
|
||||
[`docs/CLI-TOOLS.md`](CLI-TOOLS.md) may describe a subset of these modules; when it disagrees with the filesystem, this table and the directory listing are authoritative.
|
||||
|
||||
|
||||
@@ -215,6 +215,8 @@ export default tseslint.config(
|
||||
'gsd-core/bin/lib/markdown-sectionizer.cjs',
|
||||
// ADR-2143: tsc-generated runtime artifact — lint the src/markdown-table.cts source.
|
||||
'gsd-core/bin/lib/markdown-table.cjs',
|
||||
// ADR-2143: tsc-generated runtime artifact — lint the src/write-set.cts source.
|
||||
'gsd-core/bin/lib/write-set.cjs',
|
||||
// ADR-1239 Phase C-1 (#1680): tsc-generated — lint src/embedding-adapter.cts + src/adapter-declarative.cts.
|
||||
'gsd-core/bin/lib/embedding-adapter.cjs',
|
||||
'gsd-core/bin/lib/adapter-declarative.cjs',
|
||||
|
||||
@@ -3,17 +3,19 @@
|
||||
* (ADR-2143, epic #2143). Pure functions, Node built-ins only, string-in/value-out,
|
||||
* no I/O. Compiled by tsc to gsd-core/bin/lib/markdown-table.cjs.
|
||||
*
|
||||
* NOTE: the `Result<T>` here is the ADR-2143 parse-result shape {ok,value|reason} —
|
||||
* deliberately distinct from command-routing-hub's dispatch `Result` {ok,data|kind};
|
||||
* the two never mix (different modules).
|
||||
* NOTE: the `Result<T>` here is the ADR-2143 §5 parse-result shape {ok,value|reason},
|
||||
* now defined once in `./write-set.cjs` (the shared fail-loud + write-set seam) and
|
||||
* re-exported here so existing importers of `Result` from this module keep working
|
||||
* unchanged — deliberately distinct from command-routing-hub's dispatch `Result`
|
||||
* {ok,data|kind}; the two never mix (different modules).
|
||||
*/
|
||||
|
||||
import { collectSection, replaceSection } from './markdown-sectionizer.cjs';
|
||||
import type { Result } from './write-set.cjs';
|
||||
export type { Result } from './write-set.cjs';
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
export type Result<T> = { ok: true; value: T } | { ok: false; reason: string };
|
||||
|
||||
/** A parsed GFM pipe table: header column names + rows addressed by column name. */
|
||||
export interface MarkdownTable {
|
||||
columns: string[];
|
||||
|
||||
@@ -18,6 +18,8 @@ import { platformWriteSync, platformEnsureDir, execGit, retryRenameSync } from '
|
||||
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
|
||||
import { realClock } from './clock.cjs';
|
||||
import { transitionCore } from './state-transition.cjs';
|
||||
import { writeSetComplete } from './write-set.cjs';
|
||||
import type { WriteSet } from './write-set.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import ioMod = require('./io.cjs');
|
||||
const { output, error } = ioMod;
|
||||
@@ -79,6 +81,18 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool
|
||||
// a missing row only counts as drift when a table actually exists.
|
||||
const hasTable = /^\|\s*Requirement\s*\|/im.test(reqContent);
|
||||
|
||||
// ADR-2143 §6 per-surface write-set, tracked PER requirement ID: a
|
||||
// multi-ID batch must not OR one ID's surface outcome into another's —
|
||||
// that is the exact #2140 class one level up (an ID whose traceability
|
||||
// row is absent/unmatched must not have its partial write masked by a
|
||||
// different ID in the same invocation that fully reconciled). Reported
|
||||
// additively as `write_set` below — it does not change the existing
|
||||
// marked_complete/already_complete/not_found/table_unmatched/updated
|
||||
// computation, which stays byte-for-behaviour identical (#2140's tactical
|
||||
// fix already surfaces the checkbox-only-partial-write case via
|
||||
// table_unmatched; this only adds the structured ADR-2143 shape on top).
|
||||
const writeSet: WriteSet = [];
|
||||
|
||||
for (const reqId of reqIds) {
|
||||
const reqEscaped = escapeRegex(reqId);
|
||||
|
||||
@@ -96,6 +110,16 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool
|
||||
const tableHit = afterTable !== reqContent;
|
||||
if (tableHit) reqContent = afterTable;
|
||||
|
||||
// ADR-2143 §6 per-ID write-set entries: this ID's checkbox surface is
|
||||
// always tracked; the traceability surface is tracked only when the file
|
||||
// has a traceability table at all (same `hasTable` gate the existing
|
||||
// required-surface logic below uses) — omitted entirely, not a false
|
||||
// `applied:false`, when no table is required of this file.
|
||||
writeSet.push({ requirement: reqId, surface: 'checkbox', applied: checkboxHit });
|
||||
if (hasTable) {
|
||||
writeSet.push({ requirement: reqId, surface: 'traceability', applied: tableHit });
|
||||
}
|
||||
|
||||
// Coverage of the traceability surface for this ID (computed after any flip).
|
||||
// hasRow keys on the ID + a second cell (`| ID | <phase> |`) so a bare mention
|
||||
// of the ID in a non-traceability table does not masquerade as a real row.
|
||||
@@ -129,6 +153,17 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool
|
||||
platformWriteSync(reqPath, reqContent);
|
||||
}
|
||||
|
||||
// ADR-2143 §6: `writeSet` above already carries one WriteOutcome per
|
||||
// (requirement, surface) this invocation could have written to — per ID,
|
||||
// not ORed across the batch. `write_set` and `write_set_complete` are
|
||||
// additive: they do not replace or gate `updated` / `marked_complete` /
|
||||
// `already_complete` / `not_found` / `table_unmatched`, which remain
|
||||
// computed exactly as before (see #2140 note above — that fix already
|
||||
// surfaces a checkbox-only partial write via `table_unmatched`;
|
||||
// `write_set_complete` is a structured, ADR-2143-shaped read of the SAME
|
||||
// per-surface, per-ID facts, `false` if ANY id's ANY required surface did
|
||||
// not apply, since `writeSetComplete` requires EVERY entry to have
|
||||
// applied, never an OR across surfaces OR across IDs).
|
||||
output(
|
||||
{
|
||||
updated: updated.length > 0,
|
||||
@@ -137,6 +172,8 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool
|
||||
not_found: notFound,
|
||||
table_unmatched: tableUnmatched,
|
||||
total: reqIds.length,
|
||||
write_set: writeSet,
|
||||
write_set_complete: writeSetComplete(writeSet),
|
||||
},
|
||||
raw,
|
||||
`${updated.length}/${reqIds.length} requirements marked complete`,
|
||||
|
||||
@@ -62,43 +62,49 @@ export function deriveProgressFromRoadmap(roadmapContent: string): RoadmapProgre
|
||||
let totalPhases: number | null = null;
|
||||
let totalPlans: number | null = null;
|
||||
|
||||
try {
|
||||
// ADR-2143 §3: read the Progress table by column NAME (order/injection-invariant),
|
||||
// via the markdown-table seam. Scope to the `## Progress` section when present
|
||||
// (#2012 decoy avoidance); a headingless milestone slice (#1445) falls back to the
|
||||
// whole input. Requires the canonical Phase/Plans Complete/Status/Completed columns
|
||||
// in any order (extra columns ignored) — supersedes findTableBySchema's exact-schema lookup.
|
||||
const progressMatch = roadmapContent.match(/^##[ \t]+Progress\b/im);
|
||||
let scoped = roadmapContent;
|
||||
if (progressMatch && progressMatch.index !== undefined) {
|
||||
const afterHeading = roadmapContent.slice(progressMatch.index);
|
||||
const nextHeading = afterHeading.search(/\n#{1,2}[ \t]/);
|
||||
scoped = nextHeading >= 0 ? afterHeading.slice(0, nextHeading) : afterHeading;
|
||||
// ADR-2143 §5 (fail-loud, no null-swallow): this used to be wrapped in a
|
||||
// try/catch that silently fell through to the existing (null) values on any
|
||||
// thrown error. `findTableWithColumns`/`parseMarkdownTable` never throw —
|
||||
// an unparseable or absent table resolves to `null` /
|
||||
// `{ ok: false, reason }`, not an exception — so the catch was masking
|
||||
// nothing but dead code paths. Removed per ADR-2143 §5; the public
|
||||
// `RoadmapProgress` contract (nulls = absent) is unchanged.
|
||||
//
|
||||
// ADR-2143 §3: read the Progress table by column NAME (order/injection-invariant),
|
||||
// via the markdown-table seam. Scope to the `## Progress` section when present
|
||||
// (#2012 decoy avoidance); a headingless milestone slice (#1445) falls back to the
|
||||
// whole input. Requires the canonical Phase/Plans Complete/Status/Completed columns
|
||||
// in any order (extra columns ignored) — supersedes findTableBySchema's exact-schema lookup.
|
||||
const progressMatch = roadmapContent.match(/^##[ \t]+Progress\b/im);
|
||||
let scoped = roadmapContent;
|
||||
if (progressMatch && progressMatch.index !== undefined) {
|
||||
const afterHeading = roadmapContent.slice(progressMatch.index);
|
||||
const nextHeading = afterHeading.search(/\n#{1,2}[ \t]/);
|
||||
scoped = nextHeading >= 0 ? afterHeading.slice(0, nextHeading) : afterHeading;
|
||||
}
|
||||
const table = findTableWithColumns(scoped, ['Phase', 'Plans Complete', 'Status', 'Completed']);
|
||||
|
||||
if (table) {
|
||||
const allRows = table.rows;
|
||||
|
||||
const completed = allRows.filter((r) => /^complete$/i.test((r['Status'] ?? '').trim())).length;
|
||||
completedPhases = completed > 0 ? completed : null;
|
||||
|
||||
// Data rows only (exclude 999.x backlog phases). Mirrors init.cts /^999(?:\.|$)/ filter.
|
||||
const dataRows = allRows.filter((r) => {
|
||||
const phase = (r['Phase'] ?? '').trim();
|
||||
return /^\d/.test(phase) && !/^999\b/.test(phase);
|
||||
});
|
||||
totalPhases = dataRows.length > 0 ? dataRows.length : null;
|
||||
|
||||
let totalPlansSum = 0;
|
||||
for (const r of allRows) {
|
||||
const cell = (r['Plans Complete'] ?? '').trim();
|
||||
const m = /(\d+)\s*\/\s*(\d+)/.exec(cell);
|
||||
if (m) totalPlansSum += parseInt(m[2], 10);
|
||||
}
|
||||
const table = findTableWithColumns(scoped, ['Phase', 'Plans Complete', 'Status', 'Completed']);
|
||||
|
||||
if (table) {
|
||||
const allRows = table.rows;
|
||||
|
||||
const completed = allRows.filter((r) => /^complete$/i.test((r['Status'] ?? '').trim())).length;
|
||||
completedPhases = completed > 0 ? completed : null;
|
||||
|
||||
// Data rows only (exclude 999.x backlog phases). Mirrors init.cts /^999(?:\.|$)/ filter.
|
||||
const dataRows = allRows.filter((r) => {
|
||||
const phase = (r['Phase'] ?? '').trim();
|
||||
return /^\d/.test(phase) && !/^999\b/.test(phase);
|
||||
});
|
||||
totalPhases = dataRows.length > 0 ? dataRows.length : null;
|
||||
|
||||
let totalPlansSum = 0;
|
||||
for (const r of allRows) {
|
||||
const cell = (r['Plans Complete'] ?? '').trim();
|
||||
const m = /(\d+)\s*\/\s*(\d+)/.exec(cell);
|
||||
if (m) totalPlansSum += parseInt(m[2], 10);
|
||||
}
|
||||
totalPlans = totalPlansSum > 0 ? totalPlansSum : null;
|
||||
}
|
||||
} catch { /* intentionally empty — fall through to existing values */ }
|
||||
totalPlans = totalPlansSum > 0 ? totalPlansSum : null;
|
||||
}
|
||||
|
||||
return { completedPhases, totalPhases, totalPlans };
|
||||
}
|
||||
|
||||
55
src/write-set.cts
Normal file
55
src/write-set.cts
Normal file
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* Write-Set — shared fail-loud parse `Result` and per-surface write-set
|
||||
* contracts (ADR-2143, epic #2143). Pure, Node built-ins only, no I/O.
|
||||
* Compiled by tsc to gsd-core/bin/lib/write-set.cjs.
|
||||
*
|
||||
* ADR-2143 §5 (fail-loud parsing, no null-swallow): seam parse operations
|
||||
* and document-model accessors return a typed `Result<T>` — never a bare
|
||||
* `null` a caller can mistake for "empty but fine." This is the same
|
||||
* `{ ok: true; value: T } | { ok: false; reason: string }` shape
|
||||
* `markdown-table.cts` already defined for `parseMarkdownTable` /
|
||||
* `appendQuickTaskRow`; this module is now the single source of truth for
|
||||
* it and `markdown-table.cjs` re-exports the type so existing importers of
|
||||
* `Result` from that module keep working unchanged.
|
||||
*
|
||||
* NOTE: deliberately distinct from command-routing-hub's dispatch `Result`
|
||||
* (`{ok,data}|{ok:false,kind}`) — the two never mix (different modules,
|
||||
* different shapes, different purposes).
|
||||
*
|
||||
* ADR-2143 §6 (write-set results for multi-surface commands, no
|
||||
* OR-into-one-flag): a command that mutates more than one surface returns
|
||||
* an explicit per-surface write-set — `{ surface, applied }` outcomes — and
|
||||
* its top-level "did this fully succeed" signal is true only if EVERY
|
||||
* surface in the set applied. ORing independent surfaces into a single
|
||||
* boolean is the direct anti-pattern that let a checkbox-only partial
|
||||
* write (#2140) report full success.
|
||||
*/
|
||||
|
||||
export type Result<T> = { ok: true; value: T } | { ok: false; reason: string };
|
||||
|
||||
/**
|
||||
* One surface's outcome within a multi-surface write (ADR-2143 §6).
|
||||
*
|
||||
* `requirement` identifies which entity/ID this surface outcome belongs to,
|
||||
* so a multi-entity command's write-set does not collapse independent
|
||||
* entities into one aggregate — the §6 "no OR into one flag" rule applies
|
||||
* per entity too, not just per surface.
|
||||
*/
|
||||
export interface WriteOutcome {
|
||||
surface: string;
|
||||
applied: boolean;
|
||||
requirement?: string;
|
||||
}
|
||||
|
||||
/** The full set of per-surface outcomes for one multi-surface write. */
|
||||
export type WriteSet = WriteOutcome[];
|
||||
|
||||
/**
|
||||
* True only if the write-set is non-empty AND every surface in it applied.
|
||||
* An empty write-set is never "complete" — there is nothing to be complete
|
||||
* about, so treating it as vacuously true would let a no-op masquerade as
|
||||
* a full success (the same OR-into-one-flag class ADR-2143 §6 prohibits).
|
||||
*/
|
||||
export function writeSetComplete(ws: WriteSet): boolean {
|
||||
return ws.length > 0 && ws.every((o) => o.applied);
|
||||
}
|
||||
@@ -647,6 +647,19 @@ describe('requirements mark-complete command', () => {
|
||||
assert.ok(content.includes('- [x] **FOO-01**'), 'checkbox should be checked');
|
||||
// The table is untouched (no FOO-01 row synthesized).
|
||||
assert.ok(!content.includes('FOO-01 | Phase'), 'no FOO-01 row should be invented');
|
||||
|
||||
// ADR-2143 §6 write-set: additive structured read of the same per-surface
|
||||
// facts — checkbox surface applied (fresh write this run), traceability
|
||||
// surface did NOT (no row existed to flip), so the set is not complete.
|
||||
// This does not change `updated`/`marked_complete` above. Per-ID: each
|
||||
// outcome carries `requirement` so a multi-ID batch cannot OR one ID's
|
||||
// outcome into another's (the #2140 class one level up).
|
||||
assert.deepStrictEqual(out.write_set, [
|
||||
{ requirement: 'FOO-01', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'FOO-01', surface: 'traceability', applied: false },
|
||||
]);
|
||||
assert.strictEqual(out.write_set_complete, false,
|
||||
'writeSetComplete requires EVERY surface applied, not an OR — a checkbox-only write is not complete');
|
||||
});
|
||||
|
||||
test('#2140 re-run on the half-written file does NOT mask the drift as already_complete', () => {
|
||||
@@ -664,6 +677,15 @@ describe('requirements mark-complete command', () => {
|
||||
'nothing flipped on re-run, so not marked_complete');
|
||||
assert.ok(out.table_unmatched.includes('FOO-01'),
|
||||
'the drift must still be surfaced as table_unmatched on re-run');
|
||||
|
||||
// ADR-2143 §6 write-set: nothing was written THIS run on either surface
|
||||
// (checkbox was already [x], the row still doesn't exist), so both
|
||||
// surfaces report applied:false and the set is not complete.
|
||||
assert.deepStrictEqual(out.write_set, [
|
||||
{ requirement: 'FOO-01', surface: 'checkbox', applied: false },
|
||||
{ requirement: 'FOO-01', surface: 'traceability', applied: false },
|
||||
]);
|
||||
assert.strictEqual(out.write_set_complete, false);
|
||||
});
|
||||
|
||||
test('#2140 no traceability table at all → still a clean success (no table_unmatched)', () => {
|
||||
@@ -676,6 +698,104 @@ describe('requirements mark-complete command', () => {
|
||||
assert.ok(out.marked_complete.includes('NO-TABLE-01'));
|
||||
assert.ok(!out.table_unmatched || !out.table_unmatched.includes('NO-TABLE-01'),
|
||||
'no table_unmatched when there is no traceability table');
|
||||
|
||||
// ADR-2143 §6 write-set: the traceability surface is omitted entirely
|
||||
// (not reported as a false applied:false) when the file has no
|
||||
// traceability table — nothing was required of it. A single-surface
|
||||
// write-set that fully applied is complete.
|
||||
assert.deepStrictEqual(out.write_set, [
|
||||
{ requirement: 'NO-TABLE-01', surface: 'checkbox', applied: true },
|
||||
]);
|
||||
assert.strictEqual(out.write_set_complete, true);
|
||||
});
|
||||
|
||||
test('#2140-class multi-ID: one ID\'s partial reconcile is not masked by another ID\'s full one', () => {
|
||||
// Adversarial-review regression: write_set/write_set_complete were built
|
||||
// from two INVOCATION-WIDE booleans OR-accumulated across every ID in the
|
||||
// batch, so a fully-reconciled REQ-02 in the same call could mask a
|
||||
// checkbox-only partial write on REQ-01. The write-set must be tracked
|
||||
// PER (requirement, surface) so REQ-01's unmatched traceability row
|
||||
// cannot be hidden by REQ-02 reconciling cleanly.
|
||||
writeRequirements(
|
||||
tmpDir,
|
||||
`# Requirements
|
||||
|
||||
## Coverage
|
||||
- [ ] **REQ-01**: feature one (no traceability row)
|
||||
- [ ] **REQ-02**: feature two (has traceability row)
|
||||
|
||||
## Traceability
|
||||
|
||||
| Requirement | Phase | Status |
|
||||
|-------------|-------|--------|
|
||||
| REQ-02 | Phase 1 | Pending |
|
||||
`,
|
||||
);
|
||||
|
||||
const result = runGsdTools('requirements mark-complete REQ-01,REQ-02', tmpDir);
|
||||
assert.ok(result.success);
|
||||
const out = JSON.parse(result.output);
|
||||
|
||||
// Pre-existing fields are unchanged: both checkboxes flip (marked_complete),
|
||||
// REQ-01 has no table row so it surfaces as table_unmatched.
|
||||
assert.deepStrictEqual(out.marked_complete.sort(), ['REQ-01', 'REQ-02']);
|
||||
assert.deepStrictEqual(out.table_unmatched, ['REQ-01']);
|
||||
assert.deepStrictEqual(out.not_found, []);
|
||||
assert.deepStrictEqual(out.already_complete, []);
|
||||
|
||||
const content = readRequirements(tmpDir);
|
||||
assert.ok(content.includes('- [x] **REQ-01**'), 'REQ-01 checkbox should be checked');
|
||||
assert.ok(content.includes('- [x] **REQ-02**'), 'REQ-02 checkbox should be checked');
|
||||
assert.ok(content.includes('| REQ-02 | Phase 1 | Complete |'), 'REQ-02 table row should be Complete');
|
||||
assert.ok(!content.includes('REQ-01 | Phase'), 'no REQ-01 row should be invented');
|
||||
|
||||
// The write-set is now per-ID: REQ-01's traceability surface did NOT
|
||||
// apply (no row to flip) even though REQ-02's did — the aggregate must
|
||||
// not OR REQ-02's success into REQ-01's outcome.
|
||||
assert.deepStrictEqual(out.write_set, [
|
||||
{ requirement: 'REQ-01', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'REQ-01', surface: 'traceability', applied: false },
|
||||
{ requirement: 'REQ-02', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'REQ-02', surface: 'traceability', applied: true },
|
||||
]);
|
||||
// Because REQ-01's traceability entry did not apply, the batch as a
|
||||
// whole is NOT complete — this is the exact bug the fix closes.
|
||||
assert.strictEqual(out.write_set_complete, false,
|
||||
'REQ-01\'s unmatched traceability row must not be masked by REQ-02 fully reconciling');
|
||||
});
|
||||
|
||||
test('#2140-class multi-ID: fully-reconciled batch reports write_set_complete:true', () => {
|
||||
writeRequirements(
|
||||
tmpDir,
|
||||
`# Requirements
|
||||
|
||||
## Coverage
|
||||
- [ ] **REQ-01**: feature one
|
||||
- [ ] **REQ-02**: feature two
|
||||
|
||||
## Traceability
|
||||
|
||||
| Requirement | Phase | Status |
|
||||
|-------------|-------|--------|
|
||||
| REQ-01 | Phase 1 | Pending |
|
||||
| REQ-02 | Phase 1 | Pending |
|
||||
`,
|
||||
);
|
||||
|
||||
const result = runGsdTools('requirements mark-complete REQ-01,REQ-02', tmpDir);
|
||||
assert.ok(result.success);
|
||||
const out = JSON.parse(result.output);
|
||||
|
||||
assert.deepStrictEqual(out.marked_complete.sort(), ['REQ-01', 'REQ-02']);
|
||||
assert.deepStrictEqual(out.table_unmatched, []);
|
||||
|
||||
assert.deepStrictEqual(out.write_set, [
|
||||
{ requirement: 'REQ-01', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'REQ-01', surface: 'traceability', applied: true },
|
||||
{ requirement: 'REQ-02', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'REQ-02', surface: 'traceability', applied: true },
|
||||
]);
|
||||
assert.strictEqual(out.write_set_complete, true);
|
||||
});
|
||||
|
||||
test('marks single requirement complete (checkbox + table)', () => {
|
||||
@@ -692,6 +812,14 @@ describe('requirements mark-complete command', () => {
|
||||
assert.ok(content.includes('- [x] **TEST-01**'), 'checkbox should be checked');
|
||||
assert.ok(content.includes('| TEST-01 | Phase 1 | Complete |'), 'table row should be Complete');
|
||||
assert.ok(content.includes('- [ ] **TEST-02**'), 'TEST-02 should remain unchecked');
|
||||
|
||||
// ADR-2143 §6 write-set: both surfaces got a fresh write this run, so the
|
||||
// write-set is complete.
|
||||
assert.deepStrictEqual(output.write_set, [
|
||||
{ requirement: 'TEST-01', surface: 'checkbox', applied: true },
|
||||
{ requirement: 'TEST-01', surface: 'traceability', applied: true },
|
||||
]);
|
||||
assert.strictEqual(output.write_set_complete, true);
|
||||
});
|
||||
|
||||
test('handles mixed prefixes in single call (TEST-XX, REG-XX, INFRA-XX)', () => {
|
||||
|
||||
97
tests/write-set.test.cjs
Normal file
97
tests/write-set.test.cjs
Normal file
@@ -0,0 +1,97 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Behavioral tests for write-set.cjs
|
||||
*
|
||||
* Module: gsd-core/bin/lib/write-set.cjs
|
||||
* Exports: writeSetComplete (Result<T> is a pure type re-exported at compile
|
||||
* time only — nothing to assert on it at runtime beyond markdown-table.cjs's
|
||||
* own re-export continuing to work, covered by tests/markdown-table.test.cjs).
|
||||
*
|
||||
* Covers (ADR-2143 §5/§6):
|
||||
* - writeSetComplete happy path: all-applied → true, one-not-applied → false
|
||||
* - BOUNDARY coverage: empty write-set (0 outcomes), single outcome, two outcomes
|
||||
* - an empty write-set is never "complete" (no vacuous-true on a no-op)
|
||||
* - fast-check property: writeSetComplete(ws) === (ws.length > 0 && every applied)
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
|
||||
const { writeSetComplete } = require('../gsd-core/bin/lib/write-set.cjs');
|
||||
|
||||
describe('writeSetComplete', () => {
|
||||
test('every surface applied → true', () => {
|
||||
assert.equal(
|
||||
writeSetComplete([
|
||||
{ surface: 'checkbox', applied: true },
|
||||
{ surface: 'traceability', applied: true },
|
||||
]),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test('one surface not applied → false (AND across surfaces, never OR)', () => {
|
||||
assert.equal(
|
||||
writeSetComplete([
|
||||
{ surface: 'checkbox', applied: true },
|
||||
{ surface: 'traceability', applied: false },
|
||||
]),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test('no surfaces applied → false', () => {
|
||||
assert.equal(
|
||||
writeSetComplete([
|
||||
{ surface: 'checkbox', applied: false },
|
||||
{ surface: 'traceability', applied: false },
|
||||
]),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
// BOUNDARY: 0 outcomes (limit-1 relative to the smallest real set), 1 outcome
|
||||
// (limit), 2 outcomes (limit+1) — the write-set shape has no upper bound, but
|
||||
// the meaningful boundary here is "is there anything to be complete about".
|
||||
test('BOUNDARY: empty write-set (0 outcomes) is never complete', () => {
|
||||
assert.equal(writeSetComplete([]), false,
|
||||
'an empty write-set must not vacuously report complete — that would let a ' +
|
||||
'no-op masquerade as full success, the exact OR-into-one-flag class ADR-2143 §6 prohibits');
|
||||
});
|
||||
|
||||
test('BOUNDARY: single-outcome write-set (1 outcome) — applied true is complete', () => {
|
||||
assert.equal(writeSetComplete([{ surface: 'checkbox', applied: true }]), true);
|
||||
});
|
||||
|
||||
test('BOUNDARY: single-outcome write-set (1 outcome) — applied false is not complete', () => {
|
||||
assert.equal(writeSetComplete([{ surface: 'checkbox', applied: false }]), false);
|
||||
});
|
||||
|
||||
test('BOUNDARY: two-outcome write-set (2 outcomes) — both true is complete', () => {
|
||||
assert.equal(
|
||||
writeSetComplete([
|
||||
{ surface: 'checkbox', applied: true },
|
||||
{ surface: 'traceability', applied: true },
|
||||
]),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
// ─── Property test ───────────────────────────────────────────────────────────
|
||||
|
||||
const outcomeArb = fc.record({
|
||||
surface: fc.string({ minLength: 1, maxLength: 12 }),
|
||||
applied: fc.boolean(),
|
||||
});
|
||||
|
||||
test('property: writeSetComplete(ws) === (ws.length > 0 && every outcome applied)', () => {
|
||||
fc.assert(
|
||||
fc.property(fc.array(outcomeArb, { maxLength: 8 }), (ws) => {
|
||||
const expected = ws.length > 0 && ws.every((o) => o.applied);
|
||||
assert.equal(writeSetComplete(ws), expected);
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user