fix(shellcheck): follow `source=` directives, and move the check into the editor by Syndic · Pull Request #232 · Syndic/unnatural_designs

Syndic · 2026-08-18T19:30:32Z

What

.shellcheckrc with external-sources=true — makes shellcheck's verdict independent of how
the caller batches its arguments.
Doc corrections — six claims about shellcheck and the pre-commit hooks that disagreed with
.pre-commit-config.yaml.
//meta/scripts:test_precommit_docs — a guard so the README's hook documentation can't drift
from the config again.
shellcheck moves out of pre-commit and into the editor — it was the one verification-only
check that blocked a commit.

Why

pre-commit run --all-files failed SC1091 on
meta/devcontainer-base/scripts/devcontainer-plumbing.sh, pre-existing on main:

. "$lib"
SC1091 (info): Not following: meta/devcontainer-base/scripts/lib.sh was not specified as input

The # shellcheck source= directive was already there — what was missing was permission to follow
it. shellcheck follows a directive only into a file that was itself in the invocation's argument
list. ci.yml passes every tracked *.sh at once, so lib.sh counts as an input and the source
resolves; a single-file invocation gets the dispatcher on its own and reports SC1091. Same tree,
same config, opposite verdicts.

An rc file rather than -x per caller: .shellcheckrc is one config every caller reads, and it
sits alongside .golangci.yml and .bazelrc at the root. That choice matters more after change 4
than before it — the editor lints the open buffer alone, which is exactly the single-file shape
that raised SC1091.

shellcheck now runs in the editor, not pre-commit

The repo's stated policy is that verification-only checks live in the editor so they surface
findings without blocking a commit; shellcheck was the lone exception. timonwong.shellcheck
replaces the hook, with two settings that are load-bearing rather than cosmetic:

shellcheck.executablePath: /usr/bin/shellcheck — the devcontainer's copy, not the extension's
bundled binary, so the editor and CI can only disagree by the version skew ci.yml documents.
shellcheck.useWorkspaceRootAsCwd: true — source= directives are repo-root-relative and
.shellcheckrc is at the root, so linting from the file's own directory breaks both. Verified:
from a subdirectory the directive fails to resolve.

CI's shellcheck job is untouched and is now the only enforcing gate.

The drift guard

Three of the six doc errors were a hook's files: pattern drifting from a doc that names it, so
the fix is a test rather than another round of proofreading. It reads the real README.md and
.pre-commit-config.yaml — the shape //.devcontainer:test_base_image_pin already uses — and
asserts the table lists every hook in config order, the prose accounts for each one, and the prose
names none the config lacks. It rides bazel test //... rather than costing a CI job.

Two things worth a reviewer's eye:

