Skip to content

fix(hooks): a linked worktree is one whose git-dir differs from its git-common-dir - #7749

Draft
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-7259-structural-linked-worktree-test
Draft

fix(hooks): a linked worktree is one whose git-dir differs from its git-common-dir#7749
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-7259-structural-linked-worktree-test

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #7259

Sibling PR (objectstack, same defect, same lines, one flight): objectstack-ai/objectstack#15924
Sibling card: objectstack-ai/objectstack#11809

What was wrong

Both worktree-first guards decided "am I in a linked worktree?" by substring-matching the git-dir path:

case "$gitdir" in
  */worktrees/*) exit 0 ;;   # any path containing `worktrees`, not a linked worktree
esac

That is a test for the characters worktrees appearing anywhere in a path. A primary checkout that merely lives under a directory named worktrees matched it, and both guards allowed edits into a shared primary checkout — the exact failure worktree-first exists to stop.

It was also depth-dependent, which is what made it reachable in practice. git rev-parse --git-dir prints a RELATIVE .git at a repo toplevel and an ABSOLUTE path from any subdirectory, and the guards hand git the edited file's nearest EXISTING ancestor. Measured on the fixture, unchanged from the card:

target ancestor handed to git verdict before
ODD/README.md repo toplevel block — correct only by accident
ODD/brand/new/f.ts walks up to toplevel block — ditto
ODD/pkg/x.ts a subdirectory allow — WRONG
ODD/pkg/brand/new/f.ts a subdirectory allow — WRONG

In a real repo almost every edit is to a file in a subdirectory that already exists. Note the fixture path is not exotic: an operator who keeps trees under a worktrees/ parent directory gets a silently unguarded primary checkout.

The fix — structural, not a sharper pattern

A linked worktree's git-dir (.git/worktrees/NAME) differs from its git-common-dir (.git). A primary checkout has the two equal, and so does a submodule (.git/modules/NAME for both) — which is why neither needs a special case. This holds regardless of how the path is spelled.

canon_dir() { ( cd "$1" 2>/dev/null && pwd -P ) || printf '%s' "$1"; }

gitdir="$(git -C "$d" rev-parse --absolute-git-dir 2>/dev/null)" || exit 0
commondir="$(git -C "$d" rev-parse --git-common-dir 2>/dev/null)" || exit 0
case "$commondir" in /*) ;; *) commondir="$d/$commondir" ;; esac
[ "$(canon_dir "$gitdir")" != "$(canon_dir "$commondir")" ] && exit 0

Two details are load-bearing, and both were measured on this box (git 2.43.0) rather than assumed:

  1. --git-common-dir prints RELATIVE to the directory queried.git at a toplevel, ../.git from a subdirectory. It must be resolved against that directory before the comparison. Ablating only that one line turns 47 matrix cases from block to allow: compared raw it never equals the absolute git-dir, so the guard fails open at every depth.
  2. --absolute-git-dir alone is NOT a fix, exactly as the sibling card's second comment warns. It removes the toplevel/subdirectory asymmetry by making the guard fail open everywhere instead of somewhere. Measurement (1) is the direct evidence for that.

Both sides go through one canonicalisation helper, so symlinked temp dirs and git's relative printing cannot make two spellings of the same directory look different.

A failed rev-parse keeps today's behaviour — not being inside a repo is not this hook's business.

Red / green

The self-test edits landed first, so the flip is shown in both directions. The old hooks were taken from HEAD into a scratch directory; the checked-in tree was never mutated.

New matrices against the OLD hooks:

guard-main-checkout.selftest.sh        114 passed, 2 failed
  FAIL want=block got=allow  $ODD/pkg/x.ts
  FAIL want=block got=allow  $ODD/pkg/brand/new/f.ts
guard-main-checkout-bash.selftest.sh   124 passed, 2 failed
  FAIL want=block got=allow  sed -i s/a/b/ $ODD/pkg/x.ts
  FAIL want=block got=allow  echo x > $ODD/pkg/x.ts

Only the subdirectory cases redden — the toplevel ones passed against the old hook too, because they blocked by accident. That is the sibling card's sharpened trigger condition reproducing exactly.

New matrices against the NEW hooks, and every other hook matrix in this repo:

guard-main-checkout-bash.selftest.sh   126 passed, 0 failed   exit=0
guard-main-checkout.selftest.sh        116 passed, 0 failed   exit=0
guard-shared-stash.selftest.sh          48 passed, 0 failed   exit=0
guard-tree-enum.selftest.sh             36 passed, 0 failed   exit=0

The Bash matrix goes 121 to 126 (the worktrees-segment fixture it never had). The Edit/Write matrix stays at 116 — four cases flipped rather than added. Hook Self-Tests discovers all four by find, so nothing here needed a workflow edit.

Non-vacuity. The recipe printed in the matrix footer was re-aimed at the line that now exists, and then run, so it is not a dead mutation: deleting the linked-worktree escape reddens 29 cases. The deletion was confirmed on disk by a before/after grep count (1 to 0) before the reading was trusted.

Self-test changes

  • The KNOWN HOLE banner and its "record of today's behaviour" prose are gone. The four worktrees-segment cases stay, re-homed under a section titled for the primary checkout they describe, and all four now expect block.
  • The positive direction is named where it was already pinned: $WT is a real linked worktree made with git worktree add, at a path carrying no worktrees segment, allowed at both its toplevel ($WT/README.md) and in a subdirectory ($WT/pkg/x.ts). Both depths matter because git answers them differently. No new case was needed — a comment now says why those rows are load-bearing.
  • The Bash matrix gains the ODD fixture and five cases: three blocks into it (subdirectory via sed -i, subdirectory via redirection, toplevel) plus the primary-checkout control and a linked-worktree control beside them.
  • Submodules keep their existing pins and stay green with no special case, because git-dir equals git-common-dir there. Verified directly on a real submodule fixture.
  • Two stale sentences in this file's PORTED header are corrected rather than left to rot: it claimed "the only edit below the header is the one remaining KNOWN HOLE section" (there is no longer such a section) and measured the cross-repo residual at 9 diff lines (now 10, see below).

Cross-repo alignment

Both repos move together in one flight, and the port stays verbatim. After the change, measured with diff -u:

  • guard-main-checkout.sh10 changed diff lines, all inside comment blocks (the pre-existing worktree-recipe block, plus the one line carrying each repo's own card number). Stripping comments and blank lines, the 41 code lines are identical.
  • guard-main-checkout-bash.sh — 82 changed diff lines: 76 comments, and 6 lines of message prose inside the blocked-message heredoc (each repo's own issue reference and its reflow). Stripping comments, blank lines and heredoc bodies, the 342 code lines are identical.

So the residual is comment-and-message-prose only; no executable line differs between the two repos in either hook — which is what this matrix's own header demands.

Gates

Derived from this repo's AGENTS.md, package.json and .github/workflows/ for a .claude/hooks/** change. All at HEAD 2248bba, exit codes captured before any pipe:

command exit
node scripts/check-changeset-presence.mjs 0
pnpm check:control-bytes 0
pnpm check:shell-escape-residue 0
pnpm check:governed-queue-guard (self-test) 0
pnpm lint (whole repo, turbo run lint, 47/47 tasks) 0
all four .claude/hooks/*.selftest.sh (what Hook Self-Tests runs) 0
node scripts/check-governed-queue-guard.mjs --test on the four paths 3 — GOVERNED

Changeset. This repo has no skip-changeset label and none was invented or applied. The authority is scripts/check-changeset-presence.mjs, and its verdict line on this diff is:

Compared the working tree with 565f2b6aa (merge-base with origin/main): 4 file(s) changed,
0 of them published source of a package the release covers, 0 of them a manifest whose
published contract moved, 0 under a package changesets ignores, 0 changeset(s) added.
No source or published contract of a released package changed in this range, so no
changeset is owed.

So no .changeset/*.md is added — not by exemption, but because nothing guarded changed. Whole-repo lint is reported as a full run rather than a narrowing; for the record its scan surface is **/*.{ts,tsx}, and this diff contains no TypeScript, so the 2883 warnings it reports are pre-existing and untouched.

