Follow-ups from PR #38: bootstrap re-extract guard, message attribution, and the #31 rebase reconciliation #39

New Issue

clawbot · 2026-08-09T10:20:38+02:00

clawbot commented

2026-08-09 10:20:38 +02:00

Non-blocking findings carried out of PR #38's fourth review, plus one reconciliation task that must not be lost at merge time. Filed rather than spending a fifth review cycle on cosmetics.

1. Re-extract guard misses a deleted `gofmt` (N1)

script/bootstrap (around :414) keys its re-extract decision only on $GO_DIR/bin/go. Delete $TOOLCHAIN/go-1.25.7/bin/gofmt — from inside bootstrap's own managed directory — and you get a dangling ~/.local/bin/gofmt plus a permanent exit 2. Verified: both runs exit 2, it never converges.

It fails closed and is only reachable by tampering inside a directory bootstrap owns, which is why it is not blocking. But it is a one-line fix:

if [ ! -x "$GO_DIR/bin/go" ] || [ ! -x "$GO_DIR/bin/gofmt" ]; then

Guard checks both binaries. Verify by deleting each in turn from the toolchain directory and confirming bootstrap re-extracts and converges.

2. Failure-message attribution (N2)

script/bootstrap (around :567-568): the "wrong version" bucket can print "resolve to a build this script did not provision" about the binary bootstrap did provision, while the tool actually being shadowed (go) is not listed. The remedy still converges, so this is diagnostic accuracy, not correctness.

The message names the tool that is actually shadowed.
Verify by reproducing the shadowed-PATH case and reading the output.

3. Over-claims in comments and docs (N3)

script/bootstrap:19-20 and the corresponding README.md bullet still say "anything… is symlinked" and "everything… hash-verified". Both are slightly broader than the truth — the rewritten PR body already disclaims them, which is the tell that the source comments were not updated to match.

Both statements narrowed to what the script actually does. pnpm/pnpx/yarnpkg are deliberately not linked; the system package manager's installs are not hash-verified by us.

4. PR #38 body over-generalisations (N4/N5)

Two statements in #38's description: dangling symlinks are classified by their target rather than by being dangling, and the "empty PATH + unset BIN_DIR" transcript describes a state make bootstrap cannot actually reach. Cosmetic, and #38 will be merged by then — fix only if editing that body for another reason. Recorded for completeness.

5. THE ONE THAT MATTERS — the #31 rebase reconciliation

This is not a cosmetic follow-up. It is a task that must be performed correctly when #38 is rebased after #31 lands, and it has a trap.

Three coupled constants move together:

GOLANGCI_CONFIG_SHA256 in backend/script/lint — provisional 33ba2bf7fe4a44779d09b0fb31d6daf03685f8dc9d2bc417f963d7aabb0d17dc becomes canonical 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.
GOLANGCI_LINT_VERSION and its four archive sha256 values in script/bootstrap — 2.7.2 becomes 2.12.2, matching #31's Dockerfile.backend pin (commit c0d3ddc9cf3faa61a4e378e879ece580256d76e5). Fetch the hashes from that release's checksums.txt; do not carry any value over.
GO_VERSION / GO_MAX_MINOR in script/bootstrap — this is the trap.

PR #38 established that the Go pin is coupled to the linter's build toolchain, not to anything about this repo. golangci-lint links go/types from whatever Go compiled it, which is why a host Go 1.26 produces panic: file requires newer Go version go1.26 against a linter built with go1.25.4. The current window [1.25.5, 1.25.x] exists precisely because golangci-lint 2.7.2 was built with go1.25.4.

Determine what Go golangci-lint v2.12.2 was built with and re-derive the window from that. Do not assume it is still 1.25. golangci-lint version reports its build toolchain — that is the authoritative source.

Getting this wrong silently reintroduces the exact panic that cost PR #38 a full review cycle. Both merge orders were verified to fail closed on the config hash (item 1), so a missed reconciliation there is loud. Items 2 and 3 have no equivalent guard.

Definition of done for item 5

