ci: add shellcheck to CI and lefthook#22
Conversation
Add shellcheck.yml workflow triggered on .sh file changes, and a shellcheck pre-commit hook via lefthook. Matches the pattern used in the ar monorepo.
There was a problem hiding this comment.
Pull request overview
Adds ShellCheck linting to the repo via both GitHub Actions CI and a local lefthook pre-commit hook, aligning shell script linting with existing Go CI checks.
Changes:
- Add a new GitHub Actions workflow to run ShellCheck on
.shchanges. - Add a lefthook pre-commit job to run ShellCheck on staged
.shfiles (with a skip message if not installed).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lefthook.yml |
Adds a shellcheck pre-commit job for staged .sh files. |
.github/workflows/shellcheck.yml |
Introduces a CI workflow to run ShellCheck when .sh files change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - name: shellcheck | ||
| glob: "**/*.sh" | ||
| run: command -v shellcheck >/dev/null 2>&1 && shellcheck {staged_files} || echo "shellcheck not installed — skipping" |
There was a problem hiding this comment.
The A && shellcheck ... || echo "shellcheck not installed" construct will also hit the || echo ... branch when shellcheck is installed but returns a non-zero exit code (i.e., lint failures), causing the hook to pass and hiding real issues. Restructure this so the "skip" path only happens when shellcheck is missing, and otherwise propagate shellcheck's exit status.
| run: command -v shellcheck >/dev/null 2>&1 && shellcheck {staged_files} || echo "shellcheck not installed — skipping" | |
| run: if command -v shellcheck >/dev/null 2>&1; then shellcheck {staged_files}; else echo "shellcheck not installed — skipping"; fi |
| - name: shellcheck | ||
| glob: "**/*.sh" | ||
| run: command -v shellcheck >/dev/null 2>&1 && shellcheck {staged_files} || echo "shellcheck not installed — skipping" |
There was a problem hiding this comment.
The header comment says the run commands "check the entire module — not just staged files", but this new job runs shellcheck only on {staged_files}. Either update the comment to reflect this exception or adjust the command to lint all tracked .sh files for consistency.
| - "**/*.sh" | ||
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - "**/*.sh" |
There was a problem hiding this comment.
With paths: ["**/*.sh"] only, this workflow will not run when the workflow file itself is modified (or when adding/configuring ShellCheck via config files). Consider including .github/workflows/shellcheck.yml (and any ShellCheck config files you expect) in the paths list so changes to the workflow are validated in PRs.
| - "**/*.sh" | |
| pull_request: | |
| branches: [main] | |
| paths: | |
| - "**/*.sh" | |
| - "**/*.sh" | |
| - ".github/workflows/shellcheck.yml" | |
| pull_request: | |
| branches: [main] | |
| paths: | |
| - "**/*.sh" | |
| - ".github/workflows/shellcheck.yml" |
- Fix lefthook command to not swallow shellcheck failures - Add workflow file itself to path triggers - Update header comment to clarify staged-only for shellcheck
Summary
shellcheck.ymlworkflow triggered on.shfile changesTest plan
.shfile change and verify shellcheck workflow runs.shfiles