diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 56f4f4741..ec76708c9 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -392,7 +392,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/skill-reconciler-diff`](../tools/skill-reconciler-diff/) | `substrate:framework-dev` | Deterministic structural diff between two skill trees — parses frontmatter, section headings, step inventory, placeholders, support files, and safety-baseline clauses into a JSON diff object for the `skill-reconciler` skill | | [`tools/vcs`](../tools/vcs/) | `contract:source-control` | Backend-dispatching implementation of the source-control (VCS) capability ([`tools/github/source-control.md`](../tools/github/source-control.md)); complete Git, Mercurial (Hg), Fossil, and Subversion (SVN) backends | | [`tools/sourcehut`](../tools/sourcehut/) | `contract:tracker` + `contract:source-control` + `contract:mail-archive` | SourceHut (sr.ht) forge bridge: todo.sr.ht, lists.sr.ht, builds.sr.ht, and git/hg repository reads | -| [`tools/hook-manager`](../tools/hook-manager/) | `contract:hook-manager` | Contract for git-hook managers under the secure agent setup: the install-target and home rules, and the registry of adapters (prek, overcommit and lefthook today) that the hook-manager shim, the whole-user dispatcher and `sandbox-lint` are generated from | +| [`tools/hook-manager`](../tools/hook-manager/) | `contract:hook-manager` | Contract for git-hook managers under the secure agent setup: the install-target and home rules, and the registry of adapters (prek, overcommit, lefthook and captainhook today) that the hook-manager shim, the whole-user dispatcher and `sandbox-lint` are generated from | | [`tools/tracker`](../tools/tracker/) | `contract:tracker` | Interface spec for the tracker contract's read-only contributor-activity queries (issues filed, triaged and commented on; issues created; first maintainer reply). Pure interface spec; GitHub and Jira implement it, and the rest of the contract is described by `tools/github/tool.md` | | [`tools/body-field-contract`](../tools/body-field-contract/) | `contract:body-field` | The body-field contract: how every reader and writer finds a tracker body's `### ` fields, with a language-neutral conformance suite run against every implementation. Pure interface spec; implementations: `tools/github-body-field/` (reference), `tools/vetted-ops/`, `tools/local-tracker/`, `tools/jira/` | | [`tools/security-tracker`](../tools/security-tracker/) | `contract:security-tracker` | Adapter contract for the security tracker the `security-*` skills read and write, selected by `tracker.platform`. Pure interface spec; backends: `tools/github/` (default), `tools/local-tracker/`, `tools/jira/` | diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 95604bf4c..9898cbd7b 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -67,19 +67,24 @@ - [Root cause](#root-cause-11) - [Fix](#fix-11) - [Notes](#notes-11) - - [Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time](#reads-of-claudemagpie-or-tmpclaude-uid-ask-for-approval-every-time) + - [CaptainHook's Composer plugin replaced the shared hook directory](#captainhooks-composer-plugin-replaced-the-shared-hook-directory) - [Symptom](#symptom-12) - [Root cause](#root-cause-12) - [Fix](#fix-12) - [Notes](#notes-12) - - [Every Bash call fails with `bwrap: Can't create file … Read-only file system`](#every-bash-call-fails-with-bwrap-cant-create-file--read-only-file-system) + - [Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time](#reads-of-claudemagpie-or-tmpclaude-uid-ask-for-approval-every-time) - [Symptom](#symptom-13) - [Root cause](#root-cause-13) - [Fix](#fix-13) - - [`git status` shows files you never created, such as `.mcp.json` or `.bash_profile`](#git-status-shows-files-you-never-created-such-as-mcpjson-or-bash_profile) + - [Notes](#notes-13) + - [Every Bash call fails with `bwrap: Can't create file … Read-only file system`](#every-bash-call-fails-with-bwrap-cant-create-file--read-only-file-system) - [Symptom](#symptom-14) - [Root cause](#root-cause-14) - [Fix](#fix-14) + - [`git status` shows files you never created, such as `.mcp.json` or `.bash_profile`](#git-status-shows-files-you-never-created-such-as-mcpjson-or-bash_profile) + - [Symptom](#symptom-15) + - [Root cause](#root-cause-15) + - [Fix](#fix-15) - [Adding a new entry](#adding-a-new-entry) @@ -1300,6 +1305,64 @@ The setup installs the shim only for a repository that uses lefthook: a root `le --- +## CaptainHook's Composer plugin replaced the shared hook directory + +### Symptom + +After a `composer install` or `composer update`, commits stop running the +repository's own hooks, or a repository that never had CaptainHook starts +failing on hooks it did not configure. +`~/.claude/git-hooks/` holds a CaptainHook-generated `pre-commit` and an +`old-hooks/` directory instead of the framework's dispatcher symlinks. + +### Root cause + +The `captainhook/hook-installer` Composer plugin (or the older +`captainhook/plugin-composer`) runs the install on every dependency update. +It does not go through `~/.claude/bin/captainhook`, so the shim cannot +point it at the repository's own hooks, and it writes into the shared +`core.hooksPath` directory behind the dispatcher. + +### Fix + +Disable the plugin in the repository's `composer.json`, reinstall the +dispatcher, and run CaptainHook's install through the shim once: + +```jsonc +// composer.json +"extra": { + "captainhook": { "disable-plugin": true } +} +``` + +```bash +# restore the dispatcher symlinks, then install into the repo's own hooks +cd ~/.claude/git-hooks +for h in post-checkout pre-commit prepare-commit-msg commit-msg \ + post-commit pre-push post-merge post-rewrite \ + pre-rebase pre-merge-commit; do + ln -sf git-hook-dispatcher.sh "$h" +done +~/.claude/bin/captainhook install +``` + +The shim falls back to `/vendor/bin/captainhook` when the +binary is not on `PATH`, so the last command works from anywhere in the +Composer project. A global `captainhook` on `PATH` wins over the project's +pinned one. + +### Notes + +- An install through the shim warns on stderr when `composer.json` requires + either plugin and neither `extra.captainhook.disable-plugin` nor + `config.allow-plugins` turns it off, so the next update does not undo the + fix silently. +- `setup-isolated-setup-verify` check 8 reports a repository with the + plugin enabled as a warning, and install Step P.3b offers to add + `disable-plugin` before installing the shim. + +--- + ## Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time ### Symptom diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index 750affe36..184cea18f 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -1457,17 +1457,33 @@ original argv + inherited stdin, so a failing local `pre-commit` / `pre-push` still aborts the git operation. A repo with **no** local hook is a clean no-op. -**The hook-manager PATH shim.** prek and overcommit honour `core.hooksPath`, -so a bare install under whole-user scope would write its hook into the -*shared* dir, not the repo. The framework ships a transparent shim +**The hook-manager PATH shim.** prek, overcommit and CaptainHook honour +`core.hooksPath`, so a bare install under whole-user scope would write its +hook into the *shared* dir, not the repo. The framework ships a transparent +shim ([`tools/agent-isolation/hook-manager-shim.sh`](../../tools/agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` with `~/.claude/bin` prepended to -PATH) that rewrites **only** the install subcommand — for prek, injecting -`--git-dir "$(git rev-parse --git-common-dir)"`; for overcommit, setting -`core.hooksPath` to `/hooks` — so the hook lands -in the repo-local `.git/hooks/` where the dispatcher finds it. +PATH) that rewrites **only** the install action — for prek, injecting +`--git-dir "$(git rev-parse --git-common-dir)"`; for overcommit and +CaptainHook, setting `core.hooksPath` to `/hooks` — +so the hook lands in the repo-local `.git/hooks/` where the dispatcher +finds it. +CaptainHook's Symfony Console CLI also accepts the command after global +options (`captainhook -n install`) or as an abbreviation (`captainhook +inst`), so the shim matches the action at any non-option argument that +prefixes it. Every other command passes straight through, and the shim is a no-op on hosts with no global `core.hooksPath`. +Composer keeps CaptainHook in `vendor/bin`, which is not on `PATH`, so +when the shim finds no real binary there it runs +`/vendor/bin/captainhook`; a global binary on `PATH` wins +over the project's pinned one, and a `vendor/bin` symlink back to the shim +is refused instead of exec'd. +CaptainHook's `captainhook/hook-installer` and `captainhook/plugin-composer` +Composer plugins still run outside the shim, so its install is one explicit +`~/.claude/bin/captainhook install` with `extra.captainhook.disable-plugin` +set to `true` in `composer.json`; an install through the shim warns when a +plugin is enabled. When the dispatcher chains into an overcommit hook, it exports the same `core.hooksPath` override so overcommit's run-time self-check accepts the hook instead of looking for its hook in the shared dir. @@ -1487,6 +1503,7 @@ dispatchers + hook-manager shim): |---|---| | prek shim in `.git/hooks/` (installed via the shim) | dispatcher → prek runs that repo's `.pre-commit-config.yaml` | | overcommit hook in `.git/hooks/` (installed via the shim) | dispatcher → overcommit runs that repo's `.overcommit.yml`; its self-check sees the repo's hooks path | +| CaptainHook hook in `.git/hooks/` (installed via the shim) | dispatcher → CaptainHook runs that repo's `captainhook.json`; the shared dir keeps its dispatcher symlinks | | local hook installed by a manager requiring no install fix (`simple-git-hooks`, `cargo-husky`, `git-code-format-maven-plugin`, `git-build-hook`) | dispatcher → local hook runs natively (installs straight to `.git/hooks/` ignoring global `core.hooksPath`) | | hand-written `.git/hooks/`, or one a manager without an adapter put there | dispatcher → that hook runs (heterogeneous tooling coexists) | | none | dispatcher no-op; git operation proceeds cleanly | diff --git a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md index 834ba6eb5..2b7e1f6ee 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md +++ b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md @@ -335,7 +335,8 @@ for the full rationale + the validated behaviour matrix. [hook-manager registry](../../../../tools/hook-manager/registry.json) with an install strategy other than `none` that a repository the operator works in uses — `~/.claude/bin/prek`, - `~/.claude/bin/overcommit` and `~/.claude/bin/lefthook` today. + `~/.claude/bin/overcommit`, `~/.claude/bin/lefthook` and + `~/.claude/bin/captainhook` today. A manager whose install strategy is `none` needs no install fix and gets no shim. A repository uses a manager when one of its config files is at the root @@ -343,7 +344,8 @@ for the full rationale + the validated behaviour matrix. hooks runs it ([Detection](../../../../tools/hook-manager/README.md#detection)); for prek that is `prek.toml`, `.pre-commit-config.yaml` or `.pre-commit-config.yml`; for overcommit, `.overcommit.yml`; for lefthook, - its root `lefthook.*`, `.lefthook.*` and `-local` names. + its root `lefthook.*`, `.lefthook.*` and `-local` names; for CaptainHook, + `captainhook.json`. Install no shim for a manager no repository uses. The shim takes the tool from the name it is installed under and rewrites only the install action: for prek it injects @@ -351,15 +353,30 @@ for the full rationale + the validated behaviour matrix. passed `--git-dir`; for overcommit it rewrites `overcommit --install` (or `-i`, `-fi`, `--inst`, anywhere in argv), pointing `core.hooksPath` at `/hooks`; + for CaptainHook it does the same for `install`, which its Symfony Console + CLI also accepts after global options (`captainhook -n install`) or as an + abbreviation (`captainhook inst`); for lefthook it runs the install with `GIT_CONFIG_GLOBAL=/dev/null` while the global config holds one, which hides the global `core.hooksPath` from lefthook's own check and lets its target resolve to the common dir. Each leaves the install alone when the caller asked for `--help` or is outside a git work tree, and **every other invocation passes through unchanged.** + When `PATH` holds no real binary, the shim runs + `/vendor/bin/` for an adapter that sets + `install.vendor_bin`, which covers CaptainHook in a Composer project. It is a no-op on hosts with no global `core.hooksPath`. A host that installed the older `prek-shim.sh` as `~/.claude/bin/prek` keeps working; replacing it is optional. + CaptainHook's `captainhook/hook-installer` and + `captainhook/plugin-composer` Composer plugins install on every + `composer install` and `composer update`, outside the shim. Before + installing the shim for a repository whose `composer.json` requires either + plugin, check `extra.captainhook.disable-plugin`: when it is not `true`, + warn the operator that the next dependency update will install into the + shared hooks dir behind the shim, and offer to add + `"captainhook": {"disable-plugin": true}` under `extra`. + 3. **Surface the PATH line for the operator to add** to their `~/.bashrc` / `~/.zshrc` (per the golden rules — never edit the rc file directly): diff --git a/plugins/magpie-setup/skills/isolated-setup-verify/conditional-checks.md b/plugins/magpie-setup/skills/isolated-setup-verify/conditional-checks.md index 910af8150..f221c8ca1 100644 --- a/plugins/magpie-setup/skills/isolated-setup-verify/conditional-checks.md +++ b/plugins/magpie-setup/skills/isolated-setup-verify/conditional-checks.md @@ -75,6 +75,15 @@ reading this file; read the section for a check whose condition holds. any other, the remediation is `git config --local --unset core.hooksPath` in that repository, run by the operator; the dispatcher still chains to the repository's own `.git/hooks/`. + - ⚠ for a repository that uses CaptainHook + ([detection](../../../../tools/hook-manager/README.md#detection)) while + `composer.json` requires `captainhook/hook-installer` or + `captainhook/plugin-composer` without `extra.captainhook.disable-plugin: + true` (or an `allow-plugins` entry that turns it off): the plugin + installs on every `composer install` / `composer update` outside the + shim and replaces the dispatcher in the shared directory. Remediation: + add the flag and re-run `~/.claude/bin/captainhook install`, per + [troubleshooting](../../../../docs/setup/sandbox-troubleshooting.md#captainhooks-composer-plugin-replaced-the-shared-hook-directory). If `core.hooksPath` is unset (or points elsewhere), the operator is in **per-project** scope (the default). No further diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index 259d19b0e..1ebe33786 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:9879281c90f28d55" +FRAMEWORK_FINGERPRINT = "sha256:ce36f6fbba7ce017" diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index c7c923fe9..b6b243ae7 100644 --- a/tools/agent-isolation/README.md +++ b/tools/agent-isolation/README.md @@ -98,7 +98,7 @@ per runtime — see [`docs/adapters/add-a-harness.md`](../../docs/adapters/add-a | [`sandbox-add-project-root.sh`](sandbox-add-project-root.sh) | Adds the current adopter repo's project root (and, with `--all-worktrees`, every linked git worktree's working dir) as an explicit absolute path to `sandbox.filesystem.allowRead` and `allowWrite` in the project-local, gitignored `/.claude/settings.local.json` — one entry per worktree, each in that worktree's own settings file. Defensive against [issue #197](https://github.com/apache/magpie/issues/197) — `allowRead: ["."]` does not in practice cover CWD because the harness pre-resolves the `.` literal away from the read side. The same harness behaviour drops the committed home-directory entries, so it also adds the dev-tool paths `prek` and `uv` need (`~/.gitconfig`, `~/.config/git`, `~/.cache`, `~/.local/share/uv`, `~/.local/bin`; the cache and uv dirs writable) as absolute paths — never the credential paths; `--no-tool-paths` skips them. See [`prek` or `uv` not found inside the sandbox](../../docs/setup/sandbox-troubleshooting.md#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox). Never modifies user-scope or committed project-scope. Idempotent, atomic, tolerant of missing prereqs. Invoked from `setup-isolated-setup-install`, `/magpie-setup:setup` (adopt / upgrade / worktree-init), and the `post-checkout` git hook installed by `/magpie-setup:setup adopt`. | | [`git-global-post-checkout.sh`](git-global-post-checkout.sh) | Universal `post-checkout` git hook installed at `~/.claude/git-hooks/post-checkout` when the operator picks the **simple whole-user** flavour in `setup-isolated-setup-install`. Activated by `git config --global core.hooksPath ~/.claude/git-hooks/` so every `git checkout` / `git clone` / `git worktree add` across the host invokes it. Best-effort + idempotent + `\|\| true`: invokes `sandbox-add-project-root.sh` for any worktree with a `.claude/` directory. Trade-off documented in [`docs/setup/secure-agent-setup.md` → *Per-project vs whole-user scope*](../../docs/setup/secure-agent-setup.md#per-project-vs-whole-user-scope): `core.hooksPath` shadows per-repo `.git/hooks/*` across every repo on the host. The **dispatcher** flavour (below) supersedes this file. | | [`git-hook-dispatcher.sh`](git-hook-dispatcher.sh) | Universal, basename-keyed git-hook **dispatcher** for the **whole-user dispatcher** flavour. Symlinked to every hook name under `~/.claude/git-hooks/`. For each git operation it runs the framework's own logic for that hook type (the `post-checkout` sandbox-allowlist sync) **and then chains through to the repo-local `.git/hooks/`** (resolved via `git rev-parse --git-common-dir`, worktree-safe; `exec`-ed with original argv + inherited stdin so a failing local hook still aborts the git op). Restores per-repo hooks (prek / overcommit / pre-commit / lefthook / husky / hand-written) under global `core.hooksPath`; a repo with no local hook is a clean no-op. Outside the sandbox it also gives every hook manager in the [hook-manager registry](../hook-manager/registry.json) its host-only home (the contract's home rule), and before chaining into an overcommit hook it exports the same `core.hooksPath` override the shim used so overcommit's run-time self-check accepts the hook. See [`docs/setup/secure-agent-setup.md` → *Whole-user with the per-repo dispatcher*](../../docs/setup/secure-agent-setup.md#whole-user-with-the-per-repo-dispatcher). | -| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek`, `overcommit` and `lefthook` today). The tool is the name it is installed under. Rewrites **only** the adapter's install action so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir — for prek, injecting `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`, asked for `--help`, or is outside a git work tree; for overcommit, finding `--install` or its `-i` alias anywhere in argv and setting `core.hooksPath` to `/hooks` in the next free `GIT_CONFIG_*` slot; for lefthook, running the install with `GIT_CONFIG_GLOBAL=/dev/null` while the global config holds a `core.hooksPath`, so lefthook's own `core.hooksPath` refusal does not fire and a host without one keeps its global settings. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | +| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek`, `overcommit`, `lefthook` and `captainhook` today). The tool is the name it is installed under. Rewrites **only** the adapter's install action so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir — for prek, injecting `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`, asked for `--help`, or is outside a git work tree; for overcommit, finding `--install` or its `-i` alias anywhere in argv and setting `core.hooksPath` to `/hooks` in the next free `GIT_CONFIG_*` slot; for CaptainHook, the same override, with `install` recognised at any non-option argument that prefixes it (`captainhook -n install`, `captainhook inst`); for lefthook, running the install with `GIT_CONFIG_GLOBAL=/dev/null` while the global config holds a `core.hooksPath`, so lefthook's own `core.hooksPath` refusal does not fire and a host without one keeps its global settings. When `PATH` holds no real binary, an adapter with `install.vendor_bin` runs `/vendor/bin/`, refusing a symlink back to the shim; a CaptainHook install also warns when one of its Composer plugins is enabled. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | ## Usage at a glance diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index b899aca0b..53c8d8c8c 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -17,11 +17,11 @@ # under the License. # # hook-manager-shim.sh — a transparent front for a git-hook manager -# (prek, overcommit and lefthook today; see tools/hook-manager/registry.json), -# installed as `~/.claude/bin/` with `~/.claude/bin` prepended -# to PATH so it shadows the real binary. The tool is the name it is -# installed under (`basename "$0"`), so one script serves every -# adapter. +# (prek, overcommit, lefthook and CaptainHook today; see +# tools/hook-manager/registry.json), installed as `~/.claude/bin/` +# with `~/.claude/bin` prepended to PATH so it shadows the real binary. +# The tool is the name it is installed under (`basename "$0"`), so one +# script serves every adapter. # # WHY THIS EXISTS # --------------- @@ -50,10 +50,15 @@ # target, asked for help, or we are not inside a git work tree. # A flag-style action and its registry aliases are found anywhere # in argv (`overcommit --force --install`, `overcommit -i`); a -# positional action is only argv[0] (`prek install`). +# positional action is argv[0] (`prek install`) unless the adapter +# sets `install.match: prefix`, for a manager whose option parser +# takes global options before the command and resolves a prefix of +# it (`captainhook -n install`, `captainhook inst`) — then any +# non-option argument that prefixes the action counts. # git-dir-flag: inject ` ` (prek: --git-dir) # hooks-path-env: append GIT_CONFIG_* so the manager writes to -# `/hooks` (overcommit: --install, -i) +# `/hooks` (overcommit: --install, -i; captainhook: +# install) # hide-global-config: run the install with # GIT_CONFIG_GLOBAL=/dev/null, for a manager that writes to # the common dir itself but refuses while a global @@ -66,6 +71,12 @@ # * every other invocation (`prek run`, `prek autoupdate`, # `--version`, bare `prek`, ...) is passed through to the real # binary **unchanged**. +# * when PATH holds no real binary, an adapter with +# `install.vendor_bin` runs the copy a Composer project keeps in +# `/vendor/bin/`, which is not on PATH. +# * a CaptainHook install warns when a CaptainHook Composer plugin is +# enabled, because that plugin installs on every `composer install` +# and `composer update` without passing through this shim. # # The shim is a no-op on hosts that never set a global # core.hooksPath — installing into `.git/hooks/` is exactly where a @@ -80,6 +91,7 @@ tool="$(basename "$0")" # BEGIN generated by tools/dev/sync-hook-managers.py; do not edit adapter() { case "$1" in + captainhook) install_subcommand=install; install_strategy=hooks-path-env; install_match=prefix; install_vendor_bin=1 ;; lefthook) install_subcommand=install; install_strategy=hide-global-config ;; overcommit) install_subcommand=--install; install_strategy=hooks-path-env; install_aliases='--install -i' ;; prek) install_subcommand=install; install_strategy=git-dir-flag; install_flag=--git-dir ;; @@ -95,25 +107,138 @@ if ! adapter "$tool"; then fi # --- locate the REAL binary: the next `$tool` on PATH that is not us ------- -self_canon="$(readlink -f "$0" 2>/dev/null || echo "$0")" +# `readlink -f` is not on every macOS, so fall back to resolving the symlink +# chain and the directory with the POSIX tools that are everywhere. +_canonical() { + if _cp="$(readlink -f "$1" 2>/dev/null)"; then + printf '%s\n' "$_cp" + return 0 + fi + _cp="$1" + while [ -L "$_cp" ]; do + _ct="$(readlink "$_cp" 2>/dev/null)" || break + case "$_ct" in + /*) _cp="$_ct" ;; + *) _cp="$(dirname "$_cp")/$_ct" ;; + esac + done + _cd="$(cd "$(dirname "$_cp")" 2>/dev/null && pwd -P)" || { + printf '%s\n' "$_cp" + return 0 + } + printf '%s\n' "$_cd/$(basename "$_cp")" +} + +self_canon="$(_canonical "$0")" real_tool="" IFS=':' read -ra _path_dirs <<<"${PATH:-}" for _d in "${_path_dirs[@]}"; do [ -n "$_d" ] || continue _cand="$_d/$tool" [ -x "$_cand" ] || continue - _cand_canon="$(readlink -f "$_cand" 2>/dev/null || echo "$_cand")" + _cand_canon="$(_canonical "$_cand")" [ "$_cand_canon" = "$self_canon" ] && continue # skip ourselves real_tool="$_cand" break done +if [ -z "$real_tool" ] && [ "${install_vendor_bin:-0}" = 1 ]; then + # A Composer project keeps the binary in vendor/bin, which is not on + # PATH; resolve it from the repository root the command runs in. The + # candidate can be a symlink back to this shim, so it gets the same + # self-check as the PATH walk. + if _root="$(git rev-parse --show-toplevel 2>/dev/null)" && [ -x "$_root/vendor/bin/$tool" ]; then + _cand="$_root/vendor/bin/$tool" + _cand_canon="$(_canonical "$_cand")" + if [ "$_cand_canon" = "$self_canon" ]; then + echo "hook-manager-shim: $_cand is this shim" >&2 + echo "hook-manager-shim: (this wrapper must sit ahead of $tool, not replace it)" >&2 + exit 127 + fi + real_tool="$_cand" + fi +fi + if [ -z "$real_tool" ]; then echo "hook-manager-shim: could not locate the real '$tool' binary on PATH" >&2 echo "hook-manager-shim: (this wrapper must sit ahead of $tool, not replace it)" >&2 exit 127 fi +# CaptainHook's Composer plugins install on every `composer install` and +# `composer update`, behind this shim. Warn when an install through the shim +# would be undone by the next dependency update: the plugin is enabled when +# composer.json requires it and neither its own `disable-plugin` block nor +# `config.allow-plugins` turns it off. +warn_composer_plugin() { + [ "$tool" = captainhook ] || return 0 + _root="$(git rev-parse --show-toplevel 2>/dev/null)" || return 0 + [ -f "$_root/composer.json" ] || return 0 + _state="$(_composer_plugin_state "$_root/composer.json")" || return 0 + [ "$_state" = enabled ] || return 0 + echo "hook-manager-shim: a CaptainHook Composer plugin is enabled in $_root/composer.json" >&2 + echo "hook-manager-shim: composer install/update will install behind this shim; set extra.captainhook.disable-plugin to true" >&2 +} + +# `none`, `enabled` or `disabled`, read from the plugin's own keys rather +# than a text match, so a `disable-plugin` under another package's `extra` +# cannot silence the warning. Needs python3; without it the check is +# skipped rather than guessed. +_composer_plugin_state() { + command -v python3 >/dev/null 2>&1 || return 1 + python3 - "$1" <<'PY' +import json +import re +import sys + +PACKAGES = ("captainhook/hook-installer", "captainhook/plugin-composer") + +try: + with open(sys.argv[1], encoding="utf-8") as handle: + data = json.load(handle) +except (OSError, ValueError): + raise SystemExit(1) + +if not isinstance(data, dict): + raise SystemExit(1) + +required = [ + name + for section in ("require", "require-dev") + for name in (data.get(section) or {}) + if name in PACKAGES +] +if not required: + print("none") + raise SystemExit(0) + +extra = data.get("extra") +block = extra.get("captainhook") if isinstance(extra, dict) else None +if isinstance(block, dict) and block.get("disable-plugin") is True: + print("disabled") + raise SystemExit(0) + + +def allowed(name, rules): + """Composer's own reading: `*` is the only wildcard, the match ignores + case, and the first matching key wins (`captainhook/*: false`).""" + for pattern, value in rules.items(): + regex = ".*".join(re.escape(part) for part in str(pattern).split("*")) + if re.fullmatch(regex, name, flags=re.IGNORECASE): + return value + return None + + +config = data.get("config") +allow = config.get("allow-plugins") if isinstance(config, dict) else None +if allow is False or (isinstance(allow, dict) and all(allowed(name, allow) is False for name in required)): + print("disabled") + raise SystemExit(0) + +print("enabled") +PY +} + # --- only rewrite the install action; everything else is transparent ------- # A positional subcommand (`prek install`) is only the first argument. A # flag-style action (`overcommit --install`) may follow other options, so scan @@ -156,7 +281,25 @@ case "$install_subcommand" in done ;; *) - [ "${1:-}" = "$install_subcommand" ] && install_invoked=1 + if [ "${install_match:-exact}" = prefix ]; then + # A Symfony Console app takes global options before the command and + # resolves an unambiguous prefix of it (`captainhook -n install`, + # `captainhook inst`). Any non-option argument that prefixes the + # action counts, so a global option's value cannot hide the command + # behind it; treating a run as an install only points core.hooksPath + # at the repository's own hooks, which is harmless. + for _a in "$@"; do + case "$_a" in + -*) continue ;; + esac + if [ "${install_subcommand#"$_a"}" != "$install_subcommand" ]; then + install_invoked=1 + break + fi + done + else + [ "${1:-}" = "$install_subcommand" ] && install_invoked=1 + fi ;; esac @@ -215,6 +358,7 @@ case "$install_strategy" in export "GIT_CONFIG_KEY_$_i=core.hooksPath" export "GIT_CONFIG_VALUE_$_i=${gitdir%/}/hooks" export GIT_CONFIG_COUNT=$((_i + 1)) + warn_composer_plugin "$@" exec "$real_tool" "$@" fi # not in a git work tree: let the real tool produce its own error diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index 18d19fdef..2e3b91160 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -21,6 +21,7 @@ import os import subprocess +import sys from pathlib import Path import pytest @@ -49,7 +50,12 @@ def _setup(tmp_path: Path, name: str = "prek") -> tuple[dict[str, str], Path]: for key, value in os.environ.items() if not key.startswith("GIT_CONFIG_") } - env["PATH"] = f"{shims}:{real}:/usr/bin:/bin" + # A python3 the shim can find, without the rest of the interpreter's bin + # directory (a real manager may sit there and shadow the fake one). + pydir = tmp_path / "py" + pydir.mkdir() + (pydir / "python3").symlink_to(sys.executable) + env["PATH"] = f"{shims}:{real}:{pydir}:/usr/bin:/bin" return env, argv @@ -88,9 +94,16 @@ def _setup_lefthook(tmp_path: Path) -> tuple[dict[str, str], Path, Path]: home.mkdir() (home / ".gitconfig").write_text(f"[core]\n\thooksPath = {shared}\n") env = dict(os.environ) - for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT", "GIT_CONFIG_KEY_0", "GIT_CONFIG_VALUE_0"): + for name in ( + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_KEY_0", + "GIT_CONFIG_VALUE_0", + ): env.pop(name, None) - env.update(PATH=f"{shims}:{real}:/usr/bin:/bin", HOME=str(home), GIT_CONFIG_NOSYSTEM="1") + env.update( + PATH=f"{shims}:{real}:/usr/bin:/bin", HOME=str(home), GIT_CONFIG_NOSYSTEM="1" + ) return env, record, shared @@ -221,8 +234,77 @@ def test_overcommit_install_preserves_existing_git_config(tmp_path: Path) -> Non ] -def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: - env, argv = _setup(tmp_path) +def _hooks_env(repo: Path) -> list[str]: + return ["1", "core.hooksPath", f"{_common_dir(repo)}/hooks", "", ""] + + +@pytest.mark.parametrize( + "args", + [ + ["install"], + ["-n", "install"], + ["--ansi", "install"], + ["inst"], + ["ins"], + ["--configuration", "captainhook.json", "install"], + ["--configuration=captainhook.json", "install"], + ], +) +def test_captainhook_install_targets_the_common_hooks_dir( + tmp_path: Path, args: list[str] +) -> None: + """A Symfony Console app takes global options before the command and prefixes it.""" + env, argv = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + assert _run(env, repo, *args, name="captainhook").returncode == 0 + assert argv.read_text().splitlines() == args + assert (tmp_path / "env").read_text().splitlines() == _hooks_env(repo) + + +@pytest.mark.parametrize( + "args", + [["hook:pre-commit"], ["hook:commit-msg"], ["validate"], ["--version"]], +) +def test_captainhook_other_invocations_pass_through( + tmp_path: Path, args: list[str] +) -> None: + env, argv = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + assert _run(env, repo, *args, name="captainhook").returncode == 0 + assert argv.read_text().splitlines() == args + assert (tmp_path / "env").read_text().splitlines() == ["", "", "", "", ""] + + +def test_captainhook_install_help_passes_through(tmp_path: Path) -> None: + env, argv = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + assert _run(env, repo, "install", "--help", name="captainhook").returncode == 0 + assert argv.read_text().splitlines() == ["install", "--help"] + assert (tmp_path / "env").read_text().splitlines() == ["", "", "", "", ""] + + +def test_captainhook_install_preserves_existing_git_config(tmp_path: Path) -> None: + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + env.update( + { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "user.name", + "GIT_CONFIG_VALUE_0": "Someone", + } + ) + assert _run(env, repo, "install", name="captainhook").returncode == 0 + assert (tmp_path / "env").read_text().splitlines() == [ + "2", + "user.name", + "Someone", + "core.hooksPath", + f"{_common_dir(repo)}/hooks", + ] + + +def _worktree(tmp_path: Path) -> Path: + """A linked worktree of a repo that has one commit, so `git worktree add` works.""" repo = _repo(tmp_path) # A developer's global commit.gpgsign would make this commit wait for (or # fail on) their signing key; the throwaway repo has nothing to sign. @@ -252,14 +334,30 @@ def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: subprocess.run( ["git", "-C", str(repo), "worktree", "add", "-q", str(worktree)], check=True ) + return worktree + + +def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: + env, argv = _setup(tmp_path) + worktree = _worktree(tmp_path) assert _run(env, worktree, "install").returncode == 0 assert argv.read_text().splitlines() == [ "install", "--git-dir", - str((repo / ".git").resolve()), + str((tmp_path / "repo" / ".git").resolve()), ] +def test_captainhook_in_a_linked_worktree_targets_the_main_checkout( + tmp_path: Path, +) -> None: + env, argv = _setup(tmp_path, name="captainhook") + worktree = _worktree(tmp_path) + assert _run(env, worktree, "install", name="captainhook").returncode == 0 + assert argv.read_text().splitlines() == ["install"] + assert (tmp_path / "env").read_text().splitlines() == _hooks_env(tmp_path / "repo") + + def test_lefthook_install_hides_the_global_hooks_path(tmp_path: Path) -> None: env, record, shared = _setup_lefthook(tmp_path) repo = _repo(tmp_path) @@ -297,7 +395,18 @@ def test_lefthook_install_in_a_linked_worktree_targets_the_main_checkout( env, record, _shared = _setup_lefthook(tmp_path) repo = _repo(tmp_path) subprocess.run( - ["git", "-C", str(repo), "-c", "commit.gpgsign=false", "commit", "-q", "--allow-empty", "-m", "x"], + [ + "git", + "-C", + str(repo), + "-c", + "commit.gpgsign=false", + "commit", + "-q", + "--allow-empty", + "-m", + "x", + ], check=True, env=dict( os.environ, @@ -353,7 +462,8 @@ def test_lefthook_other_invocations_pass_through(tmp_path: Path) -> None: def test_lefthook_help_passes_through(tmp_path: Path, help_flag: str) -> None: env, record, _shared = _setup_lefthook(tmp_path) assert ( - _run(env, _repo(tmp_path), "install", help_flag, name="lefthook").returncode == 0 + _run(env, _repo(tmp_path), "install", help_flag, name="lefthook").returncode + == 0 ) assert record.read_text().splitlines() == [ "GIT_CONFIG_GLOBAL=", @@ -410,3 +520,176 @@ def test_a_missing_real_binary_is_reported(tmp_path: Path) -> None: result = _run(env, tmp_path, "run") assert result.returncode == 127 assert "could not locate the real 'prek'" in result.stderr + + +def _vendor_bin(tmp_path: Path, repo: Path, name: str = "captainhook") -> Path: + """Move the fake binary into the project's vendor/bin, off PATH.""" + vendored = repo / "vendor" / "bin" + vendored.mkdir(parents=True) + (vendored / name).write_text((tmp_path / "real" / name).read_text()) + (vendored / name).chmod(0o755) + (tmp_path / "real" / name).unlink() + return vendored / name + + +def test_captainhook_falls_back_to_the_projects_vendor_bin(tmp_path: Path) -> None: + """A Composer project keeps the binary in vendor/bin, which is not on PATH.""" + env, argv = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + _vendor_bin(tmp_path, repo) + subdir = repo / "src" + subdir.mkdir() + assert _run(env, subdir, "install", name="captainhook").returncode == 0 + assert argv.read_text().splitlines() == ["install"] + assert (tmp_path / "env").read_text().splitlines() == _hooks_env(repo) + + +def test_a_vendor_bin_symlink_to_the_shim_is_refused(tmp_path: Path) -> None: + """A workaround that points vendor/bin at the shim must not exec itself.""" + env, argv = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (tmp_path / "real" / "captainhook").unlink() + vendored = repo / "vendor" / "bin" + vendored.mkdir(parents=True) + (vendored / "captainhook").symlink_to(SHIM) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 127 + assert "must sit ahead of captainhook" in result.stderr + assert not argv.exists() + + +@pytest.mark.parametrize( + ("name", "args"), + [("prek", ["install"]), ("overcommit", ["--install"])], +) +def test_only_a_composer_adapter_reads_vendor_bin( + tmp_path: Path, name: str, args: list[str] +) -> None: + env, argv = _setup(tmp_path, name=name) + repo = _repo(tmp_path) + _vendor_bin(tmp_path, repo, name=name) + result = _run(env, repo, *args, name=name) + assert result.returncode == 127 + assert f"could not locate the real '{name}'" in result.stderr + assert not argv.exists() + + +PLUGIN_COMPOSER = """{ + "require-dev": { + "captainhook/captainhook": "^5.29", + "captainhook/hook-installer": "^1.0" + } +} +""" + + +@pytest.mark.parametrize("args", [["install"], ["-n", "install"], ["inst"]]) +def test_captainhook_warns_when_a_composer_plugin_is_enabled( + tmp_path: Path, args: list[str] +) -> None: + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text(PLUGIN_COMPOSER) + result = _run(env, repo, *args, name="captainhook") + assert result.returncode == 0 + assert "a CaptainHook Composer plugin is enabled" in result.stderr + assert "extra.captainhook.disable-plugin" in result.stderr + + +def test_the_plugin_warning_covers_plugin_composer(tmp_path: Path) -> None: + """The older auto-installing plugin is checked too.""" + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text( + '{"require": {"captainhook/plugin-composer": "^2.0"}}\n' + ) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 0 + assert "a CaptainHook Composer plugin is enabled" in result.stderr + + +def test_the_plugin_warning_respects_disable_plugin(tmp_path: Path) -> None: + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text( + """{ + "require-dev": { + "captainhook/captainhook": "^5.29", + "captainhook/hook-installer": "^1.0" + }, + "extra": {"captainhook": {"disable-plugin": true}} +} +""" + ) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 0 + assert "Composer plugin is enabled" not in result.stderr + + +def test_a_disable_plugin_under_another_package_does_not_silence_the_warning( + tmp_path: Path, +) -> None: + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text( + """{ + "require-dev": {"captainhook/hook-installer": "^1.0"}, + "extra": {"some-other-package": {"disable-plugin": true}} +} +""" + ) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 0 + assert "a CaptainHook Composer plugin is enabled" in result.stderr + + +@pytest.mark.parametrize( + "config", + [ + '"config": {"allow-plugins": false}', + '"config": {"allow-plugins": {"captainhook/hook-installer": false}}', + '"config": {"allow-plugins": {"captainhook/*": false}}', + '"config": {"allow-plugins": {"CaptainHook/Hook-*": false, "*": true}}', + ], +) +def test_the_plugin_warning_respects_allow_plugins(tmp_path: Path, config: str) -> None: + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text( + f'{{"require-dev": {{"captainhook/hook-installer": "^1.0"}}, {config}}}\n' + ) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 0 + assert "Composer plugin is enabled" not in result.stderr + + +def test_the_first_matching_allow_plugins_key_wins(tmp_path: Path) -> None: + """Composer takes the first matching key, so an earlier allow beats a wildcard deny.""" + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text( + '{"require-dev": {"captainhook/hook-installer": "^1.0"}, ' + '"config": {"allow-plugins": {"captainhook/hook-installer": true, "captainhook/*": false}}}\n' + ) + result = _run(env, repo, "install", name="captainhook") + assert result.returncode == 0 + assert "a CaptainHook Composer plugin is enabled" in result.stderr + + +def test_the_plugin_warning_stays_off_run_commands(tmp_path: Path) -> None: + """The installed hook runs on every commit; a warning there would be noise.""" + env, _ = _setup(tmp_path, name="captainhook") + repo = _repo(tmp_path) + (repo / "composer.json").write_text(PLUGIN_COMPOSER) + result = _run(env, repo, "hook:pre-commit", name="captainhook") + assert result.returncode == 0 + assert "Composer plugin is enabled" not in result.stderr + + +def test_the_plugin_warning_is_captainhooks_only(tmp_path: Path) -> None: + env, _ = _setup(tmp_path, name="overcommit") + repo = _repo(tmp_path) + (repo / "composer.json").write_text(PLUGIN_COMPOSER) + result = _run(env, repo, "--install", name="overcommit") + assert result.returncode == 0 + assert "Composer plugin is enabled" not in result.stderr diff --git a/tools/dev/sync-hook-managers.py b/tools/dev/sync-hook-managers.py index d1764295d..24971ab6e 100755 --- a/tools/dev/sync-hook-managers.py +++ b/tools/dev/sync-hook-managers.py @@ -57,6 +57,11 @@ #: Install strategies the shim implements. A new one lands with its shim branch. STRATEGIES = {"git-dir-flag", "hooks-path-env", "hide-global-config", "none"} +#: How the shim recognises a positional install action: `exact` (argv[0] is the +#: action) or `prefix` (a non-option argument prefixes it, as a Symfony Console +#: application resolves an unambiguous abbreviation after global options). +INSTALL_MATCHES = {"exact", "prefix"} + _NAME = re.compile(r"^[a-z][a-z0-9-]*$") _SUBCOMMAND = re.compile(r"^(?:--)?[a-z][a-z0-9-]*$") _ALIAS = re.compile(r"^--?[a-z][a-z0-9-]*$") @@ -138,8 +143,20 @@ def validate(registry: dict[str, Any]) -> list[str]: ) elif strategy != "git-dir-flag" and "flag" in install: errors.append(f"{where}.install.flag: strategy {strategy!r} takes no flag") + match = install.get("match", "exact") + if match not in INSTALL_MATCHES: + errors.append( + f"{where}.install.match: {match!r} is not how the shim finds the install " + f"action ({', '.join(sorted(INSTALL_MATCHES))})" + ) + elif match == "prefix" and not ( + isinstance(subcommand, str) and not subcommand.startswith("-") + ): + errors.append(f"{where}.install.match: prefix matching needs a positional subcommand") + if "vendor_bin" in install and not isinstance(install["vendor_bin"], bool): + errors.append(f"{where}.install.vendor_bin: must be true or false") else: - for key in ("flag", "aliases"): + for key in ("flag", "aliases", "match", "vendor_bin"): if key in install: errors.append(f"{where}.install.{key}: strategy 'none' takes no {key}") if "runtime_env" in install and not isinstance(install["runtime_env"], bool): @@ -192,6 +209,10 @@ def render(registry: dict[str, Any]) -> dict[Path, str]: fields.append(f"install_aliases='{actions}'") if "flag" in install: fields.append(f"install_flag={install['flag']}") + if install.get("match") == "prefix": + fields.append("install_match=prefix") + if install.get("vendor_bin"): + fields.append("install_vendor_bin=1") cases.append(f" {name}) {'; '.join(fields)} ;;\n") shim = 'adapter() {\n case "$1" in\n' + "".join(cases) + " *) return 1 ;;\n esac\n}\n" homes = [(n, a) for n, a in sorted(adapters.items()) if a.get("home")] diff --git a/tools/dev/tests/test_sync_hook_managers.py b/tools/dev/tests/test_sync_hook_managers.py index e120c52b8..6d24e05c7 100644 --- a/tools/dev/tests/test_sync_hook_managers.py +++ b/tools/dev/tests/test_sync_hook_managers.py @@ -132,6 +132,46 @@ def test_bad_install_aliases_are_refused(registry: dict[str, Any], aliases: obje assert any(message in e for e in mod.validate(broken)), mod.validate(broken) +def test_captainhook_matches_a_prefix_and_reads_vendor_bin(registry: dict[str, Any]) -> None: + adapter = registry["adapters"]["captainhook"] + assert "home" not in adapter + assert adapter["install"] == { + "subcommand": "install", + "strategy": "hooks-path-env", + "match": "prefix", + "vendor_bin": True, + } + rendered = mod.render(registry) + assert ( + "captainhook) install_subcommand=install; install_strategy=hooks-path-env; " + "install_match=prefix; install_vendor_bin=1" in rendered[mod.SHIM] + ) + assert "captainhook:CaptainHook" not in rendered[mod.DISPATCHER] + + +@pytest.mark.parametrize( + ("key", "value", "message"), + [ + ("match", "fuzzy", "install.match"), + ("vendor_bin", "yes", "install.vendor_bin"), + ], +) +def test_bad_captainhook_install_keys_are_refused( + registry: dict[str, Any], key: str, value: object, message: str +) -> None: + broken = copy.deepcopy(registry) + broken["adapters"]["captainhook"]["install"][key] = value + assert any(message in e for e in mod.validate(broken)), mod.validate(broken) + + +def test_prefix_matching_needs_a_positional_subcommand(registry: dict[str, Any]) -> None: + broken = copy.deepcopy(registry) + broken["adapters"]["overcommit"]["install"]["match"] = "prefix" + assert any("prefix matching needs a positional subcommand" in e for e in mod.validate(broken)), ( + mod.validate(broken) + ) + + def test_aliases_need_a_flag_style_subcommand(registry: dict[str, Any]) -> None: broken = copy.deepcopy(registry) broken["adapters"]["prek"]["install"]["aliases"] = ["-i"] diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index fa04a4ed2..ac3d5b9c1 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -69,6 +69,8 @@ A manager's install must put its hook in the repository's own `$(git rev-parse - The fix is an **install strategy**, applied by [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` ahead of the real binary on `PATH`. The shim takes the tool from the name it is installed under, rewrites only the install action, and passes every other invocation through unchanged. A positional action (`prek install`) is only the first argument; a flag-style action (`overcommit --install`) and its registry aliases (`overcommit -i`) are found anywhere in argv, including a single-letter alias inside a bundle of short switches (`-fi`) and any shortening of a long flag (`--inst`). +An adapter sets `install.match: prefix` for a manager whose option parser takes global options before the command and resolves a prefix of it, so `captainhook -n install` and `captainhook inst` count as well; any non-option argument that prefixes the action matches, and rewriting a run only points `core.hooksPath` at the repository's own hooks, which is harmless. +An adapter sets `install.vendor_bin` for a manager that Composer installs into a project's `vendor/bin`, which is not on `PATH`; when the `PATH` walk finds no real binary the shim runs `/vendor/bin/`, and a symlink back to the shim is refused rather than exec'd. A global binary on `PATH` always wins over the project's pinned one. It leaves the install alone when the caller already chose a target, asked for help, or is not in a git work tree. | Strategy | What the shim does | Status | @@ -122,6 +124,18 @@ Only the bare root names are in the registry: the contract's `detect.config_file `GIT_CONFIG_GLOBAL=/dev/null` hides the global value from that read, and the path then resolves to the common directory, linked worktrees included. Remote configs are cloned into `/lefthook-remotes`, inside `.git`, so the registry omits `home`. +How CaptainHook configures itself, from its source (v5.29.2): it reads `captainhook.json` at the repository root, or the path given with `--configuration`. +`Repository::getHooksDir()` reads `git config --get core.hooksPath` (`Command/Config/Get.php:53`), uses an absolute value as-is, and refuses only `/dev/null` (`Installer.php:291-296`), so under the whole-user setup an unshimmed install writes into the shared dispatcher directory. +A relative value is joined to the repository root; in a linked worktree with no `core.hooksPath` the hooks dir is built from the worktree's absolute gitdir, which does not exist (`Repository.php:88-101`). +The `hooks-path-env` override fixes both, because the install then reads the common dir's `hooks` as an absolute value. +Its CLI is a Symfony Console application, so `install` may follow global options (`captainhook -n install`) or be an unambiguous prefix (`captainhook inst`); that is what `install.match: prefix` covers. +Composer keeps the binary in `vendor/bin`, which is not on `PATH`, so `install.vendor_bin` lets the shim fall back to `/vendor/bin/captainhook`; `~/.claude/bin/captainhook install` therefore works from anywhere in the repository, including a linked worktree. +The `captainhook/hook-installer` and `captainhook/plugin-composer` Composer plugins still install on every `composer install` and `composer update`, outside the shim. +The documented path is one explicit `~/.claude/bin/captainhook install` from the repository root after a clone, with `extra.captainhook.disable-plugin` set to `true` in `composer.json` so the plugin does not install into the shared directory behind the shim's back. +An install through the shim warns when `composer.json` requires either plugin and neither that key nor `config.allow-plugins` turns it off; the check reads the keys with `python3` rather than matching text, so a `disable-plugin` under another package's `extra` cannot silence it, and is skipped when `python3` is not on `PATH`. +The generated shell and PHP templates run the binary and configuration through repository-relative paths and rely on git running hooks from the top level, which the dispatcher keeps; the PHP template additionally derives `.git` from the hook file's location (`Local/PHP.php:84`), so check it from the common dir in a linked worktree. +`CAPTAINHOOK_SKIP_HOOKS=1` skips a run. + ## Registry [`registry.json`](registry.json) holds one adapter per manager under `adapters.`: @@ -137,6 +151,8 @@ Remote configs are cloned into `/lefthook-remotes`, inside `.git`, so | `install.aliases` | Other spellings of a flag-style subcommand (`overcommit`: `-i`), matched anywhere in argv. | | `install.strategy` | One of the install strategies above. | | `install.flag` | For `git-dir-flag`: the long option that takes the git directory. | +| `install.match` | How a positional subcommand is recognised: `exact` (the default, argv[0] is the action) or `prefix` (any non-option argument that prefixes it, for a Symfony Console app that takes global options before the command). | +| `install.vendor_bin` | `true` for a manager Composer installs into a project's `vendor/bin`: the shim falls back to `/vendor/bin/` when `PATH` holds no real binary. | | `install.runtime_env` | For `hooks-path-env`: also give every hook run the same `core.hooksPath` override, so a manager that re-checks its install at run time accepts the chained hook. | | `home` | Omitted for a manager that keeps nothing outside the repository. | | `home.env` | The environment variable that relocates the home. | @@ -159,7 +175,7 @@ Its operations are carried out by the scripts generated from the registry: | Operation | Run by | Invocation | Behaviour | |---|---|---|---| -| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install`, `lefthook install` or `overcommit --install` (also `-i`, or after other options) | Applies the adapter's install strategy: `git-dir-flag` runs `prek install --git-dir [args…]`; `hooks-path-env` runs `overcommit --install` or `-i` with `core.hooksPath` pointed at `/hooks`, appending to any existing `GIT_CONFIG_*` entries; `hide-global-config` runs the install with `GIT_CONFIG_GLOBAL=/dev/null` when the global config holds a `core.hooksPath`. Left unchanged when `args` already carry the target flag or ask for `-h` / `--help`, or outside a git work tree; a `hide-global-config` install carrying `--force` or `--reset-hooks-path` still gets the override, so neither reaches the shared directory through the shim. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim. Exits 127 when the name it is installed under has no adapter or no real binary is found. | +| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install`, `lefthook install`, `overcommit --install` (also `-i`, or after other options) or `captainhook -n install` (also `inst`) | Applies the adapter's install strategy: `git-dir-flag` runs `prek install --git-dir [args…]`; `hooks-path-env` runs `overcommit --install` or `-i`, or `captainhook install` in any of its spellings, with `core.hooksPath` pointed at `/hooks`, appending to any existing `GIT_CONFIG_*` entries; `hide-global-config` runs the install with `GIT_CONFIG_GLOBAL=/dev/null` when the global config holds a `core.hooksPath`. Left unchanged when `args` already carry the target flag or ask for `-h` / `--help`, or outside a git work tree; a `hide-global-config` install carrying `--force` or `--reset-hooks-path` still gets the override, so neither reaches the shared directory through the shim. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim, or `/vendor/bin/` for an adapter with `install.vendor_bin`. Exits 127 when the name it is installed under has no adapter or no real binary is found. | | Host home and run-time override | [`git-hook-dispatcher.sh`](../agent-isolation/git-hook-dispatcher.sh), from `~/.claude/git-hooks/` | Every hook git runs, before it chains to the repository's own hook | Outside the sandbox (no `SANDBOX_RUNTIME`), exports `=~/.local/share/` for every registered manager unless the operator's value lies outside `~/.cache`, and moves `XDG_CACHE_HOME` to `~/.local/share/prek-host/cache` by the same rule. Sandboxed hooks keep their values. Before chaining to a hook whose marker matches an adapter with `install.runtime_env`, exports the same `core.hooksPath` override the shim used, so the manager's own run-time check accepts the hook. | | Home check | [`sandbox-lint`](../sandbox-lint/) | `uv run --project tools/sandbox-lint sandbox-lint` | Fails a settings file whose `permissions.deny` lacks `Edit(/**)` for any registered manager, and, for a project that uses a manager (see [Detection](#detection)), one whose `env.` is missing or overlaps the default home. | | Registry sync | [`tools/dev/sync-hook-managers.py`](../dev/sync-hook-managers.py) | `tools/dev/sync-hook-managers.py [--check]`; the `sync-hook-managers` prek hook | Validates `registry.json`, rewrites the generated blocks of the three scripts above, and checks the committed settings and the sandbox-lint baseline. Exits 1 when a block changed (re-stage and commit), or, with `--check`, when one would. | @@ -182,6 +198,7 @@ What a repository or an operator configures is where each manager's environments | Adapter | Language | Install strategy | Home | Status | |---|---|---|---|---| +| `captainhook` | PHP | `hooks-path-env` (`install`, matched as a prefix at any non-option argument); detected by `captainhook.json` or a hook mentioning `CaptainHook`; `vendor/bin` fallback | none (no external home) | shipping (verified: v5.29.2 on PHP 8.5.12 with Composer 2.11.1; the install through the shim uses the `vendor/bin` fallback and writes the repository's own hooks, and a bare install replaces the shared dispatcher) | | `cargo-husky` | Rust | `none`; detected by a hook mentioning `cargo-husky` | none (no external home) | shipping (in linked worktrees `build.rs` skips hook install: searches for a `.git` directory) | | `git-build-hook` | Java | `none`; detected by `pom.xml` (nested too) or a hook mentioning `git-build-hook` | none (no external home) | shipping (JGit `getGitDir()` targets `/hooks` in linked worktrees; `configure` goal can set repo-local `core.hooksPath`) | | `git-code-format-maven-plugin` | Java | `none`; detected by `pom.xml` (nested too) or a hook mentioning `git-code-format-maven-plugin` | none (no external home) | shipping (JGit `getCommonDirectory()` targets common hooks dir, coexisting cleanly in linked worktrees) | @@ -230,11 +247,13 @@ An install strategy is written from the manager's source. Where a run against th - **`overcommit`, v0.73.0 on Ruby 3.2.3, Ubuntu 24.04, whole-user dispatcher.** `overcommit --install` through the shim wrote `/hooks/` (a hook carrying the `overcommit` marker) and left `~/.claude/git-hooks/` identical — no `overcommit-hook` in it, no `old-hooks/`, the global `core.hooksPath` unchanged, and no repository-level `core.hooksPath` left behind. A commit behind the dispatcher ran the repository's own `.overcommit.yml`: its `PreCommit` `TrailingWhitespace` hook rejected a trailing-whitespace commit, and a clean commit went through with no self-update failure. Both controls fail without the adapter, which is why it needs a strategy of its own. A bare `overcommit --install` (no shim) installs *into* the shared dir and moves the dispatcher's symlinks aside — `Hook '…/pre-commit' already exists and was not installed by Overcommit. Moving to '…/.claude/git-hooks/old-hooks'` — and a correct install under a dispatcher that does not export the run-time override dies in overcommit's own check with `No such file or directory @ rb_file_s_size - ~/.claude/git-hooks/overcommit-hook` (`installer.rb:59`, `compare_file`). +- **`captainhook`, v5.29.2 on PHP 8.5.12, Composer 2.11.1 and git 2.54.0 in a `composer:2` container, whole-user dispatcher.** The global `core.hooksPath` named a shared dir with `git-hook-dispatcher.sh` symlinked under every hook name; the repository required `captainhook/captainhook` through Composer, so its binary was only at `vendor/bin/captainhook`. `~/.claude/bin/captainhook install` exited 0 through the shim: it found the binary with the `vendor/bin` fallback, installed `pre-commit` (525 bytes, carrying the CaptainHook marker) and the other hooks in the repository's own `.git/hooks/`, left every shared symlink and the global `core.hooksPath` unchanged, and created no `old-hooks/`. A commit behind the dispatcher ran the repository's own `captainhook.json`: its `pre-commit` `Exists` action failed on the missing file, printed `Error: 1 file(s) could not be found`, and the commit was rejected. + The control fails without the adapter. A bare `vendor/bin/captainhook install` with the shim removed from `PATH` wrote through the shared dispatcher symlinks, so `git-hook-dispatcher.sh` became CaptainHook's last-installed hook instead of the dispatcher. The next commit did not run the repository's own hook and died inside the wrong CaptainHook subcommand. ## Adding an adapter 1. Read the manager's source for the answers the registry needs — where its install writes and whether it honours, refuses or sets `core.hooksPath`; whether a `PATH` shim can reach the install; the home variable, its default, and whether it expands `~`; whether a hook run executes code from the home. -2. Add the adapter to `registry.json`. If it needs an install strategy the shim does not implement, add the strategy to the shim and to `STRATEGIES` in the generator in the same change. +2. Add the adapter to `registry.json`. If it needs an install strategy the shim does not implement, add the strategy to the shim and to `STRATEGIES` in the generator in the same change. A positional action a manager accepts after global options or in shortened form sets `install.match: prefix`; one Composer installs into `vendor/bin` sets `install.vendor_bin`. 3. Give the detection rules: the config file names, whether nested configs count, and a marker every hook it installs contains. 4. Deny the default home in `.claude/settings.json` and `tools/sandbox-lint/expected.json`, and, if this repository uses the manager, give the sandbox home in their `env`. 5. Run `tools/dev/sync-hook-managers.py`, then the agent-isolation, sandbox-lint and dev tests, and add shim and dispatcher tests for the new strategy. diff --git a/tools/hook-manager/registry.json b/tools/hook-manager/registry.json index 39ee8acc5..4d296e809 100644 --- a/tools/hook-manager/registry.json +++ b/tools/hook-manager/registry.json @@ -1,6 +1,23 @@ { "$comment": "Hook-manager adapters for the secure agent setup. The contract is tools/hook-manager/README.md; tools/dev/sync-hook-managers.py validates this file and writes it into the scripts that implement the contract.", "adapters": { + "captainhook": { + "language": "PHP", + "upstream": "https://github.com/captainhook-git/captainhook", + "detect": { + "config_files": [ + "captainhook.json" + ], + "nested": false, + "hook_marker": "CaptainHook" + }, + "install": { + "subcommand": "install", + "strategy": "hooks-path-env", + "match": "prefix", + "vendor_bin": true + } + }, "cargo-husky": { "language": "Rust", "upstream": "https://github.com/rhysd/cargo-husky",