diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 8c18002..74e7ea7 100644 --- a/.claude/CLAUDE.md +++ b/.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 diff --git a/.devcontainer/BUILD.bazel b/.devcontainer/BUILD.bazel index 3dc3d11..319a311 100644 --- a/.devcontainer/BUILD.bazel +++ b/.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"], diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 9921ec2..8dd945c 100644 --- a/.devcontainer/Dockerfile +++ b/.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. diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 9262e37..e5a9bde 100644 --- a/.devcontainer/devcontainer.json +++ b/.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" } } } diff --git a/.devcontainer/test_devcontainer_config.py b/.devcontainer/test_devcontainer_config.py index 2f23811..03a2792 100644 --- a/.devcontainer/test_devcontainer_config.py +++ b/.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")) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 817b2f4..aeb22aa 100644 --- a/.github/workflows/ci.yml +++ b/.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' diff --git a/.github/workflows/renovate-derived-files.yml b/.github/workflows/renovate-derived-files.yml index ebd8de1..f044c4d 100644 --- a/.github/workflows/renovate-derived-files.yml +++ b/.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" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5b67d8..d906eb2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -64,15 +64,6 @@ repos: pass_filenames: false files: \.go$ - # The devcontainer plumbing runs privileged writes against /etc and host-absolute - # paths, and `set -e` interacts badly enough with && chains that the foot-guns are - # worth catching mechanically. Mirrored by the shellcheck job in ci.yml. - - id: shellcheck - name: shellcheck - language: system - entry: shellcheck - files: \.sh$ - - id: check-secrets-dir name: check secrets directory language: python diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 0000000..0e48776 --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,6 @@ +# Follow `# shellcheck source=` directives into files outside the invocation's argument list. +# Without this, whether SC1091 fires depends on how the caller batches its arguments: ci.yml +# passes every tracked *.sh at once, so lib.sh counts as an input and the source is followed, +# while the editor lints the open buffer alone and would flag devcontainer-plumbing.sh on every +# save. An rc file rather than `-x` per caller so both read one config. +external-sources=true diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 3a03be8..041d6f4 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,19 +1,21 @@ { + // Host-window recommendations, not a mirror of what the devcontainer installs. That list is + // devcontainer.json's `customizations.vscode.extensions`, which auto-installs inside the + // container; this one is a dismissable prompt and is the only list that does anything in a + // window that has not reopened in the container. + // + // It carries what is useful for editing this repo on the host: markdown, YAML, workflows, + // CSVs. Most of the rest cannot work there at all — go, ty, bazel, the coverage report and + // the on-save task scripts all drive tooling that lives in the container. Ruff and ShellCheck + // are the two that could, since both ship a bundled binary, and are left out deliberately + // rather than for that reason: Python and shell are edited in the container, where the pinned + // ruff and shellcheck are the same versions CI runs. Their settings are container-scoped to + // match. "recommendations": [ - "astral-sh.ty", - "bazelbuild.vscode-bazel", - "charliermarsh.ruff", "esbenp.prettier-vscode", "github.vscode-github-actions", - "golang.go", "gruntfuggly.todo-tree", - "gruntfuggly.triggertaskonsave", "mechatroner.rainbow-csv", - "ms-azuretools.vscode-containers", - "ms-kubernetes-tools.vscode-kubernetes-tools", - "ms-python.debugpy", - "ms-python.python", "redhat.vscode-yaml", - "ryanluker.vscode-coverage-gutters", ] } diff --git a/.vscode/settings.json b/.vscode/settings.json index 016079b..59b7c2a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -18,14 +18,9 @@ "prettier.proseWrap": "always", "prettier.trailingComma": "all", "python.languageServer": "Default", - "// 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.", - "[python]": { - "editor.defaultFormatter": "charliermarsh.ruff", - "editor.codeActionsOnSave": { - "source.fixAll.ruff": "explicit", - "source.organizeImports.ruff": "explicit" - } - }, + "// ShellCheck": "timonwong.shellcheck surfaces shellcheck diagnostics inline, matching what the CI `shellcheck` job enforces. The extension is container-side only, so executablePath is set in devcontainer.json's customizations.vscode.settings; these two keys are harmless where it is absent, unlike a formatter reference. useWorkspaceRootAsCwd is load-bearing: `# shellcheck source=` directives are repo-root-relative and .shellcheckrc lives at the root, so linting from the file's own directory would break both.", + "shellcheck.run": "onSave", + "shellcheck.useWorkspaceRootAsCwd": true, "// Task Manager favorites": "Pins the Bazel build/test tasks to the cnshenj.vscode-task-manager sidebar (recommended in the devcontainer; see devcontainer.json). Names must match the `label` fields in tasks.json. The `UnnaturalDesigns/Workspace/` prefix is Task Manager's own scoping convention.", "taskManager.favorites": [ "UnnaturalDesigns/Workspace/Bazel Build all", @@ -44,7 +39,7 @@ "[]", "[x]" ], - "// Repo-health checks on save": "These scripts run via the VS Code tasks defined in tasks.json, triggered by gruntfuggly.triggertaskonsave (recommended in extensions.json) when the matching files are saved. The matchers in tasks.json turn each `path:line: message` failure into a Problems-panel entry with a squiggle at the offending line.", + "// Repo-health checks on save": "These scripts run via the VS Code tasks defined in tasks.json, triggered by gruntfuggly.triggertaskonsave (installed by devcontainer.json; it drives the container's tooling, so it is not among the host-window recommendations) when the matching files are saved. The matchers in tasks.json turn each `path:line: message` failure into a Problems-panel entry with a squiggle at the offending line.", "triggerTaskOnSave.tasks": { "check: modules": [ "**/go.mod", diff --git a/BUILD.bazel b/BUILD.bazel index 4d7c4eb..9c334f9 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -8,6 +8,21 @@ gazelle(name = "gazelle") # the env var devcontainer.yml sets for devcontainer.json's BASE_IMAGE override. A stale copy # stays green without this, so the tests read the real file. exports_files([ + ".github/workflows/ci.yml", ".github/workflows/devcontainer.yml", ".github/workflows/renovate-derived-files.yml", ]) + +# Same posture for the pre-commit config and the README that documents it: the hook table is a +# hand-copy of the config's ids, so //meta/scripts:test_precommit_docs reads both real files. +exports_files([ + ".pre-commit-config.yaml", + "README.md", +]) + +# Editor settings name extensions, and which list has to carry one depends on the settings +# scope — see //.devcontainer:test_devcontainer_config. +exports_files([ + ".vscode/extensions.json", + ".vscode/settings.json", +]) diff --git a/README.md b/README.md index 3726d27..5b2886b 100644 --- a/README.md +++ b/README.md @@ -33,11 +33,11 @@ Container_ from the Command Palette. First build takes a few minutes; subsequent [`gh`](https://cli.github.com), [`uv`](https://docs.astral.sh/uv/) (Python package manager), [`ruff`](https://docs.astral.sh/ruff/) (Python format + lint), [`ty`](https://docs.astral.sh/ty/) (Python type checker, alpha), -[pre-commit](https://pre-commit.com), and [`golangci-lint`](https://golangci-lint.run). All Python -tools (`ruff`, `ty`, `pre-commit`) are installed via `uv tool install` at image build time, so the -devcontainer has a single Python package manager (uv) and no `pip install --user` in post-create. -Named volumes (`ud-bazel-cache`, `ud-go-cache`) preserve the Bazel and Go caches across container -rebuilds. +[pre-commit](https://pre-commit.com), [`golangci-lint`](https://golangci-lint.run), and +[`shellcheck`](https://www.shellcheck.net) (shell lint). All Python tools (`ruff`, `ty`, +`pre-commit`) are installed via `uv tool install` at image build time, so the devcontainer has a +single Python package manager (uv) and no `pip install --user` in post-create. Named volumes +(`ud-bazel-cache`, `ud-go-cache`) preserve the Bazel and Go caches across container rebuilds. **Base image**: all of that is layered on top of [`meta/devcontainer-base/`](meta/devcontainer-base/README.md)'s published image, which this repo @@ -215,23 +215,31 @@ each commit. To install: pre-commit install ``` -Only hooks that either fix the problem they detect (`bazel-mod-tidy`, `gazelle`, `uv-lock-fresh`, -`ruff-check`, `ruff-format`) or prevent unsafe content from entering the repo (`check-secrets-dir`) -run here. Verification-only checks live in the editor instead (see **Editor integration** below) so -they can surface findings without blocking a commit when you want to switch contexts. +Hooks that fix the problem they detect (`bazel-mod-tidy`, `uv-lock-fresh`, `base-image-pin`, +`ruff-check`, `ruff-format`, `gazelle`) or prevent unsafe content from entering the repo +(`check-secrets-dir`) run here. Verification-only checks live in the editor instead (see **Editor +integration** below) so they can surface findings without blocking a commit when you want to switch +contexts. `//meta/scripts:test_precommit_docs` keeps this list and the table below honest +against [`.pre-commit-config.yaml`](.pre-commit-config.yaml). | Hook | Triggers on | | ------------------- | -------------------------------------------- | | `bazel-mod-tidy` | `go.mod`, `go.work`, `go.sum` | | `uv-lock-fresh` | `pyproject.toml`, `uv.lock`, `requirements_lock.txt` | +| `base-image-pin` | `meta/devcontainer-base/`, `MODULE.bazel`, `.bazelversion` | | `ruff-check` | `*.py` files | | `ruff-format` | `*.py` files | | `gazelle` | `*.go` files | | `check-secrets-dir` | files under `secrets/` | -**Editor integration** (via `.vscode/`) - runs the non-fixing checks on save. Works in VS Code and -VS Code-derived editors (e.g. Google Antigravity). Recommended extensions -(`.vscode/extensions.json`): +**Editor integration** (via `.vscode/`) - runs the checks listed below on save, so findings surface +inline rather than at commit time or in CI. Works in VS Code and VS Code-derived editors (e.g. +Google Antigravity). These extensions are installed automatically in the devcontainer, via +`customizations.vscode.extensions` in [`devcontainer.json`](.devcontainer/devcontainer.json); +[`.vscode/extensions.json`](.vscode/extensions.json) recommends the subset meant for host-window +editing. The rest are container-side: most because their tooling lives there, and `ruff` and +`shellcheck` — which ship bundled binaries and would run on a host — by choice, so the versions +match CI: - [`golang.go`](https://marketplace.visualstudio.com/items?itemName=golang.go) - runs `golangci-lint` on save at package scope, surfacing inline findings that match what CI enforces. @@ -246,6 +254,13 @@ VS Code-derived editors (e.g. Google Antigravity). Recommended extensions `.vscode/settings.json`). - [`ryanluker.vscode-coverage-gutters`](https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters) - paints gutter marks in Go files from a local `bazel coverage //...` run. +- [`timonwong.shellcheck`](https://marketplace.visualstudio.com/items?itemName=timonwong.shellcheck) - + surfaces `shellcheck` diagnostics inline on save, matching what the CI `shellcheck` job + enforces. Config lives in [`.shellcheckrc`](.shellcheckrc), shared with that job, and the + extension is pointed at the container's own `shellcheck` by `devcontainer.json`. Its version + is pinned once, as `SHELLCHECK_VERSION` in the Dockerfile, which the CI job reads — so the + editor and the gate run the same binary. Container-side only: shell is edited in the + devcontainer, so a host window gets no shell lint. | On-save check | Triggers on | | -------------------- | ------------------------------------------ | @@ -254,6 +269,7 @@ VS Code-derived editors (e.g. Google Antigravity). Recommended extensions | `ty` (type diagnostics) | `*.py` files | | `check-modules` | `go.mod`, `pyproject.toml`, `uv.lock`, `requirements_lock.txt`, workflow `.yml`, `.golangci.yml` | | `check-go-work` | `go.mod`, `go.work` | +| `shellcheck` | `*.sh` files | **Viewing coverage locally**: run `bazel coverage //...` from the repo root, then open the Command Palette and pick _Coverage Gutters: Display Coverage_ (or _Watch_ for live updates). The merged lcov diff --git a/docs/future-considerations.md b/docs/future-considerations.md index 4457b0f..45bbc0e 100644 --- a/docs/future-considerations.md +++ b/docs/future-considerations.md @@ -187,8 +187,10 @@ pattern is added. ## Devcontainer: Docker / Kubernetes Extensions Not Fully Wired -The devcontainer recommends a set of VS Code extensions that mirrors `.vscode/extensions.json`, -including `ms-azuretools.vscode-containers` and `ms-kubernetes-tools.vscode-kubernetes-tools`. These extensions install cleanly but are **not +The devcontainer installs a set of VS Code extensions (`customizations.vscode.extensions` in +`devcontainer.json` — no longer a mirror of `.vscode/extensions.json`, which now recommends only +what works in a host window), including `ms-azuretools.vscode-containers` and +`ms-kubernetes-tools.vscode-kubernetes-tools`. These extensions install cleanly but are **not functional inside the container**: - The container extension needs access to a Docker daemon. We have not added Docker-outside-of-Docker diff --git a/meta/devcontainer-base/README.md b/meta/devcontainer-base/README.md index 2463930..630d4c0 100644 --- a/meta/devcontainer-base/README.md +++ b/meta/devcontainer-base/README.md @@ -293,7 +293,8 @@ file it names — and honours an allowed_signers a user provisioned some other w factored into side-effect-free `plumbing_*` / `initialize_*` functions and the tests source the scripts to exercise them under `bazel test //...` — `test_plumbing.py` here for the library, `.devcontainer/test_initialize.py` for this repo's host stub, each next to the code it covers. - `shellcheck` covers the rest, wired as both a pre-commit hook and a CI job. + `shellcheck` covers the rest, wired as a CI job in `unnatural_designs` and surfaced inline in + the editor there. - **`lib.sh` sets no shell options.** It is sourced into callers that own their own; the dispatcher sets `-euo pipefail`. - **Failure policy is per step.** The git-common bridge fails loud — the consumer's postCreate diff --git a/meta/scripts/BUILD.bazel b/meta/scripts/BUILD.bazel index 59a1284..4238267 100644 --- a/meta/scripts/BUILD.bazel +++ b/meta/scripts/BUILD.bazel @@ -62,6 +62,19 @@ py_test( ], ) +# No script half: the coupling is between two checked-in files, so the assertion is the whole +# gate. Rides `bazel test //...` rather than costing a CI job of its own. +py_test( + name = "test_precommit_docs", + size = "small", + srcs = ["test_precommit_docs.py"], + data = [ + "//:.pre-commit-config.yaml", + "//:README.md", + ], + main = "test_precommit_docs.py", +) + py_library( name = "classify_changed_paths_lib", srcs = ["classify_changed_paths.py"], diff --git a/meta/scripts/README.md b/meta/scripts/README.md index 13037c6..553f98c 100644 --- a/meta/scripts/README.md +++ b/meta/scripts/README.md @@ -1,8 +1,9 @@ # meta/scripts Repo-health gates. Each `check_*.py` enforces a cross-cutting invariant that doesn't fit inside a -single language toolchain — they run in CI, in pre-commit (where they fix or block), and on save -in the editor (where they surface findings without blocking). +single language toolchain. All of them run in CI; the table says which also run in pre-commit +(where a check can fix or block) and which run on save in the editor (where they surface findings +without blocking). | Script | Enforces | CI job (`.github/workflows/`) | Pre-commit hook | On-save (`.vscode/settings.json`) | | ----------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------- | --------------------- | --------------------------------- | @@ -15,6 +16,11 @@ in the editor (where they surface findings without blocking). module enumeration). The leading underscore signals it's not a public API; `test__workspace.py` covers it directly. +`test_precommit_docs.py` has no script half. It asserts that README's pre-commit hook table, and +the paragraph that classifies each hook, still agree with `.pre-commit-config.yaml` — a coupling +between two checked-in files rather than a check over the tree, so the assertion is the whole gate +and it rides `bazel test //...` instead of costing a CI job. + `smoke_py/` is a transient `py_test` that proves the end-to-end Python plumbing chain (`pyproject.toml` → `uv.lock` → `requirements_lock.txt` → `pip.parse` → `@unnatural_designs_pypi//...`) by importing `requests` and asserting it loads. Slated for deletion once gazelle_python is wired (see diff --git a/meta/scripts/test_precommit_docs.py b/meta/scripts/test_precommit_docs.py new file mode 100644 index 0000000..1291945 --- /dev/null +++ b/meta/scripts/test_precommit_docs.py @@ -0,0 +1,119 @@ +"""Guards README's pre-commit documentation against .pre-commit-config.yaml. + +The hook table and the paragraph above it both name hooks by id, and nothing else couples +them to the config — so a hook added, removed, or renamed leaves the prose stale with every +check still green. That is not hypothetical: the table sat two hooks behind the config, and +the paragraph's list of fixing hooks omitted one of them, for as long as nobody thought to +compare the two files. + +Scope is membership, order, and classification — not the "Triggers on" column. That column +paraphrases each hook's `files` regex for a reader; asserting a gloss against a regex would +either restate the regex in the README or accept anything. +""" + +import re +import unittest +from pathlib import Path + +# Not .resolve(): both files are cross-package data deps and live in the runfiles tree beside +# this one, which a resolved symlink would lead back out of. +_ROOT = Path(__file__).parent.parent.parent +_CONFIG = _ROOT / ".pre-commit-config.yaml" +_README = _ROOT / "README.md" + +_HOOK_ID_RE = re.compile(r"^\s+- id: (\S+)\s*$", re.M) +_TABLE_ROW_RE = re.compile(r"^\| `([^`]+)`\s*\|", re.M) + +# Fenced blocks first: the install snippet's ``` fences are backticks too, and leaving them in +# offsets every inline-code pair after them into nonsense. The inline pattern then refuses to +# span newlines, so one stray backtick garbles a line rather than the rest of the paragraph. +_FENCE_RE = re.compile(r"^```.*?^```", re.M | re.S) +_INLINE_CODE_RE = re.compile(r"`([^`\n]+)`") + +# A backticked token shaped like a hook id. Hyphens are optional on purpose: half the ids are +# single words, so requiring kebab-case would let a removed `gazelle` or `shellcheck` sit in +# the prose unnoticed. What does the work is the exclusions — `go.mod`, `uv.lock`, `secrets/`, +# and the linked `.pre-commit-config.yaml` all appear in this paragraph and none is a hook. +_HOOK_SHAPED_RE = re.compile(r"\A[a-z][a-z0-9-]*\Z") + +# Words that are hook-shaped but name something else. Strict by design: this paragraph exists +# to sort hooks into categories, so a bare backticked word in it reads as a hook id. Add here +# only when the prose genuinely needs one that isn't. +_NOT_A_HOOK = frozenset({"pre-commit"}) + + +def config_hook_ids() -> list[str]: + """Hook ids in .pre-commit-config.yaml, in file order.""" + return _HOOK_ID_RE.findall(_CONFIG.read_text(encoding="utf-8")) + + +def readme_intro_and_table() -> tuple[str, str]: + """The prose between the **Pre-commit hooks** heading and the hook table, and the table.""" + text = _README.read_text(encoding="utf-8") + start = text.index("**Pre-commit hooks**") + header = text.index("| Hook", start) + end = text.index("\n\n", header) + return text[start:header], text[header:end] + + +def hook_shaped_names(text: str) -> set[str]: + """Backticked tokens in `text` that read as hook ids.""" + return { + token + for token in _INLINE_CODE_RE.findall(_FENCE_RE.sub("", text)) + if _HOOK_SHAPED_RE.match(token) and token not in _NOT_A_HOOK + } + + +class HookShapedNamesTest(unittest.TestCase): + """The scan is what makes the README assertions non-vacuous, so it is tested directly.""" + + def test_picks_out_hook_ids_and_ignores_everything_else(self): + text = ( + "```\npre-commit install\n```\n\n" + "`ruff-check` and `gazelle` run here, over `go.mod`, `uv.lock` and `secrets/`; " + "see [`.pre-commit-config.yaml`](.pre-commit-config.yaml) and `pre-commit`." + ) + self.assertEqual(hook_shaped_names(text), {"ruff-check", "gazelle"}) + + def test_a_single_word_name_after_a_fence_is_still_seen(self): + """The bug this guards: ``` fences offset the pairing and hid a stale mention.""" + self.assertEqual(hook_shaped_names("```\nx\n```\n\n`shellcheck` is gone."), {"shellcheck"}) + + +class PrecommitDocsTest(unittest.TestCase): + def setUp(self): + self.hooks = config_hook_ids() + self.intro, self.table = readme_intro_and_table() + # A parser that silently matched nothing would make every assertion below vacuous. + self.assertTrue(self.hooks, "parsed no hook ids out of .pre-commit-config.yaml") + self.assertTrue(_TABLE_ROW_RE.findall(self.table), "parsed no rows out of the hook table") + self.assertTrue(hook_shaped_names(self.intro), "parsed no hook names out of the prose") + + def test_table_lists_every_hook_in_config_order(self): + """The table is the reader's index of the config; a drifted row sends them wrong.""" + self.assertEqual( + _TABLE_ROW_RE.findall(self.table), + self.hooks, + "README's hook table disagrees with .pre-commit-config.yaml", + ) + + def test_intro_accounts_for_every_hook(self): + """The prose sorts hooks into fixing / blocking; an unmentioned hook is unexplained.""" + missing = [hook for hook in self.hooks if f"`{hook}`" not in self.intro] + self.assertEqual( + missing, [], "hooks the README's pre-commit paragraph does not account for" + ) + + def test_intro_names_no_hook_the_config_lacks(self): + """The mirror case: prose describing a hook that has been renamed or removed.""" + self.assertEqual( + sorted(hook_shaped_names(self.intro) - set(self.hooks)), + [], + "names in the README's pre-commit paragraph that are not hooks in the config " + "(if one of these is not meant to be a hook id, add it to _NOT_A_HOOK)", + ) + + +if __name__ == "__main__": + unittest.main()