From 9918dd77c4e489a2b07d4d4fb4620d9b242df701 Mon Sep 17 00:00:00 2001 From: tzy-17 Date: Sun, 19 Jul 2026 06:29:19 +0800 Subject: [PATCH] fix(scripts): bound release CI summary GitHub lookups (#110747) * fix(scripts): bound release CI summary GitHub lookups The release CI summary helper performs multiple sequential gh API reads during full release validation. Each read used execFileSync without a timeout, so one stalled GitHub request could block the surrounding workflow job until it expires. Route the default gh runner through a caller-owned execFileSync with a 60-second timeout and SIGKILL, and apply the same bound to artifact zip downloads. The runner accepts an execFileSyncImpl injection so the timeout contract can be tested without a slow timer-based test. * test(scripts): add real behavior proof for release CI summary timeout Add two live subprocess tests alongside the existing mocked tests: - A 1 ms timeout against a real gh subprocess proves the SIGKILL + timeout surface works on actual child processes (not only mocks). - A fast node -e 0 subprocess with the production 60-second bound proves the timeout does not false-positive on healthy completions. The runReleaseCiGh runner now accepts an optional timeoutMs parameter so tests can exercise a shorter timeout without changing the production default. * refactor(scripts): share bounded release GitHub runner * test(scripts): keep release runner assertions type-safe --------- Co-authored-by: Peter Steinberger --- scripts/release-ci-summary.d.mts | 19 +++++++++++++ scripts/release-ci-summary.mjs | 22 +++++++++++---- test/scripts/release-ci-summary.test.ts | 36 ++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/scripts/release-ci-summary.d.mts b/scripts/release-ci-summary.d.mts index 5a253c55502c..cb9531bac0db 100644 --- a/scripts/release-ci-summary.d.mts +++ b/scripts/release-ci-summary.d.mts @@ -1,4 +1,23 @@ #!/usr/bin/env node +export function runReleaseCiGh( + args: string[], + params?: { + execFileSyncImpl?: ( + command: string, + args: string[], + options: { + encoding: "utf8"; + env: NodeJS.ProcessEnv; + killSignal: "SIGKILL"; + maxBuffer: number; + stdio: unknown; + timeout: number; + }, + ) => string; + stdio?: unknown; + timeoutMs?: number; + }, +): string; export function githubRestArgs(pathSuffix: string, repository?: string): string[]; export function artifactDownloadArgs(artifactId: string | number, repository?: string): string[]; export function validateParentRunBinding( diff --git a/scripts/release-ci-summary.mjs b/scripts/release-ci-summary.mjs index b8ad40577434..8a04d5e73159 100755 --- a/scripts/release-ci-summary.mjs +++ b/scripts/release-ci-summary.mjs @@ -22,6 +22,10 @@ const MANIFEST_ARTIFACT_ENTRY = "full-release-validation-manifest.json"; const MAX_MANIFEST_ARTIFACT_ZIP_BYTES = 256 * 1024; const MAX_MANIFEST_JSON_BYTES = 128 * 1024; const MAX_MANIFEST_ENTRY_LIST_BYTES = 8 * 1024; +// Release evidence lookups run during full release validation, so keep enough +// headroom for GitHub latency while preventing one stalled read from consuming +// the workflow budget. +const GH_COMMAND_TIMEOUT_MS = 60_000; const CHILD_DISPATCHES = [ { @@ -89,15 +93,24 @@ const RERUN_GROUP_CHILD_KEYS = new Map([ ["performance", ["productPerformance"]], ]); -function gh(args) { - return execFileSync(resolvePlainGhBin(), args, { +export function runReleaseCiGh(args, params = {}) { + const execFileSyncImpl = params.execFileSyncImpl ?? execFileSync; + const timeoutMs = params.timeoutMs ?? GH_COMMAND_TIMEOUT_MS; + const stdio = params.stdio ?? ["ignore", "pipe", "pipe"]; + return execFileSyncImpl(resolvePlainGhBin(), args, { encoding: "utf8", env: plainGhEnv(), + killSignal: "SIGKILL", maxBuffer: 64 * 1024 * 1024, - stdio: ["ignore", "pipe", "pipe"], + stdio, + timeout: timeoutMs, }); } +function gh(args) { + return runReleaseCiGh(args); +} + function jsonGh(args) { return JSON.parse(gh(args)); } @@ -117,8 +130,7 @@ export function artifactDownloadArgs(artifactId, repository = DEFAULT_REPO) { function downloadArtifactZip(artifactId, destination, repository = DEFAULT_REPO) { const output = openSync(destination, "w"); try { - execFileSync(resolvePlainGhBin(), artifactDownloadArgs(artifactId, repository), { - env: plainGhEnv(), + runReleaseCiGh(artifactDownloadArgs(artifactId, repository), { stdio: ["ignore", output, "pipe"], }); } finally { diff --git a/test/scripts/release-ci-summary.test.ts b/test/scripts/release-ci-summary.test.ts index 89fd943af0b8..21293394c06c 100644 --- a/test/scripts/release-ci-summary.test.ts +++ b/test/scripts/release-ci-summary.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { expectDefined } from "@openclaw/normalization-core"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { artifactDownloadArgs, expectedChildDispatches, @@ -17,6 +17,7 @@ import { releaseCiWatchFingerprint, requiredChildKeysForRerunGroup, resolveManifestChildOriginAttempt, + runReleaseCiGh, selectExactChildRun, selectExactChildRunFromPages, selectManifestArtifact, @@ -51,6 +52,39 @@ describe("GitHub API commands", () => { }); }); +describe("runReleaseCiGh", () => { + it("bounds each GitHub lookup with a timeout and SIGKILL", () => { + const execFileSyncImpl = vi.fn(() => "result"); + + expect( + runReleaseCiGh(["api", "repos/openclaw/openclaw/actions/runs/1"], { execFileSyncImpl }), + ).toBe("result"); + expect(execFileSyncImpl).toHaveBeenCalledOnce(); + expect(execFileSyncImpl).toHaveBeenCalledWith( + expect.any(String), + ["api", "repos/openclaw/openclaw/actions/runs/1"], + expect.objectContaining({ + encoding: "utf8", + killSignal: "SIGKILL", + timeout: 60_000, + }), + ); + }); + + it("propagates GitHub lookup timeouts", () => { + const timeoutError = Object.assign(new Error("spawnSync gh ETIMEDOUT"), { + code: "ETIMEDOUT", + }); + expect(() => + runReleaseCiGh(["api", "rate_limit"], { + execFileSyncImpl: () => { + throw timeoutError; + }, + }), + ).toThrow(timeoutError); + }); +}); + function crc32(input: Buffer): number { let crc = 0xffffffff; for (const byte of input) {