fix(githooks): stop install and uninstall from destroying the hook they preserve

docs/integrations.md

@@ -85,11 +85,11 @@ and shows both gitleaks setups.
 
 ## husky
 
-husky sets `core.hooksPath` to `.husky`. `aimhooman init` refuses to install
-into or over an external or shared hooks directory by design: `.husky` is
-tracked repository content, and a dispatcher written there would stage this
-machine's absolute CLI and Node paths for everyone who clones
-(`src/githooks.mjs` has the full rule set). Two ways out:
+husky 9 sets `core.hooksPath` to `.husky/_` (older versions used `.husky`).
+`aimhooman init` refuses to install into or over an external or shared hooks
+directory by design: it is worktree content, and a dispatcher written there
+would stage this machine's absolute CLI and Node paths for everyone who clones
+(`src/githooks.mjs` has the full rule set). Three ways out:
 
 - Keep husky and call `aimhooman check --staged` from `.husky/pre-commit`.
   You get the CLI check only. The agent-tier guard (PreToolUse) asks for
@@ -98,6 +98,9 @@ machine's absolute CLI and Node paths for everyone who clones
 - Drop husky for the guarded hooks and let `aimhooman init` manage them.
   Existing hooks are not lost: init preserves each one as a chained
   predecessor, and the dispatcher runs it before its own check.
+- Keep the hooks path local: add it to `.git/info/exclude` (for husky 9,
+  `.husky/_/`) and re-run `aimhooman init`. It then manages that directory and
+  chains husky's shim, and nothing machine-specific is shared with clones.
 
 ## lint-staged
 

src/githooks.mjs

