Skip to content

Implement wt sync command for worktrees#14

Merged
dev-ankit merged 1 commit into
mainfrom
claude/implement-wt-sync-eAbD7
Jan 18, 2026
Merged

Implement wt sync command for worktrees#14
dev-ankit merged 1 commit into
mainfrom
claude/implement-wt-sync-eAbD7

Conversation

@dev-ankit

Copy link
Copy Markdown
Owner

Add comprehensive sync functionality to wt-worktree tool:

  • Add git operations (stash, pull, rebase) in git.py
  • Implement sync_worktree and sync_worktrees methods in worktree.py
  • Add wt sync command with --all, --include, --exclude, --rebase options
  • Add 6 comprehensive tests for sync functionality
  • Update PRD.md and notes.md with implementation details

The sync command:

  • Stashes uncommitted changes before syncing
  • Pulls from upstream branch
  • Optionally rebases onto default base
  • Restores stashed changes
  • Handles conflicts gracefully by continuing with other worktrees
  • Provides detailed status output for each worktree

All tests passing (72/72).

Add comprehensive sync functionality to wt-worktree tool:

- Add git operations (stash, pull, rebase) in git.py
- Implement sync_worktree and sync_worktrees methods in worktree.py
- Add wt sync command with --all, --include, --exclude, --rebase options
- Add 6 comprehensive tests for sync functionality
- Update PRD.md and notes.md with implementation details

The sync command:
- Stashes uncommitted changes before syncing
- Pulls from upstream branch
- Optionally rebases onto default base
- Restores stashed changes
- Handles conflicts gracefully by continuing with other worktrees
- Provides detailed status output for each worktree

All tests passing (72/72).
Copilot AI review requested due to automatic review settings January 18, 2026 15:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements comprehensive sync functionality for the wt-worktree tool, adding the ability to synchronize worktrees with their upstream branches. The implementation includes automatic stashing of uncommitted changes, pulling from upstream, optional rebasing onto a default base branch, and restoration of stashed changes.

Changes:

  • Added git operations (stash, pull, rebase, fetch) in git.py
  • Implemented sync_worktree and sync_worktrees methods in worktree.py with conflict handling
  • Added wt sync CLI command with --all, --include, --exclude, and --rebase options

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tools/wt-worktree/wt/git.py Added stash_changes, stash_pop, pull_branch, rebase_branch, and fetch_remote functions for sync operations
tools/wt-worktree/wt/worktree.py Implemented sync_worktree and sync_worktrees methods to orchestrate sync operations across worktrees
tools/wt-worktree/wt/cli.py Added sync command with argument validation and output formatting for multiple sync scenarios
tools/wt-worktree/tests/test_cli.py Added 6 basic tests covering command invocation patterns but not actual sync functionality with remotes
tools/wt-worktree/notes.md Documented implementation details, error handling approach, and lessons learned
tools/wt-worktree/PRD.md Updated to mark Story 8 as complete and removed sync from non-goals

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +563 to +565
for line in result["message"].split('\n'):
if line:
info(f"[{wt_name}] {line}")

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output formatting in the success message print loop is incorrect. When splitting multi-line messages that contain "\n✓" prefixes, the loop will print lines like "[worktree_name] ✓ Already based on origin/main" instead of the intended formatting. The newline prefixes added in sync_worktree at lines 486, 491, and 502 will cause the first character of continuation lines to be a newline, resulting in empty strings when split.

Suggested change
for line in result["message"].split('\n'):
if line:
info(f"[{wt_name}] {line}")
message = result["message"].lstrip('\n')
first_line_printed = False
for line in message.split('\n'):
if not line:
continue
if not first_line_printed:
info(f"[{wt_name}] {line}")
first_line_printed = True
else:
# Indent continuation lines without repeating the worktree name
info(f" {line}")

Copilot uses AI. Check for mistakes.
result["error"] = "failed to stash changes"
return result

try:

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sync operation does not fetch from the remote before pulling. The pull_branch function directly pulls from the remote branch, but without a prior fetch, it will use potentially stale remote-tracking branch information. Consider adding a git.fetch_remote(remote, wt_path) call before pulling to ensure the remote-tracking branches are up to date, or ensure that users understand they need to fetch separately.

Suggested change
try:
try:
# Ensure remote-tracking branches are up to date before pulling
info(f"[{wt_name}] Fetching from {remote}...")
git.fetch_remote(remote, wt_path)

