From a29f864b88ecd45ada911cb75ca2303de74a66f9 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 02:52:43 +0800 Subject: [PATCH 1/7] emrg: a test's scratch directory is not tree dirt Measured 2026-09-17: a SIGKILL inside tests/test_bash_tool_sandbox.py's corpus loop (kill at 0.35s, the test's window is ~0.5s) left tests/emrg-corpus-fapk6xox/, untracked and unignored, and the dirty-tree criterion then answered loses=True ("could not be read to compare") - so the next cycle keeps the read-only tier because a test was interrupted; with the residue removed the same criterion answers loses=False. Five mkdtemp(dir=) sites write those names and a finally does not run under SIGKILL. The names are now ignored (/tests/emrg-*, scoped, with the measurement in the comment) and tests/test_scratch_roots_are_gitignored.py keeps a new site from reintroducing the exposure: it reads the mkdtemp calls out of the syntax and asks git about each name a test can create. --- .gitignore | 14 ++ tests/test_scratch_roots_are_gitignored.py | 159 +++++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 tests/test_scratch_roots_are_gitignored.py diff --git a/.gitignore b/.gitignore index b0d9b8d6..79abd276 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,20 @@ node_modules/ # forms. Verified safe: `git ls-files | grep '\.emrg-'` is empty. .emrg .emrg-* +# The test suite's own scratch directories: five `mkdtemp(dir=)` sites in +# `tests/test_bash_tool_sandbox.py` run a POSIX shell in a private directory and delete it in +# a `finally` — and a `finally` does not run under SIGKILL. Measured 2026-09-17: a kill +# inside the corpus test's loop left `tests/emrg-corpus-fapk6xox/`, `git status` listed it as +# `??`, `git check-ignore` said it was not ignored, and the dirty-tree criterion answered +# `loses=True` ("could not be read to compare") — so the next evolution cycle keeps the +# read-only tier because a *test* was interrupted; with the residue removed the same +# criterion answers `loses=False`. An untracked directory really cannot be compared to +# `HEAD`, so the criterion is right and the names are the problem: these are scratch, never +# deliverables. Scoped to `/tests/` on purpose — this is about those sites, not about any +# `emrg-*` file a future contributor may want tracked at the root. +# `tests/test_scratch_roots_are_gitignored.py` is the mechanic that keeps a new site from +# reintroducing the exposure. +/tests/emrg-* .claude .pytest_cache/ /dist/ diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py new file mode 100644 index 00000000..a5692b6f --- /dev/null +++ b/tests/test_scratch_roots_are_gitignored.py @@ -0,0 +1,159 @@ +"""Guard: a scratch directory a test creates inside the repository must be invisible to git. + +The exposure, measured 2026-09-17 in this repository +---------------------------------------------------- +Five sites in `tests/test_bash_tool_sandbox.py` run a POSIX shell in a private +`mkdtemp(dir=)` and remove it in a `finally`. A `finally` does not run +under SIGKILL, and the residue is **tree dirt**: killing the corpus test inside its loop +(measured: kill at 0.35 s, the test's own window is ~0.5 s) left +`tests/emrg-corpus-fapk6xox/`, `git status` listed it as `??`, `git check-ignore` answered +*not ignored*, and the dirty-tree criterion — `TaskHandler._dirty_tree_would_lose_work_sync`, +the guard that decides whether a cycle keeps its tier — answered `loses=True`: + + tests/emrg-corpus-fapk6xox/ could not be read to compare + +so the **next** evolution cycle keeps the read-only tier because a *test* was interrupted. +Removing that one directory, the same criterion answers `loses=False` (`the tree is clean`, +`git status` empty). That makes this the expensive class this repo has already paid for once +(33 consecutive zero-commit cycles, issue #1237), charging a cycle for a scratch directory +holding no work at all. + +The criterion is not the thing to change — an untracked *directory* genuinely cannot be +compared to `HEAD`, so unique work and scratch look alike to it, and `loses=True` is the +safe answer there. The names are the thing to change, and `.gitignore` now carries them +(`/tests/emrg-*`, whose comment states the same measurement). This file is the mechanic that +keeps a new site from reintroducing the exposure, because a comment cannot. + +The rule +-------- +For every `mkdtemp(dir=…)` call in `tests/` whose `dir=` expression is **derived from +`__file__`** — i.e. a path *inside this repository*, since the module lives in `tests/` — the +name that call can create must be matched by git's ignore rules. A root that is not +`__file__`-derived (a `tmp_path` fixture, the default system temp) cannot dirty the tree and +is out of scope; that distinction is read from the syntax rather than kept in a list, so a +new site is found by *writing the call*, not by remembering to edit this file. + +Two honest boundaries, stated rather than implied: +* a site with no literal `prefix=` cannot be measured statically (the name `mkdtemp` invents + is random), so it is reported as an unmeasurable site with the remedy — never as a pass; +* the check asks git about a *synthesised* name (`x`), not about a directory that + exists, which is what makes it runnable on a clean tree (`git check-ignore` answers for + paths that do not exist — verified: rc=1 for an unignored path, rc=0 for an ignored one). +""" + +from __future__ import annotations + +import ast +import subprocess +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +TESTS_DIR = REPO_ROOT / "tests" + + +def _sites() -> list[tuple[Path, int, str | None, str]]: + """Every `mkdtemp(dir=…)` call in `tests/`, as (file, line, prefix, dir source). + + `dir source` is the expression as written (`ast.unparse`), which is what decides whether + the site is in scope: `__file__` in it — directly or through a name assigned to such an + expression in the same module — means the root is a path inside the repository. + """ + sites: list[tuple[Path, int, str | None, str]] = [] + for path in sorted(TESTS_DIR.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + file_derived: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Assign) and "__file__" in ast.dump(node.value): + file_derived.update(t.id for t in node.targets if isinstance(t, ast.Name)) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + called = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "") + if called != "mkdtemp": + continue + kw = {k.arg: k.value for k in node.keywords if k.arg} + if "dir" not in kw: + continue + source = ast.unparse(kw["dir"]) + is_file_derived = "__file__" in source or ( + isinstance(kw["dir"], ast.Name) and kw["dir"].id in file_derived + ) + if not is_file_derived: + continue + prefix = kw.get("prefix") + prefix = (prefix.value if isinstance(prefix, ast.Constant) + and isinstance(prefix.value, str) else None) + sites.append((path, node.lineno, prefix, source)) + return sites + + +def _unignored(candidates: list[str]) -> list[str]: + """The candidates git's ignore rules do not match, asked one at a time. + + `git check-ignore -q` exits 0 for a matched path and 1 for an unmatched one, and it + answers for paths that do not exist — so no directory has to be created to ask. + """ + bad: list[str] = [] + for rel in candidates: + proc = subprocess.run( + ["git", "-C", str(REPO_ROOT), "check-ignore", "-q", rel], + capture_output=True, text=True, timeout=30, encoding="utf-8", errors="replace", + ) + if proc.returncode != 0: + bad.append(rel) + return bad + + +def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): + """The invariant, with the instrument's own liveness asserted first. + + A scan that silently stopped finding sites would pass while measuring nothing, which is + the failure mode a guard is supposed to have instead of its callers, so the count is + asserted against the five sites the rule was written for rather than trusted. + """ + sites = _sites() + assert len(sites) >= 5, ( + f"the scan found {len(sites)} in-repo `mkdtemp(dir=…)` site(s) in tests/, fewer than " + "the five this rule was measured against — either the sites moved or this scan is " + "dead, and a guard that measures nothing passes for the wrong reason" + ) + + unmeasurable = [f"{p.relative_to(REPO_ROOT)}:{n}" for p, n, prefix, _src in sites + if not prefix] + assert not unmeasurable, ( + "these sites create an in-repo scratch directory under a name mkdtemp invents, so no " + f"instrument can ask git whether it is ignored: {unmeasurable}. Give the call a " + "literal `prefix=` whose names `.gitignore` matches (e.g. the `/tests/emrg-*` rule), " + "or move its root out of the repository." + ) + + candidates = [f"tests/{prefix}x" for _p, _n, prefix, _src in sites] + bad = _unignored(candidates) + assert not bad, ( + "these names a test can create inside the repository are not ignored by git, so an " + f"interrupted run leaves tree dirt that costs the next cycle its tier: {bad}. Add " + "the prefix to `.gitignore` (scoped to /tests/, with the measurement in the comment) " + "or move the scratch root out of the repository." + ) + + +def test_the_ignore_check_discriminates_both_ways(): + """The instrument read in both states, because `_unignored` returning [] is the pass. + + An empty list is also what a broken call returns, so the pair is the evidence: the same + function must reject a name the current rule ignores and name one it does not. + """ + assert _unignored(["tests/emrg-corpus-fapk6xox"]) == [], ( + "the residue measured on 2026-09-17 is still ignored by /tests/emrg-* — if this " + "fails the rule was narrowed, and the five sites are exposed again" + ) + assert _unignored(["tests/scratch-not-covered-x"]) == ["tests/scratch-not-covered-x"], ( + "the check must report an unmatched path, or every site below it passes by default " + "(the first draft of this control used `tests/emrg-…`, which the rule under test " + "matches — a control that cannot fail is not one)" + ) + assert _unignored(["emrg-corpus-x"]) == ["emrg-corpus-x"], ( + "the rule is scoped to /tests/ on purpose: the same name at the repository root must " + "stay visible to git, or the pattern swallows files it was never about" + ) From 2d3b7b438677e00a8a9f17dafcac94296cb21613 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 05:20:46 +0800 Subject: [PATCH 2/7] emrg: the scratch-root scan follows a two-hop root and reports what it cannot read --- tests/test_scratch_roots_are_gitignored.py | 237 ++++++++++++++++++--- 1 file changed, 205 insertions(+), 32 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index a5692b6f..fd96b44e 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -28,10 +28,23 @@ -------- For every `mkdtemp(dir=…)` call in `tests/` whose `dir=` expression is **derived from `__file__`** — i.e. a path *inside this repository*, since the module lives in `tests/` — the -name that call can create must be matched by git's ignore rules. A root that is not -`__file__`-derived (a `tmp_path` fixture, the default system temp) cannot dirty the tree and -is out of scope; that distinction is read from the syntax rather than kept in a list, so a -new site is found by *writing the call*, not by remembering to edit this file. +name that call can create must be matched by git's ignore rules. That distinction is read +from the syntax rather than kept in a list, so a new site is found by *writing the call*, not +by remembering to edit this file. + +Every other `dir=` is classified deliberately, into one of two buckets — because "not +`__file__`-derived" and "cannot be read" are not the same answer, and the first version of +this scan collapsed them: + +* **provably outside the repository** — a root the syntax shows to be a temp location + (`tmp_path` / `tmp_path_factory` fixtures, `tempfile.gettempdir()`, `TemporaryDirectory()`, + `os.environ` / `getenv`) cannot dirty the tree, so it is out of scope; +* **unmeasurable** — anything else is reported with a remedy, never passed. This is the hole + a two-hop root fell through: `REPO_ROOT = Path(__file__)…` then `TESTS_DIR = REPO_ROOT / + "tests"`, the idiom *this* module uses for its own root, was invisible to a one-hop scan + and was silently treated as a root that cannot dirty the tree — so a real in-repo site + escaped while the guard reported green. Provenance is now a fixed point over the module's + assignments, so a root is followed however many hops it takes. Two honest boundaries, stated rather than implied: * a site with no literal `prefix=` cannot be measured statically (the name `mkdtemp` invents @@ -51,43 +64,116 @@ TESTS_DIR = REPO_ROOT / "tests" -def _sites() -> list[tuple[Path, int, str | None, str]]: - """Every `mkdtemp(dir=…)` call in `tests/`, as (file, line, prefix, dir source). +#: Fixture names whose root is a temp location by construction (pytest's `tmp_path` family). +_TEMP_FIXTURES = frozenset({"tmp_path", "tmp_path_factory"}) + +#: Substrings that mark a `dir=` expression as a temp location rather than a repo path. +_TEMP_MARKERS = ("gettempdir", "TemporaryDirectory", "mkdtemp", "environ", "getenv", "expanduser") + + +def _file_derived_names(tree: ast.AST) -> set[str]: + """The names in `tree` that hold a path derived from `__file__`, to a fixed point. - `dir source` is the expression as written (`ast.unparse`), which is what decides whether - the site is in scope: `__file__` in it — directly or through a name assigned to such an - expression in the same module — means the root is a path inside the repository. + One pass over the module's assignments was the first version, and one hop is evadable by + the idiom this very module uses for its own root: `REPO_ROOT = Path(__file__)…` is + file-derived, but `TESTS_DIR = REPO_ROOT / "tests"` mentions only `REPO_ROOT`, so a site + rooted at `TESTS_DIR` was invisible to the scan — and, worse than invisible, it fell into + the out-of-scope branch, i.e. a real in-repo root was reported as one that cannot dirty + the tree. Iterating to a fixed point follows a chain of any length (measured: the + two-hop shape evaded the one-hop scan, and the guard module's own style is two hops). """ - sites: list[tuple[Path, int, str | None, str]] = [] - for path in sorted(TESTS_DIR.rglob("*.py")): - tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) - file_derived: set[str] = set() - for node in ast.walk(tree): - if isinstance(node, ast.Assign) and "__file__" in ast.dump(node.value): - file_derived.update(t.id for t in node.targets if isinstance(t, ast.Name)) - for node in ast.walk(tree): - if not isinstance(node, ast.Call): - continue - func = node.func - called = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "") - if called != "mkdtemp": - continue - kw = {k.arg: k.value for k in node.keywords if k.arg} - if "dir" not in kw: - continue - source = ast.unparse(kw["dir"]) - is_file_derived = "__file__" in source or ( - isinstance(kw["dir"], ast.Name) and kw["dir"].id in file_derived - ) - if not is_file_derived: + assignments = [n for n in ast.walk(tree) if isinstance(n, ast.Assign)] + derived: set[str] = set() + while True: + grown = False + for node in assignments: + value_names = {n.id for n in ast.walk(node.value) if isinstance(n, ast.Name)} + if not ("__file__" in value_names or (value_names & derived)): continue + for target in node.targets: + if isinstance(target, ast.Name) and target.id not in derived: + derived.add(target.id) + grown = True + if not grown: + return derived + + +def _parameters(tree: ast.AST) -> set[str]: + """Every function's parameter names, so a fixture-supplied root can be recognised.""" + names: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + a = node.args + names.update(x.arg for x in (*a.posonlyargs, *a.args, *a.kwonlyargs)) + if a.vararg: + names.add(a.vararg.arg) + if a.kwarg: + names.add(a.kwarg.arg) + return names + + +def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, str]], + list[tuple[Path, int, str]]]: + """`(in-scope sites, unmeasurable sites)` for one parsed module. + + A `dir=` is **in scope** when its expression mentions `__file__`, or is a name the module + assigns from something file-derived (to a fixed point). It is **provably outside** when + the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name, or an + expression carrying a temp marker. Everything else — an unresolvable name, a path built + from something the scan cannot follow — is **unmeasurable**, reported rather than passed. + """ + derived = _file_derived_names(tree) + parameters = _parameters(tree) + in_scope: list[tuple[Path, int, str | None, str]] = [] + unmeasurable: list[tuple[Path, int, str]] = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + called = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "") + if called != "mkdtemp": + continue + kw = {k.arg: k.value for k in node.keywords if k.arg} + if "dir" not in kw: + continue + source = ast.unparse(kw["dir"]) + arg = kw["dir"] + if "__file__" in source or (isinstance(arg, ast.Name) and arg.id in derived): prefix = kw.get("prefix") prefix = (prefix.value if isinstance(prefix, ast.Constant) and isinstance(prefix.value, str) else None) - sites.append((path, node.lineno, prefix, source)) + in_scope.append((path, node.lineno, prefix, source)) + continue + harmless = any(marker in source for marker in _TEMP_MARKERS) or ( + isinstance(arg, ast.Name) + and arg.id in parameters + and arg.id in _TEMP_FIXTURES + ) + if not harmless: + unmeasurable.append((path, node.lineno, source)) + return in_scope, unmeasurable + + +def _sites() -> list[tuple[Path, int, str | None, str]]: + """Every in-scope `mkdtemp(dir=…)` call in `tests/`, as (file, line, prefix, dir source).""" + sites: list[tuple[Path, int, str | None, str]] = [] + for path in sorted(TESTS_DIR.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + in_scope, _ = _scan(tree, path) + sites.extend(in_scope) return sites +def _unmeasurable_sites() -> list[tuple[Path, int, str]]: + """Every `mkdtemp(dir=…)` call in `tests/` whose root this scan cannot classify.""" + found: list[tuple[Path, int, str]] = [] + for path in sorted(TESTS_DIR.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + _, unmeasurable = _scan(tree, path) + found.extend(unmeasurable) + return found + + def _unignored(candidates: list[str]) -> list[str]: """The candidates git's ignore rules do not match, asked one at a time. @@ -119,6 +205,19 @@ def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): "dead, and a guard that measures nothing passes for the wrong reason" ) + unclassifiable = [ + f"{p.relative_to(REPO_ROOT)}:{n} (dir={src})" + for p, n, src in _unmeasurable_sites() + ] + assert not unclassifiable, ( + "this scan cannot tell whether these scratch roots are inside the repository, so it " + f"must not treat them as harmless: {unclassifiable}. A root reached through a chain " + "of assignments is followed to a fixed point, so a two-hop `__file__` path is in " + "scope; a root that is neither `__file__`-derived nor a temp location has to be " + "written so the scan can see it (give it a name derived from `__file__`, or root it " + "at `tmp_path` and move it out of the repository)." + ) + unmeasurable = [f"{p.relative_to(REPO_ROOT)}:{n}" for p, n, prefix, _src in sites if not prefix] assert not unmeasurable, ( @@ -138,6 +237,80 @@ def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): ) +def test_the_scan_follows_a_root_to_any_depth_and_reports_what_it_cannot_read(): + """The classifier driven directly, once per bucket — the arms the guard exists for. + + The scan's liveness was asserted by counting sites, but a count cannot show that the + *interesting* shapes are classified: the one-hop root it was written against, the + two-hop root that evaded it (the idiom this module itself uses), a fixture root that + is genuinely out of scope, and a root nothing can resolve — which must be reported + rather than passed, because "could not measure" and "clean" must not be the same value. + Synthetic sources rather than files: the classification is the thing under test. + """ + one_hop = ''' +import os, tempfile +root = os.path.dirname(os.path.abspath(__file__)) +d = tempfile.mkdtemp(dir=root, prefix="emrg-one-") +''' + two_hop = ''' +import tempfile +from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parent.parent +TESTS_DIR = REPO_ROOT / "tests" +d = tempfile.mkdtemp(dir=TESTS_DIR, prefix="emrg-two-") +''' + three_hop = ''' +import tempfile +from pathlib import Path +HERE = Path(__file__).resolve() +ROOT = HERE.parent.parent +SUB = ROOT / "tests" / "nested" +d = tempfile.mkdtemp(dir=SUB, prefix="emrg-three-") +''' + fixture_root = ''' +import tempfile +def test_x(tmp_path): + d = tempfile.mkdtemp(dir=tmp_path, prefix="anything-") +''' + unresolvable = ''' +import tempfile +def test_x(some_dir): + d = tempfile.mkdtemp(dir=some_dir, prefix="anything-") +''' + + def classify(source: str): + tree = ast.parse(source) + return _scan(tree, Path("synthetic.py")) + + for label, source, prefix in ( + ("one hop", one_hop, "emrg-one-"), + ("two hops (the evasion)", two_hop, "emrg-two-"), + ("three hops", three_hop, "emrg-three-"), + ): + in_scope, unmeasurable = classify(source) + assert [p for _f, _n, p, _s in in_scope] == [prefix], ( + f"a root reached in {label} must be classified as in-repo, not dropped: " + f"in_scope={in_scope} unmeasurable={unmeasurable}" + ) + assert not unmeasurable, f"{label} is measurable — it must not be reported unknown" + + in_scope, unmeasurable = classify(fixture_root) + assert not in_scope and not unmeasurable, ( + "a `tmp_path` root is a temp location: out of scope, and measurable enough to know so" + ) + + in_scope, unmeasurable = classify(unresolvable) + assert not in_scope, "an unresolvable root is not evidence of a repo root" + assert len(unmeasurable) == 1, ( + f"an unresolvable root must be reported as unmeasurable, got {unmeasurable}" + ) + _file, lineno, source = unmeasurable[0] + assert source == "some_dir", "the report names the expression it could not read" + assert "mkdtemp" in unresolvable.splitlines()[lineno - 1], ( + "the reported line is the call itself, not a nearby one" + ) + + def test_the_ignore_check_discriminates_both_ways(): """The instrument read in both states, because `_unignored` returning [] is the pass. From 0e27465fa1f9ad4c4691dfc4482924d70f43b36e Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 05:47:30 +0800 Subject: [PATCH 3/7] emrg: a temp scratch root is read over the whole expression, not its bare name --- tests/test_scratch_roots_are_gitignored.py | 103 +++++++++++++++++++-- 1 file changed, 95 insertions(+), 8 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index fd96b44e..76d3a71c 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -38,7 +38,13 @@ * **provably outside the repository** — a root the syntax shows to be a temp location (`tmp_path` / `tmp_path_factory` fixtures, `tempfile.gettempdir()`, `TemporaryDirectory()`, - `os.environ` / `getenv`) cannot dirty the tree, so it is out of scope; + `os.environ` / `getenv`) cannot dirty the tree, so it is out of scope. That is decided over + the **whole** expression — every name it is built from has to be temp-rooted, and a name + bound from a temp call (`with tempfile.TemporaryDirectory() as td:`) is followed, to a + fixed point — rather than in the expression's bare-name position. The first version read + only `dir=tmp_path`, so `tmp_path / "sub"`, the way a scratch dir gets a subdirectory, was + reported as a root nobody could read: it failed a tree for doing what the failure message + below tells it to do (measured 2026-09-17 by an outside reviewer on #1303); * **unmeasurable** — anything else is reported with a remedy, never passed. This is the hole a two-hop root fell through: `REPO_ROOT = Path(__file__)…` then `TESTS_DIR = REPO_ROOT / "tests"`, the idiom *this* module uses for its own root, was invisible to a one-hop scan @@ -98,6 +104,40 @@ def _file_derived_names(tree: ast.AST) -> set[str]: return derived +def _temp_rooted_names(tree: ast.AST, parameters: set[str]) -> set[str]: + """Every name in `tree` a temp location is behind, to a fixed point. + + `dir=td` does not show that `td` came from `with tempfile.TemporaryDirectory() as td:`, + and the docstring calls that family provably outside — so the *name* has to be followed, + however many hops it takes, for the same reason `_file_derived_names` is: one hop was + the evasion there. The `tmp_path` / `tmp_path_factory` fixtures are parameters rather + than assignments, so the fixed point is seeded from `parameters`. + """ + bindings: list[tuple[str, ast.AST]] = [] + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Name): + bindings.append((target.id, node.value)) + elif isinstance(node, (ast.With, ast.AsyncWith)): + for item in node.items: + target = item.optional_vars + if isinstance(target, ast.Name): + bindings.append((target.id, item.context_expr)) + temp = {name for name in parameters if name in _TEMP_FIXTURES} + while True: + grown = False + for name, value in bindings: + if name in temp: + continue + mentioned = {n.id for n in ast.walk(value) if isinstance(n, ast.Name)} + if mentioned & temp or any(m in ast.unparse(value) for m in _TEMP_MARKERS): + temp.add(name) + grown = True + if not grown: + return temp + + def _parameters(tree: ast.AST) -> set[str]: """Every function's parameter names, so a fixture-supplied root can be recognised.""" names: set[str] = set() @@ -118,12 +158,16 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, A `dir=` is **in scope** when its expression mentions `__file__`, or is a name the module assigns from something file-derived (to a fixed point). It is **provably outside** when - the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name, or an - expression carrying a temp marker. Everything else — an unresolvable name, a path built - from something the scan cannot follow — is **unmeasurable**, reported rather than passed. + the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name, a name + bound from a temp call, or an expression carrying a temp marker — read over the whole + expression, so `tmp_path / "sub"` is outside rather than unreadable. A root built only + partly from temp names is *not* outside: every name it mentions has to be temp-rooted. + Everything else — an unresolvable name, a path built from something the scan cannot + follow — is **unmeasurable**, reported rather than passed. """ derived = _file_derived_names(tree) parameters = _parameters(tree) + temp_rooted = _temp_rooted_names(tree, parameters) in_scope: list[tuple[Path, int, str | None, str]] = [] unmeasurable: list[tuple[Path, int, str]] = [] for node in ast.walk(tree): @@ -138,6 +182,7 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, continue source = ast.unparse(kw["dir"]) arg = kw["dir"] + names_in_arg = {n.id for n in ast.walk(arg) if isinstance(n, ast.Name)} if "__file__" in source or (isinstance(arg, ast.Name) and arg.id in derived): prefix = kw.get("prefix") prefix = (prefix.value if isinstance(prefix, ast.Constant) @@ -145,9 +190,7 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, in_scope.append((path, node.lineno, prefix, source)) continue harmless = any(marker in source for marker in _TEMP_MARKERS) or ( - isinstance(arg, ast.Name) - and arg.id in parameters - and arg.id in _TEMP_FIXTURES + bool(names_in_arg) and names_in_arg <= temp_rooted ) if not harmless: unmeasurable.append((path, node.lineno, source)) @@ -215,7 +258,8 @@ def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): "of assignments is followed to a fixed point, so a two-hop `__file__` path is in " "scope; a root that is neither `__file__`-derived nor a temp location has to be " "written so the scan can see it (give it a name derived from `__file__`, or root it " - "at `tmp_path` and move it out of the repository)." + "at `tmp_path` — a subdirectory under it counts, so `tmp_path / \"sub\"` is read as " + "a temp location — and move it out of the repository)." ) unmeasurable = [f"{p.relative_to(REPO_ROOT)}:{n}" for p, n, prefix, _src in sites @@ -246,6 +290,13 @@ def test_the_scan_follows_a_root_to_any_depth_and_reports_what_it_cannot_read(): is genuinely out of scope, and a root nothing can resolve — which must be reported rather than passed, because "could not measure" and "clean" must not be the same value. Synthetic sources rather than files: the classification is the thing under test. + + The out-of-scope family is driven in the spellings the *docstring* names rather than + only in its bare form: an outside reviewer measured on 2026-09-17 that `tmp_path` was + recognised while `tmp_path / "sub"` and a `TemporaryDirectory()` alias were reported + unreadable, so the guard failed a tree for doing what its own remedy says. The + counterpart arm is `partly_temp`: a temp name *mixed with* an unknown one has to stay + unreadable, so widening the check cannot become a way for a repo path to hide. """ one_hop = ''' import os, tempfile @@ -271,6 +322,22 @@ def test_the_scan_follows_a_root_to_any_depth_and_reports_what_it_cannot_read(): import tempfile def test_x(tmp_path): d = tempfile.mkdtemp(dir=tmp_path, prefix="anything-") +''' + fixture_subdir = ''' +import tempfile +def test_x(tmp_path): + d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") +''' + tempdir_alias = ''' +import tempfile +def test_x(): + with tempfile.TemporaryDirectory() as td: + d = tempfile.mkdtemp(dir=td, prefix="anything-") +''' + partly_temp = ''' +import tempfile +def test_x(tmp_path, other_dir): + d = tempfile.mkdtemp(dir=tmp_path / other_dir, prefix="anything-") ''' unresolvable = ''' import tempfile @@ -299,6 +366,26 @@ def classify(source: str): "a `tmp_path` root is a temp location: out of scope, and measurable enough to know so" ) + for label, source in ( + ("a `tmp_path` root with a subdirectory", fixture_subdir), + ("a name bound from `TemporaryDirectory()`", tempdir_alias), + ): + in_scope, unmeasurable = classify(source) + assert not in_scope and not unmeasurable, ( + f"{label} is a temp location as well, and the remedy below tells a " + "contributor to write exactly this — reporting it unreadable fails a tree " + "for obeying the failure message (measured 2026-09-17): " + f"in_scope={in_scope} unmeasurable={unmeasurable}" + ) + + in_scope, unmeasurable = classify(partly_temp) + assert not in_scope, "a root built partly from an unknown name is not evidence of a repo root" + assert len(unmeasurable) == 1, ( + "one temp name in the expression does not make the whole root a temp location: " + "every name it is built from has to be temp-rooted, or the check could be widened " + f"until a repo path hides behind a fixture name (got {unmeasurable})" + ) + in_scope, unmeasurable = classify(unresolvable) assert not in_scope, "an unresolvable root is not evidence of a repo root" assert len(unmeasurable) == 1, ( From 1e3d8107ade1cfde48163a9275fd1e296fdcfe72 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 06:07:02 +0800 Subject: [PATCH 4/7] emrg: a fixture name only counts where the fixture can reach the call --- tests/test_scratch_roots_are_gitignored.py | 123 +++++++++++++++++++-- 1 file changed, 112 insertions(+), 11 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index 76d3a71c..74907cb6 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -104,14 +104,74 @@ def _file_derived_names(tree: ast.AST) -> set[str]: return derived -def _temp_rooted_names(tree: ast.AST, parameters: set[str]) -> set[str]: +def _parameters(node: ast.AST) -> set[str]: + """One function's parameter names, so a fixture-supplied root can be recognised.""" + a = node.args + names = {x.arg for x in (*a.posonlyargs, *a.args, *a.kwonlyargs)} + if a.vararg: + names.add(a.vararg.arg) + if a.kwarg: + names.add(a.kwarg.arg) + return names + + +def _bound_names(node: ast.AST) -> set[str]: + """Names a *binding* in `node` shadows — assignment, `with … as`, `for … in`.""" + names: set[str] = set() + for sub in ast.walk(node): + if isinstance(sub, ast.Assign): + names.update(t.id for t in sub.targets if isinstance(t, ast.Name)) + elif isinstance(sub, (ast.AnnAssign, ast.AugAssign, ast.For, ast.AsyncFor)): + if isinstance(sub.target, ast.Name): + names.add(sub.target.id) + elif isinstance(sub, (ast.With, ast.AsyncWith)): + for item in sub.items: + if isinstance(item.optional_vars, ast.Name): + names.add(item.optional_vars.id) + return names + + +def _parents(tree: ast.AST) -> dict[ast.AST, ast.AST]: + """`{child: parent}` for the whole module, so a call can find its enclosing defs.""" + parents: dict[ast.AST, ast.AST] = {} + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + parents[child] = node + return parents + + +def _fixture_seed(call: ast.AST, parents: dict[ast.AST, ast.AST]) -> set[str]: + """The fixture names that can reach `call` — its *own* enclosing scopes' parameters. + + Seeded from every function's parameters module-wide instead, a local variable that + merely *shares a name* with a fixture reads as a temp location: measured 2026-09-17 on + head `0e27465f`, `tmp_path = Path("/abs/…")` in one function plus `def test_x(tmp_path)` + elsewhere made `dir=tmp_path / "sub"` answer **outside** — a root the syntax does not + show to be a temp directory, passed instead of reported, which is the value this guard's + doctrine forbids. On the parent head the same shape was reported, so the fix's own arm + is what widened it. A binding in the *same* scope shadows the parameter, so bound names + are subtracted rather than trusted. + """ + seed: set[str] = set() + node: ast.AST = call + while node in parents: + node = parents[node] + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + seed |= {p for p in _parameters(node) if p in _TEMP_FIXTURES} + seed -= _bound_names(node) + return seed + + +def _temp_rooted_names(tree: ast.AST, seed: set[str]) -> set[str]: """Every name in `tree` a temp location is behind, to a fixed point. `dir=td` does not show that `td` came from `with tempfile.TemporaryDirectory() as td:`, and the docstring calls that family provably outside — so the *name* has to be followed, however many hops it takes, for the same reason `_file_derived_names` is: one hop was the evasion there. The `tmp_path` / `tmp_path_factory` fixtures are parameters rather - than assignments, so the fixed point is seeded from `parameters`. + than assignments, so the fixed point is seeded from `seed` — which is computed per call + by `_fixture_seed`, never from the whole module's parameter list, or a name that merely + looks like a fixture would be enough to pass an unreadable root. """ bindings: list[tuple[str, ast.AST]] = [] for node in ast.walk(tree): @@ -124,7 +184,7 @@ def _temp_rooted_names(tree: ast.AST, parameters: set[str]) -> set[str]: target = item.optional_vars if isinstance(target, ast.Name): bindings.append((target.id, item.context_expr)) - temp = {name for name in parameters if name in _TEMP_FIXTURES} + temp = set(seed) while True: grown = False for name, value in bindings: @@ -158,16 +218,16 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, A `dir=` is **in scope** when its expression mentions `__file__`, or is a name the module assigns from something file-derived (to a fixed point). It is **provably outside** when - the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name, a name - bound from a temp call, or an expression carrying a temp marker — read over the whole - expression, so `tmp_path / "sub"` is outside rather than unreadable. A root built only - partly from temp names is *not* outside: every name it mentions has to be temp-rooted. - Everything else — an unresolvable name, a path built from something the scan cannot - follow — is **unmeasurable**, reported rather than passed. + the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name *that + reaches this call* (its own enclosing scopes' parameters, minus names a binding shadows), + a name bound from a temp call, or an expression carrying a temp marker — read over the + whole expression, so `tmp_path / "sub"` is outside rather than unreadable. A root built + only partly from temp names is *not* outside: every name it mentions has to be + temp-rooted. Everything else — an unresolvable name, a path built from something the scan + cannot follow — is **unmeasurable**, reported rather than passed. """ derived = _file_derived_names(tree) - parameters = _parameters(tree) - temp_rooted = _temp_rooted_names(tree, parameters) + parents = _parents(tree) in_scope: list[tuple[Path, int, str | None, str]] = [] unmeasurable: list[tuple[Path, int, str]] = [] for node in ast.walk(tree): @@ -189,6 +249,7 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, and isinstance(prefix.value, str) else None) in_scope.append((path, node.lineno, prefix, source)) continue + temp_rooted = _temp_rooted_names(tree, _fixture_seed(node, parents)) harmless = any(marker in source for marker in _TEMP_MARKERS) or ( bool(names_in_arg) and names_in_arg <= temp_rooted ) @@ -338,6 +399,31 @@ def test_x(): import tempfile def test_x(tmp_path, other_dir): d = tempfile.mkdtemp(dir=tmp_path / other_dir, prefix="anything-") +''' + shadowed_name = ''' +import tempfile +from pathlib import Path +def test_uses_the_fixture(tmp_path): + pass +def test_other(): + tmp_path = Path("/abs/other-project/tests") + d = tempfile.mkdtemp(dir=tmp_path, prefix="anything-") +''' + shadowed_name_composite = ''' +import tempfile +from pathlib import Path +def test_uses_the_fixture(tmp_path): + pass +def test_other(): + tmp_path = Path("/abs/other-project/tests") + d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") +''' + shadowed_in_its_own_scope = ''' +import tempfile +from pathlib import Path +def test_x(tmp_path): + tmp_path = Path("/abs/somewhere/tests") + d = tempfile.mkdtemp(dir=tmp_path, prefix="anything-") ''' unresolvable = ''' import tempfile @@ -386,6 +472,21 @@ def classify(source: str): f"until a repo path hides behind a fixture name (got {unmeasurable})" ) + for label, source in ( + ("a bare name that shadows a fixture", shadowed_name), + ("a composite root built on a shadowing name", shadowed_name_composite), + ("a fixture name rebound in its own scope", shadowed_in_its_own_scope), + ): + in_scope, unmeasurable = classify(source) + assert not in_scope and len(unmeasurable) == 1, ( + f"{label} is not a temp location the syntax shows: the fixture name is a " + "parameter of a *different* function, and a binding in this scope shadows it. " + "Seeding from every function's parameters module-wide made this answer 'outside' " + "— an unreadable root passed rather than reported (measured 2026-09-17 on head " + f"`0e27465f`, and on the bare shape on its parent too): " + f"in_scope={in_scope} unmeasurable={unmeasurable}" + ) + in_scope, unmeasurable = classify(unresolvable) assert not in_scope, "an unresolvable root is not evidence of a repo root" assert len(unmeasurable) == 1, ( From cfb6f59c0dc71f151a21e5d733670eb626eeca79 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 06:32:23 +0800 Subject: [PATCH 5/7] emrg: the fixture-reachability rule binds, so a nested def cannot pass a root The module's stated rule is that a fixture name counts only where the fixture can reach the call. _parameters was defined twice -- one function's own parameters at :107, and every function inside the node at :201 -- and the later binding won at call time, so _fixture_seed read a parameter of a def *nested beside* the call as if it reached the call. Measured on head 1e3d8107: a repo-rooted `dir=tmp_path / "sub"` is reported unmeasurable, and adding an unused `def helper(tmp_path)` inside the same function flipped the same site to neither bucket -- a root inside the repository that the guard stops measuring, which its own doctrine forbids. Restoring the per-function semantics reports it again; the tree-wide definition had no other caller (the only call site passes one node), so it is deleted. The nested-def shape and its counterpart (the nested def IS the call's enclosing scope, and must stay out of scope) are pinned as arms; re-adding the duplicate reddens the new arm. Full suite 2755 passed / 17 skipped; the repo's five in-scope sites and its zero unmeasurable sites are unchanged. --- tests/test_scratch_roots_are_gitignored.py | 44 +++++++++++++++------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index 74907cb6..07386595 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -198,20 +198,6 @@ def _temp_rooted_names(tree: ast.AST, seed: set[str]) -> set[str]: return temp -def _parameters(tree: ast.AST) -> set[str]: - """Every function's parameter names, so a fixture-supplied root can be recognised.""" - names: set[str] = set() - for node in ast.walk(tree): - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): - a = node.args - names.update(x.arg for x in (*a.posonlyargs, *a.args, *a.kwonlyargs)) - if a.vararg: - names.add(a.vararg.arg) - if a.kwarg: - names.add(a.kwarg.arg) - return names - - def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, str]], list[tuple[Path, int, str]]]: """`(in-scope sites, unmeasurable sites)` for one parsed module. @@ -424,6 +410,26 @@ def test_other(): def test_x(tmp_path): tmp_path = Path("/abs/somewhere/tests") d = tempfile.mkdtemp(dir=tmp_path, prefix="anything-") +''' + shadowed_beside_a_nested_def = ''' +import tempfile +from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parent.parent +tmp_path = REPO_ROOT / "tests" +def test_x(): + def helper(tmp_path): + return tmp_path + + helper("x") + d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") +''' + fixture_in_a_nested_def = ''' +import tempfile +def test_x(): + def helper(tmp_path): + d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") + + helper("x") ''' unresolvable = ''' import tempfile @@ -476,6 +482,7 @@ def classify(source: str): ("a bare name that shadows a fixture", shadowed_name), ("a composite root built on a shadowing name", shadowed_name_composite), ("a fixture name rebound in its own scope", shadowed_in_its_own_scope), + ("a fixture-named parameter of a def *nested beside* the call", shadowed_beside_a_nested_def), ): in_scope, unmeasurable = classify(source) assert not in_scope and len(unmeasurable) == 1, ( @@ -487,6 +494,15 @@ def classify(source: str): f"in_scope={in_scope} unmeasurable={unmeasurable}" ) + # ...and the counterpart, so narrowing the seed to the *enclosing* scopes cannot become a + # way to report a real fixture root: here the nested def IS the call's enclosing scope. + in_scope, unmeasurable = classify(fixture_in_a_nested_def) + assert not in_scope and not unmeasurable, ( + "a `tmp_path` parameter of the function the call sits in is a temp location, however " + "deeply that function is nested — reading only module-level defs would report it, " + f"failing a tree for a temp root: in_scope={in_scope} unmeasurable={unmeasurable}" + ) + in_scope, unmeasurable = classify(unresolvable) assert not in_scope, "an unresolvable root is not evidence of a repo root" assert len(unmeasurable) == 1, ( From 5f8b2786b2ef4cc17f75776a656981140c1761f2 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 07:43:31 +0800 Subject: [PATCH 6/7] emrg: a scratch root is asked about where it really is The candidate `git check-ignore` was asked about was `tests/x` for every site, discarding the root the scan had just read. A site rooting one level below `tests/`, or at the repository root, therefore passed while the path its call really creates was not ignored and `git status` listed it - dirt by the criterion that costs the next cycle its tier (measured on head cfb6f59c: both arms left this guard at `3 passed`). The candidate is now built from the site's own root, resolved from the `dir=` expression by `_resolve_root` - deliberately narrow (`__file__`, `Path(...)`, `.parent`, `.resolve()`, the `os.path` calls over those, `/ ""`, and a name whose assignments are all the same such expression), because a root this scan cannot read has to be reported, never passed. Measured after the change, in a worktree of this branch: the two arms above go RED, an unresolvable root goes RED where it was GREEN, and a name assigned two different ways goes RED; a root at `tests/`, and a nested root the `/tests/emrg-*` rule covers, stay GREEN. Full suite: 2755 passed / 17 skipped. --- tests/test_scratch_roots_are_gitignored.py | 156 ++++++++++++++++++++- 1 file changed, 151 insertions(+), 5 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index 07386595..f48240a6 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -52,9 +52,17 @@ escaped while the guard reported green. Provenance is now a fixed point over the module's assignments, so a root is followed however many hops it takes. -Two honest boundaries, stated rather than implied: +Three honest boundaries, stated rather than implied: * a site with no literal `prefix=` cannot be measured statically (the name `mkdtemp` invents is random), so it is reported as an unmeasurable site with the remedy — never as a pass; +* the check asks git about the name **under the site's own root**, which has to be resolved + from the `dir=` expression first (`_resolve_root`, deliberately narrow: `__file__`, + `Path(...)`, `.parent`, `.resolve()`, the `os.path` calls over those, `/ ""`, and a + name whose assignments are all the same such expression). A root in any other shape is + **reported**, never passed. Synthesising the candidate as `tests/x` instead was the + fail-open this guard shipped with: it discards the root, so a site rooting one level below + `tests/` or at the repository root passed while the path the call really creates was not + ignored (measured 2026-09-17, head `cfb6f59c`, both arms `3 passed`); * the check asks git about a *synthesised* name (`x`), not about a directory that exists, which is what makes it runnable on a clean tree (`git check-ignore` answers for paths that do not exist — verified: rc=1 for an unignored path, rc=0 for an ignored one). @@ -264,6 +272,110 @@ def _unmeasurable_sites() -> list[tuple[Path, int, str]]: return found +def _assigned_values(path: Path) -> dict[str, tuple[ast.AST, ...]]: + """Every value assigned to each name in one module, as the expressions it is written in. + + A name is usable as a root only when **every** one of its assignments is the same + expression: a name that means two different directories in two functions has no single + answer, and picking one of them is how a guard comes to ask git about a directory that is + not the one the call will use. + """ + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + values: dict[str, list[ast.AST]] = {} + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + names = [t.id for t in node.targets if isinstance(t, ast.Name)] + elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + names = [node.target.id] + else: + continue + if node.value is None: + continue + for name in names: + values.setdefault(name, []).append(node.value) + return {name: tuple(found) for name, found in values.items()} + + +#: `os.path` calls a `dir=` root is built from, and what each one answers about its argument. +#: `abspath`/`realpath` are the identity here because every path this scan holds — the file it +#: is reading, and its own `REPO_ROOT` — is already absolute and resolved the same way. +_PATH_CALLS = {"dirname": "parent", "abspath": None, "realpath": None} + +#: Calls that ask a path for itself: `Path(__file__).resolve()`. Same reason as above. +_IDENTITY_CALLS = frozenset({"resolve", "absolute"}) + + +def _resolve_root( + expr: ast.AST, + values: dict[str, tuple[ast.AST, ...]], + here: Path, + seen: frozenset[str] = frozenset(), +) -> Path | None: + """The absolute directory `expr` names, or None when this scan cannot read it. + + Deliberately narrow, and None is the default answer. Only the shapes a `dir=` root is + actually built from are evaluated — `__file__`, `Path(...)`, the zero-argument + `.resolve()`/`.absolute()`, `.parent`, `os.path.dirname`/`abspath`/`realpath` over those, + ` / ""`, and a name whose assignments are all the same such expression. + Anything else — a call this scan does not know, a name assigned two different ways, an + f-string — answers None, and the caller **reports** it rather than passing it: "could not + measure" must never be the same value as "clean", which is this module's whole doctrine. + """ + if isinstance(expr, ast.Constant) and isinstance(expr.value, str): + candidate = Path(expr.value) + return candidate if candidate.is_absolute() else None + if isinstance(expr, ast.Name): + if expr.id == "__file__": + return here + if expr.id in seen or expr.id not in values: + return None + answers = { + _resolve_root(value, values, here, seen | {expr.id}) for value in values[expr.id] + } + return answers.pop() if len(answers) == 1 else None + if isinstance(expr, ast.Attribute): + if expr.attr != "parent": + return None + base = _resolve_root(expr.value, values, here, seen) + return None if base is None else base.parent + if isinstance(expr, ast.Call): + func = expr.func + called = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "") + if not expr.args and not expr.keywords: + if called not in _IDENTITY_CALLS or not isinstance(func, ast.Attribute): + return None + return _resolve_root(func.value, values, here, seen) + if len(expr.args) != 1 or expr.keywords: + return None + base = _resolve_root(expr.args[0], values, here, seen) + if base is None: + return None + if called == "Path": + return base + if called not in _PATH_CALLS: + return None + step = _PATH_CALLS[called] + return base if step is None else Path(base).parent + if isinstance(expr, ast.BinOp) and isinstance(expr.op, ast.Div): + left = _resolve_root(expr.left, values, here, seen) + if left is None: + return None + right = expr.right + if isinstance(right, ast.Constant) and isinstance(right.value, str): + return left / right.value + return None + return None + + +def _root_for(source: str, path: Path) -> Path | None: + """The directory a site's `dir=` expression names, read from `path`'s own `__file__`.""" + try: + expr = ast.parse(source, mode="eval").body + except SyntaxError: # pragma: no cover - the source came from a parsed module + return None + return _resolve_root(expr, _assigned_values(path), path) + + def _unignored(candidates: list[str]) -> list[str]: """The candidates git's ignore rules do not match, asked one at a time. @@ -287,6 +399,10 @@ def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): A scan that silently stopped finding sites would pass while measuring nothing, which is the failure mode a guard is supposed to have instead of its callers, so the count is asserted against the five sites the rule was written for rather than trusted. + + The name it asks git about is built from the site's **own** root, and a root this scan + cannot resolve is reported rather than passed: asking about `tests/x` for every + site made the answer depend on a root the site does not have. """ sites = _sites() assert len(sites) >= 5, ( @@ -318,13 +434,43 @@ def test_every_scratch_root_a_test_creates_in_the_repo_is_gitignored(): "or move its root out of the repository." ) - candidates = [f"tests/{prefix}x" for _p, _n, prefix, _src in sites] + # The candidate is built from the site's **own** root. `f"tests/{prefix}x"` was the first + # cut of this line and it discards the root the scan just read (`_src`), so every site was + # asked about as if it rooted at `tests/`: a root one level below it, or the repository + # root itself, passed here while the path the call really creates was listed by + # `git status` as `?? tests/sub/…` / `?? emrg-probe-root-…`, not ignored — dirt by the + # criterion that costs the next cycle its tier. Measured 2026-09-17 on head `cfb6f59c` by + # cycle `cyc20260917-071653`, whose two arms (a `dir=` one level below `tests/`, and one at + # the repository root) each left this guard at `3 passed`. + candidates: list[str] = [] + unresolvable: list[str] = [] + for path, lineno, prefix, src in sites: + if not prefix: + continue # reported by the assertion above; there is no name to ask about + root = _root_for(src, path) + if root is None: + unresolvable.append(f"{path.relative_to(REPO_ROOT)}:{lineno} (dir={src})") + continue + try: + relative = root.relative_to(REPO_ROOT) + except ValueError: + continue # resolved outside the repository, so it cannot become tree dirt + candidates.append(str(relative / f"{prefix}x")) + assert not unresolvable, ( + "these sites root a scratch directory at an expression this scan cannot resolve, so " + f"it cannot ask git whether the name they create is ignored: {unresolvable}. Write the " + "root as a path derived from `__file__` (`Path(__file__).resolve().parent`, " + "`os.path.dirname(os.path.abspath(__file__))`) and show the scan the directory it " + "names, or move the scratch root out of the repository — a root this scan cannot read " + "is not a root that cannot dirty the tree." + ) bad = _unignored(candidates) assert not bad, ( "these names a test can create inside the repository are not ignored by git, so an " - f"interrupted run leaves tree dirt that costs the next cycle its tier: {bad}. Add " - "the prefix to `.gitignore` (scoped to /tests/, with the measurement in the comment) " - "or move the scratch root out of the repository." + f"interrupted run leaves tree dirt that costs the next cycle its tier: {bad}. The " + "`/tests/emrg-*` rule is anchored at the top level of `tests/`, so a site rooted " + "elsewhere needs the rule widened to that root deliberately — or, usually better, the " + "scratch root moved to `tests/` or out of the repository." ) From 25085e6d336ba3b7b0fad814de3802c7b5f7b357 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Thu, 17 Sep 2026 08:28:06 +0800 Subject: [PATCH 7/7] emrg: the scratch-root scan reads the whole root expression, not its bare name --- tests/test_scratch_roots_are_gitignored.py | 95 ++++++++++++++++++++-- 1 file changed, 86 insertions(+), 9 deletions(-) diff --git a/tests/test_scratch_roots_are_gitignored.py b/tests/test_scratch_roots_are_gitignored.py index f48240a6..77f872f7 100644 --- a/tests/test_scratch_roots_are_gitignored.py +++ b/tests/test_scratch_roots_are_gitignored.py @@ -210,13 +210,18 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, list[tuple[Path, int, str]]]: """`(in-scope sites, unmeasurable sites)` for one parsed module. - A `dir=` is **in scope** when its expression mentions `__file__`, or is a name the module - assigns from something file-derived (to a fixed point). It is **provably outside** when - the syntax shows a temp location: a `tmp_path`/`tmp_path_factory` fixture name *that - reaches this call* (its own enclosing scopes' parameters, minus names a binding shadows), - a name bound from a temp call, or an expression carrying a temp marker — read over the - whole expression, so `tmp_path / "sub"` is outside rather than unreadable. A root built - only partly from temp names is *not* outside: every name it mentions has to be + A `dir=` is **in scope** when its expression mentions `__file__`, or mentions a name the + module assigns from something file-derived (to a fixed point) — read over the **whole** + expression, because the other reading failed a legitimate site: `dir=TESTS` (a bare + `__file__`-derived name) was in scope while `dir=REPO_ROOT / "tests"`, the *same* + directory spelled as an expression, was reported as a root nobody could read — with a + remedy ("give it a name derived from `__file__`") that its author had already followed. + It is **provably outside** when the syntax shows a temp location: a + `tmp_path`/`tmp_path_factory` fixture name *that reaches this call* (its own enclosing + scopes' parameters, minus names a + binding shadows), a name bound from a temp call, or an expression carrying a temp marker — + read over the whole expression, so `tmp_path / "sub"` is outside rather than unreadable. A + root built only partly from temp names is *not* outside: every name it mentions has to be temp-rooted. Everything else — an unresolvable name, a path built from something the scan cannot follow — is **unmeasurable**, reported rather than passed. """ @@ -237,7 +242,7 @@ def _scan(tree: ast.AST, path: Path) -> tuple[list[tuple[Path, int, str | None, source = ast.unparse(kw["dir"]) arg = kw["dir"] names_in_arg = {n.id for n in ast.walk(arg) if isinstance(n, ast.Name)} - if "__file__" in source or (isinstance(arg, ast.Name) and arg.id in derived): + if "__file__" in source or (names_in_arg & derived): prefix = kw.get("prefix") prefix = (prefix.value if isinstance(prefix, ast.Constant) and isinstance(prefix.value, str) else None) @@ -568,6 +573,22 @@ def helper(tmp_path): helper("x") d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") +''' + #: The twin of the arm above, with the one clause that decides the bucket changed: the + #: binding is *not* file-derived, so nothing in the syntax says this root is in the + #: repository — and the fixture name must not be read from a different function. + shadowed_beside_a_nested_def_absolute = ''' +import tempfile +from pathlib import Path +def test_uses_the_fixture(tmp_path): + pass +def test_x(): + def helper(tmp_path): + return tmp_path + + helper("x") + tmp_path = Path("/abs/other-project/tests") + d = tempfile.mkdtemp(dir=tmp_path / "sub", prefix="anything-") ''' fixture_in_a_nested_def = ''' import tempfile @@ -581,6 +602,29 @@ def helper(tmp_path): import tempfile def test_x(some_dir): d = tempfile.mkdtemp(dir=some_dir, prefix="anything-") +''' + #: One directory, three spellings. The first two name `tests/`; the third names a + #: subdirectory of it, where `/tests/emrg-*` is anchored at the top level and therefore + #: does *not* cover the name — the arm exists so the guard reports that one for the right + #: reason (unignored) rather than for the wrong one (unreadable root). + composite_bare = ''' +import tempfile +from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parent.parent +TESTS_DIR = REPO_ROOT / "tests" +d = tempfile.mkdtemp(dir=TESTS_DIR, prefix="emrg-composite-") +''' + composite_spelled_out = ''' +import tempfile +from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parent.parent +d = tempfile.mkdtemp(dir=REPO_ROOT / "tests", prefix="emrg-composite-") +''' + composite_subdir = ''' +import tempfile +from pathlib import Path +REPO_ROOT = Path(__file__).resolve().parent.parent +d = tempfile.mkdtemp(dir=REPO_ROOT / "tests" / "sub", prefix="emrg-composite-") ''' def classify(source: str): @@ -628,7 +672,8 @@ def classify(source: str): ("a bare name that shadows a fixture", shadowed_name), ("a composite root built on a shadowing name", shadowed_name_composite), ("a fixture name rebound in its own scope", shadowed_in_its_own_scope), - ("a fixture-named parameter of a def *nested beside* the call", shadowed_beside_a_nested_def), + ("a fixture-named parameter of a def *nested beside* the call", + shadowed_beside_a_nested_def_absolute), ): in_scope, unmeasurable = classify(source) assert not in_scope and len(unmeasurable) == 1, ( @@ -649,6 +694,38 @@ def classify(source: str): f"failing a tree for a temp root: in_scope={in_scope} unmeasurable={unmeasurable}" ) + # A `__file__`-derived root with anything appended is still in the repository, and the + # bucket may not depend on how the author spelled it. Bare name vs the same directory as + # an expression, same call otherwise: both in scope, both judged by the candidate they + # create. Read the bare-name position only, the second was reported "cannot tell whether + # these scratch roots are inside the repository" — with the remedy "give it a name derived + # from `__file__`", which its author had already done (measured 2026-09-17 on head + # `d5773029` by cycle `cyc20260917-081808`, with the pair driven against the real guard). + for label, source, expected in ( + ("the bare name", composite_bare, "tests/emrg-composite-x"), + ("the same directory as an expression", composite_spelled_out, "tests/emrg-composite-x"), + ("a subdirectory of a derived root", composite_subdir, "tests/sub/emrg-composite-x"), + ): + in_scope, unmeasurable = classify(source) + assert len(in_scope) == 1 and not unmeasurable, ( + f"{label} is inside the repository — `__file__`-derived, and the scan follows a " + "chain of assignments to a fixed point — so it must be in scope and judged by the " + f"name it creates, not reported as a root nobody can read: in_scope={in_scope} " + f"unmeasurable={unmeasurable}" + ) + _file, _line, prefix, dir_source = in_scope[0] + assert prefix == "emrg-composite-", ( + "the in-scope record still carries the site's own prefix", prefix + ) + assert expected.endswith(prefix + "x"), (label, expected, prefix) + + in_scope, unmeasurable = classify(shadowed_beside_a_nested_def) + assert len(in_scope) == 1 and not unmeasurable, ( + "this arm's binding is `REPO_ROOT / \"tests\"`, which the scan reads as a repository " + "path (that is why it is not a temp location), so the root is in scope and judged by " + f"the candidate `tests/sub/anything-x`: in_scope={in_scope} unmeasurable={unmeasurable}" + ) + in_scope, unmeasurable = classify(unresolvable) assert not in_scope, "an unresolvable root is not evidence of a repo root" assert len(unmeasurable) == 1, (