All three constants reconciled in the rebase commit, with the Go window re-derived from v2.12.2's actual build toolchain rather than assumed.
sha256sum backend/.golangci.yml on the merged result is 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb and make lint passes.
Prove the coupling holds: in a container whose host Go is one minor version newer than the window's ceiling, make bootstrap && make check is green — i.e. the newer host Go is ignored and the pinned one installed beside it.
Bare debian:bookworm-slim (make/git/curl/ca-certificates only): make bootstrap && make check green.
Root make check green on main after the merge.

Implementation requirements

Items 1-4 are small and may be batched into one commit. Item 5 is not optional and is not a follow-up — it happens as part of the #38 rebase, not afterwards. It is recorded here only so it survives if the rebase is done by someone who has not read #38's manager note.
Do not edit .golangci.yml. Ever.
Every archive hash comes from the official release manifest, verified before use. No curl | sh.
make targets and script/ entrypoints only.
Not on the 1.0.0 milestone — items 1-4 do not block a tag. Item 5 is merge mechanics, not milestone work.
No attribution trailers in the commit message.

Non-blocking findings carried out of PR #38's fourth review, plus one reconciliation task that must not be lost at merge time. Filed rather than spending a fifth review cycle on cosmetics. ## 1. Re-extract guard misses a deleted `gofmt` (N1) `script/bootstrap` (around `:414`) keys its re-extract decision only on `$GO_DIR/bin/go`. Delete `$TOOLCHAIN/go-1.25.7/bin/gofmt` — from inside bootstrap's **own** managed directory — and you get a dangling `~/.local/bin/gofmt` plus a permanent exit 2. Verified: both runs exit 2, it never converges. It fails closed and is only reachable by tampering inside a directory bootstrap owns, which is why it is not blocking. But it is a one-line fix: ```sh if [ ! -x "$GO_DIR/bin/go" ] || [ ! -x "$GO_DIR/bin/gofmt" ]; then ``` - [ ] Guard checks both binaries. Verify by deleting each in turn from the toolchain directory and confirming bootstrap re-extracts and converges. ## 2. Failure-message attribution (N2) `script/bootstrap` (around `:567-568`): the "wrong version" bucket can print "resolve to a build this script did not provision" about the binary bootstrap *did* provision, while the tool actually being shadowed (`go`) is not listed. The remedy still converges, so this is diagnostic accuracy, not correctness. - [ ] The message names the tool that is actually shadowed. - [ ] Verify by reproducing the shadowed-PATH case and reading the output. ## 3. Over-claims in comments and docs (N3) `script/bootstrap:19-20` and the corresponding `README.md` bullet still say "anything… is symlinked" and "everything… hash-verified". Both are slightly broader than the truth — the rewritten PR body already disclaims them, which is the tell that the source comments were not updated to match. - [ ] Both statements narrowed to what the script actually does. `pnpm`/`pnpx`/`yarnpkg` are deliberately not linked; the system package manager's installs are not hash-verified by us. ## 4. PR #38 body over-generalisations (N4/N5) Two statements in #38's description: dangling symlinks are classified by their target rather than by being dangling, and the "empty `PATH` + unset `BIN_DIR`" transcript describes a state `make bootstrap` cannot actually reach. Cosmetic, and #38 will be merged by then — fix only if editing that body for another reason. Recorded for completeness. ## 5. THE ONE THAT MATTERS — the #31 rebase reconciliation This is not a cosmetic follow-up. It is a task that must be performed correctly when #38 is rebased after #31 lands, and it has a trap. Three coupled constants move together: 1. **`GOLANGCI_CONFIG_SHA256`** in `backend/script/lint` — provisional `33ba2bf7fe4a44779d09b0fb31d6daf03685f8dc9d2bc417f963d7aabb0d17dc` becomes canonical `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. 2. **`GOLANGCI_LINT_VERSION`** and its four archive sha256 values in `script/bootstrap` — `2.7.2` becomes `2.12.2`, matching #31's `Dockerfile.backend` pin (commit `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`). Fetch the hashes from that release's `checksums.txt`; do not carry any value over. 3. **`GO_VERSION` / `GO_MAX_MINOR`** in `script/bootstrap` — **this is the trap.** PR #38 established that the Go pin is coupled to the *linter's build toolchain*, not to anything about this repo. golangci-lint links `go/types` from whatever Go compiled it, which is why a host Go 1.26 produces `panic: file requires newer Go version go1.26` against a linter built with `go1.25.4`. The current window `[1.25.5, 1.25.x]` exists precisely because golangci-lint 2.7.2 was built with `go1.25.4`. **Determine what Go golangci-lint v2.12.2 was built with and re-derive the window from that.** Do not assume it is still 1.25. `golangci-lint version` reports its build toolchain — that is the authoritative source. Getting this wrong silently reintroduces the exact panic that cost PR #38 a full review cycle. Both merge orders were verified to fail **closed** on the config hash (item 1), so a missed reconciliation there is loud. **Items 2 and 3 have no equivalent guard.** ### Definition of done for item 5 - [ ] All three constants reconciled in the rebase commit, with the Go window re-derived from v2.12.2's actual build toolchain rather than assumed. - [ ] `sha256sum backend/.golangci.yml` on the merged result is `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` and `make lint` passes. - [ ] Prove the coupling holds: in a container whose host Go is one minor version **newer** than the window's ceiling, `make bootstrap && make check` is green — i.e. the newer host Go is ignored and the pinned one installed beside it. - [ ] Bare `debian:bookworm-slim` (make/git/curl/ca-certificates only): `make bootstrap && make check` green. - [ ] Root `make check` green on `main` after the merge. ## Implementation requirements - Items 1-4 are small and may be batched into one commit. **Item 5 is not optional and is not a follow-up** — it happens as part of the #38 rebase, not afterwards. It is recorded here only so it survives if the rebase is done by someone who has not read #38's manager note. - Do not edit `.golangci.yml`. Ever. - Every archive hash comes from the official release manifest, verified before use. No `curl | sh`. - `make` targets and `script/` entrypoints only. - Not on the `1.0.0` milestone — items 1-4 do not block a tag. Item 5 is merge mechanics, not milestone work. - No attribution trailers in the commit message.