Copilot uses AI. Check for mistakes.
conflict_type = f["error"].replace("_", " ")
msg = f" {f['name']} - {conflict_type}, run 'wt switch {f['name']}' to resolve"
if f.get("stashed"):
msg += "\n stashed changes preserved in stash@{0}"

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stash conflict message has inconsistent indentation. Line 554 adds a newline followed by 9 spaces of indentation, but line 552 only has 2 spaces. This will result in misaligned output. The indentation should match or the formatting should be adjusted for better readability.

Suggested change
msg += "\n stashed changes preserved in stash@{0}"
msg += "\n stashed changes preserved in stash@{0}"

Copilot uses AI. Check for mistakes.
Comment on lines +452 to +456
pull_success, pull_msg = git.pull_branch(remote_branch, wt_path, remote)

if not pull_success:
result["error"] = f"pull {pull_msg}"
return result

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a pull results in a conflict, git leaves the repository in a conflicted merge state, but the code does not abort the merge (unlike the rebase conflict handler at line 479). This means if stash pop is attempted after a pull conflict, it will likely fail because the working directory is not clean. Consider adding a check to abort the merge on pull conflicts using git merge --abort to ensure the repository is left in a clean state, similar to how rebase conflicts are handled.

Copilot uses AI. Check for mistakes.
return True, "merged"
else:
# Check for conflict
if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr:

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull_branch function detects conflicts but does not abort the merge, leaving the repository in a conflicted state. When this function returns False with "conflict" message, the caller in sync_worktree should handle cleanup by aborting the merge using git merge --abort before returning, to ensure the repository is left in a clean state for potential stash pop operations.

Suggested change
if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr:
if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr:
# Abort the merge to leave repo in clean state
run_git(["merge", "--abort"], cwd=path, check=False)

Copilot uses AI. Check for mistakes.
Comment on lines +463 to +467
try:
ahead, _ = git.get_ahead_behind(branch, upstream, self.repo_root)
result["message"] = f"✓ Fast-forward: {ahead} commits"
except git.GitError:
result["message"] = "✓ Fast-forward"

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit counting logic after a fast-forward is incorrect. The function get_ahead_behind compares the local branch with its upstream, but immediately after a successful pull, the local branch should be equal to or ahead of the upstream (not behind). The ahead value returned here represents how many commits the local branch is ahead of the upstream, which would be 0 after a fast-forward pull. To show how many commits were pulled, you should compare the branch state before and after the pull, or parse the pull output for the commit range.

Copilot uses AI. Check for mistakes.
Comment on lines +489 to +493
try:
ahead, _ = git.get_ahead_behind(branch, default_base, self.repo_root)
result["message"] += f"\n✓ Rebased, {ahead} commits ahead"
except git.GitError:
result["message"] += "\n✓ Rebased"

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit counting logic after a rebase is incorrect. The function get_ahead_behind compares the local branch with the default base branch, but this will show how many commits the local branch is ahead of the base (which could be the same as before rebase if there were no new commits on the base). To show the actual number of commits that were rebased, you would need to track the commit count before the rebase or parse the rebase output.

Copilot uses AI. Check for mistakes.
Comment on lines +486 to +502
result["message"] += "\n✓ Already based on " + default_base
else:
# Count commits ahead of base
try:
ahead, _ = git.get_ahead_behind(branch, default_base, self.repo_root)
result["message"] += f"\n✓ Rebased, {ahead} commits ahead"
except git.GitError:
result["message"] += "\n✓ Rebased"

result["success"] = True

finally:
# Step 4: Pop stash if we stashed earlier
if result["stashed"]:
info(f"[{wt_name}] Restoring uncommitted changes...")
if git.stash_pop(wt_path):
result["message"] += "\n✓ Stash applied"

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message formatting contains inconsistent newlines. The function adds "\n✓" prefix for multi-line messages (lines 486, 491, 502), but then splits on newlines and prints each line individually with the worktree name prefix (lines 563-565). This will result in lines starting with just "✓" instead of proper formatting. Either remove the "\n" prefixes and let the printing loop handle formatting, or print the message as-is without splitting.

Copilot uses AI. Check for mistakes.
name, message (for succeeded) or name, error, stashed (for failed)
"""
# Determine which worktrees to sync
all_worktrees = self.list_worktrees()

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable all_worktrees is not used.

Suggested change
all_worktrees = self.list_worktrees()

Copilot uses AI. Check for mistakes.
@dev-ankit dev-ankit merged commit 242f363 into main Jan 18, 2026
9 checks passed
@dev-ankit dev-ankit deleted the claude/implement-wt-sync-eAbD7 branch January 18, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants