diff --git a/.dockerignore b/.dockerignore index 5414d56..86b2d1a 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,58 @@ +# .dockerignore does NOT use .gitignore semantics. Docker matches with +# moby/patternmatcher: filepath.Match plus `**`, so `*` does not cross +# `/` and an unprefixed pattern is anchored at the context root. Every +# depth-independent pattern therefore needs `**/`, or `config/.env` and +# `certs/server.key` still ship while the file reads as solved. Only +# genuinely root-anchored entries go unprefixed. Never transplant these +# into .gitignore, where `**/` is wrong. +# +# Matching is case-sensitive, so secrets use character ranges rather +# than an ALL-CAPS twin, which would still miss `Server.Key`. +# +# Extend with this repo's own host-built artifacts, written anchored: +# `/myapp`, never `**/myapp`, which also matches `cmd/myapp/` and +# deletes the package directory from the context. + +# Excluding .git means `git describe` cannot run in any build stage and +# fails quietly there; pass the version in with --build-arg VERSION. .git -node_modules -.DS_Store + +# Agent scratch: one full checkout of the repo per in-flight agent. +# Anchored because it occurs once where agents run at the repo root. +# KNOWN GAP: a repo running agents in subdirectories still ships +# `services/api/.claude/` and must add its own anchored entry. +.claude + +# Environment files. `*.env` covers bare `.env` and the `prod.env` +# convention. Re-include a committed template with a negation if the +# build needs one: `!docs/example.env`. +**/*.[eE][nN][vV] +**/.[eE][nN][vV].* +**/.[eE][nN][vV][rR][cC] + +# Private keys and the bundles carrying them. Public certificates +# (*.crt, *.cer) are deliberately absent: they are legitimate inputs. +**/*.[pP][eE][mM] +**/*.[kK][eE][yY] +**/*.[pP]12 +**/*.[pP][fF][xX] +**/[iI][dD]_[rR][sS][aA] +**/[iI][dD]_[dD][sS][aA] +**/[iI][dD]_[eE][cC][dD][sS][aA] +**/[iI][dD]_[eE][dD]25519 + +# Dependencies: restored inside the image, never copied in. +**/node_modules + +# OS metadata. +**/.DS_Store +**/Thumbs.db + +# Editor state: never a build input, and it churns COPY. +**/*.swp +**/*.swo +**/*~ +**/*.bak +**/.idea +**/.vscode +**/*.sublime-* diff --git a/.gitignore b/.gitignore index c5a0882..3558b76 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,12 @@ Thumbs.db .vscode/ *.sublime-* +# Agent scratch (worktrees of this repo, created and destroyed by +# in-flight tooling). Unanchored: .gitignore patterns already match at +# every depth, so no prefix is wanted here. This is not a .dockerignore +# entry and must not be given a `**/` prefix on the way into one. +.claude/ + # Node node_modules/ diff --git a/Dockerfile b/Dockerfile index cb6489a..f9619d8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,13 +3,26 @@ FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e3 WORKDIR /app -# script/bootstrap installs all prerequisites (make via apk here; node -# and yarn are already in the base image, so those steps are skipped). -# Dependency manifests are copied first so the bootstrap layer is -# cached until they change. +# Makes script/lint run the linter directly rather than building +# Dockerfile.lint, which would need a docker daemon here. +ENV LINT_IN_CONTAINER=1 + +# script/bootstrap installs all prerequisites. Manifests are copied +# first so that layer stays cached until dependencies change. COPY script/ script/ COPY package.json yarn.lock ./ RUN script/bootstrap COPY . . -RUN make check + +# CHECK_EPOCH is a per-invocation nonce from script/cibuild and +# script/docker; without it an unchanged tree serves this layer from +# cache and the build reports a green it never ran. ARG is stage-scoped, +# so declare it in every stage that runs checks. The guard fails a bare +# `docker build .`, which would otherwise reuse the empty (and therefore +# stable) cache key. The value is also expanded into the check command, +# so the cache miss does not depend on BuildKit's handling of an +# unreferenced ARG; keep both references. +ARG CHECK_EPOCH +RUN [ -n "$CHECK_EPOCH" ] || exit 1 +RUN echo "check epoch: ${CHECK_EPOCH}" && make check diff --git a/Dockerfile.lint b/Dockerfile.lint new file mode 100644 index 0000000..7957ba9 --- /dev/null +++ b/Dockerfile.lint @@ -0,0 +1,27 @@ +# Lint-only image, built by script/lint when it is not already inside a +# container. Linting is a build step, so a successful build is a clean +# lint, and nothing is bind-mounted, which matters when the daemon is +# remote. +# +# node 22-alpine, 2026-02-22 +FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 + +WORKDIR /app + +# Makes script/lint run the linter directly instead of recursing into +# another docker build, which has no daemon here. +ENV LINT_IN_CONTAINER=1 + +COPY script/ script/ +COPY package.json yarn.lock ./ +RUN script/bootstrap + +COPY . . + +# ARG sits after the dependency layer so that layer stays cached and +# only the lint re-runs. The guard fails a bare `docker build +# -f Dockerfile.lint .`, which would otherwise reuse the empty (stable) +# cache key and report a lint it never ran. +ARG CHECK_EPOCH +RUN [ -n "$CHECK_EPOCH" ] || exit 1 +RUN echo "lint epoch: ${CHECK_EPOCH}" && make lint diff --git a/README.md b/README.md index 3991bfc..15ba785 100644 --- a/README.md +++ b/README.md @@ -117,15 +117,21 @@ alpine. We provide: - `script/projectname` — output the project name (our own extension); used by `script/docker` for the image tag - `script/test` — run the test suite (no tests defined here) -- `script/lint` — lint the markdown files with prettier +- `script/lint` — lint the markdown files with prettier. Inside a container + (`LINT_IN_CONTAINER=1`, set by both Dockerfiles) it runs prettier directly; on + a host it builds `Dockerfile.lint` so the linter still runs in a container - `script/fmt` — format all markdown files with prettier (writes) - `script/fmt-check` — check formatting (read-only) - `script/check` — run all checks: `test`, `lint`, `fmt-check` (our own extension) - `script/docker` — build the Docker image, tagged via `script/projectname` - (byte-identical across repos) -- `script/cibuild` — cd to the repo root and `docker build .` (what CI runs; the - image build runs `script/check`) + (byte-identical across repos); passes the same `CHECK_EPOCH` nonce as + `script/cibuild` +- `script/cibuild` — cd to the repo root, assign `epoch="$(date +%s%N)$$"`, then + `docker build --build-arg CHECK_EPOCH="$epoch" .` (what CI runs; the image + build runs `script/check`, and the per-invocation `CHECK_EPOCH` nonce is what + stops Docker serving that check from cache on an unchanged tree — a bare + `docker build .` fails closed on purpose) - `script/precommit` — run by the git pre-commit hook (our own extension); calls `script/check` - `script/install-precommit` — installs the git pre-commit hook (our own diff --git a/TODO.md b/TODO.md index fac919c..bf8d674 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,114 @@ fmt-check, and commit. # Completed Steps +- 2026-08-10: Moved every lint run into a container. `script/lint` now runs the + linter directly when `LINT_IN_CONTAINER=1` and otherwise builds + `Dockerfile.lint`, so the linter never runs on a developer host — closing the + content-keyed result cache that produced a confirmed false green, the + host-global `$TMPDIR/golangci-lint.lock`, and host/container version skew. + Detection is on that marker alone: a false negative inside a container fails + loudly on the missing daemon, while a false positive on a host would silently + restore host linting, so `/.dockerenv` is rejected outright — measured absent + inside BuildKit `RUN` steps and present on hosts that are themselves + containers. Everything else keeps its existing shape: `make check` still runs + in the image, `script/cibuild` is still one build, and the Go multistage lint + stage survives with `ENV LINT_IN_CONTAINER=1`. `Dockerfile.lint` carries the + same `CHECK_EPOCH` guard, with the `ARG` below the dependency layer so only + the lint re-runs. The `script/bootstrap` golangci-lint install and the + per-checkout cache/lock/`.lint-cache` wrapper are deleted as superseded; a JS + repo's `yarn install` stays, since the rule is about where a verdict comes + from, not about which binaries exist. `golangci-lint config verify` was kept + on measurement: a bogus config key passes `golangci-lint run` with `0 issues` + and fails `config verify`, and every case reproduced byte-identically under + `--network none`, so the schema is embedded and the line costs no network. + Comment blocks across the touched files were cut hard in the same pass. +- 2026-08-09: Made a golangci-lint result belong to the tree that asked for it. + REPO_POLICIES.md now carries the canonical Go `script/lint`, which gives the + linter per-checkout `GOLANGCI_LINT_CACHE` and per-checkout `TMPDIR`. The two + are separate defects and the second is the one that gets dropped: the result + cache is keyed on file content rather than location, so checkouts holding + identical files serve each other's findings under the other's path, while the + concurrency lock is `$TMPDIR/golangci-lint.lock` — host-global, independent of + the cache, and unaffected by isolating it. Moving workers from worktrees to + their own clones does not help either half; it only removes the foreign-path + artefact that made the defect visible. The lock error is retried rather than + surfaced, because it is not a result: it exits non-zero exactly as findings + do, and reporting it as findings sends a correct branch back for rework. + Detection is on the stderr stream and never on exit status, so a finding + quoting the lock message in source cannot be retried away, and exhaustion + exits 75 with a VOID message rather than passing or failing quietly. + `--allow-serial-runners` (which keeps the guard and queues) covers the + same-checkout overlap that `TMPDIR` scoping cannot; `--allow-parallel-runners` + is rejected outright. The stdout and stderr capture files are per invocation + rather than per checkout, because serialising the linter does not serialise + the shell's redirections: two runs in one checkout — the overlap the flag + exists to support — would otherwise truncate and read each other's output, + which is the same defect one layer above where it was fixed. Both checklists + gained the corresponding items, since a half-fix that sets only the cache + reads as complete. `GOCACHE` was measured and does not need isolating. + Verified with the snippet extracted from the committed document and executed + as a consuming repo would adopt it, against paired controls: contamination + reproduced on the pre-fix form and absent on the adopted one, retry engaged, + exhaustion loud, a genuine finding still reported, and a held host lock + failing the pre-fix script while leaving the adopted one untouched. +- 2026-08-09: Kept in-repo agent scratch out of the Docker build context and out + of version control. `.claude/` holds one worktree — an entire additional + checkout of the repo — per in-flight agent, and under `COPY . .` all of it was + reaching the image: another session's unreviewed, sometimes uncommitted work, + inflating the context by a multiple of the repo and invalidating `COPY` for + reasons unrelated to the repo's own content. The `.dockerignore` entry is + root-anchored, because the directory occurs exactly once where agents run at + the repo root and the `**/` form additionally deletes any nested directory of + that name — with the residual gap that follows from anchoring (a monorepo + running agents in subdirectories still ships `services/api/.claude/`) stated + in the canonical `.dockerignore`, the policy and the existing-repo checklist, + since consuming repos receive the files rather than the tracker; the + `.gitignore` entry is unanchored, because `.gitignore` patterns already match + at every depth, and each file is written to its own semantics rather than + derived from the other. Also closed the consequence that ships broken + silently: excluding `.git` means `git describe` cannot run in any build stage + and yields an empty version without erroring, so `script/docker` and + `script/cibuild` now compute the version on the host and pass + `--build-arg VERSION`, and `REPO_POLICIES.md` states where `VERSION` comes + from instead of leaving the reader to fill the gap with `git describe` inside + the build. The two Go documents that carry the `GOLDFLAGS` pattern were + corrected in the same pass, from `:=` to `?=`, since a `$(shell git describe)` + evaluated inside a build stage is exactly the empty version this closes. + Verified by enumerating a probe image before, after, and against the + `**/`-prefixed form, with a positive control and the `CHECK_EPOCH` cache + verification re-run under the changed build context. +- 2026-08-09: Closed the secret exposure in the canonical `.dockerignore`: a + developer's local `.env`, `*.pem` or `*.key` was reaching the Docker build + context under `COPY . .`, invisible to every git-based check because + `.gitignore` covers it. The patterns are written to `.dockerignore`'s own + `moby/patternmatcher` semantics — `**/`-prefixed so they hold at every depth, + which also fixes nested `node_modules` — rather than transplanted from + `.gitignore`, whose unprefixed form protects only the repository root while + reading as solved. Coverage extends past the `.env`/`.pem`/`.key` trio to the + `prod.env` convention, `.envrc`, PKCS#12 bundles and extensionless SSH keys, + every one of them case-folded with character ranges because matching is + case-sensitive and an ALL-CAPS twin per pattern still misses `Server.Key`. + `REPO_POLICIES.md` and both repo checklists now state that asymmetry and + require verification by enumerating the image rather than by reading the + patterns. Verified with a probe image before, against three naive forms + (unprefixed, lowercase-only, ALL-CAPS-doubled), and after. +- 2026-08-09: Made the pinned golangci-lint actually propagate: REPO_POLICIES.md + now carries the canonical `script/bootstrap` snippet for Go repos, which + installs when the installed version does not match the pin (the old + `if missing` guard tested PATH presence only, so pins were inert on any + provisioned machine and CI silently disagreed with local) and then re-resolves + the binary through `PATH` and fails loudly, naming the shadowing path, when + the install did not take effect — the failure mode the naive + compare-then-install fix leaves behind while reporting success. +- 2026-08-09: Fixed the false green in the canonical CI gate: `script/cibuild` + and `script/docker` now pass a per-invocation `CHECK_EPOCH` nonce, and the + `Dockerfile` (plus the Go multistage template in REPO_POLICIES.md, in both its + lint and builder stages) declares `ARG CHECK_EPOCH` with a guard that makes a + bare `docker build .` fail closed. Corrected the org-canonical text that + asserted a successful build implies all checks pass, across every document + carrying it: `REPO_POLICIES.md`, both repo checklists (which still told agents + to write the pre-fix `script/cibuild` and ended on an acceptance item the + guard makes unsatisfiable), and the Go styleguide. - 2026-08-07: Set the canonical `.golangci.yml` to the org-standard v2-schema config already deployed byte-identical across the org's Go repos (settings under `linters.settings` so thresholds like lll/funlen/cyclop/dupl actually diff --git a/prompts/CODE_STYLEGUIDE_GO.md b/prompts/CODE_STYLEGUIDE_GO.md index 043e8f1..3b4a5a8 100644 --- a/prompts/CODE_STYLEGUIDE_GO.md +++ b/prompts/CODE_STYLEGUIDE_GO.md @@ -1,6 +1,6 @@ --- title: Code Styleguide — Go -last_modified: 2026-03-18 +last_modified: 2026-08-10 --- 1. Try to hard wrap long lines at 77 characters or less. @@ -49,7 +49,20 @@ last_modified: 2026-03-18 ``` ```make - VERSION := $(shell git describe --always --dirty) + # ?= rather than := because this `$(shell git describe ...)` is only + # correct on the host. `.dockerignore` excludes `.git`, so evaluated + # inside a build stage it expands to the empty string without failing + # and the binary reports no version at all. The version is computed on + # the host by `script/docker` / `script/cibuild` and passed with + # `--build-arg VERSION=...`. If this repo's Dockerfile compiles by + # invoking make (`RUN make build`), `ARG VERSION` in that stage puts the + # value in the environment and `?=` defers to it. The canonical Go + # template in REPO_POLICIES.md instead runs `go build` directly with + # `-ldflags "... -X main.Version=${VERSION}"`, so there this Makefile is + # a host-only path — but it is still `?=`, because a repo that later + # moves the build behind make must not silently start shipping an empty + # version. See the git-describe rule in REPO_POLICIES.md. + VERSION ?= $(shell git describe --always --dirty) BUILDARCH := $(shell uname -m) GOLDFLAGS += -X main.Version=$(VERSION) @@ -98,12 +111,23 @@ last_modified: 2026-03-18 1. For anything beyond a simple script or tool, or anything that is going to run in any sort of "production" anywhere, make sure it passes - `golangci-lint`. + `golangci-lint`. Run it with `make lint`, never by invoking the binary: the + linter always runs in a container, and `golangci-lint` is not installed on + the host by any repo. Invoked directly on a shared host it reads a result + cache keyed on file content rather than location, and a host-global lock, so + its answer may belong to another checkout entirely. 1. Write a `Dockerfile` for every repo, even if it only runs the tests and - linting. `docker build .` should always make sure that the code is in an - able-to-be-compiled state, linted, and any tests run. The Docker build - should fail if linting doesn't pass. + linting. `script/cibuild` and `script/docker` should always make sure that + the code is in an able-to-be-compiled state, linted, and any tests run, and + the build should fail if linting doesn't pass. That guarantee holds only + because those scripts pass a per-invocation `CHECK_EPOCH` build arg that + busts the check layers out of the Docker cache; without it an unchanged tree + serves those layers from cache and the build reports a green it never ran. A + bare `docker build .` fails closed by design, on the `[ -n "$CHECK_EPOCH" ]` + guard — always go through `script/cibuild` or `script/docker`. See + [Repository Policies](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md) + for the canonical form. 1. Every repo must have a `Makefile`. See [Repository Policies](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md) diff --git a/prompts/EXISTING_REPO_CHECKLIST.md b/prompts/EXISTING_REPO_CHECKLIST.md index 2f45550..59360d6 100644 --- a/prompts/EXISTING_REPO_CHECKLIST.md +++ b/prompts/EXISTING_REPO_CHECKLIST.md @@ -1,6 +1,6 @@ --- title: Existing Repo Checklist -last_modified: 2026-07-06 +last_modified: 2026-08-10 --- Use this checklist when beginning work in a repo that may not yet conform to our @@ -24,15 +24,66 @@ with your task. - [ ] `LICENSE` file exists and matches the README - [ ] `REPO_POLICIES.md` exists and version date is current — fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md` -- [ ] `.gitignore` is comprehensive (OS, editor, language artifacts, secrets) — - fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` - if missing +- [ ] `.gitignore` is comprehensive (OS, editor, agent scratch, language + artifacts, secrets) — fetch from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` if missing. + An existing repo usually has a hand-written one that is never re-fetched, + so check the entries rather than the file's presence: `.claude/` in + particular, unanchored, so agent worktrees cannot be committed by + accident. Do not give it a `**/` prefix — that is a `.dockerignore` form + and is wrong here. - [ ] `.editorconfig` exists — fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.editorconfig` -- [ ] `Dockerfile` and `.dockerignore` exist; Dockerfile runs `make check` as a - build step — fetch `.dockerignore` from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` -- [ ] Gitea Actions workflow in `.gitea/workflows/` runs `docker build .` on +- [ ] `Dockerfile` and `.dockerignore` exist (fetch `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore`); + Dockerfile runs `make check` as a build step, and every stage containing a + check-running `RUN` declares `ARG CHECK_EPOCH` with the + `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard immediately below it — see the + `CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is + served from cache on an unchanged tree and the build reports a green it + never ran. +- [ ] **Every stage that runs checks sets `ENV LINT_IN_CONTAINER=1`** — the lint + stage and the build stage both. This is the item an existing repo most + often fails after adopting the containerised lint: without it + `script/lint` tries to build `Dockerfile.lint` from inside a build step, + where there is no daemon. +- [ ] `Dockerfile.lint` exists and `script/lint` builds it when not already in a + container — see the containerised-lint rule in `REPO_POLICIES.md`. Base + image pinned by sha256 with a version/date comment, `ARG CHECK_EPOCH` + **after** the dependency layer with the guard below it. +- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled + binaries, test binaries, coverage output), written root-anchored — + `/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An + existing repo is where such a binary is likeliest to already be sitting in + the build context, invisible to git. +- [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/` + prefix. Agent worktrees are entire checkouts of the repo, so they inflate + the context by a multiple of it and can copy another session's unreviewed + work into an image layer. Confirm by enumerating the image, not by reading + the file — `.gitignore` hides these from `git status` too. +- [ ] **Do agents in this repo run anywhere other than the repo root?** The + scratch directory is created in the agent's working directory, so the + canonical anchored entry misses `services/api/.claude/` in a monorepo with + a per-service agent — it still reaches the build context and the image. An + existing repo is where such a layout already exists, so check it here + rather than assuming the canonical entry covers you: add anchored entries + for the subdirectories that have one (`/services/api/.claude`), or + `**/.claude` once you have confirmed no legitimately named nested + directory would be caught. +- [ ] If the repo embeds a version in a binary, that version is computed on the + host and passed with `--build-arg VERSION=...` by `script/docker` and + `script/cibuild`. No stage calls `git describe`: `.dockerignore` excludes + `.git`, so it yields an empty version without failing the build. A + tag-derived version additionally needs `fetch-depth: 0` on the CI checkout + step, which clones shallow and fetches no tags by default. +- [ ] Every depth-independent pattern in `.dockerignore` carries a `**/` prefix; + only genuinely root-anchored entries such as `.git` are unprefixed, and + `.gitignore`'s patterns have not been transplanted unmodified. + `.dockerignore` anchors an unprefixed pattern at the context root, so the + transplanted form leaves `config/.env` and `certs/server.key` in the build + context while reading as solved — see the `.dockerignore` rule in + `REPO_POLICIES.md`. +- [ ] Gitea Actions workflow in `.gitea/workflows/` runs `script/cibuild` on push — reference `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml` - [ ] Language-specific config: @@ -58,6 +109,24 @@ with your task. `script/install-precommit`, shimmed by `make hooks`) runs it - [ ] README has an **Entrypoints** section documenting the `script/` entrypoints and linking the standard +- [ ] `script/lint` is the canonical detect-and-branch form, and no host + invocation anywhere in the repo can produce a lint **verdict** — grep for + the linter's own name across `script/`, the `Makefile` and CI config, not + just `script/lint`. A second path is likeliest here: a `make lint-fast`, + an older container-versus-host branch, or a CI step calling the binary + directly. **Expected hits that are not the defect**: `script/fmt`, and in + a repo whose formatter is also its linter, `script/fmt-check`. Everything + else the grep finds is a real second path and goes. +- [ ] Detection is on `LINT_IN_CONTAINER` alone. Reject any `/.dockerenv` or + cgroup heuristic: absent in BuildKit `RUN` steps, present on hosts that + are themselves containers, and a false positive lints on the host. +- [ ] `script/bootstrap` installs no golangci-lint. Delete the block, its + version and ref variables, and its call site. A JS repo's `yarn install` + stays — it brings a linter along with every other dependency, which is + fine as long as no verdict is taken from it. +- [ ] The per-checkout lint state is gone: no `GOLANGCI_LINT_CACHE` or `TMPDIR` + exports, no `--allow-serial-runners`, and `.lint-cache/` removed from + `.gitignore` and `.dockerignore`. - [ ] `make check` does not modify any files in the repo - [ ] `make test` has a 30-second timeout - [ ] `make test` runs real tests, not a no-op (at minimum, import/compile @@ -104,5 +173,9 @@ with your task. # Final - [ ] `make check` passes -- [ ] `docker build` succeeds +- [ ] `make lint` runs twice on an unchanged tree with the lint layer `DONE` + both times, never `CACHED` and never sub-second +- [ ] `script/cibuild` succeeds (a bare `docker build .` or + `docker build -f Dockerfile.lint .` fails closed by design, on the + `CHECK_EPOCH` guard) - [ ] Commit and merge fixes before starting your actual task diff --git a/prompts/GO_HTTP_SERVER_CONVENTIONS.md b/prompts/GO_HTTP_SERVER_CONVENTIONS.md index 3a53b3c..06d44f0 100644 --- a/prompts/GO_HTTP_SERVER_CONVENTIONS.md +++ b/prompts/GO_HTTP_SERVER_CONVENTIONS.md @@ -1,6 +1,6 @@ --- title: Go HTTP Server Conventions -last_modified: 2026-02-22 +last_modified: 2026-08-09 --- This document defines the architectural patterns, design decisions, and @@ -991,7 +991,14 @@ func main() { Use ldflags to inject version information at build time: ```makefile -VERSION := $(shell git describe --tags --always) +# ?= rather than := because this `$(shell git describe ...)` is only correct +# on the host: `.dockerignore` excludes `.git`, so evaluated inside a build +# stage it expands to the empty string without failing and the binary reports +# no version. The version is computed on the host by `script/docker` / +# `script/cibuild` and passed with `--build-arg VERSION=...`; where the build +# stage invokes make, `ARG VERSION` puts it in the environment and `?=` defers +# to it. See the git-describe rule in REPO_POLICIES.md. +VERSION ?= $(shell git describe --tags --always) BUILDARCH := $(shell go env GOARCH) build: diff --git a/prompts/NEW_REPO_CHECKLIST.md b/prompts/NEW_REPO_CHECKLIST.md index 2eb58ea..9b8f15d 100644 --- a/prompts/NEW_REPO_CHECKLIST.md +++ b/prompts/NEW_REPO_CHECKLIST.md @@ -1,6 +1,6 @@ --- title: New Repo Checklist -last_modified: 2026-07-06 +last_modified: 2026-08-10 --- Use this checklist when creating a new repository from scratch. Follow the steps @@ -35,7 +35,11 @@ Template files can be fetched from: - [ ] `.gitignore` — fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore`, extend for - language-specific artifacts + language-specific artifacts. Extensions are written to `.gitignore`'s own + semantics, where an unanchored pattern already matches at every depth: + never add a `**/` prefix here, which is a `.dockerignore` form. The + canonical file already carries `.claude/` so agent worktrees cannot be + committed by accident. - [ ] `.editorconfig` — fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.editorconfig` - [ ] `Makefile` — fetch from @@ -52,10 +56,44 @@ Template files can be fetched from: `https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md` - [ ] `Dockerfile` and `.dockerignore` — fetch `.dockerignore` from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` - - All Dockerfiles must run `make check` as a build step + - Extend `.dockerignore` with the repo's own host-built artifacts, giving + every depth-independent pattern a `**/` prefix — but write a repo-root + binary anchored, `/myapp` and never `**/myapp`, which would also match + `cmd/myapp/` and delete the package directory. Do not transplant + `.gitignore`'s patterns: `.dockerignore` anchors an unprefixed pattern at + the context root, so the copied form leaves `config/.env` in the build + context while reading as solved. See the `.dockerignore` rule in + `REPO_POLICIES.md`. The canonical file's `.claude` entry is anchored for + the same reason as a repo-root binary; leave it that way, but note it only + covers agents running at the repo root — if this repo will run them in + subdirectories, `services/api/.claude/` is not excluded and needs its own + anchored entry. + - If the image embeds a version in a binary, the version is computed on the + host and passed with `--build-arg VERSION=...`. `ARG VERSION=dev` is + declared in the stage that compiles, and **no stage calls `git describe`** + — `.dockerignore` excludes `.git`, so it yields an empty version without + failing the build. + - All Dockerfiles must run `make check` as a build step, and every stage + containing a check-running `RUN` must declare `ARG CHECK_EPOCH` with the + `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard immediately below it — see the + `CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is + served from cache on an unchanged tree and the build reports a green it + never ran. + - Every stage that runs checks sets `ENV LINT_IN_CONTAINER=1`, so + `script/lint` runs the linter natively instead of trying to build + `Dockerfile.lint` where there is no daemon. + - Go repos: separate `lint` stage on the `golangci/golangci-lint` image, + with `COPY --from=lint /src/go.sum /dev/null` in the build stage to force + the ordering. Re-prove that ordering warm after adopting `CHECK_EPOCH`. - Server: also builds and runs the application - Non-server: brings up dev environment and runs `make check` - Image pinned by sha256 hash with version/date comment +- [ ] `Dockerfile.lint` — the lint-only image `script/lint` builds when it is + not already inside a container. Sets `ENV LINT_IN_CONTAINER=1`; same + `ARG CHECK_EPOCH` + guard + expanded-value discipline as above, with the + `ARG` **after** the dependency layer so only the lint re-runs. Base image + pinned by sha256 with a version/date comment. Copy from + `REPO_POLICIES.md`. - [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs `script/cibuild` on push — reference `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml` @@ -82,7 +120,15 @@ are thin shims calling them. Model scripts: then `install-precommit`, plus repo-specific init - [ ] `script/test` / `make test` — runs real tests, not a no-op (30-second timeout) -- [ ] `script/lint` / `make lint` — runs linter +- [ ] `script/lint` / `make lint` — runs the linter directly when + `LINT_IN_CONTAINER=1`, otherwise `epoch="$(date +%s%N)$$"` on its own line + then `docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .`. + No lint verdict may come from a host invocation. Copy from + `REPO_POLICIES.md`. Detect on `LINT_IN_CONTAINER` only — never + `/.dockerenv`, which is absent in BuildKit `RUN` steps and present on + hosts that are themselves containers. Without the nonce this exits 0 on an + unchanged tree having linted nothing; without `-f Dockerfile.lint` it + builds the main image and lints nothing at all. - [ ] `script/fmt` / `make fmt` — formats code (writes) - [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only) - [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not @@ -90,8 +136,33 @@ are thin shims calling them. Model scripts: - [ ] `script/projectname` — outputs the project name (used by `script/docker` for the image tag) - [ ] `script/docker` / `make docker` — builds Docker image, tagged via - `script/projectname` (byte-identical across repos) -- [ ] `script/cibuild` — cd to repo root, `docker build .` (what CI runs) + `script/projectname` (byte-identical across repos); carries the same three + version lines as `script/cibuild` below, and passes + `--build-arg CHECK_EPOCH="$epoch"` and `--build-arg VERSION="$version"` +- [ ] `script/cibuild` — cd to repo root, then, each on its own line: + + ```sh + epoch="$(date +%s%N)$$" + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + --build-arg VERSION="$version" \ + . + ``` + + (what CI runs). Both build args are mandatory, and both assignments must be + on their own line: a failing command substitution inside an argument does + not trip `set -e`, so the inline form degrades silently to an empty + constant. The `[ -n "$version" ]` line is a live check that fires on an + export with no `.git` and on a repo with no commits — keep it, and do not + collapse it into `|| echo unknown`, which makes it unreachable. See the + `CHECK_EPOCH` and git-describe rules in `REPO_POLICIES.md` for why each + element is load-bearing. A bare `docker build .` fails closed by design, and + so does a bare `docker build -f Dockerfile.lint .`. The image runs + `make check`, which includes lint, so `script/cibuild` needs no separate + lint step. + - [ ] `script/precommit` — called by the pre-commit hook; runs `script/check` - [ ] `script/install-precommit` — installs the pre-commit hook that runs `script/precommit` @@ -102,7 +173,11 @@ are thin shims calling them. Model scripts: # 4. Verify - [ ] `make check` passes +- [ ] `make lint` demonstrably runs the linter rather than returning a cached + build: run it twice on an unchanged tree and confirm the lint layer says + `DONE`, never `CACHED`, both times - [ ] `make docker` succeeds +- [ ] `script/cibuild` succeeds and demonstrably executes - [ ] No secrets in repo - [ ] No mutable image/package references - [ ] No unnecessary files in repo root diff --git a/prompts/REPO_POLICIES.md b/prompts/REPO_POLICIES.md index 79d2fb7..424a877 100644 --- a/prompts/REPO_POLICIES.md +++ b/prompts/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-08-07 +last_modified: 2026-08-10 --- This document covers repository structure, tooling, and workflow standards. Code @@ -60,7 +60,11 @@ style conventions are in separate documents: prerequisite since nvm requires bash. yarn is then pinned via `corepack prepare yarn@ --activate`. Never install "latest" or "lts"; always exact versions. `script/cibuild` runs the CI build: it changes to the - repo root and runs `docker build .`; the Gitea workflow calls it. Four further + repo root and runs + `docker build --build-arg CHECK_EPOCH="$epoch" --build-arg VERSION="$version" .`, + where `epoch` is a per-invocation nonce (see the `CHECK_EPOCH` rule below) and + `version` is computed on the host because `.git` is not in the build context + (see the git-describe rule below); the Gitea workflow calls it. Four further scripts are our own extensions to the standard: `script/check` runs `script/test`, `script/lint`, and `script/fmt-check`; `script/precommit` is what the git pre-commit hook runs, and it calls `script/check`; @@ -90,42 +94,292 @@ style conventions are in separate documents: reading the Makefile. - Every repo should have a `Dockerfile`. All Dockerfiles must run `make check` - as a build step so the build fails if the branch is not green. For non-server - repos, the Dockerfile should bring up a development environment and run - `make check`. For server repos, `make check` should run as an early build - stage before the final image is assembled. Dockerfiles install development - prerequisites by running `script/bootstrap` rather than duplicating installs - inline; COPY `script/` and the dependency manifests (`package.json` + - `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it so the bootstrap - layer stays cached until dependencies change. + as a build step so the build fails if the branch is not green — the one + exception being `Dockerfile.lint`, which runs `make lint` alone because that + is its entire purpose — which requires `ARG CHECK_EPOCH` and its guard in + every stage containing a check-running `RUN`, per the `CHECK_EPOCH` rule + below. Without them a Dockerfile satisfies this criterion while its check + layers are served from cache, so the build cannot fail on a branch that is not + green. + + **Every Dockerfile must also set `ENV LINT_IN_CONTAINER=1`**, above the + checks. `script/lint` builds `Dockerfile.lint` when it is not already in a + container; without the marker it would try that from inside a build step, + where there is no daemon. See the containerised-lint rule below. + + For non-server repos, the Dockerfile should bring up a development + environment and run `make check`. For server repos, `make check` should run + as an early build stage before the final image is assembled. Dockerfiles + install development prerequisites by running `script/bootstrap` rather than + duplicating installs inline; COPY `script/` and the dependency manifests + (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it + so the bootstrap layer stays cached until dependencies change. + +- **Every check-running `RUN` must be cache-busted with `CHECK_EPOCH`.** Docker + invalidates a `COPY` layer only when the copied content changes, so on an + unchanged tree the check layer is served from cache, the suite never runs, and + the build still exits 0. A sub-second `docker build` reporting success is a + cache hit, not a result. This applies to **every** file that runs checks in a + build step — `Dockerfile` and `Dockerfile.lint` alike; a `Dockerfile.lint` + without the cache-bust is a lint that never ran, reported as a pass. The + canonical form, in **every** stage containing a check-running `RUN`, placed + **after** the dependency-install layer so that layer stays cached: + + ```dockerfile + ENV LINT_IN_CONTAINER=1 + ARG CHECK_EPOCH + RUN [ -n "$CHECK_EPOCH" ] || exit 1 + RUN echo "check epoch: ${CHECK_EPOCH}" && make check + ``` + + `ENV LINT_IN_CONTAINER=1` belongs in every such stage too, and is the line + most often missed: without it `make check` reaches `script/lint`, which + tries to build `Dockerfile.lint` from inside a build step where there is no + daemon. See the containerised-lint rule below. + + and in both `script/cibuild` and `script/docker`: + + ```sh + epoch="$(date +%s%N)$$" + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + --build-arg VERSION="$version" \ + . + ``` + + `script/lint` needs the same nonce but is **not** this command: it builds a + different file with `-f Dockerfile.lint` and passes no version. Copy its + form from the containerised-lint rule below, not this block — a + `docker build` with no `-f` builds the main image and lints nothing. + + The `VERSION` lines are there for a different reason, covered by the + git-describe rule below; they are shown here so the two rules do not each + document half a command. All four `CHECK_EPOCH` elements are load-bearing; + none is optional, and each guards a failure mode that otherwise fails green: + - `ARG` is stage-scoped, so a single declaration leaves the other check + stages frozen while the fix reviews as complete. Declare it in every stage + that runs checks, immediately above the first such `RUN`. + - Expand the value into the command. This makes the cache miss contractual + rather than dependent on BuildKit's handling of an unreferenced `ARG`, and + it puts the epoch in the build log. The guard is itself value-keyed, for + the same reason: it references `$CHECK_EPOCH`, so BuildKit renders the + epoch into that layer's description (rendered as + `RUN [ -n "" ] || exit 1`) and re-runs it whenever the value + changes. Each stage therefore has two independent invalidation points, and + the guard always precedes the check `RUN`. Keep both: the expansion is + defence in depth, and it is what makes the epoch visible in the build + output. + - The `[ -n ... ]` guard is required: an unset `ARG` is empty, and empty is + a stable cache key, so without it a bare `docker build .` still produces + the false green. Failed steps are never cached, so the guard fails on + every such invocation, loudly. A bare `docker build .` failing is by + design. + - Assign `epoch=` on its own line, never inline in the `--build-arg` + argument: a failing command substitution inside an argument does not trip + `set -e`, so the inline form silently degrades to an empty constant. The + `$$` suffix is required because busybox `date` drops `%N` and exits 0, so + on an alpine host the epoch would degrade to second granularity and + concurrent invocations would collide. + + This invalidates the check layers and everything after them while leaving + `go mod download` and `script/bootstrap` cached, so it does not push against + the five-minute Docker build ceiling. Blanket `--no-cache` is not an + acceptable substitute: it also busts the dependency layer, so every run + reinstalls dependencies over the network instead of only the first and those + after a manifest change. Never reach for `docker builder prune` — the build + cache is shared with every other build on the host. + +- **Every lint run happens in a container.** `script/lint` runs the linter + directly when it is already inside one, and otherwise builds `Dockerfile.lint` + so that it is. Either way the linter never runs on a developer host, where its + answer is not trustworthy: + - **Confirmed false green.** golangci-lint keys cached results on file + **content, not location**, so a second checkout of the same commit serves + its findings. One implementer reported `0 issues` on a branch genuinely + red with a `goconst` finding. Own-clones-instead-of-worktrees does not + help; two clones are byte-identical exactly as two worktrees were. + - **False reds**: findings reported against other checkouts and against + worktrees already deleted; in one case 399 issues returned to a clean + clone that genuinely lints 0. + - **Lock contention indistinguishable from findings.** golangci-lint flocks + `$TMPDIR/golangci-lint.lock` (`pkg/commands/run.go`, `acquireFileLock()`), + host-global and independent of `GOLANGCI_LINT_CACHE`, 5-second timeout. It + prints `parallel golangci-lint is running`, analyzes nothing, exits + non-zero. Not fixed by per-cache isolation — measured. + - **Version skew**: a host linter differing from the pinned one, with the + container surfacing thirteen findings the host missed. + + A container has its own cache, its own `TMPDIR` and a binary pinned by + digest, so none of it is reachable. This supersedes the per-checkout + `GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper, which existed only to make a host + run trustworthy; delete it on adoption. + + The canonical `script/lint`, whose executable lines are the same in every + repo apart from the native lint command: + + ```sh + #!/bin/sh + # script/lint: run the linter. Inside a container, run it directly; on a + # host, build Dockerfile.lint so it runs in one anyway. + # + # LINT_IN_CONTAINER is set by this repo's Dockerfiles and is the ONLY + # accepted signal. Do not add a /.dockerenv fallback: it is absent inside + # BuildKit RUN steps and present on hosts that are themselves containers, + # so it both misses and false-positives — and a false positive silently + # restores host linting. + set -eu + + ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + + main() { + cd "$ROOT" + + if [ "${LINT_IN_CONTAINER:-}" = "1" ]; then + # config verify lives here, not in a Dockerfile, so every path + # that lints inherits it — the lint stage of the main image as + # well as Dockerfile.lint. Duplicating it into each Dockerfile + # is how one of them silently loses it. + golangci-lint config verify --config .golangci.yml + exec golangci-lint run --config .golangci.yml ./... + fi + + # Own line, and `$$` because busybox `date` drops %N silently. + # Without a fresh nonce the lint layer is cached and this exits 0 + # having linted nothing. + epoch="$(date +%s%N)$$" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + -f Dockerfile.lint \ + . + } + + main "$@" + ``` + + and `Dockerfile.lint`, the standalone path for a developer host: + + ```dockerfile + # Lint-only image, built by script/lint when not already in a container. + # golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 + FROM golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 + + WORKDIR /src + ENV LINT_IN_CONTAINER=1 + + COPY go.mod go.sum ./ + RUN go mod download + + COPY . . + + # ARG after the dependency layer so only the lint re-runs. + ARG CHECK_EPOCH + RUN [ -n "$CHECK_EPOCH" ] || exit 1 + RUN echo "lint epoch: ${CHECK_EPOCH}" && make lint + ``` + + Load-bearing properties: + - **Detection rests on `LINT_IN_CONTAINER=1` and nothing else.** Every + Dockerfile in the repo sets it; a host does not. The asymmetry is the + whole design: a **false negative** inside a container tries a nested + `docker build`, finds no daemon and fails loudly, while a **false + positive** on a host silently lints there — the exact defect this rule + exists to kill. So the signal must be one only our own images can produce. + `/.dockerenv` is not such a signal and must not be used, even as a + fallback: measured, it is **absent** inside BuildKit `RUN` steps and + **present** on any host that is itself a container, which is the common + case for CI runners and agent sandboxes. It fails in both directions, and + one of them is the dangerous one. + - **`CHECK_EPOCH`, not `--no-cache`.** `docker build -f Dockerfile.lint .` + on an unchanged tree returns a sub-second cached success having linted + nothing. The `ARG` goes **after** the dependency layer so only the lint + re-runs; `--no-cache` would also reinstall dependencies on every lint. + - **Non-Go repos get the same pattern around their own linter** — `eslint`, + `ruff`, `prettier`, `shellcheck`. Only the base image and the native lint + command change. + - **Keep `golangci-lint config verify`, put it in `script/lint`, and it + costs no network.** It goes in the native branch, not in a Dockerfile, so + the lint stage of the main image inherits it along with `Dockerfile.lint`; + putting it in one Dockerfile leaves the other path unverified. The two + commands catch disjoint classes, measured under the pinned v2.12.2: a + bogus top-level key and a bogus key under `linters.settings.lll` both pass + `golangci-lint run` with **exit 0 and `0 issues`** while `config verify` + exits 3 and names them; an invalid value type fails both; an unknown + linter name fails `run` and passes `config verify`. So `run` alone + silently ignores an unknown key — the mode where a threshold reads as + configured and is not applied. It needs no network: every case reproduced + byte-identically under `docker run --network none`, in a container where + `getent hosts golangci-lint.run` exits 2. The schema is embedded in the + pinned binary. Re-run that control when bumping the pin. + - **A failed `script/lint` that names no finding is not a lint result.** On + the host path `docker build` exits 1 both for findings and for a build + that never got there (daemon down, image unpullable, disk full). BuildKit + names the failing step; read it, fix the environment, re-run. Do not + record a verdict from a run that did not lint. + + **Scope: this rule is about linters, and a formatter is not one.** + `script/fmt` writes your working tree, so it can only run on the host, and + `script/fmt-check` is its read-only twin. In a repo whose formatter **is** + its linter (prettier over markdown; this repo), `script/bootstrap` therefore + installs the linter on the host as an ordinary dependency and + `script/fmt-check` runs it there. That is accepted: the version is pinned in + `package.json` and installed into the repo's own `node_modules`, so there is + no shared content-keyed cache, no host-global lock and nothing to skew + against. What is forbidden is taking a **lint verdict** from it — + `script/lint` stays the only source of one. A repo auditing itself will see + those hits and should leave them; anything else the grep finds is a real + second path to the linter and goes. + + **What a consuming repo does to adopt this**, in order: + 1. Add `Dockerfile.lint`. + 2. Replace `script/lint` with the form above, with its own native lint + command. + 3. Add `ENV LINT_IN_CONTAINER=1` to **every** stage of every Dockerfile that + runs checks — the lint stage and the build stage both. + 4. Delete any golangci-lint install from `script/bootstrap`, with its + version and ref variables and its call site. No lint verdict comes from + the host any more, so it can only reintroduce version skew. A JS repo's + `yarn install` stays. + 5. Delete the per-checkout lint state: `GOLANGCI_LINT_CACHE` and `TMPDIR` + exports, `--allow-serial-runners`, the retry/VOID wrapper, and + `.lint-cache/` from both `.gitignore` and `.dockerignore`. + 6. Verify by running `make lint` twice on an unchanged tree: the lint layer + must be `DONE` both times, never `CACHED`. Then plant a violation, + confirm it fails naming the finding, revert. A bare + `docker build -f Dockerfile.lint .` must fail on the guard. + + `script/check`, `script/cibuild`, `script/docker` and the `Dockerfile` are + unchanged by this: `make check` still runs inside the image, and + `script/lint` there takes the native path. - **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go repos use a multistage build where linting runs in an independent stage based - on the `golangci/golangci-lint` image (pinned by hash). This stage runs - `make fmt-check` and `make lint` before the full build begins. The build stage - then declares an explicit dependency on the lint stage via - `COPY --from=lint /src/go.sum /dev/null`, which forces BuildKit to complete - linting before proceeding to compilation and tests. This ensures lint failures - surface in seconds rather than minutes, without blocking on dependency - download or compilation in the build stage. - - The standard pattern for a Go repo Dockerfile is: + on the `golangci/golangci-lint` image (pinned by hash), so lint failures + surface in seconds rather than after a full compile. The build stage declares + an explicit dependency on it via `COPY --from=lint /src/go.sum /dev/null`, + which forces BuildKit — which runs stages in parallel by default — to finish + linting first. The canonical Go repo `Dockerfile`: ```dockerfile # Lint stage — fast feedback on formatting and lint issues # golangci/golangci-lint:v2.x.x, YYYY-MM-DD FROM golangci/golangci-lint@sha256:... AS lint WORKDIR /src + ENV LINT_IN_CONTAINER=1 COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make fmt-check + ARG CHECK_EPOCH + RUN [ -n "$CHECK_EPOCH" ] || exit 1 + RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check RUN make lint # Build stage # golang:1.x-alpine, YYYY-MM-DD FROM golang@sha256:... AS builder WORKDIR /src + ENV LINT_IN_CONTAINER=1 # Force BuildKit to run the lint stage before proceeding COPY --from=lint /src/go.sum /dev/null @@ -133,8 +387,13 @@ style conventions are in separate documents: COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make test + ARG CHECK_EPOCH + RUN [ -n "$CHECK_EPOCH" ] || exit 1 + RUN echo "check epoch: ${CHECK_EPOCH}" && make test + # VERSION comes from the host via --build-arg; see the git-describe rule + # below. Never run `git describe` here: .dockerignore excludes .git, so + # it yields an empty version without failing the build. ARG VERSION=dev RUN CGO_ENABLED=0 go build -trimpath \ -ldflags="-s -w -X main.Version=${VERSION}" \ @@ -147,29 +406,48 @@ style conventions are in separate documents: ``` Key points: - - The lint stage uses the `golangci/golangci-lint` image directly (it - includes both Go and the linter), so there is no need to install the - linter separately. - - `COPY --from=lint /src/go.sum /dev/null` is a no-op file copy that creates - a stage dependency. BuildKit runs stages in parallel by default; without - this line, the build stage would not wait for lint to finish and a lint - failure might not fail the overall build. - - If the project uses `//go:embed` directives that reference build artifacts - (e.g. a web frontend compiled in a separate stage), the lint stage must - create placeholder files so the embed directives resolve. Example: - `RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`. - The lint stage should not depend on the actual build output — it exists to - fail fast. - - If the project requires CGO or system libraries for linting (e.g. - `vips-dev`), install them in the lint stage with `apk add`. - - The build stage runs `make test` after compilation setup. Tests run in the - build stage, not the lint stage, because they may require compiled + - The lint stage uses the `golangci/golangci-lint` image directly (it has + both Go and the linter), so nothing needs installing. `make lint` there + runs `script/lint`, which sees `LINT_IN_CONTAINER=1` and invokes + `golangci-lint` natively instead of building `Dockerfile.lint`. Without + that `ENV` the stage would attempt a nested build and fail. + - `COPY --from=lint /src/go.sum /dev/null` is a no-op copy that exists only + to create the stage dependency; without it a lint failure might not fail + the overall build. + - **Re-prove that ordering on a warm cache after adopting `CHECK_EPOCH`.** + The cache-bust turns the no-op `COPY` into a content-cache hit, so an + ordering guarantee established cold does not automatically carry over. It + was re-proved in another org repo using the same trick and held, but that + result does not transfer by assumption — re-check it warm. + - If the project uses `//go:embed` referencing build artifacts, the lint + stage must create placeholders so the directives resolve: + `RUN mkdir -p web/dist && touch web/dist/index.html`. + - If linting needs CGO or system libraries (e.g. `vips-dev`), `apk add` them + in the lint stage. + - Tests run in the build stage, not the lint stage: they may need compiled artifacts or heavier dependencies. + - `ARG CHECK_EPOCH` appears in **both** stages, because `ARG` is + stage-scoped: declaring it only in the lint stage leaves `make test` + frozen at its last cached result. In each stage the guard sits immediately + below the `ARG` and the value is expanded into the first check `RUN`. + Later `RUN`s in the same stage need no expansion; their parent layer is + already busted. + - `ARG VERSION=dev` is declared in the build stage and supplied by + `script/docker` and `script/cibuild`. **No stage may call + `git describe`**: `.dockerignore` excludes `.git`, so it yields an empty + version without failing. See the git-describe rule further down. - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that - runs `script/cibuild` (which runs `docker build .`) on push. Since the - Dockerfile already runs `make check`, a successful build implies all checks - pass. + runs `script/cibuild` (which runs + `docker build --build-arg CHECK_EPOCH="$epoch" --build-arg VERSION="$version" .`) + on push. The Dockerfile runs `make check`, so a successful build implies all + checks pass — but that implication holds **only** because of the `CHECK_EPOCH` + cache-bust described above. Without it, an unchanged tree serves the check + layer from cache and the build reports a green it never earned. A bare + `docker build .` fails closed by design, on the `[ -n "$CHECK_EPOCH" ]` guard; + always go through `script/cibuild` or `script/docker`. Never accept a pass as + evidence without confirming it ran: a sub-second wall time, or `CACHED` on the + check layer, means nothing was executed. - Use platform-standard formatters: `black` for Python, `prettier` for JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with @@ -239,10 +517,140 @@ style conventions are in separate documents: must be in `.gitignore`. No exceptions. - `.gitignore` should be comprehensive from the start: OS files (`.DS_Store`), - editor files (`.swp`, `*~`), language build artifacts, and `node_modules/`. - Fetch the standard `.gitignore` from + editor files (`.swp`, `*~`), in-repo agent scratch directories (`.claude/`, + which holds one worktree — an entire additional checkout of the repo — per + in-flight agent), language build artifacts, and `node_modules/`. Fetch the + standard `.gitignore` from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when setting up - a new repo. + a new repo. These patterns are written to `.gitignore`'s own semantics, in + which an unanchored pattern already matches at every depth. They are not a + `.dockerignore` and must not be transplanted into one unmodified — see the + next rule. + +- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns + across unmodified leaves secrets in the build context.** Docker matches with + `moby/patternmatcher`: Go `filepath.Match` semantics plus a `**` extension, + compiled to a regexp — plain `filepath.Match` has no `**` at all. So `*` does + not cross `/`, and a pattern without a leading `**/` is anchored at the + build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key` + therefore excludes only the copies at the repository root; `config/.env` and + `certs/server.key` still reach the context and can land in an image layer. + That file is more dangerous than a short one with no secret patterns at all, + because it reads as solved and stops anyone looking. Give every + depth-independent pattern the `**/` prefix — `**/node_modules`, + `**/.DS_Store`, and the secret patterns in the canonical file, which are + additionally case-folded per the rule below — and leave only genuinely + root-anchored entries unprefixed: `.git`, the in-repo agent scratch directory + `.claude`, and the repo's own host-built binary. The inverse move is equally + wrong: never apply `**/` to `.gitignore`, where it is redundant and produces a + file that is wrong in a way that looks careful. Each file is written to its + own semantics; neither is derived from the other. Fetch the standard + `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend + it with the repo's own host-built artifacts — a host `make build` that leaves + a compiled binary in the repo root puts that binary in the build context, + where `.gitignore` hides it from every git-based check. Write that binary + anchored, `/myapp` and never `**/myapp`: the prefixed form also matches + `cmd/myapp/` and deletes the package directory from the context. + +- **In-repo agent scratch belongs in both files, written to each file's own + semantics.** `.claude/` holds one worktree per in-flight agent — an entire + additional checkout of the repo — so with `COPY . .` the build context + inflates by a multiple of the repo, and another session's unreviewed, + sometimes uncommitted work can be copied into an image layer. The directory is + also created and destroyed constantly, so it invalidates `COPY . .` for + reasons that have nothing to do with the repo's own content. In `.gitignore` + the entry is `.claude/`, unanchored, which already matches at every depth. In + `.dockerignore` it is `.claude`, anchored and with **no** `**/` prefix: the + directory occurs exactly once **where agents run at the repo root**, and the + prefixed form would also match any nested directory of that name and delete it + from the build. It is not case-folded the way the secret patterns are, because + tooling creates it in exactly one spelling, so a folded pattern would add no + coverage. + + **Known gap that comes with the anchored form.** The directory is created in + the agent's working directory, so the "exactly once, at the root" premise is + a property of how agents are run and not of the tooling. Where agents run in + subdirectories — a monorepo with a per-service agent is the ordinary case — + `services/api/.claude/` is **not** excluded by the canonical entry and still + reaches the build context and the image, which is the exposure the entry + exists to close. A repo in that shape adds its own anchored entries + (`/services/api/.claude`), or `**/.claude` once it has confirmed no + legitimately named nested directory would be caught. This is stated in the + canonical `.dockerignore` itself, since that file is what consuming repos + receive. + +- **`.dockerignore` matching is case-sensitive, so cover capitalisation with + character classes rather than by doubling patterns.** `**/*.key` does not + match `certs/SERVER.KEY`, which is reachable on the case-insensitive + filesystems most laptops use. Adding an ALL-CAPS twin for each pattern is not + the fix: it still misses `Server.Key` and `Ca.Pem` while reading as though + case were handled — the same manufactured confidence as the root-anchored + form. The matcher supports character ranges, so one line covers every + spelling: `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`. Apply this to every secret + name, not only to extensions: the extensionless SSH keys and `.envrc` need it + for the same reason, since on the very filesystems that make `SERVER.KEY` + reachable, direnv reads `.ENVRC` and ssh reads `ID_RSA`. Note that `*` matches + the empty string, so `**/*.[eE][nN][vV]` already covers a bare `.ENV` and no + separate literal `.env` entry is needed. + +- **A pattern that also catches something the build needs is re-included with a + negation, not deleted.** The canonical `**/*.[eE][nN][vV]` excludes a + committed env template such as `example.env`; a repo whose build genuinely + reads one adds `!docs/example.env` after the pattern. Deleting the pattern + instead reopens the exposure for every other file it covers. + +- **Verify `.dockerignore` by enumerating the image, not by reading the + patterns.** Plant files at the root _and_ at least two directories deep, build + a probe image that does `COPY . .`, and list what actually landed + (`docker run --rm --entrypoint find IMAGE /app`). Reading the patterns and + agreeing they look right is exactly what lets the root-only form through. The + `transferring context` size is not a substitute: a nested secret is a few + bytes, and BuildKit transfers only the delta from the previous build, so the + reported size describes the transfer and not the contents of the image. + +- **Excluding `.git` means `git describe` cannot run inside any build stage, and + it fails quietly there.** The `GOLDFLAGS` version-embedding pattern assumes + `.git` is present; in a build stage there is no repository, so `git describe` + writes nothing to stdout and the `-X main.Version=` value comes out **empty** + rather than erroring. The binary then reports no version at all and the build + still exits 0. Compute the version **on the host** and thread it in as a build + arg. `script/docker` and `script/cibuild` do this, byte-identically across + repos: + + ```sh + # Assign on its own line: a failing command substitution inside an + # argument does not trip `set -e`, so the inline form degrades to an + # empty constant — the same silent-empty failure this rule is about. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + --build-arg VERSION="$version" \ + . + ``` + + `--always` makes an untagged repo yield the abbreviated commit hash instead + of failing. `|| true` keeps a failing `git describe` from tripping `set -e` + and leaves the value empty, so the `[ -n "$version" ]` line is the single + place the fallback is applied — and it is a **live** check, not defence in + depth: it fires on a build from an export with no `.git`, and on a + repository with no commits yet. Do not fold the fallback into the + substitution as `|| echo unknown`; that makes the guard unreachable, and a + guard that cannot fire is indistinguishable from one that works to everyone + who copies it. The result is non-empty by construction either way, which is + the point: an empty version reads as a successful one, while `unknown` is + visibly wrong. The Dockerfile's side is `ARG VERSION=dev` in the stage that + compiles, declared there and not inherited, because `ARG` is stage-scoped + exactly as `CHECK_EPOCH` is. Passing `VERSION` to a repo whose Dockerfile + declares no such `ARG` is silently ignored by BuildKit and costs nothing, + which is why the scripts stay byte-identical rather than growing a per-repo + variant. + + One consequence for CI: the standard checkout action clones shallow and + fetches no tags, so `git describe --tags` there falls back to a bare commit + hash. A repo that embeds a tag-derived version must set `fetch-depth: 0` on + its checkout step; a repo that does not embed a version needs no change. - **No build artifacts in version control.** Code-derived data (compiled bundles, minified output, generated assets) must never be committed to the @@ -261,9 +669,111 @@ style conventions are in separate documents: - `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only manually by the user. Fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. The - canonical golangci-lint version is v2.12.2 (released 2026-05-06), installed - commit-pinned via - `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. + canonical golangci-lint version is v2.12.2 (released 2026-05-06), pinned as + the image digest in `Dockerfile.lint` + (`golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`, + which reports + `golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9`). That + digest is the only pin: golangci-lint is not installed on the host by any + repo. Bumping the version means changing that one digest. + +- **`script/bootstrap` must not install golangci-lint.** This supersedes the + pinned host install that used to be canonical here. `script/lint` never runs + it on the host — it either builds `Dockerfile.lint` or is already in a + container that ships the binary — so a host install has no caller, and its + only remaining effect is to put a second, independently-versioned linter where + somebody eventually runs it by hand and believes the result. Delete the block, + its version and ref variables, and its call site. + + This is not a ban on host dependency installs generally. A JS or docs repo's + `script/bootstrap` runs `yarn install`, which brings its linter along with + every other dependency; that is unavoidable and fine. The rule is about a + **dedicated** linter install, and about where a verdict may come from. + + **The version-enforcement principle it established still applies to any + other tool a repo pins and installs on the host**, and it is the part worth + keeping, because each of its four properties guards a failure that otherwise + reports success: + - **Compare the installed version against the pin, never test presence.** A + `if missing ; then install; fi` guard tests `PATH` presence and + never version, so on any already-provisioned machine the pin is inert and + a version bump is a silent no-op. Compare the **whole** version token, + exactly: a parser that stops at the first `-` reports `2.12.2` for a host + running `2.12.2-rc1` and skips the install — the original defect, + reintroduced through the comparison meant to fix it. + - **After installing, re-resolve the binary the way callers resolve it** — + through `PATH`, not the directory the installer wrote to — and assert the + reported version is the pin. An installer that writes to `GOBIN` while a + different binary shadows it earlier in `PATH` genuinely succeeds and + changes nothing any caller sees, which is worse than no fix: it converts a + known-stale tool into one everyone believes is pinned. Run `hash -r` first + so the shell does not answer from its own lookup cache, and when the + assertion fails, name the path `command -v` found, the version it reports, + and the directory the install wrote to. Diagnose from the resolved path + rather than asserting a cause: only a path **outside** the install + directory is shadowing. + - **A mis-parse must fall through to reinstall, never to a false match.** + Absent binary, non-zero exit, empty output and unrecognised output should + all yield an empty string, which compares unequal to the pin. The failure + direction is always a redundant install, never a skipped one. + - **Call it, and say so on success.** A function defined and never called is + a silent no-op indistinguishable from success: exit 0, nothing installed, + no output. Both success branches must print a line naming the version. + + Verifying such logic requires a negative control in an environment where a + shadowing binary exists earlier in `PATH` than the install target — without + it the control passes against the naive compare-then-install form too and + proves nothing — plus a mis-parse control that feeds unparseable `--version` + output and confirms a reinstall. Run those controls against the block as a + consuming repo would adopt it: pasted into a `script/bootstrap`-shaped file + that is then executed, never by sourcing it and invoking the function + yourself. Driving the function directly tests something the artifact does + not do, and it is exactly how a missing call site passes every control while + the adopted snippet does nothing. + + Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`. + +- **Superseded: the per-checkout `GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper for + `script/lint`.** It existed only to make a host lint run trustworthy, and the + containerised-lint rule above removes the host run. Delete the wrapper, the + `--allow-serial-runners` flag, and `.lint-cache/` from both `.gitignore` and + `.dockerignore`. Two of its conclusions outlive it: **`GOCACHE` does not need + isolating** (measured — content-addressed, no foreign paths in its entries, no + global lock), and **verifying lint plumbing requires paired controls** run + against the artifact as a consuming repo would adopt it, since a control that + passes against the broken form proves nothing. + +- **Interim rule for reading a lint result produced on the host, in a repo that + has not yet adopted the containerised lint above.** A lint run is **VOID** + unless both hold: + - the output contains no `parallel golangci-lint is running`, and + - no reported file path begins with `../`, and none is an absolute path + outside the tree the run was launched from. + + Do not record a verdict from a void run, and do not "fix" findings in files + the change does not touch — chasing phantom findings across untouched files + puts unrelated edits into a reviewed diff, which is more expensive than the + wasted rework. + + The `../` clause is the one that actually bites, and it is why a filter + keyed on `/tmp` or on absolute prefixes is not enough: golangci-lint reports + paths relative to its own resolved root rather than yours, and three of the + org's reported sightings had relative paths and would have passed such a + filter. Both clauses are needed and neither alone is sufficient — one + reproduction exited non-zero with the lock error and no foreign paths at + all, and another reported 34 well-formed findings, every one of them against + another checkout. + + **State the limit of these tests rather than treating them as a guarantee.** + They catch contamination that **names** foreign files. They cannot catch + contamination that **suppresses** findings through a poisoned entry for + colliding content, which has no wall-clock tell either — **no evidence of + that mode has been observed, and nobody should go chasing it**; the point is + the reach of the tests, not a claim that the mode exists. They are a filter + for the loud mode, not a proof of soundness — which is the whole argument + for containerising the linter instead of documenting a discipline that + depends on every agent remembering to apply it. Adopt the rule above and + this one stops applying to the repo entirely. - When pinning images or packages by hash, add a comment above the reference with the version and date (YYYY-MM-DD). diff --git a/script/cibuild b/script/cibuild index 75cc3e6..c75ed5a 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,13 +1,28 @@ #!/bin/sh -# script/cibuild: run the CI build. The Dockerfile runs script/check, so -# a successful build implies all checks pass. +# script/cibuild: run the CI build. The Dockerfile runs script/check, but +# that only proves anything because CHECK_EPOCH is a fresh nonce on every +# invocation: without it Docker serves the check layer from cache and the +# build exits 0 without running the suite. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - docker build . + # Both assignments on their own line: a failing command substitution + # inside an argument does not trip `set -e`, so the inline form + # degrades silently to an empty constant. `$$` because busybox `date` + # drops %N without erroring. VERSION is computed here because + # .dockerignore excludes .git, so `git describe` in a build stage + # yields an empty version without failing; the guard below is the + # single place the fallback is applied. + epoch="$(date +%s%N)$$" + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + --build-arg VERSION="$version" \ + . } main "$@" diff --git a/script/docker b/script/docker index 9b9ea86..e52f2e1 100755 --- a/script/docker +++ b/script/docker @@ -1,6 +1,9 @@ #!/bin/sh # script/docker: build the Docker image tagged with the project name. -# Identical in all repos; the tag comes from script/projectname. +# Identical in all repos; the tag comes from script/projectname. The +# Dockerfile's checks only actually run because CHECK_EPOCH is a fresh +# nonce on every invocation; without it a warm cache turns this into a +# green that proves nothing. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -8,7 +11,19 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - docker build -t "$("$SCRIPT_DIR/projectname")" . + # Both assignments on their own line: a failing command substitution + # inside an argument does not trip `set -e`, so the inline form + # degrades silently to an empty constant. `$$` because busybox `date` + # drops %N without erroring. VERSION is computed here because + # .dockerignore excludes .git, so `git describe` in a build stage + # yields an empty version without failing. + epoch="$(date +%s%N)$$" + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . } main "$@" diff --git a/script/lint b/script/lint index c489634..b6d3db8 100755 --- a/script/lint +++ b/script/lint @@ -1,13 +1,34 @@ #!/bin/sh -# script/lint: run the linter. +# script/lint: run the linter. Inside a container, run it directly; +# on a host, build Dockerfile.lint so it runs in one anyway. The linter +# is never run on a developer host, where a shared result cache, a +# host-global lock and a stale toolchain make its answer untrustworthy. +# +# LINT_IN_CONTAINER is set by this repo's Dockerfiles and is the ONLY +# accepted signal. Do not add a /.dockerenv fallback: it is absent +# inside BuildKit RUN steps and present on hosts that are themselves +# containers, so it both misses and false-positives — and a false +# positive silently restores host linting. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - echo "Linting markdown files..." - yarn run prettier --check '**/*.md' --tab-width 4 --prose-wrap always + + if [ "${LINT_IN_CONTAINER:-}" = "1" ]; then + exec yarn run prettier --check '**/*.md' \ + --tab-width 4 --prose-wrap always + fi + + # Own line, and `$$` because busybox `date` drops %N silently. + # Without a fresh nonce the lint layer is cached and this exits 0 + # having linted nothing. + epoch="$(date +%s%N)$$" + docker build \ + --build-arg CHECK_EPOCH="$epoch" \ + -f Dockerfile.lint \ + . } main "$@"