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

Claude · 2026-09-30T16:08:41Z

Claude commented

2026-09-30 16:08:41 +00:00

Summary

Pass --no-sort-keys to pretty-format-json. apm emits insertion order and apm audit --ci diffs its output byte-for-byte, so the formatter's default key sort rewrites apm-owned JSON into a form apm would never produce and the pre-push gate reports drift on a file with an empty git diff.
.claude/settings.json and .claude/apm-hooks.json now round-trip untouched, so both leave the hook's exclude:. .claude-plugin/marketplace.json stays excluded: it carries literal em dashes, and the formatter re-escapes them as ASCII unicode escapes.
Update the docs/spec/gates.md section and the LESSONS.md 2026-08-14 entry, both of which told readers to add excludes.

Why this instead of the gate proposed in #102

Analysis of #102 on current main found it partly stale: the exclude list is 3 paths (not 16), three of the five precedent gates were deleted in #135, and only 5 tracked JSON files exist. Removing the cause is smaller than policing it: a check-formatter-scope-sync gate would be ~250 lines plus a test and a gates.md entry, and would still need a hand-maintained list of tool-owned paths because apm.lock.yaml does not enumerate settings.json, apm-hooks.json, marketplace.json or itself. No ADR: it is a config choice with no design tension.

No dedicated regression test is added. apm-audit-ci at pre-push already fails if the flag is dropped, so a test would only improve the diagnosis (see the second commit, which removes one that was first included).

Test plan

With the exclude removed and no flag, settings.json, apm-hooks.json and marketplace.json are rewritten by the formatter; with the flag, settings.json and apm-hooks.json round-trip byte-identical.
A key-sorted .claude/settings.json makes apm audit --ci fail with drift; the committed file passes.
Pre-push gates pass on the branch (test suite, apm audit --ci, apm pack --check-clean, check-useless-excludes, others).

Out of scope / follow-up

--no-ensure-ascii would let marketplace.json round-trip and remove the last exclude, but it changes output for every JSON file. Left for a separate decision.
Refs #102. Its title and text still describe the gate framing (stale precedents, "sixteenth exclude", wrong ADR-0013 citation), so this PR does not close it. Retitle or close it after review.
A new tool-owned file in the scope of a different autofixer is still only caught after the fact, as apm-audit-ci drift, not by a derived gate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

## Summary - Pass `--no-sort-keys` to `pretty-format-json`. apm emits insertion order and `apm audit --ci` diffs its output byte-for-byte, so the formatter's default key sort rewrites apm-owned JSON into a form apm would never produce and the pre-push gate reports drift on a file with an empty `git diff`. - `.claude/settings.json` and `.claude/apm-hooks.json` now round-trip untouched, so both leave the hook's `exclude:`. `.claude-plugin/marketplace.json` stays excluded: it carries literal em dashes, and the formatter re-escapes them as ASCII unicode escapes. - Update the `docs/spec/gates.md` section and the `LESSONS.md` 2026-08-14 entry, both of which told readers to add excludes. ## Why this instead of the gate proposed in #102 Analysis of #102 on current `main` found it partly stale: the exclude list is 3 paths (not 16), three of the five precedent gates were deleted in #135, and only 5 tracked JSON files exist. Removing the cause is smaller than policing it: a `check-formatter-scope-sync` gate would be ~250 lines plus a test and a `gates.md` entry, and would still need a hand-maintained list of tool-owned paths because `apm.lock.yaml` does not enumerate `settings.json`, `apm-hooks.json`, `marketplace.json` or itself. No ADR: it is a config choice with no design tension. No dedicated regression test is added. `apm-audit-ci` at pre-push already fails if the flag is dropped, so a test would only improve the diagnosis (see the second commit, which removes one that was first included). ## Test plan - [x] With the exclude removed and no flag, `settings.json`, `apm-hooks.json` and `marketplace.json` are rewritten by the formatter; with the flag, `settings.json` and `apm-hooks.json` round-trip byte-identical. - [x] A key-sorted `.claude/settings.json` makes `apm audit --ci` fail with drift; the committed file passes. - [x] Pre-push gates pass on the branch (test suite, `apm audit --ci`, `apm pack --check-clean`, `check-useless-excludes`, others). ## Out of scope / follow-up - `--no-ensure-ascii` would let `marketplace.json` round-trip and remove the last exclude, but it changes output for every JSON file. Left for a separate decision. - Refs #102. Its title and text still describe the gate framing (stale precedents, "sixteenth exclude", wrong ADR-0013 citation), so this PR does not close it. Retitle or close it after review. - A new tool-owned file in the scope of a different autofixer is still only caught after the fact, as `apm-audit-ci` drift, not by a derived gate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

