Skip to content

Commit 3f6339e

Browse files
johanzanderclaude
andcommitted
feat: block edits from the main checkout, and detect two writers on a branch
Two halves of the same failure: work landing outside a worktree, and nobody noticing when it does. ## The hook CLAUDE.md has said "never edit any file on main, even a one-line doc fix" unconditionally for a long time, and it keeps being skipped. The reason is structural, not carelessness: it is prose, so it has to be REMEMBERED at the moment of the first edit — and that is exactly the moment a session which opened as a question has no reason to reconsider it. Six live sessions currently sit in the main checkout for perfectly good read-only reasons; nothing catches the one that quietly starts editing. check-worktree-path.sh already guarded CROSS-checkout edits and passed same-checkout ones, so main-to-main sailed through. It now also refuses any edit made from the main checkout, detected by --git-dir equalling --git-common-dir. That is a path comparison, the only shape docs/agents/rules.md sanctions here — it never guesses what a command will touch. Linked worktrees and sibling checkouts both differ, so both still work; the rule is "be in a worktree", not "be under .claude/". The denial names the remedy (EnterWorktree) and says what the main checkout still does — questions, gh, backlog, dispatch — because a block without a next move gets worked around. Residual gap, stated plainly: this governs Edit/Write/NotebookEdit. A Bash `sed -i` still writes. Guarding that would mean parsing command strings, which rules.md forbids for this hook and which has produced false positives here four times. ## The detector pr-state.sh gains a local-writer section. GitHub cannot see this: a branch with two writers looks normal through the API, because the divergence exists only between a local checkout and the remote and it collapses into an ordinary merge the moment someone reconciles. #619 is the worked example. One writer took the branch at 08:09 and worked from that base; another pushed 23031e7 at 09:34. The reviewer reviewed 23031e7 three times, twice with blocking findings, while the first line never held that commit. Fifteen hours later it landed as `Merge remote-tracking branch 'origin/fix/...' into fix/...` — a branch merged into itself, which is the fingerprint. `git rev-list --left-right` would have caught it at 09:34. Run against the live fleet it also surfaces the precursor state: #437 is 5 commits behind its own remote and #614 is 3, so a commit in either worktree diverges immediately. Skipped LOUDLY outside a checkout, since this script is also meant for a container fleet where a silent skip would read as "no divergence found". Verified by mutation: disabling the main-checkout guard reddens 2 of the 6 hook tests. The divergence detector is tested against a real two-clone scenario, not fixtures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e73c008 commit 3f6339e

5 files changed

Lines changed: 413 additions & 1 deletion

File tree

.claude/hooks/check-worktree-path.sh

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,45 @@ if [ "$target_root" != "$session_root" ]; then
5353
exit 0
5454
fi
5555

