From b92118621b26f2a486aaf675aec399171a65f376 Mon Sep 17 00:00:00 2001 From: Tejas Date: Tue, 27 Oct 2026 22:13:00 +0530 Subject: [PATCH 1/4] feat(agent-isolation): add the overcommit hook-manager adapter Why: overcommit reads core.hooksPath unscoped and installs into whatever it names, then re-checks its install on every hook run. Under the whole-user setup that would replace the dispatcher for every repository on the host and then fail behind it, so it needs both an install strategy and a run-time override that prek's --git-dir flag does not cover. The new hooks-path-env strategy runs the manager's install with GIT_CONFIG_* pointing core.hooksPath at the repository's own /hooks, and the dispatcher re-exports the same override before chaining into a hook that carries the adapter's marker (registry install.runtime_env). Refs #1821. Generated-by: Buffy (DeepSeek V4.1 Flash) --- docs/labels-and-capabilities.md | 2 +- docs/setup/secure-agent-setup.md | 19 ++++--- .../step-p-sandbox-allowlists.md | 32 ++++++----- tools/agent-isolation/README.md | 4 +- tools/agent-isolation/git-hook-dispatcher.sh | 23 +++++++- tools/agent-isolation/hook-manager-shim.sh | 17 +++++- .../tests/test_git_hook_dispatcher.py | 35 ++++++++++++ .../tests/test_hook_manager_shim.py | 53 ++++++++++++++++++- tools/dev/sync-hook-managers.py | 26 +++++++-- tools/dev/tests/test_sync_hook_managers.py | 22 +++++++- tools/hook-manager/README.md | 21 ++++++-- tools/hook-manager/registry.json | 16 ++++++ 12 files changed, 231 insertions(+), 39 deletions(-) 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 0d2831821..beaf5d05e 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -1426,16 +1426,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 @@ -1448,6 +1452,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 | @@ -1477,7 +1482,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..97b076815 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,31 @@ 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`, 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 +363,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 +376,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/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index 226353a7a..a9e665c23 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 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; for overcommit, setting `core.hooksPath` to `/hooks`. 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..b0d29bd44 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,26 @@ 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: :. +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 + export GIT_CONFIG_COUNT=1 + export GIT_CONFIG_KEY_0=core.hooksPath + export GIT_CONFIG_VALUE_0="$common/hooks" + 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..a880f79a8 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. @@ -48,6 +48,8 @@ # checkout's hooks dir) — UNLESS the caller already chose a # target, asked for help, or we are not inside a git work tree. # git-dir-flag: inject ` ` (prek: --git-dir) +# hooks-path-env: set GIT_CONFIG_* so the manager writes to +# `/hooks` (overcommit: --install) # * every other invocation (`prek run`, `prek autoupdate`, # `--version`, bare `prek`, ...) is passed through to the real # binary **unchanged**. @@ -65,6 +67,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 ;; prek) install_subcommand=install; install_strategy=git-dir-flag; install_flag=--git-dir ;; *) return 1 ;; esac @@ -109,8 +112,12 @@ 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 @@ -123,6 +130,12 @@ if [ "$inject" = 1 ]; then git-dir-flag) exec "$real_tool" "$install_subcommand" "$install_flag" "$gitdir" "$@" ;; + hooks-path-env) + export GIT_CONFIG_COUNT=1 + export GIT_CONFIG_KEY_0=core.hooksPath + export GIT_CONFIG_VALUE_0="${gitdir%/}/hooks" + exec "$real_tool" "$install_subcommand" "$@" + ;; esac fi # not in a git work tree: let the real tool produce its own error diff --git a/tools/agent-isolation/tests/test_git_hook_dispatcher.py b/tools/agent-isolation/tests/test_git_hook_dispatcher.py index b5acee0dc..1434441fc 100644 --- a/tools/agent-isolation/tests/test_git_hook_dispatcher.py +++ b/tools/agent-isolation/tests/test_git_hook_dispatcher.py @@ -53,6 +53,17 @@ 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 @@ -95,6 +106,30 @@ 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_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..79601d711 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -33,7 +33,12 @@ 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-}}" "${{GIT_CONFIG_KEY_0-}}" ' + f'"${{GIT_CONFIG_VALUE_0-}}" > {env_file}\n' + ) (real / name).chmod(0o755) shims = tmp_path / "shims" shims.mkdir() @@ -70,13 +75,57 @@ 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() == ["", "", ""] + + 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, diff --git a/tools/dev/sync-hook-managers.py b/tools/dev/sync-hook-managers.py index 63f15fa2d..40cd90b19 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,10 @@ 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-]*$") _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,8 +114,10 @@ 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") + if not isinstance(install.get("subcommand"), str) or not _SUBCOMMAND.match( + install["subcommand"] + ): + errors.append(f"{where}.install.subcommand: missing or not a word or --flag") if strategy == "git-dir-flag" and not ( isinstance(install.get("flag"), str) and _FLAG.match(install["flag"]) ): @@ -122,6 +126,12 @@ def validate(registry: dict[str, Any]) -> list[str]: ) elif "flag" in install: errors.append(f"{where}.install.flag: strategy 'none' takes no flag") + 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 @@ -166,10 +176,18 @@ def render(registry: dict[str, Any]) -> dict[Path, str]: 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..3fe433609 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,25 @@ 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", + "strategy": "hooks-path-env", + "runtime_env": True, + } + rendered = mod.render(registry) + assert "overcommit) install_subcommand=--install; install_strategy=hooks-path-env" in rendered[mod.SHIM] + assert 'hook_manager_runtime_envs="overcommit:overcommit"' in rendered[mod.DISPATCHER] + + 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: @@ -140,6 +157,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..1b604d5ab 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) @@ -73,7 +74,7 @@ It leaves the install alone when the caller already chose a target, asked for he |---|---|---| | `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 `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. | 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. @@ -128,6 +129,7 @@ The two YAML names are shared with pre-commit, so a repository with one uses bot | `install.subcommand` | The subcommand the shim rewrites. | | `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 +138,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 +151,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` | Applies the adapter's install strategy — for prek, runs `prek install --git-dir [args…]`; for overcommit, runs `overcommit --install` with `core.hooksPath` pointed at `/hooks`. 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 +178,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`); 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 +210,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..db7c1c2d9 100644 --- a/tools/hook-manager/registry.json +++ b/tools/hook-manager/registry.json @@ -66,6 +66,22 @@ "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", + "strategy": "hooks-path-env", + "runtime_env": true + } + }, "simple-git-hooks": { "language": "JavaScript", "upstream": "https://github.com/toplenboren/simple-git-hooks", From eaea14998d57f1e73b507494e24ec28e59b1551b Mon Sep 17 00:00:00 2001 From: Tejas Date: Fri, 30 Oct 2026 12:43:48 +0530 Subject: [PATCH 2/4] fix(agent-isolation): preserve GIT_CONFIG entries and overcommit aliases --- tools/agent-isolation/README.md | 2 +- tools/agent-isolation/git-hook-dispatcher.sh | 15 ++++- tools/agent-isolation/hook-manager-shim.sh | 55 +++++++++++++---- .../tests/test_git_hook_dispatcher.py | 29 +++++++++ .../tests/test_hook_manager_shim.py | 61 +++++++++++++++++-- tools/dev/sync-hook-managers.py | 27 ++++++-- tools/dev/tests/test_sync_hook_managers.py | 36 ++++++++++- tools/hook-manager/README.md | 12 ++-- tools/hook-manager/registry.json | 3 + .../specs/agent-isolation-sandbox.md | 9 ++- 10 files changed, 216 insertions(+), 33 deletions(-) diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index a9e665c23..4ef4f5b34 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 / 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 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; for overcommit, setting `core.hooksPath` to `/hooks`. 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` 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 b0d29bd44..e572fbbf7 100755 --- a/tools/agent-isolation/git-hook-dispatcher.sh +++ b/tools/agent-isolation/git-hook-dispatcher.sh @@ -158,13 +158,22 @@ fi # 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 - export GIT_CONFIG_COUNT=1 - export GIT_CONFIG_KEY_0=core.hooksPath - export GIT_CONFIG_VALUE_0="$common/hooks" + _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 } diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index a880f79a8..b9038ac9f 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -41,15 +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: set GIT_CONFIG_* so the manager writes to -# `/hooks` (overcommit: --install) +# 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**. @@ -67,7 +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 ;; + 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 @@ -100,13 +103,32 @@ 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`). +install_invoked=0 +case "$install_subcommand" in + -*) + install_actions="${install_aliases:-$install_subcommand}" + for _a in "$@"; do + for _action in $install_actions; do + if [ "$_a" = "$_action" ]; then + install_invoked=1 + break 2 + fi + 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 @@ -128,17 +150,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) - export GIT_CONFIG_COUNT=1 - export GIT_CONFIG_KEY_0=core.hooksPath - export GIT_CONFIG_VALUE_0="${gitdir%/}/hooks" - exec "$real_tool" "$install_subcommand" "$@" + # 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 1434441fc..c40a9fdda 100644 --- a/tools/agent-isolation/tests/test_git_hook_dispatcher.py +++ b/tools/agent-isolation/tests/test_git_hook_dispatcher.py @@ -69,6 +69,7 @@ def _run(repo: Path, hook: Path, env_extra: dict[str, str]) -> None: 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) @@ -117,6 +118,34 @@ def test_an_overcommit_hook_gets_the_repository_hooks_path(tmp_path: Path) -> No 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: diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index 79601d711..3d51000dd 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -36,14 +36,20 @@ def _setup(tmp_path: Path, name: str = "prek") -> tuple[dict[str, str], Path]: env_file = tmp_path / "env" (real / name).write_text( f'#!/bin/sh\nprintf "%s\\n" "$@" > {argv}\n' - f'printf "%s\\n" "${{GIT_CONFIG_COUNT-}}" "${{GIT_CONFIG_KEY_0-}}" ' - f'"${{GIT_CONFIG_VALUE_0-}}" > {env_file}\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 @@ -95,6 +101,8 @@ def test_overcommit_install_targets_the_common_hooks_dir(tmp_path: Path) -> None "1", "core.hooksPath", f"{_common_dir(repo)}/hooks", + "", + "", ] @@ -105,7 +113,51 @@ def test_overcommit_install_help_passes_through(tmp_path: Path) -> None: == 0 ) assert argv.read_text().splitlines() == ["--install", "--help"] - assert (tmp_path / "env").read_text().splitlines() == ["", "", ""] + assert (tmp_path / "env").read_text().splitlines() == ["", "", "", "", ""] + + +@pytest.mark.parametrize( + "args", + [ + ["-i"], + ["--force", "--install"], + ["--force", "-i"], + ], +) +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", + "", + "", + ] + + +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: @@ -154,6 +206,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 40cd90b19..331f10b6f 100755 --- a/tools/dev/sync-hook-managers.py +++ b/tools/dev/sync-hook-managers.py @@ -59,6 +59,7 @@ _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.-]*$") @@ -114,18 +115,31 @@ def validate(registry: dict[str, Any]) -> list[str]: f"({', '.join(sorted(STRATEGIES))})" ) if strategy != "none": - if not isinstance(install.get("subcommand"), str) or not _SUBCOMMAND.match( - install["subcommand"] - ): + 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": @@ -171,6 +185,9 @@ 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") diff --git a/tools/dev/tests/test_sync_hook_managers.py b/tools/dev/tests/test_sync_hook_managers.py index 3fe433609..ecca144db 100644 --- a/tools/dev/tests/test_sync_hook_managers.py +++ b/tools/dev/tests/test_sync_hook_managers.py @@ -105,14 +105,41 @@ def test_overcommit_uses_its_flag_and_runtime_env(registry: dict[str, Any]) -> N 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" in rendered[mod.SHIM] + 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) == [] @@ -141,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"} diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 1b604d5ab..852a1bfec 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -67,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. 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. | implemented | +| `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. @@ -126,7 +127,8 @@ 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. | @@ -151,7 +153,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` or `overcommit --install` | Applies the adapter's install strategy — for prek, runs `prek install --git-dir [args…]`; for overcommit, runs `overcommit --install` with `core.hooksPath` pointed at `/hooks`. 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. | +| 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. | @@ -178,7 +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`); detected by `.overcommit.yml` or a hook mentioning `overcommit` | none — it keeps its own state in git config and the repository | 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:* diff --git a/tools/hook-manager/registry.json b/tools/hook-manager/registry.json index db7c1c2d9..19b9b9a11 100644 --- a/tools/hook-manager/registry.json +++ b/tools/hook-manager/registry.json @@ -78,6 +78,9 @@ }, "install": { "subcommand": "--install", + "aliases": [ + "-i" + ], "strategy": "hooks-path-env", "runtime_env": true } diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index f5365ff9e..4e07432da 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -383,10 +383,17 @@ 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; 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. From 76856715cda4dc8fd1e9add48f493fc9bc01a1ae Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 20:33:33 +0100 Subject: [PATCH 3/4] fix(agent-isolation): rewrite bundled and abbreviated overcommit installs overcommit parses its options with Ruby's OptionParser, which takes a single-letter switch inside a bundle (`overcommit -fi`) and an unambiguous prefix of a long one (`overcommit --inst`). The shim matched the install flag and its aliases by exact string, so both spellings reached the real binary unchanged and installed into the shared hooks directory. The shim now also matches a single-letter alias inside a short-switch bundle and a long flag shortened to three letters or more; rewriting a non-install run only points core.hooksPath at the repository's own hooks. Tests cover both spellings and pin that --run, --uninstall and the ambiguous --in still pass through. The Step P walkthrough, the hook-manager contract and the spec say the same, and the secure-setup fingerprint is restamped. Generated-by: Claude Opus 5 --- .../step-p-sandbox-allowlists.md | 3 +- .../setup_preflight/isolated_fingerprint.py | 2 +- tools/agent-isolation/hook-manager-shim.sh | 34 ++++++++++++++++--- .../tests/test_hook_manager_shim.py | 12 +++++++ tools/hook-manager/README.md | 2 +- .../specs/agent-isolation-sandbox.md | 4 ++- 6 files changed, 48 insertions(+), 9 deletions(-) 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 97b076815..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 @@ -348,7 +348,8 @@ for the full rationale + the validated behaviour matrix. `--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 it rewrites only - `overcommit --install`, pointing `core.hooksPath` at + `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` 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 79eddff65..61202c6c0 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:be5231e45d7c001e" +FRAMEWORK_FINGERPRINT = "sha256:e7384778d5955e6d" diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index b9038ac9f..7aae04d77 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -106,17 +106,41 @@ fi # --- 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`). +# 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 - if [ "$_a" = "$_action" ]; then - install_invoked=1 - break 2 - fi + case "$_action" in + --*) + # Three letters at least (`--ins`); shorter prefixes may name another option. + if [ "${#_a}" -ge 5 ] && [ "${_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 ;; diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index 3d51000dd..921dde31a 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -122,6 +122,9 @@ def test_overcommit_install_help_passes_through(tmp_path: Path) -> None: ["-i"], ["--force", "--install"], ["--force", "-i"], + ["-fi"], + ["--inst"], + ["--force", "--insta"], ], ) def test_overcommit_install_flag_is_found_anywhere( @@ -140,6 +143,15 @@ def test_overcommit_install_flag_is_found_anywhere( ] +@pytest.mark.parametrize("args", [["--run"], ["--uninstall"], ["--in"]]) +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) diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 852a1bfec..043e0001e 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -68,7 +68,7 @@ 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. +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 a long flag shortened to three letters or more (`--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 | diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index 4e07432da..9cace2f9e 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -390,7 +390,9 @@ See `docs/adapters/goose.md`. `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; the + registry aliases (`-i`) anywhere in argv, not only first, including + inside a bundle of short switches (`-fi`) and as a long-flag prefix + of three letters or more (`--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. From ef134d691b5fd20423e6bb20e63733d639c3cc61 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 20:39:04 +0100 Subject: [PATCH 4/4] fix(agent-isolation): treat any prefix of overcommit's --install as an install A wrongly rewritten run only points core.hooksPath at the repository's own hooks, so fail closed on every OptionParser abbreviation (--i, --in, ...) instead of requiring three letters. Generated-by: Claude Opus 5 --- .../skills/setup/setup_preflight/isolated_fingerprint.py | 2 +- tools/agent-isolation/hook-manager-shim.sh | 4 ++-- tools/agent-isolation/tests/test_hook_manager_shim.py | 3 ++- tools/hook-manager/README.md | 2 +- tools/spec-loop/specs/agent-isolation-sandbox.md | 4 ++-- 5 files changed, 8 insertions(+), 7 deletions(-) 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 61202c6c0..577b3362b 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:e7384778d5955e6d" +FRAMEWORK_FINGERPRINT = "sha256:b03fc4a836490a2b" diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index 7aae04d77..f8253d854 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -119,8 +119,8 @@ case "$install_subcommand" in for _action in $install_actions; do case "$_action" in --*) - # Three letters at least (`--ins`); shorter prefixes may name another option. - if [ "${#_a}" -ge 5 ] && [ "${_action#"$_a"}" != "$_action" ]; then + # 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 diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index 921dde31a..4380191a7 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -123,6 +123,7 @@ def test_overcommit_install_help_passes_through(tmp_path: Path) -> None: ["--force", "--install"], ["--force", "-i"], ["-fi"], + ["--i"], ["--inst"], ["--force", "--insta"], ], @@ -143,7 +144,7 @@ def test_overcommit_install_flag_is_found_anywhere( ] -@pytest.mark.parametrize("args", [["--run"], ["--uninstall"], ["--in"]]) +@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) diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 043e0001e..d9cfbafec 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -68,7 +68,7 @@ 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 a long flag shortened to three letters or more (`--inst`). +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 | diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index 9cace2f9e..6cdeddef6 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -391,8 +391,8 @@ See `docs/adapters/goose.md`. `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 a long-flag prefix - of three letters or more (`--inst`); the + 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.