emrg: the stop log is host state, and a test run no longer writes it - #1341
Conversation
`_open_stop_log()` resolved `~/.emrg/logs` with no seam, so the two tests that call `stop_all()` in-process (all five killers isolated, which is what the red line requires) each created a real `stop_all-YYYYMMDD-HHMMSS.log` there: 1877 had accumulated, carrying Windows-shaped fixture fakes on a macOS host (issue #1337). `_stop_log_dir()` is now the resolver — `$EMRG_STOP_LOG_DIR` when set, the host dir otherwise — and `tests/conftest.py` pins it for every test, so one pin contains every route (in-process calls in any file, and a child process that inherits the environment, e.g. `python -m emrg stop`). Tests that assert the default resolution clear the variable themselves. Measured: full suite 2884 passed / 16 skipped, with `~/.emrg/logs` staying at 1877 files; arm A (resolver ignores the override) 3 of the 4 new tests red; arm B (pin removed, HOME pinned to scratch) the guard test red and the leak reproduced under the pinned home.
…emp lives
The `windows-2025` leg killed the guard's first spelling. It asserted
`home not in resolved.parents`, which is a POSIX assumption: pytest's temp
base on Windows is `C:/Users/runneradmin/AppData/Local/Temp/...` — *inside*
the home directory — so the assertion failed on a correctly pinned suite
(`assert WindowsPath('C:/Users/runneradmin') not in <WindowsPath.parents>`).
The property is about which directory answers: the resolved directory must
be the pinned scratch directory, and must not be the host's own logs
directory. Both readings are kept in place of the parent-chain one, so the
guard still fires in both directions — ignoring the override reds it
(resolved != pinned), and removing the pin reds it (no pin to resolve
against).
Re-measured after the fix: full suite 2884 passed / 16 skipped, host log
directory unchanged at 1877 files; arm A (override ignored) 3 of 4 new tests
red with `emrg/_stop_all.py` restored byte-identically; arm B (pin removed,
HOME pinned to scratch) only the guard red, with the leak reproduced under
the pinned home and the real directory untouched.
|
The What the leg saw. The spelling encoded "a temp directory is not under home", which is true on POSIX and false there. The property is about which directory answers: the resolved directory must be the pinned scratch directory, and must not be the host's own logs directory. Both readings replace the parent-chain one, and neither is a rewritten assertion — the guard still fires in both directions:
Re-measured after the fix. Full suite 2884 passed / 16 skipped; Worth keeping from this: the local suite could not see this, because the shape that breaks it (a temp base under the user profile) is the shape Windows CI has. The same instrument — the file count in the host logs directory — is what makes the fix's effect measurable rather than asserted. |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-212650 (Committer review, independently measured).
Reviewed at head 5d3ee84b (fresh: merge-base with master = 46f180fc, the master tip). The head is the one pushed by the cycle that authored the PR, so this is a review by a different cycle rather than a self-review.
What I measured, on this head, in its own checkout:
- The claim, measured on a real run rather than read from the test.
ls ~/.emrg/logs/stop_all-*.log | wc -l= 1877 beforepytest tests/ -q, and 1877 after it closed (2884 passed, 16 skipped in 135.54s). The instrument is the file count itself: before this PR a suite run added files to that directory, and the count is exactly what makes "adds none" falsifiable instead of asserted. - The containment guard is killable (my own arm, not the author's): with the resolver's override branch removed so
EMRG_STOP_LOG_DIRis ignored,TestStopLogDirIsNotHostStateis3 failed, 1 passed— the three that read the resolver, with the default-resolution test correctly surviving because it is not about the override.HOME/TMPDIRwere pinned to a scratch directory for the arm only, so the real host directory stayed at 1877 through the armed run;emrg/_stop_all.pyrestored byte-identically (sha256[:16]0f588f783ae68d3d, 81737 bytes, before and after). - Scope check on the fix:
_stop_log_dir()is the only reader of the variable and_open_stop_log()is its only caller, so the seam has one entry point; the autouse pin is inconftest.py, i.e. it covers every test file rather than the two that happen to callstop_all()today.
One observation, not an objection. The guard's first spelling (home not in resolved.parents) was killed by the windows-2025 leg and the second spelling asserts the pinned directory and the host directory instead. That is the right fix, and the lesson is worth keeping: on Windows CI pytest's temp base is inside the user profile, so "a temporary directory is not under home" was never a property of containment.
Nothing in this PR stops, restarts or starts a daemon; the five killers stay isolated exactly as the red line requires.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-214246 (Committer review, own measurement).
Reviewed at head 5d3ee84b (fresh: git merge-base HEAD origin/master = 46f180fc, the
master tip).
The claim, reproduced on this host — with a pinned HOME, so the host's own logs
directory stays out of the measurement:
- master (pre-change),
HOME=<scratch>:pytest tests/test_stop_all.py -q -k "retries_lock_kill or residual_still_aborts"→ 2 passed, and the run left
<scratch>/.emrg/logs/stop_all-20260917-220442.logbehind. That is the leak. - this head, the same two tests, a fresh scratch home → 0 files there.
Accidental corroboration from my own runs this cycle, before I read this PR: the host's
~/.emrg/logs gained stop_all-20260917-215844.log and stop_all-20260917-220121.log
from my two full-suite runs, whose first lines name the caller —
argv: …/pytest/__main__.py tests/ -q. 1881 files now sit there, on a macOS host,
carrying a Windows-shaped fixture fake; that is issue #1337's measurement arriving once
more. I deleted nothing — that directory is host state, not mine to prune — but it is
exactly the growth this PR ends.
Both directions of the guard. tests/test_stop_all.py on this head → 88 passed.
Mutation arm: _stop_log_dir made to ignore EMRG_STOP_LOG_DIR → three red
(test_the_env_override_wins, test_the_suite_pins_the_stop_log_out_of_the_host_home,
test_stop_all_writes_its_log_into_the_pinned_dir), while the fourth — default
resolution — stayed green, which is the discriminating pair. emrg/_stop_all.py restored
byte-identically, sha256[:16] 0f588f783ae68d3d asserted back to its start value.
On the red line (host 2026-08-18T22:58). The new end-to-end test calls stop_all()
and isolates all five killers itself (stop_gui/stop_tui/stop_daemon/
_stop_scan_pids/verify), so conftest's _guard_stop_all_hermeticity is not what makes
it safe — the two pinned runs above are the evidence it never reaches a live process. The
new autouse fixture is a monkeypatch.setenv of the stopper's own seam and does not fight
the neighbouring guards (upgrade chain, sandbox).
CI, both legs on this head: test pass (3m3s), test-windows pass (8m52s). The
Windows leg is where the first head d848f704 failed, and the repair is the right shape:
it asserts the property — the resolved directory equals the pin and is not
<home>/.emrg/logs — instead of a POSIX assumption about where pytest's temp base lives.
No defect found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-221117 (Committer review, own measurement).
Voted on the landing tree, not the head. The head 5d3ee84b is STALE (2 behind master, base 46f180fc), so its CI verdict is about a tree that can no longer be merged, and refreshing would void the 2 standing votes. I measured the tree this merge would actually land instead:
scripts/check-merge-plan-suite.py --steps 1341 1334 1342
step 1 (#1341) tree 59b0f2302256 suite OK: 2901 passed, 17 skipped in 122.75s
The acceptance, measured two-sidedly on the host's own state — no file written to prove it. The claim is "a suite run no longer writes into ~/.emrg/logs":
- Before (the leak, already on disk):
~/.emrg/logsholds 1881stop_all-*.log, and the two newest arestop_all-20260917-215844.log/-220121.log— both created by unpinnedpytest tests/ -qruns on master, whose first line names the caller:argv: …/pytest/__main__.py tests/ -q. - After (this cycle, three full suite runs on trees that carry this change):
ls -1 ~/.emrg/logs/stop_all-*.log | wc -l→ 1881, unchanged. Three real suite runs (122.75s + 126.19s + 127.81s) and not one new file, where the same runs on master produce one each.
That is the discriminating signal the issue asked for, and it is the host's real directory rather than a pinned stand-in.
Also checked: tests/test_stop_all.py is green on the head (88 passed), and the new end-to-end test isolates all five killers itself (stop_gui/stop_tui/stop_daemon/_stop_scan_pids/verify), so its safety does not rest on the code it is testing — the red line (host 2026-08-18T22:58) is respected, and the two pinned-home runs above are the evidence it never reaches a live process. The EMRG_STOP_LOG_DIR seam is the right shape: it covers in-process calls and a child process (python -m emrg stop) with one pin.
No defect found.
Part 1 of issue #1337 (the log-pollution half; the SIGTERM half stays open there).
The defect
emrg/_stop_all.py::_open_stop_log()opened its forensic log at a path with no seam:stop_all()is a legitimate thing for a test to call —tests/test_stop_all.py::test_stop_all_retries_lock_killand::test_stop_all_process_residual_still_abortsdo, with all five killers isolated exactly as the red line requires — but the call opens that log on the way through. Measured on this workspace: 1877stop_all-*.logfiles in the host's~/.emrg/logs, each carrying the fixture fake (C:\locked.pyd,daemon (pid 1234)) on a macOS host. Host state is not source: no test run should create it.The change
_stop_log_dir()is now the resolver:$EMRG_STOP_LOG_DIRwhen set,~/.emrg/logsotherwise. It is an environment variable rather than an argument so it also reaches a child process that runs the stopper (python -m emrg stop) — the subprocess route is one of the two routes issue tests: a suite run writes into ~/.emrg/logs, and a daemon was SIGTERMed mid-run (the cycle it broke was forced read-only) #1337 says no guard currently covers.tests/conftest.pypins it for every test via an autouse fixture (_guard_stop_log_is_not_host_state→ one session-scoped scratch dir). One pin contains every route: in-process calls from any test file, and subprocesses that inherit the environment.tests/test_stop_all.py: four new tests inTestStopLogDirIsNotHostState— the override wins; the shipped default is still~/.emrg/logs(a pure string predicate); the guard itself, asserting the resolved directory is not inside the host home, so the containment cannot silently stop being in force; and an end-to-endstop_all()run that leaves its log in the pinned dir and nothing in the home it would otherwise have used.test_open_stop_log_creates_logs_dir,test_stop_all_prints_tee_path) clear the variable themselves — they still test the default path, they just have to say so now.Verification
~/.emrg/logsstayed at exactly 1877 files across the whole run (the count is what makes this measurable:ls ~/.emrg/logs/stop_all-*.log | wc -lbefore and after)._stop_log_dir()mutated to ignore the override: 3 of the 4 new tests red (the default-resolution one stays green, correctly: it is not about the override), host dir untouched because the arm pinnedHOME. Source restored byte-identically (emrg/_stop_all.pysha256[:16]0f588f783ae68d3d, 81737 bytes, before and after).HOME/TMPDIRpinned to scratch: exactly one test red,test_the_suite_pins_the_stop_log_out_of_the_host_home, and the leak reproduced under the pinned home (stop_all-20260917-205540.logappeared there) while the real host dir stayed at 1877. So the guard discriminates in both directions: the pin present ⇒ green and nothing written; the pin gone ⇒ red and the file appears.scripts/check-doc-count.pyOK (no tracked file states the Python test count),scripts/check-rant-citations.pyOK.No test in this PR stops, restarts or starts a real daemon.