Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/hook-skips-when-index-not-tracked.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@zettelgeist/cli': patch
---

Pre-commit hook now also self-disables on branches where `specs/INDEX.md` is not part of the branch's git history. Before this, a back-in-time checkout (or a fresh `zettelgeist init` before the first `regen`) blocked every commit with `error: specs/INDEX.md is missing` because the hook's `regen --check` had nothing to compare against. The hook now performs a `git ls-files --error-unmatch specs/INDEX.md` pre-flight: if the index file isn't tracked on this branch, there's nothing to be stale about and the hook exits 0. Re-run `zettelgeist install-hook` to update existing hooks. The full pre-commit flow now also has end-to-end test coverage in `@zettelgeist/git-hook`'s test suite, regression-guarding both this bug and the earlier non-zg-repo bug.
20 changes: 16 additions & 4 deletions packages/git-hook/src/install-hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,25 @@ export const HOOK_MARKER_END = '# <<< zettelgeist <<<';
// run with the user's login PATH, which won't include ./node_modules/.bin —
// so we fall back to the workspace-local binary if PATH lookup misses.
//
// The leading `.zettelgeist.yaml` guard makes the hook self-disabling in
// repos that aren't zettelgeist repos: a stale install left over from a
// removed config, or a partial init, would otherwise block every commit
// with `error: not a zettelgeist repo`.
// Two pre-flight guards keep the hook from blocking commits in states
// where its check is meaningless:
//
// 1. No `.zettelgeist.yaml` → not a zettelgeist repo. A stale install
// from a removed config or a partial init would otherwise fail every
// commit with `error: not a zettelgeist repo`.
//
// 2. `specs/INDEX.md` not tracked in HEAD on this branch → the index
// file isn't part of this branch's history (back-in-time checkout,
// fresh repo before the first regen, branch that predates the
// `init` commit). There's nothing to be stale about; without this
// guard, `regen --check` fails with `specs/INDEX.md is missing` and
// the user can't commit on the branch. We hardcode the `specs/`
// path because `specs_dir` overrides are out of scope for the
// shell-level guard; users who override can write their own hook.
export const HOOK_BLOCK =
HOOK_MARKER_BEGIN + '\n' +
'[ -f .zettelgeist.yaml ] || exit 0\n' +
'git ls-files --error-unmatch specs/INDEX.md >/dev/null 2>&1 || exit 0\n' +
'if command -v zettelgeist >/dev/null 2>&1; then\n' +
' zettelgeist regen --check\n' +
'elif [ -x ./node_modules/.bin/zettelgeist ]; then\n' +
Expand Down
6 changes: 6 additions & 0 deletions packages/git-hook/tests/install-hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ describe('mergeHookContent', () => {
it('HOOK_BLOCK self-disables when .zettelgeist.yaml is missing', () => {
expect(HOOK_BLOCK).toContain('[ -f .zettelgeist.yaml ] || exit 0');
});

it('HOOK_BLOCK self-disables when specs/INDEX.md is not tracked in HEAD', () => {
expect(HOOK_BLOCK).toContain(
'git ls-files --error-unmatch specs/INDEX.md >/dev/null 2>&1 || exit 0',
);
});
});

describe('HOOK_BLOCK execution', () => {
Expand Down
244 changes: 244 additions & 0 deletions packages/git-hook/tests/pre-commit-e2e.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
/**
* End-to-end pre-commit hook tests.
*
* Spins up real git repos, installs the hook via the same code path
* users run (`installPreCommitHook`), and drives `git commit` through
* every state the hook is supposed to handle. These tests exist to
* regression-guard the two real bugs that hit users on `sensor-sender`:
*
* 1. A stale install in a non-zg repo blocked every commit with
* `error: not a zettelgeist repo`.
* 2. A back-in-time checkout where `specs/INDEX.md` was not yet part
* of branch history blocked commits with `specs/INDEX.md is
* missing`, because the hook ran `regen --check` unconditionally.
*
* The hook's "should I run the check at all?" pre-flight is pure shell;
* the `regen --check` invocation behind it is the CLI's job and is
* unit-tested elsewhere. We stub the CLI here so we can observe whether
* the pre-flight reached it (skip cases must NOT reach the stub; the
* pass-through case MUST).
*/

import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { execFile } from 'node:child_process';
import { promises as fs } from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { promisify } from 'node:util';
import { installPreCommitHook } from '../src/install-hook.js';

const execFileP = promisify(execFile);

async function setupRepo(): Promise<string> {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'zg-hook-e2e-'));
await execFileP('git', ['init', '-q'], { cwd: tmp });
// Deterministic identity so commits don't bail on missing config.
await execFileP('git', ['config', 'user.email', 'test@e2e.local'], { cwd: tmp });
await execFileP('git', ['config', 'user.name', 'E2E'], { cwd: tmp });
// Some CIs default to no initial branch; pin one for repeatability.
await execFileP('git', ['symbolic-ref', 'HEAD', 'refs/heads/main'], { cwd: tmp });
return tmp;
}

