From 2d3b2c528bdd3ad6d1231cc3bcd29d90f52072ef Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 13:25:35 +0200 Subject: [PATCH 1/9] docs(adr): the picture, the tables, and where declarations belong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, the last found by falsifying the ADRs against the architecture rules rather than against themselves. THE PICTURE. Neither ADR contained one, and neither had a table saying which repository does what — you had to assemble that from prose that said "this repository" throughout. The maintainer read both and was lost three times; that was data about the documents. ADR-017 §5 gains a three-party diagram and a role table. The diagram's point is the arrow that is not there: both sides point at the registry, neither points at the other. The table's "needs access to" column is what shows why no credential is required. ADR-016 §4 gains the role table. Its diagram was drafted, rendered badly, and was DELETED rather than fixed — it duplicated the table beside it, and the table is better. For that document the table was the picture. RENDERING, kept out of the source. Fenced blocks split across pages in PDF. Fixed with a pandoc Lua filter that makes each block an unbreakable minipage, living beside the PDFs rather than as raw LaTeX in the markdown — the markdown is the artifact, the PDF a view. The ADR-017 diagram was also rebuilt with its columns computed rather than eyeballed, so the arrows sit over the boxes they rise from. WHERE DECLARATIONS BELONG — new C-88, and the one substantive finding. tests/conftest.py is pytest's fixture file and now holds four unrelated groups: the package taxonomy, the sibling repositories, the consumer mapping, and two git helpers. None is a fixture. ADR-016 §4 asserted "the list of siblings is code, not a comment" and named that file without ever asking whether it was the right home — which is exactly the question the repository's own architecture rules are about. The cost is already paid, not hypothetical: scripts/build_gaul_lookup.py needs the same sibling-location fact and cannot import it, because a script importing from tests/ is the dependency direction backwards. So it declares "VIEWS_DATAFACTORY" a second time. That duplication is defended on WET grounds and the defence is sound — but it was not a free choice, and a structural constraint and a design decision reached the same answer with only one of them examined. §4 now says where the declaration lives, what that costs, and the trigger for moving it. Not fixed here: moving it touches a dozen imports for no immediate gain, and the second incident is what tells you the right shape. Probes that found nothing: hedging language (three hits, all legitimate prose); OCP — adding a sibling is data plus a workflow block, no rule changes; ISP — the record is down to three fields and its two audiences are coherent. Register 87/14/73 -> 88/15/73. Pre-existing on development and NOT from this branch: three failures — two stale registry pins and the consumer-name check, which broke when views-faoapi tidied the file it reads. That is #238's work and is awaiting the maintainer's discussion. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 31 +++++++++++++++++ .../017_facts_across_a_private_boundary.md | 27 +++++++++++++++ reports/technical_risk_register.md | 33 +++++++++++++++++-- 3 files changed, 88 insertions(+), 3 deletions(-) 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 ef27514..44debfd 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -86,6 +86,37 @@ nothing able to check it, will eventually be wrong and nobody will find out. That is all. It replaces a comment with something a test can read. +**Where that code lives, and why the question is worth asking.** The declaration sits in +`tests/conftest.py`, because that is where this repository already keeps facts several +test modules share. It is not obviously the right home: `conftest.py` is pytest's fixture +file, and none of these declarations is a fixture — they are statements about the platform +that tests happen to read. + +The cost is already visible. `scripts/build_gaul_lookup.py` needs the same +sibling-location fact and cannot import it, because a script importing from `tests/` is +the dependency direction backwards. So it declares the variable name a second time. That +duplication is defensible on its own merits — the two contracts genuinely differ — but it +was not a free choice, and a reader should know that a structural constraint and a design +decision happened to agree. + +**Named trigger for moving it (ADR-014 §4):** a second non-test consumer needing one of +these declarations, or a fifth declaration arriving in that file. The shape it would move +to is a small module owning the platform declarations, importable by tests and scripts +alike. Not committed to here, because the second incident is what tells you whether that +shape is right. Tracked as register **C-88**. + +Who is who, once and in one table: + +| repository | public? | fetched by CI? | why not, if not | +|---|---|---|---| +| **views-appwrite** | yes | **yes** | — carries the registry drift checks | +| **views-crafdapi** | yes | **yes**, temporarily | — retires when ADR-017 §7 lands | +| **views-datafactory** | yes | no | its checks need raw data that is not in its git repository; fetching it turns an honest skip into a crash | +| **views-faoapi** | **no** | no | private — no amount of workflow configuration reaches it. That case is [ADR-017](017_facts_across_a_private_boundary.md) | + +The table is not decoration: it is `tests/conftest.py::SIBLINGS` in prose, and a test fails +if the two disagree. + ### §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 fd85af5..c419527 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -96,6 +96,33 @@ must decide for the category, not for today's instance. > surface. Each side verifies itself against that declaration. Neither side reads the > other's source code.** +The same thing as a picture: + +``` + views-appwrite + the registry (public) + "un_fao" + + ▲ ▲ + │ │ + views-postprocessing views-faoapi + writes the name reads the name + + these two never read each other +``` + +**The arrow that is missing is the point.** Neither repository reads the other, so it does +not matter that one of them is private, and no credential is needed by anyone. Both arrows +point at a third place that is public to everybody. + +Who does what, once and in one table: + +| repository | its role | what it checks | needs access to | +|---|---|---|---| +| **views-appwrite** | holds the fact | nothing — it is the authority | — | +| **views-postprocessing** (here) | writes the name onto every upload | its own copy against the registry | the registry only | +| **views-faoapi** (private) | opens files with that name | its own served name against the registry | the registry only | + Concretely, for the delivery label: 1. The value is declared once, in the platform's public coordinate registry diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 035ab40..f0594c4 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -4,9 +4,9 @@ |-------------------|--------------------------------------| | Project | views-postprocessing | | Owner | Dylan Pinheiro / PRIO MD&D Team | -| Last Updated | 2026-08-10 | -| Total Concerns | 87 | -| Open Concerns | 14 | +| Last Updated | 2026-08-11 | +| Total Concerns | 88 | +| Open Concerns | 15 | | Resolved Concerns | 73 | --- @@ -161,6 +161,33 @@ that indexes only deleted code is noise. ## Open Concerns +### C-88: The platform declarations live in pytest's fixture file, so nothing outside the test tree can reach them + +| Field | Value | +|-------|-------| +| ID | C-88 | +| Tier | 3 — no correctness risk and nothing silent. It is a boundary that has already forced one duplication for structural rather than design reasons, and it will force the next one the same way. | +| Source | `falsify` against the SOLID / component-principle lens, 2026-08-11 | +| Trigger | **Either.** (a) A second non-test consumer needs one of these declarations and has to copy it. (b) A fifth declaration is added to `tests/conftest.py` — the file is at four, and the threshold for "dumping ground" is not a number but the moment nobody can say in one sentence what the file is for. | +| Owner | Whoever adds the next declaration, or the next non-test consumer. Not urgent; it gets more expensive slowly. | +| Location | `tests/conftest.py` (271 lines); `scripts/build_gaul_lookup.py:73`; ADR-016 §4. | + +`tests/conftest.py` is pytest's fixture file. It currently holds **four unrelated groups**: the package taxonomy (`PARTNER_PACKAGES`, `MACHINERY_PACKAGES`), the sibling repositories (`Sibling`, `SIBLINGS`, and three resolver functions), the consumer mapping (`CONSUMER_REPO`), and two git helpers (`git_output`, `commit_is_on_main`). None of those is a test fixture. They are declarations about the platform that happen to be consumed by tests. + +**The concrete cost, which has already been paid once.** `scripts/build_gaul_lookup.py` needs the same sibling-location fact and cannot have it: *"a script must not import from `tests/` — that is the dependency direction backwards."* So it hardcodes `"VIEWS_DATAFACTORY"` at `:73` while `SIBLINGS` declares the same string in `conftest.py`. + +That duplication is **defended on WET grounds and the defence is sound** — the two contracts genuinely differ (the script returns a `Path` even when the checkout is absent so it can raise its own message; the test helper returns `None` because a missing sibling is a normal skip), and a guard asserts the two resolve to the same place. This entry does not ask for that to be merged. + +**What it records is that the choice was not free.** The script could not have reused the declaration even if reuse had been right, because of where the declaration lives. A structural constraint and a design decision reached the same answer, and only one of them was examined. + +**Why this is Tier 3 and not higher.** Nothing is wrong today. Every guard works, the duplication is guarded, and moving the declarations would touch a dozen imports for no immediate gain. The risk is the slope: `conftest.py` is where a declaration goes when nobody asks where it belongs, and each addition makes the next one more natural. + +**What "fixed" would look like**, when the trigger fires: a small module that owns the platform declarations — importable by tests, scripts and, if ever needed, package code — with `conftest.py` reduced to what pytest actually needs from it. That is the shape, not a commitment; the point of the trigger is that the second incident tells you whether it is right. + +Cross-refs: **C-46** (which records the builder's separate resolver and the guard that they agree), ADR-016 §4, ADR-002 (dependency direction). + +--- + ### C-86: The release path now depends on two other repositories, and there is no way past a red build | Field | Value | From bcdf5225192d7d4bb0cddd7dd98c6a95040db57a Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 13:32:49 +0200 Subject: [PATCH 2/9] test(readability): four things a maintainer new to this decision could not have known MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Falsified the TESTS against "readable by someone unfamiliar with the decision" — a different lens from the previous passes, which checked the documents against themselves. Four findings, all real, two of them contradictions with things written days earlier. THE NUMBERING GAP WAS FIXED IN THE ADR AND LEFT IN THE CODE. test_ci_sibling_coverage runs G1, G2, G4, G5, G6, G7. ADR-016 now explains why two numbers are missing; the test file did not, and the test file is what someone opens when a build goes red. It says so now: G3 and G8 were removed as circular, survivors keep their names, the gap is deliberate. INVERTED FAILURE SEMANTICS, UNSIGNPOSTED — the sharpest of the four. Half this file fails when the workflow is wrong; the other half fails when the RULE is wrong, because those tests feed a deliberately broken world to a rule and demand it complain. Nothing said so. A maintainer seeing "[G6] did not object to: outside _siblings/" would go and stare at run_pytest.yml, which is fine. The module docstring now names the inversion, the assertion says "Fix the rule, not run_pytest.yml", and test_each_rule_bites_on_a_broken_world has its docstring back — it lost one in the refactor that moved _MUTANTS out. Mutation-proven: breaking G6 produces that message. "TWO FIELDS" OVER A THREE-FIELD RECORD. Sibling's docstring counted ci_checkout and note and forgot env, the field two other modules actually use. CONFTEST'S FIRST LINE SAID IT HOLDS ONE THING; C-88 SAYS FOUR. Registered that yesterday and left the docstring claiming "Shared test fixtures — currently just one". It now opens by listing the four groups, defines "sibling" where a newcomer first meets the word, and points at C-88 rather than defending the arrangement. Its second line also opened with four private identifiers (S6 / #187 / C-46 / C-57) — the same thing the maintainer objected to in the ADRs, in the file a newcomer opens first. Not registered, consistently with the previous three passes: each is a defect fixed in the same change, and the only candidate trigger — "whenever a test is edited" — is perpetual, which the register's own quality gate rejects. Say so if you want them tracked anyway. Pre-existing and NOT from this change: three failures on development — two stale registry pins and the consumer-name check, which broke when views-faoapi tidied the file it reads. That is #238, awaiting discussion. Co-Authored-By: Claude Opus 5 (1M context) --- tests/conftest.py | 26 +++++++++++++++++----- tests/test_ci_sibling_coverage.py | 36 +++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2fe5ccd..18c1d6f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,23 @@ -"""Shared test fixtures — currently just one: how to find a sibling repository. +"""Facts several test modules share, and the helpers that read them. -**Why this exists (S6 / #187, S7 / #188; register C-46, C-57).** Three places needed a -views-platform sibling checkout and each found it differently: +**Read this first if you are new.** Despite the name, almost nothing here is a pytest +fixture. The file holds four separate things: + +1. **which packages exist** — ``PARTNER_PACKAGES`` and ``MACHINERY_PACKAGES``, the + two kinds of top-level package under ``views_postprocessing/``; +2. **the sibling repositories** — ``Sibling``/``SIBLINGS`` plus three functions for + locating one on disk. A *sibling* is another repository in the views-platform + organisation that sits beside this one in a developer's folder — never something we + install; +3. **which repository consumes which partner's delivery** — ``CONSUMER_REPO``; +4. **two read-only git helpers** used by cross-repository checks. + +Those four are not obviously one file's worth of responsibility, and that is recorded +rather than defended: register **C-88**, with the trigger for splitting it. Read it +before adding a fifth thing here. + +**Why the sibling helpers exist.** Three places needed to locate a views-platform +sibling checkout and each did it differently: - ``scripts/build_gaul_lookup.py`` resolved ``$VIEWS_DATAFACTORY``, then the sibling directory, with a ``--datafactory`` override and a fail-loud message naming both — @@ -65,8 +81,8 @@ class Sibling: """What this repository declares about one views-platform sibling (ADR-016). - Two fields, and the second is the one that matters. ``ci_checkout`` says whether CI - fetches this repository; ``note`` says why not, whenever the answer is no. + Three fields. ``env`` says where to find this repository on disk; ``ci_checkout`` + says whether CI fetches it; ``note`` says why not, whenever the answer is no. **An earlier version also carried ``public`` and ``public_checked``.** They were removed on 2026-08-10 after a review found them circular: ``public`` was read by diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index 6952307..82de91f 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -22,6 +22,20 @@ the build, because the default `GITHUB_TOKEN` is scoped to this repository. Rule G5 keeps that failure loud. +**Why the rules run G1, G2, G4, G5, G6, G7 with two numbers missing.** G3 paired a +sibling's visibility against whether its checkout used a credential; G8 checked that a +recorded date was plausible. Both were deleted on 2026-08-10 after a review found them +circular — each existed to protect a field that existed to feed it. The survivors keep +their original names rather than being renumbered, so anything written about "G6" +elsewhere still means this G6. The gap is deliberate; nothing is missing. + +**Two sections of this file have inverted failure semantics, and that trips people.** +Everything above `── the rules, against the real workflow ──` fails when *the workflow or +the declaration* is wrong. Everything below `── the rules, against synthetic mutants ──` +fails when **the rule itself** is wrong — those tests feed a deliberately broken world to +a rule and demand it complain. If one of those fails, do not go looking at +`run_pytest.yml`; the workflow is fine and the guard has stopped guarding. + **Every rule is a pure function of (workflow, siblings)**, so each can be run against a synthetic mutant rather than only against the real file. A guard that can only be demonstrated by editing CI is a guard nobody ever watches fail (ADR-014 §2) — this repo @@ -418,10 +432,28 @@ def test_every_rule_has_a_mutant_that_proves_it_bites(): @pytest.mark.parametrize("label, workflow, siblings, why", _MUTANTS) def test_each_rule_bites_on_a_broken_world(label, workflow, siblings, why): + """Feed a rule a deliberately broken world and demand that it complains. + + **A failure here means the RULE is broken, not the workflow.** That inversion is the + one thing about this file worth knowing before you debug it: `run_pytest.yml` is not + involved, the world being fed in is a synthetic dict defined below, and a green real + workflow is entirely consistent with this failing. A guard that no longer objects to + a violation is a guard that has silently stopped working, which is the failure this + whole file exists to make loud (ADR-014 §2). + + The second assertion is not pedantry: a rule that objects without naming which + sibling is at fault leaves a maintainer grepping a workflow file by hand. + """ violations = _RULES[label](workflow, siblings) - assert violations, f"[{label}] did not object to: {why}" + assert violations, ( + f"[{label}] did not object to a world that is deliberately broken: {why}. " + "This means the RULE has stopped working — the real workflow is not involved " + "and is probably fine. Fix the rule, not run_pytest.yml." + ) assert any("views-appwrite" in v for v in violations), ( - f"[{label}] objected but did not name the offending sibling: {violations}" + f"[{label}] objected, but its message does not name the offending sibling: " + f"{violations}. A maintainer reading only the failure would not know which " + "repository to look at." ) From 2d414741c5e6c3d5e28d78043d9c469eba32dd5f Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 15:04:09 +0200 Subject: [PATCH 3/9] test(registry): match on the rows we read, not on the edition label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry check compared version strings, so it fired on every upstream edition — five in four days, four of them recording console observations that carried no obligation here (C-86). Two of those failures were red on development. The diagnosis is ADR-014 §3: when a guard fires on something legitimate, ask whether the MATCHING is wrong before narrowing the scope. It matched on meta.version, a label meaning "anything at all changed". What this package depends on is a set of rows. Two earlier proposals of mine were wrong, and the reasons are in the ADR erratum. The second — read everything at the pinned commit — would have frozen both sides of every comparison, so upstream could rename every coordinate and the suite would stay green. ADR-016 §7b already adjudicated that fork, in a document I wrote days ago. Three checks replace one: the pinned commit declares the pinned version. Both sides frozen, so upstream can never redden it. This closes a hole that was live: "Do not bump one alone — the pair is the claim" existed only inside an error-message string, and nothing compared the two. fcf32c9 does declare 1.4.4 — true by luck. every top-level table upstream is classified here. _declared_classes parses three and was silently ignoring four. `[contract.*]` arrived in v1.5.0 carrying a live obligation and the ONLY thing that noticed was the noisy version check. Directional on purpose: a new table fails, a vanished IGNORED table does not. no row we declare differs between the pinned edition and the current one. Catches ROTATION, which no other check here can see — a rotated bucket id keeps its name and its class, so names-and-classes passes while every delivery goes elsewhere. The no-copy rule forbids keeping expected values in our source, so the pinned edition is the only lawful baseline for that comparison. The reader is deliberately NOT built on conftest.git_output, which turns any failure into "" — and tomllib.loads("") is {}, so an unreadable pin would produce an empty baseline compared against an empty projection, passing forever. That is the shape ADR-014 §2 is about, and building it into a drift detector's own fix would be the joke writing itself. It distinguishes a non-zero exit from an empty file and refuses a parse lacking anchors. Module-local, not conftest: C-88's trigger is a fifth declaration there and this has one consumer. Seven mutations proven, three of them now permanent tests that need no sibling: a bogus sha must raise rather than return {}; a new table must fire while a vanished IGNORED one must not; a rotation must fire where names-and-classes cannot. ADR-016 gains an erratum. §7a's "not adopted here — needs the upstream side first" was wrong: fixing the matching removes the false alarms without upstream doing anything. §7b's two categories become three — a differential tripwire reads a pinned baseline AND current, and only that shape can see a rotation. C-86 amended with the measured rate change and explicitly NOT closed: the coupling, the checkout and the zero bypass actors are untouched. Its trigger is still a console action. Suite 401 passed / 1 skipped / 40 xfailed. The one remaining failure is the faoapi source-read, which is Change 2 (#238 D4). Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 29 ++ reports/technical_risk_register.md | 20 ++ tests/test_env_declaration.py | 285 +++++++++++++++++- 3 files changed, 321 insertions(+), 13 deletions(-) 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 44debfd..e764885 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -221,6 +221,35 @@ branch. Both live here, and they answer different questions: G7 makes both read `main` rather than a default branch. It does not make either of them track the tip for its own sake. +**Erratum, 2026-08-11 — there is a third kind, and §7a's "not adopted here" is stale.** + +§7a recorded views-appwrite's suggestion — pin against the contract *version* and treat +observation-only bumps as non-blocking — and marked it *not adopted here, because it needs +the upstream side first*. That framing was wrong in a way worth correcting rather than +quietly editing: it treated the noise as something only upstream could fix. + +The registry-edition check was matching on `meta.version`, a label meaning *"anything at +all changed"*. What this repository actually depends on is a set of rows. Matching on the +rows instead removes the false alarms **without needing anything from upstream** — which +is ADR-014 §3 applied properly: when a guard fires on something legitimate, the first +question is whether the matching is wrong, not whether the scope is too wide. The matching +was wrong. + +So the two categories above become three: + +- **Drift tripwires** read the sibling's `main` because movement is the signal. +- **Reachability checks** read a pinned commit; movement is noise. +- **Differential tripwires** read **both** — a pinned edition as the baseline, `main` as + the comparison — and fire only when something *we declare* differs between them. This is + the shape the registry check now has. It catches a rotation, which is invisible to the + other two, and it is silent through prose edits and version bumps. + +The third kind exists because the no-copy rule forbids writing expected coordinate values +into this repository. A pinned edition is the only lawful place to keep a baseline for +comparing them. + +views-appwrite#76 is still worth doing; it is no longer a precondition for anything here. + --- This section overrides an earlier internal recommendation not to couple per-pull-request CI diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index f0594c4..40bc65a 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -210,6 +210,26 @@ ADR-016 has CI check out `views-appwrite` and `views-crafdapi` so that cross-rep **What would resolve it, in order of preference:** views-appwrite#76 makes the obligation-carrying distinction machine-readable, so observation-only bumps stop firing the check at all — filed, and that seat volunteered it. Failing that, a bypass actor restores the escape. Failing both, the coupling stands as ADR-016 §7 describes, which is defensible but should be chosen rather than discovered. Cross-refs: **C-81** (the enforcement half, whose fix activates this), **C-46** (whose recommendation against per-PR sibling checkouts this overrode, with the reasoning recorded there), ADR-016 §7/§7a/§7b, views-appwrite#76. +**Partial mitigation 2026-08-11 — the false-alarm rate, not the coupling.** The check that +kept firing compared registry *version strings*. It has been replaced by three that match +on the facts this repository actually declares: the pinned commit must declare the pinned +version; every top-level table upstream must be classified here; and no row we read may +differ between the pinned edition and the current one. + +Measured across the window that prompted this entry — v1.4.4 → v1.5.2, three editions in a +week — the new checks are **green**, because none of those editions touched a row this +package reads. Under the old check every one of them was a red build blocking a release. + +**This does not close the entry, and the distinction matters.** C-86 is about CI depending +on two other repositories with no way past a red build. That dependency is untouched: the +sibling checkout still happens, `protect_main` still has zero bypass actors, and a change +upstream that *does* touch a row we read will still redden this repository and block a +merge — correctly, and that is the point. What changed is that it now fires for reasons +that carry an obligation. + +The entry's trigger is unchanged and remains a console action: **a status check becomes +required on `main`**, at which moment the coupling stops being latent. + --- diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 74d3267..49a3d60 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -485,6 +485,96 @@ def _load_registry(repo: Path) -> dict: return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) +#: Every top-level table the registry may carry, and what this repository does with it. +#: +#: **Declared, and asserted against the live registry** — a table appearing upstream that +#: nobody here has classified is a change this repository has not looked at. That is not +#: hypothetical: `[contract.*]` arrived in v1.5.0 carrying a live obligation (the +#: ADR-017 delivery-label mirror), and the ONLY mechanism here that noticed was a +#: version-string comparison that fired for the wrong reason and was deleted with this +#: change. Four tables were being ignored silently at the time. +_TABLE_ROLE = { + "connection": "CONSUMED", # parsed by _declared_classes; coordinates we read + "target": "CONSUMED", # parsed by _declared_classes; coordinates we read + "secret": "CONSUMED", # parsed by _declared_classes; the operator's slots + "contract": "MIRRORED", # values legitimately live in our source — ADR-017; see below + "excluded": "IGNORED", # names the registry records as deliberately NOT coordinates + "test_environment": "IGNORED", # a fact about the platform, not about this package + "meta": "METADATA", # the edition and its amendment log +} + +#: Tables whose absence is our problem. An IGNORED table vanishing is not — that +#: direction is deliberately unchecked, which is why v1.5.1 dropping `[unmodelled]` is +#: silent while v1.5.0 adding `[contract]` is a red build with something to do. +_TABLES_WE_DEPEND_ON = tuple( + name for name, role in _TABLE_ROLE.items() if role in ("CONSUMED", "MIRRORED") +) + + +class _RegistryReadError(RuntimeError): + """The registry could not be read at a pinned commit — never a silent empty dict.""" + + +def _registry_at(repo: Path, ref: str) -> dict: + """The registry as it stood at ``ref``, or raise. Never returns ``{}`` quietly. + + **Why not ``conftest.git_output``.** That helper turns any failure into ``""``, and + ``tomllib.loads("")`` returns ``{}``. An empty registry compared against an empty + projection passes — forever, silently. That is the vanished-directory shape ADR-014 + §2 exists for, and building it into the fix for a drift detector would be the joke + writing itself. So this distinguishes a non-zero exit from an empty file, and + refuses a parse that lacks the anchors every real edition has. + + Module-local rather than in ``conftest.py``: register C-88 records that file already + holding four unrelated groups, and its trigger is a fifth. There is one consumer. + """ + tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") + result = subprocess.run( + ["git", "-C", str(repo), "show", f"{ref}:{_REGISTRY_RELPATH.as_posix()}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if result.returncode != 0: + raise _RegistryReadError( + f"cannot read the registry at {ref!r} in {repo}: git exited " + f"{result.returncode} ({result.stderr.strip()[:200]}). A pinned commit that " + "cannot be read is not a baseline — run `git fetch` in that checkout." + ) + if not result.stdout.strip(): + raise _RegistryReadError( + f"the registry is EMPTY at {ref!r} in {repo}. Treating that as a registry " + "with no coordinates would make every comparison below pass." + ) + registry = tomllib.loads(result.stdout) + if not registry.get("meta", {}).get("version") or not registry.get("connection"): + raise _RegistryReadError( + f"the registry at {ref!r} parsed but carries no meta.version or no " + "connection rows. Every real edition has both, so this is not the file we " + "think it is — refusing rather than comparing against it." + ) + return registry + + +def _projection(registry: dict, names: set[str]) -> dict[str, tuple]: + """``name -> (section, class, value-or-absent)`` for the names we declare. + + The unit of comparison between two editions. Restricted to our own surface, so an + unrelated row moving is invisible here — that is the whole point, and it is what + stops an observation-only bump from reddening this repository (register C-86). + + ``value`` is included because **rotation is the change no other check can see**: a + bucket id whose value changes keeps its name and its class, so the names-and-classes + check passes while every delivery goes to the wrong place. The no-copy rule forbids + writing the expected values into this repository, so the pinned edition is the only + lawful place a baseline for that comparison can live. + """ + out = {} + for section in ("connection", "target", "secret"): + for name, body in (registry.get(section) or {}).items(): + if name in names: + out[name] = (section, body.get("class"), body.get("value", "")) + return out + + def _declared_classes(registry: dict) -> dict[str, str]: """name -> the class the registry DECLARES for it (never inferred from the name).""" return { @@ -520,24 +610,116 @@ def test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_a @pytest.mark.parametrize("partner", _PARTNERS) -def test_the_pinned_contract_edition_still_matches_the_registry(partner): - """The check that catches everything the other one cannot — including additions. +def test_the_pinned_commit_declares_the_pinned_version(partner): + """The pair is a claim, and until now nothing checked it. + + ``appwrite_env.py`` declares a version AND a commit, and the old drift check's own + message said *"Do not bump one alone — the pair is the claim."* That sentence lived + only inside an error string: no test compared the two. Someone could bump either + alone and the whole suite stayed green, leaving a pin that names an edition nobody + published. - Names and classes catch a rename. The edition catches **any** other change: a new - target this repo ought to adopt, a retired secret slot, a reworded rule. It fails - loudly and tells you what to do rather than what broke. + Verified 2026-08-11: ``fcf32c9`` does declare ``1.4.4`` — true by luck, not by check. - It is also the check that would have caught #211's stale crafd pin on the day it - was written, had it been looking at crafd. It was not. It is now. + Both sides of this comparison are frozen, so it is the one registry check in this + file that upstream activity can never redden. It fails only when *we* are wrong. """ repo = require_sibling("views-appwrite") module = _PARTNER_ENV[partner][0] - actual = _load_registry(repo)["meta"]["version"] - assert actual == module.SEAM_CONTRACT_VERSION, ( - f"the Appwrite Seam Contract's registry moved to v{actual}; " - f"{partner}/appwrite_env.py declares v{module.SEAM_CONTRACT_VERSION}. Re-verify " - f"that module's declaration against v{actual}, then bump SEAM_CONTRACT_VERSION " - "and SEAM_CONTRACT_COMMIT together. Do not bump one alone — the pair is the claim." + try: + pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) + except _RegistryReadError as exc: + pytest.fail(str(exc)) + assert pinned["meta"]["version"] == module.SEAM_CONTRACT_VERSION, ( + f"[{partner}] the pin is internally inconsistent: commit " + f"{module.SEAM_CONTRACT_COMMIT} declares registry v{pinned['meta']['version']}, " + f"but SEAM_CONTRACT_VERSION says v{module.SEAM_CONTRACT_VERSION}. One of the two " + "was bumped alone. The pair is the claim — fix whichever is wrong." + ) + + +def test_every_table_in_the_registry_is_classified_here(): + """A table nobody classified is an upstream change nobody looked at. + + This replaces the old edition-equality check, and it is worth being precise about + what that check was really doing. It compared version *strings*, so it fired on every + edition — five in four days, four of them recording console observations that carried + no obligation for anyone here (register C-86). A guard that cries wolf gets bumped + reflexively, which is how its one real firing goes unread. + + But it did have a real job underneath the noise, and this is that job. On 2026-08-10 + the registry grew a ``[contract.*]`` table carrying a live obligation for this + repository — the ADR-017 delivery-label mirror — and the edition check was the *only* + mechanism here that noticed, because ``_declared_classes`` parses three tables and + 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 + why v1.5.1 dropping ``[unmodelled]`` is silent while v1.5.0 adding ``[contract]`` is + a red build with something to do. Two events in the registry's life, both of which + this repository needed to see. + """ + repo = require_sibling("views-appwrite") + registry = _load_registry(repo) + + unclassified = sorted(set(registry) - set(_TABLE_ROLE)) + assert not unclassified, ( + f"the registry has table(s) this repository has never classified: " + f"{unclassified}. Decide what each one is — CONSUMED (we read it), MIRRORED (its " + "values live in our source), or IGNORED (with a reason) — and add it to " + "_TABLE_ROLE. A new table is how an obligation arrives; `[contract.*]` arrived " + "exactly this way and only a noisy version check happened to catch it." + ) + vanished = sorted(n for n in _TABLES_WE_DEPEND_ON if n not in registry) + assert not vanished, ( + f"table(s) this repository depends on are gone from the registry: {vanished}. " + "Either they were retired upstream and this package must stop reading them, or " + "the file being read is not the registry." + ) + + +@pytest.mark.parametrize("partner", _PARTNERS) +def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): + """The drift check, matching on the facts rather than on the edition label. + + ADR-014 §3: when a guard fires on something legitimate, ask whether the *matching* is + wrong before you narrow the scope. The old check matched on ``meta.version``, which + is a label for "anything at all changed". This matches on the rows this package + actually declares, compared between the edition we pinned and the edition upstream + has now. + + **Fires on:** a rename, a reclassification, a removal, a valueless row gaining a + value — and on **rotation**, which is the one no other check in this file can see. A + rotated bucket id keeps its name and its class, so names-and-classes passes while + every delivery goes somewhere else. The no-copy rule forbids writing the expected + values into this repository, so the pinned edition is the only lawful baseline for + that comparison. + + **Silent through:** prose edits, ``[meta]`` bumps, and rows belonging to anyone else. + Measured across the real v1.4.4 -> v1.5.2 window (three editions, one week): green. + """ + repo = require_sibling("views-appwrite") + module, _, expected_class = _PARTNER_ENV[partner] + try: + pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) + except _RegistryReadError as exc: + pytest.fail(str(exc)) + + names = set(expected_class) + then, now = _projection(pinned, names), _projection(_load_registry(repo), names) + + changed = { + name: {"at the pin": then.get(name, ""), "now": now.get(name, "")} + for name in sorted(names) + if then.get(name) != now.get(name) + } + assert not changed, ( + f"[{partner}] the registry changed something this package reads, since the " + f"edition it was verified against (v{module.SEAM_CONTRACT_VERSION}, commit " + f"{module.SEAM_CONTRACT_COMMIT}): {changed}. Re-verify this module's declaration " + "against the current edition, then move SEAM_CONTRACT_VERSION and " + "SEAM_CONTRACT_COMMIT together. If a value rotated, the delivery is pointing " + "somewhere else until the launcher's environment is updated too." ) @@ -593,6 +775,83 @@ def test_the_pinned_commit_is_reachable_from_the_contract_repos_main(partner): ) +def test_the_pinned_reader_refuses_rather_than_returning_an_empty_registry(): + """The trap this reader exists to avoid, proven rather than asserted in prose. + + ``conftest.git_output`` turns any git failure into ``""``, and ``tomllib.loads("")`` + is ``{}``. Had this reader been built on it, an unreadable pin would have produced an + empty baseline, an empty projection, and a comparison that passes forever — the + vanished-directory shape ADR-014 §2 exists for, rebuilt inside the fix for a drift + detector. + + Runs everywhere: it needs a git repository, not the sibling, and this repository is + one. + """ + with pytest.raises(_RegistryReadError, match="cannot read the registry"): + _registry_at(_REPO, "0000000000000000000000000000000000000000") + + +def test_the_table_partition_would_catch_a_new_table_and_a_vanished_one(): + """Both directions, against synthetic registries — no sibling needed. + + The asymmetry is the design: an *unclassified* table fails (an obligation may have + arrived), a *missing IGNORED* table does not (not our business). Getting that + backwards would either miss the `[contract.*]` case or cry wolf on v1.5.1's removal + of `[unmodelled]`, and both have actually happened upstream. + """ + base = {name: {} for name in _TABLE_ROLE} + + arrived = dict(base) | {"brand_new_table": {}} + assert sorted(set(arrived) - set(_TABLE_ROLE)) == ["brand_new_table"], ( + "a table nobody classified went unnoticed — that is how `[contract.*]` arrived" + ) + + lost_consumed = {k: v for k, v in base.items() if k != "target"} + assert [n for n in _TABLES_WE_DEPEND_ON if n not in lost_consumed] == ["target"], ( + "a table this package reads vanished and the check did not object" + ) + + lost_ignored = {k: v for k, v in base.items() if k != "excluded"} + assert not [n for n in _TABLES_WE_DEPEND_ON if n not in lost_ignored], ( + "an IGNORED table disappearing must NOT fire — v1.5.1 removed `[unmodelled]` " + "and that carried no obligation for anyone here (ADR-014 §3: prefer the false " + "negative to the false alarm)" + ) + + +@pytest.mark.parametrize("partner", _PARTNERS) +def test_the_drift_check_would_catch_a_rotation_that_names_and_classes_cannot(partner): + """Rotation is the whole reason the projection carries ``value``. + + A rotated bucket id keeps its name and its class, so + ``test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_as`` + passes while every delivery goes somewhere else. This proves the differential sees + what that check structurally cannot — and proves it without a sibling checkout, so + it runs on every machine rather than only where views-appwrite happens to be. + + No real coordinate value appears here: both editions are synthetic, and the point is + that the two *differ*, not what either says. + """ + _, own_store, expected_class = _PARTNER_ENV[partner] + canary = own_store[0] + names = set(expected_class) + + def edition(value: str) -> dict: + return {"target": {canary: {"class": "target", "value": value}}} + + then = _projection(edition("value-at-the-pin"), names) + now = _projection(edition("value-after-rotation"), names) + + assert then[canary][:2] == now[canary][:2], ( + "this fixture is meant to change ONLY the value — if the name or class differs " + "it is not proving what it claims" + ) + assert then != now, ( + f"[{partner}] a rotated value went unnoticed. Names and classes are identical " + "on both sides, so nothing else in this file can see it." + ) + + @pytest.mark.parametrize("partner", _PARTNERS) def test_the_drift_check_would_catch_a_rename(partner): """A gated test that cannot fail is decoration — so prove this one bites in CI. From 3f3f5709baab26ec9b526966de2ad2cc31fb3d20 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 15:07:22 +0200 Subject: [PATCH 4/9] =?UTF-8?q?test(product):=20#238=20D3+D4=20=E2=80=94?= =?UTF-8?q?=20the=20delivery=20label=20reads=20the=20registry,=20not=20the?= =?UTF-8?q?=20consumer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D3. The consumer-name check now reads the public coordinate registry instead of the consumer's source, for BOTH partners. No credential, and the private partner is reachable for the first time — that is ADR-017 §5 in one assertion. The check it replaces broke on 2026-08-11, and how it broke is the argument: views-faoapi tidied managers/api.py into a named constant, so our regex went looking for an APIPathManager(...) construction that had moved. Their code got better and our check went red. ADR-017 §7 — we were never entitled to depend on another repository's file layout. D4 is PER PARTNER, and I got this wrong once before catching it. My first pass deleted the source-read for both, which violates ADR-017 §5's own sequencing constraint: a partner's source-read goes only when THAT partner's consumer-side check exists. Otherwise the registry row is a string a human typed and a green build proves only that two values this platform authored agree. views-faoapi#379 merged 2026-08-11 -> the FAO source-read is gone views-crafdapi#53 open -> the crafd source-read stays The survivor is scoped by a declared, deliberately SHRINKING map (_CONSUMER_SELF_CHECK_PENDING) naming the issue that retires each entry, and its docstring says plainly that the test is meant to be deleted. The crafd sibling fetch lives or dies with it, which conftest's note already records. BOTH PARTNERS, correcting #238. That issue says CRAF'd's label "is not a decided value" and defers it. views-appwrite declared both rows in v1.5.0 and the CRAF'd row corrects this itself: "Both sides already use this value in code today — this row DECLARES a standing fact, it does not decide a new one", citing both sides' source. ADR-017 Appendix B updated. ADR-017 Appendix B also carried a contradiction, fixed before scoping rather than discovered mid-PR: step 2 said "in the same change, stop fetching views-crafdapi", while §5 forbids retiring that partner's check until #53 lands — and that fetch's sole consumer IS that check. The two could not both be followed. The no-copy scan is now scoped BY THE PARTITION rather than an inline tuple, so `contract` cannot be swept in by a one-word edit. It must not be: ADR-017 §5 REQUIRES those values in our source, so banning them would forbid what the contract obliges. Not an exception to "never copy a coordinate" — a different class, declared upstream, whose values no reader ever emits into a process environment. Proven: reclassifying contract as CONSUMED reddens on our own product.py, so the widening cannot be quiet. Suite 409 passed / 1 skipped / 40 xfailed — green for the first time in this arc. Co-Authored-By: Claude Opus 5 (1M context) --- .../017_facts_across_a_private_boundary.md | 32 ++-- tests/test_env_declaration.py | 18 ++- tests/test_product.py | 147 +++++++++++++++--- 3 files changed, 163 insertions(+), 34 deletions(-) diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index c419527..d327e68 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -357,17 +357,29 @@ visibility can be inferred for future APIs. 1. **views-appwrite** — declare the label for each partner in the coordinate registry. Filed as views-appwrite#75. Nothing here can proceed before it. - **The two partners are not equally ready, and the halves should land separately.** The - FAO label is fully specified — `un_fao`, in force, evidenced in ADR-013 §4.1a. The - CRAF'd label is **not a value anyone has decided yet**: that partner serves a different - dataset whose targets, columns and entity model are still an open data-contract question - on the consumer side. Bundling both into one edit stalls the ready half behind the - blocked one, so **the FAO row should land on its own** and the CRAF'd row should follow - its data contract. + **Superseded 2026-08-11 — both rows now exist and both are in force.** This step used + to say the CRAF'd label was "not a value anyone has decided yet" and should follow its + data contract. views-appwrite declared both rows in v1.5.0, and the CRAF'd row states + the correction itself: *"Both sides already use this value in code today — this row + DECLARES a standing fact, it does not decide a new one."* It cites both sides' source. + + So step 1 is **done for both partners**, and step 2 covers both. What remains split is + step 3, which is per-partner and is what step 2's retirement half waits on. 2. **views-postprocessing** — switch the check to read the registry rather than the - consumer's source, for both partners. Ours, blocked on step 1. **In the same change**, - stop fetching `views-crafdapi` in CI: §7 shows that fetch then serves nothing, and - ADR-016's rules require a sibling that is not fetched to say why. + consumer's source, for both partners. Ours, blocked on step 1. + + **Erratum, 2026-08-11 — this step used to say "in the same change, stop fetching + `views-crafdapi` in CI", and that contradicted §5.** §5 constrains the order: a + partner's source-reading check is removed only once *that partner's* consumer-side + check exists. The crafd source-read is the sole consumer of the crafd fetch, so + retiring the fetch retires the check — before views-crafdapi#53 has landed. The + instruction and the constraint could not both be followed. + + Corrected: **retirement is per partner, and follows that partner's step 3.** For FAO + that is now met (views-faoapi#379 merged 2026-08-11), so the FAO source-read goes. For + CRAF'd it is not (views-crafdapi#53 open), so the crafd source-read stays — and with + it the crafd fetch, whose `note` in `tests/conftest.py` already records that it lives + or dies with that one check. 3. **views-faoapi** — verify its own served label against the declaration; §8's other half. Filed as views-faoapi#379, and the consumer-side maintainer has accepted it. diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 49a3d60..e9bcc7a 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -497,7 +497,8 @@ def _load_registry(repo: Path) -> dict: "connection": "CONSUMED", # parsed by _declared_classes; coordinates we read "target": "CONSUMED", # parsed by _declared_classes; coordinates we read "secret": "CONSUMED", # parsed by _declared_classes; the operator's slots - "contract": "MIRRORED", # values legitimately live in our source — ADR-017; see below + "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 "meta": "METADATA", # the edition and its amendment log @@ -937,9 +938,22 @@ def test_no_coordinate_value_is_copied_into_this_repo(): if repo is None: pytest.skip("views-appwrite checkout not found — set VIEWS_APPWRITE") registry = _load_registry(repo) + # Scoped by the declared partition, NOT by an inline tuple — so a new table cannot + # be swept in by a one-word edit, and `contract` cannot be swept in at all. + # + # `[contract.*]` values are MIRRORED: ADR-017 §5 requires them to appear in this + # package's source, because we write them onto every upload. Banning them here would + # forbid the thing the contract obliges. That is not an exception to "never copy a + # coordinate" — it is a different class, declared upstream: the registry's own header + # says no reader scans `[contract.*]`, so no value there ever reaches a process + # environment. The no-copy rule protects values the launcher supplies; a mirror is + # the inverse by construction. + scanned_sections = tuple( + name for name, role in _TABLE_ROLE.items() if role == "CONSUMED" and name != "secret" + ) values = { body["value"] - for section in ("connection", "target") + for section in scanned_sections for body in registry.get(section, {}).values() if isinstance(body.get("value"), str) and len(body["value"]) > 6 } diff --git a/tests/test_product.py b/tests/test_product.py index 1fd606f..aa2281f 100644 --- a/tests/test_product.py +++ b/tests/test_product.py @@ -178,17 +178,125 @@ def test_every_partner_has_a_declared_consumer_repository(): ) +#: The platform registry, relative to a views-appwrite checkout. +#: +#: Loaded locally rather than imported from ``tests/test_env_declaration.py``: a test +#: module importing another test module is a dependency nobody declared, and the two read +#: different things — that file reads coordinate sections, this one reads one contract +#: row. Two four-line readers that are understood beat one shared one that has to serve +#: both (WET before DRY). +_REGISTRY_RELPATH = Path("docs") / "ADRs" / "platform" / "coordinate_registry.toml" + + +def _registry(repo: Path) -> dict: + tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") + return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) + + +#: partner -> the registry row that is the AUTHORITY for its delivery label. +#: +#: Declared, never derived from the partner name: `unfao` -> `UNFAO_...` is mechanical, +#: `crafd` -> `UNCRAFD_...` is not, and guessing it would be the inference ADR-003 forbids. +_CONTRACT_ROW = { + "unfao": "UNFAO_CONSUMER_DOCUMENT_NAME", + "crafd": "UNCRAFD_CONSUMER_DOCUMENT_NAME", +} + + +def test_every_partner_has_a_declared_contract_row(): + """Assert this file's declared scope is the real one (ADR-014 §2). + + A partner missing from ``_CONTRACT_ROW`` is a partner whose delivery label is checked + against nothing, and the suite stays green — which is exactly how ``crafd`` arrived + the first time. + """ + assert set(_CONTRACT_ROW) == set(PARTNER_PACKAGES), ( + f"partners with no declared contract row: " + f"{sorted(set(PARTNER_PACKAGES) - set(_CONTRACT_ROW))}; declared but no longer a " + f"partner: {sorted(set(_CONTRACT_ROW) - set(PARTNER_PACKAGES))}." + ) + + @pytest.mark.parametrize("partner", PARTNER_PACKAGES) -def test_the_declared_consumer_name_still_matches_the_consumer(partner): - """§4.1a across the seam — the half a local literal cannot carry. +def test_the_declared_consumer_name_matches_the_registry(partner): + """ADR-017 §5, the producer half: our mirror against the public declaration. + + **What this replaces, and why.** Until 2026-08-11 this check read the *consumer's + source* — a regex over `managers/api.py` looking for the argument to an + `APIPathManager(...)` construction. That worked on a laptop and never in CI for the + private partner, and it broke on 2026-08-11 when views-faoapi tidied that file into a + named constant. Their code got better and our check went looking for something that + had moved. ADR-017 §7: we were never entitled to depend on another repository's file + layout. + + Now both sides read one public declaration and neither reads the other. The registry + lives in views-appwrite, which is public, so this needs no credential even for the + private partner — that is the whole of ADR-017 §5 in one assertion. + + The value it guards is the one whose failure is silent: a delivery filed under a name + the consumer does not ask for is uploaded, stored, billed, and invisible — no error + anywhere (ADR-013 §4.1a). + """ + repo = require_sibling("views-appwrite") + registry = _registry(repo) + + contract = registry.get("contract") or {} + row = _CONTRACT_ROW[partner] + assert row in contract, ( + f"[{partner}] the registry has no `[contract.{row}]` row. That row is the " + "authority this package's CONSUMER_DOCUMENT_NAME mirrors (ADR-017 §5); without " + "it there is nothing to check the mirror against. Either it was retired upstream " + "or this file names the wrong row." + ) + declared = contract[row].get("value") + ours = _product(partner).CONSUMER_DOCUMENT_NAME + assert declared == ours, ( + f"[{partner}] this package writes every delivery under {ours!r}, but the platform " + f"registry declares {declared!r} in `[contract.{row}]`. The registry is the " + "authority — the consumer owns this name and changing it is a contract amendment. " + "A delivery under a name the consumer does not ask for is uploaded, stored, and " + "invisible (ADR-013 §4.1a)." + ) + assert declared == _CONSUMER_DOCUMENT_NAME[partner], ( + f"[{partner}] the pin in this file disagrees with the registry — the local " + "always-on test above should have caught a mismatch with the module first." + ) + + +#: Partners whose consumer-side self-check has NOT landed yet, so this repository still +#: reads their source as well as the registry (ADR-017 §5's sequencing constraint). +#: +#: Declared rather than inferred, and deliberately a *shrinking* list: an entry leaves it +#: when that partner's own check lands, and the day it empties this whole mechanism goes. +_CONSUMER_SELF_CHECK_PENDING = { + "crafd": "views-crafdapi#53", +} + + +def test_the_pending_list_names_only_real_partners(): + """A stale name here would keep a retired mechanism alive against nothing.""" + unknown = sorted(set(_CONSUMER_SELF_CHECK_PENDING) - set(PARTNER_PACKAGES)) + assert not unknown, f"not partners: {unknown}" + - The consumer decides what it filters on. This repository declares what it writes. - Agreement between the two is the whole of §4.1a, and it is not a fact this - repository owns — so it is checked against the consumer's checkout, exactly as the - coordinate registry is checked against views-appwrite's. +@pytest.mark.parametrize("partner", sorted(_CONSUMER_SELF_CHECK_PENDING)) +def test_the_consumer_still_filters_on_the_name_until_it_checks_itself(partner): + """The source-read, kept ONLY for partners whose own check does not exist yet. - Gated: skips without the sibling, naming the variable to set. The always-on pin - above still runs everywhere, so CI is never left with nothing. + ADR-017 §5 constrains the order: this repository stops reading a consumer's source + when *that consumer* starts checking itself against the registry. Otherwise there is + a window where the registry row is a string a human typed, our check compares our + copy to it, and nothing anywhere consults what the consumer actually does — a green + build proving only that two values this platform authored agree. + + views-faoapi#379 landed on 2026-08-11, so the FAO half of this check is gone. + views-crafdapi#53 has not, so CRAF'd's stays — and with it the crafd sibling fetch, + whose ``note`` in ``tests/conftest.py`` records that it lives or dies with this one + check. + + **This test is meant to be deleted.** When #53 lands, remove that partner from + ``_CONSUMER_SELF_CHECK_PENDING``; when the map empties, remove this test, the regexes + it uses, and the fetch. """ repo = require_sibling(CONSUMER_REPO[partner]) pkg = _consumer_package(repo) @@ -197,25 +305,20 @@ def test_the_declared_consumer_name_still_matches_the_consumer(partner): found = _CONSUMER_PATH_MANAGER.findall(api) assert len(found) == 1, ( f"expected exactly one APIPathManager(...) construction in " - f"{CONSUMER_REPO[partner]}'s managers/api.py, found {found}. More than one " - "means the consumer serves several document names and this check no longer " - "knows which one is ours." + f"{CONSUMER_REPO[partner]}'s managers/api.py, found {found}. This is the fragile " + "half ADR-017 retires — a layout change there breaks a check here, which is what " + "happened to the FAO half on 2026-08-11. It survives only until " + f"{_CONSUMER_SELF_CHECK_PENDING[partner]} lands." ) assert found[0] == _CONSUMER_DOCUMENT_NAME[partner], ( f"{CONSUMER_REPO[partner]} filters on {found[0]!r}; this repository declares " - f"{_CONSUMER_DOCUMENT_NAME[partner]!r} and writes it as " - f"{partner}/product.py's CONSUMER_DOCUMENT_NAME. A delivery under a name the " - "consumer does not ask for is uploaded, stored, and invisible (ADR-013 §4.1a)." - ) - assert found[0] == _product(partner).CONSUMER_DOCUMENT_NAME, ( - "the declared pin in this file and the module's constant disagree — the " - "always-on test above should have caught this first" + f"{_CONSUMER_DOCUMENT_NAME[partner]!r}. A delivery under a name the consumer does " + "not ask for is uploaded, stored, and invisible (ADR-013 §4.1a)." ) manager = (pkg / "managers" / "prediction" / "manager.py").read_text() assert _CONSUMER_FILTER in manager, ( - f"{CONSUMER_REPO[partner]} no longer selects by " - f"{_CONSUMER_FILTER!r}. The name may still match while the consumer filters on " - "something else entirely — same invisibility, different cause. Re-read its " - "selection path before assuming this repository's deliveries are reachable." + f"{CONSUMER_REPO[partner]} no longer selects by {_CONSUMER_FILTER!r}. The name " + "may still match while the consumer filters on something else entirely — same " + "invisibility, different cause." ) From 8e1b874b68e3272bf635bc7cd1b9b7d9601cdbe1 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 15:15:58 +0200 Subject: [PATCH 5/9] chore(seam): re-pin both partners to registry v1.5.2 / b703cab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requested by the views-appwrite seat on #238. Their three facts all check out: b703cab declares 1.5.2, it is reachable from their main, and it is the tag appwrite-seam-v1.5.2. ONE CORRECTION TO THEIR PREMISE. The issue is titled "we are breaking your build" and names test_the_pinned_contract_edition_still_matches_the_registry. That test no longer exists — the preceding commit deleted it, because comparing version STRINGS is what made three observation-only editions look like three obligations. So nothing was broken and no push was going to break. Worth telling them, since they filed it as an outage they had caused. Their advice survives the correction, and is the better half of the message: "actually read the diff rather than just moving the number." Done, both ways. By hand — git diff appwrite-seam-v1.4.4..v1.5.2 on the registry is, in full: + [contract.UNFAO_CONSUMER_DOCUMENT_NAME] value = "un_fao" + [contract.UNCRAFD_CONSUMER_DOCUMENT_NAME] value = "un_crafd" - [unmodelled.session_auth_email_password] ... and prose Every element is already handled by the checks that replaced the string comparison: the two contract rows are classified MIRRORED and asserted in test_product; the unmodelled removal is the IGNORED direction that is deliberately silent; and nothing in connection, target or secret moved. Mechanically — test_nothing_this_repo_reads_has_changed_since_the_pin was green across the whole 1.4.4 -> 1.5.2 window before this commit. That is the re-verification the ratchet asks for, done by a check rather than by a promise, and it is what makes moving the baseline safe rather than merely tidy. Note what re-pinning costs: the differential now spans nothing until upstream moves again. That is correct — a baseline that has been verified moves forward — but it means the evidence for this window lives in this message rather than in a test from here on. Docstring version and the pinned blob URL moved with the constants; the pin-pair check confirms b703cab declares 1.5.2 rather than taking it on trust. Suite 409 passed. Co-Authored-By: Claude Opus 5 (1M context) --- views_postprocessing/crafd/appwrite_env.py | 8 ++++---- views_postprocessing/unfao/appwrite_env.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/views_postprocessing/crafd/appwrite_env.py b/views_postprocessing/crafd/appwrite_env.py index 57ab560..9a379c6 100644 --- a/views_postprocessing/crafd/appwrite_env.py +++ b/views_postprocessing/crafd/appwrite_env.py @@ -6,9 +6,9 @@ views-appwrite and referenced by pinned URL, never copied — copies were the platform's original failure (þing-01 sáttmál S6): - https://github.com/views-platform/views-appwrite/blob/fcf32c9/docs/ADRs/platform/coordinate_registry.toml + https://github.com/views-platform/views-appwrite/blob/b703cab/docs/ADRs/platform/coordinate_registry.toml -That pin is registry **v1.4.4** — declared below as ``SEAM_CONTRACT_VERSION`` / +That pin is registry **v1.5.2** — declared below as ``SEAM_CONTRACT_VERSION`` / ``SEAM_CONTRACT_COMMIT`` so the pin is a value a test can check rather than a fact buried in prose. A pinned URL does not rot, but it does go stale, and nothing in this repository could previously tell you it had (register C-57). @@ -47,8 +47,8 @@ #: declaration against that edition of the registry. ``tests/test_env_declaration.py`` #: enforces the pair — for this package and for ``unfao`` alike — against a local #: views-appwrite checkout when one is present. -SEAM_CONTRACT_VERSION = "1.4.4" -SEAM_CONTRACT_COMMIT = "fcf32c9" +SEAM_CONTRACT_VERSION = "1.5.2" +SEAM_CONTRACT_COMMIT = "b703cab" CONNECTION_ENV = ( "APPWRITE_ENDPOINT", diff --git a/views_postprocessing/unfao/appwrite_env.py b/views_postprocessing/unfao/appwrite_env.py index ffd6169..b13fade 100644 --- a/views_postprocessing/unfao/appwrite_env.py +++ b/views_postprocessing/unfao/appwrite_env.py @@ -5,9 +5,9 @@ views-appwrite and referenced by pinned URL, never copied — copies were the platform's original failure (þing-01 sáttmál S6): - https://github.com/views-platform/views-appwrite/blob/fcf32c9/docs/ADRs/platform/coordinate_registry.toml + https://github.com/views-platform/views-appwrite/blob/b703cab/docs/ADRs/platform/coordinate_registry.toml -That pin is registry **v1.4.4** — declared below as +That pin is registry **v1.5.2** — declared below as ``SEAM_CONTRACT_VERSION`` / ``SEAM_CONTRACT_COMMIT`` so the pin is a value a test can check rather than a fact buried in prose. A pinned URL does not rot, but it does go stale, and nothing in this repository could previously tell you it had (register C-57). @@ -44,8 +44,8 @@ #: Bumping these is not bookkeeping: it asserts that someone re-checked this module's #: declaration against that edition of the registry. ``tests/test_env_declaration.py`` #: enforces the pair against a local views-appwrite checkout when one is present. -SEAM_CONTRACT_VERSION = "1.4.4" -SEAM_CONTRACT_COMMIT = "fcf32c9" +SEAM_CONTRACT_VERSION = "1.5.2" +SEAM_CONTRACT_COMMIT = "b703cab" CONNECTION_ENV = ( "APPWRITE_ENDPOINT", From 898a0aa05a9cd91062b7d61b235bea0183054277 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 16:22:01 +0200 Subject: [PATCH 6/9] fix(tests): fifteen findings from /code-review max, one of them shipped-and-dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The design had been reviewed before implementation and the ADRs falsified four times. The ~470 lines of test code had been reviewed by nobody. Fifteen findings; most were mutation-proven by the reviewer against my own code. THE ONE THAT WAS ALREADY LIVE. SEAM_CONTRACT_COMMIT = "b703cab" is an annotated TAG OBJECT, not a commit. git cat-file -t says `tag`; it peels to c7b597e. So the pinned blob URL both PUBLIC partner modules publish returned HTTP 404 — verified over the network — three lines above the sentence "A pinned URL does not rot." Every test passed because git peels tags silently in show, merge-base and cat-file alike. Pinned c7b597e. SECURITY. The drift check's failure message interpolated projection tuples, which carry coordinate VALUES, into a PUBLIC repository's world-readable CI log — on the one event the check exists to fire on. Values are now digested (sha256 prefix); names and fields are reported in the clear. BOTH "PERMANENT PROOFS" WERE TAUTOLOGIES. They re-typed the production expressions instead of calling them. Proven: replacing the real predicates with `return []` left the table proof green; making the drift comparison ignore `value` — literally the thing the rotation proof exists to disprove — left that one green too. So the guards this PR sells as mutation-proven had no detector at all. Predicates extracted; both proofs now call production code; both re-proven by breaking the real thing. _TABLE_ROLE WAS DECORATIVE. Two of its three consumers kept hardcoded copies of the section tuple, so classifying a table CONSUMED satisfied the guard demanding it and added zero coverage — C-74's shape arriving through a guard's own remediation advice. One _rows() reader now drives all three. Role strings are a closed vocabulary, because a typo silently narrowed the security scan from twelve values to two. THE DELETED CHECK'S REAL JOB WAS NOT FULLY REPLACED. A new upstream row in a table we read was invisible, and nothing forced a re-pin: restoring the pin to 1.4.4 against the current registry passed everything. Added: coordinates ARRIVING in a table we read now fail with "decide whether this package must adopt them" — the half worth keeping, since `[contract.*]` arrived exactly that way. "MAIN" WAS THE WORKING TREE. The differential compared against whatever branch the sibling clone sat on — #196's shape, grading this repo against unreviewed content. Reads origin/main now, which is what the ADR already claimed. Also: an unvalidated ref meant a blanked or branch-valued pin compared the registry to itself and passed forever; three exception types escaped a reader whose whole job is failing legibly; the reader's proof exercised one branch for the wrong reason (five branches now, against a purpose-built scratch repo); an emptied _CONSUMER_SELF_CHECK_ PENDING silently SKIPPED rather than prompting deletion. Nine prose claims corrected, including three repetitions of an [unmodelled] worked example that was simply wrong — it was never in the partition, so it would have been a red build, not the silent pass it was cited as. And an ADR table asserting machine enforcement that does not exist, in the document whose thesis is that unbacked claims go wrong unnoticed. Suite 410 passed / 1 skipped / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 37 +- reports/technical_risk_register.md | 4 +- tests/conftest.py | 9 +- tests/test_env_declaration.py | 356 ++++++++++++++---- tests/test_product.py | 45 ++- views_postprocessing/crafd/appwrite_env.py | 4 +- views_postprocessing/unfao/appwrite_env.py | 4 +- 7 files changed, 355 insertions(+), 104 deletions(-) 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 e764885..a67b249 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -41,9 +41,12 @@ the boundary and says so, rather than implying a coverage it does not have. Most tests here check our own code. A few check something different: whether a **claim this repository makes about a different repository** is still true. -The clearest example. Each partner's `appwrite_env.py` says, in effect, *"we were written -against edition 1.4.4 of the shared configuration registry."* That registry lives in -another repository. A test opens that repository and checks the edition is still 1.4.4. +The clearest example. Each partner's `appwrite_env.py` records which edition of the +shared configuration registry it was verified against, by naming a commit in another +repository. Tests open that repository at that commit and ask whether anything this +package reads has moved since — a rename, a reclassification, a rotated value, or a new +coordinate that has arrived. *(Until 2026-08-11 they asked a cruder question — whether the +edition **label** still matched — which fired on every upstream edit. See §7b's erratum.)* This is not hypothetical housekeeping. On 2026-08-05 that test failed — the registry had moved to a new edition while nobody here was looking. Two days earlier, the same family of @@ -110,12 +113,16 @@ Who is who, once and in one table: | repository | public? | fetched by CI? | why not, if not | |---|---|---|---| | **views-appwrite** | yes | **yes** | — carries the registry drift checks | -| **views-crafdapi** | yes | **yes**, temporarily | — retires when ADR-017 §7 lands | +| **views-crafdapi** | yes | **yes**, temporarily | — retires when views-crafdapi#53 lands, not before (ADR-017 §5 is per partner) | | **views-datafactory** | yes | no | its checks need raw data that is not in its git repository; fetching it turns an honest skip into a crash | | **views-faoapi** | **no** | no | private — no amount of workflow configuration reaches it. That case is [ADR-017](017_facts_across_a_private_boundary.md) | -The table is not decoration: it is `tests/conftest.py::SIBLINGS` in prose, and a test fails -if the two disagree. +**What of this table is enforced, and what is not.** `ci_checkout` is: the workflow and +`SIBLINGS` are compared by `tests/test_ci_sibling_coverage.py`, and disagreement fails. +The **`public?` column is not, and cannot be** — which is why `public` was deleted from the +`Sibling` record on 2026-08-10, after a review found nothing could verify it. It is here +because a reader needs it to follow the argument. Nothing reads this table; if it drifts +from `SIBLINGS`, only a human will notice. ### §5 CI downloads exactly what that list says, and a test enforces it @@ -239,10 +246,14 @@ So the two categories above become three: - **Drift tripwires** read the sibling's `main` because movement is the signal. - **Reachability checks** read a pinned commit; movement is noise. -- **Differential tripwires** read **both** — a pinned edition as the baseline, `main` as - the comparison — and fire only when something *we declare* differs between them. This is - the shape the registry check now has. It catches a rotation, which is invisible to the - other two, and it is silent through prose edits and version bumps. +- **Differential tripwires** read **both** — a pinned edition as the baseline, the + sibling's `main` as the comparison — and fire when something *we declare* differs between + them, or when a coordinate arrives in a table we read. This is the shape the registry + check now has. It catches a rotation, which is invisible to the other two, and it is + silent through prose edits and version bumps. *(Corrected 2026-08-11: the first + implementation compared against the sibling's **working tree**, so a developer whose + clone sat on a feature branch was grading this repository against unreviewed content — + #196's shape. It reads `origin/main` now.)* The third kind exists because the no-copy rule forbids writing expected coordinate values into this repository. A pinned edition is the only lawful place to keep a baseline for @@ -276,6 +287,12 @@ Until that declaration exists, the affected check runs on a maintainer's machine in CI. That is stated in the sibling's `note`, which rule G4 requires and which must name the record that owns it. +**Update 2026-08-11.** The declaration now exists (views-appwrite#75), views-faoapi +checks itself against it (their #379), and this repository's check reads the registry +rather than that repository's source. So the FAO half is no longer laptop-only, and no +credential was issued. What remains is CRAF'd's consumer-side check (views-crafdapi#53); +until it lands, that partner's source-read stays, and with it that partner's fetch. + --- ## Consequences diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 40bc65a..d3c72bf 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -170,7 +170,7 @@ that indexes only deleted code is noise. | Source | `falsify` against the SOLID / component-principle lens, 2026-08-11 | | Trigger | **Either.** (a) A second non-test consumer needs one of these declarations and has to copy it. (b) A fifth declaration is added to `tests/conftest.py` — the file is at four, and the threshold for "dumping ground" is not a number but the moment nobody can say in one sentence what the file is for. | | Owner | Whoever adds the next declaration, or the next non-test consumer. Not urgent; it gets more expensive slowly. | -| Location | `tests/conftest.py` (271 lines); `scripts/build_gaul_lookup.py:73`; ADR-016 §4. | +| Location | `tests/conftest.py` (grew again this week; `wc -l` for the number, which moves); `scripts/build_gaul_lookup.py:73`; ADR-016 §4. | `tests/conftest.py` is pytest's fixture file. It currently holds **four unrelated groups**: the package taxonomy (`PARTNER_PACKAGES`, `MACHINERY_PACKAGES`), the sibling repositories (`Sibling`, `SIBLINGS`, and three resolver functions), the consumer mapping (`CONSUMER_REPO`), and two git helpers (`git_output`, `commit_is_on_main`). None of those is a test fixture. They are declarations about the platform that happen to be consumed by tests. @@ -630,7 +630,7 @@ CI now checks that repository out and those seven run on every pull request — **What remains, and it is two different things:** -1. **One dark check.** `views-faoapi` is genuinely private — the consumer-name pin is still laptop-only. That is one test, not seven, and it is the one whose failure mode is invisible rather than loud. ADR-016 §8 defers the credential and names the trigger: FAO declining the request to make that repository public, or a second private sibling appearing. +1. **~~One dark check.~~ CLOSED 2026-08-11, and not by a credential.** This read: *"views-faoapi is genuinely private — the consumer-name pin is still laptop-only. That is one test, and it is the one whose failure mode is invisible rather than loud."* ADR-017 moved that check onto the public coordinate registry, so it runs in CI for every partner and needs no access to any private repository. The credential ADR-016 §8 deferred was never issued and is no longer the route. What remains of this half is CRAF'd's consumer-side check (views-crafdapi#53), which is the other repository's work, not a dark check here. 2. **The enforcement half is entirely untouched.** `protect_main`'s ref-name include-list was empty; it now targets the default branch, but **no status check is required**, so a pull request with a red CI can still be merged to `main`. Since merging to `main` *is* the production release, this is the half that matters most and the half that has not moved. **And it no longer stands alone — read C-86 before closing this.** Measured 2026-08-10: `protect_main` also lists **zero bypass actors**, so once a status check *is* required, nobody can merge past it, administrator or otherwise. Meanwhile ADR-016 made this repository's CI depend on two other repositories. Adding the required check therefore does two things at once: it closes this entry, and it makes an unbypassable external dependency live on the release path. Both are defensible; doing them in one unremarked step is not. If the escape is wanted, a bypass actor is the same console session. diff --git a/tests/conftest.py b/tests/conftest.py index 18c1d6f..839acaa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -149,10 +149,11 @@ class Sibling: "PRIVATE — the only one, and that is why it is not fetched. Checking it " "out needs a credential, which is an " "operator decision deferred pending a request to FAO to make the repository " - "public. One check is dark meanwhile — the consumer-name pin, whose failure " - "mode is a delivery nobody can find. ADR-017 decides what happens instead " - "of a credential: the fact moves to the public coordinate registry, and " - "neither side reads the other. See also register C-81." + "public. **Nothing is dark because of this any more** (2026-08-11): ADR-017 " + "moved the delivery-label check onto the public coordinate registry, which " + "needs no credential, so neither side reads the other. This entry stays " + "PRIVATE because the fact is still true, not because anything is blocked on " + "it. See ADR-017 and register C-81." ), ), "views-crafdapi": Sibling( diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index e9bcc7a..b96a944 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -24,6 +24,7 @@ """ import ast +import hashlib import logging import re import subprocess @@ -485,6 +486,10 @@ def _load_registry(repo: Path) -> dict: return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) +#: Sentinel for "this row carries no value", distinct from any string a registry +#: could legitimately hold. +_ABSENT = object() + #: Every top-level table the registry may carry, and what this repository does with it. #: #: **Declared, and asserted against the live registry** — a table appearing upstream that @@ -504,14 +509,59 @@ def _load_registry(repo: Path) -> dict: "meta": "METADATA", # the edition and its amendment log } -#: Tables whose absence is our problem. An IGNORED table vanishing is not — that -#: direction is deliberately unchecked, which is why v1.5.1 dropping `[unmodelled]` is +#: 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. +#: 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. +_ROLES = ("CONSUMED", "MIRRORED", "IGNORED", "METADATA") + +#: Tables this package reads rows out of. **One source, three consumers** — the class +#: check, the drift projection and the no-copy scan all derive from here rather than each +#: keeping a copy of the tuple. They did keep copies, which made classifying a table as +#: CONSUMED add exactly zero coverage while satisfying the guard that demanded it — the +#: guard's own remediation advice producing register C-74's shape. +_CONSUMED_TABLES = tuple(n for n, role in _TABLE_ROLE.items() if role == "CONSUMED") + _TABLES_WE_DEPEND_ON = tuple( name for name, role in _TABLE_ROLE.items() if role in ("CONSUMED", "MIRRORED") ) +def _unclassified_tables(registry: dict) -> list[str]: + """Tables upstream that nobody here has classified. THE production predicate. + + Extracted so the proof can call it instead of re-typing it. The previous proof + re-typed the set arithmetic, so neutering this to ``return []`` left it green — a + detector for the detector that detected nothing. + """ + return sorted(set(registry) - set(_TABLE_ROLE)) + + +def _missing_dependencies(registry: dict) -> list[str]: + """Tables we depend on that are absent upstream. THE production predicate.""" + return sorted(n for n in _TABLES_WE_DEPEND_ON if n not in registry) + + +def _rows(registry: dict, sections: tuple) -> dict[str, tuple]: + """``name -> (section, class, value-or-absent)`` across ``sections``. + + The single reader the class check, the drift projection and the no-copy scan share. + Two hand-copied versions had already diverged on null handling: one used + ``registry.get(s, {})`` and raised ``AttributeError`` on a null section, the other + used ``(registry.get(s) or {})`` and 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() + } + + class _RegistryReadError(RuntimeError): """The registry could not be read at a pinned commit — never a silent empty dict.""" @@ -530,6 +580,37 @@ def _registry_at(repo: Path, ref: str) -> dict: holding four unrelated groups, and its trigger is a fifth. There is one consumer. """ tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") + + # The ref must name a FROZEN commit, and must be the one we asked for. + # + # Without this, `git show ":"` reads the INDEX and `git show "main:"` + # reads a branch — both exit 0, both parse, both pass every anchor below. A pin + # blanked by a bad edit or set to "main" by someone who thinks that is what a pin + # means would make the differential compare the current registry against itself and + # report green against every future edition. That is the same silent-baseline trap + # this reader exists to close, wearing a different hat. + # + # ``^{commit}`` also peels an annotated TAG. That matters here: a tag object is not + # a commit, is reachable only through refs/tags, and — measured 2026-08-11 — yields + # a 404 from the pinned blob URL these modules publish. So this refuses one. + resolved = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if resolved.returncode != 0 or not resolved.stdout.strip(): + 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, a branch name reads a moving tip, and " + "either would make the comparison below compare the registry to itself." + ) + if ref not in ("origin/main", "main") and not resolved.stdout.strip().startswith(ref): + raise _RegistryReadError( + f"{ref!r} resolves to commit {resolved.stdout.strip()[:12]}, which does not " + f"start with it. That happens when the pin names an annotated TAG rather than " + "a commit — git peels it silently, every check here passes, and the blob URL " + "these modules publish returns 404. Pin the commit." + ) + result = subprocess.run( ["git", "-C", str(repo), "show", f"{ref}:{_REGISTRY_RELPATH.as_posix()}"], capture_output=True, text=True, check=False, timeout=30, @@ -545,7 +626,15 @@ def _registry_at(repo: Path, ref: str) -> dict: f"the registry is EMPTY at {ref!r} in {repo}. Treating that as a registry " "with no coordinates would make every comparison below pass." ) - registry = tomllib.loads(result.stdout) + try: + registry = tomllib.loads(result.stdout) + except Exception as exc: # TOMLDecodeError, and whatever a non-TOML blob raises + raise _RegistryReadError( + f"the registry at {ref!r} in {repo} did not parse as TOML: {exc}. A raw " + "traceback from a helper whose justification is failing legibly would be its " + "own defeat — this happens when the path names a directory, or the blob is " + "not the file we think it is." + ) from exc if not registry.get("meta", {}).get("version") or not registry.get("connection"): raise _RegistryReadError( f"the registry at {ref!r} parsed but carries no meta.version or no " @@ -568,21 +657,70 @@ def _projection(registry: dict, names: set[str]) -> dict[str, tuple]: writing the expected values into this repository, so the pinned edition is the only lawful place a baseline for that comparison can live. """ + return {n: row for n, row in _rows(registry, _CONSUMED_TABLES).items() if n in names} + + +def _registry_current(repo: Path) -> dict: + """The registry as the sibling's ``main`` has it — not its working tree. + + A sibling clone normally sits on whatever branch its own agent last worked on. + Comparing against that grades us on unreviewed content, or reddens on a branch nobody + merged — #196 verbatim, the case that cost this platform a withdrawn pull request. + The ``ref: main`` discipline lived only inside the CI workflow, so a developer running + the suite locally was comparing against something else entirely, and ADR-016's erratum + claimed otherwise. + """ + for ref in ("origin/main", "main"): + try: + return _registry_at(repo, ref) + except _RegistryReadError: + continue + raise _RegistryReadError( + f"neither origin/main nor main resolves in {repo}. The comparison is against that " + "repository's ratified content, so there is nothing to compare against — run " + "`git fetch` there." + ) + + +def _describe_changes(then: dict, now: dict, names: set[str]) -> dict[str, str]: + """What differs, said WITHOUT printing a coordinate value. + + This repository is public and its CI logs are world-readable. The first version of + this comparison interpolated the projection tuples straight into the assertion + message — so the single event this check exists to fire on, a rotated coordinate, + would have printed the old and new values into a public log. Longer than the + six-character threshold the no-copy check itself uses to define a leak, and the + outcome ``tests/test_redaction_guard.py`` exists to prevent. + + So: name the field that moved, and for a value give a short digest — enough to see + that two editions disagree and to match against a rotation you performed, never + enough to be the value. + """ + def digest(value) -> str: + if value is _ABSENT: + return "" + return f"" + out = {} - for section in ("connection", "target", "secret"): - for name, body in (registry.get(section) or {}).items(): - if name in names: - out[name] = (section, body.get("class"), body.get("value", "")) + for name in sorted(names): + was, is_ = then.get(name), now.get(name) + if was == is_: + continue + if was is None or is_ is None: + out[name] = "absent at the pin" if was is None else "removed upstream" + continue + fields = [ + f"{label}: {a!r} -> {b!r}" if label != "value" else f"value: {digest(a)} -> {digest(b)}" + for label, a, b in zip(("section", "class", "value"), was, is_) + if a != b + ] + out[name] = "; ".join(fields) return out def _declared_classes(registry: dict) -> dict[str, str]: """name -> the class the registry DECLARES for it (never inferred from the name).""" - return { - name: body.get("class") - for section in ("connection", "target", "secret") - for name, body in registry.get(section, {}).items() - } + return {n: row[1] for n, row in _rows(registry, _CONSUMED_TABLES).items()} @pytest.mark.parametrize("partner", _PARTNERS) @@ -622,15 +760,14 @@ def test_the_pinned_commit_declares_the_pinned_version(partner): Verified 2026-08-11: ``fcf32c9`` does declare ``1.4.4`` — true by luck, not by check. - Both sides of this comparison are frozen, so it is the one registry check in this - file that upstream activity can never redden. It fails only when *we* are wrong. + Both sides of the *comparison* are frozen, so upstream editing the registry cannot + change its verdict. It still needs the sibling checkout to read the pinned blob, so a + missing or degraded clone reddens it — the check that runs with no sibling at all is + ``test_the_docstring_states_the_same_edition_the_constants_declare``. """ repo = require_sibling("views-appwrite") module = _PARTNER_ENV[partner][0] - try: - pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) - except _RegistryReadError as exc: - pytest.fail(str(exc)) + pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) assert pinned["meta"]["version"] == module.SEAM_CONTRACT_VERSION, ( f"[{partner}] the pin is internally inconsistent: commit " f"{module.SEAM_CONTRACT_COMMIT} declares registry v{pinned['meta']['version']}, " @@ -656,14 +793,18 @@ def test_every_table_in_the_registry_is_classified_here(): **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 - why v1.5.1 dropping ``[unmodelled]`` is silent while v1.5.0 adding ``[contract]`` is - a red build with something to do. Two events in the registry's life, both of which - this repository needed to see. + 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 + repository needed to see both. + + *(An earlier draft illustrated the silent case with v1.5.1's removal of + ``[unmodelled]``. That was wrong — ``unmodelled`` was never in the partition, so it + would have been a red build demanding classification, not a silent pass.)* """ repo = require_sibling("views-appwrite") registry = _load_registry(repo) - unclassified = sorted(set(registry) - set(_TABLE_ROLE)) + unclassified = _unclassified_tables(registry) assert not unclassified, ( f"the registry has table(s) this repository has never classified: " f"{unclassified}. Decide what each one is — CONSUMED (we read it), MIRRORED (its " @@ -671,7 +812,7 @@ def test_every_table_in_the_registry_is_classified_here(): "_TABLE_ROLE. A new table is how an obligation arrives; `[contract.*]` arrived " "exactly this way and only a noisy version check happened to catch it." ) - vanished = sorted(n for n in _TABLES_WE_DEPEND_ON if n not in registry) + vanished = _missing_dependencies(registry) assert not vanished, ( f"table(s) this repository depends on are gone from the registry: {vanished}. " "Either they were retired upstream and this package must stop reading them, or " @@ -701,19 +842,26 @@ def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): """ repo = require_sibling("views-appwrite") module, _, expected_class = _PARTNER_ENV[partner] - try: - pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) - except _RegistryReadError as exc: - pytest.fail(str(exc)) + pinned = _registry_at(repo, module.SEAM_CONTRACT_COMMIT) + current = _registry_current(repo) names = set(expected_class) - then, now = _projection(pinned, names), _projection(_load_registry(repo), names) + then, now = _projection(pinned, names), _projection(current, names) - changed = { - name: {"at the pin": then.get(name, ""), "now": now.get(name, "")} - for name in sorted(names) - if then.get(name) != now.get(name) - } + arrived = sorted( + set(_rows(current, _TABLES_WE_DEPEND_ON)) - set(_rows(pinned, _TABLES_WE_DEPEND_ON)) + ) + assert not arrived, ( + f"[{partner}] the registry gained coordinate(s) {arrived} in a table this package " + f"reads, since the edition it was verified against " + f"(v{module.SEAM_CONTRACT_VERSION}). Decide whether this package must adopt them " + "— that is a human read, which is why this reports rather than guesses — then " + "move SEAM_CONTRACT_VERSION and SEAM_CONTRACT_COMMIT together. This is the half " + "of the deleted edition check that was worth keeping: `[contract.*]` arrived " + "exactly this way, and nothing else here would have seen it." + ) + + changed = _describe_changes(then, now, names) assert not changed, ( f"[{partner}] the registry changed something this package reads, since the " f"edition it was verified against (v{module.SEAM_CONTRACT_VERSION}, commit " @@ -776,80 +924,138 @@ def test_the_pinned_commit_is_reachable_from_the_contract_repos_main(partner): ) -def test_the_pinned_reader_refuses_rather_than_returning_an_empty_registry(): - """The trap this reader exists to avoid, proven rather than asserted in prose. +def test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong(tmp_path): + """Each refusal branch, against a scratch repository built to trigger it. - ``conftest.git_output`` turns any git failure into ``""``, and ``tomllib.loads("")`` - is ``{}``. Had this reader been built on it, an unreadable pin would have produced an - empty baseline, an empty projection, and a comparison that passes forever — the - vanished-directory shape ADR-014 §2 exists for, rebuilt inside the fix for a drift - detector. + The first version of this test used a bogus sha against *this* repository — where the + registry path does not exist at any ref, so a nonsense sha and ``HEAD`` failed + identically. It proved the helper rejects *something*, not that it rejects an + unreadable **pin**, and deleting the empty-file and anchor branches left it green. - Runs everywhere: it needs a git repository, not the sibling, and this repository is - one. + Every branch here is genuinely reachable, and each is the shape of a real accident: + a pin blanked by a bad edit, a pin pointing at a branch, a pin naming an annotated + tag, a file emptied upstream, a file that parses but is not the registry. """ - with pytest.raises(_RegistryReadError, match="cannot read the registry"): - _registry_at(_REPO, "0000000000000000000000000000000000000000") + import subprocess as sp + + def git(*args): + return sp.run(["git", "-C", str(tmp_path), *args], capture_output=True, text=True, check=True) + + git("init", "-q") + git("config", "user.email", "t@t") + git("config", "user.name", "t") + target = tmp_path / _REGISTRY_RELPATH + target.parent.mkdir(parents=True) + + def commit(text: str, message: str) -> str: + target.write_text(text) + git("add", "-A") + git("commit", "-q", "-m", message) + return git("rev-parse", "--short", "HEAD").stdout.strip() + + good = commit('[meta]\nversion = "9.9.9"\n\n[connection.X]\nclass = "connection"\n', "good") + empty = commit("", "empty") + anchorless = commit('[target.X]\nclass = "target"\n', "no meta, no connection") + git("tag", "-a", "v1", "-m", "annotated") + + assert _registry_at(tmp_path, good)["meta"]["version"] == "9.9.9", ( + "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"): + _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 + with pytest.raises(_RegistryReadError, match="is EMPTY"): + _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 + + +def test_the_role_vocabulary_is_closed(): + """A typo in a role string used to silently narrow a security scan. + + ``_TABLE_ROLE``'s values drive which sections the no-copy check reads. Mistyping + ``"CONSUMED"`` dropped `target` from that scan — twelve coordinate values to two — + with nothing objecting, while also narrowing what counts as a dependency. Roles are + now a closed set, asserted. + """ + unknown = sorted({r for r in _TABLE_ROLE.values() if r not in _ROLES}) + assert not unknown, ( + f"unknown role(s) in _TABLE_ROLE: {unknown}. Roles drive which sections the " + f"no-copy scan reads; a typo here silently narrows it. Use one of {_ROLES}." + ) + assert _CONSUMED_TABLES, "no table is classified CONSUMED — every row check reads nothing" def test_the_table_partition_would_catch_a_new_table_and_a_vanished_one(): - """Both directions, against synthetic registries — no sibling needed. + """Calls the production predicates, so neutering them fails here. - The asymmetry is the design: an *unclassified* table fails (an obligation may have - arrived), a *missing IGNORED* table does not (not our business). Getting that - backwards would either miss the `[contract.*]` case or cry wolf on v1.5.1's removal - of `[unmodelled]`, and both have actually happened upstream. + The first version of this test re-typed the set arithmetic inline. Mutation-proven + afterwards: replacing the real computations with ``[]`` left the guard accepting any + upstream registry forever, and this proof still passed. It detected nothing, which is + ADR-014 §2 exactly — and it was the ONLY thing claiming the table check bites. + + The asymmetry is the design and is asserted in both directions: an unclassified table + fires, a vanished IGNORED table does not. """ base = {name: {} for name in _TABLE_ROLE} - arrived = dict(base) | {"brand_new_table": {}} - assert sorted(set(arrived) - set(_TABLE_ROLE)) == ["brand_new_table"], ( + 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" - lost_consumed = {k: v for k, v in base.items() if k != "target"} - assert [n for n in _TABLES_WE_DEPEND_ON if n not in lost_consumed] == ["target"], ( - "a table this package reads vanished and the check did not object" + 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" ) - - lost_ignored = {k: v for k, v in base.items() if k != "excluded"} - assert not [n for n in _TABLES_WE_DEPEND_ON if n not in lost_ignored], ( - "an IGNORED table disappearing must NOT fire — v1.5.1 removed `[unmodelled]` " - "and that carried no obligation for anyone here (ADR-014 §3: prefer the false " + assert not _missing_dependencies({k: v for k, v in base.items() if k != "excluded"}), ( + "an IGNORED table disappearing must NOT fire (ADR-014 §3: prefer the false " "negative to the false alarm)" ) @pytest.mark.parametrize("partner", _PARTNERS) def test_the_drift_check_would_catch_a_rotation_that_names_and_classes_cannot(partner): - """Rotation is the whole reason the projection carries ``value``. + """Rotation is why the projection carries ``value`` — proven through the real code. - A rotated bucket id keeps its name and its class, so - ``test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_as`` - passes while every delivery goes somewhere else. This proves the differential sees - what that check structurally cannot — and proves it without a sibling checkout, so - it runs on every machine rather than only where views-appwrite happens to be. + The first version of this test asserted ``then != now`` on dicts it built itself. + Mutation-proven afterwards: making the production comparison ignore ``value`` — which + is *literally* "names and classes cannot see a rotation", the thing this test exists + to disprove — left both the proof and the check green, while a rotated bucket id + shipped every delivery to the wrong place. So it now calls ``_describe_changes``, the + same function the check calls. - No real coordinate value appears here: both editions are synthetic, and the point is - that the two *differ*, not what either says. + Runs without a sibling, so it holds on every machine rather than only where + views-appwrite happens to be checked out. """ _, own_store, expected_class = _PARTNER_ENV[partner] canary = own_store[0] names = set(expected_class) def edition(value: str) -> dict: - return {"target": {canary: {"class": "target", "value": value}}} + return { + "meta": {"version": "0.0.0-fixture"}, + "connection": {"APPWRITE_ENDPOINT": {"class": "connection", "value": "e"}}, + "target": {canary: {"class": "target", "value": value}}, + } - then = _projection(edition("value-at-the-pin"), names) - now = _projection(edition("value-after-rotation"), names) + then = _projection(edition("at-the-pin"), names) + now = _projection(edition("after-rotation"), names) assert then[canary][:2] == now[canary][:2], ( - "this fixture is meant to change ONLY the value — if the name or class differs " - "it is not proving what it claims" + "this fixture must differ ONLY in the value, or it is not proving what it claims" + ) + changed = _describe_changes(then, now, names) + assert canary in changed, ( + f"[{partner}] a rotated value went unnoticed. Name and class are identical on " + "both sides, so nothing else in this file can see it." ) - assert then != now, ( - f"[{partner}] a rotated value went unnoticed. Names and classes are identical " - "on both sides, so nothing else in this file can see it." + assert "value:" in changed[canary] and "at-the-pin" not in changed[canary], ( + "the report must name the field that moved and must NOT print the value — this " + "repository is public and its CI logs are world-readable" ) diff --git a/tests/test_product.py b/tests/test_product.py index aa2281f..5e8440c 100644 --- a/tests/test_product.py +++ b/tests/test_product.py @@ -41,10 +41,13 @@ #: ADR-013 §4.1a, the defect that stranded six `orange_ensemble` documents in #: `unfao_bucket` while FAO's forecast serving read empty for months. #: -#: ``test_the_declared_consumer_name_still_matches_the_consumer`` closes that half -#: against the sibling checkout, following the pattern -#: ``tests/test_env_declaration.py`` already uses for the coordinate registry: declared -#: locally, verified across the seam when the other repo is on disk. +#: ``test_the_declared_consumer_name_matches_the_registry`` closes that half against the +#: **public coordinate registry** rather than the consumer's source (ADR-017 §5). It runs +#: for every partner and needs no credential, because the registry is public even when the +#: consumer is not — which is why it reaches the FAO partner and its predecessor could not. +#: +#: One partner is still ALSO read at the source, and only one: +#: ``_CONSUMER_SELF_CHECK_PENDING`` below names it and the issue that retires it. _CONSUMER_DOCUMENT_NAME = { "unfao": "un_fao", "crafd": "un_crafd", @@ -229,9 +232,12 @@ def test_the_declared_consumer_name_matches_the_registry(partner): had moved. ADR-017 §7: we were never entitled to depend on another repository's file layout. - Now both sides read one public declaration and neither reads the other. The registry - lives in views-appwrite, which is public, so this needs no credential even for the - private partner — that is the whole of ADR-017 §5 in one assertion. + Now both sides read one public declaration. The registry lives in views-appwrite, + which is public, so this needs no credential even for the private partner — that is + the whole of ADR-017 §5 in one assertion. + + *"Neither reads the other" is the end state, not yet the present one:* CRAF'd's source + is still read by the check below, until views-crafdapi#53 lands. The value it guards is the one whose failure is silent: a delivery filed under a name the consumer does not ask for is uploaded, stored, billed, and invisible — no error @@ -273,10 +279,31 @@ def test_the_declared_consumer_name_matches_the_registry(partner): } -def test_the_pending_list_names_only_real_partners(): - """A stale name here would keep a retired mechanism alive against nothing.""" +def test_the_pending_list_is_not_empty_and_names_only_real_partners(): + """An empty map must be a prompt to delete code, not a silent skip. + + ``parametrize`` over an empty collection is a **skip** under pytest's default + ``empty_parameter_set_mark``, and this repository declares no pytest configuration at + all. So emptying this map — by a premature cleanup, or an edit made before + views-crafdapi#53 lands — would retire the only check that reads what a consumer + actually does, and report ``1 skipped`` on a green build. The registry check would + then be comparing two values this platform authored to each other, which is the exact + failure the retained test's docstring warns about. + + So the map is asserted non-empty. When it legitimately empties, this assertion is + what tells you to delete the test, its regexes, and the sibling fetch that serves it. + """ unknown = sorted(set(_CONSUMER_SELF_CHECK_PENDING) - set(PARTNER_PACKAGES)) assert not unknown, f"not partners: {unknown}" + assert _CONSUMER_SELF_CHECK_PENDING, ( + "_CONSUMER_SELF_CHECK_PENDING is empty, which means every consumer now checks " + "itself against the registry. That is the end state ADR-017 §5 describes — so " + "finish it: delete test_the_consumer_still_filters_on_the_name_until_it_checks_" + "itself, the _CONSUMER_PATH_MANAGER and _CONSUMER_FILTER patterns, and the " + "views-crafdapi entry in tests/conftest.py::SIBLINGS whose note says that fetch " + "serves exactly this one test. Leaving them costs a clone on every pull request " + "and asserts nothing." + ) @pytest.mark.parametrize("partner", sorted(_CONSUMER_SELF_CHECK_PENDING)) diff --git a/views_postprocessing/crafd/appwrite_env.py b/views_postprocessing/crafd/appwrite_env.py index 9a379c6..fcf2917 100644 --- a/views_postprocessing/crafd/appwrite_env.py +++ b/views_postprocessing/crafd/appwrite_env.py @@ -6,7 +6,7 @@ views-appwrite and referenced by pinned URL, never copied — copies were the platform's original failure (þing-01 sáttmál S6): - https://github.com/views-platform/views-appwrite/blob/b703cab/docs/ADRs/platform/coordinate_registry.toml + https://github.com/views-platform/views-appwrite/blob/c7b597e/docs/ADRs/platform/coordinate_registry.toml That pin is registry **v1.5.2** — declared below as ``SEAM_CONTRACT_VERSION`` / ``SEAM_CONTRACT_COMMIT`` so the pin is a value a test can check rather than a fact @@ -48,7 +48,7 @@ #: enforces the pair — for this package and for ``unfao`` alike — against a local #: views-appwrite checkout when one is present. SEAM_CONTRACT_VERSION = "1.5.2" -SEAM_CONTRACT_COMMIT = "b703cab" +SEAM_CONTRACT_COMMIT = "c7b597e" CONNECTION_ENV = ( "APPWRITE_ENDPOINT", diff --git a/views_postprocessing/unfao/appwrite_env.py b/views_postprocessing/unfao/appwrite_env.py index b13fade..3e7122d 100644 --- a/views_postprocessing/unfao/appwrite_env.py +++ b/views_postprocessing/unfao/appwrite_env.py @@ -5,7 +5,7 @@ views-appwrite and referenced by pinned URL, never copied — copies were the platform's original failure (þing-01 sáttmál S6): - https://github.com/views-platform/views-appwrite/blob/b703cab/docs/ADRs/platform/coordinate_registry.toml + https://github.com/views-platform/views-appwrite/blob/c7b597e/docs/ADRs/platform/coordinate_registry.toml That pin is registry **v1.5.2** — declared below as ``SEAM_CONTRACT_VERSION`` / ``SEAM_CONTRACT_COMMIT`` so the pin is a value a test can @@ -45,7 +45,7 @@ #: declaration against that edition of the registry. ``tests/test_env_declaration.py`` #: enforces the pair against a local views-appwrite checkout when one is present. SEAM_CONTRACT_VERSION = "1.5.2" -SEAM_CONTRACT_COMMIT = "b703cab" +SEAM_CONTRACT_COMMIT = "c7b597e" CONNECTION_ENV = ( "APPWRITE_ENDPOINT", From 3d5a24433557dd61967de523ae90c29dd8f7acd8 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 16:27:57 +0200 Subject: [PATCH 7/9] =?UTF-8?q?fix(tests):=20four=20findings=20from=20/rev?= =?UTF-8?q?iew-diff=20=E2=80=94=20the=20sixteenth,=20and=20three=20more?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 4 of the loop, aimed at the remediation commit itself, because fixing fifteen things at once is when the next one gets introduced. It did. THE BYPASS REOPENED THE HOLE IT WAS ADDED BESIDE. The frozen-commit guard existed because a pin set to "" reads the git index and a pin set to "main" reads a moving tip — either makes the differential compare the registry to itself and pass forever. Then I exempted "main"/"origin/main" from that guard so _registry_current could call _registry_at, which meant a pin literally set to "main" was accepted. Verified before and after. _registry_current now resolves the branch to a SHA first, so the guard has no exceptions. I FIXED THE WORKING-TREE PROBLEM IN ONE PLACE OUT OF FOUR. The differential moved onto origin/main; the names-and-classes check, the table partition and the no-copy scan were all still reading whatever branch the sibling clone happened to sit on — which is #196's shape, the case that cost this platform a withdrawn PR. All four read origin/main now, and _load_registry, which read the working tree, is deleted rather than left as a loaded gun. Four registry readers become three. A REAL DIAGNOSIS WAS SWALLOWED. _registry_current caught _RegistryReadError per ref and then raised its own, so an anchorless or corrupt registry ON MAIN was reported as "neither origin/main nor main resolves" — a message pointing at the wrong problem. Read errors now propagate; only genuine ref-resolution failure produces that text. AND THE PROSE WAS ONE FIX AHEAD OF THE CODE. ADR-016 §7b's erratum said differential tripwires read the sibling's main. That was true of one check and false of three at the moment I wrote it. Corrected, and the correction records its own overclaim, because a sentence written to describe a fix that outran the fix is the same defect §3 diagnoses. Also caught in passing: my first attempt at deleting the dead reader cut from its `def` to the next `def`, swallowing the _TABLE_ROLE block. The suite caught it; the file was restored and the removal redone against the function's own bounds. Suite 410 passed / 1 skipped / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 13 +++-- tests/test_env_declaration.py | 49 ++++++++++--------- 2 files changed, 36 insertions(+), 26 deletions(-) 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 a67b249..ddd6024 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -250,10 +250,15 @@ So the two categories above become three: sibling's `main` as the comparison — and fire when something *we declare* differs between them, or when a coordinate arrives in a table we read. This is the shape the registry check now has. It catches a rotation, which is invisible to the other two, and it is - silent through prose edits and version bumps. *(Corrected 2026-08-11: the first - implementation compared against the sibling's **working tree**, so a developer whose - clone sat on a feature branch was grading this repository against unreviewed content — - #196's shape. It reads `origin/main` now.)* + silent through prose edits and version bumps. + +*(Corrected twice on 2026-08-11, and the second time is the instructive one. The first +implementation compared against the sibling's **working tree** — so a developer whose clone +sat on a feature branch graded this repository against unreviewed content, which is #196's +shape. The first correction moved **one** check onto `origin/main` and this paragraph then +claimed all of them did; three others were still reading the working tree. All four read +`origin/main` now. A sentence written to describe a fix, one fix ahead of the code, is the +same defect §3 diagnoses.)* The third kind exists because the no-copy rule forbids writing expected coordinate values into this repository. A pinned edition is the only lawful place to keep a baseline for diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index b96a944..eb30777 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -477,15 +477,6 @@ def test_every_declared_name_is_classified_here(partner): ) -def _load_registry(repo: Path) -> dict: - tomllib = pytest.importorskip( - "tomllib", - reason="tomllib is stdlib from Python 3.11; pyproject declares >=3.11, so a " - "conforming environment has it. CI runs 3.11.", - ) - return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) - - #: Sentinel for "this row carries no value", distinct from any string a registry #: could legitimately hold. _ABSENT = object() @@ -603,7 +594,7 @@ def _registry_at(repo: Path, ref: str) -> dict: "commit: an empty ref reads the index, a branch name reads a moving tip, and " "either would make the comparison below compare the registry to itself." ) - if ref not in ("origin/main", "main") and not resolved.stdout.strip().startswith(ref): + if not resolved.stdout.strip().startswith(ref): raise _RegistryReadError( f"{ref!r} resolves to commit {resolved.stdout.strip()[:12]}, which does not " f"start with it. That happens when the pin names an annotated TAG rather than " @@ -670,16 +661,30 @@ def _registry_current(repo: Path) -> dict: the suite locally was comparing against something else entirely, and ADR-016's erratum claimed otherwise. """ + head = "" for ref in ("origin/main", "main"): - try: - return _registry_at(repo, ref) - except _RegistryReadError: - continue - raise _RegistryReadError( - f"neither origin/main nor main resolves in {repo}. The comparison is against that " - "repository's ratified content, so there is nothing to compare against — run " - "`git fetch` there." - ) + resolved = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if resolved.returncode == 0 and resolved.stdout.strip(): + head = resolved.stdout.strip() + break + if not head: + raise _RegistryReadError( + f"neither origin/main nor main resolves in {repo}. The comparison is against " + "that repository's ratified content, so there is nothing to compare against — " + "run `git fetch` there." + ) + # Resolved to a SHA before reading, deliberately. `_registry_at` refuses any ref that + # is not a frozen commit, and that guard must have no exceptions: an earlier version + # exempted "main"/"origin/main" so this function could call it — which meant a pin + # literally set to ``"main"`` was accepted, reopening the hole the guard exists for. + # Resolving here keeps the guard absolute at the cost of one extra rev-parse. + # + # Read errors propagate unchanged: a registry that is corrupt or anchorless ON MAIN + # must say so, not be reported as "neither ref resolves". + return _registry_at(repo, head) def _describe_changes(then: dict, now: dict, names: set[str]) -> dict[str, str]: @@ -727,7 +732,7 @@ def _declared_classes(registry: dict) -> dict[str, str]: def test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_as(partner): """C-57: a rename or reclassification upstream must not be silent here.""" repo = require_sibling("views-appwrite") - declared = _declared_classes(_load_registry(repo)) + declared = _declared_classes(_registry_current(repo)) _, _, expected_class = _PARTNER_ENV[partner] missing = sorted(n for n in expected_class if n not in declared) @@ -802,7 +807,7 @@ def test_every_table_in_the_registry_is_classified_here(): would have been a red build demanding classification, not a silent pass.)* """ repo = require_sibling("views-appwrite") - registry = _load_registry(repo) + registry = _registry_current(repo) unclassified = _unclassified_tables(registry) assert not unclassified, ( @@ -1143,7 +1148,7 @@ def test_no_coordinate_value_is_copied_into_this_repo(): repo = sibling_repo("views-appwrite") if repo is None: pytest.skip("views-appwrite checkout not found — set VIEWS_APPWRITE") - registry = _load_registry(repo) + registry = _registry_current(repo) # Scoped by the declared partition, NOT by an inline tuple — so a new table cannot # be swept in by a one-word edit, and `contract` cannot be swept in at all. # From 89fc5552a6fa536caac3a0fd2d904fec5caf9f01 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 17:05:28 +0200 Subject: [PATCH 8/9] =?UTF-8?q?fix(tests):=20iteration=202=20=E2=80=94=20a?= =?UTF-8?q?=20scan=20blind=20in=20its=20own=20house=20style,=20and=20one?= =?UTF-8?q?=20shared=20reader?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loop iteration 2. Five findings plus one of mine, and the first is the serious one. THE NO-COPY SCAN WAS BLIND IN THE STYLE OF THE DOCUMENT IT WAS WRITTEN FOR. Proven by injection: real registry values written into README.md as `NAME=value # comment` passed the guard, because `(.+?)\s*$` swallowed the comment into the captured value and .strip('"\'') could not recover it. README's own Configuration block is written in exactly that form. On a PUBLIC repository that is the guard's whole purpose defeated by its own house style. Now: quoted values matched whole (a `#` inside quotes is legitimate), unquoted values truncated at a comment. The `len > 6` floor is gone too — it excluded two five-character coordinates and, measured, removing it keeps the suite green, so it was narrowing a security check for nothing. THE FOUNDING INCIDENT REAPPEARED IN THE FOUNDING FILE. run_pytest.yml still said a tokenless checkout "is what verifies the `public` field — no test does". `public` was deleted on 2026-08-10 because nothing could verify it, and ADR-016 §4 — added by THIS branch — says so explicitly. A comment in that file asserting an unverifiable fact about another repository is ADR-016 §3's founding incident, recurring in the file it happened in. C-87 WATCHED THE WRONG ISSUE. Its trigger, body and cross-refs still named views-crafdapi#39 while everything else moved to #53. Closing #39 would have fired it spuriously; #53 landing would not have cleared it. Mine, found while checking their claims: test_product.py was still reading the sibling's WORKING TREE. I moved four checks onto origin/main last iteration and left the fifth — the same partial-fix pattern review-diff had just caught. That is the second copy of the same rule disagreeing with itself, which is this repo's stated trigger for extracting, so the reader now lives in tests/seam_registry.py: one module, named for what it does, out of conftest (C-88), with every refusal branch and the reason each exists. Also: the mutant assertion hardcoded "views-appwrite" under a docstring framing it generically — correct only by coincidence of today's fixtures; and two register blocks were appended straight after a `Cross-refs:` line, rendering as run-on paragraphs below the line every other entry treats as terminal. AND THE PARTITION EARNED ITS KEEP ON LIVE DATA. Mid-fix it went red: views-appwrite had added an `[edition]` table at v1.6.0 — which is views-appwrite#76 delivered, each edition marked `obliges_consumers = true|false`. Exactly the mechanism ADR-016 §7a described and I filed for. The guard caught its arrival unprompted, on its first real encounter, and the row-level drift check stayed correctly silent because nothing we read moved. Classified IGNORED for now; adopting it is recorded as the follow-up, not folded into a remediation. Suite 410 passed / 1 skipped / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/run_pytest.yml | 11 +- reports/technical_risk_register.md | 8 +- tests/seam_registry.py | 153 +++++++++++++++++++++++++ tests/test_ci_sibling_coverage.py | 13 ++- tests/test_env_declaration.py | 175 ++++++----------------------- tests/test_product.py | 23 ++-- 6 files changed, 219 insertions(+), 164 deletions(-) create mode 100644 tests/seam_registry.py diff --git a/.github/workflows/run_pytest.yml b/.github/workflows/run_pytest.yml index b548505..94773d6 100644 --- a/.github/workflows/run_pytest.yml +++ b/.github/workflows/run_pytest.yml @@ -51,9 +51,14 @@ jobs: # someone else's test suite the first time a sibling landed outside it. # # No `token:` on any step below, and that is load-bearing rather than an omission: - # the default GITHUB_TOKEN is scoped to THIS repository, so a tokenless checkout of - # a repo declared public FAILS if it is actually private. That is what verifies the - # `public` field — no test does, because nothing in this suite touches the network. + # the default GITHUB_TOKEN is scoped to THIS repository, so a tokenless checkout of a + # private repository simply fails, loudly, at this step. + # + # An earlier version of this comment said that "verifies the `public` field". There is + # no such field: `public` was deleted from `SIBLINGS` on 2026-08-10 because nothing + # could verify it, and ADR-016 §4 now says so explicitly. A comment in THIS file + # asserting a fact about another repository that nothing checks is the founding + # incident of ADR-016 §3, and it had reappeared here. - name: Checkout views-appwrite (sibling) uses: actions/checkout@v3 with: diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index d3c72bf..6aa7687 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -210,6 +210,7 @@ ADR-016 has CI check out `views-appwrite` and `views-crafdapi` so that cross-rep **What would resolve it, in order of preference:** views-appwrite#76 makes the obligation-carrying distinction machine-readable, so observation-only bumps stop firing the check at all — filed, and that seat volunteered it. Failing that, a bypass actor restores the escape. Failing both, the coupling stands as ADR-016 §7 describes, which is defensible but should be chosen rather than discovered. Cross-refs: **C-81** (the enforcement half, whose fix activates this), **C-46** (whose recommendation against per-PR sibling checkouts this overrode, with the reasoning recorded there), ADR-016 §7/§7a/§7b, views-appwrite#76. + **Partial mitigation 2026-08-11 — the false-alarm rate, not the coupling.** The check that kept firing compared registry *version strings*. It has been replaced by three that match on the facts this repository actually declares: the pinned commit must declare the pinned @@ -240,7 +241,7 @@ required on `main`**, at which moment the coupling stops being latent. | ID | C-87 | | Tier | 2 — the failure mode is invisible by construction. The upload succeeds, the storage is paid for, the consumer's endpoint returns empty, and nothing anywhere raises. That is the shape ADR-013 §4.1a calls *"invisible to the consumer, not merely degraded"*. | | Source | ADR-017 §8, sharpened by external review (#232, #234), 2026-08-10 | -| Trigger | **Either half going missing.** (a) `views-faoapi#379` or `views-crafdapi#39` is closed without the check being written. (b) A private API operated by a **third party** becomes a consumer — at which point the second half cannot be required at all and this becomes permanent. | +| Trigger | **Either half going missing.** (a) `views-faoapi#379` or `views-crafdapi#53` is closed without the check being written. (b) A private API operated by a **third party** becomes a consumer — at which point the second half cannot be required at all and this becomes permanent. | | Owner | The consumer-side seats own the check; this repository owns noticing that it exists. | | Location | `views_postprocessing//product.py::CONSUMER_DOCUMENT_NAME`; the check in `tests/test_product.py`; ADR-017 §5, §8, Appendix B. | @@ -248,11 +249,11 @@ ADR-017 decides that the delivery label is declared in the public coordinate reg Its residual is stated plainly in §8 and belongs here rather than only in a document: **we will verify our copy against the declaration, not the consumer's code against it.** If a consumer quietly starts filtering on something else, our check passes and the delivery is invisible exactly as before. -**Why this is a risk and not merely a note.** The second half is real work in repositories this project does not control. `views-faoapi#379` has a willing owner. `views-crafdapi#39` is blocked on that partner's data contract and could sit for a long time. Until both land, the label's agreement with reality rests on the source-reading check — which ADR-017's sequencing deliberately keeps alive for exactly this reason, and which someone could remove believing the registry check replaced it. +**Why this is a risk and not merely a note.** The second half is real work in repositories this project does not control. `views-faoapi#379` has a willing owner. `views-crafdapi#53` is blocked on that partner's data contract and could sit for a long time. Until both land, the label's agreement with reality rests on the source-reading check — which ADR-017's sequencing deliberately keeps alive for exactly this reason, and which someone could remove believing the registry check replaced it. **What was already prevented.** A reviewer caught that the obvious sequence created a window where the source-reading check was deleted before the consumer-side check existed, leaving a green build proving only that two values this platform authored agreed with each other. ADR-017 now forbids that ordering. This entry exists so the ordering constraint has a home outside the document that states it. -Cross-refs: **C-77** (the same field's producer-side half, resolved), ADR-013 §4.1a, ADR-017 §5/§8/Appendix B, views-appwrite#75, views-faoapi#379, views-crafdapi#39. +Cross-refs: **C-77** (the same field's producer-side half, resolved), ADR-013 §4.1a, ADR-017 §5/§8/Appendix B, views-appwrite#75, views-faoapi#379, views-crafdapi#53. --- @@ -622,6 +623,7 @@ Where each sibling stands, after trying them: The two compound: a suite that checks less than you think, and no requirement that even that much passes. Neither is caused by this sync — both are pre-existing — but this sync is the first time `main` receives an epic whose value is largely the guards themselves. Cross-refs: **C-46** and **C-57** (both RESOLVED; this is the residual each recorded as *"a CI-cost and cross-repo-coupling decision"* and *"worth deciding once for both"* — it now has a live home and a concrete answer per sibling), **C-80** (the other verification gap found in the same audit), #188. + **Partial mitigation 2026-08-10 (ADR-016) — the coverage half is mostly closed; the enforcement half is untouched.** The coverage half rested on a claim nobody could check. This entry, and the workflow comment it drew on, said `views-appwrite` was **private**, so its seven checks needed a credential. It went public on **2026-08-08** (`views-appwrite@9d80b75`, a deliberate and recorded act), and the claim here went on being made for two days afterwards. No credential was required, and none had been the obstacle since that date. diff --git a/tests/seam_registry.py b/tests/seam_registry.py new file mode 100644 index 0000000..d6a4f90 --- /dev/null +++ b/tests/seam_registry.py @@ -0,0 +1,153 @@ +"""Reading the platform coordinate registry, and refusing to read it badly. + +The registry is a TOML file in **views-appwrite** declaring every cross-repository +coordinate on the delivery seam. This repository never copies it — it is referenced, and +read from a sibling checkout when one is present. + +**Why this is its own module.** Two test modules now need it: `test_env_declaration.py` +for the coordinate sections, `test_product.py` for the `[contract.*]` delivery labels. +It began as one reader, was hand-copied into the second, and the copies immediately +disagreed — one read the sibling's `main`, the other whatever branch the clone sat on, +which is the very defect a review had just caught in the first. That is the second +incident, which is this repository's stated trigger for extracting (WET before DRY: two +copies that are understood beat one abstraction that is guessed — until the second copy +proves the shape). + +It is deliberately **not** in `conftest.py`. That file already holds four unrelated +groups and register **C-88**'s trigger is a fifth arriving. + +**What this module refuses, and why each refusal exists.** Every one is a real accident +that a silent read would have turned into a permanently green test: + +- a ref that does not resolve, or resolves to something that is not a commit; +- a ref whose resolved sha does not begin with it — an annotated **tag**, which git peels + silently and which yields a 404 from the blob URL these modules publish; +- an **empty** file: `tomllib.loads("")` is `{}`, and an empty registry compared against + an empty projection passes forever; +- a file that parses but carries no `meta.version` and no `connection` rows, i.e. is not + the registry; +- a parse error, wrapped rather than raised raw — a helper whose justification is failing + legibly must not hand back a bare traceback. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +#: The registry, relative to a views-appwrite checkout. +REGISTRY_RELPATH = Path("docs") / "ADRs" / "platform" / "coordinate_registry.toml" + +#: A row that carries no value at all — distinct from any string a registry could hold. +ABSENT = object() + + +class RegistryReadError(RuntimeError): + """The registry could not be read — never a silently empty dict.""" + + +def registry_at(repo: Path, ref: str) -> dict: + """The registry as it stood at ``ref``. Raises rather than returning ``{}``. + + ``ref`` must name a **frozen commit**, and that has no exceptions. An empty ref reads + the git index; a branch name reads a moving tip. Either makes a pin-versus-current + comparison compare the registry to itself and report green against every future + edition — which is the trap this module exists for, so an exemption "just for the + caller that needs a branch" reopens it. Callers wanting `main` resolve it to a sha + first; :func:`registry_current` does exactly that. + """ + tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") + + resolved = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if resolved.returncode != 0 or not resolved.stdout.strip(): + 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." + ) + if not resolved.stdout.strip().startswith(ref): + raise RegistryReadError( + f"{ref!r} resolves to commit {resolved.stdout.strip()[:12]}, which does not " + "start with it — that is what an annotated TAG looks like. git peels tags " + "silently, so every check here would pass while the blob URL these modules " + "publish returns 404. Pin the commit." + ) + + result = subprocess.run( + ["git", "-C", str(repo), "show", f"{ref}:{REGISTRY_RELPATH.as_posix()}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if result.returncode != 0: + raise RegistryReadError( + f"cannot read the registry at {ref!r} in {repo}: git exited " + f"{result.returncode} ({result.stderr.strip()[:200]}). A pinned commit that " + "cannot be read is not a baseline — run `git fetch` in that checkout." + ) + if not result.stdout.strip(): + raise RegistryReadError( + f"the registry is EMPTY at {ref!r} in {repo}. Treating that as a registry " + "with no coordinates would make every comparison against it pass." + ) + try: + registry = tomllib.loads(result.stdout) + except Exception as exc: + raise RegistryReadError( + f"the registry at {ref!r} in {repo} did not parse as TOML: {exc}. This is " + "what happens when the path names a directory, or the blob is not the file " + "we think it is." + ) from exc + if not registry.get("meta", {}).get("version") or not registry.get("connection"): + raise RegistryReadError( + f"the registry at {ref!r} parsed but carries no meta.version or no connection " + "rows. Every real edition has both, so this is not the file we think it is — " + "refusing rather than comparing against it." + ) + return registry + + +def registry_current(repo: Path) -> dict: + """The registry as the sibling's ``main`` has it — never its working tree. + + A sibling clone normally sits on whatever branch its own agent last worked on. + Comparing against that grades this repository on unreviewed content, or reddens it on + a branch nobody merged — issue #196 verbatim, the case that cost this platform a + withdrawn pull request. The `ref: main` discipline lived only inside the CI workflow; + a developer running the suite locally had no such thing. + + Read errors propagate unchanged, so a registry that is corrupt or anchorless **on + main** says so, rather than being reported as "neither ref resolves". + """ + head = "" + for ref in ("origin/main", "main"): + resolved = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + capture_output=True, text=True, check=False, timeout=30, + ) + if resolved.returncode == 0 and resolved.stdout.strip(): + head = resolved.stdout.strip() + break + if not head: + raise RegistryReadError( + f"neither origin/main nor main resolves in {repo}. The comparison is against " + "that repository's ratified content, so there is nothing to compare against " + "— run `git fetch` there." + ) + return registry_at(repo, head) + + +def rows(registry: dict, sections: tuple) -> dict[str, tuple]: + """``name -> (section, class, value-or-ABSENT)`` across ``sections``. + + 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() + } diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index 82de91f..51a8898 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -450,10 +450,15 @@ def test_each_rule_bites_on_a_broken_world(label, workflow, siblings, why): "This means the RULE has stopped working — the real workflow is not involved " "and is probably fine. Fix the rule, not run_pytest.yml." ) - assert any("views-appwrite" in v for v in violations), ( - f"[{label}] objected, but its message does not name the offending sibling: " - f"{violations}. A maintainer reading only the failure would not know which " - "repository to look at." + offender = next(iter(siblings), None) or "views-appwrite" + assert any(offender in v for v in violations), ( + f"[{label}] objected, but its message does not name the offending sibling " + f"({offender!r}): {violations}. A maintainer reading only the failure would not " + "know which repository to look at.\n\n" + "The name is taken from the mutant's own sibling map rather than hardcoded — a " + "fixed \"views-appwrite\" was correct only by coincidence of today's fixtures, " + "and would have made a working rule fail for the next mutant about a different " + "repository, under this file's inverted semantics." ) diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index eb30777..871e653 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -32,6 +32,14 @@ import pytest +from tests.seam_registry import ( + ABSENT as _ABSENT, + REGISTRY_RELPATH as _REGISTRY_RELPATH, + RegistryReadError as _RegistryReadError, + registry_at as _registry_at, + registry_current as _registry_current, + rows as _rows, +) from tests.conftest import ( PARTNER_PACKAGES, broken_sibling_overrides, @@ -433,7 +441,7 @@ def test_secret_env_names_follow_the_seam_contract_naming_rule(): # and copying one into a test is the same violation as copying it into code. These # compare names, declared classes, and the edition — nothing else. -_REGISTRY_RELPATH = Path("docs") / "ADRs" / "platform" / "coordinate_registry.toml" + #: How this module treats each declared name, vs. the registry's own `class` field. #: @@ -479,8 +487,6 @@ def test_every_declared_name_is_classified_here(partner): #: Sentinel for "this row carries no value", distinct from any string a registry #: could legitimately hold. -_ABSENT = object() - #: Every top-level table the registry may carry, and what this repository does with it. #: #: **Declared, and asserted against the live registry** — a table appearing upstream that @@ -497,6 +503,13 @@ def test_every_declared_name_is_classified_here(partner): # 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 + #: 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 + #: it YET: adopting it is the follow-up that lets the drift check below stop caring + #: about editions that oblige nobody. Caught by this very partition on its first + #: encounter, which is what the partition is for. + "edition": "IGNORED", "meta": "METADATA", # the edition and its amendment log } @@ -538,103 +551,6 @@ def _missing_dependencies(registry: dict) -> list[str]: return sorted(n for n in _TABLES_WE_DEPEND_ON if n not in registry) -def _rows(registry: dict, sections: tuple) -> dict[str, tuple]: - """``name -> (section, class, value-or-absent)`` across ``sections``. - - The single reader the class check, the drift projection and the no-copy scan share. - Two hand-copied versions had already diverged on null handling: one used - ``registry.get(s, {})`` and raised ``AttributeError`` on a null section, the other - used ``(registry.get(s) or {})`` and 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() - } - - -class _RegistryReadError(RuntimeError): - """The registry could not be read at a pinned commit — never a silent empty dict.""" - - -def _registry_at(repo: Path, ref: str) -> dict: - """The registry as it stood at ``ref``, or raise. Never returns ``{}`` quietly. - - **Why not ``conftest.git_output``.** That helper turns any failure into ``""``, and - ``tomllib.loads("")`` returns ``{}``. An empty registry compared against an empty - projection passes — forever, silently. That is the vanished-directory shape ADR-014 - §2 exists for, and building it into the fix for a drift detector would be the joke - writing itself. So this distinguishes a non-zero exit from an empty file, and - refuses a parse that lacks the anchors every real edition has. - - Module-local rather than in ``conftest.py``: register C-88 records that file already - holding four unrelated groups, and its trigger is a fifth. There is one consumer. - """ - tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") - - # The ref must name a FROZEN commit, and must be the one we asked for. - # - # Without this, `git show ":"` reads the INDEX and `git show "main:"` - # reads a branch — both exit 0, both parse, both pass every anchor below. A pin - # blanked by a bad edit or set to "main" by someone who thinks that is what a pin - # means would make the differential compare the current registry against itself and - # report green against every future edition. That is the same silent-baseline trap - # this reader exists to close, wearing a different hat. - # - # ``^{commit}`` also peels an annotated TAG. That matters here: a tag object is not - # a commit, is reachable only through refs/tags, and — measured 2026-08-11 — yields - # a 404 from the pinned blob URL these modules publish. So this refuses one. - resolved = subprocess.run( - ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], - capture_output=True, text=True, check=False, timeout=30, - ) - if resolved.returncode != 0 or not resolved.stdout.strip(): - 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, a branch name reads a moving tip, and " - "either would make the comparison below compare the registry to itself." - ) - if not resolved.stdout.strip().startswith(ref): - raise _RegistryReadError( - f"{ref!r} resolves to commit {resolved.stdout.strip()[:12]}, which does not " - f"start with it. That happens when the pin names an annotated TAG rather than " - "a commit — git peels it silently, every check here passes, and the blob URL " - "these modules publish returns 404. Pin the commit." - ) - - result = subprocess.run( - ["git", "-C", str(repo), "show", f"{ref}:{_REGISTRY_RELPATH.as_posix()}"], - capture_output=True, text=True, check=False, timeout=30, - ) - if result.returncode != 0: - raise _RegistryReadError( - f"cannot read the registry at {ref!r} in {repo}: git exited " - f"{result.returncode} ({result.stderr.strip()[:200]}). A pinned commit that " - "cannot be read is not a baseline — run `git fetch` in that checkout." - ) - if not result.stdout.strip(): - raise _RegistryReadError( - f"the registry is EMPTY at {ref!r} in {repo}. Treating that as a registry " - "with no coordinates would make every comparison below pass." - ) - try: - registry = tomllib.loads(result.stdout) - except Exception as exc: # TOMLDecodeError, and whatever a non-TOML blob raises - raise _RegistryReadError( - f"the registry at {ref!r} in {repo} did not parse as TOML: {exc}. A raw " - "traceback from a helper whose justification is failing legibly would be its " - "own defeat — this happens when the path names a directory, or the blob is " - "not the file we think it is." - ) from exc - if not registry.get("meta", {}).get("version") or not registry.get("connection"): - raise _RegistryReadError( - f"the registry at {ref!r} parsed but carries no meta.version or no " - "connection rows. Every real edition has both, so this is not the file we " - "think it is — refusing rather than comparing against it." - ) - return registry - - def _projection(registry: dict, names: set[str]) -> dict[str, tuple]: """``name -> (section, class, value-or-absent)`` for the names we declare. @@ -651,42 +567,6 @@ def _projection(registry: dict, names: set[str]) -> dict[str, tuple]: return {n: row for n, row in _rows(registry, _CONSUMED_TABLES).items() if n in names} -def _registry_current(repo: Path) -> dict: - """The registry as the sibling's ``main`` has it — not its working tree. - - A sibling clone normally sits on whatever branch its own agent last worked on. - Comparing against that grades us on unreviewed content, or reddens on a branch nobody - merged — #196 verbatim, the case that cost this platform a withdrawn pull request. - The ``ref: main`` discipline lived only inside the CI workflow, so a developer running - the suite locally was comparing against something else entirely, and ADR-016's erratum - claimed otherwise. - """ - head = "" - for ref in ("origin/main", "main"): - resolved = subprocess.run( - ["git", "-C", str(repo), "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], - capture_output=True, text=True, check=False, timeout=30, - ) - if resolved.returncode == 0 and resolved.stdout.strip(): - head = resolved.stdout.strip() - break - if not head: - raise _RegistryReadError( - f"neither origin/main nor main resolves in {repo}. The comparison is against " - "that repository's ratified content, so there is nothing to compare against — " - "run `git fetch` there." - ) - # Resolved to a SHA before reading, deliberately. `_registry_at` refuses any ref that - # is not a frozen commit, and that guard must have no exceptions: an earlier version - # exempted "main"/"origin/main" so this function could call it — which meant a pin - # literally set to ``"main"`` was accepted, reopening the hole the guard exists for. - # Resolving here keeps the guard absolute at the cost of one extra rev-parse. - # - # Read errors propagate unchanged: a registry that is corrupt or anchorless ON MAIN - # must say so, not be reported as "neither ref resolves". - return _registry_at(repo, head) - - def _describe_changes(then: dict, now: dict, names: set[str]) -> dict[str, str]: """What differs, said WITHOUT printing a coordinate value. @@ -1166,7 +1046,10 @@ def test_no_coordinate_value_is_copied_into_this_repo(): body["value"] for section in scanned_sections for body in registry.get(section, {}).values() - if isinstance(body.get("value"), str) and len(body["value"]) > 6 + # No length floor: it excluded two five-character coordinates and, + # measured, removing it keeps the suite green — so it was buying nothing + # while narrowing a security check. + if isinstance(body.get("value"), str) and body["value"].strip() } copied = [] @@ -1197,8 +1080,17 @@ def test_no_coordinate_value_is_copied_into_this_repo(): # The copy is a value **assigned to its own coordinate name** — `APPWRITE_X=value` — # which is a reader's instruction to configure with that literal. That is precise # enough to have caught README.md and to ignore every legitimate mention. + # `(.+?)\s*$` used to swallow an inline comment into the captured value, so + # `NAME=value # note` compared `'value # note'` against the registry and matched + # nothing. README.md's own Configuration block is written in exactly that style, so + # the guard was blind in the house style of the document it was written for — proven + # by injecting two real coordinate values with trailing comments, both invisible. + # + # A `#` inside a quoted value is legitimate, so the quoted form is matched first and + # taken whole; only an unquoted value is truncated at a comment. assignment = re.compile( - r"^\s*(?:export\s+)?(" + "|".join(sorted(_EXPECTED_NAMES)) + r")\s*=\s*(.+?)\s*$" + r"^\s*(?:export\s+)?(" + "|".join(sorted(_EXPECTED_NAMES)) + r")\s*=" + r"""\s*(?:"([^"]*)"|'([^']*)'|([^#]*?))\s*(?:#.*)?$""" ) # This repository's OWN tracked markdown — `git ls-files`, not `rglob`. CI checks # sibling repositories out into the workspace, and their documents are not this @@ -1225,10 +1117,11 @@ def test_no_coordinate_value_is_copied_into_this_repo(): for doc in sorted(scanned): for number, line in enumerate(doc.read_text().splitlines(), 1): match = assignment.match(line) - if match and match.group(2).strip('"\'') in values: + captured = next((g for g in match.groups()[1:] if g is not None), None) if match else None + if captured is not None and captured.strip() in values: copied.append( - f"{doc.relative_to(_REPO)}:{number} = {match.group(2)!r} " - f"(assigned to {match.group(1)})" + f"{doc.relative_to(_REPO)}:{number} assigns a registry value to " + f"{match.group(1)}" ) assert not copied, ( diff --git a/tests/test_product.py b/tests/test_product.py index 5e8440c..633f0e1 100644 --- a/tests/test_product.py +++ b/tests/test_product.py @@ -26,6 +26,7 @@ import pytest from tests.conftest import CONSUMER_REPO, PARTNER_PACKAGES, SIBLINGS, require_sibling +from tests.seam_registry import registry_current #: partner -> the document name its consumer filters on, DECLARED here rather than #: read back from the module under test. @@ -181,19 +182,15 @@ def test_every_partner_has_a_declared_consumer_repository(): ) -#: The platform registry, relative to a views-appwrite checkout. +#: The registry reader is shared with ``tests/test_env_declaration.py`` — see +#: ``tests/seam_registry.py``. #: -#: Loaded locally rather than imported from ``tests/test_env_declaration.py``: a test -#: module importing another test module is a dependency nobody declared, and the two read -#: different things — that file reads coordinate sections, this one reads one contract -#: row. Two four-line readers that are understood beat one shared one that has to serve -#: both (WET before DRY). -_REGISTRY_RELPATH = Path("docs") / "ADRs" / "platform" / "coordinate_registry.toml" - - -def _registry(repo: Path) -> dict: - tomllib = pytest.importorskip("tomllib", reason="stdlib from 3.11; pyproject requires it") - return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) +#: It began here as a local four-line copy, defended as WET. The copies immediately +#: disagreed: that module was moved to read the sibling's ``main`` after a review found +#: reading its working tree meant grading this repository against unreviewed content +#: (#196's shape), and this copy was left behind still reading the working tree. Two +#: copies of a rule are fine; two copies that answer the same question differently are +#: the second incident, which is this repository's trigger for extracting. #: partner -> the registry row that is the AUTHORITY for its delivery label. @@ -244,7 +241,7 @@ def test_the_declared_consumer_name_matches_the_registry(partner): anywhere (ADR-013 §4.1a). """ repo = require_sibling("views-appwrite") - registry = _registry(repo) + registry = registry_current(repo) contract = registry.get("contract") or {} row = _CONTRACT_ROW[partner] From f7ec27168d8d087bda835443db0ee0ffa2adfd14 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Tue, 11 Aug 2026 21:13:40 +0200 Subject: [PATCH 9/9] =?UTF-8?q?fix(tests):=20iteration=203=20=E2=80=94=20t?= =?UTF-8?q?he=20scan=20was=20blind=20in=20a=20second=20house=20style,=20an?= =?UTF-8?q?d=20#76=20landed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings from /review-diff on iteration 2. Two are worth reading. THE NO-COPY SCAN WAS BLIND IN THE HOUSE STYLE AGAIN, IN A DIFFERENT WAY. Iteration 2 fixed `NAME=value # comment`. The pattern still anchored the coordinate name at `^\s*`, so it saw only an assignment that STARTS A LINE. Proven by injecting three real registry values into README.md — a markdown bullet, a table cell, and an assignment quoted mid-sentence — all three invisible, all three now caught. Thirteen forms proven caught, four legitimate mentions proven silent. The shared cause of both blindnesses is not a regex bug: the pattern described one way of writing markdown, the way this repository happens to write it today. A guard against publishing a coordinate value from a PUBLIC repository has to survive the next contributor writing a bullet instead of a fenced block. C-57 amended with both, and with the residual that stays deliberate — a value merely NAMED in a sentence is not a copy, and an earlier draft that matched those fired on a dozen documents (ADR-014 §3). VIEWS-APPWRITE#76 HAS LANDED AND TWO RECORDS SAID IT WAS "FILED". Registry v1.6.0 ships `[edition."x.y.z"].obliges_consumers` — C-86's stated first-preference resolution. C-86 still called it filed; ADR-016 §7b still called it "still worth doing"; the code comment added in the previous commit already called it delivered. Three artifacts in one branch, two of them behind. Both records amended, and the re-pin deferral that lived only in a pull-request description now has a written trigger and an owner — both halves of that trigger have already fired (ADR-014 §4). THE MUTANT OFFENDER INFERRED WHAT IT SHOULD DECLARE. `next(iter(siblings), None) or "views-appwrite"` reads as principled and is wrong for G2, whose premise is an EMPTY declaration: there is nothing to derive from, so it fell back to the hardcoded name while its own message claimed it had not. Each mutant now declares the offender its rule must name (ADR-003), proven load-bearing by feeding one a wrong name. Also: `_ABSENT`'s doc-comment survived `_ABSENT` — the block moved to seam_registry.py and left its `#:` lines attached to `_TABLE_ROLE`, so the partition was documented as a sentinel for a valueless row. And the no-copy scan was the last of seven readers still hand-rolling its skip instead of calling `require_sibling`, whose whole justification is that the hand-rolled message tells a contributor too little to run the test. Suite 410 passed / 1 skipped / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 11 +++- reports/technical_risk_register.md | 53 ++++++++++++++++++- tests/test_ci_sibling_coverage.py | 35 +++++++----- tests/test_env_declaration.py | 47 ++++++++++------ 4 files changed, 112 insertions(+), 34 deletions(-) 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 ddd6024..ae5cf56 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -264,7 +264,16 @@ The third kind exists because the no-copy rule forbids writing expected coordina into this repository. A pinned edition is the only lawful place to keep a baseline for comparing them. -views-appwrite#76 is still worth doing; it is no longer a precondition for anything here. +views-appwrite#76 has since been done. It shipped on 2026-08-11 as registry version 1.6.0, +which adds a table naming each edition and saying whether that edition obliges the +repositories that read the registry. This repository does not read it yet, and does not have +to: the third kind of check above already tells us whether anything we depend on moved. What +the new table would add is upstream's own answer to the same question, which is cheaper and +does not require a pinned baseline at all. That is worth adopting, and it is not urgent. + +It is worth saying how we learned it had shipped. Nobody told us. The check that requires +every table upstream to be classified here went red the first time it met the new one, which +is what that check is for. --- diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 6aa7687..20bc1a0 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -207,7 +207,7 @@ ADR-016 has CI check out `views-appwrite` and `views-crafdapi` so that cross-rep **The rate is not hypothetical.** views-appwrite reports five registry editions in four days (v1.4.0 2026-08-02 through v1.4.4 2026-08-05), **four of them observation-driven** — recording console facts, correcting a key's scopes — carrying no obligation for any consumer. Each would have reddened this repository and blocked a release. -**What would resolve it, in order of preference:** views-appwrite#76 makes the obligation-carrying distinction machine-readable, so observation-only bumps stop firing the check at all — filed, and that seat volunteered it. Failing that, a bypass actor restores the escape. Failing both, the coupling stands as ADR-016 §7 describes, which is defensible but should be chosen rather than discovered. +**What would resolve it, in order of preference:** views-appwrite#76 makes the obligation-carrying distinction machine-readable, so observation-only bumps stop firing the check at all — filed, and that seat volunteered it. Failing that, a bypass actor restores the escape. Failing both, the coupling stands as ADR-016 §7 describes, which is defensible but should be chosen rather than discovered. *(⚠ "filed" is stale as of 2026-08-11 — #76 has **landed**, as registry v1.6.0. See the amendment at the end of this entry; the sentence is left as written because what it asked for and what arrived are worth comparing.)* Cross-refs: **C-81** (the enforcement half, whose fix activates this), **C-46** (whose recommendation against per-PR sibling checkouts this overrode, with the reasoning recorded there), ADR-016 §7/§7a/§7b, views-appwrite#76. @@ -231,6 +231,40 @@ that carry an obligation. The entry's trigger is unchanged and remains a console action: **a status check becomes required on `main`**, at which moment the coupling stops being latent. +**Amended 2026-08-11 — the first-preference resolution has LANDED, and this entry found out +from a guard rather than from a notification.** views-appwrite#76 shipped as registry +**v1.6.0**: a new `[edition."x.y.z"]` table marking each edition `obliges_consumers = +true|false`, which is exactly the machine-readable distinction the paragraph above asks for. +The sentence *"filed, and that seat volunteered it"* was true when written and is now stale. + +Two things are worth recording about how it arrived. The partition check added the same day +went red on it unprompted, on its first live encounter with an upstream table nobody here +had classified — which is the whole reason that check is directional. And the row-level +drift check stayed **correctly silent**, because a new table this package does not read is +not drift in anything it depends on. The two behaved exactly as designed on data neither +was tested against. + +**Adoption is a follow-up, not part of the change that noticed it.** `[edition]` is +classified `IGNORED` in `tests/test_env_declaration.py::_TABLE_ROLE` — declared, not +silently unseen. Nothing here reads `obliges_consumers` yet, so the false-alarm rate is +still carried by the row-level differential rather than by upstream's own flag. + +**Deferral, with the trigger ADR-014 §4 requires** — this was carried in a pull-request +description, where deferrals go to be forgotten: + +- **What:** re-pin `SEAM_CONTRACT_VERSION` / `SEAM_CONTRACT_COMMIT` in both + `views_postprocessing/{unfao,crafd}/appwrite_env.py` (currently v1.5.2 / `c7b597e`), and + read `[edition].obliges_consumers` so an observation-only edition cannot fire anything. +- **Trigger — both halves have now FIRED:** the partition fired on v1.6.0 (2026-08-11), and + views-appwrite#76 landed. The re-pin is therefore **due**, not deferred; what remains + deferred is reading the new flag. +- **Owner:** whoever next touches an `appwrite_env.py`. Re-pinning is hygiene with no + safety consequence now that the differential exists — which is precisely why it needs a + written trigger rather than a good intention. + +Cross-refs for this amendment: **C-57** (the drift detector this rides on), ADR-016 §7a/§7b, +views-appwrite#76 (**delivered**, registry v1.6.0). + --- @@ -1470,7 +1504,22 @@ This is ADR-014 §2 in its narrow form: a guard's *scope* is part of what has to PR #211 re-pins crafd to `1.4.1` / `90fc105` and parameterises **three** of the four checks over both partner declarations — names-and-class, pinned edition, commit reachability. The fourth, the value-copy scan, was never partner-scoped: it walks `_PKG.rglob("*.py")` and so covered `crafd/` from the day it landed. A third partner is now a one-line addition, and an unguarded one is a failure. This entry stays RESOLVED — the mechanism was right, its reach was not — but the residual below now has a companion: a detector that names its subject is a detector that will miss the next subject. -Cross-refs: C-74 (the guard this paragraph vouched for), C-33 (store identity still hardcoded per store — the same env surface, different concern), C-58 (what happens when a coordinate is wrong rather than missing), C-44 (the pipeline-core version coupling that would carry a registry change), issues #134/#135/#138 (this repo's discharged þing-01 obligations), #104 (README env block placeholders). +**⚠ AMENDED 2026-08-11 — the value-copy scan was blind in this repository's own house style, twice, and the second time is the general lesson.** + +The scan was extended to tracked markdown during the development→main sync, because README.md had carried four real coordinate values. Two reviews later, it was still missing two whole classes of copy — both of them ordinary markdown, both proven by injecting real registry values and watching the guard stay green: + +| form | why it was invisible | fixed | +|---|---|---| +| `NAME=value # comment` | `(.+?)\s*$` swallowed the comment into the captured value, so nothing compared equal. **README.md's own Configuration block is written in exactly this form.** | 2026-08-11 (iteration 2) | +| `` - `NAME=value` ``, `` \| NAME=value \| ``, `` set `NAME=value` before … `` | the pattern anchored the coordinate name at `^\s*`, so it saw only an assignment that *starts a line* | 2026-08-11 (iteration 3) | + +The shared cause is not a regex bug. **The pattern described one way of writing markdown — the way this repository happens to write it today — and a guard against publishing a value has to survive the next contributor writing a bullet instead of a fenced block.** The name may now be preceded by anything that is not part of an identifier, and the value ends at whatever terminates it in prose: a comment, a closing backtick, or a table pipe. Thirteen forms are proven caught; the four legitimate mentions that must stay silent are proven silent. + +**The residual, which is deliberate and should not be "fixed".** This remains a *syntax* match. A value merely named in a sentence — "the six stranded documents in ``" — is not a copy and does not fire. An earlier draft that matched any occurrence fired on a dozen documents, and the lesson recorded above applies to itself: a guard that cries wolf gets deleted, after which the real rule is unguarded (ADR-014 §3). The class this cannot catch is a value pasted into prose with no assignment syntax anywhere near it. That is accepted, because the alternative has been tried and was worse. + +**Why this belongs on C-57 rather than in a new entry.** It is the same guard, the same failure direction, and the same lesson this entry already records one layer down: the earlier amendment found the scan's *scope* was never mutation-proven (it named `unfao` and missed `crafd`); this one finds its *matching* was never proven against the file formats it scans. Scope, matching, and now syntax-variant — three ways for a mutation-proven guard to be proven against the wrong thing. + +Cross-refs: C-74 (the guard this paragraph vouched for), C-33 (store identity still hardcoded per store — the same env surface, different concern), C-58 (what happens when a coordinate is wrong rather than missing), C-44 (the pipeline-core version coupling that would carry a registry change), **C-86** (the drift checks this scan sits beside), issues #134/#135/#138 (this repo's discharged þing-01 obligations), #104 (README env block placeholders). --- diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index 51a8898..0572e2e 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -387,27 +387,36 @@ def _replace(name: str, **changes) -> dict[str, Sibling]: return {name: replace(SIBLINGS[name], **changes)} +#: ``(rule, broken workflow, broken declaration, the sibling the message must name, why)`` +#: +#: **The offender is declared, not inferred.** It was briefly derived from the mutant's +#: own sibling map — which reads as principled and is wrong for `G2`, whose whole premise +#: is an EMPTY declaration: there is nothing in the map to derive from, so it fell back to +#: a hardcoded ``"views-appwrite"`` while the assertion message claimed it had not. For G2 +#: the offender is a name in the *workflow*, and no amount of looking at the declaration +#: will find it. Writing it out is this repository's own rule (ADR-003): declare the fact, +#: do not let a helper guess it. _MUTANTS = [ ("G1 declared checkouts are present and pointed at", {"jobs": {"test": {"steps": [{"run": "poetry run pytest tests/", "env": {}}]}}}, - _ONE, "declared ci_checkout=True but the checkout step is gone"), + _ONE, "views-appwrite", "declared ci_checkout=True but the checkout step is gone"), ("G2 every checkout is declared", - _workflow(), {}, "checked out but undeclared"), + _workflow(), {}, "views-appwrite", "checked out but undeclared"), ("G4 every note names a record", _workflow(), _replace("views-appwrite", ci_checkout=False, note=""), - "not checked out and no note"), + "views-appwrite", "not checked out and no note"), ("G4 every note names a record", _workflow(), _replace("views-appwrite", ci_checkout=False, note="because reasons"), - "note explains but names no record"), + "views-appwrite", "note explains but names no record"), ("G5 no step swallows its own failure", _workflow(**{"continue-on-error": True}), _ONE, - "a failed fetch would not fail the build"), + "views-appwrite", "a failed fetch would not fail the build"), ("G6 siblings land under the excluded path", _workflow(**{"with": {"path": "vendor/views-appwrite"}}), _ONE, - "outside _siblings/, so ruff would lint it"), + "views-appwrite", "outside _siblings/, so ruff would lint it"), ("G7 siblings are taken from main", _workflow(**{"with": {"ref": "development"}}), _ONE, - "takes a branch that is not the authority"), + "views-appwrite", "takes a branch that is not the authority"), ] @@ -430,8 +439,8 @@ def test_every_rule_has_a_mutant_that_proves_it_bites(): ) -@pytest.mark.parametrize("label, workflow, siblings, why", _MUTANTS) -def test_each_rule_bites_on_a_broken_world(label, workflow, siblings, why): +@pytest.mark.parametrize("label, workflow, siblings, offender, why", _MUTANTS) +def test_each_rule_bites_on_a_broken_world(label, workflow, siblings, offender, why): """Feed a rule a deliberately broken world and demand that it complains. **A failure here means the RULE is broken, not the workflow.** That inversion is the @@ -450,15 +459,13 @@ def test_each_rule_bites_on_a_broken_world(label, workflow, siblings, why): "This means the RULE has stopped working — the real workflow is not involved " "and is probably fine. Fix the rule, not run_pytest.yml." ) - offender = next(iter(siblings), None) or "views-appwrite" assert any(offender in v for v in violations), ( f"[{label}] objected, but its message does not name the offending sibling " f"({offender!r}): {violations}. A maintainer reading only the failure would not " "know which repository to look at.\n\n" - "The name is taken from the mutant's own sibling map rather than hardcoded — a " - "fixed \"views-appwrite\" was correct only by coincidence of today's fixtures, " - "and would have made a working rule fail for the next mutant about a different " - "repository, under this file's inverted semantics." + "Each mutant declares the name its rule must produce, in _MUTANTS. If you have " + "changed which sibling a mutant breaks, change its declared offender in the same " + "edit — under this file's inverted semantics, a stale one fails a rule that works." ) diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 871e653..0db95f8 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -46,7 +46,6 @@ commit_is_on_main, git_output, require_sibling, - sibling_repo, ) from views_postprocessing.contract import launch_config from views_postprocessing.crafd import appwrite_env as crafd_env @@ -442,7 +441,6 @@ def test_secret_env_names_follow_the_seam_contract_naming_rule(): # compare names, declared classes, and the edition — nothing else. - #: How this module treats each declared name, vs. the registry's own `class` field. #: #: **Written out, not derived.** An earlier draft built this by suffix — anything ending @@ -485,8 +483,6 @@ def test_every_declared_name_is_classified_here(partner): ) -#: Sentinel for "this row carries no value", distinct from any string a registry -#: could legitimately hold. #: Every top-level table the registry may carry, and what this repository does with it. #: #: **Declared, and asserted against the live registry** — a table appearing upstream that @@ -1025,9 +1021,12 @@ def test_no_coordinate_value_is_copied_into_this_repo(): their own: a function name is not a ``Constant``; ``"unfao_bucket datastore"`` is not equal to ``"unfao_bucket"``; docstrings are excluded outright. """ - repo = sibling_repo("views-appwrite") - if repo is None: - pytest.skip("views-appwrite checkout not found — set VIEWS_APPWRITE") + # `require_sibling`, like every other reader in this file. The hand-rolled skip that + # stood here said only "checkout not found — set VIEWS_APPWRITE", which is the + # degraded message `require_sibling` exists to replace: it names the variable but not + # the conventional path, so a contributor can watch it skip and not run it. Being the + # last call site off the shared path is how a rule ends up with two behaviours. + repo = require_sibling("views-appwrite") registry = _registry_current(repo) # Scoped by the declared partition, NOT by an inline tuple — so a new table cannot # be swept in by a one-word edit, and `contract` cannot be swept in at all. @@ -1080,17 +1079,30 @@ def test_no_coordinate_value_is_copied_into_this_repo(): # The copy is a value **assigned to its own coordinate name** — `APPWRITE_X=value` — # which is a reader's instruction to configure with that literal. That is precise # enough to have caught README.md and to ignore every legitimate mention. - # `(.+?)\s*$` used to swallow an inline comment into the captured value, so - # `NAME=value # note` compared `'value # note'` against the registry and matched - # nothing. README.md's own Configuration block is written in exactly that style, so - # the guard was blind in the house style of the document it was written for — proven - # by injecting two real coordinate values with trailing comments, both invisible. # - # A `#` inside a quoted value is legitimate, so the quoted form is matched first and - # taken whole; only an unquoted value is truncated at a comment. + # **It has been blind in this repository's own house style twice, and both blindnesses + # had the same cause: the pattern described one way of writing markdown.** + # + # 1. `(.+?)\s*$` swallowed an inline comment into the captured value, so + # `NAME=value # note` compared `'value # note'` against the registry and + # matched nothing. README.md's Configuration block is written in exactly that form. + # 2. Anchoring the name at `^\s*` saw only an assignment that *starts a line*. A + # markdown bullet — ``- `NAME=value` `` — a table cell, or an assignment quoted + # mid-sentence were all invisible, and all three are ordinary ways to document + # configuration. The fenced-block form the guard was written against is the one + # form this repository happens to use today. + # + # So the name may be preceded by anything that is not part of an identifier, and the + # value ends at whatever terminates it in prose: a comment, a closing backtick, or a + # table pipe. A `#` inside a QUOTED value is legitimate, so the quoted forms are tried + # first and taken whole; only an unquoted value is truncated. + # + # This stays a syntax match, deliberately. A value merely *named* in a sentence is not + # a copy — C-57 recorded a draft that fired on a dozen such documents, and a guard that + # cries wolf gets deleted, after which the real rule is unguarded (ADR-014 §3). assignment = re.compile( - r"^\s*(?:export\s+)?(" + "|".join(sorted(_EXPECTED_NAMES)) + r")\s*=" - r"""\s*(?:"([^"]*)"|'([^']*)'|([^#]*?))\s*(?:#.*)?$""" + r"(?:^|(?<=[\s`|>*-]))(?:export\s+)?(" + "|".join(sorted(_EXPECTED_NAMES)) + r")\s*=" + r"""\s*(?:"([^"]*)"|'([^']*)'|([^#`|]*?))\s*(?:[#`|].*)?$""" ) # This repository's OWN tracked markdown — `git ls-files`, not `rglob`. CI checks # sibling repositories out into the workspace, and their documents are not this @@ -1116,7 +1128,8 @@ def test_no_coordinate_value_is_copied_into_this_repo(): ) for doc in sorted(scanned): for number, line in enumerate(doc.read_text().splitlines(), 1): - match = assignment.match(line) + # `search`, not `match`: the assignment no longer has to start the line. + match = assignment.search(line) captured = next((g for g in match.groups()[1:] if g is not None), None) if match else None if captured is not None and captured.strip() in values: copied.append(