fix(desktop): start the terminal reaper in the desktop host-service entry

Merged
KI
4mo ago
+6-1

Summary

  • Wire startTerminalReaper(db) into the desktop host-service entry point. It was only called from packages/host-service/src/serve.ts (the standalone/CLI entry), so the packaged desktop app never ran the terminal reaper at all.
  • This is the actual fix for "ports don't show up after an update until the workspace is selected" — and it also turns on the 5-minute orphan reap (#5168), which has likewise never run in the desktop app.

Why / Context

#5325 added the reaper-driven port-scan reconcile (register daemon sessions no renderer has attached, e.g. after a host-service restart) and #5430 added 2–90s warm-up retries. Both are correct — but startTerminalReaper was never invoked by apps/desktop/src/main/host-service/index.ts, the entry the desktop app actually spawns. In the shipped app the only portManager.upsertSession caller was the renderer terminal-attach path, so ports for surviving sessions stayed invisible until the user opened the workspace.

Confirmed against a production install (v1.13.0): the asar contains the #5430 code, the daemon adopts within seconds, valid terminal_sessions rows exist — and the host-service log contains zero reaper lines across every run.

Supersedes the revert stack in #5437 (kept as a draft safety net; merging that would not fix this symptom).

How It Works

startTerminalReaper is exported from the @superset/host-service root, and each serving entry (serve.ts and the desktop entry) calls it once in its listen callback — the same per-entry pattern as installProcessSafetyNet, initTerminalBaseEnv, and connectRelay. All reaper logic stays in the package.

Two single-call-site alternatives were tried and rejected during review discussion: starting it inside createApp requires a test-only escape hatch (the reaper is a destructive background actor that reaps integration tests' staged daemon sessions mid-test — 5 failures), and a startHostRuntime wrapper was pure pass-through indirection.

Manual QA

End-to-end against the freshly built dist/main/host-service.js (run under Electron-as-Node, exactly as the app spawns it):

  • Started a scratch pty-daemon, opened one session running python3 -m http.server 18923, and disconnected — an alive, unattached daemon session, the post-update state.
  • Started the built host-service against a scratch DB with SUPERSET_PTY_DAEMON_SOCKET pointed at the scratch daemon, and inserted the session's terminal_sessions row (status: "active" + workspace id).
  • Warm-up sync logged port-scan sync: registered 1 unattached daemon session(s) for scanning within seconds — no renderer ever attached.
  • ports.getAll for that workspace returned { port: 18923, processName: "Python", ... }.
  • Verified the built bundle contains the startTerminalReaper(db) call.
  • Re-ran the scratch harness against the restructured bundle (reaper started from createApp): same result — registered 1 unattached daemon session(s) and ports.getAll returning the planted port.

Full-app verification over CDP (RENDERER_REMOTE_DEBUG_PORT=9223 bun dev, this branch):

  • Planted an unattached daemon session running python3 -m http.server 18933 (row bound to a workspace that was never selected), no renderer attach.
  • The dev app's host-service reaper logged port-scan sync: registered 1 unattached daemon session(s) for scanning.
  • The sidebar then rendered the 18933 port badge inside that workspace's item while a different workspace was selected — confirmed via CDP DOM inspection.
  • Dev-mode caveat: the dev daemon is not detached (dies with host-service by design), so the restart leg was covered by the scratch harness above; the dev app covered reaper registration + renderer display with no attach.

Known Limitations

  • The desktop entry (unlike serve.ts) doesn't kick startDaemonBootstrap at startup — the daemon boots on demand. The reaper's earliest warm-up (2s) can beat it and log port-scan warm-up sync failed: pty-daemon is not available once; later warm-ups (5–90s) and the 5-min reap succeed. Pre-existing ordering, observed once in dev; harmless but noisy.

Testing

  • bun run typecheck --filter=@superset/host-service --filter=@superset/desktop
  • bun run lint (exit 0)
  • bun test src/terminal/reaper in packages/host-service (9 pass)
  • bun turbo run build --filter=@superset/desktop

Risks / Rollout

  • Risk: the desktop app now also runs the orphan reap it was always meant to run. Rowless alive daemon sessions are killed only after two consecutive 5-minute passes, and status: "exited"/"disposed" rows only describe already-dead PTYs, so healthy sessions are unaffected — but this is behavior the desktop app has never exercised in production.
  • Rollback: revert this commit; behavior returns to today's (no reaper, ports on attach only).
Open in Stage

Summary by cubic

Start the terminal reaper from the desktop host-service entry with startTerminalReaper(db). This cleans up orphaned sessions and reconciles ports on boot, fixing ports not showing until a workspace is opened.

  • Bug Fixes
    • Start the reaper in the desktop entry and export it from @superset/host-service; both serving entries run it on boot.
    • Remove startHostRuntime and keep reaper startup outside createApp.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cleanup of desktop terminal sessions during app startup, reducing stuck or orphaned terminals.
    • Enhanced startup reliability for the desktop host service by adding an additional terminal reaping step once the service is ready.