ci(smoke): replace timeout with a perl alarm so the macOS job can run - #381
Merged
Conversation
`Build (macOS)` has never passed. smoke.yml called `timeout 60s zsh -c …` unconditionally, but `timeout` is GNU coreutils — macOS runners ship neither `timeout` nor `gtimeout` without `brew install coreutils`, so the step died with `command not found` (exit 127) before sourcing anything. Replaced with `perl -e 'alarm shift; exec @argv; …' 60 …` in BOTH jobs rather than only the macOS one. perl is preinstalled on both runner images, and the SIGALRM timer set by alarm(2) survives the exec, so a hang still dies non-zero. Using one mechanism on both platforms means the hang-guard cannot drift per-platform, and drops the coreutils assumption instead of papering over it. The `exit 127` tail is load-bearing, not decoration. perl's exec() returns only on failure, so the bare `exec @ARGV` form falls off the end of the one-liner and **exits 0 when the target binary is missing** — the Ubuntu job invokes bare `zsh`, installed by an earlier step, so a broken install would have produced a green smoke test that sourced nothing. `timeout` reported 127 there; this reproduces it. Verified against the final form: success 0, `exit 1` preserved as 1, `exit 3` preserved as 3, a hang killed at the alarm (142, at the deadline not the job limit), and a missing binary 127. Also switches the inline scripts from double- to single-quoted `zsh -c '…'`, which drops the `\"`/`\$` escaping and lets zsh expand $ZSH_VERSION itself. Closes #380
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.
Closes #380. Picks up @claude's patch from the issue thread — it did the research and chose option B, but its push was rejected because the GitHub App lacks
workflowspermission on.github/workflows/. This session has it, so I'm landing the work.One substantive change to that patch — see below; it had a hole that would have made the smoke test green while testing nothing.
The fix
timeoutis GNU coreutils. macOS runners ship neithertimeoutnorgtimeoutwithoutbrew install coreutils, soBuild (macOS)died atcommand not foundbefore sourcing anything, and has never passed since #374 added it.Replaced with
perl -e 'alarm shift; exec @ARGV; …' 60 …in both jobs, not just macOS: perl is preinstalled on both runner images, thealarm(2)timer survives theexec, and using one mechanism on both platforms means the hang-guard can't drift per-platform. That drops the coreutils assumption rather than papering over it on one side.The correction to the proposed patch
The bot's form was
perl -e 'alarm shift; exec @ARGV' 60 zsh -c '…'. perl'sexec()returns only on failure, so with no tail after it the one-liner falls off the end and exits 0 when the target binary is missing:That is not hypothetical here: the Ubuntu job invokes bare
zsh, installed by an earlier setup step. Had that install ever broken, this smoke test would have gone green having sourced nothing — the failure mode.claude/rulescalls a lying test.timeoutreported 127; the addedwarn+exit 127reproduces it.Verification
Ran the final form directly — this container has no zsh and no macOS runner, so these exercise the wrapper's semantics, which is what changed:
0exit 11exit 33sleep 60, alarm 2)142in 2scat)142in 2s1270before the correctionYAML parses and both
run:blocks render as intended.actionlintisn't installed in this container, so CI is the first real lint pass — theLintersjob covers it.The issue's acceptance criterion — "a deliberately hanging
~/.zshrcstill fails the job rather than running to the 6-hour limit" — is the hang and stdin rows above.Also
Inline scripts move from double- to single-quoted
zsh -c '…', dropping the\"/\$escaping so zsh expands$ZSH_VERSIONitself. Same output, less quoting to get wrong.Once this merges, #378 gets its base merged in so its
Build (macOS)re-runs against a green base — that's its last red check and the only thing standing between it and mergeable.Generated by Claude Code