fix(hook): resolve Charter through the repo package manager in the pre-commit hook (#366)

.githooks/pre-commit

@@ -36,7 +36,14 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then
 
   if [ -n "$SOURCES" ]; then
     echo "[pre-commit] Checking staged vendor files:$NAMES"
-    PLAN=$(node packages/cli/dist/bin.js adf tidy --dry-run --print-paths $SOURCES 2>/dev/null) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; }
+    PLAN_OK=1
+    # stderr is kept: tidy explains there a file it leaves alone (say, an unmanaged one).
+    PLAN=$(node packages/cli/dist/bin.js adf tidy --dry-run --print-paths $SOURCES 2>&1) || { echo "[pre-commit] adf tidy --dry-run failed; skipping auto-tidy."; PLAN=""; PLAN_OK=0; }
+    NOTES=$(printf '%s\n' "$PLAN" | grep -v -E '^(vendor|module) ' | grep -v '^ *$') || NOTES=""
+    if [ -n "$NOTES" ]; then
+      echo "[pre-commit] adf tidy notes:"
+      printf '%s\n' "$NOTES"
+    fi
     if printf '%s\n' "$PLAN" | grep -E '^(vendor|module) ' >/dev/null; then
       BLOCKED=""
       IGNORED=""
@@ -82,7 +89,8 @@ if [ "${CHARTER_SKIP_TIDY:-0}" != "1" ] && [ -f ".ai/manifest.adf" ]; then
         fi
         echo "[pre-commit] Vendor files tidied; re-staged only the planned files tidy wrote."
       fi
-    else
+    elif [ "$PLAN_OK" = "1" ] && [ -z "$NOTES" ]; then
+      # A failed dry run, or one that left a file alone, has said so above.
       echo "[pre-commit] Vendor config files are clean."
     fi
   fi

CHANGELOG.md

@@ -16,6 +16,31 @@ The format is based on Keep a Changelog and follows Semantic Versioning.
 
 ### Fixed
 
