Introduce ADR-1769 establishing the STATE.md Transition Module design: intent-based transitions over scattered RMW callbacks. Seven design decisions resolved via /improve-codebase-architecture + /grilling + /domain-modeling: 1. Module shape: Transition Module owns the full transaction 2. Method set: 10 transitions (lifecycle + maintenance + milestoneComplete) 3. I/O shape: pure core (content, intent, deps) -> newContent 4. Policy model: field-classification table 5. External writers: 2 migrate (milestone, phase), 1 stays (verify) 6. Core scope: writes only; body sections as constants 7. Migration: substrate first, transition-by-transition Updates CONTEXT.md with the new Module entry; updates docs/adr/README.md index. No behavior change. Implementation lands in Phases 1-7 per the ADR's migration sequence. Closes #1769 Co-authored-by: review-bot <review-bot@gsd>
This commit is contained in:
Tom Boucher
@@ -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.
|
||||
|
||||
|
||||
217
docs/adr/1769-state-md-transition-module.md
Normal file
217
docs/adr/1769-state-md-transition-module.md
Normal file
@@ -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 |
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user