fix(opencode): harden Windows live preflight cleanup
This commit is contained in:
parent
678d12219a
commit
8fde8cefbf
4 changed files with 224 additions and 27 deletions
|
|
@ -4,6 +4,10 @@ import net from 'node:net';
|
||||||
import os from 'node:os';
|
import os from 'node:os';
|
||||||
import path from 'node:path';
|
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) {
|
export async function preflightOpenCodeLiveEnvironment(input) {
|
||||||
const repoRoot = input.repoRoot;
|
const repoRoot = input.repoRoot;
|
||||||
const opencodeBin = process.env.OPENCODE_BIN?.trim() || '/opt/homebrew/bin/opencode';
|
const opencodeBin = process.env.OPENCODE_BIN?.trim() || '/opt/homebrew/bin/opencode';
|
||||||
|
|
@ -139,44 +143,54 @@ async function canStartOpenCodeHost(opencodeBin, cwd, env) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function stopChild(child) {
|
async function stopChild(child, options = {}) {
|
||||||
if (child.exitCode != null || child.killed) {
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (process.platform === 'win32' && child.pid) {
|
if (platform === 'win32' && child.pid) {
|
||||||
await taskkillProcessTree(child.pid);
|
await killProcessTree(child.pid);
|
||||||
|
} else if (!child.killed) {
|
||||||
|
sendChildSignal(child, 'SIGTERM');
|
||||||
|
}
|
||||||
|
|
||||||
|
if (await waitForChildClose(child, closeGraceMs)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
return new Promise((resolve) => {
|
if (!hasChildExited(child)) {
|
||||||
const timeout = setTimeout(() => {
|
sendChildSignal(child, 'SIGKILL');
|
||||||
if (child.exitCode == null) {
|
if (!(await waitForChildClose(child, forceCloseGraceMs))) {
|
||||||
child.kill('SIGKILL');
|
child.stdout?.destroy();
|
||||||
}
|
child.stderr?.destroy();
|
||||||
resolve();
|
child.unref?.();
|
||||||
}, 3_000);
|
}
|
||||||
child.once('close', () => {
|
}
|
||||||
clearTimeout(timeout);
|
|
||||||
resolve();
|
|
||||||
});
|
|
||||||
child.kill('SIGTERM');
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function taskkillProcessTree(pid) {
|
function taskkillProcessTree(pid) {
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
let done = false;
|
let done = false;
|
||||||
|
let taskkill = null;
|
||||||
const finish = () => {
|
const finish = () => {
|
||||||
if (done) return;
|
if (done) return;
|
||||||
done = true;
|
done = true;
|
||||||
clearTimeout(timeout);
|
clearTimeout(timeout);
|
||||||
resolve();
|
resolve();
|
||||||
};
|
};
|
||||||
const timeout = setTimeout(finish, 5_000);
|
const timeout = setTimeout(() => {
|
||||||
timeout.unref?.();
|
if (taskkill) {
|
||||||
|
sendChildSignal(taskkill, 'SIGTERM');
|
||||||
|
}
|
||||||
|
finish();
|
||||||
|
}, TASKKILL_TIMEOUT_MS);
|
||||||
try {
|
try {
|
||||||
const taskkill = spawn(
|
taskkill = spawn(
|
||||||
path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'taskkill.exe'),
|
path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'taskkill.exe'),
|
||||||
['/T', '/F', '/PID', String(pid)],
|
['/T', '/F', '/PID', String(pid)],
|
||||||
{
|
{
|
||||||
|
|
@ -184,6 +198,7 @@ function taskkillProcessTree(pid) {
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
taskkill.unref?.();
|
||||||
taskkill.once('error', finish);
|
taskkill.once('error', finish);
|
||||||
taskkill.once('close', finish);
|
taskkill.once('close', finish);
|
||||||
} catch {
|
} catch {
|
||||||
|
|
@ -192,6 +207,36 @@ function taskkillProcessTree(pid) {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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() {
|
function allocateLoopbackPort() {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const server = net.createServer();
|
const server = net.createServer();
|
||||||
|
|
@ -224,3 +269,8 @@ function skip(reason) {
|
||||||
function compactOutput(value) {
|
function compactOutput(value) {
|
||||||
return value.replace(/\s+/g, ' ').trim().slice(0, 1_200);
|
return value.replace(/\s+/g, ' ').trim().slice(0, 1_200);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export const __opencodeLivePreflightTestHooks = {
|
||||||
|
stopChild,
|
||||||
|
taskkillProcessTree,
|
||||||
|
};
|
||||||
|
|
|
||||||
|
|
@ -152,10 +152,6 @@ import * as path from 'path';
|
||||||
import pidusage from 'pidusage';
|
import pidusage from 'pidusage';
|
||||||
import * as readline from 'readline';
|
import * as readline from 'readline';
|
||||||
|
|
||||||
// pidusage's Windows gwmi fallback needs a non-zero cache window to finish its
|
|
||||||
// initial two-sample pass. maxage: 0 can recurse forever on Windows.
|
|
||||||
const RUNTIME_PIDUSAGE_OPTIONS = process.platform === 'win32' ? { maxage: 1_000 } : { maxage: 0 };
|
|
||||||
|
|
||||||
import {
|
import {
|
||||||
ANTHROPIC_HELPER_MODE_COMPETING_AUTH_ENV_KEYS,
|
ANTHROPIC_HELPER_MODE_COMPETING_AUTH_ENV_KEYS,
|
||||||
type AnthropicTeamApiKeyHelperMaterial,
|
type AnthropicTeamApiKeyHelperMaterial,
|
||||||
|
|
@ -576,6 +572,10 @@ import type {
|
||||||
ToolCallMeta,
|
ToolCallMeta,
|
||||||
} from '@shared/types';
|
} from '@shared/types';
|
||||||
|
|
||||||
|
// pidusage's Windows gwmi fallback needs a non-zero cache window to finish its
|
||||||
|
// initial two-sample pass. maxage: 0 can recurse forever on Windows.
|
||||||
|
const RUNTIME_PIDUSAGE_OPTIONS = process.platform === 'win32' ? { maxage: 1_000 } : { maxage: 0 };
|
||||||
|
|
||||||
const logger = createLogger('Service:TeamProvisioning');
|
const logger = createLogger('Service:TeamProvisioning');
|
||||||
const PREFLIGHT_DEBUG_LOG_PATH = path.join(os.tmpdir(), 'claude-team-preflight-debug.log');
|
const PREFLIGHT_DEBUG_LOG_PATH = path.join(os.tmpdir(), 'claude-team-preflight-debug.log');
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -15453,9 +15453,9 @@ describe('TeamProvisioningService', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(spawnCli).toHaveBeenCalled();
|
expect(spawnCli).toHaveBeenCalled();
|
||||||
expect(progressUpdates[0]?.warnings).toEqual(expect.arrayContaining([
|
expect(progressUpdates[0]?.warnings).toEqual(
|
||||||
expect.stringContaining('9 primary teammates'),
|
expect.arrayContaining([expect.stringContaining('9 primary teammates')])
|
||||||
]));
|
);
|
||||||
expect(progressUpdates[0]?.warnings?.join('\n')).toContain('Launches above 8 teammates');
|
expect(progressUpdates[0]?.warnings?.join('\n')).toContain('Launches above 8 teammates');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
147
test/scripts/opencodeLivePreflight.test.ts
Normal file
147
test/scripts/opencodeLivePreflight.test.ts
Normal file
|
|
@ -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<void>;
|
||||||
|
closeGraceMs?: number;
|
||||||
|
forceCloseGraceMs?: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface OpenCodeLivePreflightTestHooks {
|
||||||
|
__opencodeLivePreflightTestHooks: {
|
||||||
|
stopChild(child: FakeChild, options?: StopChildOptions): Promise<void>;
|
||||||
|
taskkillProcessTree(pid: number): Promise<void>;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
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<OpenCodeLivePreflightTestHooks> {
|
||||||
|
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<void> {
|
||||||
|
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;
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue