ADR-0169: The MOCK APPROVED Rule Enforces a Checkable Alternative, Not a Human Approver

Date: 2026-08-01 Status: Accepted Deciders: Adrian (Soft4U BV), Claude Opus (implementation + sampling), methodology audit epic Tsunami-max/s4u-methodology#23 (finding, adversarially verified)

Decision context:

• Latency: the detector scans 761 test files in ~1.5 s and runs as four tests inside the existing suite. No measurable effect on suite wall-clock.
• Dependency surface: none added. backend/tests/mock_governance.py uses re, json, pathlib and dataclasses from the standard library.
• Debuggability: the failure message names the file, the violation kind, the exact fictitious gate, and the real gates available. A missing_block failure prints the three-line template.

Context

CLAUDE.md forbids mocking by default and offers one escape hatch:

# MOCK APPROVED: <what and why>
# Approved by: [name] on [date]
# Alternative: <how to run against the real thing>

The rule's stated detector is prose — appendix-a-testing.md:309: "Files using mocks without the approval comment are flagged during code review." No mechanical check exists anywhere in either repo. The methodology's own spine convicts this shape at methodology.md:1409: "Every gate or ritual must declare … an enforcement mechanism — a hook, CI check, or script ('the agent will remember' is not a mechanism). A gate that cannot name its mechanism is a wish."

Issue #973 measured the result: 25% of mock-using files carry no block at all, and 72% of the approvals present are self-issued by the agent that wrote them.

The obvious response is to enforce the rule as written — require the block, and reject a self-issued Approved by:. Before doing that, the self-issued blocks were sampled, to answer a question the counts cannot: are these legitimate mocks (a rate-limited third party — exactly what the exception exists for), or mocks that hid a real integration?

Evidence

A 24-file stratified sample of files carrying a non-human Approved by: (every 6th of the 144-file sorted frame, spread across api/, services/ and root), plus a 4-file sample of human-named approvals and a 16-file sample of the no-block set.

Sample 1 — self-issued approvals (24 files):

Class	Count
Hid a real integration (DB session, MinIO, Temporal, first-party app.* service)	12 (50%)
Ambiguous (7 of 8 are the Neo4j cluster — no Neo4j container exists)	8
Legitimate third party (PydanticAI/OpenAI, NorthData, the eID vendor stub)	3
Not a violation (settings toggling)	1

Population-wide the same shape holds: 119 of 254 mock-using files patch a first-party app.* symbol; only 4 patch an external SDK or HTTP boundary. The most-patched targets in the whole suite are this project's own seams — graph_service.settings (52 files), get_tenant_session (51), registry_activity.update_status (44), registry_activity.MinIOService (38). 61 files build an AsyncMock() and use it as the database session. The exception was written for third-party cost and rate limits; the dominant real use is substituting infrastructure this repo has testcontainers for.

So: 72% self-issued is not a clerical problem. Half the sampled self-issued mocks replaced something the repo can genuinely run.

Sample 2 — human-named approvals (4 files). This is what changed the fix direction. Human approval does not predict legitimacy. test_ubo_service.py carries Approved by: Adrian (project owner) on 2026-06-15 over an AsyncMock database session, justified as a Neo4j mock, citing a gate that does not exist — structurally identical to the developer set. Enforcing a human name would have added friction and caught nothing.

