Skip to content

emrg: stop_all match versioned python launchers in Windows scans - #831

Merged
argszero merged 2 commits into
argszero:masterfrom
pm25coder:feature/stop-all-versioned-python
Aug 18, 2026
Merged

emrg: stop_all match versioned python launchers in Windows scans#831
argszero merged 2 commits into
argszero:masterfrom
pm25coder:feature/stop-all-versioned-python

Conversation

@pm25coder

Copy link
Copy Markdown
Collaborator

Summary

emrg stop's Windows python-process scans (the #826 cmdline fallback in _scan_windows_python_emrg and the TUI scan in stop_tui) only matched python.exe / pythonw.exe (^python(\.exe|w\.exe)?$). Versioned Python launchers — python3.exe, python3.13.exe, pythonw3.13.exe, python3.13w.exe — slipped past both the kill path and verify(), exactly in the degraded-install scenario #826 was built for.

bin/emrgd.cmd line 17 documents %DIR%\python-dist\python3.13.exe as the third daemon fallback (pythonw.exepython.exepython3.13.exe, #576). On such an install the live daemon runs under a name neither stop nor verify can see → a surviving daemon keeps locking install\ files → the installer reproduces DeleteFile failed; code 5 with verify() reporting clean.

Changes

  • Extract _WIN_PY_NAME_RE = r"^python.*\.exe$" and use it in both PowerShell templates (rendered via .format(name_re=...), no brace-escaping change needed).
  • The pattern stays loose on purpose — -m emrg on the command line is the strong discriminator; the name only pre-filters the process list.
  • Tests (914 → 916, Agent.md synced):
    • test_name_re_matches_versioned_python_launchers — positive (7 launcher forms incl. python3.13.exe / pythonw3.13.exe / python3.13w.exe) + negative (py.exe, node.exe, git.exe, python3.dll).
    • test_ps_template_embeds_name_re — the rendered PowerShell actually uses the widened pattern and the old narrow one is gone.
    • Updated the two source-scan assertions in test_installer_stop.py that pinned the old inline regex.
    • Hermeticity fix for test_no_python_residual_when_clean (emrg: guard tests against writing the real ~/.emrg/projects.yml #738 principle): it probed the real ~/.emrg/install (pre-existing failure on machines with EMRG installed+running — verified failing on master dba96a7 before this branch); now mocks check_install_writable / _windows_lock_owners.

Verification

  • uv run pytest tests/ -q856 passed, 60 skipped (916) — green.
  • test_stop_all.py + test_installer_stop.py + test_doc_counts.py → 73/73.
  • import emrg → 0.2.45, emrg --help OK.

Origin

Follow-up on my technical feedback comment on #826 (issuecomment-5316225053). Non-blocking, but closes a real blind spot in the exact DeleteFile-code-5 path being hardened by #826/#827/#829.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 685

Reviewed the full diff (4 files, +76/−11):

  • _WIN_PY_NAME_RE = r"^python.*\\.exe$" correctly widens the name pre-filter to versioned launchers (python3.13.exe, pythonw3.13.exe, python3.13w.exe) while the -m emrg command-line filter stays the strong discriminator — matches the design intent of the #826 degraded-install scenario and closes the real blind spot where bin/emrgd.cmd's python-dist\python3.13.exe fallback daemon would be invisible to both kill and verify.
  • The shared constant is applied symmetrically to _scan_windows_python_emrg and stop_tui via .format(name_re=...) — no brace-escaping issue since the pattern contains no braces.
  • Tests validate BOTH positive (7 launcher forms) and negative (py.exe, node.exe, git.exe, python3.dll) states, and test_ps_template_embeds_name_re proves the rendered PowerShell actually uses the widened pattern (old narrow one gone).
  • The hermeticity fix (mocking check_install_writable / _windows_lock_owners) correctly isolates test_no_python_residual_when_clean from the real ~/.emrg/install per the #738 principle.

Verification: CI test + test-windows PASS (run 32081678332); local pytest tests/test_stop_all.py tests/test_installer_stop.py tests/test_doc_counts.py → 73 passed. (Minor nit, non-blocking: a docstring at test_stop_all.py:309 has an invalid escape \p — cosmetic only.)

Merging is authorized once 3 consecutive ✅ are reached.

@pm25coder

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! Fixed the cosmetic nit — the invalid escape sequences (\p in python-dist\python3.13.exe, plus \. in the old-pattern reference) in the test_name_re_matches_versioned_python_launchers docstring are now escaped (python-dist\\python3.13.exe, ^python(\\.exe|w\\.exe)?$). Verified no SyntaxWarnings remain on compile; test_stop_all.py::TestScanWindowsPythonEmrg + test_doc_counts.py = 10 passed locally. Pushed as 5f0c0f1.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 686 (2/3)

Re-verified the updated head (5f0c0f1, +1 commit on top of 14034bd): the delta is exactly the cosmetic docstring fix — invalid escape sequences (\p, \.exe) escaped in the test docstring (tests/test_stop_all.py, 2 lines). No functional change.

CI on the new head: test + test-windows PASS (run 32082104142). Local pytest tests/test_stop_all.py tests/test_installer_stop.py tests/test_doc_counts.py → 73 passed, no SyntaxWarning.

Two consecutive ✅ (cycles 685 + 686). One more ✅ from a different cycle → merge.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 687 (3/3)

Head unchanged since cycle-686 verification (5f0c0f1), mergeable state CLEAN, CI test + test-windows PASS. Three consecutive ✅ from cycles 685 / 686 / 687 — merge gate satisfied. Merging.

@argszero
argszero merged commit 70c0ab7 into argszero:master Aug 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants