From 8cecf2c7eafddf709e8438bd82d7ed8bdd732ddc Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 2 Jun 2026 22:48:12 +0200 Subject: [PATCH] fix(test): reject malformed local check limits --- scripts/lib/local-heavy-check-runtime.mjs | 25 ++++++++++++++++--- scripts/lib/vitest-local-scheduling.mjs | 23 ++++++++++++++--- .../scripts/local-heavy-check-runtime.test.ts | 19 ++++++++++++++ test/scripts/vitest-local-scheduling.test.ts | 16 ++++++++++++ 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/scripts/lib/local-heavy-check-runtime.mjs b/scripts/lib/local-heavy-check-runtime.mjs index e2ca524ed9b..2ec38ca6958 100644 --- a/scripts/lib/local-heavy-check-runtime.mjs +++ b/scripts/lib/local-heavy-check-runtime.mjs @@ -201,15 +201,22 @@ export function acquireLocalHeavyCheckLockSync(params) { const timeoutMs = readPositiveInt( env.OPENCLAW_HEAVY_CHECK_LOCK_TIMEOUT_MS, DEFAULT_LOCK_TIMEOUT_MS, + "OPENCLAW_HEAVY_CHECK_LOCK_TIMEOUT_MS", + ); + const pollMs = readPositiveInt( + env.OPENCLAW_HEAVY_CHECK_LOCK_POLL_MS, + DEFAULT_LOCK_POLL_MS, + "OPENCLAW_HEAVY_CHECK_LOCK_POLL_MS", ); - const pollMs = readPositiveInt(env.OPENCLAW_HEAVY_CHECK_LOCK_POLL_MS, DEFAULT_LOCK_POLL_MS); const progressMs = readPositiveInt( env.OPENCLAW_HEAVY_CHECK_LOCK_PROGRESS_MS, DEFAULT_LOCK_PROGRESS_MS, + "OPENCLAW_HEAVY_CHECK_LOCK_PROGRESS_MS", ); const staleLockMs = readPositiveInt( env.OPENCLAW_HEAVY_CHECK_STALE_LOCK_MS, DEFAULT_STALE_LOCK_MS, + "OPENCLAW_HEAVY_CHECK_STALE_LOCK_MS", ); const startedAt = Date.now(); let waitLogBudget = 1; @@ -374,9 +381,19 @@ function resolveHostResources(hostResources) { }; } -function readPositiveInt(rawValue, fallback) { - const parsed = Number.parseInt(rawValue ?? "", 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +function readPositiveInt(rawValue, fallback, label) { + const text = rawValue?.trim(); + if (!text) { + return fallback; + } + if (!/^\d+$/u.test(text)) { + throw new Error(`${label} must be a positive integer; got: ${rawValue}`); + } + const parsed = Number(text); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + throw new Error(`${label} must be a positive integer; got: ${rawValue}`); + } + return parsed; } function writeOwnerFile(ownerPath, owner) { diff --git a/scripts/lib/vitest-local-scheduling.mjs b/scripts/lib/vitest-local-scheduling.mjs index 09893e71841..d0cb8e5e2a1 100644 --- a/scripts/lib/vitest-local-scheduling.mjs +++ b/scripts/lib/vitest-local-scheduling.mjs @@ -10,9 +10,19 @@ const LARGE_LOCAL_FULL_SUITE_VITEST_WORKERS = 2; const clamp = (value, min, max) => Math.max(min, Math.min(max, value)); -function parsePositiveInt(value) { - const parsed = Number.parseInt(value ?? "", 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : null; +function parsePositiveInt(value, label) { + const text = value?.trim(); + if (!text) { + return null; + } + if (!/^\d+$/u.test(text)) { + throw new Error(`${label} must be a positive integer; got: ${value}`); + } + const parsed = Number(text); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + throw new Error(`${label} must be a positive integer; got: ${value}`); + } + return parsed; } function isSystemThrottleDisabled(env) { @@ -79,7 +89,12 @@ export function resolveLocalVitestScheduling( system = detectVitestHostInfo(), pool = "threads", ) { - const override = parsePositiveInt(env.OPENCLAW_VITEST_MAX_WORKERS ?? env.OPENCLAW_TEST_WORKERS); + const override = parsePositiveInt( + env.OPENCLAW_VITEST_MAX_WORKERS ?? env.OPENCLAW_TEST_WORKERS, + env.OPENCLAW_VITEST_MAX_WORKERS === undefined + ? "OPENCLAW_TEST_WORKERS" + : "OPENCLAW_VITEST_MAX_WORKERS", + ); if (override !== null) { const maxWorkers = clamp(override, 1, 16); return { diff --git a/test/scripts/local-heavy-check-runtime.test.ts b/test/scripts/local-heavy-check-runtime.test.ts index d7e18b079ca..6962a1b79ca 100644 --- a/test/scripts/local-heavy-check-runtime.test.ts +++ b/test/scripts/local-heavy-check-runtime.test.ts @@ -397,6 +397,25 @@ describe("local-heavy-check-runtime", () => { expect(fs.existsSync(worktreeLockDir)).toBe(false); }); + it("rejects malformed heavy-check lock timing env values", () => { + const cwd = createTempDir("openclaw-local-heavy-check-malformed-env-"); + + expect(() => + acquireLocalHeavyCheckLockSync({ + cwd, + env: makeEnv({ OPENCLAW_HEAVY_CHECK_LOCK_TIMEOUT_MS: "10ms" }), + toolName: "oxlint", + }), + ).toThrow("OPENCLAW_HEAVY_CHECK_LOCK_TIMEOUT_MS must be a positive integer; got: 10ms"); + expect(() => + acquireLocalHeavyCheckLockSync({ + cwd, + env: makeEnv({ OPENCLAW_HEAVY_CHECK_LOCK_POLL_MS: "0" }), + toolName: "oxlint", + }), + ).toThrow("OPENCLAW_HEAVY_CHECK_LOCK_POLL_MS must be a positive integer; got: 0"); + }); + it("cleans up stale legacy test locks when acquiring the shared heavy-check lock", () => { const cwd = createTempDir("openclaw-local-heavy-check-legacy-"); const commonDir = path.join(cwd, ".git"); diff --git a/test/scripts/vitest-local-scheduling.test.ts b/test/scripts/vitest-local-scheduling.test.ts index d8dcd72f6f1..82f1301caa1 100644 --- a/test/scripts/vitest-local-scheduling.test.ts +++ b/test/scripts/vitest-local-scheduling.test.ts @@ -138,4 +138,20 @@ describe("vitest local full-suite profile", () => { vitestMaxWorkers: 1, }); }); + + it("rejects malformed explicit worker limits", () => { + const hostInfo = { + cpuCount: 10, + loadAverage1m: 0, + totalMemoryBytes: 24 * 1024 ** 3, + freeMemoryBytes: 12 * 1024 ** 3, + }; + + expect(() => + resolveLocalVitestScheduling({ OPENCLAW_VITEST_MAX_WORKERS: "8x" }, hostInfo, "threads"), + ).toThrow("OPENCLAW_VITEST_MAX_WORKERS must be a positive integer; got: 8x"); + expect(() => + resolveLocalVitestScheduling({ OPENCLAW_TEST_WORKERS: "1e0" }, hostInfo, "threads"), + ).toThrow("OPENCLAW_TEST_WORKERS must be a positive integer; got: 1e0"); + }); });