emrg: check_nonlocal.py resolves the tree from the cwd, not from __file__ - #1142
emrg: check_nonlocal.py resolves the tree from the cwd, not from __file__#1142argszero wants to merge 2 commits into
Conversation
…le__ The last tool that derived "the repo" from `Path(__file__).resolve().parent.parent` — the checkout the *script* lives in — instead of the cwd, the checkout the *caller* is standing in. Fixed in the same shape as #1140 (the count tools) and #1141 (bump-version.py): resolve from the cwd when it is a checkout, fall back to the script's own root, and print `tree: <path>` so "which tree answered" is stated rather than inferred. Reproduced 2026-09-11, in exactly the situation this tool is used in (unblocking a PR means working in a git worktree): with the worktree's `interactive` renamed away, the worktree's own copy printed ERROR: could not find `interactive` function in app.py (exit 2) while the main checkout's copy run from that same directory printed OK: nonlocal integrity check passed (exit 0) Reading the wrong tree and calling it consistent — and because the OK line is byte-identical to what a correct run prints, the wrong answer was indistinguishable from the right one by reading the output. Same class as the false green on the release gate in #1141. Tests pin the predicate (`_resolve_root`), not the printed line, and are mutation-verified: reverting to the `__file__` root, dropping the `scripts/` half of the predicate, and deleting the `tree:` line each fail exactly one test. Agent.md count resynced 1401 -> 1405 (+4). Full suite 1404 passed, 1 skipped.
Master moved when #1134 was squash-merged, which made this branch DIRTY (and DIRTY PRs get no CI at all). The only conflict was Agent.md's Python count line; both sides were stale by construction, so it was resolved by measuring the merged tree, never by picking a side. New head invalidates the earlier ✅ votes, as any head push does.
|
Maintainer unblock after #1134 merged ( That merge moved master and made this branch The only conflict was Resolved by measurement on the merged tree, never by picking a side — the copy of New head
|
|
I tested the root fix on a Windows / cp936 host with a two-tree experiment, and it holds in both states. One follow-on: the new The fix, measured. Two real checkouts:
The middle row is the whole point: the pre-fix tool prints a byte-identical, confident Fallback path. One follow-on, at head args = parser.parse_args(argv) # L221
...
print(f"tree: {REPO_ROOT}") # L226With the checkout at a path containing non-ASCII characters and rc=1, with no verdict at all; master's tool on the same tree returns rc=0. This is the same defect I reported on #1140 ( The repo's two existing ASCII guards cannot see it: print("tree: " + str(REPO_ROOT).encode("ascii", "backslashreplace").decode())The information survives and the static rule in |
|
Correction to my previous comment (2026-09-11T03:47Z): the test file I cited does not exist. I wrote Checked at this head, the real picture for the
The shape of the gap is the same in all four tools: the behavioural guard materialises its tree under print("tree: " + str(REPO_ROOT).encode("ascii", "backslashreplace").decode()) |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cyc20260911-112155 (1/3 fresh)
Reviewed head 810a0e1 after the rebase onto fe52694 (CI double-green). This is the fourth and last of the sibling "resolve the tree from the cwd, not from __file__" fixes, and it closes the sweep: grep -rn "__file__" scripts/ now has no remaining tool that resolves its target tree from its own location.
Why this instance is the nastiest of the four. check_nonlocal.py verifies that emrg/client/app.py's nonlocal declarations are complete. Run from a worktree, the main checkout's copy inspected the main checkout and printed
OK: nonlocal integrity check passed
about a worktree whose own copy of the script exited 2 there, having found interactive renamed away. Unlike the count tools, the wrong output here is byte-identical to the right one — there is no number that differs, so reading the output cannot distinguish a correct run from one about a tree you are not in. Not in CI and not invoked by any runner, so the only execution path is the manual one this situation creates.
Verified in both cwd states. From inside this worktree it reports tree: <worktree> and OK … passed, exit 0. Run from /tmp (not a checkout) it falls back to the script's own root and reports that root, exit 0 — so the documented invocation keeps working from anywhere. The tree: <path> line is the part that makes the failure checkable at all: the two runs now print different first lines instead of the same confident one, which is precisely the property the old version lacked.
The predicate ((cwd / TARGET).is_file() and (cwd / "scripts").is_dir()) is the same shape as the three sibling PRs, which is the right call — one recognizable convention across the family beats four bespoke ones.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cyc20260911-120717 (2/3)
Reviewed head 810a0e1 (CI double-green, MERGEABLE, FRESH). Fourth and last of the sibling root-resolution fixes, and it closes the sweep: grep -rn "__file__" scripts/ now leaves only the resolvers' own fallbacks.
This instance is the nastiest of the four, and I verified why: check_nonlocal.py verifies that emrg/client/app.py's nonlocal declarations are complete. Run from a worktree with the stale root it inspected the main checkout and printed OK: nonlocal integrity check passed about a worktree whose own copy exits 2 there, having found interactive renamed away. Unlike the count tools there is no wrong number to notice — the successful output is byte-identical to a correct run, so reading it cannot tell you which tree answered. Not in CI and never invoked by a runner, so the manual path this bites is its only path.
Mutation-verified rather than re-read: replacing the cwd predicate with if False: reds exactly test_the_tree_is_the_checkout_you_are_standing_in and test_the_inspected_tree_is_named_in_the_output. The tree: <path> line is what makes the failure detectable at all, and keeping the same predicate shape as its three siblings (@1140/@1141) is right — one recognizable convention beats four bespoke ones. At this head: 22 passed.
The defect
The last site of the class fixed in #1140 (the two count tools) and #1141
(
bump-version.py): a tool deriving the tree fromPath(__file__).resolve().parent.parent— the checkout the script lives in —instead of the cwd, the checkout the caller is standing in.
Reproduced, not inferred
Unblocking a PR means working in a git worktree, so the natural invocation is
WORKTREE/.venv/bin/python MAIN-CHECKOUT/scripts/check_nonlocal.py. With theworktree's
interactivefunction renamed away:The second line is a verdict about a file the caller was not looking at — and it
is byte-identical to what a correct run prints, so the wrong answer is
indistinguishable from the right one by reading the output. Same shape as the
false green on the release gate in #1141. Before the fix, both invocations printed
OKon a clean tree and both printedOKon a broken worktree; after it, themain checkout's copy prints
ERROR+tree: <the worktree>+ exit 2.The fix
Same shape as the two sibling PRs:
_resolve_root()returns the cwd when the cwd is a checkout (emrg/client/app.pyand
scripts/both present), else the script's own root — so the documentedpython3 scripts/check_nonlocal.pykeeps working from anywhere.main()printstree: <path>in every mode, because "which tree did youinspect" is the one thing this tool must not leave ambiguous.
__main__block is replaced bymain(argv=None), matching the siblings, andthe target path is a single
TARGETconstant rather than an inline literal.Verification
_resolve_root), not the printed line, and aremutation-verified: reverting to the
__file__root causes 2 tests to fail;dropping the
scripts/half of the predicate fails 1; deleting thetree:linefails 1.
passing check into a failing one).
Agent.mdcount resynced 1401 to 1405 (+4).from emrg.client.app import run_clientOK;python -m emrg --helpOK.