fix(eval): make process termination work on Windows

packages/eval/src/__tests__/process-termination.test.ts

@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import type { ChildProcess } from 'node:child_process';
+import test from 'node:test';
+import { terminateProcess } from '../process-termination.js';
+
+function childProcess(pid = 123): {
+  child: ChildProcess;
+  signals: Array<NodeJS.Signals | undefined>;
+} {
+  const signals: Array<NodeJS.Signals | undefined> = [];
+  return {
+    child: {
+      pid,
+      exitCode: null,
+      signalCode: null,
+      kill: (signal?: NodeJS.Signals) => {
+        signals.push(signal);
+        return true;
+      },
+    } as unknown as ChildProcess,
+    signals,
+  };
+}
+
+test('uses taskkill for first-stage Windows termination so descendants cannot outlive the root', async () => {
+  const { child, signals } = childProcess();
+  const pids: number[] = [];
+
+  assert.equal(
+    await terminateProcess(child, 'SIGTERM', {
+      platform: 'win32',
+      runTaskkill: async (pid) => {
+        pids.push(pid);
+        return true;
+      },
+    }),
+    true,
+  );
+  assert.deepEqual(pids, [123]);
+  assert.deepEqual(signals, []);
+});
+
+test('uses taskkill for forced Windows termination and preserves the process tree', async () => {
+  const { child, signals } = childProcess(456);
+  const pids: number[] = [];
+
+  assert.equal(
+    await terminateProcess(child, 'SIGKILL', {
+      platform: 'win32',
+      runTaskkill: async (pid) => {
+        pids.push(pid);
+        return true;
+      },
+    }),
+    true,
+  );
+  assert.deepEqual(pids, [456]);
+  assert.deepEqual(signals, []);
+});
+
+test('falls back to the root process when Windows taskkill is unavailable', async () => {
+  const { child, signals } = childProcess();
+
+  assert.equal(
+    await terminateProcess(child, 'SIGKILL', {
+      platform: 'win32',
+      runTaskkill: async () => false,
+    }),
+    true,
+  );
+  assert.deepEqual(signals, [undefined]);
+});
+
+test('preserves POSIX signal semantics', async () => {
+  const { child, signals } = childProcess();
+
+  assert.equal(await terminateProcess(child, 'SIGTERM', { platform: 'linux' }), true);
+  assert.deepEqual(signals, ['SIGTERM']);
+});
+
+test('does not signal a child that has already exited', async () => {
+  const { child, signals } = childProcess();
+  Object.assign(child, { exitCode: 0 });
+
+  assert.equal(await terminateProcess(child, 'SIGKILL', { platform: 'win32' }), false);
+  assert.deepEqual(signals, []);
+});

packages/eval/src/__tests__/process-termination.windows.test.ts

