Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 35s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 57s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 37s
PR Gates / Docs update reminder (pull_request) Successful in 1m0s
PR Gates / decisions lifecycle (pull_request) Successful in 20s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 17s
review-verdict/h10 Review-verdict: MERGEABLE @ a7d91bf (base: main)
Review verdict / Set review-verdict status (pull_request_target) Successful in 45s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m25s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m17s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 19m27s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m4s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
`docs.no-session-narrative` reaches every durable artifact, but its detector scanned only `docs/**/*.md` and root markdown, and nothing had ever swept the rest. The issue named four sites from one grep and called them a floor. Deriving the population instead — a whitespace-joined sweep over every tracked file outside the detector, for the detector's own phrasings plus the attribution and review-round class #812 found — gave 453 sites in 108 files at `fb5592971`, and a second pass for phrasings the first list missed (hyphenated `round-N`, "an earlier version", "the reviewer proved") added residuals in the same files. Every site was classified with #812's three dispositions (CUT / SEVER / KEEP with its sub-kind) under the who-benefits test; the per-site manifests are on the PR. The rejected designs, tested-and-rejected fixtures, measurements and traps stay; the attribution of who found them and the round in which they were found go. The detector's population grows to `.claude/`, `.gitea/`, `.husky/` and `scripts/` regardless of extension, minus the detector and its own test (whose fixtures ARE the phrasings) and minus `scripts/tests/fixtures/` (test data, including decision-record copies — the same reasoning as the records' own exemption, and what keeps the record's depth measurement true), and `--all` lists tracked REGULAR files only — a symlink's content is its target and a gitlink has none. The #812 argument for leaving `docs/superpowers/**` in the population runs the other way here: `--diff` sees only ADDED lines, and 287 of the 453 sites were under 30 days old — this corpus is where narrative is being added, so the advisory nudge has reach. Density agrees: 56 line-mode hits over the 113 regular files the predicate admits, against 9 over 66 docs files before #812. `web/` and C# stay out on the same measurement (3 of 74 PATTERNS-matching sites, ~4,600 files). The predicate did not grow: PATTERNS matched 74 of 453 sites, and widening the word list to the attribution class is the treadmill the withdrawn parity test ran on. The population oracle is restated over segments with the new arms, the synthetic cross product gains the process heads and non-markdown extensions, a fixture witnesses that a tracked symlink is neither scanned nor counted, a `.py.bak` axis separates a by-name exemption from a `startswith` over the same tuple, and eight mutants (drop the process arm, drop the by-name exemption, exempt by `startswith`, drop or add a prefix, drop the fixtures exemption, list only markdown, drop the symlink filter, test the mode per row instead of per path) each redden it. A pre-existing silent drop in `--diff` goes with it: git tab-terminates a `+++` filename that contains a space, and the kept tab made `is_scanned_path` refuse the file with no notice — fixed, with a positive control and its own mutant. Code is unchanged by construction, measured per file type against `origin/main`: Python modules are AST-equal with docstrings stripped, except `#` lines inside the embedded fixture programs (string literals) of three test modules; workflows differ only in `#` lines inside `run:` block scalars; shell, C#, TypeScript and jq are equal with comment lines stripped. The stated exceptions: the detector and its test, 26 vitest titles that carried review-round or severity labels or a reviewer attribution (call sites whose title changed — every changed title line walked back to its `it(` / `it.each(...)(` anchor, so a `' + '` concatenation counts once), two registry note strings and the mutation manifest's prose fields. scripts/tests: 1565 passed. Web: lint, typecheck, 1319 tests green. Closes #876. Decisions-Edit: yes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEcBoFw7ctrf3Nb7R7x7wk
113 lines
6.9 KiB
Python
113 lines
6.9 KiB
Python
"""Keep the test suite out of the PRODUCTION hook-fire log (ersatztv#776, #809, #822).
|
|
|
|
Several suites here drive real hooks as subprocesses. Four of them predate the instrumentation —
|
|
`test_merge_consent_exemption.py`, `test_merge_consent_base_change.py`,
|
|
`test_prepush_rebase_check_tag_exemption.py` and `test_pr_changed_files.py`; `test_hook_fire_log.py`
|
|
and `test_worktree_ownership_guard.py` have joined them since. Once every hook records its own
|
|
execution, running `pytest scripts/tests` wrote its synthetic invocations into
|
|
`$HOME/.cache/ersatztv/hook-fire/`: 96 `pretooluse-merge-consent` fires including two `deny`s, and
|
|
12 `blocked` decisions from `prepush-rebase-check`, none of them a real session.
|
|
|
|
That is not untidiness, it is the defect the whole change exists to remove. `scripts/hook-fire-log.sh
|
|
report` is meant to answer "what did the harness actually do", and a log carrying test artifacts
|
|
answers a different question while looking identical — inference reintroduced one layer up. The
|
|
record has no field distinguishing a test invocation from a real one, and adding one would only move
|
|
the problem, so the fix is that tests never write to the real log at all.
|
|
|
|
TWO LAYERS, because one is not enough and the two fail differently.
|
|
|
|
`pytest_configure` puts an isolated directory into `os.environ` BEFORE any test module is imported.
|
|
That is what carries the isolation to suites other than the hook-driving four: a module that
|
|
snapshots `{**os.environ}` at import time captures the isolated value, because `pytest_configure`
|
|
runs before collection. #785 shipped exactly that snapshot, inside the file added to prove #776's
|
|
hooks, and it leaked 58 records per run while every assertion stayed green — the fire-log library is
|
|
fail-open by design, so nothing can surface from the hook side. The ordering this relies on was
|
|
verified by execution rather than read: a module-level `{**os.environ}` does carry a value set in
|
|
`pytest_configure`. The autouse fixture below still re-points the variable per test, so a test that
|
|
inspects its own log dir sees only its own records.
|
|
|
|
The route the first layer cannot see is an environment built from SCRATCH rather than derived from
|
|
`os.environ`, carrying no `ETV_HOOK_FIRE_LOG_DIR` at all — so `scripts/hook-fire-log.sh` falls back
|
|
to its own default. No snapshot is involved, so nothing about import ordering helps.
|
|
`ProductionLogGuard` closes it at the launch itself: a child that does not carry the isolated
|
|
directory fails the test that launched it. Its population is every launch that actually happens,
|
|
derived at runtime rather than from a list or from source text, so a new suite is covered by
|
|
existing — every launch, that is, that goes through `subprocess.Popen` and is the DIRECT child.
|
|
`ProductionLogGuard`'s own docstring enumerates what that leaves out (a re-exec, `os.execve`, a
|
|
script that unsets the variable itself, a `Popen` bound by name before this hook ran); none occur
|
|
here today, and the list is written down so the population is not read as total.
|
|
|
|
The rule is "CARRIES AN ISOLATED DIRECTORY", not "is not the production one", and the difference is
|
|
load-bearing: the sink's default has TWO branches — `$HOME/.cache/...` and, when `HOME` is unset or
|
|
empty, `/tmp/.cache/...` — so comparing against a single resolved production path models one and
|
|
waves the other through. On the same tree, the launches whose environment is built FROM SCRATCH —
|
|
the ones the first layer cannot reach — also number 83 across the helpers this change updates;
|
|
81 of those carry no `HOME` either and so RESOLVE TO `/tmp/.cache/ersatztv/hook-fire`, which
|
|
is just as shared, just as persistent, and which the predicate above cannot see. (Two different sets
|
|
of the same size, said out loud because a later reader would otherwise reconcile them as one.)
|
|
Resolve to, not write to: those particular children are `bash`, `git` and a nested pytest that
|
|
never source the sink, and that directory does not exist on this machine. The guard is deliberately
|
|
conservative about which launches it rejects — see `ProductionLogGuard` — because whether a given
|
|
command will reach the sink is not answerable at the launch. Requiring the variable covers both
|
|
branches and does not depend on what `HOME` is on the machine running the suite — which is also what
|
|
keeps it from firing on a legitimate launch wherever `HOME` is unset or is itself
|
|
`/tmp`. Its corollary is that a from-scratch environment must carry the variable; the guard
|
|
enumerates those sites, so a missed one is a red rather than a stale sentence here.
|
|
|
|
WHY NEITHER LAYER OBSERVES THE PRODUCTION DIRECTORY. The obvious check — snapshot it around each
|
|
test and compare — is what shipped, and its oracle is state every concurrent session on the machine
|
|
writes: that directory is shared by every live Claude Code session, so an unrelated session firing a
|
|
hook inside the window reddened the suite with `a test run modified the production hook-fire log` —
|
|
an accusation about the suite when the writer was another process (#822). Observed three times on
|
|
three different branches, each time green on an immediate re-run. Both layers here are structural
|
|
and read nothing under `$HOME`.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import shutil
|
|
import tempfile
|
|
|
|
import pytest
|
|
|
|
from scripts.tests.hook_fire_isolation import ENV_VAR, ProductionLogGuard
|
|
|
|
_GUARD = ProductionLogGuard()
|
|
_SESSION_LOG_DIR: str | None = None
|
|
|
|
|
|
def pytest_configure(config):
|
|
"""Isolate BEFORE collection, so an import-time `{**os.environ}` snapshot inherits the isolation.
|
|
|
|
This is also what covers MODULE- and SESSION-scoped fixtures, which the autouse fixture below
|
|
cannot reach: they are set up outside any single test, so `os.environ` had no isolated value
|
|
while they ran. Measured once on the pre-change tree, asking which launches resolve to
|
|
`$HOME/.cache/ersatztv/hook-fire`: 83 — 37 during collection and 44 inside module- or
|
|
session-scoped fixtures. The remaining 2 came from an environment built from scratch, which is
|
|
the group this layer cannot help with and the reason there is a second one.
|
|
|
|
INSTRUMENT `Popen` ONLY. `subprocess.run`, `call` and `check_output` all reach `Popen`, so
|
|
wrapping `run` as well counts every launch twice and doubles every figure.
|
|
"""
|
|
global _SESSION_LOG_DIR
|
|
_SESSION_LOG_DIR = tempfile.mkdtemp(prefix="etv-hook-fire-session-")
|
|
os.environ[ENV_VAR] = _SESSION_LOG_DIR
|
|
_GUARD.install()
|
|
|
|
|
|
def pytest_unconfigure(config):
|
|
_GUARD.uninstall()
|
|
if _SESSION_LOG_DIR:
|
|
shutil.rmtree(_SESSION_LOG_DIR, ignore_errors=True)
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def isolate_hook_fire_log(tmp_path_factory, monkeypatch):
|
|
"""Re-point per test, so a test reading its own log dir sees only its own records.
|
|
|
|
`monkeypatch` reverts to the value in place at setup — the session directory from
|
|
`pytest_configure`, never to unset — so the layer above survives every teardown.
|
|
"""
|
|
monkeypatch.setenv(ENV_VAR, str(tmp_path_factory.mktemp("hook-fire-log")))
|