From 089b9309ef5502de7f96b2f88db30ac7358e4965 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Fri, 31 Jul 2026 22:12:17 -0700 Subject: [PATCH 01/11] feat(devcontainer): derive the base-image pin instead of bumping it The Dockerfile's FROM digest was modelled as a dependency: publish the image, wait for Renovate to notice, merge a bump. But the digest is a pure function of the source -- pkg_tar fixes timestamps and oci_push publishes the exact index Bazel assembled -- so it is knowable in the PR that changes the image, and the round-trip only bought a window where the pin named a superseded image. Verified: a local build of the merged tree produced sha256:0f31f38a..., byte-identical to what CI published. It becomes a derived file, in the sense MODULE.bazel.lock already is: - sync_base_image_pin.py rewrites the pin from the built layout index. - //.devcontainer:test_base_image_pin fails when it drifts. Not a pre-commit hook: the digest falls out of an already-built Bazel artifact and needs no daemon, so it rides `bazel test //...`. - renovate-derived-files.yml re-derives it when MODULE.bazel moves the upstream base, in the same commit as the other derived files, and after `bazel mod deps` -- that step needs a cold output base and this build would warm it. - Renovate is told to ignore the dep, so no bump PR can restate what the tree already determines. The trade, documented at each site: on a branch that edits the base, and on main until publish finishes, the pin names an image the registry does not have yet. DEVCONTAINER_BASE_IMAGE pointed at the published :latest is the way through. Consumers outside this repo (.dotfiles) cannot derive anything and keep the Renovate-bumped pin. Co-Authored-By: Claude Opus 5 --- .claude/CLAUDE.md | 13 ++ .devcontainer/BUILD.bazel | 15 ++ .devcontainer/Dockerfile | 2 +- .devcontainer/test_base_image_pin.py | 42 ++++++ .github/workflows/renovate-derived-files.yml | 21 +++ README.md | 8 +- docs/future-considerations.md | 8 +- meta/devcontainer-base/BUILD.bazel | 3 + meta/devcontainer-base/README.md | 35 ++++- meta/scripts/BUILD.bazel | 14 ++ meta/scripts/sync_base_image_pin.py | 143 +++++++++++++++++++ meta/scripts/test_sync_base_image_pin.py | 94 ++++++++++++ renovate.json | 5 + 13 files changed, 390 insertions(+), 13 deletions(-) create mode 100644 .devcontainer/test_base_image_pin.py create mode 100644 meta/scripts/sync_base_image_pin.py create mode 100644 meta/scripts/test_sync_base_image_pin.py diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 79908ac..8866553 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -134,6 +134,12 @@ the `Renovate helper`. Load-bearing facts: uv path, there's no "conflict" review: a bump `go mod tidy` can't settle just fails the job. MODULE.bazel.lock is *not* in this set — gazelle's `go_deps` extension is reproducible and absent from the lockfile, and `use_repo` tracks only direct imports (unchanged by a version bump). +- **The devcontainer base-image pin rides it too.** `.devcontainer/Dockerfile`'s `FROM` digest + is derived from `//meta/devcontainer-base:image`, which is assembled over the + `devcontainers_base_debian` pull — so a `MODULE.bazel` bump restales it. The workflow rebuilds + the image and rewrites the pin (`meta/scripts/sync_base_image_pin.py`) *after* `bazel mod deps`, + never before: that step needs a cold output base, and the build would warm it. Runs + `--config=local`, since this job carries no BuildBuddy key. - **Devcontainer feature lock rides it too.** `devcontainer upgrade` reruns when `devcontainer.json` moves. Like Go, it is *independent* of the uv→Bazel ordering and shares the job only so a grouped PR settles in one `expectedHeadOid` mutation. Needs no Docker (OCI metadata @@ -213,6 +219,13 @@ What is local to this repo: top. The `BASE_IMAGE` override and the exact three-line shape it needs are under "Consuming the image" in that README; `.devcontainer/test_devcontainer_config.py` asserts the couplings, because every wrong shape fails at container-build time or not at all. +- **That pin is a derived file, not a dependency.** The digest is reproducible from source, so the + PR that changes the image carries the new pin: `meta/scripts/sync_base_image_pin.py` writes it, + `//.devcontainer:test_base_image_pin` fails when it drifts, and Renovate is configured to ignore + the dep. The cost is that a base-editing branch pins an image the registry doesn't have yet; set + `DEVCONTAINER_BASE_IMAGE` to the published `:latest` to keep working. Don't reach for + `bazel run :load` locally — it needs a Docker daemon the devcontainer doesn't have, which is why + that path is CI's. - **`.devcontainer/initialize.sh` is the host stub** — the read-and-drop half the image cannot carry, since it runs on the host before any container exists. It writes `.git-plumbing/` and the `.host-*` symlinks `devcontainer.json` binds. diff --git a/.devcontainer/BUILD.bazel b/.devcontainer/BUILD.bazel index b7cb962..4b6edd9 100644 --- a/.devcontainer/BUILD.bazel +++ b/.devcontainer/BUILD.bazel @@ -26,3 +26,18 @@ py_test( ], main = "test_devcontainer_config.py", ) + +# The pin is a derived file (see meta/scripts/sync_base_image_pin.py). Asserting it needs the +# assembled image, which Bazel produces without a daemon — so freshness rides `bazel test //...` +# rather than a pre-commit hook or a separate CI job. +py_test( + name = "test_base_image_pin", + size = "small", + srcs = ["test_base_image_pin.py"], + data = [ + "Dockerfile", + "//meta/devcontainer-base:image", + ], + main = "test_base_image_pin.py", + deps = ["//meta/scripts:sync_base_image_pin_lib"], +) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 79f02e8..975795c 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -3,7 +3,7 @@ # canonical home for the mechanism, this three-line override shape, and why the alias is # load-bearing — do not collapse it without reading "Consuming the image" there. ARG BASE_IMAGE=pinned-base -FROM ghcr.io/syndic/unnatural_designs-devcontainer-base:latest@sha256:99e1405fcb245513ab0e033c51b190ee9b27377d0787f89f73dcea6a405d94cb AS pinned-base +FROM ghcr.io/syndic/unnatural_designs-devcontainer-base:latest@sha256:0f31f38a212f69b1831010aad682bda8f99a6f8a6b2f975aef5b9715331d1376 AS pinned-base FROM ${BASE_IMAGE} # renovate: datasource=github-releases depName=bazelbuild/bazelisk diff --git a/.devcontainer/test_base_image_pin.py b/.devcontainer/test_base_image_pin.py new file mode 100644 index 0000000..d9ad23b --- /dev/null +++ b/.devcontainer/test_base_image_pin.py @@ -0,0 +1,42 @@ +"""Asserts the Dockerfile pins the base image this tree actually builds. + +The pin is derived, not depended on: the digest is a pure function of `meta/devcontainer-base/` +plus the upstream base in `MODULE.bazel`, so a change to either restales it in the same commit. +This is the guard that says so — the same role `bazel mod tidy` plays for `MODULE.bazel.lock`, +except it can be a plain test because the digest falls out of an already-built artifact and needs +no daemon. + +Without it, a stale pin is invisible: the devcontainer still builds, from the previous image, and +the difference only shows up as plumbing that mysteriously predates your change. +""" + +import sys +import unittest +from pathlib import Path + +from meta.scripts.sync_base_image_pin import index_digest, pinned_digest + +# Not .resolve(): the image is a generated cross-package data dep, so it lives in the runfiles +# tree beside this file rather than in the source tree a resolved symlink leads back to. +_HERE = Path(__file__).parent +_DOCKERFILE = _HERE / "Dockerfile" +_LAYOUT_INDEX = _HERE.parent / "meta" / "devcontainer-base" / "image" / "index.json" + + +class TestBaseImagePinIsFresh(unittest.TestCase): + def test_pin_matches_the_built_image(self): + built = index_digest(_LAYOUT_INDEX.read_text(encoding="utf-8")) + current = pinned_digest(_DOCKERFILE.read_text(encoding="utf-8")) + self.assertEqual( + current, + built, + "The devcontainer's base-image pin is stale. Run:\n" + " bazel build //meta/devcontainer-base:image && " + "python3 meta/scripts/sync_base_image_pin.py\n" + "and commit .devcontainer/Dockerfile. The digest is reproducible, so this is the " + "one a merge will publish.", + ) + + +if __name__ == "__main__": + sys.exit(0 if unittest.main(exit=False).result.wasSuccessful() else 1) diff --git a/.github/workflows/renovate-derived-files.yml b/.github/workflows/renovate-derived-files.yml index 5349ca7..ebd8de1 100644 --- a/.github/workflows/renovate-derived-files.yml +++ b/.github/workflows/renovate-derived-files.yml @@ -199,6 +199,26 @@ jobs: if: (steps.changed.outputs.bazel == 'true' || steps.lock.outputs.moved == 'true') && steps.ratify.outputs.conflicts == '' run: bazel mod deps --lockfile_mode=update + # ── Devcontainer base image: re-derive the consumer's pin ───────────────────── + # The shared base image is assembled from MODULE.bazel's `devcontainers_base_debian` pull, + # so a bump there changes the digest .devcontainer/Dockerfile pins. That digest is knowable + # here rather than after publishing: the build is reproducible, and `oci_push` publishes the + # exact index Bazel built. //.devcontainer:test_base_image_pin is the matching check. + # + # AFTER the lock refresh above, never before: `bazel mod deps` only rewrites the pip `facts` + # on a cold output base, and this build would warm it — turning that step into the silent + # no-op its own comment warns about. + # + # `--config=local` because this job deliberately carries no BuildBuddy key (see the + # setup-bazel note above), and the default config points every action at a cache it cannot + # authenticate to. The image is two shell scripts over a pulled base; building it cold is + # cheaper than the round trip. + - name: Re-derive the devcontainer base-image pin + if: steps.changed.outputs.bazel == 'true' && steps.ratify.outputs.conflicts == '' + run: | + bazel build --config=local //meta/devcontainer-base:image + python3 meta/scripts/sync_base_image_pin.py + # ── Go: tidy each module and sync the workspace ─────────────────────────────── # Renovate's gomod manager runs `go get`, which bumps go.mod/go.sum but never # `go mod tidy` (opt-in) or `go work sync` (vendor-only in Renovate) — so the indirect @@ -330,6 +350,7 @@ jobs: uv.lock requirements_lock.txt MODULE.bazel.lock + .devcontainer/Dockerfile .devcontainer/devcontainer-lock.json ${{ steps.gotidy.outputs.files }} commit-message: "chore(deps): re-derive lock files" diff --git a/README.md b/README.md index d3f4f6b..04ed818 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,10 @@ 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 builds and shares with [Syndic/.dotfiles](https://github.com/Syndic/.dotfiles). It carries the -container-side git/host plumbing (worktree resolution, host timezone, shared-index config) and is -pinned by digest in the Dockerfile, bumped by Renovate. +container-side git/host plumbing (worktree resolution, host timezone, shared-index config). The +Dockerfile pins it by digest, and that pin is a *derived file*: the digest is reproducible from +source, so [`sync_base_image_pin.py`](meta/scripts/sync_base_image_pin.py) writes it and +`bazel test //...` fails when it drifts — see [Automation](#automation). **Feature pinning**: the `ghcr.io/devcontainers/features/*` references in [`devcontainer.json`](.devcontainer/devcontainer.json) are pinned to **full semver** @@ -346,7 +348,7 @@ allowlist): | Workflow | Trigger paths | Re-runs | Commits | | --- | --- | --- | --- | -| [`renovate-derived-files.yml`](.github/workflows/renovate-derived-files.yml) | `pyproject.toml`, `uv.lock`, `requirements_lock.txt`, `MODULE.bazel`, `.bazelversion`, `**/go.mod`, `go.work`, `.devcontainer/devcontainer.json` | [`meta/scripts/ratify_renovate_proposals.py`](meta/scripts/ratify_renovate_proposals.py) (`uv lock --upgrade-package ` + `uv export`), then `bazel mod deps --lockfile_mode=update`, `go mod tidy` + `go work sync`, and `devcontainer upgrade` | `uv.lock`, `requirements_lock.txt`, `MODULE.bazel.lock`, the touched `go.mod`/`go.sum` + `go.work.sum`, `.devcontainer/devcontainer-lock.json` | +| [`renovate-derived-files.yml`](.github/workflows/renovate-derived-files.yml) | `pyproject.toml`, `uv.lock`, `requirements_lock.txt`, `MODULE.bazel`, `.bazelversion`, `**/go.mod`, `go.work`, `.devcontainer/devcontainer.json` | [`meta/scripts/ratify_renovate_proposals.py`](meta/scripts/ratify_renovate_proposals.py) (`uv lock --upgrade-package ` + `uv export`), then `bazel mod deps --lockfile_mode=update`, `go mod tidy` + `go work sync`, and `devcontainer upgrade` | `uv.lock`, `requirements_lock.txt`, `MODULE.bazel.lock`, `.devcontainer/Dockerfile` (the base-image pin), the touched `go.mod`/`go.sum` + `go.work.sum`, `.devcontainer/devcontainer-lock.json` | The uv step runs before the Bazel step, and a Python change triggers the Bazel step when it moves `requirements_lock.txt`: `pip.parse` reads that file, and the artifact hashes it resolves are diff --git a/docs/future-considerations.md b/docs/future-considerations.md index c0f98f7..407583a 100644 --- a/docs/future-considerations.md +++ b/docs/future-considerations.md @@ -273,10 +273,10 @@ the irreducibly-per-host residue to a stub: Mend-hosted Renovate needs to read its digests; that is safe because the image holds only two shell scripts and a Debian base, no host-specific content. - **Consume the image here.** ✅ Done in Phase 3 — this repo `FROM`s it and layers go/bazel, so the - shared half is dogfooded on every CI run. The pin is a digest Renovate bumps, machinery both - repos already run, so a new version arrives as a bump gated by the consumer's own devcontainer - smoke check with no new management surface. .dotfiles doing the same, layering ansible/uv, is - what remains. The gitconfig install, the `allowedSignersFile` repoint and the socket chown are + shared half is dogfooded on every CI run. The pin is derived rather than bumped — the digest is + reproducible, so the PR that changes the image also carries the new pin, gated by the consumer's + own devcontainer smoke check. .dotfiles, which does not build the image, pins it as an ordinary + Renovate-bumped dependency instead; that adoption is what remains. The gitconfig install, the `allowedSignersFile` repoint and the socket chown are still in this repo's `post-start.sh`; they move into the shared library when .dotfiles needs them too. - **What's left on the host is a thin read-and-drop stub:** a handful of reads dropping results diff --git a/meta/devcontainer-base/BUILD.bazel b/meta/devcontainer-base/BUILD.bazel index b0f06ef..1595005 100644 --- a/meta/devcontainer-base/BUILD.bazel +++ b/meta/devcontainer-base/BUILD.bazel @@ -69,6 +69,9 @@ oci_image_index( ":image_linux_amd64", ":image_linux_arm64", ], + # Visible so the consumer can assert its own pin against this: the digest here is what + # //.devcontainer:test_base_image_pin compares the Dockerfile's `FROM` to. + visibility = ["//visibility:public"], ) # `bazel run //meta/devcontainer-base:load` puts the image in the local Docker daemon as diff --git a/meta/devcontainer-base/README.md b/meta/devcontainer-base/README.md index 74fa08f..1e05ae6 100644 --- a/meta/devcontainer-base/README.md +++ b/meta/devcontainer-base/README.md @@ -159,14 +159,38 @@ Every line of that is load-bearing, and the shapes it rules out fail quietly: This repo is the first consumer, so its `.devcontainer/` is the worked example, and `.devcontainer/test_devcontainer_config.py` asserts the couplings above from the files themselves. + +**Inside this repo the pin is a derived file, not a dependency.** The digest is a pure function of +the source — reproducible layers, and `oci_push` publishes the exact index Bazel built — so the +value a merge will publish is knowable in the PR that changes the image. `sync_base_image_pin.py` +writes it, `//.devcontainer:test_base_image_pin` fails when it drifts, and +`renovate-derived-files.yml` re-derives it when a `MODULE.bazel` bump moves the upstream base. +Renovate is told to leave that dep alone; a bump PR would only ever restate what the tree already +determines, one publish later. + +That does not transfer to a consumer outside this repo. `Syndic/.dotfiles` doesn't build the image, +so for it the pin is a genuine dependency: pin the digest, let Renovate's `docker` manager bump it, +and gate the bump on its own devcontainer check. The derived treatment is available only to the +repo that assembles the artifact. CI sets `DEVCONTAINER_BASE_IMAGE=devcontainer-base:ci` — the tag `bazel run :load` produces — whenever a PR touches the base, so a base change is smoke-tested against a real consumer *before* it publishes, rather than a Renovate bump later. One consequence for a consumer that caches its built image: the push that merges a base change seeds that cache from the candidate image while the Dockerfile still pins the previous digest, so builds miss the cache until the digest bump lands. -Note the freshness trade-off the image channel buys: `devcontainer up` reuses an existing -container, so a base bump lands on the next rebuild, not the next `up`. +Two freshness facts to keep in mind. `devcontainer up` reuses an existing container, so a base +bump lands on the next rebuild, not the next `up`. And because this repo's pin is derived, on any +branch that edits the base — and on `main` until the publish job finishes, a couple of minutes +after the merge — the pinned digest names an image the registry does not have yet. Point the +override at the last published one to get a working container meanwhile: + +``` +DEVCONTAINER_BASE_IMAGE=ghcr.io/syndic/unnatural_designs-devcontainer-base:latest \ + devcontainer up --workspace-folder . +``` + +That runs the previous plumbing, which is the right trade for getting work done: validating the +candidate is CI's job, and `bazel run :load` needs a Docker daemon the devcontainer doesn't have. ### Signed commits under the devcontainer CLI @@ -307,9 +331,10 @@ Publishing happens on pushes to `main` only, in a job gated on both smoke jobs, failed on either architecture never reaches the registry. `oci_push` publishes the exact index Bazel built rather than rebuilding it. -Consumers pin a digest and Renovate bumps it: its `bazel-module` manager reads `oci.pull` as a -`docker` dependency, which is why the base is declared with both a tag and a digest — a digest -alone would be pinned forever with nothing to compare against. +The image's *own* base is a dependency, and Renovate bumps it: its `bazel-module` manager reads +`oci.pull` as a `docker` dependency, which is why that base is declared with both a tag and a +digest — a digest alone would be pinned forever with nothing to compare against. (Not to be +confused with the consumer pin above, which is derived here.) - **The bump has to trigger the base jobs.** That is why `MODULE.bazel` is in the path classification; without it the bump moves the pin and triggers nothing. diff --git a/meta/scripts/BUILD.bazel b/meta/scripts/BUILD.bazel index aad5ec0..59a1284 100644 --- a/meta/scripts/BUILD.bazel +++ b/meta/scripts/BUILD.bazel @@ -148,6 +148,20 @@ py_test( deps = [":check_no_cgo_lib"], ) +py_library( + name = "sync_base_image_pin_lib", + srcs = ["sync_base_image_pin.py"], + visibility = ["//visibility:public"], +) + +py_test( + name = "test_sync_base_image_pin", + size = "small", + srcs = ["test_sync_base_image_pin.py"], + main = "test_sync_base_image_pin.py", + deps = [":sync_base_image_pin_lib"], +) + py_library( name = "renovate_manual_job_lib", srcs = ["renovate_manual_job.py"], diff --git a/meta/scripts/sync_base_image_pin.py b/meta/scripts/sync_base_image_pin.py new file mode 100644 index 0000000..77369c5 --- /dev/null +++ b/meta/scripts/sync_base_image_pin.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python3 +"""Sync `.devcontainer/Dockerfile`'s base-image pin with the image this repo builds. + +The pin is a **derived file**, not a dependency. `pkg_tar` fixes timestamps and `oci_push` +publishes the exact index Bazel assembled, so the digest a merge will publish is a pure function +of the source tree — knowable in the PR that changes the image, before anything is pushed. Left +as a dependency it would instead take a publish plus a Renovate round-trip to catch up, and the +pin would name a superseded image the whole time. + +The consequence to know: between a merge and the publish job finishing, and on any branch that +edits `meta/devcontainer-base/`, the pinned digest names an image that is not in the registry yet. +Point the override at the last published one to get a working container meanwhile: + + DEVCONTAINER_BASE_IMAGE=ghcr.io/syndic/unnatural_designs-devcontainer-base:latest \ + devcontainer up --workspace-folder . + +You then run the *previous* plumbing locally, which is fine — CI builds against the candidate image +(`bazel run :load`, which needs a Docker daemon this devcontainer deliberately lacks) and is what +validates the new one. + +Usage (the build has to come first — this reads its output, it does not run Bazel): + + bazel build //meta/devcontainer-base:image + python3 meta/scripts/sync_base_image_pin.py # rewrite the pin + python3 meta/scripts/sync_base_image_pin.py --check # exit 1 if it is stale + +`renovate-derived-files.yml` runs the rewrite when MODULE.bazel moves, since the upstream base it +pins feeds this digest; `//.devcontainer:test_base_image_pin` is the check that keeps a hand edit +from drifting. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +_BASE_REPOSITORY = "ghcr.io/syndic/unnatural_designs-devcontainer-base" + +# The one pinned reference in the Dockerfile: `FROM :@sha256: AS `. +# Anchored on the repository so the uv `COPY --from=` reference can never match. +_PIN_RE = re.compile( + rf"(?m)^(?PFROM\s+{re.escape(_BASE_REPOSITORY)}:[\w][\w.-]*@)" + r"(?Psha256:[0-9a-f]{64})" +) + +_DEFAULT_DOCKERFILE = Path(".devcontainer/Dockerfile") +# `bazel build` leaves the OCI layout under the convenience symlink. +_DEFAULT_LAYOUT = Path("bazel-bin/meta/devcontainer-base/image") + + +# ── Pure functions (the part the tests exercise) ────────────────────────────── + + +def index_digest(index_json: str) -> str: + """Return the image index's own digest from an OCI layout's `index.json`. + + `oci_image_index` nests the real manifest list one level down, so the digest to pin is the + single entry in the layout index — the same indirection `test_image_layers.py` walks. + """ + manifests = json.loads(index_json)["manifests"] + if len(manifests) != 1: + raise ValueError(f"expected exactly one manifest in the layout index, got {len(manifests)}") + return manifests[0]["digest"] + + +def pinned_digest(dockerfile: str) -> str: + """Return the digest currently pinned in the Dockerfile.""" + return _match(dockerfile).group("digest") + + +def replace_pin(dockerfile: str, digest: str) -> str: + """Return the Dockerfile with its pin set to `digest`. Idempotent.""" + if not re.fullmatch(r"sha256:[0-9a-f]{64}", digest): + raise ValueError(f"not a sha256 digest: {digest!r}") + match = _match(dockerfile) + return dockerfile[: match.start("digest")] + digest + dockerfile[match.end("digest") :] + + +def _match(dockerfile: str) -> re.Match[str]: + """The single pinned `FROM` — anything else means the shape moved and the caller must stop.""" + matches = list(_PIN_RE.finditer(dockerfile)) + if len(matches) != 1: + raise ValueError( + f"expected exactly one pinned `FROM {_BASE_REPOSITORY}:@sha256:…` line, " + f"found {len(matches)}" + ) + return matches[0] + + +# ── Driver ──────────────────────────────────────────────────────────────────── + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--dockerfile", type=Path, default=_DEFAULT_DOCKERFILE) + parser.add_argument( + "--layout-dir", + type=Path, + default=_DEFAULT_LAYOUT, + help="OCI layout directory built by //meta/devcontainer-base:image.", + ) + parser.add_argument( + "--check", + action="store_true", + help="Report staleness and exit 1 instead of rewriting.", + ) + args = parser.parse_args(argv) + + index = args.layout_dir / "index.json" + if not index.exists(): + print( + f"{index} not found — run `bazel build //meta/devcontainer-base:image` first.", + file=sys.stderr, + ) + return 2 + + built = index_digest(index.read_text(encoding="utf-8")) + dockerfile = args.dockerfile.read_text(encoding="utf-8") + current = pinned_digest(dockerfile) + + if current == built: + print(f"{args.dockerfile} already pins the built image ({built}).") + return 0 + + if args.check: + print(f"{args.dockerfile} pins {current}, but this tree builds {built}.", file=sys.stderr) + print( + "Run `bazel build //meta/devcontainer-base:image && " + "python3 meta/scripts/sync_base_image_pin.py` and commit the result.", + file=sys.stderr, + ) + return 1 + + args.dockerfile.write_text(replace_pin(dockerfile, built), encoding="utf-8") + print(f"{args.dockerfile}: {current} -> {built}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/meta/scripts/test_sync_base_image_pin.py b/meta/scripts/test_sync_base_image_pin.py new file mode 100644 index 0000000..f3ff5d6 --- /dev/null +++ b/meta/scripts/test_sync_base_image_pin.py @@ -0,0 +1,94 @@ +"""Tests for sync_base_image_pin.py. + +The pure functions carry the logic; the freshness assertion against a real built image lives in +`//.devcontainer:test_base_image_pin`, next to the Dockerfile it guards. + +The rewrite is the risky half: it edits a file whose `FROM` line is the difference between a +working devcontainer and one that cannot resolve its base at all, so the cases below are mostly +about refusing to guess when the shape is not exactly what is expected. +""" + +import json +import sys +import unittest + +from meta.scripts.sync_base_image_pin import index_digest, pinned_digest, replace_pin + +_REPO = "ghcr.io/syndic/unnatural_designs-devcontainer-base" +_OLD = "sha256:" + "a" * 64 +_NEW = "sha256:" + "b" * 64 + +_DOCKERFILE = f"""# comment +ARG BASE_IMAGE=pinned-base +FROM {_REPO}:latest@{_OLD} AS pinned-base +FROM ${{BASE_IMAGE}} + +COPY --from=ghcr.io/astral-sh/uv:0.12.0 /uv /uvx /usr/local/bin/ +""" + + +class TestIndexDigest(unittest.TestCase): + def test_reads_the_single_layout_entry(self): + layout = json.dumps({"schemaVersion": 2, "manifests": [{"digest": _NEW, "size": 1}]}) + self.assertEqual(index_digest(layout), _NEW) + + def test_rejects_a_layout_with_several_entries(self): + # `oci_image_index` nests the manifest list one level down, so the layout index holds + # exactly one entry. More than one means the target's shape changed and picking [0] + # would silently pin whichever happened to be first. + layout = json.dumps({"manifests": [{"digest": _NEW}, {"digest": _OLD}]}) + with self.assertRaises(ValueError): + index_digest(layout) + + +class TestPinnedDigest(unittest.TestCase): + def test_reads_the_current_pin(self): + self.assertEqual(pinned_digest(_DOCKERFILE), _OLD) + + def test_ignores_the_uv_copy_reference(self): + # `COPY --from=` names another image entirely; anchoring on the repository is what keeps + # a future digest-pinned COPY from being mistaken for the base pin. + self.assertNotIn("astral-sh", pinned_digest(_DOCKERFILE)) + + def test_rejects_a_dockerfile_with_no_pin(self): + with self.assertRaises(ValueError): + pinned_digest("FROM debian:bookworm\n") + + def test_rejects_a_dockerfile_with_two_pins(self): + # Two candidates means rewriting one of them is a coin flip. + doubled = _DOCKERFILE + f"FROM {_REPO}:latest@{_NEW} AS other\n" + with self.assertRaises(ValueError): + pinned_digest(doubled) + + +class TestReplacePin(unittest.TestCase): + def test_replaces_only_the_digest(self): + result = replace_pin(_DOCKERFILE, _NEW) + self.assertIn(f"FROM {_REPO}:latest@{_NEW} AS pinned-base", result) + self.assertNotIn(_OLD, result) + + def test_leaves_everything_else_byte_identical(self): + # The alias, the tag, the ARG above it and the consuming FROM below are all load-bearing + # (see "Consuming the image" in meta/devcontainer-base/README.md); this rewrite must not + # be the thing that disturbs them. + self.assertEqual( + replace_pin(_DOCKERFILE, _NEW).replace(_NEW, _OLD), + _DOCKERFILE, + ) + + def test_is_idempotent(self): + once = replace_pin(_DOCKERFILE, _NEW) + self.assertEqual(replace_pin(once, _NEW), once) + + def test_rejects_a_malformed_digest(self): + for bad in ("sha256:beef", "b" * 64, f"{_NEW} AS evil", ""): + with self.subTest(digest=bad), self.assertRaises(ValueError): + replace_pin(_DOCKERFILE, bad) + + def test_refuses_when_the_pin_is_missing(self): + with self.assertRaises(ValueError): + replace_pin("FROM debian:bookworm\n", _NEW) + + +if __name__ == "__main__": + sys.exit(0 if unittest.main(exit=False).result.wasSuccessful() else 1) diff --git a/renovate.json b/renovate.json index b1c79f0..b81da4a 100644 --- a/renovate.json +++ b/renovate.json @@ -87,6 +87,11 @@ "matchDepTypes": [ "oci_pull" ], "matchUpdateTypes": [ "digest" ], "groupName": "devcontainer base image" + }, + { + "matchDatasources": [ "docker" ], + "matchPackageNames": [ "ghcr.io/syndic/unnatural_designs-devcontainer-base" ], + "enabled": false } ] } From 4736ebb0c0a7db973a7e215dd628ea0bf564b182 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sat, 8 Aug 2026 18:07:40 -0700 Subject: [PATCH 02/11] build(pre-commit): rewrite the base-image pin from a hook, not just a test Deriving the pin only closed half the loop: the CI test reported drift and left you to run the command by hand, where MODULE.bazel.lock and uv.lock both get a hook that fixes it in place. The pin now gets the same treatment, and for the same reason -- a derived file nobody has to remember to regenerate. `files` covers every input to the image rather than the base directory alone: MODULE.bazel carries both the upstream pull and the rules_oci / rules_pkg versions that assemble it, and .bazelversion is in that class too. The test stays, and is not redundant. Hooks do not run for --no-verify, a web edit, or the helper app`s API commits, and they do not re-run when a branch rebases onto someone else`s base change -- two individually fresh pins can be jointly stale. Verified both ways against a corrupted pin: the hook rewrites it and fails the commit, the test fails on its own. Co-Authored-By: Claude Opus 5 --- .claude/CLAUDE.md | 7 ++++--- .devcontainer/BUILD.bazel | 8 +++++--- .devcontainer/test_base_image_pin.py | 9 ++++++--- .pre-commit-config.yaml | 13 +++++++++++++ meta/scripts/sync_base_image_pin.py | 7 ++++--- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 8866553..b92bf8c 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -220,9 +220,10 @@ What is local to this repo: image" in that README; `.devcontainer/test_devcontainer_config.py` asserts the couplings, because every wrong shape fails at container-build time or not at all. - **That pin is a derived file, not a dependency.** The digest is reproducible from source, so the - PR that changes the image carries the new pin: `meta/scripts/sync_base_image_pin.py` writes it, - `//.devcontainer:test_base_image_pin` fails when it drifts, and Renovate is configured to ignore - the dep. The cost is that a base-editing branch pins an image the registry doesn't have yet; set + PR that changes the image carries the new pin. Three callers, one per source of change: the + `base-image-pin` pre-commit hook for our edits, this workflow for Renovate's, and + `//.devcontainer:test_base_image_pin` as the check under both — hooks are bypassable and the + workflow only fires for Renovate's own PRs. Renovate is configured to ignore the dep. The cost is that a base-editing branch pins an image the registry doesn't have yet; set `DEVCONTAINER_BASE_IMAGE` to the published `:latest` to keep working. Don't reach for `bazel run :load` locally — it needs a Docker daemon the devcontainer doesn't have, which is why that path is CI's. diff --git a/.devcontainer/BUILD.bazel b/.devcontainer/BUILD.bazel index 4b6edd9..2df0dc7 100644 --- a/.devcontainer/BUILD.bazel +++ b/.devcontainer/BUILD.bazel @@ -27,9 +27,11 @@ py_test( main = "test_devcontainer_config.py", ) -# The pin is a derived file (see meta/scripts/sync_base_image_pin.py). Asserting it needs the -# assembled image, which Bazel produces without a daemon — so freshness rides `bazel test //...` -# rather than a pre-commit hook or a separate CI job. +# The pin is a derived file (see meta/scripts/sync_base_image_pin.py). The `base-image-pin` +# pre-commit hook keeps it fresh; this is the half that cannot be bypassed — hooks don't run for +# `--no-verify`, a web edit, or the helper app's API commits, and don't re-run on a rebase. +# Asserting it needs only the assembled image, which Bazel produces without a daemon, so it rides +# `bazel test //...` instead of costing a CI job of its own. py_test( name = "test_base_image_pin", size = "small", diff --git a/.devcontainer/test_base_image_pin.py b/.devcontainer/test_base_image_pin.py index d9ad23b..b079c0a 100644 --- a/.devcontainer/test_base_image_pin.py +++ b/.devcontainer/test_base_image_pin.py @@ -2,9 +2,12 @@ The pin is derived, not depended on: the digest is a pure function of `meta/devcontainer-base/` plus the upstream base in `MODULE.bazel`, so a change to either restales it in the same commit. -This is the guard that says so — the same role `bazel mod tidy` plays for `MODULE.bazel.lock`, -except it can be a plain test because the digest falls out of an already-built artifact and needs -no daemon. +The `base-image-pin` pre-commit hook rewrites it, exactly as `bazel mod tidy` does for +`MODULE.bazel.lock`. This is the backstop under that hook: hooks don't run for `--no-verify`, a +web edit, or the helper app's API commits, and don't re-run when a branch rebases onto someone +else's base change — two individually-fresh pins can be jointly stale. It can be a plain test +rather than a CI job because the digest falls out of an already-built artifact and needs no +daemon. Without it, a stale pin is invisible: the devcontainer still builds, from the previous image, and the difference only shows up as plumbing that mysteriously predates your change. diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 494efb7..0d014aa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,6 +22,19 @@ repos: pass_filenames: false files: ^(pyproject\.toml|uv\.lock|requirements_lock\.txt)$ + - id: base-image-pin + name: sync devcontainer base-image pin + language: system + # The Dockerfile's FROM digest is derived from the image this repo builds, so an edit + # to the base or to the MODULE.bazel pull it sits on restales it in the same commit. + # Rewrites rather than reports, like the two hooks above; //.devcontainer:test_base_image_pin + # is the CI-side check for commits that never ran a hook. + entry: bash -c 'set -euo pipefail; bazel build //meta/devcontainer-base:image; python3 meta/scripts/sync_base_image_pin.py' + pass_filenames: false + # Every input to the image, not just the base directory: MODULE.bazel carries the + # upstream pull plus rules_oci/rules_pkg, and .bazelversion is in the same class. + files: ^(meta/devcontainer-base/|MODULE\.bazel$|\.bazelversion$) + # ruff check first (with --fix), then ruff format — check's import # sorting can produce output the formatter wants to retouch. Both fix in # place; pre-commit re-flags modified files so the user re-stages. diff --git a/meta/scripts/sync_base_image_pin.py b/meta/scripts/sync_base_image_pin.py index 77369c5..c64387b 100644 --- a/meta/scripts/sync_base_image_pin.py +++ b/meta/scripts/sync_base_image_pin.py @@ -24,9 +24,10 @@ python3 meta/scripts/sync_base_image_pin.py # rewrite the pin python3 meta/scripts/sync_base_image_pin.py --check # exit 1 if it is stale -`renovate-derived-files.yml` runs the rewrite when MODULE.bazel moves, since the upstream base it -pins feeds this digest; `//.devcontainer:test_base_image_pin` is the check that keeps a hand edit -from drifting. +Three callers, one per source of change: the `base-image-pin` pre-commit hook for our own edits, +`renovate-derived-files.yml` for Renovate's (it moves the upstream base this digest is assembled +over), and `//.devcontainer:test_base_image_pin` as the check under both — hooks are bypassable +and the workflow only fires for Renovate's own PRs. """ from __future__ import annotations From e6c3e74edab3fd9380abe47e301408c1f21c2066 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sat, 8 Aug 2026 18:18:31 -0700 Subject: [PATCH 03/11] fix(ci): publish the base image when .bazelversion moves, not just derive it The two classifications disagreed about what feeds the image. renovate-derived-files.yml counts .bazelversion as a bazel change and re-derives the pin from it; devcontainer.yml did not, so the merge would rebuild and publish nothing. A bazel bump that moved the digest would have written a pin naming an image no job ever pushes: the consumer build cannot pull it, and no amount of re-running fixes a red PR whose failure is a config gap. Latent until now -- a bazel release changing rules_oci output is unlikely, and until the pin was derived nothing would have moved. Deriving it is what makes the asymmetry reachable, so it is worth closing in the same PR rather than leaving a trap for whoever meets it first. Cost is a no-op republish per bazel bump, which is what a MODULE.bazel edit that misses the image already costs. Co-Authored-By: Claude Opus 5 --- .github/workflows/devcontainer.yml | 9 +++++++-- meta/scripts/test_classify_changed_paths.py | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index a7c2e7a..54eb181 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -57,6 +57,11 @@ jobs: # `changed` too so the automerged bump is gated by a real consumer build, which is # what `publish` needs. # + # `.bazelversion` is in both for a narrower reason: renovate-derived-files.yml + # classifies it as a bazel change and re-derives the pin from it, so leaving it out + # here would let a bump write a pin that no job ever publishes — a red PR with no way + # to green it. The two classifications have to agree on what feeds the image. + # # Deliberately the whole file, not a grep for the pull's name. MODULE.bazel moves on # roughly 1 in 6 commits here and most of those cannot touch the image, so the false # positives are real and not cheap: two base-image runners, plus a consumer build @@ -68,8 +73,8 @@ jobs: # build when both are true. python3 meta/scripts/classify_changed_paths.py \ --base "$base" \ - --rule 'changed=^\.devcontainer/|^meta/devcontainer-base/|^MODULE\.bazel$|^\.github/workflows/devcontainer\.yml$' \ - --rule 'base=^meta/devcontainer-base/|^MODULE\.bazel$' + --rule 'changed=^\.devcontainer/|^meta/devcontainer-base/|^MODULE\.bazel$|^\.bazelversion$|^\.github/workflows/devcontainer\.yml$' \ + --rule 'base=^meta/devcontainer-base/|^MODULE\.bazel$|^\.bazelversion$' # ── Shared base image ─────────────────────────────────────────────────────────────── # meta/devcontainer-base/ is published for other Syndic repos to FROM. Bazel assembles it and diff --git a/meta/scripts/test_classify_changed_paths.py b/meta/scripts/test_classify_changed_paths.py index b1bd6e7..ea74138 100644 --- a/meta/scripts/test_classify_changed_paths.py +++ b/meta/scripts/test_classify_changed_paths.py @@ -75,7 +75,10 @@ def test_a_workflow_without_rules_raises(self): def test_regexes_survive_extraction_intact(self): # The `--rule` arguments are single-quoted in the workflow, so nothing in a regex needs # unescaping — pin that, since a quoting change would land here as a subtly wrong rule. - self.assertEqual(RULES_DEVCONTAINER["base"], r"^meta/devcontainer-base/|^MODULE\.bazel$") + self.assertEqual( + RULES_DEVCONTAINER["base"], + r"^meta/devcontainer-base/|^MODULE\.bazel$|^\.bazelversion$", + ) class TestParseRule(unittest.TestCase): @@ -252,6 +255,16 @@ def test_base_image_fires_both(self): expect_devcontainer(changed=True, base=True), ) + def test_bazelversion_fires_both(self): + # renovate-derived-files.yml re-derives the base-image pin on a .bazelversion change, + # so this workflow has to be willing to rebuild and publish what that pin will name. + # Deriving without publishing is the one combination that cannot be recovered from. + self.assertEqual(self._run([".bazelversion"]), expect_devcontainer(changed=True, base=True)) + + def test_decoy_bazelversion_suffix(self): + # `$`-anchored, matching the renovate-derived-files rule's own decoy test. + self.assertEqual(self._run([".bazelversion.bak"]), expect_devcontainer()) + def test_module_bazel_fires_both(self): # MODULE.bazel pins the base image's own base (the devcontainers_base_debian oci.pull), # and that bump automerges. Missing it here would move the pin without rebuilding, @@ -275,6 +288,7 @@ def test_base_is_a_subset_of_changed(self): "meta/devcontainer-base/scripts/lib.sh", "meta/devcontainer-base/BUILD.bazel", "MODULE.bazel", + ".bazelversion", ): with self.subTest(path=path): result = self._run([path]) From a13fce2d82c18d8ed1cd3a0ee54755c086b5c53c Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sat, 8 Aug 2026 18:20:36 -0700 Subject: [PATCH 04/11] ci(devcontainer): verify the registry serves what main pins The hook and renovate-derived-files.yml both run before the merge, so neither can see the step that fails after it. `crane push` followed by one `crane tag` per tag is not atomic: a failure in between leaves main pinned to a digest the registry does not serve, or tags trailing a digest that is. Both are silent until somebody rebuilds a devcontainer and cannot pull its base. The publish job now reads the pin back and checks it three ways: the manifest resolves by digest (what a consumer FROM needs), and :latest and sha- both point at it (what Renovate and any non-deriving consumer read). A half-publish becomes a red main, which is recoverable by re-running -- the documented remedy, now with something that says when to apply it. The pin is read through sync_base_image_pin.py --print-pinned rather than a grep local to the workflow, so there is one parser for the Dockerfile shape and no second copy to degrade into a check that always passes. That mode returns before the layout check because the publish job has no Bazel output tree; a test pins that, and a mutation making it print nothing is caught. Co-Authored-By: Claude Opus 5 --- .github/workflows/devcontainer.yml | 26 ++++++++++++++++ meta/scripts/sync_base_image_pin.py | 12 ++++++++ meta/scripts/test_sync_base_image_pin.py | 39 +++++++++++++++++++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index 54eb181..cf88ec9 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -373,3 +373,29 @@ jobs: set -euo pipefail bazel run --config=ci //meta/devcontainer-base:push -- \ --tag latest --tag "sha-${GITHUB_SHA}" + + # The one loose end neither the pre-commit hook nor renovate-derived-files.yml can see: + # both run before the merge, and this is the step that can fail after it. `crane push` + # then one `crane tag` per tag is not atomic, so a failure in between leaves main pinned + # to a digest the registry does not serve — silent until someone rebuilds a devcontainer. + # + # Reads the pin through sync_base_image_pin.py rather than a grep of its own, so the + # Dockerfile shape has one parser and this cannot degrade into a check that always passes. + - name: Verify the registry serves what main pins + run: | + set -euo pipefail + repo=ghcr.io/syndic/unnatural_designs-devcontainer-base + pinned="$(python3 meta/scripts/sync_base_image_pin.py --print-pinned)" + + # The digest is what a consumer's FROM resolves; the tags are what Renovate and any + # non-deriving consumer read, and they are the half `crane tag` can leave behind. + docker manifest inspect "${repo}@${pinned}" >/dev/null + for tag in latest "sha-${GITHUB_SHA}"; do + resolved="$(docker buildx imagetools inspect --format '{{.Manifest.Digest}}' "${repo}:${tag}")" + if [ "$resolved" != "$pinned" ]; then + echo "::error::${repo}:${tag} resolves to ${resolved}, but main pins ${pinned}." \ + "Re-run this job — crane tags non-atomically." + exit 1 + fi + done + echo "${repo}@${pinned} is published and both tags point at it." diff --git a/meta/scripts/sync_base_image_pin.py b/meta/scripts/sync_base_image_pin.py index c64387b..673d6c5 100644 --- a/meta/scripts/sync_base_image_pin.py +++ b/meta/scripts/sync_base_image_pin.py @@ -108,8 +108,20 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="Report staleness and exit 1 instead of rewriting.", ) + parser.add_argument( + "--print-pinned", + action="store_true", + help="Print the digest currently pinned and exit; needs no build.", + ) args = parser.parse_args(argv) + # Deliberately before the layout check: the publish job asks what main pins without + # building anything, and reusing this parser is what keeps a Dockerfile shape change from + # degrading that check into a silent pass. + if args.print_pinned: + print(pinned_digest(args.dockerfile.read_text(encoding="utf-8"))) + return 0 + index = args.layout_dir / "index.json" if not index.exists(): print( diff --git a/meta/scripts/test_sync_base_image_pin.py b/meta/scripts/test_sync_base_image_pin.py index f3ff5d6..5edbb8e 100644 --- a/meta/scripts/test_sync_base_image_pin.py +++ b/meta/scripts/test_sync_base_image_pin.py @@ -8,11 +8,15 @@ about refusing to guess when the shape is not exactly what is expected. """ +import contextlib +import io import json import sys +import tempfile import unittest +from pathlib import Path -from meta.scripts.sync_base_image_pin import index_digest, pinned_digest, replace_pin +from meta.scripts.sync_base_image_pin import index_digest, main, pinned_digest, replace_pin _REPO = "ghcr.io/syndic/unnatural_designs-devcontainer-base" _OLD = "sha256:" + "a" * 64 @@ -90,5 +94,38 @@ def test_refuses_when_the_pin_is_missing(self): replace_pin("FROM debian:bookworm\n", _NEW) +class TestPrintPinned(unittest.TestCase): + """The mode the publish job reads. It must answer without a build — that job has no Bazel + output tree, and a mode that needed one would fail the step for the wrong reason.""" + + def _run(self, dockerfile_text: str) -> tuple[int, str]: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "Dockerfile" + path.write_text(dockerfile_text, encoding="utf-8") + out = io.StringIO() + with contextlib.redirect_stdout(out): + # --layout-dir points at nothing on purpose: reaching it would mean the early + # return is gone and the publish job would start depending on a built image. + code = main( + [ + "--print-pinned", + "--dockerfile", + str(path), + "--layout-dir", + str(Path(tmp) / "no-such-layout"), + ] + ) + return code, out.getvalue().strip() + + def test_prints_the_pinned_digest_without_a_build(self): + self.assertEqual(self._run(_DOCKERFILE), (0, _OLD)) + + def test_refuses_a_dockerfile_it_cannot_parse(self): + # Must raise rather than print nothing: the job compares this against the registry, and + # an empty answer would compare two blanks and pass. + with self.assertRaises(ValueError): + self._run("FROM debian:bookworm\n") + + if __name__ == "__main__": sys.exit(0 if unittest.main(exit=False).result.wasSuccessful() else 1) From 3ee498cc69674a88e37dc493c385ec23a3d76068 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sat, 8 Aug 2026 18:40:29 -0700 Subject: [PATCH 05/11] docs: name the hook as the path our own edits take The root README described the pin as written by sync_base_image_pin.py and checked by bazel test, which was the shape before the hook existed. The script is what writes it; the hook is what runs the script, and for an edit made here that is the whole mechanism. Rewraps a future-considerations line this branch left at 149 characters. Co-Authored-By: Claude Opus 5 --- README.md | 5 +++-- docs/future-considerations.md | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 04ed818..30a0879 100644 --- a/README.md +++ b/README.md @@ -44,8 +44,9 @@ rebuilds. builds and shares with [Syndic/.dotfiles](https://github.com/Syndic/.dotfiles). It carries the container-side git/host plumbing (worktree resolution, host timezone, shared-index config). The Dockerfile pins it by digest, and that pin is a *derived file*: the digest is reproducible from -source, so [`sync_base_image_pin.py`](meta/scripts/sync_base_image_pin.py) writes it and -`bazel test //...` fails when it drifts — see [Automation](#automation). +source, so a pre-commit hook writes it with +[`sync_base_image_pin.py`](meta/scripts/sync_base_image_pin.py) and `bazel test //...` fails when +it drifts — see [Automation](#automation). **Feature pinning**: the `ghcr.io/devcontainers/features/*` references in [`devcontainer.json`](.devcontainer/devcontainer.json) are pinned to **full semver** diff --git a/docs/future-considerations.md b/docs/future-considerations.md index 407583a..4457b0f 100644 --- a/docs/future-considerations.md +++ b/docs/future-considerations.md @@ -276,9 +276,9 @@ the irreducibly-per-host residue to a stub: shared half is dogfooded on every CI run. The pin is derived rather than bumped — the digest is reproducible, so the PR that changes the image also carries the new pin, gated by the consumer's own devcontainer smoke check. .dotfiles, which does not build the image, pins it as an ordinary - Renovate-bumped dependency instead; that adoption is what remains. The gitconfig install, the `allowedSignersFile` repoint and the socket chown are - still in this repo's `post-start.sh`; they move into the shared library when .dotfiles needs - them too. + Renovate-bumped dependency instead; that adoption is what remains. The gitconfig install, the + `allowedSignersFile` repoint and the socket chown are still in this repo's `post-start.sh`; + they move into the shared library when .dotfiles needs them too. - **What's left on the host is a thin read-and-drop stub:** a handful of reads dropping results into `.git-plumbing/`, plus the one sudo branch (the agent-socket placeholder). It rarely changes and is too small to be worth a shared artifact, so it stays hand-copied — cheaply. From 7f54594528270ac26b53ebd1ba4d4e00c6fa35e0 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Sat, 8 Aug 2026 19:05:46 -0700 Subject: [PATCH 06/11] ci: back the two derived-file hooks that had no CI check The pin got a backstop in this PR while the other two fixers kept relying on the hook having run. Both are now checked in CI, fail-only: the regenerated file belongs in the authoring commit, not in a CI push. MODULE.bazel.lock had nothing at all -- builds run with --lockfile_mode=update, which rewrites the lock in memory and stays green, so a bypassed `bazel mod tidy` was invisible until it blocked someone else`s commit. A new ci.yml job runs `bazel mod tidy` and diffs MODULE.bazel and the lock. Verified by staging a lock with a dropped registry hash: the job`s own commands exit 1. uv.lock was half-covered. check_uv_lock_fresh compares requirements_lock against the lock, which stays consistent while the *lock* drifts from pyproject -- exactly what a bypassed hook leaves. check_uv_lock_current adds `uv lock --check`, which validates the existing resolution rather than redoing it (sub-millisecond here), so this is not the full re-lock that was deliberately kept out of CI. That new check immediately caught a defect in the old one: `uv export` updates uv.lock before exporting, so check_uv_lock_fresh was silently re-locking the tree it was checking. In CI that repairs staleness inside the runner and then compares against a file the commit does not contain. Fixed with --frozen, pinned at the argv seam next to the other flags. The pre-commit hook re-locks before exporting and needs no such flag. Co-Authored-By: Claude Opus 5 --- .claude/CLAUDE.md | 7 ++-- .github/workflows/ci.yml | 24 ++++++++++++ meta/scripts/check_modules.py | 44 +++++++++++++++++++-- meta/scripts/test_check_modules.py | 61 ++++++++++++++++++++++++++++-- 4 files changed, 127 insertions(+), 9 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index b92bf8c..5ddb84f 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -116,9 +116,10 @@ the `Renovate helper`. Load-bearing facts: make it a silent no-op. - **`.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. CI hides it — `--lockfile_mode=update` rewrites in memory and stays green — - but the `bazel mod tidy` pre-commit hook rewrites it on disk, so the staleness surfaces as a - blocked local commit on any `go.mod`/`MODULE.bazel` change. The workflow therefore triggers on + 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. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 416c967..83a341b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,30 @@ jobs: buildbuddy-api-key: ${{ secrets.BUILDBUDDY_API_KEY }} - run: bazel run --config=ci //:gazelle -- -mode=diff + # MODULE.bazel.lock is the one derived file with no CI backstop of its own: builds run with + # `--lockfile_mode=update`, which rewrites the lock in memory and stays green, so a `bazel mod + # tidy` hook that never ran is invisible until it blocks someone's commit. Fail-only on + # purpose — the regenerated lock belongs in the authoring commit, not in a CI push. + # + # `bazel mod tidy` also rewrites MODULE.bazel's `use_repo` lines, so both files are diffed. + bazel-lock-check: + name: MODULE.bazel.lock freshness + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: ./.github/actions/setup-bazel-remote + with: + buildbuddy-api-key: ${{ secrets.BUILDBUDDY_API_KEY }} + - run: | + set -euo pipefail + bazel mod tidy + if ! git diff --exit-code -- MODULE.bazel MODULE.bazel.lock; then + MSG="stale — run 'bazel mod tidy' and commit the result." + echo "::error file=MODULE.bazel.lock::${MSG}" + exit 1 + fi + echo "MODULE.bazel and MODULE.bazel.lock are what 'bazel mod tidy' produces." + # The repo's `meta/scripts/*.py` target Python 3.14 (matches the rules_python # toolchain pin in MODULE.bazel and the `target-version` in pyproject.toml's # [tool.ruff]). Ubuntu-24.04 still ships 3.12 as `python3`, so the language- diff --git a/meta/scripts/check_modules.py b/meta/scripts/check_modules.py index d9c37f2..ea6bc52 100644 --- a/meta/scripts/check_modules.py +++ b/meta/scripts/check_modules.py @@ -16,6 +16,7 @@ 4. Every discovered per-project pyproject.toml matches a glob in [tool.uv.workspace].members so workspace resolution and member iteration agree. 5. requirements_lock.txt is fresh relative to uv.lock (cheap diff via `uv export`). + 6. uv.lock itself still satisfies pyproject.toml (`uv lock --check`). Polyglot replacement for the former check_go_modules.py. Matrix-list parsing is driven by LanguageSpec.matrix_key; today only Go has a matrix key (`go_module`), and Python's @@ -276,6 +277,12 @@ def _uv_export(root: Path) -> str | None: stripping in :func:`_strip_header` handles the invocation-path difference at compare time. Returns None if uv is not on PATH (skipped silently — the pre-commit hook is the durable enforcement; this check is the CI safety net). + + `--frozen` because a check must not edit what it checks: `uv export` otherwise updates + uv.lock first, so a stale lock would be silently repaired in the runner and this would + compare against a file the commit does not contain. The pre-commit hook re-locks before + exporting, which is why it needs no such flag; when the lock is current both produce + identical output, and `check_uv_lock_current` is what says whether it is. """ with tempfile.NamedTemporaryFile(mode="w+", suffix=".txt", delete=False) as tmp: tmp_path = Path(tmp.name) @@ -285,6 +292,7 @@ def _uv_export(root: Path) -> str | None: [ "uv", "export", + "--frozen", "--format", "requirements-txt", "--no-emit-project", @@ -303,13 +311,42 @@ def _uv_export(root: Path) -> str | None: tmp_path.unlink(missing_ok=True) +def check_uv_lock_current(root: Path) -> int: + """Assert `uv.lock` still satisfies `pyproject.toml`, the half the export diff can't see. + + `check_uv_lock_fresh` compares requirements_lock.txt against the lock; both stay + consistent with each other while the *lock* drifts from the manifest, which is what a + bypassed `uv-lock-fresh` hook leaves behind. `uv lock --check` validates the existing + resolution instead of redoing it, so this is not the re-lock that was deliberately kept + out of CI — it measured under a millisecond on the current graph. + """ + try: + subprocess.run( + ["uv", "lock", "--check"], + cwd=root, + capture_output=True, + text=True, + check=True, + ) + except FileNotFoundError: + # Same posture as the export check: no uv, nothing to say. + return 0 + except subprocess.CalledProcessError as e: + detail = " / ".join((e.stderr or e.stdout or str(e)).strip().splitlines()) or str(e) + print( + "uv.lock:1:1-2: stale — out of sync with pyproject.toml. Re-run `uv lock` " + f"(or rely on the uv-lock-fresh pre-commit hook). uv said: {detail}" + ) + return 1 + return 0 + + def check_uv_lock_fresh(root: Path) -> int: """Cheap freshness check: re-export the lock and diff against the checked-in file. Mirrors the `uv-lock-fresh` pre-commit hook's export invocation exactly so the two - cannot disagree about what "fresh" means. The full re-lock (`uv lock`) lives in the - hook alone — running it in CI would cost network and resolution time we already spend - at commit time. The export step is local-only and fast. + cannot disagree about what "fresh" means. Pairs with `check_uv_lock_current`, which + covers the lock-vs-manifest half; neither runs a full re-lock, which stays in the hook. """ checked_in = root / "requirements_lock.txt" if not checked_in.is_file(): @@ -352,6 +389,7 @@ def main() -> int: errors += check_python_workspace_root(root) errors += check_python_workspace_members(root, find_python_projects(root)) + errors += check_uv_lock_current(root) errors += check_uv_lock_fresh(root) if errors == 0: diff --git a/meta/scripts/test_check_modules.py b/meta/scripts/test_check_modules.py index bf78fc6..571ed0a 100644 --- a/meta/scripts/test_check_modules.py +++ b/meta/scripts/test_check_modules.py @@ -860,9 +860,51 @@ def fake_run(cmd, **_kwargs): self.assertEqual(cmd[:2], ["uv", "export"]) self.assertNotIn("--no-hashes", cmd) self.assertIn("--no-emit-project", cmd) + # Without this the check re-locks the tree it is checking, repairing staleness in the + # runner and comparing against a file the commit does not contain. + self.assertIn("--frozen", cmd) self.assertEqual(cmd[cmd.index("--format") + 1], "requirements-txt") +# ── TestCheckUvLockCurrent ───────────────────────────────────────────────────── +# The argv contract matters as much as the return code: `uv lock` without --check +# would *rewrite* the lock from a CI job instead of reporting it, which is the +# behaviour this backstop exists to avoid. + + +class TestCheckUvLockCurrent(unittest.TestCase): + def _run_with(self, side_effect): + with ( + mock.patch("subprocess.run", side_effect=side_effect) as run, + mock.patch("sys.stdout", new_callable=io.StringIO) as stdout, + ): + rc = check_modules.check_uv_lock_current(Path("/fake")) + return rc, stdout.getvalue(), run + + def test_passes_and_only_checks(self): + rc, _, run = self._run_with(lambda *a, **k: mock.Mock(returncode=0)) + self.assertEqual(rc, 0) + self.assertEqual(run.call_args.args[0], ["uv", "lock", "--check"]) + + def test_stale_lock_reports_and_names_the_fix(self): + def fake(*_a, **_k): + raise subprocess.CalledProcessError(2, ["uv"], "", "the lockfile is not up-to-date") + + rc, out, _ = self._run_with(fake) + self.assertEqual(rc, 1) + self.assertIn("uv.lock", out) + self.assertIn("uv lock", out) + + def test_missing_uv_is_not_a_failure(self): + # Same posture as the export check: this runs wherever check_modules.py runs, and a + # host without uv has nothing to say about the lock. + def fake(*_a, **_k): + raise FileNotFoundError + + rc, _, _ = self._run_with(fake) + self.assertEqual(rc, 0) + + # ── TestMain ─────────────────────────────────────────────────────────────────── # Driver-level wiring. Mocks each per-language and per-invariant check function to # return preset error counts; asserts main aggregates correctly and prints the @@ -875,7 +917,7 @@ def _run(self, **return_values: int) -> tuple[int, str]: """Run check_modules.main() with mocks for every check function. return_values keys: 'configs_go', 'configs_python', 'matrices', 'py_root', - 'py_members', 'uv_lock'. Missing keys default to 0. + 'py_members', 'uv_lock', 'uv_current'. Missing keys default to 0. """ configs_results = { "go": return_values.get("configs_go", 0), @@ -909,6 +951,13 @@ def fake_configs(_root, language, _modules): "check_uv_lock_fresh", return_value=return_values.get("uv_lock", 0), ), + # Mocked like the rest, and load-bearing: unmocked it would shell out to `uv` + # against the real workspace from inside a unit test. + mock.patch.object( + check_modules, + "check_uv_lock_current", + return_value=return_values.get("uv_current", 0), + ), mock.patch("sys.stdout", new_callable=io.StringIO) as stdout, ): rc = check_modules.main() @@ -921,9 +970,15 @@ def test_all_clean_prints_success(self): def test_aggregates_error_counts(self): rc, _ = self._run( - configs_go=2, configs_python=1, matrices=3, py_root=1, py_members=2, uv_lock=1 + configs_go=2, + configs_python=1, + matrices=3, + py_root=1, + py_members=2, + uv_lock=1, + uv_current=1, ) - self.assertEqual(rc, 10) + self.assertEqual(rc, 11) def test_success_message_suppressed_on_any_error(self): rc, out = self._run(uv_lock=1) From 8e7e87af6c91a459c9da7917a12e48064b077590 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Tue, 11 Aug 2026 10:35:09 -0700 Subject: [PATCH 07/11] fix(devcontainer): address review on the derived pin and the backstops Behaviour: - The publish verification was single-shot against a registry whose tag->digest resolution is not documented as read-your-writes, seconds after `crane tag`. Three attempts with backoff, and one tool for all three references -- `imagetools inspect` takes a digest reference too, and `docker manifest` is still nominally experimental. Each attempt is wrapped in `timeout`: a call that never returns would hang the publish job on main. - A stale uv.lock produced two errors for one cause, the second naming requirements_lock.txt. The two uv checks are now sequential, and a test pins that the export diff does not run when the lock itself is stale. - Dropped `--check` from sync_base_image_pin. No caller, no test, and the module docstring advertised it -- the shape that rots. The bazel test is the check, and reads the same pure functions. - `//meta/devcontainer-base:image` is visible to `//.devcontainer` rather than public, which is what its own comment already claimed. Tests: main()`s rewrite path is covered -- rewrite, no-op re-run, and a missing layout returning 2 without touching the Dockerfile, the case with no backstop anywhere. test_devcontainer_config takes the digest half of its assertion from sync_base_image_pin instead of carrying a second sha256 regex. Comments and docs, all of which said something that stopped being true: the export "mirrors the hook exactly" (it deliberately differs by --frozen, in two places), the cache paragraph blaming a later digest bump that no longer exists, a missing blank line joining two paragraphs, a 175-character line, the base-image-pin hook`s ability to rewrite MODULE.bazel.lock and be blamed for it, and which cost the .bazelversion classification takes and why. The new CI job and the pre-existing shellcheck job are now in the README table. `bazel mod tidy --config=ci` matches gazelle-check next door; verified the config resolves for `mod`. Co-Authored-By: Claude Opus 5 --- .claude/CLAUDE.md | 8 ++-- .devcontainer/BUILD.bazel | 1 + .devcontainer/test_devcontainer_config.py | 10 ++++- .github/workflows/ci.yml | 4 +- .github/workflows/devcontainer.yml | 35 +++++++++++---- .pre-commit-config.yaml | 5 +++ README.md | 2 + meta/devcontainer-base/BUILD.bazel | 2 +- meta/devcontainer-base/README.md | 8 ++-- meta/scripts/check_modules.py | 15 ++++--- meta/scripts/sync_base_image_pin.py | 15 ------- meta/scripts/test_check_modules.py | 37 +++++++++++----- meta/scripts/test_sync_base_image_pin.py | 52 +++++++++++++++++++++++ 13 files changed, 146 insertions(+), 48 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 5ddb84f..7122e96 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -119,8 +119,9 @@ the `Renovate helper`. Load-bearing facts: 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, + 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 @@ -224,7 +225,8 @@ What is local to this repo: PR that changes the image carries the new pin. Three callers, one per source of change: the `base-image-pin` pre-commit hook for our edits, this workflow for Renovate's, and `//.devcontainer:test_base_image_pin` as the check under both — hooks are bypassable and the - workflow only fires for Renovate's own PRs. Renovate is configured to ignore the dep. The cost is that a base-editing branch pins an image the registry doesn't have yet; set + workflow only fires for Renovate's own PRs. Renovate is configured to ignore the dep. The cost + is that a base-editing branch pins an image the registry doesn't have yet; set `DEVCONTAINER_BASE_IMAGE` to the published `:latest` to keep working. Don't reach for `bazel run :load` locally — it needs a Docker daemon the devcontainer doesn't have, which is why that path is CI's. diff --git a/.devcontainer/BUILD.bazel b/.devcontainer/BUILD.bazel index 2df0dc7..3dc3d11 100644 --- a/.devcontainer/BUILD.bazel +++ b/.devcontainer/BUILD.bazel @@ -25,6 +25,7 @@ py_test( "//:.github/workflows/devcontainer.yml", ], main = "test_devcontainer_config.py", + deps = ["//meta/scripts:sync_base_image_pin_lib"], ) # The pin is a derived file (see meta/scripts/sync_base_image_pin.py). The `base-image-pin` diff --git a/.devcontainer/test_devcontainer_config.py b/.devcontainer/test_devcontainer_config.py index 51a98ba..2f23811 100644 --- a/.devcontainer/test_devcontainer_config.py +++ b/.devcontainer/test_devcontainer_config.py @@ -22,6 +22,8 @@ import unittest from pathlib import Path +from meta.scripts.sync_base_image_pin import pinned_digest + # Not .resolve(): devcontainer.yml is a cross-package data dep and lives in the runfiles tree, # which a resolved symlink would lead back out of. The rest read fine either way. _HERE = Path(__file__).parent @@ -257,11 +259,15 @@ def test_sentinel_is_a_stage_alias_on_the_pinned_base(self): def test_pinned_base_carries_both_a_tag_and_a_digest(self): # Digest for reproducibility, tag for Renovate to have something to compare against — # the same pairing MODULE.bazel's oci.pull uses for this image's own base. + # + # The digest half comes from sync_base_image_pin, which owns the pin and rewrites it; + # a second regex here would be the copy that silently stops agreeing with the writer. _, sentinel = self._global_arg_default() image = next(img for _, img, alias in self.froms if alias == sentinel) - reference, _, digest = image.partition("@") - self.assertRegex(digest, r"\Asha256:[0-9a-f]{64}\Z") + reference, _, _ = image.partition("@") self.assertRegex(reference, rf"\A{re.escape(_BASE_REPOSITORY)}:[\w][\w.-]*\Z") + # Raises if the Dockerfile carries no single well-formed pinned FROM. + pinned_digest(_DOCKERFILE.read_text(encoding="utf-8")) def test_alias_precedes_the_consuming_from(self): # Renovate's stage-name check only knows aliases declared above the line it is looking diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83a341b..43338b5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,7 +41,9 @@ jobs: buildbuddy-api-key: ${{ secrets.BUILDBUDDY_API_KEY }} - run: | set -euo pipefail - bazel mod tidy + # `--config=ci` for consistency with gazelle-check next door. It changes nothing + # here — `mod tidy` executes no actions, so BES and remote execution are moot. + bazel mod tidy --config=ci if ! git diff --exit-code -- MODULE.bazel MODULE.bazel.lock; then MSG="stale — run 'bazel mod tidy' and commit the result." echo "::error file=MODULE.bazel.lock::${MSG}" diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index cf88ec9..bd41840 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -62,6 +62,14 @@ jobs: # here would let a bump write a pin that no job ever publishes — a red PR with no way # to green it. The two classifications have to agree on what feeds the image. # + # This one is the expensive direction, deliberately, and it cuts against the + # false-positive argument below: the bazel version cannot enter `pkg_tar` layers over + # a pulled base, so every bazelisk bump buys a publish of a byte-identical index. The + # cheap alternative — drop it from the re-derivation instead and let + # //.devcontainer:test_base_image_pin fail if the digest ever does move — leaves the + # recovery path needing a config change at the moment someone is already confused. + # A recurring known cost beats a rare unrecoverable one. + # # Deliberately the whole file, not a grep for the pull's name. MODULE.bazel moves on # roughly 1 in 6 commits here and most of those cannot touch the image, so the false # positives are real and not cheap: two base-image runners, plus a consumer build @@ -379,22 +387,33 @@ jobs: # then one `crane tag` per tag is not atomic, so a failure in between leaves main pinned # to a digest the registry does not serve — silent until someone rebuilds a devcontainer. # - # Reads the pin through sync_base_image_pin.py rather than a grep of its own, so the - # Dockerfile shape has one parser and this cannot degrade into a check that always passes. + # Reads the pin with sync_base_image_pin.py rather than a grep of its own, so the digest + # has one parser and this cannot degrade into a check that always passes. + # + # Retried because GHCR does not document tag resolution as read-your-writes and this runs + # seconds after `crane tag`. A transient miss would turn main red for exactly the reason + # the step exists to rule out, and tell the reader to re-run a job that was fine. - name: Verify the registry serves what main pins run: | set -euo pipefail repo=ghcr.io/syndic/unnatural_designs-devcontainer-base pinned="$(python3 meta/scripts/sync_base_image_pin.py --print-pinned)" - # The digest is what a consumer's FROM resolves; the tags are what Renovate and any + # `@digest` is what a consumer's FROM resolves; the tags are what Renovate and any # non-deriving consumer read, and they are the half `crane tag` can leave behind. - docker manifest inspect "${repo}@${pinned}" >/dev/null - for tag in latest "sha-${GITHUB_SHA}"; do - resolved="$(docker buildx imagetools inspect --format '{{.Manifest.Digest}}' "${repo}:${tag}")" + # One tool for all three: `imagetools inspect` takes a digest reference too, and + # `docker manifest` is still nominally an experimental CLI command. + for ref in "@${pinned}" ":latest" ":sha-${GITHUB_SHA}"; do + resolved="" + for attempt in 1 2 3; do + resolved="$(timeout 60 docker buildx imagetools inspect \ + --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>/dev/null || true)" + [ "$resolved" = "$pinned" ] && break + sleep $((attempt * 5)) + done if [ "$resolved" != "$pinned" ]; then - echo "::error::${repo}:${tag} resolves to ${resolved}, but main pins ${pinned}." \ - "Re-run this job — crane tags non-atomically." + echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after 3 attempts," \ + "but main pins ${pinned}. Re-run this job — crane tags non-atomically." exit 1 fi done diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0d014aa..f5b67d8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,6 +29,11 @@ repos: # to the base or to the MODULE.bazel pull it sits on restales it in the same commit. # Rewrites rather than reports, like the two hooks above; //.devcontainer:test_base_image_pin # is the CI-side check for commits that never ran a hook. + # + # `bazel build` runs under the default --lockfile_mode=update, so on a MODULE.bazel-only + # commit — which bazel-mod-tidy above does not cover, its files are go.mod/work/sum — + # this can rewrite MODULE.bazel.lock too. pre-commit then blames base-image-pin for + # modifying files. The lock really was stale; the attribution is what misleads. entry: bash -c 'set -euo pipefail; bazel build //meta/devcontainer-base:image; python3 meta/scripts/sync_base_image_pin.py' pass_filenames: false # Every input to the image, not just the base directory: MODULE.bazel carries the diff --git a/README.md b/README.md index 30a0879..3726d27 100644 --- a/README.md +++ b/README.md @@ -178,12 +178,14 @@ Three GitHub Actions workflows run on every push and pull request to `main`. | Job | Trigger condition | | ---------------------------- | -------------------------------------------------------------------------------------------------- | | Gazelle check | Always - verifies BUILD files match source | +| MODULE.bazel.lock freshness | Always - verifies `bazel mod tidy` leaves MODULE.bazel and its lock unchanged | | Module completeness check | Always - verifies Go module matrix/config and Python workspace/lock invariants | | go.work check | Always - verifies all Go modules are registered in `go.work` | | Secrets check | Always - verifies the `secrets/` directory contains no committed files | | No-cgo policy check | Always - rejects `import "C"` and transitive deps that compile C/C++/cgo/SWIG | | golangci-lint | After module check passes - runs per Go module | | ruff | Always - `ruff format --check` and `ruff check` over all Python | +| shellcheck | Always - lints every tracked `*.sh` | | ty | Always - `uvx ty@ check` (Astral's static type checker, alpha) over all Python | | Build and test | After all checks above pass | | Coverage | After build and test - `bazel coverage //...`, uploads merged lcov to Codecov | diff --git a/meta/devcontainer-base/BUILD.bazel b/meta/devcontainer-base/BUILD.bazel index 1595005..5788d73 100644 --- a/meta/devcontainer-base/BUILD.bazel +++ b/meta/devcontainer-base/BUILD.bazel @@ -71,7 +71,7 @@ oci_image_index( ], # Visible so the consumer can assert its own pin against this: the digest here is what # //.devcontainer:test_base_image_pin compares the Dockerfile's `FROM` to. - visibility = ["//visibility:public"], + visibility = ["//.devcontainer:__pkg__"], ) # `bazel run //meta/devcontainer-base:load` puts the image in the local Docker daemon as diff --git a/meta/devcontainer-base/README.md b/meta/devcontainer-base/README.md index 1e05ae6..2463930 100644 --- a/meta/devcontainer-base/README.md +++ b/meta/devcontainer-base/README.md @@ -172,11 +172,13 @@ That does not transfer to a consumer outside this repo. `Syndic/.dotfiles` doesn so for it the pin is a genuine dependency: pin the digest, let Renovate's `docker` manager bump it, and gate the bump on its own devcontainer check. The derived treatment is available only to the repo that assembles the artifact. + CI sets `DEVCONTAINER_BASE_IMAGE=devcontainer-base:ci` — the tag `bazel run :load` produces — whenever a PR touches the base, so a base change is smoke-tested against a real consumer *before* -it publishes, rather than a Renovate bump later. One consequence for a consumer that caches its -built image: the push that merges a base change seeds that cache from the candidate image while the -Dockerfile still pins the previous digest, so builds miss the cache until the digest bump lands. +it publishes. One consequence for a consumer that caches its built image: a base-changing push to +`main` builds against `devcontainer-base:ci` while every later PR builds against `pinned-base`, so +the `FROM` reference string differs and the layer cache misses from that line down. It re-seeds on +the next `main` push that doesn't move the base. Two freshness facts to keep in mind. `devcontainer up` reuses an existing container, so a base bump lands on the next rebuild, not the next `up`. And because this repo's pin is derived, on any diff --git a/meta/scripts/check_modules.py b/meta/scripts/check_modules.py index ea6bc52..5905e0d 100644 --- a/meta/scripts/check_modules.py +++ b/meta/scripts/check_modules.py @@ -344,9 +344,10 @@ def check_uv_lock_current(root: Path) -> int: def check_uv_lock_fresh(root: Path) -> int: """Cheap freshness check: re-export the lock and diff against the checked-in file. - Mirrors the `uv-lock-fresh` pre-commit hook's export invocation exactly so the two - cannot disagree about what "fresh" means. Pairs with `check_uv_lock_current`, which - covers the lock-vs-manifest half; neither runs a full re-lock, which stays in the hook. + Mirrors the `uv-lock-fresh` pre-commit hook's export invocation apart from `--frozen` + (see :func:`_uv_export` for why that flag is on this side only), so the two cannot + disagree about what "fresh" means. Pairs with `check_uv_lock_current`, which covers the + lock-vs-manifest half; neither runs a full re-lock, which stays in the hook. """ checked_in = root / "requirements_lock.txt" if not checked_in.is_file(): @@ -389,8 +390,12 @@ def main() -> int: errors += check_python_workspace_root(root) errors += check_python_workspace_members(root, find_python_projects(root)) - errors += check_uv_lock_current(root) - errors += check_uv_lock_fresh(root) + # Sequential, not summed: a stale lock makes `uv export --frozen` exit non-zero too, and + # that second message names requirements_lock.txt — the innocent file. One cause, one line. + uv_errors = check_uv_lock_current(root) + if uv_errors == 0: + uv_errors = check_uv_lock_fresh(root) + errors += uv_errors if errors == 0: print("All modules and workspace invariants are consistent.") diff --git a/meta/scripts/sync_base_image_pin.py b/meta/scripts/sync_base_image_pin.py index 673d6c5..460f9d9 100644 --- a/meta/scripts/sync_base_image_pin.py +++ b/meta/scripts/sync_base_image_pin.py @@ -22,7 +22,6 @@ bazel build //meta/devcontainer-base:image python3 meta/scripts/sync_base_image_pin.py # rewrite the pin - python3 meta/scripts/sync_base_image_pin.py --check # exit 1 if it is stale Three callers, one per source of change: the `base-image-pin` pre-commit hook for our own edits, `renovate-derived-files.yml` for Renovate's (it moves the upstream base this digest is assembled @@ -103,11 +102,6 @@ def main(argv: list[str] | None = None) -> int: default=_DEFAULT_LAYOUT, help="OCI layout directory built by //meta/devcontainer-base:image.", ) - parser.add_argument( - "--check", - action="store_true", - help="Report staleness and exit 1 instead of rewriting.", - ) parser.add_argument( "--print-pinned", action="store_true", @@ -138,15 +132,6 @@ def main(argv: list[str] | None = None) -> int: print(f"{args.dockerfile} already pins the built image ({built}).") return 0 - if args.check: - print(f"{args.dockerfile} pins {current}, but this tree builds {built}.", file=sys.stderr) - print( - "Run `bazel build //meta/devcontainer-base:image && " - "python3 meta/scripts/sync_base_image_pin.py` and commit the result.", - file=sys.stderr, - ) - return 1 - args.dockerfile.write_text(replace_pin(dockerfile, built), encoding="utf-8") print(f"{args.dockerfile}: {current} -> {built}") return 0 diff --git a/meta/scripts/test_check_modules.py b/meta/scripts/test_check_modules.py index 571ed0a..82b1caf 100644 --- a/meta/scripts/test_check_modules.py +++ b/meta/scripts/test_check_modules.py @@ -839,8 +839,10 @@ def test_members_not_a_list_silently_passes(self): # Direct-seam coverage for the argv contract. check_uv_lock_fresh tests mock at the # _uv_export boundary, so they can't catch a refactor that silently swaps a flag # (e.g. --no-emit-project -> --no-emit-workspace, or --format requirements-txt -> -# --format json). The freshness check's correctness depends on these flags mirroring -# the uv-lock-fresh pre-commit hook's invocation; this is the test that catches drift. +# --format json). The freshness check's correctness depends on these flags matching the +# uv-lock-fresh pre-commit hook's invocation everywhere except `--frozen`, which is +# deliberately on this side only — the hook re-locks first, a check must not. This is the +# test that catches drift in either direction, including someone "restoring" the symmetry. class TestUvExport(unittest.TestCase): @@ -969,16 +971,31 @@ def test_all_clean_prints_success(self): self.assertIn("consistent", out) def test_aggregates_error_counts(self): + # uv_current stays 0 here: the two uv checks are sequential, not summed, so a case + # with both set would measure the suppression below rather than the aggregation. rc, _ = self._run( - configs_go=2, - configs_python=1, - matrices=3, - py_root=1, - py_members=2, - uv_lock=1, - uv_current=1, + configs_go=2, configs_python=1, matrices=3, py_root=1, py_members=2, uv_lock=1 ) - self.assertEqual(rc, 11) + self.assertEqual(rc, 10) + + def test_a_stale_lock_suppresses_the_export_diff(self): + # `uv export --frozen` also fails when the lock is stale, and its message names + # requirements_lock.txt — the innocent file. One cause must produce one message. + with ( + tempfile.TemporaryDirectory() as tmp, + mock.patch.object(check_modules, "workspace_root", return_value=Path(tmp)), + mock.patch.object(check_modules, "check_module_configs", return_value=0), + mock.patch.object(check_modules, "check_workflow_matrices", return_value=0), + mock.patch.object(check_modules, "check_python_workspace_root", return_value=0), + mock.patch.object(check_modules, "check_python_workspace_members", return_value=0), + mock.patch.object(check_modules, "check_uv_lock_current", return_value=1), + mock.patch.object(check_modules, "check_uv_lock_fresh", return_value=1) as fresh, + mock.patch("sys.stdout", new_callable=io.StringIO), + ): + rc = check_modules.main() + + self.assertEqual(rc, 1) + fresh.assert_not_called() def test_success_message_suppressed_on_any_error(self): rc, out = self._run(uv_lock=1) diff --git a/meta/scripts/test_sync_base_image_pin.py b/meta/scripts/test_sync_base_image_pin.py index 5edbb8e..8dfa504 100644 --- a/meta/scripts/test_sync_base_image_pin.py +++ b/meta/scripts/test_sync_base_image_pin.py @@ -127,5 +127,57 @@ def test_refuses_a_dockerfile_it_cannot_parse(self): self._run("FROM debian:bookworm\n") +class TestRewritePath(unittest.TestCase): + """`main()`'s mutating half — what the pre-commit hook and renovate-derived-files.yml run. + + The pure functions above cover the parsing; this covers the driver that decides whether to + touch the file at all. The missing-layout case is the one with no backstop anywhere: a + `write_text` firing on a half-built tree would pin a digest nothing published. + """ + + def _tree(self, tmp: str, digest: str | None) -> tuple[Path, Path]: + dockerfile = Path(tmp) / "Dockerfile" + dockerfile.write_text(_DOCKERFILE, encoding="utf-8") + layout = Path(tmp) / "layout" + if digest is not None: + layout.mkdir() + (layout / "index.json").write_text( + json.dumps({"manifests": [{"digest": digest}]}), encoding="utf-8" + ) + return dockerfile, layout + + def _run(self, dockerfile: Path, layout: Path) -> tuple[int, str]: + out = io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(out): + code = main(["--dockerfile", str(dockerfile), "--layout-dir", str(layout)]) + return code, out.getvalue() + + def test_rewrites_the_pin_to_the_built_digest(self): + with tempfile.TemporaryDirectory() as tmp: + dockerfile, layout = self._tree(tmp, _NEW) + code, _ = self._run(dockerfile, layout) + self.assertEqual(code, 0) + self.assertEqual(pinned_digest(dockerfile.read_text(encoding="utf-8")), _NEW) + + def test_rerunning_is_a_no_op(self): + # The hook runs on every matching commit, so "already correct" must not be an error — + # and must not rewrite, or every commit would carry a spurious Dockerfile diff. + with tempfile.TemporaryDirectory() as tmp: + dockerfile, layout = self._tree(tmp, _OLD) + before = dockerfile.read_bytes() + code, _ = self._run(dockerfile, layout) + self.assertEqual(code, 0) + self.assertEqual(dockerfile.read_bytes(), before) + + def test_missing_layout_refuses_without_touching_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + dockerfile, layout = self._tree(tmp, None) + before = dockerfile.read_bytes() + code, out = self._run(dockerfile, layout) + self.assertEqual(code, 2) + self.assertIn("bazel build", out) + self.assertEqual(dockerfile.read_bytes(), before) + + if __name__ == "__main__": sys.exit(0 if unittest.main(exit=False).result.wasSuccessful() else 1) From 8cfd0bc8deb36c811e53c35560b8ec4ea21f8e90 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Tue, 11 Aug 2026 12:54:34 -0700 Subject: [PATCH 08/11] ci(devcontainer): keep imagetools stderr, and finish a reflow Two leftovers from the review pass. The publish verification dropped stderr, so propagation lag, an auth failure and an unreachable registry all reached the log as the same empty answer -- and the annotation tells the reader to re-run the job, which only helps for the first of those. The last stderr now follows the ::error:: line. The .bazelversion bullet reflowed only as far as the long line that needed it, leaving a 29-character orphan mid-paragraph. Co-Authored-By: Claude Opus 5 --- .claude/CLAUDE.md | 7 +++---- .github/workflows/devcontainer.yml | 7 ++++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 7122e96..8c18002 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -120,10 +120,9 @@ the `Renovate helper`. Load-bearing facts: 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. + 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/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index bd41840..2a3ba4b 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -398,6 +398,7 @@ jobs: set -euo pipefail repo=ghcr.io/syndic/unnatural_designs-devcontainer-base pinned="$(python3 meta/scripts/sync_base_image_pin.py --print-pinned)" + err="${RUNNER_TEMP}/imagetools-inspect.err" # `@digest` is what a consumer's FROM resolves; the tags are what Renovate and any # non-deriving consumer read, and they are the half `crane tag` can leave behind. @@ -406,14 +407,18 @@ jobs: for ref in "@${pinned}" ":latest" ":sha-${GITHUB_SHA}"; do resolved="" for attempt in 1 2 3; do + # stderr is kept rather than dropped: propagation lag, an auth failure and an + # unreachable registry all look like the same empty answer without it. resolved="$(timeout 60 docker buildx imagetools inspect \ - --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>/dev/null || true)" + --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>"$err" || true)" [ "$resolved" = "$pinned" ] && break sleep $((attempt * 5)) done if [ "$resolved" != "$pinned" ]; then echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after 3 attempts," \ "but main pins ${pinned}. Re-run this job — crane tags non-atomically." + echo "Last stderr from imagetools inspect:" + cat "$err" exit 1 fi done From 88bafb55cca135edb8eb78281f2e46610b8d7409 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Wed, 12 Aug 2026 11:13:19 -0700 Subject: [PATCH 09/11] ci(devcontainer): drop the sleep after the final publish-check attempt The retry loop slept its full backoff after the last attempt, then fell straight out and failed -- 15s per ref, three refs, buying nothing. It also read as though the sleep belonged to an attempt rather than to the gap between two. Guarded with `||` rather than `&&` deliberately. An `&&` guard whose test fails returns 1; that survives set -e as the last line of a loop body but not as the last line of a function, so the working form here would have been a property of where it sits. The `||` form returns 0 either way. The retry count was already spelled twice (the loop list and the error text, silently coupled); a guard would have made it three. Hoisted to `attempts` so the three uses cannot drift. Exercised the failure path with stubs, which CI has never run: two sleeps for three attempts, the count interpolated into the message, and a stubbed `unauthorized` reaching the log rather than /dev/null. Co-Authored-By: Claude Opus 5 --- .github/workflows/devcontainer.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index 2a3ba4b..552c1ff 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -399,6 +399,7 @@ jobs: repo=ghcr.io/syndic/unnatural_designs-devcontainer-base pinned="$(python3 meta/scripts/sync_base_image_pin.py --print-pinned)" err="${RUNNER_TEMP}/imagetools-inspect.err" + attempts=3 # `@digest` is what a consumer's FROM resolves; the tags are what Renovate and any # non-deriving consumer read, and they are the half `crane tag` can leave behind. @@ -406,17 +407,19 @@ jobs: # `docker manifest` is still nominally an experimental CLI command. for ref in "@${pinned}" ":latest" ":sha-${GITHUB_SHA}"; do resolved="" - for attempt in 1 2 3; do + for attempt in $(seq "$attempts"); do # stderr is kept rather than dropped: propagation lag, an auth failure and an # unreachable registry all look like the same empty answer without it. resolved="$(timeout 60 docker buildx imagetools inspect \ --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>"$err" || true)" [ "$resolved" = "$pinned" ] && break - sleep $((attempt * 5)) + # `||` rather than `&&`: a failing `&&` guard returns 1, which set -e traps. + [ "$attempt" -eq "$attempts" ] || sleep $((attempt * 5)) done if [ "$resolved" != "$pinned" ]; then - echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after 3 attempts," \ - "but main pins ${pinned}. Re-run this job — crane tags non-atomically." + echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after" \ + "${attempts} attempts, but main pins ${pinned}." \ + "Re-run this job — crane tags non-atomically." echo "Last stderr from imagetools inspect:" cat "$err" exit 1 From 1a63bf4aa6af72d72220c8b37b095e2e9bb785f5 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Wed, 12 Aug 2026 11:40:00 -0700 Subject: [PATCH 10/11] ci(devcontainer): report the exit status of the last publish-check attempt A `timeout` kill writes nothing to stderr -- GNU coreutils confirms 0 bytes and exit 124 -- so that failure printed the stderr header followed by silence. It was the one case a reader could not name: empty stderr plus an empty answer looked identical to a docker failure that happened to print nothing. Capturing the status with `|| rc=$?` closes it, and replaces the `|| true` that was there for set -e. rc is reset at the top of each attempt, not only captured. `|| rc=$?` assigns on failure alone, so without the reset a run where attempt 1 timed out and attempts 2-3 exited 0 with a mismatched digest would have reported 124 for an attempt that exited cleanly -- reintroducing the misleading diagnostic this is meant to remove. The second `rc=0`, beside `resolved=""`, is the set -u guard for if `attempts` ever stops being a literal. Exercised all three with stubs, none of which CI runs: a timeout kill reports 124, an auth failure reports 1 with its message, and the stale-rc case reports 0. Co-Authored-By: Claude Opus 5 --- .github/workflows/devcontainer.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index 552c1ff..61e4315 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -407,11 +407,14 @@ jobs: # `docker manifest` is still nominally an experimental CLI command. for ref in "@${pinned}" ":latest" ":sha-${GITHUB_SHA}"; do resolved="" + rc=0 for attempt in $(seq "$attempts"); do - # stderr is kept rather than dropped: propagation lag, an auth failure and an - # unreachable registry all look like the same empty answer without it. + # stderr and the exit status are both kept: propagation lag, an auth failure and + # an unreachable registry otherwise look alike, and a `timeout` kill writes no + # stderr at all, so the status is the only thing that names that one. + rc=0 resolved="$(timeout 60 docker buildx imagetools inspect \ - --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>"$err" || true)" + --format '{{.Manifest.Digest}}' "${repo}${ref}" 2>"$err")" || rc=$? [ "$resolved" = "$pinned" ] && break # `||` rather than `&&`: a failing `&&` guard returns 1, which set -e traps. [ "$attempt" -eq "$attempts" ] || sleep $((attempt * 5)) @@ -420,7 +423,7 @@ jobs: echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after" \ "${attempts} attempts, but main pins ${pinned}." \ "Re-run this job — crane tags non-atomically." - echo "Last stderr from imagetools inspect:" + echo "Last attempt exited ${rc}; its stderr follows (124 = timed out):" cat "$err" exit 1 fi From bbc7ea4bdf19b5b3453ced6740ea3ee2b6f63ea6 Mon Sep 17 00:00:00 2001 From: Joshua Yanchar Date: Wed, 12 Aug 2026 11:46:07 -0700 Subject: [PATCH 11/11] ci(devcontainer): only name the timeout when it was one The `(124 = timed out)` legend printed on every failure, so an auth failure read `Last attempt exited 1; its stderr follows (124 = timed out):` -- a legend the reader has to decode mid-sentence, in a step whose whole purpose is saying plainly what went wrong. Now the parenthetical appears only when rc is actually 124. `||` rather than `&&` for the same set -e reason as the sleep guard above it. Re-ran the three stub paths: 124 gains ` (timed out)`, exit 1 and exit 0 carry no parenthetical. Co-Authored-By: Claude Opus 5 --- .github/workflows/devcontainer.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/devcontainer.yml b/.github/workflows/devcontainer.yml index 61e4315..6bf8d51 100644 --- a/.github/workflows/devcontainer.yml +++ b/.github/workflows/devcontainer.yml @@ -423,7 +423,9 @@ jobs: echo "::error::${repo}${ref} resolved to '${resolved:-nothing}' after" \ "${attempts} attempts, but main pins ${pinned}." \ "Re-run this job — crane tags non-atomically." - echo "Last attempt exited ${rc}; its stderr follows (124 = timed out):" + hint="" + [ "$rc" -ne 124 ] || hint=" (timed out)" + echo "Last attempt exited ${rc}${hint}; its stderr follows:" cat "$err" exit 1 fi