Repository navigation
Lint the workflow shell scripts with actionlint and shellcheck on ubuntu (#5970) - #5975
Merged
Merged
Conversation
…ntu, and fix the 7 findings (#5970)
erikdarlingdata
marked this pull request as ready for review
October 16, 2026 20:50
This was referenced Oct 16, 2026
Closed
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.
Fixes #5970.
Claude posting for Erik Darling.
Why
The actionlint step in the Guard tests job runs with shellcheck and pyflakes off, because actionlint 1.7.12 hangs on a Windows runner when it pipes a
run:script toshellcheck.exe. So CI checked workflow YAML, expressions,needs:names and action inputs, but never the bash in arun:block. Run with shellcheck on, the workflow files had 7 findings that nothing enforced.What changes
New job
workflow-lint(display name "Workflow lint (actionlint with shellcheck)") inbuild.yml:ubuntu-latest,needs: gate,if: needs.gate.outputs.run == 'true' && github.event_name != 'release', 10 minute timeout,contents: read.dorny/paths-filter@v4filter as the Windows step (.github/workflows/**, samebaseexpression), then runs therhysd/actionlint:1.7.12Docker image with shellcheck on over.github/workflows/. The lint step runs on a pull request that changed a file under.github/workflows/and on every push, exactly as the Windows step does.rhysd/actionlint:1.7.12@sha256:b1934ee5...5667, the digest of the image I ran locally). pyflakes is left on (the image default); it is clean over the current files. No-ignore.How it is wired into the aggregate: the two required result jobs (
darling-pg-result, which carries the check name "Darling PostgreSQL tests", andlite-tests-result) now listworkflow-lintinneeds, readLINT_RESULT: ${{ needs.workflow-lint.result }}, and have a new arm right after the Guard arm: any result other thansuccessfails the job with an error, exceptskippedon a release (the job does not run there). I did not touch the Guard arm or theGUARD_RESULTexpression, and did not add the job toguard-cancelor to the result jobs'if:(!cancelled()already runs them when the lint job fails). For an unrelated diff the job still runs and endssuccesswith its lint step skipped, so the aggregate readssuccess; there is no planned-skip case to handle. A declined run (draft, merge train) is caught by the existing gate arms before the new arm.GuardStageWorkflowTests.EveryJobThatTheGuardStageGates_ListsItInNeedspinned the two result jobs' exactneedslists; I addedworkflow-lintto both strings. The rule (result jobs read the guard jobs) still holds. No test was added or removed.The 7 findings (line numbers as on dev before this change)
build.yml:147SC2016, timing step: the--jq '...$newest...'argument is a jq program, and$newestis a jq variable. True false positive. Added# shellcheck disable=SC2016above theif page=$(gh api ...)line with that reason.claude-review-guard.yml:169SC2016, thejobs_jq='...$j...'assignment: also a jq program with a jq variable$j. True false positive. Added# shellcheck disable=SC2016above it with the reason.build.yml:3223(now 3399) SC2034, shadow-check step:for attempt in 1 2 3never usedattempt. Changed tofor _ in 1 2 3. No behaviour change.nightly.yml:72SC2086, re-dispatch step:map_only=${MAP_ONLY}unquoted. Nowmap_only="${MAP_ONLY}". I also replaced--repo ${{ github.repository }}with--repo "${GITHUB_REPOSITORY}"(the built-in env var, same value, quoted, and no expression spliced into the script).nightly.yml:138SC2086 twice, check step:>> $GITHUB_OUTPUTnow>> "$GITHUB_OUTPUT"on bothhas_changes=lines.sql-validation.yml:81SC2012, install loop:for script in $(ls install/[0-9]*.sql | sort)is nowfor script in install/[0-9]*.sql.Behaviour changes, named: only finding 7 is more than quoting. The glob expands in the shell's collation order where
sortused thesortcommand's order. The file names start with a zero-padded number, so the order is the same, and names with spaces now work. If the glob matched nothing the loop would run once with the literal pattern and sqlcmd would fail on a missing file, where before the loop ran zero times;install/always holds those scripts.Line endings: the working tree has CRLF for these files (index is LF);
build.ymlwas mixed in my checkout, and the commit normalizes through git as usual.Test plan
docker run ... rhysd/actionlint:1.7.12@sha256:b1934ee5... -no-colorover the whole repo, before the change: the 7 findings above. After the change: exit 0, no output.yaml.safe_loadparsesbuild.yml,claude-review-guard.yml,nightly.ymlandsql-validation.yml.GuardStageWorkflowTestsis two string changes; CI runs it.Workflow lint (actionlint with shellcheck)ran its lint step and passed, and that "Darling PostgreSQL tests" and "Lite tests (result)" are green.What the coordinator should check
workflow-lintfail fast throughguard-cancel(I left it out; a lint finding does not waste the shard run).CHANGELOG entry
SECTION: Changed
ENTRY:
.github/workflows/when a pull request changes a workflow file, and its result feeds the required Darling PostgreSQL and Lite result checks, so a quoting or unused-variable mistake in arun:block fails the pull request before it merges.REF:
[CI: run actionlint with shellcheck on an ubuntu job, and fix the 7 shell findings it reports #5970]: CI: run actionlint with shellcheck on an ubuntu job, and fix the 7 shell findings it reports #5970