56+
# ---------------------------------------------------------------------------
57+
# Second check: is this session in a worktree at all?
58+
#
59+
# CLAUDE.md's rule is unconditional -- "Never edit any file on main, even a
60+
# one-line doc fix" -- and it is prose, so it has to be REMEMBERED at the moment
61+
# of the first edit. That is precisely when it isn't: a session that opens as a
62+
# question ("why does X happen?") and drifts into implementing never re-evaluates
63+
# a rule it had no reason to consider at the start. Six live sessions currently
64+
# sit in the main checkout for exactly that legitimate read-only reason.
65+
#
66+
# The cost is not hypothetical. PR #619's branch carries a merge of itself --
67+
# `Merge remote-tracking branch 'origin/fix/...' into fix/...` -- because two
68+
# writers worked the same branch from different bases and diverged for fifteen
69+
# hours. The reviewer reviewed one line three times while the other line, based
70+
# on a commit from 08:09, never saw a single verdict.
71+
#
72+
# So the check is mechanical and fires at the transition itself: the first
73+
# Edit/Write IS the moment a question becomes an implementation. A linked
74+
# worktree has its own git dir under the main one, so `--git-dir` and
75+
# `--git-common-dir` differ there and are identical in the main checkout. That
76+
# is a path comparison, the only shape docs/agents/rules.md sanctions for this
77+
# hook -- it never has to guess what a command string will touch.
78+
#
79+
# Sibling checkouts (`../bess-manager-feature/`) are linked worktrees too, so
80+
# they pass: this enforces "work in a worktree", not "work under .claude/".
81+
git_dir=$(git rev-parse --absolute-git-dir 2>/dev/null || true)
82+
common_dir=$(cd "$(git rev-parse --git-common-dir 2>/dev/null || echo .)" 2>/dev/null && pwd || true)
83+
84+
if [ -n "$git_dir" ] && [ -n "$common_dir" ] && [ "$git_dir" = "$common_dir" ]; then
85+
branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "?")
86+
reason="BLOCKED: this session is in the MAIN checkout ('${session_root}', branch '${branch}'), not a worktree, so editing '${target_path}' would write straight into the shared checkout. CLAUDE.md: work in a worktree before ANY edit -- unconditional, including a one-line doc fix. If this session started as a question and has become implementation work, that is the common path here and this is the moment to switch: call EnterWorktree (never \`git worktree add\`, which the sandbox denies), then redo the edit there. The main checkout stays read-only: questions, \`gh\`, the backlog, and dispatch all work fine from it."
87+
jq -n --arg reason "$reason" '{
88+
hookSpecificOutput: {
89+
hookEventName: "PreToolUse",
90+
permissionDecision: "deny",
91+
permissionDecisionReason: $reason
92+
}
93+
}'
94+
exit 0
95+
fi
96+
5697
echo '{"continue": true}'

backend/tests/test_pr_state.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ def _rows(out: str) -> str:
7777
@pytest.fixture
7878
def run(tmp_path: Path):
7979
"""Shim `gh pr list` with a fixture and return the script's stdout."""
80+
outside_a_repo = tmp_path / "not-a-repo"
81+
outside_a_repo.mkdir()
8082

8183
def _run(prs: list[dict]) -> str:
8284
bin_dir = tmp_path / "bin"
@@ -94,7 +96,12 @@ def _run(prs: list[dict]) -> str:
9496
capture_output=True,
9597
text=True,
9698
env=dict(os.environ, PATH=f"{bin_dir}:{os.environ['PATH']}"),
97-
cwd=REPO_ROOT,
99+
# NOT the repo: the script's local-writer section walks real
100+
# worktrees and fetches, which would make every classifier test slow,
101+
# networked, and dependent on whatever branches happen to exist.
102+
# Outside a checkout it takes the documented skip path instead, which
103+
# `test_the_local_writer_check_skips_loudly` pins.
104+
cwd=outside_a_repo,
98105
)
99106
assert proc.returncode == 0, proc.stderr
100107
return proc.stdout
@@ -314,3 +321,14 @@ def test_liveness_is_reported_as_unknown_rather_than_guessed(run) -> None:
314321

