diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 85951ea47..6aa4d4201 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -389,7 +389,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 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 and overcommit) 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/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index a1701791c..d96537ee6 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -1428,16 +1428,20 @@ 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 honours `core.hooksPath`, so a bare -`prek install` under whole-user scope would write its shim into the +**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 ([`tools/agent-isolation/hook-manager-shim.sh`](../../tools/agent-isolation/hook-manager-shim.sh), -installed as `~/.claude/bin/prek` with `~/.claude/bin` prepended to -PATH) that rewrites **only** `prek install` — injecting -`--git-dir "$(git rev-parse --git-common-dir)"` so the shim lands +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. -Every other `prek` command passes straight through, and the shim is +Every other command passes straight through, and the shim is a no-op on hosts with no global `core.hooksPath`. +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. The shim, the host-only homes and the sandbox-lint checks all follow the [hook-manager contract](../../tools/hook-manager/README.md): one script serves every adapter in its registry, keyed by the name it is @@ -1450,6 +1454,7 @@ dispatchers + hook-manager shim): | Repo's local hook | Result | |---|---| | 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 | | 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 | @@ -1479,7 +1484,7 @@ Managers using the `none` install strategy (`tools/hook-manager/README.md`) requ Reversal is the same as simple whole-user (`git config --global --unset core.hooksPath`), plus removing `~/.claude/bin` from PATH to -restore the stock `prek`. +restore the stock manager binaries. ## Working directories under the read-outside-working-directories block 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 8238917ff..532f23420 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 @@ -326,28 +326,32 @@ for the full rationale + the validated behaviour matrix. with the original argv + inherited stdin, so a failing local `pre-commit` / `pre-push` still aborts the git operation. -2. **Install the hook-manager PATH shim** so `prek install` writes its - shim into the repo-local `.git/hooks/` (where the dispatcher - chains) instead of the shared dir. Copy +2. **Install the hook-manager PATH shim** so the manager's install + writes its hook into the repo-local `.git/hooks/` (where the + dispatcher chains) instead of the shared dir. Copy [`tools/agent-isolation/hook-manager-shim.sh`](../../../../tools/agent-isolation/hook-manager-shim.sh) into `~/.claude/bin/` (or symlink from `~/.claude-config/bin/`), mode `0755`, once for each adapter in the [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` today. - A manager whose install strategy is `none` needs no install fix - and gets no shim. + that a repository the operator works in uses — `~/.claude/bin/prek` and + `~/.claude/bin/overcommit` 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 (or tracked below it, for prek's nested projects) or one of its installed 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`. Install no shim for a manager no repository uses. The shim + `.pre-commit-config.yml`, and for overcommit it is `.overcommit.yml`. + Install no shim for a manager no repository uses. The shim takes the tool from the name it is installed under. For prek it rewrites only `prek install` (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); **every other `prek` invocation passes through - unchanged**. It is a no-op on hosts with no global + git work tree); for overcommit it rewrites only + `overcommit --install` (or `-i`, `-fi`, `--inst`, anywhere in argv), + pointing `core.hooksPath` at + `/hooks`. **Every other invocation passes + through unchanged.** 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. @@ -360,9 +364,9 @@ for the full rationale + the validated behaviour matrix. ``` Confirm the rc path with the operator; print the line for them - to paste. Until it is on PATH ahead of the real `prek`, the - shim is inert and `prek install` reverts to writing into the - shared dir. + to paste. Until it is on PATH ahead of the real manager binary, + the shim is inert and the manager's install reverts to writing + into the shared dir. 4. **Tell the operator how their existing prek repos are picked up.** Repos that already ran `prek install` before this setup have @@ -373,7 +377,10 @@ for the full rationale + the validated behaviour matrix. locally via the shim. Only repos where the shim already landed in the shared dir (from a `prek install` run *while* `core.hooksPath` was set but *before* this dispatcher setup) - need a one-time `prek install` re-run through the shim. + need a one-time `prek install` re-run through the shim. An overcommit + repository installed before the switch already has its hook in + `.git/hooks/`; running `overcommit --install` through the shim + rewrites it there after the switch. The `.` entry stays in the committed project-scope `allowRead` regardless — the explicit absolute path in 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 19c5d8bed..0b316d11f 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:656a6e3f62b13ec0" +FRAMEWORK_FINGERPRINT = "sha256:db43bec52c1d2c7c" diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index 226353a7a..4ef4f5b34 100644 --- a/tools/agent-isolation/README.md +++ b/tools/agent-isolation/README.md @@ -97,8 +97,8 @@ per runtime — see [`docs/adapters/add-a-harness.md`](../../docs/adapters/add-a | [`claude-term-bg.sh`](claude-term-bg.sh) | **Opt-in quality-of-life helper (not a security control).** Keeps a calm baseline background and tints it only when Claude genuinely wants you to act (never while working, and never when it merely *finished* a turn), so a window you've tabbed away from can't sit blocked unnoticed. Distinguishes "blocked on a decision" from "finished and idle" — which look identical at the `Stop` event — via three signals across six hooks: `Stop` → `stop` (heuristic — tints only if the final assistant message reads as a question/request; a completion stays calm; needs `python3`/`python`, else defaults calm); `PreToolUse` (matcher `AskUserQuestion`) → `wait` (exact — a structured question was posed); `PostToolUse` (matcher `*`) → `reset` (calm while working, and clears the tint the instant you approve a permission prompt or answer a question); `Notification` → `notify` (tints for permission prompts only — the plain idle ping is a no-op so it can't wipe a pending question's tint); and `UserPromptSubmit` + `SessionStart` → `reset` (you replied / fresh session clears any stale tint). Writes the OSC escape to the Claude pty discovered by walking the process tree (hooks have no controlling tty); the only deterministic reset is an explicit `CLAUDE_RESET_BG` colour via OSC 11 (iTerm2 ignores OSC 111). Colours overridable via `CLAUDE_WAIT_BG` / `CLAUDE_RESET_BG`. Tested on iTerm2 + macOS; fail-soft elsewhere. See [`docs/setup/secure-agent-setup.md` → *Waiting-for-input terminal tint*](../../docs/setup/secure-agent-setup.md#waiting-for-input-terminal-tint). | | [`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 / pre-commit / 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). 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` today). The tool is the name it is installed under. Rewrites **only** the adapter's install subcommand 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. 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). | +| [`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 / 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` and `overcommit` 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. 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/git-hook-dispatcher.sh b/tools/agent-isolation/git-hook-dispatcher.sh index 11fa36a61..e572fbbf7 100755 --- a/tools/agent-isolation/git-hook-dispatcher.sh +++ b/tools/agent-isolation/git-hook-dispatcher.sh @@ -113,7 +113,7 @@ if [ -x "$local_hook" ] && [ "$lh_canon" != "$self_canon" ]; then fi # --- hook managers' homes outside the sandbox ----------------------------- -# A hook manager (prek, ...: tools/hook-manager/registry.json) runs a hook from +# A hook manager (prek, overcommit, ...: tools/hook-manager/registry.json) runs a hook from # environments under its home, ~/.cache/ by default, which the sandbox's # ~/.cache grant makes writable. A hook run outside the sandbox (a commit from # a terminal, or one Claude Code ran with the sandbox bypassed) must not run @@ -130,6 +130,7 @@ fi # Entries: = # BEGIN generated by tools/dev/sync-hook-managers.py; do not edit hook_manager_homes="PREK_HOME=prek-host" +hook_manager_runtime_envs="overcommit:overcommit" # END generated by tools/dev/sync-hook-managers.py if [ -z "${SANDBOX_RUNTIME:-}" ]; then for _entry in $hook_manager_homes; do @@ -152,6 +153,35 @@ if [ -z "${SANDBOX_RUNTIME:-}" ]; then esac fi +# --- managers that re-read core.hooksPath at run time ----------------------- +# overcommit re-checks its install on every hook run and compares the result +# with `git config --get core.hooksPath`. While the dispatcher chains into its +# hook, that value must name the repository's own hooks dir, not the shared +# one, or overcommit's self-update check fails. Entries: :. +# The override is exported, so git commands the hook starts inherit it. A +# command aimed at another repository (a submodule, a temporary clone, +# `git -C `) then sees this repository's hooks path instead of the +# target's dispatcher; only overcommit's self-check needs the value, so this +# remains a documented limitation of the strategy (tools/hook-manager/README.md). +apply_hook_manager_runtime_env() { + for _entry in $hook_manager_runtime_envs; do + _marker="${_entry#*:}" + if grep -q -- "$_marker" "$local_hook" 2>/dev/null; then + _i="${GIT_CONFIG_COUNT:-0}" + case "$_i" in + ''|*[!0-9]*) _i=0 ;; + esac + export "GIT_CONFIG_KEY_$_i=core.hooksPath" + export "GIT_CONFIG_VALUE_$_i=$common/hooks" + export GIT_CONFIG_COUNT=$((_i + 1)) + fi + done +} + +if [ "$has_local" = 1 ]; then + apply_hook_manager_runtime_env +fi + # --- non-post-checkout: exec local for a perfect passthrough --------------- if [ "$hook" != "post-checkout" ]; then if [ "$has_local" = 1 ]; then diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index ed4a83a41..f8253d854 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -17,7 +17,7 @@ # under the License. # # hook-manager-shim.sh — a transparent front for a git-hook manager -# (prek today; see tools/hook-manager/registry.json), installed as +# (prek and overcommit 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. @@ -41,13 +41,18 @@ # # WHAT THIS SHIM DOES # ------------------- -# * ` [...]` -> rewritten per the +# * ` [...]` -> rewritten per the # adapter's install strategy so the hook lands in # "$(git rev-parse --git-common-dir)/hooks" (absolute, worktree- # safe: `--git-common-dir` resolves linked worktrees to the main # checkout's hooks dir) — UNLESS the caller already chose a # 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`). # git-dir-flag: inject ` ` (prek: --git-dir) +# hooks-path-env: append GIT_CONFIG_* so the manager writes to +# `/hooks` (overcommit: --install, -i) # * every other invocation (`prek run`, `prek autoupdate`, # `--version`, bare `prek`, ...) is passed through to the real # binary **unchanged**. @@ -65,6 +70,7 @@ tool="$(basename "$0")" # BEGIN generated by tools/dev/sync-hook-managers.py; do not edit adapter() { case "$1" in + 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 ;; *) return 1 ;; esac @@ -97,20 +103,67 @@ if [ -z "$real_tool" ]; then exit 127 fi -# --- only rewrite the install subcommand; everything else is transparent --- -if [ "${1:-}" != "$install_subcommand" ]; then +# --- 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 +# the whole argv for it and its aliases (`overcommit -i`). Option parsers such +# as Ruby's OptionParser also take a single-letter alias inside a bundle of +# short switches (`-fi`) and an unambiguous prefix of a long one (`--inst`), +# so those count too; rewriting a non-install run only points core.hooksPath +# at the repository's own hooks, which is harmless. +install_invoked=0 +case "$install_subcommand" in + -*) + install_actions="${install_aliases:-$install_subcommand}" + for _a in "$@"; do + for _action in $install_actions; do + case "$_action" in + --*) + # Any prefix (`--i` on): fail closed, since a wrongly rewritten run is harmless. + if [ "${#_a}" -ge 3 ] && [ "${_action#"$_a"}" != "$_action" ]; then + install_invoked=1 + break 2 + fi + ;; + -?) + case "$_a" in + --*) ;; + -*"${_action#-}"*) + install_invoked=1 + break 2 + ;; + esac + ;; + *) + if [ "$_a" = "$_action" ]; then + install_invoked=1 + break 2 + fi + ;; + esac + done + done + ;; + *) + [ "${1:-}" = "$install_subcommand" ] && install_invoked=1 + ;; +esac + +if [ "$install_invoked" = 0 ]; then exec "$real_tool" "$@" fi -shift # drop the install subcommand - # Respect a target the caller chose, and never interfere with help output. inject=1 for _a in "$@"; do case "$_a" in -h|--help) inject=0; break ;; - "$install_flag"|"$install_flag"=*) inject=0; break ;; esac + if [ -n "${install_flag:-}" ]; then + case "$_a" in + "$install_flag"|"$install_flag"=*) inject=0; break ;; + esac + fi done if [ "$inject" = 1 ]; then @@ -121,11 +174,24 @@ if [ "$inject" = 1 ]; then esac case "$install_strategy" in git-dir-flag) + # Only positional actions use this strategy; the action is argv[0]. + shift exec "$real_tool" "$install_subcommand" "$install_flag" "$gitdir" "$@" ;; + hooks-path-env) + # Append at the next index so a caller's GIT_CONFIG_* entries survive. + _i="${GIT_CONFIG_COUNT:-0}" + case "$_i" in + ''|*[!0-9]*) _i=0 ;; + esac + export "GIT_CONFIG_KEY_$_i=core.hooksPath" + export "GIT_CONFIG_VALUE_$_i=${gitdir%/}/hooks" + export GIT_CONFIG_COUNT=$((_i + 1)) + exec "$real_tool" "$@" + ;; esac fi # not in a git work tree: let the real tool produce its own error fi -exec "$real_tool" "$install_subcommand" "$@" +exec "$real_tool" "$@" diff --git a/tools/agent-isolation/tests/test_git_hook_dispatcher.py b/tools/agent-isolation/tests/test_git_hook_dispatcher.py index b5acee0dc..c40a9fdda 100644 --- a/tools/agent-isolation/tests/test_git_hook_dispatcher.py +++ b/tools/agent-isolation/tests/test_git_hook_dispatcher.py @@ -53,11 +53,23 @@ def _repo_with_local_hook( return repo, hooks / "pre-commit", seen +def _common_dir(repo: Path) -> str: + result = subprocess.run( + ["bash", "-c", 'cd "$(git rev-parse --git-common-dir)" && pwd -P'], + cwd=repo, + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + def _run(repo: Path, hook: Path, env_extra: dict[str, str]) -> None: env = { k: v for k, v in os.environ.items() if k not in ("PREK_HOME", "SANDBOX_RUNTIME", "XDG_CACHE_HOME") + and not k.startswith("GIT_CONFIG_") } env.update(env_extra) subprocess.run(["bash", str(hook)], cwd=repo, env=env, check=True) @@ -95,6 +107,58 @@ def test_an_operator_prek_home_outside_the_cache_is_left_alone(tmp_path: Path) - assert seen.read_text() == "/opt/prek-home" +def test_an_overcommit_hook_gets_the_repository_hooks_path(tmp_path: Path) -> None: + repo, hook, seen = _repo_with_local_hook(tmp_path, config=".overcommit.yml") + local = repo / ".git" / "hooks" / "pre-commit" + local.write_text( + f'#!/bin/sh\n# overcommit\nprintf "%s" "${{GIT_CONFIG_VALUE_0-}}" > {seen}\n' + ) + local.chmod(0o755) + _run(repo, hook, {}) + assert seen.read_text() == f"{_common_dir(repo)}/hooks" + + +def test_an_overcommit_hook_preserves_existing_git_config(tmp_path: Path) -> None: + repo, hook, seen = _repo_with_local_hook(tmp_path, config=".overcommit.yml") + local = repo / ".git" / "hooks" / "pre-commit" + local.write_text( + '#!/bin/sh\n# overcommit\n' + 'printf "%s\\n" "${GIT_CONFIG_COUNT-}" "${GIT_CONFIG_KEY_0-}" ' + '"${GIT_CONFIG_VALUE_0-}" "${GIT_CONFIG_KEY_1-}" ' + f'"${{GIT_CONFIG_VALUE_1-}}" > {seen}\n' + ) + local.chmod(0o755) + _run( + repo, + hook, + { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "user.name", + "GIT_CONFIG_VALUE_0": "Someone", + }, + ) + assert seen.read_text().splitlines() == [ + "2", + "user.name", + "Someone", + "core.hooksPath", + f"{_common_dir(repo)}/hooks", + ] + + +def test_a_hook_without_the_overcommit_marker_keeps_the_shared_path( + tmp_path: Path, +) -> None: + repo, hook, seen = _repo_with_local_hook(tmp_path) + local = repo / ".git" / "hooks" / "pre-commit" + local.write_text( + f'#!/bin/sh\nprintf "%s" "${{GIT_CONFIG_VALUE_0-}}" > {seen}\n' + ) + local.chmod(0o755) + _run(repo, hook, {}) + assert seen.read_text() == "" + + def test_a_repository_without_its_own_hook_is_a_no_op(tmp_path: Path) -> None: repo, hook, seen = _repo_with_local_hook(tmp_path) (repo / ".git" / "hooks" / "pre-commit").unlink() diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index c84b13517..4380191a7 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -33,12 +33,23 @@ def _setup(tmp_path: Path, name: str = "prek") -> tuple[dict[str, str], Path]: real = tmp_path / "real" real.mkdir() argv = tmp_path / "argv" - (real / name).write_text(f'#!/bin/sh\nprintf "%s\\n" "$@" > {argv}\n') + env_file = tmp_path / "env" + (real / name).write_text( + f'#!/bin/sh\nprintf "%s\\n" "$@" > {argv}\n' + f'printf "%s\\n" "${{GIT_CONFIG_COUNT-}}" ' + f'"${{GIT_CONFIG_KEY_0-}}" "${{GIT_CONFIG_VALUE_0-}}" ' + f'"${{GIT_CONFIG_KEY_1-}}" "${{GIT_CONFIG_VALUE_1-}}" > {env_file}\n' + ) (real / name).chmod(0o755) shims = tmp_path / "shims" shims.mkdir() (shims / name).symlink_to(SHIM) - env = dict(os.environ, PATH=f"{shims}:{real}:/usr/bin:/bin") + env = { + key: value + for key, value in os.environ.items() + if not key.startswith("GIT_CONFIG_") + } + env["PATH"] = f"{shims}:{real}:/usr/bin:/bin" return env, argv @@ -70,13 +81,116 @@ def test_install_targets_the_common_git_dir(tmp_path: Path) -> None: ] +def _common_dir(repo: Path) -> str: + result = subprocess.run( + ["bash", "-c", 'cd "$(git rev-parse --git-common-dir)" && pwd -P'], + cwd=repo, + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + +def test_overcommit_install_targets_the_common_hooks_dir(tmp_path: Path) -> None: + env, argv = _setup(tmp_path, name="overcommit") + repo = _repo(tmp_path) + assert _run(env, repo, "--install", name="overcommit").returncode == 0 + assert argv.read_text().splitlines() == ["--install"] + assert (tmp_path / "env").read_text().splitlines() == [ + "1", + "core.hooksPath", + f"{_common_dir(repo)}/hooks", + "", + "", + ] + + +def test_overcommit_install_help_passes_through(tmp_path: Path) -> None: + env, argv = _setup(tmp_path, name="overcommit") + assert ( + _run(env, _repo(tmp_path), "--install", "--help", name="overcommit").returncode + == 0 + ) + assert argv.read_text().splitlines() == ["--install", "--help"] + assert (tmp_path / "env").read_text().splitlines() == ["", "", "", "", ""] + + +@pytest.mark.parametrize( + "args", + [ + ["-i"], + ["--force", "--install"], + ["--force", "-i"], + ["-fi"], + ["--i"], + ["--inst"], + ["--force", "--insta"], + ], +) +def test_overcommit_install_flag_is_found_anywhere( + tmp_path: Path, args: list[str] +) -> None: + env, argv = _setup(tmp_path, name="overcommit") + repo = _repo(tmp_path) + assert _run(env, repo, *args, name="overcommit").returncode == 0 + assert argv.read_text().splitlines() == args + assert (tmp_path / "env").read_text().splitlines() == [ + "1", + "core.hooksPath", + f"{_common_dir(repo)}/hooks", + "", + "", + ] + + +@pytest.mark.parametrize("args", [["--run"], ["--uninstall"], ["--"]]) +def test_overcommit_other_actions_pass_through(tmp_path: Path, args: list[str]) -> None: + env, argv = _setup(tmp_path, name="overcommit") + repo = _repo(tmp_path) + assert _run(env, repo, *args, name="overcommit").returncode == 0 + assert argv.read_text().splitlines() == args + assert (tmp_path / "env").read_text().splitlines() == ["", "", "", "", ""] + + +def test_overcommit_install_preserves_existing_git_config(tmp_path: Path) -> None: + env, _ = _setup(tmp_path, name="overcommit") + 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="overcommit").returncode == 0 + assert (tmp_path / "env").read_text().splitlines() == [ + "2", + "user.name", + "Someone", + "core.hooksPath", + f"{_common_dir(repo)}/hooks", + ] + + def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: env, argv = _setup(tmp_path) 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. 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, @@ -105,6 +219,7 @@ def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: ["install", "--git-dir=/x"], ["install", "--help"], ["run", "--all-files"], + ["run", "install"], ["--version"], ], ) diff --git a/tools/dev/sync-hook-managers.py b/tools/dev/sync-hook-managers.py index 63f15fa2d..331f10b6f 100755 --- a/tools/dev/sync-hook-managers.py +++ b/tools/dev/sync-hook-managers.py @@ -26,7 +26,8 @@ and `END ...` markers: - `tools/agent-isolation/hook-manager-shim.sh`: the install rewrite per tool. -- `tools/agent-isolation/git-hook-dispatcher.sh`: the host-only home per tool. +- `tools/agent-isolation/git-hook-dispatcher.sh`: the host-only home and + run-time override per tool. - `tools/sandbox-lint/src/sandbox_lint/__init__.py`: the homes the lint checks. It also checks, without writing, that the committed settings and the @@ -54,9 +55,11 @@ SETTINGS = (Path(".claude/settings.json"), Path("tools/sandbox-lint/expected.json")) #: Install strategies the shim implements. A new one lands with its shim branch. -STRATEGIES = {"git-dir-flag", "none"} +STRATEGIES = {"git-dir-flag", "hooks-path-env", "none"} _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-]*$") _ENV = re.compile(r"^[A-Z][A-Z0-9_]*$") _FLAG = re.compile(r"^--[a-z][a-z0-9-]*$") _DIR = re.compile(r"^[a-z][a-z0-9.-]*$") @@ -112,16 +115,37 @@ def validate(registry: dict[str, Any]) -> list[str]: f"({', '.join(sorted(STRATEGIES))})" ) if strategy != "none": - if not isinstance(install.get("subcommand"), str) or not _NAME.match(install["subcommand"]): - errors.append(f"{where}.install.subcommand: missing") + subcommand = install.get("subcommand") + if not isinstance(subcommand, str) or not _SUBCOMMAND.match(subcommand): + errors.append(f"{where}.install.subcommand: missing or not a word or --flag") + aliases = install.get("aliases") + if aliases is not None: + if ( + not isinstance(aliases, list) + or not aliases + or not all(isinstance(alias, str) and _ALIAS.match(alias) for alias in aliases) + ): + errors.append(f"{where}.install.aliases: a non-empty list of flags") + elif not isinstance(subcommand, str) or not subcommand.startswith("-"): + errors.append(f"{where}.install.aliases: only a flag-style subcommand takes aliases") + elif subcommand in aliases: + errors.append(f"{where}.install.aliases: must not repeat the subcommand") if strategy == "git-dir-flag" and not ( isinstance(install.get("flag"), str) and _FLAG.match(install["flag"]) ): errors.append( f"{where}.install.flag: git-dir-flag needs the long option that takes the git dir" ) - elif "flag" in install: - errors.append(f"{where}.install.flag: strategy 'none' takes no flag") + else: + for key in ("flag", "aliases"): + 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): + errors.append(f"{where}.install.runtime_env: must be true or false") + if install.get("runtime_env") and strategy != "hooks-path-env": + errors.append( + f"{where}.install.runtime_env: only hooks-path-env needs the override at run time" + ) home = adapter.get("home") if home is None: continue # a manager that keeps nothing outside the repository @@ -161,15 +185,26 @@ def render(registry: dict[str, Any]) -> dict[Path, str]: if install.get("strategy") == "none": continue fields = [f"install_subcommand={install['subcommand']}", f"install_strategy={install['strategy']}"] + if "aliases" in install: + actions = " ".join([install["subcommand"], *install["aliases"]]) + fields.append(f"install_aliases='{actions}'") if "flag" in install: fields.append(f"install_flag={install['flag']}") 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")] + runtime = [ + f"{n}:{a['detect']['hook_marker']}" + for n, a in sorted(adapters.items()) + if a["install"].get("runtime_env") + ] dispatcher = ( 'hook_manager_homes="' + " ".join(f"{a['home']['env']}={a['home']['host'][15:]}" for _, a in homes) + '"\n' + + 'hook_manager_runtime_envs="' + + " ".join(runtime) + + '"\n' ) lint = "HOOK_MANAGER_HOMES = (\n" for name, a in homes: diff --git a/tools/dev/tests/test_sync_hook_managers.py b/tools/dev/tests/test_sync_hook_managers.py index d761d4281..ecca144db 100644 --- a/tools/dev/tests/test_sync_hook_managers.py +++ b/tools/dev/tests/test_sync_hook_managers.py @@ -75,7 +75,9 @@ def test_the_committed_registry_follows_the_contract(registry: dict[str, Any]) - @pytest.mark.parametrize( ("path", "value", "message"), [ - (("install", "strategy"), "hooks-path-env", "install.strategy"), + (("install", "strategy"), "not-a-strategy", "install.strategy"), + (("install", "subcommand"), "install --now", "install.subcommand"), + (("install", "runtime_env"), "yes", "install.runtime_env"), (("detect", "config_files"), [], "detect.config_files"), (("detect", "config_files"), ["sub/prek.toml"], "detect.config_files"), (("detect", "nested"), "yes", "detect.nested"), @@ -98,10 +100,52 @@ def test_a_registry_that_breaks_the_contract_is_refused( assert any(message in e for e in mod.validate(broken)), mod.validate(broken) +def test_overcommit_uses_its_flag_and_runtime_env(registry: dict[str, Any]) -> None: + adapter = registry["adapters"]["overcommit"] + assert "home" not in adapter + assert adapter["install"] == { + "subcommand": "--install", + "aliases": ["-i"], + "strategy": "hooks-path-env", + "runtime_env": True, + } + rendered = mod.render(registry) + assert ( + "overcommit) install_subcommand=--install; install_strategy=hooks-path-env; " + "install_aliases='--install -i'" in rendered[mod.SHIM] + ) + assert 'hook_manager_runtime_envs="overcommit:overcommit"' in rendered[mod.DISPATCHER] + + +@pytest.mark.parametrize( + ("aliases", "message"), + [ + ([], "install.aliases: a non-empty list of flags"), + ("-i", "install.aliases: a non-empty list of flags"), + (["install"], "install.aliases: a non-empty list of flags"), + (["--install"], "install.aliases: must not repeat the subcommand"), + ], +) +def test_bad_install_aliases_are_refused(registry: dict[str, Any], aliases: object, message: str) -> None: + broken = copy.deepcopy(registry) + broken["adapters"]["overcommit"]["install"]["aliases"] = aliases + assert any(message 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"] + assert any("only a flag-style subcommand takes aliases" in e for e in mod.validate(broken)), mod.validate( + broken + ) + + def test_a_manager_without_a_home_is_allowed(registry: dict[str, Any]) -> None: del registry["adapters"]["prek"]["home"] assert mod.validate(registry) == [] - assert mod.render(registry)[mod.DISPATCHER] == 'hook_manager_homes=""\n' + assert mod.render(registry)[mod.DISPATCHER] == ( + 'hook_manager_homes=""\nhook_manager_runtime_envs="overcommit:overcommit"\n' + ) def test_strategy_none_adapter_omits_shim_and_needs_no_subcommand(registry: dict[str, Any]) -> None: @@ -124,6 +168,13 @@ def test_strategy_none_with_flag_is_refused(registry: dict[str, Any]) -> None: assert any("strategy 'none' takes no flag" in e for e in errors), errors +def test_strategy_none_with_aliases_is_refused(registry: dict[str, Any]) -> None: + broken = copy.deepcopy(registry) + broken["adapters"]["simple-git-hooks"]["install"]["aliases"] = ["-i"] + errors = mod.validate(broken) + assert any("strategy 'none' takes no aliases" in e for e in errors), errors + + def _add_demo(tree: Path, registry: dict[str, Any]) -> None: extra = copy.deepcopy(registry["adapters"]["prek"]) extra["detect"] = {"config_files": ["demo.toml"], "nested": False, "hook_marker": "demo"} @@ -140,6 +191,7 @@ def test_a_new_adapter_reaches_every_block(tree: Path, registry: dict[str, Any]) assert " demo) install_subcommand=install;" in (tree / mod.SHIM).read_text() dispatcher = (tree / mod.DISPATCHER).read_text() assert 'hook_manager_homes="DEMO_HOME=demo-host PREK_HOME=prek-host"' in dispatcher + assert 'hook_manager_runtime_envs="overcommit:overcommit"' in dispatcher lint = (tree / mod.SANDBOX_LINT).read_text() assert ( ' "DEMO_HOME",\n "~/.cache/demo",\n "~/.cache/demo-sandbox",\n ("demo.toml",),' diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 0414e1e96..d9cfbafec 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -15,6 +15,7 @@ - [Operations](#operations) - [Configuration](#configuration) - [Adapters](#adapters) + - [Verified runs](#verified-runs) - [Adding an adapter](#adding-an-adapter) - [Consumers](#consumers) @@ -66,14 +67,15 @@ A manager's install must put its hook in the repository's own `$(git rev-parse - - `--git-common-dir`, not `--git-dir`: a linked worktree shares its main checkout's hooks. 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 subcommand, and passes every other invocation through unchanged. +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`). 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 | |---|---|---| | `git-dir-flag` | Inserts ` ` after the install subcommand (prek: `--git-dir`; prek refuses a bare install while a global `core.hooksPath` is set). | implemented | | `hide-global-config` | Runs the install with `GIT_CONFIG_GLOBAL=/dev/null`, for a manager that writes to the common dir but refuses while a global `core.hooksPath` is set (pre-commit, lefthook). | planned | -| `hooks-path-env` | Runs the install with `GIT_CONFIG_COUNT=1`, `GIT_CONFIG_KEY_0=core.hooksPath`, `GIT_CONFIG_VALUE_0=/hooks`, for a manager that writes into whatever `core.hooksPath` names (overcommit, CaptainHook). The value must be that absolute path: git treats an empty `core.hooksPath` as set, and hooks then resolve against the filesystem root. | planned | +| `hooks-path-env` | Runs the install with `core.hooksPath=/hooks` in the next free `GIT_CONFIG_*` slot, preserving a caller's existing entries, for a manager that writes into whatever `core.hooksPath` names (overcommit, CaptainHook). The value must be that absolute path: git treats an empty `core.hooksPath` as set, and hooks then resolve against the filesystem root. The run-time override is exported, so git commands the chained hook starts inherit it, including ones aimed at another repository; only the manager's self-check needs it, so that leak is a known limitation. | implemented | | `none` | The manager already writes to the repository's own hooks directory whatever the global value (simple-git-hooks, cargo-husky, the JGit-based Maven plugins). | implemented | A manager whose install the shim cannot reach (one run from `node_modules/.bin` by an npm script, from `vendor/bin` by a composer plugin, or from a `build.rs`, all ahead of `~/.claude/bin` or off `PATH` entirely) needs a strategy outside the shim, and a manager that writes a repository-level `core.hooksPath` itself (husky on every `npm install`) needs the dispatcher to chain to that directory instead; the adapter issues name the candidates. @@ -125,9 +127,11 @@ The two YAML names are shared with pre-commit, so a repository with one uses bot | `detect.config_files` | The config file names that say a repository uses it (bare names, no directories). | | `detect.nested` | Whether a tracked config below the root counts (rule 2 of [Detection](#detection)). | | `detect.hook_marker` | Text every hook the manager installs contains (rule 3). | -| `install.subcommand` | The subcommand the shim rewrites. | +| `install.subcommand` | The subcommand or flag the shim rewrites. | +| `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.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. | | `home.default`, `home.sandbox`, `home.host` | The three directories above. | @@ -136,7 +140,7 @@ The two YAML names are shared with pre-commit, so a repository with one uses bot [`tools/dev/sync-hook-managers.py`](../dev/sync-hook-managers.py) validates the registry against the rules and writes it into the implementations, which run where the registry is not available, as generated blocks: - [`tools/agent-isolation/hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh) — the install rewrite per tool; -- [`tools/agent-isolation/git-hook-dispatcher.sh`](../agent-isolation/git-hook-dispatcher.sh) — the host home per tool; +- [`tools/agent-isolation/git-hook-dispatcher.sh`](../agent-isolation/git-hook-dispatcher.sh) — the host home and run-time override per tool; - [`tools/sandbox-lint/`](../sandbox-lint/) — `HOOK_MANAGER_HOMES`, which fails any settings file whose deny list lacks `Edit(/**)`, and a project's settings file whose `env.` is missing or overlaps the default home when the project uses the manager. It also checks, without writing, that [`.claude/settings.json`](../../.claude/settings.json) and [`tools/sandbox-lint/expected.json`](../sandbox-lint/expected.json) deny every manager's default home and carry the sandbox home of each manager this repository uses. @@ -149,8 +153,8 @@ 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` | Applies the adapter's install strategy — for prek, runs `prek install --git-dir [args…]`. Left unchanged when `args` already carry the flag or `-h` / `--help`, or outside a git work tree. 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. | -| Host home | [`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. | +| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install` or `overcommit --install` (also `-i`, or after other options) | Applies the adapter's install strategy — for prek, runs `prek install --git-dir [args…]`; for overcommit, runs `overcommit --install` or `-i` with `core.hooksPath` pointed at `/hooks`, appending to any existing `GIT_CONFIG_*` entries. Left unchanged when `args` already carry the target flag or `-h` / `--help`, or outside a git work tree. 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. | +| 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. | @@ -176,6 +180,7 @@ What a repository or an operator configures is where each manager's environments | `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) | | `prek` | Rust | `git-dir-flag` (`--git-dir`); detected by `prek.toml`, `.pre-commit-config.yaml` or `.pre-commit-config.yml` (nested too) or a hook mentioning `prek` | `PREK_HOME`: `~/.cache/prek` → sandbox `~/.cache/prek-sandbox`, host `~/.local/share/prek-host` | shipping | +| `overcommit` | Ruby | `hooks-path-env` (`--install`, alias `-i`, found anywhere in argv); detected by `.overcommit.yml` or a hook mentioning `overcommit` | none — it keeps its own state in git config and the repository | shipping | | `simple-git-hooks` | JavaScript | `none`; detected by `package.json`, `.simple-git-hooks.{js,cjs,json}`, `simple-git-hooks.{js,cjs,json}` or a hook mentioning `simple-git-hooks` | none (no external home) | shipping | *Ecosystem notes:* @@ -207,14 +212,22 @@ What a repository or an operator configures is where each manager's environments Standard Gradle plugins and Go hook utilities remain unadapted and unregistered until someone tests them under the dispatcher. `lefthook` (Go) refuses install when a global `core.hooksPath` is set and is reserved for the planned `hide-global-config` install strategy. +### Verified runs + +An install strategy is written from the manager's source. Where a run against the real manager confirmed it, the run is recorded here, with the controls that show what fails without the adapter. + +- **`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`). + ## 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. 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 a shim test for the new strategy. +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. 6. Add a row above, and update the troubleshooting and verify pages if the manager brings a failure mode of its own. +7. Exercise the adapter against the real manager — the install through the shim and a commit behind the dispatcher, plus the control that fails without it — and record the run under [Verified runs](#verified-runs). ## Consumers diff --git a/tools/hook-manager/registry.json b/tools/hook-manager/registry.json index 8f2c5966d..19b9b9a11 100644 --- a/tools/hook-manager/registry.json +++ b/tools/hook-manager/registry.json @@ -66,6 +66,25 @@ "expands_tilde": true } }, + "overcommit": { + "language": "Ruby", + "upstream": "https://github.com/sds/overcommit", + "detect": { + "config_files": [ + ".overcommit.yml" + ], + "nested": false, + "hook_marker": "overcommit" + }, + "install": { + "subcommand": "--install", + "aliases": [ + "-i" + ], + "strategy": "hooks-path-env", + "runtime_env": true + } + }, "simple-git-hooks": { "language": "JavaScript", "upstream": "https://github.com/toplenboren/simple-git-hooks", diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index a2e56efd2..68046e5b2 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -383,10 +383,19 @@ See `docs/adapters/goose.md`. home stay on for every registered manager, because both homes are per user (#1819). `registry.json` registers prek (install strategy `git-dir-flag`) and - four managers with install strategy `none`, which already write to the + overcommit (`hooks-path-env`, with `install.runtime_env` so the + dispatcher exports the same `core.hooksPath` at run time), plus four + managers with install strategy `none`, which already write to the repository's own hooks directory whatever the global `core.hooksPath`: `simple-git-hooks`, `cargo-husky`, `git-build-hook` and `git-code-format-maven-plugin` (#1838). + overcommit's install action is a flag, so the shim finds it and its + registry aliases (`-i`) anywhere in argv, not only first, including + inside a bundle of short switches (`-fi`) and as any prefix of the + long flag (`--inst`); the + exported run-time override reaches git commands the hook starts, + including ones aimed at another repository, which is recorded as a + known limitation of the strategy. A `none` manager gets no shim; its worktree caveats (cargo-husky skips hook install in a linked worktree, the JGit plugins target `/hooks`) are recorded in the README's adapter table.