clawbot commented

2026-08-09 12:18:59 +02:00

Item 5's unknown is now answered: v2.12.2 is built with go1.26.5

Item 5 said "determine what Go golangci-lint v2.12.2 was built with and re-derive the window from that — do not assume it is still 1.25." That was the trap, and it is no longer a research task. Issue #41 records the measurement directly:

golangci-lint has version 2.12.2 built with go1.26.5

So the coupling resolves as follows.

What the window becomes after #31 lands

The constraint established by PR #38 is that the linter's build toolchain must be at least as new as the Go compiling the code — golangci-lint links go/types from whatever compiled it, and it panics with file requires newer Go version goX.Y when asked to parse a newer language version than it understands.

	linter build toolchain	resulting ceiling
today, v2.7.2	`go1.25.4`	`GO_MAX_MINOR="1.25"`
after #31, v2.12.2	`go1.26.5`	`GO_MAX_MINOR="1.26"`

So the reconciliation is a ceiling raise, not a pin move: [1.25.5, 1.25.x] becomes [1.25.5, 1.26.x]. The floor stays at GO_MIN_VERSION=1.25.5 — that comes from backend/go.mod, which #31 does not touch.

GO_VERSION (the version bootstrap installs when the host has nothing usable) does not have to move. 1.25.7 remains inside the widened window and still matches the digest-pinned golang:1.25-alpine builder. Moving it would be a separate, larger decision touching Dockerfile.backend too — do not fold that in.

One pleasant consequence

Widening the ceiling to 1.26 means a host that already has Go 1.26 — the case that produced the original B1 panic and cost a full review cycle — becomes a reuse case rather than an install case. Fewer machines will need bootstrap to download a toolchain at all.

Still verify, do not just apply

built with go1.26.5 was read off a host binary, not off the pinned artifact. Before relying on it:

