fix(ports): stop spurious "Failed to close N port(s)" toast on close-all

Merged
AV
5mo ago
+87-4

Summary

  • Closing several ports at once (the workspace "Close all ports" X) succeeded but spuriously toasted "Failed to close 1 port — Port not found in tracked ports."
  • PortManager.killPort now treats a no-longer-tracked port as already closed → success, eliminating the false-failure toast. Fixes both desktop-local and host-service kill paths (shared port-scanner package).

Why / Context

"Close all" calls killPorts(group.ports), which fires every kill concurrently (Promise.all). The first kill runs treeKillWithEscalation(pid), which tears down the entire process tree — so when two of a workspace's ports share a process tree (common: a dev server plus its HMR/websocket/inspector port), killing one also kills the process behind the sibling port. A port scan (the periodic interval, or a hint scan triggered by the dying process's terminal output) then removes the now-dead sibling from the tracked ports map.

By the time the sibling's own kill call reaches its synchronous map lookup, the entry is gone, so it returned { success: false, error: "Port not found in tracked ports" }. The renderer counted one failed result and showed the toast — even though the port really was closed, which is why it "seems to work fine."

How It Works

A port that is no longer tracked is, by definition, already closed — exactly the outcome a kill aims for. The !detectedPort branch in killPort now returns { success: true } instead of an error. This mirrors treeKillWithEscalation, which already treats an already-dead process (ESRCH) as success; the change just aligns the map-lookup path with that behavior. The workspace-mismatch and shell-process guards are unchanged.

Manual QA Checklist

  • Start a workspace with a dev server that listens on multiple ports (e.g. Vite/Next with HMR).
  • Click the workspace "Close all ports" X — ports close and no error toast appears.
  • Single per-port X still closes the port with no error toast.
  • Closing a port that is still genuinely listening returns success (process killed).

Not yet verified live in the running app — this is a code-path fix with unit coverage. Happy to run /verify against the desktop app if desired.

Testing

  • bun test packages/port-scanner/src/port-manager.test.ts — 26 pass (4 new killPort regression tests: tracked kill, untracked = success, shell-process guard, workspace mismatch)
  • bun run lint
  • bun run typecheck

Risks / Rollback

  • Risk: low. Narrows a single error path to success; the kill itself (treeKillWithEscalation) is unchanged and already idempotent for dead processes. The only behavior change is that a kill targeting an untracked port no longer reports failure.
  • Rollback: revert this commit.
Open in Stage

Summary by cubic

Stops the false "Failed to close N port(s)" toast when using Close all ports. PortManager.killPort now treats an untracked port as already closed so concurrent kills complete without errors.

  • Bug Fixes
    • Sibling kills after a shared process tree is terminated now return success instead of "Port not found in tracked ports".
    • Applies to both desktop-local and host-service paths via shared port-scanner.

Summary by CodeRabbit

  • Bug Fixes

    • Updated port termination logic to report success when targeting an untracked port, rather than reporting failure.

  • Tests

    • Added comprehensive test coverage for port termination scenarios, including successful termination, untracked port handling, shell process protection, and workspace validation checks.