fix(pre-commit): keep key order in pretty-format-json so apm-owned JSON survives #146

Merged
Defame1297 merged 2 commits from fix/pretty-json-no-sort-keys-102 into main 2026-09-30 16:47:30 +00:00
3 changed files with 34 additions and 50 deletions

No files matched your search

+17 -34
View File
@@ -27,41 +27,24 @@ repos:
stages: ['pre-commit']
- id: pretty-format-json
stages: ['pre-commit']
args: [--autofix]
# Every generated manifest lives at a KNOWN path, so every alternative is
# root-anchored and spells that path out. This was five `(^|/)`
# any-depth alternatives plus one `^` root-only one -- a mixture with no
# rationale, under which a fixture or vendored tree containing
# `.../.claude-plugin/marketplace.json` would have been silently excluded
# from formatting while an equivalent
# `.../.agents/plugins/marketplace.json` would not. Only the one root
# marketplace manifest matches now; anything else is hand-authored and
# gets formatted. The twelve per-plugin `plugin.json` alternatives were
# dropped with the plugin manifests themselves when native
# `claude plugin install` support was removed (ADR-0024) -- apm probes
# `apm.yml` and never reached them. The `.agents/plugins/` and
# `.github/plugin/` marketplace mirrors went the same way, and their
# alternations went with them: `check-useless-excludes` fails on a
# pattern that matches no file.
args: [--autofix, --no-sort-keys]
# `--no-sort-keys` is load-bearing. apm OWNS `.claude/settings.json` and its
# `.claude/apm-hooks.json` sidecar (ADR-0018, ADR-0019), and
# `apm audit --ci` replays the install and diffs the result byte-for-byte.
# apm emits insertion order (`matcher` before `hooks`); the formatter's
# default sorts keys, rewrites that into a form apm would never produce,
# and the `apm-audit-ci` pre-push hook then reports drift on a file with
# no git diff (#102, first hit at 2e395a4). Keeping insertion order means
# those two files need no exclude. Dropping the flag is caught at pre-push
# by `apm-audit-ci` as drift on `.claude/settings.json`.
#
# `.claude/settings.json` and its `.claude/apm-hooks.json` ownership
# sidecar are the last two alternations, and they are the only ones
# here for a reason other than "generated manifest":
# apm OWNS that file (ADR-0018, ADR-0019), and
# `apm audit --ci` replays the install into a scratch tree and diffs
# the result byte-for-byte. `pretty-format-json` sorts object keys
# unless `--no-sort-keys` is passed, while apm's hook integrator emits
# insertion order (`matcher` before `hooks`, `type` before `command`).
# Formatting the file therefore rewrites apm's output into a form apm
# would never produce, and the `apm-audit-ci` pre-push hook reports it
# as permanent drift on a file with no git diff -- exactly what
# happened when the SessionStart hook first landed in 2e395a4.
# Re-running `apm install` fixes the file; leaving it in scope here
# would re-break it on the very commit that carries the fix. The
# sidecar is committed so a fresh clone's install can claim the
# settings entry instead of duplicating it (ADR-0019, 2026-09-16
# correction), and it is apm output under the same byte-for-byte replay.
exclude: '^(\.claude-plugin/marketplace\.json|\.claude/(settings|apm-hooks)\.json)$'
# `.claude-plugin/marketplace.json` is the one remaining exclude. It
# round-trips except for non-ASCII: it carries literal em dashes and the
# formatter re-escapes them to `\u2014` (`--no-ensure-ascii` would fix that,
# but it changes the output for every JSON file). Root-anchored because it
# is one known path; `check-useless-excludes` fails on a pattern that
# matches no file.
exclude: '^\.claude-plugin/marketplace\.json$'
- id: check-yaml
stages: ['pre-commit']
- id: trailing-whitespace
+1 -1
View File
@@ -128,7 +128,7 @@ Widening a description-opener rule to also catch mid-sentence text looked like a
## 2026-08-14 — A formatter in the commit path manufactures drift on a file with a clean git diff
`apm audit --ci` failed on `.claude/settings.json` with an empty `git diff` — `pretty-format-json --autofix` silently re-sorts JSON keys, and this generated file was missing from its exclude list, so every commit re-sorted apm's insertion-ordered output before apm compared against it. Separately, a defect introduced 3 hours earlier on the same branch was first mis-described as "pre-existing," an unverified claim about history. Fix: add tool-owned paths to every autofixing hook's exclude the moment ownership is declared, and verify "pre-existing" claims with `git log -S` or `git branch --contains` before writing them down.
`apm audit --ci` failed on `.claude/settings.json` with an empty `git diff` — `pretty-format-json --autofix` silently re-sorts JSON keys, and this generated file was missing from its exclude list, so every commit re-sorted apm's insertion-ordered output before apm compared against it. Separately, a defect introduced 3 hours earlier on the same branch was first mis-described as "pre-existing," an unverified claim about history. Fix: add tool-owned paths to every autofixing hook's exclude the moment ownership is declared (for JSON, superseded by #102: `--no-sort-keys` makes the exclude unnecessary), and verify "pre-existing" claims with `git log -S` or `git branch --contains` before writing them down.
## 2026-08-16 — A rule reversed inside a retrofit leaves no trace unless someone writes it down (historical)
+16 -15
View File
@@ -1279,24 +1279,25 @@ point only). Machine-specific settings go in the gitignored `.claude/settings.lo
does not deploy and the replay does not compare; shared enforcement belongs in
`.pre-commit-config.yaml`.
### Why it is excluded from `pretty-format-json`
It is in the **second and last alternation** in that hook's `exclude:` pattern, and that alternation
is the only one there for a reason other than "generated manifest". Mind which number you are
quoting: the pattern is `^(\.claude-plugin/marketplace\.json|\.claude/(settings|apm-hooks)\.json)$`
— **two top-level alternations, expanding to three real tracked files**:
`.claude-plugin/marketplace.json`, this one, and its committed `.claude/apm-hooks.json` sidecar,
which is apm output under the same byte-for-byte replay and is excluded for the same reason.
### Why `pretty-format-json` runs with `--no-sort-keys`
`pretty-format-json --autofix` sorts object keys unless `--no-sort-keys` is passed, while apm's hook
integrator emits insertion order (`matcher` before `hooks`, `type` before `command`). Leaving the
file in that hook's scope therefore rewrites apm's output into a form apm would never produce on the
way into **every** commit, and `apm-audit-ci` then reports permanent drift on a file with an empty
`git diff` — exactly what happened when the `SessionStart` hook first landed in `2e395a4`. Re-running
`apm install` fixes the file; leaving it in scope would re-break it on the very commit carrying the
fix.
integrator emits insertion order (`matcher` before `hooks`, `type` before `command`). In scope with
the default, the formatter rewrites apm's output into a form apm would never produce on the way into
**every** commit, and `apm-audit-ci` then reports permanent drift on a file with an empty `git diff`
— exactly what happened when the `SessionStart` hook first landed in `2e395a4` (#102).
**Load-bearing. Do not tidy it out of that list** (see `LESSONS.md`, 2026-08-14).
The hook now passes `--no-sort-keys`, so this file and its committed `.claude/apm-hooks.json` sidecar
(apm output under the same byte-for-byte replay) need **no exclude**: the formatter's default 2-space
indent already matches apm's, and with insertion order kept they round-trip untouched. No dedicated
test pins this: dropping `--no-sort-keys` surfaces at pre-push as `apm-audit-ci` drift on
`.claude/settings.json`, which is the same gate that caught the original failure.
**Load-bearing. Do not remove `--no-sort-keys`.** `.claude-plugin/marketplace.json` is the one path
still in that hook's `exclude:`: it carries literal em dashes that the formatter re-escapes to
`\u2014`, which `--no-ensure-ascii` would stop but for every JSON file. This closes the JSON case
only; a new tool-owned file in the scope of another autofixer is still caught only by `apm-audit-ci`
drift after the fact, not by a derived gate.
## Pushing without a network