Repository navigation
ci: bound each Electron install attempt so a stalled download is retried - #6064
JasonBuildAI wants to merge 2 commits into
Conversation
A stalled Electron download neither exits nor emits the structured context the retry wrapper classifies, so the wrapper added in apache#2594 could hang `npm install` indefinitely on networks where the release CDN stops progressing. Cap each installer attempt at ten minutes (overridable with MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS), classify a timed-out attempt as retryable inside the existing five-attempt backoff, and exit with a diagnostic instead of re-raising the wrapper's own kill signal. Fixes apache#6063. Generated-by: Codex
The ETIMEDOUT classification and the aborted attempt's exit behavior only had manual coverage, so a regression there would silently reintroduce the unbounded hang. Extract a single attempt into runInstallerAttempt so the test can abort a real installer process that outlives a 2 s cap. Generated-by: Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
I reviewed ba394d7eb0b788137ebf06d1629ab9b061e2f4ec and found no verified P0–P2 issue.
The change bounds each installer subprocess, classifies its ETIMEDOUT as retryable, and preserves the existing five-attempt limit and 2/4/8/16-second backoff (scripts/install-electron-with-retry.mjs:72–97,111–151). Exhausting timeout retries exits with failure rather than re-raising the wrapper's own kill signal; ordinary terminal signals and spawn failures remain non-retryable (:169–178). Only the installer script and its tests change: there is no .github permission, workflow-trigger, dependency or protocol change.
All five installer tests and all 140 tests in the affected CI-planner/workflow-policy/Windows-harness/workspace-runner group passed locally, with no skips. Syntax, formatting and diff checks passed. I also ran the complete installer entry point against a local server that continuously trickled an unfinished response, using the real Electron installer and launcher. With a 700 ms cap, it made five requests, timed out five times, used all four backoffs and exited with code 1 after 33.58 seconds. Additional checks covered timeout-then-success and non-retryable failures. No external artifact server was used.
The documented tradeoffs remain: this is a wall-clock limit, not an inactivity limit; sufficiently slow healthy downloads can be interrupted, and aborted attempts leave partial temporary directories. The local merge with main 9a32d423b is clean and retains epoch 217. CI and Windows recovery are still running for this head. I did not test a real artifact download/extraction, run the full monorepo suite, or independently exercise this on Windows or macOS.
Summary
Bound every Electron installer attempt (10 minutes by default,
MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MSto override) and treat a timed-out attempt as retryable, sonpm ci/npm installcan no longer hang forever when the release CDN stops progressing mid-download.scripts/install-electron-with-retry.mjspreviously only retried failures that exited with a structured context line; a stalled download produces neither an exit nor a context line, so the retry wrapper added in #2594 never fired andspawnSyncblocked indefinitely. Installations on such networks now get the existing five-attempt backoff, and if every attempt stalls the install fails with a diagnostic instead of hanging.Fixes #6063
Verification
node --testagainst the wrapper with a timed-out attempt): before the change the wrapper returned the killed attempt immediately (AssertionError: null !== 0); after the change it retries and the attempt succeeds.node --test scripts/install-electron-with-retry.test.mjs— 5/5 pass. Besides the retry classification, the suite now aborts a real installer process that outlives a 2 s cap and asserts the reported result (timedOut: true,status: null,signal: SIGTERM), and guards that terminal signals (e.g. Ctrl+C) stay immediate.npm run lint,npm run format:check,npm run build,npm run typecheckpass. The CI-planner step (node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs scripts/ci-workflow-policy.test.mjs scripts/verify-windows-harness.test.mjs scripts/run-workspace-tests-parallel.test.mjs scripts/install-electron-with-retry.test.mjs) reports 140 tests: 139 pass, 0 fail, 1 skipped. The skip needs a realbash; the WSL relay on this host fails that test on unmodifiedmainas well.Known tradeoffs
MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MSraises the cap.electron-download-*directory in the OS temp directory; the artifact cache itself is never polluted (only checksum-verified downloads are moved into it).AI use
Select exactly one:
Tool(s) and scope: Codex (OpenAI) drafted the change, its tests, and this description under my direction and review.
Generated-by: Codex
Checklist
Does this PR entail a change in behavior?