diff --git a/cli/__tests__/doctor-hookspath-owner.test.mts b/cli/__tests__/doctor-hookspath-owner.test.mts new file mode 100644 index 0000000..6332628 --- /dev/null +++ b/cli/__tests__/doctor-hookspath-owner.test.mts @@ -0,0 +1,529 @@ +/** + * doctor's OWNERSHIP verdict: whose hook a commit made in THIS checkout actually runs. + * + * The sibling suite (`doctor-hook-runner.test.mts`) proves the runner reaches a new checkout. This + * one proves the checkout then uses its OWN copy — a `core.hooksPath` pinned at another checkout's + * runner makes git dispatch that checkout's hook, off that checkout's branch, with no error and + * nothing in the usual config output to suggest it. + * + * Everything here is a property of real git — config scopes, `config.worktree`, hook dispatch — so + * these are real repos and real worktrees throughout. The first test commits for real and reads back + * which hook body ran, because "the config key is empty" is not the claim the story makes. + */ + +import { execFileSync, spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { collectResults } from '../commands/doctor.mts'; +import syncHookRunner, { replacePin } from '../commands/sync/sync-hook-runner.mts'; +import { replaceableHooksPathPin } from '../lib/doctor/hook-checks.mts'; +import { runSelfHostDoctor } from '../lib/doctor/self-host-doctor.mts'; +import { rootRegistry } from './_helpers.mts'; + +const { mkTmp, cleanup } = rootRegistry(); +afterEach(cleanup); + +const CHECK = 'hooksPath owner'; +const RUNNER_CHECK = 'hook runner (worktree-safe)'; +const HUSKY_CFG = { components: { husky: true, biome: false, tsconfig: false, guards: [] } }; + +function git(root: string, ...args: string[]): string { + return execFileSync('git', ['-C', root, ...args], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); +} + +/** Commit without letting the fixture's own hooks run — setup must not depend on what we're testing. */ +function commit(root: string, message: string): void { + git(root, '-c', 'core.hooksPath=/dev/null', 'commit', '-qm', message, '--allow-empty'); +} + +/** A husky-shaped repo whose runner is TRACKED, so it checks out into every worktree. `marker` goes + * in the runner stub git actually executes, so a commit says which checkout's copy ran. */ +function huskyRepo(marker: string): string { + const root = mkTmp('doctor-hookspath-'); + git(root, 'init', '-q'); + git(root, 'config', 'user.email', 'test@example.com'); + git(root, 'config', 'user.name', 'test'); + mkdirSync(join(root, '.husky', '_'), { recursive: true }); + writeFileSync(join(root, '.husky', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + writeFileSync(join(root, '.husky', '_', '.gitignore'), '*'); + writeFileSync(join(root, '.husky', '_', 'h'), '#!/usr/bin/env sh\n'); + writeFileSync( + join(root, '.husky', '_', 'pre-commit'), + `#!/bin/sh\necho "${marker}" >&2\nexit 0\n`, + { + mode: 0o755, + }, + ); + git(root, 'config', 'core.hooksPath', '.husky/_'); + git(root, 'add', '-f', '.husky/pre-commit', '.husky/_/h', '.husky/_/pre-commit'); + commit(root, 'initial'); + return root; +} + +/** A linked worktree of `root`, on its own branch. `marker` (when given) rewrites the runner stub + * ON THAT BRANCH, so the two checkouts genuinely disagree about what the hook says. */ +function worktree(root: string, name: string, marker?: string): string { + // Inside a tracked tmp dir of its own: worktrees must not sit under `root` (they would show up as + // untracked content in the very commits these tests make) and must not collide between tests. + const wt = join(mkTmp('doctor-hookspath-wt-'), name); + git(root, '-c', 'core.hooksPath=/dev/null', 'worktree', 'add', '-q', '-b', name, wt); + if (marker) { + writeFileSync( + join(wt, '.husky', '_', 'pre-commit'), + `#!/bin/sh\necho "${marker}" >&2\nexit 0\n`, + { + mode: 0o755, + }, + ); + git(wt, 'add', '-f', '.husky/_/pre-commit'); + commit(wt, 'worktree hook'); + } + return wt; +} + +/** Pin `target` as this checkout's own core.hooksPath — what the host tooling writes after a + * `git worktree add`, and the state every test here is about. */ +function pin(root: string, wt: string, target: string): void { + git(root, 'config', 'extensions.worktreeConfig', 'true'); + git(wt, 'config', '--worktree', 'core.hooksPath', target); +} + +async function results(root: string, cfg: object = HUSKY_CFG) { + const { results: all } = await collectResults(root, cfg, { name: 'config.json', status: 'OK' }); + return all; +} + +async function ownerCheck(root: string, cfg: object = HUSKY_CFG) { + return (await results(root, cfg)).find((r) => r.name === CHECK); +} + +/** Commit for real and return everything the hook printed — both streams, since which one carries a + * hook's output is git's business, not something worth asserting on. */ +function commitAndReadHook(wt: string, name: string): string { + writeFileSync(join(wt, `${name}.txt`), 'x'); + git(wt, 'add', '-A'); + const done = spawnSync('git', ['-C', wt, 'commit', '-m', name], { encoding: 'utf8' }); + return `${done.stdout ?? ''}${done.stderr ?? ''}`; +} + +function scopedPin(wt: string): string { + try { + return git(wt, 'config', '--worktree', '--get', 'core.hooksPath').trim(); + } catch { + return ''; + } +} + +describe('doctor — whose hooks run in this checkout', () => { + it('replacing the sibling pin flips which checkout’s hook a real commit runs', async () => { + // The story's actual claim, end to end. Everything else here guards a branch of it. + const root = huskyRepo('HOOK-FROM-MAIN'); + const wt = worktree(root, 'feature', 'HOOK-FROM-WT'); + pin(root, wt, join(root, '.husky', '_')); + + // Before: the worktree's commits run the MAIN checkout's hook, off main's branch. + expect(commitAndReadHook(wt, 'before')).toContain('HOOK-FROM-MAIN'); + expect((await ownerCheck(wt))?.status).toBe('DRIFT'); + + expect(syncHookRunner([], wt)).toBe(0); + + expect(scopedPin(wt)).toBe('.husky/_'); + expect(commitAndReadHook(wt, 'after')).toContain('HOOK-FROM-WT'); + expect((await ownerCheck(wt))?.status).toBe('OK'); + }); + + it('does not append over a concurrent writer when a captured repair plan becomes stale', () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + const planned = replaceableHooksPathPin(wt); + expect(planned).not.toBeNull(); + if (!planned) throw new Error('expected a repairable sibling pin'); + + const central = join(mkTmp('doctor-hookspath-central-'), 'hooks'); + mkdirSync(central); + writeFileSync(join(central, 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + git(wt, 'config', '--worktree', 'core.hooksPath', central); + + expect(replacePin(wt, wt, planned)).toBe(false); + expect(git(wt, 'config', '--worktree', '--get-all', 'core.hooksPath').trim()).toBe(central); + }); + + it('fails safely when another Git writer already owns the config.worktree lock', () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + const original = join(root, '.husky', '_'); + pin(root, wt, original); + const gitDir = git(wt, 'rev-parse', '--absolute-git-dir').trim(); + const lock = join(gitDir, 'config.worktree.lock'); + writeFileSync(lock, 'held by another writer'); + + expect(syncHookRunner([], wt)).toBe(1); + expect(git(wt, 'config', '--worktree', '--get-all', 'core.hooksPath').trim()).toBe(original); + rmSync(lock); + }); + + it('rolls back when the local runner disappears after locked revalidation', () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + const original = join(root, '.husky', '_'); + pin(root, wt, original); + const planned = replaceableHooksPathPin(wt); + expect(planned).not.toBeNull(); + if (!planned) throw new Error('expected a repairable sibling pin'); + + // The getter places the competing filesystem write at the exact TOCTOU boundary: after the + // locked repair plan was recomputed, but before its candidate is renamed into place. + let raced = false; + const racingPlan = { + from: planned.from, + get to() { + if (!raced) { + raced = true; + rmSync(join(wt, '.husky', '_', 'pre-commit')); + } + return planned.to; + }, + }; + + expect(replacePin(wt, wt, racingPlan)).toBe(false); + expect(scopedPin(wt)).toBe(original); + }); + + it('names the checkout whose hooks run, and the command that hands them back', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + + const result = await ownerCheck(wt); + + expect(result?.status).toBe('DRIFT'); + expect(result?.detail).toContain('owned by sibling checkout'); + expect(result?.detail).toContain(root); + expect(result?.remediation).toContain('devkit sync-hook-runner'); + expect(result?.remediation).toContain('git config --worktree core.hooksPath .husky/_'); + }); + + it('leaves the pin alone when a declared hook has no stub here — replacement would un-gate it', async () => { + // The runner DIRECTORY exists, so an existence test would call this healthy and clear the pin. + // But `.husky/commit-msg` has no stub in this checkout, so commit-msg would then run nothing — + // strictly worse than the sibling hook it currently borrows. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + writeFileSync(join(wt, '.husky', 'commit-msg'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + git(wt, 'add', '-f', '.husky/commit-msg'); + commit(wt, 'declare commit-msg'); + pin(root, wt, join(root, '.husky', '_')); + + const result = await ownerCheck(wt); + expect(result?.status).toBe('DRIFT'); + expect(result?.remediation).not.toContain('--unset'); + expect(result?.remediation).toContain('bun install'); + + syncHookRunner([], wt); + expect(scopedPin(wt)).toBe(join(root, '.husky', '_')); + }); + + it('stages an untracked runner and replaces the pin in ONE run', async () => { + // The shape found live: a worktree cut before the runner was tracked. Its runner exists on disk + // but is gitignored, so it is not self-gated yet — and telling the user to `bun install` first + // would be wrong, because staging is exactly what sync-hook-runner does before it re-reads. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + git(wt, 'rm', '-q', '--cached', '.husky/_/pre-commit', '.husky/_/h'); + commit(wt, 'untrack the runner'); + // husky regenerates its self-ignoring runner locally, which is what makes the files unreachable + // rather than merely uncommitted — the distinction the whole staging half turns on. + writeFileSync(join(wt, '.husky', '_', '.gitignore'), '*'); + pin(root, wt, join(root, '.husky', '_')); + + expect((await ownerCheck(wt))?.remediation).toContain('devkit sync-hook-runner'); + + syncHookRunner([], wt); + + expect(git(wt, 'ls-files', '.husky/_').trim()).toContain('.husky/_/pre-commit'); + expect(scopedPin(wt)).toBe('.husky/_'); + }); + + it('leaves the pin alone when this checkout’s runner directory is empty', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + rmSync(join(wt, '.husky', '_'), { recursive: true, force: true }); + + syncHookRunner([], wt); + + expect(scopedPin(wt)).toBe(join(root, '.husky', '_')); + }); + + it('reports a repo-wide absolute hooksPath but never clears it — it is not this checkout’s to drop', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + // Repo-wide, so every checkout inherits it; unsetting from here would re-wire all of them. + git(root, 'config', 'core.hooksPath', join(root, '.husky', '_')); + + const result = await ownerCheck(wt); + expect(result?.status).toBe('DRIFT'); + expect(result?.remediation).toContain('repo-wide'); + expect(result?.remediation).not.toContain('--worktree'); + + syncHookRunner([], wt); + expect(git(wt, 'config', '--local', '--get', 'core.hooksPath').trim()).toBe( + join(root, '.husky', '_'), + ); + }); + + it('never treats a repo-wide value as per-checkout when the worktree extension is off', async () => { + // `git config --worktree` FALLS BACK to --local without extensions.worktreeConfig: it reads the + // repo-wide value while looking scoped, and its --unset deletes that repo-wide value. Scope has + // to come from the extension + config.worktree, never from the read succeeding. + const root = huskyRepo('MAIN'); + const outside = mkTmp('doctor-hookspath-outside-'); + mkdirSync(join(outside, 'hooks')); + writeFileSync(join(outside, 'hooks', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + git(root, 'config', 'core.hooksPath', join(outside, 'hooks')); + + const result = await ownerCheck(root); + expect(result?.remediation ?? '').not.toContain('--worktree'); + + syncHookRunner([], root); + expect(git(root, 'config', '--local', '--get', 'core.hooksPath').trim()).toBe( + join(outside, 'hooks'), + ); + }); + + it('survives a --worktree read that git refuses outright', async () => { + // Two worktrees and no extension: `git config --worktree` exits 128 rather than falling back. + const root = huskyRepo('MAIN'); + worktree(root, 'one'); + const wt = worktree(root, 'two'); + + expect(await ownerCheck(wt)).toBeUndefined(); + expect((await ownerCheck(wt, HUSKY_CFG)) ?? null).toBeNull(); + }); + + it('stays silent for a relative hooksPath whose runner is a symlink into another checkout', async () => { + // devkit's own ship gate worktrees are built this way. Resolving a RELATIVE value through its + // link target would report every single `devkit ship` as a defect. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'gate'); + rmSync(join(wt, '.husky', '_'), { recursive: true, force: true }); + symlinkSync(join(root, '.husky', '_'), join(wt, '.husky', '_')); + + expect(await ownerCheck(wt)).toBeUndefined(); + }); + + it('reports a RELATIVE pin that escapes the checkout', async () => { + // git accepts `../…` and dispatches it verbatim; treating "relative" as "inside" would miss it. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, relative(wt, join(root, '.husky', '_'))); + + expect((await ownerCheck(wt))?.status).toBe('DRIFT'); + }); + + it('stays silent for a deliberate org-shared hooks directory outside every checkout', async () => { + // A working, intentional configuration that doctor passes today. Flipping it to exit 1 would + // red a consumer whose setup is doing exactly what they asked for. + const root = huskyRepo('MAIN'); + const shared = mkTmp('doctor-hookspath-org-'); + mkdirSync(join(shared, 'githooks')); + writeFileSync(join(shared, 'githooks', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + git(root, 'config', 'core.hooksPath', join(shared, 'githooks')); + + expect(await ownerCheck(root)).toBeUndefined(); + }); + + it('preserves an intentional WORKTREE-scoped central hooks directory', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + const central = mkTmp('doctor-hookspath-central-'); + mkdirSync(join(central, 'hooks')); + writeFileSync(join(central, 'hooks', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + pin(root, wt, join(central, 'hooks')); + + expect((await ownerCheck(wt))?.status).toBe('OK'); + expect(syncHookRunner([], wt)).toBe(0); + expect(scopedPin(wt)).toBe(join(central, 'hooks')); + }); + + it('does not call a pin a shadow when it merely restates the fallback', async () => { + // `
/.git/hooks` with no shared value is byte-identical to git's own default, which every + // worktree already shares through the common dir — so this pin changes nothing and must not be + // reported as another checkout's hooks. Note git hands back `/private/var/…` while the pin was + // written as `/var/…`, so a lexical comparison alone calls this a shadow. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + git(root, 'config', '--unset', 'core.hooksPath'); + pin(root, wt, join(root, '.git', 'hooks')); + + const result = await ownerCheck(wt); + + expect(result?.status).toBe('OK'); + expect(result?.detail).not.toContain('owned by sibling checkout'); + }); + + it('reports a pin at a SIBLING worktree, not just at the main checkout', async () => { + const root = huskyRepo('MAIN'); + const sibling = worktree(root, 'sibling'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(sibling, '.husky', '_')); + + const result = await ownerCheck(wt); + expect(result?.status).toBe('DRIFT'); + expect(result?.detail).toContain(sibling); + }); + + it('reports a pin at a checkout that has since been removed', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '..', 'gone', '.husky', '_')); + + const result = await ownerCheck(wt); + expect(result?.status).toBe('MISSING'); + expect(result?.detail).toContain('resolves to nothing'); + syncHookRunner([], wt); + expect(scopedPin(wt)).toBe(join(root, '..', 'gone', '.husky', '_')); + }); + + it('repairs a missing sibling while git still retains its prunable provenance', async () => { + const root = huskyRepo('MAIN'); + const sibling = worktree(root, 'gone-sibling'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(sibling, '.husky', '_')); + rmSync(sibling, { recursive: true, force: true }); + + expect((await ownerCheck(wt))?.status).toBe('MISSING'); + expect(syncHookRunner([], wt)).toBe(0); + expect(scopedPin(wt)).toBe('.husky/_'); + }); + + it('refuses ambiguous multiple worktree values', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + git(wt, 'config', '--worktree', '--add', 'core.hooksPath', '.husky/_'); + + const before = git(wt, 'config', '--worktree', '--get-all', 'core.hooksPath'); + const all = await results(wt); + expect(all.find((result) => result.name === CHECK)?.detail).toContain( + 'worktree-scoped core.hooksPath values', + ); + const runner = all.find((result) => result.name === RUNNER_CHECK); + expect(runner?.status).toBe('DRIFT'); + expect(runner?.detail).toContain('worktree-scoped core.hooksPath values'); + expect(syncHookRunner([], wt)).toBe(0); + expect(git(wt, 'config', '--worktree', '--get-all', 'core.hooksPath')).toBe(before); + }); + + it('--dry-run reports the pin without touching it', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + + expect(syncHookRunner(['--dry-run'], wt)).toBe(0); + + expect(scopedPin(wt)).toBe(join(root, '.husky', '_')); + }); + + it('surfaces a benign per-checkout pin, which shadows every repo-wide write in silence', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(wt, '.husky', '_')); // its own runner — nothing is wrong, but it IS invisible + + const result = await ownerCheck(wt); + + expect(result?.status).toBe('OK'); + expect(result?.detail).toContain('pins its own core.hooksPath'); + syncHookRunner([], wt); + expect(scopedPin(wt)).toBe(join(wt, '.husky', '_')); + }); + + it('adds no row at all to an ordinary repo’s doctor output', async () => { + // Asserting the absence of ONE name would pass even if the check were never wired in; assert + // the whole result set instead. + const root = huskyRepo('MAIN'); + const before = (await results(root)).map((r) => r.name); + + expect(before).toContain(RUNNER_CHECK); + expect(before).not.toContain(CHECK); + }); + + it('judges a monorepo package subdir against the git root', async () => { + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + const pkg = join(wt, 'services', 'api'); + mkdirSync(pkg, { recursive: true }); + pin(root, wt, join(root, '.husky', '_')); + + expect((await ownerCheck(pkg))?.status).toBe('DRIFT'); + }); + + it('stops calling the runner healthy when an override is the only thing wiring it', async () => { + // With no shared value behind it, a pin at a sibling checkout means the only hooks that run here + // are someone else's. Reporting that as OK stated the defect as health — and would have handed + // sync-hook-runner a licence to clear the one thing still gating this checkout. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + git(root, 'config', '--unset', 'core.hooksPath'); + pin(root, wt, join(root, '.husky', '_')); + + const all = await results(wt); + expect(all.find((r) => r.name === RUNNER_CHECK)?.status).toBe('DRIFT'); + + syncHookRunner([], wt); + expect(scopedPin(wt)).toBe(join(root, '.husky', '_')); + }); +}); + +describe('self-host doctor — the dogfood repo owes itself the same verdict', () => { + it('reports a foreign pin and fails, on the path collectResults never reaches', async () => { + // devkit itself is developed almost entirely from linked worktrees, and `devkit doctor` returns + // from runSelfHostDoctor BEFORE collectResults — so wiring the check into collectResults alone + // would leave it dead in the one repo most likely to hit this. + const root = huskyRepo('MAIN'); + const wt = worktree(root, 'feature'); + pin(root, wt, join(root, '.husky', '_')); + // Drop the committed hook so the generator comparison short-circuits: rebuilding the self-host + // block needs devkit's own package.json bin map, which no fixture can stand in for. The hook + // wiring under test runs after that branch either way. + rmSync(join(wt, '.husky', 'pre-commit')); + const lines: string[] = []; + const log = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + lines.push(a.join(' ')); + }); + + const code = await runSelfHostDoctor( + wt, + { components: { skills: false, agents: false } }, + false, + ); + + log.mockRestore(); + expect(lines.join('\n')).toContain(CHECK); + expect(lines.join('\n')).toContain('run ITS hooks'); + expect(code).toBe(1); + }); +}); + +describe('sync-hook-runner — staging still works', () => { + it('force-adds a gitignored runner and then finds nothing left to do', () => { + const root = mkTmp('doctor-hookspath-stage-'); + git(root, 'init', '-q'); + mkdirSync(join(root, '.husky', '_'), { recursive: true }); + writeFileSync(join(root, '.husky', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + writeFileSync(join(root, '.husky', '_', '.gitignore'), '*'); + writeFileSync(join(root, '.husky', '_', 'h'), '#!/usr/bin/env sh\n'); + writeFileSync(join(root, '.husky', '_', 'pre-commit'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + git(root, 'config', 'core.hooksPath', '.husky/_'); + + expect(syncHookRunner([], root)).toBe(0); + + expect(git(root, 'ls-files', '.husky/_').trim().split('\n')).toContain('.husky/_/pre-commit'); + expect(existsSync(join(root, '.husky', '_', 'h'))).toBe(true); + }); +}); diff --git a/cli/__tests__/review-setup-manifest.test.mts b/cli/__tests__/review-setup-manifest.test.mts index 2444805..378e034 100644 --- a/cli/__tests__/review-setup-manifest.test.mts +++ b/cli/__tests__/review-setup-manifest.test.mts @@ -254,6 +254,14 @@ describe('review setup manifest', () => { expect(() => captureReviewSetup(ineffective.root, ineffective.manifest)).toThrow( /core\.hooksPath.*expected \.husky\/_.*doctor --fix/, ); + + // An ABSOLUTE value is git config, which `--fix` cannot rewrite — sending the reader there is a + // loop. It is also how a checkout ends up running another checkout's hooks, so name that repair. + const pinned = setup('pinned'); + git(pinned.root, 'config', 'core.hooksPath', join(pinned.root, '.husky', '_')); + expect(() => captureReviewSetup(pinned.root, pinned.manifest)).toThrow( + /core\.hooksPath.*run 'devkit doctor'.*devkit sync-hook-runner/s, + ); }); it('freezes every target-controlled Husky runner dependency', () => { diff --git a/cli/__tests__/sync-hook-runner.test.mts b/cli/__tests__/sync-hook-runner.test.mts index 9322944..7f6cc26 100644 --- a/cli/__tests__/sync-hook-runner.test.mts +++ b/cli/__tests__/sync-hook-runner.test.mts @@ -65,7 +65,9 @@ describe('devkit sync-hook-runner', () => { encoding: 'utf8', }); - expect(r).toContain('nothing to stage'); + // The command now covers both halves of "this checkout runs its own hooks" — a tracked runner + // AND no foreign core.hooksPath pin — so the no-op line names the guarantee, not just staging. + expect(r).toContain('already runs its own hooks'); }); it('--dry-run reports without staging anything', () => { diff --git a/cli/commands/sync/sync-hook-runner.mts b/cli/commands/sync/sync-hook-runner.mts index f44585b..7bbab64 100644 --- a/cli/commands/sync/sync-hook-runner.mts +++ b/cli/commands/sync/sync-hook-runner.mts @@ -1,55 +1,204 @@ /** - * `devkit sync-hook-runner` — stage (`git add -f`) whatever husky-generated runner files this repo - * needs that are currently untracked AND gitignored, so a fresh `git worktree add` can actually - * reach them. + * `devkit sync-hook-runner` — make THIS checkout gate itself with its own hooks. * - * Husky pins a RELATIVE `core.hooksPath` (`.husky/_`) and gitignores the runner it points at - * (`.husky/_/.gitignore` = `*`). A linked worktree checks out with hooksPath resolving to a MISSING - * directory — git treats "no runner" as "no hooks", so every commit made there is silently ungated. - * Tracking the runner (force-adding past husky's own ignore) fixes it permanently: a tracked file - * checks out into every worktree, so the relative path resolves everywhere. + * Two halves of one guarantee, both about a hook runner that a checkout cannot reach: * - * `devkit init` chains this into a fresh package-mode install's `prepare` script, so no NEW repo - * ever needs a manual `git add -f` — every `bun install` re-stages it if husky's install regenerated - * an untracked runner (it never will once tracked, but the chain is idempotent either way). + * 1. Stage (`git add -f`) whatever husky-generated runner files this repo needs that are currently + * untracked AND gitignored. Husky pins a RELATIVE `core.hooksPath` (`.husky/_`) and gitignores the + * runner it points at (`.husky/_/.gitignore` = `*`), so a linked worktree checks out with + * hooksPath resolving to a MISSING directory — git treats "no runner" as "no hooks", and every + * commit made there is silently ungated. Tracking the runner fixes it permanently: a tracked file + * checks out into every worktree, so the relative path resolves everywhere. + * + * 2. Replace a per-checkout `core.hooksPath` left pinned at ANOTHER checkout's runner. That pin is the + * older workaround for exactly the same problem — when a fresh worktree had no runner of its own, + * borrowing the main checkout's beat having none. Once (1) holds it stops being a workaround and + * becomes a bug: commits made here run the OTHER checkout's version of the hook, off the OTHER + * checkout's branch, with no error. Order matters — staging first can be what makes this checkout + * self-gated, and the pin is only replaced with the validated shared relative path once it is. + * + * `devkit init` chains this into a fresh package-mode install's `prepare` script, so no NEW repo ever + * needs a manual `git add -f` — every `bun install` re-stages the runner and re-checks the pin. * * A dedicated, explicitly-invoked command rather than folded into `devkit doctor --fix`: --fix only - * ever regenerates FILE content from the recorded selection, never mutates the git INDEX — staging - * is something the caller (a human, or their own prepare script) must ask for. + * ever regenerates FILE content from the recorded selection, never mutates the git INDEX or git + * CONFIG — both are things the caller (a human, or their own prepare script) must ask for. * * devkit sync-hook-runner [--dry-run] */ import { execFileSync } from 'node:child_process'; +import { + closeSync, + copyFileSync, + openSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { detectGitRoot } from '../../lib/detect-git-root.mts'; -import { unreachableRunnerFiles } from '../../lib/doctor/hook-checks.mts'; +import { + checkHookRunner, + type ReplaceableHooksPathPin, + replaceableHooksPathPin, + unreachableRunnerFiles, +} from '../../lib/doctor/hook-checks.mts'; +import { sharedHooksPath, worktreeHooksPathState } from '../../lib/doctor/hooks-path.mts'; export const meta = { name: 'sync-hook-runner', - summary: 'Stage the husky hook runner so it survives `git worktree add` (git add -f).', - help: `devkit sync-hook-runner — force-add whatever husky-generated runner files this repo needs -that are untracked AND gitignored, so a fresh \`git worktree add\` can reach them. + summary: + 'Make this checkout run its OWN hooks (stage the runner; replace a sibling hooksPath pin).', + help: `devkit sync-hook-runner — make this checkout gate itself with its own hooks. Usage: devkit sync-hook-runner [--dry-run] -A no-op (exit 0) when the runner is already fully tracked, or when core.hooksPath is unset/absolute -(nothing in-repo to stage in either case). Chained into a fresh \`devkit init\`'s package.json -"prepare" script — every \`bun install\` self-heals, so this rarely needs a manual run.`, +Force-adds whatever husky-generated runner files this repo needs that are untracked AND gitignored, +so a fresh \`git worktree add\` can reach them. Then, if this checkout pins core.hooksPath at ANOTHER +checkout's runner — the older workaround for that same gap — replaces that exact value with the +validated shared relative path, but only once this checkout provably gates itself. + +Exits 0 when there is nothing to do. Only ever touches a PER-CHECKOUT pin; a repo-wide +core.hooksPath is reported by \`devkit doctor\` and left alone. External central paths are never +replaced. Chained into a fresh \`devkit init\`'s +package.json "prepare" script — every \`bun install\` self-heals, so this rarely needs a manual run.`, }; +/** Restore the exact pre-write config only when nobody has changed our candidate since the failed + * verification. Re-acquiring Git's lock and comparing bytes makes rollback another CAS operation, + * rather than overwriting a writer that raced with the verifier. */ +function restoreConfig(file: string, original: Buffer, candidate: Buffer): boolean { + const lockPath = `${file}.lock`; + let ownsLock = false; + try { + const lock = openSync(lockPath, 'wx'); + ownsLock = true; + closeSync(lock); + if (!readFileSync(file).equals(candidate)) + throw new Error('the replacement changed again before rollback'); + writeFileSync(lockPath, original); + renameSync(lockPath, file); + ownsLock = false; + return true; + } catch { + if (ownsLock) rmSync(lockPath, { force: true }); + return false; + } +} + +/** Replace the exact sibling value while holding Git's own config.worktree lock. Git's + * `--fixed-value --replace-all` APPENDS when the old value no longer matches, so invoking it against + * the live file is not compare-and-swap. Instead we acquire the lock, revalidate the live value, + * transform a private copy through Git's parser, and atomically rename that copy into place. */ +export function replacePin(cwd: string, gitRoot: string, pin: ReplaceableHooksPathPin): boolean { + let lockPath = ''; + let ownsLock = false; + let replacedFile = ''; + let original: Buffer | null = null; + let candidateContents: Buffer | null = null; + try { + const before = worktreeHooksPathState(gitRoot); + if (before.status !== 'single' || before.value !== pin.from) + throw new Error('the worktree value changed before replacement'); + lockPath = `${before.file}.lock`; + const lock = openSync(lockPath, 'wx'); + ownsLock = true; + closeSync(lock); + + // A writer may have won immediately before our lock. Re-read the LIVE file only after every + // cooperating Git writer is excluded, then abort unless the captured repair plan is still exact. + const locked = worktreeHooksPathState(gitRoot); + if (locked.status !== 'single' || locked.file !== before.file || locked.value !== pin.from) + throw new Error('the worktree value changed while acquiring the config lock'); + const revalidated = replaceableHooksPathPin(cwd); + if (!revalidated || revalidated.from !== pin.from || revalidated.to !== pin.to) + throw new Error('this checkout stopped being a safe replacement target'); + + original = readFileSync(locked.file); + copyFileSync(locked.file, lockPath); + execFileSync( + 'git', + [ + '-C', + gitRoot, + 'config', + '--file', + lockPath, + '--fixed-value', + '--replace-all', + 'core.hooksPath', + pin.to, + pin.from, + ], + { stdio: ['ignore', 'ignore', 'ignore'] }, + ); + const candidate = execFileSync( + 'git', + ['-C', gitRoot, 'config', '--file', lockPath, '--null', '--get-all', 'core.hooksPath'], + { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }, + ) + .split('\0') + .filter((value, index, all) => value !== '' || index < all.length - 1); + if (candidate.length !== 1 || candidate[0] !== pin.to) + throw new Error('the locked replacement did not produce one exact fallback value'); + + candidateContents = readFileSync(lockPath); + renameSync(lockPath, locked.file); + ownsLock = false; + replacedFile = locked.file; + } catch (e: unknown) { + if (ownsLock) rmSync(lockPath, { force: true }); + const msg = e instanceof Error ? e.message.split('\n')[0] : ''; + console.log( + `devkit sync-hook-runner: did not replace core.hooksPath (${msg || 'the value changed or git refused the update'}) — the live config was left intact`, + ); + return false; + } + const after = worktreeHooksPathState(gitRoot); + if ( + after.status !== 'single' || + after.value !== pin.to || + sharedHooksPath(gitRoot) !== pin.to || + checkHookRunner(cwd).status !== 'OK' + ) { + const restored = + original !== null && candidateContents !== null + ? restoreConfig(replacedFile, original, candidateContents) + : false; + console.log( + `devkit sync-hook-runner: the locked core.hooksPath replacement did not verify cleanly and ${restored ? 'the prior pin was restored' : 'could not be rolled back safely'} — inspect with 'devkit doctor'`, + ); + return false; + } + console.log( + `devkit sync-hook-runner: replaced sibling core.hooksPath ${pin.from} with ${pin.to} — this checkout now runs its own hooks`, + ); + return true; +} + export default function run(args: string[], cwd: string): number { const { gitRoot } = detectGitRoot(cwd); + const dryRun = args.includes('--dry-run'); const files = unreachableRunnerFiles(gitRoot); - if (!files.length) { - console.log('devkit sync-hook-runner: hook runner already reachable — nothing to stage'); - return 0; - } - if (args.includes('--dry-run')) { + if (files.length && dryRun) console.log(`devkit sync-hook-runner: [dry-run] would git add -f ${files.join(' ')}`); - return 0; + else if (files.length) { + execFileSync('git', ['-C', gitRoot, 'add', '-f', ...files], { stdio: 'inherit' }); + console.log(`devkit sync-hook-runner: staged ${files.join(', ')}`); } - execFileSync('git', ['-C', gitRoot, 'add', '-f', ...files], { stdio: 'inherit' }); - console.log(`devkit sync-hook-runner: staged ${files.join(', ')}`); + // Read AFTER staging: force-adding the runner is one of the things that can make this checkout + // self-gated, and therefore make the pin safe to drop in the same run. + const pin = replaceableHooksPathPin(cwd); + if (pin && dryRun) + console.log( + `devkit sync-hook-runner: [dry-run] would replace sibling core.hooksPath ${pin.from} with ${pin.to}`, + ); + else if (pin && !replacePin(cwd, gitRoot, pin)) return 1; + if (!files.length && !pin) + console.log( + 'devkit sync-hook-runner: this checkout already runs its own hooks — nothing to do', + ); return 0; } diff --git a/cli/lib/doctor/hook-checks.mts b/cli/lib/doctor/hook-checks.mts index 98576ca..9e4cb75 100644 --- a/cli/lib/doctor/hook-checks.mts +++ b/cli/lib/doctor/hook-checks.mts @@ -1,7 +1,9 @@ /** - * The two pre-commit hook health checks, kept together because they answer complementary halves of + * The three pre-commit hook health checks, kept together because they answer complementary parts of * one question: `checkHusky` asks whether the hook exists and still calls the selected gates in THIS - * checkout; `checkHookRunner` asks whether that hook survives `git worktree add` at all. + * checkout; `checkHookRunner` asks whether that hook survives `git worktree add` at all; and + * `checkHooksPathOwner` asks whose hook a commit made HERE actually runs — delivery into a new + * checkout and ownership of the current one being different failures with the same symptom. * * They live here beside the other doctor checks (see `asset-checks.mts`) rather than in * `doctor.mts`, which is at its line budget. @@ -16,6 +18,15 @@ import { markEnd, markStart } from '../husky/husky.mts'; import { extractGuardBlock, QAVIS_ADVISORY_ID } from '../husky/husky-block.mts'; import { firstLine } from '../standalone.mts'; import { type CheckResult, check } from './check-result.mts'; +import { + foreignPin, + hooksDir, + isInside, + isInsideResolved, + sharedHooksPath, + worktreeHooksPathState, + worktreeScopedPin, +} from './hooks-path.mts'; import { strayGateCalls } from './stray-gate-calls.mts'; import { checkFailOpenGuards } from './unguarded-gate-calls.mts'; @@ -125,6 +136,10 @@ export function checkHusky(cwd: string, selectedGuards: string[]): CheckResult { } const RUNNER = 'hook runner (worktree-safe)'; +// Named apart from RUNNER on purpose: the two answer different questions and would read as +// contradictory duplicates side by side. RUNNER judges DELIVERY into a new checkout; OWNER judges +// which checkout's hooks run in THIS one. +const OWNER = 'hooksPath owner'; /** Git's hook names — the same set husky generates stubs for. Used to tell a real hook apart from * an unrelated file sitting in the hooks directory. */ @@ -145,19 +160,6 @@ const GIT_HOOKS = new Set([ 'post-rewrite', ]); -/** A `core.hooksPath` at one config scope. Absent/unreadable (e.g. `--worktree` without - * `extensions.worktreeConfig`) reads as ''. */ -function hooksPathAt(gitRoot: string, scope: '--local' | '--worktree'): string { - try { - return execFileSync('git', ['-C', gitRoot, 'config', scope, '--get', 'core.hooksPath'], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); - } catch { - return ''; - } -} - function gitSucceeds(gitRoot: string, args: string[]): boolean { try { execFileSync('git', ['-C', gitRoot, ...args], { stdio: 'ignore' }); @@ -186,6 +188,23 @@ function defaultHookDelegatesToHusky(gitRoot: string): boolean { } } +/** + * Does this checkout gate itself — would the hooks that run once the pin is gone actually be its + * own, and actually run? + * + * Reuses `checkHookRunner`'s whole verdict rather than a second existence test, because a runner + * DIRECTORY can be there while nothing runs: an empty runner dir, or a hook declared in `.husky/` + * whose stub was never generated, are both states it already reports and `existsSync` cannot see. + * Clearing a pin in either state would swap the sibling checkout's working hook for nothing at all. + * The containment test is separate and comes first: `checkHookRunner` judges the shared value's + * health, not whether that value points at US. + */ +function selfGated(cwd: string, gitRoot: string): boolean { + const shared = sharedHooksPath(gitRoot); + if (!shared || !isInside(gitRoot, hooksDir(gitRoot, shared))) return false; + return checkHookRunner(cwd).status === 'OK'; +} + // A file reaches a new worktree iff it is TRACKED. Merely-untracked is transient (the next commit // carries it), but untracked AND IGNORED is permanent: no ordinary `git add` can ever pick it up. // That pairing — load-bearing yet unreachable — is the actual defect. @@ -204,7 +223,7 @@ function isUnreachable(gitRoot: string, relPath: string): boolean { * already-reviewed check's shape stays untouched. */ export function unreachableRunnerFiles(gitRoot: string): string[] { - const shared = hooksPathAt(gitRoot, '--local'); + const shared = sharedHooksPath(gitRoot); if (!shared || isAbsolute(shared) || !existsSync(join(gitRoot, shared))) return []; const huskyDir = join(gitRoot, '.husky'); const runnerDir = join(gitRoot, shared); @@ -231,7 +250,15 @@ export function unreachableRunnerFiles(gitRoot: string): string[] { // Detection only (never `fixable`): the repair stages files, which `--fix` must not do unasked. export function checkHookRunner(cwd: string): CheckResult { const { gitRoot } = detectGitRoot(cwd); - const shared = hooksPathAt(gitRoot, '--local'); + const scopedState = worktreeHooksPathState(gitRoot); + if (scopedState.status === 'ambiguous' || scopedState.status === 'unreadable') + return check( + RUNNER, + 'DRIFT', + `cannot determine the effective worktree core.hooksPath: ${scopedState.detail}`, + 'inspect `git config --show-origin --show-scope --get-all core.hooksPath`', + ); + const shared = sharedHooksPath(gitRoot); // Unset → git's default hooks dir, which every linked worktree shares via the common dir. if (!shared) { // ...but an INSTALLED hook that git will never reach is the same silent-no-gates failure, one @@ -251,14 +278,25 @@ export function checkHookRunner(cwd: string): CheckResult { 'run `bun install` (husky sets core.hooksPath), or `devkit init` for a husky-less install', ); } - const scoped = hooksPathAt(gitRoot, '--worktree'); - return check( - RUNNER, - 'OK', - scoped - ? `shared core.hooksPath unset (git default); this worktree overrides to ${scoped}` - : 'core.hooksPath unset (git default, shared with worktrees)', - ); + const scoped = worktreeScopedPin(gitRoot); + if (!scoped) + return check(RUNNER, 'OK', 'core.hooksPath unset (git default, shared with worktrees)'); + // A per-checkout override is healthy only while it points at something this checkout owns. With + // no shared value behind it, an override at a SIBLING checkout means the only hooks that run + // here are someone else's — reporting that as OK would state the defect as health, and would + // also hand `sync-hook-runner` a false licence to clear the one thing still gating. + return foreignPin(gitRoot) + ? check( + RUNNER, + 'DRIFT', + `shared core.hooksPath unset; this checkout overrides to ${scoped}, which is not its own`, + `see the "${OWNER}" check`, + ) + : check( + RUNNER, + 'OK', + `shared core.hooksPath unset (git default); this checkout overrides to ${scoped}`, + ); } // Absolute → inherited verbatim by every worktree; it only has to exist. if (isAbsolute(shared)) { @@ -331,11 +369,127 @@ export function checkHookRunner(cwd: string): CheckResult { ); } +/** + * Whose hooks a commit made HERE actually runs. + * + * Returns a LIST so the call site is one line: `doctor.mts` is at its recorded line ceiling, and the + * `CheckResult | null` shape used elsewhere costs two. Empty for every healthy repo, so ordinary + * `devkit doctor` output does not grow a row. + * + * Inspects the repo-wide (`--local`) and per-checkout (`config.worktree`) scopes only — the two + * devkit and husky write. A `core.hooksPath` arriving via `GIT_CONFIG_*`, `--global` or `--system` + * is invisible here, while `devkit review` reads the fully merged value and does see it; that split + * is documented in `docs/troubleshooting.md` rather than guessed at from this check's silence. + */ +export function checkHooksPathOwner(cwd: string): CheckResult[] { + const { gitRoot } = detectGitRoot(cwd); + const state = worktreeHooksPathState(gitRoot); + if (state.status === 'ambiguous' || state.status === 'unreadable') + return [ + check( + OWNER, + 'DRIFT', + `cannot establish this checkout's core.hooksPath ownership: ${state.detail}`, + 'inspect `git config --show-origin --show-scope --get-all core.hooksPath`; devkit will not repair an ambiguous value', + false, + ), + ]; + const pin = foreignPin(gitRoot); + if (!pin) { + // A benign per-checkout pin still shadows every repo-wide write — `devkit init`, `devkit clean`, + // and husky's own `prepare` all write at `--local` and would silently fail to take effect here. + // It is invisible to `git config --get`, so surface it even though nothing is wrong. + if (state.status !== 'single') return []; + const dir = hooksDir(gitRoot, state.value); + if (!isInsideResolved(gitRoot, dir)) + return existsSync(dir) + ? [ + check( + OWNER, + 'OK', + `this checkout pins an external core.hooksPath (${state.value}); it is not attributable to another checkout and is left unchanged`, + ), + ] + : [ + check( + OWNER, + 'MISSING', + `this checkout pins core.hooksPath at ${state.value}, which is external to every registered checkout and resolves to nothing`, + 'restore that external hooks directory or explicitly repoint this checkout; devkit cannot prove ownership and will not replace it', + false, + ), + ]; + return [check(OWNER, 'OK', `this checkout pins its own core.hooksPath (${state.value})`)]; + } + const scopeLabel = pin.scope === '--worktree' ? 'this checkout pins' : 'this repo pins'; + const shared = sharedHooksPath(gitRoot); + const manual = + pin.scope === '--worktree' + ? `git config --worktree core.hooksPath ${shared || '.husky/_'}` + : 'git config --unset core.hooksPath'; + // `--fix` is deliberately not offered: it regenerates FILE content from the recorded selection and + // never mutates git state. `sync-hook-runner` is devkit's one sanctioned mutator, and it refuses + // to clear anything until this checkout provably gates itself — so when it does not, the remedy + // has to be the ordered sequence that gets it there. Pointing straight at sync-hook-runner would + // send the user to a guaranteed no-op: it stages nothing while the shared value is unset. + const remedy = + pin.scope === '--local' + ? // Repo-wide: the pin IS the shared value, so there is no local fallback to be gated by and + // no per-checkout override to drop. Repointing it is a decision for the repo, not for us. + `${manual} — repo-wide, so devkit will not replace it for you; repoint it at a runner each checkout carries (e.g. .husky/_)` + : // Not self-gated YET is still sync-hook-runner's job whenever the reason is a runner it can + // stage: it stages first and re-reads, so one run tracks the runner and then drops the pin. + // Only when there is nothing to stage does the user have to go and produce a runner first. + selfGated(cwd, gitRoot) || unreachableRunnerFiles(gitRoot).length + ? `devkit sync-hook-runner (stages this checkout's own runner if needed, then replaces the sibling pin with ${shared}), or: ${manual}` + : 'this checkout has no runner of its own to fall back on — run `bun install` here (husky generates it and sets core.hooksPath), then `devkit sync-hook-runner`, then `devkit doctor`'; + return [ + check( + OWNER, + pin.exists ? 'DRIFT' : 'MISSING', + `${scopeLabel} core.hooksPath at ${pin.dir} — owned by sibling checkout ${pin.siblingRoot}${pin.exists ? '' : ' (now missing)'}, so commits made here ${pin.exists ? 'run ITS hooks' : 'run no hooks'}, not this checkout's own ${shared || '(none)'}`, + remedy, + false, + ), + ]; +} + +/** + * The per-checkout `core.hooksPath` that `devkit sync-hook-runner` may safely replace, or null. + * + * Four conditions, all load-bearing. The pin must be structurally per-checkout, because changing a + * repo-wide value would silently re-wire every other checkout too. It must point somewhere that is + * positively owned by an enumerated sibling, rather than an intentional external hook directory. + * The shared fallback must be relative and stay inside this checkout. And this checkout must already + * gate itself, or the "heal" swaps a working sibling hook for no hook at all — on every `bun install`, + * since the command is chained into the generated `prepare`. + */ +export interface ReplaceableHooksPathPin { + from: string; + to: string; +} + +export function replaceableHooksPathPin(cwd: string): ReplaceableHooksPathPin | null { + const { gitRoot } = detectGitRoot(cwd); + const pin = foreignPin(gitRoot); + if (pin?.scope !== '--worktree') return null; + const shared = sharedHooksPath(gitRoot); + if (!shared || isAbsolute(shared)) return null; + const fallback = hooksDir(gitRoot, shared); + if (!isInside(gitRoot, fallback)) return null; + return selfGated(cwd, gitRoot) ? { from: pin.value, to: shared } : null; +} + /** * Every hook-shaped check, as one list. Exists so `devkit doctor` can gain a hook check without * growing its own call site — cli/commands/doctor.mts sits on its recorded size budget and the * ratchet is shrink-only. */ export function hookChecks(cwd: string, guards: string[]): CheckResult[] { - return [checkHusky(cwd, guards), checkHookRunner(cwd), checkFailOpenGuards(cwd)]; + return [ + checkHusky(cwd, guards), + checkHookRunner(cwd), + ...checkHooksPathOwner(cwd), + checkFailOpenGuards(cwd), + ]; } diff --git a/cli/lib/doctor/hooks-path.mts b/cli/lib/doctor/hooks-path.mts new file mode 100644 index 0000000..0773758 --- /dev/null +++ b/cli/lib/doctor/hooks-path.mts @@ -0,0 +1,202 @@ +/** + * Reading `core.hooksPath` at the right scope, and resolving it the way git does. + * + * Split out of `hook-checks.mts` (which is at its line budget) because getting this right takes more + * care than the checks that consume it. Two traps in particular: + * + * - `git config --worktree` is only per-checkout when `extensions.worktreeConfig` is enabled. + * Otherwise it silently reads — and `--unset` silently DELETES — the repo-wide value, while + * every symptom says "worktree scope". Scope has to be established structurally, from the + * extension plus the file the value actually lives in. + * - a relative `core.hooksPath` must be resolved LEXICALLY. devkit's own ship gate worktrees are + * handed a `.husky/_` that is a symlink into the main checkout, so following one through its + * link target would call every `devkit ship` a defect. + * + * `rev-parse --git-path hooks` looks like the obvious way to ask "where do hooks come from" and is + * not usable here: it HONOURS core.hooksPath, so it echoes the pin back instead of resolving it. + */ + +import { execFileSync, spawnSync } from 'node:child_process'; +import { existsSync, realpathSync } from 'node:fs'; +import { isAbsolute, join, relative, resolve, sep } from 'node:path'; + +/** Trimmed stdout of a git command run against `gitRoot`; '' when git fails for any reason. */ +export function gitOut(gitRoot: string, args: string[]): string { + try { + return execFileSync('git', ['-C', gitRoot, ...args], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + } catch { + return ''; + } +} + +/** The repo-wide `core.hooksPath` — the one `git worktree add` hands to every new checkout, and the + * one this checkout falls back to if its own pin is cleared. '' when unset. Read at `--local` and + * never at `--worktree`, for the reason in this module's header. */ +export function sharedHooksPath(gitRoot: string): string { + return gitOut(gitRoot, ['config', '--local', '--get', 'core.hooksPath']); +} + +export type WorktreePinState = + | { status: 'none' } + | { status: 'single'; value: string; file: string } + | { status: 'ambiguous' | 'unreadable'; detail: string }; + +/** The worktree-owned value parsed by git itself. Includes and duplicates are deliberately + * non-repairable: scope decides whether sync-hook-runner may write, so a plausible value is not + * enough. Reading `git config --worktree` is unsafe here because it aliases `--local` while the + * extension is disabled. */ +export function worktreeHooksPathState(gitRoot: string): WorktreePinState { + const enabled = gitOut(gitRoot, [ + 'config', + '--local', + '--type=bool', + '--get', + 'extensions.worktreeConfig', + ]); + if (enabled !== 'true') return { status: 'none' }; + const gitDir = gitOut(gitRoot, ['rev-parse', '--absolute-git-dir']); + if (!gitDir) return { status: 'unreadable', detail: 'git directory is unreadable' }; + const file = join(gitDir, 'config.worktree'); + if (!existsSync(file)) return { status: 'none' }; + const result = spawnSync( + 'git', + [ + '-C', + gitRoot, + 'config', + '--file', + file, + '--includes', + '--show-origin', + '--null', + '--get-all', + 'core.hooksPath', + ], + { encoding: 'utf8' }, + ); + if (result.status === 1) return { status: 'none' }; + if (result.status !== 0) + return { status: 'unreadable', detail: 'config.worktree could not be parsed by git' }; + const fields = (result.stdout ?? '').split('\0'); + if (fields.at(-1) === '') fields.pop(); + if (fields.length !== 2) + return { + status: 'ambiguous', + detail: `${fields.length / 2 || 0} worktree-scoped core.hooksPath values`, + }; + const [origin, value] = fields; + const originFile = origin.startsWith('file:') ? origin.slice('file:'.length) : ''; + if (!originFile || !sameDir(originFile, file)) + return { + status: 'ambiguous', + detail: `core.hooksPath comes from an included config (${origin || 'unknown origin'})`, + }; + return { status: 'single', value, file }; +} + +/** The single unambiguous value recorded for THIS checkout, or ''. */ +export function worktreeScopedPin(gitRoot: string): string { + const state = worktreeHooksPathState(gitRoot); + return state.status === 'single' ? state.value : ''; +} + +/** Is `target` `root` itself, or inside it? Compares path SEGMENTS, so `/a/bc` is not inside `/a/b`. */ +export function isInside(root: string, target: string): boolean { + const rel = relative(root, target); + return rel === '' || (rel !== '..' && !rel.startsWith(`..${sep}`) && !isAbsolute(rel)); +} + +/** Where git will look for hooks: a relative value resolved against the working tree root. Lexical + * by design — `resolve`, never `realpathSync` (see the header). */ +export function hooksDir(gitRoot: string, value: string): string { + return isAbsolute(value) ? value : resolve(gitRoot, value); +} + +/** `isInside` that also survives macOS's `/tmp`→`/private/tmp` and `/var`→`/private/var` symlinks, + * which would otherwise make a checkout's own ABSOLUTE path look foreign. Only reached after the + * lexical test has already said "outside". */ +export function isInsideResolved(root: string, target: string): boolean { + if (isInside(root, target)) return true; + // macOS reports registered worktrees through /private even after the checkout disappears, when + // realpath can no longer normalize the configured /tmp or /var spelling for us. + const alias = (path: string) => + path.startsWith('/tmp/') || path.startsWith('/var/') ? `/private${path}` : path; + if (isInside(alias(root), alias(target))) return true; + try { + return isInside(realpathSync(root), realpathSync(target)); + } catch { + return false; + } +} + +/** Same directory, tolerant of the `/private` aliasing. Load-bearing: git reports absolute paths + * realpath'd (`/private/var/…`) while a configured value keeps whatever form it was written in + * (`/var/…`), so a purely lexical comparison calls a pin that restates its own fallback a shadow. */ +function sameDir(a: string, b: string): boolean { + return isInsideResolved(a, b) && isInsideResolved(b, a); +} + +/** Every checkout of this repo, main worktree included. */ +function checkouts(gitRoot: string): string[] { + const result = spawnSync('git', ['-C', gitRoot, 'worktree', 'list', '--porcelain', '-z'], { + encoding: 'utf8', + }); + if (result.status !== 0) return []; + return (result.stdout ?? '') + .split('\0') + .filter((field) => field.startsWith('worktree ')) + .map((field) => field.slice('worktree '.length)); +} + +export interface ForeignPin { + /** The configured value, verbatim — what a remedy has to name. */ + value: string; + /** Where it resolves to. */ + dir: string; + scope: '--worktree' | '--local'; + exists: boolean; + /** The enumerated sibling checkout that supplies positive ownership provenance. */ + siblingRoot: string; +} + +/** + * The hooks directory THIS checkout will actually use, when it belongs to somebody else. + * + * A per-checkout `core.hooksPath` pinned at a sibling checkout's runner is how a commit made in + * worktree A comes to be gated by checkout B's hook — B's version of the gates, from B's branch, + * with no error and nothing in the usual `git config --get` output to suggest it. Tooling writes it + * when a fresh worktree has no runner of its own, back when husky gitignored the one it pins; once + * the runner is tracked and reaches every checkout, the pin only shadows a working local one. + * + * Three deliberate silences, each a working configuration this must not turn red: + * - a value resolving INSIDE this checkout, however it was spelled (relative, or absolute through + * a `/private` symlink); + * - a value that merely restates what this checkout would use anyway — a pin naming the very + * fallback it shadows changes nothing; + * - any value landing outside every checkout of this repo, e.g. an org-shared + * `/opt/company/githooks`. Scope expresses precedence, not ownership intent, so external and + * already-pruned targets are diagnostic-only. Only Git's own worktree registry is positive + * provenance for the borrowed-runner defect. + */ +export function foreignPin(gitRoot: string): ForeignPin | null { + const scoped = worktreeScopedPin(gitRoot); + const shared = sharedHooksPath(gitRoot); + const value = scoped || shared; + // Unset → git's own hooks dir, which every linked worktree shares via the common dir. + if (!value) return null; + const dir = hooksDir(gitRoot, value); + if (isInsideResolved(gitRoot, dir)) return null; + // What this checkout would fall back to if the value vanished. + const common = gitOut(gitRoot, ['rev-parse', '--path-format=absolute', '--git-common-dir']); + const fallback = scoped && shared ? hooksDir(gitRoot, shared) : join(common, 'hooks'); + if (sameDir(dir, fallback)) return null; + const scope = scoped ? '--worktree' : '--local'; + const siblingRoot = checkouts(gitRoot).find( + (wt) => !sameDir(gitRoot, wt) && isInsideResolved(wt, dir), + ); + if (!siblingRoot) return null; + return { value, dir, scope, exists: existsSync(dir), siblingRoot }; +} diff --git a/cli/lib/doctor/self-host-doctor.mts b/cli/lib/doctor/self-host-doctor.mts index 76039bf..c19fda9 100644 --- a/cli/lib/doctor/self-host-doctor.mts +++ b/cli/lib/doctor/self-host-doctor.mts @@ -21,7 +21,7 @@ import { checkAdhdSkill } from '../install/adhd-skill.mts'; import { checkAgents, checkSkills } from './asset-checks.mts'; import type { CheckResult } from './check-result.mts'; import { adviseSearchIndex } from './guard-config-checks.mts'; -import { checkHookRunner } from './hook-checks.mts'; +import { checkHookRunner, checkHooksPathOwner } from './hook-checks.mts'; import { printStrayGateCalls } from './stray-gate-calls.mts'; import { inspectHookFailOpen, renderUnguardedGateCalls } from './unguarded-gate-calls.mts'; @@ -96,9 +96,13 @@ export async function runSelfHostDoctor( // The dogfood repo is gated by the same mechanism devkit ships to consumers, so it owes itself the // same worktree-safety verdict — a self-host repo whose runner is unreachable gates nothing either. - const runner = checkHookRunner(cwd); - console.log(` ${runner.status === 'OK' ? '✓' : '⚠'} ${runner.name}: ${runner.detail}`); - if (runner.status !== 'OK') console.log(` → ${runner.remediation}`); + // For the same reason it owes itself the ownership verdict: devkit is developed almost entirely + // from linked worktrees, which is exactly where a foreign core.hooksPath hides. + const hookState = [checkHookRunner(cwd), ...checkHooksPathOwner(cwd)]; + for (const r of hookState) { + console.log(` ${r.status === 'OK' ? '✓' : '⚠'} ${r.name}: ${r.detail}`); + if (r.status !== 'OK') console.log(` → ${r.remediation}`); + } - return hookOk && runner.status === 'OK' ? 0 : 1; + return hookOk && hookState.every((r) => r.status === 'OK') ? 0 : 1; } diff --git a/cli/lib/ship/review/setup-manifest.mts b/cli/lib/ship/review/setup-manifest.mts index fd4b446..3265561 100644 --- a/cli/lib/ship/review/setup-manifest.mts +++ b/cli/lib/ship/review/setup-manifest.mts @@ -2,7 +2,7 @@ import { spawnSync } from 'node:child_process'; import { lstatSync, readdirSync, readFileSync, realpathSync } from 'node:fs'; -import { dirname, join, relative, resolve, sep } from 'node:path'; +import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; import { writeFileAtomic } from '../../atomic-write.mts'; import type { ReviewProfile } from '../../components.mts'; import { detectGitRoot } from '../../detect-git-root.mts'; @@ -256,7 +256,11 @@ function effectiveHooksPath( if (!overlay) { if (value !== '.husky/_') fail( - `core.hooksPath is ${JSON.stringify(value || '(unset)')}, expected .husky/_ — ${DOCTOR}`, + `core.hooksPath is ${JSON.stringify(value || '(unset)')}, expected .husky/_ — ${ + isAbsolute(value) + ? `run 'devkit doctor' for the ownership diagnosis, then 'devkit sync-hook-runner' to repair a proven sibling-worktree pin; otherwise repoint it explicitly: git config core.hooksPath .husky/_` + : DOCTOR + }`, ); return value; } diff --git a/dist/cli/commands/sync/sync-hook-runner.mjs b/dist/cli/commands/sync/sync-hook-runner.mjs index fc22a9a..21706ae 100644 --- a/dist/cli/commands/sync/sync-hook-runner.mjs +++ b/dist/cli/commands/sync/sync-hook-runner.mjs @@ -1,52 +1,166 @@ /** - * `devkit sync-hook-runner` — stage (`git add -f`) whatever husky-generated runner files this repo - * needs that are currently untracked AND gitignored, so a fresh `git worktree add` can actually - * reach them. + * `devkit sync-hook-runner` — make THIS checkout gate itself with its own hooks. * - * Husky pins a RELATIVE `core.hooksPath` (`.husky/_`) and gitignores the runner it points at - * (`.husky/_/.gitignore` = `*`). A linked worktree checks out with hooksPath resolving to a MISSING - * directory — git treats "no runner" as "no hooks", so every commit made there is silently ungated. - * Tracking the runner (force-adding past husky's own ignore) fixes it permanently: a tracked file - * checks out into every worktree, so the relative path resolves everywhere. + * Two halves of one guarantee, both about a hook runner that a checkout cannot reach: * - * `devkit init` chains this into a fresh package-mode install's `prepare` script, so no NEW repo - * ever needs a manual `git add -f` — every `bun install` re-stages it if husky's install regenerated - * an untracked runner (it never will once tracked, but the chain is idempotent either way). + * 1. Stage (`git add -f`) whatever husky-generated runner files this repo needs that are currently + * untracked AND gitignored. Husky pins a RELATIVE `core.hooksPath` (`.husky/_`) and gitignores the + * runner it points at (`.husky/_/.gitignore` = `*`), so a linked worktree checks out with + * hooksPath resolving to a MISSING directory — git treats "no runner" as "no hooks", and every + * commit made there is silently ungated. Tracking the runner fixes it permanently: a tracked file + * checks out into every worktree, so the relative path resolves everywhere. + * + * 2. Replace a per-checkout `core.hooksPath` left pinned at ANOTHER checkout's runner. That pin is the + * older workaround for exactly the same problem — when a fresh worktree had no runner of its own, + * borrowing the main checkout's beat having none. Once (1) holds it stops being a workaround and + * becomes a bug: commits made here run the OTHER checkout's version of the hook, off the OTHER + * checkout's branch, with no error. Order matters — staging first can be what makes this checkout + * self-gated, and the pin is only replaced with the validated shared relative path once it is. + * + * `devkit init` chains this into a fresh package-mode install's `prepare` script, so no NEW repo ever + * needs a manual `git add -f` — every `bun install` re-stages the runner and re-checks the pin. * * A dedicated, explicitly-invoked command rather than folded into `devkit doctor --fix`: --fix only - * ever regenerates FILE content from the recorded selection, never mutates the git INDEX — staging - * is something the caller (a human, or their own prepare script) must ask for. + * ever regenerates FILE content from the recorded selection, never mutates the git INDEX or git + * CONFIG — both are things the caller (a human, or their own prepare script) must ask for. * * devkit sync-hook-runner [--dry-run] */ import { execFileSync } from 'node:child_process'; +import { closeSync, copyFileSync, openSync, readFileSync, renameSync, rmSync, writeFileSync, } from 'node:fs'; import { detectGitRoot } from "../../lib/detect-git-root.mjs"; -import { unreachableRunnerFiles } from "../../lib/doctor/hook-checks.mjs"; +import { checkHookRunner, replaceableHooksPathPin, unreachableRunnerFiles, } from "../../lib/doctor/hook-checks.mjs"; +import { sharedHooksPath, worktreeHooksPathState } from "../../lib/doctor/hooks-path.mjs"; export const meta = { name: 'sync-hook-runner', - summary: 'Stage the husky hook runner so it survives `git worktree add` (git add -f).', - help: `devkit sync-hook-runner — force-add whatever husky-generated runner files this repo needs -that are untracked AND gitignored, so a fresh \`git worktree add\` can reach them. + summary: 'Make this checkout run its OWN hooks (stage the runner; replace a sibling hooksPath pin).', + help: `devkit sync-hook-runner — make this checkout gate itself with its own hooks. Usage: devkit sync-hook-runner [--dry-run] -A no-op (exit 0) when the runner is already fully tracked, or when core.hooksPath is unset/absolute -(nothing in-repo to stage in either case). Chained into a fresh \`devkit init\`'s package.json -"prepare" script — every \`bun install\` self-heals, so this rarely needs a manual run.`, +Force-adds whatever husky-generated runner files this repo needs that are untracked AND gitignored, +so a fresh \`git worktree add\` can reach them. Then, if this checkout pins core.hooksPath at ANOTHER +checkout's runner — the older workaround for that same gap — replaces that exact value with the +validated shared relative path, but only once this checkout provably gates itself. + +Exits 0 when there is nothing to do. Only ever touches a PER-CHECKOUT pin; a repo-wide +core.hooksPath is reported by \`devkit doctor\` and left alone. External central paths are never +replaced. Chained into a fresh \`devkit init\`'s +package.json "prepare" script — every \`bun install\` self-heals, so this rarely needs a manual run.`, }; +/** Restore the exact pre-write config only when nobody has changed our candidate since the failed + * verification. Re-acquiring Git's lock and comparing bytes makes rollback another CAS operation, + * rather than overwriting a writer that raced with the verifier. */ +function restoreConfig(file, original, candidate) { + const lockPath = `${file}.lock`; + let ownsLock = false; + try { + const lock = openSync(lockPath, 'wx'); + ownsLock = true; + closeSync(lock); + if (!readFileSync(file).equals(candidate)) + throw new Error('the replacement changed again before rollback'); + writeFileSync(lockPath, original); + renameSync(lockPath, file); + ownsLock = false; + return true; + } + catch { + if (ownsLock) + rmSync(lockPath, { force: true }); + return false; + } +} +/** Replace the exact sibling value while holding Git's own config.worktree lock. Git's + * `--fixed-value --replace-all` APPENDS when the old value no longer matches, so invoking it against + * the live file is not compare-and-swap. Instead we acquire the lock, revalidate the live value, + * transform a private copy through Git's parser, and atomically rename that copy into place. */ +export function replacePin(cwd, gitRoot, pin) { + let lockPath = ''; + let ownsLock = false; + let replacedFile = ''; + let original = null; + let candidateContents = null; + try { + const before = worktreeHooksPathState(gitRoot); + if (before.status !== 'single' || before.value !== pin.from) + throw new Error('the worktree value changed before replacement'); + lockPath = `${before.file}.lock`; + const lock = openSync(lockPath, 'wx'); + ownsLock = true; + closeSync(lock); + // A writer may have won immediately before our lock. Re-read the LIVE file only after every + // cooperating Git writer is excluded, then abort unless the captured repair plan is still exact. + const locked = worktreeHooksPathState(gitRoot); + if (locked.status !== 'single' || locked.file !== before.file || locked.value !== pin.from) + throw new Error('the worktree value changed while acquiring the config lock'); + const revalidated = replaceableHooksPathPin(cwd); + if (!revalidated || revalidated.from !== pin.from || revalidated.to !== pin.to) + throw new Error('this checkout stopped being a safe replacement target'); + original = readFileSync(locked.file); + copyFileSync(locked.file, lockPath); + execFileSync('git', [ + '-C', + gitRoot, + 'config', + '--file', + lockPath, + '--fixed-value', + '--replace-all', + 'core.hooksPath', + pin.to, + pin.from, + ], { stdio: ['ignore', 'ignore', 'ignore'] }); + const candidate = execFileSync('git', ['-C', gitRoot, 'config', '--file', lockPath, '--null', '--get-all', 'core.hooksPath'], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }) + .split('\0') + .filter((value, index, all) => value !== '' || index < all.length - 1); + if (candidate.length !== 1 || candidate[0] !== pin.to) + throw new Error('the locked replacement did not produce one exact fallback value'); + candidateContents = readFileSync(lockPath); + renameSync(lockPath, locked.file); + ownsLock = false; + replacedFile = locked.file; + } + catch (e) { + if (ownsLock) + rmSync(lockPath, { force: true }); + const msg = e instanceof Error ? e.message.split('\n')[0] : ''; + console.log(`devkit sync-hook-runner: did not replace core.hooksPath (${msg || 'the value changed or git refused the update'}) — the live config was left intact`); + return false; + } + const after = worktreeHooksPathState(gitRoot); + if (after.status !== 'single' || + after.value !== pin.to || + sharedHooksPath(gitRoot) !== pin.to || + checkHookRunner(cwd).status !== 'OK') { + const restored = original !== null && candidateContents !== null + ? restoreConfig(replacedFile, original, candidateContents) + : false; + console.log(`devkit sync-hook-runner: the locked core.hooksPath replacement did not verify cleanly and ${restored ? 'the prior pin was restored' : 'could not be rolled back safely'} — inspect with 'devkit doctor'`); + return false; + } + console.log(`devkit sync-hook-runner: replaced sibling core.hooksPath ${pin.from} with ${pin.to} — this checkout now runs its own hooks`); + return true; +} export default function run(args, cwd) { const { gitRoot } = detectGitRoot(cwd); + const dryRun = args.includes('--dry-run'); const files = unreachableRunnerFiles(gitRoot); - if (!files.length) { - console.log('devkit sync-hook-runner: hook runner already reachable — nothing to stage'); - return 0; - } - if (args.includes('--dry-run')) { + if (files.length && dryRun) console.log(`devkit sync-hook-runner: [dry-run] would git add -f ${files.join(' ')}`); - return 0; + else if (files.length) { + execFileSync('git', ['-C', gitRoot, 'add', '-f', ...files], { stdio: 'inherit' }); + console.log(`devkit sync-hook-runner: staged ${files.join(', ')}`); } - execFileSync('git', ['-C', gitRoot, 'add', '-f', ...files], { stdio: 'inherit' }); - console.log(`devkit sync-hook-runner: staged ${files.join(', ')}`); + // Read AFTER staging: force-adding the runner is one of the things that can make this checkout + // self-gated, and therefore make the pin safe to drop in the same run. + const pin = replaceableHooksPathPin(cwd); + if (pin && dryRun) + console.log(`devkit sync-hook-runner: [dry-run] would replace sibling core.hooksPath ${pin.from} with ${pin.to}`); + else if (pin && !replacePin(cwd, gitRoot, pin)) + return 1; + if (!files.length && !pin) + console.log('devkit sync-hook-runner: this checkout already runs its own hooks — nothing to do'); return 0; } diff --git a/dist/cli/lib/doctor/hook-checks.mjs b/dist/cli/lib/doctor/hook-checks.mjs index 750d40d..889af51 100644 --- a/dist/cli/lib/doctor/hook-checks.mjs +++ b/dist/cli/lib/doctor/hook-checks.mjs @@ -1,7 +1,9 @@ /** - * The two pre-commit hook health checks, kept together because they answer complementary halves of + * The three pre-commit hook health checks, kept together because they answer complementary parts of * one question: `checkHusky` asks whether the hook exists and still calls the selected gates in THIS - * checkout; `checkHookRunner` asks whether that hook survives `git worktree add` at all. + * checkout; `checkHookRunner` asks whether that hook survives `git worktree add` at all; and + * `checkHooksPathOwner` asks whose hook a commit made HERE actually runs — delivery into a new + * checkout and ownership of the current one being different failures with the same symptom. * * They live here beside the other doctor checks (see `asset-checks.mts`) rather than in * `doctor.mts`, which is at its line budget. @@ -15,6 +17,7 @@ import { markEnd, markStart } from "../husky/husky.mjs"; import { extractGuardBlock, QAVIS_ADVISORY_ID } from "../husky/husky-block.mjs"; import { firstLine } from "../standalone.mjs"; import { check } from "./check-result.mjs"; +import { foreignPin, hooksDir, isInside, isInsideResolved, sharedHooksPath, worktreeHooksPathState, worktreeScopedPin, } from "./hooks-path.mjs"; import { strayGateCalls } from "./stray-gate-calls.mjs"; import { checkFailOpenGuards } from "./unguarded-gate-calls.mjs"; /** @@ -100,6 +103,10 @@ export function checkHusky(cwd, selectedGuards) { return check('.husky/pre-commit', 'OK', gates.length ? `block calls: ${gates.join(', ')}` : 'block present (no guards selected)'); } const RUNNER = 'hook runner (worktree-safe)'; +// Named apart from RUNNER on purpose: the two answer different questions and would read as +// contradictory duplicates side by side. RUNNER judges DELIVERY into a new checkout; OWNER judges +// which checkout's hooks run in THIS one. +const OWNER = 'hooksPath owner'; /** Git's hook names — the same set husky generates stubs for. Used to tell a real hook apart from * an unrelated file sitting in the hooks directory. */ const GIT_HOOKS = new Set([ @@ -118,19 +125,6 @@ const GIT_HOOKS = new Set([ 'pre-auto-gc', 'post-rewrite', ]); -/** A `core.hooksPath` at one config scope. Absent/unreadable (e.g. `--worktree` without - * `extensions.worktreeConfig`) reads as ''. */ -function hooksPathAt(gitRoot, scope) { - try { - return execFileSync('git', ['-C', gitRoot, 'config', scope, '--get', 'core.hooksPath'], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); - } - catch { - return ''; - } -} function gitSucceeds(gitRoot, args) { try { execFileSync('git', ['-C', gitRoot, ...args], { stdio: 'ignore' }); @@ -160,6 +154,23 @@ function defaultHookDelegatesToHusky(gitRoot) { return false; } } +/** + * Does this checkout gate itself — would the hooks that run once the pin is gone actually be its + * own, and actually run? + * + * Reuses `checkHookRunner`'s whole verdict rather than a second existence test, because a runner + * DIRECTORY can be there while nothing runs: an empty runner dir, or a hook declared in `.husky/` + * whose stub was never generated, are both states it already reports and `existsSync` cannot see. + * Clearing a pin in either state would swap the sibling checkout's working hook for nothing at all. + * The containment test is separate and comes first: `checkHookRunner` judges the shared value's + * health, not whether that value points at US. + */ +function selfGated(cwd, gitRoot) { + const shared = sharedHooksPath(gitRoot); + if (!shared || !isInside(gitRoot, hooksDir(gitRoot, shared))) + return false; + return checkHookRunner(cwd).status === 'OK'; +} // A file reaches a new worktree iff it is TRACKED. Merely-untracked is transient (the next commit // carries it), but untracked AND IGNORED is permanent: no ordinary `git add` can ever pick it up. // That pairing — load-bearing yet unreachable — is the actual defect. @@ -178,7 +189,7 @@ function isUnreachable(gitRoot, relPath) { * already-reviewed check's shape stays untouched. */ export function unreachableRunnerFiles(gitRoot) { - const shared = hooksPathAt(gitRoot, '--local'); + const shared = sharedHooksPath(gitRoot); if (!shared || isAbsolute(shared) || !existsSync(join(gitRoot, shared))) return []; const huskyDir = join(gitRoot, '.husky'); @@ -203,7 +214,10 @@ export function unreachableRunnerFiles(gitRoot) { // Detection only (never `fixable`): the repair stages files, which `--fix` must not do unasked. export function checkHookRunner(cwd) { const { gitRoot } = detectGitRoot(cwd); - const shared = hooksPathAt(gitRoot, '--local'); + const scopedState = worktreeHooksPathState(gitRoot); + if (scopedState.status === 'ambiguous' || scopedState.status === 'unreadable') + return check(RUNNER, 'DRIFT', `cannot determine the effective worktree core.hooksPath: ${scopedState.detail}`, 'inspect `git config --show-origin --show-scope --get-all core.hooksPath`'); + const shared = sharedHooksPath(gitRoot); // Unset → git's default hooks dir, which every linked worktree shares via the common dir. if (!shared) { // ...but an INSTALLED hook that git will never reach is the same silent-no-gates failure, one @@ -216,10 +230,16 @@ export function checkHookRunner(cwd) { !defaultHookDelegatesToHusky(gitRoot)) { return check(RUNNER, 'DRIFT', '.husky/pre-commit is installed but core.hooksPath is unset — git runs its own hooks dir and never reaches it, so NOTHING gates', 'run `bun install` (husky sets core.hooksPath), or `devkit init` for a husky-less install'); } - const scoped = hooksPathAt(gitRoot, '--worktree'); - return check(RUNNER, 'OK', scoped - ? `shared core.hooksPath unset (git default); this worktree overrides to ${scoped}` - : 'core.hooksPath unset (git default, shared with worktrees)'); + const scoped = worktreeScopedPin(gitRoot); + if (!scoped) + return check(RUNNER, 'OK', 'core.hooksPath unset (git default, shared with worktrees)'); + // A per-checkout override is healthy only while it points at something this checkout owns. With + // no shared value behind it, an override at a SIBLING checkout means the only hooks that run + // here are someone else's — reporting that as OK would state the defect as health, and would + // also hand `sync-hook-runner` a false licence to clear the one thing still gating. + return foreignPin(gitRoot) + ? check(RUNNER, 'DRIFT', `shared core.hooksPath unset; this checkout overrides to ${scoped}, which is not its own`, `see the "${OWNER}" check`) + : check(RUNNER, 'OK', `shared core.hooksPath unset (git default); this checkout overrides to ${scoped}`); } // Absolute → inherited verbatim by every worktree; it only has to exist. if (isAbsolute(shared)) { @@ -261,11 +281,90 @@ export function checkHookRunner(cwd) { } return check(RUNNER, 'OK', `runner reachable (${required.length} files) — survives \`git worktree add\``); } +/** + * Whose hooks a commit made HERE actually runs. + * + * Returns a LIST so the call site is one line: `doctor.mts` is at its recorded line ceiling, and the + * `CheckResult | null` shape used elsewhere costs two. Empty for every healthy repo, so ordinary + * `devkit doctor` output does not grow a row. + * + * Inspects the repo-wide (`--local`) and per-checkout (`config.worktree`) scopes only — the two + * devkit and husky write. A `core.hooksPath` arriving via `GIT_CONFIG_*`, `--global` or `--system` + * is invisible here, while `devkit review` reads the fully merged value and does see it; that split + * is documented in `docs/troubleshooting.md` rather than guessed at from this check's silence. + */ +export function checkHooksPathOwner(cwd) { + const { gitRoot } = detectGitRoot(cwd); + const state = worktreeHooksPathState(gitRoot); + if (state.status === 'ambiguous' || state.status === 'unreadable') + return [ + check(OWNER, 'DRIFT', `cannot establish this checkout's core.hooksPath ownership: ${state.detail}`, 'inspect `git config --show-origin --show-scope --get-all core.hooksPath`; devkit will not repair an ambiguous value', false), + ]; + const pin = foreignPin(gitRoot); + if (!pin) { + // A benign per-checkout pin still shadows every repo-wide write — `devkit init`, `devkit clean`, + // and husky's own `prepare` all write at `--local` and would silently fail to take effect here. + // It is invisible to `git config --get`, so surface it even though nothing is wrong. + if (state.status !== 'single') + return []; + const dir = hooksDir(gitRoot, state.value); + if (!isInsideResolved(gitRoot, dir)) + return existsSync(dir) + ? [ + check(OWNER, 'OK', `this checkout pins an external core.hooksPath (${state.value}); it is not attributable to another checkout and is left unchanged`), + ] + : [ + check(OWNER, 'MISSING', `this checkout pins core.hooksPath at ${state.value}, which is external to every registered checkout and resolves to nothing`, 'restore that external hooks directory or explicitly repoint this checkout; devkit cannot prove ownership and will not replace it', false), + ]; + return [check(OWNER, 'OK', `this checkout pins its own core.hooksPath (${state.value})`)]; + } + const scopeLabel = pin.scope === '--worktree' ? 'this checkout pins' : 'this repo pins'; + const shared = sharedHooksPath(gitRoot); + const manual = pin.scope === '--worktree' + ? `git config --worktree core.hooksPath ${shared || '.husky/_'}` + : 'git config --unset core.hooksPath'; + // `--fix` is deliberately not offered: it regenerates FILE content from the recorded selection and + // never mutates git state. `sync-hook-runner` is devkit's one sanctioned mutator, and it refuses + // to clear anything until this checkout provably gates itself — so when it does not, the remedy + // has to be the ordered sequence that gets it there. Pointing straight at sync-hook-runner would + // send the user to a guaranteed no-op: it stages nothing while the shared value is unset. + const remedy = pin.scope === '--local' + ? // Repo-wide: the pin IS the shared value, so there is no local fallback to be gated by and + // no per-checkout override to drop. Repointing it is a decision for the repo, not for us. + `${manual} — repo-wide, so devkit will not replace it for you; repoint it at a runner each checkout carries (e.g. .husky/_)` + : // Not self-gated YET is still sync-hook-runner's job whenever the reason is a runner it can + // stage: it stages first and re-reads, so one run tracks the runner and then drops the pin. + // Only when there is nothing to stage does the user have to go and produce a runner first. + selfGated(cwd, gitRoot) || unreachableRunnerFiles(gitRoot).length + ? `devkit sync-hook-runner (stages this checkout's own runner if needed, then replaces the sibling pin with ${shared}), or: ${manual}` + : 'this checkout has no runner of its own to fall back on — run `bun install` here (husky generates it and sets core.hooksPath), then `devkit sync-hook-runner`, then `devkit doctor`'; + return [ + check(OWNER, pin.exists ? 'DRIFT' : 'MISSING', `${scopeLabel} core.hooksPath at ${pin.dir} — owned by sibling checkout ${pin.siblingRoot}${pin.exists ? '' : ' (now missing)'}, so commits made here ${pin.exists ? 'run ITS hooks' : 'run no hooks'}, not this checkout's own ${shared || '(none)'}`, remedy, false), + ]; +} +export function replaceableHooksPathPin(cwd) { + const { gitRoot } = detectGitRoot(cwd); + const pin = foreignPin(gitRoot); + if (pin?.scope !== '--worktree') + return null; + const shared = sharedHooksPath(gitRoot); + if (!shared || isAbsolute(shared)) + return null; + const fallback = hooksDir(gitRoot, shared); + if (!isInside(gitRoot, fallback)) + return null; + return selfGated(cwd, gitRoot) ? { from: pin.value, to: shared } : null; +} /** * Every hook-shaped check, as one list. Exists so `devkit doctor` can gain a hook check without * growing its own call site — cli/commands/doctor.mts sits on its recorded size budget and the * ratchet is shrink-only. */ export function hookChecks(cwd, guards) { - return [checkHusky(cwd, guards), checkHookRunner(cwd), checkFailOpenGuards(cwd)]; + return [ + checkHusky(cwd, guards), + checkHookRunner(cwd), + ...checkHooksPathOwner(cwd), + checkFailOpenGuards(cwd), + ]; } diff --git a/dist/cli/lib/doctor/hooks-path.mjs b/dist/cli/lib/doctor/hooks-path.mjs new file mode 100644 index 0000000..b1ca055 --- /dev/null +++ b/dist/cli/lib/doctor/hooks-path.mjs @@ -0,0 +1,182 @@ +/** + * Reading `core.hooksPath` at the right scope, and resolving it the way git does. + * + * Split out of `hook-checks.mts` (which is at its line budget) because getting this right takes more + * care than the checks that consume it. Two traps in particular: + * + * - `git config --worktree` is only per-checkout when `extensions.worktreeConfig` is enabled. + * Otherwise it silently reads — and `--unset` silently DELETES — the repo-wide value, while + * every symptom says "worktree scope". Scope has to be established structurally, from the + * extension plus the file the value actually lives in. + * - a relative `core.hooksPath` must be resolved LEXICALLY. devkit's own ship gate worktrees are + * handed a `.husky/_` that is a symlink into the main checkout, so following one through its + * link target would call every `devkit ship` a defect. + * + * `rev-parse --git-path hooks` looks like the obvious way to ask "where do hooks come from" and is + * not usable here: it HONOURS core.hooksPath, so it echoes the pin back instead of resolving it. + */ +import { execFileSync, spawnSync } from 'node:child_process'; +import { existsSync, realpathSync } from 'node:fs'; +import { isAbsolute, join, relative, resolve, sep } from 'node:path'; +/** Trimmed stdout of a git command run against `gitRoot`; '' when git fails for any reason. */ +export function gitOut(gitRoot, args) { + try { + return execFileSync('git', ['-C', gitRoot, ...args], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + } + catch { + return ''; + } +} +/** The repo-wide `core.hooksPath` — the one `git worktree add` hands to every new checkout, and the + * one this checkout falls back to if its own pin is cleared. '' when unset. Read at `--local` and + * never at `--worktree`, for the reason in this module's header. */ +export function sharedHooksPath(gitRoot) { + return gitOut(gitRoot, ['config', '--local', '--get', 'core.hooksPath']); +} +/** The worktree-owned value parsed by git itself. Includes and duplicates are deliberately + * non-repairable: scope decides whether sync-hook-runner may write, so a plausible value is not + * enough. Reading `git config --worktree` is unsafe here because it aliases `--local` while the + * extension is disabled. */ +export function worktreeHooksPathState(gitRoot) { + const enabled = gitOut(gitRoot, [ + 'config', + '--local', + '--type=bool', + '--get', + 'extensions.worktreeConfig', + ]); + if (enabled !== 'true') + return { status: 'none' }; + const gitDir = gitOut(gitRoot, ['rev-parse', '--absolute-git-dir']); + if (!gitDir) + return { status: 'unreadable', detail: 'git directory is unreadable' }; + const file = join(gitDir, 'config.worktree'); + if (!existsSync(file)) + return { status: 'none' }; + const result = spawnSync('git', [ + '-C', + gitRoot, + 'config', + '--file', + file, + '--includes', + '--show-origin', + '--null', + '--get-all', + 'core.hooksPath', + ], { encoding: 'utf8' }); + if (result.status === 1) + return { status: 'none' }; + if (result.status !== 0) + return { status: 'unreadable', detail: 'config.worktree could not be parsed by git' }; + const fields = (result.stdout ?? '').split('\0'); + if (fields.at(-1) === '') + fields.pop(); + if (fields.length !== 2) + return { + status: 'ambiguous', + detail: `${fields.length / 2 || 0} worktree-scoped core.hooksPath values`, + }; + const [origin, value] = fields; + const originFile = origin.startsWith('file:') ? origin.slice('file:'.length) : ''; + if (!originFile || !sameDir(originFile, file)) + return { + status: 'ambiguous', + detail: `core.hooksPath comes from an included config (${origin || 'unknown origin'})`, + }; + return { status: 'single', value, file }; +} +/** The single unambiguous value recorded for THIS checkout, or ''. */ +export function worktreeScopedPin(gitRoot) { + const state = worktreeHooksPathState(gitRoot); + return state.status === 'single' ? state.value : ''; +} +/** Is `target` `root` itself, or inside it? Compares path SEGMENTS, so `/a/bc` is not inside `/a/b`. */ +export function isInside(root, target) { + const rel = relative(root, target); + return rel === '' || (rel !== '..' && !rel.startsWith(`..${sep}`) && !isAbsolute(rel)); +} +/** Where git will look for hooks: a relative value resolved against the working tree root. Lexical + * by design — `resolve`, never `realpathSync` (see the header). */ +export function hooksDir(gitRoot, value) { + return isAbsolute(value) ? value : resolve(gitRoot, value); +} +/** `isInside` that also survives macOS's `/tmp`→`/private/tmp` and `/var`→`/private/var` symlinks, + * which would otherwise make a checkout's own ABSOLUTE path look foreign. Only reached after the + * lexical test has already said "outside". */ +export function isInsideResolved(root, target) { + if (isInside(root, target)) + return true; + // macOS reports registered worktrees through /private even after the checkout disappears, when + // realpath can no longer normalize the configured /tmp or /var spelling for us. + const alias = (path) => path.startsWith('/tmp/') || path.startsWith('/var/') ? `/private${path}` : path; + if (isInside(alias(root), alias(target))) + return true; + try { + return isInside(realpathSync(root), realpathSync(target)); + } + catch { + return false; + } +} +/** Same directory, tolerant of the `/private` aliasing. Load-bearing: git reports absolute paths + * realpath'd (`/private/var/…`) while a configured value keeps whatever form it was written in + * (`/var/…`), so a purely lexical comparison calls a pin that restates its own fallback a shadow. */ +function sameDir(a, b) { + return isInsideResolved(a, b) && isInsideResolved(b, a); +} +/** Every checkout of this repo, main worktree included. */ +function checkouts(gitRoot) { + const result = spawnSync('git', ['-C', gitRoot, 'worktree', 'list', '--porcelain', '-z'], { + encoding: 'utf8', + }); + if (result.status !== 0) + return []; + return (result.stdout ?? '') + .split('\0') + .filter((field) => field.startsWith('worktree ')) + .map((field) => field.slice('worktree '.length)); +} +/** + * The hooks directory THIS checkout will actually use, when it belongs to somebody else. + * + * A per-checkout `core.hooksPath` pinned at a sibling checkout's runner is how a commit made in + * worktree A comes to be gated by checkout B's hook — B's version of the gates, from B's branch, + * with no error and nothing in the usual `git config --get` output to suggest it. Tooling writes it + * when a fresh worktree has no runner of its own, back when husky gitignored the one it pins; once + * the runner is tracked and reaches every checkout, the pin only shadows a working local one. + * + * Three deliberate silences, each a working configuration this must not turn red: + * - a value resolving INSIDE this checkout, however it was spelled (relative, or absolute through + * a `/private` symlink); + * - a value that merely restates what this checkout would use anyway — a pin naming the very + * fallback it shadows changes nothing; + * - any value landing outside every checkout of this repo, e.g. an org-shared + * `/opt/company/githooks`. Scope expresses precedence, not ownership intent, so external and + * already-pruned targets are diagnostic-only. Only Git's own worktree registry is positive + * provenance for the borrowed-runner defect. + */ +export function foreignPin(gitRoot) { + const scoped = worktreeScopedPin(gitRoot); + const shared = sharedHooksPath(gitRoot); + const value = scoped || shared; + // Unset → git's own hooks dir, which every linked worktree shares via the common dir. + if (!value) + return null; + const dir = hooksDir(gitRoot, value); + if (isInsideResolved(gitRoot, dir)) + return null; + // What this checkout would fall back to if the value vanished. + const common = gitOut(gitRoot, ['rev-parse', '--path-format=absolute', '--git-common-dir']); + const fallback = scoped && shared ? hooksDir(gitRoot, shared) : join(common, 'hooks'); + if (sameDir(dir, fallback)) + return null; + const scope = scoped ? '--worktree' : '--local'; + const siblingRoot = checkouts(gitRoot).find((wt) => !sameDir(gitRoot, wt) && isInsideResolved(wt, dir)); + if (!siblingRoot) + return null; + return { value, dir, scope, exists: existsSync(dir), siblingRoot }; +} diff --git a/dist/cli/lib/doctor/self-host-doctor.mjs b/dist/cli/lib/doctor/self-host-doctor.mjs index 3acfb1a..8992b18 100644 --- a/dist/cli/lib/doctor/self-host-doctor.mjs +++ b/dist/cli/lib/doctor/self-host-doctor.mjs @@ -12,7 +12,7 @@ import { buildSelfHostBlock, installSelfHostHook, SELF_HOST_EXTRAS, SELF_HOST_ST import { checkAdhdSkill } from "../install/adhd-skill.mjs"; import { checkAgents, checkSkills } from "./asset-checks.mjs"; import { adviseSearchIndex } from "./guard-config-checks.mjs"; -import { checkHookRunner } from "./hook-checks.mjs"; +import { checkHookRunner, checkHooksPathOwner } from "./hook-checks.mjs"; import { printStrayGateCalls } from "./stray-gate-calls.mjs"; import { inspectHookFailOpen, renderUnguardedGateCalls } from "./unguarded-gate-calls.mjs"; export async function runSelfHostDoctor(cwd, cfg, fix) { @@ -71,9 +71,13 @@ export async function runSelfHostDoctor(cwd, cfg, fix) { printQavisAdvisoryHealth(cwd, sel.guards ?? []); // The dogfood repo is gated by the same mechanism devkit ships to consumers, so it owes itself the // same worktree-safety verdict — a self-host repo whose runner is unreachable gates nothing either. - const runner = checkHookRunner(cwd); - console.log(` ${runner.status === 'OK' ? '✓' : '⚠'} ${runner.name}: ${runner.detail}`); - if (runner.status !== 'OK') - console.log(` → ${runner.remediation}`); - return hookOk && runner.status === 'OK' ? 0 : 1; + // For the same reason it owes itself the ownership verdict: devkit is developed almost entirely + // from linked worktrees, which is exactly where a foreign core.hooksPath hides. + const hookState = [checkHookRunner(cwd), ...checkHooksPathOwner(cwd)]; + for (const r of hookState) { + console.log(` ${r.status === 'OK' ? '✓' : '⚠'} ${r.name}: ${r.detail}`); + if (r.status !== 'OK') + console.log(` → ${r.remediation}`); + } + return hookOk && hookState.every((r) => r.status === 'OK') ? 0 : 1; } diff --git a/dist/cli/lib/ship/review/setup-manifest.mjs b/dist/cli/lib/ship/review/setup-manifest.mjs index 2ecc7cf..0664441 100644 --- a/dist/cli/lib/ship/review/setup-manifest.mjs +++ b/dist/cli/lib/ship/review/setup-manifest.mjs @@ -1,7 +1,7 @@ /** Stable, typed capture of the target-controlled setup that `devkit review` will execute. */ import { spawnSync } from 'node:child_process'; import { lstatSync, readdirSync, readFileSync, realpathSync } from 'node:fs'; -import { dirname, join, relative, resolve, sep } from 'node:path'; +import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; import { writeFileAtomic } from "../../atomic-write.mjs"; import { detectGitRoot } from "../../detect-git-root.mjs"; import { reviewHookDrift } from "../../husky/review-drift.mjs"; @@ -187,7 +187,9 @@ function effectiveHooksPath(root, overlay, context) { const value = readHooksPath(root); if (!overlay) { if (value !== '.husky/_') - fail(`core.hooksPath is ${JSON.stringify(value || '(unset)')}, expected .husky/_ — ${DOCTOR}`); + fail(`core.hooksPath is ${JSON.stringify(value || '(unset)')}, expected .husky/_ — ${isAbsolute(value) + ? `run 'devkit doctor' for the ownership diagnosis, then 'devkit sync-hook-runner' to repair a proven sibling-worktree pin; otherwise repoint it explicitly: git config core.hooksPath .husky/_` + : DOCTOR}`); return value; } const rejection = overlayHooksPathRejection(value, context); diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 9aeafb0..8b224e9 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -28,7 +28,7 @@ timeline. New rationale lives in the per-axis file. | [decisions-recall-suite](decisions-recall-suite.md) | A new suite (gate-engine/decisions/eval/recall/) scores guard-decisions query --json with pure set arithmetic and substring matching — zero LLM calls, seconds per run, therefore affordable per-commit. It reports FIVE independent metric families against five separate denominators and NO headline number: Containment@5, Buried@5 (conditional on containment), CSA/SFER, FANR/FAR as a raw 2x2, and SetRecall + macro PartialRecall. The corpus is always a FROZEN snapshot hashed as storeHash, never the live tree. Two tiers: a committed neutralised seed corpus that reproduces real measured pathologies (both schema generations, a re-target, a Target falsified by its own notes, duplicate live rulings, an INDEX hole, a dead INDEX row) which CI scores; and an uncommitted local snapshot via --corpus for evidence-only numbers. A mechanical token-Jaccard leakage gate fails any question above 0.5 overlap with its gold axis. | Retrieval quality was measured by three unit assertions and nothing… | 2026-07-25 | | [detect-judge-evidence-only-input](detect-judge-evidence-only-input.md) | The judge never sees the raw diff. buildDetectJudgeInput deterministically extracts EVIDENCE: a capped changed-file list, only the smell-contributing files' segments (per-segment 4k / hard total 8k caps, lockfiles never evidence), and explicit omission accounting where cap-dropped smell evidence names itself INCOMPLETE (engaging the insufficient-evidence -> DECISION fail-safe). Prefixes are forced off-config on the gate's diff call so consumer diff.noprefix/mnemonicPrefix cannot blind the extractor. | The smell-downgrade judge received the first 12000 chars of the raw… | 2026-07-01 | | [devkit-gates-repo-not-harness](devkit-gates-repo-not-harness.md) | The carve-out widens one precise notch: the priorArtGate hook may additionally substring-match devkit OWN prescribed acknowledgment token (a `Prior-art:` line) in the gated call text — ExitPlanMode tool_input.plan and the feature-critique Task tool_input.prompt. A gated call carrying the token passes silently and marks the session acknowledged; one without it is denied once as before. This is recognition of a devkit workflow token, not validation of a foreign tool argument shape: no schema checks, no length or content heuristics, no judgment of foreign args. Everything outside that token match stays rejected exactly as this axis has always ruled. | The priorArtGate carve-out (this axis, previous Target) let a devki… | 2026-08-10 | -| [devkit-owned-hook-runner-delivery](devkit-owned-hook-runner-delivery.md) | devkit owns closing this permanently for every FUTURE consumer repo via a new 'devkit sync-hook-runner' command (git add -f the untracked-and-gitignored runner files, matched against real git hook names) chained into a fresh package-mode devkit init's generated package.json prepare script: 'husky && (command -v devkit >/dev/null 2>&1 && devkit sync-hook-runner true)'. Every bun/npm install then re-stages the runner past husky's own ignore, so no NEW repo ever needs a manual git add -f. Kept as an explicit, separate, user-invoked command rather than folded into 'devkit doctor --fix' — fix only ever regenerates FILE content from the recorded selection, it never mutates the git INDEX; staging is something the caller must ask for. The existing checkHookRunner doctor check stays the detection layer for repos that predate this; its remedy now names sync-hook-runner as the fix. | The husky-generated .husky/_ runner is gitignored by husky's own in… | 2026-07-22 | +| [devkit-owned-hook-runner-delivery](devkit-owned-hook-runner-delivery.md) | A third doctor check, 'hooksPath owner' (new cli/lib/doctor/hooks-path.mts, consumed by hook-checks.mts), reports when the hooksPath THIS checkout will use belongs to another checkout of the same repo. 'devkit sync-hook-runner' may repair it only when Git's own worktree registry positively attributes one worktree-scoped value to an enumerated sibling checkout, the repo-wide fallback is relative and resolves inside this checkout, and checkHookRunner already proves that fallback complete. Repair holds Git's config.worktree lock, revalidates that the exact single sibling value still exists while locked, asks Git's parser to transform a copy, and atomically renames that copy over config.worktree — never unset-then-restore. | The 2026-07-22 Target closed the case where a fresh worktree has NO… | 2026-08-04 | | [devkit-self-dogfood](devkit-self-dogfood.md) | devkit init gains a first-class self-host mode, auto-detected by package name @norvalbv/devkit. It adds NO self-dependency (skips patchPackageJson) and generates the pre-commit hook from the SAME generator as consumers, rewriting each bunx guard-* invocation to node gate-engine/*.mts (source). devkit upgrade regenerates it and a parity test locks it to the generator; assets still sync via sync-skills/sync-agents and the configs stay hand-owned. | The hand-authored .husky/pre-commit was a SECOND source of truth fo… | 2026-07-13 | | [dup-gate-evidence-verified-against-worktree](dup-gate-evidence-verified-against-worktree.md) | Every pair the scan gate reports must be backed by code in the working tree. For each side, the indexed chunk BODY — the tail of raw_code after search-code's import prelude, which must be stripped: measured over 898 chunks with the symbol's lines deleted, a prelude-inclusive comparison still called 17% of them fresh, the body 0.2% — is looked up by rowid and checked for presence ANYWHERE in the file on disk: verbatim first, then, as formatting tolerance only, the best ALIGNED window of significant lines scoring >= 0.8. Alignment (not set membership) is load-bearing in both directions: it rejects a deleted symbol whose generic guard clauses live on scattered through sibling functions, and it means a symbol whose CONTENT changed since the last index reads as stale — correctly, since its embedding describes text that is no longer there. Presence anywhere, never at the recorded range: line drift must stay invisible or the gate goes quiet on ordinary commits. A side whose body is absent, or whose file cannot be read, drops the pair. The layer is strictly DROP-ONLY and lives only in runScan — never reconcile or baseline, where over-dropping deletes real human approvals — and it disables itself wholesale on an index carrying no raw_code/id, on GUARD_DUP_VERIFY_TREE=0, on any DB error, and on the root-mismatch fuse (every probed path missing, i.e. an index rooted elsewhere). Every drop is named in the output with a re-index hint, and a block decided on unverified evidence says so. | guard-dup reported duplication pairs purely from the search-code in… | 2026-07-25 | | [electron-backends-toggle-externalised](electron-backends-toggle-externalised.md) | Keep electron's `eslint.config.mjs` as a devkit-owned snapshot (full-replaceable), and externalise the ONE consumer-editable fact — *which backend processes to structure-lint* — to `guard.config.json` as `backends: { socketServer, vercel }`, read at lint-load. The template builds the backend flat-config blocks conditionally from that boolean; the file is fully devkit-owned again, so full-replace is safe and `migrate.mjs` needs zero change (the existing `guardConfigChange` merge already preserves the consumer's value). Template default is **both-on** (`true,true`) — the template already declares both backends in `scanRoots`/`boundaries`/`review.backendRoots` and loads their baselines unconditionally; a both-off default would let migrate re-disable an existing consumer's governance through the front door (the very bug). Missing/partial key degrades to off via `?? {}`. | `devkit migrate` treats `eslint.config.mjs` as fully devkit-owned a… | 2026-06-29 | diff --git a/docs/decisions/devkit-owned-hook-runner-delivery.md b/docs/decisions/devkit-owned-hook-runner-delivery.md index a2d4179..9efb20d 100644 --- a/docs/decisions/devkit-owned-hook-runner-delivery.md +++ b/docs/decisions/devkit-owned-hook-runner-delivery.md @@ -15,3 +15,17 @@ created: 2026-07-22 **Vision-fit:** n/a — internal devkit tooling (the solo dev's cross-repo guardrail distribution), not a Frink product surface. **Scope:** cli/commands/init.mts,cli/commands/sync/sync-hook-runner.mts,cli/lib/doctor/hook-checks.mts,cli/index.mts **Source:** manual + +## Target · 2026-08-04 — a checkout must run its OWN hooks — sync-hook-runner may replace a proven sibling core.hooksPath pin + +**Context:** The 2026-07-22 Target closed the case where a fresh worktree has NO runner. sc-1465 is its mirror: a worktree that HAS a working tracked runner still ran the MAIN checkout's hook, because a per-checkout core.hooksPath (.git/worktrees//config.worktree, with extensions.worktreeConfig=true) was pinned at an ABSOLUTE path into that other checkout. Host worktree tooling writes that pin, correctly, when the relative runner is absent. Once the runner is tracked the pin becomes the bug: commits are gated by another checkout's branch state, wrongly blocking valid work and wrongly passing invalid work, with no error. devkit was blind to it: checkHookRunner judges the SHARED (--local) scope by design, so it reported OK ('runner reachable — survives git worktree add') while the commit ran elsewhere; its shared-unset branch went further and printed the override itself as health. +**Ruling:** A third doctor check, 'hooksPath owner' (new cli/lib/doctor/hooks-path.mts, consumed by hook-checks.mts), reports when the hooksPath THIS checkout will use belongs to another checkout of the same repo. 'devkit sync-hook-runner' may repair it only when Git's own worktree registry positively attributes one worktree-scoped value to an enumerated sibling checkout, the repo-wide fallback is relative and resolves inside this checkout, and checkHookRunner already proves that fallback complete. Repair holds Git's config.worktree lock, revalidates the whole repair plan while locked, asks Git's parser to transform a copy, and atomically renames that copy over config.worktree — never unset-then-restore. There is no absent-config window: a concurrent writer that wins the lock first makes devkit abort, while one that runs later observes the completed replacement. Post-write verification also rechecks the shared fallback and runner; if either races the locked write, devkit reacquires Git's lock and restores the exact original bytes only when the live file still byte-matches its candidate, so rollback cannot overwrite a later writer. Scope and origin come from Git's config parser with extensions.worktreeConfig enabled; ambiguous, included, unreadable, external-central, already-pruned, and repo-wide values are diagnostic-only, and both ownership and runner checks classify ambiguous worktree values as drift. Relative values are resolved LEXICALLY and never realpath'd, because devkit's own ship gate worktrees are handed a .husky/_ that symlinks into the main checkout; realpath is confined to path-identity comparisons where /private aliasing would otherwise manufacture a foreign verdict. +**Consequences:** +- Positive: The borrowed-runner class is now detected in package/standalone and self-host doctor, and self-heals on every bun install through the prepare chain — reaching exactly the existing repos the 2026-07-22 Target could not retroactively fix. The heal is monotone: it cannot leave a checkout less gated than it found it. A commit made in a worktree is once again gated by the branch it is actually on, which is the whole point of reviewing that branch. +- Negative: This Target deliberately GIVES UP that record's stated 'with zero new git-config writes' property. sync-hook-runner now makes one narrowly preconditioned worktree-config replacement, and through the generated prepare script it may do so unprompted on install. Held to be different from the rejected broad rewrite: it changes only a value positively owned by a sibling checkout, replaces it atomically with the already-validated repo fallback, and never changes a repo-wide or unknown external policy. The surviving cross-mode invariant is validate-before-pointer-change and no implicit index mutation from doctor; overlay doctor --fix keeps its separately recorded guarded config write. sync-hook-runner remains the ordinary/self-host repair surface. No telemetry is added — doctor has no telemetry surface by design. +**Vision-fit:** n/a — internal devkit tooling (cross-repo guardrail distribution), not a Frink product surface. +**Researched:** Reproduced against real git: 'git config --worktree --get' returns the --local value with rc=0 when extensions.worktreeConfig is off (and a naive unset can delete it), 'rev-parse --git-path hooks' honours core.hooksPath so cannot compute the fallback, Git's worktree porcelain retains prunable sibling provenance, duplicate worktree values are last-wins to hook execution, and Git reports /private path aliases differently from configured text. Upstream Husky writes a relative .husky/_ and provides no ownership repair. Prior-art found a peer that unsets the same value before validation; two feature-critique passes narrowed repair to positive sibling provenance and rejected unset-then-rollback because restoration can race another writer. Completeness review then falsified the first fixed-value replacement: Git appends the new value when its old-value pattern no longer matches, so the lock-and-revalidate transaction is required to make the comparison real. Correctness review found the remaining cross-resource race: the local runner can disappear after the first validation, requiring full under-lock revalidation plus a byte-CAS rollback after post-write verification. +**Rejected:** (a) changing the pin on a bare existsSync of the runner dir — REJECTED: hook-checks.mts already reports states where that directory exists and nothing runs, so the replacement could swap a working sibling hook for no hook on every install. The precondition reuses checkHookRunner's complete verdict. (b) treating every absolute or every external per-worktree value as stale — REJECTED: central hooks are legitimate at either scope and scope expresses precedence, not ownership; only membership in Git's enumerated sibling worktrees is repair provenance. (c) unset followed by post-check restoration — REJECTED: restoration cannot atomically avoid overwriting a concurrent writer; replace the exact old value with the validated fallback in one locked transaction instead. (d) folding ordinary/self-host repair into doctor --fix — REJECTED: doctor remains diagnostic there; overlay's existing guarded doctor --fix exception is preserved. (e) wiring the new check into overlay doctor — DEFERRED: overlay already owns a distinct exact-path diagnosis and repair contract. (f) an unlocked `git config --fixed-value --replace-all` — REJECTED after real-Git verification: a non-matching old value makes Git append rather than refuse, overriding the concurrent writer and manufacturing an ambiguous config. +**Scope:** cli/lib/doctor/hooks-path.mts,cli/lib/doctor/hook-checks.mts,cli/commands/sync/sync-hook-runner.mts,cli/commands/doctor.mts,cli/lib/doctor/self-host-doctor.mts,cli/lib/ship/review/setup-manifest.mts +**Source:** manual +**Evidence-change:** That Target's own fix changed the facts its neighbour depended on. Making the .husky/_ runner TRACKED removed the premise of the absolute per-checkout core.hooksPath pin — a fresh worktree now carries its own working runner, so borrowing a sibling's is no longer the lesser evil but a live defect. The record also reasoned about an absolute hooksPath only as something DEVKIT might write, and rejected it on that basis; it did not rule on one already written by other tooling, which is what sc-1465 found in 5 worktrees of a live consumer repo. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bab69e2..bd565e7 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -24,6 +24,30 @@ In **overlay mode** a plain `git commit` (or an IDE/GUI commit) runs the **repo' that's the **self-heal** gap. Commit via the per-clone `git ci` alias instead, or enable the opt-in global shim with `devkit init --overlay --global-commit-gate`. See **overlay self-heal** in the glossary. +## My commit in a worktree ran a DIFFERENT checkout's hook +Symptom: a commit made in a linked worktree is blocked by a gate that worktree's own +`.husky/pre-commit` doesn't even contain (or sails past one it does). The hook that ran belongs to +another checkout — usually the main one, on its branch, at its version. + +Diagnose in one line, from the worktree: + + git config --worktree --get core.hooksPath + +An **absolute** path into another checkout is the fault. Some worktree tooling writes it right after +`git worktree add`, back when husky gitignored the `.husky/_` runner and a fresh worktree genuinely +had none — borrowing the main checkout's beat having no gates at all. Once the runner is **tracked** +(`devkit sync-hook-runner`) every checkout carries its own, and the pin only shadows it. + +Fix: `devkit sync-hook-runner` in that worktree. Once the checkout provably gates itself, it replaces +the exact sibling value with the repo's relative fallback (usually `.husky/_`) in one locked Git +config write; `devkit doctor` reports the state as **hooksPath owner** either way. It will not replace +an external central-hooks path, an ambiguous value, or a target Git no longer records as a sibling. + +Two scopes are *not* covered, by design. A **repo-wide** `core.hooksPath` (`git config --local`) is +reported but never replaced — it belongs to the repo, not to one checkout. And a value arriving via +`GIT_CONFIG_*`, `--global` or `--system` is invisible to `devkit doctor`, while `devkit review` reads +the fully merged value and *does* see it — so review can fail on a hooksPath doctor calls fine. + ## `devkit doctor` reports skills/agents drift A synced copy in `.claude/` or `.cursor/` diverged from its **manifest** (or devkit's source moved ahead). Re-run `devkit sync-skills` / `devkit sync-agents` (NOT a hand edit). `devkit doctor --fix` also repairs it. diff --git a/vitest.config.mjs b/vitest.config.mjs index f12fa40..8ad65d1 100644 --- a/vitest.config.mjs +++ b/vitest.config.mjs @@ -7,6 +7,7 @@ const TEST_INCLUDE = [ ]; const GIT_INTEGRATION_TESTS = [ 'cli/__tests__/asset-conflicts.test.mts', + 'cli/__tests__/doctor-hookspath-owner.test.mts', 'cli/__tests__/guard-branch.test.mts', 'cli/__tests__/overlay-global-hook.test.mts', 'cli/__tests__/overlay.test.mts',