/**
* Write a `zettelgeist` stub into a sandbox `bin/` dir so we can:
* (a) detect whether the hook's pre-flight skipped it (file present
* but stub-was-called marker absent), and
* (b) force a deterministic pass/fail when the pre-flight does invoke
* it.
*
* Returns the bin dir; prepend it to PATH when running commits to
* make the stub the resolution target of `command -v zettelgeist`.
*/
async function installZettelgeistStub(
repo: string,
opts: { exitCode: number; marker?: string },
): Promise<string> {
const binDir = path.join(repo, '.test-bin');
await fs.mkdir(binDir, { recursive: true });
const stubPath = path.join(binDir, 'zettelgeist');
const markerPath = path.join(repo, '.test-stub-was-called');
// Touch the marker file so the test can assert the stub ran. Exit
// with the requested code so we can drive both pass and fail paths.
const body =
'#!/bin/sh\n' +
`touch "${markerPath}"\n` +
`exit ${opts.exitCode}\n`;
await fs.writeFile(stubPath, body);
await fs.chmod(stubPath, 0o755);
return binDir;
}

async function stubWasCalled(repo: string): Promise<boolean> {
return fs
.access(path.join(repo, '.test-stub-was-called'))
.then(() => true)
.catch(() => false);
}

interface CommitOpts {
pathOverride?: string;
expectFail?: boolean;
}

async function commitFile(
repo: string,
file: string,
content: string,
message: string,
opts: CommitOpts = {},
): Promise<{ ok: boolean; stderr: string }> {
const abs = path.join(repo, file);
await fs.mkdir(path.dirname(abs), { recursive: true });
await fs.writeFile(abs, content);
await execFileP('git', ['add', file], { cwd: repo });
const env = opts.pathOverride
? { ...process.env, PATH: opts.pathOverride }
: process.env;
try {
await execFileP('git', ['commit', '-q', '-m', message], { cwd: repo, env });
return { ok: true, stderr: '' };
} catch (err) {
const e = err as { stderr?: string };
return { ok: false, stderr: e.stderr ?? '' };
}
}

