docs(#4906): ADR-4910 amendment — a write refuses on a document with any unreadable node (#4913)

* docs(#4906): ADR-4910 amendment — a write refuses on a document with any unreadable node

§5 scoped the parse error to the node. That is correct for reads and was never
examined for writes.

§5 was reasoned entirely from #4899, a read bug. Node-scoping is right there: a
ragged Progress table must not make phase list and init.progress fail, because
those are the commands a user needs to see what to repair. But the rule was
stated unqualified, and it licensed something never considered — phase.complete
mutating a ROADMAP.md whose Progress table it could not read. A partial-view
write persists a wrong answer rather than merely returning one.

Amendment: reads stay node-scoped; a write refuses when ANY node in the document
carries a parse error, whether or not the mutation targets it. A reader answers a
bounded question from a bounded region; a writer asserts that the document it
emits is the document it read, and cannot make that assertion about a region it
could not parse. §3's byte-stability does not rescue it — splicing untouched
bytes faithfully is not the same as knowing they were consistent with the change.

Rejected: refusing only when the mutation's own target node is unreadable. It is
the appealing middle and it does not hold — phase.complete writes **Plans:**
while deriving that value from plan/summary counts in a different region. The
regions a write depends on are not statically the regions it touches.

Downstream: Phase 1 ships both scopes plus a hasUnreadableNodes predicate the
serializer consults; Phase 2 gains a new acceptance criterion (a refused write
leaves the file byte-identical on disk); Phase 4's census gains a second axis and
must publish both counts.

Appended as a dated section per docs/contributor-standards.md pattern 1 — the
original Decision body is unchanged.

Refs #4906
Refs #4910

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

* docs(#4906): correct the amendment's motivating example and state the limit it exposes

An isolated review pass falsified the first draft's central example. The
correction is recorded in the amendment rather than quietly patched, 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 come from findPhaseInternal at src/phase.cts:3429-3434 — a
  filesystem scan of the phase directory. No PlanningDoc node holds them.
  That widens the dependency rather than narrowing it, and it makes an explicit
  limit necessary: a document-scoped write refusal protects the document's own
  consistency and says nothing about the correctness of a value sourced from
  outside the document. Recorded as a stated limit and named out of scope for
  this epic.

- The rejected alternative (refuse only when the mutation's own target node is
  unreadable) is re-grounded on two arguments that survive: it would almost
  never fire, since a verb locates the node in order to write it; and it guards
  something smaller than the operation performs, because serialization re-emits
  the whole file.

- §5's body names `phase list` as a consumer an unreadable Progress table would
  block. It is not one — cmdPhasesList (src/phase.cts:207) enumerates phases/
  and never opens ROADMAP.md. init.progress and roadmap.analyze are the real
  instances; the argument never needed three. §5's body left unmodified per the
  append-only rule, correction carried in the amendment, fold in at ratification.

- Clarified that the new WriteOutcome refusal sits on a different axis from §5's
  reserved document-level Result<PlanningDoc> parse failure, so no reader can
  conclude that reservation was reopened. A document can be valid to open and
  still refuse to be written.

Refs #4906
Refs #4910

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

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-21 00:17:10 -04:00
committed by GitHub
parent 01dbda9c49
commit fac0e9de86

View File

@@ -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 - 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`) - 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) - 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<PlanningDoc>`
*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.