Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 56 additions & 8 deletions docs/ADRs/016_ci_read_access_to_private_siblings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
78 changes: 71 additions & 7 deletions docs/ADRs/017_facts_across_a_private_boundary.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
13 changes: 13 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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."
),
),
}

Expand Down
36 changes: 24 additions & 12 deletions tests/test_ci_sibling_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down
Loading