Confirm the pinned v2.12.2 (commit c0d3ddc9cf3faa61a4e378e879ece580256d76e5) reports the same build toolchain — run golangci-lint version from the pinned artifact or image, not from whatever is on the host PATH.
Keep the DoD bullet requiring a container whose host Go is one minor newer than the new ceiling (i.e. Go 1.27 once it exists, or simulate it) to prove the window still rejects rather than reuses. A ceiling that has been raised but not re-tested is exactly how the original bug would come back.

Also relevant to the same reconciliation

PR #40 (feat/backend-dockerfile-lint-stage, open) replaces Dockerfile.backend's go install-from-source with a FROM golangci/golangci-lint@sha256:... lint stage. If #40 lands before #31, then #31's linter-version change becomes a lint-stage image digest bump rather than a go install commit-pin edit — a different hunk in a different place. Whoever performs the reconciliation should check which of #40/#31 landed first and look in the right place; #40's PR body documents both paths.

Net effect on item 5: three constants still move together, but only one of them is now genuinely unknown-until-measured, and that measurement is recorded above.

## Item 5's unknown is now answered: v2.12.2 is built with **go1.26.5** Item 5 said "determine what Go golangci-lint v2.12.2 was built with and re-derive the window from that — do not assume it is still 1.25." That was the trap, and it is no longer a research task. Issue #41 records the measurement directly: ``` golangci-lint has version 2.12.2 built with go1.26.5 ``` So the coupling resolves as follows. ### What the window becomes after #31 lands The constraint established by PR #38 is that the linter's **build** toolchain must be at least as new as the Go compiling the code — golangci-lint links `go/types` from whatever compiled it, and it panics with `file requires newer Go version goX.Y` when asked to parse a newer language version than it understands. | | linter build toolchain | resulting ceiling | | --- | --- | --- | | today, v2.7.2 | `go1.25.4` | `GO_MAX_MINOR="1.25"` | | after #31, v2.12.2 | `go1.26.5` | `GO_MAX_MINOR="1.26"` | So the reconciliation is a **ceiling raise**, not a pin move: `[1.25.5, 1.25.x]` becomes `[1.25.5, 1.26.x]`. The floor stays at `GO_MIN_VERSION=1.25.5` — that comes from `backend/go.mod`, which #31 does not touch. `GO_VERSION` (the version bootstrap installs when the host has nothing usable) does **not** have to move. 1.25.7 remains inside the widened window and still matches the digest-pinned `golang:1.25-alpine` builder. Moving it would be a separate, larger decision touching `Dockerfile.backend` too — do not fold that in. ### One pleasant consequence Widening the ceiling to 1.26 means a host that already has Go 1.26 — the case that produced the original B1 panic and cost a full review cycle — becomes a **reuse** case rather than an install case. Fewer machines will need bootstrap to download a toolchain at all. ### Still verify, do not just apply `built with go1.26.5` was read off a host binary, not off the pinned artifact. Before relying on it: - [ ] Confirm the **pinned** v2.12.2 (commit `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`) reports the same build toolchain — run `golangci-lint version` from the pinned artifact or image, not from whatever is on the host `PATH`. - [ ] Keep the DoD bullet requiring a container whose host Go is one minor **newer than the new ceiling** (i.e. Go 1.27 once it exists, or simulate it) to prove the window still rejects rather than reuses. A ceiling that has been raised but not re-tested is exactly how the original bug would come back. ### Also relevant to the same reconciliation PR #40 (`feat/backend-dockerfile-lint-stage`, open) replaces `Dockerfile.backend`'s `go install`-from-source with a `FROM golangci/golangci-lint@sha256:...` lint stage. If #40 lands before #31, then #31's linter-version change becomes a **lint-stage image digest bump** rather than a `go install` commit-pin edit — a different hunk in a different place. Whoever performs the reconciliation should check which of #40/#31 landed first and look in the right place; #40's PR body documents both paths. Net effect on item 5: three constants still move together, but only one of them is now genuinely unknown-until-measured, and that measurement is recorded above.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/netwatch#39

Follow-ups from PR #38: bootstrap re-extract guard, message attribution, and the #31 rebase reconciliation #39

1. Re-extract guard misses a deleted gofmt (N1)