Skip to content

fix(ci): scope shellcheck.yml to changed files only (#313)#314

Merged
alexandervazquez98 merged 1 commit into
mainfrom
fix-313-shellcheck-yml-scope
Jun 21, 2026
Merged

fix(ci): scope shellcheck.yml to changed files only (#313)#314
alexandervazquez98 merged 1 commit into
mainfrom
fix-313-shellcheck-yml-scope

Conversation

@alexandervazquez98

Copy link
Copy Markdown
Owner

Summary

Fixes GitHub issue #313 by scoping .github/workflows/shellcheck.yml to lint only the shell scripts the PR actually changed, matching the proven pattern in .github/workflows/lint.yml L97-123.

One-file workflow body rewrite, no new files, no version bumps, no # shellcheck disable=, no edits to lint.yml.

Why

shellcheck.yml ran shellcheck -x scripts/*.sh monitor_performance.sh docker/neo4j/entrypoint.sh against every shell script in the repo with no filter and no severity floor. Any PR touching scripts/ picked up pre-existing findings from files it did not modify and was blocked.

lint.yml L11 documents the repo's convention: "shell script lint on changed files." lint.yml L97-123 implements that convention via tj-actions/changed-files. shellcheck.yml ignored it.

Validation case — PR #308 (fix(scripts): renew stale frontend node_modules anonymous volumes, issue #306): state MERGEABLE but shellcheck.yml/shellcheck check FAILURE. The failing run produced 27 diagnostics: 6 pre-existing (SC2317 ×4 + SC2034 in scripts/test-check-frontend-deps.sh; SC2088 in scripts/validate-env.sh) plus 22 PR-308-owned findings in the PR's new test files.

This change is the parallel of PR #312 (issue #309) — same pattern, different organ. PR #312 fixed the actionlint+shellcheck cascade on .github/workflows/cd.yml; this PR fixes the equivalent on scripts/*.sh.

What changes

.github/workflows/shellcheck.yml jobs block (lines 19-30) — full rewrite:

  • Drop the redundant Install ShellCheck step (Ubuntu 24.04 runners ship shellcheck preinstalled per the existing comment in lint.yml).
  • Add tj-actions/changed-files@ed68ef82c095e0d48ec87eccea555d944a631a4c # v46.0.5 — SHA pin (CVE-2025-30022)same SHA as lint.yml.
  • files_yaml: scripts/**/*.sh, docker/neo4j/entrypoint.sh, monitor_performance.sh — same set as lint.yml.
  • Run ShellCheck step guarded by if: steps.changed.outputs.any_changed == 'true' and filters to *.sh files only.
  • Else branch prints No shell files changed — skipping.

Trigger block (lines 1-18) is unchangedpull_request and push paths preserved verbatim.

Diff: 1 file, +23/-7.

Verification expectation

After this lands on main:

Out of scope

  • Editing .github/workflows/lint.yml (proven working).
  • Cleaning pre-existing findings in scripts/test-check-frontend-deps.sh / scripts/validate-env.sh (a follow-up cleanup PR is owed but is separate from this fix).
  • Adding .shellcheckrc, pre-commit hooks, or shellcheck severity policy changes.
  • Bumping tj-actions/changed-files beyond ed68ef82/v46.0.5 (must stay in lockstep with lint.yml).
  • Changing the if: false real-deploy step or other workflow files.

Refs: #313, #308

Mirror the proven changed-files pattern from .github/workflows/lint.yml
L97-123: detect touched shell files with tj-actions/changed-files (same
SHA pin and CVE-2025-30022 comment as lint.yml) and run shellcheck -x
only against that subset. Trigger paths filters and the push-to-main
trigger are preserved unchanged.

Drops the redundant Install ShellCheck step — Ubuntu 24.04 runners ship
shellcheck preinstalled, the same assumption lint.yml relies on. Adds
an explicit 'No shell files changed — skipping.' branch to match the
proven skip behavior.

Fixes #313 by unblocking PR #308 (and any
future PR that touches a non-shell path): pre-existing findings in
unmodified scripts no longer gate unrelated work. PR #308's own 22
findings in scripts/test-refresh-frontend-deps.sh and
scripts/test-safe-rebuild-frontend-volume.sh remain PR #308's
responsibility.
@alexandervazquez98 alexandervazquez98 merged commit 8d68fda into main Jun 21, 2026
7 checks passed
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