diff --git a/test/automation/pull-requests/pr-review-advisor-openshell.test.ts b/test/automation/pull-requests/pr-review-advisor-openshell.test.ts index 79b18e36a3f..0342cf9b224 100644 --- a/test/automation/pull-requests/pr-review-advisor-openshell.test.ts +++ b/test/automation/pull-requests/pr-review-advisor-openshell.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { execFileSync, spawnSync } from "node:child_process"; +import { EventEmitter } from "node:events"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -30,6 +31,7 @@ import { prepareAdvisorSandboxInputs, runAdvisorSandboxAsync, runOpenShellAdvisorCommand, + waitForAdvisorSandboxTermination, verifyAdvisorGitWorktree, } from "../../../tools/pr-review-advisor/openshell.mts"; import { @@ -525,6 +527,30 @@ describe("PR review advisor specialist lifecycle", () => { }); describe("PR review advisor OpenShell wrapper", () => { + it.each(["SIGTERM", "SIGINT"] as const)( + "initializes and keeps the sandbox entrypoint alive until OpenShell sends %s (#10791)", + async (signal) => { + const signals = new EventEmitter(); + const initialize = vi.fn(); + let settled = false; + const waiting = runOpenShellAdvisorCommand("initialize", initialize, () => + waitForAdvisorSandboxTermination(signals), + ).then(() => { + settled = true; + }); + + await Promise.resolve(); + expect(initialize).toHaveBeenCalledOnce(); + expect(settled).toBe(false); + + signals.emit(signal); + await waiting; + expect(settled).toBe(true); + expect(signals.listenerCount("SIGTERM")).toBe(0); + expect(signals.listenerCount("SIGINT")).toBe(0); + }, + ); + it("permits only the pinned image login files required by stable OpenShell exec", () => { const policy = YAML.parse( fs.readFileSync("tools/pr-review-advisor/openshell-policy.yaml", "utf8"), @@ -550,14 +576,6 @@ describe("PR review advisor OpenShell wrapper", () => { }); }); - it("dispatches sandbox runtime initialization", () => { - const initialize = vi.fn(); - - runOpenShellAdvisorCommand("initialize", initialize); - - expect(initialize).toHaveBeenCalledOnce(); - }); - it.each([ [undefined, "openshell command is required"], ["prepare", "Unsupported OpenShell advisor command: prepare"], @@ -569,10 +587,10 @@ describe("PR review advisor OpenShell wrapper", () => { ["delete", "Unsupported OpenShell advisor command: delete"], ["check", "Unsupported OpenShell advisor command: check"], ["unknown", "Unsupported OpenShell advisor command: unknown"], - ])("rejects unsupported OpenShell command %s", (command, message) => { + ])("rejects unsupported OpenShell command %s", async (command, message) => { const initialize = vi.fn(); - expect(() => runOpenShellAdvisorCommand(command, initialize)).toThrow(message); + await expect(runOpenShellAdvisorCommand(command, initialize)).rejects.toThrow(message); expect(initialize).not.toHaveBeenCalled(); }); diff --git a/tools/pr-review-advisor/openshell.mts b/tools/pr-review-advisor/openshell.mts index a379773641f..74a9e8d7b57 100755 --- a/tools/pr-review-advisor/openshell.mts +++ b/tools/pr-review-advisor/openshell.mts @@ -564,19 +564,40 @@ export function initializeAdvisorSandboxRuntime(): void { checkAdvisorSandboxRuntime(); } -export function runOpenShellAdvisorCommand( +export async function runOpenShellAdvisorCommand( command: string | undefined, initialize: () => void = initializeAdvisorSandboxRuntime, -): void { + waitForTermination: () => Promise = waitForAdvisorSandboxTermination, +): Promise { const requiredCommand = required(command, "openshell command"); if (requiredCommand !== "initialize") { throw new Error(`Unsupported OpenShell advisor command: ${requiredCommand}`); } initialize(); + await waitForTermination(); +} + +type AdvisorSandboxSignals = { + once(event: "SIGINT" | "SIGTERM", listener: () => void): unknown; + removeListener(event: "SIGINT" | "SIGTERM", listener: () => void): unknown; +}; + +export function waitForAdvisorSandboxTermination( + signals: AdvisorSandboxSignals = process, +): Promise { + return new Promise((resolve) => { + const finish = () => { + signals.removeListener("SIGTERM", finish); + signals.removeListener("SIGINT", finish); + resolve(); + }; + signals.once("SIGTERM", finish); + signals.once("SIGINT", finish); + }); } async function main(): Promise { - runOpenShellAdvisorCommand(process.argv[2]); + await runOpenShellAdvisorCommand(process.argv[2]); } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {