Configure prettier and make fmt-check cover markdown (closes #69) #88

Merged
clawbot merged 1 commits from prettier-fmt-check into next 2026-08-10 16:16:43 +02:00
Collaborator

Closes #69.

script/fmt ran prettier with default settings over root-level *.md and
*.json and swallowed every failure with || true, while
script/fmt-check checked gofmt only. The formatter and the gate
disagreed silently: make fmt rewrote markdown that make check never
looked at, including REPO_POLICIES.md, a verbatim copy of an
authoritative upstream document that local tooling must not touch.

What changed

Configuration

  • .prettierrc: the two policy deviations from prettier defaults —
    tabWidth: 4 and proseWrap: "always" — and nothing else.
  • .prettierignore: REPO_POLICIES.md, so no local run can drift it from
    upstream again; .golangci.yml (user-owned — not reachable by the
    current file set, listed so widening that set can never start rewriting
    it); node_modules/, vendor/, bin/.

One canonical file set

  • New script/prettier takes --write or --check and applies the same
    patterns in both modes, so script/fmt and script/fmt-check cannot
    drift apart by construction. Patterns are repo-wide (**/*.md,
    **/*.json) rather than root-only — the deliberate answer to the
    subdirectory question in the issue, so a future docs/ is covered from
    day one.
  • || true is gone. So is --no-error-on-unmatched-pattern: both
    patterns always match tracked files (README.md, package.json), so an
    empty match means the glob broke, and prettier erroring is preferable to
    a vacuous pass. A missing prettier is a hard error naming
    script/bootstrap, never a skip.

Pinned prettier

  • package.json / yarn.lock pin prettier 3.9.6. The lockfile carries
    the sha512 integrity hash and --frozen-lockfile enforces it, so this
    is hash-pinned; no curl | sh, no unpinned global install.
  • script/prettier prefers node_modules/.bin/prettier and warns on
    stderr when it falls back to a PATH prettier of unknown version.
  • script/bootstrap now installs node, yarn, and the locked JS deps. Its
    NODE_VERSION / YARN_VERSION pins already existed and were unchanged.

The Docker gate

REPO_POLICIES.md requires docker build . to be the authoritative gate,
and the lint stage runs on golangci/golangci-lint, which I confirmed by
running it has no node, npm, or yarn (command -v node exits 127). So
script/fmt-check could not simply be hard-required there.

Rather than weaken the check, the markdown half got its own stage:

  • script/fmt-check-go — the Go half of fmt-check, extracted. The lint
    stage runs make fmt-check-go.
  • New mdfmt stage on node@sha256:b04ce4ae...
    (node:22.17.0-bookworm-slim), which ships node 22.17.0 and yarn
    1.22.22 — exactly the versions script/bootstrap already pins. It runs
    yarn install --frozen-lockfile then script/prettier --check. That
    image has no make, so it calls the script/ entrypoint directly.
  • The builder stage takes COPY --from=mdfmt /src/go.sum /dev/null, the
    same dependency trick already used for the lint stage, so BuildKit
    cannot skip it.

Result: a markdown formatting violation fails docker build .. Nothing
skips silently anywhere — the whole point of the issue.

make fmt-check locally still covers both halves
(script/fmt-check-go + script/prettier --check), and make check
therefore fails on misformatted markdown too. Makefile gains
fmt-check-go and fmt-check-md shims; README.md Entrypoints documents
the new scripts.

Markdown files other than REPO_POLICIES.md are reformatted here for the
first time under the policy settings — that is the bulk of the diff and it
is formatting only.

Verification

All via make / script/ entrypoints.

  • make check green (tests, lint, fmt-check).
  • script/cibuild (docker build .) green.
  • make fmt then git diff -- REPO_POLICIES.md empty, and the file still
    cmp-identical to the authoritative copy;
    sha256 117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775
    before and after.
  • Stronger version of the same: appended a deliberately misformatted line
    to REPO_POLICIES.md, ran make fmt — sha256 unchanged, prettier never
    touched it — and make fmt-check still exited 0, confirming the ignore
    rule holds in both directions. Reverted afterwards.
  • make fmt then make fmt-check clean.
  • Negative test: appended a badly indented, over-80-column list item to
    FORMAT.md; make fmt-check exited non-zero, make check exited
    non-zero, and script/cibuild failed in [mdfmt 6/6] RUN script/prettier --check with [warn] FORMAT.md. Reverted afterwards.
  • Missing-prettier test: moved node_modules aside and ran
    script/prettier --check with a stripped PATH — exit 1 with the
    Install it with: script/bootstrap message, no silent pass.

Scope notes

  • This takes only the prettier slice of #68 (bootstrap installing a
    hash-pinned prettier), because script/fmt-check cannot fail loudly on
    markdown without prettier being installable. The rest of #68 — gofumpt
    in bootstrap, hash-pinning the Makefile Go tool installs — is
    deliberately left there, so the two do not get done twice.
  • .golangci.yml not touched.
  • The pre-existing ensure_pb duplication between script/fmt and the
    extracted script/fmt-check-go was left as found; out of scope here.
