diff --git a/docs/adr/4910-planning-document-seam.md b/docs/adr/4910-planning-document-seam.md index dd0d46e8f..c52cfe1db 100644 --- a/docs/adr/4910-planning-document-seam.md +++ b/docs/adr/4910-planning-document-seam.md @@ -497,3 +497,119 @@ Two consequences for the phases: - Fail-loud precedent: [ADR-1411](1411-resolution-provenance.md) — report provenance rather than fall open silently - Sibling consolidation: [#2121](https://github.com/open-gsd/gsd-core/issues/2121) (`phase-id.cts`) - Absorbed: [#4736](https://github.com/open-gsd/gsd-core/issues/4736), [#4793](https://github.com/open-gsd/gsd-core/issues/4793), [#4852](https://github.com/open-gsd/gsd-core/issues/4852), [#4862](https://github.com/open-gsd/gsd-core/issues/4862), [#4899](https://github.com/open-gsd/gsd-core/issues/4899), [#4900](https://github.com/open-gsd/gsd-core/issues/4900), [#4837](https://github.com/open-gsd/gsd-core/issues/4837), [#4865](https://github.com/open-gsd/gsd-core/issues/4865), [#4661](https://github.com/open-gsd/gsd-core/issues/4661), [#4605](https://github.com/open-gsd/gsd-core/issues/4605), [#4606](https://github.com/open-gsd/gsd-core/issues/4606), [#4499](https://github.com/open-gsd/gsd-core/issues/4499) + +## Amendment (2026-09-21): a write refuses on a document with any unreadable node + +**§5's node-scoping is correct for reads and was never examined for writes.** This amendment adds +the write rule. The original `## Decision` body is unchanged and §5 still holds as written. + +### What was missed + +§5 was reasoned entirely from [#4899](https://github.com/open-gsd/gsd-core/issues/4899), which is a +**read** bug — `roadmap.analyze` returning `phases: []`. Node-scoping is the right answer there: a +ragged Progress table should not make `init.progress` fail as well, and it must not, because that is +a command a user needs in order to *see what to repair*. A hand-editable format whose single typo +bricks every command that could diagnose it is a worse tool. + +> **Correction to §5's own wording, carried here rather than silently.** §5's body names `phase list` +> alongside `init.progress` as a consumer that would be blocked. That is wrong: `cmdPhasesList` +> (`src/phase.cts:207`) enumerates the `phases/` directory and never opens `ROADMAP.md`, so an +> unreadable Progress table cannot affect it either way. `init.progress` (`src/init.cts:3625`) does +> read `ROADMAP.md` and is a real instance; `roadmap.analyze` is the other. The argument stands on +> those two — it never needed three — but the example was not checked when §5 was written. §5's body +> is left unmodified per the append-only rule; fold this correction in at ratification. + +But "the error lives on the node" was then stated unqualified, and it silently licensed something the +original never considered: **`phase.complete` re-serializing a `ROADMAP.md` whose Progress table it +could not read.** Serialization re-emits the whole file, so the verb republishes every region — +including the one nobody could parse — while having understood only part of it. That is a +partial-view write, and it is a strictly worse failure than the one #4899 reports, because the read +bug returns a wrong answer while this one *persists* one. + +### The rule + +**Reads are node-scoped. Writes are document-scoped.** + +- A **read** of node *n* resolves independently: it returns *n*'s values, or `could-not-parse` with + *n*'s span. A sibling node's parse failure is invisible to it. Unchanged from §5. +- A **write** — any `PlanningDoc` mutation reaching the serializer — **refuses** when *any* node in + the document carries a parse error, whether or not the mutation targets that node. The refusal is a + typed `WriteOutcome` naming the unreadable node and its span, never a silent skip and never a + partial apply. + +**This does not reopen §5's "reserved for".** §5 reserves the document-level `Result` +*parse* failure for a document that is not a planning artifact at all, and that reservation stands +untouched. The refusal added here is a different type on a different axis — a `WriteOutcome` at +mutate/serialize time, on a document that parsed successfully and merely contains one or more nodes +carrying their own errors. A document can therefore be perfectly valid to *open* and still refuse to +be *written*, which is the intended state. + +The asymmetry is deliberate and is the point of the amendment: a reader is answering a bounded +question and can honestly answer it from a bounded region, while a writer is asserting that the +document it emits is the document it read. A writer that cannot read a region cannot make that +assertion about it, and §3's byte-stability guarantee does not rescue it — splicing untouched bytes +through faithfully is not the same as knowing they were consistent with the change being written. + +### Why not the alternatives + +- **Document-scoped for reads too** (strictest, one error type, simplest to test). Rejected: one + unreadable table blocks every consumer of that file, including the diagnostic commands, and the + artifacts are hand-editable by design — the epic's own constraint. +- **Node-scoped for writes** (what §5 left implied). Rejected on the partial-view write above. +- **Refuse only when the mutation's own target node is unreadable.** Rejected, for two reasons that + survive scrutiny: + 1. **It would almost never fire.** A verb locates the node in order to write it, so the target is + readable in the ordinary case by construction. A precondition that is satisfied whenever the + write is attempted is not a precondition. + 2. **It does not match the blast radius of the operation it guards.** Serialization re-emits the + **whole file** (§3). A write is therefore a document-wide assertion — *this is the document I + read* — regardless of how narrow the mutated span is. A document-wide act takes a + document-wide precondition; scoping it to one node guards something smaller than what is + actually happening. + +### Stated limit: this rule does not cover a write whose input comes from outside the document + +An isolated review of this amendment falsified its first draft's motivating example, and the true +mechanism is worth recording because it bounds what the rule can promise. + +The draft claimed `phase.complete` derives the value it writes into `**Plans:**` from *a different +region of the same document*. It does not. `planCount` and `summaryCount` are read at +`src/phase.cts:3429-3434` from `findPhaseInternal(cwd, phaseNum)` — a **filesystem scan of the phase +directory**. No `PlanningDoc` node holds those counts at all. + +That makes the dependency wider than the draft claimed, and it makes the limit explicit: + +> **A document-scoped write refusal protects the document's own consistency. It says nothing about +> the correctness of a value sourced from outside the document.** If the phase directory is +> mid-write, partially synced, or otherwise disagrees with reality, `phase.complete` writes a wrong +> count into a perfectly readable `**Plans:**` line and this rule will not object — correctly, since +> every node parsed. + +The write rule still holds on its own terms (see the two reasons above; neither depends on where the +written *value* came from). But the seam does not make a write *correct*, only *whole*, and an +implementer must not read §5-plus-this-amendment as a guarantee that a successful write was right. +Closing the outside-input gap is a separate concern about where verbs source their values, not about +how documents are parsed, and it is **not** in this epic's scope. It is recorded here so that the +next reader finds it stated rather than re-derives it from a surprise in production. + +### What changes downstream + +- **Phase 1** ships both scopes: the node-scoped read error of §5, **and** a document-level + `hasUnreadableNodes` predicate the serializer consults before applying any mutation. Both get + positive controls. +- **Phase 2** gains an acceptance criterion that is new, not reworded: *a mutation against a document + carrying any unreadable node refuses, with a typed outcome naming that node — and the file on disk + is byte-identical afterwards.* The last clause matters: "refused" must mean nothing was written, + which is a filesystem assertion, not a return-value one. +- **Phase 4's census gains a second axis.** It was "every read path that can return `[]` for an input + it failed to parse". It is now that, **plus** every write path that can apply a mutation to a + document with an unreadable node. Those are different call sets and the phase must publish both + counts. + +### Provenance + +Raised by a maintainer ruling on 2026-09-21, after §5 shipped in +[#4911](https://github.com/open-gsd/gsd-core/pull/4911). The gap was real: §5's node-scoping had been +recorded in that PR as an interpretation of #4906's *"an unparseable shape surfaces `could-not-parse` +with the offending span"* — a sentence that carries no read-or-write qualifier — and the +interpretation was made without examining the write side at all.