+- **The generated pre-commit hook could not run Charter under Yarn Plug'n'Play, and could fetch an unrelated package** (`#366`). Every Charter call in the hook (vendor tidy, the scoped `doctor --staged` gate #365 added, `adf evidence`) was a bare `npx charter`. Under PnP there is no `node_modules/.bin`, so npx missed the installed CLI and fell back to the registry package `charter` (0.0.2, which has no binary), and the gate failed before `yarn run verify:adf` was reached. The same fallback fetched that package in any repo without Charter installed. The hook now picks the package manager once. It takes the first candidate whose marker exists and whose binary is on `PATH`, in this order:
+  1. Yarn Plug'n'Play (`.pnp.cjs`, `.pnp.js`, or `.yarnrc.yml` with `nodeLinker: pnp`) with `yarn`.
+  2. `pnpm-lock.yaml` with `pnpm`.
+  3. `yarn.lock` with `yarn`.
+  4. npm otherwise.
+
+  This is #365's `verify:adf` cascade with PnP in front, and `verify:adf` uses the same choice. When the first marker found has no binary on `PATH`, the hook warns on stderr with the runner it uses instead; under PnP the warning adds `corepack enable`.
+
+  How Charter runs depends on the package manager. A local install means `node_modules/.bin/charter` (or `charter.cmd`) exists; a global install means a `charter` on `PATH`.
+  - **Yarn Plug'n'Play:** always `yarn charter`. There is no `node_modules` to check, so a global install is never used.
+  - **Yarn with a `node_modules` linker:** `yarn charter` (`yarn --silent charter` on Yarn 1) with a local install. Otherwise a global `charter`, run directly, as 2.1.0's `npx charter` would have found it.
+  - **pnpm:** `pnpm exec charter`, which also finds a global install on `PATH`.
+  - **npm:** with a local install, `npm exec --no --no-update-notifier --fetch-retries=0 -- @stackbilt/cli`. It names Charter's own package, never the bare name `charter`, and makes no registry request. Otherwise a global `charter`, run directly.
+  - **Nothing found:** no Charter or package-manager call at all, so no network request. The hook says once to add `@stackbilt/cli` as a devDependency, skips tidy, and fails the gate with no `Run:` command.
+
+  The hook never installs a package or runs a downloaded one. The only network use left is Corepack downloading Yarn or pnpm on first use. Measured offline with npm 11.17, a commit took 6-7 s with a local install, 0 s with no install at all, and 2 s with a global one. The `Run:` hints print exactly the command the hook ran.
+
+  The hook runs `--version` once per commit, before any other Charter call, and keeps its stderr:
+  - **Not resolvable:** the devDependency hint, once.
+  - **Installed, but `--version` failed:** `Charter is installed but failed to run:` with the first error line, and no devDependency hint.
+  - **Output without `charter v<digit>`:** a warning that the `charter` found is not `@stackbilt/cli`.
+  - In each of those three cases, Charter is not called again.
+  - **1.x:** one warning to upgrade `@stackbilt/cli` to 2.x (as a devDependency, or your global install), since 1.x ignores `--staged` and `--print-paths`.
+
+  A doctor that ran and reported findings gets no devDependency hint. When tidy leaves a staged vendor file alone (for example, an unmanaged one), its note is shown under `[pre-commit] adf tidy notes:`. Neither that case nor a failed tidy dry run goes on to say "Vendor config files are clean." `charter doctor` and the hook now detect PnP from one shared source. The `CHARTER_HOOK_SCOPED_DOCTOR` token flow is unchanged. Scope: this fix covers the pre-commit hook only; the CI workflow `charter setup` generates is fixed separately (`#380`). **Installed hooks do not update themselves:** run `charter hook install --pre-commit` again (no `--force` needed for a Charter-managed hook). This is needed under Yarn PnP and recommended everywhere else. `charter doctor` reports an older hook as INFO in a PnP repo.
 - **The GitHub workflow that `charter setup --ci github` and `charter bootstrap --ci github` generate no longer runs a bare `npx charter`** (`#380`). When `@stackbilt/cli` was not installed, `npx charter` fetched the unrelated `charter` registry package (no executable, so the step failed with `could not determine executable to run`). Each step now runs the CLI the install step put in `node_modules`: `pnpm exec charter …` for pnpm, and `npm exec --no --package=@stackbilt/cli -- charter …` for npm. Neither form installs anything; without a local `@stackbilt/cli` the step fails. `setup` and `bootstrap` add it to `devDependencies`, except under `--no-dependency-sync` or in a repo with no root `package.json`. The npm form still contains `charter validate --ci`, so doctor's gate-enforcement check recognizes it. The template has no Yarn form (it supports npm and pnpm only), so a Yarn repo gets the npm form. The previous template (003ec8d, with `npx charter`) has the same permissions and triggers, so `setup`/`bootstrap` refresh it without a warning, as before. Its `KNOWN_WORKFLOWS` entry stays `safe` and records what the refresh changes.
 - **`charter adf compile --target claude --write --mode compiled` no longer carries the template's empty `## Environment` placeholder into the retained block** (`#364`). A `CLAUDE.md` written by any bootstrap since 0.3.3 (the 1.9.3 upgrade path included) holds `## Environment` with only the `<!-- Add runtime/OS/shell-specific notes here (not stack rules) -->` comment. When you had added a second `## Environment` of your own, converting the pointer to compiled output carried both, comment included, and reported `kept ## Environment, ## Environment`. Each retained section is now compared with the pointer templates on its own (ignoring CRLF, whitespace at line ends and blank lines around the body), so the untouched placeholder is dropped as template text and only your section is carried. A placeholder you wrote under is yours and is carried as before. A retained block that an earlier compile already wrote is copied verbatim as always: if yours holds the placeholder, delete it from the block by hand.
 - **`charter adf migrate` and `charter bootstrap` wrote the generic "This project uses [ADF]…" pointer instead of the named template when run from the CLI** (`#355`). This hit `GEMINI.md`, `AGENTS.md`, `.cursorrules` and `copilot-instructions.md` in the default mode, and `CLAUDE.md` too under `--replace`. `adf-migrate` built its template table when the module loaded, and the CLI loads it while `adf.ts` is still initialising (`index` → `doctor` → `adf` → `adf-migrate`), so every entry was `undefined`. The template is now looked up when the pointer is written. Unit tests that imported `adf-migrate` directly never saw the cycle.

docs/cli-reference.md

@@ -78,8 +78,40 @@ 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 "<files>"` ([#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 <file>` and `git add` the result.
-  - 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-<token>`, 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.
+  - The scoped doctor runs in every repo, including one whose custom `verify:adf` leaves doctor out: such a repo now gets `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 `<pm> run verify:adf`, where `<pm>` is the package manager described in the next item.
+  - Every Charter call in the generated hook (tidy, the scoped doctor, `adf evidence`) runs through the repo's package manager ([#366](https://github.com/Stackbilt-dev/charter/issues/366)), picked once at the top of the hook. The hook walks these candidates in order and takes the first one whose marker exists **and** whose binary is on `PATH`:
+    1. Yarn Plug'n'Play (`.pnp.cjs` or `.pnp.js` at the repo root, or a `.yarnrc.yml` line `nodeLinker: pnp`, optionally quoted) with `yarn` on `PATH`: Yarn.
+    2. `pnpm-lock.yaml` with `pnpm` on `PATH`: pnpm.
+    3. `yarn.lock` with `yarn` on `PATH`: Yarn.
+    4. Otherwise: npm.
+
+    So a repo with both lockfiles uses pnpm, or Yarn when only Yarn is installed. If the first marker found has no binary on `PATH`, the hook prints a warning on stderr naming the runner it uses instead. Under Plug'n'Play the warning adds `corepack enable`, since PnP packages load only through Yarn. `verify:adf` runs with the same package manager (`<pm> run verify:adf`).
+  - Charter then runs as exactly one of these commands, and the hook's `Run:` hints print the same command. A local install means `node_modules/.bin/charter` (or `charter.cmd`) exists; a global install means a `charter` on `PATH`.
+
+    | Package manager | Local install | Global install only | Neither |
+    |---|---|---|---|
+    | Yarn Plug'n'Play | `yarn charter` | `yarn charter` (no global fallback) | `yarn charter` |
+    | Yarn, `node_modules` linker | `yarn charter` (`yarn --silent charter` on Yarn 1) | `charter`, run directly | nothing |
+    | pnpm | `pnpm exec charter` | `pnpm exec charter` | nothing |
+    | npm | `npm exec --no --no-update-notifier --fetch-retries=0 -- @stackbilt/cli` | `charter`, run directly | nothing |
+
+    - **Yarn Plug'n'Play** has no `node_modules` to check, so the hook always runs `yarn charter`, which resolves only a dependency of the repo. A global install is not used there.
+    - **Yarn with a `node_modules` linker:** `yarn charter` would not find a global install, so the hook runs the global `charter` itself. Yarn 1 gets `--silent` so its run banner stays out of the output tidy parses.
+    - **pnpm:** `pnpm exec` looks in `node_modules/.bin` first and then on `PATH`, so it also finds a global install.
+    - **npm:** the command names Charter's own package, never the bare name `charter`. That is an unrelated registry package, which a plain `npx charter` fetches where Charter is not installed. With a local install it makes no registry request.
+    - **Nothing found** (no local install, and no global one where the table falls back to it): the hook calls neither Charter nor the package manager. It prints once that Charter could not be found and to add `@stackbilt/cli` as a devDependency, skips tidy, and fails the gate. It prints no `Run:` command, since none would work.
+  - The hook never installs a package or runs a downloaded one. The only network use left is Corepack downloading Yarn or pnpm itself the first time the hook calls one. Measured offline (registry unreachable, npm 11.17): a commit with a local install took 6-7 s, one with no install at all 0 s, and one with only a global install 2 s.
+  - **Once per commit,** before any other Charter call, the hook runs Charter's `--version` and keeps its stdout and stderr:
+    - **Exit 127, or no binary was found** (as under Plug'n'Play when Yarn cannot resolve it): the devDependency hint once, then as "nothing found" above.
+    - **A binary exists but `--version` fails:** `Charter is installed but failed to run:` and the first line of its error, with no devDependency hint. Tidy is skipped and the gate fails, still printing the `Run:` command so you can reproduce it.
+    - **Output with no `charter v<digit>` in it:** the hook warns that the `charter` it found is not `@stackbilt/cli`, runs nothing else, and fails the gate.
+    - **`charter v0.x` or `v1.x`:** one warning to upgrade `@stackbilt/cli` to 2.x (as a devDependency, or your global install). A 1.x CLI ignores `--staged` and `--print-paths`, so its tidy does nothing and its gate checks every file. The hook still runs it.
+    - When a gate step fails after a successful probe, the hook prints only the command to re-run. A doctor that ran and reported problems gets no devDependency hint.
+  - **Global installs** of any version are used as the table shows, so a global 1.x CLI gets the warning above. Install `@stackbilt/cli` as a devDependency, so the hook runs the version the repo pins.
+  - Tidy's own notes about a staged vendor file it leaves alone (for example, one not managed by Charter) are printed under `[pre-commit] adf tidy notes:`. The hook then does not say the vendor files are clean.
+  - On Windows the hook runs under Git for Windows' `sh`, which finds `yarn`, `pnpm` and `npm` as an extensionless shim (as npm and corepack install them) or an `.exe`. A package manager present only as a `.cmd` file is not found there, so the cascade skips it. A `package.json` script named `charter` shadows the binary for `yarn charter`.
+  - 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-<token>`, 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. A hook installed before #366 calls a bare `npx charter`. It fails under Yarn Plug'n'Play unless Charter is installed globally, and where Charter is not installed at all it fetches the unrelated `charter` package. Reinstalling is recommended in every repo and needed under Plug'n'Play, where `charter doctor` reports such a hook as INFO. Elsewhere `npx charter` still finds `node_modules/.bin`, so doctor shows no row for it.
 - `--force` — overwrite existing non-Charter hooks
 
 Must specify at least one of `--commit-msg` or `--pre-commit`.