Closes #69. `script/fmt` ran prettier with default settings over root-level `*.md` and `*.json` and swallowed every failure with `|| true`, while `script/fmt-check` checked `gofmt` only. The formatter and the gate disagreed silently: `make fmt` rewrote markdown that `make check` never looked at, including `REPO_POLICIES.md`, a verbatim copy of an authoritative upstream document that local tooling must not touch. ## What changed **Configuration** - `.prettierrc`: the two policy deviations from prettier defaults — `tabWidth: 4` and `proseWrap: "always"` — and nothing else. - `.prettierignore`: `REPO_POLICIES.md`, so no local run can drift it from upstream again; `.golangci.yml` (user-owned — not reachable by the current file set, listed so widening that set can never start rewriting it); `node_modules/`, `vendor/`, `bin/`. **One canonical file set** - New `script/prettier` takes `--write` or `--check` and applies the same patterns in both modes, so `script/fmt` and `script/fmt-check` cannot drift apart by construction. Patterns are repo-wide (`**/*.md`, `**/*.json`) rather than root-only — the deliberate answer to the subdirectory question in the issue, so a future `docs/` is covered from day one. - `|| true` is gone. So is `--no-error-on-unmatched-pattern`: both patterns always match tracked files (`README.md`, `package.json`), so an empty match means the glob broke, and prettier erroring is preferable to a vacuous pass. A missing prettier is a hard error naming `script/bootstrap`, never a skip. **Pinned prettier** - `package.json` / `yarn.lock` pin prettier 3.9.6. The lockfile carries the `sha512` integrity hash and `--frozen-lockfile` enforces it, so this is hash-pinned; no `curl | sh`, no unpinned global install. - `script/prettier` prefers `node_modules/.bin/prettier` and warns on stderr when it falls back to a `PATH` prettier of unknown version. - `script/bootstrap` now installs node, yarn, and the locked JS deps. Its `NODE_VERSION` / `YARN_VERSION` pins already existed and were unchanged. **The Docker gate** `REPO_POLICIES.md` requires `docker build .` to be the authoritative gate, and the lint stage runs on `golangci/golangci-lint`, which I confirmed by running it has no node, npm, or yarn (`command -v node` exits 127). So `script/fmt-check` could not simply be hard-required there. Rather than weaken the check, the markdown half got its own stage: - `script/fmt-check-go` — the Go half of `fmt-check`, extracted. The lint stage runs `make fmt-check-go`. - New `mdfmt` stage on `node@sha256:b04ce4ae...` (`node:22.17.0-bookworm-slim`), which ships node 22.17.0 and yarn 1.22.22 — exactly the versions `script/bootstrap` already pins. It runs `yarn install --frozen-lockfile` then `script/prettier --check`. That image has no `make`, so it calls the `script/` entrypoint directly. - The builder stage takes `COPY --from=mdfmt /src/go.sum /dev/null`, the same dependency trick already used for the lint stage, so BuildKit cannot skip it. Result: a markdown formatting violation fails `docker build .`. Nothing skips silently anywhere — the whole point of the issue. `make fmt-check` locally still covers both halves (`script/fmt-check-go` + `script/prettier --check`), and `make check` therefore fails on misformatted markdown too. `Makefile` gains `fmt-check-go` and `fmt-check-md` shims; `README.md` Entrypoints documents the new scripts. Markdown files other than `REPO_POLICIES.md` are reformatted here for the first time under the policy settings — that is the bulk of the diff and it is formatting only. ## Verification All via `make` / `script/` entrypoints. - `make check` green (tests, lint, `fmt-check`). - `script/cibuild` (`docker build .`) green. - `make fmt` then `git diff -- REPO_POLICIES.md` empty, and the file still `cmp`-identical to the authoritative copy; `sha256 117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775` before and after. - Stronger version of the same: appended a deliberately misformatted line to `REPO_POLICIES.md`, ran `make fmt` — sha256 unchanged, prettier never touched it — and `make fmt-check` still exited 0, confirming the ignore rule holds in both directions. Reverted afterwards. - `make fmt` then `make fmt-check` clean. - Negative test: appended a badly indented, over-80-column list item to `FORMAT.md`; `make fmt-check` exited non-zero, `make check` exited non-zero, and `script/cibuild` failed in `[mdfmt 6/6] RUN script/prettier --check` with `[warn] FORMAT.md`. Reverted afterwards. - Missing-prettier test: moved `node_modules` aside and ran `script/prettier --check` with a stripped `PATH` — exit 1 with the `Install it with: script/bootstrap` message, no silent pass. ## Scope notes - This takes only the prettier slice of #68 (bootstrap installing a hash-pinned prettier), because `script/fmt-check` cannot fail loudly on markdown without prettier being installable. The rest of #68 — gofumpt in bootstrap, hash-pinning the Makefile Go tool installs — is deliberately left there, so the two do not get done twice. - `.golangci.yml` not touched. - The pre-existing `ensure_pb` duplication between `script/fmt` and the extracted `script/fmt-check-go` was left as found; out of scope here.
clawbot added the needs-review label 2026-08-09 07:05:31 +02:00
clawbot self-assigned this 2026-08-09 07:05:37 +02:00
Author
Collaborator

Built

One commit, b3ac072, on prettier-fmt-check off origin/main
(6d19de7).

  • .prettierrc (tabWidth: 4, proseWrap: "always") and
    .prettierignore (REPO_POLICIES.md, .golangci.yml,
    node_modules/, vendor/, bin/).
  • script/prettier — new entrypoint, takes --write or --check and
    applies the identical pattern set in both modes. This is the whole
    reason script/fmt and script/fmt-check can no longer disagree about
    which files are covered: there is only one place the patterns exist.
    Repo-wide **/*.md and **/*.json, not root-only.
  • script/fmt-check-go — the Go half of fmt-check, extracted so the
    node-less lint image can run it alone.
  • script/fmt and script/fmt-check now both delegate to
    script/prettier. || true is gone from both prettier invocations; a
    missing prettier exits 1 with an actionable message.
  • package.json / yarn.lock pin prettier 3.9.6 by lockfile integrity
    hash; script/bootstrap installs node, yarn and the locked deps using
    its existing pinned NODE_VERSION / YARN_VERSION.
  • Dockerfile: lint stage runs make fmt-check-go; new mdfmt stage on
    a digest-pinned node:22.17.0-bookworm-slim runs
    yarn install --frozen-lockfile then script/prettier --check; builder
    takes a COPY --from=mdfmt dependency so BuildKit cannot skip it.
  • Makefile gains fmt-check-go / fmt-check-md; .gitignore and
    .dockerignore gain node_modules; README.md Entrypoints documents
    the new scripts; TODO.md updated in this same commit.
  • AGENTS.md, FORMAT.md, README.md, TODO.md reformatted under the
    policy settings — formatting only, and the bulk of the line count.

Verified

Everything through make / script/ entrypoints, nothing raw.

  • make check — green.
  • script/cibuild (docker build .) — green, including the new mdfmt
    stage.
  • REPO_POLICIES.md byte-identity, before and after make fmt:
    cmp against the authoritative copy passes and sha256 stays
    117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775;
    git diff -- REPO_POLICIES.md empty.
  • Ignore rule proven in both directions: appended a deliberately
    misformatted line to REPO_POLICIES.md, ran make fmt — sha256
    unchanged, prettier did not touch it — and make fmt-check still exited
    0. Reverted.
  • make fmt then make fmt-check — clean, no churn.
  • Misformatted FORMAT.md (badly indented, over-80-column list item):
    make fmt-check non-zero, make check non-zero, and script/cibuild
    failed at [mdfmt 6/6] RUN script/prettier --check with
    [warn] FORMAT.md. Reverted.
  • node_modules moved aside and PATH stripped: script/prettier --check exits 1 with the bootstrap message rather than passing.

Not done here

Only the prettier slice of #68 is included, and only because a
loud-failing markdown check needs prettier to be installable. gofumpt in
bootstrap and hash-pinning the Makefile Go tool installs are still #68's.
.golangci.yml untouched. The pre-existing ensure_pb duplication
between script/fmt and script/fmt-check-go left as found.

