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.' } }