sneak/dnswatcher
1
0
Fork 0
Code Issues 37 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

script/bootstrap installs pinned tools only when missing, so the golangci-lint pin is inert on any machine that already has one #117

New Issue
Open
opened 2026-08-09 07:39:18 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-09 07:39:18 +02:00
Collaborator
Copy Link

script/bootstrap guards its pinned go install calls behind a presence check:

missing() {
    ! command -v "$1" >/dev/null 2>&1
}
...
if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi
if missing goimports; then go install "$GOIMPORTS_REF"; fi

missing tests only whether the binary is on PATH, not whether it is the pinned version. So on any machine that already has some golangci-lint — any version, from any source — bootstrap silently skips the install and the pin does nothing.

Why this matters here specifically

This repo moved to golangci-lint v2.12.2 (commit pin c0d3ddc9cf3faa61a4e378e879ece580256d76e5) with an org-standard v2-schema .golangci.yml. A v1.x linter cannot parse that config at all, and a different v2.x will not necessarily produce the same findings.

So the failure modes are:

  • A developer or agent with a pre-existing v1.x linter runs make lint and gets a config schema error that looks like a broken repo rather than a stale tool.
  • Worse: someone with a different v2.x gets lint results that silently disagree with CI. make check passes locally, then fails in the Docker build — or, more insidiously, passes in both while a finding CI would have caught is never surfaced.
  • A future version bump is inert. Changing GOLANGCI_LINT_REF in this file updates the Dockerfile-matching comment and the pin, and changes nothing for anyone who has already bootstrapped. The pin exists precisely so local and CI linting agree; this guard defeats that.

The Dockerfile is unaffected — it runs go install unconditionally in a clean image, which is why local and CI can diverge without anyone noticing.

The comment at the top of the file states the intent the code fails to deliver: "golangci-lint and goimports are installed via go install at the same pinned commits the Dockerfile uses (never 'latest')."

