JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Run actionlint too, so the shell inside workflow run: blocks is checked as well #70

New Issue
Closed
opened 2026-08-24 21:03:56 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-24 21:03:56 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Split out of #69, which added shellcheck to CI. Named there as the half that PR deliberately does not cover.

The gap

#69 runs shellcheck over git ls-files '*.sh'. That is four scripts. It does not read the
inline run: blocks in the workflows, and a good deal of this repo's bash lives there — the release
verification in build.yml, the emulator setup and teardown across status_check.yml and
api37-debug.yml. So "shellcheck runs in CI" is currently true of the files and not of the blocks.

actionlint closes it: it parses each workflow and runs shellcheck over every run:, on top of its
own checks for expression syntax, needs: references, matrix keys and action input names.

It already finds something

Run against the tree at baaaa93, actionlint reports exactly one issue:

.github/workflows/build.yml:83:9: shellcheck reported issue in this script:
  SC2012:info:1:7: Use find instead of ls to better handle non-alphanumeric filenames

APK=$(ls app/build/outputs/apk/release/*.apk | head -1)

Harmless as it stands — Gradle's output names have no spaces — but the glob is already there and a
bash array reads it without the ls:

apks=(app/build/outputs/apk/release/*.apk)
APK="${apks[0]}"

Why it was not just added

Every action in this repo is pinned by SHA (actions/checkout@3d3c42e5...). actionlint's
documented install is bash <(curl -s .../download-actionlint.bash) off a moving branch — running
that in CI would contradict the repo's own supply-chain posture more than the linter is worth.

Doing it properly means pinning: either rhysd/actionlint by image digest, or a third-party
action by SHA. That is a real decision about what this repo is willing to depend on, which is why it
is a ticket rather than a line in #69.

Done means

  • actionlint runs in CI, pinned by digest or SHA, with the pin recorded the way the others are.
  • build.yml's SC2012 is fixed (or carries a # shellcheck disable with a reason, matching how #69
    handled its two false positives).
  • CLAUDE.md's shellcheck bullet loses the "does not cover inline run: blocks" caveat, because it
    no longer applies.
  • The mutation: put a deliberately broken expression in a run: block — an unquoted $VAR in a
    [ ] test, say — and confirm CI goes red. A linter that cannot be shown to catch a plant is not
    yet wired in.
_Split out of #69, which added shellcheck to CI. Named there as the half that PR deliberately does not cover._ ### The gap #69 runs shellcheck over `git ls-files '*.sh'`. That is four scripts. It does **not** read the inline `run:` blocks in the workflows, and a good deal of this repo's bash lives there — the release verification in `build.yml`, the emulator setup and teardown across `status_check.yml` and `api37-debug.yml`. So "shellcheck runs in CI" is currently true of the files and not of the blocks. `actionlint` closes it: it parses each workflow and runs shellcheck over every `run:`, on top of its own checks for expression syntax, `needs:` references, matrix keys and action input names. ### It already finds something Run against the tree at `baaaa93`, actionlint reports exactly one issue: ``` .github/workflows/build.yml:83:9: shellcheck reported issue in this script: SC2012:info:1:7: Use find instead of ls to better handle non-alphanumeric filenames ``` ```bash APK=$(ls app/build/outputs/apk/release/*.apk | head -1) ``` Harmless as it stands — Gradle's output names have no spaces — but the glob is already there and a bash array reads it without the `ls`: ```bash apks=(app/build/outputs/apk/release/*.apk) APK="${apks[0]}" ``` ### Why it was not just added **Every action in this repo is pinned by SHA** (`actions/checkout@3d3c42e5...`). actionlint's documented install is `bash <(curl -s .../download-actionlint.bash)` off a moving branch — running that in CI would contradict the repo's own supply-chain posture more than the linter is worth. Doing it properly means pinning: either `rhysd/actionlint` by **image digest**, or a third-party action by SHA. That is a real decision about what this repo is willing to depend on, which is why it is a ticket rather than a line in #69. ### Done means - actionlint runs in CI, pinned by digest or SHA, with the pin recorded the way the others are. - `build.yml`'s SC2012 is fixed (or carries a `# shellcheck disable` with a reason, matching how #69 handled its two false positives). - `CLAUDE.md`'s shellcheck bullet loses the "does not cover inline `run:` blocks" caveat, because it no longer applies. - **The mutation:** put a deliberately broken expression in a `run:` block — an unquoted `$VAR` in a `[ ]` test, say — and confirm CI goes red. A linter that cannot be shown to catch a plant is not yet wired in.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels confirmed enhancement sev:low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#70