From 84659e2eab3bb1e6ced6ec575f8a8bd6af2e9c29 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Sat, 22 Aug 2026 00:19:26 -0700
Subject: [PATCH] fix(ports): report a Stop as succeeded when the listener
 already exited (#15888)

* fix(ports): report a Stop as succeeded when the listener already exited

`killWorkspacePort` surfaced the raw `kill ESRCH` when the pid exited between
the authorizing re-scan and the signal. The port is free at that point -- which
is exactly what Stop was asked for -- so the UI reported a failure for work that
had already completed.

Also pins the pid we signal: `netstat -ano` and `lsof` report the process that
owns the socket, so the scanned pid is the listener itself, not a supervising
wrapper. Escalating this to a tree kill would reach descendants nobody asked to
stop without freeing anything extra.

* test(ports): give the spawn-stall test a budget longer than the stall

It blocks the calling thread for the full watchdog budget plus margin (5.2s) and
then asserts against vitest's 5s default, so it has been failing deterministically
since #12217.
---
 .../ports/port-scan-command-execution.test.ts |  5 +-
 .../ports/workspace-port-ownership.test.ts    | 54 +++++++++++++++++++
 src/main/ports/workspace-port-ownership.ts    |  7 +++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/src/main/ports/port-scan-command-execution.test.ts b/src/main/ports/port-scan-command-execution.test.ts
index 15e97d4a3a9..2ebc0c9e777 100644
--- a/src/main/ports/port-scan-command-execution.test.ts
+++ b/src/main/ports/port-scan-command-execution.test.ts
@@ -42,7 +42,10 @@ describe('runPortScanCommandInProcess', () => {
 
     expect(result.stdout).toBe(LSOF_OUTPUT)
     expect(result.spawnMs).toBeGreaterThanOrEqual(SPAWN_STALL_MS - 100)
-  })
+    // Why the explicit budget: the stall is deliberately longer than the whole
+    // watchdog budget (5.2s), so on vitest's 5s default this test could never
+    // pass -- it has been red since #12217 landed.
+  }, 15_000)
 
   it('kills the child and times out when the callback never arrives', async () => {
     vi.useFakeTimers()
diff --git a/src/main/ports/workspace-port-ownership.test.ts b/src/main/ports/workspace-port-ownership.test.ts
index b367fcbf29c..d4410dd0a23 100644
--- a/src/main/ports/workspace-port-ownership.test.ts
+++ b/src/main/ports/workspace-port-ownership.test.ts
@@ -7,6 +7,25 @@ vi.mock('./local-workspace-port-scanner', () => ({
   scanWorkspacePorts: scanWorkspacePortsMock
 }))
 
+function workspacePortScan(pid: number, port: number) {
+  return {
+    platform: 'win32' as const,
+    scannedAt: 0,
+    ports: [
+      {
+        id: `127.0.0.1:${port}:${pid}`,
+        bindHost: '127.0.0.1',
+        connectHost: '127.0.0.1',
+        port,
+        pid,
+        protocol: 'http' as const,
+        kind: 'workspace' as const,
+        worktreeId: worktrees[0]!.id
+      }
+    ]
+  }
+}
+
 const worktrees = [{ id: 'repo::/repo', repoId: 'repo', displayName: 'main', path: '/repo' }]
 
 describe('killWorkspacePort', () => {
@@ -15,6 +34,41 @@ describe('killWorkspacePort', () => {
     vi.restoreAllMocks()
   })
 
+  it('reports success when the listener exited before the signal landed', async () => {
+    // Why: the re-scan authorizes the pid, then the dev server exits on its own
+    // before `kill` runs. The port is free -- which is what Stop asked for --
+    // but the raw ESRCH surfaced as "Stop failed" on a port that was gone.
+    vi.spyOn(process, 'kill').mockImplementation(() => {
+      throw Object.assign(new Error('kill ESRCH'), { code: 'ESRCH' })
+    })
+    scanWorkspacePortsMock.mockResolvedValue(workspacePortScan(123, 5173))
+
+    expect(await killWorkspacePort(worktrees, { pid: 123, port: 5173 })).toEqual({ ok: true })
+  })
+
+  it('still reports a real failure to stop', async () => {
+    vi.spyOn(process, 'kill').mockImplementation(() => {
+      throw Object.assign(new Error('kill EPERM'), { code: 'EPERM' })
+    })
+    scanWorkspacePortsMock.mockResolvedValue(workspacePortScan(123, 5173))
+
+    expect(await killWorkspacePort(worktrees, { pid: 123, port: 5173 })).toEqual({
+      ok: false,
+      reason: 'kill EPERM'
+    })
+  })
+
+  it('signals the pid the socket scan named, which is the listener itself', async () => {
+    // Why pinned: netstat -ano / lsof report the process that owns the socket,
+    // not a supervising wrapper, so escalating this to a tree kill would reach
+    // descendants nobody asked to stop without freeing anything extra.
+    const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true)
+    scanWorkspacePortsMock.mockResolvedValue(workspacePortScan(123, 5173))
+
+    expect(await killWorkspacePort(worktrees, { pid: 123, port: 5173 })).toEqual({ ok: true })
+    expect(killSpy).toHaveBeenCalledExactlyOnceWith(123, 'SIGTERM')
+  })
+
   // Regression for #11161 review: on an EDR-hooked host the background poller
   // alternates the metadata skip, so an unscoped skip would fail Stop with
   // "Only workspace-owned local processes can be stopped here" every other try.
diff --git a/src/main/ports/workspace-port-ownership.ts b/src/main/ports/workspace-port-ownership.ts
index f9020d76682..31c7d193c62 100644
--- a/src/main/ports/workspace-port-ownership.ts
+++ b/src/main/ports/workspace-port-ownership.ts
@@ -109,6 +109,13 @@ export async function killWorkspacePort(
     process.kill(pid, 'SIGTERM')
     return { ok: true }
   } catch (error) {
+    // Why ESRCH is success: the pid exited between the authorizing re-scan and
+    // this signal, so the listener is gone and the port is free -- which is
+    // what Stop was asked for. Surfacing the raw `kill ESRCH` made a Stop that
+    // had already succeeded read as a failure.
+    if ((error as NodeJS.ErrnoException)?.code === 'ESRCH') {
+      return { ok: true }
+    }
     const message = error instanceof Error ? error.message : String(error)
     return { ok: false, reason: message || 'Failed to stop the process.' }
   }