Skip to content

Commit d3ab7e9

Browse files
vincentkocvincentkoc
committed
fix(ci): harden ARM smoke and browser checks
1 parent acacd32 commit d3ab7e9

15 files changed

Lines changed: 473 additions & 54 deletions

‎scripts/ensure-playwright-chromium.mjs‎

Lines changed: 87 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,23 @@ import { chromium } from "playwright";
77
import { resolvePnpmRunner } from "./pnpm-runner.mjs";
88

99
const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
10-
const playwrightInstallArgs = ["--dir", "ui", "exec", "playwright", "install", "chromium"];
10+
const playwrightInstallArgs = [
11+
"--dir",
12+
"ui",
13+
"exec",
14+
"playwright",
15+
"install",
16+
"chromium",
17+
];
18+
const playwrightInstallWithDepsArgs = [
19+
"--dir",
20+
"ui",
21+
"exec",
22+
"playwright",
23+
"install",
24+
"--with-deps",
25+
"chromium",
26+
];
1127
const executableOverrideEnvKey = "PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH";
1228
export const systemChromiumExecutableCandidates = [
1329
"/snap/bin/chromium",
@@ -17,8 +33,22 @@ export const systemChromiumExecutableCandidates = [
1733
"/usr/bin/google-chrome-stable",
1834
];
1935

20-
export function resolveSystemChromiumExecutablePath(existsSync = existsSyncImpl) {
21-
return systemChromiumExecutableCandidates.find((candidate) => existsSync(candidate)) ?? "";
36+
export function canRunChromiumExecutable(executablePath, spawnSync = spawnSyncImpl) {
37+
const result = spawnSync(executablePath, ["--version"], {
38+
stdio: "ignore",
39+
});
40+
return result.status === 0;
41+
}
42+
43+
export function resolveSystemChromiumExecutablePath(
44+
existsSync = existsSyncImpl,
45+
spawnSync = spawnSyncImpl,
46+
) {
47+
return (
48+
systemChromiumExecutableCandidates.find(
49+
(candidate) => existsSync(candidate) && canRunChromiumExecutable(candidate, spawnSync),
50+
) ?? ""
51+
);
2252
}
2353

2454
export function resolvePlaywrightInstallRunner(options = {}) {
@@ -27,10 +57,23 @@ export function resolvePlaywrightInstallRunner(options = {}) {
2757
comSpec: options.comSpec ?? env.ComSpec ?? env.COMSPEC,
2858
npmExecPath: env === process.env ? env.npm_execpath : (env.npm_execpath ?? ""),
2959
platform: options.platform,
30-
pnpmArgs: playwrightInstallArgs,
60+
pnpmArgs: options.withDeps ? playwrightInstallWithDepsArgs : playwrightInstallArgs,
3161
});
3262
}
3363

64+
export function shouldInstallPlaywrightSystemDependencies(options = {}) {
65+
const env = options.env ?? process.env;
66+
const platform = options.platform ?? process.platform;
67+
const getuid = options.getuid ?? process.getuid;
68+
if (platform !== "linux") {
69+
return false;
70+
}
71+
if (typeof getuid === "function" && getuid() === 0) {
72+
return true;
73+
}
74+
return env.CI === "true" || env.GITHUB_ACTIONS === "true";
75+
}
76+
3477
export function isDirectScriptExecution(
3578
argvEntry = process.argv[1],
3679
modulePath = fileURLToPath(import.meta.url),
@@ -56,22 +99,25 @@ export function ensurePlaywrightChromium(options = {}) {
5699
const spawnSync = options.spawnSync ?? spawnSyncImpl;
57100

58101
if (executableOverride) {
59-
if (existsSync(executableOverride)) {
102+
if (existsSync(executableOverride) && canRunChromiumExecutable(executableOverride, spawnSync)) {
60103
return 0;
61104
}
62105
log(
63-
`[ui-e2e] ${executableOverrideEnvKey} points to ${executableOverride}, but that browser does not exist.`,
106+
`[ui-e2e] ${executableOverrideEnvKey} points to ${executableOverride}, but that browser is not runnable.`,
64107
);
65108
return 1;
66109
}
67110

68-
if (existsSync(executablePath)) {
111+
if (existsSync(executablePath) && canRunChromiumExecutable(executablePath, spawnSync)) {
69112
return 0;
70113
}
71114

72115
const systemExecutablePath =
73-
options.systemExecutablePath ?? resolveSystemChromiumExecutablePath(existsSync);
74-
if (systemExecutablePath) {
116+
options.systemExecutablePath ?? resolveSystemChromiumExecutablePath(existsSync, spawnSync);
117+
if (
118+
systemExecutablePath &&
119+
canRunChromiumExecutable(systemExecutablePath, spawnSync)
120+
) {
75121
log(`[ui-e2e] Using system Chromium at ${systemExecutablePath}.`);
76122
return 0;
77123
}
@@ -83,7 +129,7 @@ export function ensurePlaywrightChromium(options = {}) {
83129
return 0;
84130
}
85131

86-
log(`[ui-e2e] Playwright Chromium is missing at ${executablePath}; installing chromium.`);
132+
log(`[ui-e2e] Playwright Chromium is not runnable at ${executablePath}; installing chromium.`);
87133
const runner = resolvePlaywrightInstallRunner({
88134
comSpec: options.comSpec,
89135
env,
@@ -101,9 +147,38 @@ export function ensurePlaywrightChromium(options = {}) {
101147
return status;
102148
}
103149

104-
if (!existsSync(executablePath)) {
150+
if (!existsSync(executablePath) || !canRunChromiumExecutable(executablePath, spawnSync)) {
151+
if (shouldInstallPlaywrightSystemDependencies({
152+
env,
153+
getuid: options.getuid,
154+
platform: options.platform,
155+
})) {
156+
log(
157+
`[ui-e2e] Chromium is installed but still cannot start; installing Linux system dependencies.`,
158+
);
159+
const depsRunner = resolvePlaywrightInstallRunner({
160+
comSpec: options.comSpec,
161+
env,
162+
platform: options.platform,
163+
withDeps: true,
164+
});
165+
const depsResult = spawnSync(depsRunner.command, depsRunner.args, {
166+
cwd: options.cwd ?? repoRoot,
167+
env,
168+
shell: depsRunner.shell,
169+
stdio: options.stdio ?? "inherit",
170+
windowsVerbatimArguments: depsRunner.windowsVerbatimArguments,
171+
});
172+
const depsStatus = depsResult.status ?? 1;
173+
if (depsStatus !== 0) {
174+
return depsStatus;
175+
}
176+
if (existsSync(executablePath) && canRunChromiumExecutable(executablePath, spawnSync)) {
177+
return 0;
178+
}
179+
}
105180
log(
106-
`[ui-e2e] Playwright install completed but Chromium is still missing at ${executablePath}.`,
181+
`[ui-e2e] Playwright install completed but Chromium is still not runnable at ${executablePath}.`,
107182
);
108183
return 1;
109184
}

‎scripts/test-cleanup-docker.sh‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,37 @@ ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
55
source "$ROOT_DIR/scripts/lib/docker-build.sh"
66
source "$ROOT_DIR/scripts/lib/docker-e2e-container.sh"
77
IMAGE_NAME="${OPENCLAW_CLEANUP_SMOKE_IMAGE:-openclaw-cleanup-smoke:local}"
8-
PLATFORM="${OPENCLAW_CLEANUP_SMOKE_PLATFORM:-linux/amd64}"
98
DOCKER_COMMAND_TIMEOUT="${DOCKER_COMMAND_TIMEOUT:-${OPENCLAW_CLEANUP_SMOKE_DOCKER_TIMEOUT:-600s}}"
109

10+
resolve_default_cleanup_platform() {
11+
local host_arch
12+
if [[ -n "${OPENCLAW_CLEANUP_SMOKE_PLATFORM:-}" ]]; then
13+
printf "%s" "$OPENCLAW_CLEANUP_SMOKE_PLATFORM"
14+
return
15+
fi
16+
host_arch="$(uname -m)"
17+
if [[ "${CI:-}" == "true" || "${GITHUB_ACTIONS:-}" == "true" ]]; then
18+
case "$host_arch" in
19+
arm64 | aarch64)
20+
printf "linux/arm64"
21+
return
22+
;;
23+
esac
24+
printf "linux/amd64"
25+
return
26+
fi
27+
case "$host_arch" in
28+
arm64 | aarch64)
29+
printf "linux/arm64"
30+
;;
31+
*)
32+
printf "linux/amd64"
33+
;;
34+
esac
35+
}
36+
37+
PLATFORM="$(resolve_default_cleanup_platform)"
38+
1139
echo "==> Build image: $IMAGE_NAME"
1240
docker_build_run cleanup-build \
1341
-t "$IMAGE_NAME" \

‎scripts/test-install-sh-docker.sh‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,30 @@ run_install_smoke_container() {
1414
}
1515

1616
resolve_default_smoke_platform() {
17-
local host_os
1817
local host_arch
1918
if [[ -n "${OPENCLAW_INSTALL_SMOKE_PLATFORM:-}" ]]; then
2019
printf "%s" "$OPENCLAW_INSTALL_SMOKE_PLATFORM"
2120
return
2221
fi
22+
host_arch="$(uname -m)"
2323
if [[ "${CI:-}" == "true" || "${GITHUB_ACTIONS:-}" == "true" ]]; then
24+
case "$host_arch" in
25+
arm64 | aarch64)
26+
printf "linux/arm64"
27+
return
28+
;;
29+
esac
2430
printf "linux/amd64"
2531
return
2632
fi
27-
host_os="$(uname -s)"
28-
host_arch="$(uname -m)"
29-
if [[ "$host_os" == "Darwin" && "$host_arch" == "arm64" ]]; then
30-
printf "linux/arm64"
31-
return
32-
fi
33-
printf "linux/amd64"
33+
case "$host_arch" in
34+
arm64 | aarch64)
35+
printf "linux/arm64"
36+
;;
37+
*)
38+
printf "linux/amd64"
39+
;;
40+
esac
3441
}
3542

3643
print_pack_audit() {

‎src/agents/bash-tools.exec-gateway-approval.e2e.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ const TEST_ENV_KEYS = [
3030
"OPENCLAW_SKIP_PROVIDERS",
3131
"OPENCLAW_TEST_MINIMAL_GATEWAY",
3232
];
33+
const GATEWAY_CONNECT_TIMEOUT_MS = 120_000;
34+
const EXEC_APPROVAL_E2E_TIMEOUT_MS = 180_000;
3335

3436
type Cleanup = () => Promise<void> | void;
3537

@@ -128,7 +130,8 @@ describe("gateway-hosted exec approvals", () => {
128130
clientDisplayName: "approval operator",
129131
mode: GATEWAY_CLIENT_MODES.TEST,
130132
scopes: [ADMIN_SCOPE],
131-
timeoutMs: 60_000,
133+
requestTimeoutMs: GATEWAY_CONNECT_TIMEOUT_MS,
134+
timeoutMs: GATEWAY_CONNECT_TIMEOUT_MS,
132135
});
133136
cleanup.push(() => disconnectGatewayClient(operator));
134137

@@ -171,5 +174,5 @@ describe("gateway-hosted exec approvals", () => {
171174
expect(outcome.status).toBe("completed");
172175
expect(outcome.exitCode).toBe(0);
173176
expect(outcome.aggregated).toBe("smoke");
174-
}, 120_000);
177+
}, EXEC_APPROVAL_E2E_TIMEOUT_MS);
175178
});

