Reuse validated worktree metadata within operations

Author: skarim

internal/git/gitops.go

@@ -124,7 +124,7 @@ func CurrentOps() Ops {
 // --- defaultOps method implementations ---
 
 func (d *defaultOps) GitDir() (string, error) {
-	return d.path("rev-parse", "--absolute-git-dir")
+	return d.directory("--absolute-git-dir")
 }
 
 func (d *defaultOps) RootDir() (string, error) {

internal/git/gitops_test.go

@@ -1058,6 +1058,39 @@ func TestIntegration_ForWorktreeIndependentExecutors(t *testing.T) {
 	assert.Equal(t, "main", gitExec(t, dir, "branch", "--show-current"))
 }
 
+func TestIntegration_WorktreeDirectoriesReuseOnlyCurrentValidation(t *testing.T) {
+	root, dir := setupWorktreeRepo(t)
+	linked, path := addTestWorktree(t, root, dir, "feature")
+	tracePath := filepath.Join(t.TempDir(), "trace.jsonl")
+	t.Setenv("GIT_TRACE2_EVENT", tracePath)
+	common, err := linked.CommonDir()
+	require.NoError(t, err)
+	assert.Equal(t, canonicalGitTestPath(t, filepath.Join(dir, ".git")), common)
+	gitDir, err := linked.GitDir()
+	require.NoError(t, err)
+	assert.NotEqual(t, common, gitDir)
+	requireWorktree(t, linked, path)
+	trace, err := os.ReadFile(tracePath)
+	require.NoError(t, err)
+	probes := 0
+	for _, line := range strings.Split(strings.TrimSpace(string(trace)), "\n") {
+		var event struct {
+			Event string
+			Argv  []string
+		}
+		require.NoError(t, json.Unmarshal([]byte(line), &event))
+		if event.Event == "start" && slices.Contains(event.Argv, "rev-parse") {
+			probes++
+		}
+	}
+	assert.Equal(t, 8, probes, "directory queries reuse current validation; selecting a scope still validates both directories")
+	require.NoError(t, os.Rename(path, path+"-moved"))
+	_, err = linked.CommonDir()
+	require.Error(t, err, "the previous validation must not be cached across calls")
+	_, err = linked.GitDir()
+	require.Error(t, err)
+}
+
 func TestIntegration_WorktreeMissingCheckoutDoesNotReportStagedChanges(t *testing.T) {
 	t.Parallel()
 	root, dir := setupWorktreeRepo(t)

internal/git/worktree.go

@@ -32,42 +32,81 @@ func (d *defaultOps) path(args ...string) (string, error) {
 }
 
 func (d *defaultOps) CommonDir() (string, error) {
-	return d.path("rev-parse", "--path-format=absolute", "--git-common-dir")
+	return d.directory("--git-common-dir")
 }
 
-func (d *defaultOps) gitClient() (*cligit.Client, error) {
+type validatedGitClient struct {
+	client    *cligit.Client
+	gitDir    string
+	commonDir string
+}
+
+func (d *defaultOps) inspectGitClient() (validatedGitClient, error) {
 	c := d.client
 	if c == nil {
 		c = client
 	}
+	validated := validatedGitClient{client: c}
 	for _, directory := range []struct {
 		option string
 		info   os.FileInfo
+		path   *string
 	}{
-		{"--absolute-git-dir", d.gitDir},
-		{"--git-common-dir", d.commonDir},
+		{"--absolute-git-dir", d.gitDir, &validated.gitDir},
+		{"--git-common-dir", d.commonDir, &validated.commonDir},
 	} {
 		if directory.info == nil {
 			continue
 		}
 		cmd, err := c.Command(context.Background(), "rev-parse", "--path-format=absolute", directory.option)
 		if err != nil {
-			return nil, err
+			return validatedGitClient{}, err
 		}
 		d.configureCommand(cmd)
 		out, err := cmd.Output()
 		if err != nil {
-			return nil, fmt.Errorf("inspecting worktree %q: %w", c.RepoDir, err)
+			return validatedGitClient{}, fmt.Errorf("inspecting worktree %q: %w", c.RepoDir, err)
 		}
-		info, err := os.Stat(strings.TrimSuffix(string(out), "\n"))
+		path := strings.TrimSuffix(string(out), "\n")
+		info, err := os.Stat(path)
 		if err != nil {
-			return nil, fmt.Errorf("inspecting worktree %q: %w", c.RepoDir, err)
+			return validatedGitClient{}, fmt.Errorf("inspecting worktree %q: %w", c.RepoDir, err)
 		}
 		if !os.SameFile(directory.info, info) {
-			return nil, fmt.Errorf("worktree %q no longer refers to the selected Git directory; rediscover worktrees before continuing", c.RepoDir)
+			return validatedGitClient{}, fmt.Errorf("worktree %q no longer refers to the selected Git directory; rediscover worktrees before continuing", c.RepoDir)
 		}
+		*directory.path = path
 	}
-	return c, nil
+	return validated, nil
+}
+
+func (d *defaultOps) gitClient() (*cligit.Client, error) {
+	validated, err := d.inspectGitClient()
+	return validated.client, err
+}
+
+func (d *defaultOps) directory(option string) (string, error) {
+	validated, err := d.inspectGitClient()
+	if err != nil {
+		return "", err
+	}
+	// Reuse only paths validated by this call, never across commands.
+	if option == "--absolute-git-dir" && validated.gitDir != "" {
+		return validated.gitDir, nil
+	}
+	if option == "--git-common-dir" && validated.commonDir != "" {
+		return validated.commonDir, nil
+	}
+	cmd, err := validated.client.Command(context.Background(), "rev-parse", "--path-format=absolute", option)
+	if err != nil {
+		return "", err
+	}
+	d.configureCommand(cmd)
+	out, err := cmd.Output()
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSuffix(string(out), "\n"), nil
 }
 
 // ForWorktree resolves relative paths against this receiver's execution
@@ -76,13 +115,17 @@ func (d *defaultOps) ForWorktree(path string) (Ops, error) {
 	if path == "" {
 		return nil, errors.New("worktree path must not be empty")
 	}
-	c, err := d.gitClient()
+	validated, err := d.inspectGitClient()
 	if err != nil {
 		return nil, err
 	}
-	common, err := d.CommonDir()
-	if err != nil {
-		return nil, fmt.Errorf("locating the source repository: %w", err)
+	c := validated.client
+	common := validated.commonDir
+	if common == "" {
+		common, err = d.CommonDir()
+		if err != nil {
+			return nil, fmt.Errorf("locating the source repository: %w", err)
+		}
 	}
 	expectedCommon, err := os.Stat(common)
 	if err != nil {

internal/worktree/context.go

@@ -202,36 +202,40 @@ func CheckClean(ops git.Ops, path string) error {
 func (c *Context) Preflight(branches []string) error {
 	checked := make(map[string]bool)
 	for _, branch := range branches {
-		ops, err := c.Ops(branch)
-		if err != nil {
+		if _, err := c.preflightBranch(branch, checked); err != nil {
 			return err
 		}
-		location := c.Location(branch)
-		if !checked[location.Path] {
-			if err := CheckClean(ops, location.Path); err != nil {
-				return err
-			}
-			checked[location.Path] = true
+	}
+	return nil
+}
+
+func (c *Context) preflightBranch(branch string, checked map[string]bool) (git.Ops, error) {
+	ops, err := c.Ops(branch)
+	if err != nil {
+		return nil, err
+	}
+	location := c.Location(branch)
+	if !checked[location.Path] {
+		if err := CheckClean(ops, location.Path); err != nil {
+			return nil, err
 		}
-		if !SamePath(location.Path, c.Origin.Path) {
-			current, err := ops.CurrentBranch()
-			if err != nil {
-				return fmt.Errorf("checking current branch in worktree %s: %w", location.Path, err)
-			}
-			if current != branch {
-				return fmt.Errorf("worktree %s no longer has branch %s checked out", location.Path, branch)
-			}
+		checked[location.Path] = true
+	}
+	if !SamePath(location.Path, c.Origin.Path) {
+		current, err := ops.CurrentBranch()
+		if err != nil {
+			return nil, fmt.Errorf("checking current branch in worktree %s: %w", location.Path, err)
+		}
+		if current != branch {
+			return nil, fmt.Errorf("worktree %s no longer has branch %s checked out", location.Path, branch)
 		}
 	}
-	return nil
+	return ops, nil
 }
 
 // Prepare may switch the initiating worktree, but never a different worktree.
 func (c *Context) Prepare(branch string) (git.Ops, error) {
-	if err := c.Preflight([]string{branch}); err != nil {
-		return nil, err
-	}
-	ops, err := c.Ops(branch)
+	ops, err := c.preflightBranch(branch, make(map[string]bool))
 	if err != nil {
 		return nil, err
 	}

internal/worktree/context_test.go

@@ -248,3 +248,68 @@ func TestStartRejectsCommitsAfterSnapshot(t *testing.T) {
 	assert.Empty(t, ctx.Pending)
 	assert.Empty(t, ctx.Touched)
 }
+
+func TestPrepareReusesOnlyTheCurrentPreflightExecutor(t *testing.T) {
+	common := t.TempDir()
+	current := "main"
+	resolutions := 0
+	scoped := &git.MockOps{
+		CommonDirFn:     func() (string, error) { return common, nil },
+		GitDirFn:        func() (string, error) { return common, nil },
+		CurrentBranchFn: func() (string, error) { return current, nil },
+		CheckoutBranchFn: func(branch string) error {
+			current = branch
+			return nil
+		},
+	}
+	restore := git.SetOps(&git.MockOps{
+		CommonDirFn: func() (string, error) { return common, nil },
+		ForWorktreeFn: func(string) (git.Ops, error) {
+			resolutions++
+			return scoped, nil
+		},
+	})
+	defer restore()
+	ctx := &Context{Origin: Location{Path: common}}
+	for i := 1; i <= 2; i++ {
+		ops, err := ctx.Prepare("feature")
+		require.NoError(t, err)
+		assert.Same(t, scoped, ops)
+		assert.Equal(t, i, resolutions, "resolve once per preparation, not once per context")
+		assert.Equal(t, "feature", current)
+	}
+
+	scoped.HasUncommittedChangesFn = func() (bool, error) { return true, nil }
+	ops, err := ctx.Prepare("other")
+	require.ErrorContains(t, err, "uncommitted changes")
+	assert.Nil(t, ops)
+	assert.Equal(t, 3, resolutions)
+	assert.Equal(t, "feature", current)
+}
+
+func TestPreflightChecksSharedOwnerOncePerCall(t *testing.T) {
+	common := t.TempDir()
+	resolutions, checks := 0, 0
+	scoped := &git.MockOps{
+		CommonDirFn: func() (string, error) { return common, nil },
+		GitDirFn:    func() (string, error) { return common, nil },
+		HasUncommittedChangesFn: func() (bool, error) {
+			checks++
+			return false, nil
+		},
+	}
+	restore := git.SetOps(&git.MockOps{
+		CommonDirFn: func() (string, error) { return common, nil },
+		ForWorktreeFn: func(string) (git.Ops, error) {
+			resolutions++
+			return scoped, nil
+		},
+	})
+	defer restore()
+	ctx := &Context{Origin: Location{Path: common}}
+	for i := 1; i <= 2; i++ {
+		require.NoError(t, ctx.Preflight([]string{"one", "two"}))
+		assert.Equal(t, 2*i, resolutions, "each branch still resolves its current owner")
+		assert.Equal(t, i, checks, "cleanliness is shared only within this preflight")
+	}
+}