diff --git a/.github/workflows/run_pytest.yml b/.github/workflows/run_pytest.yml index 7e7b354..f259bf3 100644 --- a/.github/workflows/run_pytest.yml +++ b/.github/workflows/run_pytest.yml @@ -40,7 +40,7 @@ jobs: # `actions/checkout` takes the sibling's OWN default branch — which for # views-appwrite is `development` — while ADR-014 §3 says the authority for a claim # about another repository is that repository's `main`. Two checks in - # test_env_declaration read the working tree and a third demands reachability from + # test_env_declaration read the sibling's `main` and a third demands reachability from # `main`; pointed at `development` they would eventually demand contradictory things. # # `actions/checkout` refuses a path outside $GITHUB_WORKSPACE, so the siblings go diff --git a/docs/ADRs/014_claims_and_the_guards_that_carry_them.md b/docs/ADRs/014_claims_and_the_guards_that_carry_them.md index afddd36..102b66e 100644 --- a/docs/ADRs/014_claims_and_the_guards_that_carry_them.md +++ b/docs/ADRs/014_claims_and_the_guards_that_carry_them.md @@ -79,6 +79,40 @@ Where a guard's inputs are declared (a path list, a module list, a set of names) that the inputs are real**. `rglob` on a nonexistent directory yields nothing rather than raising; a list of paths that no longer exist does not fail a scan, it empties it. +**Amended 2026-08-12.** Mutation-proving is necessary and it is not sufficient, and the +gap has a shape worth naming. + +A mutant list written by the author of the guard covers the cases the author already had +in mind — which are, by construction, the cases the guard already handles. Measured on the +guard that keeps coordinate values out of this public repository: **thirteen input forms +proposed by its author, thirteen caught; twenty-nine proposed independently, fifteen +missed.** Every case in the first list genuinely passed. *"Proven against N mutations"* +reads as a statement about the guard when it is a statement about N. + +The useful rule is a **diagnostic**, not a process: + +> If a guard can only be proven against inputs you invented, that is the signal the guard +> is on the wrong side of a boundary — it is verifying a fact you do not own. + +Prefer moving the check to where the fact lives, or anchoring the mutant list in something +real: this repository's own corpus, the registry's actual rows, an outcome you can observe. +The no-copy scan's stopping rule is the worked example — it derives its list of accepted +forms from this repository's tracked markdown, so a form no document here uses is not a gap. + +Where that is impossible, two cheap obligations: + +- the mutant list is a **declared artifact in the test file**, not a paragraph in a pull + request (`tests/test_ci_sibling_coverage.py::_MUTANTS` is the pattern); +- **a proof must be able to fail** — mutate the function, watch the proof go red, record + it. An assertion whose inputs are derived from the thing under test proves nothing, and + three of those shipped here in two days. + +Deliberately **not** adopted: a requirement that someone other than the author write the +mutants for every guard. The friction would exceed the disease for a single maintainer. +The diagnostic above is the load-bearing part; independent mutants are worth buying only +for the silent-failure class — a leak, an invisible delivery — where the cost of missing +is not a red build but a thing nobody ever notices. + ### §3 Prefer a false negative to a false alarm A guard that cries wolf gets deleted, and then the rule it carried is unguarded — which is diff --git a/docs/ADRs/016_ci_read_access_to_private_siblings.md b/docs/ADRs/016_ci_read_access_to_private_siblings.md index bb0cba9..3179400 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -126,6 +126,12 @@ The **`public?` column is not, and cannot be** — which is why `public` was del because a reader needs it to follow the argument. Nothing reads this table; if it drifts from `SIBLINGS`, only a human will notice. +**Trigger, since an unenforced claim needs one (ADR-014 §4):** re-read this column the next +time a repository in it changes visibility, or the next time CI fails to check one out. The +owner is whoever makes that change. It is deliberately not machine-checked — verifying it +means a network call from a test suite that makes none, and the failure it would catch +(a tokenless checkout of something now private) already fails loudly at the checkout step. + ### §5 CI downloads exactly what that list says, and a test enforces it The workflow downloads every sibling marked `ci_checkout=True`. diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index 2710c15..80ae053 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -142,9 +142,6 @@ Concretely, for the delivery label: ### The order these land in is part of the decision, not an afterthought -**None of those three is in place yet**, and the present tense above describes the decided -end state rather than today's behaviour. - **The obvious sequence has a hole, and it is green.** If step 1 lands, then step 2 replaces the source-reading check with a registry read, and step 3 has not happened yet, the state is: the registry declares a string a human typed; we check our copy against that string and @@ -250,6 +247,52 @@ because a download that has quietly stopped earning its place is how the previou of all this went wrong, and because ADR-016 cannot know it: the decision that obsoletes it is this one. +### §7a What a check in this repository may rest on + +The rule §7 arrives at for one case generalises, and it is worth stating on its own, +because every defect this document's arc produced sat on the wrong side of it. + +> **A check here may rest on a fact we own, on a fact another repository has *declared* in +> the public registry, or on an *outcome* we can observe. It may never rest on another +> repository's implementation.** +> +> Where a needed fact is none of those three, the check is not built here. It is requested +> as a declaration, requested as a consumer obligation, or registered as an accepted gap +> with a named owner and trigger. + +[ADR-003](003_authority_of_declarations_over_inference.md) already says declarations over +inference. It was applied rigorously to the product and not at all across the repository +boundary, and that asymmetry is where the cost landed. + +**The evidence, because this is a rule bought with two days.** The source-reading check +broke twice in twenty-four hours — views-faoapi on 11 August, views-crafdapi on the 12th +— each time because that repository refactored a literal argument into a named constant. +Verified: commits `8615574` and `0c493ae` each introduce that constant *and* add that +repository's registry-binding test. **The improvement and the breakage were one edit.** +A mechanism a consumer breaks by improving itself will keep breaking. + +The corollary is the one that costs something to accept: when a fact is none of the three, +the honest move is to say so and register it, not to build a proxy. The delivery-label +composition is exactly that — see §8 and register C-92. + +### §7b An ADR records a decision, and never the current state of the code + +This document and [ADR-016](016_ci_read_access_to_private_siblings.md) both described what +the code did at the moment of writing — *"this is the shape the registry check now has"*, +*"today the check still reads the consumer's source"*. Every such sentence needed an +erratum the first time the code moved, and between them they produced nine stale claims in +two days, one of which repealed a rule in this document on a premise that was false by six +hours. + +> **An ADR states a decision, its reasoning, and what would reverse it. It does not +> describe the current implementation.** Where a reader needs to know what the code does +> now, the ADR names the test or module that answers, and the answer lives there. + +A decision record that doubles as a description of the code is a second copy of the code, +and it rots on a schedule nobody is watching. This clause is written into ADR-017 rather +than ADR-000 because it was learned here; if it survives contact with a second document it +belongs in the ADR conventions. + ### §8 What this does **not** verify, stated plainly This checks **our copy against the declaration**. It does **not** check the consumer's code diff --git a/pyproject.toml b/pyproject.toml index d76c633..043be9d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "views-postprocessing" -version = "1.0.0" +version = "1.1.0" description = "" authors = [ "Dylan Pinheiro ", diff --git a/reports/post_mortems/2026-08-12_the_guards_that_did_not_guard.md b/reports/post_mortems/2026-08-12_the_guards_that_did_not_guard.md new file mode 100644 index 0000000..bf010ac --- /dev/null +++ b/reports/post_mortems/2026-08-12_the_guards_that_did_not_guard.md @@ -0,0 +1,154 @@ +# Post-Mortem — The Guards That Did Not Guard + +**Period:** 2026-08-11 → 2026-08-12 (two execution days; PR #239 merged 19:16 on the 11th, PR #240 — the release — merged 20:31 on the 12th). + +**Scope:** Build a cross-repository guard surface that detects drift between this repository and its siblings; discover through independent review that most of that surface did not do what it claimed; repair it; ship. Covers PRs #239, #252–#255 and the release #240, plus epic #241 (stories #242–#250) and the risk-register range **C-89…C-97**. + +**Final stats:** 6 PRs merged · 44 commits on `main` · 9 new register entries (97 total, 24 open) · 3 ADR errata across ADR-016/ADR-017 · 2 cross-repository issues filed (views-faoapi#390, views-crafdapi#55) · `tests/*.py` 9,200 → 9,308 against a package of 2,998 lines · **zero delivered bytes changed** (`CONTRACT_VERSION` 1.5 on both sides, `tests/fixtures/` byte-identical). + +--- + +## Why this effort + +Three problems arrived together. + +**The registry drift check was blocking releases.** It compared this repository's pinned edition *label* against views-appwrite's current one. That registry moved five editions in four days, four of them recording console observations that obliged nobody. Every move reddened this repository, and merging to `main` is the release to the UN FAO. Registered as **C-86**. + +**Two consumer-facing checks were owed** — issue #238's D3 and D4: verify each partner's delivery label against the public coordinate registry, and retire the check that read the consumer's own source once that consumer checked itself. + +**And the failure mode underneath both is invisible by construction.** If the name this repository uploads under and the name the consumer filters on drift apart, the upload succeeds, the file is stored and billed, the consumer's endpoint returns empty, and nothing raises anywhere. ADR-013 §4.1a calls it *"invisible to the consumer, not merely degraded."* It is the one failure this platform most needs a machine to notice. + +PR #239 addressed all three. It was reviewed twice before merging, and merged green. + +**Then a post-merge review found fifteen defects in it, and every one reproduced.** + +--- + +## Timeline + +| Time (CEST) | PR / commit | What happened | +|---|---|---| +| 08-11 19:16 | **#239** | Registry check rewritten to match on rows rather than the edition label; #238's D3/D4 land. Two review rounds before merge. | +| 08-12 ~10:00 | — | Post-merge `/code-review max`. **Fifteen defects, all reproduced.** Registered C-89…C-93. | +| 08-12 ~11:00 | — | `/expert-code-review` on seven standing decisions. Finds that ADR-017's integration-test prohibition is **conditional** and *grants* read-only preflight — a mechanism believed closed for weeks because the register cited the wrong þing verdict. C-94…C-96. | +| 08-12 ~12:00 | **#241** | Epic opened, nine stories, three gates. Re-scoped an hour later after the maintainer's intervention (below) from nine stories of widening into mostly deletion. | +| 08-12 15:07 | **#252** | **#242** — the no-copy guard printed the value it exists to hide. **Four iterations.** | +| 08-12 16:13 | **#253** | **#245** — delete the arrival check. Three iterations. | +| 08-12 19:55 | **#254** | **#248** — delete both consumer source-reads. **Twelve commits.** | +| 08-12 20:24 | **#255** | **#243** — pair-matching and the corpus stopping rule. Five commits. | +| 08-12 20:31 | **#240** | **Release.** 61 commits to `main`. Verified after: 415 passed, ruff clean, `CONTRACT_VERSION` unchanged. | + +--- + +## What we did + +### The three defects that would have shipped + +| | | +|---|---| +| **The guard against publishing a coordinate value printed it.** | `tests/test_env_declaration.py`'s scan had two reporting branches. One was taught on 08-11 not to interpolate the value; the other was not, and went on doing it for a day. It fires only when a coordinate has actually been copied — so on the single event it exists for, into a world-readable CI log, it published the thing it forbids. | +| **The drift check subscribed us to another repository's changelog.** | Its arrival half compared every row of every table this package depends on. Measured: each partner reads **13 of 25** rows; **6 belong to no repository here**. An API key issued upstream for an unrelated consumer would redden this repository and block a release — C-86's own failure class, arriving through the guard written to reduce it. | +| **Fifteen of twenty-nine markdown assignment forms escaped the no-copy scan.** | The author proposed thirteen forms and proved all thirteen caught. An independent review proposed twenty-nine. | + +### The remedy was deletion three times out of five + +| Story | Change | Net | +|---|---|---| +| #242 | value removed from the reporter's signature; behavioural guard replaces a source-reading one | +55 then −140 | +| #245 | **delete** the arrival half; keep the row differential | deletion | +| #248 | **delete** both consumer source-reads and the sibling fetch they justified | −95 lines | +| #243 | **delete** the parser; match the pair. Stopping rule becomes a test | +~100 (five closed mutations) | + +### The cross-repository outcome + +The source-reading check broke **twice in twenty-four hours** — views-faoapi on the 11th, views-crafdapi on the 12th — each time because that repository refactored `APIPathManager("literal")` into `APIPathManager(CONSUMER_DOCUMENT_NAME)`. Verified: commits `8615574` and `0c493ae` each introduce that constant **and** add that repository's registry-binding test. **The improvement and the breakage were one edit.** + +We deleted the mechanism rather than repairing it a third time, registered the residual as **C-92**, and filed the ask where the fact lives: **views-faoapi#390** and **views-crafdapi#55**, both asking each consumer to prove its *query* uses the name it declares. + +--- + +## What we learned + +**1. A guard must be pointed at the thing it claims to guard.** This recurred three times in two days. A guard asserting that findings were *routed* through one formatter (routing is not safety — the value could be smuggled through either parameter). A silence guard calling the helper *underneath* the check it named (re-adding the deleted arrival half left the suite green). A rotation proof calling `_describe_changes` rather than the check. Each read plausibly; each watched the wrong subject. + +**2. A mutation proof written by the author of the guard tests the author's imagination.** Thirteen forms proposed, thirteen caught; twenty-nine proposed independently, fifteen missed. The proof was real and every case genuinely passed. *"Proven against N mutations"* reads as a statement about the guard when it is a statement about N. Registered as **C-93**. + +**3. Measure before building.** The plan called for excluding registry values spelled like this repository's own code. Measured against what the AST branch actually sees — non-docstring string *constants* — **the collision does not exist**. The exclusion would have narrowed a security scan for a problem that branch does not have. Built, measured, deleted. + +**4. A check that reads another repository's source is not a check this repository builds.** Not a preference — a measurement. Two breakages in a day, both caused by the other repository improving itself. + +**5. The register's own conventions are load-bearing.** An amendment heading was written as *"Mitigated"*, which is neither of the two phrasings the register declares. It passed `test_register_integrity` by **evading the string match** rather than by complying — the identical escape ADR-014 §5 records C-15 making. + +**6. A permission can hide inside a prohibition.** The register cited "þing-02 D2" for the ban on integration tests against the production Appwrite project. The ruling is **þing-01 D2**, it is conditional (*"until the operator creates one"*), and it explicitly **grants** read-only preflight validation. A whole class of mechanism — the producer-side findability check that would close C-94 — was believed closed for weeks because a citation pointed at the wrong document. + +--- + +## Process — what did not work + +This is the section that matters, and the honest summary is: **the review loop worked; the rate at which the author generated new defects did not.** + +### The cost, measured + +| | | +|---|---| +| Commits per story | #242 **7** · #245 **7** · #248 **12** · #243 **5** | +| Lines added to code | **341** | +| Lines added to docs and register | **452** | + +**One and a third lines of prose for every line of code.** And nearly every defect the reviews found lived in the prose, not the code. + +### Four failures of process, in order of seriousness + +**1. A governance rule was repealed on a false claim about another repository.** The first ADR-017 §5 erratum stated the sequencing constraint *"dissolved rather than being satisfied"* and that views-crafdapi#53 *"has not landed."* #53 closed at **12:15**; the commit was **18:18** — six hours later. Both partners' gates had been met. The deletion was still correct, on §7's grounds, but the reason written into a permanent decision record was untrue. Writing a repeal into ADR-017 on a false claim about another repository is §3's failure inside §5's own text. + +**2. Each remediation introduced a defect of the same class as the one it fixed.** A tautological assertion added in the commit that fixed *"nothing proves this check fires"*. A cry-wolf helper (`_carries`, an 8-character window) added to defeat an adversarial mutation nobody writes by accident — and it was **strictly worse than plain `in`**, condemning a bare, safe file path. Two directional pointers ("further down this entry", "the rotation proof below") that both pointed the wrong way, in the same commit. + +**3. Numbers were quoted from memory rather than measured.** A line-budget figure was posted wrong in the tracking issue, corrected — and the correction's per-file figures were also wrong. Twice in consecutive comments. A mechanical rule was adopted mid-sprint: *no number reaches a comment, commit message or ADR unless a command produced it in the same turn.* It should have been the rule from the start. + +**4. A review was reported as running when nothing had been launched.** Caught by the maintainer asking. + +### The root cause, and it is not the guards + +The initial diagnosis was *"the guard surface is bloated"* — 9,200 test lines against 2,998 package lines, with the largest test file 45% meta-tests. That diagnosis produced the wrong acceptance criterion (`wc -l tests/*.py` must end lower), and it was wrong. + +The measured diagnosis is narrower. **The prose was bloated; the guards were mostly missing.** Three of the four things #243 closed were holes, not decorations. Every line added after #242 exists because a *measured mutation survived* — not one was speculative. + +And the specific engine of the prose cost is architectural: **ADR-016 and ADR-017 contain descriptions of current implementation** — *"this is the shape the registry check now has"*, *"today the check still reads the consumer's source"*. An ADR records a decision. The moment it also describes the code, it becomes a second copy of the code, and it rots on every change. Most of #248's twelve commits were repairing implementation descriptions that should never have been in an ADR. + +### What was changed mid-sprint, and what it bought + +The maintainer's intervention — *"things that you have created have become so complicated and heavy with technical debt that you are not even able to maintain it. Then no one can."* — was the turning point, and it was correct. It produced: + +- a re-scoped epic: nine stories of widening became mostly deletion, with **#240 shipping after three issues instead of nine**; +- the **one-home rule**: the reasoning lives in the register, docstrings point at it, ADRs are touched only when the *decision* changes. + +The rule was applied for the first time in #243. **#248 took twelve commits; #243 took five.** That is one data point, not a trend, but it is the only intervention that moved the number. + +### What worked + +- **Independent review.** Five parallel reviewers on #242 found what four rounds of self-review had not. The adversarial mutation reviewer — briefed only to supply mutants the author had not thought of — was the single highest-value input of the arc. +- **Deletion as the default remedy.** Three of five stories were net deletions, and each closed its defect more completely than a repair would have. +- **Refusing to chase.** Five surviving mutations were explicitly declined in writing rather than fixed: four routed to the story that owned the scope, one declared infinite regress. Writing down what is *not* being chased, with the reason, prevented a sixth round. + +--- + +## What remains + +Four stories, none blocking anything, all opened by this arc's own reviews: + +| | | +|---|---| +| **#246** | A tautological mutation proof; `registry_current` — the function `tests/seam_registry.py` exists to provide — has no test. | +| **#247** | Three legibility fixes: a stale clone produces four errors and three explanations; `rows()` raises `AttributeError` on a shape the live registry already has; a scratch repo that can hang the suite under `commit.gpgsign`. | +| **#249** | Eight stale claims, plus the þing-01/þing-02 mis-citation. | +| **#250** | Two short ADR amendments — what a check may rest on, and what *mutation-proven* is allowed to mean — **plus the rule that an ADR records a decision and never current implementation state.** The highest-leverage item left. | + +**Operator-owned and now unblocked:** cut a tag (views-models#364 and views-crafdapi#43 are waiting); make `test` a required status check (read C-86 first — `protect_main` has zero bypass actors); views-appwrite#86, whose gates are both met. + +**Deferred with triggers, not omitted:** the producer-side read-only findability preflight (**C-94** — legal under þing-01 D2, needs a read credential in the launcher); adopting `[edition].obliges_consumers`; extracting a platform declarations module (**C-88**); consolidating the leak guards into `tests/test_redaction_guard.py`. + +--- + +## The one-sentence version + +We built a large guard surface, discovered through independent review that a substantial part of it verified facts this repository is not entitled to know — and therefore could only be proven against its author's imagination — deleted most of that part, and shipped; the lasting lesson is that **a check may rest on a fact we own, on a fact another repository has declared, or on an outcome we can observe, and never on another repository's implementation.** diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index f2ebe2d..e331c63 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -6,8 +6,8 @@ | Owner | Dylan Pinheiro / PRIO MD&D Team | | Last Updated | 2026-08-12 | | Total Concerns | 97 | -| Open Concerns | 24 | -| Resolved Concerns | 73 | +| Open Concerns | 21 | +| Resolved Concerns | 76 | --- @@ -203,7 +203,9 @@ Cross-refs: **C-57** (the exclusion and why), **C-89** (the traceback amplificat The second half is a **permission**, and this repository spent weeks believing the whole clause was a prohibition. Reclassify and read it, or record why a permission that changes what we may build is not a fact we depend on. -Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the mis-citation that compounded it), issue #249. +**Partly addressed 2026-08-12 (#249).** The classification stays `IGNORED` — nothing here reads the table, and its rows are bare strings rather than sub-tables, so feeding it to `rows()` would raise (C-91). What changed is the *reason*: the comment said "a fact about the platform, not about this package", which is what let a permission read as a prohibition. It now says the table governs which live checks this package may build, and points at C-95 and C-96. Reading it mechanically waits on C-91. + +Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the mis-citation that compounded it), **C-91** (why it is not read yet), issue #249. --- @@ -216,7 +218,7 @@ Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the m | Source | `/expert-code-review` of the standing decisions, 2026-08-12 | | Trigger | Anyone reasons from the integration-test prohibition — for a preflight, a drill, or a new ADR. | | Owner | This repository. | -| Location | `reports/technical_risk_register.md` (~:475, ~:1628, ~:1717); anywhere else citing þing-02 D2 for this. | +| Location | `reports/technical_risk_register.md` — corrected at all three sites 2026-08-12 (#249); this entry is the record, and the remaining mentions of `þing-02 D2` are its own narration. | This register cites **þing-02 D2** for the ruling that integration tests against the production Appwrite project are forbidden. þing-02 D2 is about identity and key separation. The ruling is **þing-01 D2** (`þingit/01_identity_secrets_config/orð_dómr.md:53-61`), and it differs from the paraphrase in two ways that matter: it is **conditional** (*"until the operator creates one"*), and it **grants** read-only preflight validation as the permitted live check. It also records that creating a test project is **assigned to the operator** and gates the provisioning-path drill — an open assignment, not a closed door. @@ -247,29 +249,6 @@ Cross-refs: **C-92** (the check we deleted rather than replaced), **C-87** (the --- -### C-93: A mutation proof written by whoever wrote the guard tests that author's imagination, not the guard - -| Field | Value | -|-------|-------| -| ID | C-93 | -| Tier | 3 — no defect of its own. It is the reason several of the defects below survived three reviews, and it changes what "mutation-proven" is allowed to mean in this codebase. | -| Source | `/code-review max` on PR #239 post-merge, 2026-08-11, corroborated by measurement | -| Trigger | The next time a guard is defended in a pull-request description as "mutation-proven" against a list of cases the same change authored. | -| Owner | Whoever writes the next guard; the standard belongs in ADR-014 §2. | -| Location | ADR-014 §2; `tests/test_env_declaration.py::test_no_coordinate_value_is_copied_into_this_repo`; every `_MUTANTS`-style proof in `tests/`. | - -ADR-014 §2 says a guard is mutation-proven or it is decoration. That is right, and it is not sufficient. **A mutation proof is only as good as the mutant list, and a mutant list written by the author of the guard covers the cases the author already had in mind — which are, by construction, the cases the guard already handles.** - -Measured, on the guard that protects a public repository from publishing a coordinate value. Its author (this assistant) proposed thirteen input forms, proved all thirteen caught, and wrote that result into C-57 and into a commit message. An independent review then proposed twenty-nine forms. **Fifteen missed.** The thirteen were not a sample of how people write markdown; they were a sample of what the author had thought of, and every one of them happened to share the property the guard depended on. - -This is not the same as ADR-014 §2 failing. The proof was real, it was executed, and every case in it genuinely passed. The gap is that "proven against N mutations" reads as a statement about the guard when it is a statement about N. - -**What would change.** Two candidates, and they are not exclusive: (a) for a guard whose failure is silent — a leak, an invisible delivery — the mutant list is written or extended by something other than the change that wrote the guard; (b) the mutant list is a *declared, named* artifact in the test file rather than a paragraph in a commit message, so the next contributor can see what was and was not tried. `tests/test_ci_sibling_coverage.py::_MUTANTS` is already (b) and is the pattern to copy. - -Cross-refs: **C-57** and **C-89** (the guard this was measured on), **C-90** (a proof that proved nothing at all), ADR-014 §2. - ---- - ### C-89: The guard against publishing a coordinate value publishes it — in one branch of three, proven on one side of two | Field | Value | @@ -334,60 +313,6 @@ Cross-refs: **C-57** (the scan's own entry and its history), **C-93** (why the a --- -### C-90: A mutation proof that cannot fail, and the untested function a module was extracted to create - -| Field | Value | -|-------|-------| -| ID | C-90 | -| Tier | 2 — this is the entry PR #239 was written to close, reopened by the code that closed it. It reinstates release-blocking false alarms on the path that is this project's production release, and it does so under a docstring saying the opposite. | -| Source | `/code-review max` on PR #239 post-merge, 2026-08-11; verified by direct measurement against views-appwrite `origin/main` | -| Trigger | **Both remaining halves are proof defects, not runtime ones.** (a) Someone mutates `_unclassified_tables` or `_TABLE_ROLE` and believes the tautological proof covers it. (b) Someone changes `registry_current` — the reason `tests/seam_registry.py` exists — and the suite stays green. *(The original trigger, an unrelated coordinate arriving upstream, died with `arrived` in #245.)* | -| Owner | This repository. | -| Location | `tests/test_env_declaration.py` — `test_the_table_partition_would_catch_a_new_table_and_a_vanished_one` (the tautology); `tests/seam_registry.py::registry_current` (untested). Function names, not line numbers: this entry has cited stale ones before. | - -**~~`arrived` is not filtered by the names this package reads.~~ RESOLVED 2026-08-12 (#245) — deleted; see the mitigation below. Left visible because the reasoning it prompted is the entry's most useful part.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use. - -That is the exact failure class C-86 records and that PR #239 was written to remove, and the same test's docstring seven lines above says **"Silent through: prose edits, `[meta]` bumps, and rows belonging to anyone else."** The prose describes the check that was designed; the code implements a wider one. - -There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt. That argues for table-granularity on arrival and row-granularity on change. **They cannot both stand.** - -**DECIDED 2026-08-12 (#245): the docstring won, and the cost is real.** The arrival half was deleted. A coordinate views-appwrite issues *for this package* — or a second `[contract.*]` row for a future partner such as views-productionapi — is now **silent** until a human reads the registry: no test, no run-time assert, nothing. `assert_env_declared` cannot see it, because it iterates the names this package already declares. That is the accepted price of not being reddened by every unrelated row, and it is recorded here rather than left to be discovered. *(The half's own defence — that `[contract.*]` "arrived exactly this way and nothing else here would have seen it" — was false at table granularity, where the partition check catches it, and true at row granularity, which is exactly the cost now accepted.)* - -**The partition's mutation proof cannot fail.** `assert not _unclassified_tables(base)` where `base = {name: {} for name in _TABLE_ROLE}` reduces to `set(_TABLE_ROLE) - set(_TABLE_ROLE)`, empty for every possible input. Its message — *"the real registry's tables must all classify"* — asserts a fact about a file this test never opens. It is decoration inside the test whose own docstring is about removing decoration. - -**`registry_current` has no test.** The module `tests/seam_registry.py` was extracted for one reason: two copies of the reader disagreed about whether to read the sibling's `main` or its working tree, and reading the working tree is issue #196 verbatim. The function that settles it is called by five tests and is the subject of none. Replacing its body with `rev-parse HEAD` — the defect it exists to prevent — leaves the suite at its exact baseline. Three of its error branches are executed by nothing. - -**Partial mitigation 2026-08-12 (#245) — the false-alarm half is gone; the two proof defects are not.** - -`arrived` is **deleted**. Measured before deleting: each partner reads 13 of the 25 rows in the tables this package depends on, and 8 of those rows belong to no repository here — so the check subscribed this repository to another repo's changelog. Mutation-proven after: an unrelated API key and a third partner's contract row are silent; a rotation and a removal still fire. The docstring and the code now agree, and the stopping rule sits above the check. - -**Still open, and routed to #246**: the partition's mutation proof is a tautology (`base` is built from `_TABLE_ROLE`, so `_unclassified_tables(base)` is empty for every possible input), and `registry_current` — the function `tests/seam_registry.py` was extracted to create — has no test, so replacing it with a working-tree read leaves the suite at baseline. This entry stays open until both land. - -Cross-refs: **C-86** (whose partial-mitigation paragraph this falsifies), **C-89** (the sibling defect in the no-copy scan), **C-93**, **C-91**, ADR-014 §1/§2, issue #196. - ---- - -### C-91: The git plumbing this arc added turns ordinary developer states into hard errors, bare tracebacks, and one possible hang - -| Field | Value | -|-------|-------| -| ID | C-91 | -| Tier | 3 — no wrong data and nothing silent. It taxes every contributor who does not already have the exact sibling checkout this repository assumes, and it does so with diagnoses that point at the wrong cause. | -| Source | `/code-review max` on PR #239 post-merge, 2026-08-11 | -| Trigger | Any of: a contributor clones views-appwrite shallow, single-branch, or before the pinned commit; a table classified `CONSUMED` upstream is written as flat keys rather than sub-tables; a contributor has `commit.gpgsign` or a global `core.hooksPath` set. | -| Owner | This repository. | -| Location | `tests/seam_registry.py:71` (refusal diagnoses), `:150` (`rows`), `tests/test_env_declaration.py:820` (the scratch repo). | - -**A stale clone produces four errors carrying the wrong explanation.** The reader the extraction replaced read the file off disk, so an older checkout simply read an older file. Now a clone that predates the pinned commit — or is shallow, or was made `--single-branch`, which matters because views-appwrite's default branch is not `main` — raises *"does not resolve to a commit … an empty ref reads the index and a branch reads a moving tip"*. That names neither cause and does not say `git fetch`. Meanwhile `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` detects the identical root cause and *skips* with the right remedy. One condition, one skip, four errors, three explanations. - -**`rows()` raises a bare `AttributeError` on a shape the live registry already has.** It guards a null section and not a scalar row. Verified: views-appwrite's `[test_environment]` holds `status` and `fact` as top-level strings. That table is `IGNORED`, so nothing breaks today — but when the partition check fires on a new upstream table, its own message instructs the maintainer to classify it `CONSUMED` or `MIRRORED`, and doing so for a table written that way returns a traceback pointing into a dict comprehension. From the module whose docstring says a helper justified by failing legibly must not hand back a bare traceback. - -**The scratch repo inherits the developer's global git config and has no timeout.** `test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong` sets `user.name` and `user.email` and stops. With `commit.gpgsign = true` it fails with a bare `CalledProcessError` — `capture_output=True` swallows git's explanation. With a passphrase-protected key it blocks on pinentry with no `timeout`, hanging the whole run; `conftest.git_output`, which this helper bypasses, caps at 30 seconds. The leak was anticipated for identity and not for the setting that blocks. - -Cross-refs: **C-90** (the same module's untested core), **C-88** (why the module exists outside `conftest.py`), ADR-008 (explicit failure), issue #196. - ---- - ### C-92: Nothing here checks that a consumer SELECTS by the delivery label — for either partner | Field | Value | @@ -670,7 +595,7 @@ The FAO delivery authenticates with the `UN FAO` key. That key expires **2026-11 **What this repo can and cannot do.** It cannot rotate anything; it holds no credentials and must not (þing-01 D3). What it can do is fail early and legibly rather than mid-delivery — and it does not currently. `appwrite_env.py` validates that the declared variables are *present*, which an expired key still is. An expired key is indistinguishable from a valid one until the first request comes back unauthorised, by which point a delivery is part-way through. -**Deliberately not fixed here, and the reason is C-84's own shape.** A preflight that checks key validity means an authenticated call at startup, and the only project to make it against is production — which þing-02 **D2** forbids for tests and this would not quite be. The honest position is that this is a *date to act on*, not a mechanism to build, and inventing a mechanism would be building the wrong thing to feel busy. Registered so the date is not discovered by an outage. +**Deliberately not fixed here, and the reason is C-84's own shape.** A preflight that checks key validity means an authenticated call at startup, and the only project to make it against is production — which **þing-01 D2** forbids for tests and this would not quite be — and which that verdict explicitly permits as *read-only preflight validation*, so the obstacle here is the authenticated call, not the prohibition (see C-95). The honest position is that this is a *date to act on*, not a mechanism to build, and inventing a mechanism would be building the wrong thing to feel busy. Registered so the date is not discovered by an outage. Cross-refs: **C-81** (the same operator session's other half — branch protection and the CI token), **C-27** (no rotation mechanism for a secret value upstream), **C-57** (the pinned-registry detector, which is how this arrived here at all — it demanded the v1.4.4 bump and the bump is what surfaced the expiry), þing-02 A3(i), views-appwrite C-65 and C-66. @@ -1080,6 +1005,114 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-91: The git plumbing this arc added turns ordinary developer states into hard errors, bare tracebacks, and one possible hang — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-91 | +| Tier | 3 — no wrong data and nothing silent. It taxes every contributor who does not already have the exact sibling checkout this repository assumes, and it does so with diagnoses that point at the wrong cause. | +| Source | `/code-review max` on PR #239 post-merge, 2026-08-11 | +| Trigger | Any of: a contributor clones views-appwrite shallow, single-branch, or before the pinned commit; a table classified `CONSUMED` upstream is written as flat keys rather than sub-tables; a contributor has `commit.gpgsign` or a global `core.hooksPath` set. | +| Owner | This repository. | +| Location | `tests/seam_registry.py:71` (refusal diagnoses), `:150` (`rows`), `tests/test_env_declaration.py:820` (the scratch repo). | + +**A stale clone produces four errors carrying the wrong explanation.** The reader the extraction replaced read the file off disk, so an older checkout simply read an older file. Now a clone that predates the pinned commit — or is shallow, or was made `--single-branch`, which matters because views-appwrite's default branch is not `main` — raises *"does not resolve to a commit … an empty ref reads the index and a branch reads a moving tip"*. That names neither cause and does not say `git fetch`. Meanwhile `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` detects the identical root cause and *skips* with the right remedy. One condition, one skip, four errors, three explanations. + +**`rows()` raises a bare `AttributeError` on a shape the live registry already has.** It guards a null section and not a scalar row. Verified: views-appwrite's `[test_environment]` holds `status` and `fact` as top-level strings. That table is `IGNORED`, so nothing breaks today — but when the partition check fires on a new upstream table, its own message instructs the maintainer to classify it `CONSUMED` or `MIRRORED`, and doing so for a table written that way returns a traceback pointing into a dict comprehension. From the module whose docstring says a helper justified by failing legibly must not hand back a bare traceback. + +**The scratch repo inherits the developer's global git config and has no timeout.** `test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong` sets `user.name` and `user.email` and stops. With `commit.gpgsign = true` it fails with a bare `CalledProcessError` — `capture_output=True` swallows git's explanation. With a passphrase-protected key it blocks on pinentry with no `timeout`, hanging the whole run; `conftest.git_output`, which this helper bypasses, caps at 30 seconds. The leak was anticipated for identity and not for the setting that blocks. + +**RESOLVED 2026-08-12 (#247) — all three.** + +**One condition, one diagnosis.** A ref this clone cannot see and a ref that is not a frozen commit used to share a message that named neither cause and never said `git fetch`. They are now separate branches with separate remedies, and a third — an empty pin — is called what it is: a defect in the pin, not the checkout. The bogus-sha case is deliberately classified as *"this clone cannot see it"*, because that is the truth: the reader cannot tell a bad pin from a missing fetch, and the message says so rather than guessing. + +**`rows()` refuses a scalar row by name.** `[test_environment]` on the live registry is top-level strings; classifying such a table CONSUMED — which the partition check's own remediation message invites — used to return an `AttributeError` from a dict comprehension, in the module whose justification is failing legibly. + +**The scratch repositories are hermetic.** All three now run git with `-c commit.gpgsign=false -c core.hooksPath=/dev/null` and an explicit timeout. Verified by running the suite under a `HOME` whose `.gitconfig` sets `commit.gpgsign = true` and points `core.hooksPath` at a nonexistent directory: four tests pass where they would previously have failed opaquely or blocked on pinentry with no timeout. + +Mutation-proven three ways, each reverted: removing the scalar-row refusal, the missing-object branch, and the empty-pin branch. + +Cross-refs: **C-90** (the same module's untested core), **C-88** (why the module exists outside `conftest.py`), ADR-008 (explicit failure), issue #196. + +--- + +--- + + +### C-90: A mutation proof that cannot fail, and the untested function a module was extracted to create — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-90 | +| Tier | 2 — this is the entry PR #239 was written to close, reopened by the code that closed it. It reinstates release-blocking false alarms on the path that is this project's production release, and it does so under a docstring saying the opposite. | +| Source | `/code-review max` on PR #239 post-merge, 2026-08-11; verified by direct measurement against views-appwrite `origin/main` | +| Trigger | **Both remaining halves are proof defects, not runtime ones.** (a) Someone mutates `_unclassified_tables` or `_TABLE_ROLE` and believes the tautological proof covers it. (b) Someone changes `registry_current` — the reason `tests/seam_registry.py` exists — and the suite stays green. *(The original trigger, an unrelated coordinate arriving upstream, died with `arrived` in #245.)* | +| Owner | This repository. | +| Location | `tests/test_env_declaration.py` — `test_the_table_partition_would_catch_a_new_table_and_a_vanished_one` (the tautology); `tests/seam_registry.py::registry_current` (untested). Function names, not line numbers: this entry has cited stale ones before. | + +**~~`arrived` is not filtered by the names this package reads.~~ RESOLVED 2026-08-12 (#245) — deleted; see the mitigation below. Left visible because the reasoning it prompted is the entry's most useful part.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use. + +That is the exact failure class C-86 records and that PR #239 was written to remove, and the same test's docstring seven lines above says **"Silent through: prose edits, `[meta]` bumps, and rows belonging to anyone else."** The prose describes the check that was designed; the code implements a wider one. + +There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt. That argues for table-granularity on arrival and row-granularity on change. **They cannot both stand.** + +**DECIDED 2026-08-12 (#245): the docstring won, and the cost is real.** The arrival half was deleted. A coordinate views-appwrite issues *for this package* — or a second `[contract.*]` row for a future partner such as views-productionapi — is now **silent** until a human reads the registry: no test, no run-time assert, nothing. `assert_env_declared` cannot see it, because it iterates the names this package already declares. That is the accepted price of not being reddened by every unrelated row, and it is recorded here rather than left to be discovered. *(The half's own defence — that `[contract.*]` "arrived exactly this way and nothing else here would have seen it" — was false at table granularity, where the partition check catches it, and true at row granularity, which is exactly the cost now accepted.)* + +**The partition's mutation proof cannot fail.** `assert not _unclassified_tables(base)` where `base = {name: {} for name in _TABLE_ROLE}` reduces to `set(_TABLE_ROLE) - set(_TABLE_ROLE)`, empty for every possible input. Its message — *"the real registry's tables must all classify"* — asserts a fact about a file this test never opens. It is decoration inside the test whose own docstring is about removing decoration. + +**`registry_current` has no test.** The module `tests/seam_registry.py` was extracted for one reason: two copies of the reader disagreed about whether to read the sibling's `main` or its working tree, and reading the working tree is issue #196 verbatim. The function that settles it is called by five tests and is the subject of none. Replacing its body with `rev-parse HEAD` — the defect it exists to prevent — leaves the suite at its exact baseline. Three of its error branches are executed by nothing. + +**Partial mitigation 2026-08-12 (#245) — the false-alarm half is gone; the two proof defects are not.** + +`arrived` is **deleted**. Measured before deleting: each partner reads 13 of the 25 rows in the tables this package depends on, and 8 of those rows belong to no repository here — so the check subscribed this repository to another repo's changelog. Mutation-proven after: an unrelated API key and a third partner's contract row are silent; a rotation and a removal still fire. The docstring and the code now agree, and the stopping rule sits above the check. + +**RESOLVED 2026-08-12 (#246) — both proof defects closed.** + +The tautology is gone. `assert not _unclassified_tables(base)` where `base` was built from `_TABLE_ROLE` reduced to `set(x) - set(x)`, empty for every possible input, while claiming *"the real registry's tables must all classify"* about a file the test never opens. The silent direction is now asserted against an input the function did not derive from itself, and mutation-proven by making `_unclassified_tables` report everything. + +`registry_current` has tests — three of them, plus two for `registry_at`'s refusal branches that nothing reached. The scratch repository differs on `main`, on `origin/main` and on disk, so preferring the wrong one is visible. Mutation-proven four ways: reading `HEAD` (issue #196's defect, which used to leave the suite green), preferring `main` over `origin/main`, dropping the unreadable-blob refusal, and dropping the TOML-parse wrapper. All four now fail. + +The scratch repository also runs git with `-c commit.gpgsign=false -c core.hooksPath=/dev/null`, which is C-91's third item arriving early: a contributor's global signing config would otherwise fail opaquely or block on pinentry with no timeout. + +Cross-refs: **C-86** (whose partial-mitigation paragraph this falsifies), **C-89** (the sibling defect in the no-copy scan), **C-93**, **C-91**, ADR-014 §1/§2, issue #196. + +--- + +--- + + +### C-93: A mutation proof written by whoever wrote the guard tests that author's imagination, not the guard — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-93 | +| Tier | 3 — no defect of its own. It is the reason several of the defects below survived three reviews, and it changes what "mutation-proven" is allowed to mean in this codebase. | +| Source | `/code-review max` on PR #239 post-merge, 2026-08-11, corroborated by measurement | +| Trigger | The next time a guard is defended in a pull-request description as "mutation-proven" against a list of cases the same change authored. | +| Owner | Whoever writes the next guard; the standard belongs in ADR-014 §2. | +| Location | ADR-014 §2; `tests/test_env_declaration.py::test_no_coordinate_value_is_copied_into_this_repo`; every `_MUTANTS`-style proof in `tests/`. | + +ADR-014 §2 says a guard is mutation-proven or it is decoration. That is right, and it is not sufficient. **A mutation proof is only as good as the mutant list, and a mutant list written by the author of the guard covers the cases the author already had in mind — which are, by construction, the cases the guard already handles.** + +Measured, on the guard that protects a public repository from publishing a coordinate value. Its author (this assistant) proposed thirteen input forms, proved all thirteen caught, and wrote that result into C-57 and into a commit message. An independent review then proposed twenty-nine forms. **Fifteen missed.** The thirteen were not a sample of how people write markdown; they were a sample of what the author had thought of, and every one of them happened to share the property the guard depended on. + +This is not the same as ADR-014 §2 failing. The proof was real, it was executed, and every case in it genuinely passed. The gap is that "proven against N mutations" reads as a statement about the guard when it is a statement about N. + +**RESOLVED 2026-08-12 (#250) — ADR-014 §2 amended.** + +The rule landed is a **diagnostic**, not a process: *if a guard can only be proven against inputs you invented, that is the signal the guard is on the wrong side of a boundary — it is verifying a fact you do not own.* Prefer moving the check to where the fact lives, or anchoring the mutant list in something real — this repository's corpus, the registry's rows, an observable outcome. The no-copy scan's stopping rule is the worked example and shipped in #243. + +Two cheap obligations where that is impossible: the mutant list is a declared artifact in the test file, not a paragraph in a pull request; and a proof must be able to fail. + +**Deliberately not adopted:** requiring an independent mutant author for every guard. The friction would exceed the disease for a single maintainer. Independent mutants are worth buying only for the silent-failure class — a leak, an invisible delivery — and the arc that produced this entry is the evidence for both halves: five parallel reviewers found what four rounds of self-review had not, and that cost was proportionate exactly once. + +Cross-refs: **C-57** and **C-89** (the guard this was measured on), **C-90** (a proof that proved nothing at all), ADR-014 §2. + +--- + +--- + + ### C-82: Governance-artifact prose carries numbers and statuses that nothing checks — RESOLVED 2026-08-05 | Field | Value | @@ -1926,7 +1959,7 @@ Cross-refs: C-15 (the provenance this field serves), C-22 (the recall process th Replaced by tests of `contract/historical.assert_metadata_complete` — the code that actually gates a delivery — parametrised over the **imported** `METADATA_COLS`, plus source-scan pins that the gate stays at build time and is still invoked. Verified 2026-08-02: `pytest -q tests/test_validation.py` → **14 passed**. Mutation-tested: narrowing the gate to a single column fails **9 of 14**; the old suite passed that mutation untouched, because it was not testing the gate. -**Residual 2 — the enrich→validate end-to-end test — RELOCATED to #18**, per the Register Conventions' relocation rule (a relocation is not complete until the destination exists and is cited by number). Every *leg* is now covered — enrichment (**no longer a leg**: `GaulLookupEnricher` and `test_enrichment.py` were deleted in #90/B3b, the lookup join having moved into the frame build; recorded here because the closure above was argued from a list this deletion shortened), artifact build (`test_historical_builder.py`, 7), reader parity (`test_historical_parity.py`, 3), the invariants on primitives (`test_input_integrity_e2e.py`, 8), the wire end to end (`test_hop_b_sink_e2e.py`, 6), the null-gate (`test_validation.py`, 14). What remains uncovered is **the manager orchestrating them**, which needs views-pipeline-core and a production-like Appwrite environment — and þing-02 **D2** forbids integration tests against the production project, no non-production one existing. +**Residual 2 — the enrich→validate end-to-end test — RELOCATED to #18**, per the Register Conventions' relocation rule (a relocation is not complete until the destination exists and is cited by number). Every *leg* is now covered — enrichment (**no longer a leg**: `GaulLookupEnricher` and `test_enrichment.py` were deleted in #90/B3b, the lookup join having moved into the frame build; recorded here because the closure above was argued from a list this deletion shortened), artifact build (`test_historical_builder.py`, 7), reader parity (`test_historical_parity.py`, 3), the invariants on primitives (`test_input_integrity_e2e.py`, 8), the wire end to end (`test_hop_b_sink_e2e.py`, 6), the null-gate (`test_validation.py`, 14). What remains uncovered is **the manager orchestrating them**, which needs views-pipeline-core and a production-like Appwrite environment — and **þing-01 D2** forbids integration tests against the production project while no non-production one exists (see C-95 — the ruling is conditional, and creating that project is an open operator assignment). That gap has **two standing trackers already**, which is why keeping a third here is noise rather than signal: issue **#18** (open since 2026-06-04) and `tests/test_falsification_campaign_3_5.py`, an `xfail(strict)` probe that **flips to XPASS the moment someone writes the test** — a self-surfacing tracker, which is more than this entry was doing. | | Tier | 3 | diff --git a/tests/seam_registry.py b/tests/seam_registry.py index d6a4f90..beb870b 100644 --- a/tests/seam_registry.py +++ b/tests/seam_registry.py @@ -65,10 +65,34 @@ def registry_at(repo: Path, ref: str) -> dict: capture_output=True, text=True, check=False, timeout=30, ) if resolved.returncode != 0 or not resolved.stdout.strip(): + # Two conditions used to share this message, and they have different remedies. + # A ref this clone has never heard of is an environment problem — a shallow or + # `--single-branch` clone, or one made before the pin. A ref that resolves to + # something that is not a commit is a pin defect. Only the second is this + # repository's fault; only the first is fixed by fetching (register C-91). + if not ref.strip(): + raise RegistryReadError( + "the pin is empty, and an empty ref does not resolve to a commit — " + "`git show ':'` reads the INDEX, so a blanked pin would compare " + "the registry to itself and report green against every future edition. " + "This is a defect in the pin, not in the checkout." + ) + known = subprocess.run( + ["git", "-C", str(repo), "cat-file", "-e", f"{ref}^{{object}}"], + capture_output=True, text=True, check=False, timeout=30, + ).returncode == 0 + if not known: + raise RegistryReadError( + f"{repo} has no object {ref!r}. This is almost always a clone that is " + "shallow, `--single-branch`, or simply older than the pin — run " + f"`git -C {repo} fetch --tags origin` and try again. It is not a defect " + "in the pin: nothing here can tell whether that commit is good until " + "this checkout can see it." + ) raise RegistryReadError( - f"{ref!r} does not resolve to a commit in {repo}. A pin must name a frozen " - "commit: an empty ref reads the index and a branch reads a moving tip, and " - "either would make the comparison compare the registry to itself." + f"{ref!r} exists in {repo} but does not resolve to a commit. A pin must name " + "a frozen commit: an empty ref reads the index and a branch reads a moving " + "tip, and either would make the comparison compare the registry to itself." ) if not resolved.stdout.strip().startswith(ref): raise RegistryReadError( @@ -146,8 +170,20 @@ def rows(registry: dict, sections: tuple) -> dict[str, tuple]: The one shared projection. Two hand-copied versions had already diverged on null handling — one raised ``AttributeError`` on a null section, the other did not. """ - return { - name: (section, body.get("class"), body.get("value", ABSENT)) - for section in sections - for name, body in (registry.get(section) or {}).items() - } + out = {} + for section in sections: + for name, body in (registry.get(section) or {}).items(): + if not isinstance(body, dict): + # `[test_environment]` on the live registry is exactly this — top-level + # strings, not sub-tables. Reaching it means someone classified such a + # table CONSUMED, which the partition check's own remediation message + # invites. Refusing by name beats an AttributeError from a comprehension + # in the module whose justification is failing legibly (register C-91). + raise RegistryReadError( + f"[{section}].{name} is a bare {type(body).__name__}, not a table. " + "This section's rows are scalars, so it carries no class or value to " + "read — it cannot be CONSUMED. Classify it IGNORED with a reason, or " + "read it with something other than `rows()`." + ) + out[name] = (section, body.get("class"), body.get("value", ABSENT)) + return out diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index ee0238a..66f5d0a 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -251,7 +251,7 @@ def _g7_siblings_are_taken_from_main( Without it `actions/checkout` takes the sibling's own default branch — which for views-appwrite is `development`. ADR-014 §3 makes `main` the authority for a claim about another repository, and `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` - already enforces that. Reading the working tree from `development` while demanding + already enforces that. Reading a sibling's `development` while demanding reachability from `main` is two guards in one file asking for different things. """ return [ diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 5a59f7c..2f36f7d 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -35,10 +35,15 @@ from tests.seam_registry import ( ABSENT as _ABSENT, + REGISTRY_RELPATH, REGISTRY_RELPATH as _REGISTRY_RELPATH, + RegistryReadError, RegistryReadError as _RegistryReadError, + registry_at, registry_at as _registry_at, + registry_current, registry_current as _registry_current, + rows, rows as _rows, ) from tests.conftest import ( @@ -514,7 +519,12 @@ def test_every_declared_name_is_classified_here(partner): "contract": "MIRRORED", # values live in our source by design (ADR-017 §5); # checked by tests/test_product.py, not by _declared_classes "excluded": "IGNORED", # names the registry records as deliberately NOT coordinates - "test_environment": "IGNORED", # a fact about the platform, not about this package + # IGNORED because nothing here READS it, not because it is none of our business — + # it is the clause that says which live checks this package may build, and a reader + # who believed the older comment spent weeks thinking a permission was a prohibition + # (register C-95, C-96). Its rows are bare strings, not tables, so it must stay out of + # `rows()` until that is handled (C-91). + "test_environment": "IGNORED", #: Arrived at registry v1.6.0, and it is views-appwrite#76 delivered — each edition #: marked ``obliges_consumers = true|false``, so a consumer can tell a console #: observation from a change it must act on. IGNORED only because nothing here reads @@ -525,12 +535,11 @@ def test_every_declared_name_is_classified_here(partner): "meta": "METADATA", # the edition and its amendment log } -#: direction is deliberately unchecked. (An earlier version of this comment offered -#: v1.5.1's removal of `[unmodelled]` as the worked example of a silent case. That was -#: WRONG: `unmodelled` was never in this partition, so against v1.4.4 it would have -#: been a RED build demanding classification. The rule is right; the illustration -#: was not, and it had been repeated in three places.) -#: silent while v1.5.0 adding `[contract]` is a red build with something to do. +#: An IGNORED table vanishing upstream is deliberately unchecked; a new, unclassified one +#: is a red build with something to do. (An earlier version of this comment illustrated +#: the silent case with v1.5.1's removal of `[unmodelled]`, which was wrong — `unmodelled` +#: was never in this partition, so it would have been a red build demanding +#: classification. The rule was right; the illustration was not, in three places.) #: The only roles that mean anything. A typo in `_TABLE_ROLE` used to be silent, and it #: silently narrowed a security scan: mistyping "CONSUMED" dropped `target` from the #: no-copy check's sections, taking it from twelve values to two, with no test objecting. @@ -689,9 +698,8 @@ def test_every_table_in_the_registry_is_classified_here(): was silently ignoring four. **Directional on purpose.** Every table upstream must be classified; only the tables - we depend on must exist. An IGNORED table disappearing is not our business, which is - an IGNORED table disappearing is silent while a new, unclassified one is a red build - with something to do. Two such events in the registry's life so far, and this + we depend on must exist. An IGNORED table disappearing is not our business, so it is + silent, while a new, unclassified one is a red build with something to do. Two such events in the registry's life so far, and this repository needed to see both. *(An earlier draft illustrated the silent case with v1.5.1's removal of @@ -808,6 +816,34 @@ def test_the_docstring_states_the_same_edition_the_constants_declare(partner): ) +@pytest.mark.parametrize("partner", _PARTNERS) +def test_the_docstring_url_points_at_the_commit_the_constant_declares(partner): + """The docstring publishes a blob URL. Its sha is a third copy of the pin, unguarded. + + The neighbouring test compares the docstring's *version*; nothing compared its *sha*. + That is how an annotated tag reached the pin: git peeled it, every check passed, and + two public modules published a URL returning 404 three lines above the sentence "a + pinned URL does not rot". Third time this class has bitten — register C-57. + """ + module = _PARTNER_ENV[partner][0] + urls = re.findall( + r"views-appwrite/blob/([0-9a-f]{7,40})/docs/ADRs/platform/coordinate_registry\.toml", + module.__doc__ or "", + ) + assert urls, ( + f"{partner}/appwrite_env.py's docstring no longer publishes a registry blob URL in " + "the expected form. If the URL moved, teach this test its new shape — do not delete " + "the check, or the sha goes unguarded again." + ) + wrong = sorted({u for u in urls if not module.SEAM_CONTRACT_COMMIT.startswith(u[:7])}) + assert not wrong, ( + f"[{partner}] the docstring's blob URL names commit(s) {wrong} while " + f"SEAM_CONTRACT_COMMIT declares {module.SEAM_CONTRACT_COMMIT}. A reader following " + "that link reads a different edition from the one this module was verified against, " + "and if the sha is not a commit at all the link 404s." + ) + + @pytest.mark.parametrize("partner", _PARTNERS) def test_the_pinned_commit_is_reachable_from_the_contract_repos_main(partner): """Existence is not reachability, and that distinction cost a merged PR (#196). @@ -847,7 +883,15 @@ def test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong(tmp_path): import subprocess as sp def git(*args): - return sp.run(["git", "-C", str(tmp_path), *args], capture_output=True, text=True, check=True) + # `-c`, and a timeout. A contributor's global `commit.gpgsign` makes `git commit` + # fail with a bare CalledProcessError here — capture_output swallows git's + # explanation — and with a passphrase-protected key it blocks on pinentry with + # stdin inherited and no timeout, hanging the whole run (register C-91). + return sp.run( + ["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false", + "-c", "core.hooksPath=/dev/null", *args], + capture_output=True, text=True, check=True, timeout=30, + ) git("init", "-q") git("config", "user.email", "t@t") @@ -870,7 +914,11 @@ def commit(text: str, message: str) -> str: "the reader must accept a well-formed registry, or the refusals below prove nothing" ) - with pytest.raises(_RegistryReadError, match="does not resolve to a commit"): + # The five refusals, and note that two of them now say DIFFERENT things. A ref this + # clone cannot see and a ref that is not a commit used to share one message; they have + # different remedies — `git fetch` versus fix the pin — and conflating them sent + # contributors to the wrong one (register C-91). + with pytest.raises(_RegistryReadError, match="the pin is empty"): _registry_at(tmp_path, "") # a blanked pin reads the INDEX with pytest.raises(_RegistryReadError, match="annotated TAG|does not start with it"): _registry_at(tmp_path, "v1") # a tag object: git peels it, the URL 404s @@ -878,8 +926,9 @@ def commit(text: str, message: str) -> str: _registry_at(tmp_path, empty) # exits 0, stdout empty with pytest.raises(_RegistryReadError, match="no meta.version or no"): _registry_at(tmp_path, anchorless) # parses, but is not the registry - with pytest.raises(_RegistryReadError, match="does not resolve to a commit"): - _registry_at(tmp_path, "0" * 40) # a pin that names nothing + with pytest.raises(_RegistryReadError, match="has no object"): + _registry_at(tmp_path, "0" * 40) # this clone cannot see it; it cannot tell + # a bad pin from a missing fetch, and says so def test_the_role_vocabulary_is_closed(): @@ -914,7 +963,15 @@ def test_the_table_partition_would_catch_a_new_table_and_a_vanished_one(): assert _unclassified_tables(base | {"brand_new_table": {}}) == ["brand_new_table"], ( "a table nobody classified went unnoticed — that is how `[contract.*]` arrived" ) - assert not _unclassified_tables(base), "the real registry's tables must all classify" + # NOT `assert not _unclassified_tables(base)` — `base` is built from `_TABLE_ROLE`, + # so that reduces to `set(x) - set(x)` and is empty for every possible input. It + # shipped, claiming "the real registry's tables must all classify" about a file this + # test never opens. The silent direction is worth asserting; it just has to be + # asserted about something the function did not derive from itself. + assert not _unclassified_tables({"connection": {}, "target": {}}), ( + "a subset of the classified tables was reported as unclassified — the check is " + "inverted, and every real registry would fail it" + ) assert _missing_dependencies({k: v for k, v in base.items() if k != "target"}) == ["target"], ( "a table this package reads rows out of vanished and the check did not object" @@ -1334,3 +1391,113 @@ def test_the_scan_understands_every_assignment_form_this_repo_writes(): "document that introduced it — that is the stopping rule, and it is why the " "form list is derived from this repository's own corpus rather than invented." ) + + +def _scratch_repo(tmp_path: Path): + """A throwaway git repo whose registry differs on `main`, on `origin/main`, and on disk. + + `-c` rather than `git config`: a contributor's global `commit.gpgsign` or + `core.hooksPath` would otherwise reach in and either fail opaquely or block on + pinentry with no timeout. + """ + def git(*args): + return subprocess.run( + ["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false", + "-c", "core.hooksPath=/dev/null", *args], + capture_output=True, text=True, check=True, timeout=30, + ) + + target = tmp_path / REGISTRY_RELPATH + target.parent.mkdir(parents=True) + + def edition(marker: str) -> str: + return f'[meta]\nversion = "{marker}"\n\n[connection.X]\nclass = "connection"\n' + + git("init", "-q", "-b", "main") + git("config", "user.email", "t@t") + git("config", "user.name", "t") + target.write_text(edition("on-main")) + git("add", "-A") + git("commit", "-q", "-m", "main") + + # a remote-tracking ref that is AHEAD of main, so preferring one over the other shows + git("checkout", "-q", "-b", "upstream") + target.write_text(edition("on-origin-main")) + git("add", "-A") + git("commit", "-q", "-m", "origin") + git("update-ref", "refs/remotes/origin/main", "HEAD") + git("checkout", "-q", "main") + + # and a dirty working tree, which is what #196 was about + target.write_text(edition("in-the-working-tree")) + return tmp_path + + +def test_registry_current_reads_origin_main_not_the_working_tree(tmp_path): + """The reason `tests/seam_registry.py` exists, and until now the only untested part. + + A sibling clone sits on whatever branch its own agent last worked on. Comparing + against that grades this repository on unreviewed content — issue #196, which cost a + withdrawn pull request. Replacing this function with a working-tree or `HEAD` read + used to leave the whole suite green. + """ + repo = _scratch_repo(tmp_path) + assert registry_current(repo)["meta"]["version"] == "on-origin-main", ( + "registry_current read something other than origin/main. A working-tree read is " + "#196 verbatim; a bare `main` read misses that the sibling's remote has moved." + ) + + +def test_registry_current_refuses_a_repo_with_neither_ref(tmp_path): + """No `origin/main` and no `main` must say so, not return an empty registry.""" + subprocess.run(["git", "init", "-q", str(tmp_path)], + capture_output=True, text=True, check=True, timeout=30) + with pytest.raises(RegistryReadError, match="neither origin/main nor main"): + registry_current(tmp_path) + + +def test_registry_at_refuses_a_commit_whose_registry_is_missing_or_unparseable(tmp_path): + """`git show` failing, and a blob that is not TOML — two refusal branches nothing reached.""" + def git(*args): + return subprocess.run( + ["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false", + "-c", "core.hooksPath=/dev/null", *args], + capture_output=True, text=True, check=True, timeout=30, + ) + git("init", "-q", "-b", "main") + git("config", "user.email", "t@t") + git("config", "user.name", "t") + + (tmp_path / "unrelated.txt").write_text("no registry here\n") + git("add", "-A") + git("commit", "-q", "-m", "no registry") + absent = git("rev-parse", "--short", "HEAD").stdout.strip() + + target = tmp_path / REGISTRY_RELPATH + target.parent.mkdir(parents=True) + target.write_text("this is not toml = = =\n") + git("add", "-A") + git("commit", "-q", "-m", "not toml") + garbage = git("rev-parse", "--short", "HEAD").stdout.strip() + + with pytest.raises(RegistryReadError, match="cannot read the registry"): + registry_at(tmp_path, absent) + with pytest.raises(RegistryReadError, match="did not parse as TOML"): + registry_at(tmp_path, garbage) + + +def test_rows_refuses_a_section_whose_entries_are_not_tables(): + """`[test_environment]` on the live registry is scalars, not sub-tables. + + Nothing breaks today because that table is IGNORED — but the partition check's own + remediation message tells a maintainer to classify a new table CONSUMED, and doing + that for one written this way used to return an `AttributeError` from a dict + comprehension. Register C-91. + """ + scalars = {"test_environment": {"status": "none", "fact": "a sentence"}} + with pytest.raises(RegistryReadError, match=r"\[test_environment\]\.(status|fact) is a bare str"): + rows(scalars, ("test_environment",)) + + # and the ordinary shape still works, or the refusal above proves nothing + tables = {"target": {"APPWRITE_X": {"class": "target", "value": "v"}}} + assert rows(tables, ("target",)) == {"APPWRITE_X": ("target", "target", "v")} diff --git a/tests/test_release_version.py b/tests/test_release_version.py new file mode 100644 index 0000000..731bc69 --- /dev/null +++ b/tests/test_release_version.py @@ -0,0 +1,81 @@ +"""The package version and the git tag that installs it must agree. + +Consumers install this package by tag — `views-models`' launchers do exactly that — and +then read `importlib.metadata.version(...)` to decide what the installed build can do +(views-models#294). So the number in `pyproject.toml` is what a consumer *believes* it +installed, and the tag is what it *actually* installed. Nothing compared them. + +Measured 2026-08-13: tag `1.1.0` was cut at `main` while `pyproject.toml` still said +`1.0.0`, so an install from that tag reported the previous release. Caught before any +consumer pinned. Third time in this arc a version has been declared twice with a guard on +one copy (register C-80, C-82). +""" + +import re +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parent.parent + + +def _declared_version() -> str: + text = (_REPO / "pyproject.toml").read_text() + match = re.search(r'^version = "([^"]+)"', text, re.M) + assert match, ( + "pyproject.toml no longer declares a version in the form this guard reads. If the " + "packaging changed, teach this test the new form — do not delete it, or the number " + "a consumer installs goes unchecked again." + ) + return match.group(1) + + +def _tags_at(commit: str) -> list[str]: + out = subprocess.run( + ["git", "-C", str(_REPO), "tag", "--points-at", commit], + capture_output=True, text=True, check=False, timeout=30, + ) + return sorted(t for t in out.stdout.split() if re.fullmatch(r"\d+\.\d+\.\d+", t)) + + +def test_a_release_tag_declares_the_version_the_package_declares(): + """If HEAD carries a release tag, `pyproject.toml` must say the same number. + + Silent off a tagged commit — most commits are not releases, and firing on them would + be the false alarm that gets a guard deleted (ADR-014 §3). + """ + tags = _tags_at("HEAD") + if not tags: + pytest.skip("HEAD carries no release tag; nothing to compare") + + declared = _declared_version() + assert tags == [declared], ( + f"HEAD is tagged {tags} but pyproject.toml declares {declared!r}. A consumer " + "installing by tag would read the wrong version from importlib.metadata, which is " + "what views-models#294's capability assert reads. Bump the file and re-point the " + "tag, or tag a commit that already carries the right number." + ) + + +def test_the_newest_release_tag_is_not_ahead_of_the_declared_version(): + """A tag newer than the file means a release was cut without bumping. + + This is the direction that actually bit: the tag moved, the file did not. Runs on + every commit, not only tagged ones, so the gap is visible the moment it opens. + """ + out = subprocess.run( + ["git", "-C", str(_REPO), "tag", "-l", "--sort=-v:refname"], + capture_output=True, text=True, check=False, timeout=30, + ) + releases = [t for t in out.stdout.split() if re.fullmatch(r"\d+\.\d+\.\d+", t)] + if not releases: + pytest.skip("no release tags in this checkout (a shallow clone, or none cut yet)") + + newest, declared = releases[0], _declared_version() + as_tuple = lambda v: tuple(int(p) for p in v.split(".")) # noqa: E731 + assert as_tuple(newest) <= as_tuple(declared), ( + f"the newest release tag is {newest} but pyproject.toml declares {declared}. A " + "release was cut without bumping the file, so an install from that tag reports a " + "version older than itself." + )