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
8 changes: 7 additions & 1 deletion reports/technical_risk_register.md
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,13 @@ What actually keeps it open is neither of those — it is the two deferrals belo
- Deferral 1's trigger is *"when #243 finishes touching `tests/test_env_declaration.py`"*. #243 finished. The leak guards are still in that file; `tests/test_redaction_guard.py` is only cross-referenced.
- Deferral 2 was *"routed to #243"* — and #243 closed without it. `test_the_drift_check_would_catch_a_rename` still rebuilds its subject's comparison with its own comprehension rather than driving the checked function.

Both were unowned, which is worse than deferred — **now filed as #265**, with acceptance criteria and the reason each is not urgent. That issue closing is what closes this entry: its stated condition was already met by #243. *(An earlier draft of this amendment named the problem and left it there, which under ADR-014 §4 converts two compliant deferrals into two non-compliant items. Naming is not rehoming.)*
Both were unowned, which is worse than deferred — filed as **#265**, and **both discharged 2026-08-14**. *(An earlier draft of this amendment named the problem and left it there, which under ADR-014 §4 converts two compliant deferrals into two non-compliant items. Naming is not rehoming.)*

**Deferral 2 is fixed as filed.** The comparison the gated check ran inline is now `_name_and_class_drift`, called by both it and the proof, so the proof drives its subject instead of a copy of it. Mutation-proven: making that function return `([], {})` now fails the proof for both partners, where before it stayed green. *(The sibling defects were repaired by driving the real check under `monkeypatch` instead; here the comparison is a pure function of two dicts that both callers want whole, so sharing it is the same guarantee with less machinery.)* It also gained an assertion that the report names *both* sides of a mismatch — what this package expects and what the registry declares — since a reader who cannot tell which moved cannot act on it.

**Deferral 1 is answered "no", with the reason recorded rather than deferred a third time.** The coordinate-value scan **stays in `tests/test_env_declaration.py`**. Its subject is `_EXPECTED_NAMES`, derived from `_PARTNER_ENV` — the declaration of what each partner reads, which is the substance of that module. Moving a guard away from the declaration it guards, so that a filename reads better, trades a real coupling (CCP) for a filing convenience, and would require exporting a private name from one test module into another.

What *was* misfiled moved instead: `registry_at` / `registry_current` / `rows` are the shared **reader**, not this package's environment declarations, and their five refusal tests plus `_scratch_repo` are now `tests/test_seam_registry.py`. That is the boundary that was actually wrong: **109 lines of test code moved out**, no shared private state left behind — the new module imports only from `tests/seam_registry.py`. `test_env_declaration.py` is 1406 lines against 1503 before this change — the split removed more than that and the shared comparison above put some back.

**Two deferrals, both with triggers (ADR-014 §4).**

Expand Down
165 changes: 37 additions & 128 deletions tests/test_env_declaration.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,10 @@

from tests.seam_registry import (
ABSENT as _ABSENT,
REGISTRY_RELPATH,
REGISTRY_RELPATH as _REGISTRY_RELPATH,
RegistryReadError,
RegistryReadError as _RegistryReadError,
registry_at,
registry_at as _registry_at,
registry_current,
registry_current as _registry_current,
rows,
rows as _rows,
)
from tests.conftest import (
Expand Down Expand Up @@ -629,24 +624,45 @@ def _declared_classes(registry: dict) -> dict[str, str]:
return {n: row[1] for n, row in _rows(registry, _CONSUMED_TABLES).items()}


def _name_and_class_drift(expected_class: dict, declared: dict) -> tuple[list, dict]:
"""What this package expects vs what the registry declares.

Returns ``(names the registry does not carry, {name: (expected, declared)})``.

Extracted 2026-08-14 (issue #265) so the gated check below and the ungated proof of
it further down run the **same** comparison. They did not: the proof rebuilt this
with its own comprehensions, so blanking the assertions here left it green.

The two sibling defects were repaired differently — by adding a test that drives the
real check under ``monkeypatch`` (``test_the_drift_check_fires_when_a_row_this_partner_reads_rotates``).
That works there because the check reads a registry the test can substitute. Here the
comparison is a pure function of two dicts and both callers want exactly it, so
sharing the function is the same guarantee with less machinery. Extracting is
justified by *this* being the second time the pattern has bitten, not by a rule about
line counts.
"""
missing = sorted(n for n in expected_class if n not in declared)
misclassified = {
n: (expected, declared[n])
for n, expected in expected_class.items()
if n in declared and declared[n] != expected
}
return missing, misclassified


@pytest.mark.parametrize("partner", _PARTNERS)
def test_every_declared_name_exists_in_the_registry_with_the_class_we_treat_it_as(partner):
"""C-57: a rename or reclassification upstream must not be silent here."""
repo = require_sibling("views-appwrite")
declared = _declared_classes(_registry_current(repo))
_, _, expected_class = _PARTNER_ENV[partner]

missing = sorted(n for n in expected_class if n not in declared)
missing, misclassified = _name_and_class_drift(expected_class, declared)
assert not missing, (
f"[{partner}] names this package requires are absent from the Appwrite Seam "
f"Contract's 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"[{partner}] class mismatch (expected, registry) {misclassified}. Class is "
"DECLARED by the registry, never inferred from a name's prefix — a coordinate "
Expand Down Expand Up @@ -1140,17 +1156,20 @@ def test_the_drift_check_would_catch_a_rename(partner):
"not the mutation this test believes it is"
)
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 canary in mismatched, (
# The gated check's own comparison, not a copy of it (issue #265). This rebuilt the
# logic with its own comprehensions until 2026-08-14, which made it a proof of a
# reimplementation: blank the real check's assertions and it stayed green.
missing, misclassified = _name_and_class_drift(expected_class, declared)
assert missing, "the detector reported no missing names against a registry that omits most"
assert canary in misclassified, (
f"[{partner}] a target reclassified as a secret went unnoticed — that is the "
"case where getting it wrong leaks or hides a value"
)
assert misclassified[canary] == ("target", "secret"), (
"the detector must report BOTH sides of the mismatch — what this package "
"expects and what the registry declares — or a reader cannot tell which moved"
)


def _docstring_nodes(tree: ast.AST) -> set[int]:
Expand Down Expand Up @@ -1391,113 +1410,3 @@ def test_the_scan_understands_every_assignment_form_this_repo_writes():
"document that introduced it — that is the stopping rule, and it is why the "
"form list is derived from this repository's own corpus rather than invented."
)


def _scratch_repo(tmp_path: Path):
"""A throwaway git repo whose registry differs on `main`, on `origin/main`, and on disk.

`-c` rather than `git config`: a contributor's global `commit.gpgsign` or
`core.hooksPath` would otherwise reach in and either fail opaquely or block on
pinentry with no timeout.
"""
def git(*args):
return subprocess.run(
["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false",
"-c", "core.hooksPath=/dev/null", *args],
capture_output=True, text=True, check=True, timeout=30,
)

target = tmp_path / REGISTRY_RELPATH
target.parent.mkdir(parents=True)

def edition(marker: str) -> str:
return f'[meta]\nversion = "{marker}"\n\n[connection.X]\nclass = "connection"\n'

git("init", "-q", "-b", "main")
git("config", "user.email", "t@t")
git("config", "user.name", "t")
target.write_text(edition("on-main"))
git("add", "-A")
git("commit", "-q", "-m", "main")

# a remote-tracking ref that is AHEAD of main, so preferring one over the other shows
git("checkout", "-q", "-b", "upstream")
target.write_text(edition("on-origin-main"))
git("add", "-A")
git("commit", "-q", "-m", "origin")
git("update-ref", "refs/remotes/origin/main", "HEAD")
git("checkout", "-q", "main")

# and a dirty working tree, which is what #196 was about
target.write_text(edition("in-the-working-tree"))
return tmp_path


def test_registry_current_reads_origin_main_not_the_working_tree(tmp_path):
"""The reason `tests/seam_registry.py` exists, and until now the only untested part.

A sibling clone sits on whatever branch its own agent last worked on. Comparing
against that grades this repository on unreviewed content — issue #196, which cost a
withdrawn pull request. Replacing this function with a working-tree or `HEAD` read
used to leave the whole suite green.
"""
repo = _scratch_repo(tmp_path)
assert registry_current(repo)["meta"]["version"] == "on-origin-main", (
"registry_current read something other than origin/main. A working-tree read is "
"#196 verbatim; a bare `main` read misses that the sibling's remote has moved."
)


def test_registry_current_refuses_a_repo_with_neither_ref(tmp_path):
"""No `origin/main` and no `main` must say so, not return an empty registry."""
subprocess.run(["git", "init", "-q", str(tmp_path)],
capture_output=True, text=True, check=True, timeout=30)
with pytest.raises(RegistryReadError, match="neither origin/main nor main"):
registry_current(tmp_path)


def test_registry_at_refuses_a_commit_whose_registry_is_missing_or_unparseable(tmp_path):
"""`git show` failing, and a blob that is not TOML — two refusal branches nothing reached."""
def git(*args):
return subprocess.run(
["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false",
"-c", "core.hooksPath=/dev/null", *args],
capture_output=True, text=True, check=True, timeout=30,
)
git("init", "-q", "-b", "main")
git("config", "user.email", "t@t")
git("config", "user.name", "t")

(tmp_path / "unrelated.txt").write_text("no registry here\n")
git("add", "-A")
git("commit", "-q", "-m", "no registry")
absent = git("rev-parse", "--short", "HEAD").stdout.strip()

target = tmp_path / REGISTRY_RELPATH
target.parent.mkdir(parents=True)
target.write_text("this is not toml = = =\n")
git("add", "-A")
git("commit", "-q", "-m", "not toml")
garbage = git("rev-parse", "--short", "HEAD").stdout.strip()

with pytest.raises(RegistryReadError, match="cannot read the registry"):
registry_at(tmp_path, absent)
with pytest.raises(RegistryReadError, match="did not parse as TOML"):
registry_at(tmp_path, garbage)


def test_rows_refuses_a_section_whose_entries_are_not_tables():
"""`[test_environment]` on the live registry is scalars, not sub-tables.

Nothing breaks today because that table is IGNORED — but the partition check's own
remediation message tells a maintainer to classify a new table CONSUMED, and doing
that for one written this way used to return an `AttributeError` from a dict
comprehension. Register C-91.
"""
scalars = {"test_environment": {"status": "none", "fact": "a sentence"}}
with pytest.raises(RegistryReadError, match=r"\[test_environment\]\.(status|fact) is a bare str"):
rows(scalars, ("test_environment",))

# and the ordinary shape still works, or the refusal above proves nothing
tables = {"target": {"APPWRITE_X": {"class": "target", "value": "v"}}}
assert rows(tables, ("target",)) == {"APPWRITE_X": ("target", "target", "v")}
140 changes: 140 additions & 0 deletions tests/test_seam_registry.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
"""Refusals of the shared seam-registry reader (`tests/seam_registry.py`).

These moved out of `tests/test_env_declaration.py` on 2026-08-14 (issue #265). Their
subject is the *reader* — `registry_at`, `registry_current`, `rows` — not what this
package declares about its environment, and a 1503-line module that had become the home
for both was the clearest signal in the repo that a boundary was wrong.

What deliberately did **not** move with them: the coordinate-value scan. Its subject is
`_EXPECTED_NAMES`, derived from the partner declarations that are the substance of
`test_env_declaration.py`, and separating a guard from the declaration it guards to make
a filename read better trades a real coupling for a filing convenience. The reasoning is
in register C-89.

Every refusal here is a real failure someone hit: an empty pin, a commit this clone does
not have, a ref that is not a commit, a registry that will not parse, and a section whose
rows are scalars rather than tables (C-91).
"""

import subprocess
from pathlib import Path

import pytest

from tests.seam_registry import (
REGISTRY_RELPATH,
RegistryReadError,
registry_at,
registry_current,
rows,
)


def _scratch_repo(tmp_path: Path):
"""A throwaway git repo whose registry differs on `main`, on `origin/main`, and on disk.

`-c` rather than `git config`: a contributor's global `commit.gpgsign` or
`core.hooksPath` would otherwise reach in and either fail opaquely or block on
pinentry with no timeout.
"""
def git(*args):
return subprocess.run(
["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false",
"-c", "core.hooksPath=/dev/null", *args],
capture_output=True, text=True, check=True, timeout=30,
)

target = tmp_path / REGISTRY_RELPATH
target.parent.mkdir(parents=True)

def edition(marker: str) -> str:
return f'[meta]\nversion = "{marker}"\n\n[connection.X]\nclass = "connection"\n'

git("init", "-q", "-b", "main")
git("config", "user.email", "t@t")
git("config", "user.name", "t")
target.write_text(edition("on-main"))
git("add", "-A")
git("commit", "-q", "-m", "main")

# a remote-tracking ref that is AHEAD of main, so preferring one over the other shows
git("checkout", "-q", "-b", "upstream")
target.write_text(edition("on-origin-main"))
git("add", "-A")
git("commit", "-q", "-m", "origin")
git("update-ref", "refs/remotes/origin/main", "HEAD")
git("checkout", "-q", "main")

# and a dirty working tree, which is what #196 was about
target.write_text(edition("in-the-working-tree"))
return tmp_path


def test_registry_current_reads_origin_main_not_the_working_tree(tmp_path):
"""The reason `tests/seam_registry.py` exists, and until now the only untested part.

A sibling clone sits on whatever branch its own agent last worked on. Comparing
against that grades this repository on unreviewed content — issue #196, which cost a
withdrawn pull request. Replacing this function with a working-tree or `HEAD` read
used to leave the whole suite green.
"""
repo = _scratch_repo(tmp_path)
assert registry_current(repo)["meta"]["version"] == "on-origin-main", (
"registry_current read something other than origin/main. A working-tree read is "
"#196 verbatim; a bare `main` read misses that the sibling's remote has moved."
)


def test_registry_current_refuses_a_repo_with_neither_ref(tmp_path):
"""No `origin/main` and no `main` must say so, not return an empty registry."""
subprocess.run(["git", "init", "-q", str(tmp_path)],
capture_output=True, text=True, check=True, timeout=30)
with pytest.raises(RegistryReadError, match="neither origin/main nor main"):
registry_current(tmp_path)


def test_registry_at_refuses_a_commit_whose_registry_is_missing_or_unparseable(tmp_path):
"""`git show` failing, and a blob that is not TOML — two refusal branches nothing reached."""
def git(*args):
return subprocess.run(
["git", "-C", str(tmp_path), "-c", "commit.gpgsign=false",
"-c", "core.hooksPath=/dev/null", *args],
capture_output=True, text=True, check=True, timeout=30,
)
git("init", "-q", "-b", "main")
git("config", "user.email", "t@t")
git("config", "user.name", "t")

(tmp_path / "unrelated.txt").write_text("no registry here\n")
git("add", "-A")
git("commit", "-q", "-m", "no registry")
absent = git("rev-parse", "--short", "HEAD").stdout.strip()

target = tmp_path / REGISTRY_RELPATH
target.parent.mkdir(parents=True)
target.write_text("this is not toml = = =\n")
git("add", "-A")
git("commit", "-q", "-m", "not toml")
garbage = git("rev-parse", "--short", "HEAD").stdout.strip()

with pytest.raises(RegistryReadError, match="cannot read the registry"):
registry_at(tmp_path, absent)
with pytest.raises(RegistryReadError, match="did not parse as TOML"):
registry_at(tmp_path, garbage)


def test_rows_refuses_a_section_whose_entries_are_not_tables():
"""`[test_environment]` on the live registry is scalars, not sub-tables.

Nothing breaks today because that table is IGNORED — but the partition check's own
remediation message tells a maintainer to classify a new table CONSUMED, and doing
that for one written this way used to return an `AttributeError` from a dict
comprehension. Register C-91.
"""
scalars = {"test_environment": {"status": "none", "fact": "a sentence"}}
with pytest.raises(RegistryReadError, match=r"\[test_environment\]\.(status|fact) is a bare str"):
rows(scalars, ("test_environment",))

# and the ordinary shape still works, or the refusal above proves nothing
tables = {"target": {"APPWRITE_X": {"class": "target", "value": "v"}}}
assert rows(tables, ("target",)) == {"APPWRITE_X": ("target", "target", "v")}
Loading