From 33c0d1dde66849749d7ee188c39c74355864a478 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Fri, 16 Oct 2026 08:05:09 -0500 Subject: [PATCH 1/2] feat(hook): pre-commit tidies only staged vendor files and re-stages what it wrote (closes #320) Co-Authored-By: Claude Opus 5.5 --- .githooks/pre-commit | 60 +- CHANGELOG.md | 2 + docs/cli-reference.md | 24 +- packages/cli/README.md | 2 +- .../src/__tests__/adf-tidy-sources.test.ts | 266 +++++++++ .../tidy-single-source.main-01d2834.json | 83 +++ packages/cli/src/__tests__/hook.test.ts | 159 +++++- .../integration/precommit-hook.test.ts | 519 ++++++++++++------ packages/cli/src/commands/adf-tidy.ts | 62 ++- packages/cli/src/commands/adf.ts | 2 +- .../src/commands/doctor-gate-enforcement.ts | 27 + packages/cli/src/commands/doctor.ts | 8 +- packages/cli/src/commands/hook.ts | 94 +++- packages/cli/src/flags.ts | 16 + 14 files changed, 1053 insertions(+), 271 deletions(-) create mode 100644 packages/cli/src/__tests__/adf-tidy-sources.test.ts create mode 100644 packages/cli/src/__tests__/fixtures/tidy-single-source.main-01d2834.json diff --git a/.githooks/pre-commit b/.githooks/pre-commit index b4181924..85847da8 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -4,41 +4,53 @@ set -euo pipefail echo '[pre-commit] Running TypeScript check (pnpm exec tsc --noEmit)...' pnpm exec tsc --noEmit -# Vendor file auto-tidy: extract bloat from staged vendor config files +# Vendor file auto-tidy (#320): tidy only the staged vendor files, then re-stage only the files tidy wrote. +# Charter pre-commit vendor tidy: scoped to staged files # Skip with CHARTER_SKIP_TIDY=1 if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then - VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md GEMINI.md copilot-instructions.md" - STAGED_VENDORS="" + VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md" + STAGED=$(git diff --cached --name-only --diff-filter=d) + SOURCES="" + NAMES="" for vf in $VENDOR_FILES; do - if git diff --cached --name-only | grep -qx "$vf"; then - # Excluding compiled output here means a commit staging only compiled files - # skips tidy and the "git add .ai/*.adf" re-stage entirely. When tidy does run, - # it skips compiled files itself (#319). + if printf '%s\n' "$STAGED" | grep -xF "$vf" >/dev/null; then + # Fast path: compiled output is never tidied (#319), so a commit staging only + # compiled files never invokes the CLI. if [ -f "$vf" ] && grep -q 'GENERATED by charter adf compile' "$vf"; then continue fi - STAGED_VENDORS="$STAGED_VENDORS $vf" + if ! git diff --quiet -- "$vf"; then + echo "[pre-commit] $vf has unstaged changes; not auto-tidying it. Stage or stash them, or run: charter adf tidy --source $vf" + continue + fi + SOURCES="$SOURCES --source $vf" + NAMES="$NAMES $vf" fi done - if [ -n "$STAGED_VENDORS" ]; then - echo "[pre-commit] Checking vendor config files:$STAGED_VENDORS" - TIDY_OUTPUT=$(node packages/cli/dist/bin.js adf tidy --dry-run --format json 2>/dev/null || echo '{}') - EXTRACTED=$(echo "$TIDY_OUTPUT" | grep -o '"totalExtracted": *[0-9][0-9]*' | grep -o '[0-9][0-9]*' || echo "0") - - if [ "$EXTRACTED" -gt 0 ] 2>/dev/null; then - echo "[pre-commit] Found $EXTRACTED items beyond thin pointer — auto-tidying..." - node packages/cli/dist/bin.js adf tidy 2>/dev/null - - # Re-stage tidied vendor files and modified .adf modules - for vf in $STAGED_VENDORS; do - if [ -f "$vf" ]; then - git add "$vf" + if [ -n "$SOURCES" ]; then + echo "[pre-commit] Checking staged vendor files:$NAMES" + PLAN=$(node packages/cli/dist/bin.js adf tidy --dry-run --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; } + if printf '%s\n' "$PLAN" | grep -E '^(vendor|module) ' >/dev/null; then + BLOCKED="" + for p in $(printf '%s\n' "$PLAN" | sed -n 's/^module //p'); do + if ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then + BLOCKED="$BLOCKED $p" fi done - git add .ai/*.adf 2>/dev/null || true - - echo "[pre-commit] Vendor files tidied and re-staged." + if [ -n "$BLOCKED" ]; then + echo "[pre-commit] Not auto-tidying: tidy would write into files with unstaged work:$BLOCKED" + echo "[pre-commit] Stage or stash them and commit again, or run: charter adf tidy$SOURCES" + else + echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." + WROTE=$(node packages/cli/dist/bin.js adf tidy --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy failed; nothing re-staged."; WROTE=""; } + for line in $(printf '%s\n' "$WROTE" | sed 's/ /:/'); do + if printf '%s\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then + git add -- "${line#*:}" + fi + done + echo "[pre-commit] Vendor files tidied; re-staged only the files tidy wrote." + fi else echo "[pre-commit] Vendor config files are clean." fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 58feb869..3f219ff3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. ### Changed +- **The pre-commit hook tidies only the staged vendor files and re-stages only the files tidy wrote** (`#320`, `#318` part 4) — re-run `charter hook install --pre-commit` to pick up staged-only tidy. An installed hook keeps its old behaviour until you do, and `charter doctor` reports it with an INFO `pre-commit hook` row (INFO, so `--ci` does not fail). Reinstalling overwrites a Charter-managed hook without `--force`, as before. The old hook ran a whole-tree `adf tidy` whenever any vendor file was staged, then ran `git add` on every staged vendor file and on all of `.ai/*.adf`. That swept unrelated `.ai/` edits into the commit and rewrote vendor files that were not part of it. The new hook passes each staged vendor file to `adf tidy` with `--source` and stages only the paths that both the dry run and the real run report. A staged vendor file that also has unstaged changes is skipped with a message, because tidy works on the working tree and staging its result would commit those hunks. If tidy would write into a module that has unstaged changes or is untracked, nothing is tidied for that commit and a message says why. A staged lowercase `agents.md` is now tidied (`#302`), and a commit staging only compiled output still makes no CLI call. The tidy block never fails a commit. The ADF evidence gate after it is unchanged, so in the generated hook vendor bloat that a skipped tidy leaves in the working tree still fails `charter doctor --adf-only --ci`. - **BREAKING: `charter adf compile --write` no longer converts pointer files without `--mode compiled`** (`#318`, part 2) — compile used to overwrite any Charter thin pointer with compiled output, silently changing its mode. Each target's mode now comes from the `VENDORS` record, or from its markers when there is no entry, and `--write` only writes targets that are compiled or absent. Under `--target all`, pointer and unmanaged targets are skipped (exit 0), and each `[skip]` line names the command that converts it. A single such `--target` is refused (exit 1). `--force` alone never changes a mode. `--mode compiled` records `compiled` before writing. If that cannot be recorded, nothing is written (`refused-record`). If the write then fails, the record is restored and the run continues. A converted file is reported as `Written CLAUDE.md (mode: pointer -> compiled)`. JSON output adds `skipped` and per-target `targets`. A compiled-only repo with no `VENDORS` section compiles as before. - **`charter bootstrap --force` no longer overwrites hand-authored or compiled vendor files** (`#318`, part 3) — this changes what a documented flag does. `--force` still re-scaffolds `.ai/*.adf` (with backups in `.ai/.backup/`) and still rewrites Charter thin pointers from the current template. It now skips a `CLAUDE.md`, `AGENTS.md`, `.cursorrules`, `GEMINI.md` or `copilot-instructions.md` that has no Charter marker, carries the compile banner, or is recorded `compiled` or `unmanaged` in `VENDORS`, with a warning naming the next step (`adf migrate --source `, or `adf vendor set pointer && adf vendor restore ` for compiled output). With `--yes`, phase 4 then migrates a hand-authored file that has no `VENDORS` entry into `.ai/`, as it would without `--force`, and the phase 3 warning says so; a hand-authored file that already has an entry is left as it is. Phase 4 no longer migrates a file recorded `unmanaged`, and valid `VENDORS` entries are carried over to the re-scaffolded `manifest.adf`. To replace a hand-authored file without migrating it, delete it and re-run bootstrap. @@ -20,6 +21,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. ### Added +- **`charter adf tidy --source` is repeatable, and `--print-paths` lists the files tidy writes** (`#320`) — each `--source` is processed in argument order, and two names for one file (`AGENTS.md` and `agents.md` on a case-insensitive filesystem, or a hard link) are tidied once, under the first name. A `--source` with no value is now a usage error (exit 2); it used to tidy every vendor file silently. `--print-paths` prints one `vendor ` or `module ` line per file tidy writes (or would write, under `--dry-run`) and nothing else on stdout; warnings go to stderr. It cannot be combined with `--format json` (exit 2). JSON output adds `paths` (`vendor` and `modules`); `modulesModified` is unchanged. A single `--source` gives the same output as before. - **`charter adf vendor restore `** (`#318`, part 3) — rewrites one vendor file as its thin pointer template (`CLAUDE.md` gets the hybrid template with its module index, as bootstrap writes it), carrying `## Environment` and other retained sections over verbatim. It runs only for a file in `pointer` mode, or an absent file with no entry, and refuses (exit 1, nothing written) whenever content would be lost: hand-authored content, content `adf tidy` would extract, any line of a pointer that is not from a Charter template and that the new pointer would not carry (an edited title, an extra note, edited `## Session Start` text), your own sections in a compiled file, or a compiled file that differs from what `.ai/` compiles to now (checked in memory). It is the fix doctor names for a recorded pointer that now holds compiled output. A file with no entry gets `pointer` recorded first; a failed file write puts the manifest back and is reported as `write-failed` with `writeError`. `--dry-run` and `--format json` are supported. - **`charter adf migrate` registers the modules it routes content into** (`#239`, completes the migrate half of `#241`) — migrate could create `infra.adf`, `frontend.adf` or `core.adf` without adding them to `.ai/manifest.adf` or the `CLAUDE.md` module index, so no agent knew to load them and `charter doctor` failed until someone wired them by hand. Each target module that is not already in the manifest is now registered after the module merge and before the pointer is written, so the hybrid `CLAUDE.md` index includes it. A source other than `CLAUDE.md` that adds a module refreshes that index afterwards. `core.adf` and `state.adf` go to `DEFAULT_LOAD`, through the same classification `doctor` checks. Other modules go to `ON_DEMAND` with up to six triggers inferred from the headings of the items routed there. The heading word the router matched counts double, a named module's curated triggers come first, words that already trigger another module are dropped, and the lowercase module stem is the fallback. To supply that heading word, `@stackbilt/adf`'s `RoutingTrace` gains an optional `headingKeyword`. `classifyElement` now always sets `routingTrace`, even without a trigger map, in which case `candidateScores` is `{}`. That is an additive change, but code that read a defined `routingTrace` as "content routing ran" should check `candidateScores` instead. Registration is append-only. New bullets are inserted as text after the section's last item, in that item's indentation and bullet style, and a missing section is appended at the end of the file. Comments, HTML comments, tables, section order, blank lines and CRLF or LF line endings are left byte-for-byte. Before writing, the edited manifest is re-parsed and must equal the old one plus exactly the new entries; otherwise nothing is written and a warning is printed. An existing entry is never rewritten, moved or given new triggers, and a re-run writes nothing. An existing entry is reported as it stands, in its real section with its triggers as spelled. A section header with no items counts as an empty list and is filled. `core.adf` under `ON_DEMAND` is left in place with a warning naming the move. A missing `manifest.adf` warns to run `charter adf init`, an unparseable one warns to fix it or run `charter doctor`, and in both cases registration is skipped. The plan prints a `REGISTER` line per module, and `--format json` adds `sources[].registrations[]` (`module`, `loadPolicy`, `triggers`, `triggerSources`, `reason`, `status`, `skipReason`). `--dry-run` writes nothing and reports each planned module once across sources. As a known gap, it does not simulate an earlier source's new triggers when routing a later one. Bootstrap's two orphan writers now use the same shared writer, which also removes a line-splice ON_DEMAND writer that could register a module twice. That brings three behavior changes. Orphan triggers are now the lowercase stem (`Observability.adf` gets `observability`). A `DEFAULT_LOAD` that is not a list is now skipped with a warning, where it used to have a raw `DEFAULT_LOAD:` block appended. And with `--format json`, writer warnings now appear in the phase 3 step's `warnings`. A bootstrap `--yes` run registers migrate's modules in phase 4, so a re-run finds no orphans. `charter adf create` keeps its own upsert writer, and the stale `charter adf register` hint in bootstrap is tracked separately in `#347`. - **A record of each vendor file's mode, and `charter adf vendor status|set|init`** (`#318`, part 1) — nothing recorded whether `CLAUDE.md`, `AGENTS.md`, `.cursorrules` or `GEMINI.md` was meant to be a thin pointer to `.ai/` or `adf compile` output, so every command guessed from the markers in the file. `.ai/manifest.adf` can now carry a `VENDORS` list (` - CLAUDE.md: pointer`) that gives each file's mode as `pointer`, `compiled` or `unmanaged`. `charter adf vendor status` is read-only. It shows each file's recorded mode, or the mode inferred from its markers when none is recorded, and flags a recorded mode that the file on disk contradicts. Inference follows `adf tidy`: the banner means `compiled`, a pointer marker means `pointer`, and anything else is `unmanaged`. `charter adf vendor set ` records one file, and `charter adf vendor init` records the inferred mode of every unrecorded file on disk. `agents.md` and `copilot-instructions.md` may never be `compiled`, since no compile target writes them. `bootstrap`, `adf init --emit-pointers`, `adf migrate` and `adf compile --write` now record the mode of each vendor file they write. They only add a missing entry, so an existing one is kept with a warning. Under `--dry-run` nothing is recorded. A record that cannot be written produces a warning and changes no exit code or JSON. Under `--format json` that warning goes into bootstrap's phase-3 `warnings`, and to stderr for the other writers. The writer reuses the text-level helpers from `#239`, so other bytes and line endings are kept, and the edit is re-parsed before writing. `@stackbilt/adf`'s `parseManifest` gains a permissive `vendors` field. `VENDORS` gets the bare 🏷 decoration (U+1F3F7 without U+FE0F, which the parser drops, so a written block stays `adf fmt` canonical), sorts after `SYNC`, and is never rendered into compiled output. When `AGENTS.md` and `agents.md` are one physical file (case-insensitive filesystem or hard link), every writer records it once, under the spelling already recorded. `charter score` leaves the section out of its manifest path-reference check. In this part no command acted on the record yet. Part 2 (above) makes `compile` and `tidy` read it, and `bootstrap`, `migrate`, `doctor`, `score` and both pre-commit hooks still decide as before until later parts of `#318`. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 8dac3d49..63b75992 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -65,13 +65,19 @@ Installs git hooks for commit-time governance enforcement. ```bash npx charter hook install --commit-msg # trailer gate + normalization hook -npx charter hook install --pre-commit # ADF evidence gate hook +npx charter hook install --pre-commit # vendor tidy + ADF evidence gate hook npx charter hook install --commit-msg --pre-commit # both hooks npx charter hook install --commit-msg --force # overwrite existing hooks ``` - `--commit-msg` — normalizes `Governed-By` and `Resolves-Request` trailers via `git interpret-trailers`; rejects a message with neither trailer in its final trailer block whenever `charter validate --ci` would fail it, reading `.charter/config.json` with `validate`'s defaults (`requireTrailers` on unless falsy, `citationStrictness` `FAIL`, an unparseable file treated as the defaults). It blocks under `FAIL`/`STRICT` or `ci.failOnWarn` and warns under advisory strictness. No `.charter/config.json` at all, or no `node` on `PATH`, skips the gate with a warning. `#` lines are stripped before the check, as the default editor cleanup does, so under `-m` or `--cleanup=verbatim` a trailer followed by a `#...` line can pass locally and fail CI. -- `--pre-commit` — runs ADF evidence checks (LOC ceiling validation) before each commit. Only gates when `.ai/manifest.adf` exists. +- `--pre-commit` — tidies the staged vendor files, then runs ADF evidence checks (LOC ceiling validation) before each commit. Only gates when `.ai/manifest.adf` exists. + - Tidy covers only staged vendor files (`CLAUDE.md`, `.cursorrules`, `AGENTS.md`, `agents.md`, `GEMINI.md`, `copilot-instructions.md`), each passed with `adf tidy --source`. A file carrying the compile banner is skipped without calling the CLI. + - A staged vendor file that also has unstaged changes is skipped, with a message. + - If tidy would write into a `.ai/` module that has unstaged changes or is untracked, nothing is tidied for that commit, with a message. + - Only the files tidy wrote are re-staged, never `.ai/*.adf` as a whole. A tidy failure prints one line and the commit proceeds. Skip tidy with `CHARTER_SKIP_TIDY=1`. + - The evidence gate after tidy still fails on vendor bloat left in the working tree. + - A hook installed before [#320](https://github.com/Stackbilt-dev/charter/issues/320) re-stages all of `.ai/*.adf`. Re-run `charter hook install --pre-commit` to replace it (no `--force` needed for a Charter-managed hook). `charter doctor` reports an old hook as INFO. - `--force` — overwrite existing non-Charter hooks Must specify at least one of `--commit-msg` or `--pre-commit`. @@ -159,6 +165,12 @@ are present; doctor names it and asks for a re-install. The local hook checks only the commit being written, so CI should still run `charter validate --ci`; the workflow generated by `charter setup --ci github` includes that step. +A `pre-commit hook` INFO row appears when the active hooks directory (honouring +`core.hooksPath`) holds a Charter pre-commit hook generated before staged-only +tidy ([#320](https://github.com/Stackbilt-dev/charter/issues/320)). That hook re-stages +all of `.ai/*.adf`; re-run `charter hook install --pre-commit`. A current, foreign +or missing hook adds no row. It is INFO, not WARN, so `--ci` does not fail on upgrade. + #### Vendor file modes The `adf vendor mode` check compares each vendor file's `VENDORS` entry (see [`charter adf vendor`](#charter-adf-vendor)) with the markers in the file. It runs under `--adf-only`, so the pre-commit hook runs it too. It is one check whose status is its worst row, and `--format json` adds a `rows` array to that check only: `kind`, `status`, `file`, `recordedMode`, `inferredMode`, `source`, `raw`, `remedy`. @@ -562,16 +574,18 @@ npx charter adf tidy # classify and route, restore pointe npx charter adf tidy --dry-run # preview without modifying files npx charter adf tidy --dry-run --ci # exit 1 if bloat found (pre-commit gating) npx charter adf tidy --source CLAUDE.md # single file +npx charter adf tidy --source CLAUDE.md --source GEMINI.md --dry-run --print-paths # what the hook runs ``` - `--dry-run` — preview without modifying files - `--ci` — exit 1 if bloat is found; combine with `--dry-run` for gating -- `--source ` — tidy a single file instead of scanning all vendor configs +- `--source ` — tidy this file instead of scanning all vendor configs. Repeat it to tidy several files, in argument order. Two names for one file (`AGENTS.md` and `agents.md` on a case-insensitive filesystem, or a hard link) are tidied once. A `--source` with no value is a usage error (exit 2). +- `--print-paths` — print one line per file tidy writes (or would write, under `--dry-run`): `vendor ` lines, then `module ` lines, repo-relative with `/`. Nothing else goes to stdout; warnings go to stderr. No lines means tidy writes nothing. Cannot be combined with `--format json` (exit 2). - `--ai-dir ` — custom `.ai/` directory (default: `.ai`) Retained headings (`## Environment`, `## Module Index`, `## Session*`, and operational protocol headings) are kept in the vendor file verbatim rather than routed into `.ai/`. -Tidy reads each vendor file's mode from the `VENDORS` record and processes only pointers. Compiler output is skipped. A file carrying the `GENERATED by charter adf compile` banner is not a thin pointer — it is the full ruleset rendered *from* `.ai/` — so tidy reports it and leaves it alone rather than folding `.ai/` back into itself ([#296](https://github.com/Stackbilt-dev/charter/issues/296)). Edit the `.ai/` modules and re-run `charter adf compile --write` instead. The banner wins whatever the record says. A file recorded `compiled` without a banner is also skipped (status `compiled`); its warning names the content-preserving fix first (`adf vendor set pointer`, then `adf tidy --source `) and says how many items regenerating would discard, and an `unmanaged` file (recorded, or hand-authored with no Charter marker; it used to report `not-found`) gets status `unmanaged`. A file recorded `pointer` that has no pointer marker is skipped rather than guessed at. JSON rows add `vendorFile`, `mode`, `source` and `skipReason`. A `--source` path that is not a repo-root vendor name keeps the marker logic. `AGENTS.md` and `agents.md` that are one file on disk are tidied once. Tidy never writes the record. +Tidy reads each vendor file's mode from the `VENDORS` record and processes only pointers. Compiler output is skipped. A file carrying the `GENERATED by charter adf compile` banner is not a thin pointer — it is the full ruleset rendered *from* `.ai/` — so tidy reports it and leaves it alone rather than folding `.ai/` back into itself ([#296](https://github.com/Stackbilt-dev/charter/issues/296)). Edit the `.ai/` modules and re-run `charter adf compile --write` instead. The banner wins whatever the record says. A file recorded `compiled` without a banner is also skipped (status `compiled`); its warning names the content-preserving fix first (`adf vendor set pointer`, then `adf tidy --source `) and says how many items regenerating would discard, and an `unmanaged` file (recorded, or hand-authored with no Charter marker; it used to report `not-found`) gets status `unmanaged`. A file recorded `pointer` that has no pointer marker is skipped rather than guessed at. JSON rows add `vendorFile`, `mode`, `source` and `skipReason`, and the JSON result adds `paths` (`vendor`, `modules`), the files `--print-paths` lists. A `--source` path that is not a repo-root vendor name keeps the marker logic. `AGENTS.md` and `agents.md` that are one file on disk are tidied once. Tidy never writes the record. ### charter adf populate @@ -654,7 +668,7 @@ One physical file has one entry. When two vendor names are the same file (`AGENT `bootstrap` and `adf init --emit-pointers` record `pointer` for each pointer file they write. `adf migrate` records `pointer` for each source it replaces with a pointer, and `adf compile --write` records `compiled` for each target it writes (with `--mode compiled`, replacing the entry). The other writers only add missing entries. If a file is already recorded with another mode, the entry is kept and a warning names `adf vendor set` as the way to change it. Nothing is recorded under `--dry-run`, and a failed record write is only a warning, except under `adf compile --write --mode compiled`, where the target is refused (`refused-record`) and its file is not written. Every manifest edit is text-level and keeps line endings and every other byte, and the block it writes is already in `adf fmt` canonical form. It is re-parsed before writing, and if the result would change anything else, nothing is written. -`adf compile` (`--check` and `--write`), `adf tidy`, `doctor`, `score`, `bootstrap` and `adf migrate` act on the record; see their sections. The pre-commit hook still decides from the markers in each file; a later change under [#318](https://github.com/Stackbilt-dev/charter/issues/318) makes it read the record. `score` skips the section when it checks manifest path references. +`adf compile` (`--check` and `--write`), `adf tidy`, `doctor`, `score`, `bootstrap` and `adf migrate` act on the record; see their sections. The pre-commit hook passes each staged vendor file to `adf tidy --source`, so it acts on the record too. Its compile-banner check only saves the CLI call on commits that stage compiled output alone. `score` skips the section when it checks manifest path references. ### charter blast diff --git a/packages/cli/README.md b/packages/cli/README.md index a6a4d289..e50d5362 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -299,7 +299,7 @@ charter hook install --pre-commit ``` - `--commit-msg`: Install a commit-msg hook that normalizes `Governed-By` and `Resolves-Request` trailers using `git interpret-trailers`, and rejects a message carrying neither whenever `charter validate --ci` would fail it. The hook reads `.charter/config.json` with the same defaults as `charter validate`: `git.requireTrailers` is on unless set to a falsy value, `validation.citationStrictness` defaults to `FAIL`, and an unparseable file falls back to those defaults (the hook says so on stderr). It blocks under `FAIL` or `STRICT`, or with `ci.failOnWarn: true`, and only warns under advisory strictness. Two deliberate exceptions skip the gate with a stderr warning: no `.charter/config.json` at all, since the hook cannot tell the repo adopted Charter, and no `node` on `PATH`, since the config cannot be read. The hook strips `#` comment lines before checking, as `git commit` does with its default editor cleanup; with `-m` or `--cleanup=verbatim` git keeps them, so a trailer followed by a `#...` line can pass locally and still fail CI. -- `--pre-commit`: Install a pre-commit hook that enforces ADF routing with `charter doctor --adf-only --ci` and validates ceilings with `charter adf evidence --auto-measure --ci` (or `pnpm run verify:adf` when that script exists). Only gates when `.ai/manifest.adf` exists — no-op otherwise. +- `--pre-commit`: Install a pre-commit hook that tidies the staged vendor files, enforces ADF routing with `charter doctor --adf-only --ci`, and validates ceilings with `charter adf evidence --auto-measure --ci` (or `pnpm run verify:adf` when that script exists). Only gates when `.ai/manifest.adf` exists — no-op otherwise. Tidy runs on each staged vendor file (`adf tidy --source`) and re-stages only the files it wrote. It skips, with a message, a file that also has unstaged changes, and skips the whole commit when it would write into a `.ai/` module with unstaged or untracked work. Skip it with `CHARTER_SKIP_TIDY=1`. Hooks installed before #320 re-stage all of `.ai/*.adf`; re-run `charter hook install --pre-commit` to upgrade (`charter doctor` flags an old hook as INFO). - `--force` (or global `--yes`) allows overwrite when a non-Charter hook already exists. - Both flags can be passed together to install both hooks in one command. diff --git a/packages/cli/src/__tests__/adf-tidy-sources.test.ts b/packages/cli/src/__tests__/adf-tidy-sources.test.ts new file mode 100644 index 00000000..69894607 --- /dev/null +++ b/packages/cli/src/__tests__/adf-tidy-sources.test.ts @@ -0,0 +1,266 @@ +/** + * `adf tidy --source` is repeatable and `--print-paths` reports what tidy wrote + * (#320, PR 4 of the #318 sequence). The pre-commit hook relies on both. + * + * Pins INV-P4-5 (a single --source, and no --source, behave exactly as on main + * @ 01d2834, against output captured from that commit) and INV-P4-6 (the JSON + * change is the additive `paths` field only). + */ + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { COMPILE_BANNER_MARKER } from '@stackbilt/adf'; +import type { CLIOptions } from '../index'; +import { adfCommand, POINTER_AGENTS_MD, POINTER_CLAUDE_MD, POINTER_GEMINI_MD } from '../commands/adf'; + +// Identity is mocked per test so case-insensitive de-dup is tested without +// relying on the host filesystem's case sensitivity. +const FAKE_IDENTITY = vi.hoisted((): Record => ({})); + +vi.mock('../commands/adf-migrate', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fileIdentity: (fullPath: string) => FAKE_IDENTITY[path.basename(fullPath)] ?? actual.fileIdentity(fullPath), + }; +}); + +const TEXT: CLIOptions = { configPath: '.charter', format: 'text', ciMode: false, yes: false }; +const JSON_OPTS: CLIOptions = { ...TEXT, format: 'json' }; + +// The scenario the golden fixture was captured from (main @ 01d2834). +const BASE_MANIFEST = ['ADF: 0.1', '', '\u{1F3AF} ROLE: Test project', '', '\u{1F4E6} DEFAULT_LOAD:', ' - core.adf', ' - state.adf', '', '\u{1F4C2} ON_DEMAND:', ' - frontend.adf (Triggers on: React, CSS)', ''].join('\n'); +const CORE_ADF = 'ADF: 0.1\n\n\u{1F4CB} CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n'; +const BLOATED_CLAUDE = `${POINTER_CLAUDE_MD}\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n`; +const BLOATED_GEMINI = `${POINTER_GEMINI_MD}\n## Testing\n\n- Always run pnpm test before pushing to main.\n`; + +interface GoldenRun { code: number; out: string; err: string; files: Record } +const GOLDEN = JSON.parse(fs.readFileSync(path.join(__dirname, 'fixtures', 'tidy-single-source.main-01d2834.json'), 'utf-8')) as Record; + +const originalCwd = process.cwd(); +let tmp = ''; +let out: string[] = []; +let err: string[] = []; + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'charter-tidy-sources-')); + process.chdir(tmp); + out = []; + err = []; + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { out.push(a.map(String).join(' ')); }); + vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { err.push(a.map(String).join(' ')); }); + vi.spyOn(console, 'warn').mockImplementation((...a: unknown[]) => { err.push(a.map(String).join(' ')); }); + for (const key of Object.keys(FAKE_IDENTITY)) delete FAKE_IDENTITY[key]; +}); + +afterEach(() => { + vi.restoreAllMocks(); + process.chdir(originalCwd); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +function scaffold(): void { + fs.mkdirSync('.ai', { recursive: true }); + fs.writeFileSync('.ai/manifest.adf', BASE_MANIFEST); + fs.writeFileSync('.ai/core.adf', CORE_ADF); + fs.writeFileSync('.ai/state.adf', 'ADF: 0.1\n\n\u{1F9E0} STATE:\n CURRENT: Active development\n'); + fs.writeFileSync('CLAUDE.md', BLOATED_CLAUDE); +} + +function snapshot(): Record { + const files: Record = {}; + for (const f of ['CLAUDE.md', ...fs.readdirSync('.ai').sort().map(x => `.ai/${x}`)]) files[f] = fs.readFileSync(f, 'utf-8'); + return files; +} + +async function run(args: string[], options: CLIOptions = TEXT): Promise<{ code: number; out: string; err: string }> { + out = []; + err = []; + const code = await adfCommand(options, args); + return { code, out: out.join('\n'), err: err.join('\n') }; +} + +interface TidyJson { + files: Array<{ file: string; status: string }>; + totalExtracted: number; + modulesModified: string[]; + paths: { vendor: string[]; modules: string[] }; +} + +async function runJson(args: string[]): Promise<{ code: number; json: TidyJson; raw: string }> { + const r = await run([...args, '--format', 'json'], JSON_OPTS); + return { code: r.code, json: JSON.parse(r.out) as TidyJson, raw: r.out }; +} + +/** Main's JSON for the same run: the new output with the additive `paths` key removed. */ +function withoutPaths(raw: string): string { + const parsed = JSON.parse(raw) as Record; + expect(Object.keys(parsed).pop()).toBe('paths'); + delete parsed.paths; + return JSON.stringify(parsed, null, 2); +} + +describe('INV-P4-5: a single --source, and no --source, match main byte for byte', () => { + it.each([ + ['dryText', ['tidy', '--dry-run', '--source', 'CLAUDE.md'], TEXT], + ['realText', ['tidy', '--source', 'CLAUDE.md'], TEXT], + ['missingText', ['tidy', '--source', 'GEMINI.md'], TEXT], + ['wholeText', ['tidy', '--dry-run'], TEXT], + ] as const)('%s', async (label, args, options) => { + scaffold(); + const r = await run([...args], options); + const golden = GOLDEN[label]; + expect(r.code).toBe(golden.code); + expect(r.out).toBe(golden.out); + expect(r.err).toBe(golden.err); + expect(snapshot()).toEqual(golden.files); + }); + + it.each([ + ['dryJson', ['tidy', '--dry-run', '--source', 'CLAUDE.md']], + ['realJson', ['tidy', '--source', 'CLAUDE.md']], + ['missingJson', ['tidy', '--source', 'GEMINI.md']], + ] as const)('%s (INV-P4-6: only `paths` is added)', async (label, args) => { + scaffold(); + const r = await run([...args, '--format', 'json'], JSON_OPTS); + const golden = GOLDEN[label]; + expect(r.code).toBe(golden.code); + expect(withoutPaths(r.out)).toBe(golden.out); + expect(snapshot()).toEqual(golden.files); + }); +}); + +describe('--source is repeatable', () => { + it('processes every value in argument order, one result per file', async () => { + scaffold(); + fs.writeFileSync('GEMINI.md', BLOATED_GEMINI); + const r = await runJson(['tidy', '--dry-run', '--source', 'GEMINI.md', '--source', 'CLAUDE.md']); + expect(r.json.files.map(f => [f.file, f.status])).toEqual([['GEMINI.md', 'tidied'], ['CLAUDE.md', 'tidied']]); + expect(r.json.paths.vendor).toEqual(['GEMINI.md', 'CLAUDE.md']); + }); + + it('leaves an unnamed vendor file alone (INV-P4-2 at the CLI level)', async () => { + scaffold(); + fs.writeFileSync('GEMINI.md', BLOATED_GEMINI); + await run(['tidy', '--source', 'CLAUDE.md']); + expect(fs.readFileSync('GEMINI.md', 'utf-8')).toBe(BLOATED_GEMINI); + expect(fs.readFileSync('CLAUDE.md', 'utf-8')).toBe(POINTER_CLAUDE_MD); + }); + + it('drops a second name for the same file when identities match (case-insensitive mount, mocked)', async () => { + scaffold(); + // Two real files, so the result does not depend on the host's case folding; + // the mocked identity says they are one file, as on a case-insensitive mount. + fs.mkdirSync('upper'); + fs.mkdirSync('lower'); + fs.writeFileSync('upper/AGENTS.md', `${POINTER_AGENTS_MD}\n## Testing\n\n- Always run pnpm test before pushing to main.\n`); + fs.writeFileSync('lower/agents.md', 'not read'); + FAKE_IDENTITY['AGENTS.md'] = '9:42'; + FAKE_IDENTITY['agents.md'] = '9:42'; + const r = await runJson(['tidy', '--dry-run', '--source', 'upper/AGENTS.md', '--source', 'lower/agents.md']); + expect(r.json.files.map(f => f.file)).toEqual(['upper/AGENTS.md']); + }); + + it('drops a hard link to a file already named', async () => { + scaffold(); + fs.linkSync('CLAUDE.md', 'CLAUDE-link.md'); + const r = await runJson(['tidy', '--dry-run', '--source', 'CLAUDE.md', '--source', 'CLAUDE-link.md', '--source', './CLAUDE.md']); + expect(r.json.files.map(f => f.file)).toEqual(['CLAUDE.md']); + }); + + it('keeps one result for a repeated missing path, reported not-found', async () => { + scaffold(); + const r = await runJson(['tidy', '--dry-run', '--source', 'GEMINI.md', '--source', 'GEMINI.md']); + expect(r.code).toBe(0); + expect(r.json.files.map(f => [f.file, f.status])).toEqual([['GEMINI.md', 'not-found']]); + }); + + it.each([ + ['as the last argument', ['tidy', '--source', 'CLAUDE.md', '--source']], + ['followed by another flag', ['tidy', '--source', '--dry-run']], + ])('a --source with no value is a usage error (exit 2) %s, and nothing is written', async (_label, args) => { + scaffold(); + const before = snapshot(); + await expect(run(args)).rejects.toMatchObject({ + name: 'CLIError', + exitCode: 2, + message: '--source needs a file path, e.g. --source CLAUDE.md. Repeat it to tidy several files.', + }); + expect(snapshot()).toEqual(before); + }); +}); + +describe('--print-paths', () => { + it('prints exactly the typed lines under --dry-run, matching JSON paths, and writes nothing', async () => { + scaffold(); + fs.writeFileSync('GEMINI.md', BLOATED_GEMINI); + const before = snapshot(); + const r = await run(['tidy', '--dry-run', '--print-paths', '--source', 'GEMINI.md', '--source', 'CLAUDE.md']); + expect(r.code).toBe(0); + expect(r.out).toBe(['vendor GEMINI.md', 'vendor CLAUDE.md', 'module .ai/qa.adf', 'module .ai/frontend.adf'].join('\n')); + expect(snapshot()).toEqual(before); + const json = await runJson(['tidy', '--dry-run', '--source', 'GEMINI.md', '--source', 'CLAUDE.md']); + expect(json.json.paths).toEqual({ vendor: ['GEMINI.md', 'CLAUDE.md'], modules: ['.ai/qa.adf', '.ai/frontend.adf'] }); + expect(json.json.modulesModified).toEqual(['qa.adf', 'frontend.adf']); + }); + + it('a real run prints the same lines for the files it wrote, and they are the files that changed', async () => { + scaffold(); + const before = snapshot(); + const r = await run(['tidy', '--print-paths', '--source', 'CLAUDE.md']); + expect(r.out).toBe(['vendor CLAUDE.md', 'module .ai/qa.adf', 'module .ai/frontend.adf'].join('\n')); + const after = snapshot(); + const changed = [...new Set([...Object.keys(before), ...Object.keys(after)])].filter(f => before[f] !== after[f]).sort(); + expect(changed).toEqual(['.ai/frontend.adf', '.ai/qa.adf', 'CLAUDE.md']); + }); + + it('prints nothing for a clean file, and sends skip warnings to stderr, not stdout', async () => { + scaffold(); + fs.writeFileSync('CLAUDE.md', POINTER_CLAUDE_MD); + fs.writeFileSync('AGENTS.md', `${COMPILE_BANNER_MARKER}\n# Compiled\n\n## Constraints\n\n- Keep API contracts stable\n`); + const r = await run(['tidy', '--dry-run', '--print-paths', '--source', 'CLAUDE.md', '--source', 'AGENTS.md']); + expect(r.code).toBe(0); + expect(r.out).toBe(''); + expect(r.err).toContain('AGENTS.md is `charter adf compile` output'); + }); + + it('reports a restore-only rewrite as a vendor line with no module line (CP-5)', async () => { + scaffold(); + // The only bloat item is already in core.adf, so nothing is extracted, but + // tidy still rewrites the pointer to drop it. + fs.writeFileSync('CLAUDE.md', `${POINTER_CLAUDE_MD}\n## Rules\n\n- No secrets in commits\n`); + const dry = await run(['tidy', '--dry-run', '--print-paths', '--source', 'CLAUDE.md']); + expect(dry.out).toBe('vendor CLAUDE.md'); + const real = await run(['tidy', '--print-paths', '--source', 'CLAUDE.md']); + expect(real.out).toBe('vendor CLAUDE.md'); + expect(fs.readFileSync('CLAUDE.md', 'utf-8')).toBe(POINTER_CLAUDE_MD); + }); + + it('prints repo-relative POSIX paths for a nested source and a non-default --ai-dir', async () => { + scaffold(); + fs.renameSync('.ai', 'ctx'); + fs.mkdirSync('docs'); + fs.writeFileSync('docs/CLAUDE.md', BLOATED_CLAUDE); + const r = await run(['tidy', '--dry-run', '--print-paths', '--ai-dir', 'ctx', '--source', path.join('docs', 'CLAUDE.md')]); + expect(r.out).toBe(['vendor docs/CLAUDE.md', 'module ctx/qa.adf', 'module ctx/frontend.adf'].join('\n')); + }); + + it('cannot be combined with --format json (exit 2)', async () => { + scaffold(); + await expect(run(['tidy', '--print-paths', '--source', 'CLAUDE.md', '--format', 'json'], JSON_OPTS)).rejects.toMatchObject({ + name: 'CLIError', + exitCode: 2, + message: '--print-paths prints plain paths; it cannot be combined with --format json.', + }); + expect(fs.readFileSync('CLAUDE.md', 'utf-8')).toBe(BLOATED_CLAUDE); + }); + + it('prints nothing at all when there are no vendor files', async () => { + fs.mkdirSync('.ai'); + fs.writeFileSync('.ai/manifest.adf', BASE_MANIFEST); + const r = await run(['tidy', '--print-paths']); + expect(r.out).toBe(''); + }); +}); diff --git a/packages/cli/src/__tests__/fixtures/tidy-single-source.main-01d2834.json b/packages/cli/src/__tests__/fixtures/tidy-single-source.main-01d2834.json new file mode 100644 index 00000000..0cde5370 --- /dev/null +++ b/packages/cli/src/__tests__/fixtures/tidy-single-source.main-01d2834.json @@ -0,0 +1,83 @@ +{ + "dryText": { + "code": 0, + "out": " Would tidy 1 vendor file(s):\n CLAUDE.md: 3 items → qa.adf (2), frontend.adf (1)\n\n Routing plan (run without --dry-run to apply, or adjust triggers first):\n \"Always run pnpm test before pushing to main.\"\n → qa.adf / CONSTRAINTS\n \"Never skip the integration suite in CI.\"\n → qa.adf / CONSTRAINTS\n \"Use React hooks and CSS modules for styling.\"\n → frontend.adf / CONSTRAINTS\n\n Routing trace:\n \"Always run pnpm test before pushing to main.\"\n → qa.adf / CONSTRAINTS [scores: frontend.adf=0]\n \"Never skip the integration suite in CI.\"\n → qa.adf / CONSTRAINTS [scores: frontend.adf=0]\n \"Use React hooks and CSS modules for styling.\"\n → frontend.adf / CONSTRAINTS [scores: frontend.adf=2]\n\n Run without --dry-run to apply.", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "dryJson": { + "code": 0, + "out": "{\n \"dryRun\": true,\n \"files\": [\n {\n \"file\": \"CLAUDE.md\",\n \"vendorFile\": \"CLAUDE.md\",\n \"mode\": \"pointer\",\n \"source\": \"inferred\",\n \"skipReason\": null,\n \"status\": \"tidied\",\n \"itemsExtracted\": 3,\n \"routing\": {\n \"qa.adf\": 2,\n \"frontend.adf\": 1\n }\n }\n ],\n \"totalExtracted\": 3,\n \"modulesModified\": [\n \"qa.adf\",\n \"frontend.adf\"\n ],\n \"moduleWarnings\": [],\n \"warnings\": [],\n \"itemRoutes\": [\n {\n \"item\": \"Always run pnpm test before pushing to main.\",\n \"targetModule\": \"qa.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"qa.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 0\n }\n },\n {\n \"item\": \"Never skip the integration suite in CI.\",\n \"targetModule\": \"qa.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"qa.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 0\n }\n },\n {\n \"item\": \"Use React hooks and CSS modules for styling.\",\n \"targetModule\": \"frontend.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"frontend.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 2\n }\n }\n ]\n}", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "realText": { + "code": 0, + "out": " Tidied 1 vendor file(s):\n CLAUDE.md: 3 items → qa.adf (2), frontend.adf (1)", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/frontend.adf": "ADF: 0.1\n\n⚠️ CONSTRAINTS [advisory]:\n - Use React hooks and CSS modules for styling.\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/qa.adf": "ADF: 0.1\n\n⚠️ CONSTRAINTS [advisory]:\n - Always run pnpm test before pushing to main.\n - Never skip the integration suite in CI.\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "realJson": { + "code": 0, + "out": "{\n \"dryRun\": false,\n \"files\": [\n {\n \"file\": \"CLAUDE.md\",\n \"vendorFile\": \"CLAUDE.md\",\n \"mode\": \"pointer\",\n \"source\": \"inferred\",\n \"skipReason\": null,\n \"status\": \"tidied\",\n \"itemsExtracted\": 3,\n \"routing\": {\n \"qa.adf\": 2,\n \"frontend.adf\": 1\n }\n }\n ],\n \"totalExtracted\": 3,\n \"modulesModified\": [\n \"qa.adf\",\n \"frontend.adf\"\n ],\n \"moduleWarnings\": [],\n \"warnings\": [],\n \"itemRoutes\": [\n {\n \"item\": \"Always run pnpm test before pushing to main.\",\n \"targetModule\": \"qa.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"qa.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 0\n }\n },\n {\n \"item\": \"Never skip the integration suite in CI.\",\n \"targetModule\": \"qa.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"qa.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 0\n }\n },\n {\n \"item\": \"Use React hooks and CSS modules for styling.\",\n \"targetModule\": \"frontend.adf\",\n \"targetSection\": \"CONSTRAINTS\",\n \"headingModule\": \"frontend.adf\",\n \"candidateScores\": {\n \"frontend.adf\": 2\n }\n }\n ]\n}", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/frontend.adf": "ADF: 0.1\n\n⚠️ CONSTRAINTS [advisory]:\n - Use React hooks and CSS modules for styling.\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/qa.adf": "ADF: 0.1\n\n⚠️ CONSTRAINTS [advisory]:\n - Always run pnpm test before pushing to main.\n - Never skip the integration suite in CI.\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "missingText": { + "code": 0, + "out": " All remaining vendor files are clean.", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "missingJson": { + "code": 0, + "out": "{\n \"dryRun\": false,\n \"files\": [\n {\n \"file\": \"GEMINI.md\",\n \"vendorFile\": \"GEMINI.md\",\n \"mode\": null,\n \"source\": null,\n \"skipReason\": null,\n \"status\": \"not-found\",\n \"itemsExtracted\": 0,\n \"routing\": {}\n }\n ],\n \"totalExtracted\": 0,\n \"modulesModified\": [],\n \"moduleWarnings\": [],\n \"warnings\": [],\n \"itemRoutes\": []\n}", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + }, + "wholeText": { + "code": 0, + "out": " Would tidy 1 vendor file(s):\n CLAUDE.md: 3 items → qa.adf (2), frontend.adf (1)\n\n Routing plan (run without --dry-run to apply, or adjust triggers first):\n \"Always run pnpm test before pushing to main.\"\n → qa.adf / CONSTRAINTS\n \"Never skip the integration suite in CI.\"\n → qa.adf / CONSTRAINTS\n \"Use React hooks and CSS modules for styling.\"\n → frontend.adf / CONSTRAINTS\n\n Routing trace:\n \"Always run pnpm test before pushing to main.\"\n → qa.adf / CONSTRAINTS [scores: frontend.adf=0]\n \"Never skip the integration suite in CI.\"\n → qa.adf / CONSTRAINTS [scores: frontend.adf=0]\n \"Use React hooks and CSS modules for styling.\"\n → frontend.adf / CONSTRAINTS [scores: frontend.adf=2]\n\n Run without --dry-run to apply.", + "err": "", + "files": { + "CLAUDE.md": "# CLAUDE.md\n\n> **DO NOT add rules, constraints, or context to this file.**\n> This file is auto-managed by Charter. All project rules live in `.ai/`.\n> New rules should be added to the appropriate `.ai/*.adf` module.\n> See `.ai/manifest.adf` for the module routing manifest.\n\n## Session Start\n\nCall `charter_context` (MCP tool) before any other action. It returns the live constraint surface — open work, recent decisions, and metric ceilings — so you act within current project state rather than reconstructing it from scratch.\n\nIf `charter serve` is not running, read `.ai/context.snapshot.json` directly or run `charter context-refresh` to generate it.\n\n## Environment\n\n\n## Testing\n\n- Always run pnpm test before pushing to main.\n- Never skip the integration suite in CI.\n\n## Frontend\n\n- Use React hooks and CSS modules for styling.\n", + ".ai/core.adf": "ADF: 0.1\n\n📋 CONTEXT:\n - TypeScript monorepo\n\n⚠️ CONSTRAINTS [load-bearing]:\n - No secrets in commits\n", + ".ai/manifest.adf": "ADF: 0.1\n\n🎯 ROLE: Test project\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - frontend.adf (Triggers on: React, CSS)\n", + ".ai/state.adf": "ADF: 0.1\n\n🧠 STATE:\n CURRENT: Active development\n" + } + } +} \ No newline at end of file diff --git a/packages/cli/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 079fe2a9..0027d977 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -3,7 +3,15 @@ import { execFileSync, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { hookCommand, printClaudeHookConfig } from '../commands/hook'; +import { + HOOK_TIDY_BLOCK_TEMPLATE, + hookCommand, + PRE_COMMIT_HOOK_CONTENT, + PRE_COMMIT_HOOK_MARKER, + PRE_COMMIT_SCOPE_MARKER, + printClaudeHookConfig, + renderHookTidyBlock, +} from '../commands/hook'; import { doctorCommand } from '../commands/doctor'; import type { CLIOptions } from '../index'; @@ -196,25 +204,142 @@ describe('hookCommand', () => { }); }); - it('generates the same staged-vendor loop, compiled-file skip included, as the repo pre-commit hook (#320)', async () => { - vi.spyOn(console, 'log').mockImplementation(() => {}); + describe('pre-commit tidy block (#320)', () => { + const REPO_CLI = 'node packages/cli/dist/bin.js'; + const repoHook = (): string => fs.readFileSync(path.resolve(__dirname, '../../../../.githooks/pre-commit'), 'utf-8').replace(/\r\n/g, '\n'); + + /** From the block's first comment to the `fi` that closes it, inclusive. */ + function tidyBlock(script: string): string { + const start = script.indexOf('# Vendor file auto-tidy'); + const end = script.indexOf('\n# ADF evidence gate'); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + return script.slice(start, end); + } - const exitCode = await hookCommand(baseOptions, ['install', '--pre-commit']); - expect(exitCode).toBe(0); + async function generatedHook(): Promise { + vi.spyOn(console, 'log').mockImplementation(() => {}); + expect(await hookCommand(baseOptions, ['install', '--pre-commit'])).toBe(0); + return fs.readFileSync(path.join(tempDir, '.git', 'hooks', 'pre-commit'), 'utf-8'); + } - const generated = fs.readFileSync(path.join(tempDir, '.git', 'hooks', 'pre-commit'), 'utf-8'); - const repoHook = fs.readFileSync(path.resolve(__dirname, '../../../../.githooks/pre-commit'), 'utf-8'); - const vendorLoop = (script: string): string => { - const match = script.replace(/\r\n/g, '\n').match(/ VENDOR_FILES=[\s\S]*?\n done\n/); - expect(match).not.toBeNull(); - return match![0]; - }; + it('INV-P4-4: the generated and repo blocks are the template, differing only in {CLI}', async () => { + const generated = tidyBlock(await generatedHook()); + const repo = tidyBlock(repoHook()); + expect(generated).toBe(renderHookTidyBlock('npx charter')); + expect(repo).toBe(renderHookTidyBlock(REPO_CLI)); + expect(generated.split('npx charter').join('{CLI}')).toBe(repo.split(REPO_CLI).join('{CLI}')); + expect(generated.split('npx charter').join('{CLI}')).toBe(HOOK_TIDY_BLOCK_TEMPLATE); + }); + + it('pins the scope marker, the compiled fast path, --print-paths and staged-only re-staging', async () => { + const hook = await generatedHook(); + for (const block of [tidyBlock(hook), tidyBlock(repoHook())]) { + expect(block).toContain(PRE_COMMIT_SCOPE_MARKER); + expect(block).toContain("grep -q 'GENERATED by charter adf compile' \"$vf\""); + expect(block).toContain('adf tidy --dry-run --print-paths $SOURCES'); + expect(block).toContain('adf tidy --print-paths $SOURCES'); + expect(block).toContain('git diff --cached --name-only --diff-filter=d'); + expect(block).toContain('VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md"'); + // CP-1: only files tidy wrote are staged; the .ai/ sweep is gone for good. + expect(block).not.toMatch(/git add[^\n]*\.ai\/\*/); + expect(block.match(/git add/g)).toEqual(['git add']); + expect(block).toContain('git add -- "${line#*:}"'); + // #320 note: the whole-tree totalExtracted grep is gone deliberately. + expect(block).not.toContain('totalExtracted'); + } + expect(hook).toContain(PRE_COMMIT_HOOK_MARKER); + expect(hook).toBe(PRE_COMMIT_HOOK_CONTENT); + }); + + it('INV-P4-7: the tidy block uses no bash-only syntax, jq or node -e', async () => { + const block = tidyBlock(await generatedHook()); + expect(block).not.toMatch(/\[\[|\bjq\b|node -e|\blocal\b|\bdeclare\b|<<<|\$\(\(|\bfunction\b|==/); + expect(block).not.toMatch(/\w+=\(/); + }); + + it.each([ + ['sh', ['-n']], + ['dash', ['-n']], + ['bash', ['--posix', '-n']], + ])('INV-P4-7: %s parses the generated and repo hooks', async (shell, flags) => { + const probe = spawnSync(shell, ['-c', 'exit 0']); + if (probe.error || probe.status !== 0) return; // shell not installed here + await generatedHook(); + const files = [path.join(tempDir, '.git', 'hooks', 'pre-commit'), path.resolve(__dirname, '../../../../.githooks/pre-commit')]; + for (const file of files) { + const result = spawnSync(shell, [...flags, file], { encoding: 'utf-8' }); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + } + }); + }); + + describe('doctor: pre-commit hook row (#320)', () => { + const OLD_HOOK = `#!/usr/bin/env sh\n${PRE_COMMIT_HOOK_MARKER}\nset -eu\n\n# Vendor file auto-tidy: extract bloat from staged vendor config files\ngit add .ai/*.adf 2>/dev/null || true\n`; + + async function doctorChecks(): Promise> { + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => logs.push(args.map(String).join(' '))); + await doctorCommand({ ...baseOptions, format: 'json' }, []); + vi.restoreAllMocks(); + return (JSON.parse(logs.join('\n')) as { checks: Array<{ name: string; status: string; details: string }> }).checks; + } + + function writeHook(dir: string, content: string): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'pre-commit'), content); + } + + it('INV-P4-8: INFO, naming the hook and the fix, for a pre-#320 Charter hook', async () => { + writeHook(path.join(tempDir, '.git', 'hooks'), OLD_HOOK); + const rows = (await doctorChecks()).filter(c => c.name === 'pre-commit hook'); + expect(rows).toEqual([{ + name: 'pre-commit hook', + status: 'INFO', + details: 'The Charter pre-commit hook at .git/hooks/pre-commit predates staged-only tidy: it tidies every vendor file and re-stages all of .ai/*.adf, which can sweep unrelated edits into a commit. Re-run: charter hook install --pre-commit', + }]); + }); - // The compiled-file skip keeps STAGED_VENDORS empty for compiled-only commits, - // which is what stops tidy (and the .ai/*.adf re-stage) from running at all. - expect(generated).toContain("grep -q 'GENERATED by charter adf compile' \"$vf\""); - expect(generated).toContain('"totalExtracted": *[0-9][0-9]*'); - expect(vendorLoop(generated)).toBe(vendorLoop(repoHook)); + it('re-running hook install --pre-commit replaces the old hook without --force, and the row goes away', async () => { + writeHook(path.join(tempDir, '.git', 'hooks'), OLD_HOOK); + vi.spyOn(console, 'log').mockImplementation(() => {}); + expect(await hookCommand(baseOptions, ['install', '--pre-commit'])).toBe(0); + vi.restoreAllMocks(); + expect(fs.readFileSync(path.join(tempDir, '.git', 'hooks', 'pre-commit'), 'utf-8')).toBe(PRE_COMMIT_HOOK_CONTENT); + expect((await doctorChecks()).some(c => c.name === 'pre-commit hook')).toBe(false); + }); + + it.each([ + ['a current hook', PRE_COMMIT_HOOK_CONTENT], + ['a foreign hook', '#!/usr/bin/env sh\necho custom\ngit add .ai/*.adf\n'], + ['no hook', null], + ])('no row at all for %s', async (_label, content) => { + if (content !== null) writeHook(path.join(tempDir, '.git', 'hooks'), content); + const checks = await doctorChecks(); + expect(checks.some(c => c.name === 'pre-commit hook')).toBe(false); + }); + + it('honours core.hooksPath: reads the configured dir, not .git/hooks', async () => { + writeHook(path.join(tempDir, '.git', 'hooks'), OLD_HOOK); + writeHook(path.join(tempDir, 'custom hooks'), PRE_COMMIT_HOOK_CONTENT); + execFileSync('git', ['config', 'core.hooksPath', 'custom hooks'], { stdio: 'ignore' }); + expect((await doctorChecks()).some(c => c.name === 'pre-commit hook')).toBe(false); + + writeHook(path.join(tempDir, 'custom hooks'), OLD_HOOK); + const row = (await doctorChecks()).find(c => c.name === 'pre-commit hook'); + expect(row?.status).toBe('INFO'); + expect(row?.details).toContain('custom hooks/pre-commit'); + }); + + it('INV-P4-8: the row never makes doctor --ci fail', async () => { + writeHook(path.join(tempDir, '.git', 'hooks'), OLD_HOOK); + vi.spyOn(console, 'log').mockImplementation(() => {}); + const withOld = await doctorCommand({ ...baseOptions, ciMode: true, format: 'json' }, ['--adf-only']); + fs.rmSync(path.join(tempDir, '.git', 'hooks', 'pre-commit')); + const without = await doctorCommand({ ...baseOptions, ciMode: true, format: 'json' }, ['--adf-only']); + expect(withOld).toBe(without); + }); }); it('skips existing non-charter hook without --force', async () => { diff --git a/packages/cli/src/__tests__/integration/precommit-hook.test.ts b/packages/cli/src/__tests__/integration/precommit-hook.test.ts index 026d2101..12ecad70 100644 --- a/packages/cli/src/__tests__/integration/precommit-hook.test.ts +++ b/packages/cli/src/__tests__/integration/precommit-hook.test.ts @@ -1,19 +1,25 @@ /** - * Integration tests: real pre-commit hook simulation + * Integration tests: the real pre-commit hook in real git repos (#320). * - * v3 scenario — sets up actual git repos with hooks, stages bloated - * vendor files, runs git commit, and verifies the hook auto-tidies - * and re-stages corrected files. + * Each repo installs the hook `charter hook install --pre-commit` generates + * (PRE_COMMIT_HOOK_CONTENT), with `npx charter` replaced by the built CLI behind + * a logging shim, so these tests exercise the shipped text and can prove which + * CLI calls the hook made. Paths are passed to the shell double-quoted with + * forward slashes: Node accepts C:/... on Windows and Git's sh leaves them alone. + * Every repo and the shim live under a directory whose name contains a space. * - * These tests are slower (real git operations) but validate the full - * end-to-end contract that the pre-commit hook provides. + * The ADF evidence gate that follows the tidy block (doctor --adf-only --ci) + * is unchanged by #320 and fails on vendor bloat left in the working tree. Tests + * that deliberately leave bloat behind (a skipped or failed tidy) therefore + * install the tidy block alone (`tidyOnly`), and one test pins that interaction. */ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { execFileSync, execSync } from 'node:child_process'; +import { execFileSync, spawnSync } from 'node:child_process'; import { afterEach, describe, expect, it } from 'vitest'; +import { PRE_COMMIT_HOOK_CONTENT } from '../../commands/hook'; // ============================================================================ // Fixture Helpers @@ -21,35 +27,90 @@ import { afterEach, describe, expect, it } from 'vitest'; const tempDirs: string[] = []; const CLI_BIN = path.resolve(__dirname, '../../../dist/bin.js'); +const posix = (p: string): string => p.split(path.sep).join('/'); + +const GIT_ENV = { GIT_AUTHOR_NAME: 'Test', GIT_AUTHOR_EMAIL: 'test@test.com', GIT_COMMITTER_NAME: 'Test', GIT_COMMITTER_EMAIL: 'test@test.com' }; + +interface Repo { + dir: string; + /** One line per CLI call the hook made: its arguments, space-joined. */ + calls(): string[]; +} function makeTempDir(label: string): string { - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), `charter-integ-hook-${label}-`)); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), `charter integ hook-${label}-`)); tempDirs.push(tmp); return tmp; } function git(tmp: string, ...args: string[]): string { - return execFileSync('git', args, { - cwd: tmp, - encoding: 'utf-8', - env: { ...process.env, GIT_AUTHOR_NAME: 'Test', GIT_AUTHOR_EMAIL: 'test@test.com', GIT_COMMITTER_NAME: 'Test', GIT_COMMITTER_EMAIL: 'test@test.com' }, - }).trim(); + return execFileSync('git', args, { cwd: tmp, encoding: 'utf-8', env: { ...process.env, ...GIT_ENV } }).trim(); +} + +/** Run `git commit`; the hook's output arrives on stdout or stderr, so both are returned. */ +function commit(tmp: string, message: string, env: Record = {}): { status: number | null; output: string } { + const r = spawnSync('git', ['commit', '-m', message], { cwd: tmp, encoding: 'utf-8', env: { ...process.env, ...GIT_ENV, ...env } }); + return { status: r.status, output: `${r.stdout}\n${r.stderr}` }; +} + +function committedFiles(tmp: string): string[] { + return git(tmp, 'show', '--name-only', '--format=', 'HEAD').split('\n').filter(Boolean).sort(); } +function porcelain(tmp: string): string[] { + return execFileSync('git', ['status', '--porcelain', '--untracked-files=all'], { cwd: tmp, encoding: 'utf-8' }) + .split('\n').filter(Boolean).sort(); +} + +function show(tmp: string, spec: string): string { + return execFileSync('git', ['show', spec], { cwd: tmp, encoding: 'utf-8' }); +} + +const read = (tmp: string, file: string): string => fs.readFileSync(path.join(tmp, file), 'utf-8'); + +const POINTER = `# CLAUDE.md + +> **DO NOT add rules, constraints, or context to this file.** +> This file is auto-managed by Charter. All project rules live in \`.ai/\`. +> New rules should be added to the appropriate \`.ai/*.adf\` module. +> See \`.ai/manifest.adf\` for the module routing manifest. + +## Environment +- Node 20 +`; + +const DB_BLOAT = ` +## Database Rules +- All database migrations must be reversible +- Schema changes require migration scripts +`; + /** - * Scaffold a full git repo with ADF, a pre-commit hook that uses - * the built CLI, and an initial clean commit. + * Scaffold a git repo with ADF, the generated pre-commit hook (via a logging + * shim around the built CLI) under core.hooksPath, and an initial clean commit. */ -function writeHookFixtureRepo(tmp: string): void { - // Init git +function writeHookFixtureRepo(label: string, opts: { tidyOnly?: boolean } = {}): Repo { + const tmp = makeTempDir(label); + const shimDir = makeTempDir(`${label}-shim`); + const log = path.join(shimDir, 'calls.log'); + const shim = path.join(shimDir, 'charter shim.cjs'); + fs.writeFileSync(log, ''); + fs.writeFileSync(shim, [ + `const args = process.argv.slice(2);`, + `require('fs').appendFileSync(${JSON.stringify(log)}, args.join(' ') + '\\n');`, + `if (process.env.HOOK_TEST_FAIL_TIDY === '1' && args[0] === 'adf' && args[1] === 'tidy') process.exit(3);`, + `require(${JSON.stringify(CLI_BIN)});`, + '', + ].join('\n')); + git(tmp, 'init'); git(tmp, 'config', 'user.email', 'test@test.com'); git(tmp, 'config', 'user.name', 'Test'); + git(tmp, 'config', 'core.autocrlf', 'false'); - // ADF structure fs.mkdirSync(path.join(tmp, '.ai'), { recursive: true }); fs.mkdirSync(path.join(tmp, '.charter'), { recursive: true }); - + fs.writeFileSync(path.join(tmp, '.charter', '.gitignore'), 'telemetry/\n'); fs.writeFileSync(path.join(tmp, '.ai', 'manifest.adf'), `ADF: 0.1 📦 DEFAULT_LOAD: @@ -62,86 +123,35 @@ function writeHookFixtureRepo(tmp: string): void { 💰 BUDGET: MAX_TOKENS: 4000 `); - - fs.writeFileSync(path.join(tmp, '.ai', 'core.adf'), `ADF: 0.1 - -📐 RULES: - - All changes require tests -`); - + fs.writeFileSync(path.join(tmp, '.ai', 'core.adf'), 'ADF: 0.1\n\n📐 RULES:\n - All changes require tests\n'); fs.writeFileSync(path.join(tmp, '.ai', 'state.adf'), 'ADF: 0.1\n\n📋 STATE:\n - CURRENT: testing\n'); + fs.writeFileSync(path.join(tmp, '.ai', 'backend.adf'), 'ADF: 0.1\n\n📐 RULES:\n - Database access through repository pattern\n'); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER); + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), POINTER.replace('# CLAUDE.md', '# GEMINI.md')); - fs.writeFileSync(path.join(tmp, '.ai', 'backend.adf'), `ADF: 0.1 - -📐 RULES: - - Database access through repository pattern -`); - - // Clean thin pointer - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), `# CLAUDE.md - -> **DO NOT add rules, constraints, or context to this file.** -> This file is auto-managed by Charter. All project rules live in \`.ai/\`. -> New rules should be added to the appropriate \`.ai/*.adf\` module. -> See \`.ai/manifest.adf\` for the module routing manifest. - -## Environment -- Node 20 -`); - - // Pre-commit hook — simplified version that just runs tidy + // The generated hook, as `charter hook install --pre-commit` writes it. const hookDir = path.join(tmp, '.githooks'); fs.mkdirSync(hookDir, { recursive: true }); - - // Build hook script avoiding JS template ${} expansion issues - // by using string concatenation for the CLI_BIN path - const hookScript = [ - '#!/usr/bin/env bash', - 'set -euo pipefail', - '', - '# Vendor file auto-tidy', - 'if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then', - ' VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md GEMINI.md copilot-instructions.md"', - ' STAGED_VENDORS=""', - ' for vf in $VENDOR_FILES; do', - ' if git diff --cached --name-only | grep -qx "$vf"; then', - " if [ -f \"$vf\" ] && grep -q 'GENERATED by charter adf compile' \"$vf\"; then", - ' continue', - ' fi', - ' STAGED_VENDORS="$STAGED_VENDORS $vf"', - ' fi', - ' done', - '', - ' if [ -n "$STAGED_VENDORS" ]; then', - ` TIDY_OUTPUT=$(node ${CLI_BIN} adf tidy --dry-run --format json 2>/dev/null || echo '{}')`, - ' EXTRACTED=$(echo "$TIDY_OUTPUT" | grep -o \'"totalExtracted": *[0-9][0-9]*\' | grep -o \'[0-9][0-9]*\' || echo "0")', - '', - ' if [ "$EXTRACTED" -gt 0 ] 2>/dev/null; then', - ` node ${CLI_BIN} adf tidy 2>/dev/null`, - '', - ' for vf in $STAGED_VENDORS; do', - ' if [ -f "$vf" ]; then', - ' git add "$vf"', - ' fi', - ' done', - ' git add .ai/*.adf 2>/dev/null || true', - ' fi', - ' fi', - 'fi', - '', - ].join('\n'); - - fs.writeFileSync(path.join(hookDir, 'pre-commit'), hookScript); + const full = PRE_COMMIT_HOOK_CONTENT.split('npx charter').join(`node "${posix(shim)}"`); + const hook = opts.tidyOnly ? full.slice(0, full.indexOf('# ADF evidence gate')) : full; + expect(hook).toContain('# Charter pre-commit vendor tidy: scoped to staged files'); + fs.writeFileSync(path.join(hookDir, 'pre-commit'), hook); fs.chmodSync(path.join(hookDir, 'pre-commit'), 0o755); - - // Point git to custom hooks dir git(tmp, 'config', 'core.hooksPath', '.githooks'); - // Initial commit with clean state git(tmp, 'add', '-A'); git(tmp, 'commit', '-m', 'initial'); + fs.writeFileSync(log, ''); + + return { + dir: tmp, + calls: () => fs.readFileSync(log, 'utf-8').split('\n').filter(Boolean), + }; } +/** Only the tidy calls (the evidence gate after the block calls the CLI too). */ +const tidyCalls = (repo: Repo): string[] => repo.calls().filter(c => c.startsWith('adf tidy')); + // ============================================================================ // Lifecycle // ============================================================================ @@ -154,65 +164,176 @@ afterEach(() => { }); // ============================================================================ -// v3 Pre-commit Hook Simulation +// The generated pre-commit hook // ============================================================================ describe('pre-commit hook auto-tidy (integration)', () => { it('auto-tidies bloated CLAUDE.md during git commit', { timeout: 60000 }, () => { - const tmp = makeTempDir('hook-tidy'); - writeHookFixtureRepo(tmp); - - // Inject bloat into CLAUDE.md - const bloatedContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8') + ` + const repo = writeHookFixtureRepo('tidy'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + ` ## Architecture - Layered architecture with repository pattern - All database queries go through the repository layer - API endpoints validate input before processing +${DB_BLOAT}`); -## Database Rules -- All database migrations must be reversible -- Schema changes require migration scripts -`; - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloatedContent); + git(tmp, 'add', 'CLAUDE.md'); + const result = commit(tmp, 'add bloated content'); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Checking staged vendor files: CLAUDE.md'); + expect(result.output).toContain('re-staged only the files tidy wrote'); + // Real tidy ran, scoped to the staged file. + expect(tidyCalls(repo)).toEqual([ + 'adf tidy --dry-run --print-paths --source CLAUDE.md', + 'adf tidy --print-paths --source CLAUDE.md', + ]); + const committedClaude = show(tmp, 'HEAD:CLAUDE.md'); + expect(committedClaude).toContain('DO NOT add rules'); + expect(committedClaude).not.toContain('## Architecture'); + expect(committedClaude).not.toContain('## Database Rules'); + expect(committedClaude).toContain('## Environment'); + // Everything tidy wrote was committed; nothing is left behind. + expect(porcelain(tmp)).toEqual([]); + }); + + it('passes through cleanly when CLAUDE.md has no bloat', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('clean'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER.trimEnd() + '\n- pnpm 9\n'); + + git(tmp, 'add', 'CLAUDE.md'); + const result = commit(tmp, 'add env item'); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Vendor config files are clean.'); + expect(tidyCalls(repo)).toEqual(['adf tidy --dry-run --print-paths --source CLAUDE.md']); + expect(read(tmp, 'CLAUDE.md')).toContain('- pnpm 9'); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + }); + + it('re-stages modified .adf modules after tidy, and only those', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('restage'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT + '- API rate limiting applies to all database endpoints\n'); - // Stage and commit — hook should auto-tidy git(tmp, 'add', 'CLAUDE.md'); - const commitOutput = git(tmp, 'commit', '-m', 'add bloated content'); + expect(commit(tmp, 'auto-routed content').status).toBe(0); + + const committedClaude = show(tmp, 'HEAD:CLAUDE.md'); + expect(committedClaude).toContain('DO NOT add rules'); + expect(committedClaude).not.toContain('## Database Rules'); + const committed = committedFiles(tmp); + expect(committed).toContain('CLAUDE.md'); + const modules = committed.filter(f => f.startsWith('.ai/')); + expect(modules.length).toBeGreaterThan(0); + const routed = modules.map(m => show(tmp, `HEAD:${m}`)).join('\n'); + expect(routed).toContain('All database migrations must be reversible'); + expect(porcelain(tmp)).toEqual([]); + }); - // Verify commit succeeded - expect(commitOutput).toContain('add bloated content'); + it('INV-P4-1: unrelated .ai/ work in progress stays out of the commit', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('wip-unrelated'); + const tmp = repo.dir; + fs.appendFileSync(path.join(tmp, '.ai', 'state.adf'), ' - NEXT: half-written idea\n'); + fs.writeFileSync(path.join(tmp, '.ai', 'context.adf'), 'ADF: 0.1\n\n📋 STATE:\n - session output\n'); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); - // Verify CLAUDE.md was tidied (restored to thin pointer) - const finalContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8'); - expect(finalContent).toContain('DO NOT add rules'); - expect(finalContent).not.toContain('## Architecture'); - expect(finalContent).not.toContain('## Database Rules'); - expect(finalContent).toContain('## Environment'); + git(tmp, 'add', 'CLAUDE.md'); + expect(commit(tmp, 'tidy with unrelated wip').status).toBe(0); + + const committed = committedFiles(tmp); + expect(committed).not.toContain('.ai/state.adf'); + expect(committed).not.toContain('.ai/context.adf'); + expect(porcelain(tmp)).toEqual([' M .ai/state.adf', '?? .ai/context.adf']); + expect(read(tmp, '.ai/state.adf')).toContain('half-written idea'); }); - it('passes through cleanly when CLAUDE.md has no bloat', { timeout: 60000 }, () => { - const tmp = makeTempDir('hook-clean'); - writeHookFixtureRepo(tmp); + it('INV-P4-2: never tidies a vendor file that is not staged', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('unstaged-vendor', { tidyOnly: true }); + const tmp = repo.dir; + const geminiBloated = read(tmp, 'GEMINI.md') + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), geminiBloated); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); - // Make a trivial change to CLAUDE.md (add env item, not bloat) - const content = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8'); - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), content.trimEnd() + '\n- pnpm 9\n'); + git(tmp, 'add', 'CLAUDE.md'); + expect(commit(tmp, 'only claude staged').status).toBe(0); + expect(tidyCalls(repo).every(c => !c.includes('GEMINI.md'))).toBe(true); + expect(read(tmp, 'GEMINI.md')).toBe(geminiBloated); + expect(committedFiles(tmp)).not.toContain('GEMINI.md'); + expect(porcelain(tmp)).toEqual([' M GEMINI.md']); + }); + + it('INV-P4-1: skips a staged vendor file that also has unstaged changes (CP-3)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('partial', { tidyOnly: true }); + const tmp = repo.dir; + const staged = POINTER + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), staged); git(tmp, 'add', 'CLAUDE.md'); - const commitOutput = git(tmp, 'commit', '-m', 'add env item'); + const worktree = staged + '- not staged yet\n'; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), worktree); + + const result = commit(tmp, 'partially staged'); - expect(commitOutput).toContain('add env item'); + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes; not auto-tidying it.'); + expect(tidyCalls(repo)).toEqual([]); + expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(staged); + expect(read(tmp, 'CLAUDE.md')).toBe(worktree); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + }); - // CLAUDE.md should be unchanged (env addition preserved) - const finalContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8'); - expect(finalContent).toContain('DO NOT add rules'); + it.each([ + ['unstaged edits to a planned module', (tmp: string) => fs.appendFileSync(path.join(tmp, '.ai', 'backend.adf'), ' - wip rule\n'), ' M .ai/backend.adf'], + ['a planned module deleted but not staged', (tmp: string) => fs.rmSync(path.join(tmp, '.ai', 'backend.adf')), ' D .ai/backend.adf'], + ])('CP-4: %s blocks tidy for this commit, and the commit still succeeds', { timeout: 60000 }, (_label, makeWip, wipStatus) => { + const repo = writeHookFixtureRepo('wip-planned', { tidyOnly: true }); + const tmp = repo.dir; + const bloated = POINTER + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloated); + git(tmp, 'add', 'CLAUDE.md'); + // Learn the plan first, so the WIP lands in a module tidy really targets. + const plan = execFileSync('node', [CLI_BIN, 'adf', 'tidy', '--dry-run', '--print-paths', '--source', 'CLAUDE.md'], { cwd: tmp, encoding: 'utf-8' }); + expect(plan).toContain('module .ai/backend.adf'); + makeWip(tmp); + const before = fs.existsSync(path.join(tmp, '.ai', 'backend.adf')) ? read(tmp, '.ai/backend.adf') : null; + + const result = commit(tmp, 'planned module wip'); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Not auto-tidying: tidy would write into files with unstaged work: .ai/backend.adf'); + expect(tidyCalls(repo)).toEqual(['adf tidy --dry-run --print-paths --source CLAUDE.md']); + expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(bloated); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(fs.existsSync(path.join(tmp, '.ai', 'backend.adf')) ? read(tmp, '.ai/backend.adf') : null).toBe(before); + expect(porcelain(tmp)).toEqual([wipStatus]); }); - it('does not feed compiled vendor output back into ADF modules', { timeout: 60000 }, () => { - const tmp = makeTempDir('hook-compiled'); - writeHookFixtureRepo(tmp); + it('CP-4: an untracked file at a planned module path blocks tidy', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('wip-untracked', { tidyOnly: true }); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + '\n## Testing\n- Always run the test suite before pushing\n- Never skip flaky tests\n'); + git(tmp, 'add', 'CLAUDE.md'); + const plan = execFileSync('node', [CLI_BIN, 'adf', 'tidy', '--dry-run', '--print-paths', '--source', 'CLAUDE.md'], { cwd: tmp, encoding: 'utf-8' }); + const created = plan.split('\n').filter(l => l.startsWith('module ')).map(l => l.slice('module '.length)) + .find(p => !fs.existsSync(path.join(tmp, p))); + expect(created).toBeDefined(); + fs.writeFileSync(path.join(tmp, created!), 'ADF: 0.1\n\n📐 RULES:\n - my own untracked draft\n'); + + const result = commit(tmp, 'untracked planned module'); + + expect(result.status).toBe(0); + expect(result.output).toContain(`unstaged work: ${created}`); + expect(read(tmp, created!)).toContain('my own untracked draft'); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + }); + it('INV-P4-3: a commit staging only compiled vendor output makes no tidy call', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('compiled'); + const tmp = repo.dir; const compiled = ` # Compiled agent context @@ -221,82 +342,114 @@ describe('pre-commit hook auto-tidy (integration)', () => { - Keep API contracts stable `; fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), compiled); - const coreBefore = fs.readFileSync(path.join(tmp, '.ai/core.adf'), 'utf-8'); + const coreBefore = read(tmp, '.ai/core.adf'); git(tmp, 'add', 'CLAUDE.md'); - git(tmp, 'commit', '-m', 'commit compiled vendor output'); + expect(commit(tmp, 'commit compiled vendor output').status).toBe(0); - expect(fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8')).toBe(compiled); - expect(fs.readFileSync(path.join(tmp, '.ai/core.adf'), 'utf-8')).toBe(coreBefore); + expect(tidyCalls(repo)).toEqual([]); + expect(repo.calls().length).toBeGreaterThan(0); // the shim was wired: the evidence gate called it + expect(read(tmp, 'CLAUDE.md')).toBe(compiled); + expect(read(tmp, '.ai/core.adf')).toBe(coreBefore); }); - it('skips tidy when CHARTER_SKIP_TIDY=1 is set', { timeout: 60000 }, () => { - const tmp = makeTempDir('hook-skip'); - writeHookFixtureRepo(tmp); + it('INV-P4-3: a commit staging no vendor file makes no tidy call', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('no-vendor'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'notes.md'), 'hello\n'); + git(tmp, 'add', 'notes.md'); + expect(commit(tmp, 'no vendor').status).toBe(0); + expect(tidyCalls(repo)).toEqual([]); + }); - // Inject bloat - const bloatedContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8') + ` -## Architecture -- Layered architecture pattern -`; - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloatedContent); + it('tidies a staged lowercase agents.md (CP-2, #302)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('agents-lower'); + const tmp = repo.dir; + const pointer = execFileSync('node', ['-e', `process.stdout.write(require(${JSON.stringify(path.resolve(__dirname, '../../../dist/commands/adf.js'))}).POINTER_AGENTS_MD)`], { encoding: 'utf-8' }); + fs.writeFileSync(path.join(tmp, 'agents.md'), pointer + DB_BLOAT); + + git(tmp, 'add', 'agents.md'); + expect(commit(tmp, 'lowercase agents').status).toBe(0); + + expect(tidyCalls(repo)).toEqual([ + 'adf tidy --dry-run --print-paths --source agents.md', + 'adf tidy --print-paths --source agents.md', + ]); + expect(show(tmp, 'HEAD:agents.md')).toBe(pointer); + expect(porcelain(tmp)).toEqual([]); + }); + + it('stages a restore-only rewrite of a staged vendor file (CP-5)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('restore-only'); + const tmp = repo.dir; + // The only bloat is already in core.adf: no module is written, the pointer is. + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + '\n## Rules\n- All changes require tests\n'); - // Stage and commit with skip flag git(tmp, 'add', 'CLAUDE.md'); - execFileSync('git', ['commit', '-m', 'bloat preserved'], { - cwd: tmp, - encoding: 'utf-8', - env: { - ...process.env, - CHARTER_SKIP_TIDY: '1', - GIT_AUTHOR_NAME: 'Test', - GIT_AUTHOR_EMAIL: 'test@test.com', - GIT_COMMITTER_NAME: 'Test', - GIT_COMMITTER_EMAIL: 'test@test.com', - }, - }); - - // Bloat should still be there (tidy was skipped) - const finalContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8'); - expect(finalContent).toContain('## Architecture'); + expect(commit(tmp, 'restore only').status).toBe(0); + + expect(tidyCalls(repo)).toHaveLength(2); + expect(show(tmp, 'HEAD:CLAUDE.md')).not.toContain('## Rules'); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(porcelain(tmp)).toEqual([]); }); - it('re-stages modified .adf modules after tidy', { timeout: 60000 }, () => { - const tmp = makeTempDir('hook-restage'); - writeHookFixtureRepo(tmp); + it('H6: a tidy failure prints one line and the commit proceeds untouched', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('tidy-fails', { tidyOnly: true }); + const tmp = repo.dir; + const bloated = POINTER + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloated); - // Inject bloat with trigger keywords that route to backend.adf - const bloatedContent = fs.readFileSync(path.join(tmp, 'CLAUDE.md'), 'utf-8') + ` -## Database Rules -- All database migrations must be reversible -- Schema changes require migration scripts -- API rate limiting applies to all database endpoints -`; - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloatedContent); + git(tmp, 'add', 'CLAUDE.md'); + const result = commit(tmp, 'tidy fails', { HOOK_TEST_FAIL_TIDY: '1' }); - // Record pre-commit state of backend.adf - const beforeBackend = fs.readFileSync(path.join(tmp, '.ai', 'backend.adf'), 'utf-8'); + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] adf tidy --dry-run failed; skipping auto-tidy.'); + expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(bloated); + expect(porcelain(tmp)).toEqual([]); + }); + + it('skips tidy when CHARTER_SKIP_TIDY=1 is set', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('skip', { tidyOnly: true }); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + '\n## Architecture\n- Layered architecture pattern\n'); git(tmp, 'add', 'CLAUDE.md'); - git(tmp, 'commit', '-m', 'auto-routed content'); + expect(commit(tmp, 'bloat preserved', { CHARTER_SKIP_TIDY: '1' }).status).toBe(0); - // Verify the committed CLAUDE.md is clean - const committedClaude = execSync( - 'git show HEAD:CLAUDE.md', - { cwd: tmp, encoding: 'utf-8' }, - ); - expect(committedClaude).toContain('DO NOT add rules'); - expect(committedClaude).not.toContain('## Database Rules'); + expect(tidyCalls(repo)).toEqual([]); + expect(read(tmp, 'CLAUDE.md')).toContain('## Architecture'); + }); + + it('H6 boundary: the unchanged evidence gate still fails on bloat a skipped tidy left behind', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + fs.appendFileSync(path.join(tmp, 'CLAUDE.md'), '- not staged yet\n'); + + const result = commit(tmp, 'partial with full hook'); + + // The tidy block skipped the file without failing; the gate after it fails. + expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes; not auto-tidying it.'); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(result.status).toBe(1); + expect(tidyCalls(repo)).toEqual([]); + }); + + it('handles a staged file whose name contains a space alongside a vendor file', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('space'); + const tmp = repo.dir; + expect(tmp).toContain(' '); + fs.writeFileSync(path.join(tmp, 'release notes.md'), '## Notes\n- shipped\n'); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); - // Verify .adf modules were modified and committed - // (the tidy should have routed content somewhere) - const afterBackend = fs.readFileSync(path.join(tmp, '.ai', 'backend.adf'), 'utf-8'); - const coreAfter = fs.readFileSync(path.join(tmp, '.ai', 'core.adf'), 'utf-8'); + git(tmp, 'add', 'CLAUDE.md', 'release notes.md'); + expect(commit(tmp, 'space in name').status).toBe(0); - // At least one module should have grown - const anyModuleGrew = afterBackend.length > beforeBackend.length || - coreAfter.includes('migration') || coreAfter.includes('database') || - afterBackend.includes('migration') || afterBackend.includes('database'); - expect(anyModuleGrew).toBe(true); + expect(committedFiles(tmp)).toContain('release notes.md'); + expect(read(tmp, 'release notes.md')).toBe('## Notes\n- shipped\n'); + expect(show(tmp, 'HEAD:CLAUDE.md')).not.toContain('## Database Rules'); + expect(porcelain(tmp)).toEqual([]); }); }); diff --git a/packages/cli/src/commands/adf-tidy.ts b/packages/cli/src/commands/adf-tidy.ts index 43240320..82e0d399 100644 --- a/packages/cli/src/commands/adf-tidy.ts +++ b/packages/cli/src/commands/adf-tidy.ts @@ -21,10 +21,10 @@ import { } from '@stackbilt/adf'; import type { AdfDocument, PatchOperation, MigrationItem, TriggerMap } from '@stackbilt/adf'; import type { CLIOptions } from '../index'; -import { EXIT_CODE } from '../index'; -import { getFlag } from '../flags'; +import { CLIError, EXIT_CODE } from '../index'; +import { getFlag, getFlagValues } from '../flags'; import { POINTER_MARKERS, POINTER_CLAUDE_MD, POINTER_CURSORRULES, POINTER_AGENTS_MD, POINTER_GEMINI_MD, POINTER_COPILOT_MD } from './adf'; -import { discoverAgentConfigFiles } from './adf-migrate'; +import { discoverAgentConfigFiles, fileIdentity } from './adf-migrate'; import { isVendorFile, resolveVendorMode } from './adf-vendor'; import type { ModeSource, VendorFile, VendorMode } from './adf-vendor'; @@ -53,6 +53,8 @@ export const TIDY_MSG = { recordedCompiledHand: '{file} is recorded as compiled but has no compile banner — skipping. If it is hand-authored: charter adf vendor set {file} unmanaged. To regenerate it as compiled output instead (discards its hand-authored content): charter adf compile --target {target} --write --force', unmanaged: '{file} is unmanaged ({source}) — skipping. To move its rules into {aiDir}/: charter adf migrate --source {file}', pointerNoMarker: '{file} is recorded as a pointer but has no Charter pointer marker — skipping rather than guessing. See: charter adf vendor status', + sourceMissingValue: '--source needs a file path, e.g. --source CLAUDE.md. Repeat it to tidy several files.', + printPathsWithJson: '--print-paths prints plain paths; it cannot be combined with --format json.', } as const; function fill(template: string, values: Record): string { @@ -178,6 +180,36 @@ interface TidyResult { warnings: string[]; /** Populated when --verbose: per-item routing trace for each migrated item. */ itemRoutes?: ItemRoute[]; + /** The files tidy wrote (or would write under --dry-run), repo-relative POSIX (#320). Additive. */ + paths: TidyPaths; +} + +interface TidyPaths { + /** Vendor files with status 'tidied', in source order. */ + vendor: string[]; + /** `${aiDir}/${module}` for each modulesModified entry, in that order. */ + modules: string[]; +} + +/** A path relative to the working directory, with forward slashes. */ +function repoRelative(p: string): string { + return path.relative(process.cwd(), path.resolve(p)).split(path.sep).join('/'); +} + +/** + * The --source values with names for the same physical file dropped, keeping + * the first spelling: AGENTS.md and agents.md on a case-insensitive mount, or a + * hard link (#320). A missing path is keyed by its resolved path. + */ +function dedupeSources(sources: string[]): string[] { + const seen = new Set(); + return sources.filter(source => { + const full = path.resolve(source); + const key = fs.existsSync(full) ? `id:${fileIdentity(full)}` : `path:${full}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); } // ============================================================================ @@ -188,18 +220,21 @@ export async function adfTidyCommand(options: CLIOptions, args: string[]): Promi const dryRun = args.includes('--dry-run'); const ciMode = args.includes('--ci'); const verbose = args.includes('--verbose'); - const sourceFile = getFlag(args, '--source'); + const printPaths = args.includes('--print-paths'); + const sources = dedupeSources(getFlagValues(args, '--source', TIDY_MSG.sourceMissingValue)); + const named = sources.length > 0; const aiDir = getFlag(args, '--ai-dir') || '.ai'; + if (printPaths && options.format === 'json') throw new CLIError(TIDY_MSG.printPathsWithJson); // Determine which files to scan. discoverAgentConfigFiles reports one physical // file once, so AGENTS.md and agents.md on a case-insensitive mount are not // walked twice (#302). - const targets = sourceFile ? [sourceFile] : discoverAgentConfigFiles(); + const targets = named ? sources : discoverAgentConfigFiles(); if (targets.length === 0) { if (options.format === 'json') { - console.log(JSON.stringify({ dryRun, files: [], totalExtracted: 0, modulesModified: [], moduleWarnings: [], warnings: [] }, null, 2)); - } else { + console.log(JSON.stringify({ dryRun, files: [], totalExtracted: 0, modulesModified: [], moduleWarnings: [], warnings: [], paths: { vendor: [], modules: [] } }, null, 2)); + } else if (!printPaths) { console.log(' No vendor config files found.'); } return EXIT_CODE.SUCCESS; @@ -217,7 +252,7 @@ export async function adfTidyCommand(options: CLIOptions, args: string[]): Promi const content = fs.existsSync(fullPath) ? fs.readFileSync(fullPath, 'utf-8') : null; const gate = content === null ? { vendorFile: vendorNameOf(file), mode: null, source: null } - : tidyGate(file, content, aiDir, Boolean(sourceFile)); + : tidyGate(file, content, aiDir, named); const base = { file, vendorFile: gate.vendorFile, mode: gate.mode, source: gate.source, skipReason: null }; if (gate.skip) { @@ -309,11 +344,20 @@ export async function adfTidyCommand(options: CLIOptions, args: string[]): Promi } } - const result: TidyResult = { dryRun, files: fileResults, totalExtracted, modulesModified, moduleWarnings, warnings, itemRoutes }; + const paths: TidyPaths = { + vendor: fileResults.filter(f => f.status === 'tidied').map(f => repoRelative(f.file)), + modules: modulesModified.map(m => repoRelative(path.join(aiDir, m))), + }; + const result: TidyResult = { dryRun, files: fileResults, totalExtracted, modulesModified, moduleWarnings, warnings, itemRoutes, paths }; // Output if (options.format === 'json') { console.log(JSON.stringify(result, null, 2)); + } else if (printPaths) { + // stdout is exactly one typed line per file written, for the pre-commit hook. + for (const warning of warnings) console.error(` ⚠ ${warning}`); + for (const p of paths.vendor) console.log(`vendor ${p}`); + for (const p of paths.modules) console.log(`module ${p}`); } else { printTextResult(result); } diff --git a/packages/cli/src/commands/adf.ts b/packages/cli/src/commands/adf.ts index 80be31ac..b53d47fa 100644 --- a/packages/cli/src/commands/adf.ts +++ b/packages/cli/src/commands/adf.ts @@ -953,7 +953,7 @@ function printHelp(): void { console.log(' --no-backup: skip creating .pre-adf-migrate.bak backups'); console.log(' --merge-strategy: append (always add), dedupe (skip duplicates, default), replace'); console.log(''); - console.log(' charter adf tidy [--dry-run] [--source ] [--ai-dir ] [--ci]'); + console.log(' charter adf tidy [--dry-run] [--source ]... [--print-paths] [--ai-dir ] [--ci]'); console.log(' Scan vendor config files for content added beyond the thin pointer,'); console.log(' classify and route to ADF modules, restore thin pointer.'); console.log(' --dry-run: preview without modifying files'); diff --git a/packages/cli/src/commands/doctor-gate-enforcement.ts b/packages/cli/src/commands/doctor-gate-enforcement.ts index 7345b5ae..1f8f2e3e 100644 --- a/packages/cli/src/commands/doctor-gate-enforcement.ts +++ b/packages/cli/src/commands/doctor-gate-enforcement.ts @@ -1,6 +1,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { runGit } from '../git-helpers'; +import { PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER } from './hook'; interface GateEnforcementCheck { name: 'gate enforcement'; @@ -41,6 +42,32 @@ export function checkGateEnforcement(): GateEnforcementCheck { }; } +interface PreCommitScopeCheck { + name: 'pre-commit hook'; + status: 'INFO'; + details: string; +} + +export const DOCTOR_OLD_PRE_COMMIT_MSG = 'The Charter pre-commit hook at {hookPath} predates staged-only tidy: it tidies every vendor file and re-stages all of .ai/*.adf, which can sweep unrelated edits into a commit. Re-run: charter hook install --pre-commit'; + +/** + * INFO when the active pre-commit hook is a Charter-generated one from before + * #320 (it carries PRE_COMMIT_HOOK_MARKER but not PRE_COMMIT_SCOPE_MARKER). + * Undefined, so no row at all, for a current, foreign or missing hook. + * INFO, not WARN: doctor --ci fails on WARN, and an old hook over-stages but + * enforces nothing less, so warning would break CI on upgrade (#186). + */ +export function checkPreCommitScope(): PreCommitScopeCheck | undefined { + const hookPath = path.join(resolveHooksDir(), 'pre-commit'); + const content = readText(hookPath); + if (!content.includes(PRE_COMMIT_HOOK_MARKER) || content.includes(PRE_COMMIT_SCOPE_MARKER)) return undefined; + return { + name: 'pre-commit hook', + status: 'INFO', + details: DOCTOR_OLD_PRE_COMMIT_MSG.replace('{hookPath}', displayPath(hookPath)), + }; +} + function resolveHooksDir(): string { const repoRoot = getGitPath(['rev-parse', '--show-toplevel']) || process.cwd(); const configured = getGitPath(['config', '--get', 'core.hooksPath']); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index b8be9d85..fd1e46c2 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -18,7 +18,7 @@ import type { CompileTarget } from '@stackbilt/adf'; import { countExtractable } from './adf-tidy'; import { readVendorRecord, resolveVendorModes, vendorFileGroups } from './adf-vendor'; import type { ModeSource, VendorEntryProblem, VendorFile, VendorMode } from './adf-vendor'; -import { checkGateEnforcement } from './doctor-gate-enforcement'; +import { checkGateEnforcement, checkPreCommitScope } from './doctor-gate-enforcement'; import { findMissingDefaultLoad, formatMissingDefaultLoad, REQUIRED_DEFAULT_LOAD_MODULES } from './adf-wiring'; interface DoctorResult { @@ -403,6 +403,12 @@ export async function doctorCommand(options: CLIOptions, args: string[] = []): P } } + // An old Charter pre-commit hook over-stages .ai/ (#320). No row unless found. + if (inGitRepo) { + const preCommitCheck = checkPreCommitScope(); + if (preCommitCheck) checks.push(preCommitCheck); + } + // ADF readiness checks const aiDir = '.ai'; const manifestPath = path.join(aiDir, 'manifest.adf'); diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index f6c7991d..d440e6e5 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -119,51 +119,84 @@ if [ "$gate_mode" = "block" ] || [ "$gate_mode" = "warn" ]; then fi `; -const PRE_COMMIT_HOOK_MARKER = '# Managed by Charter: pre-commit evidence gate'; -const PRE_COMMIT_HOOK_CONTENT = `#!/usr/bin/env sh -${PRE_COMMIT_HOOK_MARKER} -set -eu +export const PRE_COMMIT_HOOK_MARKER = '# Managed by Charter: pre-commit evidence gate'; +// Second marker so doctor can tell a staged-only tidy hook from the pre-#320 one, +// which tidied every vendor file and re-staged all of .ai/*.adf. Both carry +// PRE_COMMIT_HOOK_MARKER, which keeps old installs recognised as managed. +export const PRE_COMMIT_SCOPE_MARKER = '# Charter pre-commit vendor tidy: scoped to staged files'; -# Vendor file auto-tidy: extract bloat from staged vendor config files +/** + * The vendor tidy block of the pre-commit hook (#320). {CLI} is `npx charter` + * here and `node packages/cli/dist/bin.js` in the repo's own .githooks/pre-commit; + * that is the only difference allowed between the two, and hook.test.ts checks it. + * POSIX sh: no arrays, no [[, no jq. Works under set -eu and set -euo pipefail: + * greps fed by a pipe read all their input (no -q), so under pipefail an early + * exit can never SIGPIPE the writer and turn a match into a miss. + */ +export const HOOK_TIDY_BLOCK_TEMPLATE = `# Vendor file auto-tidy (#320): tidy only the staged vendor files, then re-stage only the files tidy wrote. +${PRE_COMMIT_SCOPE_MARKER} # Skip with CHARTER_SKIP_TIDY=1 if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then - VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md GEMINI.md copilot-instructions.md" - STAGED_VENDORS="" + VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md" + STAGED=$(git diff --cached --name-only --diff-filter=d) + SOURCES="" + NAMES="" for vf in $VENDOR_FILES; do - if git diff --cached --name-only | grep -qx "$vf"; then - # Excluding compiled output here means a commit staging only compiled files - # skips tidy and the "git add .ai/*.adf" re-stage entirely. When tidy does run, - # it skips compiled files itself (#319). + if printf '%s\\n' "$STAGED" | grep -xF "$vf" >/dev/null; then + # Fast path: compiled output is never tidied (#319), so a commit staging only + # compiled files never invokes the CLI. if [ -f "$vf" ] && grep -q 'GENERATED by charter adf compile' "$vf"; then continue fi - STAGED_VENDORS="$STAGED_VENDORS $vf" + if ! git diff --quiet -- "$vf"; then + echo "[pre-commit] $vf has unstaged changes; not auto-tidying it. Stage or stash them, or run: charter adf tidy --source $vf" + continue + fi + SOURCES="$SOURCES --source $vf" + NAMES="$NAMES $vf" fi done - if [ -n "$STAGED_VENDORS" ]; then - echo "[pre-commit] Checking vendor config files:\$STAGED_VENDORS" - TIDY_OUTPUT=\$(npx charter adf tidy --dry-run --format json 2>/dev/null || echo '{}') - EXTRACTED=\$(echo "\$TIDY_OUTPUT" | grep -o '"totalExtracted": *[0-9][0-9]*' | grep -o '[0-9][0-9]*' || echo "0") - - if [ "\$EXTRACTED" -gt 0 ] 2>/dev/null; then - echo "[pre-commit] Found \$EXTRACTED items beyond thin pointer — auto-tidying..." - npx charter adf tidy 2>/dev/null - - for vf in \$STAGED_VENDORS; do - if [ -f "\$vf" ]; then - git add "\$vf" + if [ -n "$SOURCES" ]; then + echo "[pre-commit] Checking staged vendor files:$NAMES" + PLAN=$({CLI} adf tidy --dry-run --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; } + if printf '%s\\n' "$PLAN" | grep -E '^(vendor|module) ' >/dev/null; then + BLOCKED="" + for p in $(printf '%s\\n' "$PLAN" | sed -n 's/^module //p'); do + if ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then + BLOCKED="$BLOCKED $p" fi done - git add .ai/*.adf 2>/dev/null || true - - echo "[pre-commit] Vendor files tidied and re-staged." + if [ -n "$BLOCKED" ]; then + echo "[pre-commit] Not auto-tidying: tidy would write into files with unstaged work:$BLOCKED" + echo "[pre-commit] Stage or stash them and commit again, or run: charter adf tidy$SOURCES" + else + echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." + WROTE=$({CLI} adf tidy --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy failed; nothing re-staged."; WROTE=""; } + for line in $(printf '%s\\n' "$WROTE" | sed 's/ /:/'); do + if printf '%s\\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then + git add -- "\${line#*:}" + fi + done + echo "[pre-commit] Vendor files tidied; re-staged only the files tidy wrote." + fi else echo "[pre-commit] Vendor config files are clean." fi fi fi +`; + +/** The tidy block with {CLI} replaced by the command that runs Charter. */ +export function renderHookTidyBlock(cli: string): string { + return HOOK_TIDY_BLOCK_TEMPLATE.split('{CLI}').join(cli); +} + +export const PRE_COMMIT_HOOK_CONTENT = `#!/usr/bin/env sh +${PRE_COMMIT_HOOK_MARKER} +set -eu +${renderHookTidyBlock('npx charter')} # ADF evidence gate: check LOC ceilings if manifest exists if [ -f ".ai/manifest.adf" ]; then echo '[pre-commit] Running ADF evidence check...' @@ -409,9 +442,10 @@ function printHelp(): void { console.log(' Resolves-Request trailers using git interpret-trailers, and rejects a message with'); console.log(' neither whenever charter validate --ci would fail it (same .charter/config.json defaults).'); console.log(''); - console.log(' --pre-commit: Install a git pre-commit hook that auto-tidies vendor config files'); - console.log(' (CLAUDE.md, .cursorrules, AGENTS.md, etc.) and runs ADF evidence checks.'); - console.log(' Vendor file bloat is extracted, routed to .adf modules, and re-staged.'); + console.log(' --pre-commit: Install a git pre-commit hook that auto-tidies the staged vendor config'); + console.log(' files (CLAUDE.md, .cursorrules, AGENTS.md, etc.) and runs ADF evidence checks.'); + console.log(' Bloat is routed to .adf modules; only the files tidy wrote are re-staged. A vendor file'); + console.log(' with unstaged changes is skipped. Re-run this command to upgrade an older hook.'); console.log(' Skip tidy with CHARTER_SKIP_TIDY=1. Only gates when .ai/manifest.adf exists.'); console.log(''); console.log(' print --claude: Print the Claude Code session hook config snippet to stdout.'); diff --git a/packages/cli/src/flags.ts b/packages/cli/src/flags.ts index 4d8b8be6..013c033a 100644 --- a/packages/cli/src/flags.ts +++ b/packages/cli/src/flags.ts @@ -20,6 +20,22 @@ export function getFlag(args: string[], flag: string): string | undefined { return undefined; } +/** + * Every value of a repeatable flag, in argument order. An occurrence with no + * value (the last argument, or followed by another `--flag`) throws CLIError + * (exit 2) with `missingMessage`, rather than being silently dropped. + */ +export function getFlagValues(args: string[], flag: string, missingMessage: string): string[] { + const values: string[] = []; + args.forEach((arg, i) => { + if (arg !== flag) return; + const value = args[i + 1]; + if (value === undefined || value.startsWith('--')) throw new CLIError(missingMessage); + values.push(value); + }); + return values; +} + /** * Read a file referenced by a flag value, throwing CLIError if missing. */ From 560c3408de013c405e73d027a1e6787ed63785f6 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Fri, 16 Oct 2026 14:24:38 -0500 Subject: [PATCH 2/2] fix(hook): never abort on staging; explain skipped tidy and gate blocks (refs #320) Co-Authored-By: Claude Opus 5.5 --- .githooks/pre-commit | 44 ++++- CHANGELOG.md | 2 +- docs/cli-reference.md | 5 +- packages/cli/README.md | 2 +- packages/cli/src/__tests__/hook.test.ts | 6 +- .../integration/precommit-hook.test.ts | 152 ++++++++++++++++-- packages/cli/src/commands/adf.ts | 2 +- packages/cli/src/commands/hook.ts | 53 +++++- 8 files changed, 230 insertions(+), 36 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 85847da8..137dc41b 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -7,6 +7,9 @@ pnpm exec tsc --noEmit # Vendor file auto-tidy (#320): tidy only the staged vendor files, then re-stage only the files tidy wrote. # Charter pre-commit vendor tidy: scoped to staged files # Skip with CHARTER_SKIP_TIDY=1 +# This block never fails the commit; the ADF evidence gate after it may. +# TIDY_SKIPPED lists the vendor files it skipped, so the gate can say why it blocks. +TIDY_SKIPPED="" if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md" STAGED=$(git diff --cached --name-only --diff-filter=d) @@ -20,7 +23,9 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then continue fi if ! git diff --quiet -- "$vf"; then - echo "[pre-commit] $vf has unstaged changes; not auto-tidying it. Stage or stash them, or run: charter adf tidy --source $vf" + echo "[pre-commit] $vf has unstaged changes, so auto-tidy is skipped for it in this commit." + echo "[pre-commit] If it carries bloat, the ADF evidence gate may still block this commit. Ways out: stage or stash the unstaged changes; or run: charter adf tidy --source $vf, then git add the result; or bypass the hooks with git commit --no-verify." + TIDY_SKIPPED="$TIDY_SKIPPED $vf" continue fi SOURCES="$SOURCES --source $vf" @@ -33,23 +38,48 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then PLAN=$(node packages/cli/dist/bin.js adf tidy --dry-run --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; } if printf '%s\n' "$PLAN" | grep -E '^(vendor|module) ' >/dev/null; then BLOCKED="" + IGNORED="" + set -f for p in $(printf '%s\n' "$PLAN" | sed -n 's/^module //p'); do - if ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then + # A gitignored module (say, an .ai/ kept out of git) is never staged, so it is not WIP. + if git check-ignore -q -- "$p"; then + IGNORED="$IGNORED $p" + elif ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then BLOCKED="$BLOCKED $p" fi done + set +f if [ -n "$BLOCKED" ]; then - echo "[pre-commit] Not auto-tidying: tidy would write into files with unstaged work:$BLOCKED" - echo "[pre-commit] Stage or stash them and commit again, or run: charter adf tidy$SOURCES" + echo "[pre-commit] Auto-tidy skipped for this commit: tidy would write into files with unstaged or untracked work:$BLOCKED" + echo "[pre-commit] The ADF evidence gate may still block this commit while$NAMES carries bloat. Ways out: stage or stash that work; or run: charter adf tidy$SOURCES, then git add the result; or bypass the hooks with git commit --no-verify." + TIDY_SKIPPED="$TIDY_SKIPPED$NAMES" else echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." WROTE=$(node packages/cli/dist/bin.js adf tidy --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy failed; nothing re-staged."; WROTE=""; } + EXTRA="" + UNSTAGED="" + set -f for line in $(printf '%s\n' "$WROTE" | sed 's/ /:/'); do - if printf '%s\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then - git add -- "${line#*:}" + f="${line#*:}" + if ! printf '%s\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then + EXTRA="$EXTRA $f" + elif git check-ignore -q -- "$f"; then + : + elif ! git add -- "$f" 2>/dev/null; then + UNSTAGED="$UNSTAGED $f" fi done - echo "[pre-commit] Vendor files tidied; re-staged only the files tidy wrote." + set +f + if [ -n "$IGNORED" ]; then + echo "[pre-commit] Gitignored, so left as-is and not staged:$IGNORED" + fi + if [ -n "$EXTRA" ]; then + echo "[pre-commit] Tidy also wrote files its dry run did not plan; left unstaged, review and stage them yourself:$EXTRA" + fi + if [ -n "$UNSTAGED" ]; then + echo "[pre-commit] Could not stage, left in the working tree; stage them yourself:$UNSTAGED" + fi + echo "[pre-commit] Vendor files tidied; re-staged only the planned files tidy wrote." fi else echo "[pre-commit] Vendor config files are clean." diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f219ff3..d97e636a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. ### Changed -- **The pre-commit hook tidies only the staged vendor files and re-stages only the files tidy wrote** (`#320`, `#318` part 4) — re-run `charter hook install --pre-commit` to pick up staged-only tidy. An installed hook keeps its old behaviour until you do, and `charter doctor` reports it with an INFO `pre-commit hook` row (INFO, so `--ci` does not fail). Reinstalling overwrites a Charter-managed hook without `--force`, as before. The old hook ran a whole-tree `adf tidy` whenever any vendor file was staged, then ran `git add` on every staged vendor file and on all of `.ai/*.adf`. That swept unrelated `.ai/` edits into the commit and rewrote vendor files that were not part of it. The new hook passes each staged vendor file to `adf tidy` with `--source` and stages only the paths that both the dry run and the real run report. A staged vendor file that also has unstaged changes is skipped with a message, because tidy works on the working tree and staging its result would commit those hunks. If tidy would write into a module that has unstaged changes or is untracked, nothing is tidied for that commit and a message says why. A staged lowercase `agents.md` is now tidied (`#302`), and a commit staging only compiled output still makes no CLI call. The tidy block never fails a commit. The ADF evidence gate after it is unchanged, so in the generated hook vendor bloat that a skipped tidy leaves in the working tree still fails `charter doctor --adf-only --ci`. +- **The pre-commit hook tidies only the staged vendor files and re-stages only the files tidy wrote** (`#320`, `#318` part 4) — re-run `charter hook install --pre-commit` to pick up staged-only tidy. An installed hook keeps its old behaviour until you do, and `charter doctor` reports it with an INFO `pre-commit hook` row (INFO, so `--ci` does not fail). Reinstalling overwrites a Charter-managed hook without `--force`, as before. The old hook ran a whole-tree `adf tidy` whenever any vendor file was staged, then ran `git add` on every staged vendor file and on all of `.ai/*.adf`. That swept unrelated `.ai/` edits into the commit and rewrote vendor files that were not part of it. The new hook passes each staged vendor file to `adf tidy` with `--source` and stages only the paths that both the dry run and the real run report. A staged vendor file that also has unstaged changes is skipped with a message, because tidy works on the working tree and staging its result would commit those hunks. If tidy would write into a module that has unstaged changes or is untracked, nothing is tidied for that commit and a message says why. A gitignored module (for example an `.ai/` kept out of git) is not treated as work in progress: tidy writes it, and the hook says it was left unstaged. Any file the hook cannot stage, and any file the real run wrote that the dry run did not plan, is named and left unstaged rather than aborting the hook. A staged lowercase `agents.md` is now tidied (`#302`), and a commit staging only compiled output still makes no CLI call. The tidy block itself never fails a commit, but the ADF evidence gate after it can, and it is unchanged. In the generated hook (no `verify:adf` script), vendor bloat that a skipped tidy leaves in the working tree still fails `charter doctor --adf-only --ci`. The skip message now says so and names the ways out: stage or stash the unstaged work; run `charter adf tidy --source ` and `git add` the result; or `git commit --no-verify`. When the gate then fails, one line says which skipped file still carries how many items. A new hook with an older local CLI that does not know `--print-paths` reports the files as clean and tidies nothing, so upgrade the CLI too. - **BREAKING: `charter adf compile --write` no longer converts pointer files without `--mode compiled`** (`#318`, part 2) — compile used to overwrite any Charter thin pointer with compiled output, silently changing its mode. Each target's mode now comes from the `VENDORS` record, or from its markers when there is no entry, and `--write` only writes targets that are compiled or absent. Under `--target all`, pointer and unmanaged targets are skipped (exit 0), and each `[skip]` line names the command that converts it. A single such `--target` is refused (exit 1). `--force` alone never changes a mode. `--mode compiled` records `compiled` before writing. If that cannot be recorded, nothing is written (`refused-record`). If the write then fails, the record is restored and the run continues. A converted file is reported as `Written CLAUDE.md (mode: pointer -> compiled)`. JSON output adds `skipped` and per-target `targets`. A compiled-only repo with no `VENDORS` section compiles as before. - **`charter bootstrap --force` no longer overwrites hand-authored or compiled vendor files** (`#318`, part 3) — this changes what a documented flag does. `--force` still re-scaffolds `.ai/*.adf` (with backups in `.ai/.backup/`) and still rewrites Charter thin pointers from the current template. It now skips a `CLAUDE.md`, `AGENTS.md`, `.cursorrules`, `GEMINI.md` or `copilot-instructions.md` that has no Charter marker, carries the compile banner, or is recorded `compiled` or `unmanaged` in `VENDORS`, with a warning naming the next step (`adf migrate --source `, or `adf vendor set pointer && adf vendor restore ` for compiled output). With `--yes`, phase 4 then migrates a hand-authored file that has no `VENDORS` entry into `.ai/`, as it would without `--force`, and the phase 3 warning says so; a hand-authored file that already has an entry is left as it is. Phase 4 no longer migrates a file recorded `unmanaged`, and valid `VENDORS` entries are carried over to the re-scaffolded `manifest.adf`. To replace a hand-authored file without migrating it, delete it and re-run bootstrap. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 63b75992..02b19fc0 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -75,8 +75,9 @@ npx charter hook install --commit-msg --force # overwrite existing hooks - Tidy covers only staged vendor files (`CLAUDE.md`, `.cursorrules`, `AGENTS.md`, `agents.md`, `GEMINI.md`, `copilot-instructions.md`), each passed with `adf tidy --source`. A file carrying the compile banner is skipped without calling the CLI. - A staged vendor file that also has unstaged changes is skipped, with a message. - If tidy would write into a `.ai/` module that has unstaged changes or is untracked, nothing is tidied for that commit, with a message. - - Only the files tidy wrote are re-staged, never `.ai/*.adf` as a whole. A tidy failure prints one line and the commit proceeds. Skip tidy with `CHARTER_SKIP_TIDY=1`. - - The evidence gate after tidy still fails on vendor bloat left in the working tree. + - Only the files tidy wrote that its dry run planned are re-staged, never `.ai/*.adf` as a whole. A gitignored module is written but not staged, and the hook says so. A file the hook cannot stage, or one the real run wrote beyond the plan, is named and left unstaged. Skip tidy with `CHARTER_SKIP_TIDY=1`. + - The tidy block never fails the commit: a tidy failure prints one line and the hook carries on. + - The evidence gate after it can. In the generated hook it runs `charter doctor --adf-only --ci`, which fails on vendor bloat left in the working tree, for example in a file whose tidy was skipped. The skip message names the ways out: stage or stash the unstaged work; run `charter adf tidy --source ` and `git add` the result; or `git commit --no-verify`. If the gate then fails, one line names each skipped file that still carries items beyond its pointer. - A hook installed before [#320](https://github.com/Stackbilt-dev/charter/issues/320) re-stages all of `.ai/*.adf`. Re-run `charter hook install --pre-commit` to replace it (no `--force` needed for a Charter-managed hook). `charter doctor` reports an old hook as INFO. - `--force` — overwrite existing non-Charter hooks diff --git a/packages/cli/README.md b/packages/cli/README.md index e50d5362..9656fe5a 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -299,7 +299,7 @@ charter hook install --pre-commit ``` - `--commit-msg`: Install a commit-msg hook that normalizes `Governed-By` and `Resolves-Request` trailers using `git interpret-trailers`, and rejects a message carrying neither whenever `charter validate --ci` would fail it. The hook reads `.charter/config.json` with the same defaults as `charter validate`: `git.requireTrailers` is on unless set to a falsy value, `validation.citationStrictness` defaults to `FAIL`, and an unparseable file falls back to those defaults (the hook says so on stderr). It blocks under `FAIL` or `STRICT`, or with `ci.failOnWarn: true`, and only warns under advisory strictness. Two deliberate exceptions skip the gate with a stderr warning: no `.charter/config.json` at all, since the hook cannot tell the repo adopted Charter, and no `node` on `PATH`, since the config cannot be read. The hook strips `#` comment lines before checking, as `git commit` does with its default editor cleanup; with `-m` or `--cleanup=verbatim` git keeps them, so a trailer followed by a `#...` line can pass locally and still fail CI. -- `--pre-commit`: Install a pre-commit hook that tidies the staged vendor files, enforces ADF routing with `charter doctor --adf-only --ci`, and validates ceilings with `charter adf evidence --auto-measure --ci` (or `pnpm run verify:adf` when that script exists). Only gates when `.ai/manifest.adf` exists — no-op otherwise. Tidy runs on each staged vendor file (`adf tidy --source`) and re-stages only the files it wrote. It skips, with a message, a file that also has unstaged changes, and skips the whole commit when it would write into a `.ai/` module with unstaged or untracked work. Skip it with `CHARTER_SKIP_TIDY=1`. Hooks installed before #320 re-stage all of `.ai/*.adf`; re-run `charter hook install --pre-commit` to upgrade (`charter doctor` flags an old hook as INFO). +- `--pre-commit`: Install a pre-commit hook that tidies the staged vendor files, enforces ADF routing with `charter doctor --adf-only --ci`, and validates ceilings with `charter adf evidence --auto-measure --ci` (or `pnpm run verify:adf` when that script exists). Only gates when `.ai/manifest.adf` exists — no-op otherwise. Tidy runs on each staged vendor file (`adf tidy --source`) and re-stages only the files it wrote. It skips, with a message, a file that also has unstaged changes, and skips tidy for that commit when it would write into a `.ai/` module with unstaged or untracked work. Skip it with `CHARTER_SKIP_TIDY=1`. Hooks installed before #320 re-stage all of `.ai/*.adf`; re-run `charter hook install --pre-commit` to upgrade (`charter doctor` flags an old hook as INFO). - `--force` (or global `--yes`) allows overwrite when a non-Charter hook already exists. - Both flags can be passed together to install both hooks in one command. diff --git a/packages/cli/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 0027d977..0ebfcab9 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -243,8 +243,10 @@ describe('hookCommand', () => { expect(block).toContain('VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md"'); // CP-1: only files tidy wrote are staged; the .ai/ sweep is gone for good. expect(block).not.toMatch(/git add[^\n]*\.ai\/\*/); - expect(block.match(/git add/g)).toEqual(['git add']); - expect(block).toContain('git add -- "${line#*:}"'); + expect(block.match(/git add --/g)).toEqual(['git add --']); + expect(block).toContain('elif ! git add -- "$f" 2>/dev/null; then'); + expect(block).toContain('git check-ignore -q -- "$p"'); + expect(block).toContain('f="${line#*:}"'); // #320 note: the whole-tree totalExtracted grep is gone deliberately. expect(block).not.toContain('totalExtracted'); } diff --git a/packages/cli/src/__tests__/integration/precommit-hook.test.ts b/packages/cli/src/__tests__/integration/precommit-hook.test.ts index 12ecad70..2ac5890d 100644 --- a/packages/cli/src/__tests__/integration/precommit-hook.test.ts +++ b/packages/cli/src/__tests__/integration/precommit-hook.test.ts @@ -99,6 +99,13 @@ function writeHookFixtureRepo(label: string, opts: { tidyOnly?: boolean } = {}): `const args = process.argv.slice(2);`, `require('fs').appendFileSync(${JSON.stringify(log)}, args.join(' ') + '\\n');`, `if (process.env.HOOK_TEST_FAIL_TIDY === '1' && args[0] === 'adf' && args[1] === 'tidy') process.exit(3);`, + // Test hooks for paths tidy reports: a ghost path in both runs, or an extra write in the real run only. + `if (args[0] === 'adf' && args[1] === 'tidy' && args.includes('--print-paths')) process.on('exit', () => {`, + ` const fs = require('fs');`, + ` if (process.env.HOOK_TEST_GHOST) fs.writeSync(1, 'module ' + process.env.HOOK_TEST_GHOST + String.fromCharCode(10));`, + ` const extra = process.env.HOOK_TEST_EXTRA_WRITE;`, + ` if (extra && !args.includes('--dry-run')) { fs.writeFileSync(extra, 'ADF: 0.1' + String.fromCharCode(10)); fs.writeSync(1, 'module ' + extra + String.fromCharCode(10)); }`, + `});`, `require(${JSON.stringify(CLI_BIN)});`, '', ].join('\n')); @@ -184,7 +191,7 @@ ${DB_BLOAT}`); expect(result.status).toBe(0); expect(result.output).toContain('[pre-commit] Checking staged vendor files: CLAUDE.md'); - expect(result.output).toContain('re-staged only the files tidy wrote'); + expect(result.output).toContain('re-staged only the planned files tidy wrote'); // Real tidy ran, scoped to the staged file. expect(tidyCalls(repo)).toEqual([ 'adf tidy --dry-run --print-paths --source CLAUDE.md', @@ -279,7 +286,8 @@ ${DB_BLOAT}`); const result = commit(tmp, 'partially staged'); expect(result.status).toBe(0); - expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes; not auto-tidying it.'); + expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes, so auto-tidy is skipped for it in this commit.'); + expect(result.output).toContain('Ways out: stage or stash the unstaged changes; or run: charter adf tidy --source CLAUDE.md, then git add the result; or bypass the hooks with git commit --no-verify.'); expect(tidyCalls(repo)).toEqual([]); expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(staged); expect(read(tmp, 'CLAUDE.md')).toBe(worktree); @@ -289,7 +297,7 @@ ${DB_BLOAT}`); it.each([ ['unstaged edits to a planned module', (tmp: string) => fs.appendFileSync(path.join(tmp, '.ai', 'backend.adf'), ' - wip rule\n'), ' M .ai/backend.adf'], ['a planned module deleted but not staged', (tmp: string) => fs.rmSync(path.join(tmp, '.ai', 'backend.adf')), ' D .ai/backend.adf'], - ])('CP-4: %s blocks tidy for this commit, and the commit still succeeds', { timeout: 60000 }, (_label, makeWip, wipStatus) => { + ])('CP-4: %s skips tidy for this commit; the tidy block alone does not fail the commit', { timeout: 60000 }, (_label, makeWip, wipStatus) => { const repo = writeHookFixtureRepo('wip-planned', { tidyOnly: true }); const tmp = repo.dir; const bloated = POINTER + DB_BLOAT; @@ -304,7 +312,8 @@ ${DB_BLOAT}`); const result = commit(tmp, 'planned module wip'); expect(result.status).toBe(0); - expect(result.output).toContain('[pre-commit] Not auto-tidying: tidy would write into files with unstaged work: .ai/backend.adf'); + expect(result.output).toContain('[pre-commit] Auto-tidy skipped for this commit: tidy would write into files with unstaged or untracked work: .ai/backend.adf'); + expect(result.output).toContain('or run: charter adf tidy --source CLAUDE.md, then git add the result; or bypass the hooks with git commit --no-verify.'); expect(tidyCalls(repo)).toEqual(['adf tidy --dry-run --print-paths --source CLAUDE.md']); expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(bloated); expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); @@ -312,7 +321,7 @@ ${DB_BLOAT}`); expect(porcelain(tmp)).toEqual([wipStatus]); }); - it('CP-4: an untracked file at a planned module path blocks tidy', { timeout: 60000 }, () => { + it('CP-4: an untracked file at a planned module path skips tidy; the tidy block alone does not fail the commit', { timeout: 60000 }, () => { const repo = writeHookFixtureRepo('wip-untracked', { tidyOnly: true }); const tmp = repo.dir; fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + '\n## Testing\n- Always run the test suite before pushing\n- Never skip flaky tests\n'); @@ -326,7 +335,7 @@ ${DB_BLOAT}`); const result = commit(tmp, 'untracked planned module'); expect(result.status).toBe(0); - expect(result.output).toContain(`unstaged work: ${created}`); + expect(result.output).toContain(`unstaged or untracked work: ${created}`); expect(read(tmp, created!)).toContain('my own untracked draft'); expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); }); @@ -421,20 +430,133 @@ ${DB_BLOAT}`); expect(read(tmp, 'CLAUDE.md')).toContain('## Architecture'); }); - it('H6 boundary: the unchanged evidence gate still fails on bloat a skipped tidy left behind', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate'); + describe('full generated hook: the evidence gate after a skipped tidy (option A)', () => { + const LINK = /\[pre-commit\] Blocked because CLAUDE\.md still carries [1-9]\d* item\(s\) beyond its pointer: auto-tidy was skipped for it in this commit\. See the \[pre-commit\] lines above for the ways out\./; + + it('CP-3: a partially staged bloated file is skipped, and the gate blocks with a line linking the two', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-partial'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + fs.appendFileSync(path.join(tmp, 'CLAUDE.md'), '- not staged yet\n'); + + const result = commit(tmp, 'partial with full hook'); + + expect(result.status).toBe(1); + expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes, so auto-tidy is skipped for it in this commit.'); + expect(result.output).toContain('the ADF evidence gate may still block this commit'); + // doctor's own output is not suppressed, and the linking line follows it. + expect(result.output).toContain('adf vendor bloat'); + expect(result.output).toMatch(LINK); + expect(result.output.search(LINK)).toBeGreaterThan(result.output.indexOf('adf vendor bloat')); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + // Only the read-only count ran; nothing was tidied. + expect(tidyCalls(repo)).toEqual(['adf tidy --dry-run --format json --source CLAUDE.md']); + expect(porcelain(tmp)).toEqual(['MM CLAUDE.md']); + }); + + it('CP-4: planned-module WIP skips tidy, and the gate blocks with a line linking the two', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-wip'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + fs.appendFileSync(path.join(tmp, '.ai', 'backend.adf'), ' - wip rule\n'); + + const result = commit(tmp, 'wip with full hook'); + + expect(result.status).toBe(1); + expect(result.output).toContain('[pre-commit] Auto-tidy skipped for this commit: tidy would write into files with unstaged or untracked work: .ai/backend.adf'); + expect(result.output).toMatch(LINK); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(porcelain(tmp)).toEqual([' M .ai/backend.adf', 'M CLAUDE.md']); + }); + + it('prints no linking line when the gate fails for another reason', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-other'); + const tmp = repo.dir; + // GEMINI.md bloat is not staged and was not skipped by tidy; the gate fails on it alone. + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), read(tmp, 'GEMINI.md') + DB_BLOAT); + fs.writeFileSync(path.join(tmp, 'notes.md'), 'hello\n'); + git(tmp, 'add', 'notes.md'); + + const result = commit(tmp, 'other failure'); + + expect(result.status).toBe(1); + expect(result.output).not.toContain('[pre-commit] Blocked because'); + expect(tidyCalls(repo)).toEqual([]); + }); + }); + + it('a gitignored .ai/ (local-only setup) is not WIP: tidy runs, ignored modules are reported and never staged', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('ignored-ai'); const tmp = repo.dir; + git(tmp, 'rm', '-r', '-q', '--cached', '.ai'); + fs.appendFileSync(path.join(tmp, '.gitignore'), '.ai/\n'); + git(tmp, 'add', '.gitignore'); + git(tmp, 'commit', '-q', '--no-verify', '-m', 'local-only .ai'); fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); git(tmp, 'add', 'CLAUDE.md'); - fs.appendFileSync(path.join(tmp, 'CLAUDE.md'), '- not staged yet\n'); - const result = commit(tmp, 'partial with full hook'); + const result = commit(tmp, 'tidy with ignored .ai'); - // The tidy block skipped the file without failing; the gate after it fails. - expect(result.output).toContain('[pre-commit] CLAUDE.md has unstaged changes; not auto-tidying it.'); - expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); - expect(result.status).toBe(1); - expect(tidyCalls(repo)).toEqual([]); + expect(result.status).toBe(0); + expect(result.output).not.toContain('Auto-tidy skipped'); + expect(result.output).toContain('[pre-commit] Gitignored, so left as-is and not staged: .ai/backend.adf'); + expect(show(tmp, 'HEAD:CLAUDE.md')).not.toContain('## Database Rules'); + expect(read(tmp, '.ai/backend.adf')).toContain('All database migrations must be reversible'); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(porcelain(tmp)).toEqual([]); + }); + + it('a single ignored module that tidy creates is reported, not staged, and never aborts the hook', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('ignored-module'); + const tmp = repo.dir; + git(tmp, 'rm', '-q', '--cached', '.ai/backend.adf'); + fs.rmSync(path.join(tmp, '.ai', 'backend.adf')); + fs.appendFileSync(path.join(tmp, '.gitignore'), '.ai/backend.adf\n'); + git(tmp, 'add', '.gitignore'); + git(tmp, 'commit', '-q', '--no-verify', '-m', 'ignore backend.adf'); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + + const result = commit(tmp, 'tidy creates an ignored module'); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Gitignored, so left as-is and not staged: .ai/backend.adf'); + expect(result.output).not.toContain('paths are ignored'); + expect(fs.existsSync(path.join(tmp, '.ai', 'backend.adf'))).toBe(true); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(porcelain(tmp)).toEqual([]); + }); + + it('a git add failure is collected into one line and the tidy block carries on (H6)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('add-fails', { tidyOnly: true }); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + // Stand in for any git add failure: the shim adds a planned path that does not + // exist to both runs, so git add fails on it ("pathspec did not match"). + const result = commit(tmp, 'add fails', { HOOK_TEST_GHOST: '.ai/ghost.adf' }); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Could not stage, left in the working tree; stage them yourself: .ai/ghost.adf'); + expect(result.output).not.toContain('fatal:'); + expect(show(tmp, 'HEAD:CLAUDE.md')).not.toContain('## Database Rules'); + expect(result.output).toContain('[pre-commit] Vendor files tidied; re-staged only the planned files tidy wrote.'); + }); + + it('paths the real run wrote beyond the dry-run plan are named and left unstaged', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('diverge', { tidyOnly: true }); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + + const result = commit(tmp, 'diverging run', { HOOK_TEST_EXTRA_WRITE: '.ai/extra.adf' }); + + expect(result.status).toBe(0); + expect(result.output).toContain('[pre-commit] Tidy also wrote files its dry run did not plan; left unstaged, review and stage them yourself: .ai/extra.adf'); + expect(committedFiles(tmp)).not.toContain('.ai/extra.adf'); + expect(porcelain(tmp)).toEqual(['?? .ai/extra.adf']); }); it('handles a staged file whose name contains a space alongside a vendor file', { timeout: 60000 }, () => { diff --git a/packages/cli/src/commands/adf.ts b/packages/cli/src/commands/adf.ts index b53d47fa..9b61ca03 100644 --- a/packages/cli/src/commands/adf.ts +++ b/packages/cli/src/commands/adf.ts @@ -953,7 +953,7 @@ function printHelp(): void { console.log(' --no-backup: skip creating .pre-adf-migrate.bak backups'); console.log(' --merge-strategy: append (always add), dedupe (skip duplicates, default), replace'); console.log(''); - console.log(' charter adf tidy [--dry-run] [--source ]... [--print-paths] [--ai-dir ] [--ci]'); + console.log(' charter adf tidy [--dry-run] [--source ]... [--print-paths] [--ai-dir ] [--ci]'); console.log(' Scan vendor config files for content added beyond the thin pointer,'); console.log(' classify and route to ADF modules, restore thin pointer.'); console.log(' --dry-run: preview without modifying files'); diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index d440e6e5..2840ab5a 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -136,6 +136,9 @@ export const PRE_COMMIT_SCOPE_MARKER = '# Charter pre-commit vendor tidy: scoped export const HOOK_TIDY_BLOCK_TEMPLATE = `# Vendor file auto-tidy (#320): tidy only the staged vendor files, then re-stage only the files tidy wrote. ${PRE_COMMIT_SCOPE_MARKER} # Skip with CHARTER_SKIP_TIDY=1 +# This block never fails the commit; the ADF evidence gate after it may. +# TIDY_SKIPPED lists the vendor files it skipped, so the gate can say why it blocks. +TIDY_SKIPPED="" if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then VENDOR_FILES="CLAUDE.md .cursorrules AGENTS.md agents.md GEMINI.md copilot-instructions.md" STAGED=$(git diff --cached --name-only --diff-filter=d) @@ -149,7 +152,9 @@ if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then continue fi if ! git diff --quiet -- "$vf"; then - echo "[pre-commit] $vf has unstaged changes; not auto-tidying it. Stage or stash them, or run: charter adf tidy --source $vf" + echo "[pre-commit] $vf has unstaged changes, so auto-tidy is skipped for it in this commit." + echo "[pre-commit] If it carries bloat, the ADF evidence gate may still block this commit. Ways out: stage or stash the unstaged changes; or run: charter adf tidy --source $vf, then git add the result; or bypass the hooks with git commit --no-verify." + TIDY_SKIPPED="$TIDY_SKIPPED $vf" continue fi SOURCES="$SOURCES --source $vf" @@ -162,23 +167,48 @@ if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then PLAN=$({CLI} adf tidy --dry-run --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; } if printf '%s\\n' "$PLAN" | grep -E '^(vendor|module) ' >/dev/null; then BLOCKED="" + IGNORED="" + set -f for p in $(printf '%s\\n' "$PLAN" | sed -n 's/^module //p'); do - if ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then + # A gitignored module (say, an .ai/ kept out of git) is never staged, so it is not WIP. + if git check-ignore -q -- "$p"; then + IGNORED="$IGNORED $p" + elif ! git diff --quiet -- "$p" || { [ -e "$p" ] && ! git ls-files --error-unmatch -- "$p" >/dev/null 2>&1; }; then BLOCKED="$BLOCKED $p" fi done + set +f if [ -n "$BLOCKED" ]; then - echo "[pre-commit] Not auto-tidying: tidy would write into files with unstaged work:$BLOCKED" - echo "[pre-commit] Stage or stash them and commit again, or run: charter adf tidy$SOURCES" + echo "[pre-commit] Auto-tidy skipped for this commit: tidy would write into files with unstaged or untracked work:$BLOCKED" + echo "[pre-commit] The ADF evidence gate may still block this commit while$NAMES carries bloat. Ways out: stage or stash that work; or run: charter adf tidy$SOURCES, then git add the result; or bypass the hooks with git commit --no-verify." + TIDY_SKIPPED="$TIDY_SKIPPED$NAMES" else echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." WROTE=$({CLI} adf tidy --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy failed; nothing re-staged."; WROTE=""; } + EXTRA="" + UNSTAGED="" + set -f for line in $(printf '%s\\n' "$WROTE" | sed 's/ /:/'); do - if printf '%s\\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then - git add -- "\${line#*:}" + f="\${line#*:}" + if ! printf '%s\\n' "$PLAN" | sed 's/ /:/' | grep -xF "$line" >/dev/null; then + EXTRA="$EXTRA $f" + elif git check-ignore -q -- "$f"; then + : + elif ! git add -- "$f" 2>/dev/null; then + UNSTAGED="$UNSTAGED $f" fi done - echo "[pre-commit] Vendor files tidied; re-staged only the files tidy wrote." + set +f + if [ -n "$IGNORED" ]; then + echo "[pre-commit] Gitignored, so left as-is and not staged:$IGNORED" + fi + if [ -n "$EXTRA" ]; then + echo "[pre-commit] Tidy also wrote files its dry run did not plan; left unstaged, review and stage them yourself:$EXTRA" + fi + if [ -n "$UNSTAGED" ]; then + echo "[pre-commit] Could not stage, left in the working tree; stage them yourself:$UNSTAGED" + fi + echo "[pre-commit] Vendor files tidied; re-staged only the planned files tidy wrote." fi else echo "[pre-commit] Vendor config files are clean." @@ -210,6 +240,15 @@ if [ -f ".ai/manifest.adf" ]; then else npx charter doctor --adf-only --ci --format text && npx charter adf evidence --auto-measure --ci || { echo '' + # Link a bloat failure to the tidy skip that caused it (#320). + set -f + for f in $TIDY_SKIPPED; do + n=$(npx charter adf tidy --dry-run --format json --source "$f" 2>/dev/null | sed -n 's/^ "totalExtracted": \\([0-9][0-9]*\\),*$/\\1/p') || n="" + if [ -n "$n" ] && [ "$n" -gt 0 ]; then + echo "[pre-commit] Blocked because $f still carries $n item(s) beyond its pointer: auto-tidy was skipped for it in this commit. See the [pre-commit] lines above for the ways out." + fi + done + set +f echo '[pre-commit] ADF verification FAILED.' echo ' Run: npx charter doctor --adf-only --ci --format text' echo ' && npx charter adf evidence --auto-measure --ci'