Prevent repo sync from corrupting linked worktrees by sergiou87 · Pull Request #14060 · cli/cli

sergiou87 · 2026-08-04T10:59:33Z

Description

When gh repo sync updates a local branch that is not checked out in the current worktree, it previously used git update-ref. If that branch was checked out in another worktree, update-ref moved the shared branch ref without updating that worktree's index or files. The other worktree then incorrectly reported the difference from the new HEAD as staged changes.

Coming from #14076 (thanks @williammartin 🙇‍♂️): Before updating an existing non-current branch, inspect git worktree list --porcelain to determine whether another worktree has the branch checked out. If so, refuse the sync with an error that identifies the worktree and suggests running gh repo sync from there.

When the branch is not checked out in another worktree, update it with git branch --force -- <branch> <ref>. Unlike git update-ref, this command also refuses to move a branch that Git considers checked out, providing a mutation-time safety net for cases the pre-check may not detect.

How did you test this change?

Added unit and real-Git tests covering:

Parsing git worktree list --porcelain, including detached and bare worktrees, branch names with slashes, paths containing spaces, and Windows line endings.
Detecting when the synchronized branch is checked out in another worktree.
Returning a curated error containing the worktree path and recovery guidance.
Preserving the rejected branch ref and keeping the linked worktree clean.
Fast-forwarding a non-current branch.
Force-updating a diverged non-current branch.
Updating a branch in a normal single-worktree repository.

Added an acceptance test that creates a remote repository and linked worktree, verifies that syncing from the wrong worktree is refused without changing the target worktree, and confirms that syncing from the worktree where the branch is checked out still succeeds.

Ran:

$ go test ./pkg/cmd/repo/sync -count=1
ok      github.com/cli/cli/v2/pkg/cmd/repo/sync 0.062s

Key points

The explicit worktree check and git branch --force serve different purposes:

git worktree list --porcelain lets gh identify the owning worktree and return a curated, actionable error before attempting the update.
git branch --force remains a safety net at the mutation point. It protects cases the pre-check cannot identify and reduces the impact of state changing between detection and update.

There is still a check-then-update window, so the implementation does not rely on the pre-check alone for safety.

The command intentionally refuses instead of modifying another worktree. It directs the user to run gh repo sync from the worktree where the target branch is checked out, preserving the existing dirty-worktree checks and making the directory being changed explicit.

Authorship and follow-up

Who wrote this:

A human wrote it.
An agent wrote it under close human direction.
An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

@sergiou87 will read and reply directly. Name the account.
An agent will draft replies and @username will read them before they are posted.
Nobody has explicitly committed to replying.

Add a new integration-style test for `executeLocalRepoSync` that reproduces syncing a branch (`trunk`) currently checked out in another worktree and verifies the operation succeeds with a clean worktree. This covers a regression-prone Git edge case. The change also adds a `runGit` test helper plus required imports (`os`, `os/exec`, `filepath`, `strings`, and `require`) to set up and validate the repository state.

sergiou87 · 2026-08-04T11:03:26Z

+	}
+
+	err := executeLocalRepoSync(ghrepo.New("OWNER", "REPO"), "origin", opts)
+	require.Error(t, err)


Started writing this whole test to reproduce the problem, where an error wasn't thrown in this scenario. But now using git branch --force -- …, the user gets an error.

Copilot

Pull request overview

This pull request updates gh repo sync’s local-branch update mechanism to avoid corrupting linked worktrees when syncing a branch that’s checked out elsewhere. Instead of mutating refs directly, it delegates worktree-safety checks to Git so the operation is rejected when unsafe, and adds real-Git coverage for the relevant scenarios.

Changes:

Replace git update-ref usage with git branch --force -- <branch> <ref> when updating an existing non-current local branch.
Add real-Git tests covering fast-forward/force-update of non-current branches, single-worktree behavior, and rejection/cleanliness when the target branch is checked out in another worktree.

