fix(ci): bound package-test child commands so a hang fails fast - #582
Open
goosewobbler wants to merge 1 commit into
Open
fix(ci): bound package-test child commands so a hang fails fast#582goosewobbler wants to merge 1 commit into
goosewobbler wants to merge 1 commit into
Conversation
A wedged child in test-package.ts had nothing to stop it: no command carried a timeout and the package job set no timeout-minutes, so it inherited GitHub's 6-hour default. One run burned ~2 hours of a macOS runner after pnpm printed its "Done" summary and then never exited, the registry connection having dropped mid-install. Bound each command at 30 minutes (env-overridable for a cold Rust build) and add a 45-minute job backstop, against a slowest-real-job time of ~15 minutes. A timeout is terminal rather than retried — retrying a hang just spends the budget twice — and reports which command stalled instead of an opaque kill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsgeivZCqduTQmWK55zqiT
Contributor
Release Preview — no release
Updated automatically by ReleaseKit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A wedged child process in
scripts/test-package.tshad nothing to stop it. No command carried a timeout, and the package job set notimeout-minutes, so it inherited GitHub's 6-hour default.That is not hypothetical — it cost ~2 hours of a macOS runner on #580 before I cancelled it manually:
pnpm printed its completion summary and then never exited, after the registry connection dropped mid-install (~40
ENOTCONNretries immediately prior).execSyncwaited on a child that was never coming back. The step normally takes seconds.Nothing here is specific to that failure mode — any child that stops making progress produces the same outcome.
Changes
execCommandandexecCommandAsync, so the browser-mode path is covered too. Overridable viaPACKAGE_TEST_COMMAND_TIMEOUT_MSfor a cold Rust build on a slow machine.SIGKILLstill takes the normal retry path._ci-lint10,_ci-smoke-autoinstall-driver15,_ci-rust-clippy30, the mobile builds 40–60). Catches anything the per-command bound can't see.Calibration
Measured across 42
Package - *jobs on the last threemainruns: slowest is 15.1 minutes end to end (Package - Electron [Windows] - esm), and that whole job is many commands. No single command comes close to 30 minutes, so the bound only trips on something genuinely stuck — while cutting the worst case from 6 hours to 45 minutes.Verification
execSyncwith a 2s bound onsleep 30is killed at 2002ms and takes the timeout branch, not the retry branch.node ./scripts/test-package.ts --package=electron-script-app-esmpasses end to end on this branch.format:checkandlintclean.Found while investigating #580, kept separate since it's unrelated to that upgrade and applies equally on
maintoday.