‎src/agents/bash-tools.exec.resolve-env-hook.test.ts‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const mocks = vi.hoisted(() => ({
66
hookRunner: undefined as
77
| {
88
hasHooks: ReturnType<typeof vi.fn>;
9-
runResolveExecEnv: ReturnType<typeof vi.fn>;
9+
runResolveExecEnv?: ReturnType<typeof vi.fn>;
1010
runBeforeToolCall?: ReturnType<typeof vi.fn>;
1111
}
1212
| undefined,
@@ -245,7 +245,7 @@ describe("exec resolve_exec_env hook wiring", () => {
245245
expect(mocks.beforeToolCallParams[0]?.env).toEqual({
246246
EXISTING: "request",
247247
});
248-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenCalledTimes(1);
248+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenCalledTimes(1);
249249
expect(mocks.gatewayParams[0]?.requestedEnv).toEqual({
250250
EXISTING: "request",
251251
PLUGIN_SAFE: "yes",
@@ -293,7 +293,7 @@ describe("exec resolve_exec_env hook wiring", () => {
293293
expect(mocks.beforeToolCallParams[0]?.env).toEqual({
294294
REQUEST_SAFE: "request",
295295
});
296-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenCalledTimes(1);
296+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenCalledTimes(1);
297297
expect(mocks.gatewayParams[0]?.requestedEnv).toEqual({
298298
LAZY_PLUGIN_SAFE: "yes",
299299
REQUEST_SAFE: "request",
@@ -335,13 +335,13 @@ describe("exec resolve_exec_env hook wiring", () => {
335335
testExtensionContext,
336336
);
337337

338-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenCalledTimes(2);
339-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenNthCalledWith(
338+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenCalledTimes(2);
339+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenNthCalledWith(
340340
1,
341341
expect.objectContaining({ host: "gateway" }),
342342
expect.anything(),
343343
);
344-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenNthCalledWith(
344+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenNthCalledWith(
345345
2,
346346
expect.objectContaining({ host: "node" }),
347347
expect.anything(),
@@ -386,7 +386,29 @@ describe("exec resolve_exec_env hook wiring", () => {
386386
testExtensionContext,
387387
);
388388

389-
expect(mocks.hookRunner.runResolveExecEnv).not.toHaveBeenCalled();
389+
expect(mocks.hookRunner.runResolveExecEnv!).not.toHaveBeenCalled();
390+
expect(mocks.gatewayParams[0]?.requestedEnv).toEqual({
391+
REQUEST_SAFE: "request",
392+
});
393+
});
394+
395+
it("skips stale hook runners that report resolve_exec_env without the runner method", async () => {
396+
mocks.hookRunner = {
397+
hasHooks: vi.fn((hookName: string) => hookName === "resolve_exec_env"),
398+
};
399+
400+
const tool = createExecTool({
401+
host: "gateway",
402+
security: "full",
403+
ask: "off",
404+
sessionKey: "agent:main:telegram:chat-1",
405+
});
406+
await tool.execute("call-stale-hook-runner", {
407+
command: "echo ok",
408+
env: { REQUEST_SAFE: "request" },
409+
yieldMs: 120_000,
410+
});
411+
390412
expect(mocks.gatewayParams[0]?.requestedEnv).toEqual({
391413
REQUEST_SAFE: "request",
392414
});
@@ -431,7 +453,7 @@ describe("exec resolve_exec_env hook wiring", () => {
431453
expect(mocks.beforeToolCallParams[0]?.env).toEqual({
432454
REQUEST_SAFE: "request",
433455
});
434-
expect(mocks.hookRunner.runResolveExecEnv).toHaveBeenCalledTimes(1);
456+
expect(mocks.hookRunner.runResolveExecEnv!).toHaveBeenCalledTimes(1);
435457
expect(mocks.gatewayParams[0]?.requestedEnv).toEqual({
436458
PLUGIN_SAFE: "yes",
437459
REQUEST_SAFE: "request",

‎src/agents/bash-tools.exec.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1411,7 +1411,10 @@ export function createExecTool(
14111411
return markResolveExecEnvPrepared(params);
14121412
}
14131413
const hookRunner = getGlobalHookRunner();
1414-
if (!hookRunner?.hasHooks("resolve_exec_env")) {
1414+
if (
1415+
!hookRunner?.hasHooks("resolve_exec_env") ||
1416+
typeof hookRunner.runResolveExecEnv !== "function"
1417+
) {
14151418
return markResolveExecEnvPrepared(params);
14161419
}
14171420
let host: ExecHost;

‎src/gateway/test-helpers.e2e.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export async function connectGatewayClient(params: {
4646
deviceIdentity?: DeviceIdentity;
4747
onEvent?: (evt: { event?: string; payload?: unknown }) => void;
4848
connectChallengeTimeoutMs?: number;
49+
requestTimeoutMs?: number;
4950
timeoutMs?: number;
5051
timeoutMessage?: string;
5152
}) {
@@ -90,6 +91,7 @@ export async function connectGatewayClient(params: {
9091
...(params.connectChallengeTimeoutMs !== undefined
9192
? { connectChallengeTimeoutMs: params.connectChallengeTimeoutMs }
9293
: {}),
94+
...(params.requestTimeoutMs !== undefined ? { requestTimeoutMs: params.requestTimeoutMs } : {}),
9395
clientName: params.clientName ?? GATEWAY_CLIENT_NAMES.TEST,
9496
clientDisplayName: params.clientDisplayName ?? "vitest",
9597
clientVersion: params.clientVersion ?? "dev",

0 commit comments

Comments
 (0)