Skip to content

standards-check: shellcheck SC1091 fails on sibling sources because CI's checkout layout differs from local #168

Description

@twistedmelonman

Summary

standards-check runs shellcheck -S info, and so does a developer checking
the same file locally — but the two see different file layouts, so they
disagree. A script that sources a sibling file passes locally and fails in CI.
No local dry-run can catch this class of finding, which makes it invisible until
push.

Observed

On twistedmelonman/claude-config PR #503, standards-check failed:

In hooks/run-review.sh line 1196:
source "${_LIB_DIR}/lib-review-issues.sh"
       ^-- SC1091 (info): Not following: lib-review-issues.sh was not specified as input (see shellcheck -x).

hooks/run-review.sh sources two tracked sibling libraries via a
_LIB_DIR-resolved path. Both files exist and are exercised by that repo's test
suite (118 assertions). The script already carried the directives intended to
fix exactly this:

# shellcheck source-path=SCRIPTDIR
# shellcheck source=lib-review-issues.sh
source "${_LIB_DIR}/lib-review-issues.sh"

Those directives resolve the siblings locally. They do not resolve under the CI
checkout, which places the repository in a ./repo subdirectory, so
SCRIPTDIR-relative resolution lands somewhere else. -S info then promotes
the unresolved source to a job failure.

Why it stayed hidden

The job logs scope: files changed since <merge-base> — it lints only changed
files. run-review.sh is rarely edited, so it is rarely linted. In PR #503 a
two-line comment edit pulled it into scope for the first time and surfaced a
finding that had been latent on main.

Confirmed pre-existing: extracting origin/main's own copy of the script and
running shellcheck -S info on it reproduces both SC1091s. The PR did not
introduce them.

Reproduction

mkdir -p /tmp/cisim/repo/hooks
git show origin/main:hooks/run-review.sh > /tmp/cisim/repo/hooks/run-review.sh
cd /tmp/cisim && shellcheck -S info repo/hooks/run-review.sh   # both SC1091s

Run the same command with the libs present beside the script and it is clean.
The script is the control variable; only the layout changes.

Suggested fix

Add -x (follow sourced files) to the shellcheck invocation. It is what SC1091's
own message recommends, and it makes the checker agree with the local result for
multi-file shell projects.

Worth considering alongside it:

  • Run shellcheck from the checkout root so SCRIPTDIR-relative directives
    resolve the way they do locally.
  • Document the exact CI-equivalent command in the README so a contributor can
    reproduce a failure before pushing.

Either of the first two would have avoided the disable that PR #503 had to add.

Impact

Affects every consuming repo with multi-file shell scripts. The failure mode is
not a wrong result so much as an unpredictable one: the finding appears only
when an unrelated edit happens to pull a sourcing script into the changed-file
scope, so it lands on whoever touched it last rather than whoever introduced it.

Filed from a session on twistedmelonman/claude-config PR #503.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions