Fix Git submodule being incorrectly identified as a worktree#129
Conversation
Both Git submodules and worktrees utilize a `.git` file with a `gitdir:` entry, but only worktrees point to the main repository's `.git/worktrees/...` administrative directory. Refines the `isWorktree` function to specifically verify that the `gitdir:` path references a `worktrees` administrative directory. Adds associated unit tests. Co-authored-by: athal7 <467872+athal7@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthrough
ChangesWorktree detection
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🤖 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 `@plugin/core/git.js`:
- Around line 233-235: Update the worktree detection logic around the gitdir
content check to parse the target path and require it to resolve beneath the
repository’s own .git/worktrees directory, rather than matching any path segment
named worktrees. Preserve valid worktree detection while rejecting nested
submodule paths such as .git/modules/worktrees/foo, and add a regression test
covering that case.
In `@test/unit/git-worktree.test.js`:
- Around line 78-79: Update the submodule setup in the git worktree test to
remove the global protocol.file.allow configuration and invoke submodule add
with the per-command git -c setting. Pass subRepo through the command execution
API’s argument mechanism rather than interpolating it into the shell command,
while preserving mainRepo as the working directory.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 927e206f-ecc6-4baf-beac-5dcd63f4897a
📒 Files selected for processing (2)
plugin/core/git.jstest/unit/git-worktree.test.js
| // Verify it contains a gitdir reference pointing to a worktree administrative directory | ||
| const content = await readFile(gitPath, 'utf-8') | ||
| return content.startsWith('gitdir:') | ||
| return content.startsWith('gitdir:') && /[/\\]worktrees[/\\][^/\\]+\s*$/.test(content) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file first, then inspect the relevant section.
ast-grep outline plugin/core/git.js --view expanded || true
echo '--- relevant lines ---'
sed -n '180,280p' plugin/core/git.js
echo '--- search for worktree/gitdir helpers ---'
rg -n "worktree|gitdir|modules|commonDir|worktrees" plugin/core/git.js plugin -g '!**/node_modules/**' || trueRepository: athal7/opencode-devcontainers
Length of output: 18821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline plugin/core/git.js --view expanded || true
echo '--- relevant lines ---'
sed -n '180,280p' plugin/core/git.js
echo '--- search for worktree/gitdir helpers ---'
rg -n "worktree|gitdir|modules|commonDir|worktrees" plugin/core/git.js plugin -g '!**/node_modules/**' || trueRepository: athal7/opencode-devcontainers
Length of output: 18821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- usages of isWorktree and getWorktreeMainRepo ---'
rg -n "\bisWorktree\b|\bgetWorktreeMainRepo\b" plugin -g '!**/node_modules/**' || true
echo '--- inspect create/list call sites ---'
sed -n '1,120p' plugin/core/worktree.js
echo '---'
sed -n '120,220p' plugin/core/worktree.js
echo '---'
sed -n '400,470p' plugin/index.jsRepository: athal7/opencode-devcontainers
Length of output: 9175
Anchor the gitdir: path to the repo’s own .git/worktrees/ directory.
/[/\\]worktrees[/\\][^/\\]+\s*$ also matches submodule admin dirs like .git/modules/worktrees/foo, so a submodule mounted at worktrees/foo can be misclassified as a worktree. Parse the gitdir: target and require it to live under the repository’s .git/worktrees/ tree; add a regression test for a nested worktrees/... submodule.
🤖 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 `@plugin/core/git.js` around lines 233 - 235, Update the worktree detection
logic around the gitdir content check to parse the target path and require it to
resolve beneath the repository’s own .git/worktrees directory, rather than
matching any path segment named worktrees. Preserve valid worktree detection
while rejecting nested submodule paths such as .git/modules/worktrees/foo, and
add a regression test covering that case.
| execSync('git config --global protocol.file.allow always', { cwd: mainRepo }) | ||
| execSync(`git submodule add ${subRepo} sub`, { cwd: mainRepo }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' test/unit/git-worktree.test.jsRepository: athal7/opencode-devcontainers
Length of output: 7385
Avoid global Git config and shell interpolation here
git config --global protocol.file.allow always leaks into later Git commands. Use git -c protocol.file.allow=always submodule add and pass subRepo as an argument instead of interpolating it into the shell command.
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 79-79: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 `@test/unit/git-worktree.test.js` around lines 78 - 79, Update the submodule
setup in the git worktree test to remove the global protocol.file.allow
configuration and invoke submodule add with the per-command git -c setting. Pass
subRepo through the command execution API’s argument mechanism rather than
interpolating it into the shell command, while preserving mainRepo as the
working directory.
Source: Linters/SAST tools
1 participant
Both Git submodules and worktrees use a
.gitfile containing agitdir: <path>reference. However, a worktree'sgitdiralways points to the main repository's.git/worktrees/...folder, whereas a submodule'sgitdirpoints to.git/modules/....Because
isWorktreepreviously only checked whether the.gitfile exists and starts withgitdir:, submodules were incorrectly identified as worktrees. This blocked users from executing worktree commands from within submodules.This PR refines the
isWorktreefunction to inspect thegitdirvalue, ensuring it refers to aworktreesadministrative directory (with/worktrees/followed by at least one directory component to prevent false positives from repository names containing 'worktrees'). It also adds corresponding unit tests to verify both cases.Fixes #127
PR created automatically by Jules for task 8200182990159883666 started by @athal7
Summary by CodeRabbit