## Built One commit, `b3ac072`, on `prettier-fmt-check` off `origin/main` (`6d19de7`). - `.prettierrc` (`tabWidth: 4`, `proseWrap: "always"`) and `.prettierignore` (`REPO_POLICIES.md`, `.golangci.yml`, `node_modules/`, `vendor/`, `bin/`). - `script/prettier` — new entrypoint, takes `--write` or `--check` and applies the identical pattern set in both modes. This is the whole reason `script/fmt` and `script/fmt-check` can no longer disagree about which files are covered: there is only one place the patterns exist. Repo-wide `**/*.md` and `**/*.json`, not root-only. - `script/fmt-check-go` — the Go half of `fmt-check`, extracted so the node-less lint image can run it alone. - `script/fmt` and `script/fmt-check` now both delegate to `script/prettier`. `|| true` is gone from both prettier invocations; a missing prettier exits 1 with an actionable message. - `package.json` / `yarn.lock` pin prettier 3.9.6 by lockfile integrity hash; `script/bootstrap` installs node, yarn and the locked deps using its existing pinned `NODE_VERSION` / `YARN_VERSION`. - `Dockerfile`: lint stage runs `make fmt-check-go`; new `mdfmt` stage on a digest-pinned `node:22.17.0-bookworm-slim` runs `yarn install --frozen-lockfile` then `script/prettier --check`; builder takes a `COPY --from=mdfmt` dependency so BuildKit cannot skip it. - `Makefile` gains `fmt-check-go` / `fmt-check-md`; `.gitignore` and `.dockerignore` gain `node_modules`; `README.md` Entrypoints documents the new scripts; `TODO.md` updated in this same commit. - `AGENTS.md`, `FORMAT.md`, `README.md`, `TODO.md` reformatted under the policy settings — formatting only, and the bulk of the line count. ## Verified Everything through `make` / `script/` entrypoints, nothing raw. - `make check` — green. - `script/cibuild` (`docker build .`) — green, including the new `mdfmt` stage. - `REPO_POLICIES.md` byte-identity, before and after `make fmt`: `cmp` against the authoritative copy passes and sha256 stays `117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775`; `git diff -- REPO_POLICIES.md` empty. - Ignore rule proven in both directions: appended a deliberately misformatted line to `REPO_POLICIES.md`, ran `make fmt` — sha256 unchanged, prettier did not touch it — and `make fmt-check` still exited 0. Reverted. - `make fmt` then `make fmt-check` — clean, no churn. - Misformatted `FORMAT.md` (badly indented, over-80-column list item): `make fmt-check` non-zero, `make check` non-zero, and `script/cibuild` failed at `[mdfmt 6/6] RUN script/prettier --check` with `[warn] FORMAT.md`. Reverted. - `node_modules` moved aside and `PATH` stripped: `script/prettier --check` exits 1 with the bootstrap message rather than passing. ## Not done here Only the prettier slice of #68 is included, and only because a loud-failing markdown check needs prettier to be installable. gofumpt in bootstrap and hash-pinning the Makefile Go tool installs are still #68's. `.golangci.yml` untouched. The pre-existing `ensure_pb` duplication between `script/fmt` and `script/fmt-check-go` left as found.
Author
Collaborator

Manager notes. Labelled needs-review, assigned clawbot. An independent
adversarial review is in flight and will be posted as its own comment.

Merge ordering matters here, so recording it before it bites.

This PR is branched from main at 6d19de7 and is currently mergeable.
But PR #59 is also open, is already merge-ready, and is assigned to sneak
for merge. The two overlap on three files — Dockerfile, Makefile, and
TODO.md — and this PR additionally rewraps README.md in full.

#59 should merge first. It is the larger change, it is already through two
review rounds, and it establishes the lint gate every later PR has to pass.
When it lands, this branch will need a rebase, and the README.md and
TODO.md conflicts will be textual rather than semantic — both sides are
touching prose, not logic.

Concretely: if #59 merges before this one, expect needs-rebase on this PR
and treat the review verdict below as applying to the pre-rebase tree. I will
re-verify the gates after any rebase rather than carrying the old result
forward.

Two things I want the review to land on, flagged so they are not lost if
the verdict is otherwise clean.

First, scope. Satisfying a markdown-formatting issue has pulled
package.json, yarn.lock, a node_modules/ dependency, a third Docker
stage, and node + yarn installation in script/bootstrap into what is
otherwise a pure Go CLI repo. That may well be the right answer — the
alternative designs really do reintroduce the silent-skip bug #69 exists to
kill — but it is a large structural change to justify on formatting grounds
and it deserves an explicit judgement rather than an implicit one.

Second, the pin. The PR argues that yarn.lock plus --frozen-lockfile
satisfies the hash-pinning policy. That claim needs checking rather than
accepting: whether the lockfile actually carries an integrity hash, whether
--frozen-lockfile verifies that hash or merely checks the lockfile against
package.json, and whether the package.json version spec is exact rather
than a caret range. The policy on this is absolute and has no exceptions, so
a near-miss here is a blocking finding, not a nit.

Related: script/prettier falls back to a PATH prettier of unknown version
with only a warning. This repo has already been bitten once this week by
exactly that failure mode — a local golangci-lint v2.10.1 disagreeing with
the pinned v2.12.2 by ten findings, which cost a review cycle. A different
prettier version formats differently, so the same trap is being re-laid one
tool over.

Manager notes. Labelled `needs-review`, assigned `clawbot`. An independent adversarial review is in flight and will be posted as its own comment. **Merge ordering matters here, so recording it before it bites.** This PR is branched from `main` at `6d19de7` and is currently `mergeable`. But PR #59 is also open, is already `merge-ready`, and is assigned to `sneak` for merge. The two overlap on three files — `Dockerfile`, `Makefile`, and `TODO.md` — and this PR additionally rewraps `README.md` in full. #59 should merge first. It is the larger change, it is already through two review rounds, and it establishes the lint gate every later PR has to pass. When it lands, this branch will need a rebase, and the `README.md` and `TODO.md` conflicts will be textual rather than semantic — both sides are touching prose, not logic. Concretely: if #59 merges before this one, expect `needs-rebase` on this PR and treat the review verdict below as applying to the pre-rebase tree. I will re-verify the gates after any rebase rather than carrying the old result forward. **Two things I want the review to land on, flagged so they are not lost if the verdict is otherwise clean.** First, scope. Satisfying a markdown-formatting issue has pulled `package.json`, `yarn.lock`, a `node_modules/` dependency, a third Docker stage, and node + yarn installation in `script/bootstrap` into what is otherwise a pure Go CLI repo. That may well be the right answer — the alternative designs really do reintroduce the silent-skip bug #69 exists to kill — but it is a large structural change to justify on formatting grounds and it deserves an explicit judgement rather than an implicit one. Second, the pin. The PR argues that `yarn.lock` plus `--frozen-lockfile` satisfies the hash-pinning policy. That claim needs checking rather than accepting: whether the lockfile actually carries an integrity hash, whether `--frozen-lockfile` verifies that hash or merely checks the lockfile against `package.json`, and whether the `package.json` version spec is exact rather than a caret range. The policy on this is absolute and has no exceptions, so a near-miss here is a blocking finding, not a nit. Related: `script/prettier` falls back to a `PATH` prettier of unknown version with only a warning. This repo has already been bitten once this week by exactly that failure mode — a local `golangci-lint` v2.10.1 disagreeing with the pinned v2.12.2 by ten findings, which cost a review cycle. A different prettier version formats differently, so the same trap is being re-laid one tool over.
Author
Collaborator

Review of PR #88 — independent verification

Verdict: PASS.

Head b3ac072, base main 6d19de7. All verification below was run from a
detached worktree at the PR head, via make / script/ entrypoints only,
except where a forensic experiment on the Dockerfile is explicitly noted.


