From 1dcdcd9cbd78eb083779ee5bb32c981cb5415500 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Fri, 16 Oct 2026 21:39:54 -0500 Subject: [PATCH 1/2] fix(hook): scope the doctor gate in repos with a verify:adf script; run it with the repo's package manager Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/cli-reference.md | 8 +- packages/cli/src/__tests__/hook.test.ts | 59 +++++- .../integration/doctor-staged-bloat.test.ts | 78 +++++++ .../integration/precommit-hook.test.ts | 198 +++++++++++++++--- .../src/commands/doctor-gate-enforcement.ts | 10 +- packages/cli/src/commands/doctor.ts | 32 +++ packages/cli/src/commands/hook-scope.ts | 13 ++ packages/cli/src/commands/hook.ts | 63 ++++-- 9 files changed, 416 insertions(+), 46 deletions(-) create mode 100644 packages/cli/src/commands/hook-scope.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9afd340..02ad1b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,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. +- **The pre-commit hook's commit-scoped gate now also applies in repos with a `verify:adf` script, and runs that script with the repo's package manager** (`#357`) — re-run `charter hook install --pre-commit` to pick this up. The scoped gate above applied only when `package.json` had no `verify:adf` script. `charter setup` and `charter bootstrap` add one, so in most Node repos the hook ran `pnpm run verify:adf` instead, and that script's `charter doctor --adf-only --ci` still blocked a commit on bloat in vendor files it did not include. The hook also called `pnpm` by name, so the gate failed in npm and yarn repos on machines without pnpm. The hook now always runs the scoped doctor first, then `verify:adf` if the script exists, or `adf evidence` if it does not. It runs `verify:adf` with `pnpm` when there is a `pnpm-lock.yaml` and pnpm is installed, with `yarn` when there is a `yarn.lock` and yarn is installed, and with `npm` otherwise. While `verify:adf` runs, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token and writes the same token to a file in the git directory, which it deletes when it exits. Doctor skips the `adf vendor bloat` check only when the variable and the file match. It then reports the check as an INFO row, `adf vendor bloat (checked by pre-commit hook)`, and prints a line saying so on stderr. Every other doctor check in `verify:adf` still blocks, and so do the script's other commands. Setting the variable without the file changes nothing, so a CI job that sets it by mistake still gets a full `charter doctor --ci`. `charter doctor` now also reports a hook installed with the earlier `#357` fix with an INFO `pre-commit hook` row. - **`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 0057de9..5a021a6 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -78,7 +78,8 @@ npx charter hook install --commit-msg --force # overwrite existing hooks - 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 --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. + - The scoped doctor runs in every repo. Next, if `package.json` has a `verify:adf` script (`charter setup` and `charter bootstrap` add one), the gate runs it; otherwise it runs `charter adf evidence --auto-measure --ci`. `verify:adf` runs with `pnpm` when the repo has a `pnpm-lock.yaml` and pnpm is installed, with `yarn` for a `yarn.lock` and yarn installed, and with `npm` otherwise, so pnpm is never required. The doctor inside `verify:adf` would check vendor bloat across the whole tree again. To prevent that, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token for that run and writes the token to a file in the git directory, which it removes on exit. When the two match, doctor reports `adf vendor bloat (checked by pre-commit hook)` as INFO and prints a line on stderr. All its other checks still block, and so do the script's other commands. The variable without the matching file does nothing, so `charter doctor --ci` in CI keeps full strictness even if the variable is set there. Run by hand, `npm run verify:adf` checks every vendor file, as CI does. + - 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, and so, in a repo with a `verify:adf` script, does one from the first #357 fix, which also ran `pnpm run verify:adf` by name. Re-run `charter hook install --pre-commit` to replace any of them (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`. @@ -160,6 +161,7 @@ npx charter doctor --ci --format json # CI mode: exit 1 on warnings - `--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). +- `CHARTER_HOOK_SCOPED_DOCTOR` (environment) — set only by the generated pre-commit hook while it runs `verify:adf`, after its own `--staged` doctor. Doctor honours it only when its value matches the token the hook wrote to `charter-hook-scoped-doctor` in the git directory. It then reports the `adf vendor bloat` check as INFO (`adf vendor bloat (checked by pre-commit hook)`) and says so on stderr. No other check changes. The variable on its own (no matching file) has no effect, and `--staged` takes precedence over it. 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 @@ -174,7 +176,9 @@ tidy ([#320](https://github.com/Stackbilt-dev/charter/issues/320)). That hook re 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 +bloat in files the commit does not include, and for a hook from the first #357 fix, +whose gate stayed unscoped in repos with a `verify:adf` script and ran it with +`pnpm` by name; 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/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 49702d6..60f7b8d 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -4,9 +4,12 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { + HOOK_SCOPED_DOCTOR_ENV, + HOOK_SCOPED_DOCTOR_FILE, HOOK_TIDY_BLOCK_TEMPLATE, hookCommand, PRE_COMMIT_GATE_SCOPE_MARKER, + PRE_COMMIT_VERIFY_SCOPE_MARKER, PRE_COMMIT_HOOK_CONTENT, PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER, @@ -280,6 +283,26 @@ describe('hookCommand', () => { 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`; + // The gate #360 shipped: scoped only when package.json has no verify:adf script. + const GATE_360 = `# 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 + pnpm run verify:adf || { + echo '' + echo '[pre-commit] ADF verification FAILED.' + echo ' Run: pnpm run verify:adf' + exit 1 + } + else + npx charter doctor --adf-only --ci --staged --tidy-skipped "$TIDY_SKIPPED" --format text && npx charter adf evidence --auto-measure --ci || { + exit 1 + } + fi +fi +`; + const HOOK_360 = PRE_COMMIT_HOOK_CONTENT.slice(0, PRE_COMMIT_HOOK_CONTENT.indexOf('# ADF evidence gate')) + GATE_360; async function doctorChecks(): Promise> { const logs: string[] = []; @@ -305,8 +328,8 @@ 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 + // The #356 hook: the #360 text without the gate marker and with its unscoped gate. + const hook356 = HOOK_360 .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); @@ -326,10 +349,38 @@ describe('hookCommand', () => { 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', () => { + it('#357: INFO for a #360-era hook (gate unscoped, and pnpm-only, in repos with verify:adf)', async () => { + expect(HOOK_360).toContain(PRE_COMMIT_GATE_SCOPE_MARKER); + expect(HOOK_360).not.toContain(PRE_COMMIT_VERIFY_SCOPE_MARKER); + writeHook(path.join(tempDir, '.git', 'hooks'), HOOK_360); + 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 gate for repos with a verify:adf script: there it runs pnpm run verify:adf instead, whose doctor blocks a commit on vendor bloat in files the commit does not include, and which fails where pnpm is not installed. 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 always runs the scoped doctor, then verify:adf (with the token) or evidence', () => { 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).toContain(PRE_COMMIT_VERIFY_SCOPE_MARKER); + const doctorAt = gate.indexOf(' npx charter doctor --adf-only --ci --staged --tidy-skipped "$TIDY_SKIPPED" --format text || {'); + const branchAt = gate.indexOf(`if [ -f "package.json" ] && grep -q '"verify:adf"' package.json; then`); + // The scoped doctor runs before, and outside, the verify:adf branch. + expect(doctorAt).toBeGreaterThan(-1); + expect(branchAt).toBeGreaterThan(doctorAt); + expect(gate).toContain(' "$PM" run verify:adf || {'); + expect(gate).not.toContain('pnpm run verify:adf'); + expect(gate).toContain(`SCOPE_FILE=$(git rev-parse --git-path ${HOOK_SCOPED_DOCTOR_FILE})`); + expect(gate).toContain(` ${HOOK_SCOPED_DOCTOR_ENV}="$SCOPE_TOKEN"\n export ${HOOK_SCOPED_DOCTOR_ENV}\n`); + expect(gate).toContain(`trap 'rm -f "$SCOPE_FILE"' EXIT`); + expect(gate).toContain(' 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 diff --git a/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts index cfe51d1..3679f41 100644 --- a/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts +++ b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts @@ -15,6 +15,7 @@ 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'; +import { HOOK_SCOPED_DOCTOR_ENV, HOOK_SCOPED_DOCTOR_FILE } from '../../commands/hook'; const options: CLIOptions = { configPath: '.charter', format: 'json', ciMode: true, yes: false }; @@ -191,3 +192,80 @@ describe('doctor --staged', () => { expect(logs.join('\n')).toContain(` [info] adf vendor bloat (not blocking): ${BLOAT_SCOPE_MSG.outsideHeader}`); }); }); + +describe(`${HOOK_SCOPED_DOCTOR_ENV}: the hook's verify:adf run leaves bloat to the scoped gate`, () => { + const TOKEN = '4242-1700000000'; + const tokenFile = (): string => path.join(tmp, '.git', HOOK_SCOPED_DOCTOR_FILE); + const UNSCOPED_WARN = { name: 'adf vendor bloat', status: 'WARN', details: `Vendor file bloat detected:\n ${CLAUDE_BLOAT}\n ${GEMINI_BLOAT}\n Run: charter adf tidy --dry-run` }; + + async function withEnv(value: string | undefined, fn: () => Promise): Promise { + const before = process.env[HOOK_SCOPED_DOCTOR_ENV]; + if (value === undefined) delete process.env[HOOK_SCOPED_DOCTOR_ENV]; + else process.env[HOOK_SCOPED_DOCTOR_ENV] = value; + try { + return await fn(); + } finally { + if (before === undefined) delete process.env[HOOK_SCOPED_DOCTOR_ENV]; + else process.env[HOOK_SCOPED_DOCTOR_ENV] = before; + } + } + + async function doctorWithStderr(args: string[]): Promise<{ code: number; checks: Check[]; stderr: string[] }> { + const stderr: string[] = []; + const spy = vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { stderr.push(a.map(String).join(' ')); }); + try { + const r = await doctor(args); + return { code: r.code, checks: r.checks, stderr }; + } finally { + spy.mockRestore(); + } + } + + it.each([ + ['set to 1 with no token file (a CI misconfiguration)', '1', null], + ['set to the token with no token file', TOKEN, null], + ['set, with a token file holding a different token (a stale file)', '1', TOKEN], + ['unset, with a token file present', undefined, TOKEN], + ])('%s: full strictness, exactly as without it', async (_label, value, fileToken) => { + const baseline = await withEnv(undefined, () => doctor(['--adf-only'])); + if (fileToken !== null) fs.writeFileSync(tokenFile(), `${fileToken}\n`); + const r = await withEnv(value, () => doctorWithStderr(['--adf-only'])); + expect(r.code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(bloatRows(r.checks)).toEqual([UNSCOPED_WARN]); + expect(r.checks).toEqual(baseline.checks); + expect(r.stderr).toEqual([]); + }); + + it('with the matching token file: the bloat row is INFO, said on stderr, and --ci passes', async () => { + fs.writeFileSync(tokenFile(), `${TOKEN}\n`); + const r = await withEnv(TOKEN, () => doctorWithStderr(['--adf-only'])); + expect(r.code).toBe(EXIT_CODE.SUCCESS); + expect(bloatRows(r.checks)).toEqual([{ + name: 'adf vendor bloat (checked by pre-commit hook)', + status: 'INFO', + details: `${BLOAT_SCOPE_MSG.hookChecked}\n ${BLOAT_SCOPE_MSG.hookCheckedFound}\n ${CLAUDE_BLOAT}\n ${GEMINI_BLOAT}`, + }]); + expect(r.stderr).toEqual([BLOAT_SCOPE_MSG.hookCheckedStderr]); + }); + + it('with the matching token file, every other WARN (malformed VENDORS, missing default module) still fails --ci', async () => { + fs.appendFileSync('.ai/manifest.adf', '\n\u{1F3F7} VENDORS:\n - CLAUDE.md: bogus\n'); + fs.rmSync('.ai/state.adf'); + fs.writeFileSync(tokenFile(), `${TOKEN}\n`); + const r = await withEnv(TOKEN, () => doctorWithStderr(['--adf-only'])); + expect(r.code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(r.checks.find(c => c.name === 'adf vendor mode')?.status).toBe('WARN'); + expect(r.checks.find(c => c.name === 'adf default modules')?.status).toBe('WARN'); + expect(bloatRows(r.checks).some(c => c.status === 'WARN')).toBe(false); + }); + + it('--staged takes precedence: the token never widens or narrows the scoped check', async () => { + git('add', 'CLAUDE.md'); + const scoped = await withEnv(undefined, () => doctor(['--adf-only', '--staged'])); + fs.writeFileSync(tokenFile(), `${TOKEN}\n`); + const r = await withEnv(TOKEN, () => doctorWithStderr(['--adf-only', '--staged'])); + expect(r.checks).toEqual(scoped.checks); + expect(r.code).toBe(EXIT_CODE.POLICY_VIOLATION); + expect(r.stderr).toEqual([]); + }); +}); diff --git a/packages/cli/src/__tests__/integration/precommit-hook.test.ts b/packages/cli/src/__tests__/integration/precommit-hook.test.ts index 11e51b7..ecf803e 100644 --- a/packages/cli/src/__tests__/integration/precommit-hook.test.ts +++ b/packages/cli/src/__tests__/integration/precommit-hook.test.ts @@ -19,7 +19,8 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { execFileSync, spawnSync } from 'node:child_process'; import { afterEach, describe, expect, it } from 'vitest'; -import { PRE_COMMIT_HOOK_CONTENT } from '../../commands/hook'; +import { HOOK_SCOPED_DOCTOR_FILE, PRE_COMMIT_HOOK_CONTENT } from '../../commands/hook'; +import { BLOAT_SCOPE_MSG } from '../../commands/doctor'; // ============================================================================ // Fixture Helpers @@ -35,8 +36,18 @@ interface Repo { dir: string; /** One line per CLI call the hook made: its arguments, space-joined. */ calls(): string[]; + /** `git commit` with the repo's PATH (a failing pnpm first, when it has verify:adf). */ + commit(message: string, env?: Record): { status: number | null; output: string }; + /** Lines the failing pnpm shim logged: one per time the hook ran pnpm. */ + pnpmCalls(): string[]; } +/** Bootstrapped repos get this verify:adf, the one setup writes, with `charter` pointed at the shim. */ +const verifyAdfScript = (cli: string): string => + `${cli} doctor --adf-only --ci --format json && ${cli} adf evidence --auto-measure --ci --format json`; + +const PATH_KEY = Object.keys(process.env).find(k => k.toUpperCase() === 'PATH') ?? 'PATH'; + function makeTempDir(label: string): string { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), `charter integ hook-${label}-`)); tempDirs.push(tmp); @@ -89,12 +100,29 @@ const DB_BLOAT = ` * 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(label: string, opts: { tidyOnly?: boolean } = {}): Repo { +interface FixtureOpts { + tidyOnly?: boolean; + /** + * Give the repo a package.json with a verify:adf script (setup's, unless a + * string is passed: then `{CLI}` in it is the shim) and this lockfile. PATH + * gets a pnpm that logs and fails, so a hook that runs pnpm where it should not fails. + */ + verifyAdf?: { script?: string; lockfile: 'package-lock.json' | 'pnpm-lock.yaml' }; +} + +function writeHookFixtureRepo(label: string, opts: FixtureOpts = {}): 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'); + const pnpmLog = path.join(shimDir, 'pnpm.log'); + const binDir = path.join(shimDir, 'bin'); fs.writeFileSync(log, ''); + fs.writeFileSync(pnpmLog, ''); + fs.mkdirSync(binDir); + fs.writeFileSync(path.join(binDir, 'pnpm'), `#!/bin/sh\necho "pnpm $*" >> "${posix(pnpmLog)}"\necho 'fake pnpm: not installed' >&2\nexit 97\n`); + fs.chmodSync(path.join(binDir, 'pnpm'), 0o755); + fs.writeFileSync(path.join(binDir, 'pnpm.cmd'), `@echo pnpm %*>> "${pnpmLog}"\r\n@exit /b 97\r\n`); fs.writeFileSync(shim, [ `const args = process.argv.slice(2);`, `require('fs').appendFileSync(${JSON.stringify(log)}, args.join(' ') + '\\n');`, @@ -135,6 +163,14 @@ function writeHookFixtureRepo(label: string, opts: { tidyOnly?: boolean } = {}): 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')); + if (opts.verifyAdf) { + const cli = `node "${posix(shim)}"`; + const script = (opts.verifyAdf.script ?? verifyAdfScript('{CLI}')).split('{CLI}').join(cli); + fs.writeFileSync(path.join(tmp, 'package.json'), JSON.stringify({ name: 'fixture', private: true, scripts: { 'verify:adf': script } }, null, 2) + '\n'); + fs.writeFileSync(path.join(tmp, opts.verifyAdf.lockfile), opts.verifyAdf.lockfile === 'package-lock.json' + ? JSON.stringify({ name: 'fixture', lockfileVersion: 3, requires: true, packages: { '': { name: 'fixture' } } }, null, 2) + '\n' + : "lockfileVersion: '9.0'\n"); + } // The generated hook, as `charter hook install --pre-commit` writes it. const hookDir = path.join(tmp, '.githooks'); @@ -147,12 +183,16 @@ function writeHookFixtureRepo(label: string, opts: { tidyOnly?: boolean } = {}): git(tmp, 'config', 'core.hooksPath', '.githooks'); git(tmp, 'add', '-A'); - git(tmp, 'commit', '-m', 'initial'); + if (opts.verifyAdf) git(tmp, 'commit', '--no-verify', '-m', 'initial'); + else git(tmp, 'commit', '-m', 'initial'); fs.writeFileSync(log, ''); + const pathEnv = { [PATH_KEY]: `${binDir}${path.delimiter}${process.env[PATH_KEY] ?? ''}` }; return { dir: tmp, calls: () => fs.readFileSync(log, 'utf-8').split('\n').filter(Boolean), + commit: (message, env = {}) => commit(tmp, message, { ...pathEnv, ...env }), + pnpmCalls: () => fs.readFileSync(pnpmLog, 'utf-8').split(/\r?\n/).filter(Boolean), }; } @@ -430,7 +470,10 @@ ${DB_BLOAT}`); expect(read(tmp, 'CLAUDE.md')).toContain('## Architecture'); }); - describe('full generated hook: the evidence gate is scoped to the commit (#357)', () => { + describe.each([ + ['no verify:adf script', {}], + ['a verify:adf script (bootstrapped repo, npm lockfile, failing pnpm on PATH)', { verifyAdf: { lockfile: 'package-lock.json' } }], + ] as Array<[string, FixtureOpts]>)('full generated hook, %s: the evidence gate is scoped to the commit (#357)', (_flavor, flavor) => { const NOT_BLOCKING = '[info] adf vendor bloat (not blocking):'; const BLOAT_WARN = '[warn] adf vendor bloat:'; @@ -455,17 +498,43 @@ ${DB_BLOAT}`); const head = (tmp: string): string => git(tmp, 'rev-parse', 'HEAD'); + /** + * The gate went past the scoped doctor. With verify:adf: it ran (via npm, + * never pnpm), its doctor left bloat to the scoped gate, and the token file + * is gone. Without: the evidence check ran. + */ + function expectRestOfGateRan(repo: Repo, output: string): void { + if (!flavor.verifyAdf) { + expect(repo.calls()).toContain('adf evidence --auto-measure --ci'); + return; + } + expect(repo.calls()).toContain('doctor --adf-only --ci --format json'); + expect(repo.calls()).toContain('adf evidence --auto-measure --ci --format json'); + expect(output).toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); + expect(output).toContain('"name": "adf vendor bloat (checked by pre-commit hook)"'); + expect(repo.pnpmCalls()).toEqual([]); + expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + } + + /** The scoped doctor blocked, so nothing after it ran. */ + function expectGateStoppedAtScopedDoctor(repo: Repo): void { + expect(repo.calls().filter(c => c.startsWith('doctor ') && !c.includes('--staged'))).toEqual([]); + expect(repo.calls().filter(c => c.startsWith('adf evidence'))).toEqual([]); + expect(repo.pnpmCalls()).toEqual([]); + } + 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 repo = writeHookFixtureRepo('gate-b', flavor); 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'); - const result = commit(tmp, 'clean claude edit'); + const result = repo.commit('clean claude edit'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); expect(result.output).toContain(NOT_BLOCKING); expect(result.output).toContain('GEMINI.md (not staged): '); expect(result.output).not.toContain(BLOAT_WARN); @@ -477,16 +546,17 @@ ${DB_BLOAT}`); }); 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 repo = writeHookFixtureRepo('gate-b2', flavor); 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'); + const result = repo.commit('bloated claude, unstaged gemini'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); 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'); @@ -495,15 +565,16 @@ ${DB_BLOAT}`); }); it('N: a commit staging no vendor file is not blocked by vendor bloat elsewhere', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-n'); + const repo = writeHookFixtureRepo('gate-n', flavor); 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'); + const result = repo.commit('no vendor staged'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); expect(result.output).toContain('GEMINI.md (not staged): '); expect(tidyCalls(repo)).toEqual([]); expect(committedFiles(tmp)).toEqual(['notes.md']); @@ -511,16 +582,17 @@ ${DB_BLOAT}`); }); 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 repo = writeHookFixtureRepo('gate-partial', flavor); 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'); + const result = repo.commit('partial with full hook'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); 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('[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): '); @@ -534,16 +606,17 @@ ${DB_BLOAT}`); }); it('D (CP-4): planned-module WIP skips tidy and does not block', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-wip'); + const repo = writeHookFixtureRepo('gate-wip', flavor); const tmp = repo.dir; 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'); + const result = repo.commit('wip with full hook'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); 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('CLAUDE.md (auto-tidy skipped for it in this commit): '); expect(result.output).not.toContain(BLOAT_WARN); @@ -553,7 +626,7 @@ ${DB_BLOAT}`); }); it('D3 (CP-4): an untracked planned module skips tidy and does not block', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-untracked'); + const repo = writeHookFixtureRepo('gate-untracked', flavor); 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); @@ -565,9 +638,10 @@ ${DB_BLOAT}`); 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'); + const result = repo.commit('untracked planned module, full hook'); expect(result.status).toBe(0); + expectRestOfGateRan(repo, result.output); expect(result.output).toContain(`unstaged or untracked work: ${created}`); expect(result.output).not.toContain(BLOAT_WARN); expect(committedFiles(tmp)).toEqual(['CLAUDE.md']); @@ -575,15 +649,16 @@ ${DB_BLOAT}`); }); 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 repo = writeHookFixtureRepo('gate-unfixable', flavor); 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'); + const result = repo.commit('unfixable bloat'); expect(result.status).toBe(1); + expectGateStoppedAtScopedDoctor(repo); 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)]'); @@ -594,15 +669,16 @@ ${DB_BLOAT}`); }); it('still blocks on a staged bloated file when tidy fails (not a deliberate skip)', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-tidy-fails'); + const repo = writeHookFixtureRepo('gate-tidy-fails', flavor); 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' }); + const result = repo.commit('tidy fails, full hook', { HOOK_TEST_FAIL_TIDY: '1' }); expect(result.status).toBe(1); + expectGateStoppedAtScopedDoctor(repo); 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); @@ -610,16 +686,17 @@ ${DB_BLOAT}`); }); it('still blocks on a staged bloated file under CHARTER_SKIP_TIDY=1', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-skip-env'); + const repo = writeHookFixtureRepo('gate-skip-env', flavor); const tmp = repo.dir; fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER + DB_BLOAT); fs.writeFileSync(path.join(tmp, 'GEMINI.md'), read(tmp, 'GEMINI.md') + DB_BLOAT); git(tmp, 'add', 'CLAUDE.md'); const before = head(tmp); - const result = commit(tmp, 'skip tidy, full hook', { CHARTER_SKIP_TIDY: '1' }); + const result = repo.commit('skip tidy, full hook', { CHARTER_SKIP_TIDY: '1' }); expect(result.status).toBe(1); + expectGateStoppedAtScopedDoctor(repo); 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): '); @@ -627,7 +704,7 @@ ${DB_BLOAT}`); }); it('a non-bloat doctor WARN (malformed VENDORS entry) still blocks, even alongside a skipped file', { timeout: 60000 }, () => { - const repo = writeHookFixtureRepo('gate-vendors'); + const repo = writeHookFixtureRepo('gate-vendors', flavor); 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); @@ -635,9 +712,10 @@ ${DB_BLOAT}`); fs.appendFileSync(path.join(tmp, 'CLAUDE.md'), '- not staged yet\n'); const before = head(tmp); - const result = commit(tmp, 'malformed vendors'); + const result = repo.commit('malformed vendors'); expect(result.status).toBe(1); + expectGateStoppedAtScopedDoctor(repo); 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): '); @@ -647,6 +725,78 @@ ${DB_BLOAT}`); }); }); + describe('full generated hook with a verify:adf script', () => { + const head = (tmp: string): string => git(tmp, 'rev-parse', 'HEAD'); + const withUnstagedGeminiBloat = (tmp: string): void => { + fs.writeFileSync(path.join(tmp, 'GEMINI.md'), read(tmp, 'GEMINI.md') + DB_BLOAT); + fs.writeFileSync(path.join(tmp, 'CLAUDE.md'), POINTER.trimEnd() + '\n- pnpm 9\n'); + git(tmp, 'add', 'CLAUDE.md'); + }; + + it('a failing custom command in verify:adf still blocks, and the hint names npm', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('verify-custom', { + verifyAdf: { lockfile: 'package-lock.json', script: `${verifyAdfScript('{CLI}')} && node -e "process.exit(4)"` }, + }); + withUnstagedGeminiBloat(repo.dir); + const before = head(repo.dir); + + const result = repo.commit('custom check fails'); + + expect(result.status).toBe(1); + expect(repo.calls()).toContain('adf evidence --auto-measure --ci --format json'); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(result.output).toContain(' Run: npm run verify:adf (outside this hook'); + expect(repo.pnpmCalls()).toEqual([]); + expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + expect(head(repo.dir)).toBe(before); + }); + + it('a non-bloat WARN that only the doctor inside verify:adf sees still blocks', { timeout: 60000 }, () => { + // Without --adf-only, doctor also wants .charter/config.json, which this repo lacks. + const repo = writeHookFixtureRepo('verify-nonbloat', { + verifyAdf: { lockfile: 'package-lock.json', script: '{CLI} doctor --ci --format json' }, + }); + withUnstagedGeminiBloat(repo.dir); + const before = head(repo.dir); + + const result = repo.commit('config missing'); + + expect(result.status).toBe(1); + expect(repo.calls()).toContain('doctor --ci --format json'); + expect(result.output).toContain('"name": "config file"'); + expect(result.output).toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(head(repo.dir)).toBe(before); + }); + + it('a pnpm-lock.yaml with pnpm on PATH runs verify:adf with pnpm', { timeout: 60000 }, () => { + // The pnpm on PATH is the failing shim, so the commit fails: that proves which runner the hook chose. + const repo = writeHookFixtureRepo('verify-pnpm', { verifyAdf: { lockfile: 'pnpm-lock.yaml' } }); + withUnstagedGeminiBloat(repo.dir); + + const result = repo.commit('pnpm repo'); + + expect(result.status).toBe(1); + expect(repo.pnpmCalls()).toEqual(['pnpm run verify:adf']); + expect(result.output).toContain(' Run: pnpm run verify:adf (outside this hook'); + expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + }); + + it('verify:adf run by hand, outside the hook, still blocks on the unstaged bloat (as CI does)', { timeout: 60000 }, () => { + const repo = writeHookFixtureRepo('verify-by-hand', { verifyAdf: { lockfile: 'package-lock.json' } }); + withUnstagedGeminiBloat(repo.dir); + expect(repo.commit('clean claude edit').status).toBe(0); + + const r = spawnSync('npm', ['run', 'verify:adf'], { + cwd: repo.dir, encoding: 'utf-8', shell: process.platform === 'win32', env: { ...process.env, CHARTER_HOOK_SCOPED_DOCTOR: '1' }, + }); + expect(r.status).not.toBe(0); + expect(r.stdout).toContain('"name": "adf vendor bloat"'); + expect(r.stdout).toContain('"status": "WARN"'); + expect(`${r.stdout}${r.stderr}`).not.toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); + }); + }); + 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; diff --git a/packages/cli/src/commands/doctor-gate-enforcement.ts b/packages/cli/src/commands/doctor-gate-enforcement.ts index eca28c3..d8468d4 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_GATE_SCOPE_MARKER, PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER } from './hook'; +import { PRE_COMMIT_GATE_SCOPE_MARKER, PRE_COMMIT_HOOK_MARKER, PRE_COMMIT_SCOPE_MARKER, PRE_COMMIT_VERIFY_SCOPE_MARKER } from './hook'; interface GateEnforcementCheck { name: 'gate enforcement'; @@ -50,11 +50,14 @@ 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'; +export const DOCTOR_VERIFY_ADF_GATE_MSG = 'The Charter pre-commit hook at {hookPath} predates the commit-scoped gate for repos with a verify:adf script: there it runs pnpm run verify:adf instead, whose doctor blocks a commit on vendor bloat in files the commit does not include, and which fails where pnpm is not installed. 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), or - * from #320 but before #357 (no PRE_COMMIT_GATE_SCOPE_MARKER). + * from #320 but before #357 (no PRE_COMMIT_GATE_SCOPE_MARKER), or from #360, + * which left the gate unscoped in repos with a verify:adf script (no + * PRE_COMMIT_VERIFY_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). @@ -65,7 +68,8 @@ export function checkPreCommitScope(): PreCommitScopeCheck | 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; + : !content.includes(PRE_COMMIT_VERIFY_SCOPE_MARKER) ? DOCTOR_VERIFY_ADF_GATE_MSG + : undefined; if (!message) return undefined; return { name: 'pre-commit hook', diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 6e531fa..51bfa6e 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -20,6 +20,7 @@ import { countExtractable } from './adf-tidy'; 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 { HOOK_SCOPED_DOCTOR_ENV, HOOK_SCOPED_DOCTOR_FILE } from './hook-scope'; import { findMissingDefaultLoad, formatMissingDefaultLoad, REQUIRED_DEFAULT_LOAD_MODULES } from './adf-wiring'; interface DoctorResult { @@ -362,6 +363,9 @@ export const BLOAT_SCOPE_MSG = { 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', + hookChecked: `Not enforced in this run: the Charter pre-commit hook already checked vendor bloat for this commit (doctor --staged) and set ${HOOK_SCOPED_DOCTOR_ENV} for its verify:adf run. charter doctor --ci outside the hook, as CI runs it, checks every vendor file.`, + hookCheckedFound: 'Bloat this run would otherwise report:', + hookCheckedStderr: `[doctor] adf vendor bloat: not enforced in this run; the Charter pre-commit hook already checked it for this commit (${HOOK_SCOPED_DOCTOR_ENV}).`, } as const; /** @@ -397,6 +401,25 @@ function resolveBloatScope(args: string[]): BloatScope | null { return { staged, skipped }; } +/** + * True only while the generated pre-commit hook runs verify:adf, after its own + * doctor --staged has checked vendor bloat for this commit. The hook sets + * HOOK_SCOPED_DOCTOR_ENV to a token and writes the same token to + * HOOK_SCOPED_DOCTOR_FILE in the git dir, removing it when the hook exits. Both + * must match: the variable alone (say, left in a CI environment) does nothing, + * and while it is unset git is not even asked. Only the bloat check is affected. + */ +function hookAlreadyCheckedBloat(): boolean { + const token = process.env[HOOK_SCOPED_DOCTOR_ENV]?.trim(); + if (!token) return false; + try { + const file = runGit(['rev-parse', '--git-path', HOOK_SCOPED_DOCTOR_FILE]).trim(); + return fs.readFileSync(file, 'utf-8').trim() === token; + } catch { + return false; + } +} + export async function doctorCommand(options: CLIOptions, args: string[] = []): Promise { const checks: DoctorResult['checks'] = []; const adfOnly = args.includes('--adf-only'); @@ -741,6 +764,15 @@ export async function doctorCommand(options: CLIOptions, args: string[] = []): P details: `${BLOAT_SCOPE_MSG.outsideHeader}\n ${outside.map(b => `${label(b.file)}: ${b.warnings}`).join('\n ')}\n ${BLOAT_SCOPE_MSG.outsideRun}`, }); } + } else if (hookAlreadyCheckedBloat()) { + // Inside the hook's verify:adf run: reported, never a WARN, and said out loud. + console.error(BLOAT_SCOPE_MSG.hookCheckedStderr); + checks.push({ + name: 'adf vendor bloat (checked by pre-commit hook)', + status: 'INFO', + details: BLOAT_SCOPE_MSG.hookChecked + + (bloatWarnings.length > 0 ? `\n ${BLOAT_SCOPE_MSG.hookCheckedFound}\n ${bloatWarnings.join('\n ')}` : ''), + }); } else if (bloatWarnings.length > 0) { checks.push({ name: 'adf vendor bloat', diff --git a/packages/cli/src/commands/hook-scope.ts b/packages/cli/src/commands/hook-scope.ts new file mode 100644 index 0000000..776c28d --- /dev/null +++ b/packages/cli/src/commands/hook-scope.ts @@ -0,0 +1,13 @@ +/** + * Shared by the generated pre-commit hook (hook.ts) and doctor. Kept free of + * imports so doctor can read it without loading hook.ts and its import graph. + * + * HOOK_SCOPED_DOCTOR_ENV is set only by the generated hook while it runs + * verify:adf. Its value is a token the hook also writes to HOOK_SCOPED_DOCTOR_FILE + * in the git dir and removes on exit; doctor skips the vendor bloat check + * (reported as INFO) only when the two match, so the variable alone, as a CI + * misconfiguration might set it, does nothing. See hookAlreadyCheckedBloat in doctor.ts. + */ +export const HOOK_SCOPED_DOCTOR_ENV = 'CHARTER_HOOK_SCOPED_DOCTOR'; +/** File name passed to `git rev-parse --git-path`. */ +export const HOOK_SCOPED_DOCTOR_FILE = 'charter-hook-scoped-doctor'; diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index a03d158..70b6640 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -10,6 +10,9 @@ import type { CLIOptions } from '../index'; import { CLIError, EXIT_CODE } from '../index'; import { runGit, isGitRepo, getGitErrorMessage } from '../git-helpers'; import { getFlag } from '../flags'; +import { HOOK_SCOPED_DOCTOR_ENV, HOOK_SCOPED_DOCTOR_FILE } from './hook-scope'; + +export { HOOK_SCOPED_DOCTOR_ENV, HOOK_SCOPED_DOCTOR_FILE }; interface HookInstallResult { status: 'INSTALLED' | 'SKIPPED'; @@ -128,6 +131,11 @@ export const PRE_COMMIT_SCOPE_MARKER = '# Charter pre-commit vendor tidy: scoped // 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'; +// Fourth marker: a #360-era hook carries the three markers above but, in a repo +// with a verify:adf script, ran `pnpm run verify:adf` instead of the scoped +// doctor, so its whole-tree doctor still blocked on unrelated bloat, and the gate +// failed wherever pnpm is not installed. +export const PRE_COMMIT_VERIFY_SCOPE_MARKER = '# Charter pre-commit evidence gate: scoped in verify:adf repos too, run with the repo package manager'; /** * The vendor tidy block of the pre-commit hook (#320). {CLI} is `npx charter` @@ -234,28 +242,55 @@ set -eu ${renderHookTidyBlock('npx charter')} # ADF evidence gate: check LOC ceilings if manifest exists ${PRE_COMMIT_GATE_SCOPE_MARKER} +${PRE_COMMIT_VERIFY_SCOPE_MARKER} if [ -f ".ai/manifest.adf" ]; then echo '[pre-commit] Running ADF evidence check...' + # --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. Runs with or without verify:adf. + npx charter doctor --adf-only --ci --staged --tidy-skipped "$TIDY_SKIPPED" --format text || { + echo '' + echo '[pre-commit] ADF verification FAILED.' + 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 + exit 1 + } if [ -f "package.json" ] && grep -q '"verify:adf"' package.json; then - pnpm run verify:adf || { + # The package manager the lockfile names, when it is installed; npm otherwise + # (it ships with node, and npx above needs it anyway). + PM=npm + if [ -f "pnpm-lock.yaml" ] && command -v pnpm >/dev/null 2>&1; then + PM=pnpm + elif [ -f "yarn.lock" ] && command -v yarn >/dev/null 2>&1; then + PM=yarn + fi + # The doctor inside verify:adf would re-check vendor bloat in every file. The + # scoped doctor above has checked it for this commit, so verify:adf gets a + # one-time token, matched against a file in the git dir that is removed on + # exit: with both, doctor reports the bloat check as INFO. Every other check, + # and the rest of verify:adf, still blocks. + SCOPE_FILE=$(git rev-parse --git-path ${HOOK_SCOPED_DOCTOR_FILE}) + SCOPE_TOKEN="$$-$(date +%s)" + trap 'rm -f "$SCOPE_FILE"' EXIT + trap 'rm -f "$SCOPE_FILE"; exit 1' HUP INT TERM + if printf '%s\\n' "$SCOPE_TOKEN" > "$SCOPE_FILE" 2>/dev/null; then + ${HOOK_SCOPED_DOCTOR_ENV}="$SCOPE_TOKEN" + export ${HOOK_SCOPED_DOCTOR_ENV} + fi + "$PM" run verify:adf || { echo '' echo '[pre-commit] ADF verification FAILED.' - echo ' Run: pnpm run verify:adf' + echo " Run: $PM run verify:adf (outside this hook, its doctor also checks vendor bloat in files the commit leaves out)" exit 1 } else - # --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 || { + npx charter adf evidence --auto-measure --ci || { echo '' echo '[pre-commit] ADF verification FAILED.' - 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' + echo ' Run: npx charter adf evidence --auto-measure --ci' exit 1 } fi @@ -489,7 +524,9 @@ function printHelp(): void { 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. 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(' staged files tidy did not skip. If package.json has a verify:adf script, it runs next,'); + console.log(' with the package manager the lockfile names (npm if none), and its doctor leaves'); + console.log(' vendor bloat to that scoped check. 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.'); From 13840730f60b69d30ce0fa7381907322247265ab Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Sat, 17 Oct 2026 04:49:34 -0500 Subject: [PATCH 2/2] fix(hook): per-run scope token, clear fallback notice, accurate gate INFO (refs #357) Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/cli-reference.md | 14 ++- packages/cli/src/__tests__/hook.test.ts | 18 ++- .../integration/doctor-staged-bloat.test.ts | 35 ++++-- .../integration/precommit-hook.test.ts | 103 +++++++++++++++++- .../src/commands/doctor-gate-enforcement.ts | 20 +++- packages/cli/src/commands/doctor.ts | 14 ++- packages/cli/src/commands/hook-scope.ts | 19 +++- packages/cli/src/commands/hook.ts | 12 +- 9 files changed, 195 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ad1b7..df33b8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,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. -- **The pre-commit hook's commit-scoped gate now also applies in repos with a `verify:adf` script, and runs that script with the repo's package manager** (`#357`) — re-run `charter hook install --pre-commit` to pick this up. The scoped gate above applied only when `package.json` had no `verify:adf` script. `charter setup` and `charter bootstrap` add one, so in most Node repos the hook ran `pnpm run verify:adf` instead, and that script's `charter doctor --adf-only --ci` still blocked a commit on bloat in vendor files it did not include. The hook also called `pnpm` by name, so the gate failed in npm and yarn repos on machines without pnpm. The hook now always runs the scoped doctor first, then `verify:adf` if the script exists, or `adf evidence` if it does not. It runs `verify:adf` with `pnpm` when there is a `pnpm-lock.yaml` and pnpm is installed, with `yarn` when there is a `yarn.lock` and yarn is installed, and with `npm` otherwise. While `verify:adf` runs, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token and writes the same token to a file in the git directory, which it deletes when it exits. Doctor skips the `adf vendor bloat` check only when the variable and the file match. It then reports the check as an INFO row, `adf vendor bloat (checked by pre-commit hook)`, and prints a line saying so on stderr. Every other doctor check in `verify:adf` still blocks, and so do the script's other commands. Setting the variable without the file changes nothing, so a CI job that sets it by mistake still gets a full `charter doctor --ci`. `charter doctor` now also reports a hook installed with the earlier `#357` fix with an INFO `pre-commit hook` row. +- **The pre-commit hook's commit-scoped gate now also applies in repos with a `verify:adf` script, and runs that script with the repo's package manager** (`#357`) — re-run `charter hook install --pre-commit` to pick this up. The scoped gate above applied only when `package.json` had no `verify:adf` script. `charter setup` and `charter bootstrap` add one, so in most Node repos the hook ran `pnpm run verify:adf` instead, and that script's `charter doctor --adf-only --ci` still blocked a commit on bloat in vendor files it did not include. The hook also called `pnpm` by name, so the gate failed in npm and yarn repos on machines without pnpm. The hook now always runs the scoped doctor first, then `verify:adf` if the script exists, or `adf evidence` if it does not. This means a repo with a `verify:adf` script now gets `npx charter doctor --adf-only --ci --staged …` on every commit, even when its custom `verify:adf` leaves doctor out. It runs `verify:adf` with `pnpm` when there is a `pnpm-lock.yaml` and pnpm is installed, with `yarn` when there is a `yarn.lock` and yarn is installed, and with `npm` otherwise. While `verify:adf` runs, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token and writes the same token to its own file in the git directory, named after the token, which it deletes when it exits. Two commits running at once in one worktree therefore never delete each other's file. Doctor skips the `adf vendor bloat` check only when the variable and that file match. If the hook cannot write the file, it prints one `[pre-commit]` line saying so, and the doctor inside `verify:adf` checks every vendor file that time, as CI does. It then reports the check as an INFO row, `adf vendor bloat (checked by pre-commit hook)`, and prints a line saying so on stderr. Every other doctor check in `verify:adf` still blocks, and so do the script's other commands. Setting the variable without the file changes nothing, so a CI job that sets it by mistake still gets a full `charter doctor --ci`. `charter doctor` now also reports a hook installed with the earlier `#357` fix with an INFO `pre-commit hook` row, but only in a repo with a `verify:adf` script. Without one, that hook already runs the scoped gate. - **`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 5a021a6..72c0b1a 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -78,8 +78,8 @@ npx charter hook install --commit-msg --force # overwrite existing hooks - 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 --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. - - The scoped doctor runs in every repo. Next, if `package.json` has a `verify:adf` script (`charter setup` and `charter bootstrap` add one), the gate runs it; otherwise it runs `charter adf evidence --auto-measure --ci`. `verify:adf` runs with `pnpm` when the repo has a `pnpm-lock.yaml` and pnpm is installed, with `yarn` for a `yarn.lock` and yarn installed, and with `npm` otherwise, so pnpm is never required. The doctor inside `verify:adf` would check vendor bloat across the whole tree again. To prevent that, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token for that run and writes the token to a file in the git directory, which it removes on exit. When the two match, doctor reports `adf vendor bloat (checked by pre-commit hook)` as INFO and prints a line on stderr. All its other checks still block, and so do the script's other commands. The variable without the matching file does nothing, so `charter doctor --ci` in CI keeps full strictness even if the variable is set there. Run by hand, `npm run verify:adf` checks every vendor file, as CI does. - - 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, and so, in a repo with a `verify:adf` script, does one from the first #357 fix, which also ran `pnpm run verify:adf` by name. Re-run `charter hook install --pre-commit` to replace any of them (no `--force` needed for a Charter-managed hook). `charter doctor` reports an old hook as INFO. + - The scoped doctor runs in every repo, including one whose custom `verify:adf` leaves doctor out: such a repo now gets `npx charter doctor --adf-only --ci --staged …` on every commit. Next, if `package.json` has a `verify:adf` script (`charter setup` and `charter bootstrap` add one), the gate runs it; otherwise it runs `charter adf evidence --auto-measure --ci`. `verify:adf` runs with `pnpm` when the repo has a `pnpm-lock.yaml` and pnpm is installed, with `yarn` for a `yarn.lock` and yarn installed, and with `npm` otherwise, so pnpm is never required. The doctor inside `verify:adf` would check vendor bloat across the whole tree again. To prevent that, the hook sets `CHARTER_HOOK_SCOPED_DOCTOR` to a one-time token for that run and writes the token to its own file in the git directory (`charter-hook-scoped-doctor-`, one per hook run, so overlapping commits in one worktree do not interfere), which it removes on exit. If the file cannot be written, the hook prints one `[pre-commit]` line saying so, and that time the doctor inside `verify:adf` checks every vendor file. When the two match, doctor reports `adf vendor bloat (checked by pre-commit hook)` as INFO and prints a line on stderr. All its other checks still block, and so do the script's other commands. The variable without the matching file does nothing, so `charter doctor --ci` in CI keeps full strictness even if the variable is set there. Run by hand, `npm run verify:adf` checks every vendor file, as CI does. + - 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, and so, in a repo with a `verify:adf` script, does one from the first #357 fix, which also ran `pnpm run verify:adf` by name. Re-run `charter hook install --pre-commit` to replace any of them (no `--force` needed for a Charter-managed hook). `charter doctor` reports an old hook as INFO. It reports a hook from the first #357 fix only where `package.json` has a `verify:adf` script, since without one that hook already runs the scoped gate. - `--force` — overwrite existing non-Charter hooks Must specify at least one of `--commit-msg` or `--pre-commit`. @@ -161,7 +161,7 @@ npx charter doctor --ci --format json # CI mode: exit 1 on warnings - `--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). -- `CHARTER_HOOK_SCOPED_DOCTOR` (environment) — set only by the generated pre-commit hook while it runs `verify:adf`, after its own `--staged` doctor. Doctor honours it only when its value matches the token the hook wrote to `charter-hook-scoped-doctor` in the git directory. It then reports the `adf vendor bloat` check as INFO (`adf vendor bloat (checked by pre-commit hook)`) and says so on stderr. No other check changes. The variable on its own (no matching file) has no effect, and `--staged` takes precedence over it. +- `CHARTER_HOOK_SCOPED_DOCTOR` (environment) — set only by the generated pre-commit hook while it runs `verify:adf`, after its own `--staged` doctor. Doctor honours it only when its value has the hook's token form (`-`) and the file `charter-hook-scoped-doctor-` in the git directory holds that token. It then reports the `adf vendor bloat` check as INFO (`adf vendor bloat (checked by pre-commit hook)`) and says so on stderr. No other check changes. The variable on its own (no matching file) has no effect, and `--staged` takes precedence over it. 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 @@ -176,9 +176,11 @@ tidy ([#320](https://github.com/Stackbilt-dev/charter/issues/320)). That hook re 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, and for a hook from the first #357 fix, -whose gate stayed unscoped in repos with a `verify:adf` script and ran it with -`pnpm` by name; the fix is the same. A current, foreign +bloat in files the commit does not include. It also appears for a hook from the +first #357 fix when `package.json` has a `verify:adf` script, because that hook's +gate stays unscoped there and runs the script with `pnpm` by name. Without the +script, that hook already runs the scoped gate, so it gets no row. The fix in +each case 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/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 60f7b8d..8318a57 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -353,11 +353,12 @@ fi expect(HOOK_360).toContain(PRE_COMMIT_GATE_SCOPE_MARKER); expect(HOOK_360).not.toContain(PRE_COMMIT_VERIFY_SCOPE_MARKER); writeHook(path.join(tempDir, '.git', 'hooks'), HOOK_360); + fs.writeFileSync(path.join(tempDir, 'package.json'), JSON.stringify({ scripts: { 'verify:adf': 'charter doctor --adf-only --ci' } })); 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 gate for repos with a verify:adf script: there it runs pnpm run verify:adf instead, whose doctor blocks a commit on vendor bloat in files the commit does not include, and which fails where pnpm is not installed. Re-run: charter hook install --pre-commit', + details: 'The Charter pre-commit hook at .git/hooks/pre-commit predates the commit-scoped gate for repos with a verify:adf script, as this one has: it runs pnpm run verify:adf in place of that gate, so its doctor blocks a commit on vendor bloat in files the commit does not include, and the gate fails where pnpm is not installed. Re-run: charter hook install --pre-commit', }]); vi.spyOn(console, 'log').mockImplementation(() => {}); @@ -366,6 +367,15 @@ fi expect((await doctorChecks()).some(c => c.name === 'pre-commit hook')).toBe(false); }); + it.each([ + ['no package.json', null], + ['a package.json without verify:adf', JSON.stringify({ scripts: { test: 'vitest' } })], + ])('#357: no row for a #360-era hook with %s (its gate is already scoped there)', async (_label, pkg) => { + writeHook(path.join(tempDir, '.git', 'hooks'), HOOK_360); + if (pkg !== null) fs.writeFileSync(path.join(tempDir, 'package.json'), pkg); + expect((await doctorChecks()).some(c => c.name === 'pre-commit hook')).toBe(false); + }); + it('#357: the generated gate always runs the scoped doctor, then verify:adf (with the token) or evidence', () => { const gate = PRE_COMMIT_HOOK_CONTENT.slice(PRE_COMMIT_HOOK_CONTENT.indexOf('\n# ADF evidence gate')); expect(gate).toContain(PRE_COMMIT_GATE_SCOPE_MARKER); @@ -377,7 +387,11 @@ fi expect(branchAt).toBeGreaterThan(doctorAt); expect(gate).toContain(' "$PM" run verify:adf || {'); expect(gate).not.toContain('pnpm run verify:adf'); - expect(gate).toContain(`SCOPE_FILE=$(git rev-parse --git-path ${HOOK_SCOPED_DOCTOR_FILE})`); + // One file per hook run, named after its token; a failing rev-parse never aborts the hook. + expect(gate).toContain(`SCOPE_FILE=$(git rev-parse --git-path "${HOOK_SCOPED_DOCTOR_FILE}-$SCOPE_TOKEN" 2>/dev/null) || SCOPE_FILE=""`); + // The braces put the redirection's own error under 2>/dev/null too. + expect(gate).toContain(`if [ -n "$SCOPE_FILE" ] && { printf '%s\\n' "$SCOPE_TOKEN" > "$SCOPE_FILE"; } 2>/dev/null; then`); + expect(gate).toContain('[pre-commit] Could not write the scope token in the git dir'); expect(gate).toContain(` ${HOOK_SCOPED_DOCTOR_ENV}="$SCOPE_TOKEN"\n export ${HOOK_SCOPED_DOCTOR_ENV}\n`); expect(gate).toContain(`trap 'rm -f "$SCOPE_FILE"' EXIT`); expect(gate).toContain(' npx charter adf evidence --auto-measure --ci || {'); diff --git a/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts index 3679f41..8cb943a 100644 --- a/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts +++ b/packages/cli/src/__tests__/integration/doctor-staged-bloat.test.ts @@ -195,7 +195,9 @@ describe('doctor --staged', () => { describe(`${HOOK_SCOPED_DOCTOR_ENV}: the hook's verify:adf run leaves bloat to the scoped gate`, () => { const TOKEN = '4242-1700000000'; - const tokenFile = (): string => path.join(tmp, '.git', HOOK_SCOPED_DOCTOR_FILE); + const OTHER = '4243-1700000001'; + /** The per-run file for a token (the hook names it after the token). */ + const tokenFile = (token: string = TOKEN): string => path.join(tmp, '.git', `${HOOK_SCOPED_DOCTOR_FILE}-${token}`); const UNSCOPED_WARN = { name: 'adf vendor bloat', status: 'WARN', details: `Vendor file bloat detected:\n ${CLAUDE_BLOAT}\n ${GEMINI_BLOAT}\n Run: charter adf tidy --dry-run` }; async function withEnv(value: string | undefined, fn: () => Promise): Promise { @@ -221,14 +223,21 @@ describe(`${HOOK_SCOPED_DOCTOR_ENV}: the hook's verify:adf run leaves bloat to t } } - it.each([ - ['set to 1 with no token file (a CI misconfiguration)', '1', null], - ['set to the token with no token file', TOKEN, null], - ['set, with a token file holding a different token (a stale file)', '1', TOKEN], - ['unset, with a token file present', undefined, TOKEN], - ])('%s: full strictness, exactly as without it', async (_label, value, fileToken) => { + it.each<[string, string | undefined, Array<[string, string]>]>([ + ['set to 1 with no token file (a CI misconfiguration)', '1', []], + ['set to the token with no token file', TOKEN, []], + ['set to the token, with only another run\'s file present', TOKEN, [[`${HOOK_SCOPED_DOCTOR_FILE}-${OTHER}`, OTHER]]], + ['set to the token, with its file holding a different token', TOKEN, [[`${HOOK_SCOPED_DOCTOR_FILE}-${TOKEN}`, OTHER]]], + ['set to the token, with only the old shared file name holding it', TOKEN, [[HOOK_SCOPED_DOCTOR_FILE, TOKEN]]], + ['set to a path-like value whose target holds it', '../x', [['../x', '../x'], [`${HOOK_SCOPED_DOCTOR_FILE}-../x`, '../x']]], + ['unset, with a token file present', undefined, [[`${HOOK_SCOPED_DOCTOR_FILE}-${TOKEN}`, TOKEN]]], + ])('%s: full strictness, exactly as without it', async (_label, value, files) => { const baseline = await withEnv(undefined, () => doctor(['--adf-only'])); - if (fileToken !== null) fs.writeFileSync(tokenFile(), `${fileToken}\n`); + for (const [name, content] of files) { + const p = path.join(tmp, '.git', name); + if (name.includes('/')) fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, `${content}\n`); + } const r = await withEnv(value, () => doctorWithStderr(['--adf-only'])); expect(r.code).toBe(EXIT_CODE.POLICY_VIOLATION); expect(bloatRows(r.checks)).toEqual([UNSCOPED_WARN]); @@ -248,6 +257,16 @@ describe(`${HOOK_SCOPED_DOCTOR_ENV}: the hook's verify:adf run leaves bloat to t expect(r.stderr).toEqual([BLOAT_SCOPE_MSG.hookCheckedStderr]); }); + it('two hook runs\' files side by side: each token relaxes with its own file, and loses it when only that file goes', async () => { + fs.writeFileSync(tokenFile(TOKEN), `${TOKEN}\n`); + fs.writeFileSync(tokenFile(OTHER), `${OTHER}\n`); + expect((await withEnv(TOKEN, () => doctorWithStderr(['--adf-only']))).code).toBe(EXIT_CODE.SUCCESS); + expect((await withEnv(OTHER, () => doctorWithStderr(['--adf-only']))).code).toBe(EXIT_CODE.SUCCESS); + fs.rmSync(tokenFile(OTHER)); + expect((await withEnv(TOKEN, () => doctorWithStderr(['--adf-only']))).code).toBe(EXIT_CODE.SUCCESS); + expect((await withEnv(OTHER, () => doctorWithStderr(['--adf-only']))).code).toBe(EXIT_CODE.POLICY_VIOLATION); + }); + it('with the matching token file, every other WARN (malformed VENDORS, missing default module) still fails --ci', async () => { fs.appendFileSync('.ai/manifest.adf', '\n\u{1F3F7} VENDORS:\n - CLAUDE.md: bogus\n'); fs.rmSync('.ai/state.adf'); diff --git a/packages/cli/src/__tests__/integration/precommit-hook.test.ts b/packages/cli/src/__tests__/integration/precommit-hook.test.ts index ecf803e..a64aa29 100644 --- a/packages/cli/src/__tests__/integration/precommit-hook.test.ts +++ b/packages/cli/src/__tests__/integration/precommit-hook.test.ts @@ -17,7 +17,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { execFileSync, spawnSync } from 'node:child_process'; +import { execFileSync, spawn, spawnSync } from 'node:child_process'; import { afterEach, describe, expect, it } from 'vitest'; import { HOOK_SCOPED_DOCTOR_FILE, PRE_COMMIT_HOOK_CONTENT } from '../../commands/hook'; import { BLOAT_SCOPE_MSG } from '../../commands/doctor'; @@ -40,6 +40,10 @@ interface Repo { commit(message: string, env?: Record): { status: number | null; output: string }; /** Lines the failing pnpm shim logged: one per time the hook ran pnpm. */ pnpmCalls(): string[]; + /** A directory on the hook's PATH ahead of everything else. */ + binDir: string; + /** Run the pre-commit hook alone (`git hook run`), asynchronously. */ + runHook(env?: Record, direct?: boolean): Promise<{ status: number | null; output: string }>; } /** Bootstrapped repos get this verify:adf, the one setup writes, with `charter` pointed at the shim. */ @@ -193,9 +197,23 @@ function writeHookFixtureRepo(label: string, opts: FixtureOpts = {}): Repo { calls: () => fs.readFileSync(log, 'utf-8').split('\n').filter(Boolean), commit: (message, env = {}) => commit(tmp, message, { ...pathEnv, ...env }), pnpmCalls: () => fs.readFileSync(pnpmLog, 'utf-8').split(/\r?\n/).filter(Boolean), + binDir, + runHook: (env = {}, direct = false) => new Promise(resolve => { + // `git hook run` runs the hook as `git commit` would, without committing, so two can overlap. + // direct: plain `sh`, so binDir really comes first on PATH (git puts its own dirs ahead of it). + const [cmd, args] = direct ? ['sh', ['.githooks/pre-commit']] : ['git', ['hook', 'run', 'pre-commit']]; + const child = spawn(cmd, args, { cwd: tmp, env: { ...process.env, ...GIT_ENV, ...pathEnv, ...env } }); + let output = ''; + child.stdout.on('data', (d: Buffer) => { output += d.toString(); }); + child.stderr.on('data', (d: Buffer) => { output += d.toString(); }); + child.on('close', status => resolve({ status, output })); + }), }; } +/** The hook's per-run scope token files left in the git dir. */ +const scopeFiles = (dir: string): string[] => fs.readdirSync(path.join(dir, '.git')).filter(n => n.startsWith(HOOK_SCOPED_DOCTOR_FILE)); + /** 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')); @@ -513,7 +531,7 @@ ${DB_BLOAT}`); expect(output).toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); expect(output).toContain('"name": "adf vendor bloat (checked by pre-commit hook)"'); expect(repo.pnpmCalls()).toEqual([]); - expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + expect(scopeFiles(repo.dir)).toEqual([]); } /** The scoped doctor blocked, so nothing after it ran. */ @@ -747,7 +765,7 @@ ${DB_BLOAT}`); expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); expect(result.output).toContain(' Run: npm run verify:adf (outside this hook'); expect(repo.pnpmCalls()).toEqual([]); - expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + expect(scopeFiles(repo.dir)).toEqual([]); expect(head(repo.dir)).toBe(before); }); @@ -779,7 +797,7 @@ ${DB_BLOAT}`); expect(result.status).toBe(1); expect(repo.pnpmCalls()).toEqual(['pnpm run verify:adf']); expect(result.output).toContain(' Run: pnpm run verify:adf (outside this hook'); - expect(fs.existsSync(path.join(repo.dir, '.git', HOOK_SCOPED_DOCTOR_FILE))).toBe(false); + expect(scopeFiles(repo.dir)).toEqual([]); }); it('verify:adf run by hand, outside the hook, still blocks on the unstaged bloat (as CI does)', { timeout: 60000 }, () => { @@ -795,6 +813,83 @@ ${DB_BLOAT}`); expect(r.stdout).toContain('"status": "WARN"'); expect(`${r.stdout}${r.stderr}`).not.toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); }); + + const SCOPE_NOTICE = '[pre-commit] Could not write the scope token in the git dir, so this time the doctor inside verify:adf checks vendor bloat in every file, as CI does.'; + + /** A `git` on the hook's PATH that answers the token's rev-parse its own way and passes everything else through. */ + function shimGitPath(repo: Repo, answer: string): void { + const realGit = execFileSync('sh', ['-c', 'command -v git'], { encoding: 'utf-8' }).trim(); + fs.writeFileSync(path.join(repo.binDir, 'git'), [ + '#!/bin/sh', + 'case "$*" in', + ` "rev-parse --git-path ${HOOK_SCOPED_DOCTOR_FILE}-"*) ${answer} ;;`, + 'esac', + `exec "${realGit}" "$@"`, + '', + ].join('\n')); + fs.chmodSync(path.join(repo.binDir, 'git'), 0o755); + } + + it.each([ + ['the token file cannot be written (its directory does not exist)', 'echo "/nonexistent-charter-dir/sub/token"; exit 0'], + ['git rev-parse --git-path fails', 'echo "fatal: simulated" >&2; exit 128'], + ])('when %s: one clear notice, no shell error, the hook carries on and verify:adf runs strict', { timeout: 60000 }, async (_label, answer) => { + const repo = writeHookFixtureRepo('verify-unwritable', { verifyAdf: { lockfile: 'package-lock.json' } }); + shimGitPath(repo, answer); + withUnstagedGeminiBloat(repo.dir); + const before = head(repo.dir); + + // Run directly under sh: git would put its own git ahead of the shim on the hook's PATH. + const result = await repo.runHook({}, true); + + expect(result.output).toContain(SCOPE_NOTICE); + expect(result.output).not.toMatch(/cannot create|Directory nonexistent|No such file or directory|fatal: simulated/); + // The hook did not abort: verify:adf ran, and its doctor was strict, so the unstaged bloat blocks (as CI would). + expect(repo.calls()).toContain('doctor --adf-only --ci --format json'); + expect(result.output).not.toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); + expect(result.output).toContain('"name": "adf vendor bloat",'); + expect(result.status).toBe(1); + expect(result.output).toContain('[pre-commit] ADF verification FAILED.'); + expect(head(repo.dir)).toBe(before); + expect(scopeFiles(repo.dir)).toEqual([]); + }); + + it('two overlapping hook runs in one worktree each keep their own token file, and both relax', { timeout: 120000 }, async () => { + // verify:adf first sleeps for HOOK_TEST_DELAY_MS, then lists the scope files it can see. + const helperDir = makeTempDir('verify-overlap-helper'); + const delay = path.join(helperDir, 'delay.cjs'); + fs.writeFileSync(delay, [ + `const fs = require('fs');`, + `setTimeout(() => {`, + ` const names = fs.readdirSync('.git').filter(n => n.startsWith(${JSON.stringify(HOOK_SCOPED_DOCTOR_FILE)}));`, + ` console.log('SCOPE FILES SEEN: ' + names.length);`, + `}, Number(process.env.HOOK_TEST_DELAY_MS || 0));`, + '', + ].join('\n')); + const repo = writeHookFixtureRepo('verify-overlap', { + verifyAdf: { lockfile: 'package-lock.json', script: `node "${posix(delay)}" && ${verifyAdfScript('{CLI}')}` }, + }); + withUnstagedGeminiBloat(repo.dir); + + const slow = repo.runHook({ HOOK_TEST_DELAY_MS: '8000' }); + // Start the second run only once the first has written its token file. + const deadline = Date.now() + 60000; + while (scopeFiles(repo.dir).length === 0 && Date.now() < deadline) await new Promise(r => setTimeout(r, 100)); + expect(scopeFiles(repo.dir)).toHaveLength(1); + const fast = await repo.runHook(); + // The fast run finished (its trap ran) while the slow one still sleeps: only the slow run's file remains. + expect(scopeFiles(repo.dir)).toHaveLength(1); + const slowResult = await slow; + + expect(fast.output).toContain('SCOPE FILES SEEN: 2'); + for (const r of [fast, slowResult]) { + expect(r.output).toContain(BLOAT_SCOPE_MSG.hookCheckedStderr); + expect(r.output).not.toContain(SCOPE_NOTICE); + expect(r.status).toBe(0); + } + expect(slowResult.output).toContain('SCOPE FILES SEEN: 1'); + expect(scopeFiles(repo.dir)).toEqual([]); + }); }); it('a gitignored .ai/ (local-only setup) is not WIP: tidy runs, ignored modules are reported and never staged', { timeout: 60000 }, () => { diff --git a/packages/cli/src/commands/doctor-gate-enforcement.ts b/packages/cli/src/commands/doctor-gate-enforcement.ts index d8468d4..b34389b 100644 --- a/packages/cli/src/commands/doctor-gate-enforcement.ts +++ b/packages/cli/src/commands/doctor-gate-enforcement.ts @@ -50,14 +50,18 @@ 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'; -export const DOCTOR_VERIFY_ADF_GATE_MSG = 'The Charter pre-commit hook at {hookPath} predates the commit-scoped gate for repos with a verify:adf script: there it runs pnpm run verify:adf instead, whose doctor blocks a commit on vendor bloat in files the commit does not include, and which fails where pnpm is not installed. Re-run: charter hook install --pre-commit'; +export const DOCTOR_VERIFY_ADF_GATE_MSG = 'The Charter pre-commit hook at {hookPath} predates the commit-scoped gate for repos with a verify:adf script, as this one has: it runs pnpm run verify:adf in place of that gate, so its doctor blocks a commit on vendor bloat in files the commit does not include, and the gate fails where pnpm is not installed. 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), or - * from #320 but before #357 (no PRE_COMMIT_GATE_SCOPE_MARKER), or from #360, - * which left the gate unscoped in repos with a verify:adf script (no - * PRE_COMMIT_VERIFY_SCOPE_MARKER). + * from #320 but before #357 (no PRE_COMMIT_GATE_SCOPE_MARKER), or from #360 + * (no PRE_COMMIT_VERIFY_SCOPE_MARKER) in a repo whose package.json has a + * verify:adf script. A #360 hook without that script already runs the scoped + * doctor and then adf evidence, exactly as the current hook does there, so it + * gets no row: there is nothing to fix. The script test is the hook's own + * (`grep '"verify:adf"' package.json` at the repo root), so the row appears as + * soon as the hook would take the unscoped path. * 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). @@ -68,7 +72,7 @@ export function checkPreCommitScope(): PreCommitScopeCheck | 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 - : !content.includes(PRE_COMMIT_VERIFY_SCOPE_MARKER) ? DOCTOR_VERIFY_ADF_GATE_MSG + : !content.includes(PRE_COMMIT_VERIFY_SCOPE_MARKER) && repoHasVerifyAdfScript() ? DOCTOR_VERIFY_ADF_GATE_MSG : undefined; if (!message) return undefined; return { @@ -78,6 +82,12 @@ export function checkPreCommitScope(): PreCommitScopeCheck | undefined { }; } +/** The hook's own test: package.json at the repo root mentions "verify:adf". */ +function repoHasVerifyAdfScript(): boolean { + const repoRoot = getGitPath(['rev-parse', '--show-toplevel']) || process.cwd(); + return readText(path.join(repoRoot, 'package.json')).includes('"verify:adf"'); +} + 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 51bfa6e..84fd490 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -20,7 +20,7 @@ import { countExtractable } from './adf-tidy'; 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 { HOOK_SCOPED_DOCTOR_ENV, HOOK_SCOPED_DOCTOR_FILE } from './hook-scope'; +import { HOOK_SCOPED_DOCTOR_ENV, hookScopeFileName } from './hook-scope'; import { findMissingDefaultLoad, formatMissingDefaultLoad, REQUIRED_DEFAULT_LOAD_MODULES } from './adf-wiring'; interface DoctorResult { @@ -404,16 +404,18 @@ function resolveBloatScope(args: string[]): BloatScope | null { /** * True only while the generated pre-commit hook runs verify:adf, after its own * doctor --staged has checked vendor bloat for this commit. The hook sets - * HOOK_SCOPED_DOCTOR_ENV to a token and writes the same token to - * HOOK_SCOPED_DOCTOR_FILE in the git dir, removing it when the hook exits. Both + * HOOK_SCOPED_DOCTOR_ENV to a token and writes the same token to its own file, + * hookScopeFileName(token), in the git dir, removing it when the hook exits. Both * must match: the variable alone (say, left in a CI environment) does nothing, - * and while it is unset git is not even asked. Only the bloat check is affected. + * a value that is not a hook token never becomes a path, and while the variable + * is unset git is not even asked. Only the bloat check is affected. */ function hookAlreadyCheckedBloat(): boolean { const token = process.env[HOOK_SCOPED_DOCTOR_ENV]?.trim(); - if (!token) return false; + const name = token ? hookScopeFileName(token) : undefined; + if (!token || !name) return false; try { - const file = runGit(['rev-parse', '--git-path', HOOK_SCOPED_DOCTOR_FILE]).trim(); + const file = runGit(['rev-parse', '--git-path', name]).trim(); return fs.readFileSync(file, 'utf-8').trim() === token; } catch { return false; diff --git a/packages/cli/src/commands/hook-scope.ts b/packages/cli/src/commands/hook-scope.ts index 776c28d..a530770 100644 --- a/packages/cli/src/commands/hook-scope.ts +++ b/packages/cli/src/commands/hook-scope.ts @@ -3,11 +3,20 @@ * imports so doctor can read it without loading hook.ts and its import graph. * * HOOK_SCOPED_DOCTOR_ENV is set only by the generated hook while it runs - * verify:adf. Its value is a token the hook also writes to HOOK_SCOPED_DOCTOR_FILE - * in the git dir and removes on exit; doctor skips the vendor bloat check - * (reported as INFO) only when the two match, so the variable alone, as a CI - * misconfiguration might set it, does nothing. See hookAlreadyCheckedBloat in doctor.ts. + * verify:adf. Its value is a token; the hook also writes it to the file + * hookScopeFileName(token) in the git dir, one file per hook run, and removes + * that file on exit. Doctor skips the vendor bloat check (reported as INFO) only + * when the file for the token exists and holds it, so the variable alone, as a + * CI misconfiguration might set it, does nothing. See hookAlreadyCheckedBloat in doctor.ts. */ export const HOOK_SCOPED_DOCTOR_ENV = 'CHARTER_HOOK_SCOPED_DOCTOR'; -/** File name passed to `git rev-parse --git-path`. */ +/** File name prefix passed to `git rev-parse --git-path`; the token follows a dash. */ export const HOOK_SCOPED_DOCTOR_FILE = 'charter-hook-scoped-doctor'; + +/** The hook's tokens are `-`; anything else is rejected, never used as a path. */ +const TOKEN_PATTERN = /^[0-9]{1,10}-[0-9]{1,12}$/; + +/** The per-run file name for a token, or undefined when the token is not one the hook makes. */ +export function hookScopeFileName(token: string): string | undefined { + return TOKEN_PATTERN.test(token) ? `${HOOK_SCOPED_DOCTOR_FILE}-${token}` : undefined; +} diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index 70b6640..73d99ef 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -269,16 +269,18 @@ if [ -f ".ai/manifest.adf" ]; then fi # The doctor inside verify:adf would re-check vendor bloat in every file. The # scoped doctor above has checked it for this commit, so verify:adf gets a - # one-time token, matched against a file in the git dir that is removed on - # exit: with both, doctor reports the bloat check as INFO. Every other check, - # and the rest of verify:adf, still blocks. - SCOPE_FILE=$(git rev-parse --git-path ${HOOK_SCOPED_DOCTOR_FILE}) + # one-time token, matched against a file in the git dir named after it (one + # per hook run) and removed on exit: with both, doctor reports the bloat check + # as INFO. Every other check, and the rest of verify:adf, still blocks. SCOPE_TOKEN="$$-$(date +%s)" + SCOPE_FILE=$(git rev-parse --git-path "${HOOK_SCOPED_DOCTOR_FILE}-$SCOPE_TOKEN" 2>/dev/null) || SCOPE_FILE="" trap 'rm -f "$SCOPE_FILE"' EXIT trap 'rm -f "$SCOPE_FILE"; exit 1' HUP INT TERM - if printf '%s\\n' "$SCOPE_TOKEN" > "$SCOPE_FILE" 2>/dev/null; then + if [ -n "$SCOPE_FILE" ] && { printf '%s\\n' "$SCOPE_TOKEN" > "$SCOPE_FILE"; } 2>/dev/null; then ${HOOK_SCOPED_DOCTOR_ENV}="$SCOPE_TOKEN" export ${HOOK_SCOPED_DOCTOR_ENV} + else + echo "[pre-commit] Could not write the scope token in the git dir, so this time the doctor inside verify:adf checks vendor bloat in every file, as CI does." fi "$PM" run verify:adf || { echo ''