diff --git a/build-plugins/plain-node-entry-guard.ts b/build-plugins/plain-node-entry-guard.ts index a7ad171a4..13499118b 100644 --- a/build-plugins/plain-node-entry-guard.ts +++ b/build-plugins/plain-node-entry-guard.ts @@ -26,6 +26,23 @@ const PLAIN_NODE_ENTRY_NAMES = [ 'codex/codex-app-server-grant-entry' ] as const +// Entries executed as worker threads of the main process. Electron's module is +// not registered on worker threads, so require("electron") throws +// "Cannot find module 'electron'" there too (verified on Electron 43) and kills +// the worker at startup. These carry hand-written "must stay electron-free" +// comments, which is convention, not enforcement — and the port-scan worker in +// particular sits one import away from a client module that deliberately does +// require electron. +const WORKER_THREAD_ENTRY_NAMES = [ + 'stt-worker', + 'warp-theme-parser-worker', + 'session-scanner-opencode-sqlite-worker-entry', + 'main-thread-hang-watchdog-entry', + 'port-scan-command-worker-entry' +] as const + +type EntryRuntime = 'plain-Node process' | 'worker thread' + const ELECTRON_REQUIRE_RE = /require\(\s*["']electron["']\s*\)/ function collectReachableChunks( @@ -56,13 +73,14 @@ function collectReachableChunks( function assertNoElectronRequire( entryName: string, entry: OutputChunk, - byFileName: Map + byFileName: Map, + runtime: EntryRuntime = 'plain-Node process' ): void { for (const chunk of collectReachableChunks(entry, byFileName)) { if (ELECTRON_REQUIRE_RE.test(chunk.code)) { throw new Error( `[plain-node-entry-guard] "${entryName}" reaches chunk "${chunk.fileName}" that ` + - `requires electron. "${entryName}" runs as a plain-Node process, where ` + + `requires electron. "${entryName}" runs as a ${runtime}, where ` + `require("electron") throws MODULE_NOT_FOUND and kills it at startup (the ` + `v1.4.129-rc.1 daemon outage). Keep electron imports out of its module graph.` ) @@ -125,7 +143,14 @@ export function createPlainNodeEntryGuardPlugin(): Plugin { for (const entryName of PLAIN_NODE_ENTRY_NAMES) { const entry = entryByName.get(entryName) if (entry) { - assertNoElectronRequire(entryName, entry, byFileName) + assertNoElectronRequire(entryName, entry, byFileName, 'plain-Node process') + } + } + + for (const entryName of WORKER_THREAD_ENTRY_NAMES) { + const entry = entryByName.get(entryName) + if (entry) { + assertNoElectronRequire(entryName, entry, byFileName, 'worker thread') } } diff --git a/config/knip.json b/config/knip.json index a48793b79..cb929266f 100644 --- a/config/knip.json +++ b/config/knip.json @@ -10,6 +10,7 @@ "src/main/speech/stt-worker.ts", "src/main/warp-themes/warp-theme-parser-worker.ts", "src/main/ai-vault/session-scanner-opencode-sqlite-worker-entry.ts", + "src/main/ports/port-scan-command-worker-entry.ts", "src/main/ipc/parcel-watcher-process-entry.ts", "src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts", "src/main/codex/codex-app-server-grant-entry.ts", diff --git a/config/scripts/build-windows-cli-launcher.test.mjs b/config/scripts/build-windows-cli-launcher.test.mjs index 6e4803ed7..4bb219a56 100644 --- a/config/scripts/build-windows-cli-launcher.test.mjs +++ b/config/scripts/build-windows-cli-launcher.test.mjs @@ -14,6 +14,21 @@ import { describe, expect, it } from 'vitest' const itCrossHost = process.platform === 'win32' ? it.skip : it const projectRoot = resolve(import.meta.dirname, '../..') +const WINDOWS_LOCK_CODES = ['EBUSY', 'ENOTEMPTY', 'EPERM'] + +// Why: Windows releases the image handle on a just-executed exe (and finishes the +// AV scan of the freshly compiled one) after the process exits, so tearing down the +// fixture races those locks. Retry, then leave the temp tree rather than reporting a +// teardown lock as a launcher failure. +function removeFixtureTree(path) { + try { + rmSync(path, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }) + } catch (error) { + if (process.platform !== 'win32' || !WINDOWS_LOCK_CODES.includes(error?.code)) { + throw error + } + } +} // Why: cold csc.exe startup exceeds Vitest's 5s unit budget on hosted Windows; // keep the larger allowance scoped to the real compiler integration test. function itWindows(name, test) { @@ -35,7 +50,7 @@ describe('Windows CLI launcher', () => { expect(result.stderr).toContain('Windows CLI launcher') expect(result.stderr).toContain('Windows host') } finally { - rmSync(outputRoot, { recursive: true, force: true }) + removeFixtureTree(outputRoot) } }) @@ -108,7 +123,7 @@ describe('Windows CLI launcher', () => { orcaNodeOptions: '--no-warnings' }) } finally { - rmSync(appRoot, { recursive: true, force: true }) + removeFixtureTree(appRoot) } }) @@ -161,7 +176,7 @@ describe('Windows CLI launcher', () => { pathKeys: ['PATH', 'Path'] }) } finally { - rmSync(appRoot, { recursive: true, force: true }) + removeFixtureTree(appRoot) } }) }) diff --git a/config/scripts/plain-node-entry-guard.test.ts b/config/scripts/plain-node-entry-guard.test.ts index a7583e1d4..b9601a67d 100644 --- a/config/scripts/plain-node-entry-guard.test.ts +++ b/config/scripts/plain-node-entry-guard.test.ts @@ -85,3 +85,91 @@ describe('plain Node entry guard', () => { ) }) }) + +// Why (#11161): Electron's module is not registered on worker threads either — +// require("electron") throws "Cannot find module 'electron'" inside a +// main-process worker and kills it at startup. The worker entries carried only +// hand-written "must stay electron-free" comments, and the port-scan worker sits +// one import away from a client that deliberately does require electron. +describe('worker thread entry guard', () => { + function runWorkerWriteBundle(plugin: Plugin, bundle: Rollup.OutputBundle): void { + const hook = plugin.writeBundle + if (typeof hook !== 'function') { + throw new Error('Expected writeBundle hook') + } + hook.call( + { meta: { watchMode: false } } as never, + { dir: createOutputDir() } as Rollup.NormalizedOutputOptions, + bundle + ) + } + + function workerChunk(name: string, code: string, imports: string[] = []): Rollup.OutputChunk { + return { + type: 'chunk', + code, + dynamicImports: [], + fileName: `${name}.js`, + imports, + isEntry: true, + name + } as Rollup.OutputChunk + } + + it('rejects an Electron require reachable from a worker entry', () => { + const plugin = createPlainNodeEntryGuardPlugin() + const bundle = { + 'port-scan-command-worker-entry.js': workerChunk( + 'port-scan-command-worker-entry', + 'require("electron")' + ) + } as Rollup.OutputBundle + + expect(() => runWorkerWriteBundle(plugin, bundle)).toThrow('requires electron') + }) + + it('names the worker-thread runtime so the failure is actionable', () => { + const plugin = createPlainNodeEntryGuardPlugin() + const bundle = { + 'stt-worker.js': workerChunk('stt-worker', 'require("electron")') + } as Rollup.OutputBundle + + expect(() => runWorkerWriteBundle(plugin, bundle)).toThrow('runs as a worker thread') + }) + + // The real risk is transitive: a worker entry importing a shared chunk that + // reaches the electron-requiring client, not a direct import anyone would spot. + it('follows shared chunks out of a worker entry', () => { + const plugin = createPlainNodeEntryGuardPlugin() + const bundle = { + 'session-scanner-opencode-sqlite-worker-entry.js': workerChunk( + 'session-scanner-opencode-sqlite-worker-entry', + 'require("./chunks/shared.js")', + ['chunks/shared.js'] + ), + 'chunks/shared.js': { + type: 'chunk', + code: 'require("electron")', + dynamicImports: [], + fileName: 'chunks/shared.js', + imports: [], + isEntry: false, + name: 'shared' + } as Rollup.OutputChunk + } as Rollup.OutputBundle + + expect(() => runWorkerWriteBundle(plugin, bundle)).toThrow('chunks/shared.js') + }) + + it('passes a clean worker entry', () => { + const plugin = createPlainNodeEntryGuardPlugin() + const bundle = { + 'warp-theme-parser-worker.js': workerChunk( + 'warp-theme-parser-worker', + 'require("node:worker_threads")' + ) + } as Rollup.OutputBundle + + expect(() => runWorkerWriteBundle(plugin, bundle)).not.toThrow() + }) +}) diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 39f923cb2..a6e656ac9 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -216,6 +216,11 @@ export const electronViteConfig: UserConfig = { 'session-scanner-opencode-sqlite-worker-entry': resolve( 'src/main/ai-vault/session-scanner-opencode-sqlite-worker-entry.ts' ), + // Why: libuv spawns processes inline on the calling loop, so the port + // scan's probe commands run on a worker thread instead of the UI one. + 'port-scan-command-worker-entry': resolve( + 'src/main/ports/port-scan-command-worker-entry.ts' + ), // Why: forked with ELECTRON_RUN_AS_NODE so @parcel/watcher faults // can't take down the main process (issue #7547). 'parcel-watcher-process-entry': resolve('src/main/ipc/parcel-watcher-process-entry.ts'), diff --git a/src/main/ipc/localhost-worktree-labels.ts b/src/main/ipc/localhost-worktree-labels.ts index 382072f6c..77c3c3d65 100644 --- a/src/main/ipc/localhost-worktree-labels.ts +++ b/src/main/ipc/localhost-worktree-labels.ts @@ -42,7 +42,11 @@ async function assertAllowedTarget(store: Store, targetUrl: string): Promise { if (String(port.port) !== targetPort) { return false diff --git a/src/main/ipc/workspace-ports.test.ts b/src/main/ipc/workspace-ports.test.ts index 841c04609..f5afe0362 100644 --- a/src/main/ipc/workspace-ports.test.ts +++ b/src/main/ipc/workspace-ports.test.ts @@ -102,14 +102,18 @@ describe('registerWorkspacePortHandlers', () => { worktrees: [{ id: 'attacker', path: '/tmp/not-authorized', repoId: 'local-repo' }] }) - expect(scanWorkspacePortsMock).toHaveBeenCalledWith([ - { - id: 'local-repo::/workspace/repo', - repoId: 'local-repo', - displayName: 'Primary', - path: '/workspace/repo' - } - ]) + expect(scanWorkspacePortsMock).toHaveBeenCalledWith( + [ + { + id: 'local-repo::/workspace/repo', + repoId: 'local-repo', + displayName: 'Primary', + path: '/workspace/repo' + } + ], + undefined, + undefined + ) }) it('deduplicates concurrent scans for the same store-derived probe set', async () => { @@ -139,20 +143,24 @@ describe('registerWorkspacePortHandlers', () => { await handlers.get('workspacePorts:scan')?.(null, undefined) - expect(scanWorkspacePortsMock).toHaveBeenCalledWith([ - { - id: 'local-repo::/workspace/repo', - repoId: 'local-repo', - displayName: 'Primary', - path: '/workspace/repo' - }, - { - id: 'other-repo::/workspace/other', - repoId: 'other-repo', - displayName: 'Other', - path: '/workspace/other' - } - ]) + expect(scanWorkspacePortsMock).toHaveBeenCalledWith( + [ + { + id: 'local-repo::/workspace/repo', + repoId: 'local-repo', + displayName: 'Primary', + path: '/workspace/repo' + }, + { + id: 'other-repo::/workspace/other', + repoId: 'other-repo', + displayName: 'Other', + path: '/workspace/other' + } + ], + undefined, + undefined + ) }) it('stops a process only after the current scan proves the pid owns a workspace port', async () => { diff --git a/src/main/ports/local-workspace-port-scanner.test.ts b/src/main/ports/local-workspace-port-scanner.test.ts index a6eeb21e5..132eba195 100644 --- a/src/main/ports/local-workspace-port-scanner.test.ts +++ b/src/main/ports/local-workspace-port-scanner.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi, type Mock } from 'vitest' import path from 'node:path' import { attributePortToWorkspace, @@ -9,13 +9,21 @@ import { resetWorkspacePortScanTimeoutBackoffForTests, scanWorkspacePorts } from './local-workspace-port-scanner' +import { PortScanCommandTimeoutError } from './port-scan-command-protocol' -const execFileMock = vi.hoisted(() => vi.fn()) +const runPortScanCommandMock = vi.hoisted(() => vi.fn()) -vi.mock('child_process', () => ({ - execFile: execFileMock +vi.mock('./port-scan-command-client', () => ({ + runPortScanCommand: runPortScanCommandMock, + isPortScanWorkerUnavailableError: () => false })) +const LSOF_LISTEN_OUTPUT = ['p123', 'cnode', 'n127.0.0.1:5173'].join('\n') + +function urlWatcherStub(): { lookup: () => undefined; reconcileScan: Mock } { + return { lookup: () => undefined, reconcileScan: vi.fn() } +} + const worktrees = [ { id: 'repo::/repo', @@ -177,52 +185,42 @@ describe('scanWorkspacePorts attribution work', () => { afterEach(() => { resetWorkspacePortScanTimeoutBackoffForTests() vi.restoreAllMocks() - execFileMock.mockReset() + runPortScanCommandMock.mockReset() }) it('normalizes worktree paths once per scan instead of once per port phase', async () => { vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') const win32ResolveSpy = vi.spyOn(path.win32, 'resolve') const posixResolveSpy = vi.spyOn(path.posix, 'resolve') - const invokeCallback = (callback: unknown, stdout: string): void => { - if (typeof callback !== 'function') { - throw new Error('missing execFile callback') - } - const execCallback = callback as (error: Error | null, stdout: string) => void - execCallback(null, stdout) - } - execFileMock.mockImplementation( - (command: string, args: string[], _options: unknown, callback: unknown) => { - if (command === 'lsof' && args.includes('-iTCP')) { - invokeCallback( - callback, - ['p123', 'cnode', 'n127.0.0.1:3000', 'p124', 'cnode', 'n127.0.0.1:3001'].join('\n') - ) - } else if (command === 'lsof') { - invokeCallback( - callback, - ['p123', 'n/repo/service', 'p124', 'n/repo/worktrees/feature/app'].join('\n') - ) - } else if (command === 'ps') { - invokeCallback( - callback, - [ - '123 node /repo/service/server.js', - '124 node /repo/worktrees/feature/app/server.js' - ].join('\n') - ) - } else { - invokeCallback(callback, '') + runPortScanCommandMock.mockImplementation(async (command: string, args: string[]) => { + if (command === 'lsof' && args.includes('-iTCP')) { + return { + stdout: ['p123', 'cnode', 'n127.0.0.1:3000', 'p124', 'cnode', 'n127.0.0.1:3001'].join( + '\n' + ), + spawnMs: 5 } - return { kill: vi.fn() } } - ) - - const scan = await scanWorkspacePorts(worktrees, { - lookup: () => undefined, - reconcileScan: vi.fn() + if (command === 'lsof') { + return { + stdout: ['p123', 'n/repo/service', 'p124', 'n/repo/worktrees/feature/app'].join('\n'), + spawnMs: 5 + } + } + if (command === 'ps') { + return { + stdout: [ + '123 node /repo/service/server.js', + '124 node /repo/worktrees/feature/app/server.js' + ].join('\n'), + spawnMs: 5 + } + } + return { stdout: '', spawnMs: 5 } }) + const scan = await scanWorkspacePorts(worktrees, urlWatcherStub()) + expect(scan.ports.filter((port) => port.kind === 'workspace')).toHaveLength(2) const win32WorktreePathResolveCalls = win32ResolveSpy.mock.calls.filter( ([input]) => input === '/repo' || input === '/repo/worktrees/feature' @@ -240,62 +238,39 @@ describe('scanWorkspacePorts command timeout', () => { vi.useRealTimers() resetWorkspacePortScanTimeoutBackoffForTests() vi.restoreAllMocks() - execFileMock.mockReset() + runPortScanCommandMock.mockReset() }) it('returns an unavailable scan when lsof never reports completion', async () => { - vi.useFakeTimers() vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') - const killMock = vi.fn() - execFileMock.mockImplementation(() => ({ kill: killMock })) + runPortScanCommandMock.mockRejectedValue( + new PortScanCommandTimeoutError('lsof timed out after 4000ms') + ) - let settled = false - const scanPromise = scanWorkspacePorts([], { - lookup: () => undefined, - reconcileScan: vi.fn() - }).then((scan) => { - settled = true - return scan - }) - - await vi.advanceTimersByTimeAsync(4_000) - - expect(settled).toBe(true) - await expect(scanPromise).resolves.toMatchObject({ + await expect(scanWorkspacePorts([], urlWatcherStub())).resolves.toMatchObject({ platform: 'darwin', ports: [], unavailableReason: 'Port scanning is unavailable on darwin.' }) - expect(killMock).toHaveBeenCalled() }) it('backs off after a command timeout instead of launching lsof on every scan tick', async () => { vi.useFakeTimers() vi.setSystemTime(1_000) vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') - const killMock = vi.fn() - execFileMock.mockImplementation(() => ({ kill: killMock })) + runPortScanCommandMock.mockRejectedValue( + new PortScanCommandTimeoutError('lsof timed out after 4000ms') + ) - const firstScanPromise = scanWorkspacePorts([], { - lookup: () => undefined, - reconcileScan: vi.fn() - }) - - await vi.advanceTimersByTimeAsync(4_000) - await expect(firstScanPromise).resolves.toMatchObject({ + await expect(scanWorkspacePorts([], urlWatcherStub())).resolves.toMatchObject({ platform: 'darwin', ports: [], unavailableReason: 'Port scanning is unavailable on darwin.' }) - expect(execFileMock).toHaveBeenCalledTimes(1) + expect(runPortScanCommandMock).toHaveBeenCalledTimes(1) const cooldownScans = await Promise.all( - Array.from({ length: 10 }, () => - scanWorkspacePorts([], { - lookup: () => undefined, - reconcileScan: vi.fn() - }) - ) + Array.from({ length: 10 }, () => scanWorkspacePorts([], urlWatcherStub())) ) expect(cooldownScans).toHaveLength(10) @@ -306,25 +281,172 @@ describe('scanWorkspacePorts command timeout', () => { expect( cooldownScans.every((scan) => scan.unavailableReason?.includes('temporarily paused')) ).toBe(true) - expect(execFileMock).toHaveBeenCalledTimes(1) + expect(runPortScanCommandMock).toHaveBeenCalledTimes(1) vi.setSystemTime(65_001) await vi.advanceTimersByTimeAsync(0) - execFileMock.mockImplementation( - (_command: string, args: string[], _options: unknown, callback: unknown) => { - const execCallback = callback as (error: Error | null, stdout: string) => void - const output = args.includes('-iTCP') ? 'p123\ncnode\nn127.0.0.1:3000' : '' - execCallback(null, output) - return { kill: vi.fn() } - } - ) + runPortScanCommandMock.mockImplementation(async (_command: string, args: string[]) => ({ + stdout: args.includes('-iTCP') ? 'p123\ncnode\nn127.0.0.1:3000' : '', + spawnMs: 5 + })) - const recoveredScan = await scanWorkspacePorts([], { - lookup: () => undefined, - reconcileScan: vi.fn() - }) + const recoveredScan = await scanWorkspacePorts([], urlWatcherStub()) expect(recoveredScan.unavailableReason).toBeUndefined() - expect(execFileMock).toHaveBeenCalledTimes(4) + expect(runPortScanCommandMock).toHaveBeenCalledTimes(4) }) }) + +describe('scanWorkspacePorts with delayed process creation', () => { + afterEach(() => { + resetWorkspacePortScanTimeoutBackoffForTests() + vi.restoreAllMocks() + runPortScanCommandMock.mockReset() + }) + + // Regression for #11161: an endpoint-security hook makes CreateProcessW take + // seconds, so the command's own budget must not be charged for the spawn. + it('does not report a command timeout when only process creation was delayed', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + runPortScanCommandMock.mockResolvedValue({ stdout: LSOF_LISTEN_OUTPUT, spawnMs: 4_200 }) + + const first = await scanWorkspacePorts([], urlWatcherStub()) + const second = await scanWorkspacePorts([], urlWatcherStub()) + + expect(first.unavailableReason).toBeUndefined() + expect(second.unavailableReason).toBeUndefined() + expect(first.ports).toHaveLength(1) + }) + + it('skips the optional metadata commands for one cycle after a stalled spawn', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + runPortScanCommandMock.mockResolvedValue({ stdout: LSOF_LISTEN_OUTPUT, spawnMs: 4_200 }) + + const scan = await scanWorkspacePorts([], urlWatcherStub()) + + expect(runPortScanCommandMock).toHaveBeenCalledTimes(1) + expect(scan.ports).toHaveLength(1) + }) + + // Regression for #11161 review: a metadata-less scan must not be reconciled as + // "the listener vanished" — that evicts advertised URLs only a PTY can restore. + it('does not reconcile advertised URLs for a scan that skipped metadata', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + runPortScanCommandMock.mockResolvedValue({ stdout: LSOF_LISTEN_OUTPUT, spawnMs: 4_200 }) + const watcher = urlWatcherStub() + + await scanWorkspacePorts(worktrees, watcher) + + expect(watcher.reconcileScan).not.toHaveBeenCalled() + }) + + it('re-probes metadata on the scan after a skip instead of degrading forever', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + runPortScanCommandMock.mockImplementation(async (command: string, args: string[]) => { + if (command === 'lsof' && args.includes('-iTCP')) { + return { stdout: LSOF_LISTEN_OUTPUT, spawnMs: 4_200 } + } + if (command === 'lsof') { + return { stdout: ['p123', 'n/repo'].join('\n'), spawnMs: 4_200 } + } + return { stdout: '123 node /repo/server.js', spawnMs: 4_200 } + }) + const watcher = urlWatcherStub() + + const skipped = await scanWorkspacePorts(worktrees, watcher) + const recovered = await scanWorkspacePorts(worktrees, watcher) + + expect(skipped.ports[0]?.kind).toBe('external') + expect(recovered.ports[0]).toMatchObject({ kind: 'workspace' }) + expect(runPortScanCommandMock).toHaveBeenCalledTimes(4) + expect(watcher.reconcileScan).toHaveBeenCalledTimes(worktrees.length) + }) + + it('still collects process metadata when process creation was fast', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + runPortScanCommandMock.mockImplementation(async (_command: string, args: string[]) => ({ + stdout: args.includes('-iTCP') ? LSOF_LISTEN_OUTPUT : '', + spawnMs: 5 + })) + + await scanWorkspacePorts([], urlWatcherStub()) + + expect(runPortScanCommandMock).toHaveBeenCalledTimes(3) + }) + + // Regression for #11161 review: the skip parity is driven by the 30s poller, + // so a one-shot user action would otherwise land on a random parity. + it('keeps probing metadata for callers that require attribution', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + mockStalledDarwinScan() + + const scan = await scanWorkspacePorts(worktrees, urlWatcherStub(), { requireMetadata: true }) + + expect(scan.ports[0]).toMatchObject({ kind: 'workspace' }) + expect(runPortScanCommandMock).toHaveBeenCalledTimes(3) + }) + + it('does not let a required-metadata scan reset the background skip parity', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + mockStalledDarwinScan() + + await scanWorkspacePorts(worktrees, urlWatcherStub()) + await scanWorkspacePorts(worktrees, urlWatcherStub(), { requireMetadata: true }) + await scanWorkspacePorts(worktrees, urlWatcherStub()) + + // 1 skipped + 3 required + 3 recovered; a reset parity would skip twice. + expect(runPortScanCommandMock).toHaveBeenCalledTimes(7) + }) + + // Regression for #11161 review: without carry-forward the panel moves every + // workspace port into External on each skipped cycle. + it('carries the previous cycle attribution through a skipped scan', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + let listenSpawnMs = 5 + runPortScanCommandMock.mockImplementation(async (command: string, args: string[]) => { + if (command === 'lsof' && args.includes('-iTCP')) { + return { stdout: LSOF_LISTEN_OUTPUT, spawnMs: listenSpawnMs } + } + return { stdout: command === 'lsof' ? ['p123', 'n/repo'].join('\n') : '', spawnMs: 5 } + }) + + const full = await scanWorkspacePorts(worktrees, urlWatcherStub()) + listenSpawnMs = 4_200 + const skipped = await scanWorkspacePorts(worktrees, urlWatcherStub()) + + expect(full.ports[0]).toMatchObject({ kind: 'workspace' }) + expect(skipped.ports[0]).toMatchObject({ kind: 'workspace', processName: 'node' }) + }) + + it('does not hand carried-forward metadata to a different listener', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + let listenOutput = LSOF_LISTEN_OUTPUT + let listenSpawnMs = 5 + runPortScanCommandMock.mockImplementation(async (command: string, args: string[]) => { + if (command === 'lsof' && args.includes('-iTCP')) { + return { stdout: listenOutput, spawnMs: listenSpawnMs } + } + return { stdout: command === 'lsof' ? ['p123', 'n/repo'].join('\n') : '', spawnMs: 5 } + }) + + await scanWorkspacePorts(worktrees, urlWatcherStub()) + listenOutput = ['p123', 'cnode', 'n127.0.0.1:9999'].join('\n') + listenSpawnMs = 4_200 + const skipped = await scanWorkspacePorts(worktrees, urlWatcherStub()) + + expect(skipped.ports[0]?.kind).toBe('external') + }) +}) + +/** Darwin scan where every spawn stalls past the metadata-skip threshold. */ +function mockStalledDarwinScan(): void { + runPortScanCommandMock.mockImplementation(async (command: string, args: string[]) => { + if (command === 'lsof' && args.includes('-iTCP')) { + return { stdout: LSOF_LISTEN_OUTPUT, spawnMs: 4_200 } + } + if (command === 'lsof') { + return { stdout: ['p123', 'n/repo'].join('\n'), spawnMs: 4_200 } + } + return { stdout: '123 node /repo/server.js', spawnMs: 4_200 } + }) +} diff --git a/src/main/ports/local-workspace-port-scanner.ts b/src/main/ports/local-workspace-port-scanner.ts index 1d8f4dbf8..ce2b7b768 100644 --- a/src/main/ports/local-workspace-port-scanner.ts +++ b/src/main/ports/local-workspace-port-scanner.ts @@ -1,6 +1,5 @@ /* eslint-disable max-lines -- Why: the platform-specific scan paths share parsing, attribution, and normalization rules that must stay in lockstep. */ -import { execFile } from 'node:child_process' import { readFile, readdir, readlink } from 'node:fs/promises' import path from 'node:path' import type { @@ -11,14 +10,36 @@ import type { } from '../../shared/workspace-ports' import { getProcessOutputFields } from '../../shared/process-output-field-scanner' import { advertisedUrlWatcher, type AdvertisedUrlWatcher } from './advertised-url-watcher' +import { isPortScanWorkerUnavailableError, runPortScanCommand } from './port-scan-command-client' +import { PortScanCommandTimeoutError } from './port-scan-command-protocol' import { WorkspacePortScanTimeoutBackoff } from './workspace-port-scan-timeout-backoff' -const COMMAND_TIMEOUT_MS = 4_000 +// Why (#11161): on an EDR-hooked host process creation alone can take seconds. +// Past this, skip the scan's optional metadata commands for one cycle so a scan +// costs roughly one stall instead of three. +const SLOW_SPAWN_SKIP_METADATA_MS = 2_000 const MAX_PORTS = 200 const HTTP_PORTS = new Set([80, 3000, 3001, 4200, 5000, 5173, 5174, 8000, 8080, 8888]) const HTTPS_PORTS = new Set([443, 8443]) const commandTimeoutBackoff = new WorkspacePortScanTimeoutBackoff() +let loggedWorkerUnavailable = false +// Why (#11161): a hooked host stalls every spawn, so gating only on the current +// scan would drop metadata forever. Never skip twice running, so attribution — +// and the Stop action and advertised-URL matching that ride on it — recovers on +// the next tick. +let skippedMetadataOnLastScan = false +// Why (#11161): a skipped cycle would otherwise report every listener as +// external, so the panel flip-flops and the Stop action loses its owner. Carry +// the previous cycle's metadata forward, keyed tightly enough that a recycled +// pid cannot inherit it. +let lastListenerMetadata = new Map() + +export type WorkspacePortScanOptions = { + /** Set by attribution-dependent callers (Stop, the localhost-label allowlist) + * that must never trade owner metadata for scan latency. */ + requireMetadata?: boolean +} type RawListeningPort = { host: string @@ -40,9 +61,16 @@ type NormalizedWorkspacePortProbe = { normalizedPath: string } +type PlatformListeningPortScan = { + ports: RawListeningPort[] + /** False when a stalled spawn made the scan skip its cwd/command-line probes. */ + metadataAvailable: boolean +} + export async function scanWorkspacePorts( worktrees: WorkspacePortProbe[], - urlWatcher: Pick = advertisedUrlWatcher + urlWatcher: Pick = advertisedUrlWatcher, + options: WorkspacePortScanOptions = {} ): Promise { const cooldown = commandTimeoutBackoff.snapshot() if (cooldown.isCoolingDown) { @@ -54,10 +82,15 @@ export async function scanWorkspacePorts( } try { - const rawPorts = await scanPlatformListeningPorts() + const { ports: rawPorts, metadataAvailable } = await scanPlatformListeningPorts(options) commandTimeoutBackoff.recordSuccess() const normalizedWorktrees = normalizeWorkspacePortProbes(worktrees) - reconcileAdvertisedUrls(rawPorts, normalizedWorktrees, urlWatcher) + // Why (#11161): without cwd/command-line every port looks unattributed, and + // reconciling that would read as "the listener vanished" and evict cached + // advertised URLs that only live PTY output can ever restore. + if (metadataAvailable) { + reconcileAdvertisedUrls(rawPorts, normalizedWorktrees, urlWatcher) + } const ports = rawPorts .map((port) => enrichPort(port, normalizedWorktrees, urlWatcher)) .sort(compareWorkspacePorts) @@ -67,13 +100,66 @@ export async function scanWorkspacePorts( if (isCommandTimeoutError(error)) { commandTimeoutBackoff.recordTimeout() } - console.warn('[workspace-ports] scan failed', error) + warnScanFailure(error) return makeUnavailableScan(`Port scanning is unavailable on ${process.platform}.`) } } export function resetWorkspacePortScanTimeoutBackoffForTests(): void { commandTimeoutBackoff.reset() + loggedWorkerUnavailable = false + skippedMetadataOnLastScan = false + lastListenerMetadata = new Map() +} + +function shouldSkipMetadataCommands(spawnMs: number, opts: WorkspacePortScanOptions): boolean { + if (opts.requireMetadata) { + // Leave the skip parity alone: it belongs to the background scan cadence, + // and a one-shot user action must not shift which tick degrades. + return false + } + const skip = spawnMs > SLOW_SPAWN_SKIP_METADATA_MS && !skippedMetadataOnLastScan + skippedMetadataOnLastScan = skip + return skip +} + +/** Identity tight enough that a recycled pid cannot inherit stale metadata. */ +function listenerMetadataKey(port: RawListeningPort): string { + return `${port.pid ?? 'unknown'}:${port.host}:${port.port}` +} + +function rememberListenerMetadata(ports: readonly RawListeningPort[]): void { + lastListenerMetadata = new Map( + ports.map((port) => [ + listenerMetadataKey(port), + { processName: port.processName, commandLine: port.commandLine, cwd: port.cwd } + ]) + ) +} + +function recallListenerMetadata(port: RawListeningPort): RawListeningPort { + const remembered = lastListenerMetadata.get(listenerMetadataKey(port)) + if (!remembered) { + return port + } + return { + ...port, + processName: port.processName ?? remembered.processName, + commandLine: port.commandLine ?? remembered.commandLine, + cwd: port.cwd ?? remembered.cwd + } +} + +// Why: a mispackaged probe worker fails identically forever, so logging it on +// every 30s scan tick is pure noise. +function warnScanFailure(error: unknown): void { + if (isPortScanWorkerUnavailableError(error)) { + if (loggedWorkerUnavailable) { + return + } + loggedWorkerUnavailable = true + } + console.warn('[workspace-ports] scan failed', error) } function makeUnavailableScan(reason: string): WorkspacePortScanResult { @@ -194,38 +280,73 @@ export function parseProcNetTcp(content: string): { host: string; port: number; return results } -async function scanPlatformListeningPorts(): Promise { +async function scanPlatformListeningPorts( + options: WorkspacePortScanOptions +): Promise { + const scan = await dispatchPlatformListeningPortScan(options) + if (scan.metadataAvailable) { + rememberListenerMetadata(scan.ports) + return scan + } + return { ...scan, ports: scan.ports.map(recallListenerMetadata) } +} + +async function dispatchPlatformListeningPortScan( + options: WorkspacePortScanOptions +): Promise { if (process.platform === 'linux') { return scanLinuxProcPorts() } if (process.platform === 'darwin') { - return scanDarwinLsofPorts() + return scanDarwinLsofPorts(options) } if (process.platform === 'win32') { - return scanWindowsNetstatPorts() + return scanWindowsNetstatPorts(options) } throw new Error(`Port scanning is not supported on ${process.platform}`) } -async function scanDarwinLsofPorts(): Promise { - const { stdout } = await runCommand('lsof', ['-nP', '-iTCP', '-sTCP:LISTEN', '-F', 'pcn']) +async function scanDarwinLsofPorts( + options: WorkspacePortScanOptions +): Promise { + const { stdout, spawnMs } = await runPortScanCommand('lsof', [ + '-nP', + '-iTCP', + '-sTCP:LISTEN', + '-F', + 'pcn' + ]) const ports = parseLsofListeningOutput(stdout) + if (shouldSkipMetadataCommands(spawnMs, options)) { + return { ports, metadataAvailable: false } + } const metadata = await loadDarwinProcessMetadata( new Set(ports.flatMap((p) => (p.pid ? [p.pid] : []))) ) - return ports.map((port) => ({ ...metadata.get(port.pid ?? -1), ...port })) + return { + ports: ports.map((port) => ({ ...metadata.get(port.pid ?? -1), ...port })), + metadataAvailable: true + } } -async function scanWindowsNetstatPorts(): Promise { - const { stdout } = await runCommand('netstat', ['-ano', '-p', 'tcp']) +async function scanWindowsNetstatPorts( + options: WorkspacePortScanOptions +): Promise { + const { stdout, spawnMs } = await runPortScanCommand('netstat', ['-ano', '-p', 'tcp']) const ports = parseNetstatListeningOutput(stdout) + if (shouldSkipMetadataCommands(spawnMs, options)) { + return { ports, metadataAvailable: false } + } const metadata = await loadWindowsProcessMetadata( new Set(ports.flatMap((p) => (p.pid ? [p.pid] : []))) ) - return ports.map((port) => ({ ...metadata.get(port.pid ?? -1), ...port })) + return { + ports: ports.map((port) => ({ ...metadata.get(port.pid ?? -1), ...port })), + metadataAvailable: true + } } -async function scanLinuxProcPorts(): Promise { +async function scanLinuxProcPorts(): Promise { const [tcp4, tcp6] = await Promise.all([ readProcNet('/proc/net/tcp'), readProcNet('/proc/net/tcp6') @@ -248,7 +369,7 @@ async function scanLinuxProcPorts(): Promise { }) } - return dedupeRawPorts(rawPorts) + return { ports: dedupeRawPorts(rawPorts), metadataAvailable: true } } async function readProcNet( @@ -321,10 +442,24 @@ async function loadDarwinProcessMetadata(pids: Set): Promise null), - runCommand('ps', ['-p', pidList, '-o', 'pid=', '-o', 'command=']).catch(() => null) - ]) + // Why (#11161): sequential, not Promise.all — the probe worker dispatches one + // command at a time, so issuing both at once would only queue the second. + const cwdOutput = await runPortScanCommand('lsof', [ + '-a', + '-p', + pidList, + '-d', + 'cwd', + '-Fn' + ]).catch(() => null) + const commandOutput = await runPortScanCommand('ps', [ + '-p', + pidList, + '-o', + 'pid=', + '-o', + 'command=' + ]).catch(() => null) let currentPid: number | null = null for (const line of cwdOutput?.stdout.split('\n') ?? []) { @@ -360,7 +495,7 @@ async function loadWindowsProcessMetadata( .filter(Number.isFinite) .map((pid) => `ProcessId=${pid}`) .join(' OR ') - const { stdout } = await runCommand('powershell.exe', [ + const { stdout } = await runPortScanCommand('powershell.exe', [ '-NoProfile', '-Command', `Get-CimInstance Win32_Process -Filter "${pidFilter}" | Select-Object ProcessId,Name,CommandLine | ConvertTo-Json -Compress` @@ -382,62 +517,8 @@ async function loadWindowsProcessMetadata( return result } -async function runCommand(command: string, args: string[]): Promise<{ stdout: string }> { - return await new Promise((resolve, reject) => { - let settled = false - let child: ReturnType | undefined - const timer = setTimeout(() => { - if (settled) { - return - } - settled = true - child?.kill() - reject(new CommandTimeoutError(command, COMMAND_TIMEOUT_MS)) - }, COMMAND_TIMEOUT_MS) - - const settle = (callback: () => void): void => { - if (settled) { - return - } - settled = true - clearTimeout(timer) - callback() - } - - // Why: Node's execFile timeout only signals the child; if the callback - // never arrives, the workspace port scan would otherwise hang forever. - try { - child = execFile( - command, - args, - { - timeout: COMMAND_TIMEOUT_MS, - maxBuffer: 2 * 1024 * 1024, - windowsHide: true - }, - (error, stdout) => { - if (error) { - settle(() => reject(error)) - return - } - settle(() => resolve({ stdout: String(stdout) })) - } - ) - } catch (error) { - settle(() => reject(error)) - } - }) -} - -class CommandTimeoutError extends Error { - constructor(command: string, timeoutMs: number) { - super(`${command} timed out after ${timeoutMs}ms`) - this.name = 'CommandTimeoutError' - } -} - function isCommandTimeoutError(error: unknown): boolean { - return error instanceof CommandTimeoutError + return error instanceof PortScanCommandTimeoutError } async function readTextIfAvailable(filePath: string): Promise { diff --git a/src/main/ports/port-scan-command-client.test.ts b/src/main/ports/port-scan-command-client.test.ts new file mode 100644 index 000000000..d1a15d297 --- /dev/null +++ b/src/main/ports/port-scan-command-client.test.ts @@ -0,0 +1,300 @@ +import { readFileSync } from 'node:fs' +import { join, sep } from 'node:path' +import { Worker } from 'node:worker_threads' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + CALL_DEADLINE_MS, + MAX_QUEUED_CALLS, + PortScanCommandClient, + isPortScanWorkerUnavailableError, + resolveWorkerEntryPath +} from './port-scan-command-client' +import { + PortScanCommandTimeoutError, + type PortScanCommandRequest +} from './port-scan-command-protocol' + +type PortScanCommandResponseBody = + | { ok: true; stdout: string; spawnMs: number } + | { ok: false; timedOut: boolean; error: string } + +const execFileMock = vi.hoisted(() => vi.fn()) + +vi.mock('child_process', () => ({ + execFile: execFileMock +})) + +// A worker_threads stand-in the tests drive directly: it records posted requests +// and lets a test emit message/error/exit without a built worker bundle. +class FakeWorker { + postedRequests: PortScanCommandRequest[] = [] + terminated = false + private listeners = new Map void>>() + + on(event: string, listener: (arg?: unknown) => void): this { + const set = this.listeners.get(event) ?? new Set() + set.add(listener) + this.listeners.set(event, set) + return this + } + + off(event: string, listener: (arg?: unknown) => void): this { + this.listeners.get(event)?.delete(listener) + return this + } + + removeAllListeners(): void { + this.listeners.clear() + } + + unref(): void {} + + async terminate(): Promise { + this.terminated = true + return 1 + } + + postMessage(request: PortScanCommandRequest): void { + this.postedRequests.push(request) + } + + emit(event: string, arg?: unknown): void { + // Copy first: the client removes its listeners synchronously during a fault. + for (const listener of Array.from(this.listeners.get(event) ?? [])) { + listener(arg) + } + } + + respond(body: PortScanCommandResponseBody): void { + const last = this.postedRequests.at(-1) + if (!last) { + throw new Error('no request posted to fake worker') + } + this.emit('message', { id: last.id, ...body }) + } +} + +function makeFactory(workers: FakeWorker[]): () => Worker { + return () => { + const worker = new FakeWorker() + workers.push(worker) + return worker as unknown as Worker + } +} + +function makeClient(workers: FakeWorker[]): PortScanCommandClient { + return new PortScanCommandClient({ workerFactory: makeFactory(workers), log() {} }) +} + +describe('PortScanCommandClient', () => { + afterEach(() => { + vi.useRealTimers() + execFileMock.mockReset() + }) + + it('dispatches one command at a time so a stalled spawn cannot fan out', async () => { + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const first = client.run('lsof', ['-nP']) + const second = client.run('ps', ['-p', '1']) + await Promise.resolve() + + // Why (#11161): a second in-flight request would have its deadline armed + // while the worker's loop is still blocked inside the first uv_spawn. + expect(workers[0].postedRequests.map((request) => request.command)).toEqual(['lsof']) + + workers[0].respond({ ok: true, stdout: 'lsof-out', spawnMs: 12 }) + await expect(first).resolves.toEqual({ stdout: 'lsof-out', spawnMs: 12 }) + expect(workers[0].postedRequests.map((request) => request.command)).toEqual(['lsof', 'ps']) + + workers[0].respond({ ok: true, stdout: 'ps-out', spawnMs: 9 }) + await expect(second).resolves.toEqual({ stdout: 'ps-out', spawnMs: 9 }) + }) + + it('ignores responses that do not correlate with the active request', async () => { + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const pending = client.run('lsof', []) + await Promise.resolve() + workers[0].emit('message', { id: 999, ok: true, stdout: 'stale', spawnMs: 1 }) + workers[0].respond({ ok: true, stdout: 'fresh', spawnMs: 3 }) + + await expect(pending).resolves.toMatchObject({ stdout: 'fresh' }) + }) + + it('rehydrates a worker-side command timeout so the scan can back off', async () => { + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const pending = client.run('lsof', []) + await Promise.resolve() + workers[0].respond({ ok: false, timedOut: true, error: 'lsof timed out after 4000ms' }) + + await expect(pending).rejects.toBeInstanceOf(PortScanCommandTimeoutError) + }) + + it('reports a worker crash as a non-timeout error and respawns for the next call', async () => { + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const pending = client.run('lsof', []) + await Promise.resolve() + workers[0].emit('error', new Error('worker blew up')) + + const error = await pending.catch((err: unknown) => err) + expect(error).toBeInstanceOf(Error) + expect(error).not.toBeInstanceOf(PortScanCommandTimeoutError) + expect(workers[0].terminated).toBe(true) + + const next = client.run('ps', []) + await Promise.resolve() + expect(workers).toHaveLength(2) + workers[1].respond({ ok: true, stdout: 'ps-out', spawnMs: 2 }) + await expect(next).resolves.toMatchObject({ stdout: 'ps-out' }) + }) + + it('terminates a silent worker at the call deadline without arming the backoff', async () => { + vi.useFakeTimers() + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const pending = client.run('netstat', ['-ano']) + const settled = pending.catch((err: unknown) => err) + await vi.advanceTimersByTimeAsync(CALL_DEADLINE_MS) + + const error = await settled + expect(error).toBeInstanceOf(Error) + expect(error).not.toBeInstanceOf(PortScanCommandTimeoutError) + expect(workers[0].terminated).toBe(true) + }) + + it('rejects overflow instead of growing the queue without bound', async () => { + const workers: FakeWorker[] = [] + const client = makeClient(workers) + + const accepted = Array.from({ length: MAX_QUEUED_CALLS + 1 }, () => client.run('lsof', [])) + const overflow = client.run('lsof', []) + + const error = await overflow.catch((err: unknown) => err) + expect(error).toBeInstanceOf(Error) + expect(error).not.toBeInstanceOf(PortScanCommandTimeoutError) + + for (let i = 0; i < accepted.length; i++) { + workers[0].respond({ ok: true, stdout: 'drained', spawnMs: 1 }) + } + await expect(Promise.all(accepted)).resolves.toHaveLength(MAX_QUEUED_CALLS + 1) + }) + + it('fails closed instead of spawning the command on this thread', async () => { + const client = new PortScanCommandClient({ + workerFactory: () => { + throw new Error('worker entry not found') + }, + log() {} + }) + + const error = await client.run('lsof', []).catch((err: unknown) => err) + + expect(isPortScanWorkerUnavailableError(error)).toBe(true) + expect(execFileMock).not.toHaveBeenCalled() + }) +}) + +// Why: the packaged branch never runs in dev or e2e (both take the __dirname +// path), so it is pinned here at the path-construction level. Whether Electron's +// asar shim can load a Worker entry from inside app.asar is not testable here — +// that still needs a packaged smoke test. +describe('resolveWorkerEntryPath', () => { + const WORKER_ENTRY_FILENAME = 'port-scan-command-worker-entry.js' + + it('resolves a packaged build under resourcesPath/app.asar/out/main', () => { + const resourcesPath = join(sep, 'Applications', 'Orca.app', 'Contents', 'Resources') + + const resolved = resolveWorkerEntryPath({ + isPackaged: true, + resourcesPath, + moduleDir: join(sep, 'unpackaged', 'out', 'main') + }) + + expect(resolved.startsWith(`${resourcesPath}${sep}`)).toBe(true) + expect(resolved.slice(resourcesPath.length + 1).split(sep)).toEqual([ + 'app.asar', + 'out', + 'main', + WORKER_ENTRY_FILENAME + ]) + }) + + it('ignores resourcesPath when the app is not packaged', () => { + const moduleDir = join(sep, 'repo', 'out', 'main') + + const resolved = resolveWorkerEntryPath({ + isPackaged: false, + resourcesPath: join(sep, 'Applications', 'Orca.app', 'Contents', 'Resources'), + moduleDir + }) + + expect(resolved).toBe(join(moduleDir, WORKER_ENTRY_FILENAME)) + expect(resolved).not.toContain('app.asar') + }) + + // A rename in the build config would leave both branches pointing at a file + // that is never emitted, and only the packaged one fails silently. + it('names the entry the main build actually emits', () => { + const config = readFileSync( + join(import.meta.dirname, '..', '..', '..', 'electron.vite.config.ts'), + 'utf8' + ) + + expect(config).toContain("'port-scan-command-worker-entry': resolve(") + }) +}) + +const REAL_WORKER_BLOCK_MS = 800 + +// Mirrors the worker entry's protocol but blocks its own thread first, standing +// in for the endpoint-security hook that makes CreateProcessW take seconds. +const BLOCKING_WORKER_SCRIPT = ` +const { parentPort } = require('node:worker_threads') +const { execFile } = require('node:child_process') +parentPort.on('message', (request) => { + const startedAt = Date.now() + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ${REAL_WORKER_BLOCK_MS}) + execFile(process.execPath, ['-e', ''], () => { + parentPort.postMessage({ + id: request.id, + ok: true, + stdout: 'ok', + spawnMs: Date.now() - startedAt + }) + }) +}) +` + +describe('PortScanCommandClient on a real worker thread', () => { + it('keeps the calling event loop responsive while a spawn stalls', async () => { + const client = new PortScanCommandClient({ + workerFactory: () => new Worker(BLOCKING_WORKER_SCRIPT, { eval: true }), + log() {} + }) + + let last = Date.now() + let maxStallMs = 0 + const probe = setInterval(() => { + const now = Date.now() + maxStallMs = Math.max(maxStallMs, now - last - 10) + last = now + }, 10) + try { + const results = await Promise.all([client.run('lsof', []), client.run('ps', [])]) + + expect(results.map((result) => result.stdout)).toEqual(['ok', 'ok']) + expect(results[0].spawnMs).toBeGreaterThanOrEqual(REAL_WORKER_BLOCK_MS - 100) + expect(maxStallMs).toBeLessThan(400) + } finally { + clearInterval(probe) + } + }, 30_000) +}) diff --git a/src/main/ports/port-scan-command-client.ts b/src/main/ports/port-scan-command-client.ts new file mode 100644 index 000000000..9748a8cfc --- /dev/null +++ b/src/main/ports/port-scan-command-client.ts @@ -0,0 +1,364 @@ +import { existsSync } from 'node:fs' +import { join } from 'node:path' +import { Worker } from 'node:worker_threads' +import { + PORT_SCAN_COMMAND_TIMEOUT_MS, + PortScanCommandTimeoutError, + type PortScanCommandRequest, + type PortScanCommandResponse +} from './port-scan-command-protocol' + +// Why (#11161): a lazily-spawned, unref'd worker runs the port scan's probe +// spawns off the Electron main-process event loop, because libuv performs +// process creation inline on the calling thread. Lifecycle (FIFO one-at-a-time +// dispatch, per-call deadlines, respawn-on-fault, idle teardown, fail-closed) +// mirrors src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.ts; +// the duplicated ~150 lines are cheaper than a premature shared abstraction, so +// a third adopter should extract one. +// +// This module contains the literal text require('electron'), so it must never +// become reachable from a plain-Node fork entry (build-plugins/ +// plain-node-entry-guard.ts fails the build on that text, try/catch or not). + +// Why: the worker's own loop absorbs the spawn stall, so the client only needs +// a backstop for a wedged thread. Kept at 30s because a scan sits on the +// user-blocking localhost-label allowlist path (src/main/ipc/ +// localhost-worktree-labels.ts). +export const WORKER_STALL_GRACE_MS = 26_000 +export const CALL_DEADLINE_MS = PORT_SCAN_COMMAND_TIMEOUT_MS + WORKER_STALL_GRACE_MS +// Deliberately far longer than the 30s scan cadence so a visible window does not +// re-create the worker every tick; the renderer stops the interval when hidden, +// so this is effectively the hidden-window teardown. +export const IDLE_TEARDOWN_MS = 5 * 60_000 +export const MAX_CONSECUTIVE_DEATHS = 3 +// One scan issues at most three commands; anything beyond this is pile-up. +export const MAX_QUEUED_CALLS = 8 + +export type PortScanCommandResult = { stdout: string; spawnMs: number } +export type PortScanWorkerFactory = () => Worker + +// Distinguishes "no worker at all" from a timeout or crash so the scanner can +// log it once and callers never mistake it for a command timeout. +class PortScanWorkerUnavailableError extends Error {} + +/** True when a scan failed because the probe worker could not be started. */ +export function isPortScanWorkerUnavailableError(error: unknown): boolean { + return error instanceof PortScanWorkerUnavailableError +} + +type PendingCall = { + request: PortScanCommandRequest + resolve: (value: PortScanCommandResult) => void + reject: (error: Error) => void + timer: NodeJS.Timeout | null +} + +/** + * Main-thread bridge that runs port-scan probe commands on a persistent worker + * thread. Dispatches one command at a time (FIFO), times each call out from + * dispatch, respawns after faults (capped by `MAX_CONSECUTIVE_DEATHS`), tears + * the worker down after `IDLE_TEARDOWN_MS`, and fails closed when no worker can + * be spawned rather than moving process creation back onto the main thread. + */ +export class PortScanCommandClient { + private worker: Worker | null = null + private active: PendingCall | null = null + private queue: PendingCall[] = [] + private idleTimer: NodeJS.Timeout | null = null + private consecutiveDeaths = 0 + private nextId = 1 + private loggedWorkerUnavailable = false + private cleanupWorkerListeners: (() => void) | null = null + private readonly workerFactory: PortScanWorkerFactory + private readonly log: (message: string) => void + + constructor(options: { workerFactory: PortScanWorkerFactory; log?: (message: string) => void }) { + this.workerFactory = options.workerFactory + this.log = options.log ?? ((message) => console.warn(message)) + } + + /** + * Run one probe command on the worker. + * @param command - Executable name (lsof, ps, netstat, powershell.exe). + * @param args - Argument vector passed verbatim to execFile. + * @returns The command's stdout plus its measured process-creation latency. + */ + run(command: string, args: string[]): Promise { + return new Promise((resolve, reject) => { + if (this.queue.length >= MAX_QUEUED_CALLS) { + reject(new Error(`Port scan command queue is full; dropped ${command}.`)) + return + } + // A fresh burst from full idle starts a new scan: clear any death count + // carried from a prior scan so the respawn cap can't drain this scan early. + if (!this.active && this.queue.length === 0) { + this.consecutiveDeaths = 0 + } + this.queue.push({ + request: { id: this.nextId++, command, args }, + resolve, + reject, + timer: null + }) + this.pump() + }) + } + + private pump(): void { + if (this.active || this.queue.length === 0) { + return + } + const worker = this.ensureWorker() + if (!worker) { + this.failQueuedAsUnavailable() + return + } + const call = this.queue.shift() + if (!call) { + return + } + this.active = call + this.clearIdleTimer() + // Why (#11161): one at a time. uv_spawn blocks the worker's own loop, so a + // second concurrent request would have its deadline armed while the first + // spawn is still stalling the thread, producing a false timeout. + call.timer = setTimeout(() => this.onDeadline(call), CALL_DEADLINE_MS) + call.timer.unref?.() + worker.postMessage(call.request) + } + + private ensureWorker(): Worker | null { + if (this.worker) { + return this.worker + } + try { + const worker = this.workerFactory() + const onMessage = (response: PortScanCommandResponse): void => this.onMessage(response) + const onError = (error: Error): void => this.onWorkerFault(error) + const onExit = (code: number): void => this.onWorkerExit(code) + worker.on('message', onMessage) + worker.on('error', onError) + worker.on('exit', onExit) + this.cleanupWorkerListeners = () => { + worker.off('message', onMessage) + worker.off('error', onError) + worker.off('exit', onExit) + } + // Never keep the app alive for a port scan. + worker.unref?.() + this.worker = worker + return worker + } catch (err) { + // Why (#11161): never fall back to in-process execFile here; a missing + // bundle must report port scanning as unavailable rather than reintroduce + // the main-thread freeze this worker boundary exists to prevent. + if (!this.loggedWorkerUnavailable) { + this.loggedWorkerUnavailable = true + this.log(`[workspace-ports] probe worker unavailable. ${errorMessage(err)}`) + } + return null + } + } + + private onMessage(response: PortScanCommandResponse): void { + const call = this.active + if (!call || call.request.id !== response.id) { + return + } + this.consecutiveDeaths = 0 + if (response.ok) { + this.settle(call, () => call.resolve({ stdout: response.stdout, spawnMs: response.spawnMs })) + } else { + const error = response.timedOut + ? new PortScanCommandTimeoutError(response.error) + : new Error(response.error) + this.settle(call, () => call.reject(error)) + } + this.afterSettle() + } + + private onDeadline(call: PendingCall): void { + if (this.active !== call) { + return + } + // Plain Error on purpose: a wedged worker is not a command timeout and must + // never feed the scanner's timeout backoff. + this.onWorkerFault(new Error(`Port scan probe worker stalled after ${CALL_DEADLINE_MS}ms`)) + } + + private onWorkerExit(code: number): void { + // A clean self-exit is not a death, but the stale handle must be dropped or + // the next dispatch would post into a dead worker and stall to its deadline. + if (code === 0 && !this.active && this.queue.length === 0) { + this.destroyWorker() + return + } + this.onWorkerFault(new Error(`Port scan probe worker exited with code ${code}`)) + } + + private onWorkerFault(error: Error): void { + const failed = this.active + this.destroyWorker() + this.consecutiveDeaths++ + if (failed) { + this.settle(failed, () => failed.reject(error)) + } + if (this.consecutiveDeaths >= MAX_CONSECUTIVE_DEATHS) { + this.drainQueueAfterCrashLoop(error) + return + } + if (this.queue.length > 0) { + this.pump() + } + } + + private drainQueueAfterCrashLoop(error: Error): void { + const pending = this.queue + this.queue = [] + this.consecutiveDeaths = 0 + const drainError = new Error(`Port scan probe worker crashed repeatedly (${error.message})`) + for (const call of pending) { + this.settle(call, () => call.reject(drainError)) + } + } + + private failQueuedAsUnavailable(): void { + const pending = this.queue + this.queue = [] + for (const call of pending) { + this.settle(call, () => + call.reject(new PortScanWorkerUnavailableError('port scan probe worker spawn failed')) + ) + } + } + + private settle(call: PendingCall, run: () => void): void { + if (call.timer) { + clearTimeout(call.timer) + call.timer = null + } + if (this.active === call) { + this.active = null + } + run() + } + + private afterSettle(): void { + if (this.queue.length > 0) { + this.pump() + } else { + this.scheduleIdleTeardown() + } + } + + private scheduleIdleTeardown(): void { + this.clearIdleTimer() + if (!this.worker) { + return + } + this.idleTimer = setTimeout(() => this.teardownIfIdle(), IDLE_TEARDOWN_MS) + this.idleTimer.unref?.() + } + + private teardownIfIdle(): void { + this.idleTimer = null + // Only tear down with nothing active AND nothing queued: a request arriving + // as the timer fires must never be lost to a self-exiting worker. + if (this.active || this.queue.length > 0) { + return + } + this.destroyWorker() + } + + private clearIdleTimer(): void { + if (this.idleTimer) { + clearTimeout(this.idleTimer) + this.idleTimer = null + } + } + + private destroyWorker(): void { + this.clearIdleTimer() + const worker = this.worker + this.worker = null + if (!worker) { + return + } + this.cleanupWorkerListeners?.() + this.cleanupWorkerListeners = null + worker.removeAllListeners() + // Terminating can orphan a probe child mid-spawn; the worker reaps what it + // can on exit, and every probe here is short-lived. + void worker.terminate().catch(() => undefined) + } +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +const WORKER_ENTRY_FILENAME = 'port-scan-command-worker-entry.js' + +/** Where the built worker entry can live: packaged resources or the build dir. */ +export type WorkerEntryLayout = { + isPackaged: boolean + resourcesPath: string + moduleDir: string +} + +/** + * Resolve the built worker entry for one runtime layout. + * @param layout - Packaged flag plus both candidate roots. + * @returns Path passed to `new Worker()`. + */ +export function resolveWorkerEntryPath(layout: WorkerEntryLayout): string { + // Packaged builds leave this entry inside app.asar — only forked child + // processes are asarUnpack'd — so it resolves off resourcesPath rather than + // the bundler's __dirname, matching the shipped stt/warp/opencode workers. + // Split out from the electron read so the packaged branch is testable without + // a packaged build. + if (layout.isPackaged) { + return join(layout.resourcesPath, 'app.asar', 'out', 'main', WORKER_ENTRY_FILENAME) + } + return join(layout.moduleDir, WORKER_ENTRY_FILENAME) +} + +function currentWorkerEntryLayout(): WorkerEntryLayout { + let app: { isPackaged: boolean } | null = null + try { + app = require('electron').app ?? null + } catch { + app = null + } + return { + isPackaged: app?.isPackaged === true, + resourcesPath: process.resourcesPath, + moduleDir: __dirname + } +} + +function defaultWorkerFactory(): Worker { + const workerPath = resolveWorkerEntryPath(currentWorkerEntryLayout()) + // Why: a missing built entry must throw synchronously so the client can fail + // closed before it waits on a worker that can never post a result. + if (!existsSync(workerPath)) { + throw new Error(`Port scan command worker entry not found: ${workerPath}`) + } + return new Worker(workerPath) +} + +let sharedClient: PortScanCommandClient | null = null + +/** + * Run a port-scan probe command through the process-wide worker client. + * @param command - Executable name (lsof, ps, netstat, powershell.exe). + * @param args - Argument vector passed verbatim to execFile. + * @returns The command's stdout plus its measured process-creation latency. + */ +export function runPortScanCommand( + command: string, + args: string[] +): Promise { + sharedClient ??= new PortScanCommandClient({ workerFactory: defaultWorkerFactory }) + return sharedClient.run(command, args) +} diff --git a/src/main/ports/port-scan-command-execution.test.ts b/src/main/ports/port-scan-command-execution.test.ts new file mode 100644 index 000000000..15e97d4a3 --- /dev/null +++ b/src/main/ports/port-scan-command-execution.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { runPortScanCommandInProcess } from './port-scan-command-execution' +import { + PORT_SCAN_COMMAND_TIMEOUT_MS, + PortScanCommandTimeoutError, + WATCHDOG_GRACE_MS +} from './port-scan-command-protocol' + +const execFileMock = vi.hoisted(() => vi.fn()) + +vi.mock('child_process', () => ({ + execFile: execFileMock +})) + +// Why (#11161): must outlast the whole watchdog budget, otherwise a watchdog +// armed before execFile still survives the stall and the ordering goes unpinned. +const SPAWN_STALL_MS = PORT_SCAN_COMMAND_TIMEOUT_MS + WATCHDOG_GRACE_MS + 200 +const LSOF_OUTPUT = ['p123', 'cnode', 'n127.0.0.1:5173'].join('\n') + +/** Emulates a hooked CreateProcessW: blocks the calling thread inside uv_spawn. */ +function blockCallingThread(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms) +} + +describe('runPortScanCommandInProcess', () => { + afterEach(() => { + vi.useRealTimers() + execFileMock.mockReset() + }) + + it('arms the watchdog only after process creation returns', async () => { + execFileMock.mockImplementation( + (_command: string, _args: string[], _options: unknown, callback: unknown) => { + blockCallingThread(SPAWN_STALL_MS) + // The command itself is healthy once it finally starts. + setTimeout(() => (callback as (e: null, out: string) => void)(null, LSOF_OUTPUT), 5) + return { kill: vi.fn() } + } + ) + + const result = await runPortScanCommandInProcess('lsof', ['-nP', '-iTCP']) + + expect(result.stdout).toBe(LSOF_OUTPUT) + expect(result.spawnMs).toBeGreaterThanOrEqual(SPAWN_STALL_MS - 100) + }) + + it('kills the child and times out when the callback never arrives', async () => { + vi.useFakeTimers() + const killMock = vi.fn() + execFileMock.mockImplementation(() => ({ kill: killMock })) + + let settled = false + const promise = runPortScanCommandInProcess('lsof', []).catch((error: unknown) => { + settled = true + return error + }) + + await vi.advanceTimersByTimeAsync(PORT_SCAN_COMMAND_TIMEOUT_MS) + expect(settled).toBe(false) + + await vi.advanceTimersByTimeAsync(WATCHDOG_GRACE_MS) + + expect(await promise).toBeInstanceOf(PortScanCommandTimeoutError) + expect(killMock).toHaveBeenCalled() + }) + + it("classifies Node's own execFile timeout kill as a command timeout", async () => { + execFileMock.mockImplementation( + (_command: string, _args: string[], _options: unknown, callback: unknown) => { + const killed = Object.assign(new Error('Command failed: lsof'), { + killed: true, + signal: 'SIGTERM' + }) + setTimeout(() => (callback as (e: Error) => void)(killed), 0) + return { kill: vi.fn() } + } + ) + + await expect(runPortScanCommandInProcess('lsof', [])).rejects.toBeInstanceOf( + PortScanCommandTimeoutError + ) + }) + + it('leaves a genuine command failure unclassified so the scan does not back off', async () => { + execFileMock.mockImplementation( + (_command: string, _args: string[], _options: unknown, callback: unknown) => { + setTimeout(() => (callback as (e: Error) => void)(new Error('spawn ENOENT')), 0) + return { kill: vi.fn() } + } + ) + + const error = await runPortScanCommandInProcess('lsof', []).catch((err: unknown) => err) + + expect(error).toBeInstanceOf(Error) + expect(error).not.toBeInstanceOf(PortScanCommandTimeoutError) + }) +}) diff --git a/src/main/ports/port-scan-command-execution.ts b/src/main/ports/port-scan-command-execution.ts new file mode 100644 index 000000000..9c63fc9aa --- /dev/null +++ b/src/main/ports/port-scan-command-execution.ts @@ -0,0 +1,115 @@ +import { execFile, type ChildProcess } from 'node:child_process' +import { + PORT_SCAN_COMMAND_TIMEOUT_MS, + PortScanCommandTimeoutError, + WATCHDOG_GRACE_MS, + portScanCommandTimeoutMessage +} from './port-scan-command-protocol' + +// Why (#11161): libuv runs uv_spawn inline on whichever event loop calls it, so +// an endpoint-security hook on CreateProcessW stalls that thread for the whole +// spawn. This module is only ever entered from the worker thread +// (port-scan-command-worker-entry.ts); nothing on the main thread may import it. + +const WATCHDOG_TIMEOUT_MS = PORT_SCAN_COMMAND_TIMEOUT_MS + WATCHDOG_GRACE_MS + +const activeChildren = new Set() + +/** + * Run a port-scan probe command and report how long process creation took. + * @param command - Executable name resolved against the worker's PATH. + * @param args - Argument vector passed verbatim, never shell-interpolated. + * @returns The command's stdout plus `spawnMs`, the measured process-creation + * latency callers use to skip optional follow-up commands on a stalled host. + * @throws PortScanCommandTimeoutError when the command itself outran its budget. + */ +export async function runPortScanCommandInProcess( + command: string, + args: string[] +): Promise<{ stdout: string; spawnMs: number }> { + return await new Promise((resolve, reject) => { + let settled = false + let timer: NodeJS.Timeout | null = null + let spawnMs = 0 + let child: ChildProcess | undefined + + const settle = (callback: () => void): void => { + if (settled) { + return + } + settled = true + if (timer) { + clearTimeout(timer) + } + if (child) { + activeChildren.delete(child) + } + callback() + } + + const startedAt = Date.now() + try { + child = execFile( + command, + args, + { + timeout: PORT_SCAN_COMMAND_TIMEOUT_MS, + maxBuffer: 2 * 1024 * 1024, + windowsHide: true + }, + (error, stdout) => { + if (error) { + // Node kills its own execFile timeout with a signal, which surfaces + // as killed:true — the only way to tell a timeout from a real error. + const timedOut = (error as { killed?: boolean }).killed === true + settle(() => + reject( + timedOut + ? new PortScanCommandTimeoutError( + portScanCommandTimeoutMessage(command, PORT_SCAN_COMMAND_TIMEOUT_MS) + ) + : error + ) + ) + return + } + settle(() => resolve({ stdout: String(stdout), spawnMs })) + } + ) + } catch (error) { + settle(() => reject(error)) + return + } + + // Why (#11161): measured after execFile returns, because that call blocks + // for the whole of process creation. Arming the watchdog earlier would + // charge the spawn stall against the command's budget and fire immediately. + spawnMs = Date.now() - startedAt + if (settled || !child) { + return + } + activeChildren.add(child) + timer = setTimeout(() => { + settle(() => { + child?.kill() + reject( + new PortScanCommandTimeoutError( + portScanCommandTimeoutMessage(command, WATCHDOG_TIMEOUT_MS) + ) + ) + }) + }, WATCHDOG_TIMEOUT_MS) + }) +} + +/** Best-effort reap so a terminated worker does not orphan an in-flight probe. */ +export function killActivePortScanCommands(): void { + for (const child of activeChildren) { + try { + child.kill() + } catch { + // Already exited; nothing to reap. + } + } + activeChildren.clear() +} diff --git a/src/main/ports/port-scan-command-import-boundary.test.ts b/src/main/ports/port-scan-command-import-boundary.test.ts new file mode 100644 index 000000000..bd9b9d05f --- /dev/null +++ b/src/main/ports/port-scan-command-import-boundary.test.ts @@ -0,0 +1,18 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +// Why (#11161): libuv runs process creation inline on the calling event loop, +// so the port scan only stays off CrBrowserMain while these main-thread modules +// spawn nothing themselves. Spawning belongs to port-scan-command-execution.ts, +// which only the worker entry imports. +const MAIN_THREAD_MODULES = ['local-workspace-port-scanner.ts', 'port-scan-command-client.ts'] + +describe('port scan main-thread spawn boundary', () => { + it.each(MAIN_THREAD_MODULES)('keeps %s free of child_process', (fileName) => { + const source = readFileSync(join(import.meta.dirname, fileName), 'utf8') + + expect(source).not.toMatch(/from\s+['"](node:)?child_process['"]/) + expect(source).not.toMatch(/require\(\s*['"](node:)?child_process['"]\s*\)/) + }) +}) diff --git a/src/main/ports/port-scan-command-protocol.ts b/src/main/ports/port-scan-command-protocol.ts new file mode 100644 index 000000000..02beeb982 --- /dev/null +++ b/src/main/ports/port-scan-command-protocol.ts @@ -0,0 +1,36 @@ +// Why (#11161): request/response shapes plus the timeout error shared by the +// port-scan worker entry and its main-thread client. Kept free of Electron, +// node:worker_threads and node:child_process so importing it from either side +// can never drag the other side's dependencies across the boundary. + +export const PORT_SCAN_COMMAND_TIMEOUT_MS = 4_000 +// Node's own execFile timeout is the primary killer; the manual watchdog only +// covers "the callback never arrived", so it must fire strictly later. +export const WATCHDOG_GRACE_MS = 1_000 + +export type PortScanCommandRequest = { + id: number + command: string + args: string[] +} + +export type PortScanCommandResponse = + | { id: number; ok: true; stdout: string; spawnMs: number } + | { id: number; ok: false; timedOut: boolean; error: string } + +/** + * Raised when a port-scan command was killed after exceeding its budget. Errors + * do not survive structured clone as subclasses, so the worker reports a + * `timedOut` flag and the client reconstructs this type from it. + */ +export class PortScanCommandTimeoutError extends Error { + constructor(message: string) { + super(message) + this.name = 'PortScanCommandTimeoutError' + } +} + +/** Shared wording so worker-side and client-side timeouts read identically. */ +export function portScanCommandTimeoutMessage(command: string, timeoutMs: number): string { + return `${command} timed out after ${timeoutMs}ms` +} diff --git a/src/main/ports/port-scan-command-worker-entry.ts b/src/main/ports/port-scan-command-worker-entry.ts new file mode 100644 index 000000000..b5cb5ad5a --- /dev/null +++ b/src/main/ports/port-scan-command-worker-entry.ts @@ -0,0 +1,53 @@ +import { parentPort } from 'node:worker_threads' +import { + killActivePortScanCommands, + runPortScanCommandInProcess +} from './port-scan-command-execution' +import { + PortScanCommandTimeoutError, + type PortScanCommandRequest, + type PortScanCommandResponse +} from './port-scan-command-protocol' + +// Why (#11161): process creation blocks the event loop that issues it. Running +// the port scan's probe commands on this worker thread keeps an EDR-hooked +// CreateProcessW off CrBrowserMain. The client dispatches one request at a +// time, so this loop stays serial; imports must remain electron-free. + +if (!parentPort) { + throw new Error('Port scan command worker must run with a parent port.') +} +const port = parentPort + +process.once('exit', killActivePortScanCommands) + +async function handleRequest(request: PortScanCommandRequest): Promise { + try { + const { stdout, spawnMs } = await runPortScanCommandInProcess(request.command, request.args) + return { id: request.id, ok: true, stdout, spawnMs } + } catch (err) { + return { + id: request.id, + ok: false, + timedOut: err instanceof PortScanCommandTimeoutError, + error: err instanceof Error ? err.message : String(err) + } + } +} + +port.on('message', (request: PortScanCommandRequest) => { + void handleRequest(request).then((response) => { + try { + port.postMessage(response) + } catch { + // A non-cloneable result would otherwise post nothing and leave the client + // waiting out its deadline; fail that request fast instead. + port.postMessage({ + id: request.id, + ok: false, + timedOut: false, + error: 'Port scan command result could not be serialized.' + }) + } + }) +}) diff --git a/src/main/ports/workspace-port-ownership.test.ts b/src/main/ports/workspace-port-ownership.test.ts new file mode 100644 index 000000000..b367fcbf2 --- /dev/null +++ b/src/main/ports/workspace-port-ownership.test.ts @@ -0,0 +1,57 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { killWorkspacePort } from './workspace-port-ownership' + +const scanWorkspacePortsMock = vi.hoisted(() => vi.fn()) + +vi.mock('./local-workspace-port-scanner', () => ({ + scanWorkspacePorts: scanWorkspacePortsMock +})) + +const worktrees = [{ id: 'repo::/repo', repoId: 'repo', displayName: 'main', path: '/repo' }] + +describe('killWorkspacePort', () => { + afterEach(() => { + scanWorkspacePortsMock.mockReset() + vi.restoreAllMocks() + }) + + // 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. + it('requires owner metadata from the authorizing re-scan', async () => { + scanWorkspacePortsMock.mockResolvedValue({ platform: 'darwin', scannedAt: 0, ports: [] }) + + await killWorkspacePort(worktrees, { pid: 123, port: 5173 }) + + expect(scanWorkspacePortsMock).toHaveBeenCalledWith(worktrees, undefined, { + requireMetadata: true + }) + }) + + it('refuses a pid the re-scan does not attribute to a workspace', async () => { + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true) + scanWorkspacePortsMock.mockResolvedValue({ + platform: 'darwin', + scannedAt: 0, + ports: [ + { + id: '127.0.0.1:5173:123', + bindHost: '127.0.0.1', + connectHost: '127.0.0.1', + port: 5173, + pid: 123, + protocol: 'http', + kind: 'external' + } + ] + }) + + const result = await killWorkspacePort(worktrees, { pid: 123, port: 5173 }) + + expect(result).toEqual({ + ok: false, + reason: 'Only workspace-owned local processes can be stopped here.' + }) + expect(killSpy).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ports/workspace-port-ownership.ts b/src/main/ports/workspace-port-ownership.ts index 9ae5bf658..9f7df2465 100644 --- a/src/main/ports/workspace-port-ownership.ts +++ b/src/main/ports/workspace-port-ownership.ts @@ -8,7 +8,7 @@ import type { WorkspacePortProbe, WorkspacePortScanResult } from '../../shared/workspace-ports' -import { scanWorkspacePorts } from './local-workspace-port-scanner' +import { scanWorkspacePorts, type WorkspacePortScanOptions } from './local-workspace-port-scanner' export type WorkspacePortProbeInput = WorkspacePortProbe & { connectionId?: string | null @@ -82,7 +82,9 @@ export async function killWorkspacePort( return { ok: false, reason: 'Invalid process or port.' } } - const scan = await scanWorkspacePorts([...worktrees]) + // Why (#11161): this re-scan is the authorization check for SIGTERM, so it + // must never land on a cycle that skipped the owner-attribution metadata. + const scan = await scanWorkspacePorts([...worktrees], undefined, { requireMetadata: true }) const port = scan.ports.find( (candidate) => candidate.pid === args.pid && candidate.port === args.port ) @@ -113,7 +115,8 @@ export async function killWorkspacePort( } export async function scanWorkspacePortProbes( - worktrees: readonly WorkspacePortProbe[] + worktrees: readonly WorkspacePortProbe[], + options?: WorkspacePortScanOptions ): Promise { - return scanWorkspacePorts([...worktrees]) + return scanWorkspacePorts([...worktrees], undefined, options) }