Skip to content

emrg: stop-emrg.cmd — kill only EMRG-owned git trees, never host Git Bash sh/vim (rant 2026-08-11T18:56:58) - #689

Merged
argszero merged 2 commits into
masterfrom
feature/stop-emrg-host-gitbash-fix
Aug 11, 2026
Merged

emrg: stop-emrg.cmd — kill only EMRG-owned git trees, never host Git Bash sh/vim (rant 2026-08-11T18:56:58)#689
argszero merged 2 commits into
masterfrom
feature/stop-emrg-host-gitbash-fix

Conversation

@argszero

Copy link
Copy Markdown
Owner

Windows installer: stop-emrg.cmd step 4 now kills only EMRG-owned git subprocess trees — never the host's Git Bash sh/vim (host rant 2026-08-11T18:56:58).

Bug (host-verified on v0.2.26)

The #683 step-4 blanket ExecutablePath -like "$env:USERPROFILE\.emrg\install\git\*" kill caught the host's own Git Bash session tools (sh.exe + vim.exe launched from install\git\), not just EMRG's evolution-cycle git orphans. Consequences:

  • host's vim session killed (unsaved edits lost!)
  • interactive sh/vim can't be killed → :verify survival check sees them → exit 1 → installer aborts ("could not stop all running processes")

This is the same class as R125 (CloseApplications=no, rant 17:03:00): Inno must not interfere with host tools; only EMRG processes are ours to stop.

Fix (bin/stop-emrg.cmd + tests)

  • step 4 kill: a process under install\git\ is killed only if its ancestor chain (≤5 levels, ParentProcessId walk) contains the daemon (pythonw.exe -m emrg.server) or the TUI (python.exe -m emrg) — i.e. it is an EMRG-spawned git/ssh/bash subprocess. Host Git Bash sh/vim have explorer/terminal ancestors → never matched, never killed.
  • :verify survival check: now reports exit 1 only when an EMRG-owned git subtree survives (could not be killed → installer aborts with the existing restart hint). Host sh/vim still alive is not a failure.
  • Quick-ref entry for emrg: Windows installer — kill orphaned bundled-git processes before overwrite (rant 2026-08-11T17:56:25) #683 updated to reflect the corrected direction.

Verification

  • tests/test_installer_stop.py updated + passing: anchors ParentProcessId ancestor walk + -m emrg ownership check in both step 4 and verify; asserts the old blanket if (Get-CimInstance …) { exit 1 } verify check is gone
  • pytest 694 passed · import + --help OK
  • Windows real-machine behavior: host verifies per acceptance (install with a Git Bash sh/vim open → installer continues, vim untouched; real EMRG evolution git subprocess → killed, install succeeds)

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

❌ Needs fix: ancestor walk cannot resolve DEAD parents — the #683 primary orphan case regresses.

The bug: step 3 kills the daemon (taskkill /F /PID %DPID%), THEN step 4 walks UP from each install\git process checking CommandLine -match '-m emrg'. But the #683 scenario is exactly orphans — git/ssh/bash whose daemon parent was ALREADY killed. The walk's parent lookup ($all | Where-Object { $_.ProcessId -eq $cur.ParentProcessId }) returns $null for the dead daemon (not in the Win32_Process snapshot) → the loop terminates → $emrg stays false → the orphan is NOT killed. The :verify block has the same flaw → reports no EMRG git processes → installer proceeds → DeleteFile failed; code 5 (the original #683 symptom) returns.

Trace:

  • A. True orphan (daemon killed mid-git-op): git → sh/bash (alive, no -m emrg in cmdline) → daemon (DEAD, $null lookup) → loop ends, not killed. ❌ primary case broken
  • B. Daemon still alive: git → sh → daemon (-m emrg found) → killed ✓
  • C. Host Git Bash sh/vim: explorer/terminal ancestors → not killed ✓ (this part works)

The test anchors pattern presence (ParentProcessId, -match '-m emrg') but not the dead-parent semantics, so it passes while the logic fails scenario A.

