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..494d9ff8f 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -264,16 +264,23 @@ 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 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. 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/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..b30de215b 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,27 @@ const config = (key) => { return result.stdout.trim(); }; process.chdir(git("rev-parse", "--show-toplevel")); -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"); +// 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", "--includes"); +const worktreeConfig = + 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( + `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 +42,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..a53aa24bc 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -81,14 +81,26 @@ 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"]); 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 @@ -216,32 +228,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 +281,108 @@ 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("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("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"));