315322
assert "not a GitHub fact and is not" in out
316323
assert "guessed here" in out
324+
325+
326+
def test_the_local_writer_check_skips_loudly_outside_a_checkout(run) -> None:
327+
"""The script is meant to run against a container fleet too, where no
328+
worktree exists. A silent skip there would read as "no divergence found",
329+
which is the same silent-cap failure `sweep-prs` warns about — so the skip
330+
has to say that it proves nothing."""
331+
out = run([_pr(1)])
332+
333+
assert "SKIPPED" in out
334+
assert "says nothing about writers" in out
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
"""Tests for pr-state.sh's local-writer section — the two-writers detector.
2+
3+
This is the one thing GitHub cannot tell you. A branch with two writers looks
4+
completely normal through the API: the divergence exists only between a local
5+
checkout and the remote, and it collapses into an ordinary merge commit the
6+
moment somebody reconciles it.
7+
8+
PR #619 is why this exists. One writer took the branch at 08:09 and worked from
9+
that base; another pushed 23031e78 at 09:34. The reviewer reviewed 23031e78
10+
three times, twice with blocking findings, while the first line never held that
11+
commit at all. Fifteen hours later it landed as `Merge remote-tracking branch
12+
'origin/fix/...' into fix/...` — a branch merged into itself.
13+
14+
So the scenario below is built for real: a bare "origin", two clones that both
15+
commit to one branch, and the assertion that the script names it.
16+
"""
17+
18+
import json
19+
import os
20+
import stat
21+
import subprocess
22+
from pathlib import Path
23+
24+
import pytest
25+
26+
REPO_ROOT = Path(__file__).resolve().parents[2]
27+
SCRIPT = REPO_ROOT / "scripts" / "pr-state.sh"
28+
29+
GIT_ENV = {
30+
"PATH": "/usr/bin:/bin:/usr/local/bin:/opt/homebrew/bin",
31+
"GIT_CONFIG_GLOBAL": "/dev/null",
32+
"GIT_CONFIG_SYSTEM": "/dev/null",
33+
"GIT_AUTHOR_NAME": "t",
34+
"GIT_AUTHOR_EMAIL": "t@t",
35+
"GIT_COMMITTER_NAME": "t",
36+
"GIT_COMMITTER_EMAIL": "t@t",
37+
}
38+
39+
BRANCH = "fix/issue-592-vpp-idle-at-floor"
40+
41+
42+
def _git(cwd: Path, *args: str) -> str:
43+
proc = subprocess.run(
44+
["git", *args],
45+
cwd=cwd,
46+
check=True,
47+
capture_output=True,
48+
text=True,
49+
env={**GIT_ENV, "HOME": str(cwd)},
50+
)
51+
return proc.stdout.strip()
52+
53+
54+
def _commit(repo: Path, name: str) -> None:
55+
(repo / name).write_text(name)
56+
_git(repo, "add", name)
57+
_git(repo, "commit", "-q", "-m", name)
58+
59+
60+
@pytest.fixture
61+
def two_writers(tmp_path: Path) -> Path:
62+
"""A checkout whose branch has diverged from its own remote.
63+
64+
Writer B pushes; writer A, which branched earlier, commits locally without
65+
pulling. That is #619's shape exactly.
66+
"""
67+
origin = tmp_path / "origin.git"
68+
origin.mkdir()
69+
_git(origin, "init", "-q", "--bare", "-b", "main")
70+
71+
writer_b = tmp_path / "writer-b"
72+
_git(tmp_path, "clone", "-q", str(origin), "writer-b")
73+
_commit(writer_b, "seed")
74+
_git(writer_b, "push", "-q", "origin", "main")
75+
_git(writer_b, "checkout", "-q", "-b", BRANCH)
76+
_commit(writer_b, "from-b")
77+
_git(writer_b, "push", "-q", "origin", BRANCH)
78+
79+
writer_a = tmp_path / "writer-a"
80+
_git(tmp_path, "clone", "-q", str(origin), "writer-a")
81+
_git(writer_a, "checkout", "-q", "-b", BRANCH, "--no-track", "origin/main")
82+
_commit(writer_a, "from-a")
83+
# A never pulled B's push, so the two lines have no common tip.
84+
return writer_a
85+
86+
87+
def _run(cwd: Path, tmp_path: Path, prs: list[dict]) -> str:
88+
bin_dir = tmp_path / "bin"
89+
bin_dir.mkdir(exist_ok=True)
90+
(bin_dir / "prs.json").write_text(json.dumps(prs))
91+
gh = bin_dir / "gh"
92+
gh.write_text(f"#!/bin/sh\ncat '{bin_dir}/prs.json'\n")
93+
gh.chmod(gh.stat().st_mode | stat.S_IEXEC)
94+
95+
proc = subprocess.run(
96+
["bash", str(SCRIPT)],
97+
capture_output=True,
98+
text=True,
99+
cwd=cwd,
100+
env=dict(os.environ, PATH=f"{bin_dir}:{os.environ['PATH']}"),
101+
)
102+
assert proc.returncode == 0, proc.stderr
103+
return proc.stdout
104+
105+
106+
def _pr(number: int, branch: str) -> dict:
107+
return {
108+
"number": number,
109+
"title": f"pr {number}",
110+
"isDraft": True,
111+
"mergeable": "MERGEABLE",
112+
"mergeStateStatus": "CLEAN",
113+
"headRefName": branch,
114+
"updatedAt": "2026-08-17T00:00:00Z",
115+
"reviews": [],
116+
"commits": [{"committedDate": "2026-08-17T00:00:00Z"}],
117+
"comments": [
118+
{"body": "@claude-bot review", "createdAt": "2026-08-18T00:00:00Z"}
119+
],
120+
"statusCheckRollup": [
121+
{"name": "Fast tests", "status": "COMPLETED", "conclusion": "SUCCESS"}
122+
],
123+
}
124+
125+
126+
def test_two_writers_on_one_branch_are_named(two_writers: Path, tmp_path: Path) -> None:
127+
"""The #619 shape. Local has a commit the remote lacks AND the remote has a
128+
commit local lacks — which cannot happen with a single writer."""
129+
out = _run(two_writers, tmp_path, [_pr(619, BRANCH)])
130+
131+
assert "DIVERGED" in out
132+
assert "TWO WRITERS" in out
133+
assert "#619" in out
134+
135+
136+
def test_a_branch_in_sync_is_not_flagged(two_writers: Path, tmp_path: Path) -> None:
137+
"""Guards against a detector that shouts on every branch — which would get
138+
it ignored, the way a CONFLICTING PR with no checks got ignored."""
139+
_git(two_writers, "fetch", "-q", "origin")
140+
_git(two_writers, "reset", "-q", "--hard", f"origin/{BRANCH}")
141+
out = _run(two_writers, tmp_path, [_pr(619, BRANCH)])
142+
143+
assert "DIVERGED" not in out
144+
assert "in sync" in out
145+
146+
147+
def test_a_branch_with_no_open_pr_is_not_reported(
148+
two_writers: Path, tmp_path: Path
149+
) -> None:
150+
"""The fleet has ~47 worktrees and most have no open PR. Listing them all
151+
would bury the one line that matters."""
152+
out = _run(two_writers, tmp_path, [_pr(999, "some/other-branch")])
153+
154+
assert BRANCH not in out.split("Local writers")[-1]
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
"""Tests for .claude/hooks/check-worktree-path.sh.
2+
3+
The hook is the only mechanical enforcement of "work in a worktree before ANY
4+
edit". CLAUDE.md has stated that rule unconditionally for a long time and it
5+
still gets skipped, because prose has to be REMEMBERED at the moment of the
6+
first edit — and that is exactly the moment a session which opened as a question
7+
has no reason to reconsider it.
8+
9+
The damage is on the record. PR #619's branch carries a merge of itself
10+
(`Merge remote-tracking branch 'origin/fix/...' into fix/...`) because two
11+
writers worked the same branch from different bases and diverged for fifteen
12+
hours; the reviewer reviewed one line three times while the other, based on a
13+
commit from 08:09, never saw a verdict.
14+
15+
So these tests pin the decision the hook makes, against real git checkouts built
16+
in tmp_path rather than the developer's own layout.
17+
"""
18+
19+
import json
20+
import subprocess
21+
from pathlib import Path
22+
23+
import pytest
24+
25+
REPO_ROOT = Path(__file__).resolve().parents[2]
26+
HOOK = REPO_ROOT / ".claude" / "hooks" / "check-worktree-path.sh"
27+
28+
29+
def _git(cwd: Path, *args: str) -> None:
30+
subprocess.run(
31+
["git", *args],
32+
cwd=cwd,
33+
check=True,
34+
capture_output=True,
35+
env={
36+
"PATH": "/usr/bin:/bin:/usr/local/bin:/opt/homebrew/bin",
37+
"HOME": str(cwd),
38+
"GIT_CONFIG_GLOBAL": "/dev/null",
39+
"GIT_CONFIG_SYSTEM": "/dev/null",
40+
"GIT_AUTHOR_NAME": "t",
41+
"GIT_AUTHOR_EMAIL": "t@t",
42+
"GIT_COMMITTER_NAME": "t",
43+
"GIT_COMMITTER_EMAIL": "t@t",
44+
},
45+
)
46+
47+
48+
@pytest.fixture
49+
def repo(tmp_path: Path) -> Path:
50+
"""A real main checkout on `main` with one commit."""
51+
main = tmp_path / "main-checkout"
52+
main.mkdir()
53+
_git(main, "init", "-q", "-b", "main")
54+
(main / "seed.txt").write_text("seed\n")
55+
_git(main, "add", "seed.txt")
56+
_git(main, "commit", "-q", "-m", "seed")
57+
return main
58+
59+
60+
@pytest.fixture
61+
def worktree(repo: Path, tmp_path: Path) -> Path:
62+
"""A linked worktree of that checkout — the sanctioned place to edit."""
63+
wt = tmp_path / "linked-worktree"
64+
_git(repo, "worktree", "add", "-q", "-b", "feat/x", str(wt))
65+
return wt
66+
67+
68+
def _decide(cwd: Path, target: Path) -> dict:
69+
"""Run the hook as Claude Code would, and return its decision."""
70+
proc = subprocess.run(
71+
["bash", str(HOOK)],
72+
cwd=cwd,
73+
input=json.dumps({"tool_input": {"file_path": str(target)}}),
74+
capture_output=True,
75+
text=True,
76+
)
77+
assert proc.returncode == 0, proc.stderr
78+
return json.loads(proc.stdout)
79+
80+
81+
def _denial(decision: dict) -> str | None:
82+
hook_out = decision.get("hookSpecificOutput")
83+
if not hook_out or hook_out.get("permissionDecision") != "deny":
84+
return None
85+
return hook_out["permissionDecisionReason"]
86+
87+
88+
def test_editing_from_the_main_checkout_is_blocked(repo: Path) -> None:
89+
"""The gap this closes. The pre-existing check only fired when the target
90+
was a DIFFERENT checkout, so an edit made from the main checkout to a file
91+
in the main checkout — the exact shape of a question-session that drifted
92+
into implementing — sailed through."""
93+
reason = _denial(_decide(repo, repo / "seed.txt"))
94+
95+
assert reason is not None
96+
assert "MAIN checkout" in reason
97+
# The remedy has to be in the message: an agent that is blocked without
98+
# being told the next move retries or works around it.
99+
assert "EnterWorktree" in reason
100+
101+
102+
def test_the_message_names_the_branch_and_the_read_only_escape(repo: Path) -> None:
103+
"""Blocking is only half the job. The main checkout has legitimate
104+
read-only uses — six live sessions sit there asking questions, running
105+
`gh`, and dispatching — so the denial must say what still works, or it
106+
reads as "this session is useless here"."""
107+
reason = _denial(_decide(repo, repo / "seed.txt"))
108+
109+
assert reason is not None
110+
assert "branch 'main'" in reason
111+
assert "read-only" in reason
112+
113+
114+
def test_editing_inside_a_linked_worktree_is_allowed(worktree: Path) -> None:
115+
"""The rule must not block the sanctioned path, or it just gets disabled."""
116+
decision = _decide(worktree, worktree / "seed.txt")
117+
118+
assert _denial(decision) is None
119+
assert decision.get("continue") is True
120+
121+
122+
def test_a_new_file_in_a_worktree_is_allowed(worktree: Path) -> None:
123+
"""Resolution goes via the parent directory so a not-yet-created file still
124+
lands in the right checkout — pinned because most edits during
125+
implementation create files."""
126+
decision = _decide(worktree, worktree / "brand-new.py")
127+
128+
assert _denial(decision) is None
129+
130+
131+
def test_cross_checkout_edits_are_still_blocked(repo: Path, worktree: Path) -> None:
132+
"""Regression guard on the original purpose: a stale absolute path from an
133+
earlier turn writing into a different checkout."""
134+
reason = _denial(_decide(worktree, repo / "seed.txt"))
135+
136+
assert reason is not None
137+
assert "DIFFERENT checkout" in reason
138+
139+
140+
def test_outside_a_git_repo_the_hook_stays_out_of_the_way(tmp_path: Path) -> None:
141+
"""The hook governs this repo's worktree discipline, not the filesystem."""
142+
plain = tmp_path / "not-a-repo"
143+
plain.mkdir()
144+
decision = _decide(plain, plain / "file.txt")
145+
146+
assert _denial(decision) is None

0 commit comments

Comments
 (0)