diff --git a/scripts/actionlint.mjs b/scripts/actionlint.mjs index 0501bd611d..89cf5556b6 100644 --- a/scripts/actionlint.mjs +++ b/scripts/actionlint.mjs @@ -2,6 +2,7 @@ import { readFileSync, readdirSync } from "node:fs"; import { createRequire } from "node:module"; import { join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; +import { resolveActionlintDownloadAttempts } from "./lib/actionlint-download-attempts.mjs"; const require = createRequire(import.meta.url); const { actionlint } = require("github-actionlint"); @@ -16,7 +17,7 @@ if (files.length === 0) { process.exit(1); } -const maxAttempts = Math.max(1, Number.parseInt(process.env.ACTIONLINT_DOWNLOAD_ATTEMPTS ?? "4", 10) || 4); +const maxAttempts = resolveActionlintDownloadAttempts(process.env.ACTIONLINT_DOWNLOAD_ATTEMPTS); const retryDelaysMs = [1000, 3000, 7000]; const retryableSetupError = /Download failed: (?:408|425|429|5\d\d)\b|ECONNRESET|ETIMEDOUT|EAI_AGAIN|ENOTFOUND|socket hang up/i; diff --git a/scripts/lib/actionlint-download-attempts.d.mts b/scripts/lib/actionlint-download-attempts.d.mts new file mode 100644 index 0000000000..ed47ec18ca --- /dev/null +++ b/scripts/lib/actionlint-download-attempts.d.mts @@ -0,0 +1 @@ +export function resolveActionlintDownloadAttempts(raw: string | undefined): number; diff --git a/scripts/lib/actionlint-download-attempts.mjs b/scripts/lib/actionlint-download-attempts.mjs new file mode 100644 index 0000000000..666270351c --- /dev/null +++ b/scripts/lib/actionlint-download-attempts.mjs @@ -0,0 +1,15 @@ +/** + * Resolve the actionlint download-retry attempt count from a raw env value (ACTIONLINT_DOWNLOAD_ATTEMPTS). + * + * Uses an explicit finiteness check rather than a `Number.parseInt(...) || 4` fallback (#7773): `0` is falsy + * in JS, so `Number.parseInt("0", 10) || 4` wrongly yields 4, silently ignoring an operator's explicit `"0"`. + * A missing/blank/non-integer value defaults to 4; any parsed integer is honored and floored to a minimum of + * 1. Mirrors the validation shape in migrate-selfhost-sqlite-to-postgres.ts (`Number.isFinite`, not `||`). + * + * @param {string | undefined} raw + * @returns {number} attempt count, always >= 1 + */ +export function resolveActionlintDownloadAttempts(raw) { + const parsed = Number.parseInt(raw ?? "4", 10); + return Math.max(1, Number.isFinite(parsed) ? parsed : 4); +} diff --git a/test/unit/actionlint-download-attempts.test.ts b/test/unit/actionlint-download-attempts.test.ts new file mode 100644 index 0000000000..c5372ad7de --- /dev/null +++ b/test/unit/actionlint-download-attempts.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; + +import { resolveActionlintDownloadAttempts } from "../../scripts/lib/actionlint-download-attempts.mjs"; + +// #7773: an explicit ACTIONLINT_DOWNLOAD_ATTEMPTS="0" must be respected (floored to 1), not silently turned +// into the default of 4 by a `parseInt(...) || 4` fallback (0 is falsy). +describe("resolveActionlintDownloadAttempts (#7773)", () => { + it("respects an explicit 0, flooring it to 1 (not the default 4)", () => { + expect(resolveActionlintDownloadAttempts("0")).toBe(1); + }); + + it("defaults to 4 when unset, blank, or non-integer", () => { + expect(resolveActionlintDownloadAttempts(undefined)).toBe(4); + expect(resolveActionlintDownloadAttempts("")).toBe(4); + expect(resolveActionlintDownloadAttempts("abc")).toBe(4); + }); + + it("honors an explicit positive integer", () => { + expect(resolveActionlintDownloadAttempts("1")).toBe(1); + expect(resolveActionlintDownloadAttempts("7")).toBe(7); + }); + + it("floors a negative value to 1", () => { + expect(resolveActionlintDownloadAttempts("-3")).toBe(1); + }); +});