The scan behind the third rule is unit-tested on synthetic text, not just against the README.
Written against the README alone it passed vacuously: the ``` fences in the install
snippet are backticks too, and they offset every inline-code pair after them into nonsense.
The "Triggers on" column is deliberately not asserted. It paraphrases each files regex for a
reader; checking a gloss against a regex would either restate the regex in the README or accept
anything.

The guard earned itself immediately: when the shellcheck hook came out, it failed on both the stale
table row and the stale prose mention.

Doc corrections (commit 2)

Where	Was
`README.md` hook table	Listed 6 of 8 hooks; `base-image-pin` and `shellcheck` were absent
`README.md` policy paragraph	Omitted `base-image-pin` from the fixing hooks, and claimed verification-only checks live in the editor — which was false for `shellcheck` at the time, and is true again as of commit 4
`README.md` editor lead-in	"runs the non-fixing checks on save" — wrong in both directions (ruff fixes on save; shellcheck was not wired there)
`README.md` toolchain list	Omitted `shellcheck`
`renovate-derived-files.yml`, `.claude/CLAUDE.md`	Routed a `.bazelversion`-induced stale lock through the `bazel mod tidy` hook, whose `files` is `go\.(mod\|work\|sum)$` and never sees `.bazelversion`
`meta/scripts/README.md`	Read as if each guard ran on all three surfaces; only `check_secrets_dir.py` has a hook, `check_no_cgo.py` has no on-save task
`ci.yml` shellcheck comment	Described the scope as two files when `*.sh` selects five

The misattributed hook was confirmed by pre-commit run --files .bazelversion --verbose:
bazel mod tidy is Skipped, base-image-pin is the hook that runs.

Verification

All inside the devcontainer:

pre-commit run --all-files → passes, now with seven hooks and no shellcheck.
CI's batched invocation → exit 0. A single-file, workspace-root-cwd invocation against the
container binary — what the extension does — → exit 0.
bazel test //... → 22/22.
Each commit is green on its own: the guard was run against commit 3's own tree, before the hook
was removed.
.vscode/settings.json and extensions.json parse.

Still open

Nothing automated notices if .shellcheckrc is deleted. CI would stay green — its single batched
invocation makes lib.sh an input by accident — and the breakage would show up only as SC1091 in
each developer's editor. Making the CI job lint per file (5 files) would align its invocation shape
with the editor's and close that. Not done here; it changes the job's shape rather than fixing
anything reported.

codecov · 2026-08-18T19:37:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.45%. Comparing base (04cc512) to head (2947da2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #232   +/-   ##
=======================================
  Coverage   84.45%   84.45%           
=======================================
  Files          40       40           
  Lines        2193     2193           
  Branches       99       99           
=======================================
  Hits         1852     1852           
  Misses        325      325           
  Partials       16       16

Components	Coverage Δ
Go	`86.14% <ø> (ø)`
Python	`79.79% <ø> (ø)`
Category: apps	`∅ <ø> (∅)`
Category: infra	`∅ <ø> (∅)`
Category: libs	`∅ <ø> (∅)`
Category: meta	`79.79% <ø> (ø)`
Category: services	`∅ <ø> (∅)`
Category: tools	`86.14% <ø> (ø)`
Project: meta/scripts	`79.79% <ø> (ø)`
Project: tools/network_infrastructure_maintenance	`86.14% <ø> (ø)`

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04cc512...2947da2. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…hing `pre-commit run --all-files` failed SC1091 on the devcontainer plumbing dispatcher: shellcheck follows a `# shellcheck source=` directive only into a file that was itself in the invocation's argument list. ci.yml passes every tracked *.sh at once, so lib.sh counts as an input and the source resolves; pre-commit partitions the file list across cores, leaving devcontainer-plumbing.sh alone in its invocation. Same tree, same config, opposite verdicts -- what the check reported depended on how the caller happened to batch its arguments, so main was red on --all-files and green everywhere else. An rc file rather than `-x` on the hook entry. The hook and the CI job are documented mirrors of each other, so a flag would have to be duplicated in both to keep them agreeing, and a hand-run would still disagree with both; .shellcheckrc is one config every caller reads, alongside .golangci.yml and .bazelrc at the root. Verified in the devcontainer: `pre-commit run --all-files` is clean, each tracked *.sh passes on its own, CI's batched invocation is unchanged, and `bazel test //...` is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Six claims across README, CLAUDE.md, and two workflows disagreed with .pre-commit-config.yaml. README's hook table listed six of the eight hooks -- base-image-pin and shellcheck both run and were absent -- and the paragraph above it left base-image-pin out of the fixing hooks and claimed verification-only checks live in the editor instead of pre-commit. shellcheck is verification-only, blocks the commit, and has no editor counterpart: no extension in .vscode/extensions.json, no on-save task. It is now stated as the deliberate exception, pointing at the hook's own comment rather than restating the why. The editor-integration lead-in claimed to run "the non-fixing checks", wrong in both directions (ruff fixes on save, shellcheck is not wired there at all), and the toolchain list omitted shellcheck entirely. renovate-derived-files.yml and .claude/CLAUDE.md both routed a .bazelversion-induced stale lock through the `bazel mod tidy` hook, whose `files` is go\.(mod|work|sum)$ and never sees .bazelversion. Verified with `pre-commit run --files .bazelversion --verbose`: bazel mod tidy is skipped and base-image-pin is the hook that runs, rewriting the lock as a side effect of its `bazel build`. meta/scripts/README.md read as if each guard ran on all three surfaces; only check_secrets_dir.py has a hook and check_no_cgo.py has no on-save task, which its own table already showed. ci.yml's shellcheck comment described the scope as two files when *.sh selects five. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The table and the paragraph above it name hooks by id, and nothing coupled either to .pre-commit-config.yaml -- so a hook added, removed, or renamed left the prose stale with every check still green. Four of the six doc errors fixed in the previous commit were exactly that. It reads both real files rather than restating either, in the shape //.devcontainer:test_base_image_pin already uses: the coupling is between two checked-in files, so the assertion is the whole gate, and it rides `bazel test //...` instead of costing a CI job. Three rules -- the table lists every hook in config order, the prose accounts for each one, and the prose names none the config lacks. The scan behind that last rule is unit-tested on synthetic text, because writing it against the real README first produced a rule that passed vacuously: the ``` fences in the install snippet are backticks too, and they offset every inline-code pair after them into nonsense. Deliberately not asserted: the "Triggers on" column. It paraphrases each `files` regex for a reader, and checking a gloss against a regex would either restate the regex in the README or accept anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Verification-only checks belong in the editor in this repo -- they surface findings without blocking a commit when you want to switch contexts, and CI is what gates them. shellcheck was the lone exception, documented as deliberate but never earning it: the CI job enforces shell lint either way, so the hook only bought an earlier failure at the cost of a blocked commit. timonwong.shellcheck takes over, pointed at the devcontainer's /usr/bin/shellcheck rather than the extension's bundled binary, so the editor and CI can only disagree by the version skew ci.yml already documents. useWorkspaceRootAsCwd is load-bearing rather than cosmetic: `# shellcheck source=` directives are repo-root-relative and .shellcheckrc sits at the root, so linting from the file's own directory breaks both. This makes .shellcheckrc more load-bearing, not less. The editor lints the open buffer alone, which is the single-file invocation shape that raised SC1091 in the first place; without external-sources it would flag devcontainer-plumbing.sh on every save while CI's batched run stayed clean. The docs follow the hook. README's pre-commit paragraph loses its exception clause and is literally true again, the hook table loses a row, the on-save table and the extension list gain one, and meta/devcontainer-base/README.md stops claiming a pre-commit hook that a consuming repo would not find. Verified: `pre-commit run --all-files` passes with seven hooks and no shellcheck; the batched CI invocation and a single-file editor-shaped one both exit 0; `bazel test //...` is 22/22, including the drift guard added in the previous commit, which caught both the stale table row and the stale prose mention when the hook came out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Included by an over-broad `git add -A` in the previous commit; they are working files, not part of the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…nary pin Review caught the editor half of the previous commit never actually arriving. `.vscode/extensions.json` only raises a dismissable "install recommended extensions?" prompt; devcontainer.json's customizations.vscode.extensions is the list the CLI installs. shellcheck was in the first and not the second, so the shellcheck.* settings bound to nothing -- and with the pre-commit hook already gone, a rebuilt container had no local shell lint at all while README claimed on-save coverage. The two lists were an exact containment before this branch (every recommendation installed; devcontainer.json's only extra is the container-only task manager) and nothing held them that way, so //.devcontainer:test_devcontainer_config now asserts it. The compare is case-folded: the files already disagree on the case of Gruntfuggly.triggertaskonsave, which is fine, because marketplace ids are case-insensitive. shellcheck.executablePath moves to devcontainer.json's container-scoped settings, beside the /usr/bin/rg pin that is there for the same reason. .vscode/settings.json is workspace-scoped and loads in a host window too, where /usr/bin/shellcheck does not exist and the extension fails to spawn rather than falling back. Split this way a host window lints with the bundled binary and the container keeps the pinned one; the README bullet now says so. Verified: the coupling assertion fails when the extension is taken back out of devcontainer.json and passes with it; `bazel test //...` is 22/22; `pre-commit run --all-files` passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The previous commit asserted containment: every entry in .vscode/extensions.json also in devcontainer.json's install list. That rule is wrong -- it collapses the two lists into one and leaves no way to recommend an extension without bundling it into the image, which is a thing the recommendation list exists to allow. The lists already differ in the other direction too: cnshenj.vscode-task-manager is installed and deliberately not recommended. The invariant worth holding is narrower. An extension this repo *configures* has to be installed, or the settings bind to nothing and the feature is silently absent -- which is exactly how the shellcheck wiring first shipped: settings written, README updated, extension never installed, no error anywhere. So the assertion now runs settings -> extension, over both scopes (.vscode/settings.json and devcontainer.json's own settings block), and says nothing about recommendations in either direction. Three supporting rules, because the mapping from a settings namespace to the extension that contributes it has no offline registry -- it lives in each extension's package.json -- and so has to be a hand-kept table: - every namespace is either mapped to an extension or listed as a VS Code built-in, so a new extension's settings cannot quietly miss the table and go unchecked; - no mapping outlives the settings that justified it; - `editor.defaultFormatter` values are checked too, since that setting names an extension by id rather than by namespace. Verified by breaking each rule in turn: a configured extension pulled from the install list, an unclassified namespace, a stale mapping, and a formatter naming an uninstalled extension each fail their own test -- and a recommendation that is not installed keeps the suite green, which the previous rule did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The recommendation list had become a duplicate: all 16 entries were also in devcontainer.json's install list, so it added nothing for anyone using the devcontainer and was a second hand-copy to keep in step -- the drift review caught on this branch was between exactly these two files. They serve different windows, though, so the answer is to make them differ on purpose rather than to delete one. devcontainer.json installs into the container silently; .vscode/extensions.json is a dismissable prompt and the only list that does anything in a window that has not reopened in the container -- which .claude/CLAUDE.md sanctions for editing files. So the recommendations keep only what a host window can actually use: prettier, yaml, todo-tree, rainbow-csv, github-actions. The tooling behind the other eleven -- go, ruff, ty, bazel, the coverage report, the on-save task scripts, shellcheck's pinned binary -- lives in the container, and recommending an extension whose tooling is absent just installs something broken. Editing markdown or YAML on the host is the case this serves, and settings.json's prettier and todo-tree settings do apply there. The Docker and Kubernetes extensions come out too. They would work host-side, but operating containers and clusters is not editing this repo, and their in-container state is already a known limitation. Nothing is lost in the container: all seventeen installs are unchanged. Two comments that described the old mirror are corrected -- settings.json attributed triggertaskonsave to the recommendation list, and the future-considerations item said the devcontainer list mirrors it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Review caught the pruning leaving a reference behind. `editor.formatOnSave` plus a `[python]` block naming charliermarsh.ruff stayed at workspace scope, which a host window reads, while the extension itself stopped being recommended there. VS Code raises "configured as formatter but it is not available" on every save in that window, and the file goes unformatted. `editor.defaultFormatter` is the only setting in these files that behaves that way. An unknown `go.*`, `coverage-gutters.*`, `taskManager.*` or `triggerTaskOnSave.*` key is inert where its extension is absent, which is why those can stay put. The block moves into devcontainer.json's customizations.vscode.settings, the same move 5617098 made for shellcheck.executablePath and for the same reason: Python is edited in the container, where the pinned ruff is the one CI runs. The test now takes scope into account rather than checking every formatter against the install list, which is what let this through: a formatter named in workspace settings has to be reachable from both lists, and one named in container settings only from the install list. Verified by putting the [python] block back at workspace scope -- the new assertion fails with the fix it wants. Two prose claims went with it. The README bullet promised host windows a fallback to the extension's bundled shellcheck, which the pruning had already removed, and extensions.json justified its exclusions by tooling living in the container -- true for go, ty, bazel and the task scripts, but not for ruff and shellcheck, which both ship a bundled binary. Those two are container-side by choice, so the comment now says that instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

c5db85d corrected why ruff and shellcheck are left out of the recommendations -- both ship bundled binaries and would run on a host, so they are container-side by choice rather than by necessity -- in extensions.json's own comment and in the shellcheck bullet. The lead-in seventeen lines above still gave the retired reason, "since the tooling behind the rest lives in the container", which is the first version a reader meets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Review pointed out that "the pinned versions are the ones CI runs" held for ruff and not for shellcheck: the Dockerfile installed it as a bare `apt-get install shellcheck` and CI used whatever the runner image shipped -- two sources, separately versioned, exactly as ci.yml's own comment admitted. Rather than soften the claim, make it true. The Dockerfile now installs the upstream release at SHELLCHECK_VERSION, carrying the same `# renovate: datasource=github-releases` comment as BAZELISK_VERSION and BUILDIFIER_VERSION beside it, so the existing Dockerfile regex manager tracks it with no renovate.json change. Release assets use uname-style arch names, which dpkg's differ from, hence the case; an unmapped arch fails the build rather than skipping the install. CI reads that ARG out of the Dockerfile instead of restating it. A second pin in the workflow could drift from the container's, and the failure mode of that drift is a finding whose existence depends on which window you were looking at -- the thing this job is supposed to be authoritative about. An ARG that moves or gets renamed fails the step rather than falling through to the runner's copy. v0.11.0 rather than Debian's 0.10.0, which is what apt was giving us: verified clean on every tracked *.sh, batched and one file at a time, before pinning. The binary moves to /usr/local/bin with the other downloaded tools, so devcontainer.json's executablePath follows it -- and the old path is now absent, which would have left the extension unable to spawn. Two silent couplings, so both are asserted from the files: the extension points where the Dockerfile installs, and ci.yml names no version of its own. Verified in a rebuilt container: shellcheck resolves to /usr/local/bin/shellcheck at 0.11.0, dpkg no longer owns /usr/bin/ shellcheck, lint is clean batched and single-file, and both new assertions fail when their coupling is broken. bazel test //... is 22/22 and pre-commit run --all-files passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The job installed the pinned shellcheck to /usr/local/bin and printed the resolved version, but never compared the two. "Ahead of the runner's own copy on PATH" is an assumption about the runner image, and several directories precede /usr/local/bin there -- linuxbrew, pipx_bin, ~/.local/bin -- as does any tool-cache dir a setup-* action prepends. If one ever carries a shellcheck, the gate silently returns to the two-version world the pin exists to end, on a green run whose only evidence is a log line nobody reads on a passing build. The check goes in the install step rather than the lint step because that is where the pin is already in scope; PATH is the same across both, and nothing between them touches it. Also narrows the companion assertion's failure output. assertIn renders its haystack, so a renamed ARG dumped all ~10KB of ci.yml -- the same reason the install path above it is pulled out before comparing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Syndic changed the title ~~fix(shellcheck): follow source= directives regardless of input batching~~ fix(shellcheck): follow source= directives, and move the check into the editor Aug 18, 2026