golangci config drift guard: failure message loops the operator when the org standard legitimately updates #34

Open
opened 2026-08-09 04:25:53 +02:00 by clawbot · 1 comment
Collaborator

Problem

Follow-up to PR #31, which added an offline sha256 drift guard to backend/Makefile's lint target. The guard itself is correct and fails closed — these are three wording defects in its failure output, graded non-blocking during review and deliberately not sent back for a second rework cycle.

NB1 — the failure message tells the operator to do the thing that loops

The guard conflates two very different situations and prints the same message for both:

  1. Someone edited backend/.golangci.yml locally. The message is right: restore it from the org standard.
  2. The org standard itself legitimately moved. The operator syncs the new canonical file, the hash no longer matches the constant, and the guard fires with Restore it verbatim from sneak/prompts; do not edit it. — which is exactly what they just did. Following the instruction reproduces the failure indefinitely.

The message never names GOLANGCI_CONFIG_SHA256 in backend/Makefile, which is the one thing that actually has to change in case 2. Whoever next syncs the config gets a confusing debugging session for no reason.

This is the one worth fixing. Given that the whole point of #14 was that the org standard had drifted, case 2 is not hypothetical — it is the expected steady-state maintenance path.

NB2 — a missing hash tool is misreported as config drift

If sha256sum / shasum is absent or fails, actual comes back empty, the comparison fails, and the operator is told their config has drifted. It has not.

Verified during review that this fails closed (make lint SHA256SUM=definitely-not-a-real-command exits 1), which is the property that actually matters — so this is a diagnosis bug, not a safety bug. Same wrong message if .golangci.yml is missing entirely, and on a BSD host where the tool is sha256 rather than sha256sum and the Makefile falls into the else branch.

NB3 — PR #31's body still says "local make check"

Root make check never reaches the backend. The surrounding qualifier saves the meaning, so this is not a recurrence of the earlier N4 finding, but the phrase is still loose. Cosmetic; fix only if touching the PR body for another reason. Not worth a commit on its own.

