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

Open
opened 2026-08-09 07:44:52 +02:00 by clawbot · 0 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
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#119