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
19 changes: 13 additions & 6 deletions docs/ADRs/016_ci_read_access_to_private_siblings.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ this repository makes about a different repository** is still true.
The clearest example. Each partner's `appwrite_env.py` records which edition of the
shared configuration registry it was verified against, by naming a commit in another
repository. Tests open that repository at that commit and ask whether anything this
package reads has moved since — a rename, a reclassification, a rotated value, or a new
coordinate that has arrived. *(Until 2026-08-11 they asked a cruder question — whether the
edition **label** still matched — which fired on every upstream edit. See §7b's erratum.)*
package reads has moved since — a rename, a reclassification, a rotated value. *(Until
2026-08-11 they asked a cruder question — whether the edition **label** still matched —
which fired on every upstream edit. A second attempt also asked whether any coordinate had
**arrived**, which fired on other repositories' rows and was deleted on 2026-08-12. See
§7b's erratum.)*

This is not hypothetical housekeeping. On 2026-08-05 that test failed — the registry had
moved to a new edition while nobody here was looking. Two days earlier, the same family of
Expand Down Expand Up @@ -248,9 +250,14 @@ So the two categories above become three:
- **Reachability checks** read a pinned commit; movement is noise.
- **Differential tripwires** read **both** — a pinned edition as the baseline, the
sibling's `main` as the comparison — and fire when something *we declare* differs between
them, or when a coordinate arrives in a table we read. This is the shape the registry
check now has. It catches a rotation, which is invisible to the other two, and it is
silent through prose edits and version bumps.
them. This is the shape the registry check now has. It catches a rotation, which is
invisible to the other two, and it is silent through prose edits and version bumps.

*(Erratum, 2026-08-12. This paragraph read "…or when a coordinate **arrives** in a table
we read" and called that the check's current shape. That half was deleted the same day: it
compared every row of every table this package depends on, so another repository's key
reddened this one. The cost of removing it — a coordinate issued **for** this package now
goes unnoticed until a human reads the registry — is carried in register C-90.)*

*(Corrected twice on 2026-08-11, and the second time is the instructive one. The first
implementation compared against the sibling's **working tree** — so a developer whose clone
Expand Down
88 changes: 71 additions & 17 deletions reports/technical_risk_register.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,27 +334,35 @@ Cross-refs: **C-57** (the scan's own entry and its history), **C-93** (why the a

---

### C-90: The replacement drift checks assert less than their docstrings say, and one of their mutation proofs is a tautology
### C-90: A mutation proof that cannot fail, and the untested function a module was extracted to create

| Field | Value |
|-------|-------|
| ID | C-90 |
| Tier | 2 — this is the entry PR #239 was written to close, reopened by the code that closed it. It reinstates release-blocking false alarms on the path that is this project's production release, and it does so under a docstring saying the opposite. |
| Source | `/code-review max` on PR #239 post-merge, 2026-08-11; verified by direct measurement against views-appwrite `origin/main` |
| Trigger | views-appwrite declares any new coordinate in `connection`, `target`, `secret` or `contract` — including one belonging to another repository entirely. On the current registry that is 8 of 25 rows' worth of surface. |
| Trigger | **Both remaining halves are proof defects, not runtime ones.** (a) Someone mutates `_unclassified_tables` or `_TABLE_ROLE` and believes the tautological proof covers it. (b) Someone changes `registry_current` — the reason `tests/seam_registry.py` exists — and the suite stays green. *(The original trigger, an unrelated coordinate arriving upstream, died with `arrived` in #245.)* |
| Owner | This repository. |
| Location | `tests/test_env_declaration.py:732` (`arrived`), `:890` (the tautology), `tests/seam_registry.py:113` (`registry_current`). |
| Location | `tests/test_env_declaration.py` — `test_the_table_partition_would_catch_a_new_table_and_a_vanished_one` (the tautology); `tests/seam_registry.py::registry_current` (untested). Function names, not line numbers: this entry has cited stale ones before. |

**`arrived` is not filtered by the names this package reads.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use.
**~~`arrived` is not filtered by the names this package reads.~~ RESOLVED 2026-08-12 (#245) — deleted; see the mitigation below. Left visible because the reasoning it prompted is the entry's most useful part.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use.

That is the exact failure class C-86 records and that PR #239 was written to remove, and the same test's docstring seven lines above says **"Silent through: prose edits, `[meta]` bumps, and rows belonging to anyone else."** The prose describes the check that was designed; the code implements a wider one.

There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt — `[contract.*]` arrived exactly that way and nothing else here would have seen it. That argues for table-granularity on arrival and row-granularity on change. If that is the intent it is defensible, and then the docstring is wrong; if the docstring is right, the code is. **They cannot both stand.**
There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt. That argues for table-granularity on arrival and row-granularity on change. **They cannot both stand.**

**DECIDED 2026-08-12 (#245): the docstring won, and the cost is real.** The arrival half was deleted. A coordinate views-appwrite issues *for this package* — or a second `[contract.*]` row for a future partner such as views-productionapi — is now **silent** until a human reads the registry: no test, no run-time assert, nothing. `assert_env_declared` cannot see it, because it iterates the names this package already declares. That is the accepted price of not being reddened by every unrelated row, and it is recorded here rather than left to be discovered. *(The half's own defence — that `[contract.*]` "arrived exactly this way and nothing else here would have seen it" — was false at table granularity, where the partition check catches it, and true at row granularity, which is exactly the cost now accepted.)*

**The partition's mutation proof cannot fail.** `assert not _unclassified_tables(base)` where `base = {name: {} for name in _TABLE_ROLE}` reduces to `set(_TABLE_ROLE) - set(_TABLE_ROLE)`, empty for every possible input. Its message — *"the real registry's tables must all classify"* — asserts a fact about a file this test never opens. It is decoration inside the test whose own docstring is about removing decoration.

**`registry_current` has no test.** The module `tests/seam_registry.py` was extracted for one reason: two copies of the reader disagreed about whether to read the sibling's `main` or its working tree, and reading the working tree is issue #196 verbatim. The function that settles it is called by five tests and is the subject of none. Replacing its body with `rev-parse HEAD` — the defect it exists to prevent — leaves the suite at its exact baseline. Three of its error branches are executed by nothing.

**Partial mitigation 2026-08-12 (#245) — the false-alarm half is gone; the two proof defects are not.**

`arrived` is **deleted**. Measured before deleting: each partner reads 13 of the 25 rows in the tables this package depends on, and 8 of those rows belong to no repository here — so the check subscribed this repository to another repo's changelog. Mutation-proven after: an unrelated API key and a third partner's contract row are silent; a rotation and a removal still fire. The docstring and the code now agree, and the stopping rule sits above the check.

**Still open, and routed to #246**: the partition's mutation proof is a tautology (`base` is built from `_TABLE_ROLE`, so `_unclassified_tables(base)` is empty for every possible input), and `registry_current` — the function `tests/seam_registry.py` was extracted to create — has no test, so replacing it with a working-tree read leaves the suite at baseline. This entry stays open until both land.

Cross-refs: **C-86** (whose partial-mitigation paragraph this falsifies), **C-89** (the sibling defect in the no-copy scan), **C-93**, **C-91**, ADR-014 §1/§2, issue #196.

---
Expand Down Expand Up @@ -509,18 +517,64 @@ description, where deferrals go to be forgotten:
Cross-refs for this amendment: **C-57** (the drift detector this rides on), ADR-016 §7a/§7b,
views-appwrite#76 (**delivered**, registry v1.6.0).

**⚠ CORRECTED 2026-08-12 — the partial mitigation above overstates what shipped.** The
paragraph beginning *"Partial mitigation 2026-08-11"* says the replacement checks "match on
the facts this repository actually declares". **One of the three does not.** The arrival
half of the drift check is computed over every row of every table this package depends on,
with no filter for the names it reads — 8 of 25 rows on the live registry belong to other
repositories. So the false-alarm class this entry records is *narrowed*, not removed:
observation-only edits no longer fire it, but any new coordinate for any repo still does.

The measurement quoted above (v1.4.4 → v1.5.2, three editions, green) remains true and was
not the wrong measurement — none of those editions added a row. It was simply not a
measurement of the case that now fires. Registered as **C-90**, and this entry's rate claim
should be re-stated once that is fixed rather than before.
**⚠ CORRECTED 2026-08-12, and PARTLY RESOLVED the same day (#245).** The paragraph
beginning *"Partial mitigation 2026-08-11"* claimed the replacement checks "match on the
facts this repository actually declares". **Two of the three did not**, and an earlier
draft of this correction said one — the review that caught it is the reason this paragraph
is longer than it wants to be.

**The first, now fixed.** The drift check's arrival half ran over every row of every table
this package depends on, with no filter for the names it reads. Measured on the live
registry: each partner reads **13 of 25** rows, and **6 belong to no repository here** —
three caller keys for other consumers, and three platform key slots still marked
`status = "planned"`, one of which names *"un_fao delivery"* among the identities it would
be issued for. C-90's original wording was "belong to no repository here"; an earlier draft
of this paragraph upgraded it to "other repositories entirely", which is the same
overstatement this paragraph exists to correct, two sentences from correcting it. The other two rows this package does not
read are its own delivery labels — both declare `producer = "views-postprocessing"` — and
they are covered by `tests/test_product.py`, not by this check. *An earlier draft of this
paragraph said "8 belong to no repository here", which upgraded a careful claim in C-90
into a false one and propagated it to four places.*

**And it never actually fired in anger.** Measured: **zero rows have arrived in the
depended-on tables since the pin** — the arrival half was green on every real edition it
ever saw, from its introduction on 2026-08-11 to its deletion on 2026-08-12. An earlier
draft said an upstream key "reddened this repository and blocked a release". That is a
mutation result written in the past tense. What is true: a mutation shows it *would* fire,
and it *would* block once C-81's required check lands — `protect_main` today carries
`deletion, non_fast_forward, pull_request` and no required status check, which is this
entry's own "latent rather than live" paragraph.

**The arrival half is now deleted** (#245). The drift check fires on exactly one condition —
a row this partner declares differs between the pinned edition and the sibling's `main` —
and that stopping rule is written above the check itself. Its self-defence turned out to be
false: it claimed `[contract.*]` *"arrived exactly this way and nothing else here would have
seen it"*, but `[contract]` is a top-level **table**, caught by the partition check directly
above it.

Mutation-proven both ways against the live registry: an unrelated API key and a third
partner's contract row are now **silent**; a rotation and a removal of a row this partner
reads still **fire**. A new test asserts the silence direction, so a third widening meets
an objection rather than a paragraph.

*(That silence test needed two attempts, and the first is worth recording because it is
this entry's own disease. It called `_describe_changes` directly — the helper underneath
the check — and was worthless: re-adding the deleted arrival half to the real check left
the whole suite green, because that half never lived in the helper the test was asking.
The "mutation proof" offered for it was invalid too: it mutated the test rather than the
code, which proves nothing. It now drives the real check through monkeypatched readers,
and re-adding the arrival half fails it. **A guard must be pointed at the thing it claims
to guard**, which is the sentence this whole entry keeps re-learning.)*

**The second inaccuracy stands, and is this entry's remaining rate risk.** *"Every
top-level table upstream must be classified here"* fires on any new table regardless of
whether this repository declares anything about it — and **it has already fired for exactly
that reason**: `[edition]` arrived at v1.6.0, an edition the registry's own `[meta]` calls
*"additive and opt-in; obliges nobody"*, and it reddened this repository. That event is
recorded **earlier** in this entry — the partition "went red on it unprompted", "which is
the whole reason that check is directional" — and that record is true of the *detection*
and silent about the *cost*. Adopting `[edition].obliges_consumers` is the deferral that
would fix it, and its trigger is below.


---
Expand Down
Loading
Loading