Skip to content

Commit 834e90a

Browse files
fix(cli): check never hangs inside an agent's macOS sandbox (#5836)
Inside Codex's seatbelt the GPU probe's Metal Chrome is always killed, and when it dies after puppeteer's setAutoAttach is answered, puppeteer waits forever for its initial targets. The probe now answers software in a named seatbelt without starting Chrome, and every CLI browser launch is bounded by its timeout, so a stuck launch rejects and a signal can end the command.
1 parent 95a686c commit 834e90a

3 files changed

Lines changed: 51 additions & 6 deletions

File tree

‎packages/cli/src/browser/launch.noListen.test.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { EventEmitter } from "node:events";
22
import type { LaunchOptions, PuppeteerNode } from "puppeteer-core";
3-
import { describe, expect, it, vi } from "vitest";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
44

55
const probe = vi.hoisted(() => ({ options: [] as LaunchOptions[] }));
66

@@ -36,3 +36,19 @@ describe("the GPU probe where nothing may listen", () => {
3636
expect(probe.options[1]!.args).toEqual(["--headless"]);
3737
});
3838
});
39+
40+
describe("the GPU probe inside an agent's macOS seatbelt", () => {
41+
const platform = process.platform;
42+
afterEach(() => {
43+
vi.unstubAllEnvs();
44+
Object.defineProperty(process, "platform", { value: platform });
45+
});
46+
47+
it("answers software without starting a Chrome the seatbelt would crash", async () => {
48+
vi.stubEnv("CODEX_SANDBOX", "seatbelt");
49+
Object.defineProperty(process, "platform", { value: "darwin" });
50+
probe.options.length = 0;
51+
await expect(resolveManagedGpuMode("auto", "/chrome")).resolves.toBe("software");
52+
expect(probe.options).toEqual([]);
53+
});
54+
});

‎packages/cli/src/browser/launch.ts‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const closing = new WeakMap<Browser, Promise<void>>();
2222
const pending = new Map<Promise<Browser>, AbortController>();
2323
const sessionCleanups = new Set<() => Promise<void>>();
2424
const CLOSE_GRACE_MS = 1_000;
25+
const LAUNCH_TIMEOUT_MS = 30_000;
2526
const EXIT_POLL_MS = 50;
2627
let listening = false;
2728
let shutdown: Promise<void> | undefined;
@@ -57,6 +58,7 @@ export async function resolveManagedGpuMode(
5758
} = {},
5859
): Promise<ResolvedBrowserGpuMode> {
5960
ownSignals();
61+
if (requestedMode === "auto" && inAgentSandbox()) return "software";
6062
return resolveCaptureBrowserGpuMode(requestedMode, chromePath, {
6163
launch: async (puppeteer, options) => {
6264
context.signal?.throwIfAborted();
@@ -108,12 +110,18 @@ async function launchOwnedBrowser(
108110
ownSignals();
109111
const abort = new AbortController();
110112
const signal = options.signal ? AbortSignal.any([options.signal, abort.signal]) : abort.signal;
111-
const launch = puppeteer.launch({
112-
...options,
113+
const launch = untilLaunchOrAbort(
114+
puppeteer.launch({ ...options, signal, handleSIGINT: false, handleSIGTERM: false }),
113115
signal,
114-
handleSIGINT: false,
115-
handleSIGTERM: false,
116-
});
116+
);
117+
const limit = options.timeout ?? LAUNCH_TIMEOUT_MS;
118+
const timer =
119+
limit > 0
120+
? setTimeout(
121+
() => abort.abort(new Error(`The browser did not finish starting within ${limit} ms.`)),
122+
limit,
123+
)
124+
: undefined;
117125
pending.set(launch, abort);
118126
try {
119127
const browser = await launch;
@@ -123,10 +131,22 @@ async function launchOwnedBrowser(
123131
child.once("exit", () => browsers.delete(browser));
124132
return browser;
125133
} finally {
134+
clearTimeout(timer);
126135
pending.delete(launch);
127136
}
128137
}
129138

139+
function untilLaunchOrAbort(launch: Promise<Browser>, signal: AbortSignal): Promise<Browser> {
140+
const aborted = new Promise<never>((_, reject) => {
141+
if (signal.aborted) reject(signal.reason);
142+
signal.addEventListener("abort", () => reject(signal.reason), { once: true });
143+
});
144+
void launch
145+
.then((late) => (signal.aborted ? closeManagedBrowser(late) : undefined))
146+
.catch(() => undefined);
147+
return Promise.race([launch, aborted]);
148+
}
149+
130150
/** Resolved launches keep their signal live; only pending launches are aborted during shutdown. */
131151
async function closeManagedBrowsers(): Promise<void> {
132152
for (const abort of pending.values()) abort.abort();

‎packages/cli/src/browser/sandboxDenied.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,12 @@ describe("a browser inside an agent's macOS seatbelt", () => {
6969
expect(launches[1]!.args).toEqual(["--headless", "--single-process"]);
7070
});
7171
});
72+
73+
describe("a browser launch that never settles", () => {
74+
it("gives up at its timeout, as when Chrome dies while puppeteer waits for its first targets", async () => {
75+
const stuck = { launch: () => new Promise<Browser>(() => {}) } as unknown as PuppeteerNode;
76+
await expect(launchManagedBrowser(stuck, { timeout: 50 })).rejects.toThrow(
77+
"The browser did not finish starting within 50 ms.",
78+
);
79+
});
80+
});

0 commit comments

Comments
 (0)