Skip to content

#247: one condition, one diagnosis — and the scratch repos stop being able to hang the suite #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Polichinel merged 1 commit into from
Aug 12, 2026
Merged

#247: one condition, one diagnosis — and the scratch repos stop being able to hang the suite #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 36 additions & 23 deletions reports/technical_risk_register.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
| Owner | Dylan Pinheiro / PRIO MD&D Team |
| Last Updated | 2026-08-12 |
| Total Concerns | 97 |
| Open Concerns | 22 |
| Resolved Concerns | 75 |
| Open Concerns | 21 |
| Resolved Concerns | 76 |

---

Expand Down Expand Up @@ -313,27 +313,6 @@ Cross-refs: **C-57** (the scan's own entry and its history), **C-93** (why the a

---

### C-91: The git plumbing this arc added turns ordinary developer states into hard errors, bare tracebacks, and one possible hang

| Field | Value |
|-------|-------|
| ID | C-91 |
| Tier | 3 — no wrong data and nothing silent. It taxes every contributor who does not already have the exact sibling checkout this repository assumes, and it does so with diagnoses that point at the wrong cause. |
| Source | `/code-review max` on PR #239 post-merge, 2026-08-11 |
| Trigger | Any of: a contributor clones views-appwrite shallow, single-branch, or before the pinned commit; a table classified `CONSUMED` upstream is written as flat keys rather than sub-tables; a contributor has `commit.gpgsign` or a global `core.hooksPath` set. |
| Owner | This repository. |
| Location | `tests/seam_registry.py:71` (refusal diagnoses), `:150` (`rows`), `tests/test_env_declaration.py:820` (the scratch repo). |

**A stale clone produces four errors carrying the wrong explanation.** The reader the extraction replaced read the file off disk, so an older checkout simply read an older file. Now a clone that predates the pinned commit — or is shallow, or was made `--single-branch`, which matters because views-appwrite's default branch is not `main` — raises *"does not resolve to a commit … an empty ref reads the index and a branch reads a moving tip"*. That names neither cause and does not say `git fetch`. Meanwhile `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` detects the identical root cause and *skips* with the right remedy. One condition, one skip, four errors, three explanations.

**`rows()` raises a bare `AttributeError` on a shape the live registry already has.** It guards a null section and not a scalar row. Verified: views-appwrite's `[test_environment]` holds `status` and `fact` as top-level strings. That table is `IGNORED`, so nothing breaks today — but when the partition check fires on a new upstream table, its own message instructs the maintainer to classify it `CONSUMED` or `MIRRORED`, and doing so for a table written that way returns a traceback pointing into a dict comprehension. From the module whose docstring says a helper justified by failing legibly must not hand back a bare traceback.

**The scratch repo inherits the developer's global git config and has no timeout.** `test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong` sets `user.name` and `user.email` and stops. With `commit.gpgsign = true` it fails with a bare `CalledProcessError` — `capture_output=True` swallows git's explanation. With a passphrase-protected key it blocks on pinentry with no `timeout`, hanging the whole run; `conftest.git_output`, which this helper bypasses, caps at 30 seconds. The leak was anticipated for identity and not for the setting that blocks.

Cross-refs: **C-90** (the same module's untested core), **C-88** (why the module exists outside `conftest.py`), ADR-008 (explicit failure), issue #196.

---

### C-92: Nothing here checks that a consumer SELECTS by the delivery label — for either partner

| Field | Value |
Expand Down Expand Up @@ -1026,6 +1005,40 @@ See also C-40 (the inheritance/representation coupling this migration unwinds),

## Resolved Concerns

### C-91: The git plumbing this arc added turns ordinary developer states into hard errors, bare tracebacks, and one possible hang — RESOLVED

| Field | Value |
|-------|-------|
| ID | C-91 |
| Tier | 3 — no wrong data and nothing silent. It taxes every contributor who does not already have the exact sibling checkout this repository assumes, and it does so with diagnoses that point at the wrong cause. |
| Source | `/code-review max` on PR #239 post-merge, 2026-08-11 |
| Trigger | Any of: a contributor clones views-appwrite shallow, single-branch, or before the pinned commit; a table classified `CONSUMED` upstream is written as flat keys rather than sub-tables; a contributor has `commit.gpgsign` or a global `core.hooksPath` set. |
| Owner | This repository. |
| Location | `tests/seam_registry.py:71` (refusal diagnoses), `:150` (`rows`), `tests/test_env_declaration.py:820` (the scratch repo). |

**A stale clone produces four errors carrying the wrong explanation.** The reader the extraction replaced read the file off disk, so an older checkout simply read an older file. Now a clone that predates the pinned commit — or is shallow, or was made `--single-branch`, which matters because views-appwrite's default branch is not `main` — raises *"does not resolve to a commit … an empty ref reads the index and a branch reads a moving tip"*. That names neither cause and does not say `git fetch`. Meanwhile `test_the_pinned_commit_is_reachable_from_the_contract_repos_main` detects the identical root cause and *skips* with the right remedy. One condition, one skip, four errors, three explanations.

**`rows()` raises a bare `AttributeError` on a shape the live registry already has.** It guards a null section and not a scalar row. Verified: views-appwrite's `[test_environment]` holds `status` and `fact` as top-level strings. That table is `IGNORED`, so nothing breaks today — but when the partition check fires on a new upstream table, its own message instructs the maintainer to classify it `CONSUMED` or `MIRRORED`, and doing so for a table written that way returns a traceback pointing into a dict comprehension. From the module whose docstring says a helper justified by failing legibly must not hand back a bare traceback.

**The scratch repo inherits the developer's global git config and has no timeout.** `test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong` sets `user.name` and `user.email` and stops. With `commit.gpgsign = true` it fails with a bare `CalledProcessError` — `capture_output=True` swallows git's explanation. With a passphrase-protected key it blocks on pinentry with no `timeout`, hanging the whole run; `conftest.git_output`, which this helper bypasses, caps at 30 seconds. The leak was anticipated for identity and not for the setting that blocks.

**RESOLVED 2026-08-12 (#247) — all three.**

**One condition, one diagnosis.** A ref this clone cannot see and a ref that is not a frozen commit used to share a message that named neither cause and never said `git fetch`. They are now separate branches with separate remedies, and a third — an empty pin — is called what it is: a defect in the pin, not the checkout. The bogus-sha case is deliberately classified as *"this clone cannot see it"*, because that is the truth: the reader cannot tell a bad pin from a missing fetch, and the message says so rather than guessing.

**`rows()` refuses a scalar row by name.** `[test_environment]` on the live registry is top-level strings; classifying such a table CONSUMED — which the partition check's own remediation message invites — used to return an `AttributeError` from a dict comprehension, in the module whose justification is failing legibly.

**The scratch repositories are hermetic.** All three now run git with `-c commit.gpgsign=false -c core.hooksPath=/dev/null` and an explicit timeout. Verified by running the suite under a `HOME` whose `.gitconfig` sets `commit.gpgsign = true` and points `core.hooksPath` at a nonexistent directory: four tests pass where they would previously have failed opaquely or blocked on pinentry with no timeout.

Mutation-proven three ways, each reverted: removing the scalar-row refusal, the missing-object branch, and the empty-pin branch.

Cross-refs: **C-90** (the same module's untested core), **C-88** (why the module exists outside `conftest.py`), ADR-008 (explicit failure), issue #196.

---

---


### C-90: A mutation proof that cannot fail, and the untested function a module was extracted to create — RESOLVED

| Field | Value |
Expand Down
52 changes: 44 additions & 8 deletions tests/seam_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,34 @@ def registry_at(repo: Path, ref: str) -> dict:
capture_output=True, text=True, check=False, timeout=30,
)
if resolved.returncode != 0 or not resolved.stdout.strip():
# Two conditions used to share this message, and they have different remedies.
# A ref this clone has never heard of is an environment problem — a shallow or
# `--single-branch` clone, or one made before the pin. A ref that resolves to
# something that is not a commit is a pin defect. Only the second is this
# repository's fault; only the first is fixed by fetching (register C-91).
if not ref.strip():
raise RegistryReadError(
"the pin is empty, and an empty ref does not resolve to a commit — "
"`git show ':<path>'` reads the INDEX, so a blanked pin would compare "
"the registry to itself and report green against every future edition. "
"This is a defect in the pin, not in the checkout."
)
known = subprocess.run(
["git", "-C", str(repo), "cat-file", "-e", f"{ref}^{{object}}"],
capture_output=True, text=True, check=False, timeout=30,
).returncode == 0
if not known:
raise RegistryReadError(
f"{repo} has no object {ref!r}. This is almost always a clone that is "
"shallow, `--single-branch`, or simply older than the pin — run "
f"`git -C {repo} fetch --tags origin` and try again. It is not a defect "
"in the pin: nothing here can tell whether that commit is good until "
"this checkout can see it."
)
raise RegistryReadError(
f"{ref!r} does not resolve to a commit in {repo}. A pin must name a frozen "
"commit: an empty ref reads the index and a branch reads a moving tip, and "
"either would make the comparison compare the registry to itself."
f"{ref!r} exists in {repo} but does not resolve to a commit. A pin must name "
"a frozen commit: an empty ref reads the index and a branch reads a moving "
"tip, and either would make the comparison compare the registry to itself."
)
if not resolved.stdout.strip().startswith(ref):
raise RegistryReadError(
Expand Down Expand Up @@ -146,8 +170,20 @@ def rows(registry: dict, sections: tuple) -> dict[str, tuple]:
The one shared projection. Two hand-copied versions had already diverged on null
handling — one raised ``AttributeError`` on a null section, the other did not.
"""
return {
name: (section, body.get("class"), body.get("value", ABSENT))
for section in sections
for name, body in (registry.get(section) or {}).items()
}
out = {}
for section in sections:
for name, body in (registry.get(section) or {}).items():
if not isinstance(body, dict):
# `[test_environment]` on the live registry is exactly this — top-level
# strings, not sub-tables. Reaching it means someone classified such a
# table CONSUMED, which the partition check's own remediation message
# invites. Refusing by name beats an AttributeError from a comprehension
# in the module whose justification is failing legibly (register C-91).
raise RegistryReadError(
f"[{section}].{name} is a bare {type(body).__name__}, not a table. "
"This section's rows are scalars, so it carries no class or value to "
"read — it cannot be CONSUMED. Classify it IGNORED with a reason, or "
"read it with something other than `rows()`."
)
out[name] = (section, body.get("class"), body.get("value", ABSENT))
return out
39 changes: 35 additions & 4 deletions tests/test_env_declaration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
registry_at as _registry_at,
registry_current,
registry_current as _registry_current,
rows,
rows as _rows,
)
from tests.conftest import (
Expand Down Expand Up @@ -882,7 +883,15 @@ def test_the_pinned_reader_refuses_every_way_a_baseline_can_be_wrong(tmp_path):
import subprocess as sp

def git(*args):
return sp.run(["git", "-C", str(tmp_path), *args], capture_output=True, text=True, check=True)
# `-c`, and a timeout. A contributor's global `commit.gpgsign` makes `git commit`
# fail with a bare CalledProcessError here — capture_output swallows git's
# explanation — and with a passphrase-protected key it blocks on pinentry with
# stdin inherited and no timeout, hanging the whole run (register C-91).
return sp.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")
git("config", "user.email", "t@t")
Expand All @@ -905,16 +914,21 @@ def commit(text: str, message: str) -> str:
"the reader must accept a well-formed registry, or the refusals below prove nothing"
)

with pytest.raises(_RegistryReadError, match="does not resolve to a commit"):
# The five refusals, and note that two of them now say DIFFERENT things. A ref this
# clone cannot see and a ref that is not a commit used to share one message; they have
# different remedies — `git fetch` versus fix the pin — and conflating them sent
# contributors to the wrong one (register C-91).
with pytest.raises(_RegistryReadError, match="the pin is empty"):
_registry_at(tmp_path, "") # a blanked pin reads the INDEX
with pytest.raises(_RegistryReadError, match="annotated TAG|does not start with it"):
_registry_at(tmp_path, "v1") # a tag object: git peels it, the URL 404s
with pytest.raises(_RegistryReadError, match="is EMPTY"):
_registry_at(tmp_path, empty) # exits 0, stdout empty
with pytest.raises(_RegistryReadError, match="no meta.version or no"):
_registry_at(tmp_path, anchorless) # parses, but is not the registry
with pytest.raises(_RegistryReadError, match="does not resolve to a commit"):
_registry_at(tmp_path, "0" * 40) # a pin that names nothing
with pytest.raises(_RegistryReadError, match="has no object"):
_registry_at(tmp_path, "0" * 40) # this clone cannot see it; it cannot tell
# a bad pin from a missing fetch, and says so


def test_the_role_vocabulary_is_closed():
Expand Down Expand Up @@ -1470,3 +1484,20 @@ def git(*args):
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