From de2cf6d90ce3cac64ffee3183eddc89ca199babd Mon Sep 17 00:00:00 2001 From: Kc23mj Date: Tue, 22 Sep 2026 00:04:07 +0530 Subject: [PATCH] fix(resume): resolve branch names Unicode-normalization-insensitively (#2551) `entire session resume ` compared and looked up branch names byte-exact, so a name differing from the on-disk ref only by Unicode normalization (NFC vs NFD) was reported "not found" even though git treats the two spellings as the same branch (core.precomposeUnicode) and checks it out fine. This bit cross-platform teams: a branch created on Linux (NFD) and synced could not be resumed on a macOS checkout (NFC), especially once refs were packed (go-git matches packed-refs byte-exact). Add ResolveLocalBranchName, which resolves a supplied name to the canonical on-disk ref by folding to NFC when an exact lookup misses, plus branchNamesEquivalent for the "already on this branch" fast path. Thread the resolved name through switchToBranchForResume so the checkout and the downstream checkpoint lookup target the real branch. Regression tests cover the NFC/NFD packed-ref case. Fixes #2551 --- cmd/entire/cli/git_operations.go | 68 +++++++++++++++++++++++++++ cmd/entire/cli/git_operations_test.go | 64 +++++++++++++++++++++++++ cmd/entire/cli/resume.go | 54 ++++++++++++--------- cmd/entire/cli/trail_cmd.go | 2 +- cmd/entire/cli/trail_resume_cmd.go | 6 +-- go.mod | 2 +- 6 files changed, 168 insertions(+), 28 deletions(-) diff --git a/cmd/entire/cli/git_operations.go b/cmd/entire/cli/git_operations.go index 1069cdbe84..f881f62351 100644 --- a/cmd/entire/cli/git_operations.go +++ b/cmd/entire/cli/git_operations.go @@ -20,6 +20,8 @@ import ( "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/storer" + "golang.org/x/text/unicode/norm" ) // The fetch output is deliberately not spliced in here: remote.Fetch already @@ -289,6 +291,72 @@ func BranchExistsLocally(ctx context.Context, branchName string) (bool, error) { return true, nil } +// branchNamesEquivalent reports whether two branch names denote the same branch +// under Unicode normalization. Git normalizes ref names (core.precomposeUnicode +// on macOS/Windows) and treats a precomposed (NFC) and decomposed (NFD) spelling +// of the same accented name as one branch; a byte-exact comparison does not, so +// entire would otherwise disagree with git about branch identity. Folding both +// sides to NFC restores agreement. The ASCII fast-path keeps the common case free. +func branchNamesEquivalent(a, b string) bool { + if a == b { + return true + } + return norm.NFC.String(a) == norm.NFC.String(b) +} + +// ResolveLocalBranchName resolves branchName to the name of the local branch it +// denotes, returning the canonical on-disk ref name and whether it was found. An +// exact ref match wins; otherwise it matches Unicode-normalization-insensitively +// (NFC vs NFD) against the existing local branches, so a name whose normalization +// differs from the stored ref — e.g. an NFD branch synced from Linux resolved on +// a macOS checkout, especially once refs are packed and go-git's byte-exact +// packed-refs lookup no longer matches — still resolves, matching git's own +// precomposeUnicode branch identity. The returned canonical name is what callers +// must hand to CheckoutBranch, since that is the name the ref is stored under. +func ResolveLocalBranchName(ctx context.Context, branchName string) (string, bool, error) { + if err := ValidateBranchName(ctx, branchName); err != nil { + return "", false, err + } + + repo, err := openRepository(ctx) + if err != nil { + return "", false, fmt.Errorf("failed to open git repository: %w", err) + } + defer repo.Close() + + // Fast path: an exact ref exists (loose or packed, byte-identical name). + if _, err := repo.Reference(plumbing.NewBranchReferenceName(branchName), true); err == nil { + return branchName, true, nil + } else if !errors.Is(err, plumbing.ErrReferenceNotFound) { + return "", false, fmt.Errorf("failed to check branch: %w", err) + } + + // Fallback: normalization-insensitive scan of existing local branches. + want := norm.NFC.String(branchName) + iter, err := repo.Branches() + if err != nil { + return "", false, fmt.Errorf("failed to list branches: %w", err) + } + defer iter.Close() + + canonical := "" + err = iter.ForEach(func(ref *plumbing.Reference) error { + name := ref.Name().Short() + if norm.NFC.String(name) == want { + canonical = name + return storer.ErrStop + } + return nil + }) + if err != nil { + return "", false, fmt.Errorf("failed to scan local branches: %w", err) + } + if canonical != "" { + return canonical, true, nil + } + return "", false, nil +} + // CheckoutBranch switches to the specified local branch. // Uses git CLI instead of go-git to work around go-git v5 bug where Checkout // deletes untracked files (see https://github.com/go-git/go-git/issues/970). diff --git a/cmd/entire/cli/git_operations_test.go b/cmd/entire/cli/git_operations_test.go index 8e2af321f9..4cd5052dc8 100644 --- a/cmd/entire/cli/git_operations_test.go +++ b/cmd/entire/cli/git_operations_test.go @@ -984,3 +984,67 @@ func TestFetchBlobsByHash_ChainBudgetBoundsTheWholeOperation(t *testing.T) { assert.Less(t, elapsed, loopWorstCase, "a ceiling at or above the loop's worst case (%s) cannot bind — that was the original defect", loopWorstCase) } + +// TestResolveLocalBranchName_UnicodeNormalizationInsensitive covers cli #2551: +// a branch whose on-disk ref differs from the supplied name only by Unicode +// normalization (NFC "é" U+00E9 vs NFD "e"+U+0301) must still resolve, since git +// treats the two spellings as the same branch (core.precomposeUnicode) while a +// byte-exact go-git lookup does not — especially once refs are packed, where the +// packed-refs match is byte-exact on every platform. +func TestResolveLocalBranchName_UnicodeNormalizationInsensitive(t *testing.T) { + testutil.IsolateGitConfigEnv(t) + const ( + nfc = "feature/café" // precomposed é + nfd = "feature/café" // decomposed e + combining acute + ) + require.NotEqual(t, nfc, nfd, "the two spellings must differ byte-for-byte for this test to mean anything") + + dir := t.TempDir() + testutil.InitRepo(t, dir) + // Store refs exactly as given, the way non-precomposing platforms (Linux) do, + // so the branch lands on disk in NFD regardless of the host running the test. + gitRun(t, dir, "config", "core.precomposeUnicode", "false") + testutil.WriteFile(t, dir, "f.txt", "init") + testutil.GitAdd(t, dir, "f.txt") + testutil.GitCommit(t, dir, "init") + defaultBranch := gitDefaultBranch(t, dir) + gitRun(t, dir, "checkout", "-b", nfd) + gitRun(t, dir, "checkout", defaultBranch) + // Pack the refs: go-git matches packed-refs byte-exact, so the NFD ref is no + // longer resolvable by its NFC spelling via an exact lookup — the regression. + gitRun(t, dir, "pack-refs", "--all") + t.Chdir(dir) + + ctx := context.Background() + + // Exact spelling still resolves via the fast path. + got, found, err := ResolveLocalBranchName(ctx, nfd) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, nfd, got) + + // The NFC spelling of the same branch resolves to the canonical on-disk (NFD) + // ref name, rather than being reported as a nonexistent branch. + got, found, err = ResolveLocalBranchName(ctx, nfc) + require.NoError(t, err) + require.True(t, found, "NFC spelling of an NFD-stored branch must resolve (cli #2551)") + require.Equal(t, nfd, got, "must return the on-disk ref name so the checkout targets the real branch") + + // A genuinely different name still does not resolve. + _, found, err = ResolveLocalBranchName(ctx, "feature/other") + require.NoError(t, err) + require.False(t, found) +} + +func TestBranchNamesEquivalent_NFCvsNFD(t *testing.T) { + t.Parallel() + const ( + nfc = "feature/café" + nfd = "feature/café" + ) + require.NotEqual(t, nfc, nfd, "the two spellings must differ byte-for-byte") + assert.True(t, branchNamesEquivalent(nfc, nfd), "NFC and NFD spellings denote the same branch") + assert.True(t, branchNamesEquivalent(nfd, nfc), "comparison is symmetric") + assert.True(t, branchNamesEquivalent("main", "main"), "identical ASCII names are equivalent") + assert.False(t, branchNamesEquivalent("feature/a", "feature/b"), "distinct names are not equivalent") +} diff --git a/cmd/entire/cli/resume.go b/cmd/entire/cli/resume.go index ba5834de46..1dcec7ec5a 100644 --- a/cmd/entire/cli/resume.go +++ b/cmd/entire/cli/resume.go @@ -85,50 +85,58 @@ func runResume(ctx context.Context, cmd *cobra.Command, branchName string, force w := cmd.OutOrStdout() errW := cmd.ErrOrStderr() - proceed, err := switchToBranchForResume(ctx, w, errW, branchName, force) + proceed, resolvedBranch, err := switchToBranchForResume(ctx, w, errW, branchName, force) if err != nil || !proceed { return err } - return resumeFromCurrentBranch(ctx, w, errW, branchName, force) + return resumeFromCurrentBranch(ctx, w, errW, resolvedBranch, force) } // switchToBranchForResume ensures the working tree is on branchName, checking it // out (or fetching it from origin) as needed. It returns proceed=false with a nil // error when the user declined to fetch a remote-only branch, so callers should // stop without treating that as a failure. -func switchToBranchForResume(ctx context.Context, w, errW io.Writer, branchName string, force bool) (bool, error) { - // Check if we're already on this branch +// +// It returns the canonical local branch name that callers should use for the +// rest of the resume: a name supplied to `entire resume` (or carried on a synced +// session state) can differ from the on-disk ref only by Unicode normalization +// (NFC vs NFD), which git treats as the same branch but entire's byte-exact +// lookups do not. Resolving to the stored ref name here keeps the checkout and +// the downstream checkpoint lookup pointed at the real branch. +func switchToBranchForResume(ctx context.Context, w, errW io.Writer, branchName string, force bool) (bool, string, error) { + // Check if we're already on this branch (normalization-insensitively, so + // standing on an NFD-named branch and resuming its NFC spelling still counts). currentBranch, err := GetCurrentBranch(ctx) - if err == nil && currentBranch == branchName { - return true, nil + if err == nil && branchNamesEquivalent(currentBranch, branchName) { + return true, currentBranch, nil } - // Check if branch exists locally - exists, err := BranchExistsLocally(ctx, branchName) + // Check if the branch exists locally, resolving to its canonical ref name. + resolvedBranch, exists, err := ResolveLocalBranchName(ctx, branchName) if err != nil { - return false, fmt.Errorf("failed to check branch: %w", err) + return false, "", fmt.Errorf("failed to check branch: %w", err) } if !exists { // Branch doesn't exist locally, check if it exists on remote remoteExists, err := BranchExistsOnRemote(ctx, branchName) if err != nil { - return false, fmt.Errorf("failed to check remote branch: %w", err) + return false, "", fmt.Errorf("failed to check remote branch: %w", err) } if !remoteExists { - return false, fmt.Errorf("branch '%s' not found locally or on origin", branchName) + return false, "", fmt.Errorf("branch '%s' not found locally or on origin", branchName) } // Ask user if they want to fetch from remote (--force skips the prompt) if !force { shouldFetch, err := promptFetchFromRemote(branchName) if err != nil { - return false, err + return false, "", err } if !shouldFetch { - return false, nil + return false, "", nil } } @@ -136,28 +144,28 @@ func switchToBranchForResume(ctx context.Context, w, errW io.Writer, branchName fmt.Fprintf(w, "Fetching branch '%s' from origin...\n", branchName) if err := FetchAndCheckoutRemoteBranch(ctx, branchName); err != nil { fmt.Fprintf(errW, "Error: failed to checkout branch: %v\n", err) - return false, NewSilentError(errors.New("failed to checkout branch")) + return false, "", NewSilentError(errors.New("failed to checkout branch")) } fmt.Fprintf(w, "✓ Switched to branch %s\n", branchName) - return true, nil + return true, branchName, nil } // Branch exists locally, check for uncommitted changes before checkout hasChanges, err := HasUncommittedChanges(ctx) if err != nil { - return false, fmt.Errorf("failed to check for uncommitted changes: %w", err) + return false, "", fmt.Errorf("failed to check for uncommitted changes: %w", err) } if hasChanges { - return false, errors.New("you have uncommitted changes. Please commit or stash them first") + return false, "", errors.New("you have uncommitted changes. Please commit or stash them first") } - // Checkout the branch - if err := CheckoutBranch(ctx, branchName); err != nil { + // Checkout the branch under its canonical (on-disk) name. + if err := CheckoutBranch(ctx, resolvedBranch); err != nil { fmt.Fprintf(errW, "Error: failed to checkout branch: %v\n", err) - return false, NewSilentError(errors.New("failed to checkout branch")) + return false, "", NewSilentError(errors.New("failed to checkout branch")) } - fmt.Fprintf(w, "✓ Switched to branch %s\n", branchName) - return true, nil + fmt.Fprintf(w, "✓ Switched to branch %s\n", resolvedBranch) + return true, resolvedBranch, nil } // resumeSessionOnBranch switches to branchName and resumes the specific session @@ -168,7 +176,7 @@ func resumeSessionOnBranch(ctx context.Context, cmd *cobra.Command, branchName s w := cmd.OutOrStdout() errW := cmd.ErrOrStderr() - proceed, err := switchToBranchForResume(ctx, w, errW, branchName, force) + proceed, _, err := switchToBranchForResume(ctx, w, errW, branchName, force) if err != nil || !proceed { return err } diff --git a/cmd/entire/cli/trail_cmd.go b/cmd/entire/cli/trail_cmd.go index 2ede2a491c..468f513d8b 100644 --- a/cmd/entire/cli/trail_cmd.go +++ b/cmd/entire/cli/trail_cmd.go @@ -1859,7 +1859,7 @@ func checkoutTrailBranch(ctx context.Context, w, errW io.Writer, found *api.Trai return nil } fmt.Fprintf(w, "Checking out %s\n", describeTrailRef(found)) - proceed, err := switchToBranchForResume(ctx, w, errW, branch, opts.Force) + proceed, _, err := switchToBranchForResume(ctx, w, errW, branch, opts.Force) if err != nil { return err } diff --git a/cmd/entire/cli/trail_resume_cmd.go b/cmd/entire/cli/trail_resume_cmd.go index 74f32a0c2f..cf2f63acdb 100644 --- a/cmd/entire/cli/trail_resume_cmd.go +++ b/cmd/entire/cli/trail_resume_cmd.go @@ -333,11 +333,11 @@ func resumeTrailLatest(ctx context.Context, cmd *cobra.Command, branch string, f if !ensureTrailResumeBranchAvailable(ctx, w, branch) { return nil } - proceed, err := switchToBranchForResume(ctx, w, errW, branch, trailResumeSkipBranchPrompts(force)) + proceed, resolvedBranch, err := switchToBranchForResume(ctx, w, errW, branch, trailResumeSkipBranchPrompts(force)) if err != nil || !proceed { return err } - sessions, err := restoreFromCurrentBranch(ctx, w, errW, branch, force) + sessions, err := restoreFromCurrentBranch(ctx, w, errW, resolvedBranch, force) if err != nil { return err } @@ -351,7 +351,7 @@ func resumeTrailCheckpoint(ctx context.Context, cmd *cobra.Command, branch strin if !ensureTrailResumeBranchAvailable(ctx, w, branch) { return nil } - proceed, err := switchToBranchForResume(ctx, w, errW, branch, trailResumeSkipBranchPrompts(force)) + proceed, _, err := switchToBranchForResume(ctx, w, errW, branch, trailResumeSkipBranchPrompts(force)) if err != nil || !proceed { return err } diff --git a/go.mod b/go.mod index 01b4fdac87..78fbf2d85a 100644 --- a/go.mod +++ b/go.mod @@ -53,6 +53,7 @@ require ( golang.org/x/sync v0.23.0 golang.org/x/sys v0.48.0 golang.org/x/term v0.46.0 + golang.org/x/text v0.42.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -146,6 +147,5 @@ require ( go.yaml.in/yaml/v3 v3.0.5 // indirect go4.org v0.0.0-20260112195520-a5071408f32f // indirect golang.org/x/exp v0.0.0-20250218142911-aa4b98e5adaa // indirect - golang.org/x/text v0.42.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect )