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 83dcf4d..ef27514 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -99,10 +99,10 @@ Six rules, and each exists because of something that has actually gone wrong: |---|---|---| | 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" | +| G4 | a repository we skip says why, and **every** note names a record | silent non-coverage reads as "nothing to see here" — and a note nothing points at is the prose this file replaces, whether it explains an exclusion or a temporary inclusion | | 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` | +| G7 | download the sibling's `main` branch | a sibling's default branch is not ours to rely on — views-appwrite's was `development` as of 2026-08-10, and is theirs to change again | 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 @@ -135,17 +135,65 @@ registry moves again — **this repository's builds go red and merges are blocke someone updates the pin.** Since merging to `main` here *is* the release to FAO, that matters. -It is accepted for three reasons: +It is accepted for two 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. + fix is a small edit rather than an investigation. + +**A third reason was offered and withdrawn, because it was not true.** An earlier draft +said the maintainer administers this repository and can therefore merge over a failing +check when something is genuinely urgent. Two external reviewers challenged it, and it does +not survive measurement: the `protect_main` ruleset lists **zero bypass actors**, and a +ruleset applies to everyone except the actors named there — so administrator status confers +no exemption. There is no classic branch protection either, hence no `enforce_admins` route. + +So **there is currently no escape hatch**, and the coupling here is accepted without one. +That is defensible — the two reasons above stand on their own — but it should be a chosen +position rather than a surprise on the day it matters. Adding a bypass actor is a console +change and would restore the third reason; it has not been made. And an override of that +kind would be one person's judgement, available only while that person is — the same +habit-dependence this document criticises in its own alternatives. + +### §7a How often this actually bites + +*"A small edit"* reads differently at once a quarter than at once a day, so the rate +belongs here rather than in a reader's imagination. + +views-appwrite reports its registry moved through **five editions in four days** — v1.4.0 +on 2026-08-02 through v1.4.4 on 2026-08-05. **Four of the five were observation-driven**, +recording what a console showed or correcting a key's scopes, and carried no obligation for +any consumer. Under this section each would have reddened this repository and blocked a +release until someone re-pinned. + +That is not an argument against the decision; it is the honest size of it. It also points +at a better shape, which that repository has volunteered to make usable: pin against the +contract **version** and treat observation-only bumps as non-blocking, rather than pinning +a commit and blocking on every edit — the upstream amendment log already marks which bumps +carry obligations. **Not adopted here**, because it needs the upstream side first and this +document should not decide another repository's format. + +### §7b Two kinds of coupling, and only one is a tripwire + +A reader could take G7 as *"always track the moving tip of `main`"*, which would sit oddly +beside the rest of the platform, where consumers pin the seam contract by tag and never by +branch. Both live here, and they answer different questions: + +- **Drift tripwires** read the sibling's `main` *because* movement is the signal. The + registry-edition check is one: if the registry moved and our pin did not, we want to know, + and a red build is the entire point. +- **Reachability checks** verify that a *pinned* commit is an ancestor of the sibling's + `main`. Here movement is noise; what matters is that what we pinned was ratified rather + than taken from someone's unmerged branch. + +G7 makes both read `main` rather than a default branch. It does not make either of them +track the tip for its own sake. -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 +--- + +This section 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. diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index e7d8bb0..fd85af5 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -99,14 +99,47 @@ must decide for the category, not for today's instance. 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`). + (`views-appwrite`, `docs/ADRs/platform/coordinate_registry.toml`) — **subject to + views-appwrite accepting the registry as the home; the request is views-appwrite#75.** + A store-document name is a contract fact rather than a coordinate, so admitting it + widens that registry's charter, and that is not this repository's call to make. (That + seat has since reviewed this document and agreed to implement it. The wording stands + anyway: the decision should rest on its own reasoning, not on assent presumed in + advance.) 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. + access to us either — **because the registry is public**, not because this repository + is. The consumer never reads us at all, which is the symmetry the rule is really about: + neither side reads the other; both read a public third place. + +### The order these land in is part of the decision, not an afterthought **None of those three is in place yet**, and the present tense above describes the decided -end state rather than today's behaviour. Today the check still reads the consumer's source +end state rather than today's behaviour. + +**The obvious sequence has a hole, and it is green.** If step 1 lands, then step 2 replaces +the source-reading check with a registry read, and step 3 has not happened yet, the state +is: the registry declares a string a human typed; we check our copy against that string and +pass; the consumer checks nothing; and the check that *did* consult the consumer's real +source has been deleted. The build is green and what it proves is that **two values this +platform authored agree with each other.** That is strictly weaker than today, and it is +precisely the objection ADR-016 raises against copies — during that window the registry is +not an authority, it is a third copy, and nothing closes the loop. + +So the sequence is constrained: **step 2 adds the registry check but does not remove the +source-reading one. The source-reading check is removed only when step 3 lands**, and until +then it keeps running wherever it can — in CI for the public partner, on a maintainer's +machine for the private one. Slightly redundant for a while, and redundancy is the correct +price for not having a window where the only thing verifying a delivery label is our own +typing. + +**One assumption worth stating rather than relying on.** The registry is versioned, so the +two sides could in principle read it at different editions and both pass while disagreeing. +That is not a live risk here because the label is contract-immutable — changing it is an +amendment, which produces a new edition both sides re-pin to, and our pin's reachability is +already checked. Recorded because an unstated assumption carrying a silent-failure mode is +what §1 is about. 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. @@ -191,6 +224,19 @@ otherwise would be the exact failure ADR-014 §1 exists to prevent. - **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. +**Scope of the prohibition, narrowed after review.** Everything above is about **declarable +facts** — short values that can sit in a registry row. There is a second kind of shared +thing this rule does not reach: agreement about **behaviour**, such as whether two +independently written readers interpret a reserved entry the same way. You cannot put a +program's behaviour in a row, so §5's mechanism does not transfer, and a blanket "never a +credential" would leave that case with no answer at all. + +The rule does extend, but one level up: **declare the semantics rather than the code** — +state in the contract what a reader must do, and each repository tests its own reader +against that statement. Still no repository reads another's source, and still no +credential. That question is live elsewhere as views-models#327 / D-05, and this document is +an argument for settling it there rather than here. + ### §10 When to revisit - **A shared fact that genuinely cannot go in the registry** — for instance one that is @@ -283,12 +329,30 @@ 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. 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. + Filed as views-faoapi#379, and the consumer-side maintainer has accepted it. + +4. **views-crafdapi** — the same self-check for the other partner. **Not yet filed at the + time of writing, and that gap matters**: §7 retires the crafdapi fetch, so without this + leg the CRAF'd label would end up checked against a registry row that nobody checks + against CRAF'd's actual code — no verification against reality at all, which is worse + than the status quo. §7 argues the rule must apply uniformly; uniform application means + uniformly filing the third leg. Sequenced after that partner's data contract settles, + per step 1. + +**Step 3 is a precondition for removing the old check**, though not for adding the new one +— see the sequencing note in §5. The order is safe to interrupt at any point: until step 3 +lands for a partner, that partner's source-reading check stays, so no window exists in which +the label is verified only against our own typing. diff --git a/tests/conftest.py b/tests/conftest.py index b29d9e3..2fe5ccd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -118,6 +118,13 @@ class Sibling: "views-appwrite": Sibling( env="VIEWS_APPWRITE", ci_checkout=True, + note=( + "PUBLIC. Fetched, and the fetch is load-bearing rather than convenient: it " + "carries the coordinate-registry drift checks, and under ADR-017 it becomes " + "the authority source for the delivery-label check too. Pruning it as an " + "unused sibling would disable both and leave a green build — the invisible " + "skip ADR-016 §6 exists to prevent." + ), ), "views-faoapi": Sibling( env="VIEWS_FAOAPI", @@ -135,6 +142,12 @@ class Sibling: "views-crafdapi": Sibling( env="VIEWS_CRAFDAPI", ci_checkout=True, + note=( + "PUBLIC, and fetched **temporarily**. Measured: this fetch serves exactly one " + "test, the consumer-name pin. ADR-017 §7 replaces that test with a read of " + "the public registry, at which point this fetch buys nothing and should be " + "removed along with it. Until then it is real coverage, not decoration." + ), ), } diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index 10b2785..6952307 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -156,30 +156,42 @@ def _g2_every_checkout_is_declared( return problems -def _g4_non_coverage_is_explained( +def _g4_every_note_names_a_record( workflow: dict, siblings: dict[str, Sibling] ) -> list[str]: - """Every sibling NOT checked out says why, and names a record. + """A sibling NOT checked out must say why, and **every** note must name a record. A deferral needs a trigger and an owner (ADR-014 §4). Requiring the note to cite a `C-nn` or an `ADR-nnn` is what makes the citation non-optional, so declared non-coverage stays attached to something a reader can follow. + + **Widened 2026-08-10 after external review.** The rule used to look only at siblings + with ``ci_checkout=False``, which left a hole in exactly the place ADR-016 calls out: + `views-crafdapi` is fetched *temporarily*, until the check it serves moves to a + registry read, and that fact lived in a note **no rule examined**. A claim about + another repository, in prose, that nothing can check — which is the failure ADR-016 + §3 diagnoses, reintroduced for the one download the document itself calls temporary. + + So the note requirement now follows the *note*, not the flag: whenever a sibling + carries one, it must cite the record that owns it, whether it explains an exclusion + or a temporary inclusion. Siblings with no note and `ci_checkout=True` are unaffected + — a permanent, unremarkable fetch needs no justification. """ problems = [] for name, sibling in siblings.items(): - if sibling.ci_checkout: - continue - if not sibling.note.strip(): + if not sibling.ci_checkout and not sibling.note.strip(): problems.append( f"{name}: not checked out in CI and no note says why. Silent " "non-coverage reads as 'nothing to see here'." ) - elif not any( - token in sibling.note for token in ("C-", "ADR-") + continue + if sibling.note.strip() and not any( + token in sibling.note for token in ("C-", "ADR-", "#") ): problems.append( - f"{name}: its note explains the non-coverage but names no record. Cite " - "the register entry or ADR that owns it, so the deferral has an owner." + f"{name}: its note explains something but names no record. Cite the " + "entry, ADR or issue that owns it — a note nothing points at is the " + "prose this file exists to replace." ) return problems @@ -239,7 +251,7 @@ def _g7_siblings_are_taken_from_main( _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, - "G4 non-coverage is explained": _g4_non_coverage_is_explained, + "G4 every note names a record": _g4_every_note_names_a_record, "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, @@ -367,10 +379,10 @@ def _replace(name: str, **changes) -> dict[str, Sibling]: _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", + ("G4 every note names a record", _workflow(), _replace("views-appwrite", ci_checkout=False, note=""), "not checked out and no note"), - ("G4 non-coverage is explained", + ("G4 every note names a record", _workflow(), _replace("views-appwrite", ci_checkout=False, note="because reasons"), "note explains but names no record"), ("G5 no step swallows its own failure",