diff --git a/scripts/lib/opencode-live-preflight.mjs b/scripts/lib/opencode-live-preflight.mjs index 73661131..1fc7d734 100644 --- a/scripts/lib/opencode-live-preflight.mjs +++ b/scripts/lib/opencode-live-preflight.mjs @@ -4,6 +4,10 @@ import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; +const CHILD_CLOSE_GRACE_MS = 3_000; +const CHILD_FORCE_CLOSE_GRACE_MS = 1_000; +const TASKKILL_TIMEOUT_MS = 5_000; + export async function preflightOpenCodeLiveEnvironment(input) { const repoRoot = input.repoRoot; const opencodeBin = process.env.OPENCODE_BIN?.trim() || '/opt/homebrew/bin/opencode'; @@ -97,6 +101,7 @@ async function canStartOpenCodeHost(opencodeBin, cwd, env) { cwd, env, stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, }); let output = ''; let spawnError = ''; @@ -138,26 +143,100 @@ async function canStartOpenCodeHost(opencodeBin, cwd, env) { } } -function stopChild(child) { - return new Promise((resolve) => { - if (child.exitCode != null || child.killed) { - resolve(); - return; +async function stopChild(child, options = {}) { + const platform = options.platform ?? process.platform; + const killProcessTree = options.killProcessTree ?? taskkillProcessTree; + const closeGraceMs = options.closeGraceMs ?? CHILD_CLOSE_GRACE_MS; + const forceCloseGraceMs = options.forceCloseGraceMs ?? CHILD_FORCE_CLOSE_GRACE_MS; + + if (hasChildExited(child)) { + return; + } + + if (platform === 'win32' && child.pid) { + await killProcessTree(child.pid); + } else if (!child.killed) { + sendChildSignal(child, 'SIGTERM'); + } + + if (await waitForChildClose(child, closeGraceMs)) { + return; + } + + if (!hasChildExited(child)) { + sendChildSignal(child, 'SIGKILL'); + if (!(await waitForChildClose(child, forceCloseGraceMs))) { + child.stdout?.destroy(); + child.stderr?.destroy(); + child.unref?.(); } - const timeout = setTimeout(() => { - if (child.exitCode == null) { - child.kill('SIGKILL'); - } - resolve(); - }, 3_000); - child.once('close', () => { + } +} + +function taskkillProcessTree(pid) { + return new Promise((resolve) => { + let done = false; + let taskkill = null; + const finish = () => { + if (done) return; + done = true; clearTimeout(timeout); resolve(); - }); - child.kill('SIGTERM'); + }; + const timeout = setTimeout(() => { + if (taskkill) { + sendChildSignal(taskkill, 'SIGTERM'); + } + finish(); + }, TASKKILL_TIMEOUT_MS); + try { + taskkill = spawn( + path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'taskkill.exe'), + ['/T', '/F', '/PID', String(pid)], + { + stdio: 'ignore', + windowsHide: true, + } + ); + taskkill.unref?.(); + taskkill.once('error', finish); + taskkill.once('close', finish); + } catch { + finish(); + } }); } +function waitForChildClose(child, timeoutMs) { + if (hasChildExited(child)) { + return Promise.resolve(true); + } + + return new Promise((resolve) => { + let done = false; + const finish = (closed) => { + if (done) return; + done = true; + clearTimeout(timeout); + resolve(closed); + }; + const timeout = setTimeout(() => finish(false), timeoutMs); + child.once('close', () => finish(true)); + }); +} + +function hasChildExited(child) { + return child.exitCode != null || child.signalCode != null; +} + +function sendChildSignal(child, signal) { + try { + child.kill(signal); + } catch { + // Process may already be gone between liveness checks and the kill call. + } +} + function allocateLoopbackPort() { return new Promise((resolve, reject) => { const server = net.createServer(); @@ -190,3 +269,8 @@ function skip(reason) { function compactOutput(value) { return value.replace(/\s+/g, ' ').trim().slice(0, 1_200); } + +export const __opencodeLivePreflightTestHooks = { + stopChild, + taskkillProcessTree, +}; diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 833cb9a5..d55d0e91 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -572,6 +572,11 @@ import type { ToolCallMeta, } from '@shared/types'; +// pidusage's Windows wmic/gwmi fallback needs a non-zero cache window to finish +// its initial two-sample pass. Keep this above slow PowerShell startup time, or +// the first sample can expire before the recursive second read and loop again. +const RUNTIME_PIDUSAGE_OPTIONS = process.platform === 'win32' ? { maxage: 10_000 } : { maxage: 0 }; + const logger = createLogger('Service:TeamProvisioning'); const PREFLIGHT_DEBUG_LOG_PATH = path.join(os.tmpdir(), 'claude-team-preflight-debug.log'); @@ -15298,7 +15303,7 @@ export class TeamProvisioningService { let rssBytes = rssPid ? rssBytesByPid.get(rssPid) : undefined; if (rssBytes == null && isSharedOpenCodeHost && typeof rssPid === 'number' && rssPid > 0) { try { - const refreshedStat = await pidusage(rssPid, { maxage: 0 }); + const refreshedStat = await pidusage(rssPid, RUNTIME_PIDUSAGE_OPTIONS); if (Number.isFinite(refreshedStat.memory) && refreshedStat.memory >= 0) { rssBytesByPid.set(rssPid, refreshedStat.memory); rssBytes = refreshedStat.memory; @@ -25558,7 +25563,7 @@ export class TeamProvisioningService { } const rssBytesByPid = new Map(); - const options = { maxage: 0 }; + const options = RUNTIME_PIDUSAGE_OPTIONS; try { const statsByPid = await pidusage(uniquePids, options); for (const [rawPid, stat] of Object.entries(statsByPid)) { diff --git a/test/main/services/team/TeamProvisioningService.test.ts b/test/main/services/team/TeamProvisioningService.test.ts index bd26e321..ad7b7129 100644 --- a/test/main/services/team/TeamProvisioningService.test.ts +++ b/test/main/services/team/TeamProvisioningService.test.ts @@ -176,6 +176,9 @@ import { } from '@features/tmux-installer/main'; import pidusage from 'pidusage'; +const EXPECTED_RUNTIME_PIDUSAGE_OPTIONS = + process.platform === 'win32' ? { maxage: 10_000 } : { maxage: 0 }; + function allowConsoleLogs() { vi.spyOn(console, 'error').mockImplementation(() => {}); vi.spyOn(console, 'warn').mockImplementation(() => {}); @@ -2490,7 +2493,7 @@ describe('TeamProvisioningService', () => { const snapshot = await svc.getTeamAgentRuntimeSnapshot('runtime-team'); - expect(pidusage).toHaveBeenCalledWith([111, 222], { maxage: 0 }); + expect(pidusage).toHaveBeenCalledWith([111, 222], EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); expect(snapshot.members['team-lead']).toMatchObject({ pid: 111, rssBytes: 123_000_000, @@ -2630,9 +2633,9 @@ describe('TeamProvisioningService', () => { const snapshot = await svc.getTeamAgentRuntimeSnapshot('runtime-team'); - expect(pidusage).toHaveBeenNthCalledWith(1, [111, 222], { maxage: 0 }); - expect(pidusage).toHaveBeenNthCalledWith(2, 111, { maxage: 0 }); - expect(pidusage).toHaveBeenNthCalledWith(3, 222, { maxage: 0 }); + expect(pidusage).toHaveBeenNthCalledWith(1, [111, 222], EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); + expect(pidusage).toHaveBeenNthCalledWith(2, 111, EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); + expect(pidusage).toHaveBeenNthCalledWith(3, 222, EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); expect(snapshot.members['team-lead']?.rssBytes).toBe(123_000_000); expect(snapshot.members.alice?.rssBytes).toBe(456_000_000); }); @@ -2744,7 +2747,7 @@ describe('TeamProvisioningService', () => { const snapshot = await svc.getTeamAgentRuntimeSnapshot('nice-team'); - expect(pidusage).toHaveBeenCalledWith([111, 333], { maxage: 0 }); + expect(pidusage).toHaveBeenCalledWith([111, 333], EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); expect(snapshot.members.alice).toMatchObject({ alive: true, providerId: 'anthropic', @@ -3256,8 +3259,8 @@ describe('TeamProvisioningService', () => { const snapshot = await svc.getTeamAgentRuntimeSnapshot('runtime-team'); - expect(pidusage).toHaveBeenCalledWith([111, 333], { maxage: 0 }); - expect(pidusage).toHaveBeenCalledWith(333, { maxage: 0 }); + expect(pidusage).toHaveBeenCalledWith([111, 333], EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); + expect(pidusage).toHaveBeenCalledWith(333, EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); expect(snapshot.members.bob).toMatchObject({ memberName: 'bob', alive: false, @@ -3332,7 +3335,7 @@ describe('TeamProvisioningService', () => { const snapshot = await svc.getTeamAgentRuntimeSnapshot('runtime-team'); - expect(pidusage).toHaveBeenCalledWith([333], { maxage: 0 }); + expect(pidusage).toHaveBeenCalledWith([333], EXPECTED_RUNTIME_PIDUSAGE_OPTIONS); expect(snapshot.members.bob).toMatchObject({ memberName: 'bob', alive: false, @@ -15450,9 +15453,9 @@ describe('TeamProvisioningService', () => { }); expect(spawnCli).toHaveBeenCalled(); - expect(progressUpdates[0]?.warnings).toEqual(expect.arrayContaining([ - expect.stringContaining('9 primary teammates'), - ])); + expect(progressUpdates[0]?.warnings).toEqual( + expect.arrayContaining([expect.stringContaining('9 primary teammates')]) + ); expect(progressUpdates[0]?.warnings?.join('\n')).toContain('Launches above 8 teammates'); }); diff --git a/test/scripts/opencodeLivePreflight.test.ts b/test/scripts/opencodeLivePreflight.test.ts new file mode 100644 index 00000000..0176d371 --- /dev/null +++ b/test/scripts/opencodeLivePreflight.test.ts @@ -0,0 +1,147 @@ +// @vitest-environment node + +import { EventEmitter } from 'events'; +import { promises as fs } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { pathToFileURL } from 'url'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +interface StopChildOptions { + platform?: string; + killProcessTree?: (pid: number) => Promise; + closeGraceMs?: number; + forceCloseGraceMs?: number; +} + +interface OpenCodeLivePreflightTestHooks { + __opencodeLivePreflightTestHooks: { + stopChild(child: FakeChild, options?: StopChildOptions): Promise; + taskkillProcessTree(pid: number): Promise; + }; +} + +const runOnPosix = process.platform === 'win32' ? it.skip : it; + +describe('opencode live preflight cleanup', () => { + let tempDir = ''; + const originalSystemRoot = process.env.SystemRoot; + const originalTaskkillArgsPath = process.env.FAKE_TASKKILL_ARGS_PATH; + + afterEach(async () => { + restoreEnvValue('SystemRoot', originalSystemRoot); + restoreEnvValue('FAKE_TASKKILL_ARGS_PATH', originalTaskkillArgsPath); + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + tempDir = ''; + } + }); + + it('waits for child close after Windows process-tree cleanup', async () => { + const { stopChild } = (await loadTestHooks()).__opencodeLivePreflightTestHooks; + const child = new FakeChild({ pid: 1234 }); + const killProcessTree = vi.fn(() => { + child.signalCode = 'SIGTERM'; + child.emit('close'); + return Promise.resolve(); + }); + + await stopChild(child, { + closeGraceMs: 5, + forceCloseGraceMs: 5, + killProcessTree, + platform: 'win32', + }); + + expect(killProcessTree).toHaveBeenCalledWith(1234); + expect(child.kill).not.toHaveBeenCalled(); + expect(child.stdout.destroy).not.toHaveBeenCalled(); + expect(child.unref).not.toHaveBeenCalled(); + }); + + it('detaches pipes when Windows process-tree cleanup and direct kill both fail to close', async () => { + const { stopChild } = (await loadTestHooks()).__opencodeLivePreflightTestHooks; + const child = new FakeChild({ pid: 5678 }); + const killProcessTree = vi.fn(() => Promise.resolve()); + + await stopChild(child, { + closeGraceMs: 1, + forceCloseGraceMs: 1, + killProcessTree, + platform: 'win32', + }); + + expect(killProcessTree).toHaveBeenCalledWith(5678); + expect(child.kill).toHaveBeenCalledWith('SIGKILL'); + expect(child.stdout.destroy).toHaveBeenCalled(); + expect(child.stderr.destroy).toHaveBeenCalled(); + expect(child.unref).toHaveBeenCalled(); + }); + + runOnPosix('invokes taskkill.exe with process-tree flags', async () => { + const { taskkillProcessTree } = (await loadTestHooks()).__opencodeLivePreflightTestHooks; + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'opencode-taskkill-test-')); + const system32Dir = path.join(tempDir, 'System32'); + const taskkillArgsPath = path.join(tempDir, 'taskkill-args.txt'); + + await fs.mkdir(system32Dir, { recursive: true }); + await writeExecutable(path.join(system32Dir, 'taskkill.exe'), fakeTaskkillScript()); + process.env.SystemRoot = tempDir; + process.env.FAKE_TASKKILL_ARGS_PATH = taskkillArgsPath; + + await taskkillProcessTree(4242); + + await expect(fs.readFile(taskkillArgsPath, 'utf8')).resolves.toBe('/T /F /PID 4242\n'); + }); +}); + +class FakeChild extends EventEmitter { + readonly kill = vi.fn(); + readonly stderr = { destroy: vi.fn() }; + readonly stdout = { destroy: vi.fn() }; + readonly unref = vi.fn(); + exitCode: number | null = null; + killed = false; + pid: number; + signalCode: string | null = null; + + constructor(input: { pid: number }) { + super(); + this.pid = input.pid; + this.kill.mockImplementation((signal: string) => { + this.killed = true; + return signal === 'SIGKILL'; + }); + } +} + +async function loadTestHooks(): Promise { + const moduleUrl = pathToFileURL( + path.join(process.cwd(), 'scripts/lib/opencode-live-preflight.mjs') + ).href; + return (await import(`${moduleUrl}?t=${Date.now()}`)) as OpenCodeLivePreflightTestHooks; +} + +async function writeExecutable(filePath: string, content: string): Promise { + await fs.writeFile(filePath, content, 'utf8'); + // eslint-disable-next-line sonarjs/file-permissions -- The taskkill fixture must be executable for child_process.spawn. + await fs.chmod(filePath, 0o755); +} + +function fakeTaskkillScript(): string { + return `#!/usr/bin/env node +const fs = require('node:fs'); + +fs.writeFileSync(process.env.FAKE_TASKKILL_ARGS_PATH, process.argv.slice(2).join(' ') + '\\n'); +process.exit(0); +`; +} + +function restoreEnvValue(name: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[name]; + return; + } + process.env[name] = value; +}