Show a summary per file

File	Description
pkg/cmd/repo/sync/git.go	Switches non-current branch update implementation to `git branch --force` to make Git enforce worktree safety.
pkg/cmd/repo/sync/sync_test.go	Adds real-Git tests exercising non-current branch updates and linked-worktree rejection behavior.

Review details

Tip

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

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Lite

sergiou87 · 2026-08-04T11:21:17Z

+	require.Equal(t, runGit(t, repoDir, "rev-parse", "FETCH_HEAD"), runGit(t, repoDir, "rev-parse", "trunk"))
+	require.Equal(t, "test", runGit(t, repoDir, "branch", "--show-current"))
+	require.Empty(t, runGit(t, repoDir, "status", "--porcelain"))
+}


The other 3 tests are scenarios suggested by Copilot when I asked about possible regressions in behavior when switching from update-ref to branch --force.

These tests passed before and after the change.

Copilot

Review details

Suppressed comments (1)

pkg/cmd/repo/sync/sync_test.go:212

The local variable name gitExecuter shadows the gitExecuter type in this scope, which makes the test harder to read and can confuse future edits (e.g. adding another composite literal later in the function). Rename the variable to avoid shadowing the type name.

	gitClient := &git.Client{RepoDir: repoDir}
	gitExecuter := &gitExecuter{client: gitClient}

	err := gitExecuter.UpdateBranch("trunk", "FETCH_HEAD")
	require.NoError(t, err)

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot

Review details

Suppressed comments (1)

pkg/cmd/repo/sync/sync_test.go:226

The real-git helper inherits the developer/CI machine’s global/system Git config, which can make these tests flaky (e.g. commit.gpgsign=true, init.templatedir, core.hooksPath, etc.). Other tests isolate Git config via GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM (see pkg/cmd/auth/shared/gitcredentials/helper_config_test.go:16-27). Consider isolating Git config for all runGit invocations to keep these tests hermetic.