Sample 3 — the 64 no-block files (16 sampled). They are cleaner than the approved ones: 5/15 are clean third-party mocks (VIES, the eID vendor's httpx client, PydanticAI), 2 are not violations, several run against real testcontainers, and 3 of the 64 are detector false positives (the word appears only in prose). Block presence anti-correlates with mock legitimacy — consistent with the blocks having been added by a remediation sweep over whatever a grep flagged, while genuinely-external mocks written in normal flow were never swept.

The finding that actually generalises. Every Alternative: line was resolved against app/config.py and against every os.getenv / os.environ read in the repo:

Cited gate family	Status
NEO4J_LIVE — cited 21×	read by nothing, anywhere
REAL_OPENAI (the methodology template's own placeholder), REAL_TAVILY, REAL_BRIGHTDATA, CRAWL4AI_LIVE, USE_REAL_TEMPORAL, REAL_REDIS, LEX_LIVE_EMBED, …	read by nothing
OSINT_MOCK_MODE, SCAN_MOCK_MODE, KYC_SCREENING_MOCK_MODE, NEO4J_ENABLED, REAL_LLM, …	real, honoured

48 files promise an escape route that does not exist. The pattern is unmistakable: authors copied REAL_OPENAI out of the template in CLAUDE.md, then generalised its shape — REAL_KBO, REAL_VIES, NEO4J_LIVE — without ever wiring one. And in four sampled files the justification names an external service (Letta, OpenSanctions, KBO, PEPPOL, VIES API) that the code never actually patches. The block is prose that was never checked against its own code — this repo's own claim_vs_check defect class, occurring inside the control meant to prevent it.

Decision

Direction (b): amend the rule to the property that carries the meaning, then enforce that. Three parts.

1. The enforced contract

A file that substitutes a real integration must carry a block with all three lines, and the Alternative: must be checkable: either an env gate that something in app/ or tests/ actually reads, or prose naming a sibling test that covers the path for real.

Approved by: remains required — a mock with no named owner is not reviewable — but who signs it is recorded, not policed. The evidence does not support gating on approver identity, and no canon text ever required a second party for a mock annotation (the ADR-0070 four-eyes analogy in #973 is the auditor's inference; ADR-0070 governs case approvals). Enforcing a proxy the data shows is uncorrelated with the harm would produce exactly the false-alarm rate that gets a control switched off — the ADR-0121 lesson.

The self-issued count (129 files) and the work-item-as-approver count (13 files) are measured and recorded in the baseline register on every run, so the number stays visible and can be revisited if the correlation ever changes.

The 25% missing-block set is treated as a hard violation under both readings, as #973 requires: a mock with no stated reason is a mock nobody can review.

2. The detector — backend/tests/mock_governance.py + test_mock_governance.py

Runs inside the normal pytest suite, so it fires in the Backend Tests CI job and on any local run. It does not depend on branch protection to have an effect — which matters, because master has none (#963).

The false-positive guards are load-bearing. This project genuinely does not mock its own infrastructure, and a detector that said otherwise would be deleted within a week. Excluded by construction: respx / httpx.MockTransport HTTP-boundary fakes (CLAUDE.md explicitly permits these); patch("...settings...") feature-flag toggling; monkeypatch (214 files); conftest.py's ~35 plain async def mock_* Temporal activity stubs (a name-based detector would fire on every one); and prose-only matches. Exclusion is per-seam, not per-file — a file that uses respx and mocks a DB session is in scope for the second.

A fourth test is a meta-guard: the cheapest way to make this gate green is to widen an exclusion until nothing is in scope, so the in-scope population is pinned at ≥200 (241 measured) and the excluded-but-mocking set is capped at a quarter of it.

3. The ratchet — mock_governance_baseline.json

137 files currently violate (59 missing block, 48 unresolvable alternative, 32 missing alternative, 4 missing approver). #973 is explicit that the backfill is "a tracked debt item, not in one sweep", so today's violations are declared and the gate enforces a one-way ratchet, the same shape as the ADR-0121 citation lockfile: a new violation fails immediately, and a repaired violation still listed also fails — so the register can only shrink and can never quietly re-absorb a regression.

Consequences

Positive. The rule has a mechanism. New mocks cannot enter without a reviewable, checkable justification. The 48 fictitious escape hatches are now enumerated, and each is a small, obvious fix (wire the gate, cite a real one, or name a sibling test). The register makes the debt a number that can only go down.

Negative / honest limits.

• The 137 declared violations are not fixed by this change. They are frozen. If nobody works the register down it stays at 137 forever, and this ADR will have bought only "it stopped getting worse". That is a real outcome, not a hidden one.
• A checkable alternative is not a correct one. Alternative: OSINT_MOCK_MODE=false passes whether or not the test actually works that way. The gate proves the escape route exists, not that it was ever taken.
• The detector is regex over source, not AST. It will miss an exotic mocking idiom and could misread an unusual patch() spelling. It was mutation-tested in both directions (13 mutations, including four deliberate false-positive probes) rather than assumed correct — and one probe found a genuine false positive in an earlier revision (patch("app.config.settings.osint_mock_mode"), flag toggling, was being flagged as an unapproved mock because the settings pattern was anchored to the final path segment). That is fixed and pinned.
• The Neo4j / Redis / Keycloak cluster is legitimately stuck. No testcontainer exists for any of them, so those mocks are unavoidable. Their defect is the fictitious NEO4J_LIVE alternative, not the mock. Adding a Neo4j container would convert ~21 ambiguous cases into either clean tests or genuine violations — a worthwhile follow-up this ADR does not take.
• This diverges from the methodology's canon text, which still describes the detector as code review and implies an approver. The divergence is deliberate and recorded here; upstreaming it to s4u-methodology is a separate change in a separate repo.

Alternatives considered

(a) Enforce the rule as written — require a human approver, reject self-issued. Rejected on the evidence. Human-named approvals in the sample were not more legitimate than agent-named ones; the single project-owner-approved file examined mocked a DB session behind a Neo4j justification with a dead gate. It would impose friction proportional to the whole suite and catch approximately none of the harm.

Fix the 137 violations in one sweep and gate at zero. Rejected: #973 explicitly asks for tracked debt rather than a sweep, and a 137-file mechanical edit across the test suite is unreviewable — precisely the objection that keeps ruff format unrun in this repo.

Ban unittest.mock outright where a testcontainer exists. Attractive, and closer to the canon's intent. Rejected for now as too large a behavioural change to land inside a governance PR; it is the natural next step once the register is worked down, because at that point the remaining entries are the list of places to do it.

Put the detector in CI only, not in pytest. Rejected: master has no branch protection (#963), so a CI-only check would be advisory. Riding the test suite means a developer's local pytest sees it.