fix(init): support git worktree hook shims#77
Open
wasabeef wants to merge 7 commits into
Open
Conversation
Why Git worktrees can run shared hooks with a worktree-specific git dir, so hooks that only looked for a local shim could miss the CLI after init from another checkout. User impact Commits made from bare or non-bare worktrees, including Agent View style directories, can now write Agent Note data through the common git dir shim while keeping session buffers isolated per worktree. Verification npm run build npm run typecheck npm run lint npm test Release note: Support Agent Note recording from git worktrees by sharing the hook shim through the common git directory.
Why Worktree behavior depends on Git's private git dir, common git dir, hook path, and user-chosen worktree directory layout. User impact The regression suite now covers bare and non-bare repositories, custom paths with spaces, detached and orphan worktrees, moved worktrees, duplicate basenames, and worktree-specific hooksPath. Verification npx tsx src/commands/init.test.ts npx tsx src/commands/deinit.test.ts npm test Release note: skip
Why The worktree implementation relies on Git's distinction between worktree-local and common git directories. User impact Maintainers can understand why session data remains per-worktree while hooks and shims are resolved through Git's shared paths. Verification npm run lint npm test Release note: skip
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds worktree-aware path resolution and dual-shim hook installation with a common-dir fallback, updates deinit to clean both local and common shims, adds extensive worktree-focused integration tests, and updates architecture/docs and an investigation entry describing the prior note-loss scenario. ChangesWorktree Support Implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@AGENTS.md`:
- Line 101: Replace the compound phrase "Agent View style worktree commits" with
the hyphenated form "Agent View-style worktree commits" in the AGENTS.md
sentence that reads "Git worktrees are supported by keeping session buffers in
each worktree's own git dir while sharing the repo-local CLI shim from the
common git dir. This must work for bare and non-bare repositories, arbitrary
worktree directory layouts, and Agent View style worktree commits after init
from the main checkout." to apply the requested hyphenation.
In `@CLAUDE.md`:
- Line 101: Replace the unhyphenated compound adjective "Agent View style
worktree commits" with the hyphenated form "Agent View-style worktree commits"
wherever it appears (e.g., in the sentence that currently reads about worktrees
sharing the repo-local CLI shim), ensuring the capitalization remains unchanged.
In `@docs/architecture.md`:
- Line 408: Replace the phrase "Agent View style worktrees" with the hyphenated
compound modifier "Agent View-style worktrees" in the document (look for the
exact string "Agent View style worktrees" around the sentence containing
"commits made inside Claude Agent View style worktrees") to improve readability.
In `@packages/cli/src/commands/deinit.test.ts`:
- Around line 9-11: Remove the unused shellSingleQuote function and replace all
execSync calls that build shell-interpolated command strings (instances using
string interpolation and quoting) with child_process.execFileSync calls that
pass the executable and argument array instead; import/require execFileSync
alongside execSync (or replace execSync usages entirely), convert calls like
execSync(`${cmd} '${arg}'`, opts) into execFileSync(cmd, [arg], opts) preserving
the original options, and update the specific test invocations that used
shellSingleQuote to use argument arrays so shell quoting is no longer needed.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 478ddfa4-7471-4eb3-ba77-39f3586d4601
⛔ Files ignored due to path filters (1)
packages/cli/dist/cli.jsis excluded by!**/dist/**,!**/dist/**
📒 Files selected for processing (9)
AGENTS.mdCLAUDE.mddocs/architecture.mddocs/knowledge/investigations.mdpackages/cli/src/commands/deinit.test.tspackages/cli/src/commands/deinit.tspackages/cli/src/commands/init.test.tspackages/cli/src/commands/init.tspackages/cli/src/paths.ts
Use execFileSync argument arrays for deinit test invocations that pass dynamic paths. Also apply CodeRabbit's hyphenation feedback for Agent View-style worktree docs. Verification: npm run typecheck; npm run lint; npx tsx src/commands/deinit.test.ts; npm test Release note: skip
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/architecture.md`:
- Around line 402-409: The hook-directory example still shows using the
`core.hooksPath` fallback, which contradicts the text that hook paths are
resolved by Git and may be worktree-specific; update the example to show
resolving the hook directory via Git (respecting worktree `$GIT_DIR`) instead of
relying on `core.hooksPath`, and adjust the `post-commit`/`pre-push` install
snippet to first check the worktree-local agent note shim (using the
Git-reported dir) then fall back to the common git-dir shim; replace references
to `core.hooksPath` in the snippet and surrounding explanation to reflect this
Git-based resolution.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 494cfcfc-bb65-4191-a259-b63f77d605d1
📒 Files selected for processing (4)
AGENTS.mdCLAUDE.mddocs/architecture.mdpackages/cli/src/commands/deinit.test.ts
Use the current branch for non-bare worktree fixture setup so CI runners whose git default branch is master still cover the same worktree layouts. Verification: npm run typecheck; npm run lint; mise x node@22.22.0 -- npx tsx src/commands/init.test.ts; mise x node@22.22.0 -- npm test Release note: skip
Explain that Agent Note asks Git for the effective hook directory and uses worktree-local then common shims at runtime. Verification: rg 'git config get core\.hooksPath|echo "\.git/hooks"' docs/architecture.md AGENTS.md CLAUDE.md docs Release note: skip
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/cli/src/commands/init.test.ts`:
- Around line 245-249: The test helper recordClaudeWorktreeCommit currently uses
execSync with shell-interpolated command strings (execSync(`git add
${shellSingleQuote(options.fileName)}`, { cwd }) and execSync(`git commit -m
${shellSingleQuote(options.commitMessage)}`, { cwd, env: withoutCodexThreadEnv()
})); replace these with execFileSync calls that pass argv arrays (e.g.,
execFileSync('git', ['add', options.fileName], { cwd }) and execFileSync('git',
['commit', '-m', options.commitMessage], { cwd, env: withoutCodexThreadEnv() }))
to avoid shell parsing issues, keeping use of shellSingleQuote unnecessary and
preserving cwd and env handling.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11d0e886-ed11-4d19-a88b-21870e489fd6
📒 Files selected for processing (2)
docs/architecture.mdpackages/cli/src/commands/init.test.ts
Use git argv calls inside the worktree commit helper so special paths and commit messages do not depend on shell parsing. Verification: npm run lint; mise x node@22.22.0 -- npx tsx src/commands/init.test.ts Release note: skip
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
git rev-parse --git-path hooks, so init works from main checkouts, linked worktrees, bare repositories, and worktree-specificcore.hooksPathsetups.Release Note Plan
Release note: Support Agent Note recording from git worktrees by sharing the hook shim through the common git directory.
Type of Change
Checklist
Notes for Reviewers
npx tsx src/commands/init.test.ts,npx tsx src/commands/deinit.test.ts,npm run build,npm run typecheck,npm run lint,npm test, andgit diff --check.docs/blog/is unrelated existing untracked content and is not included.🧑💬🤖 Agent Note
Total AI Ratio: ████████ 100%
Model:
gpt-5.4878c7ddfix(init): support git worktree hook shims51c7fd0test(init): cover git worktree layoutsf84e6b8docs: document git worktree support39aac55test(deinit): avoid shell-interpolated commands83d0fbctest(init): avoid hardcoded worktree base branch12d1a97docs: align hook directory guidance with git resolution3af909dtest(init): avoid shell interpolation in worktree helper💬 Prompts & Responses (40 shown / 47 total)
878c7ddfix(init): support git worktree hook shims51c7fd0test(init): cover git worktree layoutsf84e6b8docs: document git worktree support39aac55test(deinit): avoid shell-interpolated commands83d0fbctest(init): avoid hardcoded worktree base branch12d1a97docs: align hook directory guidance with git resolution3af909dtest(init): avoid shell interpolation in worktree helperSummary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes