Skip to content

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

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11809-structural-linked-worktree-test
Sep 5, 2026
Merged

fix(hooks): a linked worktree is one whose git-dir differs from its git-common-dir#15924
os-zhuang merged 1 commit into
mainfrom
claude/issue-11809-structural-linked-worktree-test

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #11809

Sibling PR (objectui, same defect, same lines, one flight): objectstack-ai/objectui#7749
Sibling card: objectstack-ai/objectui#7259

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.

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 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 card's sharpened trigger condition reproducing exactly.

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

guard-governed-enqueue.selftest.sh      50 passed, 0 failed   exit=0
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          51 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.

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 ($MAIN/sub/README.md, $MAIN/sub/pkg/x.ts, both block) and stay green with no special case, because git-dir equals git-common-dir there. Verified directly on a real submodule fixture.

Cross-repo alignment

Both repos move together in one flight. 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.

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived its own change set from git (4 paths). All 13 commands run, reconciled with --ran:

Run reconciliation - 13 derived, 13 run, 0 NOT-MEASURED, 0 UNRUN.

Every one exit=0 at HEAD 8527fa0352. Whole-repo pnpm lint through the shared verify lock (slot issue-11809): VERDICT command-exit 0. node scripts/pm/check-governed-merges.mjs --test on the four paths: exit 3 — GOVERNED, .claude/** x4.

One honest note on measurement: pnpm --filter @objectstack/lint run check:doc-formula-expressions first returned exit 3, PREREQUISITE NOT MET — an unbuilt workspace package, not a finding. It is recorded here as NOT MEASURED at that point, not as a red. @objectstack/formula and @objectstack/lint were built through the verify lock and the gate then ran green.

Exit codes were captured before any pipe throughout (cmd > log 2>&1; E=$?).

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。四个文件,两个钩子加它们各自的自测矩阵;objectui 同步落一份,可执行行逐字节相同。

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

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

席位意见 ——

你要做的 —— 这是 governed surface(.claude/**),按规矩只能你本人合。请看一眼两个仓的 PR(这一个和 objectui 的那一个),确认判定换法你认可,然后手动合并;两个仓要一起合,否则两边守卫会短暂不一致。席位不会翻 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 #11809).

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.

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 #11809 as ruled, one flight with objectui PR #7749 (#7259). Governed (.claude/**, predicate exit 3), 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 8527fa0352, in its own compare worktree

  • Four files, +85/−35: 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, both sides through one canon_dir helper); a failed rev-parse keeps today's behaviour. No sharper pattern anywhere.
  • 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); the hook from the merge base answers the same payload with rc 0 (the hole); a real linked worktree at a path with no worktrees segment is still allowed (rc 0).
  • All five .claude/hooks/*.selftest.sh green on the head: 50 · 126 · 116 · 51 · 36 passed, 0 failed; the Bash matrix grew 121 → 126 with the ODD fixture; the Edit/Write matrix's four ODD cases now all expect block; KNOWN HOLE residue 0 in both matrices.
  • Cross-repo parity, seat's own read: comment-stripped guard-main-checkout.sh is line-identical between this head and objectui PR A bearer-authenticated admin metadata write is stamped actor: 'system'req.user / req.userId are unset on the /meta PUT path #7749's head.
  • check-governed-merges --test exit 3 (.claude/** ×4); skip-changeset present; body line 1 Fixes #11809; CI, seat's read 15:1xZ: 32 check runs, 20 success, 12 skipped, none failing, none pending.

Two reconciliations the dev raised, both the seat's to own

  1. Branch names: the 13:59Z Claim: comments named …-linked-worktree-is-structural; the dispatch brief, written later, named …-structural-linked-worktree-test, and the dev rightly followed the brief. The seat edits its own claim comments on both cards to the landed spelling (option B), so the identity marker describes reality and no sweep reads the PRs as claim-less.
  2. Scope: the claim text scoped objectui to one hook pair; the brief ruled four files per repo, which is what landed and what parity requires. The claim text was the error; corrected in the same edit.

Implemented-by: os-dev executor, flight #11809 + objectui #7259, branches claude/issue-11809-structural-linked-worktree-test / claude/issue-7259-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)。四个文件:两个钩子 + 两个自测矩阵;objectui PR #7749 同步落一份,可执行行逐行相同。

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

风险与代价(含回滚):方向是收紧(以前放行的现在拦),理论代价是误拦;两仓五个矩阵共 379 用例全绿,含子模块、真 linked worktree、非仓库目录三类正向场景。关键坑已实测并写进注释:--git-common-dir 打印相对路径,不先解析会让守卫在所有深度失效(实测 47 用例翻车),所以卡片上「换 --absolute-git-dir 即可」的说法是错的,未采纳。无新依赖(git 2.5 起的接口)。回滚 = revert 一个 commit。两仓必须一起合,否则两边守卫短暂不一致。

席位意见:建议批准,两仓同批。四轴:业务——守卫在最常见布局下失效是实测的,不是推测;长远——用 git 的结构判据替代路径猜测,不会再被目录命名打败;防 AI 错——这正是防 agent 误写共享树的那道门,修好它保护的是别人的未提交工作;创业阶段——四文件、无新依赖、不扩面。

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


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review September 5, 2026 15:23
@os-zhuang
os-zhuang enabled auto-merge September 5, 2026 15:23
@os-zhuang
os-zhuang added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 924f0fe Sep 5, 2026
38 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11809-structural-linked-worktree-test branch September 5, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-user-decision size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

3 participants