Definition of done

  1. script/bootstrap guarantees the pinned versions are what end up installed, regardless of what was on PATH beforehand. go install at a pinned ref is idempotent and cheap when the module cache is warm, so the simplest correct fix is to drop the missing guard for these two tools and always run the install. If you prefer to keep a guard, it must compare the installed version against the pin, not merely test for presence — and a version check that can be fooled by a differently-built binary is not good enough.
  2. The missing guard stays for git, make, and go, which come from the system package manager and are genuinely presence-checks. Do not remove those.
  3. script/bootstrap remains POSIX sh (#!/bin/sh, set -eu, no bashisms) — it runs in minimal alpine containers with no bash — and keeps the $(cd "$(dirname "$0")/.." && pwd -P) root-location idiom.
  4. It stays idempotent: running it twice in a row must succeed both times and leave the same result.
  5. The header comment is corrected if the mechanism changes.
  6. Verify empirically, with a negative control. Install a deliberately wrong golangci-lint version so it is on PATH, run script/bootstrap, and confirm the pinned version is what golangci-lint --version reports afterwards. A code-reading argument is not sufficient — this is a bug about the gap between what the script says and what it does.
  7. make check green; TODO.md updated in the same commit.

The finishing commit's title must end with (closes #N) referencing this issue.

Hard constraints

  • Do not change the pins themselves. GOLANGCI_LINT_REF must stay c0d3ddc9cf3faa61a4e378e879ece580256d76e5 (v2.12.2) and GOIMPORTS_REF must stay 009367f5c17a8d4c45a961a3a509277190a9a6f0. This issue is about making the existing pins effective, not about updating them.
  • Do not modify .golangci.yml — it is org-standardised and must never be touched by an agent. Its sha256 must remain 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.
  • Keep the pins in script/bootstrap and the Dockerfile identical to each other. If you touch one, check the other.
  • Do not introduce a network fetch that is not hash-pinned. go install at a commit ref is fine (the hash is recorded in the module system); curl | sh is never acceptable in this repo.

Context

Found via cross-repo review — this is a defect in the shared Scripts to Rule Them All template, identified independently by managers on sibling repos, and is now tracked upstream in the prompts repo alongside the script/cibuild cache hole (#115) and two other template defects. Fixing it here does not depend on the upstream fix landing, and the two should be kept consistent.

`script/bootstrap` guards its pinned `go install` calls behind a presence check: ```sh missing() { ! command -v "$1" >/dev/null 2>&1 } ... if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi if missing goimports; then go install "$GOIMPORTS_REF"; fi ``` `missing` tests only whether the binary is **on `PATH`**, not whether it is the pinned *version*. So on any machine that already has some `golangci-lint` — any version, from any source — bootstrap silently skips the install and **the pin does nothing.** ## Why this matters here specifically This repo moved to golangci-lint **v2.12.2** (commit pin `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`) with an org-standard **v2-schema** `.golangci.yml`. A v1.x linter cannot parse that config at all, and a different v2.x will not necessarily produce the same findings. So the failure modes are: - A developer or agent with a pre-existing v1.x linter runs `make lint` and gets a config schema error that looks like a broken repo rather than a stale tool. - Worse: someone with a *different v2.x* gets lint results that silently disagree with CI. `make check` passes locally, then fails in the Docker build — or, more insidiously, passes in both while a finding CI would have caught is never surfaced. - **A future version bump is inert.** Changing `GOLANGCI_LINT_REF` in this file updates the Dockerfile-matching comment and the pin, and changes nothing for anyone who has already bootstrapped. The pin exists precisely so local and CI linting agree; this guard defeats that. The `Dockerfile` is unaffected — it runs `go install` unconditionally in a clean image, which is why local and CI can diverge without anyone noticing. The comment at the top of the file states the intent the code fails to deliver: "golangci-lint and goimports are installed via `go install` at the same pinned commits the Dockerfile uses (never 'latest')." ## Definition of done 1. `script/bootstrap` guarantees the pinned versions are what end up installed, regardless of what was on `PATH` beforehand. `go install` at a pinned ref is idempotent and cheap when the module cache is warm, so **the simplest correct fix is to drop the `missing` guard for these two tools and always run the install.** If you prefer to keep a guard, it must compare the *installed version* against the pin, not merely test for presence — and a version check that can be fooled by a differently-built binary is not good enough. 2. The `missing` guard stays for `git`, `make`, and `go`, which come from the system package manager and are genuinely presence-checks. Do not remove those. 3. `script/bootstrap` remains POSIX `sh` (`#!/bin/sh`, `set -eu`, no bashisms) — it runs in minimal alpine containers with no bash — and keeps the `$(cd "$(dirname "$0")/.." && pwd -P)` root-location idiom. 4. It stays idempotent: running it twice in a row must succeed both times and leave the same result. 5. The header comment is corrected if the mechanism changes. 6. **Verify empirically, with a negative control.** Install a deliberately wrong golangci-lint version so it is on `PATH`, run `script/bootstrap`, and confirm the pinned version is what `golangci-lint --version` reports afterwards. A code-reading argument is not sufficient — this is a bug about the gap between what the script says and what it does. 7. `make check` green; `TODO.md` updated in the same commit. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Hard constraints - **Do not change the pins themselves.** `GOLANGCI_LINT_REF` must stay `c0d3ddc9cf3faa61a4e378e879ece580256d76e5` (v2.12.2) and `GOIMPORTS_REF` must stay `009367f5c17a8d4c45a961a3a509277190a9a6f0`. This issue is about making the existing pins effective, not about updating them. - **Do not modify `.golangci.yml`** — it is org-standardised and must never be touched by an agent. Its sha256 must remain `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. - Keep the pins in `script/bootstrap` and the `Dockerfile` identical to each other. If you touch one, check the other. - Do not introduce a network fetch that is not hash-pinned. `go install` at a commit ref is fine (the hash is recorded in the module system); `curl | sh` is never acceptable in this repo. ## Context Found via cross-repo review — this is a defect in the shared Scripts to Rule Them All template, identified independently by managers on sibling repos, and is now tracked upstream in the `prompts` repo alongside the `script/cibuild` cache hole (#115) and two other template defects. Fixing it here does not depend on the upstream fix landing, and the two should be kept consistent.
clawbot added this to the 1.0 milestone 2026-08-09 07:39:18 +02:00
clawbot commented 2026-08-09 16:55:09 +02:00
Author
Collaborator
Copy Link

Implemented in #131 (branch fix/117-bootstrap-pin, commit db933f3).

What changed in script/bootstrap

  • golangci-lint and goimports are installed unconditionally at their pinned commit refs; the missing guard is gone for those two. go install at a fixed ref is idempotent and cheap with a warm module cache.
  • missing is retained for git, make, and go, per DoD item 2.
  • Added a non-fatal warning when command -v resolves either tool to something other than the directory go install wrote to. Installing the pin is necessary but not sufficient — a shadowing copy earlier on PATH is what make lint/make fmt would actually execute. It warns rather than fails because the remedy is the operator's PATH.
  • Header comment corrected; it previously claimed every install was guarded.
  • Still #!/bin/sh + set -eu, no bashisms, sh -n clean, $(cd "$(dirname "$0")/.." && pwd -P) idiom kept.

Pins unchanged and still identical to the Dockerfile. .golangci.yml untouched, sha256 still 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.

Empirical verification with a negative control (DoD item 6). Done in an isolated GOBIN first on PATH, so no shared tooling on this host was disturbed. golangci-lint --version invoked directly on purpose — it is the measurement.

  1. Installed golangci-lint v2.10.1 (and goimports v0.35.0) into the scratch GOBIN. golangci-lint --version reported 2.10.1.
  2. Ran script/bootstrap against that GOBIN/PATH.
  3. Afterwards golangci-lint --version reported 2.12.2, resolving to the scratch GOBIN copy; its sha256 6a8bfa407ebb902a291559f4a8a0dca6421ac96410c4722e32e4c96ea95d51d7 is byte-identical to the host's existing pinned v2.12.2 build. On main this step is a no-op and the 2.10.1 binary survives — that is the bug.

Idempotency (DoD item 4): second consecutive run exited 0 with the same versions and the same binary hashes.

Shadow warning: with an empty GOBIN and PATH still resolving both tools elsewhere, the warning fired for both and the script exited 0.

make check green (exit 0, 0 issues.), run with an isolated GOLANGCI_LINT_CACHE/TMPDIR per #121; output contained no parallel golangci-lint is running and no paths outside the worktree. TODO.md updated in the same commit.

Incidental, not fixed here: running script/bootstrap also rewrites go.mod, dropping golang.org/x/sync v0.19.0 // indirect, because go mod download with no arguments updates the main module's requirements. Pre-existing on main and outside this issue's scope; reverted rather than carried in the commit.

Implemented in https://git.eeqj.de/sneak/dnswatcher/pulls/131 (branch `fix/117-bootstrap-pin`, commit `db933f3`). **What changed in `script/bootstrap`** - `golangci-lint` and `goimports` are installed unconditionally at their pinned commit refs; the `missing` guard is gone for those two. `go install` at a fixed ref is idempotent and cheap with a warm module cache. - `missing` is retained for `git`, `make`, and `go`, per DoD item 2. - Added a non-fatal warning when `command -v` resolves either tool to something other than the directory `go install` wrote to. Installing the pin is necessary but not sufficient — a shadowing copy earlier on `PATH` is what `make lint`/`make fmt` would actually execute. It warns rather than fails because the remedy is the operator's `PATH`. - Header comment corrected; it previously claimed every install was guarded. - Still `#!/bin/sh` + `set -eu`, no bashisms, `sh -n` clean, `$(cd "$(dirname "$0")/.." && pwd -P)` idiom kept. Pins unchanged and still identical to the `Dockerfile`. `.golangci.yml` untouched, sha256 still `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. **Empirical verification with a negative control** (DoD item 6). Done in an isolated `GOBIN` first on `PATH`, so no shared tooling on this host was disturbed. `golangci-lint --version` invoked directly on purpose — it is the measurement. 1. Installed golangci-lint **v2.10.1** (and goimports v0.35.0) into the scratch `GOBIN`. `golangci-lint --version` reported `2.10.1`. 2. Ran `script/bootstrap` against that `GOBIN`/`PATH`. 3. Afterwards `golangci-lint --version` reported **`2.12.2`**, resolving to the scratch `GOBIN` copy; its sha256 `6a8bfa407ebb902a291559f4a8a0dca6421ac96410c4722e32e4c96ea95d51d7` is byte-identical to the host's existing pinned v2.12.2 build. On `main` this step is a no-op and the 2.10.1 binary survives — that is the bug. Idempotency (DoD item 4): second consecutive run exited 0 with the same versions and the same binary hashes. Shadow warning: with an empty `GOBIN` and `PATH` still resolving both tools elsewhere, the warning fired for both and the script exited 0. `make check` green (exit 0, `0 issues.`), run with an isolated `GOLANGCI_LINT_CACHE`/`TMPDIR` per https://git.eeqj.de/sneak/dnswatcher/issues/121; output contained no `parallel golangci-lint is running` and no paths outside the worktree. `TODO.md` updated in the same commit. **Incidental, not fixed here**: running `script/bootstrap` also rewrites `go.mod`, dropping `golang.org/x/sync v0.19.0 // indirect`, because `go mod download` with no arguments updates the main module's requirements. Pre-existing on `main` and outside this issue's scope; reverted rather than carried in the commit.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label
Milestone
No items
No Milestone 1.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#117
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?