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.
Summary
standards-checkrunsshellcheck -S info, and so does a developer checkingthe 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-configPR #503,standards-checkfailed:hooks/run-review.shsources two tracked sibling libraries via a_LIB_DIR-resolved path. Both files exist and are exercised by that repo's testsuite (118 assertions). The script already carried the directives intended to
fix exactly this:
Those directives resolve the siblings locally. They do not resolve under the CI
checkout, which places the repository in a
./reposubdirectory, soSCRIPTDIR-relative resolution lands somewhere else.-S infothen promotesthe unresolved
sourceto a job failure.Why it stayed hidden
The job logs
scope: files changed since <merge-base>— it lints only changedfiles.
run-review.shis rarely edited, so it is rarely linted. In PR #503 atwo-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 andrunning
shellcheck -S infoon it reproduces both SC1091s. The PR did notintroduce them.
Reproduction
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'sown message recommends, and it makes the checker agree with the local result for
multi-file shell projects.
Worth considering alongside it:
SCRIPTDIR-relative directivesresolve the way they do locally.
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-configPR #503.