fix(shellcheck): follow source= directives, and move the check into the editor

.claude/CLAUDE.md

@@ -117,12 +117,13 @@ the `Renovate helper`. Load-bearing facts:
 - **`.bazelversion` invalidates the lock too.** `MODULE.bazel.lock`'s `lockFileVersion` (and the
   shape of its recorded extensions) tracks the bazel release, so a Renovate bazel bump leaves the
   committed lock stale. Builds don't notice — `--lockfile_mode=update` rewrites in memory and stays
-  green — so the staleness surfaces either as a blocked local commit (the `bazel mod tidy`
-  pre-commit hook rewrites it on disk on any `go.mod` change) or as ci.yml's `MODULE.bazel.lock
-  freshness` job, which is the backstop for commits that ran no hook. The workflow therefore
-  triggers on `.bazelversion` and folds it into the same `bazel` classification as `MODULE.bazel`:
-  one output, one `bazel mod deps` refresh. bazelisk reads the checked-out `.bazelversion`, so the
-  regenerated lock is in the bumped version's format.
+  green — so the staleness surfaces either as a blocked local commit (`base-image-pin` selects
+  `.bazelversion` itself and rewrites the lock as a side effect of its `bazel build`;
+  `bazel mod tidy` does the same, but only on a `go.mod`/`go.work`/`go.sum` change) or as ci.yml's
+  `MODULE.bazel.lock freshness` job, which is the backstop for commits that ran no hook. The
+  workflow therefore triggers on `.bazelversion` and folds it into the same `bazel` classification
+  as `MODULE.bazel`: one output, one `bazel mod deps` refresh. bazelisk reads the checked-out
+  `.bazelversion`, so the regenerated lock is in the bumped version's format.
 - **Go tidy/sync rides the same commit.** Renovate's `go get` bumps `go.mod`/`go.sum` but never
   runs `go mod tidy` (opt-in) or `go work sync` (Renovate does it only when vendoring, which this
   repo doesn't) — so the indirect block and `go.work.sum` are left stale. The workflow runs

.devcontainer/BUILD.bazel

@@ -22,7 +22,10 @@ py_test(
         "devcontainer.json",
         "post-create.sh",
         "post-start.sh",
+        "//:.github/workflows/ci.yml",
         "//:.github/workflows/devcontainer.yml",
+        "//:.vscode/extensions.json",
+        "//:.vscode/settings.json",
     ],
     main = "test_devcontainer_config.py",
     deps = ["//meta/scripts:sync_base_image_pin_lib"],

.devcontainer/Dockerfile

@@ -12,6 +12,9 @@ ARG BAZELISK_VERSION=v1.29.0
 # renovate: datasource=github-releases depName=bazelbuild/buildtools
 ARG BUILDIFIER_VERSION=v8.5.1
 
+# renovate: datasource=github-releases depName=koalaman/shellcheck
+ARG SHELLCHECK_VERSION=v0.11.0
+
 # uv: the workspace's Python package manager. Single source of truth for the
 # uv.lock + requirements_lock.txt chain consumed by rules_python's pip.parse
 # and enforced by the `uv-lock-fresh` pre-commit hook. Installed first so
@@ -73,7 +76,6 @@ RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-ins
         ca-certificates \
         curl \
         ripgrep \
-        shellcheck \
         unzip \
         zip \
     && rm -rf /var/lib/apt/lists/*
@@ -89,6 +91,22 @@ RUN ARCH="$(dpkg --print-architecture)" \
         "https://github.com/bazelbuild/buildtools/releases/download/${BUILDIFIER_VERSION}/buildifier-linux-${ARCH}" \
     && chmod 0755 /usr/local/bin/buildifier
 
+# shellcheck from upstream rather than Debian's package, so the version is ours to pin: this ARG
+# is the repo's only shellcheck version, and ci.yml reads it out of this file instead of using
+# the runner image's copy. The editor extension is pointed at the binary this installs.
+# Release assets use uname-style arch names, which dpkg's differ from.
+RUN ARCH="$(dpkg --print-architecture)" \
+    && case "${ARCH}" in \
+        amd64) SC_ARCH=x86_64 ;; \
+        arm64) SC_ARCH=aarch64 ;; \
+        *) echo "no shellcheck release for ${ARCH}" >&2; exit 1 ;; \
+    esac \
+    && curl -fsSL \
+        "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.${SC_ARCH}.tar.xz" \
+        | tar -xJ -C /tmp \
+    && install -m 0755 "/tmp/shellcheck-${SHELLCHECK_VERSION}/shellcheck" /usr/local/bin/shellcheck \
+    && rm -rf "/tmp/shellcheck-${SHELLCHECK_VERSION}"
+
 # No host-specific layers here on purpose: the git-common-dir symlink and the host timezone are
 # applied at container start by the base image's dispatcher, which keeps this image host-agnostic.
 # See meta/devcontainer-base/README.md.

.devcontainer/devcontainer.json

@@ -91,9 +91,20 @@
         "ms-python.python",
         "redhat.vscode-yaml",
         "ryanluker.vscode-coverage-gutters",
+        "timonwong.shellcheck",
       ],
       "settings": {
-        "todo-tree.ripgrep.ripgrep": "/usr/bin/rg"
+        "todo-tree.ripgrep.ripgrep": "/usr/bin/rg",
+        "// Ruff format-on-save": "charliermarsh.ruff surfaces diagnostics inline; this block makes it the default Python formatter so `editor.formatOnSave` calls ruff format, and runs ruff check --fix + import-organize as save-time code actions. Config is read from `[tool.ruff]` in //:pyproject.toml — single source of truth shared with CI + pre-commit. Container-scoped because `editor.defaultFormatter` is the one setting that raises when its extension is absent, and ruff is not recommended for host windows.",
+        "[python]": {
+          "editor.defaultFormatter": "charliermarsh.ruff",
+          "editor.codeActionsOnSave": {
+            "source.fixAll.ruff": "explicit",
+            "source.organizeImports.ruff": "explicit"
+          }
+        },
+        "// ShellCheck binary": "Container-scoped on purpose: this path exists only in here, and .vscode/settings.json would apply it to a host window too, where the extension fails to spawn rather than falling back. The Dockerfile installs it from a pinned upstream release and ci.yml reads that same pin, so the editor and the gating job run the same version; a host window uses the extension's bundled copy instead.",
+        "shellcheck.executablePath": "/usr/local/bin/shellcheck"
       }
     }
   }

.devcontainer/test_devcontainer_config.py

@@ -1,6 +1,6 @@
-"""Tests for the wiring between devcontainer.json, the Dockerfile, and the base image.
+"""Tests for the wiring between devcontainer.json, the Dockerfile, the base image, and .vscode.
 
-Three couplings live across those files and none of them fails loudly:
+Four couplings live across those files and none of them fails loudly:
 
   - The `BASE_IMAGE` override has an exact working shape. Every nearby shape either breaks
     every local `devcontainer up` (an empty `--build-arg` overriding the Dockerfile default)
@@ -10,8 +10,11 @@
   - The lifecycle hooks call the command the base image installs. Calling the workspace copy
     instead still works here — the scripts are checked in — so the mistake would only surface
     in Syndic/.dotfiles, which has no such copy.
+  - An extension this repo configures is installed by devcontainer.json. Settings for an
+    absent extension bind to nothing, so the feature is missing with no error anywhere.
 
-The rationale for all three is in meta/devcontainer-base/README.md, "Consuming the image".
+The rationale for the first three is in meta/devcontainer-base/README.md, "Consuming the
+image".
 The parsing helpers are pure so they can be exercised directly, same split as the shell tests
 in this directory.
 """
@@ -28,6 +31,9 @@
 # which a resolved symlink would lead back out of. The rest read fine either way.
 _HERE = Path(__file__).parent
 _DEVCONTAINER_JSON = _HERE / "devcontainer.json"
+_SETTINGS_JSON = _HERE.parent / ".vscode" / "settings.json"
+_EXTENSIONS_JSON = _HERE.parent / ".vscode" / "extensions.json"
+_CI_WORKFLOW = _HERE.parent / ".github" / "workflows" / "ci.yml"
 _DOCKERFILE = _HERE / "Dockerfile"
 _HOOKS = (_HERE / "post-create.sh", _HERE / "post-start.sh")
 _DEVCONTAINER_WORKFLOW = _HERE.parent / ".github" / "workflows" / "devcontainer.yml"
@@ -171,6 +177,171 @@ def test_escaped_quote_does_not_end_the_string(self):
         )
 
 
+# Settings namespace -> the extension that contributes it. There is no offline registry for
+# this — the link lives in each extension's own package.json — so it is a hand-kept table, and
+# test_no_stale_mappings keeps it from rotting.
+_CONFIGURED_BY = {
+    "bazel": "bazelbuild.vscode-bazel",
+    "coverage-gutters": "ryanluker.vscode-coverage-gutters",
+    "go": "golang.go",
+    "prettier": "esbenp.prettier-vscode",
+    "python": "ms-python.python",
+    "shellcheck": "timonwong.shellcheck",
+    "taskManager": "cnshenj.vscode-task-manager",
+    "todo-tree": "gruntfuggly.todo-tree",
+    "triggerTaskOnSave": "Gruntfuggly.triggertaskonsave",
+    "yaml": "redhat.vscode-yaml",
+}
+
+# VS Code's own namespaces, which no extension has to supply.
+_BUILT_IN = frozenset({"editor", "files"})
+
+
+def settings_namespaces(settings: dict) -> set[str]:
+    """First segment of every real setting key. `// ...` are doc keys, `[lang]` are blocks."""
+    return {
+        key.split(".")[0]
+        for key in settings
+        if not key.startswith("//") and not key.startswith("[")
+    }
+
+
+def named_formatters(settings: dict) -> set[str]:
+    """Extension ids named outright as a formatter, at top level or in a [language] block."""
+    found = set()
+    for key, value in settings.items():
+        if key == "editor.defaultFormatter" and isinstance(value, str):
+            found.add(value)
+        elif key.startswith("[") and isinstance(value, dict):
+            found |= named_formatters(value)
+    return found
+
+
+class TestConfiguredExtensions(unittest.TestCase):
+    """Settings wire the editor to extensions; only devcontainer.json installs them.
+
+    `.vscode/extensions.json` is a recommendation list — a dismissable prompt — and is
+    deliberately not asserted against the install list in either direction. Recommending
+    something the container does not bundle is a legitimate thing to do, and the container
+    bundles one the list does not recommend (the task manager, which is container-only).
+
+    What has to hold is narrower: an extension this repo *configures* must be installed, or
+    the settings bind to nothing. That is how the shellcheck wiring first shipped — settings
+    and docs in place, extension absent, nothing raising a word about it.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        config = json.loads(strip_jsonc(_DEVCONTAINER_JSON.read_text(encoding="utf-8")))
+        vscode = config["customizations"]["vscode"]
+        cls.installed = {name.casefold() for name in vscode["extensions"]}
+        cls.workspace = json.loads(strip_jsonc(_SETTINGS_JSON.read_text(encoding="utf-8")))
+        cls.container = vscode["settings"]
+        cls.settings = (cls.workspace, cls.container)
+        cls.namespaces = set().union(*(settings_namespaces(s) for s in cls.settings))
+        recommendations = json.loads(strip_jsonc(_EXTENSIONS_JSON.read_text(encoding="utf-8")))
+        cls.recommended = {name.casefold() for name in recommendations["recommendations"]}
+
+    def test_every_configured_extension_is_installed(self):
+        configured = {_CONFIGURED_BY[ns] for ns in self.namespaces if ns in _CONFIGURED_BY}
+        missing = sorted(e for e in configured if e.casefold() not in self.installed)
+        self.assertEqual(
+            missing,
+            [],
+            "configured in settings but not in devcontainer.json's extensions list",
+        )
+
+    def test_every_settings_namespace_is_accounted_for(self):
+        """Without this, a new extension's settings simply miss the table and go unchecked."""
+        unknown = sorted(self.namespaces - set(_CONFIGURED_BY) - _BUILT_IN)
+        self.assertEqual(
+            unknown,
+            [],
+            "settings namespaces that are neither mapped in _CONFIGURED_BY nor listed in "
+            "_BUILT_IN — add each to whichever it is",
+        )
+
+    def test_no_stale_mappings(self):
+        stale = sorted(set(_CONFIGURED_BY) - self.namespaces)
+        self.assertEqual(stale, [], "_CONFIGURED_BY entries whose settings are gone")
+
+    def test_formatters_named_in_container_settings_are_installed(self):
+        """`editor.defaultFormatter` names an extension by id rather than by namespace."""
+        missing = sorted(
+            n for n in named_formatters(self.container) if n.casefold() not in self.installed
+        )
+        self.assertEqual(missing, [], "named as editor.defaultFormatter but not installed")
+
+    def test_formatters_named_in_workspace_settings_reach_both_windows(self):
+        """Scope decides which list has to carry the extension.
+
+        `editor.defaultFormatter` is the one setting that raises when its extension is absent —
+        "configured as formatter but it is not available", on every save — where an unknown
+        `go.*` or `coverage-gutters.*` key is simply inert. Workspace settings load in a host
+        window as well as in the container, so a formatter named here has to be reachable from
+        both lists.
+
+        Empty today by construction: the [python] block lives in devcontainer.json's
+        container-scoped settings precisely because ruff is not recommended host-side. That is
+        the point — this fires the moment a formatter is named at a scope the host also reads.
+        """
+        named = named_formatters(self.workspace)
+        self.assertEqual(
+            sorted(n for n in named if n.casefold() not in self.recommended),
+            [],
+            "named as a formatter in .vscode/settings.json, which a host window reads, but not "
+            "recommended there — move the block to devcontainer.json or recommend the extension",
+        )
+        self.assertEqual(
+            sorted(n for n in named if n.casefold() not in self.installed),
+            [],
+            "named as a formatter in .vscode/settings.json but not installed in the container",
+        )
+
+
+class TestPinnedShellcheck(unittest.TestCase):
+    """One binary and one pin across the Dockerfile, devcontainer.json and ci.yml.
+
+    Each reference fails quietly on its own: a wrong `executablePath` makes the extension fail
+    to spawn, so the editor shows no diagnostics and no error worth noticing, and a version
+    restated in ci.yml drifts from the container's, which makes whether a finding exists depend
+    on where you looked. Neither shows up in a build or a green run.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        cls.dockerfile = _DOCKERFILE.read_text(encoding="utf-8")
+        config = json.loads(strip_jsonc(_DEVCONTAINER_JSON.read_text(encoding="utf-8")))
+        cls.settings = config["customizations"]["vscode"]["settings"]
+        cls.workflow = _CI_WORKFLOW.read_text(encoding="utf-8")
+
+    def test_the_extension_points_at_the_binary_the_dockerfile_installs(self):
+        # Pulled out rather than matched in place, so a mismatch reports two paths instead of
+        # the whole Dockerfile.
+        installed = re.search(r'install -m 0755 "[^"]*/shellcheck" (\S+)', self.dockerfile)
+        self.assertIsNotNone(installed, "no shellcheck install line found in the Dockerfile")
+        self.assertEqual(
+            self.settings["shellcheck.executablePath"],
+            installed.group(1),
+            "devcontainer.json points the extension somewhere other than where the Dockerfile "
+            "installs shellcheck",
+        )
+
+    def test_ci_reads_the_pin_instead_of_restating_it(self):
+        # assertTrue, not assertIn: a failing assertIn renders the whole workflow as the
+        # haystack, same reason the path above is pulled out before comparing.
+        self.assertTrue(
+            "ARG SHELLCHECK_VERSION=" in self.workflow,
+            "ci.yml no longer derives the shellcheck version from the Dockerfile",
+        )
+        restated = re.findall(r"shellcheck-v\d+\.\d+", self.workflow)
+        self.assertEqual(
+            restated,
+            [],
+            "ci.yml names a shellcheck version of its own; it must read the Dockerfile's ARG",
+        )
+
+
 class TestLocalEnvParsing(unittest.TestCase):
     def test_default_is_returned(self):
         self.assertEqual(parse_local_env("${localEnv:X:fallback}"), ("X", "fallback"))

.github/workflows/ci.yml

@@ -161,18 +161,44 @@ jobs:
           args: format --check
       - run: ruff check
 
-  # shellcheck: mirrors the pre-commit hook of the same name. The runner image ships
-  # shellcheck, so there's nothing to install. Scoped to *.sh, which today means the host stub
-  # in .devcontainer/ and the shared library in meta/devcontainer-base/ — privileged writes
-  # against /etc and host-absolute paths, where `set -e` foot-guns (errexit suspension inside
-  # `||` callees, most recently) are worth catching mechanically rather than in review.
-  # The runner's shellcheck and the devcontainer's Debian package are separately versioned
-  # with no shared pin, so the two can disagree; CI is authoritative if they ever do.
+  # shellcheck: the enforcing half. The editor surfaces the same findings inline on save
+  # (timonwong.shellcheck, configured in .vscode/settings.json), but nothing blocks a commit on
+  # them, so this job is where shell lint is actually gated. Scoped to *.sh, which today means
+  # .devcontainer/'s host stub and lifecycle hooks plus meta/devcontainer-base/scripts/ —
+  # privileged writes against /etc and host-absolute paths, where `set -e` foot-guns (errexit
+  # suspension inside `||` callees, most recently) are worth catching mechanically rather than
+  # in review.
+  # The version is the devcontainer's, read out of its Dockerfile rather than restated here —
+  # the runner image ships its own shellcheck, and using it would make the gating job and the
+  # editor separately versioned, with a finding's presence depending on where you looked.
   shellcheck:
     name: shellcheck
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      - name: Install the pinned shellcheck
+        run: |
+          set -euo pipefail
+          version="$(sed -n 's/^ARG SHELLCHECK_VERSION=//p' .devcontainer/Dockerfile)"
+          # An ARG that moved or got renamed must fail here, not silently fall through to
+          # whatever the runner image happens to ship.
+          if [ -z "${version}" ]; then
+            echo "no SHELLCHECK_VERSION in .devcontainer/Dockerfile" >&2
+            exit 1
+          fi
+          curl -fsSL \
+            "https://github.com/koalaman/shellcheck/releases/download/${version}/shellcheck-${version}.linux.x86_64.tar.xz" \
+            | tar -xJ -C /tmp
+          # Ahead of the runner's own copy on PATH.
+          sudo install -m 0755 "/tmp/shellcheck-${version}/shellcheck" /usr/local/bin/shellcheck
+          # That ordering is an assumption about the runner image, and the whole point of the
+          # pin is that the gate and the editor cannot disagree — so a shadowed binary has to
+          # fail here rather than lint green under some other version.
+          on_path="$(shellcheck --version | sed -n 's/^version: //p')"
+          if [ "${on_path}" != "${version#v}" ]; then
+            echo "shellcheck on PATH is ${on_path}, expected ${version#v}" >&2
+            exit 1
+          fi
       - run: |
           set -euo pipefail
           shellcheck --version | sed -n '2p'

.github/workflows/renovate-derived-files.yml

@@ -66,7 +66,8 @@ on:
       # A bazel version bump re-derives the lock too: `lockFileVersion` and the recorded
       # extension shape track the bazel release, so a bumped `.bazelversion` leaves the
       # committed lock stale. CI hides that (`--lockfile_mode=update` rewrites in memory),
-      # but the `bazel mod tidy` pre-commit hook rewrites it on disk and blocks the commit.
+      # but the `base-image-pin` pre-commit hook — the one whose `files` selects `.bazelversion`
+      # — rewrites it on disk as a side effect of its `bazel build` and blocks the commit.
       - ".bazelversion"
       - "pyproject.toml"
       - "uv.lock"