fmt tooling is incomplete: fmt-check does not verify goimports, and no formatter covers Markdown #119

Closed
opened 2026-08-09 07:44:52 +02:00 by clawbot · 4 comments
Collaborator

Two related gaps in the formatting entrypoints. Both mean make fmt-check can pass on a tree that make fmt would change — which is exactly what a format gate exists to prevent.

1. script/fmt-check does not check what script/fmt applies

script/fmt runs two tools:

gofmt -s -w .
goimports -w .

script/fmt-check checks only the first:

files="$(gofmt -l .)"
if [ -n "$files" ]; then ... exit 1; fi

goimports is never verified. So a file with mis-grouped or mis-ordered imports — something make fmt would rewrite — sails through make fmt-check, and therefore through make check, CI, and the Docker build. The gate is asymmetric with the formatter it is supposed to police.

This is the same shape of defect as #115 (script/cibuild reporting a green it did not earn): a check that appears authoritative while silently not covering part of what it claims.

Note gofmt -l . and goimports both walk the tree from the repo root — confirm whatever you add does not descend into vendored or generated paths if any are later introduced.

2. No formatter covers Markdown at all

There is no .prettierrc and no .prettierignore in this repo, and neither script/fmt nor script/fmt-check touches Markdown. The convention across these repos is prettier with default configuration plus two exceptions — four-space indents and proseWrap: always (hard-wrap at 80 columns).

This repo carries four Markdown files that are edited constantly — README.md, TODO.md, TESTING.md, REPO_POLICIES.md — and TODO.md is touched by every commit by policy. Every one of those edits has been hand-formatted to approximate the surrounding style, because there is no tool to do it. The implementer of #99 hit exactly this and said so in their PR.

The result is drift that no gate can catch and no author can be blamed for.

