From efc20b258dd507f332a124baa4e1d8e4ae54afd8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 6 Oct 2026 11:44:45 +1100 Subject: [PATCH 1/4] fix(hooks): compose Buzz checks with worktree-local lhm dispatch Recognize lhm wrappers while preserving custom-hook safeguards and inherited configuration. Run Buzz first for commits, lhm first for pushes, replay push input, and forward other hook events. Document recovery and cover installation, ordering, failure, signal, and stdin contracts with integration tests. Signed-off-by: Matt Toohey --- docs/contributing.md | 45 ++++- scripts/install-hooks.mjs | 129 ++++++++++++-- scripts/run-hook.mjs | 51 ++++++ tests/integration/hooks.test.mjs | 284 ++++++++++++++++++++++++++++++- 4 files changed, 496 insertions(+), 13 deletions(-) create mode 100644 scripts/run-hook.mjs diff --git a/docs/contributing.md b/docs/contributing.md index 5443c7636..d93177c2d 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -272,7 +272,8 @@ bin/pnpm hooks:install 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 +alone, and supports standalone hooks or recognized lhm-generated wrappers. +Unknown or modified custom hooks are refused rather than overwritten. Repeat installation is safe. Do not run `lefthook install`: the tracked Git hook calls a custom `check-staged` group to avoid Lefthook's automatic partial-file stashing. @@ -296,6 +297,48 @@ This is a developer guardrail, not a security boundary or a substitute for CI and risk-appropriate behavior checks. Tool/config dependency changes require relevant integration evidence, not an automatic local full scan. +### Working with lhm + +When lhm is inherited (including underneath an existing Buzz `.githooks` +override), installation creates executable dispatchers under this worktree's Git +administration directory, in `buzz-hooks/dispatch-*`. Global configuration and +upstream wrappers remain untouched. Buzz runs first on pre-commit so its partial +staging guard sees the original working tree; lhm scans the resulting staged +content afterward. If lhm fails, Buzz's completed formatting and restaging remain +visible. On pre-push, lhm runs first and can reject the destination before Buzz's +three serialized jobs. Both layers receive the complete push input. Other +installed lhm events forward directly, with their arguments and stdin intact. +Pinned `bin/lefthook` is on PATH for lhm; missing tools or hooks fail visibly. + +Verify installation with: + +```sh +git config --show-origin --get core.hooksPath +git rev-parse --git-path buzz-hooks +bin/node --test tests/integration/hooks.test.mjs +# Optional bounded probe, with isolated lhm system/user configuration: +BUZZ_REAL_LHM="$(command -v lhm)" bin/node --test --test-name-pattern='real lhm' tests/integration/hooks.test.mjs +``` + +Repeat installation after moving a checkout, changing the upstream hook inventory, +or changing dispatcher generation code. Existing upstream wrappers are invoked +at runtime, so their updates take effect immediately. Reinstall rejects edited +Buzz-generated wrappers. Prior generations are retained for recovery; do not edit +them or remove a generation still in use by a Git operation. + +Each generation's `owner.json` records `upstream` and the original worktree-local +`previous` setting. To undo installation, restore that value with +`git config --worktree core.hooksPath ''`; when `previous` is empty, use +`git config --worktree --unset core.hooksPath` to resume inherited hooks. Leave +`extensions.worktreeConfig` enabled because sibling worktrees may rely on it. +Standalone installation can likewise be removed with the unset command. + +For human acceptance, use a disposable checkout with the documented setup: +commit fully staged source and confirm Buzz formats it before lhm runs; partially +stage source and confirm rejection without writes or new stashes; then confirm a +failing job in either layer blocks the operation. Automated probes do not replace +this human confirmation before PR readiness. + ### Fast pre-push feedback Pre-push runs the project TypeScript check (`tsc --noEmit`), then Vitest tests diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs index 8ea732a36..56bc223c1 100644 --- a/scripts/install-hooks.mjs +++ b/scripts/install-hooks.mjs @@ -1,10 +1,20 @@ import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, readdirSync } from "node:fs"; -import { resolve } from "node:path"; +import { + accessSync, + constants, + existsSync, + mkdtempSync, + mkdirSync, + readFileSync, + readdirSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { isAbsolute, 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 ""; @@ -12,12 +22,53 @@ const config = (key) => { throw new Error(result.stderr || `Cannot read ${key}`); return result.stdout.trim(); }; +const executable = (file) => accessSync(file, constants.X_OK); +const quote = (value) => `'${value.replaceAll("'", "'\\''")}'`; process.chdir(git("rev-parse", "--show-toplevel")); +const root = process.cwd(); +const ownedRoot = resolve(git("rev-parse", "--git-path", "buzz-hooks")); const existing = config("core.hooksPath"); -if (existing && existing !== ".githooks") - throw new Error( - `Existing core.hooksPath (${existing}); reconcile it before installing.`, +let upstream = existing; +let previous = + config("extensions.worktreeConfig") === "true" + ? config("core.hooksPath", ["--worktree"]) + : ""; +if (existing.startsWith(`${ownedRoot}/`)) { + const metadata = JSON.parse( + readFileSync(join(existing, "owner.json"), "utf8"), ); + if ( + metadata.owner !== "buzz-hooks-v1" || + metadata.upstream.startsWith(ownedRoot) + ) + throw new Error( + "Invalid Buzz dispatcher ownership; restore the previous hooksPath.", + ); + for (const [name, content] of Object.entries(metadata.files)) { + if (readFileSync(join(existing, name), "utf8") !== content) + throw new Error( + `Modified Buzz dispatcher ${name}; reconcile it before installing.`, + ); + } + if ( + readdirSync(existing).some( + (name) => name !== "owner.json" && !(name in metadata.files), + ) + ) + throw new Error( + "Custom files in Buzz dispatcher; reconcile them before installing.", + ); + upstream = metadata.upstream; + previous = metadata.previous; +} else if (existing === ".githooks") { + // Read lower scopes without temporarily disabling the active hooks. + upstream = config("core.hooksPath", ["--local"]); + if (!upstream || upstream === ".githooks") + upstream = config("core.hooksPath", ["--global"]); + if (!upstream || upstream === ".githooks") + upstream = config("core.hooksPath", ["--system"]); + if (upstream === ".githooks") upstream = ""; +} if (!existing) { const hooks = git("rev-parse", "--git-path", "hooks"); const custom = existsSync(hooks) @@ -28,15 +79,71 @@ 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.", ); + +const events = []; +if (upstream) { + upstream = resolve(upstream); + for (const name of readdirSync(upstream)) { + if (!/^[a-z]+(?:-[a-z]+)+$/.test(name)) + throw new Error( + `Unknown custom hook ${name}; reconcile it before installing.`, + ); + const file = join(upstream, name); + const content = readFileSync(file, "utf8"); + const match = content.match( + /^#!\/bin\/sh\nexec "([^"$`\\\n]+)" run-hook ([a-z-]+) "\$@"\n$/, + ); + if (!match || match[2] !== name || !isAbsolute(match[1])) + throw new Error( + `Unrecognized lhm hook ${file}; reconcile it before installing.`, + ); + executable(file); + executable(match[1]); + events.push(name); + } + if (!events.includes("pre-commit") || !events.includes("pre-push")) + throw new Error("lhm installation must include pre-commit and pre-push."); +} +for (const file of [ + "bin/node", + "bin/lefthook", + ".githooks/pre-commit", + ".githooks/pre-push", +]) + executable(resolve(file)); execFileSync(resolve("bin/lefthook"), ["validate"], { stdio: "inherit" }); +let target = ".githooks"; +if (upstream) { + mkdirSync(ownedRoot, { recursive: true }); + target = mkdtempSync(join(ownedRoot, "dispatch-")); + try { + const files = {}; + for (const event of events) { + files[event] = + event === "pre-commit" || event === "pre-push" + ? `#!/bin/sh\nexec ${quote(join(root, "bin/node"))} ${quote(join(root, "scripts/run-hook.mjs"))} ${quote(event)} ${quote(join(upstream, event))} "$@"\n` + : `#!/bin/sh\nset -eu\ntest -x ${quote(join(root, "bin/lefthook"))} || { echo "Missing pinned Lefthook" >&2; exit 1; }\nPATH=${quote(join(root, "bin"))}:"$PATH"\nexport PATH\nexec ${quote(join(upstream, event))} "$@"\n`; + writeFileSync(join(target, event), files[event], { mode: 0o755 }); + executable(join(target, event)); + execFileSync("sh", ["-n", join(target, event)]); + } + writeFileSync( + join(target, "owner.json"), + `${JSON.stringify({ owner: "buzz-hooks-v1", upstream, previous, files }, null, 2)}\n`, + ); + } catch (error) { + rmSync(target, { recursive: true, force: true }); + throw error; + } +} +// Only activate after every validation succeeds. Older generations remain usable +// for recovery; never rewrite an active wrapper underneath a running Git command. git("config", "--local", "extensions.worktreeConfig", "true"); -git("config", "--worktree", "core.hooksPath", ".githooks"); +git("config", "--worktree", "core.hooksPath", target); console.log( - "Installed Lefthook pre-commit and pre-push for this worktree only.", + `Installed Buzz hooks for this worktree only${upstream ? ` with lhm at ${upstream}` : ""}.`, ); diff --git a/scripts/run-hook.mjs b/scripts/run-hook.mjs new file mode 100644 index 000000000..f8e3f2bd4 --- /dev/null +++ b/scripts/run-hook.mjs @@ -0,0 +1,51 @@ +import { spawn } from "node:child_process"; +import { accessSync, constants, readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const [event, upstream, ...args] = process.argv.slice(2); +if (!["pre-commit", "pre-push"].includes(event) || !upstream) + throw new Error("Expected a Buzz hook event and upstream hook path."); +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const env = { + ...process.env, + PATH: `${resolve(root, "bin")}:${process.env.PATH ?? ""}`, +}; +// A missing Lefthook must not send lhm down its adapter fallback path. +for (const file of [resolve(root, "bin/lefthook"), upstream]) + accessSync(file, constants.X_OK); +const input = event === "pre-push" ? readFileSync(0) : undefined; +const buzz = resolve(root, ".githooks", event); +for (const hook of event === "pre-commit" + ? [buzz, upstream] + : [upstream, buzz]) { + const result = await new Promise((done) => { + const child = spawn(hook, args, { + env, + stdio: [input === undefined ? "inherit" : "pipe", "inherit", "inherit"], + }); + let interrupted; + const forward = (signal) => { + interrupted = signal; + child.kill(signal); + }; + const interrupt = () => forward("SIGINT"); + const terminate = () => forward("SIGTERM"); + process.on("SIGINT", interrupt); + process.on("SIGTERM", terminate); + child.on("error", (error) => console.error(error.message)); + child.on("close", (code, signal) => { + process.off("SIGINT", interrupt); + process.off("SIGTERM", terminate); + done({ code, signal: interrupted ?? signal }); + }); + if (input !== undefined) { + child.stdin.on("error", (error) => { + if (error.code !== "EPIPE") console.error(error.message); + }); + child.stdin.end(input); + } + }); + if (result.signal) process.kill(process.pid, result.signal); + if (result.code !== 0) process.exit(result.code ?? 1); +} diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index a857d6219..1f6238e63 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { + chmodSync, cpSync, existsSync, mkdirSync, @@ -769,3 +770,284 @@ test("staged icon checks reject CommonJS subpaths without changing the index", ( ); assert.equal(f.git("write-tree"), index); }); + +function lhmFixture(t) { + const f = fixture(t); + const upstream = path.join(f.dir, "upstream ' $ hooks"); + const manager = path.join(f.dir, "manager with spaces"); + f.write( + "manager with spaces", + `#!/bin/sh + event="$2" + printf '%s\\n' "$event" >> events + if [ "$event" = pre-commit ]; then git show :probe.ts > upstream-source; fi + if [ "$event" = pre-push ] || [ "$event" = post-rewrite ]; then cat > "$event-input"; fi + printf '%s\\n' "$@" > "$event-args" + exit "\${UPSTREAM_STATUS:-0}" +`, + ); + chmodSync(manager, 0o755); + const names = [ + "pre-commit", + "pre-push", + "commit-msg", + "prepare-commit-msg", + "post-rewrite", + ]; + for (const name of names) { + const file = path.join(upstream, name); + f.write( + path.relative(f.dir, file), + `#!/bin/sh\nexec "${manager}" run-hook ${name} "$@"\n`, + ); + chmodSync(file, 0o755); + } + const global = path.join(f.dir, "global-config"); + f.write("global-config", `[core]\n hooksPath = "${upstream}"\n`); + const overrides = { GIT_CONFIG_GLOBAL: global }; + const install = () => + f.run( + path.join(root, "bin/node"), + ["scripts/install-hooks.mjs"], + overrides, + ); + const result = install(); + assert.equal(result.status, 0, result.stdout + result.stderr); + const hooks = () => + f.git("config", "--worktree", "--get", "core.hooksPath").trim(); + return { ...f, upstream, manager, overrides, install, hooks }; +} + +test("lhm installation preserves inherited configuration, wrappers and sibling behavior", (t) => { + const f = lhmFixture(t); + const global = f.read("global-config"); + const wrapper = readFileSync(path.join(f.upstream, "pre-commit"), "utf8"); + const first = f.hooks(); + assert.equal(f.install().status, 0); + assert.notEqual(f.hooks(), first); + assert.equal( + JSON.parse(readFileSync(path.join(f.hooks(), "owner.json"))).upstream, + f.upstream, + ); + assert.equal(f.read("global-config"), global); + assert.equal( + readFileSync(path.join(f.upstream, "pre-commit"), "utf8"), + wrapper, + ); + assert.equal( + f + .run( + "git", + ["-C", f.sibling, "config", "--get", "core.hooksPath"], + f.overrides, + ) + .stdout.trim(), + f.upstream, + ); + f.write( + path.relative(f.dir, path.join(f.upstream, "pre-push")), + "#!/bin/sh\nexit 0\n", + ); + const active = f.hooks(); + assert.notEqual(f.install().status, 0); + assert.equal(f.hooks(), active); +}); + +test("Buzz formats before lhm, and upstream failure keeps completed formatting visible", (t) => { + const f = lhmFixture(t); + f.write("probe.ts", "export const value={a:1}\n"); + f.git("add", "probe.ts"); + const result = f.run("git", ["commit", "-qm", "probe"], { + ...f.overrides, + UPSTREAM_STATUS: "7", + }); + assert.notEqual(result.status, 0); + assert.equal(f.read("upstream-source"), "export const value = { a: 1 };\n"); + assert.equal(f.git("show", ":probe.ts"), f.read("upstream-source")); + assert.equal(f.commit().status, 0); + assert.match(f.read("events"), /prepare-commit-msg\ncommit-msg/); +}); + +test("partial staging blocks lhm before writes or stashes", (t) => { + const f = lhmFixture(t); + f.write("probe.ts", "export const value={a:1}\n"); + f.git("add", "probe.ts"); + const index = f.git("write-tree"); + f.write("probe.ts", "export const value={a:2}\n"); + const result = f.commit(); + assert.notEqual(result.status, 0); + assert.match(result.stdout + result.stderr, /Partially staged/); + assert.equal(f.git("write-tree"), index); + assert.equal(f.read("probe.ts"), "export const value={a:2}\n"); + assert.equal(existsSync(path.join(f.dir, "events")), false); + assert.equal(f.git("stash", "list"), ""); +}); + +test("dispatch replays raw push bytes to lhm and every Buzz lane, preserving literal arguments", (t) => { + const f = lhmFixture(t); + f.write( + "scripts/check-push.mjs", + `import { readFileSync, writeFileSync } from "node:fs"; +writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0));`, + ); + for (const input of [ + Buffer.alloc(0), + Buffer.from("refs \x00 \xff\n".repeat(20000)), + ]) { + const result = spawnSync( + path.join(f.hooks(), "pre-push"), + ["remote '$", "destination ;$"], + { cwd: f.dir, env, input }, + ); + assert.equal(result.status, 0, String(result.stderr)); + for (const lane of ["pre-push", "unit", "--design", "--clippy"]) + assert.deepEqual(readFileSync(path.join(f.dir, `${lane}-input`)), input); + assert.equal( + f.read("pre-push-args"), + "run-hook\npre-push\nremote '$\ndestination ;$\n", + ); + } + const result = spawnSync(path.join(f.hooks(), "post-rewrite"), ["amend"], { + cwd: f.dir, + env, + input: "old new\n", + }); + assert.equal(result.status, 0); + assert.equal(f.read("post-rewrite-input"), "old new\n"); +}); + +test("upstream push rejection and missing hooks fail before Buzz runs", (t) => { + const f = lhmFixture(t); + f.write("scripts/check-push.mjs", 'throw new Error("Buzz should not run");'); + const invoke = () => + spawnSync(path.join(f.hooks(), "pre-push"), [], { + cwd: f.dir, + env: { ...env, UPSTREAM_STATUS: "9" }, + input: "", + encoding: "utf8", + }); + assert.equal(invoke().status, 9); + rmSync(path.join(f.upstream, "pre-push")); + const missing = invoke(); + assert.notEqual(missing.status, 0); + assert.doesNotMatch(missing.stderr, /Buzz should not run/); +}); + +test("real lhm composes isolated system jobs with Buzz custom groups", { + skip: !process.env.BUZZ_REAL_LHM, +}, (t) => { + const f = lhmFixture(t); + for (const name of [ + "pre-commit", + "pre-push", + "commit-msg", + "prepare-commit-msg", + "post-rewrite", + ]) + f.write( + path.relative(f.dir, path.join(f.upstream, name)), + `#!/bin/sh\nexec "${process.env.BUZZ_REAL_LHM}" run-hook ${name} "$@"\n`, + ); + f.write( + "system/lefthook.yml", + `pre-commit: + commands: + sentinel: + run: git show :probe.ts > real-lhm-source +pre-push: + commands: + sentinel: + run: cat > real-lhm-input + use_stdin: true +`, + ); + f.write("absent-user.yml", "{}\n"); + const isolated = { + ...f.overrides, + LHM_SYSTEM_CONFIG: path.join(f.dir, "system"), + LHM_USER_CONFIG: path.join(f.dir, "absent-user.yml"), + }; + f.write("probe.ts", "export const value={a:1}\n"); + f.git("add", "probe.ts"); + const commit = f.run("git", ["commit", "-qm", "real lhm"], isolated); + assert.equal(commit.status, 0, commit.stdout + commit.stderr); + assert.equal(f.read("real-lhm-source"), "export const value = { a: 1 };\n"); + f.write( + "scripts/check-push.mjs", + `import { readFileSync, writeFileSync } from "node:fs"; +writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0));`, + ); + f.git("init", "--bare", "-q", "remote.git"); + const push = f.run( + "git", + ["push", "./remote.git", "HEAD:refs/heads/probe"], + isolated, + ); + assert.equal(push.status, 0, push.stdout + push.stderr); + for (const lane of ["unit", "--design", "--clippy"]) + assert.equal(f.read(`${lane}-input`), f.read("real-lhm-input")); + assert.match(f.read("real-lhm-input"), /refs\/heads\/probe/); +}); + +test("clean inherited lhm, owned wrapper edits and recursive metadata are handled safely", (t) => { + const f = lhmFixture(t); + f.git("config", "--worktree", "--unset", "core.hooksPath"); + assert.equal(f.install().status, 0); + const active = f.hooks(); + const metadataPath = path.join(active, "owner.json"); + const metadata = JSON.parse(readFileSync(metadataPath)); + assert.equal(metadata.previous, ""); + writeFileSync(path.join(active, "pre-commit"), "#!/bin/sh\nexit 0\n"); + assert.notEqual(f.install().status, 0); + assert.equal(f.hooks(), active); + writeFileSync(path.join(active, "pre-commit"), metadata.files["pre-commit"]); + metadata.upstream = active; + writeFileSync(metadataPath, JSON.stringify(metadata)); + assert.notEqual(f.install().status, 0); + assert.equal(f.hooks(), active); +}); + +test("missing pinned Lefthook blocks forwarded events", (t) => { + const f = lhmFixture(t); + rmSync(path.join(f.dir, "bin")); + const result = f.run(path.join(f.hooks(), "commit-msg"), ["message file"]); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Missing pinned Lefthook/); + assert.equal(existsSync(path.join(f.dir, "events")), false); +}); + +test("an interrupted child that exits successfully still stops dispatch", async (t) => { + const f = lhmFixture(t); + f.write( + ".githooks/pre-commit", + `#!/bin/sh +exec "${process.execPath}" -e 'process.on("SIGTERM", () => process.exit(0)); console.log("ready"); setInterval(() => {}, 1000);' +`, + ); + const child = spawn(path.join(f.hooks(), "pre-commit"), [], { + cwd: f.dir, + env, + stdio: ["ignore", "pipe", "pipe"], + }); + t.after(() => child.kill("SIGKILL")); + const closed = new Promise((resolve) => + child.on("close", (code, signal) => resolve({ code, signal })), + ); + await new Promise((resolve, reject) => { + child.stdout.once("data", resolve); + child.once("error", reject); + child.once("exit", () => reject(new Error("Exited before readiness"))); + }); + child.kill("SIGTERM"); + const result = await closed; + assert.notEqual(result.code, 0); + assert.equal(existsSync(path.join(f.dir, "events")), false); +}); + +test("invalid project configuration leaves the active dispatcher unchanged", (t) => { + const f = lhmFixture(t); + const active = f.hooks(); + f.write("lefthook.yml", "check-staged: [invalid\n"); + assert.notEqual(f.install().status, 0); + assert.equal(f.hooks(), active); +}); From 2e6d83c7517cd07c24659c57d16b291b19aeffe2 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 6 Oct 2026 14:27:04 +1100 Subject: [PATCH 2/4] feat(hooks): compose with lhm through standard Lefthook hooks Replace the worktree-local dispatcher, installer, custom check-staged and check-push groups and .githooks/ with ordinary pre-commit and pre-push jobs in lefthook.yml. lhm merges them with the machine policy at hook time, so there is nothing to install; without lhm, `bin/lefthook install` runs once per clone. Pin Lefthook 2.1.16, where a partial-staging restore conflict no longer discards unrelated unstaged edits, and require it through min_version. Lefthook now owns partial staging instead of the hook refusing it. Pre-commit is piped and opens with a read-only guard that refuses staged names Git would expand as globs: Lefthook restages with `git add --force -- `, so a staged `a[1].ts` would otherwise sweep `a1.ts` into the commit. check-icons.mjs accepts an explicit file list for the staged job. Dropped with the dispatcher: refusing partially staged files, the unstaged formatter-configuration check and the symlink type-change case; CI formats with committed configuration. Tests cover installation, linked worktrees, both partial-staging paths, failure ordering, push stdin and real lhm composition. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .githooks/pre-commit | 4 - .githooks/pre-push | 4 - AGENTS.md | 8 +- README.md | 5 +- ...fthook-2.1.12.pkg => .lefthook-2.1.16.pkg} | 0 bin/lefthook | 2 +- docs/contributing.md | 122 ++--- lefthook.yml | 38 +- package.json | 1 - scripts/check-push.mjs | 2 +- scripts/check-staged-names.mjs | 7 + scripts/check-staged.mjs | 101 ---- scripts/design-system/check-icons.mjs | 22 +- scripts/install-hooks.mjs | 149 ----- scripts/run-hook.mjs | 51 -- tests/integration/hooks.test.mjs | 516 +++++------------- 16 files changed, 240 insertions(+), 792 deletions(-) delete mode 100755 .githooks/pre-commit delete mode 100755 .githooks/pre-push rename bin/{.lefthook-2.1.12.pkg => .lefthook-2.1.16.pkg} (100%) create mode 100644 scripts/check-staged-names.mjs delete mode 100644 scripts/check-staged.mjs delete mode 100644 scripts/install-hooks.mjs delete mode 100644 scripts/run-hook.mjs diff --git a/.githooks/pre-commit b/.githooks/pre-commit deleted file mode 100755 index 66955e28c..000000000 --- a/.githooks/pre-commit +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -set -eu -cd "$(git rev-parse --show-toplevel)" -exec bin/lefthook run check-staged --no-auto-install diff --git a/.githooks/pre-push b/.githooks/pre-push deleted file mode 100755 index a115d12d0..000000000 --- a/.githooks/pre-push +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -set -eu -cd "$(git rev-parse --show-toplevel)" -exec bin/lefthook run check-push --no-auto-install diff --git a/AGENTS.md b/AGENTS.md index c5a60f9ef..79c20d223 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,8 +31,8 @@ Do not start development before bootstrap completes. The script copies the 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. +machine-shared, while dependencies and build output are regenerated. Git hooks +need no per-worktree setup; see [Git hooks](docs/contributing.md#git-hooks). ## Engineering standard @@ -203,8 +203,8 @@ 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; - preserve custom hooks and never bypass failures. + [hook setup](docs/contributing.md#pre-commit-checks) once per clone where lhm + is absent; 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 files: no credentials, local configuration, or raw agent/session data. diff --git a/README.md b/README.md index ee7943328..1edb4aa57 100644 --- a/README.md +++ b/README.md @@ -44,8 +44,9 @@ 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). +Staged-file pre-commit and related-test pre-push hooks run through lhm where it +is installed; otherwise run `bin/lefthook install` once per clone. See +[hook behavior and partial staging](docs/contributing.md#git-hooks). ### Design system diff --git a/bin/.lefthook-2.1.12.pkg b/bin/.lefthook-2.1.16.pkg similarity index 100% rename from bin/.lefthook-2.1.12.pkg rename to bin/.lefthook-2.1.16.pkg diff --git a/bin/lefthook b/bin/lefthook index 19449b873..fad8cb47c 120000 --- a/bin/lefthook +++ b/bin/lefthook @@ -1 +1 @@ -.lefthook-2.1.12.pkg \ No newline at end of file +.lefthook-2.1.16.pkg \ No newline at end of file diff --git a/docs/contributing.md b/docs/contributing.md index d93177c2d..7a89d2440 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1,6 +1,6 @@ # Contribution workflow -The repository pins just 1.58.0, Node.js 24.18.0, pnpm 11.8.0, Lefthook 2.1.12, +The repository pins just 1.58.0, Node.js 24.18.0, pnpm 11.8.0, Lefthook 2.1.16, and Rust 1.98.1 (including Cargo, rustfmt, and Clippy) with [Hermit](https://cashapp.github.io/hermit/). No global tool installation is required: `bin/hermit` bootstraps Hermit and tools @@ -173,8 +173,7 @@ without overwriting an existing target, then uses the new worktree's Hermit prox to run `bin/pnpm install --frozen-lockfile`. It rejects checkouts from another repository. Keychain credentials and pnpm's package cache remain machine-shared; do not copy private keys, `node_modules`, build output, `.npmrc`, or other ignored -files. Install hooks separately as described below so existing custom hooks are -never silently replaced. +files. Git hooks need no per-worktree step; see [Git hooks](#git-hooks). ### Worktree Dock labels (macOS) @@ -264,80 +263,74 @@ into an ever-growing full test suite. ### Pre-commit checks -Install once **per worktree** after `pnpm install --frozen-lockfile`: +`lefthook.yml` declares ordinary `pre-commit` and `pre-push` hooks. On a machine +with [lhm](https://github.com/block/lhm) there is nothing to install: lhm's global +hooks discover this file and merge it with the machine policy each time a hook +runs; `lhm dry-run` from the repository root prints the merged result. Without +lhm, install once per clone (linked worktrees share the installed hooks): ```sh -bin/pnpm hooks:install +bin/lefthook install ``` -The installer enables pre-commit and pre-push using Git's worktree-local -`core.hooksPath`, leaves sibling worktrees -alone, and supports standalone hooks or recognized lhm-generated wrappers. -Unknown or modified custom hooks are refused rather than overwritten. Repeat -installation is safe. 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 -JS/TS/JSON/CSS files, and rustfmt on individual staged Rust files. Remaining -warnings/errors block the commit; no unsafe lint fixes are applied. The staged -icon check also rejects known alternate icon families, direct upstream imports -outside the design-system gateway, and whole-catalog imports. Deletions and -unsupported formats (including Markdown, HTML and YAML) are not formatted here. -The hook does **not** run types, tests, builds, Clippy, or a whole-tree formatter. -`just iterate` remains the optional whole-tree fix/build command; `just scan` is -an opt-in broad diagnostic. Both reject remaining Biome warnings. - -Before writing, the hook refuses partially staged supported files, non-regular -files, and differing/untracked formatter configuration in their ancestor paths. -Format and reselect partial hunks, or stage/restore configuration, then retry. -Only checked paths are restaged after all checks succeed; a failed check can leave -safe fixes visible for review but does not update the index. Unrelated changes and -existing stashes are left alone. Do not edit/stage concurrently with a commit. -This is a developer guardrail, not a security boundary or a substitute for CI -and risk-appropriate behavior checks. Tool/config dependency changes require +The installed hooks fail rather than silently skip when they cannot find +Lefthook; `bin/lefthook uninstall` removes them. Lefthook 2.1.16 or newer is +required: `min_version` rejects older runners, including an older `lefthook` on +`PATH` under lhm (`brew upgrade lefthook`). + +Pre-commit runs these jobs in order and stops at the first failure: refuse staged +names containing `*`, `?`, `[` or `\`, which Git would expand as globs when +Lefthook restages them; the staged icon policy (known alternate icon families, +direct upstream imports outside the design-system gateway, whole-catalog +imports); pinned Biome formatting and safe lint fixes on staged JS/TS/JSON/CSS; +and rustfmt on staged Rust files. Remaining warnings/errors block the commit; no +unsafe lint fixes are applied. Deletions and unsupported formats (including +Markdown, HTML and YAML) are not formatted here. The hook does **not** run types, +tests, builds, Clippy, or a whole-tree formatter. `just iterate` remains the +optional whole-tree fix/build command; `just scan` is an opt-in broad diagnostic. +Both reject remaining Biome warnings. + +Lefthook owns partial staging: it hides the unstaged hunks of partially staged +files while the jobs run, restages the formatted files, then restores the hunks, +so unstaged hunks never enter the commit. When a formatter changes the same lines +as an unstaged hunk, the commit is blocked with "conflict while merging unstaged +changes" and the index, the files and unrelated edits are left as they were; +format the file first (`just iterate` or the editor), then reselect your hunks. +A failing read-only check leaves the index untouched; a failing formatter stages +nothing of its own, while an earlier formatter's restage stands. rustfmt follows +`mod` declarations into the working tree like `cargo fmt --all`; only staged +files are restaged. Do not edit or stage concurrently with a commit. This is a +developer guardrail, not a security boundary or a substitute for CI and +risk-appropriate behavior checks. Tool/config dependency changes require relevant integration evidence, not an automatic local full scan. -### Working with lhm +### Machine policy under lhm -When lhm is inherited (including underneath an existing Buzz `.githooks` -override), installation creates executable dispatchers under this worktree's Git -administration directory, in `buzz-hooks/dispatch-*`. Global configuration and -upstream wrappers remain untouched. Buzz runs first on pre-commit so its partial -staging guard sees the original working tree; lhm scans the resulting staged -content afterward. If lhm fails, Buzz's completed formatting and restaging remain -visible. On pre-push, lhm runs first and can reject the destination before Buzz's -three serialized jobs. Both layers receive the complete push input. Other -installed lhm events forward directly, with their arguments and stdin intact. -Pinned `bin/lefthook` is on PATH for lhm; missing tools or hooks fail visibly. +lhm runs the repository jobs first, then the machine's commands in the same hook: +`sadscan` after the pre-commit jobs and `check-push-org` after the push lanes. A +policy rejection still blocks the push, after the project checks have run. The +machine policy skips pre-commit during merges and rebases; CI remains the gate. -Verify installation with: +Checkouts set up by the previous Buzz installer carry a worktree-local +`core.hooksPath` that now points at deleted files, so Git either runs no hooks +there or fails every commit. In each such worktree run +`git config --worktree --unset core.hooksPath` and leave +`extensions.worktreeConfig` set. + +Verify the wiring with: ```sh -git config --show-origin --get core.hooksPath -git rev-parse --git-path buzz-hooks +git config --show-origin --get core.hooksPath # lhm's hooks directory, or unset bin/node --test tests/integration/hooks.test.mjs # Optional bounded probe, with isolated lhm system/user configuration: BUZZ_REAL_LHM="$(command -v lhm)" bin/node --test --test-name-pattern='real lhm' tests/integration/hooks.test.mjs ``` -Repeat installation after moving a checkout, changing the upstream hook inventory, -or changing dispatcher generation code. Existing upstream wrappers are invoked -at runtime, so their updates take effect immediately. Reinstall rejects edited -Buzz-generated wrappers. Prior generations are retained for recovery; do not edit -them or remove a generation still in use by a Git operation. - -Each generation's `owner.json` records `upstream` and the original worktree-local -`previous` setting. To undo installation, restore that value with -`git config --worktree core.hooksPath ''`; when `previous` is empty, use -`git config --worktree --unset core.hooksPath` to resume inherited hooks. Leave -`extensions.worktreeConfig` enabled because sibling worktrees may rely on it. -Standalone installation can likewise be removed with the unset command. - -For human acceptance, use a disposable checkout with the documented setup: -commit fully staged source and confirm Buzz formats it before lhm runs; partially -stage source and confirm rejection without writes or new stashes; then confirm a -failing job in either layer blocks the operation. Automated probes do not replace -this human confirmation before PR readiness. +For human acceptance, in a disposable checkout: commit a fully staged unformatted +source file and confirm it lands formatted; stage half of a file and confirm only +the staged hunks are committed with `git stash list` unchanged; introduce a Biome +warning and confirm the commit is blocked with the index unchanged. Automated +probes do not replace this confirmation before PR readiness. ### Fast pre-push feedback @@ -354,9 +347,8 @@ types/unit tests. A separate **rust-clippy** job runs the pinned Clippy over the whole Cargo workspace with the same invocation as the `native` CI lane (`cargo clippy --workspace --locked --all-targets -- -D warnings`); Rust-only changes do not run the JS/test lane, and its first cold build can take minutes. -The jobs are serialized because pinned Lefthook 2.1.12 shares -a mutable stdin reader: parallel consumers can lose Git refs and silently skip -checks. Source CSS/JS/TS, design viewer/guard files, shared +The jobs are serialized because Lefthook shares a mutable stdin reader between +jobs: parallel consumers can lose Git refs and silently skip checks. Source CSS/JS/TS, design viewer/guard files, shared configuration/dependencies and hook-runner changes select this job; a missing base runs it conservatively. Its selection is independent of the unit-test skip, so CSS-only and viewer-only errors still block a push. The Clippy lane is likewise diff --git a/lefthook.yml b/lefthook.yml index 82666abf7..c671ff2c5 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,14 +1,32 @@ -# A custom group avoids Lefthook's implicit pre-commit stash/restage machinery. -# .githooks/pre-commit invokes this group; the helper owns staged-file safety. -min_version: 2.1.12 -no_auto_install: true -check-staged: +min_version: 2.1.16 +# Without lhm, `bin/lefthook install` writes shims that fail instead of silently +# skipping when they cannot find Lefthook. lhm users install nothing: lhm merges +# this file with the machine policy at hook time. +assert_lefthook_installed: true +pre-commit: + # Stop at the first failure. Lefthook restages every successful `stage_fixed` + # job even when another job fails, so the read-only checks run first and a + # failing one leaves the index untouched. + piped: true jobs: - - name: format-and-lint - run: bin/node scripts/check-staged.mjs -check-push: - # Lefthook 2.1.12 shares a mutable cached stdin reader between jobs. Concurrent - # readers can receive truncated/empty refs and silently skip validation. + # Refuse names Git would expand as globs when Lefthook restages them. + - name: literal-names + glob: "*.{js,mjs,cjs,jsx,ts,mts,cts,tsx,json,jsonc,css,rs}" + run: bin/node scripts/check-staged-names.mjs {staged_files} + - name: icons + glob: "*.{js,mjs,cjs,jsx,ts,mts,cts,tsx,json}" + run: bin/node scripts/design-system/check-icons.mjs {staged_files} + - name: biome + glob: "*.{js,mjs,cjs,jsx,ts,mts,cts,tsx,json,jsonc,css}" + stage_fixed: true + run: bin/pnpm exec biome check --write --error-on-warnings --files-ignore-unknown=true --no-errors-on-unmatched -- {staged_files} + - name: rustfmt + glob: "*.rs" + stage_fixed: true + run: bin/rustfmt --edition 2021 -- {staged_files} +pre-push: + # Lefthook shares one cached stdin reader between jobs; serialize so every + # lane receives the complete ref list. parallel: false jobs: - name: types-and-related-unit-tests diff --git a/package.json b/package.json index 778c3bc3b..70eb470b7 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,6 @@ "preview": "vite preview", "author:build": "node scripts/build-author.mjs", "typecheck": "tsc --noEmit", - "hooks:install": "node scripts/install-hooks.mjs", "format": "biome format --write .", "format:check": "biome format .", "tauri": "tauri", diff --git a/scripts/check-push.mjs b/scripts/check-push.mjs index 1c9c58f96..d68bff86a 100644 --- a/scripts/check-push.mjs +++ b/scripts/check-push.mjs @@ -69,7 +69,7 @@ if (base.status === 0) { } if (design) { const input = - /^(?:src\/.*\.(?:css|tsx?|jsx?)$|tests\/fixtures\/design-system(?:\/|\.html$)|scripts\/design-system\/|(?:vite|vitest)\.design\.config\.|scripts\/check-push\.mjs$|lefthook\.yml$|\.githooks\/pre-push$)/s; + /^(?:src\/.*\.(?:css|tsx?|jsx?)$|tests\/fixtures\/design-system(?:\/|\.html$)|scripts\/design-system\/|(?:vite|vitest)\.design\.config\.|scripts\/check-push\.mjs$|lefthook\.yml$)/s; if (!files.some((file) => shared.test(file) || input.test(file))) { console.log( "No design-system inputs changed; remaining checks run in CI.", diff --git a/scripts/check-staged-names.mjs b/scripts/check-staged-names.mjs new file mode 100644 index 000000000..4c0ae3931 --- /dev/null +++ b/scripts/check-staged-names.mjs @@ -0,0 +1,7 @@ +// Lefthook restages fixed files with `git add --force -- `, which Git +// expands as a glob: a staged `a[1].ts` would also sweep `a1.ts` into the commit. +const expanded = process.argv.slice(2).filter((file) => /[*?[\\]/.test(file)); +if (expanded.length) + throw new Error( + `Rename before committing; Lefthook restages with glob pathspecs: ${expanded.join(", ")}`, + ); diff --git a/scripts/check-staged.mjs b/scripts/check-staged.mjs deleted file mode 100644 index 9d3785df9..000000000 --- a/scripts/check-staged.mjs +++ /dev/null @@ -1,101 +0,0 @@ -import { - checkIconSource, - checkIconManifest, -} from "./design-system/check-icons.mjs"; -import { execFileSync } from "node:child_process"; -import { lstatSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; - -const git = (...args) => execFileSync("git", args, { encoding: "utf8" }); -process.chdir(git("rev-parse", "--show-toplevel").trim()); -const paths = (output) => output.split("\0").filter(Boolean); -const staged = paths( - git("diff", "--cached", "--name-only", "--diff-filter=ACMRT", "-z", "--"), -); -const supported = /\.(?:[cm]?[jt]sx?|jsonc?|css|rs)$/; -const files = staged.filter((file) => supported.test(file)); -const unstaged = new Set(paths(git("diff", "--name-only", "-z", "--"))); -// Format using the configuration that will accompany this commit. Include nested -// configuration and untracked overrides, not just partially staged source files. -const configNames = [ - "biome.json", - "biome.jsonc", - ".editorconfig", - "rustfmt.toml", - ".rustfmt.toml", - "package.json", - "tsconfig.json", - ".gitignore", - ".ignore", -]; -const configs = new Set(); -for (const file of files) { - let directory = dirname(file); - while (true) { - for (const name of configNames) - configs.add(directory === "." ? name : `${directory}/${name}`); - if (directory === ".") break; - directory = dirname(directory); - } -} -const tracked = new Set(paths(git("ls-files", "-z"))); -for (const file of configs) { - const stat = lstatSync(file, { throwIfNoEntry: false }); - if (unstaged.has(file) || (stat && !tracked.has(file))) - throw new Error( - `Unstaged formatter configuration: ${file}. Stage or restore it before committing; nothing was changed.`, - ); - if (stat && !stat.isFile()) - throw new Error(`Refusing non-regular formatter configuration: ${file}`); -} -// Check every candidate before any formatter writes. Never broaden a partial commit. -for (const file of files) { - if (unstaged.has(file)) - throw new Error( - `Partially staged file: ${file}. Format it first, then reselect your hunks; nothing was changed.`, - ); - if (!lstatSync(file).isFile()) - throw new Error(`Refusing to format a non-regular staged file: ${file}`); -} -// Same icon policy as lint/CI, after partial-stage checks and before any writes. -for (const file of files) { - const source = readFileSync(file, "utf8"); - const errors = /\.[cm]?[jt]sx?$/.test(file) - ? checkIconSource(file, source) - : file === "package.json" || file.endsWith("/package.json") - ? checkIconManifest(JSON.parse(source)) - : []; - if (errors.length) throw new Error(`${file}: ${errors.join("\n")}`); -} -const biome = files.filter((file) => !file.endsWith(".rs")); -if (biome.length) - execFileSync( - resolve("bin/pnpm"), - [ - "exec", - "biome", - "check", - "--config-path=biome.json", - "--write", - "--error-on-warnings", - "--files-ignore-unknown=true", - "--no-errors-on-unmatched", - "--", - ...biome, - ], - { stdio: "inherit" }, - ); -for (const file of files.filter((file) => file.endsWith(".rs"))) { - // Stdin formats just this file, never follows `mod` into unstaged Rust files. - const formatted = execFileSync( - resolve("bin/rustfmt"), - ["--edition", "2021", "--emit", "stdout"], - { input: readFileSync(file), maxBuffer: 16 * 1024 * 1024 }, - ); - writeFileSync(file, formatted); -} -// A failed check leaves fixes visible but does not change the index. -if (files.length) - execFileSync("git", ["--literal-pathspecs", "add", "--", ...files], { - stdio: "inherit", - }); diff --git a/scripts/design-system/check-icons.mjs b/scripts/design-system/check-icons.mjs index 7dcc5af6a..ef356ddd2 100644 --- a/scripts/design-system/check-icons.mjs +++ b/scripts/design-system/check-icons.mjs @@ -185,14 +185,17 @@ export function checkIconSource(path, source) { visit(parsed.program); return errors; } -export function checkIcons() { - const paths = execFileSync( - "git", - ["ls-files", "--cached", "--others", "--exclude-standard", "-z"], - { cwd: root, encoding: "utf8" }, - ) - .split("\0") - .filter(Boolean); +// Check the given repository-relative paths, or every tracked/untracked file. +export function checkIcons(selected = []) { + const paths = selected.length + ? selected + : execFileSync( + "git", + ["ls-files", "--cached", "--others", "--exclude-standard", "-z"], + { cwd: root, encoding: "utf8" }, + ) + .split("\0") + .filter(Boolean); const errors = []; for (const path of new Set(paths)) { if (!existsSync(`${root}${path}`)) continue; @@ -212,4 +215,5 @@ export function checkIcons() { if (errors.length) throw new Error(errors.join("\n")); console.log("Icon dependencies and source import boundary checked."); } -if (process.argv[1] === fileURLToPath(import.meta.url)) checkIcons(); +if (process.argv[1] === fileURLToPath(import.meta.url)) + checkIcons(process.argv.slice(2)); diff --git a/scripts/install-hooks.mjs b/scripts/install-hooks.mjs deleted file mode 100644 index 56bc223c1..000000000 --- a/scripts/install-hooks.mjs +++ /dev/null @@ -1,149 +0,0 @@ -import { execFileSync, spawnSync } from "node:child_process"; -import { - accessSync, - constants, - existsSync, - mkdtempSync, - mkdirSync, - readFileSync, - readdirSync, - rmSync, - writeFileSync, -} from "node:fs"; -import { isAbsolute, join, resolve } from "node:path"; - -const git = (...args) => execFileSync("git", args, { encoding: "utf8" }).trim(); -const config = (key, scope = []) => { - const result = spawnSync("git", ["config", ...scope, "--get", key], { - encoding: "utf8", - }); - if (result.status === 1) return ""; - if (result.status !== 0) - throw new Error(result.stderr || `Cannot read ${key}`); - return result.stdout.trim(); -}; -const executable = (file) => accessSync(file, constants.X_OK); -const quote = (value) => `'${value.replaceAll("'", "'\\''")}'`; -process.chdir(git("rev-parse", "--show-toplevel")); -const root = process.cwd(); -const ownedRoot = resolve(git("rev-parse", "--git-path", "buzz-hooks")); -const existing = config("core.hooksPath"); -let upstream = existing; -let previous = - config("extensions.worktreeConfig") === "true" - ? config("core.hooksPath", ["--worktree"]) - : ""; -if (existing.startsWith(`${ownedRoot}/`)) { - const metadata = JSON.parse( - readFileSync(join(existing, "owner.json"), "utf8"), - ); - if ( - metadata.owner !== "buzz-hooks-v1" || - metadata.upstream.startsWith(ownedRoot) - ) - throw new Error( - "Invalid Buzz dispatcher ownership; restore the previous hooksPath.", - ); - for (const [name, content] of Object.entries(metadata.files)) { - if (readFileSync(join(existing, name), "utf8") !== content) - throw new Error( - `Modified Buzz dispatcher ${name}; reconcile it before installing.`, - ); - } - if ( - readdirSync(existing).some( - (name) => name !== "owner.json" && !(name in metadata.files), - ) - ) - throw new Error( - "Custom files in Buzz dispatcher; reconcile them before installing.", - ); - upstream = metadata.upstream; - previous = metadata.previous; -} else if (existing === ".githooks") { - // Read lower scopes without temporarily disabling the active hooks. - upstream = config("core.hooksPath", ["--local"]); - if (!upstream || upstream === ".githooks") - upstream = config("core.hooksPath", ["--global"]); - if (!upstream || upstream === ".githooks") - upstream = config("core.hooksPath", ["--system"]); - if (upstream === ".githooks") upstream = ""; -} -if (!existing) { - const hooks = git("rev-parse", "--git-path", "hooks"); - const custom = existsSync(hooks) - ? readdirSync(hooks).filter((name) => !name.endsWith(".sample")) - : []; - if (custom.length) - throw new Error( - `Existing hooks (${custom.join(", ")}); reconcile them before installing.`, - ); -} -if (config("core.worktree") || config("core.bare") === "true") - throw new Error( - "Nonstandard worktree configuration; install hooks manually.", - ); - -const events = []; -if (upstream) { - upstream = resolve(upstream); - for (const name of readdirSync(upstream)) { - if (!/^[a-z]+(?:-[a-z]+)+$/.test(name)) - throw new Error( - `Unknown custom hook ${name}; reconcile it before installing.`, - ); - const file = join(upstream, name); - const content = readFileSync(file, "utf8"); - const match = content.match( - /^#!\/bin\/sh\nexec "([^"$`\\\n]+)" run-hook ([a-z-]+) "\$@"\n$/, - ); - if (!match || match[2] !== name || !isAbsolute(match[1])) - throw new Error( - `Unrecognized lhm hook ${file}; reconcile it before installing.`, - ); - executable(file); - executable(match[1]); - events.push(name); - } - if (!events.includes("pre-commit") || !events.includes("pre-push")) - throw new Error("lhm installation must include pre-commit and pre-push."); -} -for (const file of [ - "bin/node", - "bin/lefthook", - ".githooks/pre-commit", - ".githooks/pre-push", -]) - executable(resolve(file)); -execFileSync(resolve("bin/lefthook"), ["validate"], { stdio: "inherit" }); -let target = ".githooks"; -if (upstream) { - mkdirSync(ownedRoot, { recursive: true }); - target = mkdtempSync(join(ownedRoot, "dispatch-")); - try { - const files = {}; - for (const event of events) { - files[event] = - event === "pre-commit" || event === "pre-push" - ? `#!/bin/sh\nexec ${quote(join(root, "bin/node"))} ${quote(join(root, "scripts/run-hook.mjs"))} ${quote(event)} ${quote(join(upstream, event))} "$@"\n` - : `#!/bin/sh\nset -eu\ntest -x ${quote(join(root, "bin/lefthook"))} || { echo "Missing pinned Lefthook" >&2; exit 1; }\nPATH=${quote(join(root, "bin"))}:"$PATH"\nexport PATH\nexec ${quote(join(upstream, event))} "$@"\n`; - writeFileSync(join(target, event), files[event], { mode: 0o755 }); - executable(join(target, event)); - execFileSync("sh", ["-n", join(target, event)]); - } - writeFileSync( - join(target, "owner.json"), - `${JSON.stringify({ owner: "buzz-hooks-v1", upstream, previous, files }, null, 2)}\n`, - ); - } catch (error) { - rmSync(target, { recursive: true, force: true }); - throw error; - } -} -// Only activate after every validation succeeds. Older generations remain usable -// for recovery; never rewrite an active wrapper underneath a running Git command. -git("config", "--local", "extensions.worktreeConfig", "true"); -git("config", "--worktree", "core.hooksPath", target); -console.log( - `Installed Buzz hooks for this worktree only${upstream ? ` with lhm at ${upstream}` : ""}.`, -); diff --git a/scripts/run-hook.mjs b/scripts/run-hook.mjs deleted file mode 100644 index f8e3f2bd4..000000000 --- a/scripts/run-hook.mjs +++ /dev/null @@ -1,51 +0,0 @@ -import { spawn } from "node:child_process"; -import { accessSync, constants, readFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; - -const [event, upstream, ...args] = process.argv.slice(2); -if (!["pre-commit", "pre-push"].includes(event) || !upstream) - throw new Error("Expected a Buzz hook event and upstream hook path."); -const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -const env = { - ...process.env, - PATH: `${resolve(root, "bin")}:${process.env.PATH ?? ""}`, -}; -// A missing Lefthook must not send lhm down its adapter fallback path. -for (const file of [resolve(root, "bin/lefthook"), upstream]) - accessSync(file, constants.X_OK); -const input = event === "pre-push" ? readFileSync(0) : undefined; -const buzz = resolve(root, ".githooks", event); -for (const hook of event === "pre-commit" - ? [buzz, upstream] - : [upstream, buzz]) { - const result = await new Promise((done) => { - const child = spawn(hook, args, { - env, - stdio: [input === undefined ? "inherit" : "pipe", "inherit", "inherit"], - }); - let interrupted; - const forward = (signal) => { - interrupted = signal; - child.kill(signal); - }; - const interrupt = () => forward("SIGINT"); - const terminate = () => forward("SIGTERM"); - process.on("SIGINT", interrupt); - process.on("SIGTERM", terminate); - child.on("error", (error) => console.error(error.message)); - child.on("close", (code, signal) => { - process.off("SIGINT", interrupt); - process.off("SIGTERM", terminate); - done({ code, signal: interrupted ?? signal }); - }); - if (input !== undefined) { - child.stdin.on("error", (error) => { - if (error.code !== "EPIPE") console.error(error.message); - }); - child.stdin.end(input); - } - }); - if (result.signal) process.kill(process.pid, result.signal); - if (result.code !== 0) process.exit(result.code ?? 1); -} diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index 1f6238e63..cb7be20d5 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { spawn, spawnSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import { chmodSync, cpSync, @@ -26,6 +26,13 @@ env.pnpm_config_verify_deps_before_run = "false"; // These commits are disposable probe fixtures, never commits in the source checkout. env.GIT_CONFIG_NOSYSTEM = "1"; env.GIT_CONFIG_GLOBAL = "/dev/null"; +// The installed shim prefers a Lefthook on PATH; pin the Hermit binary so a +// machine-wide installation cannot change which version the fixture exercises. +env.LEFTHOOK_BIN = path.join(root, "bin/lefthook"); +// Replaces the push lanes to capture the exact bytes each one receives. +const recordPushInput = `import { readFileSync, writeFileSync } from "node:fs"; +writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0)); +`; function fixture(t) { const dir = mkdtempSync(path.join(tmpdir(), "buzz-hook-")); t.after(() => rmSync(dir, { recursive: true, force: true })); @@ -50,7 +57,7 @@ function fixture(t) { git("config", "user.email", "hook-test@example.invalid"); // Git 2.50+ commits detach `maintenance run --auto`, whose worktree-prune // task deletes a `.git/worktrees/` entry that has no gitdir or lock yet. - // The sibling `worktree add` below would race that background process. + // The linked-worktree case would race that background process. git("config", "maintenance.auto", "false"); write("untouched.ts", "export const unrelated = 1;\n"); write("partial.ts", "export const first = 1;\nexport const second = 2;\n"); @@ -65,7 +72,6 @@ function fixture(t) { "package.json", "lefthook.yml", "scripts", - ".githooks", ...biomePlugins.map((plugin) => path.normalize(plugin)), ]; for (const file of configFiles) { @@ -80,47 +86,38 @@ function fixture(t) { ); git("add", ...configFiles); 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 installed = install(); + // Once per clone: Lefthook writes its shims into the shared `.git/hooks`. + const installed = run(path.join(root, "bin/lefthook"), ["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 }; + return { dir, run, git, write, read, commit }; } -test("both pre-push jobs receive the complete Git input without sharing a read cursor", (t) => { +test("every pre-push job receives 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 + // Keep the installed shim and production job configuration. Probe only the // stdin contract at the child boundary, including input larger than one read. - f.write( - "scripts/check-push.mjs", - `import { readFileSync, writeFileSync } from "node:fs"; -const lane = process.argv.includes("--design") ? "design" : process.argv.includes("--clippy") ? "clippy" : "unit"; -writeFileSync(lane + "-stdin", readFileSync(0)); -`, - ); + f.write("scripts/check-push.mjs", recordPushInput); const refs = `refs/heads/probe ${"a".repeat(40)} refs/heads/probe ${"0".repeat(40)}\n`.repeat( 1000, ); - const result = spawnSync(path.join(f.dir, ".githooks/pre-push"), [], { + const result = spawnSync(path.join(f.dir, ".git/hooks/pre-push"), [], { cwd: f.dir, env, input: refs, encoding: "utf8", }); assert.equal(result.status, 0, result.stdout + result.stderr); - for (const lane of ["design", "clippy", "unit"]) + for (const lane of ["unit", "--design", "--clippy"]) assert.equal( - f.read(`${lane}-stdin`), + f.read(`${lane}-input`), refs, `${lane} lost or duplicated Git refs`, ); }); -test("installed hook formats without rewriting borrowed dependencies or other work", (t) => { +test("installed hook formats fully staged files without rewriting borrowed dependencies or other work", (t) => { const dependencies = () => [".modules.yaml", "virtua/lib/index.js"].map((file) => readFileSync(path.join(root, "node_modules", file), "utf8"), @@ -149,7 +146,24 @@ test("installed hook formats without rewriting borrowed dependencies or other wo assert.equal(f.git("stash", "list"), ""); }); -test("partial staging fails before writes, preserving index, unrelated edits and stashes", (t) => { +test("one installation serves every linked worktree", (t) => { + const f = fixture(t); + const sibling = path.join(f.dir, "sibling"); + f.git("worktree", "add", "-q", "--detach", sibling); + for (const link of ["bin", "node_modules"]) + symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); + writeFileSync(path.join(sibling, "probe.ts"), "export const value={a:1}\n"); + const git = (...args) => f.run("git", ["-C", sibling, ...args]); + assert.equal(git("add", "probe.ts").status, 0); + const result = git("commit", "-qm", "sibling"); + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.equal( + git("show", "HEAD:probe.ts").stdout, + "export const value = { a: 1 };\n", + ); +}); + +test("non-conflicting partial staging commits only the staged hunks and restores the rest", (t) => { const f = fixture(t); f.write("untouched.ts", "export const unrelated = 7;\n"); f.git("stash", "push", "-qm", "existing user stash"); @@ -157,21 +171,56 @@ test("partial staging fails before writes, preserving index, unrelated edits and "partial.ts", "export const first={value:1}\nexport const second = 2;\n", ); + f.git("add", "partial.ts"); + f.write( + "partial.ts", + "export const first={value:1}\nexport const second = 99;\n", + ); + f.write("untouched.ts", "export const unrelated = 88;\n"); + const stashes = f.git("stash", "list"); + const result = f.commit(); + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.equal( + f.git("show", "HEAD:partial.ts"), + "export const first = { value: 1 };\nexport const second = 2;\n", + ); + assert.equal( + f.read("partial.ts"), + "export const first = { value: 1 };\nexport const second = 99;\n", + ); + assert.equal(f.read("untouched.ts"), "export const unrelated = 88;\n"); + assert.equal(f.git("stash", "list"), stashes); + assert.equal( + existsSync(path.join(f.dir, ".git/info/lefthook-unstaged.patch")), + false, + ); +}); + +test("a restore conflict blocks the commit and leaves index, files and unrelated edits as they were", (t) => { + const f = fixture(t); + f.write( + "partial.ts", + "export const first={value:1}\nexport const second = 2;\n", + ); f.write("fully.ts", "export const staged={value:1}\n"); f.git("add", "partial.ts", "fully.ts"); const index = f.git("write-tree"); - const partial = "export const first={value:1}\nexport const second = 99;\n"; + // The formatter rewrites the same line this unstaged hunk touches. + const partial = + "export const first={value:1} // note\nexport const second = 2;\n"; f.write("partial.ts", partial); f.write("untouched.ts", "export const unrelated = 88;\n"); - const stashes = f.git("stash", "list"); const result = f.commit(); assert.notEqual(result.status, 0); - assert.match(result.stdout + result.stderr, /Partially staged file/); + assert.match( + result.stdout + result.stderr, + /conflict while merging unstaged changes/, + ); assert.equal(f.git("write-tree"), index); assert.equal(f.read("partial.ts"), partial); assert.equal(f.read("fully.ts"), "export const staged={value:1}\n"); assert.equal(f.read("untouched.ts"), "export const unrelated = 88;\n"); - assert.equal(f.git("stash", "list"), stashes); + assert.equal(f.git("stash", "list"), ""); }); test("warnings reject commit without unsafe fixes or index updates", (t) => { @@ -189,19 +238,19 @@ test("warnings reject commit without unsafe fixes or index updates", (t) => { assert.equal(f.git("rev-parse", "HEAD"), head); }); -test("Rust formatting touches the staged file, not its unstaged modules", (t) => { +test("Rust formatting restages only the staged file; referenced modules are formatted in place", (t) => { const f = fixture(t); f.write("main.rs", 'mod child;\nfn main(){println!("test");}\n'); f.git("add", "main.rs"); - const child = "pub fn untouched( ){ }\n"; - f.write("child.rs", child); + f.write("child.rs", "pub fn untouched( ){ }\n"); const result = f.commit(); assert.equal(result.status, 0, result.stdout + result.stderr); assert.equal( f.git("show", "HEAD:main.rs"), 'mod child;\nfn main() {\n println!("test");\n}\n', ); - assert.equal(f.read("child.rs"), child); + // rustfmt follows `mod child;` like `cargo fmt --all`; only staged files are restaged. + assert.equal(f.read("child.rs"), "pub fn untouched() {}\n"); assert.equal(f.git("ls-files", "child.rs"), ""); }); @@ -217,87 +266,6 @@ 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) => { - const f = fixture(t); - assert.equal( - f.git("config", "--worktree", "--get", "core.hooksPath").trim(), - ".githooks", - ); - assert.equal( - f.run("git", ["config", "--local", "--get", "core.hooksPath"]).status, - 1, - ); - 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); - f.git("config", "--worktree", "--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"); - assert.notEqual(f.install().status, 0); - assert.equal( - f.git("config", "--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")); - config.linter.rules.style = { noNonNullAssertion: "off" }; - f.write("biome.json", `${JSON.stringify(config, null, 2)}\n`); - f.write( - "warning.ts", - "export const first = (values: string[]) => values[0]!;\n", - ); - f.git("add", "warning.ts"); - const index = f.git("write-tree"); - const result = f.commit(); - assert.notEqual( - result.status, - 0, - "Staged warning committed using unstaged rule disable", - ); - assert.equal(f.git("write-tree"), index); -}); - -test("type-change from symlink to source still checks warnings", (t) => { - const f = fixture(t); - symlinkSync("untouched.ts", path.join(f.dir, "changed.ts")); - f.git("add", "changed.ts"); - // Seed type-change baseline without running a hook on a symlink. - const seed = f.run("git", [ - "-c", - "core.hooksPath=/dev/null", - "commit", - "-qm", - "seed symlink", - ]); - assert.equal(seed.status, 0, seed.stderr); - rmSync(path.join(f.dir, "changed.ts")); - f.write( - "changed.ts", - "export const first = (values: string[]) => values[0]!;\n", - ); - f.git("add", "changed.ts"); - assert.match(f.git("diff", "--cached", "--name-status"), /^T\s+changed.ts/m); - const result = f.commit(); - assert.notEqual( - result.status, - 0, - "Type-changed source warning committed unchecked", - ); -}); - test("formatter failure preserves index and unrelated edits", (t) => { const f = fixture(t); f.write("safe.ts", "export const value={a:1}\n"); @@ -315,15 +283,9 @@ test("formatter failure preserves index and unrelated edits", (t) => { test("filename metacharacters and rename destination are literal", (t) => { const f = fixture(t); f.git("mv", "partial.ts", "renamed.ts"); - const names = [ - "-dash.ts", - "bracket[1].ts", - "line\nbreak.ts", - "colon:name.ts", - ]; + const names = ["-dash.ts", "line\nbreak.ts", "colon:name.ts"]; for (const name of names) f.write(name, "export const value={a:1}\n"); f.git("add", "--", ...names); - f.write("bracket1.ts", "export const untracked={a:2}\n"); const result = f.commit(); assert.equal(result.status, 0, result.stdout + result.stderr); for (const name of names) @@ -331,26 +293,44 @@ test("filename metacharacters and rename destination are literal", (t) => { f.git("show", `HEAD:${name}`), "export const value = { a: 1 };\n", ); - assert.equal(f.git("ls-files", "bracket1.ts"), ""); assert.equal(f.git("ls-files", "partial.ts"), ""); assert.equal(f.git("ls-files", "renamed.ts"), "renamed.ts\n"); }); -test("untracked nested formatter overrides fail before any source writes", (t) => { +test("names Git would expand as globs are refused before any write", (t) => { const f = fixture(t); - f.write("nested/biome.json", '{"root":false,"linter":{"enabled":false}}\n'); - const source = "export const first=(values:string[])=>values[0]!\n"; - f.write("nested/warning.ts", source); - f.git("add", "nested/warning.ts"); + const source = "export const value={a:1}\n"; + f.write("bracket[1].ts", source); + f.git("add", "--", "bracket[1].ts"); + // Lefthook's restage pathspec `bracket[1].ts` would also add this file. + f.write("bracket1.ts", "export const untracked={a:2}\n"); const index = f.git("write-tree"); const result = f.commit(); assert.notEqual(result.status, 0); assert.match( result.stdout + result.stderr, - /Unstaged formatter configuration/, + /glob pathspecs: bracket\[1\]\.ts/, + ); + assert.equal(f.git("write-tree"), index); + assert.equal(f.read("bracket[1].ts"), source); + assert.equal(f.git("ls-files", "bracket1.ts"), ""); +}); + +test("staged icon checks reject CommonJS subpaths without changing the index", (t) => { + const f = fixture(t); + f.write( + "probe.cjs", + 'const icon = require("lucide-react/dist/cjs/icons/x.js");\nmodule.exports = icon;\n', + ); + f.git("add", "probe.cjs"); + const index = f.git("write-tree"); + const result = f.commit(); + assert.notEqual(result.status, 0); + assert.match( + result.stdout + result.stderr, + /Use shared\/design-system\/icons/, ); assert.equal(f.git("write-tree"), index); - assert.equal(f.read("nested/warning.ts"), source); }); function pushFixture(t, changes) { @@ -383,7 +363,7 @@ function pushFixture(t, changes) { // a real cargo build, exactly like the fake Vitest below. rmSync(path.join(f.dir, "bin")); mkdirSync(path.join(f.dir, "bin")); - for (const tool of ["node", "lefthook", "pnpm"]) { + for (const tool of ["node", "pnpm"]) { symlinkSync( path.join(root, `bin/${tool}`), path.join(f.dir, `bin/${tool}`), @@ -665,16 +645,12 @@ test("raw activity CSS blocks an actual push; shared tokens pass without hook wr for (const file of [ "tests/fixtures/design-system/probe.ts", "scripts/design-system/check-app-foundations.mjs", - ".githooks/pre-push", ]) { test(`design-only change to ${file} runs guards before the unit-test skip`, (t) => { - const content = file.endsWith(".ts") - ? "" - : readFileSync(path.join(root, file), "utf8"); const f = pushFixture(t, { [file]: file.endsWith(".ts") ? "export const value = 2;\n" - : `${content}\n${file.startsWith(".githooks/") ? "#" : "//"} guard probe\n`, + : `${readFileSync(path.join(root, file), "utf8")}\n// guard probe\n`, }); // Pre-push has the same documented working-tree scope as types and Vitest. f.write("src/bad.css", ".root { gap: 8px; }\n"); @@ -754,200 +730,19 @@ test("the design lane disables dependency auto-repair even when inherited as tru ); }); -test("staged icon checks reject CommonJS subpaths without changing the index", (t) => { - const f = fixture(t); - f.write( - "probe.cjs", - 'const icon = require("lucide-react/dist/cjs/icons/x.js");\nmodule.exports = icon;\n', - ); - f.git("add", "probe.cjs"); - const index = f.git("write-tree"); - const result = f.commit(); - assert.notEqual(result.status, 0); - assert.match( - result.stdout + result.stderr, - /Use shared\/design-system\/icons/, - ); - assert.equal(f.git("write-tree"), index); -}); - -function lhmFixture(t) { +test("real lhm composes isolated system commands with the repository jobs", { + skip: !process.env.BUZZ_REAL_LHM, +}, (t) => { const f = fixture(t); const upstream = path.join(f.dir, "upstream ' $ hooks"); - const manager = path.join(f.dir, "manager with spaces"); - f.write( - "manager with spaces", - `#!/bin/sh - event="$2" - printf '%s\\n' "$event" >> events - if [ "$event" = pre-commit ]; then git show :probe.ts > upstream-source; fi - if [ "$event" = pre-push ] || [ "$event" = post-rewrite ]; then cat > "$event-input"; fi - printf '%s\\n' "$@" > "$event-args" - exit "\${UPSTREAM_STATUS:-0}" -`, - ); - chmodSync(manager, 0o755); - const names = [ - "pre-commit", - "pre-push", - "commit-msg", - "prepare-commit-msg", - "post-rewrite", - ]; - for (const name of names) { - const file = path.join(upstream, name); + for (const name of ["pre-commit", "pre-push"]) { f.write( - path.relative(f.dir, file), - `#!/bin/sh\nexec "${manager}" run-hook ${name} "$@"\n`, + path.relative(f.dir, path.join(upstream, name)), + `#!/bin/sh\nexec "${process.env.BUZZ_REAL_LHM}" run-hook ${name} "$@"\n`, ); - chmodSync(file, 0o755); + chmodSync(path.join(upstream, name), 0o755); } - const global = path.join(f.dir, "global-config"); f.write("global-config", `[core]\n hooksPath = "${upstream}"\n`); - const overrides = { GIT_CONFIG_GLOBAL: global }; - const install = () => - f.run( - path.join(root, "bin/node"), - ["scripts/install-hooks.mjs"], - overrides, - ); - const result = install(); - assert.equal(result.status, 0, result.stdout + result.stderr); - const hooks = () => - f.git("config", "--worktree", "--get", "core.hooksPath").trim(); - return { ...f, upstream, manager, overrides, install, hooks }; -} - -test("lhm installation preserves inherited configuration, wrappers and sibling behavior", (t) => { - const f = lhmFixture(t); - const global = f.read("global-config"); - const wrapper = readFileSync(path.join(f.upstream, "pre-commit"), "utf8"); - const first = f.hooks(); - assert.equal(f.install().status, 0); - assert.notEqual(f.hooks(), first); - assert.equal( - JSON.parse(readFileSync(path.join(f.hooks(), "owner.json"))).upstream, - f.upstream, - ); - assert.equal(f.read("global-config"), global); - assert.equal( - readFileSync(path.join(f.upstream, "pre-commit"), "utf8"), - wrapper, - ); - assert.equal( - f - .run( - "git", - ["-C", f.sibling, "config", "--get", "core.hooksPath"], - f.overrides, - ) - .stdout.trim(), - f.upstream, - ); - f.write( - path.relative(f.dir, path.join(f.upstream, "pre-push")), - "#!/bin/sh\nexit 0\n", - ); - const active = f.hooks(); - assert.notEqual(f.install().status, 0); - assert.equal(f.hooks(), active); -}); - -test("Buzz formats before lhm, and upstream failure keeps completed formatting visible", (t) => { - const f = lhmFixture(t); - f.write("probe.ts", "export const value={a:1}\n"); - f.git("add", "probe.ts"); - const result = f.run("git", ["commit", "-qm", "probe"], { - ...f.overrides, - UPSTREAM_STATUS: "7", - }); - assert.notEqual(result.status, 0); - assert.equal(f.read("upstream-source"), "export const value = { a: 1 };\n"); - assert.equal(f.git("show", ":probe.ts"), f.read("upstream-source")); - assert.equal(f.commit().status, 0); - assert.match(f.read("events"), /prepare-commit-msg\ncommit-msg/); -}); - -test("partial staging blocks lhm before writes or stashes", (t) => { - const f = lhmFixture(t); - f.write("probe.ts", "export const value={a:1}\n"); - f.git("add", "probe.ts"); - const index = f.git("write-tree"); - f.write("probe.ts", "export const value={a:2}\n"); - const result = f.commit(); - assert.notEqual(result.status, 0); - assert.match(result.stdout + result.stderr, /Partially staged/); - assert.equal(f.git("write-tree"), index); - assert.equal(f.read("probe.ts"), "export const value={a:2}\n"); - assert.equal(existsSync(path.join(f.dir, "events")), false); - assert.equal(f.git("stash", "list"), ""); -}); - -test("dispatch replays raw push bytes to lhm and every Buzz lane, preserving literal arguments", (t) => { - const f = lhmFixture(t); - f.write( - "scripts/check-push.mjs", - `import { readFileSync, writeFileSync } from "node:fs"; -writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0));`, - ); - for (const input of [ - Buffer.alloc(0), - Buffer.from("refs \x00 \xff\n".repeat(20000)), - ]) { - const result = spawnSync( - path.join(f.hooks(), "pre-push"), - ["remote '$", "destination ;$"], - { cwd: f.dir, env, input }, - ); - assert.equal(result.status, 0, String(result.stderr)); - for (const lane of ["pre-push", "unit", "--design", "--clippy"]) - assert.deepEqual(readFileSync(path.join(f.dir, `${lane}-input`)), input); - assert.equal( - f.read("pre-push-args"), - "run-hook\npre-push\nremote '$\ndestination ;$\n", - ); - } - const result = spawnSync(path.join(f.hooks(), "post-rewrite"), ["amend"], { - cwd: f.dir, - env, - input: "old new\n", - }); - assert.equal(result.status, 0); - assert.equal(f.read("post-rewrite-input"), "old new\n"); -}); - -test("upstream push rejection and missing hooks fail before Buzz runs", (t) => { - const f = lhmFixture(t); - f.write("scripts/check-push.mjs", 'throw new Error("Buzz should not run");'); - const invoke = () => - spawnSync(path.join(f.hooks(), "pre-push"), [], { - cwd: f.dir, - env: { ...env, UPSTREAM_STATUS: "9" }, - input: "", - encoding: "utf8", - }); - assert.equal(invoke().status, 9); - rmSync(path.join(f.upstream, "pre-push")); - const missing = invoke(); - assert.notEqual(missing.status, 0); - assert.doesNotMatch(missing.stderr, /Buzz should not run/); -}); - -test("real lhm composes isolated system jobs with Buzz custom groups", { - skip: !process.env.BUZZ_REAL_LHM, -}, (t) => { - const f = lhmFixture(t); - for (const name of [ - "pre-commit", - "pre-push", - "commit-msg", - "prepare-commit-msg", - "post-rewrite", - ]) - f.write( - path.relative(f.dir, path.join(f.upstream, name)), - `#!/bin/sh\nexec "${process.env.BUZZ_REAL_LHM}" run-hook ${name} "$@"\n`, - ); f.write( "system/lefthook.yml", `pre-commit: @@ -961,22 +756,26 @@ pre-push: use_stdin: true `, ); + // An empty user file parses as null and wipes the merge; `{}` is the empty layer. f.write("absent-user.yml", "{}\n"); const isolated = { - ...f.overrides, + GIT_CONFIG_GLOBAL: path.join(f.dir, "global-config"), LHM_SYSTEM_CONFIG: path.join(f.dir, "system"), LHM_USER_CONFIG: path.join(f.dir, "absent-user.yml"), + // lhm runs whichever `lefthook` is on PATH; use the pinned one. + PATH: `${path.join(root, "bin")}${path.delimiter}${env.PATH}`, }; f.write("probe.ts", "export const value={a:1}\n"); f.git("add", "probe.ts"); const commit = f.run("git", ["commit", "-qm", "real lhm"], isolated); assert.equal(commit.status, 0, commit.stdout + commit.stderr); - assert.equal(f.read("real-lhm-source"), "export const value = { a: 1 };\n"); - f.write( - "scripts/check-push.mjs", - `import { readFileSync, writeFileSync } from "node:fs"; -writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0));`, + assert.equal( + f.git("show", "HEAD:probe.ts"), + "export const value = { a: 1 };\n", ); + // The machine's command ran after the repository jobs, in the same hook. + assert.equal(f.read("real-lhm-source"), "export const value={a:1}\n"); + f.write("scripts/check-push.mjs", recordPushInput); f.git("init", "--bare", "-q", "remote.git"); const push = f.run( "git", @@ -988,66 +787,3 @@ writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0));`, assert.equal(f.read(`${lane}-input`), f.read("real-lhm-input")); assert.match(f.read("real-lhm-input"), /refs\/heads\/probe/); }); - -test("clean inherited lhm, owned wrapper edits and recursive metadata are handled safely", (t) => { - const f = lhmFixture(t); - f.git("config", "--worktree", "--unset", "core.hooksPath"); - assert.equal(f.install().status, 0); - const active = f.hooks(); - const metadataPath = path.join(active, "owner.json"); - const metadata = JSON.parse(readFileSync(metadataPath)); - assert.equal(metadata.previous, ""); - writeFileSync(path.join(active, "pre-commit"), "#!/bin/sh\nexit 0\n"); - assert.notEqual(f.install().status, 0); - assert.equal(f.hooks(), active); - writeFileSync(path.join(active, "pre-commit"), metadata.files["pre-commit"]); - metadata.upstream = active; - writeFileSync(metadataPath, JSON.stringify(metadata)); - assert.notEqual(f.install().status, 0); - assert.equal(f.hooks(), active); -}); - -test("missing pinned Lefthook blocks forwarded events", (t) => { - const f = lhmFixture(t); - rmSync(path.join(f.dir, "bin")); - const result = f.run(path.join(f.hooks(), "commit-msg"), ["message file"]); - assert.notEqual(result.status, 0); - assert.match(result.stderr, /Missing pinned Lefthook/); - assert.equal(existsSync(path.join(f.dir, "events")), false); -}); - -test("an interrupted child that exits successfully still stops dispatch", async (t) => { - const f = lhmFixture(t); - f.write( - ".githooks/pre-commit", - `#!/bin/sh -exec "${process.execPath}" -e 'process.on("SIGTERM", () => process.exit(0)); console.log("ready"); setInterval(() => {}, 1000);' -`, - ); - const child = spawn(path.join(f.hooks(), "pre-commit"), [], { - cwd: f.dir, - env, - stdio: ["ignore", "pipe", "pipe"], - }); - t.after(() => child.kill("SIGKILL")); - const closed = new Promise((resolve) => - child.on("close", (code, signal) => resolve({ code, signal })), - ); - await new Promise((resolve, reject) => { - child.stdout.once("data", resolve); - child.once("error", reject); - child.once("exit", () => reject(new Error("Exited before readiness"))); - }); - child.kill("SIGTERM"); - const result = await closed; - assert.notEqual(result.code, 0); - assert.equal(existsSync(path.join(f.dir, "events")), false); -}); - -test("invalid project configuration leaves the active dispatcher unchanged", (t) => { - const f = lhmFixture(t); - const active = f.hooks(); - f.write("lefthook.yml", "check-staged: [invalid\n"); - assert.notEqual(f.install().status, 0); - assert.equal(f.hooks(), active); -}); From 46a9c3e1d2316c44e43188d5a18c2921de4e6b87 Mon Sep 17 00:00:00 2001 From: Star Lord Date: Tue, 6 Oct 2026 11:02:13 -0700 Subject: [PATCH 3/4] fix(hooks): restore install recipe and document global hook safety Signed-off-by: Star Lord --- README.md | 3 +- docs/contributing.md | 24 +++++- justfile | 4 + tests/integration/hooks.test.mjs | 126 +++++++++++++++++++++++++++---- 4 files changed, 140 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 1edb4aa57..edbce9f2e 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,8 @@ Without live opt-in they run the shell without relay identity access. `just scan` adds tests and native checks. [PR CI](.github/workflows/ci.yml) runs those checks in cached, parallel jobs with sharded browser journeys. Staged-file pre-commit and related-test pre-push hooks run through lhm where it -is installed; otherwise run `bin/lefthook install` once per clone. See +is installed; otherwise run `just hooks` (or `bin/just hooks` without activation) +once per clone. See [hook behavior and partial staging](docs/contributing.md#git-hooks). ### Design system diff --git a/docs/contributing.md b/docs/contributing.md index 7a89d2440..47a0e6b97 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -270,9 +270,31 @@ runs; `lhm dry-run` from the repository root prints the merged result. Without lhm, install once per clone (linked worktrees share the installed hooks): ```sh -bin/lefthook install +just hooks # or bin/just hooks without Hermit activation ``` +This recipe runs the pinned `bin/lefthook install`; it never changes Git config. +If installation refuses because of a global `core.hooksPath`, **do not follow +Lefthook's suggested fixes**: `--reset-hooks-path` and +`git config --unset-all --global core.hooksPath` can disable machine hooks in +other repositories. Running `lefthook install --force` **without first setting a +clone-local path** can replace hooks in the global directory, affecting every +repository that uses it. Under lhm, install nothing. + +For a **non-lhm** global path, if you explicitly want this clone to use its own +hooks instead, first remove any stale worktree-local overrides as described +below. Then set the clone-local path before running `--force`, from either +checkout: + +```sh +git config --local core.hooksPath "$(git rev-parse --path-format=absolute --git-common-dir)/hooks" +bin/lefthook install --force +``` + +The absolute path works in the main checkout and linked worktrees; the global +setting is unchanged, but its hooks no longer run in this clone. Do not use this +override for lhm. + The installed hooks fail rather than silently skip when they cannot find Lefthook; `bin/lefthook uninstall` removes them. Lefthook 2.1.16 or newer is required: `min_version` rejects older runners, including an older `lefthook` on diff --git a/justfile b/justfile index ef11165a2..2b7d1e12b 100644 --- a/justfile +++ b/justfile @@ -4,6 +4,10 @@ default: @just --list +# Install standard hooks once per clone when lhm is not managing them. +hooks: + bin/lefthook install + [private] install: pnpm install --frozen-lockfile diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index cb7be20d5..f703074b8 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -33,7 +33,7 @@ env.LEFTHOOK_BIN = path.join(root, "bin/lefthook"); const recordPushInput = `import { readFileSync, writeFileSync } from "node:fs"; writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0)); `; -function fixture(t) { +function fixture(t, { install = true } = {}) { const dir = mkdtempSync(path.join(tmpdir(), "buzz-hook-")); t.after(() => rmSync(dir, { recursive: true, force: true })); const run = (cmd, args, overrides = {}) => @@ -71,6 +71,7 @@ function fixture(t) { "biome.json", "package.json", "lefthook.yml", + "justfile", "scripts", ...biomePlugins.map((plugin) => path.normalize(plugin)), ]; @@ -86,9 +87,11 @@ function fixture(t) { ); git("add", ...configFiles); git("commit", "-qm", "hook configuration"); - // Once per clone: Lefthook writes its shims into the shared `.git/hooks`. - const installed = run(path.join(root, "bin/lefthook"), ["install"]); - assert.equal(installed.status, 0, installed.stdout + installed.stderr); + // Once per clone: the public recipe installs into the shared `.git/hooks`. + if (install) { + const installed = run(path.join(root, "bin/just"), ["hooks"]); + assert.equal(installed.status, 0, installed.stdout + installed.stderr); + } const commit = () => run("git", ["commit", "-qm", "probe"]); return { dir, run, git, write, read, commit }; } @@ -163,6 +166,80 @@ test("one installation serves every linked worktree", (t) => { ); }); +for (const linked of [false, true]) { + test(`non-lhm global hooks stay untouched when installing from ${linked ? "a linked worktree" : "the main checkout"}`, (t) => { + const f = fixture(t, { install: false }); + const sibling = path.join(f.dir, "sibling"); + f.git("worktree", "add", "-q", "--detach", sibling); + for (const link of ["bin", "node_modules"]) + symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); + const globalHooks = path.join(f.dir, "global hooks"); + const sentinel = "#!/bin/sh\nexit 1\n"; + f.write("global hooks/pre-commit", sentinel); + const config = `[core]\n hooksPath = "${globalHooks}"\n`; + f.write("global-config", config); + const isolated = { + GIT_CONFIG_GLOBAL: path.join(f.dir, "global-config"), + }; + const cwd = linked ? sibling : f.dir; + const run = (cmd, args) => + spawnSync(cmd, args, { + cwd, + env: { ...env, ...isolated }, + encoding: "utf8", + }); + const refused = run(path.join(root, "bin/just"), ["hooks"]); + assert.notEqual(refused.status, 0); + assert.match(refused.stdout + refused.stderr, /core.hooksPath/); + assert.equal(f.read("global-config"), config); + assert.equal(f.read("global hooks/pre-commit"), sentinel); + assert.equal(existsSync(path.join(f.dir, ".git/hooks/pre-commit")), false); + + // Exercise the documented opt-in override from both invocation contexts. + const common = run("git", [ + "rev-parse", + "--path-format=absolute", + "--git-common-dir", + ]); + assert.equal(common.status, 0, common.stdout + common.stderr); + const hooks = path.join(common.stdout.trim(), "hooks"); + assert.equal(path.isAbsolute(hooks), true); + const configured = run("git", [ + "config", + "--local", + "core.hooksPath", + hooks, + ]); + assert.equal(configured.status, 0, configured.stdout + configured.stderr); + const installed = run(path.join(root, "bin/lefthook"), [ + "install", + "--force", + ]); + assert.equal(installed.status, 0, installed.stdout + installed.stderr); + assert.equal(f.read("global-config"), config); + assert.equal(f.read("global hooks/pre-commit"), sentinel); + for (const checkout of [f.dir, sibling]) { + writeFileSync( + path.join(checkout, "probe.ts"), + "export const value={a:1}\n", + ); + const git = (...args) => + f.run("git", ["-C", checkout, ...args], isolated); + assert.equal( + git("config", "--get", "core.hooksPath").stdout.trim(), + hooks, + ); + assert.equal(git("add", "probe.ts").status, 0); + const committed = git("commit", "-qm", "global path probe"); + assert.equal(committed.status, 0, committed.stdout + committed.stderr); + assert.equal( + git("show", "HEAD:probe.ts").stdout, + "export const value = { a: 1 };\n", + ); + } + }); +} + test("non-conflicting partial staging commits only the staged hunks and restores the rest", (t) => { const f = fixture(t); f.write("untouched.ts", "export const unrelated = 7;\n"); @@ -733,7 +810,11 @@ test("the design lane disables dependency auto-repair even when inherited as tru test("real lhm composes isolated system commands with the repository jobs", { skip: !process.env.BUZZ_REAL_LHM, }, (t) => { - const f = fixture(t); + const f = fixture(t, { install: false }); + const sibling = path.join(f.dir, "sibling"); + f.git("worktree", "add", "-q", "--detach", sibling); + for (const link of ["bin", "node_modules"]) + symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); const upstream = path.join(f.dir, "upstream ' $ hooks"); for (const name of ["pre-commit", "pre-push"]) { f.write( @@ -765,16 +846,31 @@ pre-push: // lhm runs whichever `lefthook` is on PATH; use the pinned one. PATH: `${path.join(root, "bin")}${path.delimiter}${env.PATH}`, }; - f.write("probe.ts", "export const value={a:1}\n"); - f.git("add", "probe.ts"); - const commit = f.run("git", ["commit", "-qm", "real lhm"], isolated); - assert.equal(commit.status, 0, commit.stdout + commit.stderr); - assert.equal( - f.git("show", "HEAD:probe.ts"), - "export const value = { a: 1 };\n", - ); - // The machine's command ran after the repository jobs, in the same hook. - assert.equal(f.read("real-lhm-source"), "export const value={a:1}\n"); + const before = f.read("global-config"); + const refused = f.run(path.join(root, "bin/just"), ["hooks"], isolated); + assert.notEqual(refused.status, 0); + assert.equal(f.read("global-config"), before); + assert.equal(existsSync(path.join(f.dir, ".git/hooks/pre-commit")), false); + for (const checkout of [f.dir, sibling]) { + writeFileSync( + path.join(checkout, "probe.ts"), + "export const value={a:1}\n", + ); + const git = (...args) => f.run("git", ["-C", checkout, ...args], isolated); + assert.equal(git("add", "probe.ts").status, 0); + const commit = git("commit", "-qm", "real lhm"); + assert.equal(commit.status, 0, commit.stdout + commit.stderr); + assert.equal( + git("show", "HEAD:probe.ts").stdout, + "export const value = { a: 1 };\n", + ); + // The machine's command ran after the jobs, before Lefthook restaged fixes. + assert.equal( + readFileSync(path.join(checkout, "real-lhm-source"), "utf8"), + "export const value={a:1}\n", + ); + } + assert.equal(f.read("global-config"), before); f.write("scripts/check-push.mjs", recordPushInput); f.git("init", "--bare", "-q", "remote.git"); const push = f.run( From 88566880cad9f57570cc140b2e4134d9fba57ae6 Mon Sep 17 00:00:00 2001 From: Star Lord Date: Tue, 6 Oct 2026 16:03:30 -0700 Subject: [PATCH 4/4] fix(hooks): isolate linked-worktree recovery with pinned runner Signed-off-by: Star Lord --- bin/{.lefthook-2.1.16.pkg => .go-1.27.0.pkg} | 0 bin/.lefthook-2.1.18-buzz.3.pkg | 1 + bin/go | 1 + bin/gofmt | 1 + bin/lefthook | 8 +- bin/lefthook-runner | 1 + bin/packages/lefthook-LICENSE | 22 + bin/packages/lefthook-recovery.md | 64 ++ bin/packages/lefthook-recovery.patch | 866 +++++++++++++++++++ bin/packages/lefthook.hcl | 24 + docs/contributing.md | 43 +- lefthook.yml | 6 +- tests/integration/hooks.test.mjs | 264 +++++- 13 files changed, 1279 insertions(+), 22 deletions(-) rename bin/{.lefthook-2.1.16.pkg => .go-1.27.0.pkg} (100%) create mode 120000 bin/.lefthook-2.1.18-buzz.3.pkg create mode 120000 bin/go create mode 120000 bin/gofmt mode change 120000 => 100755 bin/lefthook create mode 120000 bin/lefthook-runner create mode 100644 bin/packages/lefthook-LICENSE create mode 100644 bin/packages/lefthook-recovery.md create mode 100644 bin/packages/lefthook-recovery.patch create mode 100644 bin/packages/lefthook.hcl diff --git a/bin/.lefthook-2.1.16.pkg b/bin/.go-1.27.0.pkg similarity index 100% rename from bin/.lefthook-2.1.16.pkg rename to bin/.go-1.27.0.pkg diff --git a/bin/.lefthook-2.1.18-buzz.3.pkg b/bin/.lefthook-2.1.18-buzz.3.pkg new file mode 120000 index 000000000..383f4511d --- /dev/null +++ b/bin/.lefthook-2.1.18-buzz.3.pkg @@ -0,0 +1 @@ +hermit \ No newline at end of file diff --git a/bin/go b/bin/go new file mode 120000 index 000000000..f5d2d50ed --- /dev/null +++ b/bin/go @@ -0,0 +1 @@ +.go-1.27.0.pkg \ No newline at end of file diff --git a/bin/gofmt b/bin/gofmt new file mode 120000 index 000000000..f5d2d50ed --- /dev/null +++ b/bin/gofmt @@ -0,0 +1 @@ +.go-1.27.0.pkg \ No newline at end of file diff --git a/bin/lefthook b/bin/lefthook deleted file mode 120000 index fad8cb47c..000000000 --- a/bin/lefthook +++ /dev/null @@ -1 +0,0 @@ -.lefthook-2.1.16.pkg \ No newline at end of file diff --git a/bin/lefthook b/bin/lefthook new file mode 100755 index 000000000..80c68b32e --- /dev/null +++ b/bin/lefthook @@ -0,0 +1,7 @@ +#!/bin/sh +# Hermit unpacks the target before its dependencies on lazy first use. +# Provision the build tool outside the unpack lock, then run the pinned repair. +set -eu +bin=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +"$bin/go" version >/dev/null +exec "$bin/lefthook-runner" "$@" diff --git a/bin/lefthook-runner b/bin/lefthook-runner new file mode 120000 index 000000000..bdd274db8 --- /dev/null +++ b/bin/lefthook-runner @@ -0,0 +1 @@ +.lefthook-2.1.18-buzz.3.pkg \ No newline at end of file diff --git a/bin/packages/lefthook-LICENSE b/bin/packages/lefthook-LICENSE new file mode 100644 index 000000000..bf519ae20 --- /dev/null +++ b/bin/packages/lefthook-LICENSE @@ -0,0 +1,22 @@ + +The MIT License (MIT) + +Copyright (c) 2019 Arkweid + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/bin/packages/lefthook-recovery.md b/bin/packages/lefthook-recovery.md new file mode 100644 index 000000000..eebba3952 --- /dev/null +++ b/bin/packages/lefthook-recovery.md @@ -0,0 +1,64 @@ +# Temporary Lefthook recovery package + +This override builds [Lefthook](https://github.com/evilmartians/lefthook) at +`cc9969c2e2198aafff8accc63ee1aee30e9a2aa4` with `lefthook-recovery.patch`. +The source archive is SHA-256 pinned in `lefthook.hcl`; two independent downloads +produced the same digest. Hermit pins Go 1.27.0 and generates the tool proxies. +The small `bin/lefthook` launcher provisions Go before invoking the generated +`bin/lefthook-runner` proxy: lazy Hermit execution otherwise unpacks Lefthook +before Go, and recursively invoking Hermit inside unpack deadlocks. Hermit 0.52.3 also unpacks explicit installs in parallel: run +`bin/go version` before `bin/hermit install` if provisioning all packages manually. +Normal hook setup uses the launcher and needs no separate Go command. +The build uses `CGO_ENABLED=0`, `-trimpath`, and the upstream Go module checksums. +No private package service, fork release, or global tool replacement is required. + +The patch fixes the shared patch files and broad positional stash deletion +reported in [upstream #1529](https://github.com/evilmartians/lefthook/issues/1529). +[Upstream #1530](https://github.com/evilmartians/lefthook/pull/1530) addresses +worktree isolation, but this patch also replaces shared stash-list cleanup with +create-only, compare-and-delete backup refs. Shared owned refs retain backups +during garbage collection from another worktree. The patch includes upstream +unit/integration tests, recovery documentation, and an explicit version change +to `2.1.18-buzz.3` (the version is a source constant, not a linker variable). +Concurrent runs within the same worktree remain unsupported. + +## Revalidate or replace + +Never change the patch/build under the same package version: Hermit's shared +cache keys by package version. Bump the package, source version edit, repository +`min_version`, and generated proxies together after any change. Do not invoke +`bin/lefthook-runner` directly: `bin/lefthook` is the first-use entry point. + +A Git checkout of this PR replaces the old Lefthook symlink/pin with the launcher +and new package pin; no `hermit uninstall` migration is needed. To test a cold +checkout, use a disposable copy with empty `HERMIT_STATE_DIR`, `HOME`, `GOPATH` +and `GOCACHE`, then run `bin/lefthook version` followed by `bin/just hooks` (only +without lhm). Do not clear the machine’s shared cache to perform this check. + +To validate independently, unpack the pinned archive into a disposable directory, +apply `git apply /path/to/lefthook-recovery.patch`, and run with Go 1.27.0: + +```sh +# Use the resolved toolchain binary: a Hermit proxy prepends repo bin/ to PATH, +# which would make testscript invoke the launcher inside its /no-home sandbox. +go_bin="$(go env GOROOT)/bin/go" +"$go_bin" test -cpu 24 -race -count=1 -timeout=30s ./... +"$go_bin" build -trimpath -buildvcs=false -o lefthook . +PATH="$PWD:$PATH" "$go_bin" test -cpu 24 -race -count=1 -timeout=30s -tags=integration integration_test.go +``` + +In buzz-app run `bin/node --test tests/integration/*.test.mjs`, including real +lhm by setting `BUZZ_REAL_LHM` to its executable. No global hooks/configuration +are changed by those fixtures. + +At the next upstream release, check whether it includes all these protections. +`min_version` rejects current stock runners, **not future stock 2.1.18+**. Remove +the override/patch and the temporary Go pin only after an official version passes +the overlap, recovery, GC, legacy-stash and machine-policy regressions. Do not +replace this pin with an unverified newer stock binary. + +## License + +Lefthook is MIT licensed; its full copyright notice and license accompany this +patch in `lefthook-LICENSE`. The downloaded source and built package retain that +license too. diff --git a/bin/packages/lefthook-recovery.patch b/bin/packages/lefthook-recovery.patch new file mode 100644 index 000000000..837a10f90 --- /dev/null +++ b/bin/packages/lefthook-recovery.patch @@ -0,0 +1,866 @@ +diff --git a/CHANGELOG.md b/CHANGELOG.md +index 2e6471f9..873f4d48 100644 +--- a/CHANGELOG.md ++++ b/CHANGELOG.md +@@ -1,5 +1,9 @@ + # Change log + ++## Unreleased ++ ++- fix: isolate partial-staging patches per worktree and remove only the current run's backup. Backups now use `refs/lefthook/backup/`, not `git stash list`; legacy stashes remain untouched. See [recovery instructions](docs/usage.md#recovering-unstaged-changes) for retained backups. ++ + ## 2.1.7 + + - fix: don't hash submodules when checking fail_on_changes ([#1566](https://github.com/evilmartians/lefthook/pull/1566)) by [@tiagovilasboas](https://github.com/tiagovilasboas) +diff --git a/docs/usage.md b/docs/usage.md +index 8a51eaf5..cec66d75 100644 +--- a/docs/usage.md ++++ b/docs/usage.md +@@ -23,3 +23,43 @@ lefthook dump + ```bash + LEFTHOOK=0 git commit + ``` ++ ++## Recovering unstaged changes ++ ++Before running pre-commit jobs on partially staged files, Lefthook saves patches ++in that worktree's Git directory and preserves a stash-shaped backup commit under ++`refs/lefthook/backup/`. The patches are isolated between linked worktrees; ++the backup refs are shared so Git garbage collection from any worktree keeps them. ++Only the current run's backup is removed after successful restoration. Concurrent ++runs in the **same** worktree are not supported. ++ ++If restoration fails, a patch is missing, or a run is interrupted before cleanup, ++its backup ref remains. These backups do **not** appear in `git stash list`. ++Existing user stashes and legacy `lefthook auto backup` stashes are left untouched. ++ ++List and inspect the retained backups: ++ ++```bash ++git for-each-ref --format='%(refname) %(subject)' refs/lefthook/backup/ ++git stash show --patch ++``` ++ ++Preserve any current edits before attempting recovery. In a clean worktree at the ++backup's original base, apply the selected backup (include `--index` to restore ++its staged state too): ++ ++```bash ++git stash apply --index ++``` ++ ++After verifying the recovered content, remove only that backup with a ++compare-and-delete operation: ++ ++```bash ++git update-ref -d ++``` ++ ++Here `` is the full backup ref and `` is its saved commit ID. Retained ++refs are not expired automatically; review them before deleting them. Like other ++non-branch refs, they can be included by `git push --mirror`, so do not mirror ++recovery backups to a remote unintentionally. +diff --git a/internal/git/repo.go b/internal/git/repo.go +index c5d70994..a71882a0 100644 +--- a/internal/git/repo.go ++++ b/internal/git/repo.go +@@ -84,11 +84,11 @@ type Wrapper interface { + // returns wrapper.ErrNoUnstagedDiff when no diff was saved + ApplyUnstagedDiff(bool) error + +- // StoreStash saves the current tree into a stash for backup +- StoreStash() error ++ // StoreStash saves the current tree under an owned backup ref and returns its hash ++ StoreStash() (string, error) + +- // DropStash deletes the created stash +- DropStash() error ++ // DropStash deletes only the backup ref for the supplied hash ++ DropStash(string) error + + // DiscardUnstagedChanges discards the unstaged changes in given files + DiscardUnstagedChanges([]string) error +@@ -244,9 +244,9 @@ func (r *Repo) PartiallyStagedFiles() ([]string, error) { + return partiallyStaged, nil + } + +-func (r *Repo) SaveUnstagedChanges(files []string) error { ++func (r *Repo) SaveUnstagedChanges(files []string) (string, error) { + if err := r.wrapper.SaveUnstagedDiff(files); err != nil { +- return err ++ return "", err + } + + return r.wrapper.StoreStash() +@@ -267,22 +267,23 @@ func (r *Repo) CanRestoreUnstagedChanges() bool { + } + + // RestoreUnstagedChanges applies the patch with previously unstaged changes. +-func (r *Repo) RestoreUnstagedChanges() error { +- return r.restoreUnstagedChanges(false) ++func (r *Repo) RestoreUnstagedChanges(backup string) error { ++ return r.restoreUnstagedChanges(false, backup) + } + + // RestoreAllUnstagedChanges applies all unstaged changes saved before running hooks. +-func (r *Repo) RestoreAllUnstagedChanges() error { +- return r.restoreUnstagedChanges(true) ++func (r *Repo) RestoreAllUnstagedChanges(backup string) error { ++ return r.restoreUnstagedChanges(true, backup) + } + +-func (r *Repo) restoreUnstagedChanges(all bool) error { ++func (r *Repo) restoreUnstagedChanges(all bool, backup string) error { + err := r.wrapper.ApplyUnstagedDiff(all) + if errors.Is(err, wrapper.ErrNoUnstagedDiff) { + // Keep the backup if there was nothing to restore + r.logger.Warn( + "Saved unstaged changes not found. " + +- "Restore them from the 'lefthook auto backup' stash: git stash list", ++ "List recovery refs with git for-each-ref refs/lefthook/backup/ " + ++ "and restore with git stash apply ", + ) + return nil + } +@@ -290,7 +291,7 @@ func (r *Repo) restoreUnstagedChanges(all bool) error { + return err + } + +- if err = r.wrapper.DropStash(); err != nil { ++ if err = r.wrapper.DropStash(backup); err != nil { + return fmt.Errorf("failed to remove unstaged files backup: %w", err) + } + +diff --git a/internal/git/repo_test.go b/internal/git/repo_test.go +index 8cf4289d..52ff59cc 100644 +--- a/internal/git/repo_test.go ++++ b/internal/git/repo_test.go +@@ -211,7 +211,10 @@ func TestRepo_RestoreUnstagedChanges(t *testing.T) { + return tt.applyErr + } + dropped := false +- w.DropStashFunc = func() error { ++ w.DropStashFunc = func(hash string) error { ++ if hash != "backup-hash" { ++ t.Errorf("DropStash hash = %q", hash) ++ } + dropped = true + return tt.dropErr + } +@@ -223,7 +226,7 @@ func TestRepo_RestoreUnstagedChanges(t *testing.T) { + &git.Paths{}, + ) + +- err := repo.RestoreUnstagedChanges() ++ err := repo.RestoreUnstagedChanges("backup-hash") + + if !errors.Is(err, tt.err) { + t.Errorf("repo.RestoreUnstagedChanges() error = %v, want %v", err, tt.err) +@@ -271,7 +274,10 @@ func TestRepo_RestoreAllUnstagedChanges(t *testing.T) { + return tt.applyErr + } + dropped := false +- w.DropStashFunc = func() error { ++ w.DropStashFunc = func(hash string) error { ++ if hash != "backup-hash" { ++ t.Errorf("DropStash hash = %q", hash) ++ } + dropped = true + return tt.dropErr + } +@@ -283,7 +289,7 @@ func TestRepo_RestoreAllUnstagedChanges(t *testing.T) { + &git.Paths{}, + ) + +- err := repo.RestoreAllUnstagedChanges() ++ err := repo.RestoreAllUnstagedChanges("backup-hash") + + if !errors.Is(err, tt.err) { + t.Errorf("repo.RestoreAllUnstagedChanges() error = %v, want %v", err, tt.err) +diff --git a/internal/git/wrapper/apply_unstaged_diff_test.go b/internal/git/wrapper/apply_unstaged_diff_test.go +index 64e0f576..4bd31b7b 100644 +--- a/internal/git/wrapper/apply_unstaged_diff_test.go ++++ b/internal/git/wrapper/apply_unstaged_diff_test.go +@@ -15,8 +15,8 @@ import ( + + func TestWrapper_ApplyUnstagedDiff(t *testing.T) { + errApply := errors.New("apply failed") +- patch := filepath.Join("/repo/.git/info", "lefthook-unstaged.patch") +- patchAll := filepath.Join("/repo/.git/info", "lefthook-unstaged-all.patch") ++ patch := filepath.Join("/repo/.git", "lefthook-unstaged.patch") ++ patchAll := filepath.Join("/repo/.git", "lefthook-unstaged-all.patch") + apply := "git apply -v --whitespace=nowarn --recount --unidiff-zero -- " + + for name, tt := range map[string]struct { +diff --git a/internal/git/wrapper/drop_stash.go b/internal/git/wrapper/drop_stash.go +index 89d0ae68..a9854fba 100644 +--- a/internal/git/wrapper/drop_stash.go ++++ b/internal/git/wrapper/drop_stash.go +@@ -1,41 +1,18 @@ + package wrapper + +-import "regexp" +- +-var ( +- cmdListStash = []string{"git", "stash", "list"} +- reStashMessage = regexp.MustCompile(`^(?P[^ ]+):\s*` + stashMessage) ++import ( ++ "errors" ++ "fmt" + ) + +-func (w *Wrapper) DropStash() error { +- lines, err := w.cmd.cmdLines(cmdListStash) +- if err != nil { +- return err ++// DropStash removes only this run's backup, after its changes have been restored. ++func (w *Wrapper) DropStash(stashHash string) error { ++ if stashHash == "" { ++ return errors.New("cannot remove an unspecified unstaged backup") + } +- +- for i := range lines { +- line := lines[len(lines)-i-1] +- matches := reStashMessage.FindStringSubmatch(line) +- if matches == nil { +- continue +- } +- +- stashID := reStashMessage.SubexpIndex("stash") +- +- if len(matches[stashID]) > 0 { +- _, err := w.cmd.cmd([]string{ +- "git", +- "stash", +- "drop", +- "--quiet", +- "--", +- matches[stashID], +- }) +- if err != nil { +- return err +- } +- } ++ _, err := w.cmd.cmd([]string{"git", "update-ref", "-d", backupRefPrefix + stashHash, stashHash}) ++ if err != nil { ++ return fmt.Errorf("failed to remove unstaged backup: %w", err) + } +- + return nil + } +diff --git a/internal/git/wrapper/drop_stash_test.go b/internal/git/wrapper/drop_stash_test.go +index e25c2d56..7fc34138 100644 +--- a/internal/git/wrapper/drop_stash_test.go ++++ b/internal/git/wrapper/drop_stash_test.go +@@ -12,52 +12,23 @@ import ( + ) + + func TestWrapper_DropStash(t *testing.T) { +- fs := afero.NewMemMapFs() +- errList := errors.New("list failed") + errDrop := errors.New("drop failed") +- list := "stash@{0}: lefthook auto backup\n" + +- "stash@{1}: On main: other\n" + +- "stash@{2}: lefthook auto backup\n" +- +- for name, tt := range map[string]struct { +- outs []cmdtest.Out +- err error +- }{ +- "drops-from-oldest": { +- outs: []cmdtest.Out{ +- {Command: "git stash list", Output: list}, +- {Command: "git stash drop --quiet -- stash@{2}"}, +- {Command: "git stash drop --quiet -- stash@{0}"}, +- }, +- }, +- "no-lefthook-stash": { +- outs: []cmdtest.Out{ +- {Command: "git stash list", Output: "stash@{0}: On main: other\n"}, +- }, +- }, +- "list-fails": { +- outs: []cmdtest.Out{ +- {Command: "git stash list", Err: errList}, +- }, +- err: errList, +- }, +- "drop-fails": { +- outs: []cmdtest.Out{ +- {Command: "git stash list", Output: list}, +- {Command: "git stash drop --quiet -- stash@{2}", Err: errDrop}, +- }, +- err: errDrop, +- }, ++ for name, tt := range map[string]struct{ err error }{ ++ "drops-owned-ref": {}, ++ "drop-fails": {err: errDrop}, + } { + t.Run(name, func(t *testing.T) { +- cmd := cmdtest.NewFakeCmd(t, tt.outs) +- w := wrapper.New(fs, cmd, loggertest.New()) +- +- err := w.DropStash() +- +- if !errors.Is(err, tt.err) { +- t.Errorf("wrapper.DropStash() error = %v, want %v", err, tt.err) ++ cmd := cmdtest.NewFakeCmd(t, []cmdtest.Out{{Command: "git update-ref -d refs/lefthook/backup/abc123 abc123", Err: tt.err}}) ++ w := wrapper.New(afero.NewMemMapFs(), cmd, loggertest.New()) ++ if err := w.DropStash("abc123"); !errors.Is(err, tt.err) { ++ t.Errorf("DropStash error = %v, want %v", err, tt.err) + } + }) + } ++ t.Run("refuses-empty-owner", func(t *testing.T) { ++ w := wrapper.New(afero.NewMemMapFs(), cmdtest.NewFakeCmd(t, nil), loggertest.New()) ++ if err := w.DropStash(""); err == nil { ++ t.Error("DropStash without an owner must fail") ++ } ++ }) + } +diff --git a/internal/git/wrapper/save_unstaged_diff.go b/internal/git/wrapper/save_unstaged_diff.go +index f2f20cea..f0fe8a7f 100644 +--- a/internal/git/wrapper/save_unstaged_diff.go ++++ b/internal/git/wrapper/save_unstaged_diff.go +@@ -3,6 +3,8 @@ package wrapper + import "fmt" + + func (w *Wrapper) SaveUnstagedDiff(files []string) error { ++ // Keep --output and its path in one argument so Git does not mistake a linked ++ // worktree's external Git directory for a path in an implicit --no-index diff. + _, err := w.cmd.batchedCmd( + []string{ + "git", +@@ -15,8 +17,7 @@ func (w *Wrapper) SaveUnstagedDiff(files []string) error { + "--dst-prefix=b/", // force prefix for consistent behavior + "--patch", // output a patch that can be applied + "--submodule=short", // always use the default short format for submodules +- "--output", +- w.unstagedDiffPath(), ++ "--output=" + w.unstagedDiffPath(), + "--", + }, files) + if err != nil { +@@ -34,8 +35,7 @@ func (w *Wrapper) SaveUnstagedDiff(files []string) error { + "--dst-prefix=b/", + "--patch", + "--submodule=short", +- "--output", +- w.unstagedAllDiffPath(), ++ "--output=" + w.unstagedAllDiffPath(), + "--", + }) + if err != nil { +diff --git a/internal/git/wrapper/save_unstaged_diff_test.go b/internal/git/wrapper/save_unstaged_diff_test.go +index 0bcba097..a2a4efc4 100644 +--- a/internal/git/wrapper/save_unstaged_diff_test.go ++++ b/internal/git/wrapper/save_unstaged_diff_test.go +@@ -15,9 +15,9 @@ import ( + func TestWrapper_SaveUnstagedDiff(t *testing.T) { + fs := afero.NewMemMapFs() + errDiff := errors.New("diff failed") +- diffArgs := "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " +- diffFiles := diffArgs + filepath.Join("/repo/.git/info", "lefthook-unstaged.patch") + " -- a b" +- diffAll := diffArgs + filepath.Join("/repo/.git/info", "lefthook-unstaged-all.patch") + " --" ++ diffArgs := "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" ++ diffFiles := diffArgs + filepath.Join("/repo/.git", "lefthook-unstaged.patch") + " -- a b" ++ diffAll := diffArgs + filepath.Join("/repo/.git", "lefthook-unstaged-all.patch") + " --" + + for name, tt := range map[string]struct { + outs []cmdtest.Out +diff --git a/internal/git/wrapper/store_stash.go b/internal/git/wrapper/store_stash.go +index e99d4fd8..4d33cc9f 100644 +--- a/internal/git/wrapper/store_stash.go ++++ b/internal/git/wrapper/store_stash.go +@@ -1,23 +1,28 @@ + package wrapper + +-const stashMessage = "lefthook auto backup" ++import ( ++ "errors" ++ "fmt" ++) ++ ++// Shared refs keep backups reachable during garbage collection from any worktree. ++const backupRefPrefix = "refs/lefthook/backup/" + + var cmdCreateStash = []string{"git", "stash", "create"} + +-func (w *Wrapper) StoreStash() error { ++// StoreStash preserves the stash-shaped commit without modifying the shared stash list. ++func (w *Wrapper) StoreStash() (string, error) { + stashHash, err := w.cmd.cmd(cmdCreateStash) + if err != nil { +- return err ++ return "", fmt.Errorf("failed to create unstaged backup: %w", err) + } +- +- _, err = w.cmd.cmd([]string{ +- "git", +- "stash", +- "store", +- "--quiet", +- "--message", +- stashMessage, +- stashHash, +- }) +- return err ++ if stashHash == "" { ++ return "", errors.New("cannot save an empty unstaged backup") ++ } ++ // Create only: never take ownership of a backup from an earlier run. ++ _, err = w.cmd.cmd([]string{"git", "update-ref", backupRefPrefix + stashHash, stashHash, ""}) ++ if err != nil { ++ return "", fmt.Errorf("failed to preserve unstaged backup: %w", err) ++ } ++ return stashHash, nil + } +diff --git a/internal/git/wrapper/store_stash_test.go b/internal/git/wrapper/store_stash_test.go +index 5e4a0d5b..de8c6066 100644 +--- a/internal/git/wrapper/store_stash_test.go ++++ b/internal/git/wrapper/store_stash_test.go +@@ -19,11 +19,13 @@ func TestWrapper_StoreStash(t *testing.T) { + for name, tt := range map[string]struct { + outs []cmdtest.Out + err error ++ hash string + }{ + "stores-created-stash": { ++ hash: "abc123", + outs: []cmdtest.Out{ + {Command: "git stash create", Output: "abc123\n"}, +- {Command: "git stash store --quiet --message lefthook auto backup abc123"}, ++ {Command: "git update-ref refs/lefthook/backup/abc123 abc123 "}, + }, + }, + "create-fails": { +@@ -35,7 +37,7 @@ func TestWrapper_StoreStash(t *testing.T) { + "store-fails": { + outs: []cmdtest.Out{ + {Command: "git stash create", Output: "abc123"}, +- {Command: "git stash store --quiet --message lefthook auto backup abc123", Err: errStore}, ++ {Command: "git update-ref refs/lefthook/backup/abc123 abc123 ", Err: errStore}, + }, + err: errStore, + }, +@@ -44,7 +46,10 @@ func TestWrapper_StoreStash(t *testing.T) { + cmd := cmdtest.NewFakeCmd(t, tt.outs) + w := wrapper.New(fs, cmd, loggertest.New()) + +- err := w.StoreStash() ++ hash, err := w.StoreStash() ++ if hash != tt.hash { ++ t.Errorf("StoreStash hash = %q, want %q", hash, tt.hash) ++ } + + if !errors.Is(err, tt.err) { + t.Errorf("wrapper.StoreStash() error = %v, want %v", err, tt.err) +@@ -52,3 +57,11 @@ func TestWrapper_StoreStash(t *testing.T) { + }) + } + } ++ ++func TestWrapper_StoreStash_emptyBackup(t *testing.T) { ++ cmd := cmdtest.NewFakeCmd(t, []cmdtest.Out{{Command: "git stash create"}}) ++ w := wrapper.New(afero.NewMemMapFs(), cmd, loggertest.New()) ++ if hash, err := w.StoreStash(); err == nil || hash != "" { ++ t.Errorf("StoreStash() = %q, %v; want empty hash and an error", hash, err) ++ } ++} +diff --git a/internal/git/wrapper/unstaged_diff_applicable_test.go b/internal/git/wrapper/unstaged_diff_applicable_test.go +index 0129fe12..eca44dac 100644 +--- a/internal/git/wrapper/unstaged_diff_applicable_test.go ++++ b/internal/git/wrapper/unstaged_diff_applicable_test.go +@@ -13,7 +13,7 @@ import ( + ) + + func TestWrapper_UnstagedDiffApplicable(t *testing.T) { +- patch := filepath.Join("/repo/.git/info", "lefthook-unstaged.patch") ++ patch := filepath.Join("/repo/.git", "lefthook-unstaged.patch") + check := "git apply -v --whitespace=nowarn --recount --unidiff-zero --check -- " + patch + + for name, tt := range map[string]struct { +diff --git a/internal/git/wrapper/wrapper.go b/internal/git/wrapper/wrapper.go +index f66bc322..de9b7b03 100644 +--- a/internal/git/wrapper/wrapper.go ++++ b/internal/git/wrapper/wrapper.go +@@ -122,9 +122,9 @@ func (w *Wrapper) readOriginHead() string { + } + + func (w *Wrapper) unstagedDiffPath() string { +- return filepath.Join(w.infoPath, unstagedPatchName) ++ return filepath.Join(w.gitPath, unstagedPatchName) + } + + func (w *Wrapper) unstagedAllDiffPath() string { +- return filepath.Join(w.infoPath, unstagedAllPatchName) ++ return filepath.Join(w.gitPath, unstagedAllPatchName) + } +diff --git a/internal/run/controller/guard.go b/internal/run/controller/guard.go +index e5f5a8aa..3814ed69 100644 +--- a/internal/run/controller/guard.go ++++ b/internal/run/controller/guard.go +@@ -89,9 +89,10 @@ func (g *guard) withHiddenUnstagedChanges(fn func() error) error { + + g.logger.Debug("[lefthook] saving partially staged files") + +- if err := g.git.SaveUnstagedChanges(partiallyStagedFiles); err != nil { +- g.logger.Warnf("Failed to save unstaged changes: %s\n", err) +- return err ++ backup, saveErr := g.git.SaveUnstagedChanges(partiallyStagedFiles) ++ if saveErr != nil { ++ g.logger.Warnf("Failed to save unstaged changes: %s\n", saveErr) ++ return saveErr + } + + logger.NewBuilder(g.logger). +@@ -142,9 +143,9 @@ func (g *guard) withHiddenUnstagedChanges(fn func() error) error { + + var restoreErr error + if restoreAllUnstagedChanges { +- restoreErr = g.git.RestoreAllUnstagedChanges() ++ restoreErr = g.git.RestoreAllUnstagedChanges(backup) + } else { +- restoreErr = g.git.RestoreUnstagedChanges() ++ restoreErr = g.git.RestoreUnstagedChanges(backup) + } + if restoreErr != nil { + g.logger.Warnf("Failed to restore unstaged files: %s", restoreErr) +diff --git a/internal/run/controller/guard_test.go b/internal/run/controller/guard_test.go +index 22d04efa..ca2023dc 100644 +--- a/internal/run/controller/guard_test.go ++++ b/internal/run/controller/guard_test.go +@@ -86,17 +86,16 @@ func Test_guard_wrap(t *testing.T) { + failOnChanges: false, + commands: []cmdtest.Out{ + {Command: "git status --short --porcelain -z", Output: "AM file1\x00 M file2\x00 A file3\x00"}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged.patch") + + " -- file1", Output: ""}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged-all.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged-all.patch") + + " --", Output: ""}, + {Command: "git stash create", Output: ""}, +- {Command: "git stash store --quiet --message lefthook auto backup ", Output: ""}, ++ {Command: "git update-ref refs/lefthook/backup/ ", Output: ""}, + {Command: "git checkout --force -- file1", Output: ""}, +- {Command: "git stash list", Output: "0: my stash\n1: lefthook auto backup\n2: my second stash\n"}, +- {Command: "git stash drop --quiet -- 1", Output: ""}, ++ {Command: "git update-ref -d refs/lefthook/backup/ "}, + }, + }, + "stashUnstagedChanges=true failOnChanges=true with partially staged no hook changes": { +@@ -104,20 +103,19 @@ func Test_guard_wrap(t *testing.T) { + failOnChanges: true, + commands: []cmdtest.Out{ + {Command: "git status --short --porcelain -z", Output: "AM file1\x00 M file2\x00"}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged.patch") + + " -- file1", Output: ""}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged-all.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged-all.patch") + + " --", Output: ""}, + {Command: "git stash create", Output: ""}, +- {Command: "git stash store --quiet --message lefthook auto backup ", Output: ""}, ++ {Command: "git update-ref refs/lefthook/backup/ ", Output: ""}, + {Command: "git checkout --force -- file1", Output: ""}, + {Command: "git status --short --porcelain -z", Output: "A file1\x00"}, + // job run + {Command: "git status --short --porcelain -z", Output: "A file1\x00"}, +- {Command: "git stash list", Output: "0: my stash\n1: lefthook auto backup\n2: my second stash\n"}, +- {Command: "git stash drop --quiet -- 1", Output: ""}, ++ {Command: "git update-ref -d refs/lefthook/backup/ "}, + }, + }, + "stashUnstagedChanges=true failOnChanges=true with partially staged and hook changes with diff": { +@@ -126,14 +124,14 @@ func Test_guard_wrap(t *testing.T) { + failOnChangesDiff: true, + commands: []cmdtest.Out{ + {Command: "git status --short --porcelain -z", Output: "AM file1\x00"}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged.patch") + + " -- file1", Output: ""}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged-all.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged-all.patch") + + " --", Output: ""}, + {Command: "git stash create", Output: ""}, +- {Command: "git stash store --quiet --message lefthook auto backup ", Output: ""}, ++ {Command: "git update-ref refs/lefthook/backup/ ", Output: ""}, + {Command: "git checkout --force -- file1", Output: ""}, + {Command: "git status --short --porcelain -z", Output: "A file1\x00"}, + {Command: "git hash-object -- file1", Output: "hash1\n"}, +@@ -142,8 +140,7 @@ func Test_guard_wrap(t *testing.T) { + {Command: "git hash-object -- file1", Output: "hash2\n"}, + {Command: "git diff -- file1", Output: "diff --git a/file1 b/file1\n..."}, + {Command: "git checkout --force -- file1", Output: ""}, +- {Command: "git stash list", Output: "0: my stash\n1: lefthook auto backup\n2: my second stash\n"}, +- {Command: "git stash drop --quiet -- 1", Output: ""}, ++ {Command: "git update-ref -d refs/lefthook/backup/ "}, + }, + err: &FailOnChangesError{[]string{"file2"}}, + }, +@@ -261,17 +258,17 @@ func Test_guard_wrap_stageFixed(t *testing.T) { + filesToStage: []string{"file2"}, + commands: []cmdtest.Out{ + {Command: "git status --short --porcelain -z", Output: "AM file1\x00"}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged.patch") + + " -- file1", Output: ""}, +- {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output " + +- filepath.Join("root", ".git", "info", "lefthook-unstaged-all.patch") + ++ {Command: "git diff --binary --unified=0 --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --patch --submodule=short --output=" + ++ filepath.Join("root", ".git", "lefthook-unstaged-all.patch") + + " --", Output: ""}, + {Command: "git stash create", Output: ""}, +- {Command: "git stash store --quiet --message lefthook auto backup ", Output: ""}, ++ {Command: "git update-ref refs/lefthook/backup/ ", Output: ""}, + {Command: "git checkout --force -- file1", Output: ""}, + {Command: "git add --force -- file2", Err: errStaging}, +- {Command: "git stash list"}, ++ {Command: "git update-ref -d refs/lefthook/backup/ "}, + }, + err: errStaging, + }, +diff --git a/tests/helpers/gittest/wrapper.go b/tests/helpers/gittest/wrapper.go +index 6a7c505c..a07c9080 100644 +--- a/tests/helpers/gittest/wrapper.go ++++ b/tests/helpers/gittest/wrapper.go +@@ -19,8 +19,8 @@ type StubWrapper struct { + SaveUnstagedDiffFunc func([]string) error + UnstagedDiffApplicableFunc func() bool + ApplyUnstagedDiffFunc func(bool) error +- StoreStashFunc func() error +- DropStashFunc func() error ++ StoreStashFunc func() (string, error) ++ DropStashFunc func(string) error + DiscardUnstagedChangesFunc func([]string) error + DiscardAllUnstagedChangesFunc func() error + StageFilesFunc func([]string) error +@@ -57,8 +57,8 @@ func (w *StubWrapper) SaveUnstagedDiff(s []string) error { return w.Sav + func (w *StubWrapper) UnstagedDiffApplicable() bool { return w.UnstagedDiffApplicableFunc() } + + func (w *StubWrapper) ApplyUnstagedDiff(b bool) error { return w.ApplyUnstagedDiffFunc(b) } +-func (w *StubWrapper) StoreStash() error { return w.StoreStashFunc() } +-func (w *StubWrapper) DropStash() error { return w.DropStashFunc() } ++func (w *StubWrapper) StoreStash() (string, error) { return w.StoreStashFunc() } ++func (w *StubWrapper) DropStash(hash string) error { return w.DropStashFunc(hash) } + func (w *StubWrapper) DiscardUnstagedChanges(s []string) error { + return w.DiscardUnstagedChangesFunc(s) + } +diff --git a/tests/integration/restore_unstaged_on_conflict.txt b/tests/integration/restore_unstaged_on_conflict.txt +index c0373a02..289b741e 100644 +--- a/tests/integration/restore_unstaged_on_conflict.txt ++++ b/tests/integration/restore_unstaged_on_conflict.txt +@@ -30,10 +30,12 @@ grep lineUnstaged a.txt + ! grep line42 a.txt + grep lineCommitted b.txt + grep 'unrelated unstaged change' c.txt +-! exists .git/info/lefthook-unstaged.patch +-! exists .git/info/lefthook-unstaged-all.patch ++! exists .git/lefthook-unstaged.patch ++! exists .git/lefthook-unstaged-all.patch + exec git stash list + ! stdout 'lefthook auto backup' ++exec git for-each-ref refs/lefthook/backup/ ++! stdout . + + -- lefthook.yml -- + pre-commit: +diff --git a/tests/integration/run_interrupt.txt b/tests/integration/run_interrupt.txt +index 2716c53d..12335f36 100644 +--- a/tests/integration/run_interrupt.txt ++++ b/tests/integration/run_interrupt.txt +@@ -19,6 +19,8 @@ stderr 'Error: Interrupted' + grep unstaged newfile.txt + exec git stash list + ! stdout 'lefthook auto backup' ++exec git for-each-ref refs/lefthook/backup/ ++! stdout . + + -- lefthook.yml -- + pre-commit: +@@ -29,6 +31,9 @@ pre-commit: + -- hook.sh -- + #!/usr/bin/env bash + ++if [ -n "${INTERRUPT_READY:-}" ]; then ++ printf 'ready\n' > "$INTERRUPT_READY" ++fi + sleep 2 + >&2 echo hook-done + +@@ -46,9 +51,13 @@ echo unstaged >> newfile.txt + # so we first need to emulate being a terminal and enable + # job monitoring so that new PGIDs are assigned. + set -m ++export INTERRUPT_READY="$PWD/interrupt.ready" ++mkfifo "$INTERRUPT_READY" + nohup git commit -m test & + pgid=$! +-sleep 1 ++# Wait for the job to start, after the unstaged-change guard has finished saving. ++read -r ready < "$INTERRUPT_READY" ++rm "$INTERRUPT_READY" + kill -SIGINT -$pgid + wait + >&2 echo 'script-done' +diff --git a/tests/integration/worktree_recovery.txt b/tests/integration/worktree_recovery.txt +new file mode 100644 +index 00000000..569ed3b9 +--- /dev/null ++++ b/tests/integration/worktree_recovery.txt +@@ -0,0 +1,129 @@ ++[windows] skip ++ ++# FIFO gates hold both hooks after backup creation, without timing assumptions. ++chmod 0700 exercise.sh ++chmod 0700 hook.sh ++exec sh exercise.sh ++stdout 'worktree recovery passed' ++ ++-- lefthook.yml -- ++pre-commit: ++ jobs: ++ - run: sh hook.sh ++ ++-- hook.sh -- ++#!/bin/sh ++set -eu ++if [ -n "${GATE:-}" ]; then ++ printf 'ready\n' > "$GATE.ready" ++ read -r release < "$GATE.release" ++fi ++ ++-- exercise.sh -- ++#!/bin/sh ++set -eu ++export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 ++unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE ++root="$PWD" ++a= b= ++trap 'status=$?; if [ "$status" -ne 0 ]; then cat "$root/main.log" "$root/linked.log" "$root/linked/failed.log" 2>/dev/null || :; fi; [ -z "$a" ] || kill "$a" 2>/dev/null || :; [ -z "$b" ] || kill "$b" 2>/dev/null || :; exit "$status"' EXIT ++git init -q -b main ++git config user.name Fixture ++git config user.email fixture@example.test ++printf 'base\n' > file.txt ++git add file.txt hook.sh lefthook.yml ++git commit -qm initial ++git worktree add -q -b linked linked ++linked="$root/linked" ++# Existing user and legacy backups must remain exactly as they are. ++printf 'user backup\n' >> file.txt ++legacy=$(git stash create) ++git stash store -m 'lefthook auto backup' "$legacy" ++git restore file.txt ++git stash list --format='%H %gs' > expected-stashes ++git update-ref "refs/lefthook/backup/$legacy" "$legacy" '' ++for wt in "$root" "$linked"; do ++ printf 'base\nstaged\n' > "$wt/file.txt" ++ git -C "$wt" add file.txt ++ printf '%s\n' "unstaged-$wt" >> "$wt/file.txt" ++done ++lefthook install ++mkfifo "$root/main.ready" "$root/main.release" "$root/linked.ready" "$root/linked.release" ++GATE="$root/main" git commit -qm main-change > main.log 2>&1 & ++a=$! ++read -r ready < "$root/main.ready" ++main_ref= ++for ref in $(git for-each-ref --format='%(refname)' refs/lefthook/backup/); do ++ if [ "$ref" != "refs/lefthook/backup/$legacy" ]; then ++ test -z "$main_ref" ++ main_ref=$ref ++ fi ++done ++test -n "$main_ref" ++(cd "$linked" && GATE="$root/linked" git commit -qm linked-change) > linked.log 2>&1 & ++b=$! ++read -r ready < "$root/linked.ready" ++# Collecting from main must retain another worktree's live backup. ++linked_ref= ++for ref in $(git for-each-ref --format='%(refname)' refs/lefthook/backup/); do ++ if [ "$ref" != "$main_ref" ] && [ "$ref" != "refs/lefthook/backup/$legacy" ]; then ++ test -z "$linked_ref" ++ linked_ref=$ref ++ fi ++done ++test -n "$linked_ref" ++git gc --prune=now --quiet ++git -C "$linked" cat-file -e "$linked_ref^{commit}" ++printf 'release\n' > "$root/main.release" ++wait "$a" || { cat main.log; exit 1; } ++a= ++# Main's cleanup must not delete the linked worktree's still-live backup. ++git -C "$linked" cat-file -e "$linked_ref^{commit}" ++printf 'release\n' > "$root/linked.release" ++wait "$b" || { cat linked.log; exit 1; } ++b= ++for wt in "$root" "$linked"; do ++ test "$(git -C "$wt" show HEAD:file.txt)" = "$(printf 'base\nstaged')" ++ test "$(cat "$wt/file.txt")" = "$(printf 'base\nstaged\n%s' "unstaged-$wt")" ++done ++git stash list --format='%H %gs' > actual-stashes ++cmp expected-stashes actual-stashes ++test "$(git for-each-ref --format='%(refname)' refs/lefthook/backup/)" = "refs/lefthook/backup/$legacy" ++ ++# A restoration failure must keep a recoverable owned ref, even after main gc. ++cd "$linked" ++printf 'base\nnext-staged\n' > file.txt ++git add file.txt ++printf 'precious\n' >> file.txt ++cp file.txt expected-file ++cat > hook.sh <<'HOOK' ++#!/bin/sh ++# Make both saved patches invalid so the restoration itself fails. ++dir=$(git rev-parse --absolute-git-dir) ++printf 'invalid patch\n' > "$dir/lefthook-unstaged.patch" ++printf 'invalid patch\n' > "$dir/lefthook-unstaged-all.patch" ++HOOK ++if git commit -qm must-fail > failed.log 2>&1; then ++ cat failed.log; exit 1 ++fi ++backup= ++for ref in $(git for-each-ref --format='%(refname)' refs/lefthook/backup/); do ++ if [ "$ref" != "refs/lefthook/backup/$legacy" ]; then ++ test -z "$backup" ++ backup=$ref ++ fi ++done ++test -n "$backup" ++git -C "$root" gc --prune=now --quiet ++git cat-file -e "$backup^{commit}" ++# The backup is stash-shaped and restores the original index and worktree. ++git restore --staged --worktree file.txt ++git restore hook.sh ++git stash apply --index "$backup" ++cmp expected-file file.txt ++# Only the owner ref is removed, never the legacy stash or another ref. ++hash=$(git rev-parse "$backup") ++git update-ref -d "$backup" "$hash" ++git stash list --format='%H %gs' > actual-stashes ++cmp "$root/expected-stashes" actual-stashes ++printf 'worktree recovery passed\n' +--- a/internal/version/version.go ++++ b/internal/version/version.go +@@ -8,7 +8,7 @@ + "golang.org/x/mod/semver" + ) + +-const version = "2.1.17" ++const version = "2.1.18-buzz.3" + + var ( + // Is set via -X github.com/evilmartians/lefthook/v2/internal/version.commit={commit}. diff --git a/bin/packages/lefthook.hcl b/bin/packages/lefthook.hcl new file mode 100644 index 000000000..3197ca087 --- /dev/null +++ b/bin/packages/lefthook.hcl @@ -0,0 +1,24 @@ +// Temporary worktree-recovery fix; see lefthook-recovery.md before changing this pin. +description = "Lefthook with isolated worktree recovery" +repository = "https://github.com/evilmartians/lefthook" +binaries = ["lefthook-runner"] +requires = ["go-1.27.0"] +strip = 1 +source = "https://codeload.github.com/evilmartians/lefthook/tar.gz/cc9969c2e2198aafff8accc63ee1aee30e9a2aa4" +sha256 = "f8e6e7d01e77d0814a00d0cfadd9db08f05ce526252c8b388201c02fffeb55db" +version "2.1.18-buzz.3" {} + +on "unpack" { + copy { from = "lefthook-recovery.patch" to = "${root}/recovery.patch" } + run { + cmd = "/bin/sh" + args = ["-s"] + stdin = "set -eu; unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR; export GIT_CEILING_DIRECTORIES=\"$(dirname \"$PWD\")\"; git apply recovery.patch" + } + // bin/lefthook provisions Go before entering Hermit's unpack lock. + run { + cmd = "/bin/sh" + args = ["-s", "--", "${root}/../go-1.27.0/bin/go"] + stdin = "export GOTOOLCHAIN=local CGO_ENABLED=0; unset GOROOT; exec \"$1\" build -trimpath -buildvcs=false -o lefthook-runner ." + } +} diff --git a/docs/contributing.md b/docs/contributing.md index 47a0e6b97..f10610b7e 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1,10 +1,14 @@ # Contribution workflow -The repository pins just 1.58.0, Node.js 24.18.0, pnpm 11.8.0, Lefthook 2.1.16, +The repository pins just 1.58.0, Node.js 24.18.0, pnpm 11.8.0, Lefthook 2.1.18-buzz.3, and Rust 1.98.1 (including Cargo, rustfmt, and Clippy) with [Hermit](https://cashapp.github.io/hermit/). No global tool installation is required: `bin/hermit` bootstraps Hermit and tools -are downloaded on first use. Desktop development still requires the +are downloaded on first use. The temporary Lefthook package builds once from +checksum-pinned upstream source plus our worktree-recovery patch using pinned +Go 1.27.0; this first use needs network access for Go modules and takes longer. +The small `bin/lefthook` launcher provisions Go before entering Hermit’s package +unpack lock, then executes the pinned `bin/lefthook-runner`. Desktop development still requires the [Tauri platform prerequisites](https://v2.tauri.app/start/prerequisites/). From the repository root: @@ -273,6 +277,10 @@ lhm, install once per clone (linked worktrees share the installed hooks): just hooks # or bin/just hooks without Hermit activation ``` +Existing standalone clones must rerun `just hooks` once after pulling this change +to replace old shims; their old stock runner cannot self-update past the version +gate. Do this in every clone, not every linked worktree. + This recipe runs the pinned `bin/lefthook install`; it never changes Git config. If installation refuses because of a global `core.hooksPath`, **do not follow Lefthook's suggested fixes**: `--reset-hooks-path` and @@ -296,9 +304,24 @@ setting is unchanged, but its hooks no longer run in this clone. Do not use this override for lhm. The installed hooks fail rather than silently skip when they cannot find -Lefthook; `bin/lefthook uninstall` removes them. Lefthook 2.1.16 or newer is -required: `min_version` rejects older runners, including an older `lefthook` on -`PATH` under lhm (`brew upgrade lefthook`). +Lefthook; `bin/lefthook uninstall` removes them. The repository pins a temporary +patched runner, `2.1.18-buzz.3`, to fix concurrent linked-worktree recovery. +Standalone shims select `bin/lefthook`. **Under lhm, activate Hermit before +committing or pushing** (`source bin/activate-hermit`), since lhm selects +`lefthook` from `PATH` and does not read the repo's `lefthook:` setting. A +nonactivated GUI/agent shell using stock 2.1.17 or older is rejected by +`min_version` before any unstaged edits are hidden. **If no Lefthook exists on +PATH, lhm 0.14.1 can silently skip hooks instead** (especially in linked worktrees); +the repository config cannot prevent that fallback. Use Git from an activated +shell, or a client that actually inherits that shell’s environment, and verify +`command -v lefthook` resolves to this checkout’s `bin/lefthook`. Do not bypass +hooks or change machine policy. Upgrading Homebrew's stock runner is not this fix. + +This version gate is temporary, not a capability check: future stock 2.1.18+ +would pass it. Reassess the gate at the next upstream release and adopt only a +release verified to preserve the recovery guarantees. Source, license, +revalidation commands, and removal criteria are in +[`bin/packages/lefthook-recovery.md`](../bin/packages/lefthook-recovery.md). Pre-commit runs these jobs in order and stops at the first failure: refuse staged names containing `*`, `?`, `[` or `\`, which Git would expand as globs when @@ -314,7 +337,15 @@ Both reject remaining Biome warnings. Lefthook owns partial staging: it hides the unstaged hunks of partially staged files while the jobs run, restages the formatted files, then restores the hunks, -so unstaged hunks never enter the commit. When a formatter changes the same lines +so unstaged hunks never enter the commit. The patched runner isolates patches +per worktree and uses owned `refs/lefthook/backup/` recovery refs instead +of modifying the shared stash list. A failed restore retains the backup: +`git for-each-ref refs/lefthook/backup/` lists them; preserve current edits, then +use `git stash apply --index ` in a clean worktree at the original base. +After verifying recovery, delete only that ref with +`git update-ref -d `. Legacy stashes remain untouched; recovery refs +are not automatically expired and can be included by `git push --mirror`. +When a formatter changes the same lines as an unstaged hunk, the commit is blocked with "conflict while merging unstaged changes" and the index, the files and unrelated edits are left as they were; format the file first (`just iterate` or the editor), then reselect your hunks. diff --git a/lefthook.yml b/lefthook.yml index c671ff2c5..f48fb7c5b 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,4 +1,8 @@ -min_version: 2.1.16 +# Stock runners must stop before their unsafe partial-staging guard runs. +# Under lhm, activate Hermit (source bin/activate-hermit) before committing. +# Temporary gate: reassess before adopting any new upstream release. +min_version: 2.1.18-buzz.3 +lefthook: bin/lefthook # Without lhm, `bin/lefthook install` writes shims that fail instead of silently # skipping when they cannot find Lefthook. lhm users install nothing: lhm merges # this file with the machine policy at hook time. diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index f703074b8..32c25002d 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { chmodSync, cpSync, @@ -11,6 +11,7 @@ import { symlinkSync, writeFileSync, } from "node:fs"; +import { createServer } from "node:net"; import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -26,9 +27,8 @@ env.pnpm_config_verify_deps_before_run = "false"; // These commits are disposable probe fixtures, never commits in the source checkout. env.GIT_CONFIG_NOSYSTEM = "1"; env.GIT_CONFIG_GLOBAL = "/dev/null"; -// The installed shim prefers a Lefthook on PATH; pin the Hermit binary so a -// machine-wide installation cannot change which version the fixture exercises. -env.LEFTHOOK_BIN = path.join(root, "bin/lefthook"); +// Exercise production runner selection, not a fixture-only binary override. +delete env.LEFTHOOK_BIN; // Replaces the push lanes to capture the exact bytes each one receives. const recordPushInput = `import { readFileSync, writeFileSync } from "node:fs"; writeFileSync((process.argv[2] ?? "unit") + "-input", readFileSync(0)); @@ -440,7 +440,7 @@ function pushFixture(t, changes) { // a real cargo build, exactly like the fake Vitest below. rmSync(path.join(f.dir, "bin")); mkdirSync(path.join(f.dir, "bin")); - for (const tool of ["node", "pnpm"]) { + for (const tool of ["node", "pnpm", "lefthook", "lefthook-runner", "go"]) { symlinkSync( path.join(root, `bin/${tool}`), path.join(f.dir, `bin/${tool}`), @@ -807,14 +807,8 @@ test("the design lane disables dependency auto-repair even when inherited as tru ); }); -test("real lhm composes isolated system commands with the repository jobs", { - skip: !process.env.BUZZ_REAL_LHM, -}, (t) => { +function lhmFixture(t) { const f = fixture(t, { install: false }); - const sibling = path.join(f.dir, "sibling"); - f.git("worktree", "add", "-q", "--detach", sibling); - for (const link of ["bin", "node_modules"]) - symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); const upstream = path.join(f.dir, "upstream ' $ hooks"); for (const name of ["pre-commit", "pre-push"]) { f.write( @@ -843,9 +837,37 @@ pre-push: GIT_CONFIG_GLOBAL: path.join(f.dir, "global-config"), LHM_SYSTEM_CONFIG: path.join(f.dir, "system"), LHM_USER_CONFIG: path.join(f.dir, "absent-user.yml"), - // lhm runs whichever `lefthook` is on PATH; use the pinned one. - PATH: `${path.join(root, "bin")}${path.delimiter}${env.PATH}`, }; + // Exercise the documented activation command, rather than manufacturing PATH. + const activated = f.run( + "/bin/bash", + ["-c", 'eval "$(./bin/hermit env --activate)" && env -0'], + isolated, + ); + assert.equal(activated.status, 0, activated.stdout + activated.stderr); + for (const entry of activated.stdout.split("\0").filter(Boolean)) { + const equals = entry.indexOf("="); + isolated[entry.slice(0, equals)] = entry.slice(equals + 1); + } + const selected = f.run( + "/bin/sh", + ["-c", "command -v lefthook; lefthook version"], + isolated, + ); + assert.equal(selected.status, 0, selected.stdout + selected.stderr); + assert.equal(selected.stdout, `${root}bin/lefthook\n2.1.18-buzz.3\n`); + return { ...f, isolated }; +} + +test("real lhm composes isolated system commands with the repository jobs", { + skip: !process.env.BUZZ_REAL_LHM, +}, (t) => { + const f = lhmFixture(t); + const { isolated } = f; + const sibling = path.join(f.dir, "sibling"); + f.git("worktree", "add", "-q", "--detach", sibling); + for (const link of ["bin", "node_modules"]) + symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); const before = f.read("global-config"); const refused = f.run(path.join(root, "bin/just"), ["hooks"], isolated); assert.notEqual(refused.status, 0); @@ -883,3 +905,217 @@ pre-push: assert.equal(f.read(`${lane}-input`), f.read("real-lhm-input")); assert.match(f.read("real-lhm-input"), /refs\/heads\/probe/); }); + +for (const mode of ["standalone", "real lhm"]) { + test(`${mode} preserves overlapping worktree edits and recoverable failures`, { + skip: mode === "real lhm" && !process.env.BUZZ_REAL_LHM, + timeout: 30_000, + }, async (t) => { + const f = mode === "real lhm" ? lhmFixture(t) : fixture(t); + const isolated = f.isolated ?? {}; + // Retain both a legacy-named user stash and a pre-existing recovery ref. + f.write("partial.ts", "export const legacy = 9;\n"); + const legacy = f.git("stash", "create").trim(); + f.git("stash", "store", "-m", "lefthook auto backup", legacy); + f.git("restore", "partial.ts"); + f.git("update-ref", `refs/lefthook/backup/${legacy}`, legacy, ""); + const stashes = f.git("stash", "list", "--format=%H %gs"); + const sibling = path.join(f.dir, "sibling"); + f.git("worktree", "add", "-q", "--detach", sibling); + for (const link of ["bin", "node_modules"]) + symlinkSync(path.join(root, link), path.join(sibling, link), "dir"); + + // A handshake pauses inside the first production job, after the runner has + // hidden unstaged hunks. No sleep or scheduler speed determines overlap. + const server = createServer(); + const sockets = new Set(); + const waiting = new Map(); + server.on("connection", (socket) => { + sockets.add(socket); + socket.once("data", (id) => waiting.get(id.toString())(socket)); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + t.after(() => { + for (const socket of sockets) socket.destroy(); + server.close(); + }); + const gate = ` +import { connect } from "node:net"; +import { writeFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +if (process.env.BUZZ_HOOK_GATE) { + await new Promise((resolve, reject) => { + const socket = connect(Number(process.env.BUZZ_HOOK_GATE), "127.0.0.1", () => socket.write(process.env.BUZZ_HOOK_ID)); + socket.on("error", reject); + socket.once("data", () => { socket.end(); resolve(); }); + }); +} +if (process.env.BUZZ_CORRUPT_RECOVERY) { + const dir = execFileSync("git", ["rev-parse", "--absolute-git-dir"], { encoding: "utf8" }).trim(); + for (const name of ["lefthook-unstaged.patch", "lefthook-unstaged-all.patch"]) + writeFileSync(dir + "/" + name, "invalid patch\\n"); +} +`; + const original = f.read("scripts/check-staged-names.mjs"); + const staged = "export const first = 3;\nexport const second = 2;\n"; + for (const [dir, id] of [ + [f.dir, "main"], + [sibling, "linked"], + ]) { + writeFileSync( + path.join(dir, "scripts/check-staged-names.mjs"), + gate + original, + ); + writeFileSync(path.join(dir, "probe.ts"), "export const value = 1;\n"); + writeFileSync(path.join(dir, "partial.ts"), staged); + f.git("-C", dir, "add", "partial.ts", "probe.ts"); + writeFileSync( + path.join(dir, "partial.ts"), + staged + `// precious-${id}\n`, + ); + } + const refs = () => + f + .git("for-each-ref", "--format=%(refname)", "refs/lefthook/backup/") + .trim() + .split("\n"); + const start = (dir, id) => { + const ready = new Promise((resolve) => waiting.set(id, resolve)); + const child = spawn("git", ["commit", "-qm", id], { + cwd: dir, + env: { + ...env, + ...isolated, + BUZZ_HOOK_GATE: String(server.address().port), + BUZZ_HOOK_ID: id, + }, + stdio: ["ignore", "pipe", "pipe"], + }); + let output = ""; + child.stdout.on("data", (data) => { + output += data; + }); + child.stderr.on("data", (data) => { + output += data; + }); + t.after(() => { + if (child.exitCode === null) child.kill(); + }); + const done = new Promise((resolve, reject) => { + child.on("error", reject); + child.on("close", (code) => + code === 0 ? resolve() : reject(new Error(output)), + ); + }); + return { + ready: Promise.race([ + ready, + done.then(() => { + throw new Error("hook exited before gate"); + }), + ]), + done, + }; + }; + const a = start(f.dir, "main"); + const mainGate = await a.ready; + const mainRef = refs().find((ref) => !ref.endsWith(legacy)); + assert.ok(mainRef); + const b = start(sibling, "linked"); + const linkedGate = await b.ready; + const linkedRef = refs().find( + (ref) => ref !== mainRef && !ref.endsWith(legacy), + ); + assert.ok(linkedRef); + f.git("gc", "--prune=now", "--quiet"); + f.git("cat-file", "-e", `${linkedRef}^{commit}`); + mainGate.write("release"); + await a.done; + assert.ok(refs().includes(linkedRef), "main cleanup removed linked backup"); + linkedGate.write("release"); + await b.done; + for (const [dir, id] of [ + [f.dir, "main"], + [sibling, "linked"], + ]) { + assert.equal(f.git("-C", dir, "show", "HEAD:partial.ts"), staged); + assert.equal( + readFileSync(path.join(dir, "partial.ts"), "utf8"), + staged + `// precious-${id}\n`, + ); + if (mode === "real lhm") + assert.equal( + readFileSync(path.join(dir, "real-lhm-source"), "utf8"), + "export const value = 1;\n", + ); + } + assert.deepEqual(refs(), [`refs/lefthook/backup/${legacy}`]); + assert.equal(f.git("stash", "list", "--format=%H %gs"), stashes); + + // Corrupt only this disposable fixture's recovery patches. The owned ref + // must survive failure and main-worktree GC and restore the original index. + const next = staged.replace("first = 3", "first = 4"); + writeFileSync(path.join(sibling, "partial.ts"), next); + f.git("-C", sibling, "add", "partial.ts"); + const index = f.git("-C", sibling, "write-tree"); + writeFileSync( + path.join(sibling, "partial.ts"), + next + "// precious-failure\n", + ); + const failed = f.run("git", ["-C", sibling, "commit", "-qm", "must fail"], { + ...isolated, + BUZZ_CORRUPT_RECOVERY: "1", + }); + assert.notEqual(failed.status, 0, failed.stdout + failed.stderr); + const retained = refs().filter((ref) => !ref.endsWith(legacy)); + assert.equal(retained.length, 1); + f.git("gc", "--prune=now", "--quiet"); + f.git("cat-file", "-e", `${retained[0]}^{commit}`); + f.git( + "-C", + sibling, + "restore", + "--staged", + "--worktree", + "partial.ts", + "scripts/check-staged-names.mjs", + ); + f.git("-C", sibling, "stash", "apply", "--index", retained[0]); + assert.equal( + readFileSync(path.join(sibling, "partial.ts"), "utf8"), + next + "// precious-failure\n", + ); + assert.equal(f.git("-C", sibling, "write-tree"), index); + assert.equal(f.git("stash", "list", "--format=%H %gs"), stashes); + }); +} + +test("real lhm rejects an unpatched runner before changing staged or unstaged content", { + skip: !process.env.BUZZ_REAL_LHM || !process.env.BUZZ_STOCK_LEFTHOOK, +}, (t) => { + const f = lhmFixture(t); + mkdirSync(path.join(f.dir, "stock")); + symlinkSync( + process.env.BUZZ_STOCK_LEFTHOOK, + path.join(f.dir, "stock/lefthook"), + ); + f.write("partial.ts", "export const first = 3;\nexport const second = 2;\n"); + f.git("add", "partial.ts"); + f.write("partial.ts", "export const first = 3;\nexport const second = 99;\n"); + const index = f.git("write-tree"); + const content = f.read("partial.ts"); + const stashes = f.git("stash", "list", "--format=%H %gs"); + const result = f.run("git", ["commit", "-qm", "must fail"], { + ...f.isolated, + PATH: `${path.join(f.dir, "stock")}${path.delimiter}${f.isolated.PATH}`, + }); + assert.notEqual(result.status, 0, result.stdout + result.stderr); + assert.match( + result.stdout + result.stderr, + /required lefthook version.*higher than current/, + ); + assert.equal(f.git("write-tree"), index); + assert.equal(f.read("partial.ts"), content); + assert.equal(f.git("stash", "list", "--format=%H %gs"), stashes); + assert.equal(f.git("for-each-ref", "refs/lefthook/backup/"), ""); +});