diff --git a/00-overview/decisions/0034-a-requirement-that-postdates-its-work-is-recorded.md b/00-overview/decisions/0034-a-requirement-that-postdates-its-work-is-recorded.md new file mode 100644 index 0000000..dc49390 --- /dev/null +++ b/00-overview/decisions/0034-a-requirement-that-postdates-its-work-is-recorded.md @@ -0,0 +1,153 @@ +# ADR-0034: A requirement that postdates its work is recorded, not cited + +**Status:** Accepted +**Date:** 2026-09-13 + +## Context + +The canonical-spec rule says every change cites an identifier that already +exists here ([GOV-R2](../../50-governance/README.md#the-gov-r-namespace), +[GOV-R3](../../50-governance/README.md#the-gov-r-namespace)), and +[GOV-R4](../../50-governance/README.md#the-gov-r-namespace) orders a +behavioural change so that the identifier exists in time to be cited. + +Eleven requirements locked into the [v2.0.0 +manifest](../../70-operations/versions/2.0.0.toml) are cited by nothing in the +product repository, and the cause is the same in nine of them: the requirement +was written to *describe* work that had already merged, so no trailer on the +implementing commit could have named it. Two are measurable to the second — +`E2-R22`'s implementation merged at 20:31:49 and the requirement was minted at +21:02:33; `E5-R27`'s implementation merged twenty-six seconds *after* the +specification change that created it. `A5-R18` ran the same way by sixty-three +minutes. The remaining two are ordinary slips: the specification merged first, +correctly, and the trailer named a sibling identifier instead. + +Three facts make this unfixable by the obvious routes. + +**A later citation does not reach backwards.** `spec_check.py` runs per pull +request against that pull request's own text. A subsequent pull request naming +the identifier passes for itself and leaves the original exactly as uncited as +it was. The product repository recorded all eleven in one page and its author's +own verdict was that *"it records the id, it does not cite the work"*. That is +correct. + +**Amending the merged commits was considered and rejected** by the owner: +published history is not traded for one word per commit. + +**Four of the eleven were graded uncited for the shape of their documentation, +not for its absence.** `A3-R23`, `E5-R27`, `E6-R14` and `E6-R16` are named in a +pull-request body sentence rather than on a `Spec:` line. `spec_check.py` keeps +only identifiers standing on a line matching `^\s*Spec:`, so prose is invisible +to it — a guard keyed on a line format silently reclassifying *documented +differently* as *not documented*. + +On a strict reading of the [definition of +done](../../40-quality/definition-of-done.md), `releasable` was unreachable for +v2.0 and would stay unreachable however much work was done, because the only +change that could satisfy the rule has already merged. + +## Decision + +**Three parts, taken together. None of them is the whole answer.** + +1. **A record satisfies the citation box where a citation was impossible.** + Where a requirement did not exist on this repository's default branch at the + time the work it describes merged, a record in the implementing repository — + naming the identifier, the seam that implements it, and the commit that + merged it — ticks the definition of done's citation box for that requirement + ([GOV-R26](../../50-governance/README.md#the-gov-r-namespace)). + + The record is not a citation and does not pretend to be one. What it buys is + the one thing that was genuinely missing: an identifier appearing nowhere + cannot be told apart from an identifier nobody implemented, and separating + those two otherwise costs every future audit a hand re-derivation of all + eleven seams. That cost is real — of the eleven derivations that prompted + this, two came back different from the audit that had attributed them. + + It is bounded, and the bound is the point. A trailer that *could* have + carried the identifier and did not is a missing trailer, and no record + repairs it ([definition of + done](../../40-quality/definition-of-done.md#when-the-requirement-postdates-the-work)). + +2. **The ordering rule is restated about the requirement, not about the + change.** A requirement exists on the default branch before the work it + describes merges, so the implementing change can cite it + ([GOV-R27](../../50-governance/README.md#the-gov-r-namespace)). GOV-R4 + ordered a *behavioural change*; a requirement written only to describe + existing behaviour is not one, which is exactly the gap nine of the eleven + walked through. The single legitimate way to arrive late is the founding + one — this specification was written for a product that already existed — + and that case is recorded under GOV-R26 rather than excused. + +3. **The gate names what it read.** `spec_check.py` still accepts a citation + only from a `Spec:` trailer, and now reports an identifier the text names + anywhere else as *named but not cited*, on both the failing and the passing + path ([GOV-R28](../../50-governance/README.md#the-gov-r-namespace)). It can + no longer say "uncited" about an identifier written three lines above. + +## Alternatives + +| Alternative | Why it lost | +|-------------|-------------| +| **Hold the release until every requirement is cited by the change that implemented it** | Unreachable by construction. The changes in question have merged; nothing a future pull request does makes a past one cite anything. It is a rule that can only be satisfied by rewriting history, which is the next row. | +| **Amend the eleven merged commits to add the trailers** | Rejected by the owner, and rightly: it rewrites published history, invalidates every reference to those hashes, and buys one word per commit. It would also make the record *less* true — a trailer back-dated onto a commit written before the identifier existed asserts something that did not happen. | +| **Re-derive the eleven seams at every audit instead of writing them down** | This is the status quo, and it has a measured error rate: two of eleven attributions in the first derivation were wrong, both crediting the pull request that touched the seam last rather than the one that implemented the requirement. Paying that cost repeatedly, and getting a different answer each time, is worse than paying it once in public. | +| **Widen the gate to accept an identifier named in prose** | A sentence names an identifier without citing it. *"Unlike A3-R23"* and *"superseded by A3-R23"* would both pass, and the gate would be grading changes as cited on a reading no reviewer agreed to. The trailer is a deliberate, parseable act; that is its whole value. Reporting prose distinctly gets the information without spending the guarantee. | +| **Withdraw the eleven identifiers and mint new ones after the fact** | Forbidden ([GOV-R8](../../50-governance/README.md#the-gov-r-namespace), [GOV-R10](../../50-governance/README.md#the-gov-r-namespace)), and it would be a fabrication: the requirements are correct and the work is done. Renumbering to make a gate happy is the tail wagging the dog. | +| **Let the exception cover the slips too, and close all eleven** | It would close the release cleanly and it is exactly the abuse this record has to refuse. A trailer that could have named the identifier and did not is the case the citation rule exists for; an exception that swallows it is not an exception, it is a repeal. Two of the eleven stay open because of this, and that is the correct answer. | + +## Consequences + +### Positive + +- The nine requirements that could never have been cited have a stated, + bounded way to satisfy the definition of done, with the reason attached. +- The gap GOV-R4 left — a descriptive requirement is not a behavioural change + — is closed rather than relied on not to recur. +- The gate stops misreporting *documented in the wrong shape* as *not + documented*, and stops passing in silence when a squash trims a multi-id + `Spec:` line down to one. +- An audit asking *"what implements this?"* gets an answer that was derived + once, against the seam, rather than re-derived differently each time. + +### Negative + +- Two of the eleven — `A3-R23`, and the second half of `E6-R14` — are slips, + not inversions, and this record deliberately does not close them. They need a + later change on the same seam carrying the trailer, or a recorded override + ([overrides.md](../../50-governance/overrides.md)). The release question they + belong to is not settled here. +- GOV-R26 and GOV-R27 are both verified in review and by nothing else. The gate + checks that a cited identifier exists, never when it came to exist. A rule + enforced by vigilance decays, and these two now depend on it. +- "Satisfies the citation box" is a second route to a box that previously had + one route. Every such route is a place a future contributor can stand while + arguing, which is why the disqualifying cases are enumerated rather than + implied. + +### Neutral + +- No merged commit changes. No version manifest changes, and no version's + status moves; whether v2.0.0 becomes `releasable` depends on more than this. +- Consumer repositories change nothing about how they cite. The gate is + stricter in what it says and identical in what it accepts. + +## Revisit if + +- Ordering becomes machine-checked — the [open + question](../../90-appendix/open-questions.md) that would retire most of + GOV-R27's reliance on review. +- A second batch of requirements arrives needing the GOV-R26 record. Once is + history being caught up with; twice is GOV-R27 not working. +- The *named but not cited* warning turns out to fire on ordinary prose often + enough that readers learn to skip it, in which case it belongs on the failing + path only. + +## Related + +- [50-governance/canonical-spec.md](../../50-governance/canonical-spec.md) · + [50-governance/change-lifecycle.md](../../50-governance/change-lifecycle.md) +- [40-quality/definition-of-done.md](../../40-quality/definition-of-done.md#when-the-requirement-postdates-the-work) +- [ADR-0030](0030-the-tag-governs-the-workflow-not-what-it-reads.md) — the same + shape one layer down: a guard that was not reporting what it did and did not + cover diff --git a/00-overview/decisions/README.md b/00-overview/decisions/README.md index 29f7b65..b587e25 100644 --- a/00-overview/decisions/README.md +++ b/00-overview/decisions/README.md @@ -65,6 +65,7 @@ of them invent a decision that was not already made. | [0031](0031-a-signed-wrap-is-independent-of-its-channel.md) | A signed epoch wrap is trusted on its signature, not on its channel | Accepted | Supersedes part of [0016](0016-noise-transport-zero-knowledge-relay.md) | | [0032](0032-all-four-stores-additive-to-direct-download.md) | All four stores, and direct download is not retired | Accepted | Supersedes part of [0006](0006-nativephp-desktop-shell.md) and [0019](0019-asymmetric-release-publish.md) | | [0033](0033-the-relay-carries-pairing-not-the-ledger.md) | The relay carries pairing material, not the ledger, and "zero-knowledge" is retired | Accepted | Supersedes part of [0016](0016-noise-transport-zero-knowledge-relay.md) and [0004](0004-local-only-hosting.md) | +| [0034](0034-a-requirement-that-postdates-its-work-is-recorded.md) | A requirement that postdates its work is recorded, not cited | Accepted | New | > **Not here:** the spec's own governance rules. Those are not architectural — > no component's design depends on them. They live in diff --git a/40-quality/definition-of-done.md b/40-quality/definition-of-done.md index 7750a29..7e524ce 100644 --- a/40-quality/definition-of-done.md +++ b/40-quality/definition-of-done.md @@ -8,7 +8,9 @@ A change is done when every box below is ticked. Not most of them. - [ ] **It cites a requirement identifier that already exists on the canonical spec**, in a commit trailer and in the pull-request body - ([Q-R23](README.md#the-q-r-namespace)). + ([Q-R23](README.md#the-q-r-namespace)). One narrow exception, for a + requirement that did not exist when the work merged: + [below](#when-the-requirement-postdates-the-work). - [ ] The commit subject is conventional, and every commit carries a sign-off matching its author. - [ ] Formatting, static analysis, and the full test suite pass. @@ -29,6 +31,68 @@ A change is done when every box below is ticked. Not most of them. saying what changed for them — it is what the release body will carry. - [ ] The behaviour was walked in a browser, on both desktop and phone widths. +## When the requirement postdates the work + +One exception to the citation box above, and it is narrow. + +A requirement minted to *describe* code that has already merged cannot be cited +by the change that implemented it. There was no identifier to put in the trailer +when that commit was written, and no later pull request repairs it: the gate +runs per pull request against that pull request's own text, so a citation added +afterwards passes for the later change and leaves the original exactly as +uncited as it was. Amending the merged commit is not the way out either — +published history is not traded for one word. + +For that case, the box is ticked by a **record** in the implementing repository +naming, for each such identifier, the seam that implements it and the commit +that merged it +([GOV-R26](../50-governance/README.md#the-gov-r-namespace)). + +**Why a record is enough here.** An identifier that appears nowhere in the +implementation repository cannot be told apart from an identifier nobody +implemented. Both answer a search with nothing, and the second is a release +blocker while the first is bookkeeping. The record separates them, and it +separates them once — the alternative is that every future audit re-derives the +same seams by hand from the same diffs and has to be believed. That cost is not +hypothetical: of the eleven v2.0 requirements this rule was written for, two +re-derivations came back different from the audit that had attributed them. + +**What the record is not.** It is not a citation. The implementing change stays +uncited, the gate's answer about it does not change, and this section is not a +second way to satisfy +[GOV-R2](../50-governance/README.md#the-gov-r-namespace). It ticks one box, for +one requirement, on the stated ground that the box was unreachable. + +### What does not qualify + +- **A trailer that could have carried the identifier.** Where the identifier + stood on the canonical specification's default branch for the life of the + implementing pull request and the trailer left it out, that is a missing + trailer, not a missing opportunity. The box stays unticked. It closes when a + later change on that same seam carries the trailer, or by a recorded override + ([overrides.md](../50-governance/overrides.md)) — not here. +- **Work that has not merged yet.** If the code is still in flight, the + specification change goes first and the trailer names it + ([GOV-R27](../50-governance/README.md#the-gov-r-namespace)). A record written + for work that could still have cited its requirement is the failure this + exception exists to survive, wearing the exception's clothes. +- **The record and the work in the same pull request.** The same thing said + another way: two changes in flight at once are ordered, not recorded. +- **An entry that cannot be checked.** An entry naming no commit, or naming a + commit that does not implement the requirement, is worse than no entry — it + answers the audit's question wrongly and stops anyone asking again. The + implementing commit is derived against the seam the requirement is about, not + taken from whichever pull request touched the file last. +- **An entry standing in for the requirement.** The requirement still lives on + the canonical specification, written and reviewed as prose. The record lives + in the implementing repository and points at it. + +Where the two merge timestamps are close enough that the judgement is genuinely +arguable — the narrowest case on record is twenty-six seconds — the record says +which pull request was already open and waiting, and a reviewer rules on it. A +call that could honestly go either way is an override to record, not a box to +tick quietly. + ## For a new user-scoped surface - [ ] A cross-user test asserts not-found @@ -111,6 +175,9 @@ A change is done when every box below is ticked. Not most of them. - Not "I'll add the test after". A requirement without a test is a claim. - Not "the documentation can follow". For a behavioural change the specification goes **first**, not after. +- Not "a record instead of a citation". The record above exists because the + citation was impossible, never because it was inconvenient + ([GOV-R27](../50-governance/README.md#the-gov-r-namespace)). ## Related diff --git a/50-governance/README.md b/50-governance/README.md index d3c2938..d44acb1 100644 --- a/50-governance/README.md +++ b/50-governance/README.md @@ -48,6 +48,9 @@ from. | **GOV-R23** | An issue that describes a behaviour the specification does not cover MUST become a specification change before implementation. | | **GOV-R24** | The project lead holds override authority; every override MUST be recorded ([overrides.md](overrides.md)). | | **GOV-R25** | Where a decision is genuinely unmade, the document MUST say so under an explicit open-question heading rather than inventing an answer. | +| **GOV-R26** | Where a requirement did not exist on the canonical specification's default branch at the time the work it describes merged, the implementing repository MUST carry a record naming that identifier, the seam that implements it, and the commit that merged it. That record satisfies the definition of done's citation box for that requirement; it does not make the implementing change cited, and it MUST NOT be used where a trailer could have carried the identifier ([40-quality/definition-of-done.md](../40-quality/definition-of-done.md#when-the-requirement-postdates-the-work)). | +| **GOV-R27** | A requirement MUST exist on the canonical specification's default branch before the work it describes merges, so that the implementing change can cite it. Minting a requirement from work that has already merged is permitted only where that work predates this specification, and MUST be recorded under GOV-R26. | +| **GOV-R28** | The governance gate MUST read a citation only from a `Spec:` trailer, and MUST report an identifier the text names anywhere else as named-but-not-cited rather than as absent. | ## Where to ask diff --git a/50-governance/canonical-spec.md b/50-governance/canonical-spec.md index bf34950..739088a 100644 --- a/50-governance/canonical-spec.md +++ b/50-governance/canonical-spec.md @@ -39,6 +39,17 @@ Several identifiers are fine: Spec: E1-R9, E1-R13, ADR-0014 ``` +**Only the trailer counts.** The gate reads identifiers from lines beginning +`Spec:` and from nowhere else, so a body that says *"this implements B5-R13"* +cites nothing. That is deliberate: a trailer is a parseable, deliberate act and +a sentence is not — *"unlike B5-R13"* and *"superseded by B5-R13"* name an +identifier while citing none of it, and a gate that accepted them would grade a +change as cited on a reading no reviewer agreed to. What the gate does instead +is **say so**. An identifier it finds outside a trailer is reported as +named-but-not-cited, never as absent +([GOV-R28](README.md#the-gov-r-namespace)) — so a body naming four identifiers +on a `Spec:` line that a squash trimmed to one no longer passes in silence. + ## What is citable | Kind | Example | Defined in | @@ -79,7 +90,10 @@ diff. ## Ordering A behavioural change's specification change **merges first** -([GOV-R4](README.md#the-gov-r-namespace)). +([GOV-R4](README.md#the-gov-r-namespace)), and more widely: a requirement exists +before the work it describes merges, so the implementing change can cite it +([GOV-R27](README.md#the-gov-r-namespace), +[change-lifecycle.md](change-lifecycle.md#the-requirement-exists-before-the-work-merges)). **Ordering is not machine-checked** — the gate verifies existence, not sequence. It is verified in review, and hardening it is tracked work. Recorded plainly so @@ -121,6 +135,8 @@ tree under review instead. - It does not check that the citation is **apt**. Citing an unrelated identifier passes the gate and fails review. - It does not check **ordering**, as above. +- It does not accept an identifier named in prose. It **names** one, distinctly, + and passing still needs the trailer ([GOV-R28](README.md#the-gov-r-namespace)). - It does not check that the implementation actually **satisfies** the requirement. That is the reviewer's job, and the test's. diff --git a/50-governance/change-lifecycle.md b/50-governance/change-lifecycle.md index 226b153..1bc252a 100644 --- a/50-governance/change-lifecycle.md +++ b/50-governance/change-lifecycle.md @@ -44,6 +44,45 @@ change ──▶ cites GOV-R12 ──▶ review ──▶ merge Dependency bumps, formatting, pipeline mechanics, documentation corrections. Straight through. +## The requirement exists before the work merges + +[GOV-R4](README.md#the-gov-r-namespace) orders a **behavioural change**: its +specification change merges first. That left a gap, and the gap has been walked +through. A requirement written to *describe* work — not to change behaviour, +only to write down what the product already does — is not a behavioural change, +so nothing above ordered it, and it was routinely minted after the work it +describes had merged. Eleven of the requirements locked into the v2.0 manifest +are in that state, two of them measurable to the second: one implementation +merged thirty minutes before its identifier existed, and another merged +twenty-six seconds after the specification change that created it. + +The consequence is not cosmetic. A change cannot cite an identifier that does +not yet exist, so those eleven are cited by nothing — and a reader asking *"what +implements this?"* gets the same silence an unimplemented requirement gives. + +So the rule is stated about the requirement rather than about the change, and it +is wider than GOV-R4: + +> **A requirement exists on the default branch before the work it describes +> merges**, so that the implementing change can cite it +> ([GOV-R27](README.md#the-gov-r-namespace)). + +There is one legitimate way to arrive late, and it is the founding one: this +specification was written for a product that already existed, so a requirement +describing behaviour older than the specification itself was always going to +postdate its work. That case is **recorded** rather than ordered +([GOV-R26](README.md#the-gov-r-namespace), +[40-quality/definition-of-done.md](../40-quality/definition-of-done.md#when-the-requirement-postdates-the-work)). +Nothing else is: a requirement minted from work that merged while this +specification already covered its area is an inversion, and the record is not +its excuse. + +**Neither rule is machine-checked.** The gate verifies that a cited identifier +exists, not when it came to exist. Ordering is read in review, as it always was +([canonical-spec.md](canonical-spec.md#ordering)). A rule enforced by vigilance +alone decays, and this one says so about itself rather than implying a guarantee +it does not have. + ## Discussion first, for anything large Open a discussion, or ask in [the Discord](https://discord.nightworks.io), before diff --git a/50-governance/contributing.md b/50-governance/contributing.md index a45a223..0f30125 100644 --- a/50-governance/contributing.md +++ b/50-governance/contributing.md @@ -59,7 +59,9 @@ These are enforced. Fighting them is a slower path than following them. Spec: B5-R13 ``` - Routine maintenance cites `GOV-R12`. + Routine maintenance cites `GOV-R12`. A sentence naming an identifier is not a + citation — only the trailer is read — and the gate will tell you which + identifiers it found outside one. - Branch names follow a type-and-slug shape. - The default branch requires **signed** commits; configure signing before you diff --git a/90-appendix/open-questions.md b/90-appendix/open-questions.md index ec94651..6fb3744 100644 --- a/90-appendix/open-questions.md +++ b/90-appendix/open-questions.md @@ -199,7 +199,14 @@ The governance gate verifies that a cited identifier **exists**, not that the specification change **merged first**. Ordering is verified in review. Hardening it is tracked and unscheduled. -*In: [50-governance/canonical-spec.md](../50-governance/canonical-spec.md#ordering)* +[GOV-R27](../50-governance/README.md#the-gov-r-namespace) widened what is +unchecked rather than narrowing it: the ordering rule now covers every +requirement and not only a behavioural change's, and nothing mechanical reads +either. Eleven requirements in the v2.0 manifest are the evidence that review +alone did not catch it ([ADR-0034](../00-overview/decisions/0034-a-requirement-that-postdates-its-work-is-recorded.md)). + +*In: [50-governance/canonical-spec.md](../50-governance/canonical-spec.md#ordering) · +[50-governance/change-lifecycle.md](../50-governance/change-lifecycle.md#the-requirement-exists-before-the-work-merges)* ### Should a decision record whose consequences have expired say so? diff --git a/scripts/spec_check.py b/scripts/spec_check.py index dbed554..6fbd8e5 100644 --- a/scripts/spec_check.py +++ b/scripts/spec_check.py @@ -7,6 +7,9 @@ Enforces: GOV-R2 a citation is present GOV-R3 every cited identifier exists on spec@main + GOV-R28 a citation is read from a `Spec:` trailer and from nowhere else, and + an identifier named anywhere else is reported as named-but-not-cited + rather than as absent Ordering (GOV-R4) — that a behavioural change's spec PR merged first — is not machine-checked here yet; it is verified in review. Hardening this is tracked in @@ -48,6 +51,22 @@ def cited_ids(text: str) -> set[str]: return ids +def named_ids(text: str) -> set[str]: + """Identifiers the text names anywhere other than on a `Spec:` line. + + A sentence is not a citation and this does not make it one: the trailer + stays the only form that passes, because it is a parseable and deliberate + act where "unlike A3-R23" and "superseded by A3-R23" are neither. + + This exists so the gate can tell *cites nothing* apart from *names it three + lines above and never put it in a trailer*. It could not draw that + distinction before, and both read as uncited — which is how four v2 + identifiers were graded uncited for being documented in the wrong shape + (GOV-R28). + """ + return set(CITE_ANY.findall(SPEC_TRAILER.sub("", text))) + + GUIDANCE = """ This change does not cite a spec identifier that exists on spec@main. @@ -61,6 +80,7 @@ def cited_ids(text: str) -> set[str]: - Implementing something specified? Cite the requirement. - Changing behaviour? Open a spec PR first, then cite the new ID. - Routine maintenance (deps, formatting, CI)? Cite GOV-R12. + - Named it in a sentence already? Only the `Spec:` line is read. Guide: https://github.com/beatrax-app/spec/blob/main/50-governance/contributing.md """ @@ -89,8 +109,23 @@ def main() -> int: return 2 cited = cited_ids(text) + # Reported, never counted. An identifier that does not resolve is not a + # citation under any reading, so naming one in prose says nothing worth + # printing; one that does resolve is the ambiguous case worth naming. + named = ", ".join(sorted({i for i in named_ids(text) if i in defined} - cited)) + if not cited: - print("::error::no `Spec:` citation found") + if named: + print(f"::error::no `Spec:` trailer found; the text names {named} outside one") + print(f""" +A citation is read from a line beginning `Spec:` and from nowhere else, so this +text cites nothing. What it does name, in prose, the specification defines: +{named}. If this change implements that, the fix is the trailer — on a commit +and in the pull-request body: + + Spec: {named}""") + else: + print("::error::no `Spec:` citation found") print(GUIDANCE) return 1 @@ -101,6 +136,10 @@ def main() -> int: return 1 print(f"spec-check: OK — cites {', '.join(sorted(cited))}") + if named: + print(f"::warning::named but not cited: {named} — named outside a `Spec:` " + "line, so this change does not cite it. Add it to the trailer if " + "this change implements it.") return 0