What Problem This Solves
Quitting could leave the backend running, still holding its port, so the next launch failed to start. Two independent defects combined to produce it.
On the Electron side, both shutdown paths resolved as soon as a signal had been delivered:
kill()completing meanskill(2)succeeded, not that the process acted on it. The backend'scleanup_resources()drains the execution service, run coordinator, cloud sync worker and a WebSocket pool — that takes seconds, and the app quit straight through it.- The
setTimeout(..., 1000)that sendsSIGKILLwas dropped whenever the caller unwound first, which is the normal case on quit. The signal was scheduled on a timer that the exiting process never reached. - Only the parent pid was signalled, while the backend spawns workers. The workers are what hold the port, and they survived their parent.
cleanupPythonProcess also called removeAllListeners() before signalling, discarding the close event — the only reliable evidence that the process actually terminated.
On the backend side, removing the PID file ran unguarded and before the telemetry, thread-pool and WebSocket cleanup:
pid_file = dir / "run.pid"
if pid_file.exists():
pid_file.unlink() # not guardedatexit also removes that file, so a racing unlink raised FileNotFoundError and skipped every step after it — including the TerminalToolkit pool, whose non-daemon threads are precisely what prevent the process from exiting.
Solution Details
- New
electron/main/utils/processTree.tswithterminateProcessTree(): signals the process group, escalatesSIGTERM→SIGKILL, and resolves only once the child reports it exited. It returnsfalsewhen the process is still alive after the force window, so the caller can say the port may stay held rather than quitting silently over a live backend. releaseChildListeners()drops thestdout/stderrlisteners a long-lived child accumulates, and deliberately keepsclose/exit/error— dropping those is what made the old cleanup unverifiable.- Both call sites now use it:
cleanupPythonProcessinindex.ts, and the startup-failure path ininit.ts, which had the same defect. - Backend: the PID file is now removed last, guarded, and logged rather than propagated. Bookkeeping no longer comes before real cleanup.
Note the tree-kill dependency is no longer used by index.ts and its import is gone; processTree.ts signals the group directly, which is what init.ts already did elsewhere in the same codebase.
Evidence
7 tests in test/unit/electron/main/processTree.test.ts:
| Test | Pins |
|---|---|
waits for the process to actually exit before resolving | the core defect — asserts the promise is still pending before exit |
signals the process group, not just the parent | asserts the negative pid |
escalates to SIGKILL when SIGTERM is ignored | escalation |
reports failure when the process survives SIGKILL | no silent success |
does nothing for a process that is already gone | no signal to a dead pid |
keeps the listeners that report termination | the removeAllListeners regression |
3 tests in backend/tests/app/test_shutdown_cleanup.py cover the ordering and the surviving cleanup. Reintroducing the original unguarded, early unlink fails test_resource_cleanup_survives_a_failed_pid_file_removal and passes the other two, so the suite is pinned to this behaviour.
vitest run test/unit/electron/main/processTree.test.ts— 7 passedpytest tests/app/test_shutdown_cleanup.py— 3 passednpm run type-check— cleanruff check/ruff format— clean; the repo's own pre-commit hooks (Ruff, Bandit) pass on the commit
test/unit/electron/main/envUtil.test.ts has 14 failures on main before this change, unrelated to it (util.isString is not a function — the jsdom environment resolves path to a browser polyfill). Untouched here to keep the diff focused.
User Impact
Quitting actually stops the backend before the app exits, so the port is free for the next launch. If a backend ever refuses to die, that is now logged instead of silently leaving it running.
Closes #1671
Rebased onto current
mainand re-verified while it waited for review:npm run type-checkandeslintare clean.mainbaseline.mainresolved by hand; the fix is now scoped to just the files it needs.The linked issue is still open, and this is ready for a look whenever you have time.