From 3feaef6ecbab64c52507621a19e730b373d8c847 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:58:19 +0800 Subject: [PATCH] fix(dev-infra): harden worktree hook migration --- ...26-07-27-worktree-local-lefthook.i18n.yaml | 6 + .../2026-07-27-worktree-local-lefthook.md | 10 +- .../2026-07-27-worktree-local-lefthook.zh.md | 39 ++ docs/development.i18n.yaml | 4 +- docs/development.md | 6 +- docs/development.zh.md | 8 +- scripts/install-lefthook.mjs | 459 +++++++++++++++--- scripts/install-lefthook.spec.ts | 295 ++++++++++- 8 files changed, 749 insertions(+), 78 deletions(-) create mode 100644 .agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.i18n.yaml create mode 100644 .agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.zh.md diff --git a/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.i18n.yaml b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.i18n.yaml new file mode 100644 index 0000000000..301bf88490 --- /dev/null +++ b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md +2026-07-27-worktree-local-lefthook.md: 95860efca5309464e82d6d58c8320b3a390ae14f +2026-07-27-worktree-local-lefthook.zh.md: ea1639d2e45c61ba6b041eb5800f2b983a271e4b diff --git a/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md index 04db203037..95860efca5 100644 --- a/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md +++ b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md @@ -12,13 +12,13 @@ Lefthook-generated hooks prefer an absolute binary path captured from the instal ## Decision -Hook installation is worktree-scoped. The installer requires Git 2.20 or newer, upgrades a format-0 repository to format 1, enables `extensions.worktreeConfig`, and assigns the current worktree an absolute `core.hooksPath` at `$GIT_DIR/dsh-hooks`. The main worktree receives `$GIT_COMMON_DIR/dsh-hooks`; each linked worktree receives the corresponding directory under `$GIT_COMMON_DIR/worktrees/`. A repository-scoped lock serializes configuration migration and hook writes, including repeated concurrent installs. +Hook installation is worktree-scoped. The installer requires Git 2.26 or newer for configuration-scope provenance, upgrades a format-0 repository to format 1, enables `extensions.worktreeConfig`, and assigns the current worktree an absolute `core.hooksPath` at `$GIT_DIR/dsh-hooks`. The main worktree receives `$GIT_COMMON_DIR/dsh-hooks`; each linked worktree receives the corresponding directory under `$GIT_COMMON_DIR/worktrees/`. A repository-scoped lock serializes configuration migration and hook writes, including repeated concurrent installs. Each lock records a process ID and random ownership token; release verifies the same file identity and exact record. A dead or invalid lock is never broken automatically, so the diagnostic requires the contributor to confirm no installer is running and remove the lock manually. -The installer recognizes its hook directory with a private ownership marker and updates it idempotently. It refuses an unowned directory or a worktree-specific custom `core.hooksPath`. An inherited global or common-repository hook path is preserved by default; `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1` explicitly lets only the current worktree override it, so worktrees without that override continue using the inherited path. This opt-in does not attempt to chain arbitrary hook managers. +The installer recognizes its hook directory with a private ownership marker and updates it idempotently. It inspects the effective scope, origin, and value of `core.hooksPath`, then refuses an unowned directory, every command-scoped path, and every non-owned worktree-scoped path, including values loaded through `config.worktree` includes. It follows conditional includes with Git's parser and refuses a command- or worktree-scoped include whose target provides, or cannot safely be shown not to provide, a hook path; an inactive condition therefore cannot later hide a user-owned path behind the installer's direct value. The same risk in an inherited system, global, or common-repository include requires `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1`, which explicitly opts only the current worktree into Lefthook while other worktrees retain the inherited path. Unrelated conditional includes remain valid. Command-scoped Git configuration is removed from the Lefthook subprocess environment after validation. This opt-in does not attempt to chain arbitrary hook managers. -Enabling worktree config removes the standard redundant `core.bare=false` value from the common config because false remains Git's default; an explicit `core.worktree` or `core.bare=true` is refused for manual migration. If Lefthook fails during a first install, the installer removes the new worktree override so the prior inherited or common hooks remain active. Legacy files in `$GIT_COMMON_DIR/hooks` are never removed or rewritten by the worktree-local installer. +Enabling worktree config removes the standard redundant `core.bare=false` value from the common config because false remains Git's default; an explicit `core.worktree` or `core.bare=true`, whether direct or loaded through an active common-config include, is refused for manual migration. Before enabling the extension, the installer follows common-config conditional includes and refuses a target that provides, or cannot safely be shown not to provide, either migration-sensitive key; unrelated conditional includes remain valid. If Lefthook fails during a first install, the installer removes the new worktree override so the prior inherited or common hooks remain active. Legacy files in `$GIT_COMMON_DIR/hooks` are never removed or rewritten by the worktree-local installer. -[`install-lefthook.spec.ts`](../../../../scripts/install-lefthook.spec.ts) exercises main and linked worktrees, removal independence, repeated and concurrent installs, the Git version boundary, custom-path refusal and opt-in, legacy common-hook preservation, and failed-install rollback. +[`install-lefthook.spec.ts`](../../../../scripts/install-lefthook.spec.ts) exercises main and linked worktrees, removal independence, repeated and concurrent installs, stale and replaced lock ownership, the Git version boundary, migration keys loaded through active and conditional common-config includes, scoped custom-path refusal and opt-in, active and inactive worktree includes, inherited conditional paths, command-environment isolation, legacy common-hook preservation, and failed-install rollback. ## Alternatives considered @@ -34,6 +34,6 @@ Enabling worktree config removes the standard redundant `core.bare=false` value Installing or removing one worktree no longer changes another worktree's active hooks, binary path, or generated hook bytes. Concurrent installs are serialized and repeated installation is idempotent, while the jobs and latency boundary owned by [Fast local Git hooks](2026-07-22-fast-local-git-hooks.md) stay unchanged. -The repository becomes a Git format-1 repository after the first installation and rejects clients older than Git 2.20. Custom worktree hook managers require an explicit integration choice; inherited hook paths can coexist across other worktrees, but opting the current worktree into Lefthook means those inherited hooks do not run there unless the contributor chains them through `lefthook.yml`. +The repository becomes a Git format-1 repository after the first installation and rejects clients older than Git 2.26. Custom worktree hook managers require an explicit integration choice; inherited hook paths can coexist across other worktrees, but opting the current worktree into Lefthook means those inherited hooks do not run there unless the contributor chains them through `lefthook.yml`. Legacy common hooks remain on disk for unupgraded worktrees. They can become stale, but removing them automatically would break a registered worktree whose branch has not adopted this installer. diff --git a/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.zh.md b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.zh.md new file mode 100644 index 0000000000..ea1639d2e4 --- /dev/null +++ b/.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.zh.md @@ -0,0 +1,39 @@ +# Agent Note: 让 Lefthook 安装限定于各 worktree + +Status: implemented + +[English](2026-07-27-worktree-local-lefthook.md) | 中文 + +## 问题 + +每次运行 `pnpm install` 都会执行根目录的 [`postinstall`](../../../../package.json),其中的 [`install-lefthook.mjs`](../../../../scripts/install-lefthook.mjs) 会调用 `lefthook install --force`。若无额外配置,关联的 Git worktree 共用同一仓库的默认钩子目录,因此在任一 worktree 中安装都可能改写其他所有 worktree 使用的钩子。 + +Lefthook 生成的钩子会优先使用安装时从对应 worktree 记录的绝对二进制文件路径,之后才尝试当前 worktree 的回退路径。因此,共享钩子会一直运行另一个 worktree 固定版本的二进制文件,直到该 worktree 消失;并发安装还会写入同一组文件。 + +## 决策 + +钩子安装以 worktree 为作用域。为了获取配置作用域的来源信息,安装程序要求 Git 2.26 或更高版本;它会将格式版本为 0 的仓库升级到格式版本 1,启用 `extensions.worktreeConfig`,并将当前 worktree 的 `core.hooksPath` 设为指向 `$GIT_DIR/dsh-hooks` 的绝对路径。主 worktree 使用 `$GIT_COMMON_DIR/dsh-hooks`;每个关联 worktree 则使用 `$GIT_COMMON_DIR/worktrees/` 下的对应目录。仓库级锁会串行化配置迁移与钩子写入,包括并发触发的重复安装。每个锁都会记录进程 ID 和随机所有权令牌;释放锁时会验证同一个文件身份与完全一致的记录。安装程序绝不会自动破坏所属进程已结束或内容无效的锁,因此诊断会要求贡献者先确认没有安装程序正在运行,再手动移除该锁。 + +安装程序通过私有所有权标记识别其钩子目录,并以幂等方式更新该目录。它会检查 `core.hooksPath` 的生效作用域、来源和值,并拒绝没有所有权标记的目录、所有命令作用域路径,以及所有非本安装程序所有的 worktree 作用域路径,包括通过 `config.worktree` 中的 include 加载的值。安装程序会用 Git 的解析器跟踪 `includeIf`;若命令作用域或 worktree 作用域的目标配置提供钩子路径,或者无法安全证明它不会提供钩子路径,安装程序就会拒绝继续。因此,安装时未生效的条件日后也无法在安装程序的直接配置值之前隐藏用户自有路径。系统配置、全局配置或共用仓库配置中存在相同风险时,必须设置 `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1`,从而只让当前 worktree 显式启用 Lefthook,其他 worktree 则继续使用继承路径。与钩子无关的 `includeIf` 仍然有效。完成验证后,Lefthook 子进程的环境会移除命令作用域的 Git 配置。这项显式选择不会尝试串联任意钩子管理器。 + +启用 worktree 配置时,安装程序会从共用配置中移除标准但冗余的 `core.bare=false`,因为 false 仍是 Git 的默认值;无论共用配置直接设置了 `core.worktree` 或 `core.bare=true`,还是通过当前生效的 include 加载了这些值,安装程序都会拒绝继续并要求手动迁移。启用扩展之前,安装程序会跟踪共用配置中的 `includeIf`;若目标配置提供任一迁移敏感键,或者无法安全证明它不会提供这些键,安装程序就会拒绝继续。与迁移无关的 `includeIf` 仍然有效。若首次安装期间 Lefthook 失败,安装程序会移除新建的 worktree 覆盖,使原有的继承钩子或共用钩子继续生效。worktree 本地安装程序绝不会移除或改写 `$GIT_COMMON_DIR/hooks` 中的旧文件。 + +[`install-lefthook.spec.ts`](../../../../scripts/install-lefthook.spec.ts) 覆盖主 worktree 和关联 worktree、移除后的相互独立性、重复与并发安装、陈旧锁与锁所有权被替换、Git 版本边界、通过生效及条件式共用配置 include 加载的迁移键、按作用域拒绝自定义路径与显式覆盖、生效及未生效的 worktree include、继承的条件式路径、命令环境隔离、保留旧公共钩子,以及安装失败时的回滚。 + +## 考虑过的替代方案 + +**保留共享的生成钩子,并依赖其当前 worktree 回退路径。** 只要对应 worktree 仍存在,记录的绝对路径就会优先生效,因此回退路径无法提供版本或生命周期隔离。 + +**让每个 worktree 都指向同一个纳入版本控制的 `.githooks` 目录。** 使用受版本控制的相对目录可以消除生成的绝对路径,但更改共享的 `core.hooksPath` 可能会禁用旧 worktree 中的钩子,因为其分支并不包含该目录;同时,每个 worktree 仍然耦合于同一个共享配置值。 + +**构建通用的钩子管理器串联层。** 执行顺序、参数转发、失败语义和升级都会成为仓库自行负责的行为,却与 Lefthook 隔离无关。因此,安装程序会拒绝 worktree 专属的自定义路径,只将范围更窄的继承路径覆盖设为显式操作。 + +**停止自动安装钩子。** 手动设置可以避免共享写入,却会使仓库中低成本的提交与推送检查意外变成可选项,短期存在、由 agent(智能体)使用的 worktree 尤其容易受到影响。 + +## 后果 + +安装或移除任一 worktree 不再改变其他 worktree 的生效钩子、二进制文件路径或生成的钩子字节。并发安装会串行执行,重复安装保持幂等;[快速本地 Git 钩子](2026-07-22-fast-local-git-hooks.md)所规定的任务与延迟边界保持不变。 + +首次安装后,仓库会采用 Git 格式版本 1,并拒绝版本低于 Git 2.26 的客户端。自定义 worktree 钩子管理器需要明确选择集成方式;继承钩子路径可继续供其他 worktree 使用,但当前 worktree 显式启用 Lefthook 后,其中不会运行这些继承钩子,除非贡献者通过 `lefthook.yml` 将其串联起来。 + +旧的共用钩子会为尚未升级的 worktree 保留在磁盘上。它们可能逐渐陈旧,但自动删除这些钩子会破坏已注册但所在分支尚未采用本安装程序的 worktree。 diff --git a/docs/development.i18n.yaml b/docs/development.i18n.yaml index 8d19bd9880..5046d75245 100644 --- a/docs/development.i18n.yaml +++ b/docs/development.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write docs/development.md -development.md: fd7f39ae7b5aac2d44572979ca8c8f1d2df0de6f -development.zh.md: 7dd6209bad75d605e0056d2465a35b08aa091780 +development.md: 6c927c46b0a25f84295796354e7f49eb9eb7b2e9 +development.zh.md: f29d08df18ca9bead7f4c9bf3cf7f749630b1b84 diff --git a/docs/development.md b/docs/development.md index 530a5ea886..6c927c46b0 100644 --- a/docs/development.md +++ b/docs/development.md @@ -8,7 +8,7 @@ This onboarding guide helps project contributors get started with the local envi - Node.js supports 22.19+ and 24+. CI covers 22.19, 24, and 26; see the [Node engine floor Agent Note](../.agents/notes/implemented/process/2026-07-06-node-engine-floor.md). - Corepack-enabled pnpm. The repo pins `pnpm@11.7.0` in `package.json`; run `corepack enable` if `pnpm --version` does not resolve through Corepack. -- Git 2.20 or newer; hook setup enables Git's worktree-specific configuration extension. +- Git 2.26 or newer; hook setup enables Git's worktree-specific configuration extension. - Optional: a DeepSeek API key for the TUI, headless, and ACP automation demos and real-API e2e tests. ## First-time setup @@ -19,7 +19,7 @@ Install dependencies from the repo root: pnpm install ``` -The install also runs the root `postinstall` script, which installs lefthook from the repo dev dependency through `scripts/install-lefthook.mjs`. The wrapper gives the current worktree an explicit hook directory under its own Git directory; linked worktrees therefore use their own lefthook binary and configuration instead of rewriting common hooks. The first install enables Git's worktree-specific configuration extension and repository format 1; see the [worktree-local hooks Agent Note](../.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md). +The install also runs the root `postinstall` script, which installs lefthook from the repo dev dependency through `scripts/install-lefthook.mjs`. The wrapper requires Git 2.26 or newer and gives the current worktree an explicit hook directory under its own Git directory; linked worktrees therefore use their own lefthook binary and configuration instead of rewriting common hooks. The first install enables Git's worktree-specific configuration extension and repository format 1; see the [worktree-local hooks Agent Note](../.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md). If hooks are missing because dependencies were restored from cache or `postinstall` was skipped, install them manually: @@ -27,7 +27,7 @@ If hooks are missing because dependencies were restored from cache or `postinsta node scripts/install-lefthook.mjs ``` -The wrapper refuses to replace an existing user-owned `core.hooksPath`. If an inherited global or repository path should remain active in other worktrees while this worktree opts into lefthook, inspect that path first and rerun with `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1`; a worktree-specific custom path is never overwritten and must be integrated or removed explicitly. +The wrapper refuses to replace an existing user-owned `core.hooksPath`. If an inherited system, global, or common-repository path should remain active in other worktrees while this worktree opts into lefthook, inspect that path first and rerun with `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1`; command-scoped and worktree-scoped custom paths are never overridden and must be integrated or removed explicitly. The same rules apply when a currently inactive conditional include can provide a hook path; unrelated conditional includes remain valid. Before enabling the worktree-config extension, conditional common-config targets that may contain `core.worktree` or `core.bare=true` require manual migration. If the installer reports a stale or invalid lock, confirm no installer is running, remove the reported lock manually, and rerun the command. Run typecheck once after a fresh clone: diff --git a/docs/development.zh.md b/docs/development.zh.md index 7dd6209bad..f29d08df18 100644 --- a/docs/development.zh.md +++ b/docs/development.zh.md @@ -8,7 +8,7 @@ - Node.js 支持 22.19+ 与 24+。CI 覆盖 22.19、24 和 26;见 [Node 引擎下限 Agent Note](../.agents/notes/implemented/process/2026-07-06-node-engine-floor.md)。 - 启用了 Corepack 的 pnpm。仓库在 `package.json` 中固定使用 `pnpm@11.7.0`;如果 `pnpm --version` 无法通过 Corepack 解析,请先运行 `corepack enable`。 -- Git。 +- Git 2.26 或更高版本;钩子设置会启用 Git 的 worktree 专属配置扩展。 - 可选:一个 DeepSeek API key,用于 TUI、headless 和 ACP(Agent Client Protocol)自动化 agent(智能体)演示以及真实 API 的 e2e 测试。 ## 首次搭建 @@ -19,14 +19,16 @@ pnpm install ``` -安装过程同时会运行根目录的 `postinstall` 脚本,该脚本通过 `scripts/install-lefthook.mjs` 从仓库 dev 依赖安装 lefthook。包装脚本使用 lefthook 经过评审的 `--force` 模式,确保已存在 `core.hooksPath` 的关联 worktree 不会导致正常的 `pnpm run …` 命令失败。 +安装过程同时会运行根目录的 `postinstall` 脚本,该脚本通过 `scripts/install-lefthook.mjs` 从仓库 dev 依赖安装 lefthook。包装脚本要求使用 Git 2.26 或更高版本,并会为当前 worktree 在其自身的 Git 目录下设置显式钩子目录;因此,关联 worktree 会使用各自的 lefthook 二进制文件和配置,而不会改写共用钩子。首次安装会启用 Git 的 worktree 专属配置扩展和仓库格式 1;见 [worktree 本地钩子 Agent Note](../.agents/notes/implemented/process/2026-07-27-worktree-local-lefthook.md)。 如果依赖是从缓存恢复或 `postinstall` 被跳过而导致缺少钩子,请手动安装: ```sh -pnpm exec lefthook install --force +node scripts/install-lefthook.mjs ``` +包装脚本拒绝替换现有且由用户自行管理的 `core.hooksPath`。若要让继承自系统、全局或共用仓库配置的路径在其他 worktree 中继续生效,同时让当前 worktree 显式启用 lefthook,请先检查该路径,再设置 `DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1` 重新运行;命令作用域和 worktree 作用域的自定义路径绝不会被覆盖,必须显式集成或移除。当前未生效的 `includeIf` 可能提供钩子路径时,同样适用这些规则;与钩子无关的 `includeIf` 仍然有效。worktree 配置扩展启用之前,可能包含 `core.worktree` 或 `core.bare=true` 的共用配置 `includeIf` 目标需要手动迁移。若安装程序报告陈旧锁或无效锁,请先确认没有安装程序正在运行,手动移除诊断中报告的锁,再重新运行命令。 + 新克隆后请先运行一次类型检查: ```sh diff --git a/scripts/install-lefthook.mjs b/scripts/install-lefthook.mjs index 3dc19c9dca..6dd88e5ca8 100644 --- a/scripts/install-lefthook.mjs +++ b/scripts/install-lefthook.mjs @@ -1,9 +1,10 @@ #!/usr/bin/env node +import { randomUUID } from 'node:crypto' import { existsSync, lstatSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' import { spawnSync } from 'node:child_process' -import { isAbsolute, join, resolve } from 'node:path' +import { dirname, isAbsolute, join, resolve } from 'node:path' -const MINIMUM_GIT = [2, 20, 0] +const MINIMUM_GIT = [2, 26, 0] const HOOKS_DIRECTORY = 'dsh-hooks' const OWNERSHIP_MARKER = '.dsh-lefthook-owned' const OWNERSHIP_MARKER_CONTENT = 'deepseek-harness worktree-local lefthook hooks\n' @@ -11,6 +12,7 @@ const INSTALL_LOCK = 'dsh-lefthook-install.lock' const INSTALL_LOCK_TIMEOUT_MS = 30_000 const INSTALL_LOCK_POLL_MS = 50 const ALLOW_HOOKS_PATH_OVERRIDE = 'DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE' +const CONDITIONAL_INCLUDE_PATTERN = '^includeif\\..*\\.path$' function errorCode(error) { return typeof error === 'object' && error !== null && 'code' in error @@ -47,6 +49,13 @@ function nulValues(result) { return output.split('\0') } +function stripGitLineTerminator(output) { + const withoutLineFeed = output.endsWith('\n') ? output.slice(0, -1) : output + return process.platform === 'win32' && withoutLineFeed.endsWith('\r') + ? withoutLineFeed.slice(0, -1) + : withoutLineFeed +} + function fileConfigValues(root, configPath, key) { return nulValues(git( ['config', '--file', configPath, '--null', '--get-all', key], @@ -55,14 +64,76 @@ function fileConfigValues(root, configPath, key) { )) } -function effectiveConfigValue(root, key) { - const values = nulValues(git( - ['config', '--null', '--get', key], +function fileConfigEntries(root, configPath, key) { + const fields = nulValues(git( + ['config', '--file', configPath, '--includes', '--null', '--show-origin', '--get-all', key], root, { allowStatuses: [1] }, )) - if (values.length > 1) throw new Error(`git config returned multiple effective values for ${key}`) - return values[0] + if (fields.length % 2 !== 0) { + throw new Error(`git config returned invalid file entries for ${key}`) + } + const entries = [] + for (let index = 0; index < fields.length; index += 2) { + entries.push({ origin: fields[index], value: fields[index + 1] }) + } + return entries +} + +function splitConfigNameValue(field, pattern) { + const separator = field.indexOf('\n') + if (separator < 0) throw new Error(`git config returned an invalid name and value for ${pattern}`) + return { name: field.slice(0, separator), value: field.slice(separator + 1) } +} + +function fileConfigMatchingEntries(root, configPath, pattern) { + const fields = nulValues(git( + ['config', '--file', configPath, '--includes', '--null', '--show-origin', '--get-regexp', pattern], + root, + { allowStatuses: [1] }, + )) + if (fields.length % 2 !== 0) { + throw new Error(`git config returned invalid matching file entries for ${pattern}`) + } + const entries = [] + for (let index = 0; index < fields.length; index += 2) { + entries.push({ origin: fields[index], ...splitConfigNameValue(fields[index + 1], pattern) }) + } + return entries +} + +function scopedConfigMatchingEntries(root, pattern) { + const fields = nulValues(git( + ['config', '--includes', '--null', '--show-scope', '--show-origin', '--get-regexp', pattern], + root, + { allowStatuses: [1] }, + )) + if (fields.length % 3 !== 0) { + throw new Error(`git config returned invalid scoped entries for ${pattern}`) + } + const entries = [] + for (let index = 0; index < fields.length; index += 3) { + entries.push({ + scope: fields[index], + origin: fields[index + 1], + ...splitConfigNameValue(fields[index + 2], pattern), + }) + } + return entries +} + +function effectiveConfigEntry(root, key) { + const fields = nulValues(git( + ['config', '--null', '--show-scope', '--show-origin', '--get', key], + root, + { allowStatuses: [1] }, + )) + if (fields.length === 0) return undefined + if (fields.length !== 3) { + throw new Error(`git config returned an invalid scoped value for ${key}`) + } + const [scope, origin, value] = fields + return { origin, scope, value } } function parseGitBoolean(value, key) { @@ -85,11 +156,71 @@ function assertSupportedGit(root) { for (let index = 0; index < MINIMUM_GIT.length; index += 1) { if (actual[index] > MINIMUM_GIT[index]) return if (actual[index] < MINIMUM_GIT[index]) { - throw new Error(`Git 2.20 or newer is required for worktree-local hooks; found ${version}`) + throw new Error(`Git 2.26 or newer is required for worktree-local hooks; found ${version}`) } } } +function conditionalIncludeTarget(entry, root) { + if (isAbsolute(entry.value)) return entry.value + const sourcePath = configOriginPath(entry.origin, root) + if (sourcePath === undefined) return undefined + if (entry.value.startsWith('~/')) { + const home = process.env.HOME + return home === undefined ? undefined : resolve(home, entry.value.slice(2)) + } + if (entry.value.startsWith('~') || entry.value.startsWith('%(')) return undefined + return resolve(dirname(sourcePath), entry.value) +} + +function inspectConditionalConfig(root, configPath, inspect, seen = new Set()) { + const identity = normalizedPath(configPath) + if (seen.has(identity)) return undefined + seen.add(identity) + if (!existsSync(configPath)) { + return { configPath, detail: 'the included config does not exist and cannot be inspected' } + } + try { + const subject = inspect(configPath) + if (subject !== undefined) return { configPath, subject } + for (const entry of fileConfigMatchingEntries(root, configPath, CONDITIONAL_INCLUDE_PATTERN)) { + const target = conditionalIncludeTarget(entry, root) + if (target === undefined) { + return { configPath, detail: `the nested include path ${JSON.stringify(entry.value)} cannot be resolved safely` } + } + const nested = inspectConditionalConfig(root, target, inspect, seen) + if (nested !== undefined) return nested + } + return undefined + } catch (error) { + return { + configPath, + detail: `the included config could not be inspected: ${error instanceof Error ? error.message : String(error)}`, + } + } +} + +function conditionalIncludeRisk(root, entry, inspect) { + const target = conditionalIncludeTarget(entry, root) + if (target === undefined) { + return { detail: `the include path ${JSON.stringify(entry.value)} cannot be resolved safely` } + } + return inspectConditionalConfig(root, target, inspect) +} + +function migrationConfigSubject(root, configPath) { + const worktreeEntry = fileConfigEntries(root, configPath, 'core.worktree')[0] + if (worktreeEntry !== undefined) return `core.worktree (${configSource(worktreeEntry)})` + const trueBareEntry = fileConfigEntries(root, configPath, 'core.bare') + .find(entry => parseGitBoolean(entry.value, 'core.bare')) + return trueBareEntry === undefined ? undefined : `core.bare=true (${configSource(trueBareEntry)})` +} + +function hooksPathConfigSubject(root, configPath) { + const entry = fileConfigEntries(root, configPath, 'core.hooksPath')[0] + return entry === undefined ? undefined : `core.hooksPath (${configSource(entry)})` +} + function ensureWorktreeConfig(root, commonConfigPath) { const versions = fileConfigValues(root, commonConfigPath, 'core.repositoryFormatVersion') const versionText = assertSingle(versions, 'core.repositoryFormatVersion') @@ -98,17 +229,6 @@ function ensureWorktreeConfig(root, commonConfigPath) { throw new Error(`unsupported core.repositoryFormatVersion: ${JSON.stringify(versionText)}`) } - const worktrees = fileConfigValues(root, commonConfigPath, 'core.worktree') - if (worktrees.length > 0) { - throw new Error('cannot enable extensions.worktreeConfig while core.worktree is in the common config; move it to the main worktree config first') - } - - const bareText = assertSingle(fileConfigValues(root, commonConfigPath, 'core.bare'), 'core.bare') - const bare = bareText === undefined ? undefined : parseGitBoolean(bareText, 'core.bare') - if (bare === true) { - throw new Error('cannot enable extensions.worktreeConfig for a common config with core.bare=true') - } - const extensionText = assertSingle( fileConfigValues(root, commonConfigPath, 'extensions.worktreeConfig'), 'extensions.worktreeConfig', @@ -117,26 +237,79 @@ function ensureWorktreeConfig(root, commonConfigPath) { ? false : parseGitBoolean(extensionText, 'extensions.worktreeConfig') + if (!extensionEnabled) { + for (const entry of fileConfigMatchingEntries(root, commonConfigPath, CONDITIONAL_INCLUDE_PATTERN)) { + const risk = conditionalIncludeRisk( + root, + entry, + configPath => migrationConfigSubject(root, configPath), + ) + if (risk !== undefined) { + const reason = risk.subject ?? risk.detail + throw new Error( + `cannot enable extensions.worktreeConfig while common conditional include ` + + `${entry.origin}: ${entry.name}=${JSON.stringify(entry.value)} may provide migration-sensitive config (${reason}); ` + + 'audit and migrate it, then enable the extension explicitly', + ) + } + } + } + + const worktreeEntry = fileConfigEntries(root, commonConfigPath, 'core.worktree')[0] + if (worktreeEntry !== undefined) { + throw new Error( + `cannot enable extensions.worktreeConfig while core.worktree is in the common config (${configSource(worktreeEntry)}); ` + + 'move it to the main worktree config first', + ) + } + + const bareEntries = fileConfigEntries(root, commonConfigPath, 'core.bare') + const trueBareEntry = bareEntries.find(entry => parseGitBoolean(entry.value, 'core.bare')) + if (trueBareEntry !== undefined) { + throw new Error( + `cannot enable extensions.worktreeConfig for a common config with core.bare=true (${configSource(trueBareEntry)})`, + ) + } + const directBareText = assertSingle(fileConfigValues(root, commonConfigPath, 'core.bare'), 'core.bare') + const directBare = directBareText === undefined ? undefined : parseGitBoolean(directBareText, 'core.bare') + if (version === 0) { git(['config', '--file', commonConfigPath, 'core.repositoryFormatVersion', '1'], root) } if (!extensionEnabled) { git(['config', '--file', commonConfigPath, 'extensions.worktreeConfig', 'true'], root) } - if (bare === false) { + if (directBare === false) { git(['config', '--file', commonConfigPath, '--unset-all', 'core.bare'], root) } } -function lockOwnerIsAlive(lockPath) { - let owner +function readInstallLock(lockPath) { try { - owner = Number(readFileSync(lockPath, 'utf8').trim()) + return readFileSync(lockPath, 'utf8') } catch (error) { - if (errorCode(error) === 'ENOENT') return false + if (errorCode(error) === 'ENOENT') return undefined throw error } - if (!Number.isSafeInteger(owner) || owner <= 0) return true +} + +function installLockStat(lockPath) { + try { + return lstatSync(lockPath) + } catch (error) { + if (errorCode(error) === 'ENOENT') return undefined + throw error + } +} + +function parseInstallLock(record) { + const match = /^([1-9]\d*) ([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\n$/i.exec(record) + if (match === null) return undefined + const owner = Number(match[1]) + return Number.isSafeInteger(owner) ? owner : undefined +} + +function lockOwnerIsAlive(owner) { try { process.kill(owner, 0) return true @@ -147,28 +320,63 @@ function lockOwnerIsAlive(lockPath) { } } -function removeStaleLock(lockPath) { +function manualLockRecoveryError(lockPath, condition) { + return new Error( + `${condition} Lefthook installer lock ${JSON.stringify(lockPath)}. ` + + 'Confirm no Lefthook installer is running, remove it manually, and retry.', + ) +} + +function lockOwnershipChangedError(lockPath) { + return new Error(`Lefthook installer lock ownership changed for ${lockPath}; refusing to remove it`) +} + +function releaseInstallLock(lockPath, ownedRecord, ownedStat) { + const currentStat = installLockStat(lockPath) + if ( + currentStat === undefined + || !currentStat.isFile() + || currentStat.isSymbolicLink() + || currentStat.dev !== ownedStat.dev + || currentStat.ino !== ownedStat.ino + || readInstallLock(lockPath) !== ownedRecord + ) { + throw lockOwnershipChangedError(lockPath) + } try { unlinkSync(lockPath) } catch (error) { - if (errorCode(error) !== 'ENOENT') throw error - // Another waiting installer removed the same stale lock first. + if (errorCode(error) === 'ENOENT') { + throw lockOwnershipChangedError(lockPath) + } + throw error } } async function acquireInstallLock(commonDirectory) { const lockPath = join(commonDirectory, INSTALL_LOCK) const deadline = Date.now() + INSTALL_LOCK_TIMEOUT_MS + const ownedRecord = `${String(process.pid)} ${randomUUID()}\n` while (true) { try { - writeFileSync(lockPath, `${String(process.pid)}\n`, { flag: 'wx', mode: 0o600 }) - return () => removeStaleLock(lockPath) + writeFileSync(lockPath, ownedRecord, { flag: 'wx', mode: 0o600 }) + const ownedStat = installLockStat(lockPath) + if (ownedStat === undefined || !ownedStat.isFile() || ownedStat.isSymbolicLink()) { + throw lockOwnershipChangedError(lockPath) + } + return () => releaseInstallLock(lockPath, ownedRecord, ownedStat) } catch (error) { if (errorCode(error) !== 'EEXIST') throw error - if (!lockOwnerIsAlive(lockPath)) { - removeStaleLock(lockPath) - continue + const existingStat = installLockStat(lockPath) + if (existingStat === undefined) continue + if (!existingStat.isFile() || existingStat.isSymbolicLink()) { + throw manualLockRecoveryError(lockPath, 'invalid') } + const existingRecord = readInstallLock(lockPath) + if (existingRecord === undefined) continue + const owner = parseInstallLock(existingRecord) + if (owner === undefined) throw manualLockRecoveryError(lockPath, 'invalid') + if (!lockOwnerIsAlive(owner)) throw manualLockRecoveryError(lockPath, 'stale') if (Date.now() >= deadline) { throw new Error(`timed out waiting for Lefthook installer lock ${lockPath}`) } @@ -197,63 +405,176 @@ function ensureOwnedHooksDirectory(hooksPath) { } } +function environmentWithoutCommandGitConfig() { + const env = { ...process.env } + for (const key of Object.keys(env)) { + const normalized = key.toUpperCase() + if ( + normalized === 'GIT_CONFIG_PARAMETERS' + || normalized === 'GIT_CONFIG_COUNT' + || /^GIT_CONFIG_(?:KEY|VALUE)_\d+$/.test(normalized) + ) { + delete env[key] + } + } + return env +} + function runLefthook(root, lefthook) { const args = ['install', '--force'] + const env = environmentWithoutCommandGitConfig() // Node refuses to spawn Windows `.cmd` shims directly; the quoted path is // re-parsed by cmd.exe, while POSIX can execute its extensionless shim. const result = process.platform === 'win32' - ? spawnSync(`"${lefthook}"`, args, { cwd: root, stdio: 'inherit', shell: true }) - : spawnSync(lefthook, args, { cwd: root, stdio: 'inherit' }) + ? spawnSync(`"${lefthook}"`, args, { cwd: root, env, stdio: 'inherit', shell: true }) + : spawnSync(lefthook, args, { cwd: root, env, stdio: 'inherit' }) if (result.status !== 0) throw commandFailure(lefthook, args, result) } -function refuseCustomHooksPath(root, hooksPath) { - const origin = git( - ['config', '--show-origin', '--get', 'core.hooksPath'], - root, - { allowStatuses: [1] }, - ).stdout.trim() - const source = origin === '' ? hooksPath : origin +function configSource(entry) { + return `${entry.origin}: ${JSON.stringify(entry.value)}` +} + +function normalizedPath(path) { + const normalized = resolve(path) + return process.platform === 'win32' ? normalized.toLowerCase() : normalized +} + +function configOriginPath(origin, root) { + if (!origin.startsWith('file:')) return undefined + const originPath = origin.slice('file:'.length) + return isAbsolute(originPath) ? originPath : resolve(root, originPath) +} + +function originIsFile(origin, root, configPath) { + const originPath = configOriginPath(origin, root) + return originPath !== undefined && normalizedPath(originPath) === normalizedPath(configPath) +} + +function conditionalIncludeSource(entry) { + return `${entry.origin}: ${entry.name}=${JSON.stringify(entry.value)}` +} + +function conditionalIncludes(root, worktreeConfigPath) { + const entries = scopedConfigMatchingEntries(root, CONDITIONAL_INCLUDE_PATTERN) + entries.push(...fileConfigMatchingEntries(root, worktreeConfigPath, CONDITIONAL_INCLUDE_PATTERN) + .map(entry => ({ ...entry, scope: 'worktree' }))) + const unique = new Map() + for (const entry of entries) { + unique.set(`${entry.scope}\0${entry.origin}\0${entry.name}\0${entry.value}`, entry) + } + return [...unique.values()] +} + +function assertConditionalHooksPaths(root, worktreeConfigPath) { + for (const entry of conditionalIncludes(root, worktreeConfigPath)) { + const risk = conditionalIncludeRisk( + root, + entry, + configPath => hooksPathConfigSubject(root, configPath), + ) + if (risk === undefined) continue + const reason = risk.subject ?? risk.detail + if (entry.scope === 'command' || entry.scope === 'worktree') { + throw new Error( + `refusing ${entry.scope}-scoped conditional include ${conditionalIncludeSource(entry)}; ` + + `it may provide a user-owned core.hooksPath (${reason}) and cannot be overridden`, + ) + } + if (!['system', 'global', 'local'].includes(entry.scope)) { + throw new Error( + `refusing conditional include from unsupported ${entry.scope} scope ${conditionalIncludeSource(entry)}; ` + + `it may provide core.hooksPath (${reason})`, + ) + } + if (process.env[ALLOW_HOOKS_PATH_OVERRIDE] !== '1') { + throw new Error( + `refusing to replace core.hooksPath that may be provided by inherited conditional include ` + + `${conditionalIncludeSource(entry)} (${reason}). Inspect that include and rerun with ` + + `${ALLOW_HOOKS_PATH_OVERRIDE}=1 only if it may remain active in other worktrees`, + ) + } + } +} + +function refuseInheritedHooksPath(entry) { throw new Error( - `refusing to replace user-owned core.hooksPath (${source}). ` + `refusing to replace user-owned core.hooksPath (${configSource(entry)}). ` + `Chain those hooks through lefthook.yml, or, if this inherited path may remain active only in other worktrees, ` + `rerun with ${ALLOW_HOOKS_PATH_OVERRIDE}=1`, ) } +function refuseScopedHooksPath(entry) { + if (entry.scope === 'command') { + throw new Error( + `refusing to replace command-scoped core.hooksPath (${configSource(entry)}); ` + + `${ALLOW_HOOKS_PATH_OVERRIDE} cannot override transient command configuration`, + ) + } + if (entry.scope === 'worktree') { + throw new Error( + `refusing to replace worktree-scoped core.hooksPath (${configSource(entry)}); ` + + 'a worktree-specific custom path must be integrated or removed explicitly', + ) + } + throw new Error( + `refusing to replace core.hooksPath from unsupported ${entry.scope} scope (${configSource(entry)})`, + ) +} + async function main() { const probe = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }) if (probe.status !== 0) return - const root = probe.stdout.trim() + const root = stripGitLineTerminator(probe.stdout) const isWindows = process.platform === 'win32' const lefthook = join(root, 'node_modules', '.bin', isWindows ? 'lefthook.cmd' : 'lefthook') if (!existsSync(lefthook)) return assertSupportedGit(root) - const gitDirectory = git(['rev-parse', '--absolute-git-dir'], root).stdout.trim() - const commonOutput = git(['rev-parse', '--git-common-dir'], root).stdout.trim() + const gitDirectory = stripGitLineTerminator(git(['rev-parse', '--absolute-git-dir'], root).stdout) + const commonOutput = stripGitLineTerminator(git(['rev-parse', '--git-common-dir'], root).stdout) const commonDirectory = isAbsolute(commonOutput) ? commonOutput : resolve(root, commonOutput) const commonConfigPath = join(commonDirectory, 'config') const worktreeConfigPath = join(gitDirectory, 'config.worktree') const hooksPath = join(gitDirectory, HOOKS_DIRECTORY) const releaseLock = await acquireInstallLock(commonDirectory) + let installationError try { + const worktreeEntries = fileConfigEntries(root, worktreeConfigPath, 'core.hooksPath') + const includedWorktreeEntry = worktreeEntries.find( + entry => !originIsFile(entry.origin, root, worktreeConfigPath), + ) + if (includedWorktreeEntry !== undefined) { + refuseScopedHooksPath({ ...includedWorktreeEntry, scope: 'worktree' }) + } const worktreePath = assertSingle( - fileConfigValues(root, worktreeConfigPath, 'core.hooksPath'), + worktreeEntries.map(entry => entry.value), 'worktree core.hooksPath', ) - if (worktreePath !== undefined && worktreePath !== hooksPath) refuseCustomHooksPath(root, worktreePath) - - const effectivePath = effectiveConfigValue(root, 'core.hooksPath') - const effectivePathIsOwned = effectivePath === hooksPath && worktreePath === hooksPath - if ( - effectivePath !== undefined - && !effectivePathIsOwned - && process.env[ALLOW_HOOKS_PATH_OVERRIDE] !== '1' - ) { - refuseCustomHooksPath(root, effectivePath) + if (worktreePath !== undefined && worktreePath !== hooksPath) { + refuseScopedHooksPath({ origin: `file:${worktreeConfigPath}`, scope: 'worktree', value: worktreePath }) } + const effectiveEntry = effectiveConfigEntry(root, 'core.hooksPath') + if (effectiveEntry !== undefined) { + const effectivePathIsOwned = effectiveEntry.scope === 'worktree' + && effectiveEntry.value === hooksPath + && worktreePath === hooksPath + && originIsFile(effectiveEntry.origin, root, worktreeConfigPath) + if (!effectivePathIsOwned) { + if (effectiveEntry.scope === 'command' || effectiveEntry.scope === 'worktree') { + refuseScopedHooksPath(effectiveEntry) + } + if (!['system', 'global', 'local'].includes(effectiveEntry.scope)) { + refuseScopedHooksPath(effectiveEntry) + } + if (process.env[ALLOW_HOOKS_PATH_OVERRIDE] !== '1') { + refuseInheritedHooksPath(effectiveEntry) + } + } + } + assertConditionalHooksPaths(root, worktreeConfigPath) ensureOwnedHooksDirectory(hooksPath) ensureWorktreeConfig(root, commonConfigPath) @@ -262,6 +583,15 @@ async function main() { try { git(['config', '--worktree', 'core.hooksPath', hooksPath], root) pathChanged = worktreePath === undefined + const installedEntry = effectiveConfigEntry(root, 'core.hooksPath') + if ( + installedEntry === undefined + || installedEntry.scope !== 'worktree' + || installedEntry.value !== hooksPath + || !originIsFile(installedEntry.origin, root, worktreeConfigPath) + ) { + throw new Error('new worktree-local core.hooksPath did not become the effective direct worktree value') + } runLefthook(root, lefthook) } catch (error) { if (pathChanged) { @@ -269,8 +599,21 @@ async function main() { } throw error } + } catch (error) { + installationError = error + throw error } finally { - releaseLock() + try { + releaseLock() + } catch (releaseError) { + if (installationError !== undefined) { + throw new AggregateError( + [installationError, releaseError], + `Lefthook installation failed: ${String(installationError)}; installer lock release also failed: ${String(releaseError)}`, + ) + } + throw releaseError + } } } diff --git a/scripts/install-lefthook.spec.ts b/scripts/install-lefthook.spec.ts index 81f1be2897..b54a7ab864 100644 --- a/scripts/install-lefthook.spec.ts +++ b/scripts/install-lefthook.spec.ts @@ -62,7 +62,17 @@ import { execFileSync } from 'node:child_process' import { join } from 'node:path' if (process.argv.slice(2).join(' ') !== 'install --force') process.exit(64) -const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim() +const rootOutput = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }) +const root = rootOutput.endsWith('\\n') ? rootOutput.slice(0, -1) : rootOutput +const forbiddenConfigKey = process.env.DSH_TEST_FORBIDDEN_GIT_CONFIG_KEY +if (forbiddenConfigKey !== undefined) { + try { + execFileSync('git', ['config', '--get', forbiddenConfigKey], { encoding: 'utf8' }) + process.exit(92) + } catch (error) { + if (error === null || typeof error !== 'object' || !('status' in error) || error.status !== 1) throw error + } +} const hooksPath = execFileSync('git', ['config', '--get', 'core.hooksPath'], { encoding: 'utf8' }).trim() mkdirSync(hooksPath, { recursive: true }) const running = join(hooksPath, '.fake-lefthook-running') @@ -101,11 +111,11 @@ function installFakeLefthook(root: string): void { chmodSync(shim, 0o755) } -function createFixture(): Fixture { +function createFixture(names: { main?: string; linked?: string } = {}): Fixture { const container = mkdtempSync(join(tmpdir(), 'dsh-lefthook-')) fixtures.push(container) - const main = join(container, 'main') - const linked = join(container, 'linked') + const main = join(container, names.main ?? 'main') + const linked = join(container, names.linked ?? 'linked') const env: NodeJS.ProcessEnv = { ...process.env, GIT_AUTHOR_EMAIL: 'hooks@example.test', @@ -144,6 +154,18 @@ function hooksPath(fixture: Fixture, root: string): string { return join(gitDirectory(fixture, root), 'dsh-hooks') } +function installLockPath(fixture: Fixture): string { + return join(commonDirectory(fixture), 'dsh-lefthook-install.lock') +} + +async function waitForPath(path: string): Promise { + const deadline = Date.now() + 5_000 + while (!existsSync(path)) { + if (Date.now() >= deadline) throw new Error(`timed out waiting for ${path}`) + await new Promise(resolveWait => setTimeout(resolveWait, 10)) + } +} + function runInstaller( fixture: Fixture, root: string, @@ -226,6 +248,69 @@ describe('worktree-local Lefthook installer', () => { expect(existsSync(join(hooksPath(fixture, fixture.main), '.fake-lefthook-running'))).toBe(false) }) + it('leaves stale installer locks for explicit recovery', async () => { + const fixture = createFixture() + const lockPath = installLockPath(fixture) + const completed = spawnSync(process.execPath, ['-e', '']) + expect(completed.status).toBe(0) + const staleRecord = `${String(completed.pid)} 00000000-0000-4000-8000-000000000000\n` + writeFileSync(lockPath, staleRecord) + + const results = await Promise.all(Array.from( + { length: 4 }, + () => runInstaller(fixture, fixture.main), + )) + + for (const result of results) { + expect(result.status).toBe(1) + expect(result.stderr).toContain('stale Lefthook installer lock') + expect(result.stderr).toContain('remove it manually') + } + expect(readFileSync(lockPath, 'utf8')).toBe(staleRecord) + expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) + expect(gitResult(fixture, fixture.main, ['config', '--get', 'extensions.worktreeConfig']).status).toBe(1) + }) + + it('leaves invalid installer locks for explicit recovery', async () => { + const fixture = createFixture() + const lockPath = installLockPath(fixture) + const invalidRecord = 'not an installer lock\n' + writeFileSync(lockPath, invalidRecord) + + const result = await runInstaller(fixture, fixture.main) + + expect(result.status).toBe(1) + expect(result.stderr).toContain('invalid Lefthook installer lock') + expect(result.stderr).toContain('remove it manually') + expect(readFileSync(lockPath, 'utf8')).toBe(invalidRecord) + expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) + }) + + it('does not release an installer lock whose ownership changed', async () => { + const fixture = createFixture() + const lockPath = installLockPath(fixture) + const runningPath = join(hooksPath(fixture, fixture.main), '.fake-lefthook-running') + const install = runInstaller(fixture, fixture.main, { DSH_TEST_LEFTHOOK_DELAY_MS: '250' }) + await waitForPath(runningPath) + const replacementRecord = 'replacement owner\n' + writeFileSync(lockPath, replacementRecord) + + const result = await install + expect(result.status).toBe(1) + expect(result.stderr).toContain('installer lock ownership changed') + expect(readFileSync(lockPath, 'utf8')).toBe(replacementRecord) + }) + + it.skipIf(process.platform === 'win32')('preserves trailing spaces in worktree paths', async () => { + const fixture = createFixture({ main: 'main ', linked: 'linked ' }) + + for (const root of [fixture.main, fixture.linked]) { + const result = await runInstaller(fixture, root) + expect(result.status, result.stderr).toBe(0) + expect(git(fixture, root, ['config', '--worktree', '--get', 'core.hooksPath'])).toBe(hooksPath(fixture, root)) + } + }) + it('preserves user-owned hook paths unless an inherited value is explicitly overridden', async () => { const fixture = createFixture() const customHook = join(fixture.main, 'custom-hooks/pre-commit') @@ -257,6 +342,202 @@ describe('worktree-local Lefthook installer', () => { expect(git(fixture, fixture.linked, ['config', '--worktree', '--get', 'core.hooksPath'])).toBe('linked-custom-hooks') }) + it('refuses migration keys loaded through active or conditional common-config includes', async () => { + for (const includeKey of ['include.path', 'includeIf.onbranch:conditional.path']) { + for (const key of ['core.worktree', 'core.bare']) { + const fixture = createFixture() + const commonConfig = join(commonDirectory(fixture), 'config') + const includedConfig = join(fixture.container, `${includeKey.split('.')[0]}-${key.replace('.', '-')}.gitconfig`) + const value = key === 'core.worktree' ? fixture.main : 'true' + git(fixture, fixture.main, ['config', '--file', includedConfig, key, value]) + git(fixture, fixture.main, ['config', '--file', commonConfig, includeKey, includedConfig]) + + const result = await runInstaller(fixture, fixture.linked) + + expect(result.status).toBe(1) + expect(result.stderr).toContain(key) + expect(result.stderr).toContain(includedConfig) + expect(gitResult(fixture, fixture.main, ['config', '--get', 'extensions.worktreeConfig']).status).toBe(1) + expect(existsSync(join(hooksPath(fixture, fixture.linked), 'pre-commit'))).toBe(false) + } + } + }) + + it('allows a conditional common-config include unrelated to migration or hooks', async () => { + const fixture = createFixture() + const commonConfig = join(commonDirectory(fixture), 'config') + const includedConfig = join(fixture.container, 'conditional-identity.gitconfig') + git(fixture, fixture.main, ['config', '--file', includedConfig, 'user.email', 'conditional@example.test']) + git(fixture, fixture.main, [ + 'config', + '--file', + commonConfig, + 'includeIf.onbranch:conditional.path', + includedConfig, + ]) + + const result = await runInstaller(fixture, fixture.linked) + + expect(result.status, result.stderr).toBe(0) + expect(git(fixture, fixture.linked, ['config', '--get', 'core.hooksPath'])).toBe(hooksPath(fixture, fixture.linked)) + }) + + it('never overrides a command-scoped hook path', async () => { + const fixture = createFixture() + const commandHooks = join(fixture.container, 'command-hooks') + const sentinel = join(commandHooks, 'pre-commit') + write(sentinel, '#!/bin/sh\n# command-scope sentinel\n', 0o755) + + const result = await runInstaller(fixture, fixture.main, { + DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE: '1', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'core.hooksPath', + GIT_CONFIG_VALUE_0: commandHooks, + }) + + expect(result.status).toBe(1) + expect(result.stderr).toContain('command-scoped core.hooksPath') + expect(readFileSync(sentinel, 'utf8')).toBe('#!/bin/sh\n# command-scope sentinel\n') + expect(gitResult(fixture, fixture.main, ['config', '--get', 'core.hooksPath']).status).toBe(1) + expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) + }) + + it('never overrides a hook path behind a command-scoped conditional include', async () => { + const fixture = createFixture() + const includedConfig = join(fixture.container, 'command-conditional.gitconfig') + const includedHooks = join(fixture.container, 'command-conditional-hooks') + git(fixture, fixture.main, ['config', '--file', includedConfig, 'core.hooksPath', includedHooks]) + + const result = await runInstaller(fixture, fixture.main, { + DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE: '1', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'includeIf.onbranch:conditional.path', + GIT_CONFIG_VALUE_0: includedConfig, + }) + + expect(result.status).toBe(1) + expect(result.stderr).toContain('command-scoped conditional include') + expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) + }) + + it('does not pass unrelated command-scoped Git config to Lefthook', async () => { + const fixture = createFixture() + + const result = await runInstaller(fixture, fixture.main, { + DSH_TEST_FORBIDDEN_GIT_CONFIG_KEY: 'dsh.testSentinel', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'dsh.testSentinel', + GIT_CONFIG_VALUE_0: 'must-not-reach-lefthook', + }) + + expect(result.status, result.stderr).toBe(0) + expect(existsSync(join(hooksPath(fixture, fixture.main), 'pre-commit'))).toBe(true) + }) + + it('never overrides a hook path included by worktree config', async () => { + const fixture = createFixture() + const commonConfig = join(commonDirectory(fixture), 'config') + const worktreeConfig = join(gitDirectory(fixture, fixture.main), 'config.worktree') + const includedConfig = join(fixture.container, 'included-worktree.gitconfig') + const includedHooks = join(fixture.container, 'included-hooks') + const sentinel = join(includedHooks, 'pre-commit') + write(sentinel, '#!/bin/sh\n# included-worktree sentinel\n', 0o755) + git(fixture, fixture.main, ['config', '--file', includedConfig, 'core.hooksPath', includedHooks]) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'core.repositoryFormatVersion', '1']) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'extensions.worktreeConfig', 'true']) + git(fixture, fixture.main, ['config', '--file', worktreeConfig, 'include.path', includedConfig]) + + const result = await runInstaller(fixture, fixture.main, { + DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE: '1', + }) + + expect(result.status).toBe(1) + expect(result.stderr).toContain('worktree-scoped core.hooksPath') + expect(git(fixture, fixture.main, ['config', '--get', 'core.hooksPath'])).toBe(includedHooks) + expect(readFileSync(sentinel, 'utf8')).toBe('#!/bin/sh\n# included-worktree sentinel\n') + expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) + }) + + it('refuses an inactive conditional worktree include that can later provide a hook path', async () => { + const fixture = createFixture() + const commonConfig = join(commonDirectory(fixture), 'config') + const worktreeConfig = join(gitDirectory(fixture, fixture.linked), 'config.worktree') + const includedConfig = join(fixture.container, 'conditional-worktree.gitconfig') + const includedHooks = join(fixture.container, 'conditional-hooks') + const sentinel = join(includedHooks, 'pre-commit') + write(sentinel, '#!/bin/sh\n# conditional-worktree sentinel\n', 0o755) + git(fixture, fixture.main, ['config', '--file', includedConfig, 'core.hooksPath', includedHooks]) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'core.repositoryFormatVersion', '1']) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'extensions.worktreeConfig', 'true']) + git(fixture, fixture.main, [ + 'config', + '--file', + worktreeConfig, + 'includeIf.onbranch:conditional.path', + includedConfig, + ]) + + const result = await runInstaller(fixture, fixture.linked) + + expect(result.status).toBe(1) + expect(result.stderr).toContain('worktree-scoped conditional include') + expect(result.stderr).toContain('includeif.onbranch:conditional.path') + expect(gitResult(fixture, fixture.linked, ['config', '--worktree', '--get', 'core.hooksPath']).status).toBe(1) + expect(existsSync(hooksPath(fixture, fixture.linked))).toBe(false) + + git(fixture, fixture.linked, ['switch', '-c', 'conditional']) + expect(git(fixture, fixture.linked, ['config', '--get', 'core.hooksPath'])).toBe(includedHooks) + expect(readFileSync(sentinel, 'utf8')).toBe('#!/bin/sh\n# conditional-worktree sentinel\n') + }) + + it('requires opt-in for inherited conditional includes that can later provide a hook path', async () => { + for (const scope of ['local', 'global']) { + const fixture = createFixture() + const commonConfig = join(commonDirectory(fixture), 'config') + const conditionalOwner = scope === 'local' + ? commonConfig + : fixture.env.GIT_CONFIG_GLOBAL + if (conditionalOwner === undefined) throw new Error('fixture global config path is missing') + const includedConfig = join(fixture.container, `${scope}-conditional.gitconfig`) + const includedHooks = join(fixture.container, `${scope}-conditional-hooks`) + git(fixture, fixture.main, ['config', '--file', includedConfig, 'core.hooksPath', includedHooks]) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'core.repositoryFormatVersion', '1']) + git(fixture, fixture.main, ['config', '--file', commonConfig, 'extensions.worktreeConfig', 'true']) + git(fixture, fixture.main, [ + 'config', + '--file', + conditionalOwner, + 'includeIf.onbranch:conditional.path', + includedConfig, + ]) + + const refused = await runInstaller(fixture, fixture.linked) + + expect(refused.status).toBe(1) + expect(refused.stderr).toContain('inherited conditional include') + expect(refused.stderr).toContain('DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE=1') + expect(gitResult(fixture, fixture.linked, ['config', '--worktree', '--get', 'core.hooksPath']).status).toBe(1) + + const optedIn = await runInstaller(fixture, fixture.linked, { + DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE: '1', + }) + expect(optedIn.status, optedIn.stderr).toBe(0) + + git(fixture, fixture.linked, ['switch', '-c', 'conditional']) + expect(git(fixture, fixture.linked, ['config', '--get', 'core.hooksPath'])).toBe(hooksPath(fixture, fixture.linked)) + + const repeatedRefusal = await runInstaller(fixture, fixture.linked) + expect(repeatedRefusal.status).toBe(1) + expect(repeatedRefusal.stderr).toContain('inherited conditional include') + expect(git(fixture, fixture.linked, ['config', '--get', 'core.hooksPath'])).toBe(hooksPath(fixture, fixture.linked)) + + const repeatedOptIn = await runInstaller(fixture, fixture.linked, { + DSH_LEFTHOOK_ALLOW_HOOKS_PATH_OVERRIDE: '1', + }) + expect(repeatedOptIn.status, repeatedOptIn.stderr).toBe(0) + } + }) + it('restores the previous hook lookup when Lefthook installation fails', async () => { const fixture = createFixture() const common = commonDirectory(fixture) @@ -283,14 +564,14 @@ describe('worktree-local Lefthook installer', () => { expect(gitResult(fixture, fixture.main, ['config', '--get', 'extensions.worktreeConfig']).status).toBe(1) }) - it.skipIf(process.platform === 'win32')('rejects Git without worktree-config support before mutation', async () => { + it.skipIf(process.platform === 'win32')('rejects Git without config-scope support before mutation', async () => { const fixture = createFixture() const realGit = commandResult('which', ['git'], fixture.main, fixture.env).stdout.trim() const fakeBin = join(fixture.container, 'fake-bin') const fakeGit = join(fakeBin, 'git') write( fakeGit, - `#!/bin/sh\nif [ "$1" = "--version" ]; then echo "git version 2.19.0"; exit 0; fi\nexec "${realGit}" "$@"\n`, + `#!/bin/sh\nif [ "$1" = "--version" ]; then echo "git version 2.25.0"; exit 0; fi\nexec "${realGit}" "$@"\n`, 0o755, ) @@ -298,7 +579,7 @@ describe('worktree-local Lefthook installer', () => { PATH: `${fakeBin}:${fixture.env.PATH ?? ''}`, }) expect(result.status).toBe(1) - expect(result.stderr).toContain('Git 2.20 or newer is required') + expect(result.stderr).toContain('Git 2.26 or newer is required') expect(gitResult(fixture, fixture.main, ['config', '--get', 'extensions.worktreeConfig']).status).toBe(1) expect(existsSync(hooksPath(fixture, fixture.main))).toBe(false) })