@@ -73,7 +73,7 @@ function hooksDir(repo) {
         const where = scope ? `${scope} scope` : 'a non-local scope';
         let reason = where;
         if (trackedByGit) reason = `${where}, tracked by this repository`;
-        else if (worktreeContent) reason = `${where}, inside the worktree so Git will track it on the next add (add it to .gitignore or .git/info/exclude to manage it locally)`;
+        else if (worktreeContent) reason = `${where}, inside the worktree where Git can track it (add it to .gitignore or .git/info/exclude to manage it locally)`;
         else if (localScope && !inside) reason = `${where}, outside this repository`;
         return {
             dir,
@@ -427,13 +427,28 @@ function installHooksLocked(repo, cliPath, options, dir, warnings) {
             const current = entry(dest);
             if (current) {
                 const cur = readFileSync(dest);
-                if (!ownedHook(cur, name)) {
-                    mkdirSync(chainedDir, { recursive: true });
-                    const chainedPath = join(chainedDir, name);
-                    const predecessorMode = current.mode & 0o777;
-                    writeHook(chainedPath, cur, { mode: predecessorMode });
-                    chmodSync(chainedPath, predecessorMode);
-                    chained.push(name);
+                const chainedPath = join(chainedDir, name);
+                // A file already naming the backup slot as its predecessor is
+                // our own damaged output. Chaining it would point the backup at
+                // itself and the dispatcher would exec itself without bound.
+                const selfChained = cur.includes(`\nCHAINED=${shq(chainedPath)}\n`);
+                if (!ownedHook(cur, name) && !selfChained) {
+                    const stored = entry(chainedPath);
+                    if (stored && !readFileSync(chainedPath).equals(cur)) {
+                        // Something replaced the dispatcher after a chained
+                        // install. The stored backup was here first, so it is
+                        // the original worth keeping; the newer file is
+                        // replaced without one, which has to be said out loud.
+                        warnings.push(
+                            `${name} already has a chained backup at ${chainedPath}; kept it and replaced ${dest} without preserving it`
+                        );
+                    } else {
+                        mkdirSync(chainedDir, { recursive: true });
+                        const predecessorMode = current.mode & 0o777;
+                        writeHook(chainedPath, cur, { mode: predecessorMode });
+                        chmodSync(chainedPath, predecessorMode);
+                        chained.push(name);
+                    }
                 }
             }
             writeHook(dest, hookScript(name, cmd, cliPath, join(chainedDir, name)), { mode: 0o755 });
@@ -451,8 +466,41 @@ function installHooksLocked(repo, cliPath, options, dir, warnings) {
 // is recorded as a failure and the remaining hooks are still processed, so a
 // re-run self-heals (already-processed hooks are skipped via ownedHook).
 export function uninstallHooks(repo) {
-    const { dir, shared, warnings } = hooksDir(repo);
-    if (shared) return { removed: [], restored: [], warnings, failures: [] };
+    const { warnings } = hooksDir(repo);
+    const results = uninstallHookDirs(repo).map((target) => uninstallOneHookDir(repo, target, []));
+    return {
+        removed: results.flatMap((r) => r.removed).sort(),
+        restored: results.flatMap((r) => r.restored).sort(),
+        warnings: [...warnings, ...results.flatMap((r) => r.warnings)],
+        failures: results.flatMap((r) => r.failures),
+    };
+}
+
+// The directories a local uninstall must clear. A moved core.hooksPath leaves
+// dispatchers behind in .git/hooks, dormant only until the setting is unset, so
+// that directory is cleared whatever the hooks path currently says. The global
+// directory belongs to `uninstall --global` and is never touched here.
+function uninstallHookDirs(repo) {
+    const { dir, shared } = hooksDir(repo);
+    const globalCanonical = canonicalPath(globalHooksDir());
+    const targets = [];
+    const seen = new Set();
+    for (const candidate of [shared ? null : dir, join(repo.commonDir, 'hooks')]) {
+        if (!candidate) continue;
+        let key;
+        try {
+            key = canonicalPath(candidate);
+        } catch {
+            key = candidate;
+        }
+        if (key === globalCanonical || seen.has(key)) continue;
+        seen.add(key);
+        targets.push(candidate);
+    }
+    return targets;
+}
+
+function uninstallOneHookDir(repo, dir, warnings) {
     let entered = false;
     try {
         return withLock(join(dir, '.aimhooman-hooks.lock'), () => {
@@ -513,7 +561,14 @@ function uninstallHooksLocked(repo, dir, warnings) {
                 removed.push(name);
                 continue;
             }
-            if (predecessor) {
+            if (predecessor && readFileSync(chained).includes(`\nCHAINED=${shq(chained)}\n`)) {
+                // The backup names itself as its own predecessor, so installing
+                // it would leave a hook that execs itself. It is not the user's
+                // original either way: drop both and say so.
+                warnings.push(`${name} chained backup pointed at itself; removed ${chained} instead of restoring it`);
+                unlinkSync(chained);
+                unlinkSync(dest);
+            } else if (predecessor) {
                 const predecessorMode = predecessor.mode & 0o777;
                 atomicWrite(dest, readFileSync(chained), { mode: predecessorMode });
                 chmodSync(dest, predecessorMode);
@@ -542,8 +597,10 @@ function uninstallHooksLocked(repo, dir, warnings) {
 // comparing paths: two spellings of one directory differ on Windows, and
 // deciding ownership by string is the bug this change exists to remove.
 export function remainingDispatchers(repo) {
-    const { dir, shared } = hooksDir(repo);
-    if (shared) return [];
+    return uninstallHookDirs(repo).flatMap((dir) => dispatchersLeftIn(dir));
+}
+
+function dispatchersLeftIn(dir) {
     return Object.keys(MANAGED).sort().flatMap((name) => {
         const path = join(dir, name);
         try {

tests/githooks.test.mjs

@@ -27,6 +27,7 @@ import {
     installedHooks,
     MESSAGE_ANCHOR_ERE,
     pathCommandReachable,
+    remainingDispatchers,
     uninstallGlobalHooks,
     uninstallHooks,
 } from '../src/githooks.mjs';
@@ -2523,3 +2524,115 @@ test('install and uninstall refuse to follow hook symlinks', () => {
         rmSync(base, { recursive: true, force: true });
     }
 });
+
+// A hook file that already points at the backup slot is our own damaged output.
+// Chaining it makes the backup its own predecessor, so the dispatcher execs
+// itself without bound; and copying it over the slot destroys whatever original
+// hook was preserved there.
+function damagedDispatcher(hooksPath) {
+    const dispatcher = readFileSync(hooksPath, 'utf8');
+    return dispatcher.replace(/^# aimhooman-hook-fingerprint: .*$/m, '# aimhooman-hook-fingerprint: ' + '0'.repeat(64));
+}
+
+test('a damaged dispatcher is replaced rather than chained onto its own backup', () => {
+    const base = mkdtempSync(join(tmpdir(), 'aim-hooks-selfchain-'));
+    try {
+        isolatedGitConfig(base, () => {
+            const root = makeRepo(base);
+            const repo = openRepo(root);
+            const hooks = git(root, ['rev-parse', '--path-format=absolute', '--git-path', 'hooks']);
+            const original = '#!/bin/sh\necho original\n';
+            writeFileSync(join(hooks, 'pre-commit'), original, { mode: 0o755 });
+            installHooks(repo, CLI);
+
+            const backup = join(repo.stateDir, 'chained', 'pre-commit');
+            assert.equal(readFileSync(backup, 'utf8'), original);
+
+            writeFileSync(join(hooks, 'pre-commit'), damagedDispatcher(join(hooks, 'pre-commit')), { mode: 0o755 });
+            const report = installHooks(repo, CLI);
+
+            assert.equal(report.chained.includes('pre-commit'), false, 'our own damaged output must not be chained');
+            assert.equal(readFileSync(backup, 'utf8'), original, "the user's original hook must survive");
+        });
+    } finally {
+        rmSync(base, { recursive: true, force: true });
+    }
+});
+
+test('a foreign hook that replaced the dispatcher does not overwrite the stored backup', () => {
+    const base = mkdtempSync(join(tmpdir(), 'aim-hooks-backup-keep-'));
+    try {
+        isolatedGitConfig(base, () => {
+            const root = makeRepo(base);
+            const repo = openRepo(root);
+            const hooks = git(root, ['rev-parse', '--path-format=absolute', '--git-path', 'hooks']);
+            const original = '#!/bin/sh\necho original\n';
+            writeFileSync(join(hooks, 'pre-commit'), original, { mode: 0o755 });
+            installHooks(repo, CLI);
+
+            // Another tool installs over the dispatcher. The next init must not
+            // treat that as the hook worth preserving.
+            writeFileSync(join(hooks, 'pre-commit'), '#!/bin/sh\necho intruder\n', { mode: 0o755 });
+            const report = installHooks(repo, CLI);
+
+            const backup = join(repo.stateDir, 'chained', 'pre-commit');
+            assert.equal(readFileSync(backup, 'utf8'), original, 'the first backup is the genuine original');
+            assert.ok(
+                report.warnings.some((w) => w.includes('pre-commit')),
+                'skipping a backup has to be reported, not silent',
+            );
+        });
+    } finally {
+        rmSync(base, { recursive: true, force: true });
+    }
+});
+
+test('uninstall does not restore a backup that points at itself', () => {
+    const base = mkdtempSync(join(tmpdir(), 'aim-hooks-selfbackup-'));
+    try {
+        isolatedGitConfig(base, () => {
+            const root = makeRepo(base);
+            const repo = openRepo(root);
+            const hooks = git(root, ['rev-parse', '--path-format=absolute', '--git-path', 'hooks']);
+            installHooks(repo, CLI);
+
+            // Seed the poisoned state: the backup slot holds a dispatcher whose
+            // CHAINED= names the slot itself.
+            const backup = join(repo.stateDir, 'chained', 'pre-commit');
+            mkdirSync(dirname(backup), { recursive: true });
+            writeFileSync(backup, damagedDispatcher(join(hooks, 'pre-commit')), { mode: 0o755 });
+
+            uninstallHooks(repo);
+
+            assert.equal(existsSync(join(hooks, 'pre-commit')), false, 'the dispatcher must go');
+            assert.equal(existsSync(backup), false, 'and the self-referencing backup with it');
+        });
+    } finally {
+        rmSync(base, { recursive: true, force: true });
+    }
+});
+
+// Moving core.hooksPath after install leaves the dispatchers in .git/hooks.
+// They are dormant only while the setting points elsewhere: unset it and they
+// guard again, which is why uninstall has to clear that directory too rather
+// than report success over the top of it.
+test('uninstall clears dispatchers left behind when the hooks path moved', () => {
+    const base = mkdtempSync(join(tmpdir(), 'aim-hooks-moved-'));
+    try {
+        isolatedGitConfig(base, () => {
+            const root = makeRepo(base);
+            const repo = openRepo(root);
+            installHooks(repo, CLI);
+            const local = join(repo.commonDir, 'hooks');
+            assert.ok(existsSync(join(local, 'pre-commit')), 'installed into .git/hooks');
+
+            git(root, ['config', 'core.hooksPath', '.husky/_']);
+            uninstallHooks(repo);
+
+            assert.equal(existsSync(join(local, 'pre-commit')), false, 'the leftover dispatcher must be removed');
+            assert.deepEqual(remainingDispatchers(repo), [], 'and nothing may still be reported as remaining');
+        });
+    } finally {
+        rmSync(base, { recursive: true, force: true });
+    }
+});