From 3e5ca880bfe00cdd98cd61f337bc1b6bb346bed7 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 22 Jun 2026 02:29:26 +0200 Subject: [PATCH] fix(scripts): clamp parallels update timeouts --- scripts/e2e/parallels/npm-update-smoke.ts | 47 +++++++++++++++---- scripts/e2e/parallels/update-job-timeout.ts | 8 +++- .../parallels-npm-update-smoke.test.ts | 27 +++++++++++ .../parallels-update-job-timeout.test.ts | 23 +++++++++ 4 files changed, 93 insertions(+), 12 deletions(-) diff --git a/scripts/e2e/parallels/npm-update-smoke.ts b/scripts/e2e/parallels/npm-update-smoke.ts index e2d10f525584..20453eaf30e5 100755 --- a/scripts/e2e/parallels/npm-update-smoke.ts +++ b/scripts/e2e/parallels/npm-update-smoke.ts @@ -6,6 +6,11 @@ import { appendFileSync, existsSync, readFileSync, writeFileSync } from "node:fs import { copyFile, readFile, rm } from "node:fs/promises"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { + addTimerTimeoutGraceMs, + clampTimerTimeoutMs, + finiteSecondsToTimerSafeMilliseconds, +} from "@openclaw/normalization-core/number-coercion"; import { die, ensureValue, @@ -121,8 +126,24 @@ interface NpmUpdateSummary { const macosVmDefault = "macOS Tahoe"; const windowsVm = "Windows 11"; const linuxVmDefault = "Ubuntu 26.04"; + +function resolveRequiredTimerMs(timeoutMs: number): number { + return clampTimerTimeoutMs(timeoutMs) ?? 1; +} + +function resolveOptionalTimerMs(timeoutMs: number | undefined): number | undefined { + return timeoutMs === undefined || timeoutMs <= 0 ? undefined : resolveRequiredTimerMs(timeoutMs); +} + +function resolveSecondsTimerMs(timeoutSeconds: number): number { + return finiteSecondsToTimerSafeMilliseconds(timeoutSeconds) ?? 1; +} + const updateTimeoutSeconds = readPositiveIntEnv("OPENCLAW_PARALLELS_NPM_UPDATE_TIMEOUT_S", 1200); const updateCleanupBackstopMs = 60_000; +const updateTimeoutMs = resolveSecondsTimerMs(updateTimeoutSeconds); +const updateWithCleanupTimeoutMs = + addTimerTimeoutGraceMs(updateTimeoutMs, updateCleanupBackstopMs) ?? 1; const freshLaneTimeoutKillGraceMs = readPositiveIntEnv( "OPENCLAW_PARALLELS_NPM_UPDATE_FRESH_TIMEOUT_KILL_GRACE_MS", 2_000, @@ -133,7 +154,9 @@ let loggedExitCleanupInstalled = false; export function freshLaneTimeoutMs(platform: Platform): number { const defaultSeconds = platform === "windows" ? 90 * 60 : 75 * 60; - return readPositiveIntEnv("OPENCLAW_PARALLELS_NPM_UPDATE_FRESH_TIMEOUT_S", defaultSeconds) * 1000; + return resolveSecondsTimerMs( + readPositiveIntEnv("OPENCLAW_PARALLELS_NPM_UPDATE_FRESH_TIMEOUT_S", defaultSeconds), + ); } export function spawnLoggedCommand( @@ -161,19 +184,23 @@ export function spawnLoggedCommand( appendFileSync(logPath, text, "utf8"); onOutput(text); }; - const timeoutMs = options.timeoutMs ?? 0; + const timeoutMs = resolveOptionalTimerMs(options.timeoutMs); + const timeoutKillGraceMs = + options.timeoutKillGraceMs === undefined + ? resolveRequiredTimerMs(freshLaneTimeoutKillGraceMs) + : resolveRequiredTimerMs(options.timeoutKillGraceMs); const timeoutTimer = - timeoutMs > 0 + timeoutMs !== undefined ? setTimeout(() => { timedOut = true; append( `\n[${options.timeoutLabel ?? `${command} ${args.join(" ")}`} timed out after ${timeoutMs}ms]\n`, ); signalLoggedChild(child, "SIGTERM"); - forceKillAt = Date.now() + (options.timeoutKillGraceMs ?? freshLaneTimeoutKillGraceMs); + forceKillAt = Date.now() + timeoutKillGraceMs; forceKillTimer = setTimeout( () => signalLoggedChild(child, "SIGKILL"), - options.timeoutKillGraceMs ?? freshLaneTimeoutKillGraceMs, + timeoutKillGraceMs, ); }, timeoutMs) : undefined; @@ -208,7 +235,7 @@ export function spawnLoggedCommand( if (timedOut) { void finishTimedOutLoggedProcessTree(child, { forceKillAt, - timeoutKillGraceMs: options.timeoutKillGraceMs ?? freshLaneTimeoutKillGraceMs, + timeoutKillGraceMs, }).then(finish, finish); return; } @@ -926,7 +953,7 @@ export class NpmUpdateSmoke { label, run: ({ signal }) => fn({ append, logPath, signal }), timeoutDescription: `${updateTimeoutSeconds}s plus cleanup backstop`, - timeoutMs: updateTimeoutSeconds * 1000 + updateCleanupBackstopMs, + timeoutMs: updateWithCleanupTimeoutMs, writeLog: async () => undefined, }); })().finally(() => { @@ -937,15 +964,15 @@ export class NpmUpdateSmoke { } private async runMacosUpdate(ctx: UpdateJobContext): Promise { - await this.guestMacos(this.updateScript("macos"), updateTimeoutSeconds * 1000, ctx); + await this.guestMacos(this.updateScript("macos"), updateTimeoutMs, ctx); } private runWindowsUpdate(ctx: UpdateJobContext): Promise { - return this.guestWindows(this.updateScript("windows"), updateTimeoutSeconds * 1000, ctx); + return this.guestWindows(this.updateScript("windows"), updateTimeoutMs, ctx); } private async runLinuxUpdate(ctx: UpdateJobContext): Promise { - await this.guestLinux(this.updateScript("linux"), updateTimeoutSeconds * 1000, ctx); + await this.guestLinux(this.updateScript("linux"), updateTimeoutMs, ctx); } private updateScript(platform: Platform): string { diff --git a/scripts/e2e/parallels/update-job-timeout.ts b/scripts/e2e/parallels/update-job-timeout.ts index 809f686c5774..57c6b61a86db 100644 --- a/scripts/e2e/parallels/update-job-timeout.ts +++ b/scripts/e2e/parallels/update-job-timeout.ts @@ -1,4 +1,6 @@ // Update Job Timeout script supports OpenClaw repository automation. +import { resolveTimerTimeoutMs } from "@openclaw/normalization-core/number-coercion"; + interface TimedUpdateJobOptions { abortSettleMs?: number; append(this: void, chunk: string): void; @@ -21,6 +23,8 @@ export async function runTimedUpdateJob({ let timedOut = false; const controller = new AbortController(); const timeoutMessage = `${label} update timed out after ${timeoutDescription}`; + const resolvedAbortSettleMs = resolveTimerTimeoutMs(abortSettleMs, 0, 0); + const resolvedTimeoutMs = resolveTimerTimeoutMs(timeoutMs, 1); let timeout: NodeJS.Timeout | undefined; const runOutcome = Promise.resolve() .then(() => run({ signal: controller.signal })) @@ -34,13 +38,13 @@ export async function runTimedUpdateJob({ append(`${timeoutMessage}\n`); controller.abort(new Error(timeoutMessage)); resolve("timeout"); - }, timeoutMs); + }, resolvedTimeoutMs); }); try { const outcome = await Promise.race([runOutcome, timeoutPromise]); if (outcome === "timeout") { - await waitForAbortSettle(runOutcome, abortSettleMs); + await waitForAbortSettle(runOutcome, resolvedAbortSettleMs); await writeLog(); return 1; } diff --git a/test/scripts/parallels-npm-update-smoke.test.ts b/test/scripts/parallels-npm-update-smoke.test.ts index 5761228f6288..dfdff6f83821 100644 --- a/test/scripts/parallels-npm-update-smoke.test.ts +++ b/test/scripts/parallels-npm-update-smoke.test.ts @@ -2,6 +2,10 @@ import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; +import { + MAX_TIMER_TIMEOUT_MS, + MAX_TIMER_TIMEOUT_SECONDS, +} from "@openclaw/normalization-core/number-coercion"; import { afterEach, describe, expect, it, vi } from "vitest"; import { runWindowsBackgroundPowerShell } from "../../scripts/e2e/parallels/guest-transports.ts"; import { run as hostCommandRun } from "../../scripts/e2e/parallels/host-command.ts"; @@ -405,6 +409,29 @@ exit 1 withEnv({ OPENCLAW_PARALLELS_NPM_UPDATE_FRESH_TIMEOUT_S: "3" }, () => { expect(freshLaneTimeoutMs("macos")).toBe(3000); }); + + withEnv( + { OPENCLAW_PARALLELS_NPM_UPDATE_FRESH_TIMEOUT_S: String(MAX_TIMER_TIMEOUT_SECONDS + 1) }, + () => { + expect(freshLaneTimeoutMs("linux")).toBe(MAX_TIMER_TIMEOUT_MS); + }, + ); + }); + + it("clamps oversized fresh lane command timeouts before scheduling", async () => { + const root = makeTempDir(); + const logPath = path.join(root, "fresh.log"); + + const code = await spawnLoggedCommand( + process.execPath, + ["-e", "setTimeout(() => process.exit(0), 25);"], + logPath, + {}, + undefined, + { timeoutMs: MAX_TIMER_TIMEOUT_MS + 1 }, + ); + + expect(code).toBe(0); }); it.runIf(process.platform !== "win32")("times out fresh lane process groups", async () => { diff --git a/test/scripts/parallels-update-job-timeout.test.ts b/test/scripts/parallels-update-job-timeout.test.ts index af3bbe21188a..9e9233f253cb 100644 --- a/test/scripts/parallels-update-job-timeout.test.ts +++ b/test/scripts/parallels-update-job-timeout.test.ts @@ -2,6 +2,7 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; import { afterEach, describe, expect, it, vi } from "vitest"; import { runTimedUpdateJob } from "../../scripts/e2e/parallels/update-job-timeout.ts"; @@ -29,6 +30,28 @@ describe("Parallels update job timeout", () => { expect(writeLog).toHaveBeenCalledTimes(1); }); + it("clamps oversized update job timers before scheduling", async () => { + const chunks: string[] = []; + const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout"); + const writeLog = vi.fn(async () => undefined); + + try { + await expect( + runTimedUpdateJob({ + append: (chunk) => chunks.push(chunk), + label: "Linux", + run: async () => undefined, + timeoutDescription: "oversized", + timeoutMs: MAX_TIMER_TIMEOUT_MS + 1, + writeLog, + }), + ).resolves.toBe(0); + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_TIMER_TIMEOUT_MS); + } finally { + setTimeoutSpy.mockRestore(); + } + }); + it("records update failures and writes the job log", async () => { const chunks: string[] = []; const writeLog = vi.fn(async () => undefined);