Repository navigation
Conversation
…ract
The WorktreeCreate hook emitted a decision JSON on stdout, but the Claude
Code runtime expects this hook to "replace default git behavior" — it must
ensure the worktree directory exists and print ONLY that path on stdout.
The runtime read the JSON as a path, saw it was not a directory, and aborted
worktree creation ("returned a path that is not a directory"), forcing
agent-isolation / breezing worker spawns to fail.
Rewrite the Go handler and shell fallback to resolve the target path, reuse
an existing valid worktree or create one (origin default branch, HEAD
fallback), initialize .claude/state/, and print the path. The logic is
defensive about non-determinism: it never assumes the worktree does or does
not already exist, and never clobbers a non-empty pre-existing directory.
Update Go and shell regression tests to assert the path contract and
idempotent reuse. Rebuild all four platform binaries (v4.12.3).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughWorktreeCreateフックを「approve decision JSON」出力から「作成または再利用した worktree ディレクトリパスのみ」出力へ統一。無入力/不正JSON/decision JSON を cwd として扱わないガード、既存 worktree の再利用、git worktree 作成の origin フォールバック、及び .claude/state/worktree-info.json のベストエフォート記録を Go・Shell・テストで実装・検証した。 ChangesWorktreeCreate フック契約の統一化
Sequence DiagramsequenceDiagram
participant Client as stdin (worktreeInput)
participant Handler as HandleWorktreeCreate / worktree-create.sh
participant Resolver as resolveWorktreePath / jq/python3
participant Git as git (rev-parse, worktree add)
participant FS as filesystem
participant stdout as stdout
Client->>Handler: payload (session_id, cwd, tool_input)
Handler->>Resolver: extract/normalize cwd, tool_input
Resolver-->>Handler: selected TARGET
Handler->>Git: is_git_worktree? (git rev-parse)
alt already worktree
Git-->>Handler: reuse TARGET
else need create
Handler->>Git: git worktree add (-b preferred / fallback)
Git-->>Handler: result
end
Handler->>FS: mkdir .claude/state && write worktree-info.json (best-effort)
Handler->>stdout: print TARGET
🎯 4 (Complex) | ⏱️ ~45 minutesPossibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c1749341
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _, err := os.Stat(path); err != nil { | ||
| return false | ||
| } | ||
| cmd := exec.Command("git", "rev-parse", "--is-inside-work-tree") |
There was a problem hiding this comment.
This check treats any existing directory inside the main checkout as a reusable worktree, because Git documents --is-inside-work-tree as true when the cwd is “inside the work tree” (not necessarily at its root). If the default target .harness-worktrees/<session> was left behind by a partial run, or tool_input.path names an existing repo subdirectory, ensureWorktree skips git worktree add and prints a path that is still part of the main checkout; the shell fallback has the same predicate. Compare rev-parse --show-toplevel to the target (or otherwise verify the target is the actual worktree root) before reusing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f05da1c. isGitWorktree now uses git rev-parse --show-toplevel and requires it to equal the target (symlink-resolved via pwd -P), so a repo subdirectory or a leftover .harness-worktrees/ dir is no longer treated as a reusable worktree — it only matches an actual worktree root. Mirrored in the shell fallback (is_git_worktree). Regression test: TestIsGitWorktree_RepoSubdirNotWorktreeRoot.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/hook-handlers/worktree-create.sh (2)
157-160: 💤 Low valueprintf フォールバックのJSON エスケープが不完全
${SESSION_ID//\"/\\\"}はダブルクォートのみをエスケープしますが、バックスラッシュ、改行、タブなどの制御文字はエスケープされません。SESSION_IDやTARGETにこれらが含まれると不正なJSONが生成されます。通常のユースケースでは問題になりませんが、堅牢性を高めるには Python フォールバックの使用や、より完全なエスケープ処理を検討してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/hook-handlers/worktree-create.sh` around lines 157 - 160, The printf fallback that writes JSON to WORKTREE_INFO_FILE escapes only double quotes via "${SESSION_ID//\"/\\\"}" and "${TARGET//\"/\\\"}", which fails for backslashes and control characters; update the fallback to produce valid JSON by using a Python (or other JSON library) one-liner to safely serialize the object (including SESSION_ID, TARGET, CREATED_AT) or implement proper escaping for backslashes and control characters before writing; target the printf block that currently generates the JSON and replace it with the robust serializer so WORKTREE_INFO_FILE always contains valid JSON.
54-61: 💤 Low valueタブ文字を含むパスで解析が壊れる可能性
@tsvとIFS=$'\t' readを使用していますが、cwdやTOOL_WORKTREE_PATHにタブ文字が含まれている場合、フィールド境界が誤認識されます。実際にはパスにタブが含まれることは稀ですが、NUL区切り（@sh+evalまたは別のアプローチ）の方が堅牢です。現状のユースケースではタブ入りパスは想定外と思われるため、必須ではありませんが、将来の堅牢性のために検討してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/hook-handlers/worktree-create.sh` around lines 54 - 61, 現在の _parsed="$(printf '%s' "${INPUT}" | jq -r '… | `@tsv`')" と IFS=$'\t' read -r SESSION_ID CWD TOOL_WORKTREE_PATH でタブを区切りにしているため、cwd や TOOL_WORKTREE_PATH にタブが含まれると壊れます。修正するには jq 側でフィールドを NUL 文字で連結（例えば '[(...)] | join("\u0000")' など）して出力し、シェル側では read の -d '' オプションや -a 配列読み込みで NUL 区切りを扱うように変更して、INPUT → _parsed → read による SESSION_ID / CWD / TOOL_WORKTREE_PATH の復元を NUL 区切りで安全に行ってください。tests/test-worktree-create-hook.sh (1)
77-83: ⚡ Quick win
tool_input.worktreePathのテストケース追加を検討フック実装では
tool_input.worktreePathが指定された場合にそのパスを優先的に使用するロジックがありますが、このテストファイルではそのケースがカバーされていません。契約の重要な部分であるため、以下のようなテストケースの追加を推奨します：
# === 5. tool_input.worktreePath overrides default path === CUSTOM_PATH="${TMP_DIR}/custom-worktree" CUSTOM_OUT="${TMP_DIR}/custom.out" run_hook "{\"session_id\":\"worker-custom\",\"cwd\":$(json_str "${REPO}"),\"tool_input\":{\"worktreePath\":$(json_str "${CUSTOM_PATH}")}}" "${CUSTOM_OUT}" "${REPO}" CUSTOM_PRINTED="$(tr -d '\n' < "${CUSTOM_OUT}")" [ "${CUSTOM_PRINTED}" = "${CUSTOM_PATH}" ] || fail "tool_input.worktreePath not honored: ${CUSTOM_PRINTED}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test-worktree-create-hook.sh` around lines 77 - 83, Add a test that verifies the hook honors tool_input.worktreePath by invoking run_hook with a JSON payload containing "tool_input":{"worktreePath":<custom path>} (use variables like CUSTOM_PATH and CUSTOM_OUT), capture output into CUSTOM_PRINTED, and assert CUSTOM_PRINTED equals CUSTOM_PATH; if not, call fail with a clear message like "tool_input.worktreePath not honored: ${CUSTOM_PRINTED}". Ensure the test is placed after the idempotency check and uses the same run_hook and tr -d '\n' pattern as the other cases to remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go/internal/hookhandler/worktree_create.go`:
- Around line 162-170: The retry invocation builds the wrong git command: after
failing with "git worktree add -b <branch> <path>" the current retry uses
exec.Command("git", "worktree", "add", path) which will attempt to checkout a
branch named after the path instead of the intended branch; change the retry to
include the existing branch name (e.g., exec.Command("git", "worktree", "add",
path, branch) or exec.Command("git", "worktree", "add", path, branchName") using
the same branch variable used for the initial command) so the retry explicitly
checks out the intended branch into the worktree while keeping the same
CombinedOutput/error handling on retry and using the repoCWD Dir.
---
Nitpick comments:
In `@scripts/hook-handlers/worktree-create.sh`:
- Around line 157-160: The printf fallback that writes JSON to
WORKTREE_INFO_FILE escapes only double quotes via "${SESSION_ID//\"/\\\"}" and
"${TARGET//\"/\\\"}", which fails for backslashes and control characters; update
the fallback to produce valid JSON by using a Python (or other JSON library)
one-liner to safely serialize the object (including SESSION_ID, TARGET,
CREATED_AT) or implement proper escaping for backslashes and control characters
before writing; target the printf block that currently generates the JSON and
replace it with the robust serializer so WORKTREE_INFO_FILE always contains
valid JSON.
- Around line 54-61: 現在の _parsed="$(printf '%s' "${INPUT}" | jq -r '… | `@tsv`')"
と IFS=$'\t' read -r SESSION_ID CWD TOOL_WORKTREE_PATH でタブを区切りにしているため、cwd や
TOOL_WORKTREE_PATH にタブが含まれると壊れます。修正するには jq 側でフィールドを NUL 文字で連結（例えば '[(...)] |
join("\u0000")' など）して出力し、シェル側では read の -d '' オプションや -a 配列読み込みで NUL
区切りを扱うように変更して、INPUT → _parsed → read による SESSION_ID / CWD / TOOL_WORKTREE_PATH
の復元を NUL 区切りで安全に行ってください。
In `@tests/test-worktree-create-hook.sh`:
- Around line 77-83: Add a test that verifies the hook honors
tool_input.worktreePath by invoking run_hook with a JSON payload containing
"tool_input":{"worktreePath":<custom path>} (use variables like CUSTOM_PATH and
CUSTOM_OUT), capture output into CUSTOM_PRINTED, and assert CUSTOM_PRINTED
equals CUSTOM_PATH; if not, call fail with a clear message like
"tool_input.worktreePath not honored: ${CUSTOM_PRINTED}". Ensure the test is
placed after the idempotency check and uses the same run_hook and tr -d '\n'
pattern as the other cases to remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2bc752b0-68f1-4639-b1f6-8447b551c42d
📒 Files selected for processing (4)
go/internal/hookhandler/worktree_create.gogo/internal/hookhandler/worktree_create_test.goscripts/hook-handlers/worktree-create.shtests/test-worktree-create-hook.sh
…on retry Address PR Chachamaru127#168 review: - isGitWorktree used `git rev-parse --is-inside-work-tree`, which is true for ANY path inside a checkout. A repo subdirectory or a leftover .harness-worktrees/<session> dir would be wrongly reused, printing a path still part of the main checkout. Switch to `--show-toplevel` and require it to equal the target (symlink-resolved) — a true worktree root. - gitWorktreeAdd retry on an existing branch ran `git worktree add <path>`, which checks out a branch named after the dir basename instead of the intended harness/worker/<slug>. Pass the branch explicitly. Mirror both fixes in the shell fallback. Add regression tests: TestIsGitWorktree_RepoSubdirNotWorktreeRoot, TestGitWorktreeAdd_ReusesExistingBranch.
|
Heads-up on the CI here — there are two independent failures:
Thanks for the fix — the worktree-path-only contract change itself looks right. Generated by Claude Code |
Chachamaru127
left a comment
Chachamaru127
left a comment
There was a problem hiding this comment.
Reviewed locally against current main.
- Merges cleanly into
main(4 files: Go handler, shell fallback, and both tests). - Full
go test ./...passes (11 packages, exit 0) on the merged tree, including the 8 new helper unit tests andtests/test-worktree-create-hook.sh. - The fix is correct: the
WorktreeCreatehook must print only the resolved worktree path on stdout (diagnostics → stderr). The previous decision-JSON-on-stdout broke worktree creation ("returned a path that is not a directory"). ThelooksLikeHookDecisionJSONguard is a good defensive hardening against the legacy bug. - Defensive handling of pre-existing/non-empty dirs and safe-abort-on-failure (empty stdout) is sound.
The test-go / validate CI failures were against a stale base (and a sandbox-only commit-signing issue locally); re-running after updating the branch to current main.
LGTM.
Generated by Claude Code
|
Landed via #186 (merged to Two small CI fixes were needed to land on the current
All 5 checks (test-go, validate, CodeQL, Analyze, actionlint) were green on #186 before merge. Closing this in favor of #186. Generated by Claude Code |
… lands #168) Rewrite the WorktreeCreate Go handler and shell fallback to resolve/reuse/create the worktree, init .claude/state/, and print ONLY the worktree path on stdout (diagnostics to stderr), with a looksLikeHookDecisionJSON guard against the legacy decision-JSON-on-stdout bug that broke agent-isolation / breezing worker spawns. Also includes two CI fixes needed to land on current main: - gofmt worktree_create_test.go (format-lint gate) - track the .claude/state join refactor (worktreeStateDir helper) in tests/test-windows-worktree-support.sh, keeping the Windows-safe path-joining assertion. Original work by Ary Rabelo (#168). Closes #168. Co-authored-by: Ary Rabelo <aryrabelo@gmail.com>
|
Correction: my earlier comment said "#186" by mistake — this was actually landed via #188 (now merged to Summary stands: your two commits (@aryrabelo) were cherry-picked unchanged (authorship preserved, Generated by Claude Code |
Promote [Unreleased] to [4.13.3] - 2026-06-01 and bump all version surfaces from 4.13.2 to 4.13.3 (patch). - Cursor 初回利用の摩擦 4 点を解消 (Chachamaru127#193, Chachamaru127#194) - breezing 起動ナレーションを計画明示型に緩和 (Chachamaru127#190) - WorktreeCreate hook の出力修正 (Chachamaru127#188, lands Chachamaru127#168) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Problem
The
WorktreeCreatehook emitted a decision JSON ({"decision":"approve","reason":...}) on stdout. The Claude Code runtime treats this hook as a replacement for default git worktree behavior: it expects the hook to ensure the worktree directory exists and print only that path on stdout. The runtime read the JSON as a path, found it was not a directory ("returned a path that is not a directory"), and aborted worktree creation — breaking agent-isolation / breezing worker spawns.Fix
Rewrite the Go handler (
go/internal/hookhandler/worktree_create.go) and the shell fallback (scripts/hook-handlers/worktree-create.sh) so both:tool_input.path/worktreePath, else a deterministic.harness-worktrees/<session-slug>under the repo root)..claude/state/.The logic is defensive about non-determinism: it never assumes the worktree exists or not, never clobbers a non-empty pre-existing directory, and on any failure emits nothing on stdout (a safe abort). A
looksLikeHookDecisionJSONguard hardens both implementations against the legacy decision-JSON-as-cwd bug.Tests
go test ./internal/hookhandler/ -run Worktree— 17 pass, including 8 direct unit tests for the helpers (originDefaultRef,dirIsEmpty,absOrSelf,worktreeBranchName,gitWorktreeAdd,normalizeWorktreeCreateCWD,worktreeStateDir,isGitWorktree).tests/test-worktree-create-hook.sh— pass (shell parity).go vet— clean.Source-only change. Committed binaries are release-managed artifacts and are intentionally left untouched.
Summary by CodeRabbit
バグ修正
改善
テスト