Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ bun run setup:hooks

This installs the `post-merge` hook, which rebuilds the packaged dashboard when a
merge changes its source. It also removes the unmodified, retired repository
pre-push hook from Git's resolved hooks directory, including linked worktrees
and `core.hooksPath` setups. Custom pre-push hooks are preserved. Validation no
longer runs automatically on every push; existing contributors should rerun the
setup command once to migrate their hooks.
pre-push hook from Git's resolved hooks directory, including linked worktrees.
Custom pre-push hooks are preserved. For safety, setup refuses to touch hooks
when `core.hooksPath` redirects them to a potentially shared directory.
Comment thread
luvs01 marked this conversation as resolved.
Outdated
If an earlier setup already installed these hooks into that shared directory,
remove them there by hand; the refusal leaves them untouched.
Validation no longer runs automatically on every push; existing contributors
should rerun the setup command once to migrate their hooks.
24 changes: 22 additions & 2 deletions scripts/setup-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,28 @@ import { join, resolve } from "node:path";

const repoRoot = resolve(import.meta.dirname, "..");

// Resolve the real hooks dir via git so linked worktrees (`.git` file), core.hooksPath,
// and non-default git dirs all work. Hard-coding <repo>/.git/hooks breaks those setups.
// A configured hooksPath may be shared by unrelated repositories. Installing our
// cwd-dependent hooks there would replace shared policy hooks and run another
// repository's package scripts. Keep installation scoped to this repository.
try {
execFileSync("git", ["config", "--get", "core.hooksPath"], {
cwd: repoRoot,
encoding: "utf8",
// A wedged git config read must not stall the setup run: bound the call and treat a
// timeout like any other inspection failure below.
timeout: 10_000,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
console.error("setup-hooks: refusing to install because core.hooksPath is configured.");
process.exit(1);
} catch (error) {
if (typeof error === "object" && error !== null && "status" in error && error.status !== 1) {
console.error("setup-hooks: could not inspect core.hooksPath.");
process.exit(1);
}
}

// Resolve Git's repository-local hooks dir so linked worktrees and non-default
// git dirs work without assuming that <repo>/.git is a directory.
let hooksDir: string;
try {
hooksDir = execFileSync(
Expand Down
10 changes: 6 additions & 4 deletions tests/ci-workflows/setup-hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,19 @@ describe("local hook setup", () => {
expect(readFileSync(join(hooks(root), "pre-push"), "utf8")).toBe(custom);
});

test("uses a configured hooks directory without touching the default one", () => {
test("refuses a configured hooksPath that may be shared by other repositories", () => {
const root = fixture();
const original = hooks(root);
writeFileSync(join(original, "pre-push"), legacyHook);
const customDir = join(root, "custom hooks");
mkdirSync(customDir);
writeFileSync(join(customDir, "pre-push"), legacyHook);
git(root, "config", "core.hooksPath", customDir);
setup(root);
expect(existsSync(join(customDir, "pre-push"))).toBe(false);
expect(existsSync(join(customDir, "post-merge"))).toBe(true);
expect(() => setup(root)).toThrow(/core.hooksPath is configured/);
// Nothing is written anywhere: the shared directory keeps its own hook and the
// repository-local one is untouched too.
expect(readFileSync(join(customDir, "pre-push"), "utf8")).toBe(legacyHook);
expect(existsSync(join(customDir, "post-merge"))).toBe(false);
expect(readFileSync(join(original, "pre-push"), "utf8")).toBe(legacyHook);
});

Expand Down
Loading