Skip to content

Lint the workflow shell scripts with actionlint and shellcheck on ubuntu (#5970) - #5975

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/5970-actionlint-shellcheck
Oct 16, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/5970-actionlint-shellcheck

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

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 to shellcheck.exe. So CI checked workflow YAML, expressions, needs: names and action inputs, but never the bash in a run: 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)") in build.yml:

  • ubuntu-latest, needs: gate, if: needs.gate.outputs.run == 'true' && github.event_name != 'release', 10 minute timeout, contents: read.
  • Checks out, runs the same dorny/paths-filter@v4 filter as the Windows step (.github/workflows/**, same base expression), then runs the rhysd/actionlint:1.7.12 Docker 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.
  • The image is pinned by tag and digest (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.
  • The Windows step stays as it is. A comment next to each says what the other covers.

How it is wired into the aggregate: the two required result jobs (darling-pg-result, which carries the check name "Darling PostgreSQL tests", and lite-tests-result) now list workflow-lint in needs, read LINT_RESULT: ${{ needs.workflow-lint.result }}, and have a new arm right after the Guard arm: any result other than success fails the job with an error, except skipped on a release (the job does not run there). I did not touch the Guard arm or the GUARD_RESULT expression, and did not add the job to guard-cancel or to the result jobs' if: (!cancelled() already runs them when the lint job fails). For an unrelated diff the job still runs and ends success with its lint step skipped, so the aggregate reads success; 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_ListsItInNeeds pinned the two result jobs' exact needs lists; I added workflow-lint to 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)

  1. build.yml:147 SC2016, timing step: the --jq '...$newest...' argument is a jq program, and $newest is a jq variable. True false positive. Added # shellcheck disable=SC2016 above the if page=$(gh api ...) line with that reason.
  2. claude-review-guard.yml:169 SC2016, the jobs_jq='...$j...' assignment: also a jq program with a jq variable $j. True false positive. Added # shellcheck disable=SC2016 above it with the reason.
  3. build.yml:3223 (now 3399) SC2034, shadow-check step: for attempt in 1 2 3 never used attempt. Changed to for _ in 1 2 3. No behaviour change.
  4. nightly.yml:72 SC2086, re-dispatch step: map_only=${MAP_ONLY} unquoted. Now map_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).
  5. and 6. nightly.yml:138 SC2086 twice, check step: >> $GITHUB_OUTPUT now >> "$GITHUB_OUTPUT" on both has_changes= lines.
  6. sql-validation.yml:81 SC2012, install loop: for script in $(ls install/[0-9]*.sql | sort) is now for 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 sort used the sort command'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.yml was mixed in my checkout, and the commit normalizes through git as usual.

Test plan

  • docker run ... rhysd/actionlint:1.7.12@sha256:b1934ee5... -no-color over the whole repo, before the change: the 7 findings above. After the change: exit 0, no output.
  • yaml.safe_load parses build.yml, claude-review-guard.yml, nightly.yml and sql-validation.yml.
  • Not run: the test suites (this is CI configuration; the brief said not to). The pin edit in GuardStageWorkflowTests is two string changes; CI runs it.
  • The new job on a hosted runner: this PR changes workflow files, so its own Guard run is the first run of the job. The coordinator should check that 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

  • Whether to make workflow-lint fail fast through guard-cancel (I left it out; a lint finding does not waste the shard run).
  • The job's display name is new; it is not a required check by name. The result jobs carry it into the required checks.

CHANGELOG entry

SECTION: Changed
ENTRY:

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 16, 2026 20:50
@erikdarlingdata
erikdarlingdata merged commit c12233c into dev Oct 16, 2026
51 of 56 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/5970-actionlint-shellcheck branch October 16, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant