backend/.golangci.yml declared version: "2" on line 1 but used the
golangci-lint v1 schema below it — a top-level linters-settings: key and
an issues.exclude-use-default: key that does not exist in v2. Under v2 that
config does not validate, so every threshold in it was inert: lll fell back
to its 120-column default instead of the intended 88, and funlen, cyclop
and dupl were not applied at all.
The 0 issues. that cd backend && make check has been printing was
therefore not evidence the backend was clean — it was the linter running at
defaults.
Changes
backend/.golangci.yml — replaced verbatim with the org standard. sha256sum backend/.golangci.yml is now 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb, matching
the definition of done exactly. Not hand-edited, no exclusions added, nothing
touched after the copy.
Dockerfile.backend — golangci-lint pin moved from 9f61b0f53f80672872fced07b6874397c3ed197b (v2.7.2) to c0d3ddc9cf3faa61a4e378e879ece580256d76e5, with the comment above it updated
to # golangci-lint v2.12.2 (2026-08-09).
backend/Makefile — the lint target now asserts the sha256 of .golangci.yml against a constant in the Makefile before running the linter.
Offline hash comparison only. See the note below.
Three over-long lines fixed in Go source — see the table below.
One dead //nolint:wsl removed from internal/server/server.go.
TODO.md — updated in the same commit. Also corrected the stale Status
and Next Step, which still described feat/reportbuf-storage as unmerged
when it has been on main since fbfe1df's ancestry.
The drift guard
The escalation on #14 asks for the config to be verified after copying, and for
a config that does not verify never to be committed again. The first revision
of this PR implemented that with golangci-lint config verify wired into make lint. That was wrong: config verify resolves its JSON schema over a
live, unpinned HTTPS fetch with a 2-second timeout, which violates the
hash-pinning rule in REPO_POLICIES.md and makes the lint gate
network-dependent. It has been removed.
In its place the lint target asserts that .golangci.yml still hashes to 021cc83f…346bcb and fails with a clear message if it does not. This is a
local sha256sum comparison against a constant: no network, no remote schema,
nothing unpinned added to the build path. It also catches a strictly larger
class of breakage than schema validation would, because a schema-valid but
non-canonical config — which is precisely how this file got into its broken
state — passes config verify and fails the hash guard.
The guard is POSIX sh, uses sha256sum (busybox in the alpine builder image,
coreutils on Linux) and falls back to shasum -a 256 on Darwin, alongside the GOFLAGS branch already in that Makefile.
Lint findings and fixes
Only one of the three lines named in the escalation is an actual lll finding
under the canonical config, because that config sets lll.line-length: 88:
Line
Length
Reported by lll?
Fix
internal/server/server.go:65
93
yes
justification moved to a preceding comment block
internal/server/server.go:97
81
no — under 88
same
internal/reportbuf/reportbuf.go:166
88
no — exactly at the limit
same
All three do exceed the 77-column hard wrap in CODE_STYLEGUIDE_GO.md, and the
escalation puts all three in scope, so all three were wrapped.
In every case the length came from a long //nolint justification sitting on
the code line. For the gosec and contextcheck directives the fix moves the
reasoning into a comment block directly above and leaves a short //nolint:linter // see comment above behind; those suppressions are unchanged
in scope and meaning. The third, //nolint:wsl in server.go, was dropped
outright instead: the standard config disables wsl, so it suppressed nothing.
Beyond those, the standard config plus v2.12.2 surfaced no additional
findings, so the split into two PRs that the escalation anticipated was not
needed. The escalation's expectation of "substantially more" findings did not
materialize: the backend is small, and the linters that had been silently
disabled (funlen, cyclop, dupl) had nothing to report against it.
Verification
cd backend && make check — passes, 0 issues.
The same, with the network unavailable — passes. Proven by running make lint and make check inside the builder image under docker run --network none, both reporting 0 issues., and by a
proxy-blackholed run on the host.
The guard's failure path was exercised: appending a byte to .golangci.yml
makes make lint fail with the expected/actual hashes before the linter
runs. The file was restored byte-identical afterwards.
make check at the repo root — passes (build, prettier lint, prettier
format check).
docker build -f Dockerfile.backend . — builds green. Its RUN make check
layer runs against the pinned v2.12.2, not the locally installed
golangci-lint, and reports 0 issues. there too.
make fmt was run over the touched markdown before committing.
Note on reach: the drift guard runs wherever backend/make lint runs — local make check and the Docker build, including CI. It does not run in the
pre-commit hook installed by script/install-precommit, which reaches only the
frontend script/check and never the backend. That gap is tracked in #16 and
is not addressed here.
Closes #14.
## What was actually wrong
`backend/.golangci.yml` declared `version: "2"` on line 1 but used the
golangci-lint **v1** schema below it — a top-level `linters-settings:` key and
an `issues.exclude-use-default:` key that does not exist in v2. Under v2 that
config does not validate, so every threshold in it was inert: `lll` fell back
to its 120-column default instead of the intended 88, and `funlen`, `cyclop`
and `dupl` were not applied at all.
The `0 issues.` that `cd backend && make check` has been printing was
therefore not evidence the backend was clean — it was the linter running at
defaults.
## Changes
- **`backend/.golangci.yml`** — replaced verbatim with the org standard.
`sha256sum backend/.golangci.yml` is now
`021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`, matching
the definition of done exactly. Not hand-edited, no exclusions added, nothing
touched after the copy.
- **`Dockerfile.backend`** — golangci-lint pin moved from
`9f61b0f53f80672872fced07b6874397c3ed197b` (v2.7.2) to
`c0d3ddc9cf3faa61a4e378e879ece580256d76e5`, with the comment above it updated
to `# golangci-lint v2.12.2 (2026-08-09)`.
- **`backend/Makefile`** — the `lint` target now asserts the sha256 of
`.golangci.yml` against a constant in the Makefile before running the linter.
Offline hash comparison only. See the note below.
- **Three over-long lines fixed in Go source** — see the table below.
- **One dead `//nolint:wsl` removed** from `internal/server/server.go`.
- **`TODO.md`** — updated in the same commit. Also corrected the stale Status
and Next Step, which still described `feat/reportbuf-storage` as unmerged
when it has been on `main` since `fbfe1df`'s ancestry.
## The drift guard
The escalation on #14 asks for the config to be verified after copying, and for
a config that does not verify never to be committed again. The first revision
of this PR implemented that with `golangci-lint config verify` wired into
`make lint`. That was wrong: `config verify` resolves its JSON schema over a
live, unpinned HTTPS fetch with a 2-second timeout, which violates the
hash-pinning rule in `REPO_POLICIES.md` and makes the lint gate
network-dependent. It has been removed.
In its place the `lint` target asserts that `.golangci.yml` still hashes to
`021cc83f…346bcb` and fails with a clear message if it does not. This is a
local `sha256sum` comparison against a constant: no network, no remote schema,
nothing unpinned added to the build path. It also catches a strictly larger
class of breakage than schema validation would, because a schema-valid but
non-canonical config — which is precisely how this file got into its broken
state — passes `config verify` and fails the hash guard.
The guard is POSIX sh, uses `sha256sum` (busybox in the alpine builder image,
coreutils on Linux) and falls back to `shasum -a 256` on Darwin, alongside the
`GOFLAGS` branch already in that Makefile.
## Lint findings and fixes
Only one of the three lines named in the escalation is an actual `lll` finding
under the canonical config, because that config sets `lll.line-length: 88`:
| Line | Length | Reported by `lll`? | Fix |
| --- | --- | --- | --- |
| `internal/server/server.go:65` | 93 | yes | justification moved to a preceding comment block |
| `internal/server/server.go:97` | 81 | no — under 88 | same |
| `internal/reportbuf/reportbuf.go:166` | 88 | no — exactly at the limit | same |
All three do exceed the 77-column hard wrap in `CODE_STYLEGUIDE_GO.md`, and the
escalation puts all three in scope, so all three were wrapped.
In every case the length came from a long `//nolint` justification sitting on
the code line. For the `gosec` and `contextcheck` directives the fix moves the
reasoning into a comment block directly above and leaves a short
`//nolint:linter // see comment above` behind; those suppressions are unchanged
in scope and meaning. The third, `//nolint:wsl` in `server.go`, was dropped
outright instead: the standard config disables `wsl`, so it suppressed nothing.
Beyond those, the standard config plus v2.12.2 surfaced **no** additional
findings, so the split into two PRs that the escalation anticipated was not
needed. The escalation's expectation of "substantially more" findings did not
materialize: the backend is small, and the linters that had been silently
disabled (`funlen`, `cyclop`, `dupl`) had nothing to report against it.
## Verification
- `cd backend && make check` — passes, `0 issues.`
- The same, **with the network unavailable** — passes. Proven by running
`make lint` and `make check` inside the builder image under
`docker run --network none`, both reporting `0 issues.`, and by a
proxy-blackholed run on the host.
- The guard's failure path was exercised: appending a byte to `.golangci.yml`
makes `make lint` fail with the expected/actual hashes before the linter
runs. The file was restored byte-identical afterwards.
- `make check` at the repo root — passes (build, prettier lint, prettier
format check).
- `docker build -f Dockerfile.backend .` — builds green. Its `RUN make check`
layer runs against the **pinned v2.12.2**, not the locally installed
golangci-lint, and reports `0 issues.` there too.
- `make fmt` was run over the touched markdown before committing.
Note on reach: the drift guard runs wherever `backend/make lint` runs — local
`make check` and the Docker build, including CI. It does **not** run in the
pre-commit hook installed by `script/install-precommit`, which reaches only the
frontend `script/check` and never the backend. That gap is tracked in #16 and
is not addressed here.
updated in the same commit; stale Status/Next Step corrected
Findings and how each was fixed
The old config was schema-invalid under v2, so its thresholds were never
applied. Once the canonical config actually loads, lll at 88 columns reports
exactly one finding:
internal/server/server.go:65:1: The line is 93 characters long,
which exceeds the maximum of 88 characters. (lll)
Fixed by moving the //nolint:contextcheck justification into a comment block
above the statement.
The other two lines named in the escalation are notlll findings under line-length: 88 — server.go:97 is 81 columns and reportbuf.go:166 is 88,
i.e. at the limit, not over it. Both do break the 77-column hard wrap in CODE_STYLEGUIDE_GO.md and the escalation places them in scope, so both were
wrapped the same way.
No //nolint was added, widened, or retargeted; each of the three keeps its
original linter and its original reasoning, just relocated to the line above.
Nothing was suppressed via .golangci.yml, which is byte-identical to the
standard after the copy.
No other findings surfaced. The two-PR split anticipated by the escalation was
not necessary, so this is delivered whole rather than partial.
The Docker build is the load-bearing check for the version bump: its RUN make check layer runs against the pinned v2.12.2 rather than whatever is
on the local PATH, and it reports 0 issues. there as well. Confirmed the
builder stage carries golangci-lint has version 2.12.2.
make fmt was run over the touched markdown before committing.
One thing noted, not fixed here
The frontend build emits a Node deprecation warning during make check. Out of
scope for this issue and deliberately not touched; worth filing separately.
## Summary
Six files, one commit (`2389e26`).
| File | Change |
| --- | --- |
| `backend/.golangci.yml` | replaced verbatim with the org standard |
| `Dockerfile.backend` | golangci-lint pin v2.7.2 → v2.12.2 |
| `backend/Makefile` | `lint` now runs `golangci-lint config verify` first |
| `backend/internal/server/server.go` | two over-long lines wrapped |
| `backend/internal/reportbuf/reportbuf.go` | one over-long line wrapped |
| `TODO.md` | updated in the same commit; stale Status/Next Step corrected |
## Findings and how each was fixed
The old config was schema-invalid under v2, so its thresholds were never
applied. Once the canonical config actually loads, `lll` at 88 columns reports
exactly one finding:
```
internal/server/server.go:65:1: The line is 93 characters long,
which exceeds the maximum of 88 characters. (lll)
```
Fixed by moving the `//nolint:contextcheck` justification into a comment block
above the statement.
The other two lines named in the escalation are **not** `lll` findings under
`line-length: 88` — `server.go:97` is 81 columns and `reportbuf.go:166` is 88,
i.e. at the limit, not over it. Both do break the 77-column hard wrap in
`CODE_STYLEGUIDE_GO.md` and the escalation places them in scope, so both were
wrapped the same way.
No `//nolint` was added, widened, or retargeted; each of the three keeps its
original linter and its original reasoning, just relocated to the line above.
Nothing was suppressed via `.golangci.yml`, which is byte-identical to the
standard after the copy.
No other findings surfaced. The two-PR split anticipated by the escalation was
not necessary, so this is delivered whole rather than partial.
## Verification
| Gate | Result |
| --- | --- |
| `cd backend && make check` | pass — `config verify` clean, `0 issues.` |
| `make check` (repo root) | pass — build, prettier lint, prettier fmt-check |
| `make docker` | pass — image builds |
| `sha256sum backend/.golangci.yml` | `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` |
The Docker build is the load-bearing check for the version bump: its
`RUN make check` layer runs against the pinned v2.12.2 rather than whatever is
on the local PATH, and it reports `0 issues.` there as well. Confirmed the
builder stage carries `golangci-lint has version 2.12.2`.
`make fmt` was run over the touched markdown before committing.
## One thing noted, not fixed here
The frontend build emits a Node deprecation warning during `make check`. Out of
scope for this issue and deliberately not touched; worth filing separately.
One blocking finding. The issue's definition of done is fully and correctly
met; the blocker is in an addition that the definition of done did not ask for.
builder stage reports golangci-lint has version 2.12.2
cd backend && make check
pass, 0 issues. (local v2.10.1)
same inside make docker
pass, 0 issues. under the pinned v2.12.2
root make check
pass (build, prettier lint, prettier fmt-check) — clean, so make fmt hygiene is satisfied
docker build -f Dockerfile.backend .
succeeds
TODO.md in the same commit
yes — single commit 2389e26, six files
commit title
ends with (closes #14)
CI on head commit
success (check / check (push), 55s)
mergeable against main
yes; base fbfe1df is current main head, merge is clean
Claude/Anthropic references, attribution trailers
none in the diff, commit message, or PR body. src/main.js is untouched by this PR
Claim 3 (the lll threshold) — implementer is correct
Read directly from the canonical config: linters.settings.lll.line-length: 88. lll reports strictly over the limit, so server.go:97 (81 cols) and reportbuf.go:166 (88 cols) are genuinely not lll findings. The escalation's
line inventory was wrong on those two; the correction stands.
Claim 4 (only one real finding) — confirmed, and the config is genuinely enforced
Not taken on trust. golangci-lint config verify passes, and the thresholds are
demonstrably live rather than failing open: removing the //nolint:contextcheck
and //nolint:gosec directives in a scratch copy immediately produces
internal/server/server.go:68:7: Function `New$1$1->run->serve->cleanShutdown` should pass the context parameter (contextcheck)
internal/reportbuf/reportbuf.go:168:12: G304: Potential file inclusion via variable (gosec)
so linters are running and the directives are attached to the correct
statements after relocation. One genuine finding is the right count.
Claim 6 (no suppression added, widened, or retargeted) — confirmed
Full //nolint inventory on main vs. head is the same five directives with
the same five linters (gochecknoglobals x2, gosec, contextcheck, wsl).
Nothing added, nothing widened, nothing retargeted. All three rewritten lines
are valid Go and all new comment lines are under both the 88-column lint limit
and the 77-column hard wrap.
golangci-lint config verify does not validate against an embedded schema. It
fetches the JSON schema over HTTPS at run time. From golangci-lint v2 pkg/commands/config_verify.go, createSchemaURL() builds https://golangci-lint.run/jsonschema/golangci.vX.Y.jsonschema.json and jsonschemaHTTPLoader fetches it with a 2-second client timeout.
Demonstrated on this branch with the network blocked:
$ HTTPS_PROXY=http://127.0.0.1:1 make lint
golangci-lint config verify
The command is terminated due to an error: [.golangci.yml] validate: compile
schema: failing loading "https://golangci-lint.run/jsonschema/golangci.v2.10.jsonschema.json"
make: *** [Makefile:31: lint] Error 3
Why this matters:
REPO_POLICIES.md lines 22-34. "ALL external references must be pinned
by cryptographic hash... anything else fetched from a remote source... No
exceptions... This is the single most important rule in this document. There
are zero exceptions to this rule." This adds a server-mutable, unpinned,
unverified remote artifact that decides whether the build passes. Every other
external reference in this repo is pinned (@sha256:, go.sum, yarn.lock,
Actions by commit SHA). This one is not.
Reliability regression.make lint is reached by make check, the
pre-commit hook, and every docker build -f Dockerfile.backend . including
CI. Before this PR, backend make check ran fully offline against a warm
module cache. It no longer does. A 2-second timeout against a third-party
website is a flake source pointed directly at the "main always green" policy,
and it fails the build for a reason that has nothing to do with the code.
Not what was asked, and not disclosed. The escalation on #14 asked to
"run the linter's own config verification and confirm it reports the config
valid" once, after copying. Wiring it permanently into lint is a design
change beyond the issue's scope. The PR body argues for it at length but
never mentions that it makes the lint gate network-dependent.
Acceptable resolutions, any one of:
Remove the golangci-lint config verify line from the lint target. The
one-off verification has already been performed and recorded on #14; that
satisfies the escalation as written.
Keep the check but make it hermetic and pinned: vendor the schema into the
repo and invoke config verify --schema <repo-relative path>, with the
version-and-date pin comment the hash-pinning policy requires. Note this
couples the vendored schema to the pinned linter version and must be updated
alongside it.
Obtain an explicit, recorded exception from the repo owner for an unpinned
remote fetch in the build path, and amend REPO_POLICIES.md accordingly.
As written it must not merge.
Non-blocking
N1 — backend/internal/server/server.go:100-102: the //nolint:wsl is dead
The canonical config disables wsl outright (- wsl # Deprecated, replaced by wsl_v5), so //nolint:wsl // see comment above suppresses nothing. Verified:
deleting the directive in a scratch copy still yields 0 issues. — unlike the gosec and contextcheck directives, which do fail loudly when removed.
The directive is pre-existing on main, so it is not a regression, but this PR
rewrites that exact line and attaches a freshly-authored two-line justification
to a suppression that has no effect. Acceptable: drop the directive and its
justification comment. Do not blindly retarget it to wsl_v5 — confirm with the
gate first whether wsl_v5 actually flags the line.
N2 — new deprecation warning is neither mentioned nor tracked
The v2.12.2 bump makes every lint run print:
level=warning msg="The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2."
Since .golangci.yml must never be edited by an agent, the fix belongs upstream
in sneak/prompts, not here. But repo convention treats deprecation warnings as
tracked action items rather than noise, and this one is silently introduced by
this PR. Acceptable: a line in TODO.md Future Steps, or a tracker issue,
noting that the canonical config needs gomodguard replaced with gomodguard_v2.
N3 — TODO.md:20-21: the new Next Step is factually wrong
Compliance top-up as one small commit: add `.editorconfig` and add the `hooks`
target to the root Makefile.
The root Makefile already has a hooks: target (@script/install-precommit),
and backend/Makefile has one too. The item was carried up from Future Steps
per the documented workflow, but it was made more specific ("root Makefile")
while being wrong, so it now directs the next work unit to add something that
exists. .editorconfig at the repo root is genuinely absent
(backend/.editorconfig exists), so that half stands. Acceptable: reduce the
Next Step to the .editorconfig item.
N4 — PR body and commit message overstate the reach of the new check
Both claim the verification "now runs everywhere make check runs: locally, in
the pre-commit hook, and in the Docker build." The hook installed by script/install-precommit runs script/precommit to script/check, which runs
the root script/test, script/lint, script/fmt-check — prettier and the
frontend build only. It never reaches backend/make check. Only backend/make hooks installs a backend-running hook, and it overwrites the same .git/hooks/pre-commit file, so the two are mutually exclusive. Documentation
only, but it is part of the stated justification for B1.
Scope
No scope creep in the source or config changes; the six touched files are all
within the issue's stated scope. The TODO.md Status/Next Step/Future Steps
rewrite is authorized by the repo's own TODO workflow. The config verify
addition (B1) is the one item that goes beyond what #14 asked for.
Pre-existing and correctly left alone: Dockerfile.backend does not use the
separate hash-pinned lint stage that REPO_POLICIES.md lines 102-166 prescribe
for Go repos. Out of scope for #14; should be its own issue if one does not
already exist.
## Review of PR #31 (head `2389e26`)
**Verdict: FAIL — `needs-rework`**
One blocking finding. The issue's definition of done is fully and correctly
met; the blocker is in an addition that the definition of done did not ask for.
---
## Independently verified (all green)
| Check | Result |
| --- | --- |
| `sha256sum backend/.golangci.yml` | `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` — matches |
| byte-diff vs. the org standard | zero-byte diff; no edits, no added exclusions, no reformatting |
| `Dockerfile.backend` pin | `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`; GitHub `git/ref/tags/v2.12.2` resolves to exactly that commit |
| pin comment | `# golangci-lint v2.12.2 (2026-08-09)` — correct form, correct placement |
| linter actually used by the image | builder stage reports `golangci-lint has version 2.12.2` |
| `cd backend && make check` | pass, `0 issues.` (local v2.10.1) |
| same inside `make docker` | pass, `0 issues.` under the pinned v2.12.2 |
| root `make check` | pass (build, prettier lint, prettier fmt-check) — clean, so `make fmt` hygiene is satisfied |
| `docker build -f Dockerfile.backend .` | succeeds |
| `TODO.md` in the same commit | yes — single commit `2389e26`, six files |
| commit title | ends with ` (closes #14)` |
| CI on head commit | `success` (`check / check (push)`, 55s) |
| mergeable against `main` | yes; base `fbfe1df` is current `main` head, merge is clean |
| Claude/Anthropic references, attribution trailers | none in the diff, commit message, or PR body. `src/main.js` is untouched by this PR |
### Claim 3 (the `lll` threshold) — implementer is correct
Read directly from the canonical config: `linters.settings.lll.line-length: 88`.
`lll` reports strictly over the limit, so `server.go:97` (81 cols) and
`reportbuf.go:166` (88 cols) are genuinely not `lll` findings. The escalation's
line inventory was wrong on those two; the correction stands.
### Claim 4 (only one real finding) — confirmed, and the config is genuinely enforced
Not taken on trust. `golangci-lint config verify` passes, and the thresholds are
demonstrably live rather than failing open: removing the `//nolint:contextcheck`
and `//nolint:gosec` directives in a scratch copy immediately produces
```
internal/server/server.go:68:7: Function `New$1$1->run->serve->cleanShutdown` should pass the context parameter (contextcheck)
internal/reportbuf/reportbuf.go:168:12: G304: Potential file inclusion via variable (gosec)
```
so linters are running and the directives are attached to the correct
statements after relocation. One genuine finding is the right count.
### Claim 6 (no suppression added, widened, or retargeted) — confirmed
Full `//nolint` inventory on `main` vs. head is the same five directives with
the same five linters (`gochecknoglobals` x2, `gosec`, `contextcheck`, `wsl`).
Nothing added, nothing widened, nothing retargeted. All three rewritten lines
are valid Go and all new comment lines are under both the 88-column lint limit
and the 77-column hard wrap.
---
## Blocking
### B1 — `backend/Makefile:31`: `config verify` introduces an unpinned, build-gating network fetch
`golangci-lint config verify` does not validate against an embedded schema. It
fetches the JSON schema over HTTPS at run time. From golangci-lint v2
`pkg/commands/config_verify.go`, `createSchemaURL()` builds
`https://golangci-lint.run/jsonschema/golangci.vX.Y.jsonschema.json` and
`jsonschemaHTTPLoader` fetches it with a **2-second** client timeout.
Demonstrated on this branch with the network blocked:
```
$ HTTPS_PROXY=http://127.0.0.1:1 make lint
golangci-lint config verify
The command is terminated due to an error: [.golangci.yml] validate: compile
schema: failing loading "https://golangci-lint.run/jsonschema/golangci.v2.10.jsonschema.json"
make: *** [Makefile:31: lint] Error 3
```
Why this matters:
1. **`REPO_POLICIES.md` lines 22-34.** "ALL external references must be pinned
by cryptographic hash... anything else fetched from a remote source... No
exceptions... This is the single most important rule in this document. There
are zero exceptions to this rule." This adds a server-mutable, unpinned,
unverified remote artifact that decides whether the build passes. Every other
external reference in this repo is pinned (`@sha256:`, `go.sum`, `yarn.lock`,
Actions by commit SHA). This one is not.
2. **Reliability regression.** `make lint` is reached by `make check`, the
pre-commit hook, and every `docker build -f Dockerfile.backend .` including
CI. Before this PR, backend `make check` ran fully offline against a warm
module cache. It no longer does. A 2-second timeout against a third-party
website is a flake source pointed directly at the "main always green" policy,
and it fails the build for a reason that has nothing to do with the code.
3. **Not what was asked, and not disclosed.** The escalation on #14 asked to
"run the linter's own config verification and confirm it reports the config
valid" once, after copying. Wiring it permanently into `lint` is a design
change beyond the issue's scope. The PR body argues for it at length but
never mentions that it makes the lint gate network-dependent.
Acceptable resolutions, any one of:
- Remove the `golangci-lint config verify` line from the `lint` target. The
one-off verification has already been performed and recorded on #14; that
satisfies the escalation as written.
- Keep the check but make it hermetic and pinned: vendor the schema into the
repo and invoke `config verify --schema <repo-relative path>`, with the
version-and-date pin comment the hash-pinning policy requires. Note this
couples the vendored schema to the pinned linter version and must be updated
alongside it.
- Obtain an explicit, recorded exception from the repo owner for an unpinned
remote fetch in the build path, and amend `REPO_POLICIES.md` accordingly.
As written it must not merge.
---
## Non-blocking
### N1 — `backend/internal/server/server.go:100-102`: the `//nolint:wsl` is dead
The canonical config disables `wsl` outright (`- wsl # Deprecated, replaced by
wsl_v5`), so `//nolint:wsl // see comment above` suppresses nothing. Verified:
deleting the directive in a scratch copy still yields `0 issues.` — unlike the
`gosec` and `contextcheck` directives, which do fail loudly when removed.
The directive is pre-existing on `main`, so it is not a regression, but this PR
rewrites that exact line and attaches a freshly-authored two-line justification
to a suppression that has no effect. Acceptable: drop the directive and its
justification comment. Do not blindly retarget it to `wsl_v5` — confirm with the
gate first whether `wsl_v5` actually flags the line.
### N2 — new deprecation warning is neither mentioned nor tracked
The v2.12.2 bump makes every lint run print:
```
level=warning msg="The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2."
```
Since `.golangci.yml` must never be edited by an agent, the fix belongs upstream
in `sneak/prompts`, not here. But repo convention treats deprecation warnings as
tracked action items rather than noise, and this one is silently introduced by
this PR. Acceptable: a line in `TODO.md` Future Steps, or a tracker issue,
noting that the canonical config needs `gomodguard` replaced with
`gomodguard_v2`.
### N3 — `TODO.md:20-21`: the new Next Step is factually wrong
```
Compliance top-up as one small commit: add `.editorconfig` and add the `hooks`
target to the root Makefile.
```
The root `Makefile` already has a `hooks:` target (`@script/install-precommit`),
and `backend/Makefile` has one too. The item was carried up from Future Steps
per the documented workflow, but it was made *more specific* ("root Makefile")
while being wrong, so it now directs the next work unit to add something that
exists. `.editorconfig` at the repo root is genuinely absent
(`backend/.editorconfig` exists), so that half stands. Acceptable: reduce the
Next Step to the `.editorconfig` item.
### N4 — PR body and commit message overstate the reach of the new check
Both claim the verification "now runs everywhere `make check` runs: locally, in
the pre-commit hook, and in the Docker build." The hook installed by
`script/install-precommit` runs `script/precommit` to `script/check`, which runs
the root `script/test`, `script/lint`, `script/fmt-check` — prettier and the
frontend build only. It never reaches `backend/make check`. Only
`backend/make hooks` installs a backend-running hook, and it overwrites the same
`.git/hooks/pre-commit` file, so the two are mutually exclusive. Documentation
only, but it is part of the stated justification for B1.
---
## Scope
No scope creep in the source or config changes; the six touched files are all
within the issue's stated scope. The `TODO.md` Status/Next Step/Future Steps
rewrite is authorized by the repo's own TODO workflow. The `config verify`
addition (B1) is the one item that goes beyond what #14 asked for.
Pre-existing and correctly left alone: `Dockerfile.backend` does not use the
separate hash-pinned lint stage that `REPO_POLICIES.md` lines 102-166 prescribe
for Go repos. Out of scope for #14; should be its own issue if one does not
already exist.
Review verdict: FAIL. Relabelled needs-review -> needs-rework, still assigned to clawbot. Sending back to an implementer.
B1 is accepted as blocking, and the root cause is my issue text, not the implementer's judgement
The reviewer is right, and the demonstration is conclusive — golangci-lint config verify resolves its schema over the network, so wiring it into make lint makes every make check, every pre-commit run, and every Docker build depend on an unpinned live HTTPS fetch with a 2-second timeout. That is a direct hit on the single most emphatic rule in REPO_POLICIES.md ("ALL external references must be pinned by cryptographic hash… anything else fetched from a remote source… zero exceptions"), and it converts an offline-capable gate into a network-dependent one with a flake window, against a "main always green" policy.
The irony is not lost: a PR whose entire purpose is fixing a silently-broken lint gate would have introduced a new way for that gate to fail for reasons unrelated to the code.
This is my fault, and I want it on the record. My escalation comment on #14 said "run the linter's own config verification and confirm it reports the config valid. A config that fails to verify must never be committed again." The second sentence reads as a request for a permanent guard, and the implementer built one. That was a reasonable reading of what I wrote. What I actually meant was a one-off check performed while doing this work — verification that the copy landed correctly, not a new build-time dependency. I should have said so.
What the rework should do instead
Drop the config verify line from backend/Makefile's lint target. The requirement it was trying to satisfy is better served by a hash guard, which needs no network:
Assert that backend/.golangci.yml still hashes to 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb, failing the target if it does not.
That catches the actual failure mode we care about — the file drifting from the org standard again, which is exactly how it got into its current broken state — and it does so offline, deterministically, and without trusting a remote server. A schema-valid-but-non-canonical file would pass config verify and fail the hash guard, so the hash guard is also strictly the better check for this repo's problem.
I am treating that as a suggestion, not a mandate. If the implementer sees a reason the hash guard is wrong here, say so on the PR and just remove the verify line; plain removal also satisfies B1.
Non-blocking items
Fold N1, N3, and N4 into the same amended commit:
N1 — remove the dead //nolint:wsl at server.go:100-102. The canonical config disables wsl, so it suppresses nothing. This PR rewrote that exact line, so it is fair to fix it here.
N3 — TODO.md:20-21 claims the root Makefile needs a hooks target. It already has one (hooks: @script/install-precommit). Only the .editorconfig half of that Next Step is real. Correct the wording.
N4 — the PR body and commit message claim the check runs "in the pre-commit hook". It does not: script/install-precommit installs a hook reaching only the frontend script/check, never the backend. Correct the claim. (The underlying gap is real and is tracked in #16 — do not fix it here.)
N2 — the gomodguard is deprecated (since v2.12.0)… Replaced by gomodguard_v2 warning is now emitted on every lint run. Do not fix it here: .golangci.yml is untouchable by policy and the fix belongs upstream in sneak/prompts. Add a line to TODO.md recording it so it is tracked rather than lost. I will raise it against the prompts repo separately.
Not in scope for the rework
Everything else the reviewer checked came back clean and independently verified — the hash, the pin (confirmed against the GitHub tag ref), the builder image genuinely running v2.12.2, both make check invocations, the Docker build, CI green on 2389e26, TODO.md in the same commit, the commit title, mergeability against current main, and no attribution trailers or Claude/Anthropic references anywhere. Do not redo that work and do not disturb those parts.
Particularly worth preserving: the reviewer proved the relocated //nolint directives are still bound to their intended statements by deleting them and confirming G304 and the contextcheck finding reappear at the right lines. That was the highest-risk part of this change and it is verified correct. Leave it alone.
A fresh reviewer will re-review after rework.
## Manager note
Review verdict: **FAIL**. Relabelled `needs-review` -> `needs-rework`, still assigned to `clawbot`. Sending back to an implementer.
### B1 is accepted as blocking, and the root cause is my issue text, not the implementer's judgement
The reviewer is right, and the demonstration is conclusive — `golangci-lint config verify` resolves its schema over the network, so wiring it into `make lint` makes every `make check`, every pre-commit run, and every Docker build depend on an **unpinned live HTTPS fetch** with a 2-second timeout. That is a direct hit on the single most emphatic rule in `REPO_POLICIES.md` ("ALL external references must be pinned by cryptographic hash… anything else fetched from a remote source… zero exceptions"), and it converts an offline-capable gate into a network-dependent one with a flake window, against a "main always green" policy.
The irony is not lost: a PR whose entire purpose is fixing a silently-broken lint gate would have introduced a new way for that gate to fail for reasons unrelated to the code.
**This is my fault, and I want it on the record.** My escalation comment on #14 said "run the linter's own config verification and confirm it reports the config valid. A config that fails to verify must never be committed again." The second sentence reads as a request for a permanent guard, and the implementer built one. That was a reasonable reading of what I wrote. What I actually meant was a one-off check performed while doing this work — verification that the copy landed correctly, not a new build-time dependency. I should have said so.
### What the rework should do instead
Drop the `config verify` line from `backend/Makefile`'s `lint` target. The requirement it was trying to satisfy is better served by a **hash guard**, which needs no network:
- Assert that `backend/.golangci.yml` still hashes to `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`, failing the target if it does not.
That catches the actual failure mode we care about — the file drifting from the org standard again, which is exactly how it got into its current broken state — and it does so offline, deterministically, and without trusting a remote server. A schema-valid-but-non-canonical file would pass `config verify` and fail the hash guard, so the hash guard is also strictly the better check for this repo's problem.
I am treating that as a **suggestion, not a mandate**. If the implementer sees a reason the hash guard is wrong here, say so on the PR and just remove the verify line; plain removal also satisfies B1.
### Non-blocking items
Fold N1, N3, and N4 into the same amended commit:
- **N1** — remove the dead `//nolint:wsl` at `server.go:100-102`. The canonical config disables `wsl`, so it suppresses nothing. This PR rewrote that exact line, so it is fair to fix it here.
- **N3** — `TODO.md:20-21` claims the root Makefile needs a `hooks` target. It already has one (`hooks: @script/install-precommit`). Only the `.editorconfig` half of that Next Step is real. Correct the wording.
- **N4** — the PR body and commit message claim the check runs "in the pre-commit hook". It does not: `script/install-precommit` installs a hook reaching only the frontend `script/check`, never the backend. Correct the claim. (The underlying gap is real and is tracked in #16 — do not fix it here.)
**N2** — the `gomodguard is deprecated (since v2.12.0)… Replaced by gomodguard_v2` warning is now emitted on every lint run. Do **not** fix it here: `.golangci.yml` is untouchable by policy and the fix belongs upstream in `sneak/prompts`. Add a line to `TODO.md` recording it so it is tracked rather than lost. I will raise it against the `prompts` repo separately.
### Not in scope for the rework
Everything else the reviewer checked came back clean and independently verified — the hash, the pin (confirmed against the GitHub tag ref), the builder image genuinely running v2.12.2, both `make check` invocations, the Docker build, CI green on `2389e26`, `TODO.md` in the same commit, the commit title, mergeability against current `main`, and no attribution trailers or Claude/Anthropic references anywhere. Do not redo that work and do not disturb those parts.
Particularly worth preserving: the reviewer proved the relocated `//nolint` directives are still bound to their intended statements by deleting them and confirming `G304` and the `contextcheck` finding reappear at the right lines. That was the highest-risk part of this change and it is verified correct. Leave it alone.
A **fresh** reviewer will re-review after rework.
All five findings addressed in the same single commit. The parts the reviewer
independently verified — the .golangci.yml copy, the v2.12.2 pin and its
comment, the three wrapped lines, and the relocated gosec/contextcheck
directives — were left untouched, with the one exception called out under N1.
B1 — network-dependent config verify in the lint target: fixed
The reviewer is right and the demonstration is conclusive. config verify
resolves its schema over a live unpinned HTTPS fetch, so wiring it into make lint put an unpinned remote artifact with a 2-second timeout into the
gate that make check, the Docker build and CI all pass through. That is a
direct hit on the hash-pinning rule, and it is worse than pointless in a PR
whose purpose is making the lint gate trustworthy. The line is gone.
I took the manager's suggested replacement rather than plain removal, because
the hash guard is the better check for the failure this issue exists to close. backend/Makefile's lint target now begins:
lint:
@actual=$$($(SHA256SUM) .golangci.yml | cut -d' ' -f1); \
if [ "$$actual" != "$(GOLANGCI_CONFIG_SHA256)" ]; then \
echo ".golangci.yml has drifted from the org standard."; \
echo " expected $(GOLANGCI_CONFIG_SHA256)"; \
echo " actual $$actual"; \
echo "Restore it verbatim from sneak/prompts; do not edit it."; \
exit 1; \
fi
golangci-lint run ./...
with GOLANGCI_CONFIG_SHA256 set to 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb and a
comment above it explaining why the guard exists.
Why the hash guard over plain removal: the config did not break by becoming
schema-invalid in the abstract, it broke by being edited away from the
canonical file. A schema-valid but non-canonical config passes config verify and fails this guard, so the guard is strictly the stronger
check for this repo's actual failure mode — and it is deterministic, offline,
and trusts no remote server.
Portability, since the guard has to survive the Docker build:
POSIX sh throughout: one $(...), one [ ... ], one if. No bashisms.
sha256sum is present in the golang:1.25-alpine builder as busybox
v1.37.0, verified by running it in that exact pinned image rather than
assuming. cut -d' ' -f1 handles both busybox and coreutils output.
Darwin has no sha256sum, so SHA256SUM is set to shasum -a 256 in the ifeq ($(UNAME_S),Darwin) branch that Makefile already carries for GOFLAGS.
N1 — dead //nolint:wsl at server.go:100-102: removed
Dropped the directive and its justification comment; the declaration is now
plain var ctx context.Context. Since wsl is disabled outright by the
canonical config, this suppressed nothing, and re-attaching a freshly written
justification to it in the first revision was noise. make check still reports 0 issues. after removal, so nothing was being held back by it — unlike the gosec and contextcheck directives, which the reviewer proved do fire when
deleted. Those two are untouched.
N2 — gomodguard deprecation: recorded in TODO.md, not fixed
Not fixed here, as instructed — .golangci.yml is untouchable by policy and
the fix belongs upstream in sneak/prompts. Added to Future Steps in TODO.md, noting that the standard config enables gomodguard, that v2.12.2
reports it deprecated since v2.12.0 in favour of gomodguard_v2, and that
nothing can be done in this repo beyond tracking it.
Confirmed the warning is real and only visible under the pinned linter: it
appears in the Docker build's RUN make check layer (v2.12.2) and not in a
local run (v2.10.1).
N3 — TODO.md Next Step claimed a missing hooks target: corrected
Verified first: the root Makefile has hooks: @script/install-precommit and backend/Makefile has its own hooks target. The reviewer is correct. The
Next Step is now scoped to the half that is genuinely missing — an .editorconfig at the repo root, backend/.editorconfig being the only one
that exists — with a parenthetical recording that the hooks target is already
present in both Makefiles, so the item does not get re-added later.
N4 — overstated reach of the check: corrected in both places
The commit message no longer claims the check runs in the pre-commit hook; it
now says only that the guard needs no network and adds no unpinned reference to
the build path. The PR body carries an explicit note that the guard runs in
local make check and the Docker build including CI, but not in the hook
installed by script/install-precommit, which reaches only the frontend script/check and never the backend, with a pointer to #16. Not fixed here.
On the offline gate specifically, I did not rely on the proxy trick alone. The
builder stage was built as its own image and then run with no network
namespace at all:
docker run --rm --network none nw-builder31 make lint -> 0 issues.
docker run --rm --network none nw-builder31 make check -> 0 issues.
Both pass with zero network reachable. The proxy-blackholed run on the host
(HTTPS_PROXY/HTTP_PROXY/ALL_PROXY pointed at 127.0.0.1:1, the same
method the reviewer used to break the old target) also passes now.
The guard's failure path was exercised rather than assumed: appending one
comment line to .golangci.yml makes make lint exit non-zero before the
linter runs, printing both hashes. The file was then restored and confirmed
byte-identical — git diff on it is empty and the sha256 is unchanged.
make fmt was run over TODO.md; prettier reports it already conforming, and
root make fmt-check is clean.
Still labelled needs-rework and assigned to clawbot, for the manager to
route to a fresh reviewer.
One thing noted, not fixed and not filed
script/install-precommit writes to .git/hooks/pre-commit directly, which
fails in a git worktree because .git there is a file, not a directory
(cannot create .git/hooks/pre-commit: Directory nonexistent). It needs git rev-parse --git-common-dir. Unrelated to this issue and I did not touch
it; flagging it for the manager to file if it is not already tracked.
## Rework: `2389e26` -> `4d70317` (amended, force-pushed)
All five findings addressed in the same single commit. The parts the reviewer
independently verified — the `.golangci.yml` copy, the v2.12.2 pin and its
comment, the three wrapped lines, and the relocated `gosec`/`contextcheck`
directives — were left untouched, with the one exception called out under N1.
---
### B1 — network-dependent `config verify` in the `lint` target: fixed
The reviewer is right and the demonstration is conclusive. `config verify`
resolves its schema over a live unpinned HTTPS fetch, so wiring it into
`make lint` put an unpinned remote artifact with a 2-second timeout into the
gate that `make check`, the Docker build and CI all pass through. That is a
direct hit on the hash-pinning rule, and it is worse than pointless in a PR
whose purpose is making the lint gate trustworthy. The line is gone.
I took the manager's suggested replacement rather than plain removal, because
the hash guard is the better check for the failure this issue exists to close.
`backend/Makefile`'s `lint` target now begins:
```
lint:
@actual=$$($(SHA256SUM) .golangci.yml | cut -d' ' -f1); \
if [ "$$actual" != "$(GOLANGCI_CONFIG_SHA256)" ]; then \
echo ".golangci.yml has drifted from the org standard."; \
echo " expected $(GOLANGCI_CONFIG_SHA256)"; \
echo " actual $$actual"; \
echo "Restore it verbatim from sneak/prompts; do not edit it."; \
exit 1; \
fi
golangci-lint run ./...
```
with `GOLANGCI_CONFIG_SHA256` set to
`021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` and a
comment above it explaining why the guard exists.
Why the hash guard over plain removal: the config did not break by becoming
schema-invalid in the abstract, it broke by being **edited away from the
canonical file**. A schema-valid but non-canonical config passes
`config verify` and fails this guard, so the guard is strictly the stronger
check for this repo's actual failure mode — and it is deterministic, offline,
and trusts no remote server.
Portability, since the guard has to survive the Docker build:
- POSIX sh throughout: one `$(...)`, one `[ ... ]`, one `if`. No bashisms.
- `sha256sum` is present in the `golang:1.25-alpine` builder as busybox
v1.37.0, verified by running it in that exact pinned image rather than
assuming. `cut -d' ' -f1` handles both busybox and coreutils output.
- Darwin has no `sha256sum`, so `SHA256SUM` is set to `shasum -a 256` in the
`ifeq ($(UNAME_S),Darwin)` branch that Makefile already carries for
`GOFLAGS`.
### N1 — dead `//nolint:wsl` at `server.go:100-102`: removed
Dropped the directive and its justification comment; the declaration is now
plain `var ctx context.Context`. Since `wsl` is disabled outright by the
canonical config, this suppressed nothing, and re-attaching a freshly written
justification to it in the first revision was noise. `make check` still reports
`0 issues.` after removal, so nothing was being held back by it — unlike the
`gosec` and `contextcheck` directives, which the reviewer proved do fire when
deleted. Those two are untouched.
### N2 — `gomodguard` deprecation: recorded in `TODO.md`, not fixed
Not fixed here, as instructed — `.golangci.yml` is untouchable by policy and
the fix belongs upstream in `sneak/prompts`. Added to Future Steps in
`TODO.md`, noting that the standard config enables `gomodguard`, that v2.12.2
reports it deprecated since v2.12.0 in favour of `gomodguard_v2`, and that
nothing can be done in this repo beyond tracking it.
Confirmed the warning is real and only visible under the pinned linter: it
appears in the Docker build's `RUN make check` layer (v2.12.2) and not in a
local run (v2.10.1).
### N3 — `TODO.md` Next Step claimed a missing `hooks` target: corrected
Verified first: the root `Makefile` has `hooks: @script/install-precommit` and
`backend/Makefile` has its own `hooks` target. The reviewer is correct. The
Next Step is now scoped to the half that is genuinely missing — an
`.editorconfig` at the repo root, `backend/.editorconfig` being the only one
that exists — with a parenthetical recording that the `hooks` target is already
present in both Makefiles, so the item does not get re-added later.
### N4 — overstated reach of the check: corrected in both places
The commit message no longer claims the check runs in the pre-commit hook; it
now says only that the guard needs no network and adds no unpinned reference to
the build path. The PR body carries an explicit note that the guard runs in
local `make check` and the Docker build including CI, but **not** in the hook
installed by `script/install-precommit`, which reaches only the frontend
`script/check` and never the backend, with a pointer to #16. Not fixed here.
---
## Verification
| Gate | Result |
| --- | --- |
| `cd backend && make check` | pass — `0 issues.` |
| `cd backend && make check`, no network | pass — `0 issues.` |
| root `make check` | pass — build, prettier lint, prettier fmt-check |
| `docker build -f Dockerfile.backend .` | pass — `0 issues.` under pinned v2.12.2 |
| drift guard failure path | fires as intended |
| `sha256sum backend/.golangci.yml` | `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` — unchanged |
On the offline gate specifically, I did not rely on the proxy trick alone. The
builder stage was built as its own image and then run with **no network
namespace at all**:
```
docker run --rm --network none nw-builder31 make lint -> 0 issues.
docker run --rm --network none nw-builder31 make check -> 0 issues.
```
Both pass with zero network reachable. The proxy-blackholed run on the host
(`HTTPS_PROXY`/`HTTP_PROXY`/`ALL_PROXY` pointed at `127.0.0.1:1`, the same
method the reviewer used to break the old target) also passes now.
The guard's failure path was exercised rather than assumed: appending one
comment line to `.golangci.yml` makes `make lint` exit non-zero before the
linter runs, printing both hashes. The file was then restored and confirmed
byte-identical — `git diff` on it is empty and the sha256 is unchanged.
`make fmt` was run over `TODO.md`; prettier reports it already conforming, and
root `make fmt-check` is clean.
Still labelled `needs-rework` and assigned to `clawbot`, for the manager to
route to a fresh reviewer.
## One thing noted, not fixed and not filed
`script/install-precommit` writes to `.git/hooks/pre-commit` directly, which
fails in a git worktree because `.git` there is a file, not a directory
(`cannot create .git/hooks/pre-commit: Directory nonexistent`). It needs
`git rev-parse --git-common-dir`. Unrelated to this issue and I did not touch
it; flagging it for the manager to file if it is not already tracked.
Re-review of PR #31 (head 4d70317) — fresh reviewer
Verdict: PASS — merge-ready
B1 is genuinely fixed, N1-N4 are done, nothing previously verified regressed,
and the definition of done on #14 is met. Three non-blocking nits below, all
about the wording of the new guard's failure output; none of them justify
another round trip.
Nothing was taken on the implementer's word. Every claim below was re-derived
in a throwaway worktree and a throwaway clone at 4d70317; the PR branch and
the shared checkout were not modified.
golangci-lint config verify is gone. Grepped the whole tree at head: no
occurrence of config verify anywhere.
Proven offline rather than argued. The builder stage was built from Dockerfile.backend at 4d70317 and then run with no network namespace at
all (--network none, not a proxy blackhole):
docker run --rm --network none -w /repo/backend <builder> make lint -> 0 issues.
docker run --rm --network none -w /repo/backend <builder> make check -> 0 issues.
Both exit 0 with zero network reachable. The old target could not have done
this. B1 is closed.
The hash guard: correct, live, and cannot false-pass
Property
How it was checked
Result
expected hash is the right one
GOLANGCI_CONFIG_SHA256 in backend/Makefile:23 vs. sha256sum backend/.golangci.yml vs. the canonical file
all three are 021cc83f…346bcb
fires on drift (host, coreutils)
appended a comment line to a scratch copy, ran make lint
exits 1, prints both hashes, beforegolangci-lint run executes
fires on drift (alpine, busybox)
same mutation inside the pinned builder image
exits 1, identical output
fails loudly when the hash tool is absent
make lint SHA256SUM=definitely-not-a-real-command
shell prints not found, actual is empty, guard exits 1. Fails closed — it does not no-op
busybox output format
sha256sum in golang:1.25-alpine is BusyBox v1.37.0; output is <hash>␣␣<file>
cut -d' ' -f1 yields the bare hash
Darwin path
ran make lint SHA256SUM="shasum -a 256" on the host; shasum emits the same two-space format
guard passes, hash matches
POSIX sh, no bashisms
one $(...), one [ ... ], one if; recipe runs under /bin/sh (dash on the host, busybox ash in alpine)
clean in both
quoting
"$$actual" and the make-expanded constant are both quoted; empty actual degrades to a failing comparison, not a syntax error
no quoting bug found
runs inside the Docker build
docker build -f Dockerfile.backend . at 4d70317
full build green, 0 issues. under the pinned v2.12.2
There is no path by which the guard silently passes: the only way to reach golangci-lint run is for the computed hash to equal the constant.
Design assessment of the guard
The guard is the right shape for the problem — the file broke by being edited
away from canonical, which a schema check would not have caught, and it costs
one local hash comparison with no remote trust. Keep it.
The failure message is where it falls short; see NB1 and NB2.
Non-blocking
NB1 — backend/Makefile:39-44: the failure message is not actionable for the legitimate-update case
The guard conflates two different causes of a mismatch and only names one:
someone edited backend/.golangci.yml locally (the message is correct), and
sneak/prompts legitimately published a new org standard and this repo
pulled it in (the message is actively wrong).
In case 2 the output says:
Restore it verbatim from sneak/prompts; do not edit it.
which is precisely what the operator just did. Following the instruction
reproduces the failure — a loop. The message never mentions that GOLANGCI_CONFIG_SHA256 at backend/Makefile:23 is the thing that has to be
updated when the standard itself moves, so the one file that has to change is
the one the message does not name.
This is the maintenance trap in the design, and it is a one-line fix.
Acceptable: add a second sentence, e.g. If the org standard itself changed, update GOLANGCI_CONFIG_SHA256 in backend/Makefile to the new hash.
NB2 — backend/Makefile:36-44: a missing hash tool is misreported as config drift
Verified behaviour with the tool absent:
/bin/sh: 1: definitely-not-a-real-command: not found
.golangci.yml has drifted from the org standard.
expected 021cc83f…346bcb
actual
The important half is right — it fails closed, which is the property that
matters. But the diagnosis is wrong, and the same output appears if .golangci.yml is missing entirely. On a platform in the else branch that is
neither Linux nor Darwin (the BSDs ship sha256, not sha256sum) an operator
would be sent chasing a config edit that never happened. Acceptable: test for
an empty actual first and emit a distinct message naming the hash tool.
NB3 — PR body, "Note on reach": still slightly invites the N4 misreading
The body says the guard runs in "local make check and the Docker build,
including CI". Root make check never reaches the backend — it shims to script/check, which is prettier and the frontend build only. Only cd backend && make check and the RUN make check layer in Dockerfile.backend reach the guard. The preceding qualifier ("wherever backend/make lint runs") does carry the meaning, so this is not the N4 defect
recurring, but "local make check" unqualified is the exact phrase that caused
N4. Documentation only.
Review items N1-N4: all confirmed
N1 — //nolint:wsl is gone from backend/internal/server/server.go:100;
the declaration is now a bare var ctx context.Context. Full //nolint
inventory at head is four directives (gochecknoglobals x2, gosec, contextcheck) — the wsl one is the only removal. The canonical config uses default: all with wsl in disable, so wsl_v5is enabled; the gate
still reports 0 issues. under both local v2.10.1 and the pinned v2.12.2, so
nothing was being suppressed and nothing needs retargeting.
N2 — recorded in TODO.md Future Steps as an upstream sneak/prompts
item; backend/.golangci.yml is untouched by the fix (still byte-identical to
canonical). Independently confirmed the warning is real and version-gated: it
appears under v2.12.2 in the builder image and not under local v2.10.1.
N3 — the Next Step is now scoped to the .editorconfig half, with a
parenthetical recording that hooks already exists. Verified both claims:
root Makefile has hooks: @script/install-precommit, backend/Makefile has
its own hooks target, and there is no .editorconfig at the repo root while backend/.editorconfig exists.
N4 — the commit message contains no reference to the pre-commit hook at
all (grepped), and the PR body carries the corrective note with the pointer to #16. See NB3 for a residual wording nit.
c0d3ddc9cf3faa61a4e378e879ece580256d76e5; GitHub git/ref/tags/v2.12.2 dereferences to exactly that commit
pin comment
# golangci-lint v2.12.2 (2026-08-09), correct form and placement, consistent with the sibling image pins
builder genuinely runs v2.12.2
yes — the v2.12.0 gomodguard deprecation warning only appears in the image, never locally
relocated //nolint:gosec still bound
deleted it in a scratch copy: internal/reportbuf/reportbuf.go:168:12: G304: Potential file inclusion via variable (gosec) reappears at the intended line
relocated //nolint:contextcheck still bound
deleted it in a scratch copy: internal/server/server.go:68:7: Function New$1$1->run->serve->cleanShutdown should pass the context parameter (contextcheck) reappears at the intended line
relocated justifications are accurate
dataDir traces to config.DataDir <- DATA_DIR env var, so "operator-supplied" is correct; serve() does build its own context via context.WithCancel(context.Background())
root make fmt-check clean; prettier reports all files conforming
TODO.md in the same commit
yes
single commit
yes — fbfe1df..4d70317 is exactly one commit
commit title
lint: adopt org-standard .golangci.yml and golangci-lint v2.12.2 (closes #14) — ends with (closes #14)
CI on head
success, check / check (push), 1m13s (was pending when this review started)
mergeable
yes — base is current mainfbfe1df, head is a direct descendant, fast-forwardable, git merge-tree clean
attribution trailers / vendor references
none. Grepped the full diff, the commit message and the PR body. src/main.js is not among the six changed files
inclusive terminology
clean across the diff
Scope
The 2389e26 -> 4d70317 delta is exactly TODO.md, backend/Makefile, and
the //nolint:wsl removal in server.go — i.e. B1 plus N1-N4 and nothing else. backend/.golangci.yml, Dockerfile.backend and reportbuf.go are byte-for-byte
unchanged between the two heads, so the parts the first review signed off on were
demonstrably not disturbed. No scope creep.
Observations, out of scope, not blocking
backend/Makefile:35timeout 30 go test ./... is a real flake source in
the Docker gate. My first cold docker build -f Dockerfile.backend . at 4d70317 failed at exactly that timeout (make: *** [Makefile:35: test] Terminated) because the container's Go build cache starts empty and compiling
the test binaries from scratch exceeded 30s; an immediate retry compiled in
11s and the build went green. This is pre-existing on main, is untouched by
this PR, and the lint step runs after test so the v2.12.2 bump does not
worsen it — but it is pointed at the "main always green" policy and deserves
its own issue.
Dockerfile.backend still does not use the separate hash-pinned golangci/golangci-lint lint stage that REPO_POLICIES.md prescribes for Go
repos. Pre-existing, out of scope for #14, already noted by the first review.
The worktree breakage the implementer flagged (script/install-precommit
writing to .git/hooks/ and COPY .git in Dockerfile.backend) is confirmed
pre-existing and not made worse: neither file is touched by this PR. I
worked around it by reviewing in a detached worktree and building from a
separate full clone.
## Re-review of PR #31 (head `4d70317`) — fresh reviewer
**Verdict: PASS — `merge-ready`**
B1 is genuinely fixed, N1-N4 are done, nothing previously verified regressed,
and the definition of done on #14 is met. Three non-blocking nits below, all
about the wording of the new guard's failure output; none of them justify
another round trip.
Nothing was taken on the implementer's word. Every claim below was re-derived
in a throwaway worktree and a throwaway clone at `4d70317`; the PR branch and
the shared checkout were not modified.
---
## B1 — network-dependent lint gate: resolved, proven
`golangci-lint config verify` is gone. Grepped the whole tree at head: no
occurrence of `config verify` anywhere.
Proven offline rather than argued. The builder stage was built from
`Dockerfile.backend` at `4d70317` and then run with **no network namespace at
all** (`--network none`, not a proxy blackhole):
```
docker run --rm --network none -w /repo/backend <builder> make lint -> 0 issues.
docker run --rm --network none -w /repo/backend <builder> make check -> 0 issues.
```
Both exit 0 with zero network reachable. The old target could not have done
this. B1 is closed.
## The hash guard: correct, live, and cannot false-pass
| Property | How it was checked | Result |
| --- | --- | --- |
| expected hash is the right one | `GOLANGCI_CONFIG_SHA256` in `backend/Makefile:23` vs. `sha256sum backend/.golangci.yml` vs. the canonical file | all three are `021cc83f…346bcb` |
| fires on drift (host, coreutils) | appended a comment line to a scratch copy, ran `make lint` | exits 1, prints both hashes, **before** `golangci-lint run` executes |
| fires on drift (alpine, busybox) | same mutation inside the pinned builder image | exits 1, identical output |
| fails loudly when the hash tool is absent | `make lint SHA256SUM=definitely-not-a-real-command` | shell prints `not found`, `actual` is empty, guard exits 1. Fails **closed** — it does not no-op |
| busybox output format | `sha256sum` in `golang:1.25-alpine` is BusyBox v1.37.0; output is `<hash>␣␣<file>` | `cut -d' ' -f1` yields the bare hash |
| Darwin path | ran `make lint SHA256SUM="shasum -a 256"` on the host; `shasum` emits the same two-space format | guard passes, hash matches |
| POSIX sh, no bashisms | one `$(...)`, one `[ ... ]`, one `if`; recipe runs under `/bin/sh` (dash on the host, busybox ash in alpine) | clean in both |
| quoting | `"$$actual"` and the make-expanded constant are both quoted; empty `actual` degrades to a failing comparison, not a syntax error | no quoting bug found |
| runs inside the Docker build | `docker build -f Dockerfile.backend .` at `4d70317` | full build green, `0 issues.` under the pinned v2.12.2 |
There is no path by which the guard silently passes: the only way to reach
`golangci-lint run` is for the computed hash to equal the constant.
## Design assessment of the guard
The guard is the right shape for the problem — the file broke by being *edited
away from canonical*, which a schema check would not have caught, and it costs
one local hash comparison with no remote trust. Keep it.
The failure **message** is where it falls short; see NB1 and NB2.
---
## Non-blocking
### NB1 — `backend/Makefile:39-44`: the failure message is not actionable for the legitimate-update case
The guard conflates two different causes of a mismatch and only names one:
1. someone edited `backend/.golangci.yml` locally (the message is correct), and
2. `sneak/prompts` legitimately published a new org standard and this repo
pulled it in (the message is actively wrong).
In case 2 the output says:
```
Restore it verbatim from sneak/prompts; do not edit it.
```
which is precisely what the operator just did. Following the instruction
reproduces the failure — a loop. The message never mentions that
`GOLANGCI_CONFIG_SHA256` at `backend/Makefile:23` is the thing that has to be
updated when the standard itself moves, so the one file that has to change is
the one the message does not name.
This is the maintenance trap in the design, and it is a one-line fix.
Acceptable: add a second sentence, e.g. `If the org standard itself changed,
update GOLANGCI_CONFIG_SHA256 in backend/Makefile to the new hash.`
### NB2 — `backend/Makefile:36-44`: a missing hash tool is misreported as config drift
Verified behaviour with the tool absent:
```
/bin/sh: 1: definitely-not-a-real-command: not found
.golangci.yml has drifted from the org standard.
expected 021cc83f…346bcb
actual
```
The important half is right — it fails closed, which is the property that
matters. But the diagnosis is wrong, and the same output appears if
`.golangci.yml` is missing entirely. On a platform in the `else` branch that is
neither Linux nor Darwin (the BSDs ship `sha256`, not `sha256sum`) an operator
would be sent chasing a config edit that never happened. Acceptable: test for
an empty `actual` first and emit a distinct message naming the hash tool.
### NB3 — PR body, "Note on reach": still slightly invites the N4 misreading
The body says the guard runs in "local `make check` and the Docker build,
including CI". Root `make check` never reaches the backend — it shims to
`script/check`, which is prettier and the frontend build only. Only
`cd backend && make check` and the `RUN make check` layer in
`Dockerfile.backend` reach the guard. The preceding qualifier ("wherever
`backend/make lint` runs") does carry the meaning, so this is not the N4 defect
recurring, but "local `make check`" unqualified is the exact phrase that caused
N4. Documentation only.
---
## Review items N1-N4: all confirmed
- **N1** — `//nolint:wsl` is gone from `backend/internal/server/server.go:100`;
the declaration is now a bare `var ctx context.Context`. Full `//nolint`
inventory at head is four directives (`gochecknoglobals` x2, `gosec`,
`contextcheck`) — the `wsl` one is the only removal. The canonical config uses
`default: all` with `wsl` in `disable`, so `wsl_v5` **is** enabled; the gate
still reports `0 issues.` under both local v2.10.1 and the pinned v2.12.2, so
nothing was being suppressed and nothing needs retargeting.
- **N2** — recorded in `TODO.md` Future Steps as an upstream `sneak/prompts`
item; `backend/.golangci.yml` is untouched by the fix (still byte-identical to
canonical). Independently confirmed the warning is real and version-gated: it
appears under v2.12.2 in the builder image and not under local v2.10.1.
- **N3** — the Next Step is now scoped to the `.editorconfig` half, with a
parenthetical recording that `hooks` already exists. Verified both claims:
root `Makefile` has `hooks: @script/install-precommit`, `backend/Makefile` has
its own `hooks` target, and there is no `.editorconfig` at the repo root while
`backend/.editorconfig` exists.
- **N4** — the commit message contains no reference to the pre-commit hook at
all (grepped), and the PR body carries the corrective note with the pointer to
#16. See NB3 for a residual wording nit.
## Previously-verified items: re-derived, none regressed
| Item | Result at `4d70317` |
| --- | --- |
| `sha256sum backend/.golangci.yml` | `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb` |
| byte-identical to the org standard | `cmp` reports zero difference |
| `Dockerfile.backend` pin | `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`; GitHub `git/ref/tags/v2.12.2` dereferences to exactly that commit |
| pin comment | `# golangci-lint v2.12.2 (2026-08-09)`, correct form and placement, consistent with the sibling image pins |
| builder genuinely runs v2.12.2 | yes — the v2.12.0 `gomodguard` deprecation warning only appears in the image, never locally |
| relocated `//nolint:gosec` still bound | deleted it in a scratch copy: `internal/reportbuf/reportbuf.go:168:12: G304: Potential file inclusion via variable (gosec)` reappears at the intended line |
| relocated `//nolint:contextcheck` still bound | deleted it in a scratch copy: `internal/server/server.go:68:7: Function New$1$1->run->serve->cleanShutdown should pass the context parameter (contextcheck)` reappears at the intended line |
| relocated justifications are accurate | `dataDir` traces to `config.DataDir` <- `DATA_DIR` env var, so "operator-supplied" is correct; `serve()` does build its own context via `context.WithCancel(context.Background())` |
| `cd backend && make check` | pass, `0 issues.` |
| root `make check` | pass (vite build, prettier lint, prettier fmt-check) |
| `docker build -f Dockerfile.backend .` | pass, full multi-stage build green |
| `make fmt` hygiene | root `make fmt-check` clean; prettier reports all files conforming |
| `TODO.md` in the same commit | yes |
| single commit | yes — `fbfe1df..4d70317` is exactly one commit |
| commit title | `lint: adopt org-standard .golangci.yml and golangci-lint v2.12.2 (closes #14)` — ends with ` (closes #14)` |
| CI on head | `success`, `check / check (push)`, 1m13s (was `pending` when this review started) |
| mergeable | yes — base is current `main` `fbfe1df`, head is a direct descendant, fast-forwardable, `git merge-tree` clean |
| attribution trailers / vendor references | none. Grepped the full diff, the commit message and the PR body. `src/main.js` is not among the six changed files |
| inclusive terminology | clean across the diff |
## Scope
The `2389e26` -> `4d70317` delta is exactly `TODO.md`, `backend/Makefile`, and
the `//nolint:wsl` removal in `server.go` — i.e. B1 plus N1-N4 and nothing else.
`backend/.golangci.yml`, `Dockerfile.backend` and `reportbuf.go` are byte-for-byte
unchanged between the two heads, so the parts the first review signed off on were
demonstrably not disturbed. No scope creep.
## Observations, out of scope, not blocking
- **`backend/Makefile:35` `timeout 30 go test ./...` is a real flake source in
the Docker gate.** My first cold `docker build -f Dockerfile.backend .` at
`4d70317` failed at exactly that timeout (`make: *** [Makefile:35: test]
Terminated`) because the container's Go build cache starts empty and compiling
the test binaries from scratch exceeded 30s; an immediate retry compiled in
11s and the build went green. This is pre-existing on `main`, is untouched by
this PR, and the lint step runs after `test` so the v2.12.2 bump does not
worsen it — but it is pointed at the "main always green" policy and deserves
its own issue.
- `Dockerfile.backend` still does not use the separate hash-pinned
`golangci/golangci-lint` lint stage that `REPO_POLICIES.md` prescribes for Go
repos. Pre-existing, out of scope for #14, already noted by the first review.
- The worktree breakage the implementer flagged (`script/install-precommit`
writing to `.git/hooks/` and `COPY .git` in `Dockerfile.backend`) is confirmed
pre-existing and **not** made worse: neither file is touched by this PR. I
worked around it by reviewing in a detached worktree and building from a
separate full clone.
Fresh re-review (independent of both the author and the first reviewer) returned PASS with no blocking findings. Relabelled needs-rework -> merge-ready and assigned to @sneak for the merge, since main is protected.
Why I am accepting this
B1 was resolved by proof, not assertion — the reviewer built the builder stage and ran the lint gate under docker run --network none, i.e. with no network namespace at all rather than a proxy blackhole, and got 0 issues. The replacement drift guard was attacked rather than read: mutated config fires the guard before the linter runs, on both coreutils and busybox; SHA256SUM=definitely-not-a-real-command fails closed with exit 1 rather than silently passing. That last property is the one that matters most in a guard, and it was verified rather than assumed.
The previously-verified work was also re-confirmed at the new head rather than carried over on trust, including the highest-risk part: deleting the relocated //nolint directives in a scratch copy reproduces G304 at reportbuf.go:168 and the contextcheck finding at server.go:68, proving they are still bound to their intended statements after being moved.
The 2389e26 -> 4d70317 delta is exactly B1 plus N1-N4. No scope creep.
Three non-blocking findings I am deliberately not sending back for
All three are wording, none affects behaviour, and I judged another full rework-plus-fresh-review cycle to cost more than it returns. Filed as #34 instead.
NB1 is the one with real teeth: when the org standard legitimately updates, the guard fails with Restore it verbatim from sneak/prompts; do not edit it. — which is exactly what the operator just did. Following the instruction loops. The message never names GOLANGCI_CONFIG_SHA256, the constant that actually has to change. One-line fix, but a genuine trap for whoever next syncs the config.
NB2 — a missing or failing hash tool is misreported as config drift. It fails closed, so it is safe; the diagnosis is just wrong.
NB3 — the PR body still says "local make check" in one place, where root make check never reaches the backend. The surrounding qualifier saves the meaning.
Two things surfaced during review that are not this PR's problem
A real CI flake. The reviewer's first cold docker build died at backend/Makefile's timeout 30 go test ./... — empty Go build cache in a fresh container; the retry compiled in 11s and passed. Pre-existing and unchanged by this PR, but it means the Docker gate can fail for reasons unrelated to the code, against a "main always green" policy. Recorded on #21, which already owns the test-target rework.
Worktree hostility.make hooks and make docker both fail from a linked worktree. Filed as #33.
Limits of verification, stated plainly
CI job logs were not readable — the Actions API returns 403 for this account. The reviewer has the commit status only (success). They compensated by reproducing both CI steps locally: root make check and the full docker build -f Dockerfile.backend .. I am satisfied by that, but the CI log itself is unverified.
The Darwin branch is unexercised end to end. No macOS host available. The shasum -a 256 code path was tested via a make variable override on Linux and the output format confirmed compatible, but the ifeq ($(UNAME_S),Darwin) selection itself has never run. If you merge and later hit this on a Mac, that is the place to look.
Over to you for the merge.
## Manager note — merge-ready
Fresh re-review (independent of both the author and the first reviewer) returned **PASS** with no blocking findings. Relabelled `needs-rework` -> `merge-ready` and assigned to @sneak for the merge, since `main` is protected.
### Why I am accepting this
B1 was resolved by **proof, not assertion** — the reviewer built the builder stage and ran the lint gate under `docker run --network none`, i.e. with no network namespace at all rather than a proxy blackhole, and got `0 issues.` The replacement drift guard was attacked rather than read: mutated config fires the guard before the linter runs, on both coreutils and busybox; `SHA256SUM=definitely-not-a-real-command` fails **closed** with exit 1 rather than silently passing. That last property is the one that matters most in a guard, and it was verified rather than assumed.
The previously-verified work was also re-confirmed at the new head rather than carried over on trust, including the highest-risk part: deleting the relocated `//nolint` directives in a scratch copy reproduces `G304` at `reportbuf.go:168` and the `contextcheck` finding at `server.go:68`, proving they are still bound to their intended statements after being moved.
The `2389e26` -> `4d70317` delta is exactly B1 plus N1-N4. No scope creep.
### Three non-blocking findings I am deliberately not sending back for
All three are wording, none affects behaviour, and I judged another full rework-plus-fresh-review cycle to cost more than it returns. Filed as #34 instead.
- **NB1** is the one with real teeth: when the org standard *legitimately* updates, the guard fails with `Restore it verbatim from sneak/prompts; do not edit it.` — which is exactly what the operator just did. Following the instruction loops. The message never names `GOLANGCI_CONFIG_SHA256`, the constant that actually has to change. One-line fix, but a genuine trap for whoever next syncs the config.
- **NB2** — a missing or failing hash tool is misreported as config drift. It fails closed, so it is safe; the diagnosis is just wrong.
- **NB3** — the PR body still says "local `make check`" in one place, where root `make check` never reaches the backend. The surrounding qualifier saves the meaning.
### Two things surfaced during review that are not this PR's problem
- **A real CI flake.** The reviewer's first cold `docker build` died at `backend/Makefile`'s `timeout 30 go test ./...` — empty Go build cache in a fresh container; the retry compiled in 11s and passed. Pre-existing and unchanged by this PR, but it means the Docker gate can fail for reasons unrelated to the code, against a "main always green" policy. Recorded on #21, which already owns the test-target rework.
- **Worktree hostility.** `make hooks` and `make docker` both fail from a linked worktree. Filed as #33.
### Limits of verification, stated plainly
- **CI job logs were not readable** — the Actions API returns 403 for this account. The reviewer has the commit status only (`success`). They compensated by reproducing both CI steps locally: root `make check` and the full `docker build -f Dockerfile.backend .`. I am satisfied by that, but the CI log itself is unverified.
- **The Darwin branch is unexercised end to end.** No macOS host available. The `shasum -a 256` code path was tested via a make variable override on Linux and the output format confirmed compatible, but the `ifeq ($(UNAME_S),Darwin)` selection itself has never run. If you merge and later hit this on a Mac, that is the place to look.
Over to you for the merge.
Re-verified against the cached-green risk — merge-ready stands
A fleet-wide warning came in after I labelled this: script/cibuild is a bare docker build . with no cache control, so on an unchanged tree Docker can serve RUN make check from cache and the build exits 0 having run nothing. Observed elsewhere in the org as a SUCCESS in 0.262s with every layer cached.
Given that this PR is specifically about a lint gate that had already been silently doing nothing, accepting a possibly-cached green here would have been the same mistake twice. So I re-verified directly rather than trusting the recorded evidence.
Forced-uncached build at head 4d70317
Fresh clone, --no-cache, full output:
=== forced UNCACHED build of Dockerfile.backend ===
#17 7.277 ? sneak.berlin/go/netwatch/cmd/netwatch-server [no test files]
#17 7.576 ok sneak.berlin/go/netwatch/internal/handlers 0.003s
#17 7.576 ok sneak.berlin/go/netwatch/internal/reportbuf 0.003s
#17 12.12 0 issues.
#17 DONE 12.4s
EXIT=0
ELAPSED=74s
74 seconds, real go test package timings, and golangci-lint running to 0 issues. under the pinned v2.12.2. The checks demonstrably executed. This green is earned. No retraction.
Also worth noting: the drift guard added by this PR ran inside that uncached build too, so the offline hash check is exercised on the real CI path, not just locally.
Corroborating evidence already in hand
Two things from the reviews independently rule out a cached green:
Both reviewers ran docker run --network none against the built builder image. A docker run executes by definition — it cannot be served from a layer cache.
The second reviewer's first cold build failed at backend/Makefile's timeout 30 go test ./... with an empty container build cache. A cached layer would not have run the test at all, let alone timed out. That failure is itself proof of genuine execution.
The hole is real for this repo, and is now filed
I reproduced it here rather than assuming netwatch was exempt. After one warm build, a repeat docker build . on an unchanged tree:
9 CACHED layers
ELAPSED_MS=514
#13 [build 7/7] RUN make check <- CACHED, no vite output, no prettier output
514ms, exit 0, nothing ran. Filed as #37 and attached to 1.0.0, sequenced behind #16 since both rewrite script/cibuild.
That means the CI success status on 4d70317 is not by itself trustworthy evidence — but the forced-uncached build above is, and it is the basis on which I am keeping this labelled merge-ready.
What this changes going forward
I am treating a green CI status as insufficient evidence for any future PR in this repo until #37 lands. Reviewers will be instructed to demonstrate an uncached execution rather than cite the CI badge. Worth knowing that this repo has now had three independent ways to report an unearned green: an inert lint config (#14, fixed by this PR), a root make check that never touched the backend (#16), and a cacheable CI gate (#37).
## Re-verified against the cached-green risk — `merge-ready` stands
A fleet-wide warning came in after I labelled this: `script/cibuild` is a bare `docker build .` with no cache control, so on an unchanged tree Docker can serve `RUN make check` from cache and the build exits 0 having run nothing. Observed elsewhere in the org as a SUCCESS in 0.262s with every layer cached.
Given that this PR is specifically about a lint gate that had *already* been silently doing nothing, accepting a possibly-cached green here would have been the same mistake twice. So I re-verified directly rather than trusting the recorded evidence.
### Forced-uncached build at head `4d70317`
Fresh clone, `--no-cache`, full output:
```
=== forced UNCACHED build of Dockerfile.backend ===
#17 7.277 ? sneak.berlin/go/netwatch/cmd/netwatch-server [no test files]
#17 7.576 ok sneak.berlin/go/netwatch/internal/handlers 0.003s
#17 7.576 ok sneak.berlin/go/netwatch/internal/reportbuf 0.003s
#17 12.12 0 issues.
#17 DONE 12.4s
EXIT=0
ELAPSED=74s
```
74 seconds, real `go test` package timings, and `golangci-lint` running to `0 issues.` under the pinned v2.12.2. The checks demonstrably executed. **This green is earned. No retraction.**
Also worth noting: the drift guard added by this PR ran inside that uncached build too, so the offline hash check is exercised on the real CI path, not just locally.
### Corroborating evidence already in hand
Two things from the reviews independently rule out a cached green:
1. Both reviewers ran `docker run --network none` against the built builder image. A `docker run` executes by definition — it cannot be served from a layer cache.
2. The second reviewer's **first cold build failed** at `backend/Makefile`'s `timeout 30 go test ./...` with an empty container build cache. A cached layer would not have run the test at all, let alone timed out. That failure is itself proof of genuine execution.
### The hole is real for this repo, and is now filed
I reproduced it here rather than assuming netwatch was exempt. After one warm build, a repeat `docker build .` on an unchanged tree:
```
9 CACHED layers
ELAPSED_MS=514
#13 [build 7/7] RUN make check <- CACHED, no vite output, no prettier output
```
514ms, exit 0, nothing ran. Filed as **#37** and attached to `1.0.0`, sequenced behind #16 since both rewrite `script/cibuild`.
That means the CI `success` status on `4d70317` is **not** by itself trustworthy evidence — but the forced-uncached build above is, and it is the basis on which I am keeping this labelled `merge-ready`.
### What this changes going forward
I am treating a green CI status as insufficient evidence for any future PR in this repo until #37 lands. Reviewers will be instructed to demonstrate an uncached execution rather than cite the CI badge. Worth knowing that this repo has now had **three** independent ways to report an unearned green: an inert lint config (#14, fixed by this PR), a root `make check` that never touched the backend (#16), and a cacheable CI gate (#37).
backend/.golangci.yml declared version: "2" on line 1 but used the
golangci-lint v1 schema below it: a top-level linters-settings key and
an issues.exclude-use-default key that does not exist in v2. Under v2
that config does not validate, so every threshold in it was inert --
lll fell back to its 120-column default rather than the intended 88,
and funlen, cyclop and dupl were not applied at all. The `0 issues.`
result the repo has been relying on was therefore meaningless.
Replace it with the org-standard file verbatim (sha256
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb) and
repoint the golangci-lint pin in Dockerfile.backend from v2.7.2 to the
org-standard v2.12.2.
Guard against the config drifting from the standard again by asserting
its sha256 as the first step of the backend lint target. The check is a
local hash comparison against a constant in the Makefile: it needs no
network, fetches nothing, and adds no unpinned external reference to
the build path. It also catches a strictly larger class of breakage
than schema validation would, since a schema-valid but non-canonical
config is exactly how this file got into its broken state.
With the config actually loading, lll reports server.go:65 at 93
columns. Fix it, plus the two other over-long lines called out on the
issue (server.go:97 at 81 and reportbuf.go:166 at 88) which are inside
the 88-column lint limit but over the 77-column hard wrap in the Go
styleguide. All three were long //nolint justifications on the code
line; move the justification into a preceding comment block and leave
a short directive behind. No suppression is added or widened, and
.golangci.yml is not touched after the copy.
Drop the //nolint:wsl in server.go entirely rather than relocating it.
The standard config disables wsl, so the directive suppressed nothing;
removing it still yields `0 issues.`
Verified: `cd backend && make check` reports `0 issues.` and passes
with the network unavailable, root `make check` passes, and
`docker build -f Dockerfile.backend .` builds green against the pinned
v2.12.2.
Reconciliation: the RUN CGO_ENABLED=0 go install ...@9f61b0f5 line this PR used to retarget no longer exists — #40 replaced it with a digest-pinned lint stage. That hunk is therefore replaced, not rebased: the lint stage FROM and its comment now name golangci/golangci-lint:v2.12.2 at sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240. Digest resolved from the tag and verified out of that exact image: golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z.
backend/Makefile takes next's VERSION/GOLDFLAGS header and build recipe plus this PR's hash-guarded lint; the SHA256SUM selection moved out of the removed ifeq into a command -v probe. TODO.md keeps every landed bullet; the Next Step was stale (the root .editorconfig it named landed in #40's series) and is rewritten. backend/.golangci.yml still hashes to 021cc83f...46bcb, matching the Makefile constant.
Gates: root make check green; cd backend && make check green (0 issues.); docker build --no-cache -f Dockerfile.backend . green, with RUN make lint running uncached for 16.5s and reporting 0 issues. — the first run of v2.12.2 against the canonical config through the lint stage. The gomodguard deprecation is a warning only, tracked at #41.
Drift guard re-proved in the Debian-based lint image: appending a byte to backend/.golangci.yml fails make lint at 0.17s with expected/actual hashes and no golangci-lint run line; file restored, git status --short clean.
Rebased onto `next` (`f7c7f92`); head is now `6ec7de5`.
Reconciliation: the `RUN CGO_ENABLED=0 go install ...@9f61b0f5` line this PR used to retarget no longer exists — https://git.eeqj.de/sneak/netwatch/pulls/40 replaced it with a digest-pinned `lint` stage. That hunk is therefore replaced, not rebased: the `lint` stage `FROM` and its comment now name `golangci/golangci-lint:v2.12.2` at `sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`. Digest resolved from the tag and verified out of that exact image: `golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z`.
`backend/Makefile` takes `next`'s `VERSION`/`GOLDFLAGS` header and `build` recipe plus this PR's hash-guarded `lint`; the `SHA256SUM` selection moved out of the removed `ifeq` into a `command -v` probe. `TODO.md` keeps every landed bullet; the Next Step was stale (the root `.editorconfig` it named landed in https://git.eeqj.de/sneak/netwatch/pulls/40's series) and is rewritten. `backend/.golangci.yml` still hashes to `021cc83f...46bcb`, matching the Makefile constant.
Gates: root `make check` green; `cd backend && make check` green (`0 issues.`); `docker build --no-cache -f Dockerfile.backend .` green, with `RUN make lint` running uncached for 16.5s and reporting `0 issues.` — the first run of v2.12.2 against the canonical config through the lint stage. The `gomodguard` deprecation is a warning only, tracked at https://git.eeqj.de/sneak/netwatch/issues/41.
Drift guard re-proved in the Debian-based lint image: appending a byte to `backend/.golangci.yml` fails `make lint` at 0.17s with expected/actual hashes and no `golangci-lint run` line; file restored, `git status --short` clean.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #14.
What was actually wrong
backend/.golangci.ymldeclaredversion: "2"on line 1 but used thegolangci-lint v1 schema below it — a top-level
linters-settings:key andan
issues.exclude-use-default:key that does not exist in v2. Under v2 thatconfig does not validate, so every threshold in it was inert:
lllfell backto its 120-column default instead of the intended 88, and
funlen,cyclopand
duplwere not applied at all.The
0 issues.thatcd backend && make checkhas been printing wastherefore not evidence the backend was clean — it was the linter running at
defaults.
Changes
backend/.golangci.yml— replaced verbatim with the org standard.sha256sum backend/.golangci.ymlis now021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb, matchingthe definition of done exactly. Not hand-edited, no exclusions added, nothing
touched after the copy.
Dockerfile.backend— golangci-lint pin moved from9f61b0f53f80672872fced07b6874397c3ed197b(v2.7.2) toc0d3ddc9cf3faa61a4e378e879ece580256d76e5, with the comment above it updatedto
# golangci-lint v2.12.2 (2026-08-09).backend/Makefile— thelinttarget now asserts the sha256 of.golangci.ymlagainst a constant in the Makefile before running the linter.Offline hash comparison only. See the note below.
//nolint:wslremoved frominternal/server/server.go.TODO.md— updated in the same commit. Also corrected the stale Statusand Next Step, which still described
feat/reportbuf-storageas unmergedwhen it has been on
mainsincefbfe1df's ancestry.The drift guard
The escalation on #14 asks for the config to be verified after copying, and for
a config that does not verify never to be committed again. The first revision
of this PR implemented that with
golangci-lint config verifywired intomake lint. That was wrong:config verifyresolves its JSON schema over alive, unpinned HTTPS fetch with a 2-second timeout, which violates the
hash-pinning rule in
REPO_POLICIES.mdand makes the lint gatenetwork-dependent. It has been removed.
In its place the
linttarget asserts that.golangci.ymlstill hashes to021cc83f…346bcband fails with a clear message if it does not. This is alocal
sha256sumcomparison against a constant: no network, no remote schema,nothing unpinned added to the build path. It also catches a strictly larger
class of breakage than schema validation would, because a schema-valid but
non-canonical config — which is precisely how this file got into its broken
state — passes
config verifyand fails the hash guard.The guard is POSIX sh, uses
sha256sum(busybox in the alpine builder image,coreutils on Linux) and falls back to
shasum -a 256on Darwin, alongside theGOFLAGSbranch already in that Makefile.Lint findings and fixes
Only one of the three lines named in the escalation is an actual
lllfindingunder the canonical config, because that config sets
lll.line-length: 88:lll?internal/server/server.go:65internal/server/server.go:97internal/reportbuf/reportbuf.go:166All three do exceed the 77-column hard wrap in
CODE_STYLEGUIDE_GO.md, and theescalation puts all three in scope, so all three were wrapped.
In every case the length came from a long
//nolintjustification sitting onthe code line. For the
gosecandcontextcheckdirectives the fix moves thereasoning into a comment block directly above and leaves a short
//nolint:linter // see comment abovebehind; those suppressions are unchangedin scope and meaning. The third,
//nolint:wslinserver.go, was droppedoutright instead: the standard config disables
wsl, so it suppressed nothing.Beyond those, the standard config plus v2.12.2 surfaced no additional
findings, so the split into two PRs that the escalation anticipated was not
needed. The escalation's expectation of "substantially more" findings did not
materialize: the backend is small, and the linters that had been silently
disabled (
funlen,cyclop,dupl) had nothing to report against it.Verification
cd backend && make check— passes,0 issues.make lintandmake checkinside the builder image underdocker run --network none, both reporting0 issues., and by aproxy-blackholed run on the host.
.golangci.ymlmakes
make lintfail with the expected/actual hashes before the linterruns. The file was restored byte-identical afterwards.
make checkat the repo root — passes (build, prettier lint, prettierformat check).
docker build -f Dockerfile.backend .— builds green. ItsRUN make checklayer runs against the pinned v2.12.2, not the locally installed
golangci-lint, and reports
0 issues.there too.make fmtwas run over the touched markdown before committing.Note on reach: the drift guard runs wherever
backend/make lintruns — localmake checkand the Docker build, including CI. It does not run in thepre-commit hook installed by
script/install-precommit, which reaches only thefrontend
script/checkand never the backend. That gap is tracked in #16 andis not addressed here.
Summary
Six files, one commit (
2389e26).backend/.golangci.ymlDockerfile.backendbackend/Makefilelintnow runsgolangci-lint config verifyfirstbackend/internal/server/server.gobackend/internal/reportbuf/reportbuf.goTODO.mdFindings and how each was fixed
The old config was schema-invalid under v2, so its thresholds were never
applied. Once the canonical config actually loads,
lllat 88 columns reportsexactly one finding:
Fixed by moving the
//nolint:contextcheckjustification into a comment blockabove the statement.
The other two lines named in the escalation are not
lllfindings underline-length: 88—server.go:97is 81 columns andreportbuf.go:166is 88,i.e. at the limit, not over it. Both do break the 77-column hard wrap in
CODE_STYLEGUIDE_GO.mdand the escalation places them in scope, so both werewrapped the same way.
No
//nolintwas added, widened, or retargeted; each of the three keeps itsoriginal linter and its original reasoning, just relocated to the line above.
Nothing was suppressed via
.golangci.yml, which is byte-identical to thestandard after the copy.
No other findings surfaced. The two-PR split anticipated by the escalation was
not necessary, so this is delivered whole rather than partial.
Verification
cd backend && make checkconfig verifyclean,0 issues.make check(repo root)make dockersha256sum backend/.golangci.yml021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcbThe Docker build is the load-bearing check for the version bump: its
RUN make checklayer runs against the pinned v2.12.2 rather than whatever ison the local PATH, and it reports
0 issues.there as well. Confirmed thebuilder stage carries
golangci-lint has version 2.12.2.make fmtwas run over the touched markdown before committing.One thing noted, not fixed here
The frontend build emits a Node deprecation warning during
make check. Out ofscope for this issue and deliberately not touched; worth filing separately.
Review of PR #31 (head
2389e26)Verdict: FAIL —
needs-reworkOne blocking finding. The issue's definition of done is fully and correctly
met; the blocker is in an addition that the definition of done did not ask for.
Independently verified (all green)
sha256sum backend/.golangci.yml021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb— matchesDockerfile.backendpinc0d3ddc9cf3faa61a4e378e879ece580256d76e5; GitHubgit/ref/tags/v2.12.2resolves to exactly that commit# golangci-lint v2.12.2 (2026-08-09)— correct form, correct placementgolangci-lint has version 2.12.2cd backend && make check0 issues.(local v2.10.1)make docker0 issues.under the pinned v2.12.2make checkmake fmthygiene is satisfieddocker build -f Dockerfile.backend .TODO.mdin the same commit2389e26, six files(closes #14)success(check / check (push), 55s)mainfbfe1dfis currentmainhead, merge is cleansrc/main.jsis untouched by this PRClaim 3 (the
lllthreshold) — implementer is correctRead directly from the canonical config:
linters.settings.lll.line-length: 88.lllreports strictly over the limit, soserver.go:97(81 cols) andreportbuf.go:166(88 cols) are genuinely notlllfindings. The escalation'sline inventory was wrong on those two; the correction stands.
Claim 4 (only one real finding) — confirmed, and the config is genuinely enforced
Not taken on trust.
golangci-lint config verifypasses, and the thresholds aredemonstrably live rather than failing open: removing the
//nolint:contextcheckand
//nolint:gosecdirectives in a scratch copy immediately producesso linters are running and the directives are attached to the correct
statements after relocation. One genuine finding is the right count.
Claim 6 (no suppression added, widened, or retargeted) — confirmed
Full
//nolintinventory onmainvs. head is the same five directives withthe same five linters (
gochecknoglobalsx2,gosec,contextcheck,wsl).Nothing added, nothing widened, nothing retargeted. All three rewritten lines
are valid Go and all new comment lines are under both the 88-column lint limit
and the 77-column hard wrap.
Blocking
B1 —
backend/Makefile:31:config verifyintroduces an unpinned, build-gating network fetchgolangci-lint config verifydoes not validate against an embedded schema. Itfetches the JSON schema over HTTPS at run time. From golangci-lint v2
pkg/commands/config_verify.go,createSchemaURL()buildshttps://golangci-lint.run/jsonschema/golangci.vX.Y.jsonschema.jsonandjsonschemaHTTPLoaderfetches it with a 2-second client timeout.Demonstrated on this branch with the network blocked:
Why this matters:
REPO_POLICIES.mdlines 22-34. "ALL external references must be pinnedby cryptographic hash... anything else fetched from a remote source... No
exceptions... This is the single most important rule in this document. There
are zero exceptions to this rule." This adds a server-mutable, unpinned,
unverified remote artifact that decides whether the build passes. Every other
external reference in this repo is pinned (
@sha256:,go.sum,yarn.lock,Actions by commit SHA). This one is not.
make lintis reached bymake check, thepre-commit hook, and every
docker build -f Dockerfile.backend .includingCI. Before this PR, backend
make checkran fully offline against a warmmodule cache. It no longer does. A 2-second timeout against a third-party
website is a flake source pointed directly at the "main always green" policy,
and it fails the build for a reason that has nothing to do with the code.
"run the linter's own config verification and confirm it reports the config
valid" once, after copying. Wiring it permanently into
lintis a designchange beyond the issue's scope. The PR body argues for it at length but
never mentions that it makes the lint gate network-dependent.
Acceptable resolutions, any one of:
golangci-lint config verifyline from thelinttarget. Theone-off verification has already been performed and recorded on #14; that
satisfies the escalation as written.
repo and invoke
config verify --schema <repo-relative path>, with theversion-and-date pin comment the hash-pinning policy requires. Note this
couples the vendored schema to the pinned linter version and must be updated
alongside it.
remote fetch in the build path, and amend
REPO_POLICIES.mdaccordingly.As written it must not merge.
Non-blocking
N1 —
backend/internal/server/server.go:100-102: the//nolint:wslis deadThe canonical config disables
wsloutright (- wsl # Deprecated, replaced by wsl_v5), so//nolint:wsl // see comment abovesuppresses nothing. Verified:deleting the directive in a scratch copy still yields
0 issues.— unlike thegosecandcontextcheckdirectives, which do fail loudly when removed.The directive is pre-existing on
main, so it is not a regression, but this PRrewrites that exact line and attaches a freshly-authored two-line justification
to a suppression that has no effect. Acceptable: drop the directive and its
justification comment. Do not blindly retarget it to
wsl_v5— confirm with thegate first whether
wsl_v5actually flags the line.N2 — new deprecation warning is neither mentioned nor tracked
The v2.12.2 bump makes every lint run print:
Since
.golangci.ymlmust never be edited by an agent, the fix belongs upstreamin
sneak/prompts, not here. But repo convention treats deprecation warnings astracked action items rather than noise, and this one is silently introduced by
this PR. Acceptable: a line in
TODO.mdFuture Steps, or a tracker issue,noting that the canonical config needs
gomodguardreplaced withgomodguard_v2.N3 —
TODO.md:20-21: the new Next Step is factually wrongThe root
Makefilealready has ahooks:target (@script/install-precommit),and
backend/Makefilehas one too. The item was carried up from Future Stepsper the documented workflow, but it was made more specific ("root Makefile")
while being wrong, so it now directs the next work unit to add something that
exists.
.editorconfigat the repo root is genuinely absent(
backend/.editorconfigexists), so that half stands. Acceptable: reduce theNext Step to the
.editorconfigitem.N4 — PR body and commit message overstate the reach of the new check
Both claim the verification "now runs everywhere
make checkruns: locally, inthe pre-commit hook, and in the Docker build." The hook installed by
script/install-precommitrunsscript/precommittoscript/check, which runsthe root
script/test,script/lint,script/fmt-check— prettier and thefrontend build only. It never reaches
backend/make check. Onlybackend/make hooksinstalls a backend-running hook, and it overwrites the same.git/hooks/pre-commitfile, so the two are mutually exclusive. Documentationonly, but it is part of the stated justification for B1.
Scope
No scope creep in the source or config changes; the six touched files are all
within the issue's stated scope. The
TODO.mdStatus/Next Step/Future Stepsrewrite is authorized by the repo's own TODO workflow. The
config verifyaddition (B1) is the one item that goes beyond what #14 asked for.
Pre-existing and correctly left alone:
Dockerfile.backenddoes not use theseparate hash-pinned lint stage that
REPO_POLICIES.mdlines 102-166 prescribefor Go repos. Out of scope for #14; should be its own issue if one does not
already exist.
Manager note
Review verdict: FAIL. Relabelled
needs-review->needs-rework, still assigned toclawbot. Sending back to an implementer.B1 is accepted as blocking, and the root cause is my issue text, not the implementer's judgement
The reviewer is right, and the demonstration is conclusive —
golangci-lint config verifyresolves its schema over the network, so wiring it intomake lintmakes everymake check, every pre-commit run, and every Docker build depend on an unpinned live HTTPS fetch with a 2-second timeout. That is a direct hit on the single most emphatic rule inREPO_POLICIES.md("ALL external references must be pinned by cryptographic hash… anything else fetched from a remote source… zero exceptions"), and it converts an offline-capable gate into a network-dependent one with a flake window, against a "main always green" policy.The irony is not lost: a PR whose entire purpose is fixing a silently-broken lint gate would have introduced a new way for that gate to fail for reasons unrelated to the code.
This is my fault, and I want it on the record. My escalation comment on #14 said "run the linter's own config verification and confirm it reports the config valid. A config that fails to verify must never be committed again." The second sentence reads as a request for a permanent guard, and the implementer built one. That was a reasonable reading of what I wrote. What I actually meant was a one-off check performed while doing this work — verification that the copy landed correctly, not a new build-time dependency. I should have said so.
What the rework should do instead
Drop the
config verifyline frombackend/Makefile'slinttarget. The requirement it was trying to satisfy is better served by a hash guard, which needs no network:backend/.golangci.ymlstill hashes to021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb, failing the target if it does not.That catches the actual failure mode we care about — the file drifting from the org standard again, which is exactly how it got into its current broken state — and it does so offline, deterministically, and without trusting a remote server. A schema-valid-but-non-canonical file would pass
config verifyand fail the hash guard, so the hash guard is also strictly the better check for this repo's problem.I am treating that as a suggestion, not a mandate. If the implementer sees a reason the hash guard is wrong here, say so on the PR and just remove the verify line; plain removal also satisfies B1.
Non-blocking items
Fold N1, N3, and N4 into the same amended commit:
//nolint:wslatserver.go:100-102. The canonical config disableswsl, so it suppresses nothing. This PR rewrote that exact line, so it is fair to fix it here.TODO.md:20-21claims the root Makefile needs ahookstarget. It already has one (hooks: @script/install-precommit). Only the.editorconfighalf of that Next Step is real. Correct the wording.script/install-precommitinstalls a hook reaching only the frontendscript/check, never the backend. Correct the claim. (The underlying gap is real and is tracked in #16 — do not fix it here.)N2 — the
gomodguard is deprecated (since v2.12.0)… Replaced by gomodguard_v2warning is now emitted on every lint run. Do not fix it here:.golangci.ymlis untouchable by policy and the fix belongs upstream insneak/prompts. Add a line toTODO.mdrecording it so it is tracked rather than lost. I will raise it against thepromptsrepo separately.Not in scope for the rework
Everything else the reviewer checked came back clean and independently verified — the hash, the pin (confirmed against the GitHub tag ref), the builder image genuinely running v2.12.2, both
make checkinvocations, the Docker build, CI green on2389e26,TODO.mdin the same commit, the commit title, mergeability against currentmain, and no attribution trailers or Claude/Anthropic references anywhere. Do not redo that work and do not disturb those parts.Particularly worth preserving: the reviewer proved the relocated
//nolintdirectives are still bound to their intended statements by deleting them and confirmingG304and thecontextcheckfinding reappear at the right lines. That was the highest-risk part of this change and it is verified correct. Leave it alone.A fresh reviewer will re-review after rework.
Rework:
2389e26->4d70317(amended, force-pushed)All five findings addressed in the same single commit. The parts the reviewer
independently verified — the
.golangci.ymlcopy, the v2.12.2 pin and itscomment, the three wrapped lines, and the relocated
gosec/contextcheckdirectives — were left untouched, with the one exception called out under N1.
B1 — network-dependent
config verifyin thelinttarget: fixedThe reviewer is right and the demonstration is conclusive.
config verifyresolves its schema over a live unpinned HTTPS fetch, so wiring it into
make lintput an unpinned remote artifact with a 2-second timeout into thegate that
make check, the Docker build and CI all pass through. That is adirect hit on the hash-pinning rule, and it is worse than pointless in a PR
whose purpose is making the lint gate trustworthy. The line is gone.
I took the manager's suggested replacement rather than plain removal, because
the hash guard is the better check for the failure this issue exists to close.
backend/Makefile'slinttarget now begins:with
GOLANGCI_CONFIG_SHA256set to021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcband acomment above it explaining why the guard exists.
Why the hash guard over plain removal: the config did not break by becoming
schema-invalid in the abstract, it broke by being edited away from the
canonical file. A schema-valid but non-canonical config passes
config verifyand fails this guard, so the guard is strictly the strongercheck for this repo's actual failure mode — and it is deterministic, offline,
and trusts no remote server.
Portability, since the guard has to survive the Docker build:
$(...), one[ ... ], oneif. No bashisms.sha256sumis present in thegolang:1.25-alpinebuilder as busyboxv1.37.0, verified by running it in that exact pinned image rather than
assuming.
cut -d' ' -f1handles both busybox and coreutils output.sha256sum, soSHA256SUMis set toshasum -a 256in theifeq ($(UNAME_S),Darwin)branch that Makefile already carries forGOFLAGS.N1 — dead
//nolint:wslatserver.go:100-102: removedDropped the directive and its justification comment; the declaration is now
plain
var ctx context.Context. Sincewslis disabled outright by thecanonical config, this suppressed nothing, and re-attaching a freshly written
justification to it in the first revision was noise.
make checkstill reports0 issues.after removal, so nothing was being held back by it — unlike thegosecandcontextcheckdirectives, which the reviewer proved do fire whendeleted. Those two are untouched.
N2 —
gomodguarddeprecation: recorded inTODO.md, not fixedNot fixed here, as instructed —
.golangci.ymlis untouchable by policy andthe fix belongs upstream in
sneak/prompts. Added to Future Steps inTODO.md, noting that the standard config enablesgomodguard, that v2.12.2reports it deprecated since v2.12.0 in favour of
gomodguard_v2, and thatnothing can be done in this repo beyond tracking it.
Confirmed the warning is real and only visible under the pinned linter: it
appears in the Docker build's
RUN make checklayer (v2.12.2) and not in alocal run (v2.10.1).
N3 —
TODO.mdNext Step claimed a missinghookstarget: correctedVerified first: the root
Makefilehashooks: @script/install-precommitandbackend/Makefilehas its ownhookstarget. The reviewer is correct. TheNext Step is now scoped to the half that is genuinely missing — an
.editorconfigat the repo root,backend/.editorconfigbeing the only onethat exists — with a parenthetical recording that the
hookstarget is alreadypresent in both Makefiles, so the item does not get re-added later.
N4 — overstated reach of the check: corrected in both places
The commit message no longer claims the check runs in the pre-commit hook; it
now says only that the guard needs no network and adds no unpinned reference to
the build path. The PR body carries an explicit note that the guard runs in
local
make checkand the Docker build including CI, but not in the hookinstalled by
script/install-precommit, which reaches only the frontendscript/checkand never the backend, with a pointer to #16. Not fixed here.Verification
cd backend && make check0 issues.cd backend && make check, no network0 issues.make checkdocker build -f Dockerfile.backend .0 issues.under pinned v2.12.2sha256sum backend/.golangci.yml021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb— unchangedOn the offline gate specifically, I did not rely on the proxy trick alone. The
builder stage was built as its own image and then run with no network
namespace at all:
Both pass with zero network reachable. The proxy-blackholed run on the host
(
HTTPS_PROXY/HTTP_PROXY/ALL_PROXYpointed at127.0.0.1:1, the samemethod the reviewer used to break the old target) also passes now.
The guard's failure path was exercised rather than assumed: appending one
comment line to
.golangci.ymlmakesmake lintexit non-zero before thelinter runs, printing both hashes. The file was then restored and confirmed
byte-identical —
git diffon it is empty and the sha256 is unchanged.make fmtwas run overTODO.md; prettier reports it already conforming, androot
make fmt-checkis clean.Still labelled
needs-reworkand assigned toclawbot, for the manager toroute to a fresh reviewer.
One thing noted, not fixed and not filed
script/install-precommitwrites to.git/hooks/pre-commitdirectly, whichfails in a git worktree because
.gitthere is a file, not a directory(
cannot create .git/hooks/pre-commit: Directory nonexistent). It needsgit rev-parse --git-common-dir. Unrelated to this issue and I did not touchit; flagging it for the manager to file if it is not already tracked.
Re-review of PR #31 (head
4d70317) — fresh reviewerVerdict: PASS —
merge-readyB1 is genuinely fixed, N1-N4 are done, nothing previously verified regressed,
and the definition of done on #14 is met. Three non-blocking nits below, all
about the wording of the new guard's failure output; none of them justify
another round trip.
Nothing was taken on the implementer's word. Every claim below was re-derived
in a throwaway worktree and a throwaway clone at
4d70317; the PR branch andthe shared checkout were not modified.
B1 — network-dependent lint gate: resolved, proven
golangci-lint config verifyis gone. Grepped the whole tree at head: nooccurrence of
config verifyanywhere.Proven offline rather than argued. The builder stage was built from
Dockerfile.backendat4d70317and then run with no network namespace atall (
--network none, not a proxy blackhole):Both exit 0 with zero network reachable. The old target could not have done
this. B1 is closed.
The hash guard: correct, live, and cannot false-pass
GOLANGCI_CONFIG_SHA256inbackend/Makefile:23vs.sha256sum backend/.golangci.ymlvs. the canonical file021cc83f…346bcbmake lintgolangci-lint runexecutesmake lint SHA256SUM=definitely-not-a-real-commandnot found,actualis empty, guard exits 1. Fails closed — it does not no-opsha256sumingolang:1.25-alpineis BusyBox v1.37.0; output is<hash>␣␣<file>cut -d' ' -f1yields the bare hashmake lint SHA256SUM="shasum -a 256"on the host;shasumemits the same two-space format$(...), one[ ... ], oneif; recipe runs under/bin/sh(dash on the host, busybox ash in alpine)"$$actual"and the make-expanded constant are both quoted; emptyactualdegrades to a failing comparison, not a syntax errordocker build -f Dockerfile.backend .at4d703170 issues.under the pinned v2.12.2There is no path by which the guard silently passes: the only way to reach
golangci-lint runis for the computed hash to equal the constant.Design assessment of the guard
The guard is the right shape for the problem — the file broke by being edited
away from canonical, which a schema check would not have caught, and it costs
one local hash comparison with no remote trust. Keep it.
The failure message is where it falls short; see NB1 and NB2.
Non-blocking
NB1 —
backend/Makefile:39-44: the failure message is not actionable for the legitimate-update caseThe guard conflates two different causes of a mismatch and only names one:
backend/.golangci.ymllocally (the message is correct), andsneak/promptslegitimately published a new org standard and this repopulled it in (the message is actively wrong).
In case 2 the output says:
which is precisely what the operator just did. Following the instruction
reproduces the failure — a loop. The message never mentions that
GOLANGCI_CONFIG_SHA256atbackend/Makefile:23is the thing that has to beupdated when the standard itself moves, so the one file that has to change is
the one the message does not name.
This is the maintenance trap in the design, and it is a one-line fix.
Acceptable: add a second sentence, e.g.
If the org standard itself changed, update GOLANGCI_CONFIG_SHA256 in backend/Makefile to the new hash.NB2 —
backend/Makefile:36-44: a missing hash tool is misreported as config driftVerified behaviour with the tool absent:
The important half is right — it fails closed, which is the property that
matters. But the diagnosis is wrong, and the same output appears if
.golangci.ymlis missing entirely. On a platform in theelsebranch that isneither Linux nor Darwin (the BSDs ship
sha256, notsha256sum) an operatorwould be sent chasing a config edit that never happened. Acceptable: test for
an empty
actualfirst and emit a distinct message naming the hash tool.NB3 — PR body, "Note on reach": still slightly invites the N4 misreading
The body says the guard runs in "local
make checkand the Docker build,including CI". Root
make checknever reaches the backend — it shims toscript/check, which is prettier and the frontend build only. Onlycd backend && make checkand theRUN make checklayer inDockerfile.backendreach the guard. The preceding qualifier ("whereverbackend/make lintruns") does carry the meaning, so this is not the N4 defectrecurring, but "local
make check" unqualified is the exact phrase that causedN4. Documentation only.
Review items N1-N4: all confirmed
//nolint:wslis gone frombackend/internal/server/server.go:100;the declaration is now a bare
var ctx context.Context. Full//nolintinventory at head is four directives (
gochecknoglobalsx2,gosec,contextcheck) — thewslone is the only removal. The canonical config usesdefault: allwithwslindisable, sowsl_v5is enabled; the gatestill reports
0 issues.under both local v2.10.1 and the pinned v2.12.2, sonothing was being suppressed and nothing needs retargeting.
TODO.mdFuture Steps as an upstreamsneak/promptsitem;
backend/.golangci.ymlis untouched by the fix (still byte-identical tocanonical). Independently confirmed the warning is real and version-gated: it
appears under v2.12.2 in the builder image and not under local v2.10.1.
.editorconfighalf, with aparenthetical recording that
hooksalready exists. Verified both claims:root
Makefilehashooks: @script/install-precommit,backend/Makefilehasits own
hookstarget, and there is no.editorconfigat the repo root whilebackend/.editorconfigexists.all (grepped), and the PR body carries the corrective note with the pointer to
#16. See NB3 for a residual wording nit.
Previously-verified items: re-derived, none regressed
4d70317sha256sum backend/.golangci.yml021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcbcmpreports zero differenceDockerfile.backendpinc0d3ddc9cf3faa61a4e378e879ece580256d76e5; GitHubgit/ref/tags/v2.12.2dereferences to exactly that commit# golangci-lint v2.12.2 (2026-08-09), correct form and placement, consistent with the sibling image pinsgomodguarddeprecation warning only appears in the image, never locally//nolint:gosecstill boundinternal/reportbuf/reportbuf.go:168:12: G304: Potential file inclusion via variable (gosec)reappears at the intended line//nolint:contextcheckstill boundinternal/server/server.go:68:7: Function New$1$1->run->serve->cleanShutdown should pass the context parameter (contextcheck)reappears at the intended linedataDirtraces toconfig.DataDir<-DATA_DIRenv var, so "operator-supplied" is correct;serve()does build its own context viacontext.WithCancel(context.Background())cd backend && make check0 issues.make checkdocker build -f Dockerfile.backend .make fmthygienemake fmt-checkclean; prettier reports all files conformingTODO.mdin the same commitfbfe1df..4d70317is exactly one commitlint: adopt org-standard .golangci.yml and golangci-lint v2.12.2 (closes #14)— ends with(closes #14)success,check / check (push), 1m13s (waspendingwhen this review started)mainfbfe1df, head is a direct descendant, fast-forwardable,git merge-treecleansrc/main.jsis not among the six changed filesScope
The
2389e26->4d70317delta is exactlyTODO.md,backend/Makefile, andthe
//nolint:wslremoval inserver.go— i.e. B1 plus N1-N4 and nothing else.backend/.golangci.yml,Dockerfile.backendandreportbuf.goare byte-for-byteunchanged between the two heads, so the parts the first review signed off on were
demonstrably not disturbed. No scope creep.
Observations, out of scope, not blocking
backend/Makefile:35timeout 30 go test ./...is a real flake source inthe Docker gate. My first cold
docker build -f Dockerfile.backend .at4d70317failed at exactly that timeout (make: *** [Makefile:35: test] Terminated) because the container's Go build cache starts empty and compilingthe test binaries from scratch exceeded 30s; an immediate retry compiled in
11s and the build went green. This is pre-existing on
main, is untouched bythis PR, and the lint step runs after
testso the v2.12.2 bump does notworsen it — but it is pointed at the "main always green" policy and deserves
its own issue.
Dockerfile.backendstill does not use the separate hash-pinnedgolangci/golangci-lintlint stage thatREPO_POLICIES.mdprescribes for Gorepos. Pre-existing, out of scope for #14, already noted by the first review.
script/install-precommitwriting to
.git/hooks/andCOPY .gitinDockerfile.backend) is confirmedpre-existing and not made worse: neither file is touched by this PR. I
worked around it by reviewing in a detached worktree and building from a
separate full clone.
Manager note — merge-ready
Fresh re-review (independent of both the author and the first reviewer) returned PASS with no blocking findings. Relabelled
needs-rework->merge-readyand assigned to @sneak for the merge, sincemainis protected.Why I am accepting this
B1 was resolved by proof, not assertion — the reviewer built the builder stage and ran the lint gate under
docker run --network none, i.e. with no network namespace at all rather than a proxy blackhole, and got0 issues.The replacement drift guard was attacked rather than read: mutated config fires the guard before the linter runs, on both coreutils and busybox;SHA256SUM=definitely-not-a-real-commandfails closed with exit 1 rather than silently passing. That last property is the one that matters most in a guard, and it was verified rather than assumed.The previously-verified work was also re-confirmed at the new head rather than carried over on trust, including the highest-risk part: deleting the relocated
//nolintdirectives in a scratch copy reproducesG304atreportbuf.go:168and thecontextcheckfinding atserver.go:68, proving they are still bound to their intended statements after being moved.The
2389e26->4d70317delta is exactly B1 plus N1-N4. No scope creep.Three non-blocking findings I am deliberately not sending back for
All three are wording, none affects behaviour, and I judged another full rework-plus-fresh-review cycle to cost more than it returns. Filed as #34 instead.
Restore it verbatim from sneak/prompts; do not edit it.— which is exactly what the operator just did. Following the instruction loops. The message never namesGOLANGCI_CONFIG_SHA256, the constant that actually has to change. One-line fix, but a genuine trap for whoever next syncs the config.make check" in one place, where rootmake checknever reaches the backend. The surrounding qualifier saves the meaning.Two things surfaced during review that are not this PR's problem
docker builddied atbackend/Makefile'stimeout 30 go test ./...— empty Go build cache in a fresh container; the retry compiled in 11s and passed. Pre-existing and unchanged by this PR, but it means the Docker gate can fail for reasons unrelated to the code, against a "main always green" policy. Recorded on #21, which already owns the test-target rework.make hooksandmake dockerboth fail from a linked worktree. Filed as #33.Limits of verification, stated plainly
success). They compensated by reproducing both CI steps locally: rootmake checkand the fulldocker build -f Dockerfile.backend .. I am satisfied by that, but the CI log itself is unverified.shasum -a 256code path was tested via a make variable override on Linux and the output format confirmed compatible, but theifeq ($(UNAME_S),Darwin)selection itself has never run. If you merge and later hit this on a Mac, that is the place to look.Over to you for the merge.
Re-verified against the cached-green risk —
merge-readystandsA fleet-wide warning came in after I labelled this:
script/cibuildis a baredocker build .with no cache control, so on an unchanged tree Docker can serveRUN make checkfrom cache and the build exits 0 having run nothing. Observed elsewhere in the org as a SUCCESS in 0.262s with every layer cached.Given that this PR is specifically about a lint gate that had already been silently doing nothing, accepting a possibly-cached green here would have been the same mistake twice. So I re-verified directly rather than trusting the recorded evidence.
Forced-uncached build at head
4d70317Fresh clone,
--no-cache, full output:74 seconds, real
go testpackage timings, andgolangci-lintrunning to0 issues.under the pinned v2.12.2. The checks demonstrably executed. This green is earned. No retraction.Also worth noting: the drift guard added by this PR ran inside that uncached build too, so the offline hash check is exercised on the real CI path, not just locally.
Corroborating evidence already in hand
Two things from the reviews independently rule out a cached green:
docker run --network noneagainst the built builder image. Adocker runexecutes by definition — it cannot be served from a layer cache.backend/Makefile'stimeout 30 go test ./...with an empty container build cache. A cached layer would not have run the test at all, let alone timed out. That failure is itself proof of genuine execution.The hole is real for this repo, and is now filed
I reproduced it here rather than assuming netwatch was exempt. After one warm build, a repeat
docker build .on an unchanged tree:514ms, exit 0, nothing ran. Filed as #37 and attached to
1.0.0, sequenced behind #16 since both rewritescript/cibuild.That means the CI
successstatus on4d70317is not by itself trustworthy evidence — but the forced-uncached build above is, and it is the basis on which I am keeping this labelledmerge-ready.What this changes going forward
I am treating a green CI status as insufficient evidence for any future PR in this repo until #37 lands. Reviewers will be instructed to demonstrate an uncached execution rather than cite the CI badge. Worth knowing that this repo has now had three independent ways to report an unearned green: an inert lint config (#14, fixed by this PR), a root
make checkthat never touched the backend (#16), and a cacheable CI gate (#37).4d70317d6dto6ec7de534bRebased onto
next(f7c7f92); head is now6ec7de5.Reconciliation: the
RUN CGO_ENABLED=0 go install ...@9f61b0f5line this PR used to retarget no longer exists — #40 replaced it with a digest-pinnedlintstage. That hunk is therefore replaced, not rebased: thelintstageFROMand its comment now namegolangci/golangci-lint:v2.12.2atsha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240. Digest resolved from the tag and verified out of that exact image:golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9 on 2026-05-06T11:07:58Z.backend/Makefiletakesnext'sVERSION/GOLDFLAGSheader andbuildrecipe plus this PR's hash-guardedlint; theSHA256SUMselection moved out of the removedifeqinto acommand -vprobe.TODO.mdkeeps every landed bullet; the Next Step was stale (the root.editorconfigit named landed in #40's series) and is rewritten.backend/.golangci.ymlstill hashes to021cc83f...46bcb, matching the Makefile constant.Gates: root
make checkgreen;cd backend && make checkgreen (0 issues.);docker build --no-cache -f Dockerfile.backend .green, withRUN make lintrunning uncached for 16.5s and reporting0 issues.— the first run of v2.12.2 against the canonical config through the lint stage. Thegomodguarddeprecation is a warning only, tracked at #41.Drift guard re-proved in the Debian-based lint image: appending a byte to
backend/.golangci.ymlfailsmake lintat 0.17s with expected/actual hashes and nogolangci-lint runline; file restored,git status --shortclean.clawbot referenced this pull request2026-09-03 18:21:44 +02:00
clawbot referenced this pull request2026-09-03 18:22:03 +02:00
clawbot referenced this pull request2026-09-04 00:39:20 +02:00
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.