func runGit(t *testing.T, repoDir string, args ...string) string {
	t.Helper()
	client := &git.Client{RepoDir: repoDir}
	cmd, err := client.Command(stdctx.Background(), args...)
	require.NoError(t, err)

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

Add shared test helper for isolated Git configuration and use it across credential and repository sync tests.

Copilot

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

williammartin · 2026-08-05T08:55:16Z

+}
+
+// IsolateGitConfig prevents tests from reading global and system Git configuration.
+func IsolateGitConfig(t Environment) {


Is there a reason you didn't just accept testing.T here? It seems like this isn't really intended for use for anything other than testing.

`git branch --force` already refuses to move a branch that another worktree has checked out, but it surfaces a raw git fatal. Detect the case up front with `git worktree list --porcelain` and return a curated error naming the worktree, matching the guidance we already give for uncommitted changes. Detection parses porcelain output rather than matching git's message, which varies by version ("used by worktree at" on 2.53, "checked out at" on older). `branch --force` stays as the safety net for states the check can't see, such as an in-progress bisect. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f88d532-4a31-47c7-ab50-a3d0b9a77fc4

Reproduces the issue end to end: `main` checked out in the primary worktree while `gh repo sync` runs from a linked worktree. Asserts the sync refuses, the primary worktree's ref and working tree are untouched, and that syncing from the worktree that owns the branch still fast forwards. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f88d532-4a31-47c7-ab50-a3d0b9a77fc4

williammartin

I've opened #14076 with some suggestions for an acceptance test and an error message wording improvement. A little bit vibey but shows what I mean. I'll leave it to you to decide what you want to do with this PR. Feel free to push to the branch, or take the commits to yours, or not.

Curate the error when the sync target is checked out in another worktree

Copilot

Review details

Suppressed comments (2)

pkg/cmd/repo/sync/sync.go:265

This adds a pre-check via BranchWorktreePath before mutating the ref, which reintroduces the check-then-update race the PR description says it is avoiding (the branch can become checked out/un-checked-out between the list and the update). A safer approach is to attempt UpdateBranch first (so Git is the source of truth), and only if that fails then call BranchWorktreePath to enrich the error message with the worktree path/tip.

			// Updating a branch checked out in another worktree would leave that
			// worktree's index and working tree stale, so refuse and point the user at it.
			worktreePath, err := git.BranchWorktreePath(branch)
			if err != nil {
				return err

pkg/cmd/repo/sync/git.go:31

Minor grammar: “states that check can't see” reads like a missing article and is a bit hard to parse.

// UpdateBranch moves branch to ref. It uses `git branch --force` rather than
// `git update-ref` because update-ref will happily move a branch that is checked out in
// another worktree, leaving that worktree's index and working tree stale. Callers should
// use BranchWorktreePath to detect and explain that case first; this is the safety net
// for states that check can't see, such as an in-progress bisect.

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Lite

sergiou87 added 6 commits August 3, 2026 16:17

Test non-current branch sync fast-forward

beca8f3

Test forced non-current branch sync

38e180c

Test branch update in single worktree

ec74f24

Prevent updating branches checked out in worktrees

2d2c7b3

Test rejected worktree sync preserves branch

292f0c0

Copilot AI lite review requested due to automatic review settings August 4, 2026 10:59

Copilot started reviewing on behalf of sergiou87 August 4, 2026 11:00 View session

sergiou87 commented Aug 4, 2026

View reviewed changes

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread pkg/cmd/repo/sync/sync_test.go

sergiou87 commented Aug 4, 2026

View reviewed changes

Use Git client in repo sync tests

25242c1

sergiou87 requested a lite review from Copilot August 4, 2026 11:25

Copilot started reviewing on behalf of sergiou87 August 4, 2026 11:26 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Avoid shadowing gitExecuter

c6b6e78

sergiou87 requested a lite review from Copilot August 5, 2026 08:12

Copilot started reviewing on behalf of sergiou87 August 5, 2026 08:12 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Centralize Git config test isolation

0402e1a

Add shared test helper for isolated Git configuration and use it across credential and repository sync tests.

sergiou87 requested a lite review from Copilot August 5, 2026 08:36

Copilot started reviewing on behalf of sergiou87 August 5, 2026 08:36 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

williammartin reviewed Aug 5, 2026

View reviewed changes

williammartin and others added 2 commits August 5, 2026 11:52

williammartin mentioned this pull request Aug 5, 2026

Curate the error when the sync target is checked out in another worktree #14076

Merged

6 tasks

williammartin reviewed Aug 5, 2026

View reviewed changes

sergiou87 added 2 commits August 5, 2026 13:54

Merge pull request #14076 from cli/wm/repo-sync-worktree-error-message

39f9ec5

Curate the error when the sync target is checked out in another worktree

Use *testing.T for Git config isolation

d0e2b5f

sergiou87 requested a lite review from Copilot August 5, 2026 12:54

Copilot started reviewing on behalf of sergiou87 August 5, 2026 12:54 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

sergiou87 marked this pull request as ready for review August 5, 2026 13:07

sergiou87 requested a review from a team as a code owner August 5, 2026 13:07

sergiou87 requested a review from tidy-dev August 5, 2026 13:07

Navigation Menu

Uh oh!

Prevent repo sync from corrupting linked worktrees - #14060

Prevent repo sync from corrupting linked worktrees#14060
sergiou87 wants to merge 13 commits into
trunkfrom
12927-fix-repo-sync-worktree

sergiou87 commented Aug 4, 2026 •

edited

Loading

Uh oh!

sergiou87 Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

sergiou87 Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

williammartin Aug 5, 2026

Uh oh!

williammartin left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sergiou87 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How did you test this change?

Key points

Authorship and follow-up

Uh oh!

sergiou87 Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

sergiou87 Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

williammartin Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

williammartin left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergiou87 commented Aug 4, 2026 •

edited

Loading