From c6529e5509171dae9a554ca550751ab0bf1bfccd Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 22:52:54 +0200 Subject: [PATCH] =?UTF-8?q?docs(claims):=20#249=20=E2=80=94=20ten=20stale?= =?UTF-8?q?=20claims,=20and=20one=20guard=20so=20the=20worst=20class=20sto?= =?UTF-8?q?ps=20recurring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All ten re-verified against the tree as it stands, not taken from the issue. TWO GARBLED EDIT-SPLICES in tests/test_env_declaration.py — a `#:` block opening mid-sentence with its antecedent gone and a stranded trailing line, and a docstring reading "not our business, which is an IGNORED table disappearing is silent". Both were the residue of deleting a sentence from the middle of a paragraph. TWO WORKING-TREE CLAIMS, in run_pytest.yml and G7's docstring, describing readers that were moved to the sibling's `main` on 2026-08-11. Zero read the working tree now. ADR-017's "None of those three is in place yet" is DELETED rather than updated, which is #250's §7b applied on its first day: an ADR states a decision, not the current state of the code. Updating it would have been the churn §7b exists to stop. ADR-016's `public?` column keeps its honest paragraph and gains the trigger and owner ADR-014 §4 requires — re-read it when a repository changes visibility or CI fails to check one out — plus the reason it is deliberately not machine-checked: verifying it means a network call from a suite that makes none, and the failure it would catch already fails loudly at the checkout step. THE þing MIS-CITATION IS CORRECTED AT ALL THREE SITES. The ruling forbidding integration tests against the production Appwrite project is þing-01 D2, not þing-02 D2, it is CONDITIONAL, and it GRANTS read-only preflight validation. Citing the wrong verdict is how a permission read as a prohibition for weeks — and one of the two corrected sites was arguing that a preflight could not be built. `[test_environment]` stays IGNORED, but its comment no longer says "a fact about the platform, not about this package". It is the clause that says which live checks this package may build. Reading it mechanically waits on C-91, since its rows are bare strings. AND THE BLOB URL SHA NOW HAS A GUARD, which is the point of doing this story at all. The docstrings publish a registry URL whose sha is a third copy of the pin, and only the version was compared. That is how an annotated tag reached the pin: git peeled it, every check passed, and two PUBLIC modules published a link returning 404 three lines above the sentence "a pinned URL does not rot". Third time this class has bitten. Mutation-proven by desynchronising the URL from the constant. Suite 417 passed / 1 skipped / 40 xfailed, ruff clean. Closes #249. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/run_pytest.yml | 2 +- .../016_ci_read_access_to_private_siblings.md | 6 +++ .../017_facts_across_a_private_boundary.md | 3 -- reports/technical_risk_register.md | 10 ++-- tests/test_ci_sibling_coverage.py | 2 +- tests/test_env_declaration.py | 51 +++++++++++++++---- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/.github/workflows/run_pytest.yml b/.github/workflows/run_pytest.yml index 7e7b354..f259bf3 100644 --- a/.github/workflows/run_pytest.yml +++ b/.github/workflows/run_pytest.yml @@ -40,7 +40,7 @@ jobs: # `actions/checkout` takes the sibling's OWN default branch — which for # views-appwrite is `development` — while ADR-014 §3 says the authority for a claim # about another repository is that repository's `main`. Two checks in - # test_env_declaration read the working tree and a third demands reachability from + # test_env_declaration read the sibling's `main` and a third demands reachability from # `main`; pointed at `development` they would eventually demand contradictory things. # # `actions/checkout` refuses a path outside $GITHUB_WORKSPACE, so the siblings go 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 bb0cba9..3179400 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -126,6 +126,12 @@ The **`public?` column is not, and cannot be** — which is why `public` was del because a reader needs it to follow the argument. Nothing reads this table; if it drifts from `SIBLINGS`, only a human will notice. +**Trigger, since an unenforced claim needs one (ADR-014 §4):** re-read this column the next +time a repository in it changes visibility, or the next time CI fails to check one out. The +owner is whoever makes that change. It is deliberately not machine-checked — verifying it +means a network call from a test suite that makes none, and the failure it would catch +(a tokenless checkout of something now private) already fails loudly at the checkout step. + ### §5 CI downloads exactly what that list says, and a test enforces it The workflow downloads every sibling marked `ci_checkout=True`. diff --git a/docs/ADRs/017_facts_across_a_private_boundary.md b/docs/ADRs/017_facts_across_a_private_boundary.md index b580595..80ae053 100644 --- a/docs/ADRs/017_facts_across_a_private_boundary.md +++ b/docs/ADRs/017_facts_across_a_private_boundary.md @@ -142,9 +142,6 @@ Concretely, for the delivery label: ### 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. - **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 diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 948621e..e10fa5b 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -203,7 +203,9 @@ Cross-refs: **C-57** (the exclusion and why), **C-89** (the traceback amplificat The second half is a **permission**, and this repository spent weeks believing the whole clause was a prohibition. Reclassify and read it, or record why a permission that changes what we may build is not a fact we depend on. -Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the mis-citation that compounded it), issue #249. +**Partly addressed 2026-08-12 (#249).** The classification stays `IGNORED` — nothing here reads the table, and its rows are bare strings rather than sub-tables, so feeding it to `rows()` would raise (C-91). What changed is the *reason*: the comment said "a fact about the platform, not about this package", which is what let a permission read as a prohibition. It now says the table governs which live checks this package may build, and points at C-95 and C-96. Reading it mechanically waits on C-91. + +Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the mis-citation that compounded it), **C-91** (why it is not read yet), issue #249. --- @@ -216,7 +218,7 @@ Cross-refs: **C-94** (the mechanism this permission authorises), **C-95** (the m | Source | `/expert-code-review` of the standing decisions, 2026-08-12 | | Trigger | Anyone reasons from the integration-test prohibition — for a preflight, a drill, or a new ADR. | | Owner | This repository. | -| Location | `reports/technical_risk_register.md` (~:475, ~:1628, ~:1717); anywhere else citing þing-02 D2 for this. | +| Location | `reports/technical_risk_register.md` — corrected at all three sites 2026-08-12 (#249); this entry is the record, and the remaining mentions of `þing-02 D2` are its own narration. | This register cites **þing-02 D2** for the ruling that integration tests against the production Appwrite project are forbidden. þing-02 D2 is about identity and key separation. The ruling is **þing-01 D2** (`þingit/01_identity_secrets_config/orð_dómr.md:53-61`), and it differs from the paraphrase in two ways that matter: it is **conditional** (*"until the operator creates one"*), and it **grants** read-only preflight validation as the permitted live check. It also records that creating a test project is **assigned to the operator** and gates the provisioning-path drill — an open assignment, not a closed door. @@ -647,7 +649,7 @@ The FAO delivery authenticates with the `UN FAO` key. That key expires **2026-11 **What this repo can and cannot do.** It cannot rotate anything; it holds no credentials and must not (þing-01 D3). What it can do is fail early and legibly rather than mid-delivery — and it does not currently. `appwrite_env.py` validates that the declared variables are *present*, which an expired key still is. An expired key is indistinguishable from a valid one until the first request comes back unauthorised, by which point a delivery is part-way through. -**Deliberately not fixed here, and the reason is C-84's own shape.** A preflight that checks key validity means an authenticated call at startup, and the only project to make it against is production — which þing-02 **D2** forbids for tests and this would not quite be. The honest position is that this is a *date to act on*, not a mechanism to build, and inventing a mechanism would be building the wrong thing to feel busy. Registered so the date is not discovered by an outage. +**Deliberately not fixed here, and the reason is C-84's own shape.** A preflight that checks key validity means an authenticated call at startup, and the only project to make it against is production — which **þing-01 D2** forbids for tests and this would not quite be — and which that verdict explicitly permits as *read-only preflight validation*, so the obstacle here is the authenticated call, not the prohibition (see C-95). The honest position is that this is a *date to act on*, not a mechanism to build, and inventing a mechanism would be building the wrong thing to feel busy. Registered so the date is not discovered by an outage. Cross-refs: **C-81** (the same operator session's other half — branch protection and the CI token), **C-27** (no rotation mechanism for a secret value upstream), **C-57** (the pinned-registry detector, which is how this arrived here at all — it demanded the v1.4.4 bump and the bump is what surfaced the expiry), þing-02 A3(i), views-appwrite C-65 and C-66. @@ -1935,7 +1937,7 @@ Cross-refs: C-15 (the provenance this field serves), C-22 (the recall process th Replaced by tests of `contract/historical.assert_metadata_complete` — the code that actually gates a delivery — parametrised over the **imported** `METADATA_COLS`, plus source-scan pins that the gate stays at build time and is still invoked. Verified 2026-08-02: `pytest -q tests/test_validation.py` → **14 passed**. Mutation-tested: narrowing the gate to a single column fails **9 of 14**; the old suite passed that mutation untouched, because it was not testing the gate. -**Residual 2 — the enrich→validate end-to-end test — RELOCATED to #18**, per the Register Conventions' relocation rule (a relocation is not complete until the destination exists and is cited by number). Every *leg* is now covered — enrichment (**no longer a leg**: `GaulLookupEnricher` and `test_enrichment.py` were deleted in #90/B3b, the lookup join having moved into the frame build; recorded here because the closure above was argued from a list this deletion shortened), artifact build (`test_historical_builder.py`, 7), reader parity (`test_historical_parity.py`, 3), the invariants on primitives (`test_input_integrity_e2e.py`, 8), the wire end to end (`test_hop_b_sink_e2e.py`, 6), the null-gate (`test_validation.py`, 14). What remains uncovered is **the manager orchestrating them**, which needs views-pipeline-core and a production-like Appwrite environment — and þing-02 **D2** forbids integration tests against the production project, no non-production one existing. +**Residual 2 — the enrich→validate end-to-end test — RELOCATED to #18**, per the Register Conventions' relocation rule (a relocation is not complete until the destination exists and is cited by number). Every *leg* is now covered — enrichment (**no longer a leg**: `GaulLookupEnricher` and `test_enrichment.py` were deleted in #90/B3b, the lookup join having moved into the frame build; recorded here because the closure above was argued from a list this deletion shortened), artifact build (`test_historical_builder.py`, 7), reader parity (`test_historical_parity.py`, 3), the invariants on primitives (`test_input_integrity_e2e.py`, 8), the wire end to end (`test_hop_b_sink_e2e.py`, 6), the null-gate (`test_validation.py`, 14). What remains uncovered is **the manager orchestrating them**, which needs views-pipeline-core and a production-like Appwrite environment — and **þing-01 D2** forbids integration tests against the production project while no non-production one exists (see C-95 — the ruling is conditional, and creating that project is an open operator assignment). That gap has **two standing trackers already**, which is why keeping a third here is noise rather than signal: issue **#18** (open since 2026-06-04) and `tests/test_falsification_campaign_3_5.py`, an `xfail(strict)` probe that **flips to XPASS the moment someone writes the test** — a self-surfacing tracker, which is more than this entry was doing. | | Tier | 3 | diff --git a/tests/test_ci_sibling_coverage.py b/tests/test_ci_sibling_coverage.py index ee0238a..66f5d0a 100644 --- a/tests/test_ci_sibling_coverage.py +++ b/tests/test_ci_sibling_coverage.py @@ -251,7 +251,7 @@ def _g7_siblings_are_taken_from_main( Without it `actions/checkout` takes the sibling's own default branch — which for views-appwrite is `development`. ADR-014 §3 makes `main` the authority for a claim about another repository, and `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` - already enforces that. Reading the working tree from `development` while demanding + already enforces that. Reading a sibling's `development` while demanding reachability from `main` is two guards in one file asking for different things. """ return [ diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 5a59f7c..9328ffc 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -514,7 +514,12 @@ def test_every_declared_name_is_classified_here(partner): "contract": "MIRRORED", # values live in our source by design (ADR-017 §5); # checked by tests/test_product.py, not by _declared_classes "excluded": "IGNORED", # names the registry records as deliberately NOT coordinates - "test_environment": "IGNORED", # a fact about the platform, not about this package + # IGNORED because nothing here READS it, not because it is none of our business — + # it is the clause that says which live checks this package may build, and a reader + # who believed the older comment spent weeks thinking a permission was a prohibition + # (register C-95, C-96). Its rows are bare strings, not tables, so it must stay out of + # `rows()` until that is handled (C-91). + "test_environment": "IGNORED", #: Arrived at registry v1.6.0, and it is views-appwrite#76 delivered — each edition #: marked ``obliges_consumers = true|false``, so a consumer can tell a console #: observation from a change it must act on. IGNORED only because nothing here reads @@ -525,12 +530,11 @@ def test_every_declared_name_is_classified_here(partner): "meta": "METADATA", # the edition and its amendment log } -#: direction is deliberately unchecked. (An earlier version of this comment offered -#: v1.5.1's removal of `[unmodelled]` as the worked example of a silent case. That was -#: WRONG: `unmodelled` was never in this partition, so against v1.4.4 it would have -#: been a RED build demanding classification. The rule is right; the illustration -#: was not, and it had been repeated in three places.) -#: silent while v1.5.0 adding `[contract]` is a red build with something to do. +#: An IGNORED table vanishing upstream is deliberately unchecked; a new, unclassified one +#: is a red build with something to do. (An earlier version of this comment illustrated +#: the silent case with v1.5.1's removal of `[unmodelled]`, which was wrong — `unmodelled` +#: was never in this partition, so it would have been a red build demanding +#: classification. The rule was right; the illustration was not, in three places.) #: The only roles that mean anything. A typo in `_TABLE_ROLE` used to be silent, and it #: silently narrowed a security scan: mistyping "CONSUMED" dropped `target` from the #: no-copy check's sections, taking it from twelve values to two, with no test objecting. @@ -689,9 +693,8 @@ def test_every_table_in_the_registry_is_classified_here(): was silently ignoring four. **Directional on purpose.** Every table upstream must be classified; only the tables - we depend on must exist. An IGNORED table disappearing is not our business, which is - an IGNORED table disappearing is silent while a new, unclassified one is a red build - with something to do. Two such events in the registry's life so far, and this + we depend on must exist. An IGNORED table disappearing is not our business, so it is + silent, while a new, unclassified one is a red build with something to do. Two such events in the registry's life so far, and this repository needed to see both. *(An earlier draft illustrated the silent case with v1.5.1's removal of @@ -808,6 +811,34 @@ def test_the_docstring_states_the_same_edition_the_constants_declare(partner): ) +@pytest.mark.parametrize("partner", _PARTNERS) +def test_the_docstring_url_points_at_the_commit_the_constant_declares(partner): + """The docstring publishes a blob URL. Its sha is a third copy of the pin, unguarded. + + The neighbouring test compares the docstring's *version*; nothing compared its *sha*. + That is how an annotated tag reached the pin: git peeled it, every check passed, and + two public modules published a URL returning 404 three lines above the sentence "a + pinned URL does not rot". Third time this class has bitten — register C-57. + """ + module = _PARTNER_ENV[partner][0] + urls = re.findall( + r"views-appwrite/blob/([0-9a-f]{7,40})/docs/ADRs/platform/coordinate_registry\.toml", + module.__doc__ or "", + ) + assert urls, ( + f"{partner}/appwrite_env.py's docstring no longer publishes a registry blob URL in " + "the expected form. If the URL moved, teach this test its new shape — do not delete " + "the check, or the sha goes unguarded again." + ) + wrong = sorted({u for u in urls if not module.SEAM_CONTRACT_COMMIT.startswith(u[:7])}) + assert not wrong, ( + f"[{partner}] the docstring's blob URL names commit(s) {wrong} while " + f"SEAM_CONTRACT_COMMIT declares {module.SEAM_CONTRACT_COMMIT}. A reader following " + "that link reads a different edition from the one this module was verified against, " + "and if the sha is not a commit at all the link 404s." + ) + + @pytest.mark.parametrize("partner", _PARTNERS) def test_the_pinned_commit_is_reachable_from_the_contract_repos_main(partner): """Existence is not reachability, and that distinction cost a merged PR (#196).