Skip to content

feat(standards): give shellcheck a canonical config like the other linters - #172

Merged
twistedmelonman merged 1 commit into
mainfrom
claude/feat-canonical-shellcheckrc-7ba465dd
Sep 18, 2026
Merged

twistedmelonman merged 1 commit into
mainfrom
claude/feat-canonical-shellcheckrc-7ba465dd

Conversation

@twistedmelonman

Copy link
Copy Markdown
Member

shellcheck was the only linter in run-standards.sh with no canonical config. Every other linter resolves repo-local-else-canonical; shellcheck alone fell through to whatever .shellcheckrc the filesystem search happened to find — including ~/.shellcheckrc, outside the repo entirely. That is the structural cause of twistedmelonman/claude-config#534, and the reason a local run and CI could disagree with nothing in the repo to explain it.

standards/shellcheckrc is that missing canonical file, and the shellcheck block now resolves it either/or, mirroring the yamllint block.

The base, and why it is this base

Three effective settings: external-sources=true, source-path=SCRIPTDIR, disable=SC2310.

Measured across the 24 fleet repos carrying shell files: 21 are at zero findings under this base.

enable=all is deliberately excluded. Measured, it takes four otherwise-clean repos to 124+ combined findings (dotfiles 63, vpn-lan-bridge 12, claude-wrapper 10, scripts 3, LaunchAgents 1) and kebab-tax from 12 to 38. A base that forces per-repo exceptions is not a base. It stays in ~/.shellcheckrc as the stricter interactive config, which is the right place for it.

This matters more than it looks, because shellcheck rc files do not merge: a repo-root .shellcheckrc halts the ancestor search outright, and --rcfile replaces rather than layers. Any per-repo exception must restate the whole base and then silently drifts from it. Exceptions have to stay rare for the base to mean anything.

Not in this PR

Steps 3-4 of #534 — deleting the three per-repo .shellcheckrc files and annotating kebab-tax's one genuinely-excepted test file — are decided but deliberately left out. Deleting a repo's rc file exposes it to ~/.shellcheckrc unless that repo's local hook passes the canonical path explicitly, so those deletions need their own PR and their own sequencing against this one landing.

Advances twistedmelonman/claude-config#534.

https://claude.ai/code/session_011awg91UvzUos9YoXHJ2e8B

…nters

shellcheck was the only linter in run-standards.sh with no canonical config.
yamllint and markdownlint each resolve one from standards/; shellcheck ran
bare and took whatever the machine offered. Because shellcheck searches
ANCESTOR directories, a local run under a checkout whose parent carries a
~/.shellcheckrc silently picked up settings CI never saw -- 68 findings
locally against CI's 24 on the same tree. That is claude-config#534.

Resolution mirrors yamllint's at :147-150: a repo-root .shellcheckrc wins,
else standards/shellcheckrc. Passing --rcfile explicitly is what stops the
ancestor search, so this both centralizes the policy and closes the leak.

Content is external-sources + source-path=SCRIPTDIR + disable=SC2310. The
first two RESOLVE files sourced through a variable path rather than silencing
SC1091, so the sourced code is analyzed instead of skipped. SC2310 is
informational and fires throughout ordinary guard-clause style.

`enable=all` is deliberately excluded. Measured across every clone with shell
files on 2026-09-18: this config takes 21 of 24 repos to zero findings, while
adding enable=all takes four otherwise-clean repos to 124+ combined (dotfiles
63, vpn-lan-bridge 12, claude-wrapper 10, scripts 3, LaunchAgents 1). A base
that forces per-repo exceptions is not a base. enable=all stays in
~/.shellcheckrc as the stricter interactive config: local advisory, CI
enforcing.

Verified by running run-standards.sh itself against seven repos, not by
reading the diff: github-workflows, dotfiles, claude-wrapper, vpn-lan-bridge,
dev-env and claude-config all pass; kebab-tax fails with exactly the 12
findings the measurement predicted, all in one test file (10x SC2329, the
standard false positive for a harness dispatching indirectly, plus 2x SC2155).
That repo is addressed separately -- a file-level disable with a comment, not
a repo-level config.

The canonical file carries a known-bad gate in its own verification: a fixture
with an unquoted expansion must still be rejected under it, so a config that
silenced everything could not pass as clean.

Advances twistedmelonman/claude-config#534.

Claude-Session: https://claude.ai/code/session_01QkitU5UQAawPwZLEuXRajG
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

No blocking issues found.

The config_dir variable (defined at line 32 of run-standards.sh as the scripts own directory) correctly resolves ${config_dir}/shellcheckrc to the newly added standards/shellcheckrc. The repo-root-wins precedence logic mirrors the existing pattern for yamllint and markdownlint. The --rcfile flag intentionally replaces ambient ancestor-directory config search. No shell injection, no reliability regression, no silent failure path.

VERDICT: PASS

@twistedmelonman
twistedmelonman merged commit a93fe43 into main Sep 18, 2026
3 checks passed
@twistedmelonman
twistedmelonman deleted the claude/feat-canonical-shellcheckrc-7ba465dd branch September 18, 2026 18:18
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