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).