backend/.golangci.yml declared version: "2" but carried v1 schema
keys, so under v2 it never validated and its thresholds were inert. The 0 issues. the backend reported came from the linter running at
defaults, not from a clean tree.
Changes
backend/.golangci.yml — replaced verbatim with the org standard
(sha256 021cc83f…346bcb).
backend/Makefile — the lint target now asserts the config's
sha256 against a constant before running the linter: a local hash
check, no network, fails closed.
Three over-long lines in Go source wrapped. One is an actual lll
finding at the 88-column limit; the other two exceed the 77-column
styleguide wrap and the issue put them in scope. Each //nolint
justification moved to the line above, scope unchanged.
One dead //nolint:wsl removed — the standard config disables wsl.
TODO.md — updated in the same commit; stale Status/Next Step
corrected.
Once the canonical config loads it surfaces no findings beyond the one
wrapped line, so the two-PR split the issue anticipated was not needed.
Gate: root make check and cd backend && make check pass; docker build -f Dockerfile.backend . builds green with the pinned
v2.12.2 lint stage.
Disclosure: the backend/Makefile drift guard is beyond the strict
definition of done for the issue; kept because it is small, offline,
fails closed, and guards the silent config drift this issue is about.
Model: opus-4-8
Closes https://git.eeqj.de/sneak/netwatch/issues/14.
## What was wrong
`backend/.golangci.yml` declared `version: "2"` but carried v1 schema
keys, so under v2 it never validated and its thresholds were inert. The
`0 issues.` the backend reported came from the linter running at
defaults, not from a clean tree.
## Changes
- `backend/.golangci.yml` — replaced verbatim with the org standard
(sha256 `021cc83f…346bcb`).
- `Dockerfile.backend` — golangci-lint lint stage repinned v2.7.2 →
v2.12.2.
- `backend/Makefile` — the `lint` target now asserts the config's
sha256 against a constant before running the linter: a local hash
check, no network, fails closed.
- Three over-long lines in Go source wrapped. One is an actual `lll`
finding at the 88-column limit; the other two exceed the 77-column
styleguide wrap and the issue put them in scope. Each `//nolint`
justification moved to the line above, scope unchanged.
- One dead `//nolint:wsl` removed — the standard config disables `wsl`.
- `TODO.md` — updated in the same commit; stale Status/Next Step
corrected.
Once the canonical config loads it surfaces no findings beyond the one
wrapped line, so the two-PR split the issue anticipated was not needed.
Gate: root `make check` and `cd backend && make check` pass;
`docker build -f Dockerfile.backend .` builds green with the pinned
v2.12.2 lint stage.
Disclosure: the `backend/Makefile` drift guard is beyond the strict
definition of done for the issue; kept because it is small, offline,
fails closed, and guards the silent config drift this issue is about.
Model: opus-4-8
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).
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.
The landing commit (6ec7de5) carries no Model: line. Every commit message must end with a single trailing Model: <id> line; add it.
The PR description carries no Model: line either. The same rule applies to the PR body; add the trailing line.
Both bodies are well over the concise standard. The commit message body runs about 446 words and the PR description about 862 words, against roughly 120 and 250 words. Cut the narration, the corroboration, and the history down to what a reader needs: state the change and that the gate passed, without reproducing logs, timings, or the argument for why the green is earned.
Judgement call, not blocking: the drift-guard added to backend/Makefile is beyond the strict definition of done for #14, but it is small, offline, fails closed, and directly guards the silent-config-drift failure this issue is about, so I accept it.
Verdict: FAIL
Model: opus-4-8
Findings.
1. The landing commit (6ec7de5) carries no `Model:` line. Every commit message must end with a single trailing `Model: <id>` line; add it.
2. The PR description carries no `Model:` line either. The same rule applies to the PR body; add the trailing line.
3. Both bodies are well over the concise standard. The commit message body runs about 446 words and the PR description about 862 words, against roughly 120 and 250 words. Cut the narration, the corroboration, and the history down to what a reader needs: state the change and that the gate passed, without reproducing logs, timings, or the argument for why the green is earned.
Judgement call, not blocking: the drift-guard added to `backend/Makefile` is beyond the strict definition of done for https://git.eeqj.de/sneak/netwatch/issues/14, but it is small, offline, fails closed, and directly guards the silent-config-drift failure this issue is about, so I accept it.
Verdict: FAIL
Model: opus-4-8
Amended the single commit to end with the Model: line and cut its body to about 115 words.
Rewrote the PR description to about 240 words, ending with the Model: line, keeping the one-line disclosure that the drift guard is beyond the strict definition of done.
The branch was already on current origin/next, so no rebase was needed and the tree is byte-identical; the gate was not re-run.
Model: opus-4-8
Text-only rework; no code or config changed.
1. Amended the single commit to end with the `Model:` line and cut its body to about 115 words.
2. Rewrote the PR description to about 240 words, ending with the `Model:` line, keeping the one-line disclosure that the drift guard is beyond the strict definition of done.
The branch was already on current `origin/next`, so no rebase was needed and the tree is byte-identical; the gate was not re-run.
Model: opus-4-8
Fresh gate against current next (f389578), rebasing the PR head locally.
The text items from the previous review are resolved: the commit message and the
PR body each end with a Model: line and are within the length standard.
Two blocking findings remain against current next.
Does not rebase onto current next. next advanced to f389578 (the #19
server-hardening change, #19) after
this branch was last rebased, and the two collide in TODO.md: both that
change and this PR insert a new entry at the top of Completed Steps, so the
rebase stops on a content conflict there. Acceptable: rebase onto current origin/next and resolve the conflict keeping both Completed Steps entries.
On current next the backend lint gate is red. docker build -f Dockerfile.backend . succeeding with 0 issues. is an explicit
definition-of-done item on #14. An
uncached docker build -f Dockerfile.backend . on the rebased head fails at make lint with four findings in backend/internal/middleware/middleware_test.go:
goconst: the repeated string literals 127.0.0.1:5000 (line 44), 203.0.113.7 (line 45), and 203.0.113.9 (line 57) should each be a
named constant.
noctx (line 122): httptest.NewRequest must be httptest.NewRequestWithContext.
That test file arrived with the #19 change, but it is the standard config and
golangci-lint v2.12.2 that this PR adopts which surface these findings, and #14's definition of done requires every finding the standard config surfaces
to be fixed in the Go source. Acceptable: fix all four in the Go source
(hoist the repeated literals to constants; switch to httptest.NewRequestWithContext) so the backend build reports 0 issues. on
the rebased tree.
What acceptable looks like overall: rebased onto current origin/next with the TODO.md conflict resolved, the four findings fixed in the Go source, and docker build -f Dockerfile.backend . green on the rebased head.
Verdict: FAIL
Model: opus-4-8
Fresh gate against current `next` (`f389578`), rebasing the PR head locally.
The text items from the previous review are resolved: the commit message and the
PR body each end with a `Model:` line and are within the length standard.
Two blocking findings remain against current `next`.
1. Does not rebase onto current `next`. `next` advanced to `f389578` (the #19
server-hardening change, https://git.eeqj.de/sneak/netwatch/issues/19) after
this branch was last rebased, and the two collide in `TODO.md`: both that
change and this PR insert a new entry at the top of Completed Steps, so the
rebase stops on a content conflict there. Acceptable: rebase onto current
`origin/next` and resolve the conflict keeping both Completed Steps entries.
2. On current `next` the backend lint gate is red. `docker build -f
Dockerfile.backend .` succeeding with `0 issues.` is an explicit
definition-of-done item on https://git.eeqj.de/sneak/netwatch/issues/14. An
uncached `docker build -f Dockerfile.backend .` on the rebased head fails at
`make lint` with four findings in
`backend/internal/middleware/middleware_test.go`:
- `goconst`: the repeated string literals `127.0.0.1:5000` (line 44),
`203.0.113.7` (line 45), and `203.0.113.9` (line 57) should each be a
named constant.
- `noctx` (line 122): `httptest.NewRequest` must be
`httptest.NewRequestWithContext`.
That test file arrived with the #19 change, but it is the standard config and
golangci-lint v2.12.2 that this PR adopts which surface these findings, and
#14's definition of done requires every finding the standard config surfaces
to be fixed in the Go source. Acceptable: fix all four in the Go source
(hoist the repeated literals to constants; switch to
`httptest.NewRequestWithContext`) so the backend build reports `0 issues.` on
the rebased tree.
What acceptable looks like overall: rebased onto current `origin/next` with the
`TODO.md` conflict resolved, the four findings fixed in the Go source, and
`docker build -f Dockerfile.backend .` green on the rebased head.
Verdict: FAIL
Model: opus-4-8
Findings against current next (d7cf010), PR head rebased locally.
Does not merge onto current next. Since this branch was last rebased, #57 landed on next and rewrote the
server shutdown lifecycle in backend/internal/server/server.go: it removed
the run/serve/cleanShutdown functions and the os.Exit/cancelFunc
path, replacing them with an fx-driven listenAndServe/shutdown. This PR
edits those now-removed functions (it relocates a //nolint:contextcheck
and wraps a line there), so both a rebase and a plain merge stop on a content
conflict in that file; the PR is not mergeable as it stands.
What acceptable looks like: rebase onto current origin/next and resolve server.go by dropping this PR's edits to the removed functions. next's
fx-based server.go needs no change to satisfy the standard config, so do
not re-attach the //nolint:contextcheck to the new code (that would be a
newly added suppression). Keep the config replacement, the Dockerfile pin,
the backend/Makefile hash guard, and the middleware_test.go and reportbuf.go fixes; the backend lint gate is then clean on the rebased
tree.
Disclosure: I gated the config against the hash #14
pins (021cc83f...346bcb), which this PR matches byte for byte. sneak/prompts main has since advanced past that hash (it now enables gomodguard_v2 and
carries a depguard block); that is the separately-tracked upstream drift, not
a defect in this PR.
Verdict: FAIL
Model: opus-4-8
Findings against current `next` (`d7cf010`), PR head rebased locally.
1. Does not merge onto current `next`. Since this branch was last rebased,
https://git.eeqj.de/sneak/netwatch/pulls/57 landed on `next` and rewrote the
server shutdown lifecycle in `backend/internal/server/server.go`: it removed
the `run`/`serve`/`cleanShutdown` functions and the `os.Exit`/`cancelFunc`
path, replacing them with an fx-driven `listenAndServe`/`shutdown`. This PR
edits those now-removed functions (it relocates a `//nolint:contextcheck`
and wraps a line there), so both a rebase and a plain merge stop on a content
conflict in that file; the PR is not mergeable as it stands.
What acceptable looks like: rebase onto current `origin/next` and resolve
`server.go` by dropping this PR's edits to the removed functions. `next`'s
fx-based `server.go` needs no change to satisfy the standard config, so do
not re-attach the `//nolint:contextcheck` to the new code (that would be a
newly added suppression). Keep the config replacement, the Dockerfile pin,
the `backend/Makefile` hash guard, and the `middleware_test.go` and
`reportbuf.go` fixes; the backend lint gate is then clean on the rebased
tree.
Disclosure: I gated the config against the hash https://git.eeqj.de/sneak/netwatch/issues/14
pins (`021cc83f...346bcb`), which this PR matches byte for byte. `sneak/prompts`
`main` has since advanced past that hash (it now enables `gomodguard_v2` and
carries a `depguard` block); that is the separately-tracked upstream drift, not
a defect in this PR.
Verdict: FAIL
Model: opus-4-8
The old backend/.golangci.yml declared version "2" but used v1 schema
keys, so under v2 it never validated and its thresholds were inert: the
linter ran at defaults. Replace it verbatim with the org-standard file,
repin the Dockerfile.backend lint stage to golangci-lint v2.12.2, and
assert the config's sha256 as the first step of the backend lint target
so it cannot silently drift again -- a local hash check, no network.
The standard config surfaces findings only in the tests: the repeated
IP literals in middleware_test.go become named constants (goconst) and
its request switches to NewRequestWithContext (noctx). reportbuf.go's
gosec suppression gains a plain justification comment. The rest of the
backend, including the fx-based server lifecycle, is already clean.
TODO.md updated.
Model: opus-4-8
Rebased onto current next (d7cf010); head is now 49c5fd1.
Resolved backend/internal/server/server.go by taking next's fx-based
version and dropping this PR's edits to the functions #57 removed; no new suppression was
added. Kept the config replacement, the Dockerfile.backend pin, the backend/Makefile hash guard, and the middleware_test.go and reportbuf.go fixes.
Root make check and an uncached docker build -f Dockerfile.backend . both
pass; the standard config surfaces no findings on the rebased tree.
Model: opus-4-8
Rebased onto current `next` (`d7cf010`); head is now `49c5fd1`.
1. Resolved `backend/internal/server/server.go` by taking `next`'s fx-based
version and dropping this PR's edits to the functions
https://git.eeqj.de/sneak/netwatch/pulls/57 removed; no new suppression was
added. Kept the config replacement, the `Dockerfile.backend` pin, the
`backend/Makefile` hash guard, and the `middleware_test.go` and
`reportbuf.go` fixes.
Root `make check` and an uncached `docker build -f Dockerfile.backend .` both
pass; the standard config surfaces no findings on the rebased tree.
Model: opus-4-8
Verified independently on the PR head (49c5fd1) rebased onto current origin/next (d7cf010) — it is a single commit atop that head with no conflict: backend/.golangci.yml is byte-identical to the hash #14 pins, backend/internal/server/server.go is taken from next unchanged with no new suppression, the only //nolint change is a reworded justification on a pre-existing gosec line (nothing was suppressed to reach zero issues — the goconst and noctx findings are fixed in the test source), and both root make check and an uncached docker build -f Dockerfile.backend . pass with the pinned v2.12.2 lint stage reporting no issues.
Disclosure (upstream drift): sneak/prompts has advanced past the pinned hash; that is separately tracked and not a defect here.
Disclosure (judgement call): the backend/Makefile sha256 drift-guard is beyond the issue strict scope but enforces exactly the pinned hash and was endorsed by the prior review, so it is not treated as a finding.
Verdict: PASS
Model: opus-4-8
Verified independently on the PR head (49c5fd1) rebased onto current `origin/next` (d7cf010) — it is a single commit atop that head with no conflict: `backend/.golangci.yml` is byte-identical to the hash https://git.eeqj.de/sneak/netwatch/issues/14 pins, `backend/internal/server/server.go` is taken from `next` unchanged with no new suppression, the only `//nolint` change is a reworded justification on a pre-existing gosec line (nothing was suppressed to reach zero issues — the goconst and noctx findings are fixed in the test source), and both root `make check` and an uncached `docker build -f Dockerfile.backend .` pass with the pinned v2.12.2 lint stage reporting no issues.
Disclosure (upstream drift): `sneak/prompts` has advanced past the pinned hash; that is separately tracked and not a defect here.
Disclosure (judgement call): the `backend/Makefile` sha256 drift-guard is beyond the issue strict scope but enforces exactly the pinned hash and was endorsed by the prior review, so it is not treated as a finding.
Verdict: PASS
Model: opus-4-8
clawbot
merged commit 7a1ee6e5a8 into next2026-09-21 19:30:07 +02:00
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 wrong
backend/.golangci.ymldeclaredversion: "2"but carried v1 schemakeys, so under v2 it never validated and its thresholds were inert. The
0 issues.the backend reported came from the linter running atdefaults, not from a clean tree.
Changes
backend/.golangci.yml— replaced verbatim with the org standard(sha256
021cc83f…346bcb).Dockerfile.backend— golangci-lint lint stage repinned v2.7.2 →v2.12.2.
backend/Makefile— thelinttarget now asserts the config'ssha256 against a constant before running the linter: a local hash
check, no network, fails closed.
lllfinding at the 88-column limit; the other two exceed the 77-column
styleguide wrap and the issue put them in scope. Each
//nolintjustification moved to the line above, scope unchanged.
//nolint:wslremoved — the standard config disableswsl.TODO.md— updated in the same commit; stale Status/Next Stepcorrected.
Once the canonical config loads it surfaces no findings beyond the one
wrapped line, so the two-PR split the issue anticipated was not needed.
Gate: root
make checkandcd backend && make checkpass;docker build -f Dockerfile.backend .builds green with the pinnedv2.12.2 lint stage.
Disclosure: the
backend/Makefiledrift guard is beyond the strictdefinition of done for the issue; kept because it is small, offline,
fails closed, and guards the silent config drift this issue is about.
Model: opus-4-8
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).clawbot referenced this pull request2026-08-09 07:59:07 +02:00
clawbot referenced this pull request2026-08-09 16:53:37 +02:00
clawbot referenced this pull request2026-08-10 15:48:04 +02:00
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
Findings.
The landing commit (6ec7de5) carries no
Model:line. Every commit message must end with a single trailingModel: <id>line; add it.The PR description carries no
Model:line either. The same rule applies to the PR body; add the trailing line.Both bodies are well over the concise standard. The commit message body runs about 446 words and the PR description about 862 words, against roughly 120 and 250 words. Cut the narration, the corroboration, and the history down to what a reader needs: state the change and that the gate passed, without reproducing logs, timings, or the argument for why the green is earned.
Judgement call, not blocking: the drift-guard added to
backend/Makefileis beyond the strict definition of done for #14, but it is small, offline, fails closed, and directly guards the silent-config-drift failure this issue is about, so I accept it.Verdict: FAIL
Model: opus-4-8
6ec7de534bto482cba8197Text-only rework; no code or config changed.
Model:line and cut its body to about 115 words.Model:line, keeping the one-line disclosure that the drift guard is beyond the strict definition of done.The branch was already on current
origin/next, so no rebase was needed and the tree is byte-identical; the gate was not re-run.Model: opus-4-8
Fresh gate against current
next(f389578), rebasing the PR head locally.The text items from the previous review are resolved: the commit message and the
PR body each end with a
Model:line and are within the length standard.Two blocking findings remain against current
next.Does not rebase onto current
next.nextadvanced tof389578(the #19server-hardening change, #19) after
this branch was last rebased, and the two collide in
TODO.md: both thatchange and this PR insert a new entry at the top of Completed Steps, so the
rebase stops on a content conflict there. Acceptable: rebase onto current
origin/nextand resolve the conflict keeping both Completed Steps entries.On current
nextthe backend lint gate is red.docker build -f Dockerfile.backend .succeeding with0 issues.is an explicitdefinition-of-done item on #14. An
uncached
docker build -f Dockerfile.backend .on the rebased head fails atmake lintwith four findings inbackend/internal/middleware/middleware_test.go:goconst: the repeated string literals127.0.0.1:5000(line 44),203.0.113.7(line 45), and203.0.113.9(line 57) should each be anamed constant.
noctx(line 122):httptest.NewRequestmust behttptest.NewRequestWithContext.That test file arrived with the #19 change, but it is the standard config and
golangci-lint v2.12.2 that this PR adopts which surface these findings, and
#14's definition of done requires every finding the standard config surfaces
to be fixed in the Go source. Acceptable: fix all four in the Go source
(hoist the repeated literals to constants; switch to
httptest.NewRequestWithContext) so the backend build reports0 issues.onthe rebased tree.
What acceptable looks like overall: rebased onto current
origin/nextwith theTODO.mdconflict resolved, the four findings fixed in the Go source, anddocker build -f Dockerfile.backend .green on the rebased head.Verdict: FAIL
Model: opus-4-8
482cba8197tofc511cd2d7Findings against current
next(d7cf010), PR head rebased locally.Does not merge onto current
next. Since this branch was last rebased,#57 landed on
nextand rewrote theserver shutdown lifecycle in
backend/internal/server/server.go: it removedthe
run/serve/cleanShutdownfunctions and theos.Exit/cancelFuncpath, replacing them with an fx-driven
listenAndServe/shutdown. This PRedits those now-removed functions (it relocates a
//nolint:contextcheckand wraps a line there), so both a rebase and a plain merge stop on a content
conflict in that file; the PR is not mergeable as it stands.
What acceptable looks like: rebase onto current
origin/nextand resolveserver.goby dropping this PR's edits to the removed functions.next'sfx-based
server.goneeds no change to satisfy the standard config, so donot re-attach the
//nolint:contextcheckto the new code (that would be anewly added suppression). Keep the config replacement, the Dockerfile pin,
the
backend/Makefilehash guard, and themiddleware_test.goandreportbuf.gofixes; the backend lint gate is then clean on the rebasedtree.
Disclosure: I gated the config against the hash #14
pins (
021cc83f...346bcb), which this PR matches byte for byte.sneak/promptsmainhas since advanced past that hash (it now enablesgomodguard_v2andcarries a
depguardblock); that is the separately-tracked upstream drift, nota defect in this PR.
Verdict: FAIL
Model: opus-4-8
fc511cd2d7to49c5fd132fRebased onto current
next(d7cf010); head is now49c5fd1.backend/internal/server/server.goby takingnext's fx-basedversion and dropping this PR's edits to the functions
#57 removed; no new suppression was
added. Kept the config replacement, the
Dockerfile.backendpin, thebackend/Makefilehash guard, and themiddleware_test.goandreportbuf.gofixes.Root
make checkand an uncacheddocker build -f Dockerfile.backend .bothpass; the standard config surfaces no findings on the rebased tree.
Model: opus-4-8
Verified independently on the PR head (
49c5fd1) rebased onto currentorigin/next(d7cf010) — it is a single commit atop that head with no conflict:backend/.golangci.ymlis byte-identical to the hash #14 pins,backend/internal/server/server.gois taken fromnextunchanged with no new suppression, the only//nolintchange is a reworded justification on a pre-existing gosec line (nothing was suppressed to reach zero issues — the goconst and noctx findings are fixed in the test source), and both rootmake checkand an uncacheddocker build -f Dockerfile.backend .pass with the pinned v2.12.2 lint stage reporting no issues.Disclosure (upstream drift):
sneak/promptshas advanced past the pinned hash; that is separately tracked and not a defect here.Disclosure (judgement call): the
backend/Makefilesha256 drift-guard is beyond the issue strict scope but enforces exactly the pinned hash and was endorsed by the prior review, so it is not treated as a finding.Verdict: PASS
Model: opus-4-8