Definition of done

  • The guard's failure message distinguishes the two cases, or — simpler and probably better — states both remedies explicitly: restore the file from the org standard if you edited it, or update GOLANGCI_CONFIG_SHA256 in backend/Makefile if the org standard itself changed. Naming the constant is the essential part.
  • The message prints the expected and actual hashes (it already does — do not regress that).
  • A missing or non-functional hash tool produces a distinct, accurate message rather than being reported as drift. It must still fail closed — verify with make lint SHA256SUM=definitely-not-a-real-command and confirm a non-zero exit.
  • A missing .golangci.yml produces its own distinct message.
  • The guard still fires correctly on genuine drift. Verify by mutating a scratch copy, then confirm the real file is byte-identical afterwards (sha256sum backend/.golangci.yml = 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb).
  • Still POSIX sh, no bashisms, still works under busybox in the pinned alpine builder image and under coreutils on Linux.
  • cd backend && make check passes offline; root make check passes; docker build -f Dockerfile.backend . passes.
  • TODO.md updated in the same commit.
  • Commit title ends with (closes #N).

Implementation requirements

  • Do not edit backend/.golangci.yml. This issue is about the Makefile's error handling only.
  • Do not replace the hash guard with golangci-lint config verify — that was PR #31's original approach and it was rejected because it fetches its schema over an unpinned live HTTPS request. See the blocking finding on #31 for the full reasoning.
  • Not on the 1.0.0 milestone. It does not block a tag.
  • This is small. If you are already in backend/Makefile for #16 or #21, folding it in is reasonable — say so on this issue rather than opening a redundant PR.
  • make targets and script/ entrypoints only.
  • No attribution trailers in the commit message.
## Problem Follow-up to PR #31, which added an offline sha256 drift guard to `backend/Makefile`'s `lint` target. The guard itself is correct and fails closed — these are three wording defects in its failure output, graded non-blocking during review and deliberately not sent back for a second rework cycle. ### NB1 — the failure message tells the operator to do the thing that loops The guard conflates two very different situations and prints the same message for both: 1. Someone edited `backend/.golangci.yml` locally. The message is right: restore it from the org standard. 2. **The org standard itself legitimately moved.** The operator syncs the new canonical file, the hash no longer matches the constant, and the guard fires with `Restore it verbatim from sneak/prompts; do not edit it.` — which is exactly what they just did. Following the instruction reproduces the failure indefinitely. The message never names `GOLANGCI_CONFIG_SHA256` in `backend/Makefile`, which is the one thing that actually has to change in case 2. Whoever next syncs the config gets a confusing debugging session for no reason. This is the one worth fixing. Given that the whole point of #14 was that the org standard had drifted, case 2 is not hypothetical — it is the expected steady-state maintenance path. ### NB2 — a missing hash tool is misreported as config drift If `sha256sum` / `shasum` is absent or fails, `actual` comes back empty, the comparison fails, and the operator is told their config has drifted. It has not. Verified during review that this **fails closed** (`make lint SHA256SUM=definitely-not-a-real-command` exits 1), which is the property that actually matters — so this is a diagnosis bug, not a safety bug. Same wrong message if `.golangci.yml` is missing entirely, and on a BSD host where the tool is `sha256` rather than `sha256sum` and the Makefile falls into the `else` branch. ### NB3 — PR #31's body still says "local `make check`" Root `make check` never reaches the backend. The surrounding qualifier saves the meaning, so this is not a recurrence of the earlier N4 finding, but the phrase is still loose. Cosmetic; fix only if touching the PR body for another reason. Not worth a commit on its own. ## Definition of done - [ ] The guard's failure message distinguishes the two cases, or — simpler and probably better — states both remedies explicitly: restore the file from the org standard **if you edited it**, or update `GOLANGCI_CONFIG_SHA256` in `backend/Makefile` **if the org standard itself changed**. Naming the constant is the essential part. - [ ] The message prints the expected and actual hashes (it already does — do not regress that). - [ ] A missing or non-functional hash tool produces a distinct, accurate message rather than being reported as drift. It must still fail closed — verify with `make lint SHA256SUM=definitely-not-a-real-command` and confirm a non-zero exit. - [ ] A missing `.golangci.yml` produces its own distinct message. - [ ] The guard still fires correctly on genuine drift. Verify by mutating a scratch copy, then confirm the real file is byte-identical afterwards (`sha256sum backend/.golangci.yml` = `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`). - [ ] Still POSIX sh, no bashisms, still works under busybox in the pinned alpine builder image and under coreutils on Linux. - [ ] `cd backend && make check` passes offline; root `make check` passes; `docker build -f Dockerfile.backend .` passes. - [ ] `TODO.md` updated in the same commit. - [ ] Commit title ends with ` (closes #N)`. ## Implementation requirements - Do **not** edit `backend/.golangci.yml`. This issue is about the Makefile's error handling only. - Do not replace the hash guard with `golangci-lint config verify` — that was PR #31's original approach and it was rejected because it fetches its schema over an unpinned live HTTPS request. See the blocking finding on #31 for the full reasoning. - Not on the `1.0.0` milestone. It does not block a tag. - This is small. If you are already in `backend/Makefile` for #16 or #21, folding it in is reasonable — say so on this issue rather than opening a redundant PR. - `make` targets and `script/` entrypoints only. - No attribution trailers in the commit message.
Author
Collaborator

Location update: the guard is moving to backend/script/lint

This issue was written against backend/Makefile's lint target, which is where PR #31 put the drift guard. PR #38 relocates it.

Under #38, backend/Makefile's lint becomes a thin shim (@script/lint) and the guard moves into a new backend/script/lint, rewritten from a make recipe into POSIX sh — the SHA256SUM make-variable branch becomes a sha256() shell function preferring sha256sum with a shasum -a 256 fallback. Same logic, same failure text, different file and different language.

So when this issue is implemented:

  • Read the tree as it exists then. If #38 has landed, edit backend/script/lint. If it has not, edit backend/Makefile. Do not work from the line numbers in this issue's original description.
  • The constant is still named GOLANGCI_CONFIG_SHA256 in both versions, so grepping for that name is the reliable way to find it.
  • The fix is now a shell edit rather than a make-recipe edit, which if anything makes it easier — multi-line conditionals in POSIX sh are far more readable than in a make recipe with backslash continuations.

Everything else in this issue is unchanged. NB1 (the message loops the operator when the org standard legitimately updates) and NB2 (a missing hash tool is misreported as drift) both survive the move verbatim — #38 relocated the logic without altering the error text.

One addition surfaced by #38's review: the guard's error text leads with the wrong remedy. It opens by telling the operator to restore the file from the org standard, which is the correct advice in only one of the two cases. Whichever case is more likely should not be the one buried second. Worth deciding deliberately rather than leaving the existing order by default.

## Location update: the guard is moving to `backend/script/lint` This issue was written against `backend/Makefile`'s `lint` target, which is where PR #31 put the drift guard. **PR #38 relocates it.** Under #38, `backend/Makefile`'s `lint` becomes a thin shim (`@script/lint`) and the guard moves into a new `backend/script/lint`, rewritten from a make recipe into POSIX sh — the `SHA256SUM` make-variable branch becomes a `sha256()` shell function preferring `sha256sum` with a `shasum -a 256` fallback. Same logic, same failure text, different file and different language. So when this issue is implemented: - **Read the tree as it exists then.** If #38 has landed, edit `backend/script/lint`. If it has not, edit `backend/Makefile`. Do not work from the line numbers in this issue's original description. - The constant is still named `GOLANGCI_CONFIG_SHA256` in both versions, so grepping for that name is the reliable way to find it. - The fix is now a shell edit rather than a make-recipe edit, which if anything makes it easier — multi-line conditionals in POSIX sh are far more readable than in a make recipe with backslash continuations. Everything else in this issue is unchanged. NB1 (the message loops the operator when the org standard legitimately updates) and NB2 (a missing hash tool is misreported as drift) both survive the move verbatim — #38 relocated the logic without altering the error text. One addition surfaced by #38's review: the guard's error text **leads with the wrong remedy**. It opens by telling the operator to restore the file from the org standard, which is the correct advice in only one of the two cases. Whichever case is more likely should not be the one buried second. Worth deciding deliberately rather than leaving the existing order by default.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/netwatch#34