Definition of done (issue #69) — each criterion checked independently

#69 requirement Result
.prettierrc with four-space indent and proseWrap: always Present, and only those two keys. printWidth correctly omitted (prettier's default is already 80), so the "defaults with two exceptions" rule is honoured literally.
.prettierignore exists Present.
REPO_POLICIES.md excluded and byte-identical to upstream Verified: cmp against the authoritative copy passes; sha256 117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775. Not in the diff at all.
script/fmt-check verifies markdown Verified by execution, not by reading — see negative test below.
fmt and fmt-check cover the same file set Structurally guaranteed: both call script/prettier, which is the single place the patterns exist.
make fmt then make fmt-check clean Verified: make fmt on a clean tree leaves git status --porcelain empty.
make fmt-check fails on a deliberately misformatted file Verified.
|| true gone Verified: absent from script/fmt and script/fmt-check.
make check passes; TODO.md updated in the same commit Both verified.
Commit title ends (closes #69) Verified.

Independently reproduced tests

1. The markdown gate genuinely fails, in both gates. Appended an
over-80-column, badly indented list item to FORMAT.md:

  • make fmt-check -> exit 2; make check -> exit 2, with
    [warn] FORMAT.md / Code style issues found.
  • script/cibuild -> exit 1, failing at
    [mdfmt 6/6] RUN script/prettier --check with
    ERROR: process "/bin/sh -c script/prettier --check" did not complete successfully: exit code: 1.

2. The COPY --from=mdfmt dependency is effective — proven by experiment,
not by reading the Dockerfile.
In the failing build above, BuildKit executed
mdfmt 6/6, which is the step after COPY . . (5/6) brought go.sum into
the stage. BuildKit therefore resolves --from=mdfmt against the stage's
final state and cannot short-circuit at the instruction where go.sum
first appears. The stage cannot be skipped.

3. The REPO_POLICIES.md exemption holds in both directions. Appended a
deliberately misformatted, 140-column line to REPO_POLICIES.md and ran
make fmt: the appended garbage came back verbatim and unwrapped, i.e.
prettier never opened the file. make fmt-check then still exited 0. Reverted;
cmp against the authoritative prompts copy byte-identical afterwards.

4. Prettier is genuinely hash-pinned — I proved yarn enforces it. This was
the claim I most expected to fail, so I attacked it directly:

  • package.json specifies "prettier": "3.9.6" — an exact version, not a
    range. No ^, no ~.
  • yarn.lock carries
    integrity sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==.
  • I corrupted the first four base64 characters of that hash and re-ran
    script/cibuild with a cold builder. The build failed at
    [mdfmt 4/6] RUN yarn install --frozen-lockfile with
    error https://registry.yarnpkg.com/prettier/-/prettier-3.9.6.tgz: Integrity check failed for "prettier".

So the tarball really is content-verified against the lockfile hash, in the
authoritative gate, from a cold cache. This satisfies the policy's "npm
integrity hash in lockfile" clause. Not a pinning violation.

5. Docker build time — no policy violation. Cold cache, on a dedicated
docker-container buildx builder created for this review (so the shared cache
was not pruned and the measurement is genuinely cold):

  • script/cibuild wall clock: 59 seconds, exit 0. Limit is 5 minutes.
  • The new mdfmt stage costs ~7s total (node image pull 2.9s + yarn install
    3.2s + script/prettier --check 0.7s) and runs in parallel with the lint
    and builder stages, so its contribution to the critical path is close to zero.

6. make check does not modify files. make check -> exit 0;
git status --porcelain empty. The pre-existing ensure_pb mtime hazard
(#71) was left as found and not made worse: ensure_pb appears in exactly
three scripts before and after this change (script/test, script/fmt, and
script/fmt-check-go, the last being the relocation of the copy that was in
script/fmt-check). No third copy was created.

7. The pinned node image is what the comment says it is. Ran the digest:
node v22.17.0, yarn 1.22.22, Debian bookworm. Matches
node:22.17.0-bookworm-slim and matches the NODE_VERSION/YARN_VERSION
pins already in script/bootstrap. Comment carries version and date per policy.

8. Error handling in script/prettier is real, not decorative. Verified by
execution: no node_modules and a stripped PATH -> exit 1 with
prettier: not found. / Install it with: script/bootstrap; zero args, an
unknown flag, and two args all -> exit 2 with usage. No silent pass in any
path.

9. The reformatting is formatting-only. Whitespace-stripped content hashes:
FORMAT.md and AGENTS.md are identical to main after collapsing all
whitespace. README.md and TODO.md differ only by the deliberate additions,
confirmed by --word-diff --ignore-all-space: the new script/prettier /
script/fmt-check-go Entrypoints entries, the bootstrap node/yarn note, and one
new TODO.md "Completed Steps" line. No prose was smuggled in.

10. Housekeeping. CI green on b3ac072 (check / check (push) success,
37s). Head is a fast-forward descendant of current origin/main — no rebase
needed. Single commit, no trailers of any kind, no mention of any AI/LLM tooling
anywhere in the commit message, the diff, or the PR body. The only two matches
for such terms in the tree (AGENTS.md:21, TODO.md:38) both pre-date this PR
and appear in unchanged context. No non-inclusive terminology in the diff.
.golangci.yml untouched — note it does not exist on main or on this branch
at all (it lives on the unmerged golangci-v2.12.2 branch), so the
.prettierignore entry for it is forward-looking only.


On the scope question — I do not consider this scope explosion

I went in expecting to call this out and came away disagreeing. Stating the
reasoning plainly so it is on the record:

  • REPO_POLICIES.md mandates prettier for Markdown. Prettier is a
    JavaScript program. There is no way to run policy-mandated prettier without
    node. A Go-native markdown formatter would violate the formatter policy;
    npx prettier@3.9.6 would violate the hash-pinning policy; a CI-only prettier
    would leave make fmt broken on every dev box and violate #69's DoD directly.
    The alternatives are all worse.
  • The bootstrap machinery this PR turns on (ensure_node, ensure_yarn,
    install_js_deps, plus the NODE_VERSION / NVM_VERSION / NVM_SHA256 /
    YARN_VERSION constants) already existed in the file, commented out, as
    part of the canonical scripts-to-rule-them-all template. The change to
    script/bootstrap is three uncommented lines plus a comment. That is not new
    machinery, it is the template being used as designed, and REPO_POLICIES.md
    devotes an entire paragraph to prescribing exactly this node/nvm/yarn path.
  • The measurable cost is small and I measured it rather than assuming: +7s to
    a 59s cold Docker build (in parallel, so ~0s on the critical path), and
    package.json + yarn.lock totalling 18 lines. node_modules/ is
    gitignored and dockerignored, not committed.
  • The real cost is on a fresh machine with no node: script/bootstrap will now
    fetch the nvm release archive (sha256-verified, no curl | sh) and install
    node 22.17.0 before make check will work. That is a genuine new dependency
    for every developer and pre-commit hook. It is also unavoidable given the
    above, and it is what #69 explicitly asked for ("prettier must be installed by
    script/bootstrap for any of this to work on a fresh clone").

Proportionate. No finding.


Non-blocking findings

None of these block the merge. Recorded so they are not lost.

N1 — script/bootstrap:133-136 and the commit message overstate what
--frozen-lockfile does.
The comment reads "The version is pinned by
package.json/yarn.lock, whose integrity hashes --frozen-lockfile
enforces." That is not what the flag does: --frozen-lockfile only fails if the
lockfile would need to be regenerated to match package.json. Integrity
verification happens at fetch time and would happen without the flag — as the
corrupted-hash experiment above shows, the error comes from the fetcher, not
from the frozen-lockfile check. The substantive claim (prettier is hash-pinned)
is correct; only the mechanism attributed in the comment is wrong. Acceptable
would be: "package.json/yarn.lock pin the exact version; yarn verifies the
tarball against the lockfile's integrity hash on fetch, and
--frozen-lockfile additionally forbids silently updating the lockfile."

N2 — script/prettier:26-34, the PATH fallback. Falling back to an
unknown-version prettier with only a stderr warning is a version-skew hazard,
and it is worse on the --write path than the --check path: script/fmt
would let a prettier 2.x on PATH rewrite every markdown file in the repo,
after which CI (pinned 3.9.6) rejects the result. The warning is one line inside
make fmt's output and is easy to miss.

That said, I am deliberately not blocking on it, because script/fmt in the
very same commit already invokes gofumpt and golangci-lint run --fix
straight off PATH with no version check and no warning at all. Holding
prettier to a stricter standard than the two Go tools beside it would be
inconsistent, and pinning local Go tool versions is explicitly #68's scope. The
fallback here is strictly better than the existing precedent, not worse.

The cheap improvement, if it is wanted: have find_prettier compare
"$prettier_bin" --version against the version in package.json and exit
non-zero on mismatch, rather than warn. That converts a silent-drift hazard into
a loud failure for about six lines, and would be the right thing to do for
gofumpt and golangci-lint at the same time under #68.

(For the record: the prettier on this reviewing machine's PATH happens to be
3.9.6, so the fallback path was benign here. That is luck, not design.)

N3 — Makefile:51-52, fmt-check-md is dead code. Nothing invokes it. The
Docker mdfmt stage calls script/prettier --check directly because the node
image has no make, and script/fmt-check calls the script directly too. It is
harmless symmetry with fmt-check-go (which is used, by the lint stage), but
it is an unused target.

N4 — the lint stage now deviates from the canonical Dockerfile in
REPO_POLICIES.md.
Policy says the lint stage "runs make fmt-check and
make lint"; it now runs make fmt-check-go and make lint. The deviation is
correct here (the golangci/golangci-lint image has no node) and total coverage
is preserved by the mdfmt stage, which I confirmed cannot be skipped. Worth a
TODO.md line so the next policy-compliance audit does not "fix" it back and
silently drop the markdown gate.

N5 — package.json:3, "version": "0.1.0" invents a version number. The
repo is pre-1.0 with no tags, and README.md states "there has not yet been any
versioned release". For a "private": true tooling manifest the version field
is optional; omitting it avoids asserting a version that does not exist.

N6 — cosmetic. script/prettier invokes prettier twice (once per pattern),
so --check prints Checking formatting... / All matched files use Prettier code style! twice on every make check. Policy prefers clean output on
success. A single invocation with both patterns would print it once — though it
would also weaken the deliberate "an unmatched pattern is an error" property the
author documented, so this is a trade-off, not a defect.


Notes for the record

  • Prettier 3 defaults --ignore-path to both .gitignore and
    .prettierignore. I confirmed this behaviourally: a misformatted
    tmp/scratch.md (matched by /tmp in .gitignore) is silently skipped,
    while a misformatted docs/test.md is caught. So the PR's claim that a future
    docs/ is covered from day one is true (verified, not assumed), and
    gitignored scratch files will not spuriously fail make check.
  • The 80-column overruns remaining in FORMAT.md (25 lines) and README.md
    (4 lines) are markdown table rows and bare URLs respectively, neither of which
    prettier wraps. Correct output, not a miss.
## Review of PR #88 — independent verification **Verdict: PASS.** Head `b3ac072`, base `main` `6d19de7`. All verification below was run from a detached worktree at the PR head, via `make` / `script/` entrypoints only, except where a forensic experiment on the Dockerfile is explicitly noted. --- ### Definition of done (issue #69) — each criterion checked independently | #69 requirement | Result | | --- | --- | | `.prettierrc` with four-space indent and `proseWrap: always` | Present, and **only** those two keys. `printWidth` correctly omitted (prettier's default is already 80), so the "defaults with two exceptions" rule is honoured literally. | | `.prettierignore` exists | Present. | | `REPO_POLICIES.md` excluded and byte-identical to upstream | Verified: `cmp` against the authoritative copy passes; `sha256 117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775`. Not in the diff at all. | | `script/fmt-check` verifies markdown | Verified by execution, not by reading — see negative test below. | | `fmt` and `fmt-check` cover the same file set | Structurally guaranteed: both call `script/prettier`, which is the single place the patterns exist. | | `make fmt` then `make fmt-check` clean | Verified: `make fmt` on a clean tree leaves `git status --porcelain` empty. | | `make fmt-check` fails on a deliberately misformatted file | Verified. | | `\|\| true` gone | Verified: absent from `script/fmt` and `script/fmt-check`. | | `make check` passes; `TODO.md` updated in the same commit | Both verified. | | Commit title ends ` (closes #69)` | Verified. | --- ### Independently reproduced tests **1. The markdown gate genuinely fails, in both gates.** Appended an over-80-column, badly indented list item to `FORMAT.md`: - `make fmt-check` -> exit 2; `make check` -> exit 2, with `[warn] FORMAT.md` / `Code style issues found`. - `script/cibuild` -> exit 1, failing at `[mdfmt 6/6] RUN script/prettier --check` with `ERROR: process "/bin/sh -c script/prettier --check" did not complete successfully: exit code: 1`. **2. The `COPY --from=mdfmt` dependency is effective — proven by experiment, not by reading the Dockerfile.** In the failing build above, BuildKit executed `mdfmt 6/6`, which is the step *after* `COPY . .` (5/6) brought `go.sum` into the stage. BuildKit therefore resolves `--from=mdfmt` against the stage's **final** state and cannot short-circuit at the instruction where `go.sum` first appears. The stage cannot be skipped. **3. The `REPO_POLICIES.md` exemption holds in both directions.** Appended a deliberately misformatted, 140-column line to `REPO_POLICIES.md` and ran `make fmt`: the appended garbage came back **verbatim and unwrapped**, i.e. prettier never opened the file. `make fmt-check` then still exited 0. Reverted; `cmp` against the authoritative `prompts` copy byte-identical afterwards. **4. Prettier is genuinely hash-pinned — I proved yarn enforces it.** This was the claim I most expected to fail, so I attacked it directly: - `package.json` specifies `"prettier": "3.9.6"` — an **exact** version, not a range. No `^`, no `~`. - `yarn.lock` carries `integrity sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==`. - I corrupted the first four base64 characters of that hash and re-ran `script/cibuild` with a cold builder. The build failed at `[mdfmt 4/6] RUN yarn install --frozen-lockfile` with `error https://registry.yarnpkg.com/prettier/-/prettier-3.9.6.tgz: Integrity check failed for "prettier"`. So the tarball really is content-verified against the lockfile hash, in the authoritative gate, from a cold cache. This satisfies the policy's "npm integrity hash in lockfile" clause. **Not a pinning violation.** **5. Docker build time — no policy violation.** Cold cache, on a dedicated `docker-container` buildx builder created for this review (so the shared cache was not pruned and the measurement is genuinely cold): - **`script/cibuild` wall clock: 59 seconds, exit 0.** Limit is 5 minutes. - The new `mdfmt` stage costs ~7s total (node image pull 2.9s + `yarn install` 3.2s + `script/prettier --check` 0.7s) and runs **in parallel** with the lint and builder stages, so its contribution to the critical path is close to zero. **6. `make check` does not modify files.** `make check` -> exit 0; `git status --porcelain` empty. The pre-existing `ensure_pb` mtime hazard (#71) was left as found and **not** made worse: `ensure_pb` appears in exactly three scripts before and after this change (`script/test`, `script/fmt`, and `script/fmt-check-go`, the last being the relocation of the copy that was in `script/fmt-check`). No third copy was created. **7. The pinned node image is what the comment says it is.** Ran the digest: `node v22.17.0`, `yarn 1.22.22`, Debian bookworm. Matches `node:22.17.0-bookworm-slim` and matches the `NODE_VERSION`/`YARN_VERSION` pins already in `script/bootstrap`. Comment carries version and date per policy. **8. Error handling in `script/prettier` is real, not decorative.** Verified by execution: no `node_modules` and a stripped `PATH` -> exit 1 with `prettier: not found. / Install it with: script/bootstrap`; zero args, an unknown flag, and two args all -> exit 2 with usage. No silent pass in any path. **9. The reformatting is formatting-only.** Whitespace-stripped content hashes: `FORMAT.md` and `AGENTS.md` are **identical** to `main` after collapsing all whitespace. `README.md` and `TODO.md` differ only by the deliberate additions, confirmed by `--word-diff --ignore-all-space`: the new `script/prettier` / `script/fmt-check-go` Entrypoints entries, the bootstrap node/yarn note, and one new `TODO.md` "Completed Steps" line. No prose was smuggled in. **10. Housekeeping.** CI green on `b3ac072` (`check / check (push)` success, 37s). Head is a fast-forward descendant of current `origin/main` — no rebase needed. Single commit, no trailers of any kind, no mention of any AI/LLM tooling anywhere in the commit message, the diff, or the PR body. The only two matches for such terms in the tree (`AGENTS.md:21`, `TODO.md:38`) both pre-date this PR and appear in unchanged context. No non-inclusive terminology in the diff. `.golangci.yml` untouched — note it does not exist on `main` or on this branch at all (it lives on the unmerged `golangci-v2.12.2` branch), so the `.prettierignore` entry for it is forward-looking only. --- ### On the scope question — I do not consider this scope explosion I went in expecting to call this out and came away disagreeing. Stating the reasoning plainly so it is on the record: - `REPO_POLICIES.md` mandates **prettier** for Markdown. Prettier is a JavaScript program. There is no way to run policy-mandated prettier without node. A Go-native markdown formatter would violate the formatter policy; `npx prettier@3.9.6` would violate the hash-pinning policy; a CI-only prettier would leave `make fmt` broken on every dev box and violate #69's DoD directly. The alternatives are all worse. - The bootstrap machinery this PR turns on (`ensure_node`, `ensure_yarn`, `install_js_deps`, plus the `NODE_VERSION` / `NVM_VERSION` / `NVM_SHA256` / `YARN_VERSION` constants) **already existed in the file, commented out**, as part of the canonical scripts-to-rule-them-all template. The change to `script/bootstrap` is three uncommented lines plus a comment. That is not new machinery, it is the template being used as designed, and `REPO_POLICIES.md` devotes an entire paragraph to prescribing exactly this node/nvm/yarn path. - The measurable cost is small and I measured it rather than assuming: +7s to a 59s cold Docker build (in parallel, so ~0s on the critical path), and `package.json` + `yarn.lock` totalling 18 lines. `node_modules/` is gitignored and dockerignored, not committed. - The real cost is on a fresh machine with no node: `script/bootstrap` will now fetch the nvm release archive (sha256-verified, no `curl | sh`) and install node 22.17.0 before `make check` will work. That is a genuine new dependency for every developer and pre-commit hook. It is also unavoidable given the above, and it is what #69 explicitly asked for ("prettier must be installed by `script/bootstrap` for any of this to work on a fresh clone"). Proportionate. No finding. --- ### Non-blocking findings None of these block the merge. Recorded so they are not lost. **N1 — `script/bootstrap:133-136` and the commit message overstate what `--frozen-lockfile` does.** The comment reads "The version is pinned by `package.json`/`yarn.lock`, whose integrity hashes `--frozen-lockfile` enforces." That is not what the flag does: `--frozen-lockfile` only fails if the lockfile would need to be regenerated to match `package.json`. Integrity verification happens at fetch time and would happen without the flag — as the corrupted-hash experiment above shows, the error comes from the fetcher, not from the frozen-lockfile check. The substantive claim (prettier is hash-pinned) is correct; only the mechanism attributed in the comment is wrong. Acceptable would be: "`package.json`/`yarn.lock` pin the exact version; yarn verifies the tarball against the lockfile's `integrity` hash on fetch, and `--frozen-lockfile` additionally forbids silently updating the lockfile." **N2 — `script/prettier:26-34`, the `PATH` fallback.** Falling back to an unknown-version prettier with only a stderr warning is a version-skew hazard, and it is worse on the `--write` path than the `--check` path: `script/fmt` would let a prettier 2.x on `PATH` rewrite every markdown file in the repo, after which CI (pinned 3.9.6) rejects the result. The warning is one line inside `make fmt`'s output and is easy to miss. That said, I am deliberately **not** blocking on it, because `script/fmt` in the very same commit already invokes `gofumpt` and `golangci-lint run --fix` straight off `PATH` with **no** version check and **no** warning at all. Holding prettier to a stricter standard than the two Go tools beside it would be inconsistent, and pinning local Go tool versions is explicitly #68's scope. The fallback here is strictly better than the existing precedent, not worse. The cheap improvement, if it is wanted: have `find_prettier` compare `"$prettier_bin" --version` against the version in `package.json` and exit non-zero on mismatch, rather than warn. That converts a silent-drift hazard into a loud failure for about six lines, and would be the right thing to do for `gofumpt` and `golangci-lint` at the same time under #68. (For the record: the prettier on this reviewing machine's `PATH` happens to be 3.9.6, so the fallback path was benign here. That is luck, not design.) **N3 — `Makefile:51-52`, `fmt-check-md` is dead code.** Nothing invokes it. The Docker `mdfmt` stage calls `script/prettier --check` directly because the node image has no `make`, and `script/fmt-check` calls the script directly too. It is harmless symmetry with `fmt-check-go` (which *is* used, by the lint stage), but it is an unused target. **N4 — the lint stage now deviates from the canonical Dockerfile in `REPO_POLICIES.md`.** Policy says the lint stage "runs `make fmt-check` and `make lint`"; it now runs `make fmt-check-go` and `make lint`. The deviation is correct here (the `golangci/golangci-lint` image has no node) and total coverage is preserved by the `mdfmt` stage, which I confirmed cannot be skipped. Worth a `TODO.md` line so the next policy-compliance audit does not "fix" it back and silently drop the markdown gate. **N5 — `package.json:3`, `"version": "0.1.0"` invents a version number.** The repo is pre-1.0 with no tags, and `README.md` states "there has not yet been any versioned release". For a `"private": true` tooling manifest the `version` field is optional; omitting it avoids asserting a version that does not exist. **N6 — cosmetic.** `script/prettier` invokes prettier twice (once per pattern), so `--check` prints `Checking formatting...` / `All matched files use Prettier code style!` twice on every `make check`. Policy prefers clean output on success. A single invocation with both patterns would print it once — though it would also weaken the deliberate "an unmatched pattern is an error" property the author documented, so this is a trade-off, not a defect. --- ### Notes for the record - Prettier 3 defaults `--ignore-path` to **both** `.gitignore` and `.prettierignore`. I confirmed this behaviourally: a misformatted `tmp/scratch.md` (matched by `/tmp` in `.gitignore`) is silently skipped, while a misformatted `docs/test.md` is caught. So the PR's claim that a future `docs/` is covered from day one is **true** (verified, not assumed), and gitignored scratch files will not spuriously fail `make check`. - The 80-column overruns remaining in `FORMAT.md` (25 lines) and `README.md` (4 lines) are markdown table rows and bare URLs respectively, neither of which prettier wraps. Correct output, not a miss.
clawbot added merge-ready and removed needs-review labels 2026-08-09 07:18:31 +02:00
clawbot removed their assignment 2026-08-09 07:18:32 +02:00
sneak was assigned by clawbot 2026-08-09 07:18:32 +02:00
clawbot added this to the 1.0.0 milestone 2026-08-09 07:18:32 +02:00
Author
Collaborator

Manager notes. Review passed. Labelled merge-ready, assigned to sneak,
added to the 1.0.0 milestone.

Verified independently before promoting, rather than carried over from the
review:

  • package.json pins "prettier": "3.9.6" — exact, not a caret range.
  • yarn.lock carries the sha512 integrity hash for that exact tarball.
  • .prettierrc contains the two policy deviations and nothing else.
  • Gitea Actions check / check (push): success, 37s, on b3ac072.
  • mergeable: true, fast-forward descendant of main.

The review's strongest result is worth recording: it did not accept the
hash-pinning claim, it falsified it. Corrupting the first four base64
characters of the sha512 in yarn.lock made script/cibuild fail at
[mdfmt 4/6] RUN yarn install --frozen-lockfile with
Integrity check failed for "prettier". That is the difference between a
pin that is asserted and one that is enforced, and it is the standard this
repo should hold every external reference to.

Cold docker build . measured at 59 seconds on a dedicated builder against
the 5-minute policy limit, with the new mdfmt stage costing about 7s in
parallel. Measured, not estimated.

On merge order — I am overriding the review's recommendation

The review suggests merging this before the golangci-v2.12.2 branch. I
disagree, and the reasoning it gives does not hold.

That reasoning was that .golangci.yml would otherwise land without the
.prettierignore entry this PR pre-registers for it. But script/prettier
globs **/*.md and **/*.json only, and .golangci.yml is YAML — prettier
cannot reach it in either order. The review says as much itself when
describing the entry as defensive and not reachable by the current file set.
So the stated benefit is zero.

(Small correction for the record: the review refers to that branch as #87.
#87 is an issue — pinning CLI error messages. The PR is #59.)

Merge #59 first, then this one. #59 is 40 files, has been through two
full review rounds including a rework, and is already sitting merge-ready.
This PR is 17 files and most of its line count is markdown rewrapping.
Whichever merges second pays a rebase, and the rebase should land on the
cheap artifact, not the expensive one. Rebasing #59 would mean re-running the
whole gate on a change that just passed it twice.

Expect this PR to go needs-rebase once #59 lands — they overlap on
Dockerfile, Makefile, and TODO.md. I will rebase it, re-verify the
gates from scratch rather than carrying this verdict forward, and re-promote.

Non-blocking, tracked rather than fixed here

Six items the review recorded, none justifying a rework round:

  • The script/bootstrap comment and the commit message both attribute
    integrity enforcement to --frozen-lockfile. That is wrong: the flag only
    guards lockfile-versus-manifest consistency, and integrity verification
    happens at fetch time regardless. The behaviour is correct; the
    explanation of why is not.
  • The PATH-prettier fallback warns instead of failing. This is the
    version-skew trap that already cost a review cycle with golangci-lint. The
    review's argument for not blocking is sound — the gofumpt and
    golangci-lint run --fix calls sitting beside it in the same script are
    unwarned and unchecked, so this is strictly an improvement on what is
    there — but the fix belongs with #68.
  • make fmt-check-md is dead code.
  • The lint stage now runs make fmt-check-go rather than the canonical
    policy Dockerfile's make fmt-check. That deviation needs a TODO.md
    note, or a future policy audit will "correct" it back and silently drop
    the markdown gate.
  • package.json invents "version": "0.1.0" for a private manifest.
  • Duplicated "Checking formatting" banner.

I will fold the first four into #68 rather than opening a further issue, so
they are fixed alongside the bootstrap work they belong with.

Manager notes. Review passed. Labelled `merge-ready`, assigned to `sneak`, added to the `1.0.0` milestone. Verified independently before promoting, rather than carried over from the review: - `package.json` pins `"prettier": "3.9.6"` — exact, not a caret range. - `yarn.lock` carries the `sha512` integrity hash for that exact tarball. - `.prettierrc` contains the two policy deviations and nothing else. - Gitea Actions `check / check (push)`: success, 37s, on `b3ac072`. - `mergeable: true`, fast-forward descendant of `main`. The review's strongest result is worth recording: it did not accept the hash-pinning claim, it falsified it. Corrupting the first four base64 characters of the `sha512` in `yarn.lock` made `script/cibuild` fail at `[mdfmt 4/6] RUN yarn install --frozen-lockfile` with `Integrity check failed for "prettier"`. That is the difference between a pin that is asserted and one that is enforced, and it is the standard this repo should hold every external reference to. Cold `docker build .` measured at 59 seconds on a dedicated builder against the 5-minute policy limit, with the new `mdfmt` stage costing about 7s in parallel. Measured, not estimated. ## On merge order — I am overriding the review's recommendation The review suggests merging this before the `golangci-v2.12.2` branch. I disagree, and the reasoning it gives does not hold. That reasoning was that `.golangci.yml` would otherwise land without the `.prettierignore` entry this PR pre-registers for it. But `script/prettier` globs `**/*.md` and `**/*.json` only, and `.golangci.yml` is YAML — prettier cannot reach it in either order. The review says as much itself when describing the entry as defensive and not reachable by the current file set. So the stated benefit is zero. (Small correction for the record: the review refers to that branch as #87. #87 is an issue — pinning CLI error messages. The PR is #59.) **Merge #59 first, then this one.** #59 is 40 files, has been through two full review rounds including a rework, and is already sitting `merge-ready`. This PR is 17 files and most of its line count is markdown rewrapping. Whichever merges second pays a rebase, and the rebase should land on the cheap artifact, not the expensive one. Rebasing #59 would mean re-running the whole gate on a change that just passed it twice. Expect this PR to go `needs-rebase` once #59 lands — they overlap on `Dockerfile`, `Makefile`, and `TODO.md`. I will rebase it, re-verify the gates from scratch rather than carrying this verdict forward, and re-promote. ## Non-blocking, tracked rather than fixed here Six items the review recorded, none justifying a rework round: - The `script/bootstrap` comment and the commit message both attribute integrity enforcement to `--frozen-lockfile`. That is wrong: the flag only guards lockfile-versus-manifest consistency, and integrity verification happens at fetch time regardless. The behaviour is correct; the explanation of why is not. - The `PATH`-prettier fallback warns instead of failing. This is the version-skew trap that already cost a review cycle with golangci-lint. The review's argument for not blocking is sound — the `gofumpt` and `golangci-lint run --fix` calls sitting beside it in the same script are unwarned and unchecked, so this is strictly an improvement on what is there — but the fix belongs with #68. - `make fmt-check-md` is dead code. - The lint stage now runs `make fmt-check-go` rather than the canonical policy Dockerfile's `make fmt-check`. That deviation needs a `TODO.md` note, or a future policy audit will "correct" it back and silently drop the markdown gate. - `package.json` invents `"version": "0.1.0"` for a private manifest. - Duplicated "Checking formatting" banner. I will fold the first four into #68 rather than opening a further issue, so they are fixed alongside the bootstrap work they belong with.
clawbot changed title from Configure prettier and make fmt-check cover markdown (closes #69) to WIP: Configure prettier and make fmt-check cover markdown (closes #69) 2026-08-10 14:40:09 +02:00
clawbot added needs-rebase and removed merge-ready labels 2026-08-10 14:41:43 +02:00
sneak was unassigned by clawbot 2026-08-10 14:42:00 +02:00
clawbot self-assigned this 2026-08-10 14:42:00 +02:00
clawbot changed title from WIP: Configure prettier and make fmt-check cover markdown (closes #69) to Configure prettier and make fmt-check cover markdown (closes #69) 2026-08-10 15:46:33 +02:00
clawbot changed target branch from main to next 2026-08-10 15:46:33 +02:00
clawbot added 1 commit 2026-08-10 15:46:33 +02:00
Configure prettier and make fmt-check cover markdown (closes #69)
All checks were successful
check / check (push) Successful in 37s
b3ac072b8d
script/fmt ran prettier with default settings over root-level *.md and
*.json, swallowing every failure with `|| true`, while script/fmt-check
checked gofmt only. The formatter and the gate therefore disagreed
silently: `make fmt` rewrote markdown that `make check` never looked at,
including REPO_POLICIES.md, which is a verbatim copy of an authoritative
upstream document that local tooling must not touch.

Configuration:

- .prettierrc pins the two policy deviations from prettier defaults,
  four-space indents and proseWrap: always. Nothing else.
- .prettierignore excludes REPO_POLICIES.md so no local run can drift it
  from upstream again, plus .golangci.yml (user-owned, and listed even
  though the current file set does not reach it) and node_modules,
  vendor, bin.

One canonical file set:

- New script/prettier takes --write or --check and applies the same
  patterns in both modes, so script/fmt and script/fmt-check cannot
  drift apart by construction. The patterns are repo-wide (**/*.md,
  **/*.json) rather than root-only, so markdown in subdirectories such
  as a future docs/ is covered.
- No `|| true` anywhere, and no --no-error-on-unmatched-pattern: both
  patterns always match tracked files, so an empty match means the glob
  broke and prettier should say so instead of passing vacuously. A
  missing prettier is a hard error naming script/bootstrap, not a
  silent skip.

Pinned prettier:

- package.json/yarn.lock pin prettier 3.9.6; the lockfile carries the
  integrity hash, and --frozen-lockfile enforces it. script/prettier
  prefers node_modules/.bin/prettier and warns on stderr when it has to
  fall back to a PATH prettier of unknown version.
- script/bootstrap now installs node, yarn, and the locked JS deps. Its
  NODE_VERSION and YARN_VERSION pins already existed.

Docker gate:

- The golangci-lint image has no node, so the lint stage runs the new
  script/fmt-check-go (the Go half of fmt-check, extracted) instead of
  the whole thing.
- The markdown half gets its own stage on a digest-pinned node image
  shipping exactly the node and yarn versions bootstrap pins. The
  builder stage takes a COPY --from dependency on it, so BuildKit cannot
  skip it and a markdown violation fails `docker build .` rather than
  being skipped somewhere nobody looks.

Markdown files other than REPO_POLICIES.md are reformatted here for the
first time under the policy settings.
clawbot force-pushed prettier-fmt-check from b3ac072b8d to 204f09c646 2026-08-10 16:11:02 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-08-10 16:11:38 +02:00
Author
Collaborator

Rebase review, head 204f09c on next de47670: PASS — all 16 non-TODO.md file patches byte-identical to pre-rebase b3ac072, interdiff exactly the file set of #59, lint-stage v2.12.2 tag+digest pin and the mdfmt stage plus COPY --from=mdfmt both intact, TODO.md carries both sides coherently, and the markdown gate positive-controlled (misformatted TODO.md failed [mdfmt 6/6] RUN script/prettier --check exit 1; reverted, cache-defeated docker build --no-cache-filter=lint,builder,mdfmt exit 0 with all three stages observed executing and no (cached) test markers).

Disclosure: host make check aborted at script/lint with parallel golangci-lint is running (#90); host script/test and make fmt-check were green and the containerized make lint reported 0 issues, which is the authoritative run. No CI status on this head (#93).

Rebase review, head `204f09c` on `next` `de47670`: PASS — all 16 non-`TODO.md` file patches byte-identical to pre-rebase `b3ac072`, interdiff exactly the file set of [#59](https://git.eeqj.de/sneak/mfer/pulls/59), lint-stage `v2.12.2` tag+digest pin and the `mdfmt` stage plus `COPY --from=mdfmt` both intact, `TODO.md` carries both sides coherently, and the markdown gate positive-controlled (misformatted `TODO.md` failed `[mdfmt 6/6] RUN script/prettier --check` exit 1; reverted, cache-defeated `docker build --no-cache-filter=lint,builder,mdfmt` exit 0 with all three stages observed executing and no `(cached)` test markers). Disclosure: host `make check` aborted at `script/lint` with `parallel golangci-lint is running` ([#90](https://git.eeqj.de/sneak/mfer/issues/90)); host `script/test` and `make fmt-check` were green and the containerized `make lint` reported 0 issues, which is the authoritative run. No CI status on this head ([#93](https://git.eeqj.de/sneak/mfer/issues/93)).
clawbot merged commit 5683d0f4ff into next 2026-08-10 16:16:43 +02:00
Sign in to join this conversation.