fix(guardrails): align worktree creation with upstream pattern (Issue #144)#152
fix(guardrails): align worktree creation with upstream pattern (Issue #144)#152
Conversation
…144) Replace --detach with --detach --no-checkout + git reset --hard to reliably populate worktree working directories. On failure, cleanup broken worktrees with git worktree remove --force. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add git worktree remove --force cleanup when git worktree add itself fails, preventing orphaned partial worktrees. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
New PR opened -- automated review will run on the next push. To trigger a manual review, comment |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Pull request overview
Aligns delegated-agent worktree creation with an upstream Git pattern to avoid empty/incorrect worktrees during team/subagent delegation (Issue #144), while improving cleanup on error paths.
Changes:
- Switch worktree creation to
git worktree add --detach --no-checkoutfollowed bygit reset --hardto populate files. - Add
git worktree remove --forcecleanup on failures in steps 1–3. - Keep a post-creation verification via
git ls-files --cachedto detect empty worktrees.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const made = await git(dir, ["worktree", "add", "--detach", "--no-checkout", next, "HEAD"]) | ||
| if (made.code !== 0) { | ||
| await git(dir, ["worktree", "remove", "--force", next]).catch(() => {}) | ||
| throw new Error(made.err || made.out || "Failed to create git worktree") |
There was a problem hiding this comment.
The cleanup on Step 1 failure isn’t guaranteed to remove a partially-created directory: git worktree remove only works for worktrees that were successfully registered, so if worktree add fails before registration you can still end up with a leftover next/ directory that will break subsequent retries. Consider adding a filesystem fallback (scoped to the yard(dir) base) to delete next when worktree remove fails/no-ops, and/or explicitly handle the “not a working tree” case.
| throw new Error(`Worktree created but population failed: ${populated.err || populated.out}`) | ||
| } | ||
|
|
||
| // Step 3: Verify files are actually present in the working directory |
There was a problem hiding this comment.
The Step 3 comment says this verifies files “present in the working directory”, but git ls-files --cached checks the index (tracked paths), not the filesystem contents. Either adjust the comment to match what’s being checked, or switch to a check that actually validates working tree files if that’s the intent.
| // Step 3: Verify files are actually present in the working directory | |
| // Step 3: Verify the worktree has tracked files in Git after reset |
Summary
git worktree add --detachwith--detach --no-checkout+git reset --hard(upstream pattern)git worktree remove --forcecleanup on all failure paths (Step 1, 2, and 3)git ls-files --cachedverification as safety netCloses #144
Review findings addressed
Test plan
🤖 Generated with Claude Code