diff --git a/.claude/settings.json b/.claude/settings.json index 0178c29..acf6ed7 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -2,14 +2,14 @@ "hooks": { "SessionStart": [ { + "matcher": "startup", "hooks": [ { + "type": "command", "command": "\"${CLAUDE_PROJECT_DIR}/.claude/hooks/kyberforge/.apm/hooks/check-apm-current.sh\"", - "timeout": 380, - "type": "command" + "timeout": 380 } - ], - "matcher": "startup" + ] } ] } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8552a17..8018897 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,20 @@ repos: # would not. All fifteen real files (3 root marketplace manifests, 2 per # plugin x 6 plugins) match; anything else is hand-authored and gets # formatted. - exclude: '^(\.claude-plugin/marketplace\.json|\.agents/plugins/marketplace\.json|\.github/plugin/marketplace\.json|plugins/[^/]+/\.claude-plugin/plugin\.json|plugins/[^/]+/\.github/plugin/plugin\.json)$' + # + # `.claude/settings.json` is the sixteenth, and it is excluded for a + # different reason: 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. + exclude: '^(\.claude-plugin/marketplace\.json|\.agents/plugins/marketplace\.json|\.github/plugin/marketplace\.json|plugins/[^/]+/\.claude-plugin/plugin\.json|plugins/[^/]+/\.github/plugin/plugin\.json|\.claude/settings\.json)$' - id: check-yaml stages: ['pre-commit'] - id: trailing-whitespace diff --git a/LESSONS.md b/LESSONS.md index 267ef3c..65ef503 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -203,3 +203,24 @@ this skill to manage label definitions`). That is the `Kyberforge.VagueQualifier Fix: keep the opener rule opener-anchored and give mid-description prose its own rule with its own token list. A rule's scope anchor is part of its contract, not an implementation detail to relax when a new case does not fit. + +## 2026-08-14 — A formatter in the commit path manufactures drift on a file with a clean git diff + +`apm audit --ci` failed for weeks on `.claude/settings.json` while `git diff` on that file was empty — +the worst possible pairing of signals, because the file matched HEAD exactly and every instinct says +"nothing changed here". The content was identical to apm's output to the byte; only the JSON key +order differed. `pretty-format-json --autofix` sorts object keys unless `--no-sort-keys` is passed, +and its `exclude:` listed fifteen generated manifests but not this file, so from the commit that +first wrote a hook entry there (`2e395a4`) onward, apm's insertion-ordered output was silently +re-sorted on the way in. apm then replayed the install, produced its own order, and reported drift +against a file no human had touched. + +Two general points. First, a tool-owned generated file that passes through an autofixing formatter is +drifted by construction, and the diff that would reveal it never appears in `git diff` — it only +exists between the formatter's input and its output, which nothing stores. Second, the fix is +self-undoing unless the exclude lands in the same commit: correcting the file alone means the hook +re-breaks it as it is staged. Fix: when a tool declares ownership of a path, add that path to every +autofixing hook's `exclude` at the moment ownership is declared, not when the drift is noticed. This +repo gates marketplace-mirror, plugin-content and vale-style drift deterministically and has no +equivalent gate asserting tool-owned paths stay out of formatter scope — `.claude/settings.json` was +the sixteenth exclude and nothing prevents a seventeenth. diff --git a/apm.lock.yaml b/apm.lock.yaml index 172000f..899b302 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -1,5 +1,5 @@ lockfile_version: '1' -generated_at: '2026-08-14T18:44:07.701402+00:00' +generated_at: '2026-08-14T21:26:25.156410+00:00' apm_version: 0.28.0 dependencies: - repo_url: Defame1297/holocron @@ -521,7 +521,7 @@ dependencies: .claude/skills/vale-run/references/troubleshooting.md: sha256:b4c2bc67b413b102d9fe1cd7e5a248b19aba3dc3b60b4429e777977cf8021cd5 content_hash: sha256:7bc57b8852680bdcba32a6a6108b99ef6bcdf26eacfecd806c421ec5d1fd47d9 declared_license: MIT - exec_status: deployed + exec_status: gated_pending_approval deployments: - kind: project-relative target: claude diff --git a/tests/test-run-tests.sh b/tests/test-run-tests.sh index d3e80ed..39c2cf1 100644 --- a/tests/test-run-tests.sh +++ b/tests/test-run-tests.sh @@ -76,6 +76,18 @@ add_case() { # Runs the fixture's run-tests.sh over its cases/ directory, capturing output and # exit code separately. TMPDIR is private per run so a case script can locate the # scratch directory run-tests.sh mktemp -d's for itself -- see case 6. +# +# RUN_TESTS_STRICT is REMOVED from the child environment, not merely left alone. +# This suite is itself discovered and run by run-tests.sh, so when the outer run +# is the gate (`bash tests/run-tests.sh --strict`, or RUN_TESTS_STRICT=1 in CI) +# the variable is exported down the whole process tree and every fixture below +# silently became a strict run. Case 10c -- the control asserting a skip is +# tolerated WITHOUT strict -- then failed, and the suite passed ad hoc while +# failing under the exact invocation the run-tests pre-push hook uses. The +# fixture's strictness must be a property of the case, never of how this file +# happened to be launched, so strict is opted into per case: `--strict` on the +# argv (cases 10, 10d, 10f, 10h) or an explicit RUN_TESTS_STRICT=1 on the one +# invocation testing the env var (case 10b). Case 10g pins the scrub itself. FAKE_OUT="" FAKE_RC=0 run_fake() { @@ -84,7 +96,8 @@ run_fake() { priv="$(mktemp -d)" FIXTURES+=("$priv") FAKE_RC=0 - FAKE_OUT="$(TMPDIR="$priv" TEST_DIR="$dir/cases" bash "$dir/tests/run-tests.sh" "$@" 2>&1)" || FAKE_RC=$? + FAKE_OUT="$(env -u RUN_TESTS_STRICT TMPDIR="$priv" TEST_DIR="$dir/cases" \ + bash "$dir/tests/run-tests.sh" "$@" 2>&1)" || FAKE_RC=$? } # --- 1. A healthy run is green, runs the bats leg, and says so --- @@ -524,6 +537,73 @@ else fail "--strict failed but lost the stderr skip reason: $FAKE_OUT" fi +# --- 10g. An ambient RUN_TESTS_STRICT=1 must not reach a fixture that did not ask +# for it. This suite is discovered and run by run-tests.sh itself, so under the +# gate the variable is exported into every child here. That is not a hypothetical: +# `bash tests/run-tests.sh` reported 19 passed while +# `RUN_TESTS_STRICT=1 bash tests/run-tests.sh` reported this file as the one +# failure, because case 10c's deliberately-non-strict run inherited strict and +# went red. The gate was therefore RED for everyone, and the only way to see it +# was to run the gate. +# +# The variable is exported here rather than passed as a prefix on purpose: a +# prefix (`RUN_TESTS_STRICT=1 run_fake ...`) applies to the function call, and +# `env -u` inside it would strip it either way, so the prefix form cannot tell a +# working scrub from a broken one. Exporting reproduces the real leak -- the +# ambient environment this whole script runs in -- which is the state that broke. +echo "" +echo "--- an ambient RUN_TESTS_STRICT=1 does not leak into a non-strict fixture ---" +export RUN_TESTS_STRICT=1 +run_fake "$DIR10" +unset RUN_TESTS_STRICT +if [[ $FAKE_RC -ne 0 ]]; then + fail "an inherited RUN_TESTS_STRICT=1 turned a non-strict fixture strict — this suite's own result depends on how it was launched: $FAKE_OUT" +elif echo "$FAKE_OUT" | grep -q "a skip is a SETUP ERROR"; then + fail "the fixture ran strict despite not asking for it: $FAKE_OUT" +elif echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$"; then + pass "an ambient RUN_TESTS_STRICT=1 is scrubbed from fixtures that did not ask for strict" +else + fail "the scrubbed run produced the wrong summary: $FAKE_OUT" +fi + +# --- 10h. Under --strict the skip report goes to STDERR, and the stdout skip list +# is suppressed rather than printed twice --- +# Both halves are deliberate and neither was pinned. The stream matters because +# this block is what a pre-push reader is handed: pre-commit prints nothing for a +# passing hook and shows the failing hook's output, and the runner routes the +# strict diagnostic to stderr so it survives a caller that redirects stdout. The +# suppression matters because without it the same suites are listed twice a few +# lines apart, which is exactly the noise that trains a reader to scroll past the +# list instead of reading it. Every other strict case captures `2>&1`, which +# folds the two streams together and so cannot see either property. +echo "" +echo "--- --strict reports skips on stderr only, without duplicating the stdout list ---" +DIR10H="$(make_fake_repo)" +FIXTURES+=("$DIR10H") +install_healthy_bats_runner "$DIR10H" +add_case "$DIR10H" test-needs-a-binary.sh <<'EOF' +#!/usr/bin/env bash +echo "SKIP: frobnicator is not installed" +exit 77 +EOF +H_PRIV="$(mktemp -d)" +FIXTURES+=("$H_PRIV") +H_RC=0 +env -u RUN_TESTS_STRICT TMPDIR="$H_PRIV" TEST_DIR="$DIR10H/cases" \ + bash "$DIR10H/tests/run-tests.sh" --strict \ + >"$H_PRIV/stdout" 2>"$H_PRIV/stderr" || H_RC=$? +if [[ $H_RC -eq 0 ]]; then + fail "--strict passed with a skipped suite" +elif ! grep -q "a skip is a SETUP ERROR" "$H_PRIV/stderr"; then + fail "the strict skip report is not on stderr — a caller redirecting stdout loses the only explanation of the failure: $(cat "$H_PRIV/stderr")" +elif grep -q "a skip is a SETUP ERROR" "$H_PRIV/stdout"; then + fail "the strict skip report was also written to stdout" +elif grep -q "^Skipped scripts:$" "$H_PRIV/stdout"; then + fail "--strict printed the stdout skip list as well as the stderr report — the same suites are named twice" +else + pass "--strict reports skipped suites on stderr and suppresses the duplicate stdout list" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]