From 78c16178bb6f86b75c5399fca14bbccea824af3d Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 5 Oct 2026 15:10:44 -0400 Subject: [PATCH 1/3] feat(hooks): install git hooks once per clone with just hooks Hooks live in the tracked .githooks/, so a clone-wide relative core.hooksPath still runs each worktree's own branch's hooks, while per-worktree install made every new worktree a manual step. Signed-off-by: Duncan --- AGENTS.md | 4 +- README.md | 4 +- docs/contributing.md | 15 ++++--- justfile | 4 ++ scripts/install-hooks.mjs | 32 ++++++++------ tests/integration/hooks.test.mjs | 71 +++++++++++++++++++++++++++----- 6 files changed, 96 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c5a60f9ef..22d012ed5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ git-ignored `.env.local` without overwriting an existing target, then uses that worktree's Hermit proxy to run `bin/pnpm install --frozen-lockfile`. Do not copy other ignored paths: Keychain credentials and pnpm's package cache are machine-shared, while dependencies and build output are regenerated. Follow the -per-worktree hook setup in `docs/contributing.md` before committing or pushing. +once-per-clone hook setup in `docs/contributing.md` before committing or pushing. ## Engineering standard @@ -203,7 +203,7 @@ confirmation, not app runs. ## Before pushing - Use the agreed feature worktree and pinned `bin/` tools. Follow - [hook setup](docs/contributing.md#pre-commit-checks) once per worktree; + [hook setup](docs/contributing.md#pre-commit-checks) once per clone; preserve custom hooks and never bypass failures. - Refresh remote refs; confirm destination, base, and head. Review `git status`, the full PR diff, and `git diff --check` against the base. Include only intended diff --git a/README.md b/README.md index f396a4bb8..2e2ddf69a 100644 --- a/README.md +++ b/README.md @@ -44,8 +44,8 @@ Without live opt-in they run the shell without relay identity access. `just iterate` applies formatting and runs fast checks plus the frontend build. `just scan` adds tests and native checks. [PR CI](.github/workflows/ci.yml) runs those checks in cached, parallel jobs with sharded browser journeys. -Install the fast staged-file pre-commit and related-test pre-push hooks once per worktree with -`bin/pnpm hooks:install`; see [hook behavior and partial staging](docs/contributing.md#git-hooks). +Install the fast staged-file pre-commit and related-test pre-push hooks once per clone with +`just hooks`; they apply to every worktree without its own `core.hooksPath`; see [hook behavior and partial staging](docs/contributing.md#git-hooks). ### Design system diff --git a/docs/contributing.md b/docs/contributing.md index 5443c7636..02ce426cb 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -264,16 +264,19 @@ into an ever-growing full test suite. ### Pre-commit checks -Install once **per worktree** after `pnpm install --frozen-lockfile`: +Install once **per clone**; it applies to every existing and future worktree: ```sh -bin/pnpm hooks:install +just hooks ``` -The installer enables pre-commit and pre-push using Git's worktree-local -`core.hooksPath`, leaves sibling worktrees -alone, and refuses existing custom hooks rather than overwriting them. Repeat -installation is safe. Do not run `lefthook install`: the tracked Git hook calls a +`just hooks` runs `pnpm hooks:install`, which sets the clone's +`core.hooksPath` to the relative `.githooks`. Git resolves that from each +worktree's root, so every worktree runs its own branch's tracked hooks. A +worktree with its own explicit `core.hooksPath` keeps it and is not changed. The +installer refuses an existing different `core.hooksPath` or custom hooks rather +than overwriting them. Repeat installation is safe, including in clones that +used the earlier per-worktree setting. Do not run `lefthook install`: the tracked Git hook calls a custom `check-staged` group to avoid Lefthook's automatic partial-file stashing. Pre-commit runs pinned Biome formatting and safe lint fixes on fully staged diff --git a/justfile b/justfile index ef11165a2..f6bf30780 100644 --- a/justfile +++ b/justfile @@ -8,6 +8,10 @@ default: install: pnpm install --frozen-lockfile +# Install Git hooks once for every worktree of this clone. +hooks: install + pnpm hooks:install + # Run the shared frontend in a browser; forward Vite arguments (e.g. --port 1431). [positional-arguments] web *args: install diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs index 8ea732a36..d999f2ece 100644 --- a/scripts/install-hooks.mjs +++ b/scripts/install-hooks.mjs @@ -1,10 +1,10 @@ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync, readdirSync } from "node:fs"; -import { resolve } from "node:path"; +import { join, resolve } from "node:path"; const git = (...args) => execFileSync("git", args, { encoding: "utf8" }).trim(); -const config = (key) => { - const result = spawnSync("git", ["config", "--get", key], { +const config = (key, ...scope) => { + const result = spawnSync("git", ["config", ...scope, "--get", key], { encoding: "utf8", }); if (result.status === 1) return ""; @@ -13,13 +13,18 @@ const config = (key) => { return result.stdout.trim(); }; process.chdir(git("rev-parse", "--show-toplevel")); +// Refuse a different hooks path in this worktree or in the clone-wide setting +// this install replaces; a worktree override must not mask the shared one. const existing = config("core.hooksPath"); -if (existing && existing !== ".githooks") - throw new Error( - `Existing core.hooksPath (${existing}); reconcile it before installing.`, - ); -if (!existing) { - const hooks = git("rev-parse", "--git-path", "hooks"); +const shared = config("core.hooksPath", "--local"); +for (const value of [existing, shared]) + if (value && value !== ".githooks") + throw new Error( + `Existing core.hooksPath (${value}); reconcile it before installing.`, + ); +if (!shared) { + // The clone's default hooks directory, whatever this worktree overrides. + const hooks = join(resolve(git("rev-parse", "--git-common-dir")), "hooks"); const custom = existsSync(hooks) ? readdirSync(hooks).filter((name) => !name.endsWith(".sample")) : []; @@ -28,15 +33,16 @@ if (!existing) { `Existing hooks (${custom.join(", ")}); reconcile them before installing.`, ); } -// Worktree-specific hooks must not replace a sibling checkout's workflow. // Git requires special migration for explicit core.worktree/bare repositories. if (config("core.worktree") || config("core.bare") === "true") throw new Error( "Nonstandard worktree configuration; install hooks manually.", ); execFileSync(resolve("bin/lefthook"), ["validate"], { stdio: "inherit" }); -git("config", "--local", "extensions.worktreeConfig", "true"); -git("config", "--worktree", "core.hooksPath", ".githooks"); +// One clone-wide setting covers every worktree: Git resolves the relative path +// from each worktree's root, so each runs its own branch's tracked .githooks. +// Explicit per-worktree settings, including earlier installs, stay in place. +git("config", "--local", "core.hooksPath", ".githooks"); console.log( - "Installed Lefthook pre-commit and pre-push for this worktree only.", + "Installed Lefthook pre-commit and pre-push for every worktree of this clone.", ); diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index a857d6219..5417b6489 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -216,32 +216,52 @@ test("staged deletions and documentation-only commits do not rewrite source", (t assert.equal(f.read("untouched.ts"), "export const unrelated={value:1}\n"); }); -test("installation is worktree-local, repeatable, and refuses custom hooks", (t) => { +test("installation covers every worktree, is repeatable, and refuses custom hooks", (t) => { const f = fixture(t); assert.equal( - f.git("config", "--worktree", "--get", "core.hooksPath").trim(), + f.git("config", "--local", "--get", "core.hooksPath").trim(), ".githooks", ); + // A linked worktree runs its own checkout's hooks, not the main checkout's. + const hook = path.join(f.sibling, ".githooks/pre-commit"); + writeFileSync(hook, '#!/bin/sh\npwd > "$(git rev-parse --git-dir)/ran"\n'); + const sibling = (...args) => + spawnSync("git", args, { cwd: f.sibling, env, encoding: "utf8" }); assert.equal( - f.run("git", ["config", "--local", "--get", "core.hooksPath"]).status, - 1, + sibling("commit", "-q", "--allow-empty", "-m", "probe").status, + 0, + ); + const gitDir = sibling("rev-parse", "--absolute-git-dir").stdout.trim(); + assert.equal( + readFileSync(path.join(gitDir, "ran"), "utf8").trim(), + sibling("rev-parse", "--show-toplevel").stdout.trim(), ); assert.equal(f.install().status, 0); - const result = spawnSync("git", ["config", "--get", "core.hooksPath"], { - cwd: f.sibling, - env, - encoding: "utf8", - }); - assert.equal(result.status, 1); + // A clone set up per worktree by the earlier installer reinstalls cleanly + // and keeps its existing worktree setting. + f.git("config", "--local", "--unset", "core.hooksPath"); + f.git("config", "--local", "extensions.worktreeConfig", "true"); + f.git("config", "--worktree", "core.hooksPath", ".githooks"); + assert.equal(f.install().status, 0); + assert.equal( + f.git("config", "--local", "--get", "core.hooksPath").trim(), + ".githooks", + ); + assert.equal( + f.git("config", "--worktree", "--get", "core.hooksPath").trim(), + ".githooks", + ); + f.git("config", "--worktree", "--unset", "core.hooksPath"); + f.git("config", "--local", "--unset", "core.hooksPath"); f.write(".git/hooks/pre-commit", "#!/bin/sh\nexit 1\n"); const before = f.read(".git/hooks/pre-commit"); const refused = f.install(); assert.notEqual(refused.status, 0); assert.match(refused.stderr, /Existing hooks/); assert.equal(f.read(".git/hooks/pre-commit"), before); - f.git("config", "--worktree", "core.hooksPath", "custom-hooks"); + f.git("config", "--local", "core.hooksPath", "custom-hooks"); assert.notEqual(f.install().status, 0); assert.equal( f.git("config", "--get", "core.hooksPath").trim(), @@ -249,6 +269,35 @@ test("installation is worktree-local, repeatable, and refuses custom hooks", (t) ); }); +test("an old worktree override does not mask shared hooks the install would replace", (t) => { + const f = fixture(t); + const masked = () => { + f.git("config", "--local", "extensions.worktreeConfig", "true"); + f.git("config", "--worktree", "core.hooksPath", ".githooks"); + }; + // A different clone-wide path that siblings rely on. + f.git("config", "--local", "core.hooksPath", "custom-hooks"); + masked(); + const refusedPath = f.install(); + assert.notEqual(refusedPath.status, 0); + assert.match(refusedPath.stderr, /Existing core.hooksPath \(custom-hooks\)/); + assert.equal( + f.git("config", "--local", "--get", "core.hooksPath").trim(), + "custom-hooks", + ); + // Custom default hooks that a clone-wide path would stop running. + f.git("config", "--local", "--unset", "core.hooksPath"); + f.write(".git/hooks/pre-commit", "#!/bin/sh\nexit 1\n"); + const refusedHooks = f.install(); + assert.notEqual(refusedHooks.status, 0); + assert.match(refusedHooks.stderr, /Existing hooks \(pre-commit\)/); + assert.equal( + f.run("git", ["config", "--local", "--get", "core.hooksPath"]).status, + 1, + ); + assert.equal(f.read(".git/hooks/pre-commit"), "#!/bin/sh\nexit 1\n"); +}); + test("unstaged lint configuration cannot hide a staged warning", (t) => { const f = fixture(t); const config = JSON.parse(f.read("biome.json")); From 61614bf99be4d0453d421027f3eb44716c81b43e Mon Sep 17 00:00:00 2001 From: Alia Date: Tue, 6 Oct 2026 13:17:45 -0400 Subject: [PATCH 2/3] fix(hooks): ignore global hooks path during install A global core.hooksPath should not prevent this clone from installing its tracked hooks. Continue refusing conflicting clone-local and per-worktree paths, while making the precedence explicit in the integration coverage and contributor docs. Signed-off-by: Alia --- docs/contributing.md | 6 ++++-- scripts/install-hooks.mjs | 10 ++++++---- tests/integration/hooks.test.mjs | 30 ++++++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 02ce426cb..61d428582 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -274,8 +274,10 @@ just hooks `core.hooksPath` to the relative `.githooks`. Git resolves that from each worktree's root, so every worktree runs its own branch's tracked hooks. A worktree with its own explicit `core.hooksPath` keeps it and is not changed. The -installer refuses an existing different `core.hooksPath` or custom hooks rather -than overwriting them. Repeat installation is safe, including in clones that +installer refuses an existing different clone-local or worktree `core.hooksPath` +or custom hooks rather than overwriting them; a global `core.hooksPath` is +overridden for this clone, so global hooks do not run here. Repeat installation +is safe, including in clones that used the earlier per-worktree setting. Do not run `lefthook install`: the tracked Git hook calls a custom `check-staged` group to avoid Lefthook's automatic partial-file stashing. diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs index d999f2ece..e1a77ca63 100644 --- a/scripts/install-hooks.mjs +++ b/scripts/install-hooks.mjs @@ -13,11 +13,13 @@ const config = (key, ...scope) => { return result.stdout.trim(); }; process.chdir(git("rev-parse", "--show-toplevel")); -// Refuse a different hooks path in this worktree or in the clone-wide setting -// this install replaces; a worktree override must not mask the shared one. -const existing = config("core.hooksPath"); +// Refuse different hooks paths in this clone or its worktree config. Global and +// system settings are intentionally ignored because this install replaces them. const shared = config("core.hooksPath", "--local"); -for (const value of [existing, shared]) +const worktreeConfig = + config("extensions.worktreeConfig", "--local", "--type=bool") === "true"; +const worktree = worktreeConfig ? config("core.hooksPath", "--worktree") : ""; +for (const value of [shared, worktree]) if (value && value !== ".githooks") throw new Error( `Existing core.hooksPath (${value}); reconcile it before installing.`, diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index 5417b6489..a59842581 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -81,8 +81,8 @@ function fixture(t) { git("commit", "-qm", "hook configuration"); const sibling = path.join(dir, "sibling"); git("worktree", "add", "--detach", sibling); - const install = () => - run(path.join(root, "bin/node"), ["scripts/install-hooks.mjs"]); + const install = (overrides = {}) => + run(path.join(root, "bin/node"), ["scripts/install-hooks.mjs"], overrides); const installed = install(); assert.equal(installed.status, 0, installed.stdout + installed.stderr); const commit = () => run("git", ["commit", "-qm", "probe"]); @@ -298,6 +298,32 @@ test("an old worktree override does not mask shared hooks the install would repl assert.equal(f.read(".git/hooks/pre-commit"), "#!/bin/sh\nexit 1\n"); }); +test("a global hooks path is overridden for this clone", (t) => { + const f = fixture(t); + const globalConfig = path.join(f.dir, "global.gitconfig"); + f.git("config", "--local", "--unset", "core.hooksPath"); + f.git("config", "--file", globalConfig, "core.hooksPath", "global-hooks"); + const installed = f.install({ GIT_CONFIG_GLOBAL: globalConfig }); + assert.equal(installed.status, 0, installed.stdout + installed.stderr); + const effective = f.run("git", ["config", "--get", "core.hooksPath"], { + GIT_CONFIG_GLOBAL: globalConfig, + }); + assert.equal(effective.status, 0, effective.stdout + effective.stderr); + assert.equal(effective.stdout.trim(), ".githooks"); +}); + +test("a conflicting clone hooks path is refused", (t) => { + const f = fixture(t); + f.git("config", "--local", "core.hooksPath", "custom-hooks"); + const refused = f.install(); + assert.notEqual(refused.status, 0); + assert.match(refused.stderr, /Existing core.hooksPath \(custom-hooks\)/); + assert.equal( + f.git("config", "--local", "--get", "core.hooksPath").trim(), + "custom-hooks", + ); +}); + test("unstaged lint configuration cannot hide a staged warning", (t) => { const f = fixture(t); const config = JSON.parse(f.read("biome.json")); From fdd787fdf4786844b1fe547daaf11d700760c5c6 Mon Sep 17 00:00:00 2001 From: Alia Date: Tue, 6 Oct 2026 13:40:39 -0400 Subject: [PATCH 3/3] fix(hooks): inspect included repository config Follow clone and worktree include files when checking for conflicting hook paths, including the worktreeConfig extension. Refusal regressions verify both saved configuration and the effective hook path, while contributor docs state that command-scoped overrides remain outside the installer contract. Signed-off-by: Alia --- docs/contributing.md | 8 +++-- scripts/install-hooks.mjs | 13 +++++-- tests/integration/hooks.test.mjs | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 61d428582..494d9ff8f 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -276,9 +276,11 @@ worktree's root, so every worktree runs its own branch's tracked hooks. A worktree with its own explicit `core.hooksPath` keeps it and is not changed. The installer refuses an existing different clone-local or worktree `core.hooksPath` or custom hooks rather than overwriting them; a global `core.hooksPath` is -overridden for this clone, so global hooks do not run here. Repeat installation -is safe, including in clones that -used the earlier per-worktree setting. Do not run `lefthook install`: the tracked Git hook calls a +overridden for this clone, so global hooks do not run here. The installer checks +saved clone and worktree settings, including files they include; per-command +overrides such as `git -c core.hooksPath=...` or `GIT_CONFIG_*` environment +variables can still take precedence. Repeat installation is safe, including in +clones with the earlier per-worktree setting. Do not run `lefthook install`: the tracked Git hook calls a custom `check-staged` group to avoid Lefthook's automatic partial-file stashing. Pre-commit runs pinned Biome formatting and safe lint fixes on fully staged diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs index e1a77ca63..b30de215b 100644 --- a/scripts/install-hooks.mjs +++ b/scripts/install-hooks.mjs @@ -15,10 +15,17 @@ const config = (key, ...scope) => { process.chdir(git("rev-parse", "--show-toplevel")); // Refuse different hooks paths in this clone or its worktree config. Global and // system settings are intentionally ignored because this install replaces them. -const shared = config("core.hooksPath", "--local"); +const shared = config("core.hooksPath", "--local", "--includes"); const worktreeConfig = - config("extensions.worktreeConfig", "--local", "--type=bool") === "true"; -const worktree = worktreeConfig ? config("core.hooksPath", "--worktree") : ""; + config( + "extensions.worktreeConfig", + "--local", + "--includes", + "--type=bool", + ) === "true"; +const worktree = worktreeConfig + ? config("core.hooksPath", "--worktree", "--includes") + : ""; for (const value of [shared, worktree]) if (value && value !== ".githooks") throw new Error( diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index a59842581..a53aa24bc 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -89,6 +89,18 @@ function fixture(t) { return { dir, sibling, run, git, write, read, install, commit }; } +function assertIncludedHooksPathRefused(f, files, expectedPath) { + const before = files.map((file) => [file, f.read(file)]); + const refused = f.install(); + assert.notEqual(refused.status, 0); + assert.match(refused.stderr, /Existing core.hooksPath/); + for (const [file, content] of before) + assert.equal(f.read(file), content, `${file} changed after refusal`); + const effective = f.run("git", ["config", "--get", "core.hooksPath"]); + assert.equal(effective.status, 0, effective.stdout + effective.stderr); + assert.equal(effective.stdout.trim(), expectedPath); +} + test("both pre-push jobs receive the complete Git input without sharing a read cursor", (t) => { const f = fixture(t); // Keep the installed hook and production job configuration. Probe only the @@ -324,6 +336,53 @@ test("a conflicting clone hooks path is refused", (t) => { ); }); +test("a clone include before core is refused", (t) => { + const f = fixture(t); + f.git("config", "--local", "--unset", "core.hooksPath"); + f.write(".git/included-before.conf", "[core]\n\thooksPath = custom-before\n"); + const config = f.read(".git/config"); + f.write(".git/config", `[include]\n\tpath = included-before.conf\n${config}`); + assertIncludedHooksPathRefused( + f, + [".git/config", ".git/included-before.conf"], + "custom-before", + ); +}); + +test("a clone include after core is refused", (t) => { + const f = fixture(t); + f.git("config", "--local", "--unset", "core.hooksPath"); + f.write(".git/included-after.conf", "[core]\n\thooksPath = custom-after\n"); + const config = f.read(".git/config"); + f.write( + ".git/config", + `${config}\n[include]\n\tpath = included-after.conf\n`, + ); + assertIncludedHooksPathRefused( + f, + [".git/config", ".git/included-after.conf"], + "custom-after", + ); +}); + +test("an included worktree hooks path is refused", (t) => { + const f = fixture(t); + f.git("config", "--local", "extensions.worktreeConfig", "true"); + f.write( + ".git/included-worktree.conf", + "[core]\n\thooksPath = custom-worktree\n", + ); + f.write( + ".git/config.worktree", + "[include]\n\tpath = included-worktree.conf\n", + ); + assertIncludedHooksPathRefused( + f, + [".git/config.worktree", ".git/included-worktree.conf"], + "custom-worktree", + ); +}); + test("unstaged lint configuration cannot hide a staged warning", (t) => { const f = fixture(t); const config = JSON.parse(f.read("biome.json"));