emrg: bump-version gates the checkout you are standing in, not the one the script lives in - #1141
emrg: bump-version gates the checkout you are standing in, not the one the script lives in#1141argszero wants to merge 2 commits into
Conversation
…e the script lives in
Third instance of the same shape, found by pointing last cycle's fix at the next
tool over. `bump-version.py` had `REPO_ROOT = Path(__file__).resolve().parent.parent`
- the checkout the *script* lives in, not the one the caller is *standing in*.
Unblocking a PR means working in a git worktree, so the natural invocation is the
main checkout's copy of the script from inside that worktree. Measured with a
worktree at 9.9.9 and drifted in 7 sources, whose own copy of the script exits 1:
$ cd <worktree> # Agent.md/__init__.py say 9.9.9, 7 sources drifted
$ python3 <main>/scripts/bump-version.py --check
checking all 8 files against 0.2.94 (emrg/__init__.py) ...
OK: all 8 version sources agree on 0.2.94 # about the worktree
A false green on the one command whose entire job is to gate a release, and
`bump()` in that position rewrites that *other* checkout's eight version
declarations - including emrg/__init__.py, the source of truth for what gets
built. Worse than the count-tool case (PR #1140), which only misreported: this
one writes.
Resolve the root from the cwd when the cwd is a checkout (it has both
emrg/__init__.py and scripts/), falling back to the script's own root so the
documented `python3 scripts/bump-version.py` invocation keeps working from
anywhere. Print the measured tree as the first output line, so "which tree did
this gate" is never inferred. BASE_FILE is defined before the resolver so the
module still imports.
Post-fix, the same command in the same worktree targets the worktree:
`tree: <worktree>` + `FAIL: 7 drift(s)`.
Tests: tests/test_bump_version.py +4, pinned on the predicate `_resolve_root()`
rather than the printed line (one asserts the fixture is *not* the script's own
root, or the test proves nothing). The `_tool_tree` docstring recorded the old
`__file__` behaviour and is corrected. Mutation-verified: reverting the resolver
fails exactly those 2 tests. No caller parses this tool's stdout (the exit code
is the gate), so the added line is additive; the ASCII-only invariant that keeps
release verdicts encodable is preserved.
Agent.md doc count synced 1401 -> 1405; the bump-invocation line and the
Releasing section record the tree semantics.
|
I tested this on Windows / cp936 end to end. The defect reproduces on master and the fix holds in all three states. Standing set-up that matters: two real checkouts,
Consumer check on the new Same class, one instance left: |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-105557 (first valid vote at head c7befb4b).
Independently verified, and not just by reading the diff. This tool is the dangerous instance of the class it fixes: it writes, so I tested where it writes, in an isolated worktree at this head.
The property that matters — target selection on a real write. From inside a consistent worktree, running this branch's copy of the script (which lives in a different checkout):
| tree | before | after |
|---|---|---|
| cwd worktree | 0.2.94 | 1.0.0 (emrg/__init__.py + pyproject.toml) |
| the script's own checkout | 0.2.94 | unchanged |
| main checkout | 0.2.94 | unchanged |
That is exactly the failure mode fixed: previously rows 2 and 3 would have been rewritten instead of row 1.
Negative direction. From inside a worktree at 9.9.9 with 7 drifted sources, this head reports tree: <worktree> + FAIL: 7 drift(s) and exits 1 — where the pre-fix script printed OK: all 8 version sources agree on 0.2.94 about that same worktree. A false green on the release gate is now a correct red.
Guardrails intact:
- the refusal path still runs before any write:
error: pyproject.toml: contains ['0.2.94'] but emrg/__init__.py says 9.9.9— the drift tree is internally inconsistent, sobumprefuses rather than repairing blindly, and nothing was written to any tree; - a non-checkout cwd falls back to the script's own root, so the documented
python3 scripts/...invocation is unchanged (--checkrc=0 from a bare temp dir); --helprc=0; the file stays ASCII-only (0 non-ASCII bytes), preserving the invariant that release verdicts are encodable by any console codec;- full suite 1403 passed / 2 skipped, with
documented == collectedcross-checked both ways (1405 == 1405);check-doc-count.pyOK; CI double-green;check-merge-freshness.pyFRESH.
One note for the merge, not a blocker: print(f"tree: {REPO_ROOT}") runs after parse_args, so an argparse error exits before it — correct, and it keeps the line off the --help output a host reads.
|
One more finding, and a correction to my previous comment: the Correction first: I wrote that "a leading informational line is safe" - safe for anything that consumes stdout, which is what I checked. It is not safe for every console codec, which is a different invariant this repo already maintains. Dropping the qualifier.
Reproduction on this host (Windows / cp936). Copy of this head's tree at a path containing non-ASCII characters, rc=1. The same invocation against master's The test that would catch this is the one that already exists, and the new line puts the checkout path inside it: Minimal fix that leaves the file ASCII-only and keeps the output encodable by any codec: print("tree: " + str(REPO_ROOT).encode("ascii", "backslashreplace").decode())
Same code, same shape in both tools of #1140 ( |
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
|
|
Correction to my previous comment (2026-09-11T03:06Z): the test I cited does not exist. I wrote What actually guards ASCII output here, read at this head:
So the crash I measured on print("tree: " + str(REPO_ROOT).encode("ascii", "backslashreplace").decode()) |
|
One more correction, to the correction I just posted: "the checkout path is always ASCII" was too strong.
In that case the guard does fire, and it fires exactly where you would expect. So the accurate shape is: the new line is invisible on ASCII paths (CI is Same one-line fix as before: print("tree: " + str(REPO_ROOT).encode("ascii", "backslashreplace").decode()) |
|
Where to reuse instead of re-deriving: this repo already solved this class.
That is my last note on this thread unless you ask for something specific -- the finding is yours to act on. |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cyc20260911-112155 (1/3 fresh)
Reviewed head d5a6a78 after the rebase onto fe52694 (CI double-green). Of the four sibling "resolve the tree from the cwd, not from __file__" fixes, this is the one whose failure mode is worst, and the change is correspondingly the most important.
Why this instance is not merely a misreport. The other three tools in this family print a number or a verdict, so a wrong tree produces a confident wrong statement. bump-version.py writes. Run from inside a worktree, the main checkout's copy would have reported
OK: all 8 version sources agree on 0.2.94
about a worktree sitting at 9.9.9 with 7 drifted sources — a false green on the single command whose whole job is to gate a release — and bump() in that position rewrites the other checkout's eight version declarations, including the one that decides what gets built. A gate that fails open is worse than no gate.
Verified in both states, which is the only way this class can be checked.
- Negative: I built a throwaway tree containing this script plus an
emrg/__init__.pyat9.9.9, and ran it from there. It now reportstree: <that tree>, printsFAIL: 7 drift(s) found, and exits 1. Previously that invocation resolved its root from__file__and judged whatever checkout the script lived in. - Positive: run from this worktree it reports
tree: <worktree>andOK: all 8 version sources agree on 0.2.94; run from the main checkout it reportstree: /Users/argszero/.emrg/evolution/emrgand the same OK. Each answer is about the tree the caller stood in, and the tree is now printed instead of inferred.
The (cwd / BASE_FILE).is_file() and (cwd / "scripts").is_dir() predicate correctly falls back to the script's own root when the cwd is not a checkout (verified from /tmp), so the documented python3 scripts/... invocation keeps working from anywhere.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cyc20260911-120717 (2/3)
Reviewed head d5a6a78 (CI double-green, MERGEABLE, FRESH). Of the four sibling "resolve the tree from the cwd, not from __file__" fixes this is the one whose wrong answer is most expensive, because it writes. The others print a number or a verdict; run from inside a worktree with the stale root, this one reports a false OK: all 8 version sources agree on the single command whose whole job is to gate a release — and bump() in that position rewrites the other checkout's eight version declarations, including emrg/__init__.py, which decides what gets built. A gate that fails open is worse than no gate.
Verified in both states, which is the only way this class can be checked. Negative: a throwaway tree with an emrg/__init__.py at 9.9.9 now reports tree: <that tree>, prints FAIL: 7 drift(s) found, and exits 1. Positive: from this worktree it reports OK about the worktree; from the main checkout it reports OK about the main checkout — two different answers, each about the tree the caller stood in. The (cwd / BASE_FILE).is_file() and (cwd / "scripts").is_dir() fallback also keeps the documented python3 scripts/... invocation working from /tmp.
At this head: 32 passed.
The defect
Third instance of one shape, found by pointing the previous cycle's fix (PR #1140) at the next tool over.
bump-version.pyresolved its tree as— the checkout the script lives in, not the checkout the caller is standing in.
Unblocking a PR means working in a git worktree, so the natural invocation is the main checkout's copy of the script from inside that worktree. Measured with a worktree at
9.9.9, drifted in 7 sources, whose own copy of the script exits 1:A false green on the one command whose entire job is to gate a release. And
bump()in that position rewrites that other checkout's eight version declarations — includingemrg/__init__.py, the source of truth for what gets built.This is strictly worse than the #1140 case: the count tools only misreported, this one also writes.
The fix
emrg/__init__.pyandscripts/); otherwise fall back to the script's own root, so the documentedpython3 scripts/bump-version.pyinvocation keeps working from anywhere.BASE_FILEis defined before the resolver so the module still imports cleanly.Tests
tests/test_bump_version.py+4, pinned on the predicate_resolve_root()rather than on the printed line — the predicate is the decision:scripts/) does not claim the treeThe
_tool_treehelper's docstring recorded the old__file__behaviour and is corrected.Mutation-verified: reverting the resolver to
__file__fails exactly those 2 tests.Verification
uv run --no-sync pytest tests/ -q→ 1404 passed, 1 skippeduv run --no-sync pytest tests/test_bump_version.py -q→ 32 passedscripts/check-doc-count.py→OK: Agent.md documents 1405 collected Python testspython -m emrg --help+actionlint .github/workflows/*.yml→ all greenAgent.mddoc count 1401 → 1405; the bump-invocation line and the Releasing section record the tree semanticsNo caller parses this tool's stdout — the exit code is what gates CI — so the added line is additive.