fix(ci): respect an explicit ACTIONLINT_DOWNLOAD_ATTEMPTS=0 instead of silently defaulting to 4#7811
Conversation
…f silently defaulting to 4
scripts/actionlint.mjs computed maxAttempts as
`Math.max(1, Number.parseInt(env.ACTIONLINT_DOWNLOAD_ATTEMPTS ?? "4", 10) || 4)`.
Because 0 is falsy, `Number.parseInt("0", 10) || 4` is 4, so an operator setting
ACTIONLINT_DOWNLOAD_ATTEMPTS=0 silently got 4 attempts instead of the 1 the outer
Math.max would otherwise floor it to.
Extract the parse into scripts/lib/actionlint-download-attempts.mjs
(resolveActionlintDownloadAttempts) using an explicit Number.isFinite check instead
of the `|| 4` fallback — mirroring migrate-selfhost-sqlite-to-postgres.ts's
validation shape — so an explicit "0" is honored (and floored to 1), while
missing/blank/non-integer values still default to 4. A side-effect-free helper
module (with a .d.mts, like check-branding-drift.mjs) keeps actionlint.mjs's
side-effecting top-level flow importable-free for the unit test.
Add test/unit/actionlint-download-attempts.test.ts covering the 0-floored-to-1
regression plus the default/positive/negative cases. `npm run actionlint` still
exits 0.
Closes JSONbored#7773
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7811 +/- ##
=======================================
Coverage 88.57% 88.57%
=======================================
Files 725 725
Lines 76409 76409
Branches 22762 22762
=======================================
Hits 67682 67682
Misses 7681 7681
Partials 1046 1046
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 13:22:41 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
What & why
scripts/actionlint.mjscomputed the download-retry cap asMath.max(1, Number.parseInt(process.env.ACTIONLINT_DOWNLOAD_ATTEMPTS ?? "4", 10) || 4). Since0is falsy in JS,Number.parseInt("0", 10) || 4is4— so an operator who setACTIONLINT_DOWNLOAD_ATTEMPTS=0silently got 4 attempts instead of the 1 the outerMath.maxwould otherwise floor it to.Change
scripts/lib/actionlint-download-attempts.mjs(resolveActionlintDownloadAttempts) using an explicitNumber.isFinitecheck instead of the|| 4fallback — mirroringscripts/migrate-selfhost-sqlite-to-postgres.ts's validation shape. An explicit"0"is now honored (floored to 1 byMath.max); missing/blank/non-integer still default to 4..d.mts, likescripts/check-branding-drift.d.mts) so it's importable by a unit test without runningactionlint.mjs's side-effecting top-level flow (which requires the native linter and callsprocess.exit).test/unit/actionlint-download-attempts.test.tscovering the0 → 1regression plus default/positive/negative cases.scripts/**is outside the Codecovcoverage.includeset, socodecov/patchdoes not gate it — the test runs in the backend vitest suite. Verified locally: 4 new tests pass, roottsc --noEmitclean,git diff --checkclean, andnpm run actionlintstill exits 0.Closes #7773