Definition of done

  1. script/fmt-check verifies everything script/fmt applies. For goimports, use its list mode (goimports -l) and fail with the offending filenames, mirroring the existing gofmt -l handling so the output style stays consistent.
  2. .prettierrc and .prettierignore added at the repo root. .prettierrc sets four-space indentation and proseWrap: always, otherwise prettier defaults.
  3. .prettierignore must exclude static/css/tailwind.min.css. It is 9KB of minified vendor output; reformatting it would produce an enormous meaningless diff and could plausibly break the embedded stylesheet the dashboard depends on. Exclude any other generated or vendored asset you find.
  4. script/fmt formats Markdown (and any other prettier-covered files not excluded); script/fmt-check checks them read-only via prettier --check.
  5. script/bootstrap installs prettier at a pinned version, consistent with how golangci-lint and goimports are pinned. See the hard constraints below — this is the part most likely to go wrong.
  6. Both scripts stay POSIX sh (#!/bin/sh, set -eu, no bashisms) — they run in minimal alpine images with no bash — and keep the $(cd "$(dirname "$0")/.." && pwd -P) root-location idiom.
  7. Reformat the existing Markdown in the same PR so the tree is clean under the new check. Expect this to touch all four Markdown files; that is fine and expected, but keep it to formatting only — no wording changes, so the diff stays reviewable as pure reflow.
  8. Verify with a negative control, not by inspection. For each of the two gaps: introduce a deliberately mis-ordered import and confirm make fmt-check now fails naming that file; introduce a badly wrapped Markdown paragraph and confirm make fmt-check fails on it. Revert both, confirm git status is clean, confirm make check is green. Report what you planted and what each produced. A code-reading argument is not sufficient — these are bugs about a gate not doing what it claims.
  9. make check green; TODO.md updated in the same commit.

The finishing commit's title must end with (closes #N) referencing this issue.

Hard constraints

  • Every external reference must be pinned by cryptographic hash. This is the single most important rule in REPO_POLICIES.md and it has zero exceptions. If you add prettier, pin it exactly — a lockfile with integrity hashes, or a pinned container image by sha256. Never curl | sh, never npx prettier unpinned, never @latest.
  • Adding a Node/yarn toolchain to a pure-Go repo is a real cost. It affects script/bootstrap, the Dockerfile, and Docker build time (policy ceiling: 5 minutes). If the only clean way you can find to pin prettier drags a full Node install into this repo's bootstrap and image, stop and report back with what you found rather than committing it — that trade-off is worth a decision, not an assumption. A hash-pinned prettier container image used only by script/fmt/script/fmt-check may be the lighter option; evaluate it.
  • Do not modify .golangci.yml — sha256 must remain 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.
  • Do not change the golangci-lint or goimports pins.
  • Do not reformat static/css/tailwind.min.css.
  • DNS is never mocked in this repository; nothing here should touch test behaviour.

Coordination

Touches script/fmt, script/fmt-check, and script/bootstrap. #117 also changes script/bootstrap (making pinned installs actually take effect) and #115 changes script/cibuild. All three are small; whichever lands later rebases. Keep them as separate PRs — do not merge them into one.

Item 7 will conflict with any open PR that edits Markdown. At time of writing that includes PRs #97, #112, #113, and #118. Land this after those have merged, or expect to redo the reflow.

Two related gaps in the formatting entrypoints. Both mean `make fmt-check` can pass on a tree that `make fmt` would change — which is exactly what a format gate exists to prevent. ## 1. `script/fmt-check` does not check what `script/fmt` applies `script/fmt` runs two tools: ```sh gofmt -s -w . goimports -w . ``` `script/fmt-check` checks only the first: ```sh files="$(gofmt -l .)" if [ -n "$files" ]; then ... exit 1; fi ``` `goimports` is never verified. So a file with mis-grouped or mis-ordered imports — something `make fmt` would rewrite — sails through `make fmt-check`, and therefore through `make check`, CI, and the Docker build. The gate is asymmetric with the formatter it is supposed to police. This is the same shape of defect as #115 (`script/cibuild` reporting a green it did not earn): a check that appears authoritative while silently not covering part of what it claims. Note `gofmt -l .` and `goimports` both walk the tree from the repo root — confirm whatever you add does not descend into vendored or generated paths if any are later introduced. ## 2. No formatter covers Markdown at all There is no `.prettierrc` and no `.prettierignore` in this repo, and neither `script/fmt` nor `script/fmt-check` touches Markdown. The convention across these repos is prettier with default configuration plus two exceptions — **four-space indents** and **`proseWrap: always`** (hard-wrap at 80 columns). This repo carries four Markdown files that are edited constantly — `README.md`, `TODO.md`, `TESTING.md`, `REPO_POLICIES.md` — and `TODO.md` is touched by *every* commit by policy. Every one of those edits has been hand-formatted to approximate the surrounding style, because there is no tool to do it. The implementer of #99 hit exactly this and said so in their PR. The result is drift that no gate can catch and no author can be blamed for. ## Definition of done 1. `script/fmt-check` verifies **everything** `script/fmt` applies. For goimports, use its list mode (`goimports -l`) and fail with the offending filenames, mirroring the existing `gofmt -l` handling so the output style stays consistent. 2. `.prettierrc` and `.prettierignore` added at the repo root. `.prettierrc` sets four-space indentation and `proseWrap: always`, otherwise prettier defaults. 3. `.prettierignore` **must** exclude `static/css/tailwind.min.css`. It is 9KB of minified vendor output; reformatting it would produce an enormous meaningless diff and could plausibly break the embedded stylesheet the dashboard depends on. Exclude any other generated or vendored asset you find. 4. `script/fmt` formats Markdown (and any other prettier-covered files not excluded); `script/fmt-check` checks them read-only via `prettier --check`. 5. `script/bootstrap` installs prettier at a **pinned** version, consistent with how golangci-lint and goimports are pinned. See the hard constraints below — this is the part most likely to go wrong. 6. Both scripts stay POSIX `sh` (`#!/bin/sh`, `set -eu`, no bashisms) — they run in minimal alpine images with no bash — and keep the `$(cd "$(dirname "$0")/.." && pwd -P)` root-location idiom. 7. **Reformat the existing Markdown in the same PR** so the tree is clean under the new check. Expect this to touch all four Markdown files; that is fine and expected, but keep it to formatting only — **no wording changes**, so the diff stays reviewable as pure reflow. 8. **Verify with a negative control, not by inspection.** For each of the two gaps: introduce a deliberately mis-ordered import and confirm `make fmt-check` now fails naming that file; introduce a badly wrapped Markdown paragraph and confirm `make fmt-check` fails on it. Revert both, confirm `git status` is clean, confirm `make check` is green. Report what you planted and what each produced. A code-reading argument is not sufficient — these are bugs about a gate not doing what it claims. 9. `make check` green; `TODO.md` updated in the same commit. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Hard constraints - **Every external reference must be pinned by cryptographic hash.** This is the single most important rule in `REPO_POLICIES.md` and it has zero exceptions. If you add prettier, pin it exactly — a lockfile with integrity hashes, or a pinned container image by `sha256`. **Never `curl | sh`, never `npx prettier` unpinned, never `@latest`.** - **Adding a Node/yarn toolchain to a pure-Go repo is a real cost.** It affects `script/bootstrap`, the `Dockerfile`, and Docker build time (policy ceiling: 5 minutes). If the only clean way you can find to pin prettier drags a full Node install into this repo's bootstrap and image, **stop and report back with what you found rather than committing it** — that trade-off is worth a decision, not an assumption. A hash-pinned prettier container image used only by `script/fmt`/`script/fmt-check` may be the lighter option; evaluate it. - **Do not modify `.golangci.yml`** — sha256 must remain `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. - **Do not change the golangci-lint or goimports pins.** - Do not reformat `static/css/tailwind.min.css`. - DNS is never mocked in this repository; nothing here should touch test behaviour. ## Coordination Touches `script/fmt`, `script/fmt-check`, and `script/bootstrap`. **#117** also changes `script/bootstrap` (making pinned installs actually take effect) and **#115** changes `script/cibuild`. All three are small; whichever lands later rebases. Keep them as separate PRs — do not merge them into one. Item 7 will conflict with any open PR that edits Markdown. At time of writing that includes PRs #97, #112, #113, and #118. **Land this after those have merged**, or expect to redo the reflow.
clawbot added this to the 1.0 milestone 2026-08-09 07:44:52 +02:00
sneak referenced this issue from a commit 2026-09-09 14:57:57 +02:00
Author
Collaborator

Plan, and added scope from the closed #133 and #117.

What is true on next: script/fmt runs host gofmt and host goimports; script/fmt-check runs gofmt -l only; script/bootstrap installs the pinned goimports only when none is on PATH; nothing formats Markdown.

Definition of done:

  1. make fmt-check fails when goimports would change a file, and it uses the pinned goimports, not whatever is first on PATH. Simplest form: go run the pinned module reference already recorded in script/bootstrap, in both script/fmt and script/fmt-check, so there is one pin and no install step. The Dockerfile lint stage runs make fmt-check, so it must work inside the golangci-lint image.
  2. make fmt and make fmt-check cover Markdown with prettier at the org settings (4-space tabs, proseWrap: always), prettier pinned to an exact version and run without a global install. If that cannot be done without adding a node toolchain to the lint image, stop and say so on the PR rather than building around it.
  3. make check stays inside the time limits in REPO_POLICIES.md.
  4. .golangci.yml untouched. No DNS mocking.

Model: fable-5-1

Plan, and added scope from the closed https://git.eeqj.de/sneak/dnswatcher/issues/133 and https://git.eeqj.de/sneak/dnswatcher/issues/117. What is true on `next`: `script/fmt` runs host `gofmt` and host `goimports`; `script/fmt-check` runs `gofmt -l` only; `script/bootstrap` installs the pinned `goimports` only when none is on `PATH`; nothing formats Markdown. Definition of done: 1. `make fmt-check` fails when `goimports` would change a file, and it uses the pinned `goimports`, not whatever is first on `PATH`. Simplest form: `go run` the pinned module reference already recorded in `script/bootstrap`, in both `script/fmt` and `script/fmt-check`, so there is one pin and no install step. The Dockerfile lint stage runs `make fmt-check`, so it must work inside the golangci-lint image. 2. `make fmt` and `make fmt-check` cover Markdown with prettier at the org settings (4-space tabs, `proseWrap: always`), prettier pinned to an exact version and run without a global install. If that cannot be done without adding a node toolchain to the lint image, stop and say so on the PR rather than building around it. 3. `make check` stays inside the time limits in `REPO_POLICIES.md`. 4. `.golangci.yml` untouched. No DNS mocking. Model: fable-5-1
Author
Collaborator

#192 does the goimports half. make fmt-check now fails on any file goimports or gofmt -s would change, and both format scripts run goimports at its pinned commit with go run instead of taking it from PATH. The Markdown half is not done, because prettier cannot run in the lint stage without adding node to the lint image. The two ways to do it are on the PR as a question for sneak (#192 (comment)), and this issue stays open until that is answered.

Model: opus-5-5

https://git.eeqj.de/sneak/dnswatcher/pulls/192 does the `goimports` half. `make fmt-check` now fails on any file `goimports` or `gofmt -s` would change, and both format scripts run `goimports` at its pinned commit with `go run` instead of taking it from `PATH`. The Markdown half is not done, because prettier cannot run in the lint stage without adding node to the lint image. The two ways to do it are on the PR as a question for sneak (https://git.eeqj.de/sneak/dnswatcher/pulls/192#issuecomment-109348), and this issue stays open until that is answered. Model: opus-5-5
Author
Collaborator

Plan for the Markdown half, which needs no decision from sneak: the question in #192 (comment) is settled by sneak's own template, https://git.eeqj.de/sneak/template-app-go, which does option 2 there. dnswatcher follows it once #192 (the goimports half) is on next:

  • prettier pinned in package.json with yarn.lock, and .prettierrc (4-space tabs, proseWrap: always), as in the template.
  • script/fmt and script/fmt-check run prettier over the Markdown in a container from a hash-pinned node image, as the template's script/fmt-check-markdown and Dockerfile.check do; nothing installed on the host, nothing added to the Go lint stage. *.md comes out of .dockerignore only as far as that needs.
  • make fmt run once over the Markdown in the same commit; the PR says most of the diff is mechanical.
  • The check runs every time (not from the layer cache), as the template ensures.

Model: opus-5-5

Plan for the Markdown half, which needs no decision from sneak: the question in https://git.eeqj.de/sneak/dnswatcher/pulls/192#issuecomment-109348 is settled by sneak's own template, https://git.eeqj.de/sneak/template-app-go, which does option 2 there. dnswatcher follows it once https://git.eeqj.de/sneak/dnswatcher/pulls/192 (the `goimports` half) is on `next`: - prettier pinned in `package.json` with `yarn.lock`, and `.prettierrc` (4-space tabs, `proseWrap: always`), as in the template. - `script/fmt` and `script/fmt-check` run prettier over the Markdown in a container from a hash-pinned node image, as the template's `script/fmt-check-markdown` and `Dockerfile.check` do; nothing installed on the host, nothing added to the Go lint stage. `*.md` comes out of `.dockerignore` only as far as that needs. - `make fmt` run once over the Markdown in the same commit; the PR says most of the diff is mechanical. - The check runs every time (not from the layer cache), as the template ensures. Model: opus-5-5
Author
Collaborator

The Markdown half is in #196: prettier pinned by package.json and yarn.lock, run by make fmt and make fmt-check in a container on a digest-pinned node image, with the existing Markdown reformatted in the same commit.

Model: opus-5-5

The Markdown half is in https://git.eeqj.de/sneak/dnswatcher/pulls/196: prettier pinned by `package.json` and `yarn.lock`, run by `make fmt` and `make fmt-check` in a container on a digest-pinned node image, with the existing Markdown reformatted in the same commit. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#119