From 2ae1c73bff7221d543f199cd7f9d982798284919 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Mon, 10 Aug 2026 02:12:38 +0200 Subject: [PATCH 1/4] refactor(tests): drop the circular half of the sibling declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expert-code-review of the design found two fields and two rules justifying each other and nothing else. Measured rather than argued: .public read in exactly one place — rule G3 .public_checked read in exactly one place — rule G8 G3's private-branch vacuous; no private sibling is checked out So `public` existed to be verified by G3, and G3 existed to protect `public`'s verifiability. Deleting both changes no behaviour. `public_checked` was worse than useless: nothing could confirm the check had happened, so it manufactured confidence instead of recording a fact — and G8, its only reader, cried wolf on its first CI run over a timezone, which is ADR-014 §3's own worked example. Visibility now lives in a sibling's `note`, as prose, because nothing here can verify it in any case. What survives without the flag needs no field: a repository CI cannot read fails to download, and G5 keeps that failure loud. guard file 552 -> 448 lines rules 8 -> 6 (G1, G2, G4, G5, G6, G7) Sibling 5 fields -> 3 (env, ci_checkout, note) Every surviving rule has an incident behind it, not a hypothesis: G6 for the 745 lint findings when a sibling landed outside `_siblings/`, G7 for views-appwrite's default branch being `development`. Re-proven against the real workflow after the cut — and the first G7 mutation was wrong rather than the guard, having hit a comment instead of a step. ADR-016 rewritten. The maintainer could not follow the previous version past §4, and the reason was the design rather than the prose: §5-§7 were second-order, rules protecting an argument rather than protecting the code. It now opens with the decision in four sentences, gives each rule the incident behind it, and states plainly in §7 that a broken sibling can block merging here — with the reasoning, which was previously asserted rather than argued. What was removed is recorded, because the removed design is more tempting than it looks and this document once argued for it. Suite 399 passed / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 302 +++++++++--------- tests/conftest.py | 57 ++-- tests/test_ci_sibling_coverage.py | 173 ++-------- 3 files changed, 215 insertions(+), 317 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 e45d947..35b824f 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -1,206 +1,222 @@ -# ADR-016: Cross-repository checks in CI, and how siblings declare their visibility +# ADR-016: CI checks out the sibling repositories our tests read **Status:** Accepted **Date:** 2026-08-10 **Decider:** Simon Polichinel von der Maase -**Arises from:** register **C-46**, **C-57**, **C-81**, which all carried the same standing -residual: the cross-repository drift detectors ran only on a maintainer's laptop -**Related:** [ADR-003](003_authority_of_declarations_over_inference.md) (declarations over -inference), [ADR-014](014_claims_and_the_guards_that_carry_them.md) §2 (a guard nobody has +**Related:** [ADR-014](014_claims_and_the_guards_that_carry_them.md) §2 (a guard nobody has watched fail is decoration) and §4 (a deferral names a trigger and an owner) --- -## Context - -### §1 Some of this repository's tests are about other repositories - -Most tests here check our own code. A few check something else: whether **claims this code -makes about other repositories are still true**. +## The decision in four sentences -The clearest case is the shared Appwrite coordinate registry. Each partner's -`appwrite_env.py` declares, in two constants, which edition of that registry it was written -against. That is a claim about a *different repository*. A test opens that repository, reads -the real edition, and compares. +A few of this repository's tests read **other** repositories to check that things we say +about them are still true. Those tests only work when the other repository is on disk, so +they ran on a developer's laptop and skipped in CI. -This is not theoretical maintenance. On 2026-08-05 the check failed: the registry had moved -from v1.4.1 to v1.4.4 while nobody here was looking. Two days earlier, the same family of -checks fired **twice in one day**. A claim about another repository goes stale at *that* -repository's pace, and nothing in this repository's own code can notice. +**CI now downloads the repositories those tests need**, and a test fails if CI stops +downloading one that the code says it should. The single repository we cannot download — +because it is private — is named, with the reason, and a decision about it is deferred. -### §2 They only run where the other repository is on disk - -`tests/conftest.py::sibling_repo` resolves a sibling by a declared environment variable and -falls back to the conventional `../` directory. When neither resolves it returns -`None` and the test **skips** — correctly, because on a laptop a missing sibling is normal. - -A GitHub Actions job gets a checkout of **one** repository. So in CI they skipped, and -**continuous integration verified strictly less than a laptop did, precisely on the checks -that cross a repository boundary**. +--- -### §3 Why they stayed dark, which is the more interesting failure +## Context -Four siblings, and until now the decision about which ones CI could fetch lived in a -**comment** in the workflow file. That comment said `views-appwrite` was private. +### §1 What these tests actually do -It is not. It was made public on **2026-08-08** — a deliberate, recorded act in that -repository (`views-appwrite@9d80b75`, *"docs: record going public"*). The comment did not -change with it, so seven checks went on skipping in CI for no reason whatsoever. +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 first draft of this very document, written on 2026-08-09, proposed issuing a credential -to reach those seven — **one day after the fact that justified it had ceased to be true**. -That is not an embarrassing footnote to the decision below; it *is* the argument for it. A -fact about another repository, recorded in prose, with no date and nothing able to check it, -will be wrong and nobody will find out. +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. ---- +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 +checks fired **twice in one day**. -## Decision +### §2 Why they were not running -### §4 Every sibling is declared, and the declaration separates fact from decision +A test like that needs the other repository present. On a laptop all the platform +repositories sit in one folder, so it runs. A GitHub Actions job gets **one** repository, +so it skipped. -`tests/conftest.py` declares each sibling repository with four fields: +The result: **CI verified less than a laptop did, exactly on the checks that span two +repositories** — the ones no single repository can replace. -| field | kind | meaning | -|---|---|---| -| `env` | declaration | the environment variable that overrides its location | -| `public` | **fact** | its visibility on GitHub — not ours to decide | -| `public_checked` | **fact** | the ISO date that visibility was last verified | -| `ci_checkout` | **decision** | whether CI fetches it | -| `note` | reasoning | why not, required whenever `ci_checkout` is false | +### §3 Why nobody noticed for so long -Separating `public` from `ci_checkout` is the point. One is a fact about the world, the -other is a choice we make; §3 is what happens when a single sentence tries to be both. And -`public_checked` is not decoration — every measured claim in this repository carries a date, -and a bare boolean is a fact with no expiry. +Which repositories CI could download was recorded in a **comment** in the workflow file. +That comment said `views-appwrite` was private, so its seven checks were assumed to need +an access credential nobody had issued. -### §5 CI fetches exactly what the declaration says, and a test enforces the agreement +It had gone public on **2026-08-08**. The comment had not changed with it. Seven checks +stayed switched off for no reason at all, and the first draft of this very document +proposed issuing a credential to reach them — written one day after the thing that +justified it stopped being true. -`.github/workflows/run_pytest.yml` checks out every sibling declared `ci_checkout=True`, into -`_siblings/`, pointed at by the declared environment variable. +**That is the whole lesson.** A fact about another repository, written in prose, with +nothing able to check it, will eventually be wrong and nobody will find out. -`tests/test_ci_sibling_coverage.py` fails when the workflow and the declaration disagree **in -either direction** — a declared checkout that is missing, or a checkout nobody declared. Each -rule is a pure function of `(workflow, siblings)` so it can be run against a synthetic broken -world; a rule that can only be demonstrated by editing CI is one nobody ever watches fail. +--- -Today that means `views-appwrite` and `views-crafdapi` are fetched. `views-datafactory` is -public but is **not** fetched: its checks need raw GAUL parquet files that are not in its git -repository, so checking it out replaces an honest skip with a crash — measured, tried and -reverted. `views-faoapi` is the one genuinely private sibling. +## Decision -### §6 A missing sibling that CI declared must turn the build red +### §4 The list of siblings is code, not a comment -Skipping is right on a laptop and wrong in CI once we have declared the repository should be -there. If a checkout silently fails, the checks skip, the build stays green, and we are back -to §2's blindness while looking fixed. +`tests/conftest.py` holds one entry per sibling repository, and each entry says: -The mechanism already existed: -`tests/test_env_declaration.py::test_no_sibling_override_points_at_a_missing_path` fails when -a declared variable is set but resolves to nothing. Because the workflow sets those variables -unconditionally, a failed checkout is a **red build**. Naming it here so nobody deletes it -believing it to be tidiness. +- **`env`** — the environment variable that overrides where to find it; +- **`ci_checkout`** — whether CI downloads it; +- **`note`** — why not, when the answer is no. -That guard had a hole, closed in the same change: it treated a variable set to the **empty -string** as unset, which is exactly what a YAML interpolation resolving to nothing produces — -so the likeliest CI misconfiguration was the one case it could not see. (`Path("").exists()` -is `True`, so the obvious fix makes it worse.) +That is all. It replaces a comment with something a test can read. -### §7 `public` is verified by CI doing it, not by a test — and two rules keep that true +### §5 CI downloads exactly what that list says, and a test enforces it -No test in this suite touches the network. That is a deliberate convention, not an oversight: -what these tests locate is a working copy on disk. +The workflow downloads every sibling marked `ci_checkout=True`. +`tests/test_ci_sibling_coverage.py` fails if the workflow and the list disagree — in +either direction: a repository the list expects and CI does not fetch, or one CI fetches +that the list never mentions. -So `public` is verified by **the checkout itself**. The default `GITHUB_TOKEN` is scoped to -this repository, so a tokenless fetch of a sibling declared public *fails the build* if it is -actually private. +Six rules, and each exists because of something that has actually gone wrong: -Two rules exist solely to keep that argument load-bearing, and both look like fussiness until -you see what they protect: +| | rule | the incident behind it | +|---|---|---| +| G1 | a listed repository is downloaded, and the tests are pointed at it | the tests looked in the wrong place and skipped silently | +| G2 | anything CI downloads appears in the list | the list is the thing people read; CI is not | +| G4 | a repository we skip says why, naming a record | silent non-coverage reads as "nothing to see here" | +| G5 | a download step may not be marked "ignore failures" | one setting and a failed download stops failing the build | +| G6 | downloads land in `_siblings/` | a sibling put elsewhere made the linter report 745 errors in someone else's code | +| G7 | download the sibling's `main` branch | without it you get *their* default branch — which for `views-appwrite` is `development`, not `main` | + +Each rule is a plain function, so each is also run against a deliberately broken example +to prove it objects. A rule only ever tried against a correct file is a rule nobody has +watched fail. + +### §6 A repository CI expects but cannot find turns the build red + +Skipping is right on a laptop, where a missing sibling is normal. It is wrong in CI once +we have said the repository should be there — a silent skip puts us back in §2 while +looking fixed. So CI names the repositories explicitly, and a missing one is a failure. + +That guard existed already +(`tests/test_env_declaration.py::test_no_sibling_override_points_at_a_missing_path`) and +had a hole, closed here: it treated a variable set to an **empty string** as unset, which +is exactly what a mis-typed CI setting produces. So the likeliest misconfiguration was the +one case it could not see. + +### §7 A broken sibling can block merging here, and that is accepted + +This is the real cost, and it should not be buried. CI now depends on two other +repositories. If one of them changes in a way that fails a check — say the configuration +registry moves again — **this repository's builds go red and merges are blocked until +someone updates the pin.** Since merging to `main` here *is* the release to FAO, that +matters. + +It is accepted for three reasons: + +1. The problem being fixed was that these checks were **invisible**. A check that reports + but cannot block is invisible again, just more politely. +2. A red build in that situation is *correct*. It says "re-pin before you ship", and the + fix is minutes. +3. There is an escape. The maintainer administers this repository and can merge over a + failing check when something genuinely urgent is blocked. + +This overrides an earlier recommendation in the risk register (**C-46**) not to couple +per-PR CI to another repository. That recommendation's stated objection was coupling to +another repository's *default branch*, which G7 removes. The remaining coupling is real, +and is the trade above. + +### §8 One repository stays out, and the decision about it is deferred + +`views-faoapi` is private. Downloading it needs a credential, and that is **not** done +here. + +It buys **one** test. That one is admittedly the most valuable of the set — it checks our +delivery is filed under the name the consumer looks for, and when that is wrong nothing +raises an error anywhere: the upload succeeds, storage is paid for, and the consumer's +endpoint is simply empty. But one test does not justify a credential tied to one person, +with an expiry someone must remember, while a better answer is pending. -- a checkout of a **public** sibling must **not** pass a credential — add one to dodge a rate - limit and the field silently becomes an unchecked claim; -- no sibling checkout may carry `continue-on-error` — one key and a failed fetch stops - failing the build. +**The better answer being pursued:** asking FAO to consent to that repository being made +public, which removes the need entirely. Its full history has been examined — no +credentials of any kind, no partner staff email addresses, and an MIT licence already in +place. Two items remain open: internal storage identifiers appear in about twenty files, +and whether the GAUL 2024 boundary data included there may be redistributed. -**The reverse case is not detected.** A private sibling that quietly becomes public will go on -being declared private, and nobody here will notice. That is stated rather than papered over; -it costs a stale `note` and an unnecessary skip, not a wrong result. +**Trigger for revisiting (ADR-014 §4):** issue the credential if FAO declines, or if a +**second** private sibling appears. **Owner:** the maintainer. -### §8 The credential for the one private sibling is deferred +--- -`views-faoapi` is private, and reaching it needs a credential. That is **not** done here. +## Consequences -The cost/benefit is thin: it is **one** test. It is, admittedly, the most valuable single one -— it checks that our delivery is filed under the name the consumer actually filters for, and -that failure produces *no error anywhere*: the upload succeeds, storage is paid for, and the -consumer's endpoint is simply empty. But one test does not justify a credential tied to one -person, with an expiry somebody must track, while a better answer is pending. +**What this buys.** Seven cross-repository checks move from *"run when someone happens to +run them"* to *"run on every change"*. Two of them fired in earnest the week this was +written. Separately, the scan that refuses configuration **values** in this public +repository's documentation now runs on every pull request rather than only on a +maintainer's machine — worth having, since the README carried four such values once. -**The better answer being pursued:** asking FAO to consent to that repository being made -public, which removes the need entirely. An audit of its full history found no credentials of -any kind and no partner staff email addresses; it already carries an MIT licence. Two items -remain open — internal storage identifiers appear in 21 tracked files, and whether GAUL 2024's -terms permit redistribution. +**What it costs.** §7: another repository can block merging here. And the list of siblings +is now something a contributor must keep in step with the workflow, enforced by tests they +may not have read. -**Named trigger (ADR-014 §4).** Issue the credential when **either**: FAO declines, or a -**second** private sibling appears. **Owner:** the maintainer. Should it ever be issued, the -scope floor is one repository, read-only, with a deliberately chosen expiry — the default of -30 days would put this repository back in the dark within a month, with a green build -throughout. +**Where this will go wrong first.** Someone debugging a red build marks a download step +"ignore failures", or moves it out of `_siblings/`. Both are rules for that reason. --- -## Consequences +## What was tried and removed -**What this buys.** Seven cross-repository checks move from *"run when a maintainer happens to -run them"* to *"run on every change"*. Two of them fired in anger in the week before this was -written. One test also changes character: the scan refusing registry **values** in this public -repository's markdown now runs on every pull request rather than only on a maintainer's -machine — worth having, since README.md carried four such values once already. +An earlier version of this design also recorded, for each sibling, whether it was +**public** and the date that was last checked — with a rule pairing visibility against +whether the download used a credential. -**What it costs.** CI now depends on two other repositories being fetchable, so an outage or a -visibility change there turns this repository's build red. That is the intended trade: a red -build is the honest signal, and §6 exists to make sure it is what happens. +A review found the pair circular. The `public` field was read by exactly one rule, and +that rule existed to protect the `public` field's verifiability. Nothing else consulted +either. Both were deleted on 2026-08-10 and no behaviour changed. The date field was worse +than useless: nothing could confirm the check had happened, so it manufactured confidence +rather than recording a fact. -**Where this will go wrong first.** Somebody debugging a red CI adds `continue-on-error` to a -sibling checkout, or a `token:` to make a rate limit go away. Either quietly dismantles §7. -Both are rules in `test_ci_sibling_coverage.py` for that reason. +Visibility now lives in a sibling's `note` — prose, where it belongs, because no test here +can verify it in any case. What still holds without the flag is simpler and needs no +field: a repository CI cannot read fails to download, and G5 keeps that failure loud. -**Second most likely.** A sibling checkout is added to a different workflow file and escapes a -rule scoped to `run_pytest.yml`. The declaration check scans every workflow, not one. +Recorded because the removed design is more tempting than it looks, and because this +document once argued for it. --- ## Alternatives considered -**Leave it, and rely on running the suite by hand.** The status quo, and what registers C-46 -and C-57 carried as an open residual for weeks. It works exactly as well as one person's -habits, which is not a property a safety check should have. +**Leave it and rely on running the suite by hand.** The status quo for weeks. It works +exactly as well as one person's habits, which is not a property a safety check should have. -**Vendor the facts instead of reading them.** Copy the registry edition into this repository -and check the copy. Rejected outright: copies of that registry were the platform's original -failure, and the standing rule is that it is referenced by pinned URL and never copied. A test -reading a local copy compares a thing to itself. +**Copy the facts here instead of reading them.** Rejected outright: copies of that +configuration registry were the platform's original failure, and the standing rule is that +it is referenced and never copied. A test reading a local copy compares a thing to itself. -**Check `public` against the GitHub API from a test.** Rejected. It would be the only network -call in the suite, would need a credential to answer for private repositories — the very thing -under discussion — and would be flaky in exactly the conditions where a green build matters. -Verification-by-doing (§7) is weaker but honest, and its limits are written down. +**Ask GitHub whether a repository is public, from a test.** Rejected. It would be the only +network call in the suite, would need a credential to answer for private repositories — +the very thing in question — and would be unreliable exactly when a green build matters. -**Issue the credential now and fetch all four siblings.** Rejected on the arithmetic in §8: it -buys one test, and a decision that may make it unnecessary is outstanding. +**Issue the credential now and download all four siblings.** Rejected on §8's arithmetic: +it buys one test, and a decision that may make it unnecessary is outstanding. --- -## Appendix — reproducing the measurement - -Counts drift as tests are added; the command does not. From the repository root: +## Appendix — checking it yourself ``` pytest -q -VIEWS_APPWRITE=/nonexistent pytest -q -rs ``` -The first, with the sibling repositories present, is the full suite. The second is what CI -would see without the checkout — and it must **fail**, not merely skip. That is §6. +with the sibling repositories present is the full suite. Then: + +``` +VIEWS_APPWRITE=/nonexistent pytest -q +``` + +is what CI would see without the download — and it must **fail**, not merely skip. That is +§6. diff --git a/tests/conftest.py b/tests/conftest.py index f9ec51b..45db926 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -65,27 +65,24 @@ class Sibling: """What this repository declares about one views-platform sibling (ADR-016). - Two of these fields are different kinds of thing and the distinction is the point. - ``public`` is a **fact about the world** that this repository does not control. - ``ci_checkout`` is a **decision** this repository makes. Conflating them is how the - workflow ended up asserting, in a comment, that ``views-appwrite`` was private for - days after it went public — and how seven checks stayed dark in CI for no reason. - - ``public_checked`` is the date the fact was last verified, and it is not decoration: - every measured claim in this repository carries one. A bare boolean is a fact with no - expiry, which is precisely what went wrong. - - **Keyword-only and frozen, deliberately.** Two adjacent booleans are a one-token slip - between "public, not checked out" and "private, checked out" — the second being the - combination rule G3 exists to forbid. A plain dict would let a missing ``ci_checkout`` - read as ``None``, silently exempting that sibling from every guard; that is the - failure mode this repository has registered more often than any other (C-47, C-57, - #211). Here the omission is a ``TypeError`` at import. + 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. + + **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 + exactly one rule, and that rule existed to protect ``public``'s verifiability. + Nothing else consulted either field, and deleting both changed no behaviour. Whether + a sibling is public is now simply part of ``note`` — prose, where it belongs, because + nothing here could verify it anyway. + + **Keyword-only and frozen.** A plain dict would let a missing ``ci_checkout`` read as + ``None`` and silently exempt that sibling from every rule; that is the failure this + repository has registered more often than any other. Here the omission is a + ``TypeError`` at import. **No validation in ``__post_init__``.** See ``broken_sibling_overrides`` below for what raising at import time costs: one typo became three collection errors and zero - tests run. The rules live in ``tests/test_ci_sibling_coverage.py``, where a violation - is one clean failure and the other four hundred tests still report. + tests run. The rules live in ``tests/test_ci_sibling_coverage.py``. """ #: The environment variable that overrides this sibling's location. Declared, never @@ -93,15 +90,10 @@ class Sibling: #: but a future sibling need not follow the pattern and guessing it would be the #: inference ADR-003 forbids. env: str - #: Visibility on GitHub — a fact about the world, not a decision of ours. - public: bool - #: ISO date ``public`` was last verified. See the class docstring. - public_checked: str - #: Whether CI checks this sibling out. A decision, and the reason for it belongs in - #: ``note`` whenever the answer is no. + #: Whether CI fetches this sibling. ci_checkout: bool - #: Why this sibling is not checked out. Required when ``ci_checkout`` is False, and - #: must name a record, so the non-coverage has an owner rather than a shrug. + #: Why not, when it is not fetched. Required in that case, and must name a record so + #: the non-coverage has an owner rather than a shrug. note: str = "" @@ -114,11 +106,9 @@ class Sibling: SIBLINGS = { "views-datafactory": Sibling( env="VIEWS_DATAFACTORY", - public=True, - public_checked="2026-08-10", ci_checkout=False, note=( - "public, but its checks need the producer's raw GAUL parquets " + "PUBLIC, but its checks need the producer's raw GAUL parquets " "(data/raw/gaul_admin/*.parquet), which are NOT in its git repository. " "Checking it out converts an honest skip into a FileNotFoundError — measured " "2026-08-03, tried and reverted. Closing this needs the data published " @@ -127,17 +117,14 @@ class Sibling: ), "views-appwrite": Sibling( env="VIEWS_APPWRITE", - public=True, - public_checked="2026-08-10", ci_checkout=True, ), "views-faoapi": Sibling( env="VIEWS_FAOAPI", - public=False, - public_checked="2026-08-10", ci_checkout=False, note=( - "the only private sibling. Checking it out needs a credential, which is an " + "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. See ADR-016 and register C-81." @@ -145,8 +132,6 @@ class Sibling: ), "views-crafdapi": Sibling( env="VIEWS_CRAFDAPI", - public=True, - public_checked="2026-08-10", ci_checkout=True, ), } diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index ce8c31c..dd0538a 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -10,14 +10,17 @@ once**, in `tests/conftest.py::SIBLINGS`, and this file fails when the workflow and that declaration disagree — in either direction. -**What verifies the `public` field, since no test here does.** Nothing in this suite -touches the network; that is a hard convention (`conftest`'s docstring: what is located -is a working copy on disk). `public` is verified by CI *doing it*: the default -`GITHUB_TOKEN` is scoped to this repository, so a tokenless checkout of a sibling -declared public fails the build if it is actually private. Rules G3 and G5 exist to keep -that true — one `token:` or one `continue-on-error:` and the claim silently stops being -checked. The reverse case (a private sibling quietly becoming public) is **not** detected -here, and ADR-016 §7 says so rather than implying coverage it does not have. +**Whether a sibling is public is not modelled here, and that is deliberate.** An earlier +version carried a `public` flag and a rule pairing it with the presence of a checkout +token. A review found the pair circular — the field was read by exactly one rule, and +that rule existed to protect the field's verifiability — so both were removed on +2026-08-10 without any change in behaviour. Visibility now lives in a sibling's `note`, +as prose, because nothing here could verify it in any case: no test in this suite touches +the network. + +What still holds without the flag: a checkout of a repository CI cannot read simply fails +the build, because the default `GITHUB_TOKEN` is scoped to this repository. Rule G5 keeps +that failure loud. **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 @@ -28,7 +31,6 @@ from __future__ import annotations -import datetime as dt from pathlib import Path import pytest @@ -154,39 +156,6 @@ def _g2_every_checkout_is_declared( return problems -def _g3_tokens_match_visibility( - workflow: dict, siblings: dict[str, Sibling] -) -> list[str]: - """Private ⇒ must pass a token. Public ⇒ must NOT. - - The second half is the one that matters and the one that looks pointless. It is what - keeps ``public`` verified-by-doing: a tokenless checkout of a private repo fails the - build. Add a token to a public sibling — to dodge a rate limit, say — and the field - becomes an unchecked claim with nothing anywhere to catch it. - """ - problems = [] - for step in _sibling_steps(workflow): - name = _repo_name(step) - sibling = siblings.get(name) - if sibling is None: - continue # G2's business - has_token = "token" in step["with"] or "ssh-key" in step["with"] - if not sibling.public and not has_token: - problems.append( - f"{name}: declared private but checked out with no credential — the " - "step will fail. Either it is public now (update the declaration and its " - "date) or it needs a token." - ) - if sibling.public and has_token: - problems.append( - f"{name}: declared public but the checkout passes a credential. That " - "removes the only thing verifying the `public` field — a tokenless " - "checkout failing when a repo is private. Drop the token, or the claim " - "is unverified." - ) - return problems - - def _g4_non_coverage_is_explained( workflow: dict, siblings: dict[str, Sibling] ) -> list[str]: @@ -220,9 +189,10 @@ def _g5_no_step_swallows_its_own_failure( ) -> list[str]: """No sibling checkout may carry ``continue-on-error``. - One key, and the entire verification-by-doing argument becomes false: the checkout of - a repo that turned private fails, the build stays green, and the checks it enables - skip. It is also exactly the key someone reaches for while debugging a red CI. + One key and a failed fetch stops failing the build: the checks that sibling enables + go back to skipping, silently, on a green run. That is the state this whole file + exists to leave behind, and it is exactly the key someone reaches for while debugging + a red CI. """ return [ f"{_repo_name(step)}: sibling checkout carries continue-on-error, so a failed " @@ -266,58 +236,13 @@ def _g7_siblings_are_taken_from_main( ] -#: How far ahead of "today" a check date may sit before it is an error rather than a -#: clock difference. See `_g8_the_visibility_fact_carries_a_usable_date`. -_CLOCK_SKEW = dt.timedelta(days=1) - - -def _g8_the_visibility_fact_carries_a_usable_date( - workflow: dict, siblings: dict[str, Sibling] -) -> list[str]: - """``public_checked`` parses, and is not meaningfully in the future. - - **One day of slack, and it is not laziness — this rule failed CI on its first run.** - The dates were stamped from a maintainer's machine in CEST at 01:25 on 2026-08-10; - the runner was in UTC, where it was still 23:25 on 2026-08-09. A date recorded - truthfully today read as tomorrow two hours away, and the guard called it a lie. - - "Today" is not a fact a date alone determines — it depends on where the reader is — - so comparing a bare ISO date against `date.today()` is comparing two different - questions. A full day of tolerance covers every real timezone offset, and a date more - than a day ahead is still what this rule is for: a fact nobody actually checked. - - Recorded here rather than fixed silently, because the alternative repair a hurried - reader would reach for is deleting the rule (ADR-014 §3: when a guard cries wolf, - check the matching before the scope). - """ - problems = [] - horizon = dt.date.today() + _CLOCK_SKEW - for name, sibling in siblings.items(): - try: - checked = dt.date.fromisoformat(sibling.public_checked) - except ValueError: - problems.append( - f"{name}: public_checked={sibling.public_checked!r} is not an ISO date. " - "A fact with an unreadable date is a fact with no date." - ) - continue - if checked > horizon: - problems.append( - f"{name}: public_checked={sibling.public_checked} is more than a day " - "ahead of today — that is not clock skew, it is a date nobody checked." - ) - return problems - - _RULES = { "G1 declared checkouts are present and pointed at": _g1_declared_checkouts_are_present_and_pointed_at, "G2 every checkout is declared": _g2_every_checkout_is_declared, - "G3 tokens match visibility": _g3_tokens_match_visibility, "G4 non-coverage is explained": _g4_non_coverage_is_explained, "G5 no step swallows its own failure": _g5_no_step_swallows_its_own_failure, "G6 siblings land under the excluded path": _g6_siblings_land_under_the_excluded_path, "G7 siblings are taken from main": _g7_siblings_are_taken_from_main, - "G8 the visibility fact carries a usable date": _g8_the_visibility_fact_carries_a_usable_date, } @@ -437,34 +362,26 @@ def _replace(name: str, **changes) -> dict[str, Sibling]: _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"), - ("G2 every checkout is declared", _workflow(), {}, "checked out but undeclared"), - ("G3 tokens match visibility", - _workflow(**{"with": {"token": "${{ secrets.X }}"}}), _ONE, - "public sibling fetched with a credential — kills verification-by-doing"), - ("G4 non-coverage is explained", - _workflow(), _replace("views-appwrite", ci_checkout=False, note=""), - "not checked out and no note"), - ("G4 non-coverage is explained", - _workflow(), _replace("views-appwrite", ci_checkout=False, note="because reasons"), - "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"), - ("G6 siblings land under the excluded path", - _workflow(**{"with": {"path": "vendor/views-appwrite"}}), _ONE, - "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"), - ("G8 the visibility fact carries a usable date", - _workflow(), _replace("views-appwrite", public_checked="2099-01-01"), - "the fact was checked far in the future"), - ("G8 the visibility fact carries a usable date", - _workflow(), _replace("views-appwrite", public_checked="last tuesday"), - "unparseable date"), + ("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"), + ("G2 every checkout is declared", + _workflow(), {}, "checked out but undeclared"), + ("G4 non-coverage is explained", + _workflow(), _replace("views-appwrite", ci_checkout=False, note=""), + "not checked out and no note"), + ("G4 non-coverage is explained", + _workflow(), _replace("views-appwrite", ci_checkout=False, note="because reasons"), + "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"), + ("G6 siblings land under the excluded path", + _workflow(**{"with": {"path": "vendor/views-appwrite"}}), _ONE, + "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"), ] @@ -530,23 +447,3 @@ def test_the_repos_own_checkout_is_not_mistaken_for_a_sibling(): own_checkout_only = {"jobs": {"test": {"steps": [{"uses": "actions/checkout@v3"}]}}} assert not _g2_every_checkout_is_declared(own_checkout_only, SIBLINGS) assert not _sibling_steps(own_checkout_only) - - -def test_g8_tolerates_a_timezone_but_not_a_fiction(): - """The boundary either side of `_CLOCK_SKEW`, pinned. - - A rule with a tolerance needs its tolerance tested, or the number drifts into being - whatever made the last failure go away. Tomorrow is a clock difference; the day after - is a claim about a check that has not happened. - """ - today = dt.date.today() - for offset, should_object in ((0, False), (1, False), (2, True), (400, True)): - siblings = _replace( - "views-appwrite", - public_checked=(today + dt.timedelta(days=offset)).isoformat(), - ) - violations = _g8_the_visibility_fact_carries_a_usable_date(_workflow(), siblings) - assert bool(violations) is should_object, ( - f"a check date {offset} day(s) ahead of today: expected " - f"{'an objection' if should_object else 'no objection'}, got {violations}" - ) From fb10cfee3d0e1c81965995877d688c9718c15145 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Mon, 10 Aug 2026 03:20:04 +0200 Subject: [PATCH 2/4] docs(adr): ADR-017, and a falsification pass over both ADRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-017 decides what this repository does about a fact held in a repository it cannot read. The rule: the fact is declared in the public coordinate registry, each side verifies itself against it, neither reads the other's source. No credential, for any number of private APIs. Written for the category, per the maintainer's brief that private consumer APIs are permanent even if views-faoapi itself goes public. ADR-016 rescoped to say plainly that it only covers repositories CI can download, with the private case handed to ADR-017 rather than deferred. A falsification pass over every section of both documents found seven defects, all fixed: - the heading said 'four sentences' over a body of three, after the rescope removed one - ADR-016 still cited a private register ID in a document meant to stand alone - 'Rejected on §8's arithmetic' pointed at arithmetic the rewrite had removed - 'sibling' was used seventeen times and never defined - ADR-017 makes ADR-016's views-crafdapi download pointless — that fetch serves exactly one test, and ADR-017 replaces it. Neither document said so; both now do - §2's ownership claim is evidenced by ADR-013 for the FAO partner only; CRAF'd is not mentioned there at all. Now admitted, and folded into the registry work - ADR-016 rejects 'copy the facts here' while ADR-017 keeps a copy. Reconciled: what is forbidden is a copy that is ALSO the thing checked against The fix for the fourth broke the first by landing between two sentences; caught on re-run. Suite 399 passed / 40 xfailed; ruff clean. Nothing merged. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 74 +++-- .../017_facts_across_a_private_boundary.md | 288 ++++++++++++++++++ docs/ADRs/README.md | 12 +- tests/conftest.py | 4 +- 4 files changed, 348 insertions(+), 30 deletions(-) create mode 100644 docs/ADRs/017_facts_across_a_private_boundary.md 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 35b824f..2071597 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -1,22 +1,36 @@ -# ADR-016: CI checks out the sibling repositories our tests read +# ADR-016: Which sibling repositories CI downloads **Status:** Accepted **Date:** 2026-08-10 **Decider:** Simon Polichinel von der Maase +**Scope:** repositories CI **can** download — that is, public ones. What to do when a +repository **cannot** be downloaded is a different decision and is +[ADR-017](017_facts_across_a_private_boundary.md). **Related:** [ADR-014](014_claims_and_the_guards_that_carry_them.md) §2 (a guard nobody has watched fail is decoration) and §4 (a deferral names a trigger and an owner) --- -## The decision in four sentences +## The decision in three sentences A few of this repository's tests read **other** repositories to check that things we say about them are still true. Those tests only work when the other repository is on disk, so -they ran on a developer's laptop and skipped in CI. +they ran on a developer's laptop and skipped in CI. **CI now downloads the repositories +those tests need**, and a test fails if CI stops downloading one that the code says it +should. -**CI now downloads the repositories those tests need**, and a test fails if CI stops -downloading one that the code says it should. The single repository we cannot download — -because it is private — is named, with the reason, and a decision about it is deferred. +*Throughout, a "sibling" is another repository in the views-platform organisation that sits +beside this one in a developer's folder — never a dependency we install.* + +## What this document does not cover + +Downloading a repository requires being able to read it, so **everything here applies only +to repositories that are public.** A private one cannot be downloaded by our CI at all, +and no amount of workflow configuration changes that. + +That is not a gap in this decision; it is a different problem with a different answer, and +that answer is [ADR-017](017_facts_across_a_private_boundary.md). This document stops at +the boundary and says so, rather than implying a coverage it does not have. --- @@ -94,6 +108,12 @@ Each rule is a plain function, so each is also run against a deliberately broken to prove it objects. A rule only ever tried against a correct file is a rule nobody has watched fail. +**One of these downloads is temporary, and it is worth knowing which.** `views-crafdapi` is +fetched for exactly one test. [ADR-017](017_facts_across_a_private_boundary.md) §7 replaces +that test with one that reads a public declaration instead — at which point this download +buys nothing and should go. That is a decision this document cannot make on its own, which +is why it is recorded there and cross-referenced here. + ### §6 A repository CI expects but cannot find turns the build red Skipping is right on a laptop, where a missing sibling is normal. It is wrong in CI once @@ -123,30 +143,26 @@ It is accepted for three reasons: 3. There is an escape. The maintainer administers this repository and can merge over a failing check when something genuinely urgent is blocked. -This overrides an earlier recommendation in the risk register (**C-46**) not to couple -per-PR CI to another repository. That recommendation's stated objection was coupling to -another repository's *default branch*, which G7 removes. The remaining coupling is real, -and is the trade above. - -### §8 One repository stays out, and the decision about it is deferred +This overrides an earlier internal recommendation not to couple per-pull-request CI to +another repository at all. That recommendation's stated objection was coupling to another +repository's *default branch* — which G7 removes, by naming `main` explicitly instead of +accepting whatever default the other repository happens to be set to. The coupling that +remains is real, and is the trade described above. -`views-faoapi` is private. Downloading it needs a credential, and that is **not** done -here. +### §8 One repository stays out, and why that is not this document's problem -It buys **one** test. That one is admittedly the most valuable of the set — it checks our -delivery is filed under the name the consumer looks for, and when that is wrong nothing -raises an error anywhere: the upload succeeds, storage is paid for, and the consumer's -endpoint is simply empty. But one test does not justify a credential tied to one person, -with an expiry someone must remember, while a better answer is pending. +`views-faoapi` is private. Our CI cannot download it, so the one check that reads it does +not run here. -**The better answer being pursued:** asking FAO to consent to that repository being made -public, which removes the need entirely. Its full history has been examined — no -credentials of any kind, no partner staff email addresses, and an MIT licence already in -place. Two items remain open: internal storage identifiers appear in about twenty files, -and whether the GAUL 2024 boundary data included there may be redistributed. +**No credential is issued to work around that**, and that is a decision rather than an +omission — see [ADR-017](017_facts_across_a_private_boundary.md), which settles what this +repository does about facts held in repositories it cannot read. In short: the fact gets +declared somewhere public that both sides can read, so neither side needs access to the +other. -**Trigger for revisiting (ADR-014 §4):** issue the credential if FAO declines, or if a -**second** private sibling appears. **Owner:** the maintainer. +Until that declaration exists, the affected check runs on a maintainer's machine and not +in CI. That is stated in the sibling's `note`, which rule G4 requires and which must name +the record that owns it. --- @@ -201,8 +217,10 @@ it is referenced and never copied. A test reading a local copy compares a thing network call in the suite, would need a credential to answer for private repositories — the very thing in question — and would be unreliable exactly when a green build matters. -**Issue the credential now and download all four siblings.** Rejected on §8's arithmetic: -it buys one test, and a decision that may make it unnecessary is outstanding. +**Issue an access credential and download the private one too.** Rejected, and the +reasoning is [ADR-017](017_facts_across_a_private_boundary.md) §9 rather than anything here: +a credential is the wrong shape of answer to a standing category, and there is a route that +needs no credential at all. --- diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md new file mode 100644 index 0000000..443dd2b --- /dev/null +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -0,0 +1,288 @@ +# ADR-017: Facts shared with a repository we cannot read + +**Status:** Proposed +**Date:** 2026-08-10 +**Decider:** Simon Polichinel von der Maase +**Scope:** what this repository does when it depends on a fact held in a repository it +cannot download — today because that repository is private, and in principle for any +reason. The companion decision, [ADR-016](016_ci_read_access_to_private_siblings.md), +covers repositories we *can* download. +**Related:** [ADR-003](003_authority_of_declarations_over_inference.md) (declarations over +inference), [ADR-013](013_sampled_forecast_wire_contract.md) §4.1a (the field in question), +[ADR-014](014_claims_and_the_guards_that_carry_them.md) §1 and §4 + +--- + +## The decision in one sentence + +**A fact shared across a boundary we cannot see through is declared in a public place both +sides read, and neither side reads the other's source code.** + +--- + +## Context + +### §1 The failure this is about + +When this repository delivers forecasts, it uploads each file with a label — a store +document field called `name`. The consuming API filters every one of its queries on that +label. + +If our label and their filter ever stop matching: + +- the upload **succeeds**; +- the file is stored, and paid for; +- the consumer's endpoint returns **empty**; +- **nothing, anywhere, raises an error.** + +ADR-013 §4.1a states it plainly: a document uploaded under the wrong name is *"invisible to +the consumer, not merely degraded."* Every system involved reports success while the +partner receives nothing. + +This is the failure mode that most deserves an automatic check, precisely because no human +or machine downstream will notice it. + +### §2 Who owns the label, which is not what you would guess + +ADR-013 §4.1a also says: the name is **"Config-owned by views-faoapi; changing it is a +contract amendment, not a deploy detail."** + +So the consuming API owns the value. This repository holds a **copy** of it — the constant +`CONSUMER_DOCUMENT_NAME` in each partner's `product.py`, which is what we actually write +onto every upload. + +That direction matters for everything below. The question is not *"can we see into their +repository"*. The question is **"they own a fact, we keep a copy, and how do we know the +copy is still right?"** + +**One honest limit on that claim.** ADR-013 states the ownership only for the FAO API — its +table names *"the literal faoapi model name"*, and the CRAF'd partner arrived later and is +not mentioned there at all. So consumer-ownership is written down for one partner and +merely assumed for the other. §7 treats them the same, which is almost certainly right, but +"almost certainly" is not a citation. **Settling that is part of the registry work**: a +declaration naming both partners makes the ownership explicit for both, which is a second, +smaller reason to do it. + +### §3 How we check it today, and why that stops working + +The existing check opens the consumer's source, finds the name their API filters on, and +compares it with our copy. + +That works when the consumer's code is on the same machine — true on a maintainer's laptop, +where all the platform repositories sit in one folder. It is impossible in continuous +integration when the consumer is private, because a CI job cannot download a private +repository without a credential. + +So the check runs when someone happens to run it, and not on the change that would break +it. + +### §4 Why this is a standing category and not a one-off + +Two consuming APIs exist. `views-crafdapi` is public; `views-faoapi` is private. Across the +whole organisation, **15 of 33 repositories are private** — private is ordinary, not +exceptional. + +The maintainer's instruction is explicit: `views-faoapi` may or may not become public one +day, but **there will be one or more private APIs at any given time**. So this document +must decide for the category, not for today's instance. + +--- + +## Decision + +### §5 The rule + +> **A fact shared across a public/private boundary is declared in the public contract +> surface. Each side verifies itself against that declaration. Neither side reads the +> other's source code.** + +Concretely, for the delivery label: + +1. The value is declared once, in the platform's public coordinate registry + (`views-appwrite`, `docs/ADRs/platform/coordinate_registry.toml`). +2. **This repository checks its copy against that declaration.** No access to the consumer + is required, so the check runs in CI on the change that could break it. +3. **The consuming API checks its own code against the same declaration.** It needs no + access to us either — this repository is public — so that half is free for it too. + +**"But you rejected copying."** [ADR-016](016_ci_read_access_to_private_siblings.md) turns +down keeping a local copy of the shared registry, on the grounds that a test reading a copy +compares a thing to itself. This is not that, and the difference is the whole point. + +We keep a copy of the label because we have to — it is written onto every upload, so it +must exist in our code. What ADR-016 forbids is a copy that is *also the thing we check +against*. Here the copy is checked against an authority held elsewhere, which is what makes +the check mean something. A copy nobody verifies is the failure; a copy verified against a +declared authority is just a value with a source. + +### §6 Why the coordinate registry, rather than somewhere new + +Four reasons, and the fourth is the one that makes this cheap: + +- **It is the right kind of place.** The registry already holds cross-repository facts + about the delivery seam, already distinguishes secret entries from non-secret ones, and + already records which repository consumes what. +- **It is public**, so a private consumer can read it without anyone granting anything. +- **It has a change process** and an amendment log, so a change to the label is a recorded + contract amendment — which is what ADR-013 §4.1a already says it should be. +- **This repository's CI already downloads and reads that file on every run.** Adding this + check costs no new machinery, no credential, and no new dependency. + +A label is not a secret. Knowing that documents are named `un_fao` grants no access to +anything; access is controlled by keys held elsewhere. So this belongs with the non-secret +coordinates and must never be placed among the secret entries. + +### §7 The rule applies to both partners, not only the private one + +`views-crafdapi` is public, so our check *can* read its source, and does today. It will +move to the registry anyway. + +The reason is not tidiness. If only the private case moves, the platform ends up with two +mechanisms doing one job — one reading a declaration, one reading source code — and a +future contributor has to know which partner uses which. One mechanism, applied uniformly, +is the point of having a rule at all. + +It also means this repository stops reading any consumer's source, which removes a +dependency on another repository's *file layout* — something we were never entitled to +depend on. + +**And it retires a download.** [ADR-016](016_ci_read_access_to_private_siblings.md) has CI +fetch `views-crafdapi`, and measurement shows that fetch serves **exactly one test** — this +one. Once the check reads the registry instead, that download buys nothing, and +`views-crafdapi` should be marked as not fetched, with a note saying why. Stated here +because a download that has quietly stopped earning its place is how the previous version +of all this went wrong, and because ADR-016 cannot know it: the decision that obsoletes it +is this one. + +### §8 What this does **not** verify, stated plainly + +This checks **our copy against the declaration**. It does **not** check the consumer's code +against the declaration. + +So if the registry says `un_fao`, and we write `un_fao`, and the consuming API quietly +starts filtering on something else — our check passes, and the delivery is still invisible. + +That half is genuinely the consumer's to verify, and cheap for it (see §5.3). But it is +outside this repository's control, and there is a case where it may never exist: **a +private API operated by a third party**, who has no obligation to run any test of ours. + +**In that case the fact is not verifiable from here, and must be recorded as such** — an +accepted blind spot with a named owner, not a check that quietly covers nothing. Pretending +otherwise would be the exact failure ADR-014 §1 exists to prevent. + +### §9 What is deliberately not built + +- **No access credential** — no personal access token, no machine account, no GitHub App. + A credential solves one repository at a time and must be maintained forever, for a + category that grows. It is the wrong shape of answer to a standing problem, and it puts a + rotation obligation on one person. +- **No moving our checks into another repository's test suite.** It looks attractive — + the private side can read us for free — but it fails on its own extension case: a + third-party-operated API will not run our tests, and we would have no lever. A rule that + breaks on its second instance is not a rule. +- **No new file, service, or endpoint.** If a fact does not fit the existing registry, that + is information about the fact, not a reason to build a second surface. +- **No abstraction over "how to reach a private repository."** There is one such + repository, and the decision above is that we do not reach it. + +### §10 When to revisit + +- **A shared fact that genuinely cannot go in the registry** — for instance one that is + itself sensitive. Then this rule has met a case it does not cover, and the case is the + evidence needed to decide the next thing. +- **A third-party-operated private API becomes a real consumer**, making §8's blind spot + concrete rather than hypothetical. +- **The registry stops being public**, which would invalidate §6 entirely. + +**Owner:** the maintainer. None of these is a deadline; each is an event. + +--- + +## Consequences + +**What this buys.** The check that guards the invisible failure runs automatically, on the +change that could cause it, for every partner — with no credential, now or ever, however +many private APIs appear. Adding partner number three costs one registry row. + +**What it costs.** A change to a delivery label now requires editing a file in a third +repository. That is slower than editing a constant here, and deliberately so: ADR-013 +already calls this a contract amendment rather than a deploy detail, and this makes the +process match the words. + +**A dependency moves rather than disappearing.** We stop depending on the consumer's file +layout and start depending on the registry. That is a better dependency — declared, +versioned, public, with a change process — but it is not nothing, and if the registry +becomes unreliable this check inherits that. + +**Where this will go wrong first.** Someone changes the label in the consuming API and not +in the registry. Our check keeps passing, because our copy still matches the declaration, +and the delivery goes invisible exactly as before. **That is §8, and it is the whole +residual risk of this design.** It is why the consumer-side check is part of the rule and +not an optional extra. + +--- + +## Alternatives considered + +### Issue an access credential so our CI can read the private repository + +**Rejected.** It works, and it keeps both halves of the check in one place — genuinely its +strongest property. + +But the brief is a permanent category. A token is tied to a person, expires, and must be +re-scoped for each new private repository; a GitHub App is a substantially larger piece of +infrastructure to own. Either way the cost recurs per repository and per rotation, forever, +to verify a handful of short strings. And when it lapses, the checks skip and the build +stays green — the exact failure the whole of ADR-016 was written to end. + +### Move our check into the private repository's test suite + +**Rejected**, though it was the leading candidate for some time. + +The private repository can read this public one for free, so the check would cost no +credential and would fire in a real pipeline. Attractive, and correct today, when one +person maintains both. + +It fails on the case that defines the category. A private API operated by a **third party** +will not run a conformance test on our behalf, and we would have no way to require it. The +rule would then hold only for repositories we happen to operate — which is not a rule about +private APIs, it is a rule about our own repositories wearing a general-sounding name. + +### Accept the gap and check nothing + +**Rejected as the destination, though it is the honest description of where we are until +the registry entry lands.** The failure it leaves uncovered is the invisible one, and this +repository has already delivered an artifact nobody could find once. + +Worth stating because it is the fallback if the registry route stalls: the position would +then be *"this is not verified, here is the failure mode, here is who owns it"* — an +accepted blind spot, recorded. Never a check that appears to cover it and does not. + +--- + +## Appendix A — the facts behind §4 + +Measured 2026-08-10. Counts drift; the commands do not. + +``` +gh repo list views-platform --limit 60 --json name,visibility +``` +returned 33 repositories, 15 of them private. + +Consuming APIs: `views-crafdapi` public, `views-faoapi` private. `views-productionapi` and +`views-publicapi` are named in platform records but do not exist yet, so no default +visibility can be inferred for future APIs. + +## Appendix B — the work this decision requires, and in what order + +1. **views-appwrite** — declare the label for each partner in the coordinate registry. + Filed as views-appwrite#75. Nothing here can proceed before it. +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. +3. **views-faoapi** — verify its own served label against the declaration; §8's other half. + Filed as views-faoapi#379. + +Step 3 is not a precondition for steps 1 and 2, and the sequence is safe to interrupt: the +existing check keeps running on a maintainer's machine throughout. diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index d866caf..b8fd52b 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -94,7 +94,7 @@ These ADRs form the architectural constitution of the repository. demand and supply — under which it is revisited. Arises from #146 and the þing-02 ratification, which asked that the reasoning live here rather than in an issue. -- **ADR-016** — Cross-Repository Checks in CI, and How Siblings Declare Their Visibility +- **ADR-016** — Which Sibling Repositories CI Downloads A handful of tests here verify claims this repo makes about *other* repos — chiefly that the coordinate-registry edition we pinned is the one that exists. They need the sibling on disk, so they skipped in CI and ran only on a laptop. Each sibling is now declared with its @@ -104,6 +104,16 @@ These ADRs form the architectural constitution of the repository. for a dated declaration rather than prose. The credential for the one genuinely private sibling is deferred with a named trigger. +- **ADR-017** — Facts Shared With a Repository We Cannot Read + The delivery label this repository writes is owned by the consuming API and mirrored here; + if the two drift the upload succeeds, the file is stored, and the consumer's endpoint is + empty with no error anywhere. Verifying the mirror currently means reading the consumer's + source, which is impossible in CI when that consumer is private — and private consumer APIs + are a standing category, not a one-off. Decides that such a fact is declared in the public + coordinate registry and each side verifies itself against it, so neither reads the other's + source. No credential, for any number of private APIs. States plainly the half it does not + cover: the consumer's own code against its own declaration. + ADRs numbered 010 and above define: - Domain-specific decisions diff --git a/tests/conftest.py b/tests/conftest.py index 45db926..b29d9e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -127,7 +127,9 @@ class Sibling: "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. See ADR-016 and register C-81." + "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." ), ), "views-crafdapi": Sibling( From 169b97696d71f3dc2f41e3e335a15e0c225bf932 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Mon, 10 Aug 2026 03:48:17 +0200 Subject: [PATCH 3/4] =?UTF-8?q?docs(adr):=20second=20falsification=20pass?= =?UTF-8?q?=20=E2=80=94=20three=20more=20defects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New probes, not a re-run of the first pass. Three findings, all fixed: - ADR-016 is Accepted and asserted the outcome of ADR-017, which is Proposed. It said ADR-017 'settles' the private case and that its §7 'replaces' a test. An accepted decision cannot depend on an unaccepted one. Softened to conditional throughout, and the scope line now states plainly that nothing in ADR-016 depends on the proposal being adopted — if it is rejected, the private case returns as an open question. - §5's rule table runs G1, G2, G4, G5, G6, G7. Two numbers are missing and nothing said why. The removed-design section now names G3 and G8 as the removed rules and records that survivors keep their original names rather than being renumbered. - '(see §5.3)' reads as a subsection that does not exist — the ADR's own convention is §N, and §4.1a elsewhere refers to a real ADR-013 subsection. Rewritten as prose. Probes that failed to falsify, recorded because a passed probe is evidence: every factual claim verified (15 of 33 repositories private; the ADR-013 quote is verbatim; the registry does carry consumer fields; CI does read it), and the label/name/CONSUMER_DOCUMENT_NAME terminology is defined on first use and consistent thereafter. All ten findings from both passes re-checked together; no regressions. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 28 ++++++++++++------- .../017_facts_across_a_private_boundary.md | 2 +- 2 files changed, 19 insertions(+), 11 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 2071597..4e90b15 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -4,8 +4,10 @@ **Date:** 2026-08-10 **Decider:** Simon Polichinel von der Maase **Scope:** repositories CI **can** download — that is, public ones. What to do when a -repository **cannot** be downloaded is a different decision and is -[ADR-017](017_facts_across_a_private_boundary.md). +repository **cannot** be downloaded is a different decision, taken in +[ADR-017](017_facts_across_a_private_boundary.md) — **which is Proposed, not yet accepted.** +Nothing below depends on that proposal being adopted: if it is rejected, the private case +returns here as an open question, and the only thing this document loses is a pointer. **Related:** [ADR-014](014_claims_and_the_guards_that_carry_them.md) §2 (a guard nobody has watched fail is decoration) and §4 (a deferral names a trigger and an owner) @@ -109,9 +111,9 @@ to prove it objects. A rule only ever tried against a correct file is a rule nob watched fail. **One of these downloads is temporary, and it is worth knowing which.** `views-crafdapi` is -fetched for exactly one test. [ADR-017](017_facts_across_a_private_boundary.md) §7 replaces -that test with one that reads a public declaration instead — at which point this download -buys nothing and should go. That is a decision this document cannot make on its own, which +fetched for exactly one test. [ADR-017](017_facts_across_a_private_boundary.md) §7 proposes +replacing that test with one that reads a public declaration instead — at which point this +download would buy nothing and should go. That is a decision this document cannot make on its own, which is why it is recorded there and cross-referenced here. ### §6 A repository CI expects but cannot find turns the build red @@ -155,10 +157,11 @@ remains is real, and is the trade described above. not run here. **No credential is issued to work around that**, and that is a decision rather than an -omission — see [ADR-017](017_facts_across_a_private_boundary.md), which settles what this -repository does about facts held in repositories it cannot read. In short: the fact gets -declared somewhere public that both sides can read, so neither side needs access to the -other. +omission. Where that question *is* decided is +[ADR-017](017_facts_across_a_private_boundary.md), currently proposed: it argues the fact +should be declared somewhere public that both sides can read, so that neither needs access +to the other. Should that proposal not be adopted, the position here reverts to a stated +gap with a named owner — never a check that appears to cover it and does not. Until that declaration exists, the affected check runs on a maintainer's machine and not in CI. That is stated in the sibling's `note`, which rule G4 requires and which must name @@ -190,7 +193,12 @@ An earlier version of this design also recorded, for each sibling, whether it wa whether the download used a credential. A review found the pair circular. The `public` field was read by exactly one rule, and -that rule existed to protect the `public` field's verifiability. Nothing else consulted +that rule existed to protect the `public` field's verifiability. + +**This is why §5's table runs G1, G2, G4, G5, G6, G7 with two numbers missing.** The +removed rules were **G3** (visibility must match whether a credential was used) and **G8** +(the date must be a plausible one). The remaining rules keep their original names rather +than being renumbered, so that anything written about "G6" still means G6. Nothing else consulted either. Both were deleted on 2026-08-10 and no behaviour changed. The date field was worse than useless: nothing could confirm the check had happened, so it manufactured confidence rather than recording a fact. diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index 443dd2b..6b1d7a7 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -162,7 +162,7 @@ against the declaration. So if the registry says `un_fao`, and we write `un_fao`, and the consuming API quietly starts filtering on something else — our check passes, and the delivery is still invisible. -That half is genuinely the consumer's to verify, and cheap for it (see §5.3). But it is +That half is genuinely the consumer's to verify, and cheap for it — it is the third item in §5's list, and needs no access to us because this repository is public. But it is outside this repository's control, and there is a case where it may never exist: **a private API operated by a third party**, who has no obligation to run any test of ours. From a837c634c5f74440d8b7a022cade8da86c9942d7 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Mon, 10 Aug 2026 03:56:43 +0200 Subject: [PATCH 4/4] docs(adr): accept 016 and 017, and fix two findings from the diff review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ADRs accepted. ADR-017 moves Proposed -> Accepted, and ADR-016's conditional language about it resolves with it: the scope line no longer hedges on whether the proposal is adopted, and §8 now says the decision is accepted but not yet implemented, because it waits on a declaration landing in another repository's registry. /review-diff over the branch found two, both prose, both this repo's named defect class: - a docstring in test_ci_sibling_coverage said "eight rules find nothing to complain about" after the cut left six. Removed the count rather than correcting it — a number nobody checks is a claim with a half-life, which is C-82's whole finding. - ADR-017 §5 stated in the present tense that this repository checks its copy against the registry declaration. It does not: test_product.py still reads the consumer's source and will until the declaration exists. Appendix B and ADR-016 §8 both recorded it as pending, so the documents were honest taken together, but §5 read alone stated unbuilt work as fact. Now says plainly that none of the three steps is in place and that the present tense describes the decided end state. Probes that found nothing, recorded because a passed probe is evidence: no orphaned reference to the deleted `public`, `public_checked`, G3, G8 or the clock-skew constant anywhere in tests, docs or workflows; broken_sibling_overrides verified against unset, empty, whitespace-only, bad-path and valid inputs with the docstring matching each; all six rules carry a mutant and each mutant fires its own rule. Suite 399 passed / 40 xfailed; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 21 ++++++++++--------- .../017_facts_across_a_private_boundary.md | 8 ++++++- tests/test_ci_sibling_coverage.py | 2 +- 3 files changed, 19 insertions(+), 12 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 4e90b15..83dcf4d 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -5,9 +5,7 @@ **Decider:** Simon Polichinel von der Maase **Scope:** repositories CI **can** download — that is, public ones. What to do when a repository **cannot** be downloaded is a different decision, taken in -[ADR-017](017_facts_across_a_private_boundary.md) — **which is Proposed, not yet accepted.** -Nothing below depends on that proposal being adopted: if it is rejected, the private case -returns here as an open question, and the only thing this document loses is a pointer. +[ADR-017](017_facts_across_a_private_boundary.md) and accepted alongside this one. **Related:** [ADR-014](014_claims_and_the_guards_that_carry_them.md) §2 (a guard nobody has watched fail is decoration) and §4 (a deferral names a trigger and an owner) @@ -111,9 +109,10 @@ to prove it objects. A rule only ever tried against a correct file is a rule nob watched fail. **One of these downloads is temporary, and it is worth knowing which.** `views-crafdapi` is -fetched for exactly one test. [ADR-017](017_facts_across_a_private_boundary.md) §7 proposes -replacing that test with one that reads a public declaration instead — at which point this -download would buy nothing and should go. That is a decision this document cannot make on its own, which +fetched for exactly one test. [ADR-017](017_facts_across_a_private_boundary.md) §7 replaces +that test with one that reads a public declaration instead — at which point this download +buys nothing and goes. That has been decided and is not yet done; it waits on the +declaration landing in the registry. That is a decision this document cannot make on its own, which is why it is recorded there and cross-referenced here. ### §6 A repository CI expects but cannot find turns the build red @@ -158,10 +157,12 @@ not run here. **No credential is issued to work around that**, and that is a decision rather than an omission. Where that question *is* decided is -[ADR-017](017_facts_across_a_private_boundary.md), currently proposed: it argues the fact -should be declared somewhere public that both sides can read, so that neither needs access -to the other. Should that proposal not be adopted, the position here reverts to a stated -gap with a named owner — never a check that appears to cover it and does not. +[ADR-017](017_facts_across_a_private_boundary.md): the fact gets declared somewhere public +that both sides can read, so that neither needs access to the other. + +**That decision is accepted but not yet implemented** — it depends on a declaration being +added to the platform registry, which is another repository's work. Until then the affected +check runs on a maintainer's machine and not in CI, and the sibling's `note` says so. Until that declaration exists, the affected check runs on a maintainer's machine and not in CI. That is stated in the sibling's `note`, which rule G4 requires and which must name diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index 6b1d7a7..e7d8bb0 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -1,6 +1,6 @@ # ADR-017: Facts shared with a repository we cannot read -**Status:** Proposed +**Status:** Accepted **Date:** 2026-08-10 **Decider:** Simon Polichinel von der Maase **Scope:** what this repository does when it depends on a fact held in a repository it @@ -105,6 +105,12 @@ Concretely, for the delivery label: 3. **The consuming API checks its own code against the same declaration.** It needs no access to us either — this repository is public — so that half is free for it too. +**None of those three is in place yet**, and the present tense above describes the decided +end state rather than today's behaviour. Today the check still reads the consumer's source +and still skips in CI. The order the three land in, and what is blocked on what, is +Appendix B. Said here because a decision record that reads as a description of the code is +how this repository has repeatedly ended up believing work was done. + **"But you rejected copying."** [ADR-016](016_ci_read_access_to_private_siblings.md) turns down keeping a local copy of the shared registry, on the grounds that a test reading a copy compares a thing to itself. This is not that, and the difference is the whole point. diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index dd0538a..10b2785 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -253,7 +253,7 @@ def test_the_workflow_this_file_scans_actually_parses(): """A YAML restructure would empty every scan below and report success. This is by far the most likely way this file becomes decoration: rename the job, nest - `steps` differently, move the workflow, and eight rules find nothing to complain + `steps` differently, move the workflow, and every rule below finds nothing to complain about. So assert there is something to scan before trusting that there was nothing wrong. """