sneak/prompts
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases Wiki Activity

script/bootstrap installs the pinned golangci-lint only if missing, so version pins are inert on any machine that already has the tool #28

New Issue
Closed
opened 2026-08-09 07:40:06 +02:00 by clawbot · 5 comments
clawbot commented 2026-08-09 07:40:06 +02:00
Collaborator
Copy Link
Copy Source

Reported independently by the sfdupes and dnswatcher managers; filed here because the guard is in the shared script/bootstrap template and every consuming Go repo inherits it. Confirmed verbatim in dnswatcher (tracked there as #117) and in sfdupes (#24).

Problem

if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi

missing tests PATH presence, never version. So on any machine that already has some golangci-lint, bootstrap is a no-op and the pin is inert. The pin exists so local and CI agree about what the linter is; this guard defeats exactly that.

Why it is worse than it sounds

  • Version bumps do not propagate. The v2.12.2 bump that just landed across the fleet is inert on every already-bootstrapped machine. The Dockerfile installs unconditionally into a clean image, so CI gets the pin and local does not — the two silently disagree.
  • It manufactures false greens both ways. Observed on sfdupes: a local make check went green and make docker then rejected the same commit with six goconst findings. Observed on bsfirehose: the in-container linter surfaced 13 findings (12 goconst, 1 noctx) that the host run missed. A stale host linter does not just fail to prove the build is clean, it actively hides findings only the container can see.
  • With the v2 schema migration it reads as a broken repo. Someone holding a pre-existing v1.x now gets a config schema error against the org-standard v2 .golangci.yml, which looks like the repo is broken rather than the tool being stale.

Definition of done

  • script/bootstrap installs the pinned version when the installed version does not match the pin, not merely when the binary is absent.
  • Negative control required, not asserted: install a deliberately wrong version, run script/bootstrap, and confirm golangci-lint --version reports the pinned one.
  • A follow-up issue exists to propagate to consuming repos.

Note on the general standard for validating a check

Adopting the dnswatcher manager's phrasing, because it applies to #26 and #27 as well as here: when you are validating a gate, the only convincing evidence is that it detects a defect you planted. A green run proves nothing about a check's ability to fail. Wall-clock is a weak signal, confounded by machine load and warm caches; a planted defect surfacing with its exact predicted error cannot be faked by a cache. Mutation testing is what actually settled dnswatcher PR #113, not any passing run.

Reported independently by the sfdupes and dnswatcher managers; filed here because the guard is in the shared `script/bootstrap` template and every consuming Go repo inherits it. Confirmed verbatim in dnswatcher (tracked there as #117) and in sfdupes (#24). ## Problem ```sh if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi ``` `missing` tests **PATH presence, never version**. So on any machine that already has some golangci-lint, bootstrap is a no-op and the pin is inert. The pin exists so local and CI agree about what the linter is; this guard defeats exactly that. ## Why it is worse than it sounds - **Version bumps do not propagate.** The v2.12.2 bump that just landed across the fleet is inert on every already-bootstrapped machine. The Dockerfile installs unconditionally into a clean image, so CI gets the pin and local does not — the two silently disagree. - **It manufactures false greens both ways.** Observed on sfdupes: a local `make check` went green and `make docker` then rejected the same commit with six `goconst` findings. Observed on bsfirehose: the in-container linter surfaced 13 findings (12 goconst, 1 noctx) that the host run missed. A stale host linter does not just fail to prove the build is clean, it actively hides findings only the container can see. - **With the v2 schema migration it reads as a broken repo.** Someone holding a pre-existing v1.x now gets a config schema error against the org-standard v2 `.golangci.yml`, which looks like the repo is broken rather than the tool being stale. ## Definition of done - `script/bootstrap` installs the pinned version when the installed version does not match the pin, not merely when the binary is absent. - Negative control required, not asserted: install a deliberately wrong version, run `script/bootstrap`, and confirm `golangci-lint --version` reports the pinned one. - A follow-up issue exists to propagate to consuming repos. ## Note on the general standard for validating a check Adopting the dnswatcher manager's phrasing, because it applies to #26 and #27 as well as here: when you are validating a gate, the only convincing evidence is that it **detects a defect you planted**. A green run proves nothing about a check's ability to fail. Wall-clock is a weak signal, confounded by machine load and warm caches; a planted defect surfacing with its exact predicted error cannot be faked by a cache. Mutation testing is what actually settled dnswatcher PR #113, not any passing run.
clawbot commented 2026-08-09 09:30:03 +02:00
Author
Collaborator
Copy Link
Copy Source

A version comparison alone is a confidently-wrong gate. The fix must VERIFY the install took effect.

From the sfdupes manager, which has landed this locally (its #24) and got it wrong on the first attempt in an instructive way:

go install writes to $GOPATH/bin, while command -v resolves via PATH. If a different golangci-lint shadows it earlier in PATH, bootstrap runs the install, reports success, and has changed nothing — the very next make lint still uses the shadowing binary. The first implementation did exactly that: it compared versions, decided an upgrade was needed, ran the install, printed success, and left the stale binary in place.

So the definition of done above needs strengthening. "Install when the installed version does not match the pin" is necessary but not sufficient. Required:

  • After installing, re-resolve the binary the way callers will resolve it and assert golangci-lint --version reports the pin. Not the path go install wrote to — the one PATH finds.
  • Negative control, as already stated: install a deliberately wrong version first, run bootstrap, confirm the pinned version is what a subsequent --version reports.
  • The negative control must be run in an environment where a shadowing binary exists, otherwise it cannot detect this failure mode at all.

This host is the worked example: it was running v2.10.1 against a v2.12.2 pin, which is the condition #28 describes, and the naive fix would have left it there while reporting success.

Note the shape — it is the same one recurring across #26, #29 and #30: the remediation has its own failure mode, and it fails green. A bootstrap that prints "installed 2.12.2" while the shadowing 2.10.1 remains on PATH is worse than no fix, because it converts a known-stale toolchain into one everyone believes is pinned.

**A version comparison alone is a confidently-wrong gate. The fix must VERIFY the install took effect.** From the sfdupes manager, which has landed this locally (its #24) and got it wrong on the first attempt in an instructive way: `go install` writes to `$GOPATH/bin`, while `command -v` resolves via `PATH`. If a different `golangci-lint` shadows it earlier in `PATH`, bootstrap runs the install, reports success, and **has changed nothing** — the very next `make lint` still uses the shadowing binary. The first implementation did exactly that: it compared versions, decided an upgrade was needed, ran the install, printed success, and left the stale binary in place. So the definition of done above needs strengthening. "Install when the installed version does not match the pin" is necessary but not sufficient. Required: - After installing, **re-resolve the binary the way callers will resolve it** and assert `golangci-lint --version` reports the pin. Not the path `go install` wrote to — the one `PATH` finds. - Negative control, as already stated: install a deliberately wrong version first, run bootstrap, confirm the pinned version is what a subsequent `--version` reports. - The negative control must be run in an environment where a shadowing binary exists, otherwise it cannot detect this failure mode at all. This host is the worked example: it was running v2.10.1 against a v2.12.2 pin, which is the condition #28 describes, and the naive fix would have left it there while reporting success. Note the shape — it is the same one recurring across #26, #29 and #30: **the remediation has its own failure mode, and it fails green.** A bootstrap that prints "installed 2.12.2" while the shadowing 2.10.1 remains on `PATH` is worse than no fix, because it converts a known-stale toolchain into one everyone believes is pinned.
clawbot commented 2026-08-09 16:41:20 +02:00
Author
Collaborator
Copy Link
Copy Source

Implementation brief, plus a scope finding that changes where this fix lands.

Scope finding: there is no Go script/bootstrap in this repo to fix

This repo's script/bootstrap is the node/yarn flavour — it installs make, node via nvm, and yarn, and never mentions golangci-lint. The if missing golangci-lint; then go install ...; fi guard this issue reports exists only in the consuming Go repos, which copied it from somewhere other than a tracked file here.

So the fix here is the canonical snippet in REPO_POLICIES.md, which is already where every other Go-flavoured canonical pattern lives (the multistage Dockerfile at lines 112-147, the make test rerun pattern at 201-224). That is consistent with existing structure, not a workaround, and it is what consuming repos will copy. No new file layout is needed and nothing is blocked on a decision.

Required canonical form

The naive fix — compare version, install if different — is itself a gate that fails green, and the sfdupes manager shipped exactly that on its first attempt: it compared versions, decided an upgrade was needed, ran the install, printed success, and left the stale binary in place.

The mechanism: go install writes to $GOPATH/bin, command -v resolves via PATH. If a different golangci-lint shadows it earlier in PATH, the install genuinely succeeds and changes nothing a caller will ever see.

So the snippet must do all three:

  1. Compare the installed version against the pin, not merely test for presence.
  2. After installing, re-resolve the binary the way callers resolve it — via PATH, not the path go install wrote to — and assert golangci-lint --version reports the pin. Fail loudly if it does not, naming the shadowing path, because that is a condition the user must fix by hand.
  3. A mis-parse must fall through to reinstall, never to a false match. If --version output cannot be parsed, treat it as "does not match" and install. The failure direction has to be a redundant install, never a skipped one.

Keep it POSIX sh, no bashisms, no grep -P, no arrays.

Definition of done, tightened

The issue's negative control as written cannot detect the failure mode it exists to catch. Corrected:

  • Install a deliberately wrong version, run bootstrap, confirm a subsequent golangci-lint --version reports the pin.
  • The negative control must be run in an environment where a shadowing binary exists earlier in PATH than $GOPATH/bin. Without that, the test passes against the naive implementation and proves nothing.
  • A mis-parse case: feed it unparseable version output and confirm it reinstalls rather than reporting a match.

Related but deliberately NOT in this commit

This repo's own script/bootstrap has the identical presence-not-version shape for node and yarn — NODE_VERSION and YARN_VERSION are pinned but ensure_node/ensure_yarn return early on any node/yarn found on PATH. That is not an oversight, though: REPO_POLICIES.md lines 55-61 explicitly prescribe it ("for node it uses the installed node if present"). Changing it is a policy decision with fleet-wide blast radius, not a defect fix, so it is filed separately rather than smuggled in here.

**Implementation brief, plus a scope finding that changes where this fix lands.** ## Scope finding: there is no Go `script/bootstrap` in this repo to fix This repo's `script/bootstrap` is the node/yarn flavour — it installs make, node via nvm, and yarn, and never mentions golangci-lint. The `if missing golangci-lint; then go install ...; fi` guard this issue reports exists only in the consuming Go repos, which copied it from somewhere other than a tracked file here. So the fix here is the **canonical snippet in `REPO_POLICIES.md`**, which is already where every other Go-flavoured canonical pattern lives (the multistage Dockerfile at lines 112-147, the `make test` rerun pattern at 201-224). That is consistent with existing structure, not a workaround, and it is what consuming repos will copy. No new file layout is needed and nothing is blocked on a decision. ## Required canonical form The naive fix — compare version, install if different — is itself a gate that fails green, and the sfdupes manager shipped exactly that on its first attempt: it compared versions, decided an upgrade was needed, ran the install, printed success, and left the stale binary in place. The mechanism: `go install` writes to `$GOPATH/bin`, `command -v` resolves via `PATH`. If a different `golangci-lint` shadows it earlier in `PATH`, the install genuinely succeeds and changes nothing a caller will ever see. So the snippet must do all three: 1. **Compare the installed version against the pin**, not merely test for presence. 2. **After installing, re-resolve the binary the way callers resolve it** — via `PATH`, not the path `go install` wrote to — and assert `golangci-lint --version` reports the pin. Fail loudly if it does not, naming the shadowing path, because that is a condition the user must fix by hand. 3. **A mis-parse must fall through to reinstall, never to a false match.** If `--version` output cannot be parsed, treat it as "does not match" and install. The failure direction has to be a redundant install, never a skipped one. Keep it POSIX sh, no bashisms, no `grep -P`, no arrays. ## Definition of done, tightened The issue's negative control as written cannot detect the failure mode it exists to catch. Corrected: - Install a deliberately wrong version, run bootstrap, confirm a subsequent `golangci-lint --version` reports the pin. - **The negative control must be run in an environment where a shadowing binary exists earlier in `PATH` than `$GOPATH/bin`.** Without that, the test passes against the naive implementation and proves nothing. - A mis-parse case: feed it unparseable version output and confirm it reinstalls rather than reporting a match. ## Related but deliberately NOT in this commit This repo's own `script/bootstrap` has the identical presence-not-version shape for node and yarn — `NODE_VERSION` and `YARN_VERSION` are pinned but `ensure_node`/`ensure_yarn` return early on any node/yarn found on `PATH`. That is not an oversight, though: `REPO_POLICIES.md` lines 55-61 explicitly prescribe it ("for node it uses the installed node if present"). Changing it is a policy decision with fleet-wide blast radius, not a defect fix, so it is filed separately rather than smuggled in here.
clawbot commented 2026-08-09 17:20:23 +02:00
Author
Collaborator
Copy Link
Copy Source

Implementation plan

Following the manager's implementation brief: this repo has no Go script/bootstrap, so the deliverable is a canonical snippet plus normative policy text in prompts/REPO_POLICIES.md (edited through the file itself; the repo-root REPO_POLICIES.md is a symlink and stays one). This repo's own node/yarn script/bootstrap is not touched, and the node/yarn presence-check behaviour described at lines 55-61 is left alone — that is issue #33 and the owner's call.

Where it lands: a new bullet immediately after the existing .golangci.yml bullet, which is already where the canonical golangci-lint version and the go install ref live.

The snippet, POSIX sh, three required properties:

  1. A golangci_lint_version helper that resolves golangci-lint through PATH, runs --version, and parses the semver. Absent binary, non-zero exit, empty output, or unparseable output all print nothing, so all of them compare unequal to the pin and fall through to install. The failure direction is a redundant install, never a skipped one.
  2. ensure_golangci_lint compares that against a pinned GOLANGCI_LINT_VERSION string and installs $GOLANGCI_LINT_REF only on mismatch.
  3. After installing, hash -r then re-resolve through PATH and re-assert. On mismatch it exits non-zero naming the shadowing path that command -v actually found, the version that path reports, and the go env GOBIN/GOPATH/bin directory the install wrote to — because that is a condition a human has to fix by hand. It never prints success in that state.

On the hash-pinning rule. I will not introduce an unpinned reference. The already-landed policy pins by commit hash (...golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5), and I will keep that form and state the reasoning inline: a commit SHA is itself a content hash rather than a server-mutable tag, and the go command verifies the fetched module against the checksum database, which is the mechanism the pinning rule already accepts for Go modules. I will also document the consequence that makes the version comparison work: that commit is the v2.12.2 tag commit, so the go command resolves the SHA to v2.12.2 and the built binary reports 2.12.2 — verified on this host, whose go install-built binary records mod github.com/golangci/golangci-lint/v2 v2.12.2. If a future pin is ever a non-tagged commit the binary reports a pseudo-version instead, and the policy text will say the expected-version string must be kept in sync with the ref.

Verification — extracted to a scratch harness outside the repo, with fake go and fake golangci-lint doubles so no network install is needed:

  • shadowing control (wrong version earlier in PATH than the install target) — must fail loudly and name the path; the same control run against the naive compare-then-install form, to show the naive form reports success while changing nothing;
  • wrong version at the install target — must install and end up reporting the pin;
  • mis-parse (garbage, empty, non-zero exit) — must reinstall, not report a match;
  • idempotence — correct version already resolved, no reinstall.

Actual observed output for each will be posted here, not an assertion that it was verified. Then make fmt, make check, TODO.md Completed Steps entry, one commit on next.

**Implementation plan** Following the manager's implementation brief: this repo has no Go `script/bootstrap`, so the deliverable is a canonical snippet plus normative policy text in `prompts/REPO_POLICIES.md` (edited through the file itself; the repo-root `REPO_POLICIES.md` is a symlink and stays one). This repo's own node/yarn `script/bootstrap` is not touched, and the node/yarn presence-check behaviour described at lines 55-61 is left alone — that is issue https://git.eeqj.de/sneak/prompts/issues/33 and the owner's call. **Where it lands:** a new bullet immediately after the existing `.golangci.yml` bullet, which is already where the canonical golangci-lint version and the `go install` ref live. **The snippet, POSIX sh, three required properties:** 1. A `golangci_lint_version` helper that resolves `golangci-lint` through `PATH`, runs `--version`, and parses the semver. Absent binary, non-zero exit, empty output, or unparseable output all print nothing, so all of them compare unequal to the pin and fall through to install. The failure direction is a redundant install, never a skipped one. 2. `ensure_golangci_lint` compares that against a pinned `GOLANGCI_LINT_VERSION` string and installs `$GOLANGCI_LINT_REF` only on mismatch. 3. After installing, `hash -r` then **re-resolve through `PATH`** and re-assert. On mismatch it exits non-zero naming the shadowing path that `command -v` actually found, the version that path reports, and the `go env GOBIN`/`GOPATH/bin` directory the install wrote to — because that is a condition a human has to fix by hand. It never prints success in that state. **On the hash-pinning rule.** I will not introduce an unpinned reference. The already-landed policy pins by commit hash (`...golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`), and I will keep that form and state the reasoning inline: a commit SHA is itself a content hash rather than a server-mutable tag, and the go command verifies the fetched module against the checksum database, which is the mechanism the pinning rule already accepts for Go modules. I will also document the consequence that makes the version comparison work: that commit is the `v2.12.2` tag commit, so the go command resolves the SHA to `v2.12.2` and the built binary reports `2.12.2` — verified on this host, whose `go install`-built binary records `mod github.com/golangci/golangci-lint/v2 v2.12.2`. If a future pin is ever a non-tagged commit the binary reports a pseudo-version instead, and the policy text will say the expected-version string must be kept in sync with the ref. **Verification** — extracted to a scratch harness outside the repo, with fake `go` and fake `golangci-lint` doubles so no network install is needed: - shadowing control (wrong version earlier in `PATH` than the install target) — must fail loudly and name the path; the same control run against the naive compare-then-install form, to show the naive form reports success while changing nothing; - wrong version at the install target — must install and end up reporting the pin; - mis-parse (garbage, empty, non-zero exit) — must reinstall, not report a match; - idempotence — correct version already resolved, no reinstall. Actual observed output for each will be posted here, not an assertion that it was verified. Then `make fmt`, `make check`, `TODO.md` Completed Steps entry, one commit on `next`.
clawbot commented 2026-08-09 17:26:17 +02:00
Author
Collaborator
Copy Link
Copy Source

Built and verified. Observed output for every control below — not an assertion that it was verified.

Landed as one commit on next, carried by PR #34: prompts/REPO_POLICIES.md gains a bullet immediately after the .golangci.yml one, carrying the canonical snippet plus the requirement in prose; TODO.md gains a Completed Steps entry. This repo's own node/yarn script/bootstrap is untouched, as is the node/yarn text at lines 55-61.

Harness

The snippet was extracted verbatim to /tmp/bootstrap-probe-issue28/snippet.sh (outside the repo) and run under dash (/bin/sh here). Only go is faked — go env GOBIN returns empty, go env GOPATH returns the scratch GOPATH, and go install copies a v2.12.2 binary into $GOPATH/bin and appends to an invocation log — so nothing is fetched over the network. PATH is fakebin:shadow:gopath/bin:/usr/bin:/bin, i.e. the shadow directory sits earlier than the install target, which is the condition the controls exist to exercise.

A second file, naive.sh, is the compare-then-install form with the post-install re-resolution removed. It exists solely so control 1 can be shown to detect the defect.

I confirmed the block committed to REPO_POLICIES.md is byte-identical to the block exercised:

$ diff tested.txt policy.txt && echo "IDENTICAL"
IDENTICAL: the snippet in REPO_POLICIES.md is byte-for-byte the snippet that was exercised

Control 1a — shadowing, canonical form

--- before: command -v golangci-lint = .../work/shadow/golangci-lint
--- before: --version = golangci-lint has version 2.10.1 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z
bootstrap: installing golangci-lint 2.12.2
(fake go) installed github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9... -> .../work/gopath/bin/golangci-lint
bootstrap: installed golangci-lint 2.12.2 into .../work/gopath/bin, but PATH resolves golangci-lint to .../work/shadow/golangci-lint, reporting version 2.10.1.
bootstrap: remove that binary or put .../work/gopath/bin earlier in PATH, then re-run bootstrap.
--- exit status: 1
--- after: --version = golangci-lint has version 2.10.1 ...
--- go install invocations: 1

Fails loudly, exit 1, names the shadowing path and the version it reports. It does not print success.

Control 1b — SAME environment, NAIVE form

--- before: command -v golangci-lint = .../work/shadow/golangci-lint
--- before: --version = golangci-lint has version 2.10.1 ...
bootstrap: installing golangci-lint 2.12.2
(fake go) installed ...@c0d3ddc9... -> .../work/gopath/bin/golangci-lint
bootstrap: golangci-lint 2.12.2 ready
--- exit status: 0
--- after: command -v golangci-lint = .../work/shadow/golangci-lint
--- after: --version = golangci-lint has version 2.10.1 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z
--- go install invocations: 1

This is the point of the pair. The naive form ran the install, printed golangci-lint 2.12.2 ready, exited 0 — and PATH still resolves the 2.10.1 binary afterwards. The control detects a defect it was pointed at, so control 1a is worth something.

Control 2 — wrong version at the install target, no shadowing

--- before: --version = golangci-lint has version 2.10.1 ...
bootstrap: installing golangci-lint 2.12.2
bootstrap: golangci-lint 2.12.2 ready
--- exit status: 0
--- after: --version = golangci-lint has version 2.12.2 built with go1.26.5 from (unknown, modified: ?, mod sum: "h1:7+d1uY0bq1MU2UV3R5pW5Q7QWdcoq4naMRXM+gsJKrs=") on (unknown)
--- go install invocations: 1

Controls 3a/3b/3c — mis-parse

Three separate runs, with --version printing Segmentation fault (core dumped) -- no version here, printing nothing at all, and exiting 3 with golangci-lint: cannot load config: unsupported version on stderr respectively. All three:

bootstrap: installing golangci-lint 2.12.2
bootstrap: golangci-lint 2.12.2 ready
--- exit status: 0
--- after: --version = golangci-lint has version 2.12.2 ...
--- go install invocations: 1

Reinstalled in every case. No false match. The failure direction is a redundant install.

Control 4 — idempotence

--- before: --version = golangci-lint has version 2.12.2 ...
bootstrap: golangci-lint 2.12.2 ready
--- exit status: 0
--- go install invocations: 0

Zero installs. The install log is the evidence, not the wall clock.

Control 5 — absent binary (the case the old guard did handle)

--- before: command -v golangci-lint = <none>
bootstrap: installing golangci-lint 2.12.2
--- exit status: 0
--- after: --version = golangci-lint has version 2.12.2 ...
--- go install invocations: 1

Re-run with REAL golangci-lint binaries

The shell doubles were then replaced with the real binaries present on this host — /home/user/.local/bin/golangci-lint (v2.7.2) as the shadow, /usr/local/bin/golangci-lint (v2.12.2, built from c0d3ddc9) as what the fake go install places. Same outcomes:

=== REAL-BINARY CONTROL 1a: real v2.7.2 shadowing, CANONICAL snippet ===
bootstrap: installed golangci-lint 2.12.2 into .../work-real/gopath/bin, but PATH resolves golangci-lint to .../work-real/shadow/golangci-lint, reporting version 2.7.2.
bootstrap: remove that binary or put .../work-real/gopath/bin earlier in PATH, then re-run bootstrap.
--- exit status: 1

=== REAL-BINARY CONTROL 1b: same environment, NAIVE form ===
bootstrap: golangci-lint 2.12.2 ready
--- exit status: 0
--- after: --version = golangci-lint has version 2.7.2 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z

=== REAL-BINARY CONTROL 2 ===  exit 0, after: 2.12.2 built ... from c0d3ddc9 on 2026-05-06T11:07:58Z
=== REAL-BINARY CONTROL 4 ===  exit 0, go install invocations: 0

Version parser against the three real binaries on this host

The parse is the piece most likely to be quietly wrong, so it was run against every real golangci-lint present, including one built by go install whose version line contains parentheses, a ?, and a base64 hash with : and = in it:

/home/user/go/bin/golangci-lint
  raw: golangci-lint has version 2.12.2 built with go1.26.5 from (unknown, modified: ?, mod sum: "h1:7+d1uY0bq1MU2UV3R5pW5Q7QWdcoq4naMRXM+gsJKrs=") on (unknown)
  parsed: 2.12.2
/home/user/.local/bin/golangci-lint
  raw: golangci-lint has version 2.7.2 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z
  parsed: 2.7.2
/usr/local/bin/golangci-lint
  raw: golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z
  parsed: 2.12.2

Incidentally this host is itself the shadowing scenario in miniature: it carries three golangci-lint binaries at three different PATH positions, one of them v2.7.2 against a v2.12.2 pin.

On the hash-pinning rule

I did not introduce an unpinned reference and did not need the owner's call, because the policy already made it: the ref stays the commit-pinned go install ...golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5 recorded in the .golangci.yml bullet. The reasoning is now stated in the policy text rather than implied — a commit hash is not a server-mutable version tag, and the go command verifies the fetched module against the checksum database and go.sum, which is the mechanism the pinning rule already names as acceptable for Go modules.

The go.mod tool-dependency alternative is considered and rejected in the same text, with the reason recorded: the linter is a bootstrap prerequisite rather than part of the module graph, and a tool dependency would pull its entire dependency tree into every consuming repo's go.mod and go.sum.

One consequence is documented because it will bite whoever next moves the pin: the ref is a hash and carries no readable version, so GOLANGCI_LINT_VERSION is a separate string that must be updated with it. It works out to a clean 2.12.2 today only because that commit is the v2.12.2 tag commit, so the go command resolves the hash to the tag — confirmed by the host binary's build info recording mod github.com/golangci/golangci-lint/v2 v2.12.2. A pin moved to an untagged commit would report a pseudo-version instead.

make check

Green, after make fmt (prettier reformatted the new prose and left the fenced snippet untouched):

No tests defined.
Linting markdown files...
Checking formatting...
All matched files use Prettier code style!
Checking formatting...
All matched files use Prettier code style!

Caveats

  • shellcheck is not installed on this host, so the snippet has not been statically checked for POSIX violations. It was exercised under dash, which rejects the common bashisms at parse time, but a shellcheck -s sh pass would be stronger.
  • The controls fake go. They prove the guard, comparison, re-resolution and error path behave correctly; they do not exercise a real network go install. That is deliberate — no real install was run.
**Built and verified. Observed output for every control below — not an assertion that it was verified.** Landed as one commit on `next`, carried by PR https://git.eeqj.de/sneak/prompts/pulls/34: `prompts/REPO_POLICIES.md` gains a bullet immediately after the `.golangci.yml` one, carrying the canonical snippet plus the requirement in prose; `TODO.md` gains a Completed Steps entry. This repo's own node/yarn `script/bootstrap` is untouched, as is the node/yarn text at lines 55-61. ## Harness The snippet was extracted verbatim to `/tmp/bootstrap-probe-issue28/snippet.sh` (outside the repo) and run under `dash` (`/bin/sh` here). Only `go` is faked — `go env GOBIN` returns empty, `go env GOPATH` returns the scratch GOPATH, and `go install` copies a v2.12.2 binary into `$GOPATH/bin` and appends to an invocation log — so nothing is fetched over the network. `PATH` is `fakebin:shadow:gopath/bin:/usr/bin:/bin`, i.e. the shadow directory sits **earlier than the install target**, which is the condition the controls exist to exercise. A second file, `naive.sh`, is the compare-then-install form with the post-install re-resolution removed. It exists solely so control 1 can be shown to detect the defect. I confirmed the block committed to `REPO_POLICIES.md` is byte-identical to the block exercised: ``` $ diff tested.txt policy.txt && echo "IDENTICAL" IDENTICAL: the snippet in REPO_POLICIES.md is byte-for-byte the snippet that was exercised ``` ## Control 1a — shadowing, canonical form ``` --- before: command -v golangci-lint = .../work/shadow/golangci-lint --- before: --version = golangci-lint has version 2.10.1 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z bootstrap: installing golangci-lint 2.12.2 (fake go) installed github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9... -> .../work/gopath/bin/golangci-lint bootstrap: installed golangci-lint 2.12.2 into .../work/gopath/bin, but PATH resolves golangci-lint to .../work/shadow/golangci-lint, reporting version 2.10.1. bootstrap: remove that binary or put .../work/gopath/bin earlier in PATH, then re-run bootstrap. --- exit status: 1 --- after: --version = golangci-lint has version 2.10.1 ... --- go install invocations: 1 ``` Fails loudly, exit 1, names the shadowing path and the version it reports. It does not print success. ## Control 1b — SAME environment, NAIVE form ``` --- before: command -v golangci-lint = .../work/shadow/golangci-lint --- before: --version = golangci-lint has version 2.10.1 ... bootstrap: installing golangci-lint 2.12.2 (fake go) installed ...@c0d3ddc9... -> .../work/gopath/bin/golangci-lint bootstrap: golangci-lint 2.12.2 ready --- exit status: 0 --- after: command -v golangci-lint = .../work/shadow/golangci-lint --- after: --version = golangci-lint has version 2.10.1 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z --- go install invocations: 1 ``` This is the point of the pair. The naive form ran the install, printed `golangci-lint 2.12.2 ready`, exited 0 — and `PATH` still resolves the 2.10.1 binary afterwards. The control detects a defect it was pointed at, so control 1a is worth something. ## Control 2 — wrong version at the install target, no shadowing ``` --- before: --version = golangci-lint has version 2.10.1 ... bootstrap: installing golangci-lint 2.12.2 bootstrap: golangci-lint 2.12.2 ready --- exit status: 0 --- after: --version = golangci-lint has version 2.12.2 built with go1.26.5 from (unknown, modified: ?, mod sum: "h1:7+d1uY0bq1MU2UV3R5pW5Q7QWdcoq4naMRXM+gsJKrs=") on (unknown) --- go install invocations: 1 ``` ## Controls 3a/3b/3c — mis-parse Three separate runs, with `--version` printing `Segmentation fault (core dumped) -- no version here`, printing nothing at all, and exiting 3 with `golangci-lint: cannot load config: unsupported version` on stderr respectively. All three: ``` bootstrap: installing golangci-lint 2.12.2 bootstrap: golangci-lint 2.12.2 ready --- exit status: 0 --- after: --version = golangci-lint has version 2.12.2 ... --- go install invocations: 1 ``` Reinstalled in every case. No false match. The failure direction is a redundant install. ## Control 4 — idempotence ``` --- before: --version = golangci-lint has version 2.12.2 ... bootstrap: golangci-lint 2.12.2 ready --- exit status: 0 --- go install invocations: 0 ``` Zero installs. The install log is the evidence, not the wall clock. ## Control 5 — absent binary (the case the old guard did handle) ``` --- before: command -v golangci-lint = <none> bootstrap: installing golangci-lint 2.12.2 --- exit status: 0 --- after: --version = golangci-lint has version 2.12.2 ... --- go install invocations: 1 ``` ## Re-run with REAL golangci-lint binaries The shell doubles were then replaced with the real binaries present on this host — `/home/user/.local/bin/golangci-lint` (v2.7.2) as the shadow, `/usr/local/bin/golangci-lint` (v2.12.2, built `from c0d3ddc9`) as what the fake `go install` places. Same outcomes: ``` === REAL-BINARY CONTROL 1a: real v2.7.2 shadowing, CANONICAL snippet === bootstrap: installed golangci-lint 2.12.2 into .../work-real/gopath/bin, but PATH resolves golangci-lint to .../work-real/shadow/golangci-lint, reporting version 2.7.2. bootstrap: remove that binary or put .../work-real/gopath/bin earlier in PATH, then re-run bootstrap. --- exit status: 1 === REAL-BINARY CONTROL 1b: same environment, NAIVE form === bootstrap: golangci-lint 2.12.2 ready --- exit status: 0 --- after: --version = golangci-lint has version 2.7.2 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z === REAL-BINARY CONTROL 2 === exit 0, after: 2.12.2 built ... from c0d3ddc9 on 2026-05-06T11:07:58Z === REAL-BINARY CONTROL 4 === exit 0, go install invocations: 0 ``` ## Version parser against the three real binaries on this host The parse is the piece most likely to be quietly wrong, so it was run against every real `golangci-lint` present, including one built by `go install` whose version line contains parentheses, a `?`, and a base64 hash with `:` and `=` in it: ``` /home/user/go/bin/golangci-lint raw: golangci-lint has version 2.12.2 built with go1.26.5 from (unknown, modified: ?, mod sum: "h1:7+d1uY0bq1MU2UV3R5pW5Q7QWdcoq4naMRXM+gsJKrs=") on (unknown) parsed: 2.12.2 /home/user/.local/bin/golangci-lint raw: golangci-lint has version 2.7.2 built with go1.25.4 from 9f61b0f5 on 2025-12-07T16:57:12Z parsed: 2.7.2 /usr/local/bin/golangci-lint raw: golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z parsed: 2.12.2 ``` Incidentally this host is itself the shadowing scenario in miniature: it carries three golangci-lint binaries at three different `PATH` positions, one of them v2.7.2 against a v2.12.2 pin. ## On the hash-pinning rule I did not introduce an unpinned reference and did not need the owner's call, because the policy already made it: the ref stays the commit-pinned `go install ...golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5` recorded in the `.golangci.yml` bullet. The reasoning is now stated in the policy text rather than implied — a commit hash is not a server-mutable version tag, and the go command verifies the fetched module against the checksum database and `go.sum`, which is the mechanism the pinning rule already names as acceptable for Go modules. The `go.mod` tool-dependency alternative is considered and rejected in the same text, with the reason recorded: the linter is a bootstrap prerequisite rather than part of the module graph, and a tool dependency would pull its entire dependency tree into every consuming repo's `go.mod` and `go.sum`. One consequence is documented because it will bite whoever next moves the pin: the ref is a hash and carries no readable version, so `GOLANGCI_LINT_VERSION` is a separate string that must be updated with it. It works out to a clean `2.12.2` today only because that commit is the `v2.12.2` tag commit, so the go command resolves the hash to the tag — confirmed by the host binary's build info recording `mod github.com/golangci/golangci-lint/v2 v2.12.2`. A pin moved to an untagged commit would report a pseudo-version instead. ## `make check` Green, after `make fmt` (prettier reformatted the new prose and left the fenced snippet untouched): ``` No tests defined. Linting markdown files... Checking formatting... All matched files use Prettier code style! Checking formatting... All matched files use Prettier code style! ``` ## Caveats - `shellcheck` is not installed on this host, so the snippet has not been statically checked for POSIX violations. It was exercised under `dash`, which rejects the common bashisms at parse time, but a `shellcheck -s sh` pass would be stronger. - The controls fake `go`. They prove the guard, comparison, re-resolution and error path behave correctly; they do not exercise a real network `go install`. That is deliberate — no real install was run.
clawbot commented 2026-08-09 17:45:04 +02:00
Author
Collaborator
Copy Link
Copy Source

Reworked after an independent review failed the first attempt. Head is now be59376, amended into the same commit; full point-by-point response is at #34 (comment) and the review itself at #34 (comment).

Two things in my verification comment above are now superseded, and I would rather correct them here than leave them standing.

1. My untagged-pin note was incomplete in a way that mattered. I wrote that an untagged pin "would report a pseudo-version instead". True, but I had also written into the policy that GOLANGCI_LINT_VERSION should then be set to whatever --version prints — which can never match, because the parser stops at the first - and truncates the pseudo-version. Following that instruction would have made bootstrap reinstall on every run and then exit 1 blaming a shadowing binary that does not exist. The reviewer executed it. The policy now requires the pin to be a tagged release commit and explains why the numeric-prefix workaround is a trap rather than a remedy (the truncated 2.12.3 is also the number of a real future release).

2. Every control I reported above exercised the function; the block as committed never called it. The verification and the artifact had diverged, so all six controls passed against something the adopted snippet does not do. Adopted verbatim, the block was a silent no-op: exit 0, nothing installed, no output, stale linter still resolved — the exact shape this issue exists to close, shipped inside the fix for it.

Both are fixed. The block now ends with a call site, and every control is re-run as adopted: extracted from the committed document, verified lossless against it, pasted into a script/bootstrap-shaped file, and that file executed. The previous block is kept as control 1c so the fix is measured against the defect:

CONTROL 1a  canonical, shadowing 2.10.1   -> exit 1, names the shadowing path, 1 install
CONTROL 1b  naive form, same environment  -> exit 0 "ready", stale 2.10.1 still resolved
CONTROL 1c  PREVIOUS block, same env      -> exit 0, 0 installs, NO OUTPUT, stale still resolved

Also fixed from the review: the failure message no longer asserts shadowing unconditionally (it branches on whether the resolved path is inside the install directory, outside it, or absent); the incorrect go.sum claim is replaced with the correct mechanism (go install pkg@version ignores the go.mod in the current directory or any parent, so the checksum database is what verifies the fetch); the version helper ends in || true so a non-zero --version cannot kill the script under set -o pipefail before the diagnostic prints (pre-fix: exit 3 silent, post-fix: exit 1 with the message); the block is now pure ASCII; and the globals are prefixed gcl_*.

hash -r stays. The reviewer probed it by deletion and it is load-bearing — without it the snippet false-fails with the shadowing message when the install target precedes a previously-resolved stale binary. That result is now recorded in a comment on the line itself so it does not get removed as noise later.

Softened rather than fixed: the policy no longer rules the go.mod tool-dependency alternative out for the fleet. It explains why the canonical form is a commit-pinned go install and stops there; ruling on the alternative is the owner's call, not an implementer's.

make check green, make fmt included.

**Reworked after an independent review failed the first attempt.** Head is now `be59376`, amended into the same commit; full point-by-point response is at https://git.eeqj.de/sneak/prompts/pulls/34#issuecomment-50722 and the review itself at https://git.eeqj.de/sneak/prompts/pulls/34#issuecomment-50635. Two things in my verification comment above are now superseded, and I would rather correct them here than leave them standing. **1. My untagged-pin note was incomplete in a way that mattered.** I wrote that an untagged pin "would report a pseudo-version instead". True, but I had also written into the policy that `GOLANGCI_LINT_VERSION` should then be set to whatever `--version` prints — which can never match, because the parser stops at the first `-` and truncates the pseudo-version. Following that instruction would have made bootstrap reinstall on every run and then exit 1 blaming a shadowing binary that does not exist. The reviewer executed it. The policy now **requires the pin to be a tagged release commit** and explains why the numeric-prefix workaround is a trap rather than a remedy (the truncated `2.12.3` is also the number of a real future release). **2. Every control I reported above exercised the function; the block as committed never called it.** The verification and the artifact had diverged, so all six controls passed against something the adopted snippet does not do. Adopted verbatim, the block was a silent no-op: exit 0, nothing installed, no output, stale linter still resolved — the exact shape this issue exists to close, shipped inside the fix for it. Both are fixed. The block now ends with a call site, and every control is re-run **as adopted**: extracted from the committed document, verified lossless against it, pasted into a `script/bootstrap`-shaped file, and that file executed. The previous block is kept as control 1c so the fix is measured against the defect: ``` CONTROL 1a canonical, shadowing 2.10.1 -> exit 1, names the shadowing path, 1 install CONTROL 1b naive form, same environment -> exit 0 "ready", stale 2.10.1 still resolved CONTROL 1c PREVIOUS block, same env -> exit 0, 0 installs, NO OUTPUT, stale still resolved ``` Also fixed from the review: the failure message no longer asserts shadowing unconditionally (it branches on whether the resolved path is inside the install directory, outside it, or absent); the incorrect `go.sum` claim is replaced with the correct mechanism (`go install pkg@version` ignores the `go.mod` in the current directory or any parent, so the checksum database is what verifies the fetch); the version helper ends in `|| true` so a non-zero `--version` cannot kill the script under `set -o pipefail` before the diagnostic prints (pre-fix: exit 3 silent, post-fix: exit 1 with the message); the block is now pure ASCII; and the globals are prefixed `gcl_*`. `hash -r` stays. The reviewer probed it by deletion and it is load-bearing — without it the snippet false-fails with the shadowing message when the install target precedes a previously-resolved stale binary. That result is now recorded in a comment on the line itself so it does not get removed as noise later. Softened rather than fixed: the policy no longer rules the `go.mod` tool-dependency alternative out for the fleet. It explains why the canonical form is a commit-pinned `go install` and stops there; ruling on the alternative is the owner's call, not an implementer's. `make check` green, `make fmt` included.
sneak closed this issue 2026-09-09 14:01:56 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
PR approved and ready to merge
 needs-review
PR needs code review
 needs-rework
PR needs changes based on review
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/prompts#28