Claude added the Kind/Enhancement

Priority

Medium

3

labels 2026-09-30 16:08:41 +00:00

Claude added 1 commit 2026-09-30 16:08:41 +00:00

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

pretty-format-json sorts object keys by default, but apm emits insertion
order and `apm audit --ci` diffs its output byte-for-byte. A file in the
formatter's scope therefore drifts on every commit with an empty git diff.

Pass --no-sort-keys so `.claude/settings.json` and `.claude/apm-hooks.json`
round-trip untouched and drop them from the exclude. marketplace.json stays
excluded: it carries literal em dashes the formatter re-escapes to —.

tests/test-pretty-json-tool-owned.sh runs the repo's real autofix hooks over
copies of the four tracked apm-owned files and fails if any is rewritten, so
losing the flag fails a test instead of surfacing as drift.

Refs: #102

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

Claude added this to the Tooling milestone 2026-09-30 16:08:53 +00:00

Defame1297 added 1 commit 2026-09-30 16:44:22 +00:00

refactor(pre-commit): drop the tool-owned round-trip test for #102 18fbdbc8e4

The apm-audit-ci pre-push hook already fails when pretty-format-json sorts
apm-owned JSON, so a dedicated test only improved the diagnosis while adding
~100 lines of bash and a pre-commit cache dependency. Remove the test and the
comment, gates.md and LESSONS.md text that pointed at it; the --no-sort-keys
fix itself is unchanged.

Refs: #102

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

Claude commented

2026-09-30 16:44:35 +00:00

Dropped tests/test-pretty-json-tool-owned.sh in 18fbdbc. It was about 100 lines of bash and Python, and it depended on the pre-commit cache. It would only have improved the diagnosis, because apm-audit-ci already fails at pre-push if --no-sort-keys is removed (I confirmed a key-sorted .claude/settings.json fails apm audit --ci with drift).

The PR is now the one-line flag, the exclude shrink, the config comment, and the gates.md and LESSONS.md updates. The PR body is updated to match, and the comment, gates.md and LESSONS.md no longer reference the test. Pre-push gates pass on the branch.

Co-Authored-By: Claude Code noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

Dropped `tests/test-pretty-json-tool-owned.sh` in 18fbdbc. It was about 100 lines of bash and Python, and it depended on the pre-commit cache. It would only have improved the diagnosis, because `apm-audit-ci` already fails at pre-push if `--no-sort-keys` is removed (I confirmed a key-sorted `.claude/settings.json` fails `apm audit --ci` with drift). The PR is now the one-line flag, the exclude shrink, the config comment, and the `gates.md` and `LESSONS.md` updates. The PR body is updated to match, and the comment, `gates.md` and `LESSONS.md` no longer reference the test. Pre-push gates pass on the branch. Co-Authored-By: Claude Code <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi

Defame1297 merged commit 17d67fbfa9 into main

2026-09-30 16:47:30 +00:00

Defame1297 deleted branch fix/pretty-json-no-sort-keys-102

2026-09-30 16:47:30 +00:00

Defame1297 referenced this issue from a commit

2026-09-30 16:47:32 +00:00

Merge pull request 'fix(pre-commit): keep key order in pretty-format-json so apm-owned JSON survives' (#146) from fix/pretty-json-no-sort-keys-102 into main

Claude referenced this pull request

2026-09-30 17:27:09 +00:00

build: no gate asserts that tool-owned paths stay out of autofixing formatter scope #102

Sign in to join this conversation.