@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { spawn } from 'node:child_process';
+import { mkdtemp, readFile, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import test from 'node:test';
+import { terminateProcess } from '../process-termination.js';
+
+const WINDOWS_PROCESS_SETTLEMENT_MS = 5_000;
+
+test('first-stage Windows termination removes a supervisor and its descendant', {
+  skip: process.platform !== 'win32',
+  timeout: 10_000,
+}, async () => {
+  const root = await mkdtemp(join(tmpdir(), 'maka-eval-process-tree-'));
+  const workerPidPath = join(root, 'worker.pid');
+  const supervisor = spawn(
+    process.execPath,
+    [
+      '-e',
+      `
+const { spawn } = require('node:child_process');
+const { writeFileSync } = require('node:fs');
+const worker = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
+  stdio: 'ignore',
+  windowsHide: true,
+});
+writeFileSync(process.argv[1], String(worker.pid));
+setInterval(() => {}, 1000);
+`,
+      workerPidPath,
+    ],
+    { stdio: 'ignore', windowsHide: true },
+  );
+  let workerPid: number | undefined;
+  try {
+    workerPid = await waitForWorkerPid(workerPidPath);
+    assert.equal(await terminateProcess(supervisor, 'SIGTERM'), true);
+    await waitForExit(supervisor);
+    await waitForProcessToExit(workerPid);
+  } finally {
+    if (supervisor.exitCode === null && supervisor.signalCode === null) {
+      await terminateProcess(supervisor, 'SIGKILL');
+    }
+    if (workerPid !== undefined && isProcessAlive(workerPid)) {
+      try {
+        process.kill(workerPid);
+      } catch {
+        // The worker may exit between the liveness check and cleanup.
+      }
+    }
+    await rm(root, { recursive: true, force: true });
+  }
+});
+
+async function waitForWorkerPid(path: string): Promise<number> {
+  const deadline = Date.now() + WINDOWS_PROCESS_SETTLEMENT_MS;
+  while (Date.now() < deadline) {
+    try {
+      const pid = Number(await readFile(path, 'utf8'));
+      if (Number.isSafeInteger(pid) && pid > 0) return pid;
+    } catch {
+      // The supervisor has not written its worker PID yet.
+    }
+    await delay(20);
+  }
+  throw new Error('supervisor did not report its worker PID');
+}
+
+async function waitForExit(child: ReturnType<typeof spawn>): Promise<void> {
+  const deadline = Date.now() + WINDOWS_PROCESS_SETTLEMENT_MS;
+  while (child.exitCode === null && child.signalCode === null && Date.now() < deadline) {
+    await delay(20);
+  }
+  assert.ok(
+    child.exitCode !== null || child.signalCode !== null,
+    'supervisor did not exit after taskkill',
+  );
+}
+
+async function waitForProcessToExit(pid: number): Promise<void> {
+  const deadline = Date.now() + WINDOWS_PROCESS_SETTLEMENT_MS;
+  while (isProcessAlive(pid) && Date.now() < deadline) await delay(20);
+  assert.equal(isProcessAlive(pid), false, `descendant ${pid} survived tree termination`);
+}
+
+function isProcessAlive(pid: number): boolean {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function delay(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}

packages/eval/src/harbor-external-subject.ts

@@ -39,6 +39,7 @@ import {
   type ProviderUsage as Usage,
 } from './provider-metering.js';
 import { removeEvalWebTools } from './provider-web-tool-surface.js';
+import { terminateProcess } from './process-termination.js';
 import { takeRelayResultToken, writeRelayResult } from './relay-result-frame.js';
 
 const resultToken = takeRelayResultToken();
@@ -340,10 +341,10 @@ if (!systemRoot?.startsWith('/')) throw new Error('external subject system root
 let credentialPath: string | undefined;
 let child: ChildProcess | undefined;
 let stopped = false;
-const stop = (signal: NodeJS.Signals) => {
+const stop = (signal: 'SIGINT' | 'SIGTERM') => {
   stopped = true;
   removeCredential();
-  child?.kill(signal);
+  void terminateProcess(child, signal);
 };
 const terminate = () => stop('SIGTERM');
 const interrupt = () => stop('SIGINT');

packages/eval/src/harness-executor.ts

@@ -48,6 +48,7 @@ import {
   type SubjectExecutionContext,
 } from './runner.js';
 import type { EvalResult } from './result.js';
+import { terminateProcess } from './process-termination.js';
 
 export type HarnessFramework = 'harbor' | 'pier';
 type RelayTransportStage = 'ready' | 'execute' | 'receive' | 'decision';
@@ -1195,7 +1196,7 @@ async function waitForTrial(child: ChildProcess, wait: TrialWait): Promise<Trial
     };
   }
 
-  child.kill('SIGTERM');
+  await terminateProcess(child, 'SIGTERM');
   const terminated = await within(exit, wait.deadlineMs);
   if (terminated) {
     return {
@@ -1205,7 +1206,7 @@ async function waitForTrial(child: ChildProcess, wait: TrialWait): Promise<Trial
       outcome: terminated.code === 0 && terminated.signal === null ? 'confirmed' : 'terminated',
     };
   }
-  child.kill('SIGKILL');
+  await terminateProcess(child, 'SIGKILL');
   const killed = await within(exit, KILL_SETTLEMENT_DEADLINE_MS);
   if (killed) {
     return {

packages/eval/src/install-preflight.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { execFile } from 'node:child_process';
+import { execFile, type ChildProcess } from 'node:child_process';
 import { constants } from 'node:fs';
 import { access, lstat, stat } from 'node:fs/promises';
 import { dirname, isAbsolute, resolve } from 'node:path';
@@ -28,6 +28,7 @@ import {
   resolveRealPathWithinRoot,
 } from './harness-environment.js';
 import type { HarnessFramework, HarnessOptions } from './harness-executor.js';
+import { terminateProcess } from './process-termination.js';
 
 const PREFLIGHT_TIMEOUT_MS = 10_000;
 const PREFLIGHT_OUTPUT_LIMIT_BYTES = 16 * 1024;
@@ -183,26 +184,49 @@ async function runCheckedCommand(
 ): Promise<void> {
   signal?.throwIfAborted();
   await new Promise<void>((resolvePromise, rejectPromise) => {
-    execFile(
-      command,
-      [...args],
-      {
-        env: environment,
-        cwd,
-        timeout: PREFLIGHT_TIMEOUT_MS,
-        killSignal: 'SIGKILL',
-        maxBuffer: PREFLIGHT_OUTPUT_LIMIT_BYTES,
-        encoding: 'utf8',
-        ...(signal ? { signal } : {}),
-      },
-      (error, _stdout, stderr) => {
-        if (!error) {
-          resolvePromise();
-          return;
-        }
-        rejectPromise(new Error(stderr.trim() || error.message));
-      },
-    );
+    let settled = false;
+    let timeout: NodeJS.Timeout | undefined;
+    let child: ChildProcess | undefined;
+    const onAbort = () => {
+      void terminateProcess(child, 'SIGTERM');
+    };
+    const settle = (error?: Error, stderr = '') => {
+      if (settled) return;
+      settled = true;
+      if (timeout) clearTimeout(timeout);
+      signal?.removeEventListener('abort', onAbort);
+      if (!error) {
+        resolvePromise();
+        return;
+      }
+      rejectPromise(new Error(stderr.trim() || error.message));
+    };
+    try {
+      child = execFile(
+        command,
+        [...args],
+        {
+          env: environment,
+          cwd,
+          killSignal: 'SIGTERM',
+          maxBuffer: PREFLIGHT_OUTPUT_LIMIT_BYTES,
+          encoding: 'utf8',
+        },
+        (error, _stdout, stderr) => {
+          settle(error ?? undefined, stderr);
+        },
+      );
+    } catch (error) {
+      settle(error instanceof Error ? error : new Error(String(error)));
+      return;
+    }
+    timeout = setTimeout(() => {
+      void terminateProcess(child, 'SIGKILL');
+    }, PREFLIGHT_TIMEOUT_MS);
+    if (signal) {
+      if (signal.aborted) onAbort();
+      else signal.addEventListener('abort', onAbort, { once: true });
+    }
   });
 }