diff --git a/CONTEXT.md b/CONTEXT.md index 0eb8a720e..c3917daab 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -52,6 +52,9 @@ Module owning projection from dispatch results/errors to CLI `{ exitCode, stdout ### STATE.md Document Module Module owning STATE.md parse, field extraction, field replacement, status normalization, and frontmatter reconstruction. It does not scan `.planning/phases` and does not own persistence or locking; phase/plan/summary counts arrive from inventory/progress Modules as inputs, and read-modify-write paths remain Adapters. Source of truth: `gsd-core/bin/lib/state-document.cjs`. +### STATE.md Transition Module [Planned] +Module owning STATE.md lifecycle/maintenance transitions as intent-based methods (`beginPhase`, `advancePlan`, `completePhase`, `plannedPhase`, `milestoneSwitch`, `milestoneComplete`, `patch`, `sync`, `prune`, `update`). Pure core `(content, intent, deps) → newContent` with injected I/O (file read/write, lock, disk scan); consults a field-classification table that names each STATE.md field's class (`derived-from-body` | `derived-from-disk` | `derived-from-external` | `curated` | `free`) and its preservation policy. Supersedes the 14 scattered RMW callbacks in `state.cts` and the direct `writeStateMd` callers in `milestone.cts:352` and `phase.cts:1770`; verify's `regenerateState` factory-reset primitive stays as a direct `writeStateMd` call. Absorbs `syncStateFrontmatter` + `readModifyWriteStateMd`'s post-sync preservation block; Encoding 3 (`cmdStateBuildFrontmatter`) stays separate — read path concern. Sibling/super-module of the STATE.md Document Module; consumes its `stateReplaceField`/`stateExtractField` primitives. Body section structure (`## Current Position`, `## Session`, etc.) lives as a constants block inside the Module. Append-only transitions (`addDecision`, `addBlocker`, etc.) stay on today's RMW seam for now. Targets the #1760/#1761/#1743/#1695/#1264/#1255/#1257/#3242 bug cluster. Migration per ADR-1372 §T6 sequenced as substrate + `beginPhase` first (PR1), then transition-by-transition with characterization tests first per transition. Source of truth: `gsd-core/bin/lib/state-transition.cjs` (planned, generated from `src/state-transition.cts`). + ### Query Execution Policy Module Module owning query transport routing policy projection (`preferNative`, fallback policy, workstream subprocess forcing) at execution seam. diff --git a/docs/adr/1769-state-md-transition-module.md b/docs/adr/1769-state-md-transition-module.md new file mode 100644 index 000000000..1fa4a4b53 --- /dev/null +++ b/docs/adr/1769-state-md-transition-module.md @@ -0,0 +1,217 @@ +# ADR-1769: STATE.md Transition Module — intent-based transitions over scattered RMW callbacks + +- **Status:** Proposed (Phase 0); **Accepted** at Phase 7 closeout +- **Date:** 2026-06-27 (Phase 0) +- **Issue:** [#1769](https://github.com/open-gsd/gsd-core/issues/1769) — epic +- **Supersedes:** the policy portions of `syncStateFrontmatter` (`src/state.cts:1667–1743`), + `readModifyWriteStateMd`'s post-sync preservation block (`src/state.cts:2008–2119`), and + the format-detection duplication across 14 RMW callbacks in `state.cts` plus direct + `writeStateMd` callers in `phase.cts:1770` and `milestone.cts:352`. Does NOT supersede + `state-document.cjs`'s parse/extract/replace primitives — those are deep and stay. + +## Context + +`CONTEXT.md`'s "STATE.md Document Module" entry describes a deep leaf: parse, extract, +replace primitives, no persistence or locking. The friction lives in the **adapter layer** +above it. + +STATE.md writes happen via three mechanisms today: + +1. **`readModifyWriteStateMd(statePath, transformFn, cwd, options, clock)`** — the RMW seam. + Called from 14 sites in `state.cts`, each passing a closure that re-encodes format + detection, field preservation, and section mutation. +2. **Direct `writeStateMd` / `syncStateFrontmatter` calls** in `milestone.cts:352` + (milestone complete), `verify.cts:1899` (regenerateState recovery), and + `phase.cts:1770` (phase complete state-half, inside `writePlanningFileSet`). +3. **`cmdStateBuildFrontmatter`** — Encoding 3 of the preservation policy, used by the + read path (`cmdStateJson`). + +The **preservation policy** ("which fields win when frontmatter and body disagree?") is +encoded in **three places that drift independently**: + +- `syncStateFrontmatter` (`state.cts:1667–1743`) — preserves status when derived='unknown', + preserves milestone_name vs. placeholder, preserves stopped_at/paused_at/current_phase/ + current_phase_name/current_plan/progress when derived is empty. +- `readModifyWriteStateMd` post-sync block (`state.cts:2069–2113`) — restores progress + block when `resync=false`, preserves status/stopped_at when body source field unchanged + (#1230 delta heuristic). +- `cmdStateBuildFrontmatter` (`state.cts:1469+`) — own copies of the status/progress + preservation rules. + +The same field has different rules in different encodings. `progress` has two rules. +`status` has three. `stopped_at` has two. + +**Bug cluster this produces:** #1760 (prune no-ops), #1761 (sync writes wrong progress), +#1743 (patch clobbers curated field), #1695 (patch clobbers current_phase_name), #1264 +(resync=false restore), #1255/#1257 (format mismatch), #3242 (curated-progress ratchet). +Every fix is per-call-site and doesn't touch the other 13. + +The friction shape: ADR-857/1372/1508/3660 correctly identified the **leaf modules** and +gave them depth. The remaining friction is in the **adapter layer** that didn't finish +thinning. The recurring pattern is *"seam adopted for locating, hand-rolled code retained +for mutating"* — `tokenizeHeadings` was adopted, `replaceSection` was not. + +## Decision + +Introduce a **STATE.md Transition Module** as a sibling/super-module of the STATE.md Document +Module. Seven design decisions, resolved via `/grilling`: + +### 1. Module shape: Transition Module owns the full transaction + +The Module owns lock → read → apply transition → preserve policy → write. Interface is +intent-based: `beginPhase(statePath, phaseNum)`, `advancePlan(statePath, planId)`, etc. +The 14 RMW callbacks collapse to 14 one-line transition calls. + +*Rejected:* (B) Field Policy Module (locks stay outside) — leaves lock/scan bugs tangled +with policy bugs. (C) Widen state-document — shallow; format-detection + I/O concerns +leak in. + +### 2. Method set: 10 transitions (lifecycle + maintenance + milestoneComplete) + +`beginPhase`, `advancePlan`, `completePhase`, `plannedPhase`, `milestoneSwitch`, +`milestoneComplete`, `patch`, `sync`, `prune`, `update`. + +*Rejected:* (i) Lifecycle only (5) — leaves maintenance bugs #1760/#1743 alive. +(iii) All 16 writers — interface balloons, hurting depth. + +### 3. I/O shape: pure core + injected deps + +`transitionCore(content, intent, deps) → newContent` where +`deps = { progressProvider, writer, locker, clock }`. A thin adapter wires the real I/O. +Tests pass stubs. + +*Rejected:* (a) Absorb all four I/O concerns — too much fs surface in the core. +(b) Own transaction + lock; delegate disk scan — awkward coupling. + +This shape also enables `phase.cts:1770`'s use case: the transition core is called inside +`writePlanningFileSet(writes)` (multi-file transaction), not just inside `writeStateMd`. +Pure function = leverage across multiple orchestration shapes. Concretely: in Phase 3, +`completePhase` runs inside the multi-file ROADMAP+REQUIREMENTS+STATE transaction without +the transition core knowing it is inside a transaction. The core only sees +`(content, intent, deps) → newContent`; the orchestrator handles lock acquisition, +multi-file atomicity, and write ordering. This isolates the atomicity risk to the +orchestration layer, where the existing `writePlanningFileSet` already owns it. + +### 4. Policy model: field-classification table + +Each STATE.md field has a row: `{ source: body|disk|external|curated|free, preservation: +derive|preserve-when-unchanged|preserve-always|clear }`. Transitions declare which body +fields they touch; the core consults the table. + +*Rejected:* (α) Per-transition hardcoded policy — duplicates the rule × 10 transitions = +today's pain. (γ) Per-transition DSL — adds learning cost, still drifts. + +The table kills the bug class by construction: `patch` consulting the table sees +`current_phase_name` is `curated` and refuses to overwrite unless explicitly named. #1743 +impossible. + +### 5. External writers: 2 migrate, 1 stays + +- `milestone.cts:352` (milestone complete) → migrates to `milestoneComplete` transition. +- `phase.cts:1770` (phase complete state-half) → migrates to `completePhase` transition, + called as pure function inside the existing multi-file transaction. +- `verify.cts:1899` (`regenerateState` recovery) → **stays as direct `writeStateMd`**. + Factory-reset primitive, not a transition; nothing to preserve. + +### 6. Core scope: writes only + +Core owns the table + 10 transitions + format detection + preservation + frontmatter/body +invariant. Body section structure (`## Current Position`, `## Session`, etc.) lives as a +constants block at the top of the core. + +*Outside the core:* + +- Append-only transitions (`addDecision`, `addBlocker`, etc.) — stay on today's RMW. They + don't touch curated fields; routing them through the core adds interface width without + buying depth. +- Read path (`cmdStateBuildFrontmatter`, Encoding 3) — different concern (interpretation, + not preservation). Stays separate. +- `verify.cts:1899` regenerateState — factory reset, not a transition. + +*Rejected:* (B) Widen to reads — grows the table past preservation into interpretation. +(C) Absorb append-only — interface balloons to ~17 methods. + +### 7. Migration: substrate first, then transition-by-transition + +Per ADR-1372 §T6's "high risk, load-bearing, surgical, last" rating, big-bang is the wrong +shape. The migration sequence: + +- **Phase 0:** ADR + CONTEXT.md update (this PR). +- **Phase 1:** Substrate — transition core skeleton + table + section constants + + `beginPhase` migration + characterization tests. +- **Phase 2:** `advancePlan`. +- **Phase 3:** `completePhase` + `phase.cts:1770` migration (proves pure-core-inside- + multi-file-transaction). +- **Phase 4:** `plannedPhase`, `milestoneSwitch`. +- **Phase 5:** `milestoneComplete` + `milestone.cts:352` migration. +- **Phase 6:** `patch` (covers #1743, #1695). +- **Phase 7:** `sync`, `prune`, `update` (covers #1760, #1761). + +Per-transition discipline: characterization tests first (capture current behavior including +the bug-preservation we want to keep, e.g. #1230's delta heuristic), then migrate, then +verify old tests still pass, then add bug-fix tests for what the migration fixes. + +*Rejected:* (A) Big-bang — half-finished migration is what we're fixing. (C) Substrate +alongside, leave callbacks — parallel worlds don't converge (ADR-857's failure mode). + +## Consequences + +**Positive:** + +- Bug cluster killed structurally: #1760/#1761/#1743/#1695/#1264/#1255/#1257/#3242 each + become impossible-by-construction or one-row table fixes. +- ADR-1372 §T6 completed: `replaceSection` from the markdown sectionizer becomes the + body-mutation primitive inside the transition core. +- Test surface improves: tests pass `deps` stubs, no `.planning/phases/*` fixtures required + for policy tests. +- `phase.cts:1770` and `milestone.cts:352` stop reimplementing format detection and + section regexes (the `// allow-adhoc-markdown: pre-seam section write-modify` lint + suppressions in `milestone.cts` are deleted). + +**Negative:** + +- 10 transitions × characterization tests = substantial test volume in Phase 1 substrate. +- The field-classification table is a new shared artifact — adding a new STATE.md field + means one table row, but a new field *class* means updating every transition that + declares which classes it touches. (Single-enum: 5 classes; expected to be stable.) +- Migration is sequenced (8 phases, 8 PRs) — the codebase carries both old and new shapes + between Phase 1 and Phase 7. Each PR is independently shippable; the old shape is fully + retired only at Phase 7. + +**Neutral:** + +- `readModifyWriteStateMd` and `writeStateMd` stay exported during the migration; they're + used by the 7 append-only callbacks that aren't migrating. +- `cmdStateBuildFrontmatter` (Encoding 3) stays as the read path; future work may converge + it with the table, but that's out of scope here. + +## Alternatives considered + +1. **Widen STATE.md Document Module to absorb the policy.** Rejected — Document Module owns + pure parse/extract/replace; widening pulls format detection and preservation policy into + the leaf, hurting its depth. +2. **Per-transition hardcoded policy (option α in design).** Rejected — duplicates the rule + per transition; same field has different rules in different transitions = today's pain. +3. **All 16 writers in core (option iii in design).** Rejected — interface balloons to ~17 + methods, hurting depth. Append-only transitions don't touch curated fields. +4. **Big-bang migration (option A in design).** Rejected — ADR-1372 §T6 rates this surface + "high risk, surgical." Half-finished migration is what we're fixing. +5. **Substrate alongside, leave callbacks (option C in design).** Rejected — parallel worlds + don't converge. ADR-857's "decompose Core" produced today's half-finished state precisely + because new code moved and old code stayed. +6. **Defer until ADR-1372 §T6 finishes independently.** Rejected — §T6 has been deferred for + over a year precisely because there's no consumer for `replaceSection` in the state path. + The Transition Module is the consumer; the two land together. + +## Phases + +| Phase | Scope | Closes issue | Bug coverage | +|---|---|---|---| +| 0 | ADR + CONTEXT.md update | #1769 | — | +| 1 | Substrate + `beginPhase` | TBD | #1255, #1257, #3242 | +| 2 | `advancePlan` | TBD | — | +| 3 | `completePhase` + `phase.cts:1770` | TBD | — | +| 4 | `plannedPhase` + `milestoneSwitch` | TBD | — | +| 5 | `milestoneComplete` + `milestone.cts:352` | TBD | — | +| 6 | `patch` | TBD | #1743, #1695 | +| 7 | `sync`, `prune`, `update` | TBD | #1760, #1761 | diff --git a/docs/adr/README.md b/docs/adr/README.md index 5ecdce327..a1a9ee823 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -61,6 +61,7 @@ See **[CONTRIBUTING.md — "Proposing an ADR or PRD"](../../CONTRIBUTING.md#prop | [1411-resolution-provenance.md](1411-resolution-provenance.md) | Resolution must report provenance, not fall open silently | Accepted | | [1508-runtime-artifact-conversion-module.md](1508-runtime-artifact-conversion-module.md) | Runtime Artifact Conversion Module owns per-runtime content rewriting | Accepted | | [1593-skill-mapping-converter-methodology.md](1593-skill-mapping-converter-methodology.md) | Skill mapping & converter methodology across runtimes | Accepted | +| [1769-state-md-transition-module.md](1769-state-md-transition-module.md) | STATE.md Transition Module — intent-based transitions over scattered RMW callbacks | Proposed | ## Seam map