fix(ci): stop bash -e aborting the eval action's binary and summary steps - #46
Merged
Merged
Conversation
… steps The eval step's abort-before-writing-outputs was fixed already. The same defect was still live twice more in the same file, because errexit is a property of the shell and the guard opted steps in one at a time. - The `latest` release lookup assigns from a pipeline, so an assignment takes its status and a 404 or DNS failure killed the step *at that line*. The `::error::could not resolve the latest domarinn release tag` and `exit 3` written to handle exactly that were unreachable; an operator got a red step with an empty log and curl's raw exit code, which is not one of the four the gate step knows how to render. - The summary step branched on `ci-summary`'s exit code, but the thing it needs is the file. A CLI that exits 0 without writing one took the success branch and then died on the `cat`, letting a reporter fail the job the gate step is supposed to own. The guard now runs every `run:` step in the action under GitHub's exact interpreter, and `every_shell_step_is_exercised_here` fails if the action grows a step the registry does not name — opting steps out rather than in, so the next one cannot slip through the same way.
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.
Overview
Closes #41.
The half of #41 that was reported — the eval step aborting before it wrote
exit-code/results-path, so the gate annotated every failure asinfrastructure error (exit 3)— landed in #43. This closes out the rest.Errexit is a property of the shell, not of a step, and the same defect was
still live twice more in the same file:
1. The
latestrelease lookup (binstep). An assignment takes the statusof the pipeline it assigns from, so under
-e -o pipefaila 404 or a DNSfailure killed the step at that line. The
::error::could not resolve the latest domarinn release tagandexit 3written to handle exactly that casewere unreachable code that reads as live. Reproduced under GitHub's interpreter
with a
curlthat exits 22:223::error::could not resolve the latest domarinn release tag22is not one of the four codes the gate step knows how to render, and anempty log is the worst possible answer to "why is this red".
The step's other two dead ends were fine, which is why this survived review —
they are shaped
if curl …; thenandif ! command -v cargo, and a command ina condition is exempt from
-e. Same file, same shell, three call sites, oneshape wrong.
2. The
Summarizestep. It branched onci-summary's exit code, but thething it needs is the file. A CLI that exits 0 without writing one — an
--outit could not create, a run with nothing in it — took the success branchand then died on the trailing
cat, so a reporter whose own comment says "thegate step below owns the verdict" failed the job instead. The condition is now
"there is no usable summary", which is the actual precondition.
Both fixes are one line each; the rest of the diff is the guard.
Additional context
The guard from #43 executed two of the action's four
run:steps. That is howthe two above survived a fix aimed at exactly this bug — opting steps in one at
a time will keep missing a whole-shell property.
crates/domarinn-cli/tests/action_contract.rsnow works the other way round: aregistry names every
run:step with the environment a caller who set nooptional inputs would produce, and
every_shell_step_is_exercised_herefails ifaction.ymlgrows a step the registry does not name. Verified by injecting anunregistered step and watching it fail, then reverting. Steps are opted out,
not in.
Nine tests, all executing the real
run:text lifted out ofaction.ymlunderbash --noprofile --norc -e -o pipefail— reproduced rather than simplified,because under a plain
bash script.shnone of these bugs happen at all and thetests would pass against the broken action. Both new failing cases were watched
red first:
No behaviour change for a healthy run; this only affects what happens when the
network or the CLI misbehaves.
Checklist
mise run cipasses locally — every gate exceptmusl-build, whichfails on my machine for an unrelated reason (no
x86_64-linux-musl-gcccross-compiler installed).
clippy,fmt-check,test,schema-check,gen-types-check,web-lint,web-build,web-test,docsandworkflow-lintall pass.(
mise run schema/mise run gen-types) — n/a, no sources changedAI usage disclosure: YES — investigated, fixed, and tested with Claude Code.
Root cause was reproduced by extracting each step's
run:text and executing itunder GitHub's exact bash invocation before any fix was written, and both new
tests were confirmed failing against the unfixed action first.