From 8500baa48e81f097456649a7c40a8680296f1998 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Sun, 2 Aug 2026 05:50:36 +0200 Subject: [PATCH] =?UTF-8?q?test(seam):=20S6=20#187=20=E2=80=94=20detect=20?= =?UTF-8?q?coordinate-registry=20drift;=20existence=20is=20not=20reachabil?= =?UTF-8?q?ity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Appwrite Seam Contract's registry is the authority for the 13 environment names this package declares, and it is deliberately referenced, never copied — one owner, no duplicated values. That was always right. What C-57 named was the gap it left: nothing mechanical could tell you the two had diverged. appwrite_env now declares the edition it was verified against, SEAM_CONTRACT_VERSION and SEAM_CONTRACT_COMMIT. A version string and a sha are not coordinate values; what is recorded is WHICH EDITION WAS READ, and that is precisely what makes drift detectable. Bumping them is not bookkeeping — it asserts someone re-checked. Four checks, each mutation-proven with bytecode disabled: names + declared class rename a coordinate here -> 4 failures registry [meta] version pretend v1.2.0 -> 1 failure commit reachable from main pin the withdrawn b54928f -> 1 failure no value copied into code add a constant/dict/kwarg -> 1 failure each The reachability check exists because existence was not enough. #196: S3 pinned a commit resolved with `rev-parse HEAD` on a checkout sitting on an unmerged branch. The commit existed, both cited files existed at it, and every check anyone had written passed — but it had never reached main, declared a version never ratified, and was withdrawn. A pin is a claim about what the contract SAYS; only reachability supports that claim. The value-copy check took two wrong narrowings before the right one. A substring scan flagged three false positives — file_metadata is a FUNCTION NAME here, and the other two appear only in refusal labels and docstrings. Narrowing to assignments and defaults then caught neither a dict value nor a keyword argument, and the keyword argument is the shape this repo would actually produce: AppwriteConfig(bucket_id=...) is how every store is configured. The right axis was exact equality on string constants, not statement shape — it catches all three real shapes while the false positives fall out on their own. When a guard cries wolf, check whether the matching is wrong before assuming the scope is. Review also caught the expectation map deriving class from an _API_KEY suffix — the exact inference the registry forbids, inside the test enforcing the registry. Now written out, with a completeness check so an unclassified name fails. tests/conftest.py holds the sibling-repo resolution S7 (#188, C-46) will adopt for views-datafactory: declared env var, conventional sibling path, never an absolute path to one machine, and a skip that names what to set so the test is runnable rather than merely skippable. Co-Authored-By: Claude Opus 5 (1M context) --- reports/technical_risk_register.md | 79 ++++--- tests/conftest.py | 109 ++++++++++ tests/test_env_declaration.py | 238 +++++++++++++++++++++ views_postprocessing/unfao/appwrite_env.py | 17 +- 4 files changed, 415 insertions(+), 28 deletions(-) create mode 100644 tests/conftest.py diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 457a3cc..54459bb 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -6,8 +6,8 @@ | Owner | Dylan Pinheiro / PRIO MD&D Team | | Last Updated | 2026-08-02 | | Total Concerns | 74 | -| Open Concerns | 20 | -| Resolved Concerns | 54 | +| Open Concerns | 19 | +| Resolved Concerns | 55 | --- @@ -404,30 +404,6 @@ See also C-36 (the resolved strict-xfail conversion this extends), C-44 (the dat --- -### C-57: PLATFORM-001 coordinate registry is referenced by URL, so nothing detects drift between it and this repo's declared environment - -| Field | Value | -|-------|-------| -| ID | C-57 | -| Tier | 3 | -| Source | `manual` (2026-07-31) — review-rr blind-spot analysis, following the þing-01 verdict (`orð_dómr.md`, ratified as amended 2026-07-28) | -| Trigger | When views-appwrite amends `coordinate_registry.toml` — renames a coordinate, retires the legacy secret slot in favour of `APPWRITE_{READ,WRITE,PROVISION}_API_KEY`, or adds a target — verify `views_postprocessing/unfao/appwrite_env.py` still matches. Nothing mechanical will tell you: the registry is deliberately **referenced, never copied**, and the two live in different repositories | -| Location | `views_postprocessing/unfao/appwrite_env.py` (`CONNECTION_ENV`, `PROD_FORECASTS_ENV`, `UNFAO_ENV`); views-appwrite `docs/ADRs/platform/coordinate_registry.toml` (the authority); `tests/test_env_declaration.py` (guards this repo's half only); `docs/ADRs/013_sampled_forecast_wire_contract.md` §7(d) (the URL reference) | - -The þing-01 assembly (D1) settled that the PLATFORM-001 contract is **homed in views-appwrite and referenced by URL, never by copy** — a deliberate and correct choice: copies were the platform's original disease (sáttmál S6, the copy-chain this repo's own `load_dotenv` borrow was the runtime edge of, killed in #134/PR #137). But referencing-not-copying moves the failure mode rather than removing it: **the registry can now change without this repo noticing.** - -This repo's half is well guarded. `tests/test_env_declaration.py` pins that every `APPWRITE_*` name the manager reads is declared, that all three store paths validate before constructing an `AppwriteConfig`, that empty-string counts as missing, and that exactly one declared name is a secret by the D3 suffix rule. **What no test can see is the other side of the reference** — whether `coordinate_registry.toml` still spells the coordinates the way `appwrite_env.py` does. Divergence surfaces at runtime as a fail-loud `EnvironmentError` from `assert_env_declared` (good — that is D6 working), but only on a delivery run, and only after the launcher has already been reconfigured. - -Two named changes are already anticipated and will fire this trigger: the **retirement of the legacy `APPWRITE_DATASTORE_API_KEY`** in favour of the three-tier read/write/provision slots (D4), and any target-coordinate addition for the second store (issue #97). Tier 3 — coordination and cost-of-change across a repo boundary; the failure is loud, not silent, and D6's entry validation is the backstop that keeps it that way. - -**Deliberately out of scope here:** the þing-01 redaction clause is mechanically enforced (`tests/test_redaction_guard.py` — the delivery modules stay credential-blind and the provenance description is a closed keyset), and D2's ruling that **integration tests against the production Appwrite project are FORBIDDEN** (no non-production project exists) is a standing prohibition, not a drift risk. - -**⚠ CORRECTED 2026-08-02.** The word *already* above overclaimed: **C-74** showed that guard scans one of its five declared roots, four having pointed at paths that #153 moved. The keyset half is enforced; the credential-blindness half is enforced over `delivery/` only until C-74 lands. - -Cross-refs: C-74 (the guard this paragraph vouched for), C-33 (store identity still hardcoded per store — the same env surface, different concern), C-58 (what happens when a coordinate is wrong rather than missing), C-44 (the pipeline-core version coupling that would carry a registry change), issues #134/#135/#138 (this repo's discharged þing-01 obligations), #104 (README env block placeholders). - ---- - ### C-58: A wrong Appwrite coordinate auto-provisions a new empty target instead of raising — both client lineages, on every write | Field | Value | @@ -674,6 +650,57 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-57: PLATFORM-001 coordinate registry is referenced by URL, so nothing detects drift between it and this repo's declared environment — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-57 | +| Resolved | 2026-08-02 | +| Resolution | **Closed by S6 (#187).** The registry stays **referenced, never copied** — that was always the right call. What C-57 named was the gap it left: nothing mechanical could tell you the two had diverged. There is now a detector, built while the answer was known-good rather than after a failed delivery. + +`appwrite_env` declares the edition it was verified against — `SEAM_CONTRACT_VERSION = "1.3.0"` and `SEAM_CONTRACT_COMMIT = "47172af"`. **A version string and a sha are not coordinate values**; what is recorded is *which edition was read*, which is precisely what makes drift detectable. Four checks in `tests/test_env_declaration.py`, each mutation-proven: + +| check | mutation | result | +|---|---|---| +| every declared name exists, with the class the **registry declares** (never inferred from a prefix) | rename `APPWRITE_UNFAO_BUCKET_ID` here | 4 failures | +| the registry's `[meta] version` matches the pin | pretend v1.2.0 | 1 failure, naming the version to re-verify against | +| the pinned commit is **reachable from views-appwrite's `main`** | pin the withdrawn `b54928f` | 1 failure | +| no coordinate value is baked into a constant or default | add `UNFAO_BUCKET = "unfao_bucket"` | 1 failure | + +**The reachability check exists because existence was not enough.** #196: S3 pinned a commit resolved with `rev-parse HEAD` on a checkout sitting on an unmerged branch. The commit existed, both cited files existed at it, every check anyone had written passed — and it had never reached `main`, declared a version never ratified, and was withdrawn. A pin is a claim about what the contract *says*; only reachability supports that claim. + +**The value-copy check took two wrong narrowings before the right one, and both are worth recording.** A substring text scan flagged three "leaks": `file_metadata` (a **function name** in `contract/store_metadata.py`), `production_forecasts` and `unfao_bucket` (only in refusal labels and docstrings naming which store a function serves). None was a copy, and a guard that fails on `def file_metadata(record)` gets deleted — after which the real rule is unguarded. + +Narrowing to *assignments and default arguments* then failed the other way: it caught neither a dict value nor a keyword argument, and the keyword argument is the shape this repo would actually produce — `AppwriteConfig(bucket_id=os.getenv(...))` is how every store is configured, and swapping one `os.getenv` for a literal there is the violation. Verified: that draft caught **zero** of the two. + +The right axis was **exact equality on string constants**, not statement shape. It catches dict values, keyword arguments and constants alike, while all three original false positives fall out on their own: a function name is not a `Constant`; `"unfao_bucket datastore"` is not equal to `"unfao_bucket"`; docstrings are excluded outright. The lesson is narrow and reusable: **when a guard cries wolf, check whether the matching is wrong before assuming the scope is.** + +**Gated, and honestly so.** The checks need a views-appwrite checkout and skip without one, naming `VIEWS_APPWRITE` and the conventional sibling path so a contributor can run them rather than merely watch them skip. The would-catch-a-rename proof runs in CI with no checkout at all. Resolution helper shared with **C-46** (S7) in `tests/conftest.py` — the second incident, which is this repo's named trigger for extracting. + +**Residual:** the gated half does not run in CI, which needs a views-appwrite checkout in the workflow — a CI-cost and cross-repo-coupling decision, not a code fix. Same shape as **C-46**'s residual and worth deciding once for both (S7 / #188). + +A second, smaller instance of the same shape: these checks parse TOML with `tomllib`, stdlib from Python 3.11, and `pyproject` declares `>=3.11`. CI runs 3.11 and executes them. The maintainer's box runs **3.10**, below the declared floor, so they skip there — the local suite is quietly weaker than a green `pytest -q` suggests. Not a repo defect and not worth its own entry; recorded because "a gate that does not run" is exactly what C-46 is open for, and the CI decision should cover both. | +| Tier | 3 | +| Source | `manual` (2026-07-31) — review-rr blind-spot analysis, following the þing-01 verdict (`orð_dómr.md`, ratified as amended 2026-07-28) | +| Trigger | When views-appwrite amends `coordinate_registry.toml` — renames a coordinate, retires the legacy secret slot in favour of `APPWRITE_{READ,WRITE,PROVISION}_API_KEY`, or adds a target — verify `views_postprocessing/unfao/appwrite_env.py` still matches. Nothing mechanical will tell you: the registry is deliberately **referenced, never copied**, and the two live in different repositories | +| Location | `views_postprocessing/unfao/appwrite_env.py` (`CONNECTION_ENV`, `PROD_FORECASTS_ENV`, `UNFAO_ENV`); views-appwrite `docs/ADRs/platform/coordinate_registry.toml` (the authority); `tests/test_env_declaration.py` (guards this repo's half only); `docs/ADRs/013_sampled_forecast_wire_contract.md` §7(d) (the URL reference) | + +The þing-01 assembly (D1) settled that the PLATFORM-001 contract is **homed in views-appwrite and referenced by URL, never by copy** — a deliberate and correct choice: copies were the platform's original disease (sáttmál S6, the copy-chain this repo's own `load_dotenv` borrow was the runtime edge of, killed in #134/PR #137). But referencing-not-copying moves the failure mode rather than removing it: **the registry can now change without this repo noticing.** + +This repo's half is well guarded. `tests/test_env_declaration.py` pins that every `APPWRITE_*` name the manager reads is declared, that all three store paths validate before constructing an `AppwriteConfig`, that empty-string counts as missing, and that exactly one declared name is a secret by the D3 suffix rule. **What no test can see is the other side of the reference** — whether `coordinate_registry.toml` still spells the coordinates the way `appwrite_env.py` does. Divergence surfaces at runtime as a fail-loud `EnvironmentError` from `assert_env_declared` (good — that is D6 working), but only on a delivery run, and only after the launcher has already been reconfigured. + +Two named changes are already anticipated and will fire this trigger: the **retirement of the legacy `APPWRITE_DATASTORE_API_KEY`** in favour of the three-tier read/write/provision slots (D4), and any target-coordinate addition for the second store (issue #97). Tier 3 — coordination and cost-of-change across a repo boundary; the failure is loud, not silent, and D6's entry validation is the backstop that keeps it that way. + +**Deliberately out of scope here:** the þing-01 redaction clause is mechanically enforced (`tests/test_redaction_guard.py` — the delivery modules stay credential-blind and the provenance description is a closed keyset), and D2's ruling that **integration tests against the production Appwrite project are FORBIDDEN** (no non-production project exists) is a standing prohibition, not a drift risk. + +**⚠ CORRECTED 2026-08-02.** The word *already* above overclaimed: **C-74** showed that guard scans one of its five declared roots, four having pointed at paths that #153 moved. The keyset half is enforced; the credential-blindness half is enforced over `delivery/` only until C-74 lands. + +Cross-refs: C-74 (the guard this paragraph vouched for), C-33 (store identity still hardcoded per store — the same env surface, different concern), C-58 (what happens when a coordinate is wrong rather than missing), C-44 (the pipeline-core version coupling that would carry a registry change), issues #134/#135/#138 (this repo's discharged þing-01 obligations), #104 (README env block placeholders). + +--- + +--- + ### C-60: The lookup provenance stamp reaches into the producer's ledger schema and degrades to `"unknown"` on a bare except — RESOLVED | Field | Value | diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..dfee96c --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,109 @@ +"""Shared test fixtures — currently just one: how to find a sibling repository. + +**Why this exists (S6 / #187, S7 / #188; register C-46, C-57).** Three places needed a +views-platform sibling checkout and each found it differently: + +- ``scripts/build_gaul_lookup.py`` resolved ``$VIEWS_DATAFACTORY``, then the sibling + directory, with a ``--datafactory`` override and a fail-loud message naming both — + the shape that has been working in production for weeks; +- ``tests/test_gaul_lookup_fidelity.py`` had its own ``skipif``; +- ``tests/test_datafactory_deploy_readiness.py`` hardcoded an **absolute path to one + developer's laptop**, so the only cross-repo release gate in the repo has never run + anywhere else — register **C-46**. + +This is the second incident, which is this repo's named trigger for extracting. The +shape is not guessed: it is the one the build script already runs. + +**Deliberately not a package import.** The siblings are separate repositories, not +dependencies — this repo declares three (`views-pipeline-core`, `views-frames`, +`pyarrow`) and none of them is a sibling checkout. What is being located is a +*working copy on disk*, for tests that compare this repo against another repo's +current state. If that ever becomes a runtime need, it is a different problem with a +different answer (a published artifact, not a path). +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parent.parent + +#: repo name -> the environment variable that overrides its location. +#: Declared, never derived from the name: ``views-datafactory`` → ``VIEWS_DATAFACTORY`` +#: happens to be mechanical, but a future sibling need not follow the pattern and +#: guessing it would be the inference ADR-003 forbids. +SIBLING_ENV = { + "views-datafactory": "VIEWS_DATAFACTORY", + "views-appwrite": "VIEWS_APPWRITE", +} + + +def sibling_repo(name: str) -> Path | None: + """The checkout of ``name``, or ``None`` if it is not resolvable. + + Order: the declared environment variable, then the conventional sibling directory + **relative to this repository** — never an absolute path to a particular machine. + + Returns ``None`` rather than raising so callers can skip; a missing sibling is a + normal condition in CI, where only this repo is checked out. + """ + if name not in SIBLING_ENV: + raise KeyError( + f"no environment variable declared for sibling {name!r}; add it to " + f"SIBLING_ENV rather than guessing one from the name" + ) + override = os.environ.get(SIBLING_ENV[name]) + candidate = Path(override) if override else _REPO.parent / name + return candidate if candidate.exists() else None + + +def require_sibling(name: str) -> Path: + """``sibling_repo`` or skip, with a message naming what to set. + + A skip that says only "checkout not present" tells a contributor nothing. This one + tells them the variable and the conventional location, so the test is runnable + rather than merely skippable. + """ + path = sibling_repo(name) + if path is None: + pytest.skip( + f"{name} checkout not found — set {SIBLING_ENV[name]}=/path/to/{name}, " + f"or place it alongside this repo at {(_REPO.parent / name)}" + ) + return path + + +def git_output(repo: Path, *args: str) -> str: + """Run a read-only git command in ``repo``; empty string on any failure.""" + result = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, text=True, check=False, timeout=30, + ) + return result.stdout.strip() + + +def commit_is_on_main(repo: Path, commit: str) -> bool: + """Is ``commit`` an ancestor of ``origin/main`` (falling back to ``main``)? + + **Existence is not reachability, and that distinction cost a PR** (#196). S3 pinned + the Appwrite Seam Contract at a commit resolved with ``rev-parse HEAD`` on a + checkout that happened to be sitting on an unmerged feature branch. The commit + existed, both cited files existed at it, and every check anyone had written passed + — but it had never reached ``main`` and was withdrawn days later. + + A pin is a claim about what the contract *says*. Only reachability from ``main`` + supports that claim. + """ + for ref in ("origin/main", "main"): + if not git_output(repo, "rev-parse", "--verify", "--quiet", ref): + continue + result = subprocess.run( + ["git", "-C", str(repo), "merge-base", "--is-ancestor", commit, ref], + capture_output=True, check=False, timeout=30, + ) + return result.returncode == 0 + return False diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 17a23ba..85bae19 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -14,15 +14,19 @@ pinned by source scan instead — the repo's standing pattern. """ +import ast import logging import re from pathlib import Path import pytest +from tests.conftest import commit_is_on_main, git_output, require_sibling, sibling_repo from views_postprocessing.contract import launch_config from views_postprocessing.unfao import appwrite_env +_PKG = Path(__file__).resolve().parent.parent / "views_postprocessing" + _MANAGER_SOURCE = ( Path(__file__).resolve().parent.parent @@ -192,3 +196,237 @@ def test_secret_env_names_follow_the_seam_contract_naming_rule(): ) secrets = {n for n in declared if n.endswith(("_API_KEY", "_PASSWORD", "_TOKEN"))} assert secrets == {"APPWRITE_DATASTORE_API_KEY"} + + +# ── drift against the Appwrite Seam Contract's registry (S6 / #187, C-57) ──── +# +# The registry is the authority for every name below and lives in views-appwrite. It +# is deliberately **referenced, never copied** — one owner, no duplicated values, no +# drift-by-fork — and that is the right call. What it leaves is C-57's gap: nothing +# mechanical tells you when the two have diverged. +# +# Checked 2026-08-02: there is no live drift. This builds the detector while the +# answer is known-good, which is the cheap moment; the alternative is discovering it +# during a failed delivery. +# +# **Never assert on a coordinate VALUE.** The registry holds non-secret identifiers, +# and copying one into a test is the same violation as copying it into code. These +# compare names, declared classes, and the edition — nothing else. + +_REGISTRY_RELPATH = Path("docs") / "ADRs" / "platform" / "coordinate_registry.toml" + +#: How this module treats each declared name, vs. the registry's own `class` field. +#: +#: **Written out, not derived.** An earlier draft built this by suffix — anything ending +#: `_API_KEY` is a secret, everything else in CONNECTION_ENV is a connection. That is the +#: precise inference the registry's header forbids ("class is DECLARED here, never +#: inferred from a name's prefix"), reproduced inside the test written to enforce it. +#: It happened to be correct, which is what makes the habit worth breaking rather than +#: excusing. Adding a name without classifying it now fails below. +_EXPECTED_CLASS = { + "APPWRITE_ENDPOINT": "connection", + "APPWRITE_DATASTORE_PROJECT_ID": "connection", + "APPWRITE_DATASTORE_API_KEY": "secret", + "APPWRITE_PROD_FORECASTS_BUCKET_ID": "target", + "APPWRITE_PROD_FORECASTS_BUCKET_NAME": "target", + "APPWRITE_PROD_FORECASTS_COLLECTION_ID": "target", + "APPWRITE_PROD_FORECASTS_COLLECTION_NAME": "target", + "APPWRITE_UNFAO_BUCKET_ID": "target", + "APPWRITE_UNFAO_BUCKET_NAME": "target", + "APPWRITE_UNFAO_COLLECTION_ID": "target", + "APPWRITE_UNFAO_COLLECTION_NAME": "target", + "APPWRITE_METADATA_DATABASE_ID": "target", + "APPWRITE_METADATA_DATABASE_NAME": "target", +} + + +def test_every_declared_name_is_classified_here(): + """The map above must cover the module exactly — no silent gaps, no strays. + + Without this, adding a name to one of the ENV tuples would simply not be checked + against the registry, and the drift test would keep passing while covering less. + That is register **C-74**'s shape: a guard quietly narrower than it claims. + """ + declared = set( + appwrite_env.CONNECTION_ENV + appwrite_env.PROD_FORECASTS_ENV + appwrite_env.UNFAO_ENV + ) + assert set(_EXPECTED_CLASS) == declared, ( + f"unclassified: {sorted(declared - set(_EXPECTED_CLASS))}; " + f"stale: {sorted(set(_EXPECTED_CLASS) - declared)}" + ) + + +def _load_registry(repo: Path) -> dict: + tomllib = pytest.importorskip( + "tomllib", + reason="tomllib is stdlib from Python 3.11; pyproject declares >=3.11, so a " + "conforming environment has it. CI runs 3.11.", + ) + return tomllib.loads((repo / _REGISTRY_RELPATH).read_text()) + + +def _declared_classes(registry: dict) -> dict[str, str]: + """name -> the class the registry DECLARES for it (never inferred from the name).""" + return { + name: body.get("class") + for section in ("connection", "target", "secret") + for name, body in registry.get(section, {}).items() + } + + +def test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_as(): + """C-57: a rename or reclassification upstream must not be silent here.""" + repo = require_sibling("views-appwrite") + declared = _declared_classes(_load_registry(repo)) + + missing = sorted(n for n in _EXPECTED_CLASS if n not in declared) + assert not missing, ( + f"names this package requires are absent from the Appwrite Seam Contract's " + f"registry: {missing}. Either the registry retired them or this module invented " + "them; the registry is the authority." + ) + misclassified = { + n: (expected, declared[n]) + for n, expected in _EXPECTED_CLASS.items() + if declared[n] != expected + } + assert not misclassified, ( + f"class mismatch (expected, registry) {misclassified}. Class is DECLARED by the " + "registry, never inferred from a name's prefix — a coordinate treated as a " + "secret (or the reverse) is a redaction bug waiting to happen." + ) + + +def test_the_pinned_contract_edition_still_matches_the_registry(): + """The check that catches everything the other one cannot — including additions. + + Names and classes catch a rename. The edition catches **any** other change: a new + target this repo ought to adopt, a retired secret slot, a reworded rule. It fails + loudly and tells you what to do rather than what broke. + """ + repo = require_sibling("views-appwrite") + actual = _load_registry(repo)["meta"]["version"] + assert actual == appwrite_env.SEAM_CONTRACT_VERSION, ( + f"the Appwrite Seam Contract's registry moved to v{actual}; this repo declares " + f"v{appwrite_env.SEAM_CONTRACT_VERSION}. Re-verify appwrite_env's declaration " + f"against v{actual}, then bump SEAM_CONTRACT_VERSION and SEAM_CONTRACT_COMMIT " + "together. Do not bump one alone — the pair is the claim." + ) + + +def test_the_pinned_commit_is_reachable_from_the_contract_repos_main(): + """Existence is not reachability, and that distinction cost a merged PR (#196). + + S3 pinned a commit resolved with ``rev-parse HEAD`` on a views-appwrite checkout + that happened to be sitting on an unmerged branch. The commit existed. Both cited + files existed at it. Every check anyone had written passed. It had never reached + ``main``, declared a version that was never ratified, and was withdrawn. + """ + repo = require_sibling("views-appwrite") + commit = appwrite_env.SEAM_CONTRACT_COMMIT + if not git_output(repo, "cat-file", "-t", commit): + pytest.skip( + f"{commit} is not in the local views-appwrite checkout — run `git fetch` " + "there; a stale clone cannot answer whether the pin reached main" + ) + assert commit_is_on_main(repo, commit), ( + f"the pinned commit {commit!r} is not an ancestor of views-appwrite's main. A " + "pin taken from a working copy's HEAD can land on an unmerged branch — that is " + "#196, verbatim. Re-pin from `git rev-parse --short origin/main`." + ) + + +def test_the_drift_check_would_catch_a_rename(tmp_path): + """A gated test that cannot fail is decoration — so prove this one bites in CI. + + Runs with **no** views-appwrite checkout: a synthetic registry with one name + renamed and one reclassified, fed to the same comparison the gated tests use. + """ + registry = { + "meta": {"version": appwrite_env.SEAM_CONTRACT_VERSION}, + "connection": {"APPWRITE_ENDPOINT": {"class": "connection"}}, + "target": {"APPWRITE_UNFAO_BUCKET_ID": {"class": "secret"}}, # reclassified + "secret": {"APPWRITE_DATASTORE_API_KEY": {"class": "secret"}}, + } + declared = _declared_classes(registry) + + assert "APPWRITE_DATASTORE_PROJECT_ID" not in declared, "fixture should omit it" + missing = sorted(n for n in _EXPECTED_CLASS if n not in declared) + assert missing, "the detector reported no missing names against a registry that omits most" + + mismatched = [ + n for n, expected in _EXPECTED_CLASS.items() + if n in declared and declared[n] != expected + ] + assert "APPWRITE_UNFAO_BUCKET_ID" in mismatched, ( + "a target reclassified as a secret went unnoticed — that is the case where " + "getting it wrong leaks or hides a value" + ) + + +def _docstring_nodes(tree: ast.AST) -> set[int]: + """ids of the string Constants that are docstrings — prose, not values.""" + out = set() + for node in ast.walk(tree): + if isinstance(node, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + first = node.body[0] if node.body else None + if ( + isinstance(first, ast.Expr) + and isinstance(first.value, ast.Constant) + and isinstance(first.value.value, str) + ): + out.add(id(first.value)) + return out + + +def test_no_coordinate_value_is_copied_into_this_repo(): + """The registry's own rule: *"never bake a value into code, an example, or a + dataclass default."* Consumers READ and VALIDATE; the launcher supplies values. + + **Two wrong narrowings before this one, both instructive.** + + A substring text scan over every ``.py`` flagged three "leaks": ``file_metadata`` + (a **function name** in ``contract/store_metadata.py``), ``production_forecasts`` + and ``unfao_bucket`` (only in refusal labels and docstrings naming which store a + function serves). None was a copy, and a guard that fails on + ``def file_metadata(record)`` gets deleted — after which the real rule is unguarded. + + Narrowing to *assignments and default arguments* then went too far in the other + direction: it caught neither a dict value nor a keyword argument, and the keyword + argument is the shape this repo would actually produce — + ``AppwriteConfig(bucket_id=...)`` is how every store is configured, and swapping one + ``os.getenv`` for a literal there is the violation. + + The right axis was **exact equality on string constants**, not statement shape. It + catches dict values and kwargs, while all three original false positives fall out on + their own: a function name is not a ``Constant``; ``"unfao_bucket datastore"`` is not + equal to ``"unfao_bucket"``; docstrings are excluded outright. + """ + repo = sibling_repo("views-appwrite") + if repo is None: + pytest.skip("views-appwrite checkout not found — set VIEWS_APPWRITE") + registry = _load_registry(repo) + values = { + body["value"] + for section in ("connection", "target") + for body in registry.get(section, {}).values() + if isinstance(body.get("value"), str) and len(body["value"]) > 6 + } + + copied = [] + for source in sorted(_PKG.rglob("*.py")): + tree = ast.parse(source.read_text()) + docstrings = _docstring_nodes(tree) + for node in ast.walk(tree): + if ( + isinstance(node, ast.Constant) + and isinstance(node.value, str) + and node.value in values + and id(node) not in docstrings + ): + copied.append(f"{source.relative_to(_PKG)}:{node.lineno} = {node.value!r}") + assert not copied, ( + f"coordinate value(s) from the registry are copied into code: {copied}. The " + "registry is referenced, never copied — values reach this package through the " + "environment the launcher assembles, validated by assert_env_declared." + ) diff --git a/views_postprocessing/unfao/appwrite_env.py b/views_postprocessing/unfao/appwrite_env.py index a9e1597..934f372 100644 --- a/views_postprocessing/unfao/appwrite_env.py +++ b/views_postprocessing/unfao/appwrite_env.py @@ -7,8 +7,10 @@ https://github.com/views-platform/views-appwrite/blob/47172af/docs/ADRs/platform/coordinate_registry.toml -That pin is registry **v1.3.0** (ratified, þing-02). A pinned URL does not rot, but -it does go stale — naming the version is what makes the staleness legible here. +That pin is registry **v1.3.0** (ratified, þing-02) — declared below as +``SEAM_CONTRACT_VERSION`` / ``SEAM_CONTRACT_COMMIT`` so the pin is a value a test can +check rather than a fact buried in prose. A pinned URL does not rot, but it does go +stale, and nothing in this repository could previously tell you it had (register C-57). **Pin from the tip of `main`, never from a sibling checkout's `HEAD`** (#196). The first pin taken here was resolved with `git -C ../views-appwrite rev-parse HEAD` on a @@ -34,6 +36,17 @@ logger = logging.getLogger(__name__) +#: The Appwrite Seam Contract edition these names were verified against, and the commit +#: this repo cites. **A version string and a sha are not coordinate values** — the +#: registry forbids copying its values, and nothing here copies one. What is recorded is +#: *which edition was read*, which is exactly what makes drift detectable. +#: +#: Bumping these is not bookkeeping: it asserts that someone re-checked this module's +#: declaration against that edition of the registry. ``tests/test_env_declaration.py`` +#: enforces the pair against a local views-appwrite checkout when one is present. +SEAM_CONTRACT_VERSION = "1.3.0" +SEAM_CONTRACT_COMMIT = "47172af" + CONNECTION_ENV = ( "APPWRITE_ENDPOINT", "APPWRITE_DATASTORE_PROJECT_ID",