diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 137dc41..676796a 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -8,7 +8,8 @@ pnpm exec tsc --noEmit # 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 lists the vendor files it skipped; the generated hook passes it to +# doctor --staged --tidy-skipped, so their bloat does not block the commit (#357). 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" @@ -24,7 +25,7 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then fi if ! git diff --quiet -- "$vf"; then 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." + echo "[pre-commit] Its bloat, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy it: stage or stash the unstaged changes and commit again; or run: charter adf tidy --source $vf, then git add the result." TIDY_SKIPPED="$TIDY_SKIPPED $vf" continue fi @@ -51,7 +52,7 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then set +f if [ -n "$BLOCKED" ]; then 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." + echo "[pre-commit] Bloat in$NAMES, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy: stage or stash that work and commit again; or run: charter adf tidy$SOURCES, then git add the result." TIDY_SKIPPED="$TIDY_SKIPPED$NAMES" else echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." diff --git a/CHANGELOG.md b/CHANGELOG.md index d6361aa..9afd340 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. - **Compiled files that carry your own retained sections change bytes on the next `adf compile --write`** (`#303`, `#318` part 5) — a compiled `CLAUDE.md`, `AGENTS.md`, `.cursorrules` or `GEMINI.md` with a hand-added `## Environment` (or another section `adf tidy` retains) no longer loses it. The next `--write` moves it into a `` … `` block at the end of the file, and every later `--write` copies that block as it stands. A compiled file with nothing to carry gets no block and compiles byte-identically to before. The block uses HTML markers in every target, `.cursorrules` included. All-caps headings the compiler could have produced (a module since removed from `.ai/`) and `## Module Index` are never carried. `adf compile` to stdout and `charter score` still use the plain render. - **`adf compile --check` reads such a file as stale until one `--write`** (`#303`) — the expected bytes are now the render plus the file's own retained block, so an edit inside the block is never stale. A compiled file whose retained sections sit outside the block reads `[stale]` with `staleReason: "retained-unfenced"`, and the remedy (`compile --target --write`) keeps them. Each `--check` JSON row adds `staleReason` (`content`, `retained-unfenced`, or null). - **`refused-retained` is retired from `adf compile --write`, and `retained-divergent` from `adf vendor restore`** (`#303`) — JSON consumers no longer see either value. A pointer converted with `--mode compiled` now carries the retained sections that differ from its template into the block, instead of refusing. It still refuses, exit 1 with nothing written and `VENDORS` unchanged, over content the output would not carry: `refused-bloated` (items `adf tidy` would extract; the first remedy is `adf tidy --source `, then `--force`, which discards them) and `refused-would-drop` (other lines of yours). `--mode compiled --force` over a hand-authored file carries its retained sections and discards the rest. `--force` never drops retained sections. Write JSON rows add `retainedSections`, `retainedSource`, `retainedSkipped` and `retainedAmbiguous`. `retainedSkipped` lists retained-looking sections whose heading is still in a `.ai/` module, so they are rendered `.ai/` text left behind by an edit; they are not carried, and each one is named on stderr. A section left inside a generated section that `.ai/` no longer renders, with no `.ai/` match, may be stale output or your own. `--write` refuses that target as `refused-ambiguous` (new value), with "wrap it in the retained block" as the first remedy and `--force` (which discards it) as the second. `--check` reports it with `staleReason: "retained-ambiguous"`. Parsing is code-fence aware, and marker lines match as loosely as `stripCharterSentinels`. `vendor restore` of a compiled file carries its retained sections into the pointer and lists them in `keptSections`, and the block is not counted as a `compiled-differs` difference. A block holding a non-retained heading, or text before its first heading, is refused as `would-drop` (new value) with `vendor set compiled` as the first remedy, because a pointer cannot keep it. -- **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. +- **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. The gate's handling of vendor bloat outside the commit is fixed by `#357` (see Fixed). 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. @@ -34,6 +34,7 @@ The format is based on Keep a Changelog and follows Semantic Versioning. ### Fixed +- **The pre-commit hook's evidence gate blocks on vendor bloat only in files the commit includes** (`#357`) — re-run `charter hook install --pre-commit` to pick this up. After staged-only tidy (`#320`), the generated hook (no `verify:adf` script) still ran `charter doctor --adf-only --ci` over the whole working tree, so a commit could be refused for bloat it did not contain: an unstaged `GEMINI.md`, a partially staged vendor file the hook skips, or a vendor file skipped because a module tidy would write has unstaged or untracked work. The hook skips such files on purpose, and blocking on them was a regression from `#320`; the old hook only passed these commits by rewriting or staging files you had not staged. The gate now runs `charter doctor --adf-only --ci --staged --tidy-skipped ""`. `--staged` limits the `adf vendor bloat` check to vendor files staged in the index; `--tidy-skipped` takes the staged files the tidy block skipped off that list. Bloat in the rest is reported in a new INFO row, `adf vendor bloat (not blocking)`, which names each file and why it does not block, and gives a per-file `adf tidy --source` command. A staged vendor file that tidy ran on and could not clean, or that was not tidied because tidy failed or `CHARTER_SKIP_TIDY=1` is set, still blocks. Every other doctor check still blocks as before, vendor-mode drift and a malformed `VENDORS` entry included. Without `--staged`, `charter doctor` is unchanged, so CI's `charter doctor --ci` still fails on all vendor bloat. The skip messages no longer claim the gate will block, and say that a plain `charter doctor --ci` still reports the file's bloat. The gate's failure output no longer has the "Blocked because … auto-tidy was skipped" line, so a failing gate no longer runs an extra `adf tidy --dry-run` for each skipped file. `--tidy-skipped` without `--staged` is a usage error (exit 2). An installed hook keeps the old gate until you reinstall, and `charter doctor` reports a `#320`-era hook with an INFO `pre-commit hook` row. With an older local CLI, the new hook's `--staged` and `--tidy-skipped` flags are ignored and the gate behaves as before. - **`adf compile --write` no longer deletes your `## Environment` section from a compiled file** (`#303`) — the `#295` guard only covered pointer files. Re-compiling a file that already carried the compile banner silently dropped a user-added `## Environment` (or protocol) section with exit 0, and git was the only way back. Those sections are now kept in the retained block. - **`charter bootstrap --force` no longer destroys a hand-authored `CLAUDE.md` before it can be migrated** (`#318`, part 3) — phase 3 overwrote every existing pointer-file name with the template under `--force`, before phase 4 (migrate) ran. Vendor files are not backed up (only `.ai/*.adf` go to `.ai/.backup/`), so phase 4 then found a pointer, migrated nothing, and the file's rules were gone with no `.pre-adf-migrate.bak`. The file is now skipped in phase 3, and with `--yes` phase 4 migrates it into `.ai/` and writes the backup (when it has no `VENDORS` entry). - **`charter adf compile --check` and `charter adf tidy` read each vendor file's mode** (`#318`, part 2; `#302`) — `--check` compared every target byte-for-byte against compiled output, so a thin pointer was reported `[stale]` forever. Each target is now checked by its mode. A compiled file is `fresh` or `stale` as before. A pointer is `pointer-ok` when `adf tidy` would extract nothing from it, and `pointer-bloated` (exit 1, remedy `charter adf tidy --source `) when it would. An unmanaged file is `skipped-unmanaged` (exit 0). A recorded `compiled` or `pointer` file that looks like another mode on disk is `mode-drift` (exit 1), with both fixes printed: `charter adf vendor set ` to keep the file, or the command that restores it. A missing file is `missing`, and a recorded pointer that is missing names `git checkout -- ` or `adf vendor set unmanaged`. No remedy suggests `compile --write` for a pointer or unmanaged file. JSON keeps `stale` (which now also lists bloated and drifted files), `current`, `missing`, `upToDate` and `nextActions`, and adds `skipped` and `targets`. `adf tidy` processes a file only when it is a pointer: no compile banner, at least one pointer marker, and not recorded otherwise. The banner still wins whatever the record says (`#296`). A file recorded `compiled` is skipped with status `compiled`. Its warning lists the content-preserving fix first (`adf vendor set pointer`, then `adf tidy --source `) and says how many items regenerating would discard; `--check` mode-drift does the same. An `unmanaged` file (recorded, or hand-authored with no marker) now has status `unmanaged` instead of `not-found`, and its warning appears when it is recorded or named with `--source`. A recorded pointer with no marker is skipped rather than guessed at. File rows gain `vendorFile`, `mode`, `source` and `skipReason`. A `--source` path that is not a repo-root vendor name keeps the old marker logic. Tidy now finds files through the shared discovery helper, so `AGENTS.md` and `agents.md` that are one file on a case-insensitive filesystem are walked once, not twice. With no `SYNC` entries, `charter adf sync` now points to `charter adf vendor status` in text and in JSON `nextActions`, rather than ending at "Wrote empty lock file". Record entry problems are still reported only by `adf vendor status`. `doctor`, `score`, `bootstrap` and both pre-commit hooks are unchanged and are later parts of `#318`. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 8300f24..0057de9 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -77,8 +77,8 @@ npx charter hook install --commit-msg --force # overwrite existing hooks - 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 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. + - The evidence gate after it can. In the generated hook it runs `charter doctor --adf-only --ci --staged --tidy-skipped ""` ([#357](https://github.com/Stackbilt-dev/charter/issues/357)). Vendor bloat blocks the commit only in a staged vendor file that the tidy block did not skip. A staged file that tidy ran on but could not clean still blocks, and so does one left untidied because tidy failed or `CHARTER_SKIP_TIDY=1` is set. Bloat in unstaged vendor files, and in files tidy skipped, shows as an INFO row. A planned-module skip (tidy would write into a module with unstaged or untracked work) skips tidy for every staged vendor file in that commit, so it lifts the bloat gate for all of them. Scoping is by file name: the bloat check still reads each vendor file in the working tree, not its staged content. Every other doctor check blocks as usual. CI's plain `charter doctor --ci` still fails on all vendor bloat. The skip message says how to tidy the file: stage or stash the unstaged work and commit again, or run `charter adf tidy --source ` and `git add` the result. + - A hook installed before [#320](https://github.com/Stackbilt-dev/charter/issues/320) re-stages all of `.ai/*.adf`. A hook installed before #357 blocks on vendor bloat outside the commit. Re-run `charter hook install --pre-commit` to replace either one (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`. @@ -158,6 +158,8 @@ npx charter doctor --ci --format json # CI mode: exit 1 on warnings - `--adf-only` — run only ADF readiness checks, skip Charter config validation - `--ci` — non-interactive, exits with policy violation code on warnings +- `--staged` — the pre-commit hook's gate ([#357](https://github.com/Stackbilt-dev/charter/issues/357)): the `adf vendor bloat` check counts only vendor files staged in the git index (staged deletions excluded). It selects files by name and still reads their working-tree content, not the staged blob. Bloat in other vendor files moves to an INFO row, `adf vendor bloat (not blocking)`, so it does not fail `--ci`. No other check is scoped. If the index cannot be read, every vendor file counts, as without the flag. +- `--tidy-skipped ""` — with `--staged` only: space-separated vendor files the hook's tidy block skipped. Their bloat goes to the same INFO row. The value may be empty (`""`). Used without `--staged`, it is a usage error (exit 2). The hook installed by `charter hook install --commit-msg` counts as a local trailer gate: it rejects a commit with no trailer under a strict policy. A @@ -169,7 +171,10 @@ 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 +all of `.ai/*.adf`; re-run `charter hook install --pre-commit`. The row also +appears for a hook from before the commit-scoped gate +([#357](https://github.com/Stackbilt-dev/charter/issues/357)), which blocks on vendor +bloat in files the commit does not include; the fix is the same. A current, foreign or missing hook adds no row. It is INFO, not WARN, so `--ci` does not fail on upgrade. #### Vendor file modes diff --git a/packages/cli/README.md b/packages/cli/README.md index 689fb24..5d9bd9d 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -300,7 +300,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 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). +- `--pre-commit`: Install a pre-commit hook that tidies the staged vendor files, enforces ADF routing with `charter doctor --adf-only --ci --staged`, 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`. Vendor bloat blocks the commit only in staged vendor files that tidy did not skip. Bloat in unstaged or skipped files is reported as INFO, and CI's plain `charter doctor --ci` still enforces it. Hooks installed before #320 re-stage all of `.ai/*.adf`, and hooks installed before #357 block on vendor bloat outside the commit. 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 0ebfcab..49702d6 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -6,6 +6,7 @@ import * as path from 'node:path'; import { HOOK_TIDY_BLOCK_TEMPLATE, hookCommand, + PRE_COMMIT_GATE_SCOPE_MARKER, PRE_COMMIT_HOOK_CONTENT, PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER, @@ -303,6 +304,37 @@ describe('hookCommand', () => { }]); }); + it('#357: INFO for a #356-era hook (staged-only tidy, unscoped gate)', async () => { + // The #356 hook: the current text without the gate marker and with its unscoped gate. + const hook356 = PRE_COMMIT_HOOK_CONTENT + .replace(`${PRE_COMMIT_GATE_SCOPE_MARKER}\n`, '') + .replace(/npx charter doctor --adf-only --ci --staged --tidy-skipped "\$TIDY_SKIPPED" --format text/, 'npx charter doctor --adf-only --ci --format text'); + expect(hook356).toContain(PRE_COMMIT_SCOPE_MARKER); + expect(hook356).toContain('npx charter doctor --adf-only --ci --format text && '); + expect(hook356).not.toContain(PRE_COMMIT_GATE_SCOPE_MARKER); + writeHook(path.join(tempDir, '.git', 'hooks'), hook356); + 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 the commit-scoped evidence gate: it blocks a commit on vendor bloat in files the commit does not include, and in files its tidy step skipped. Re-run: charter hook install --pre-commit', + }]); + + vi.spyOn(console, 'log').mockImplementation(() => {}); + expect(await hookCommand(baseOptions, ['install', '--pre-commit'])).toBe(0); + vi.restoreAllMocks(); + expect((await doctorChecks()).some(c => c.name === 'pre-commit hook')).toBe(false); + }); + + it('#357: the generated gate passes --staged and the tidy skips, and no longer claims a skip caused the block', () => { + const gate = PRE_COMMIT_HOOK_CONTENT.slice(PRE_COMMIT_HOOK_CONTENT.indexOf('\n# ADF evidence gate')); + expect(gate).toContain(PRE_COMMIT_GATE_SCOPE_MARKER); + expect(gate).toContain('npx charter doctor --adf-only --ci --staged --tidy-skipped "$TIDY_SKIPPED" --format text && npx charter adf evidence --auto-measure --ci'); + expect(gate).not.toContain('Blocked because'); + expect(gate).not.toContain('totalExtracted'); + expect(gate.match(/npx charter/g)?.length).toBe(5); // two calls, three echoed hints + }); + 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(() => {}); diff --git a/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts new file mode 100644 index 0000000..cfe51d1 --- /dev/null +++ b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts @@ -0,0 +1,193 @@ +/** + * Integration tests: doctor --staged / --tidy-skipped (#357). + * + * The generated pre-commit hook's gate runs doctor with --staged, so vendor + * bloat blocks only in vendor files the commit stages, and not in the ones + * its tidy block skipped (--tidy-skipped). Everything else doctor checks is + * unscoped, and doctor without --staged behaves exactly as before. + */ + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { CLIOptions } from '../../index'; +import { CLIError, EXIT_CODE } from '../../index'; +import { BLOAT_SCOPE_MSG, doctorCommand } from '../../commands/doctor'; + +const options: CLIOptions = { configPath: '.charter', format: 'json', ciMode: true, yes: false }; + +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/\`. +> 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 +`; + +const CLAUDE_BLOAT = 'CLAUDE.md: "Database Rules" section overlaps ADF section "rules"; trigger keywords [database (2x)] suggest content belongs in backend.adf'; +const GEMINI_BLOAT = CLAUDE_BLOAT.replace('CLAUDE.md', 'GEMINI.md'); + +interface Check { name: string; status: string; details: string } + +let originalCwd: string; +let tmp: string; + +function git(...args: string[]): void { + execFileSync('git', args, { cwd: tmp, stdio: 'ignore' }); +} + +async function doctor(args: string[], opts: CLIOptions = options): Promise<{ code: number; status: string; checks: Check[] }> { + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { logs.push(a.map(String).join(' ')); }); + try { + const code = await doctorCommand(opts, args); + const json = JSON.parse(logs.join('\n')) as { status: string; checks: Check[] }; + return { code, status: json.status, checks: json.checks }; + } finally { + vi.restoreAllMocks(); + } +} + +const bloatRows = (checks: Check[]): Check[] => checks.filter(c => c.name.startsWith('adf vendor bloat')); + +beforeEach(() => { + originalCwd = process.cwd(); + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'charter-doctor-staged-')); + process.chdir(tmp); + git('init', '-q'); + // Repo-local identity: CI runners have no global one (commits would exit 128). + git('config', 'user.email', 'test@test.com'); + git('config', 'user.name', 'Test'); + fs.mkdirSync('.ai'); + fs.writeFileSync('.ai/manifest.adf', 'ADF: 0.1\n\n📦 DEFAULT_LOAD:\n - core.adf\n - state.adf\n\n📂 ON_DEMAND:\n - backend.adf (Triggers on: database, API, migration)\n'); + fs.writeFileSync('.ai/core.adf', 'ADF: 0.1\n\n📐 RULES:\n - All changes require tests\n'); + fs.writeFileSync('.ai/state.adf', 'ADF: 0.1\n\n📋 STATE:\n - CURRENT: testing\n'); + fs.writeFileSync('.ai/backend.adf', 'ADF: 0.1\n\n📐 RULES:\n - Database access through repository pattern\n'); + fs.writeFileSync('CLAUDE.md', POINTER + DB_BLOAT); + fs.writeFileSync('GEMINI.md', (POINTER + DB_BLOAT).replace('# CLAUDE.md', '# GEMINI.md')); +}); + +afterEach(() => { + process.chdir(originalCwd); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +describe('doctor without --staged is unchanged', () => { + it.each([[[] as string[]], [['--adf-only']]])('args %j: one WARN row naming every bloated file, whole-tree advice, exit 1', async (args) => { + git('add', 'CLAUDE.md'); // staging must not matter without --staged + const { code, checks } = await doctor(args); + expect(code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(bloatRows(checks)).toEqual([{ + name: 'adf vendor bloat', + status: 'WARN', + details: `Vendor file bloat detected:\n ${CLAUDE_BLOAT}\n ${GEMINI_BLOAT}\n Run: charter adf tidy --dry-run`, + }]); + }); + + it('--tidy-skipped without --staged is a usage error, never a silent downgrade', async () => { + await expect(doctorCommand(options, ['--adf-only', '--tidy-skipped', 'CLAUDE.md'])).rejects.toThrow(CLIError); + await expect(doctorCommand(options, ['--adf-only', '--tidy-skipped', 'CLAUDE.md'])).rejects.toThrow(BLOAT_SCOPE_MSG.tidySkippedNeedsStaged); + }); + + it('--tidy-skipped with no value is a usage error', async () => { + await expect(doctorCommand(options, ['--adf-only', '--staged', '--tidy-skipped'])).rejects.toThrow(BLOAT_SCOPE_MSG.tidySkippedMissing); + await expect(doctorCommand(options, ['--adf-only', '--staged', '--tidy-skipped', '--format'])).rejects.toThrow(BLOAT_SCOPE_MSG.tidySkippedMissing); + }); +}); + +describe('doctor --staged', () => { + it('nothing staged: bloat is INFO (not blocking), and --ci passes', async () => { + const { code, status, checks } = await doctor(['--adf-only', '--staged', '--tidy-skipped', '']); + expect(code).toBe(EXIT_CODE.SUCCESS); + expect(status).toBe('PASS'); + expect(bloatRows(checks)).toEqual([ + { name: 'adf vendor bloat', status: 'PASS', details: '0 staged vendor file(s) checked, within bloat thresholds.' }, + { + name: 'adf vendor bloat (not blocking)', + status: 'INFO', + details: `${BLOAT_SCOPE_MSG.outsideHeader}\n CLAUDE.md (not staged): ${CLAUDE_BLOAT.slice('CLAUDE.md: '.length)}\n GEMINI.md (not staged): ${GEMINI_BLOAT.slice('GEMINI.md: '.length)}\n ${BLOAT_SCOPE_MSG.outsideRun}`, + }, + ]); + }); + + it('a staged bloated file blocks, with advice scoped to it; an unstaged one is INFO', async () => { + git('add', 'CLAUDE.md'); + const { code, checks } = await doctor(['--adf-only', '--staged']); + expect(code).toBe(EXIT_CODE.POLICY_VIOLATION); + const rows = bloatRows(checks); + expect(rows[0]).toEqual({ + name: 'adf vendor bloat', + status: 'WARN', + details: `Vendor file bloat detected:\n ${CLAUDE_BLOAT}\n Run: charter adf tidy --dry-run --source CLAUDE.md`, + }); + expect(rows[1].status).toBe('INFO'); + expect(rows[1].details).toContain('GEMINI.md (not staged): '); + expect(rows[1].details).not.toContain('CLAUDE.md'); + }); + + it('a staged file named in --tidy-skipped is INFO, labelled as skipped', async () => { + git('add', 'CLAUDE.md', 'GEMINI.md'); + const { code, checks } = await doctor(['--adf-only', '--staged', '--tidy-skipped', ' CLAUDE.md']); + expect(code).toBe(EXIT_CODE.POLICY_VIOLATION); // GEMINI.md is staged and not skipped + const rows = bloatRows(checks); + expect(rows[0].status).toBe('WARN'); + expect(rows[0].details).toContain('GEMINI.md: '); + expect(rows[0].details).not.toContain('CLAUDE.md'); + expect(rows[1].details).toContain('CLAUDE.md (auto-tidy skipped for it in this commit): '); + + const both = await doctor(['--adf-only', '--staged', '--tidy-skipped', ' CLAUDE.md GEMINI.md']); + expect(both.code).toBe(EXIT_CODE.SUCCESS); + expect(bloatRows(both.checks)[0]).toEqual({ + name: 'adf vendor bloat', + status: 'PASS', + details: '0 staged vendor file(s) checked, within bloat thresholds. 2 staged vendor file(s) skipped by auto-tidy are not checked here (see the INFO row).', + }); + }); + + it('a staged deletion does not count as staged', async () => { + git('add', 'CLAUDE.md'); + git('commit', '-q', '--no-verify', '-m', 'x'); + git('rm', '-q', '--cached', 'CLAUDE.md'); + const { checks } = await doctor(['--adf-only', '--staged']); + expect(bloatRows(checks).find(c => c.status === 'WARN')).toBeUndefined(); + }); + + it('an unreadable index falls back to enforcing every vendor file', async () => { + git('add', 'CLAUDE.md'); + fs.writeFileSync(path.join(tmp, '.git', 'index'), 'not an index'); + expect(() => execFileSync('git', ['diff', '--cached', '--name-only'], { cwd: tmp, stdio: 'ignore' })).toThrow(); + const { code, checks } = await doctor(['--adf-only', '--staged', '--tidy-skipped', 'CLAUDE.md']); + expect(code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(bloatRows(checks)).toEqual([{ + name: 'adf vendor bloat', + status: 'WARN', + details: `Vendor file bloat detected:\n ${CLAUDE_BLOAT}\n ${GEMINI_BLOAT}\n Run: charter adf tidy --dry-run`, + }]); + }); + + it('only bloat is scoped: any other WARN still fails --ci', async () => { + fs.rmSync('.ai/state.adf'); + const { code, checks } = await doctor(['--adf-only', '--staged', '--tidy-skipped', 'CLAUDE.md']); + expect(code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(checks.find(c => c.name === 'adf default modules')?.status).toBe('WARN'); + expect(bloatRows(checks).some(c => c.status === 'WARN')).toBe(false); + }); + + it('text output shows the INFO row', async () => { + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { logs.push(a.map(String).join(' ')); }); + await doctorCommand({ ...options, format: 'text' }, ['--adf-only', '--staged']); + vi.restoreAllMocks(); + expect(logs.join('\n')).toContain(` [info] adf vendor bloat (not blocking): ${BLOAT_SCOPE_MSG.outsideHeader}`); + }); +}); diff --git a/packages/cli/src/__tests__/integration/precommit-hook.test.ts b/packages/cli/src/__tests__/integration/precommit-hook.test.ts index 2ac5890..11e51b7 100644 --- a/packages/cli/src/__tests__/integration/precommit-hook.test.ts +++ b/packages/cli/src/__tests__/integration/precommit-hook.test.ts @@ -8,10 +8,10 @@ * 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. * - * 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. + * The ADF evidence gate that follows the tidy block runs doctor --adf-only --ci + * --staged --tidy-skipped (#357): vendor bloat blocks only in staged files the + * tidy block did not skip. Tests of the tidy block alone install it without the + * gate (`tidyOnly`); the "full generated hook" group pins the gate's behaviour. */ import * as fs from 'node:fs'; @@ -287,7 +287,7 @@ ${DB_BLOAT}`); expect(result.status).toBe(0); 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(result.output).toContain("[pre-commit] Its bloat, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy it: stage or stash the unstaged changes and commit again; or run: charter adf tidy --source CLAUDE.md, then git add the result."); expect(tidyCalls(repo)).toEqual([]); expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(staged); expect(read(tmp, 'CLAUDE.md')).toBe(worktree); @@ -313,7 +313,7 @@ ${DB_BLOAT}`); expect(result.status).toBe(0); 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(result.output).toContain("[pre-commit] Bloat in CLAUDE.md, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy: stage or stash that work and commit again; or run: charter adf tidy --source CLAUDE.md, then git add the result."); 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']); @@ -430,60 +430,220 @@ ${DB_BLOAT}`); expect(read(tmp, 'CLAUDE.md')).toContain('## Architecture'); }); - 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\./; + describe('full generated hook: the evidence gate is scoped to the commit (#357)', () => { + const NOT_BLOCKING = '[info] adf vendor bloat (not blocking):'; + const BLOAT_WARN = '[warn] adf vendor bloat:'; + + interface DoctorJson { status: string; checks: Array<{ name: string; status: string; details: string }> } + + /** Plain doctor, as CI runs it: no --staged. */ + function plainDoctor(tmp: string): { status: number | null; json: DoctorJson } { + const r = spawnSync('node', [CLI_BIN, 'doctor', '--adf-only', '--ci', '--format', 'json'], { cwd: tmp, encoding: 'utf-8' }); + return { status: r.status, json: JSON.parse(r.stdout) as DoctorJson }; + } + + /** Plain doctor still fails on every bloated vendor file, and never scopes anything. */ + function expectPlainDoctorEnforcesAll(tmp: string, bloated: string[]): void { + const { status, json } = plainDoctor(tmp); + expect(status).toBe(1); + const bloat = json.checks.find(c => c.name === 'adf vendor bloat'); + expect(bloat?.status).toBe('WARN'); + for (const f of bloated) expect(bloat?.details).toContain(`${f}: `); + expect(bloat?.details.endsWith('Run: charter adf tidy --dry-run')).toBe(true); + expect(json.checks.some(c => c.name === 'adf vendor bloat (not blocking)')).toBe(false); + } + + const head = (tmp: string): string => git(tmp, 'rev-parse', 'HEAD'); + + it('B: unstaged GEMINI.md bloat does not block a clean staged CLAUDE.md edit, and is shown as INFO', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-b'); + const tmp = repo.dir; + const gemini = read(tmp, 'GEMINI.md') + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), gemini); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER.trimEnd() + '\n- pnpm 9\n'); + git(tmp, 'add', 'CLAUDE.md'); - 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 result = commit(tmp, 'clean claude edit'); + + expect(result.status).toBe(0); + expect(result.output).toContain(NOT_BLOCKING); + expect(result.output).toContain('GEMINI.md (not staged): '); + expect(result.output).not.toContain(BLOAT_WARN); + expect(repo.calls()).toContain('doctor --adf-only --ci --staged --tidy-skipped --format text'); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(read(tmp, 'GEMINI.md')).toBe(gemini); + expect(porcelain(tmp)).toEqual([' M GEMINI.md']); + expectPlainDoctorEnforcesAll(tmp, ['GEMINI.md']); + }); + + it('B2: a staged bloated CLAUDE.md is tidied and committed while unstaged GEMINI.md bloat stays put', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-b2'); const tmp = repo.dir; + const gemini = read(tmp, 'GEMINI.md') + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), gemini); fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); git(tmp, 'add', 'CLAUDE.md'); + + const result = commit(tmp, 'bloated claude, unstaged gemini'); + + expect(result.status).toBe(0); + expect(result.output).toContain('GEMINI.md (not staged): '); + expect(show(tmp, 'HEAD:CLAUDE.md')).not.toContain('## Database Rules'); + expect(committedFiles(tmp)).not.toContain('GEMINI.md'); + expect(read(tmp, 'GEMINI.md')).toBe(gemini); + expectPlainDoctorEnforcesAll(tmp, ['GEMINI.md']); + }); + + it('N: a commit staging no vendor file is not blocked by vendor bloat elsewhere', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-n'); + const tmp = repo.dir; + 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, 'no vendor staged'); + + expect(result.status).toBe(0); + expect(result.output).toContain('GEMINI.md (not staged): '); + expect(tidyCalls(repo)).toEqual([]); + expect(committedFiles(tmp)).toEqual(['notes.md']); + expectPlainDoctorEnforcesAll(tmp, ['GEMINI.md']); + }); + + it('C (CP-3): a partially staged bloated file is skipped by tidy and does not block; the skip is explained', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-partial'); + const tmp = repo.dir; + const staged = POINTER + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), staged); + 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.status).toBe(0); 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']); + expect(result.output).toContain('[pre-commit] Its bloat, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it.'); + expect(result.output).toContain('CLAUDE.md (auto-tidy skipped for it in this commit): '); + expect(result.output).not.toContain(BLOAT_WARN); + expect(result.output).not.toContain('[pre-commit] Blocked because'); + expect(repo.calls()).toContain('doctor --adf-only --ci --staged --tidy-skipped CLAUDE.md --format text'); + expect(tidyCalls(repo)).toEqual([]); + expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(staged); + expect(porcelain(tmp)).toEqual([' M CLAUDE.md']); + expectPlainDoctorEnforcesAll(tmp, ['CLAUDE.md']); }); - it('CP-4: planned-module WIP skips tidy, and the gate blocks with a line linking the two', { timeout: 60000 }, () => { + it('D (CP-4): planned-module WIP skips tidy and does not block', { timeout: 60000 }, () => { const repo = writeHookFixtureRepo('gate-wip'); const tmp = repo.dir; - fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + const bloated = POINTER + DB_BLOAT; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), bloated); 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.status).toBe(0); 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('CLAUDE.md (auto-tidy skipped for it in this commit): '); + expect(result.output).not.toContain(BLOAT_WARN); + expect(show(tmp, 'HEAD:CLAUDE.md')).toBe(bloated); + expect(porcelain(tmp)).toEqual([' M .ai/backend.adf']); + expectPlainDoctorEnforcesAll(tmp, ['CLAUDE.md']); + }); + + it('D3 (CP-4): an untracked planned module skips tidy and does not block', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-untracked'); + const tmp = repo.dir; + // DB_BLOAT makes the file bloated in doctor's eyes, so the gate would block without --tidy-skipped. + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + '\n## Testing\n- Always run the test suite before pushing\n- Never skip flaky tests\n' + DB_BLOAT); + 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'); + expect(plainDoctor(tmp).status).toBe(1); + + const result = commit(tmp, 'untracked planned module, full hook'); + + expect(result.status).toBe(0); + expect(result.output).toContain(`unstaged or untracked work: ${created}`); + expect(result.output).not.toContain(BLOAT_WARN); + expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); + expect(read(tmp, created!)).toContain('my own untracked draft'); + }); + + it('still blocks on staged bloat that tidy ran on but could not move (keyword density in a kept section)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-unfixable'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER.trimEnd() + ', the database is postgres and the database runs locally\n'); + git(tmp, 'add', 'CLAUDE.md'); + const before = head(tmp); + + const result = commit(tmp, 'unfixable bloat'); + + expect(result.status).toBe(1); + expect(result.output).toContain('[pre-commit] Vendor config files are clean.'); + expect(result.output).toContain(BLOAT_WARN); + expect(result.output).toContain('CLAUDE.md: trigger keywords [database (2x)]'); + expect(result.output).toContain('Run: charter adf tidy --dry-run --source CLAUDE.md'); expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); - expect(porcelain(tmp)).toEqual([' M .ai/backend.adf', 'M CLAUDE.md']); + expect(result.output).toContain(' doctor --adf-only --ci --staged --format text'); + expect(head(tmp)).toBe(before); + }); + + it('still blocks on a staged bloated file when tidy fails (not a deliberate skip)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-tidy-fails'); + const tmp = repo.dir; + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); + git(tmp, 'add', 'CLAUDE.md'); + const before = head(tmp); + + const result = commit(tmp, 'tidy fails, full hook', { HOOK_TEST_FAIL_TIDY: '1' }); + + expect(result.status).toBe(1); + expect(result.output).toContain('[pre-commit] adf tidy --dry-run failed; skipping auto-tidy.'); + expect(result.output).toContain(BLOAT_WARN); + expect(result.output).not.toContain(NOT_BLOCKING); + expect(head(tmp)).toBe(before); }); - it('prints no linking line when the gate fails for another reason', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-other'); + it('still blocks on a staged bloated file under CHARTER_SKIP_TIDY=1', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-skip-env'); 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, 'CLAUDE.md'), POINTER + DB_BLOAT); 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'); + git(tmp, 'add', 'CLAUDE.md'); + const before = head(tmp); - const result = commit(tmp, 'other failure'); + const result = commit(tmp, 'skip tidy, full hook', { CHARTER_SKIP_TIDY: '1' }); expect(result.status).toBe(1); - expect(result.output).not.toContain('[pre-commit] Blocked because'); - expect(tidyCalls(repo)).toEqual([]); + expect(result.output).toMatch(/\[warn\] adf vendor bloat: Vendor file bloat detected:\n {4}CLAUDE\.md: /); + // Only the staged file blocks; the unstaged one is reported apart. + expect(result.output).toContain('GEMINI.md (not staged): '); + expect(head(tmp)).toBe(before); + }); + + it('a non-bloat doctor WARN (malformed VENDORS entry) still blocks, even alongside a skipped file', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('gate-vendors'); + const tmp = repo.dir; + fs.appendFileSync(path.join(tmp, '.ai', 'manifest.adf'), '\n\u{1F3F7} VENDORS:\n - CLAUDE.md: bogus\n'); + 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 before = head(tmp); + + const result = commit(tmp, 'malformed vendors'); + + expect(result.status).toBe(1); + expect(result.output).toContain('[warn] adf vendor mode:'); + expect(result.output).toContain("VENDORS entry 'CLAUDE.md: bogus'"); + expect(result.output).toContain('CLAUDE.md (auto-tidy skipped for it in this commit): '); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(result.output).toContain(' doctor --adf-only --ci --staged --tidy-skipped "CLAUDE.md" --format text'); + expect(head(tmp)).toBe(before); }); }); diff --git a/packages/cli/src/commands/doctor-gate-enforcement.ts b/packages/cli/src/commands/doctor-gate-enforcement.ts index 1f8f2e3..eca28c3 100644 --- a/packages/cli/src/commands/doctor-gate-enforcement.ts +++ b/packages/cli/src/commands/doctor-gate-enforcement.ts @@ -1,7 +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'; +import { PRE_COMMIT_GATE_SCOPE_MARKER, PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER } from './hook'; interface GateEnforcementCheck { name: 'gate enforcement'; @@ -49,10 +49,12 @@ interface PreCommitScopeCheck { } 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'; +export const DOCTOR_UNSCOPED_GATE_MSG = 'The Charter pre-commit hook at {hookPath} predates the commit-scoped evidence gate: it blocks a commit on vendor bloat in files the commit does not include, and in files its tidy step skipped. 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). + * #320 (it carries PRE_COMMIT_HOOK_MARKER but not PRE_COMMIT_SCOPE_MARKER), or + * from #320 but before #357 (no PRE_COMMIT_GATE_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). @@ -60,11 +62,15 @@ export const DOCTOR_OLD_PRE_COMMIT_MSG = 'The Charter pre-commit hook at {hookPa 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; + if (!content.includes(PRE_COMMIT_HOOK_MARKER)) return undefined; + const message = !content.includes(PRE_COMMIT_SCOPE_MARKER) ? DOCTOR_OLD_PRE_COMMIT_MSG + : !content.includes(PRE_COMMIT_GATE_SCOPE_MARKER) ? DOCTOR_UNSCOPED_GATE_MSG + : undefined; + if (!message) return undefined; return { name: 'pre-commit hook', status: 'INFO', - details: DOCTOR_OLD_PRE_COMMIT_MSG.replace('{hookPath}', displayPath(hookPath)), + details: message.replace('{hookPath}', displayPath(hookPath)), }; } diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 82621d9..6e531fa 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -7,7 +7,8 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { CLIOptions } from '../index'; -import { EXIT_CODE } from '../index'; +import { CLIError, EXIT_CODE } from '../index'; +import { getFlag } from '../flags'; import { loadPatterns, loadConfig } from '../config'; import { parseAdf, parseManifest, stripCharterSentinels, evaluateLocBudgets, matchPath } from '@stackbilt/adf'; import type { LocBudgetRule } from '@stackbilt/adf'; @@ -16,7 +17,7 @@ import { CHARTER_OWNED_MARKERS } from './adf'; import { TARGET_FILENAMES } from '@stackbilt/adf'; import type { CompileTarget } from '@stackbilt/adf'; import { countExtractable } from './adf-tidy'; -import { legacyIndexEntries, legacyIndexKind, readVendorRecord, resolveVendorMode, resolveVendorModes, vendorFileGroups } from './adf-vendor'; +import { legacyIndexEntries, legacyIndexKind, readVendorRecord, resolveVendorMode, resolveVendorModes, VENDOR_FILES, vendorFileGroups } from './adf-vendor'; import type { ModeSource, VendorEntryProblem, VendorFile, VendorMode } from './adf-vendor'; import { checkGateEnforcement, checkPreCommitScope } from './doctor-gate-enforcement'; import { findMissingDefaultLoad, formatMissingDefaultLoad, REQUIRED_DEFAULT_LOAD_MODULES } from './adf-wiring'; @@ -348,9 +349,58 @@ function checkVendorModes(aiDir: string, manifestPath: string): DoctorResult['ch }; } +// ============================================================================ +// Commit-scoped vendor bloat (#357) +// ============================================================================ + +export const BLOAT_SCOPE_MSG = { + tidySkippedNeedsStaged: '--tidy-skipped requires --staged.', + tidySkippedMissing: '--tidy-skipped requires a value: the vendor files the pre-commit hook skipped (may be empty, "").', + stagedPass: '{n} staged vendor file(s) checked, within bloat thresholds.', + stagedPassSkipped: ' {skipped} staged vendor file(s) skipped by auto-tidy are not checked here (see the INFO row).', + outsideHeader: 'Vendor file bloat outside this commit (not blocking under --staged; charter doctor --ci without --staged still fails on it):', + notStaged: '{file} (not staged)', + tidySkipped: '{file} (auto-tidy skipped for it in this commit)', + outsideRun: 'Run: charter adf tidy --source , then git add the result', +} as const; + +/** + * Which vendor files' bloat can fail doctor. `null` (no --staged) is the + * unscoped default: every vendor file, exactly as before #357. + * With --staged (the generated pre-commit hook's gate), only vendor files staged + * in the index, minus the ones listed in --tidy-skipped (the files the hook's + * tidy block deliberately skipped, CP-3/CP-4). Only the bloat check is scoped. + */ +interface BloatScope { + staged: ReadonlySet; + skipped: ReadonlySet; +} + +function resolveBloatScope(args: string[]): BloatScope | null { + const hasSkipped = args.includes('--tidy-skipped'); + if (!args.includes('--staged')) { + if (hasSkipped) throw new CLIError(BLOAT_SCOPE_MSG.tidySkippedNeedsStaged); + return null; + } + const skippedRaw = hasSkipped ? getFlag(args, '--tidy-skipped') : ''; + if (skippedRaw === undefined || skippedRaw.startsWith('--')) throw new CLIError(BLOAT_SCOPE_MSG.tidySkippedMissing); + let stagedOut: string; + try { + stagedOut = runGit(['diff', '--cached', '--name-only', '--diff-filter=d']); + } catch { + // The index cannot be read: fall back to enforcing every vendor file. + return null; + } + const vendorNames = new Set(VENDOR_FILES); + const staged = new Set(stagedOut.split(/\r?\n/).map(l => l.trim()).filter(l => vendorNames.has(l))); + const skipped = new Set(skippedRaw.split(/\s+/).filter(Boolean)); + return { staged, skipped }; +} + export async function doctorCommand(options: CLIOptions, args: string[] = []): Promise { const checks: DoctorResult['checks'] = []; const adfOnly = args.includes('--adf-only'); + const bloatScope = resolveBloatScope(args); const mcpMode = args.includes('--mcp'); const configFile = path.join(options.configPath, 'config.json'); const inGitRepo = isGitRepo(); @@ -568,6 +618,7 @@ export async function doctorCommand(options: CLIOptions, args: string[] = []): P // Vendor file bloat detection: line count, section overlap, keyword density const BLOAT_LINE_THRESHOLD = 80; const bloatWarnings: string[] = []; + const bloatByFile: Array<{ file: string; warnings: string }> = []; // Collect ADF section keys for overlap detection const adfSectionKeys = new Set(); @@ -650,10 +701,47 @@ export async function doctorCommand(options: CLIOptions, args: string[] = []): P if (fileWarnings.length > 0) { bloatWarnings.push(`${file}: ${fileWarnings.join('; ')}`); + bloatByFile.push({ file, warnings: fileWarnings.join('; ') }); } } - if (bloatWarnings.length > 0) { + if (bloatScope) { + // Hook gate (#357): a file is matched under any spelling of its physical + // file, so a staged agents.md covers the AGENTS.md row on NTFS. + const groups = vendorFileGroups(); + const namesOf = (file: string): string[] => groups.find(g => (g as string[]).includes(file)) ?? [file]; + const scopeOf = (file: string): 'blocking' | 'skipped' | 'not-staged' => { + const names = namesOf(file); + if (names.some(n => bloatScope.skipped.has(n))) return 'skipped'; + return names.some(n => bloatScope.staged.has(n)) ? 'blocking' : 'not-staged'; + }; + const blocking = bloatByFile.filter(b => scopeOf(b.file) === 'blocking'); + const outside = bloatByFile.filter(b => scopeOf(b.file) !== 'blocking'); + const skippedBloated = outside.filter(b => scopeOf(b.file) === 'skipped').length; + if (blocking.length > 0) { + checks.push({ + name: 'adf vendor bloat', + status: 'WARN', + // Scoped advice: a whole-tree tidy would also rewrite files this commit leaves out. + details: `Vendor file bloat detected:\n ${blocking.map(b => `${b.file}: ${b.warnings}`).join('\n ')}\n Run: charter adf tidy --dry-run${blocking.map(b => ` --source ${b.file}`).join('')}`, + }); + } else if (pointerFiles.length > 0) { + checks.push({ + name: 'adf vendor bloat', + status: 'PASS', + details: fill(BLOAT_SCOPE_MSG.stagedPass, { n: pointerFiles.filter(p => scopeOf(p.file) === 'blocking').length }) + + (skippedBloated > 0 ? fill(BLOAT_SCOPE_MSG.stagedPassSkipped, { skipped: skippedBloated }) : ''), + }); + } + if (outside.length > 0) { + const label = (file: string): string => fill(scopeOf(file) === 'skipped' ? BLOAT_SCOPE_MSG.tidySkipped : BLOAT_SCOPE_MSG.notStaged, { file }); + checks.push({ + name: 'adf vendor bloat (not blocking)', + status: 'INFO', + details: `${BLOAT_SCOPE_MSG.outsideHeader}\n ${outside.map(b => `${label(b.file)}: ${b.warnings}`).join('\n ')}\n ${BLOAT_SCOPE_MSG.outsideRun}`, + }); + } + } else if (bloatWarnings.length > 0) { checks.push({ name: 'adf vendor bloat', status: 'WARN', diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index 2840ab5..a03d158 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -124,6 +124,10 @@ export const PRE_COMMIT_HOOK_MARKER = '# Managed by Charter: pre-commit evidence // 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'; +// Third marker (#357): the evidence gate blocks on vendor bloat only in files the +// commit stages (doctor --staged --tidy-skipped). A #356-era hook carries the two +// markers above but not this one, so doctor can tell it to re-run install. +export const PRE_COMMIT_GATE_SCOPE_MARKER = '# Charter pre-commit evidence gate: blocks on vendor bloat only in staged files'; /** * The vendor tidy block of the pre-commit hook (#320). {CLI} is `npx charter` @@ -137,7 +141,8 @@ export const HOOK_TIDY_BLOCK_TEMPLATE = `# Vendor file auto-tidy (#320): tidy on ${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 lists the vendor files it skipped; the generated hook passes it to +# doctor --staged --tidy-skipped, so their bloat does not block the commit (#357). 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" @@ -153,7 +158,7 @@ if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then fi if ! git diff --quiet -- "$vf"; then 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." + echo "[pre-commit] Its bloat, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy it: stage or stash the unstaged changes and commit again; or run: charter adf tidy --source $vf, then git add the result." TIDY_SKIPPED="$TIDY_SKIPPED $vf" continue fi @@ -180,7 +185,7 @@ if [ "\${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then set +f if [ -n "$BLOCKED" ]; then 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." + echo "[pre-commit] Bloat in$NAMES, if any, is left as-is; a plain charter doctor --ci (as CI runs it) still reports it. To tidy: stage or stash that work and commit again; or run: charter adf tidy$SOURCES, then git add the result." TIDY_SKIPPED="$TIDY_SKIPPED$NAMES" else echo "[pre-commit] Moving content beyond the thin pointer into .ai/ ..." @@ -228,6 +233,7 @@ set -eu ${renderHookTidyBlock('npx charter')} # ADF evidence gate: check LOC ceilings if manifest exists +${PRE_COMMIT_GATE_SCOPE_MARKER} if [ -f ".ai/manifest.adf" ]; then echo '[pre-commit] Running ADF evidence check...' if [ -f "package.json" ] && grep -q '"verify:adf"' package.json; then @@ -238,19 +244,17 @@ if [ -f ".ai/manifest.adf" ]; then exit 1 } else - npx charter doctor --adf-only --ci --format text && npx charter adf evidence --auto-measure --ci || { + # --staged: vendor bloat blocks only in files this commit stages, and not in + # the ones the tidy block skipped (TIDY_SKIPPED); doctor reports the rest as + # INFO. Every other doctor check blocks as before. + npx charter doctor --adf-only --ci --staged --tidy-skipped "$TIDY_SKIPPED" --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' + if [ -n "$TIDY_SKIPPED" ]; then + echo " Run: npx charter doctor --adf-only --ci --staged --tidy-skipped \\"\${TIDY_SKIPPED# }\\" --format text" + else + echo ' Run: npx charter doctor --adf-only --ci --staged --format text' + fi echo ' && npx charter adf evidence --auto-measure --ci' exit 1 } @@ -484,7 +488,8 @@ function printHelp(): void { 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(' with unstaged changes is skipped. The gate (doctor --staged) blocks on vendor bloat only in'); + console.log(' staged files tidy did not skip. 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.');