Fix suggestion: snapshot the descendant tree BEFORE killing the daemon/TUI. In step 3 (daemon still alive), walk DOWN from %DPID% (and the TUI PIDs from step 2) collecting the full subprocess PID set; then in step 4, kill any recorded PID still alive whose ExecutablePath is under %USERPROFILE%\.emrg\install\git\*. The verify check then tests the same recorded set for survivors. This preserves the host-tool protection (scenario C) while restoring orphan cleanup (scenario A).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (post-fix re-review)

The earlier ❌ (ancestor walk cannot resolve DEAD parents — the #683 primary orphan case regresses) is fixed by snapshotting the EMRG-owned tree downward before any kill:

  • New step 0: BFS from roots (emrgd.pid daemon PID + EMRG.exe + python.exe -m emrg TUI) collecting the full descendant PID set → written to %TEMP%\emrg-stop-pids.txt
  • Step 4: kills only RECORDED PIDs still alive whose executable is under install\git\ → orphans of an already-dead daemon are captured (recorded while the daemon lived), host Git Bash sh/vim are not in the recorded set → never touched
  • :verify: checks only the recorded set for survivors → exit 1 iff an EMRG git PID survives; host tools alive ≠ failure
  • Tests updated to anchor the snapshot semantics (emrg-stop-pids.txt, Set-Content, $ids -contains in step 4 + verify, no blanket verify check)
  • CI test PASS on head e67e604; pytest 694 green

Scenarios: A (dead-daemon orphan) ✓ captured pre-kill · B (daemon alive) ✓ · C (host sh/vim) ✓ not in set.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (post-fix re-review)

The R827 concern is resolved. Verified on head e67e604:

  • Step 0 snapshot (before ANY kill): roots = emrgd.pid daemon PID + EMRG.exe + TUI python.exe -m emrg; BFS fixed-point down via ParentProcessId collects the full descendant set → written to %TEMP%\emrg-stop-pids.txt
  • Dead-parent case fixed: emrgd.pid is added to the root set even if the daemon process is already gone — live orphans whose ParentProcessId == the recorded daemon PID still match → #683 primary scenario restored
  • Host-tool protection preserved: steps 4/:verify operate ONLY on the recorded set + ExecutablePath filter under install\git; host Git Bash sh/vim (not EMRG descendants) are never recorded, never touched
  • Ordering: snapshot (step 0) < daemon stop (step 3) < step 4 kill < :verify; PID file cleaned up on all exit paths
  • Tests: updated anchors ( -contains, Set-Content, emrg-stop-pids.txt, del cleanup; blanket }) { exit 1 } removed from verify) — 4 passed locally; CI Test on head (31485202131) PASS

Mergeable.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (3rd, different cycle)

Fresh re-verification of head e67e604 (unchanged since R828 post-fix review): step-0 downward snapshot (roots = emrgd.pid + EMRG.exe + TUI, BFS via ParentProcessId fixed-point) recorded before any kill; steps 4/:verify operate only on recorded set + install\git ExecutablePath filter — dead-parent orphan case (#683) restored, host Git Bash sh/vim untouched. Tests anchor the new discriminators; CI Test on head PASS (31485202131). Mergeable — proceeding to merge.

@argszero
argszero merged commit 888efb0 into master Aug 11, 2026
1 check passed

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (3rd post-fix)

Fresh re-verification on head e67e604 (unchanged since post-fix):

  • Step 0 snapshots the EMRG-owned tree DOWNWARD (BFS from emrgd.pid daemon PID + EMRG.exe + python.exe -m emrg TUI, excluding emrg.server) BEFORE any kill → %TEMP%\emrg-stop-pids.txt; dead-daemon orphans are captured because their PIDs were recorded while the daemon lived
  • Step 4 kills only recorded PIDs whose executable is under install\git\ → host Git Bash sh/vim never touched (R125 class)
  • :verify checks only the recorded set → host tools alive ≠ failure
  • The #683 quick-ref entry in evolution_prompt.md now carries the 18:56:58 correction direction (path-prefix blanket kill was wrong; EMRG-tree-only kill is right)
  • Local: pytest 694 ✓ import ✓ --help ✓; CI Test PASS on head (31485202131)

Mergeable.

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.

1 participant