Maintainer notes

This PR is governed surface (.claude/**) and is parked as a draft: no seat flips it ready, enqueues it, or arms auto-merge. Attribution for this change: authored in Claude Code session session_019RfFHiRCSs3JXLK4cwcfox.

维护者速读(草稿)

改了什么 —— 两个 worktree-first 守卫判断"我是不是在 linked worktree 里"的方式换掉了。原来靠路径里有没有 worktrees 这几个字符,现在问 git 一个结构性问题:git-dir 和 git-common-dir 是不是同一个目录,不同才是 linked worktree。四个文件,两个钩子加它们各自的自测矩阵;objectstack 同步落一份,可执行行逐字节相同。

为什么改 —— 这个守卫存在的唯一理由,就是拦住"往共享主 checkout 里写"。而它恰好在最常见的情形下失灵:只要你的仓库放在一个叫 worktrees 的目录底下(比如 ~/worktrees/objectui 这种再普通不过的布局),编辑任何子目录里的文件都会被放行 —— 而真实开发里几乎每一次编辑都是子目录里的已有文件。失灵时没有任何报错,agent 的改动就这么写进共享树,下一个 agent 切 HEAD 时静默清掉。这不是理论风险,是守卫在它最该起作用的那一类输入上直接失效。

风险与代价(含回滚) —— 改动面很小:两个钩子里各一段判定,没有新依赖,git rev-parse --git-common-dir 是老接口(本机实测 git 2.43.0)。收紧方向是"以前放行的现在拦住",所以理论上的代价是误拦 —— 但本仓四个矩阵共 326 个用例全绿(加 objectstack 那边共 379),含子模块、真 linked worktree、非仓库目录三类正向场景,没有一条正常路径被误伤。真正的坑我们提前量过并写进了代码注释:--git-common-dir 打印的是相对路径,不先解析就比较会让守卫在所有深度上失效(实测 47 条用例翻车),所以卡片上"光换 --absolute-git-dir 就行"的说法是错的,这版没有采纳。回滚成本接近零:revert 这一个 commit 即可,守卫回到今天的行为,没有数据迁移、没有配置、没有下游消费者。本仓不欠 changeset —— 不是走豁免,是 check-changeset-presence 判定这次没动任何发版包的源码。

席位意见 ——

你要做的 —— 这是 governed surface(.claude/**),按规矩只能你本人合。请看一眼两个仓的 PR(这一个和 objectstack 的 https://github.com/objectstack-ai/objectstack/pull/15924),确认判定换法你认可,然后手动合并;两个仓要一起合,否则两边守卫会短暂不一致。席位不会翻 ready、不会入队、不会开自动合并。


🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…it-common-dir

Both worktree-first guards decided "am I in a linked worktree?" by substring-matching
the git-dir path against `*/worktrees/*`. That is a test for the characters
`worktrees` appearing anywhere in a path, not a test for a linked worktree: a PRIMARY
checkout that merely lives under a directory named `worktrees` matched it, and both
guards allowed edits into a shared primary checkout — the exact failure worktree-first
exists to stop (see #7259).

The verdict was also depth-dependent, which is what made it reachable in practice.
`git rev-parse --git-dir` prints a RELATIVE `.git` at a repo toplevel and an ABSOLUTE
path from any subdirectory, and the guards hand git the edited file's nearest EXISTING
ancestor. So the same unguarded checkout blocked for a path resolving to the toplevel
and allowed for anything resolving to a subdirectory — and in a real repo almost every
edit is to a file in a subdirectory that already exists.

Replaced with the structural test: a linked worktree's git-dir (.git/worktrees/NAME)
differs from its git-common-dir (.git); a primary checkout has the two equal, and so
does a submodule (.git/modules/NAME for both), so neither needs a special case.

Two details are load-bearing and both are measured, not assumed:

  * `--git-common-dir` prints RELATIVE to the directory queried (`.git` at a toplevel,
    `../.git` from a subdirectory), so it must be resolved against that directory
    before the comparison. Compared raw it never equals the absolute git-dir and the
    guard fails open at EVERY depth — ablating just that line turns 47 matrix cases
    from block to allow.
  * `--absolute-git-dir` alone is NOT a fix. It removes the toplevel/subdirectory
    asymmetry by making the guard fail open everywhere instead of somewhere.

Both sides are canonicalised through one helper so symlinked temp dirs and git's
relative printing cannot make two spellings of the same directory look different.

Self-tests: the four `worktrees`-segment cases are re-homed out of the KNOWN HOLE
section and all four now expect `block`; the Bash matrix gains that fixture and five
cases it never had. The non-vacuity recipe is re-aimed at the line that now exists,
so it is not a dead mutation.

The executable lines stay byte-identical to the sibling repo's copies of both hooks.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox

os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

ACCEPT — lands #7259 as ruled, one flight with objectstack PR #15924 (#11809). Governed (.claude/**), so this PR stays draft at the governed terminal: both approvers requested; needs-user-decision goes on the PR with the final 「维护者速读」 posted beneath this verdict. The two repos must merge together — the executable lines are identical and the guards' whole value is that they agree.

What the seat verified, at head 2248bbad82, in its own compare worktree

  • Four files, +89/−40: both guards replace the */worktrees/* substring case with the structural test (--absolute-git-dir--git-common-dir, the relative common-dir resolved against the queried directory, one canon_dir helper); a failed rev-parse keeps today's behaviour.
  • Direct reproduction by the seat on a fresh fixture (TMP/worktrees/oddrepo, a PRIMARY checkout): the new hook answers ODD/pkg/x.ts with rc 2 (block), where the substring test allowed it.
  • All four .claude/hooks/*.selftest.sh green on the head: 126 · 116 · 48 · 36 passed, 0 failed; the Bash matrix gained the ODD fixture (121 → 126); the Edit/Write matrix's four ODD cases now all expect block; KNOWN HOLE residue 0; the PORTED header's two stale sentences corrected.
  • Cross-repo parity, seat's own read: comment-stripped guard-main-checkout.sh is line-identical between this head and objectstack PR #15924's head.
  • This repo has no skip-changeset label and none was invented; check-changeset-presence reads no changeset owed (no published source changed). Body line 1 Fixes #7259. CI, seat's read 15:1xZ: 30 check runs, 27 success, 3 skipped, none failing, none pending (Hook Self-Tests and Governed Surface Queue Guard among the green).

Reconciliation: the 13:59Z Claim: comment's branch spelling and its "first pair only" scope for this repo were the seat's text, superseded by the dispatch brief the dev correctly followed; the seat edits its own claim comment to what landed.

Implemented-by: os-dev executor, flight objectui #7259 + objectstack #11809, branches claude/issue-7259-structural-linked-worktree-test / claude/issue-11809-structural-linked-worktree-test
Reviewed-by: pm-dispatch skills seat, https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox


Generated by Claude Code

@claude
claude Bot requested review from hotlong and os-zhuang September 5, 2026 15:07

os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

维护者速读

改了什么:两个「禁止改共享主检出」的守卫钩子(Edit/Write 那个和 Bash 那个)判断「当前是不是 linked worktree」的方法换掉:原来看路径里有没有 worktrees 这几个字,现在问 git 的结构事实(git-dir 与 git-common-dir 是否不同,不同才是 linked worktree)。四个文件:两个钩子 + 两个自测矩阵;objectstack PR #15924 同步落一份,可执行行逐行相同。

为什么改:守卫的唯一职责是拦住往共享主检出里写。原判据在最常见的布局下失效:仓库放在名为 worktrees 的目录下(如 ~/worktrees/objectui),改任何子目录里的已有文件都被放行,且无任何报错;顶层文件反而被拦,让人误以为守卫在工作。席位在干净夹具上直接复现:新钩子拦截(rc 2),旧判据放行。

风险与代价(含回滚):方向是收紧(以前放行的现在拦),理论代价是误拦;本仓四个矩阵 326 用例全绿(两仓合计 379),含子模块、真 linked worktree、非仓库目录三类正向场景。关键坑已实测并写进注释:--git-common-dir 打印相对路径,不先解析会让守卫在所有深度失效(实测 47 用例翻车),所以「换 --absolute-git-dir 即可」的说法是错的,未采纳。无新依赖。本仓不欠 changeset(check-changeset-presence 判定未动任何发版包源码,不是走豁免)。回滚 = revert 一个 commit。两仓必须一起合。

席位意见:建议批准,两仓同批。四轴:业务——守卫在最常见布局下失效是实测的;长远——用 git 的结构判据替代路径猜测;防 AI 错——这正是防 agent 误写共享树的那道门;创业阶段——四文件、无新依赖、不扩面。

你要做的:两仓一起批准并人工合并(本 PR 与 objectstack #15924)。一字:是/否。


Generated by Claude Code

os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Status for the approvers: the objectstack half of this flight, PR #15924, MERGED at 16:03:37Z (924f0fed1). Until this PR lands, the two repos' worktree-first guards read a linked worktree by different tests (structural there, substring here), which is the divergence the pair was ruled to avoid. Nothing else changed on this head (2248bbad82; 30 checks green); the seat's ACCEPT and 速读 above stand.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants