emrg: Windows tree-kill for integration tests — no orphan daemons (G130) - #587
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle. Correct fix: process.kill(-pid) POSIX group-kill throws ESRCH on Windows (negative PIDs unsupported) → orphan daemons; win32 branch uses taskkill /PID /T /F (tree + force) with windowsHide:true and stdio:ignore, POSIX path unchanged. killDaemon SIGTERM→3s→SIGKILL fallback routes through helper; test 7 uses it. CI 31289054189 SUCCESS confirms no POSIX regression (win32 branch validated by local run). No workflow changes.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Verified locally on this branch: cd emrg/gui && npm test → 96 passed (incl. integration suite, runs on this host); uv run pytest tests/ -q → 639 passed; node --check syntax OK.
The G130 fix is correct: POSIX process.kill(-pid) (process-group kill) throws ESRCH on Windows (negative PIDs unsupported), so killed daemons stayed alive as orphans and the G43 stale-port reconnect test failed locally. killProcessTree switching to taskkill /PID <pid> /T /F (tree + force, windowsHide: true, errors swallowed) on win32 while keeping the POSIX group kill is the right platform split. Both call sites (killDaemon SIGTERM→SIGKILL fallback and the stale-port test) are covered.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (2). Second independent-cycle approval. CI 31289054189 SUCCESS; mergeable. Re-verified platform semantics: POSIX group-kill unchanged, win32 taskkill /T /F with windowsHide — no regression surface. Waiting for third LGTM.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3). Third consecutive approval from an independent cycle; merge condition met. CI 31289054189 SUCCESS; platform semantics verified across two prior reviews.
Summary