describe('pre-commit hook end-to-end', () => {
let tmp: string;

beforeEach(async () => {
tmp = await setupRepo();
});

afterEach(async () => {
await fs.rm(tmp, { recursive: true, force: true });
});

// ------------------------------------------------------------------
// Pre-flight guard 1: non-zg repo (no .zettelgeist.yaml)
// ------------------------------------------------------------------

it('non-zg repo: hook does not block commits and never invokes zettelgeist', async () => {
// Reproduces the original Sander bug: a stale hook install in a
// repo that has no `.zettelgeist.yaml` was blocking every commit.
await installPreCommitHook(tmp);
const binDir = await installZettelgeistStub(tmp, { exitCode: 1 });

const r = await commitFile(tmp, 'hello.txt', 'hello\n', 'first', {
pathOverride: `${binDir}:/usr/bin:/bin`,
});

expect(r.ok).toBe(true);
expect(await stubWasCalled(tmp)).toBe(false);
});

// ------------------------------------------------------------------
// Pre-flight guard 2: zg repo, but specs/INDEX.md not tracked on this
// branch (back-in-time checkout or fresh init pre-regen).
// ------------------------------------------------------------------

it('zg repo without tracked INDEX.md: hook does not block commits and never invokes zettelgeist', async () => {
// Reproduces the apple-updates branch bug: a branch that predates
// the first `regen` has `.zettelgeist.yaml` but no `specs/INDEX.md`
// in its history. The hook used to fail with `specs/INDEX.md is
// missing` and block every commit on that branch.
await fs.writeFile(path.join(tmp, '.zettelgeist.yaml'), 'format_version: "0.1"\n');
await execFileP('git', ['add', '.zettelgeist.yaml'], { cwd: tmp });
await execFileP('git', ['commit', '-q', '-m', 'init zg config'], { cwd: tmp });

await installPreCommitHook(tmp);
const binDir = await installZettelgeistStub(tmp, { exitCode: 1 });

const r = await commitFile(tmp, 'feature.txt', 'work\n', 'feature', {
pathOverride: `${binDir}:/usr/bin:/bin`,
});

expect(r.ok).toBe(true);
expect(await stubWasCalled(tmp)).toBe(false);
});

// ------------------------------------------------------------------
// Hook reaches the binary when both guards pass.
// ------------------------------------------------------------------

it('zg repo with tracked INDEX.md: hook invokes zettelgeist regen --check', async () => {
await fs.writeFile(path.join(tmp, '.zettelgeist.yaml'), 'format_version: "0.1"\n');
await fs.mkdir(path.join(tmp, 'specs'), { recursive: true });
await fs.writeFile(path.join(tmp, 'specs', 'INDEX.md'), '# generated\n');
await execFileP('git', ['add', '.'], { cwd: tmp });
await execFileP('git', ['commit', '-q', '-m', 'init with INDEX'], { cwd: tmp });

await installPreCommitHook(tmp);
// Stub returns success → commit should pass; stub_was_called → true.
const binDir = await installZettelgeistStub(tmp, { exitCode: 0 });

const r = await commitFile(tmp, 'a.txt', 'a\n', 'add a', {
pathOverride: `${binDir}:/usr/bin:/bin`,
});

expect(r.ok).toBe(true);
expect(await stubWasCalled(tmp)).toBe(true);
});

it('zg repo with tracked INDEX.md but zettelgeist exits non-zero: commit is blocked', async () => {
await fs.writeFile(path.join(tmp, '.zettelgeist.yaml'), 'format_version: "0.1"\n');
await fs.mkdir(path.join(tmp, 'specs'), { recursive: true });
await fs.writeFile(path.join(tmp, 'specs', 'INDEX.md'), '# generated\n');
await execFileP('git', ['add', '.'], { cwd: tmp });
await execFileP('git', ['commit', '-q', '-m', 'init with INDEX'], { cwd: tmp });

await installPreCommitHook(tmp);
// Stub fails → commit must fail. This is the "INDEX is stale"
// protection the hook exists for.
const binDir = await installZettelgeistStub(tmp, { exitCode: 1 });

const r = await commitFile(tmp, 'a.txt', 'a\n', 'add a', {
pathOverride: `${binDir}:/usr/bin:/bin`,
});

expect(r.ok).toBe(false);
expect(await stubWasCalled(tmp)).toBe(true);
});

// ------------------------------------------------------------------
// Cross-branch scenario: the hook does the right thing when checking
// out between a branch that has INDEX.md tracked and one that doesn't.
// ------------------------------------------------------------------

it('checking out a branch without tracked INDEX.md does not block commits there', async () => {
// Set up: main has INDEX.md tracked. Then create a branch from a
// commit that pre-dates INDEX.md and commit on it. This is the
// exact `apple-updates` shape Sander hit.
await fs.writeFile(path.join(tmp, '.zettelgeist.yaml'), 'format_version: "0.1"\n');
await fs.writeFile(path.join(tmp, 'README.md'), '# repo\n');
await execFileP('git', ['add', '.'], { cwd: tmp });
await execFileP('git', ['commit', '-q', '-m', 'pre-zg root'], { cwd: tmp });

// Capture the pre-INDEX commit so we can branch from it later.
const { stdout: rootSha } = await execFileP('git', ['rev-parse', 'HEAD'], { cwd: tmp });

// Land INDEX.md on main.
await fs.mkdir(path.join(tmp, 'specs'), { recursive: true });
await fs.writeFile(path.join(tmp, 'specs', 'INDEX.md'), '# generated\n');
await execFileP('git', ['add', '.'], { cwd: tmp });
await execFileP('git', ['commit', '-q', '-m', 'add INDEX'], { cwd: tmp });

// Install the hook AFTER both states exist so it lives across the
// checkout (the realistic install order).
await installPreCommitHook(tmp);
const binDir = await installZettelgeistStub(tmp, { exitCode: 1 });

// Branch off the pre-INDEX commit — `INDEX.md` is NOT in HEAD here.
await execFileP('git', ['checkout', '-q', '-b', 'apple-updates', rootSha.trim()], {
cwd: tmp,
});

const r = await commitFile(tmp, 'apple.txt', 'feature\n', 'apple feature', {
pathOverride: `${binDir}:/usr/bin:/bin`,
});

expect(r.ok).toBe(true);
expect(await stubWasCalled(tmp)).toBe(false);
});
});
Loading