# Don't mutate the caller's managed map in BuildPlans

`cf4957c`→[main](/content/gh/entireio/git-sync/commits/main/index.html)·

Soph·1mo ago·2 files·+47 added/-1 removed

BuildPlans passed the caller's managed map straight to addPruneCandidates,
which inserts prune candidates — so a --prune plan mutated a map the caller
still owns. BuildReplicationPlans already copied defensively. Copy inside the
prune branch in both (the only mutation happens there), so neither touches the
caller’s map and the two functions stay symmetric.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

## Changes

- internal/planner

- Mplanner.go+6/-1

- Mplanner_test.go+41

```
133 unmodified lines

134
135
136
137
138
139
140
141
142
60 unmodified lines

203
204
205
203
206
207
208
209
210
211
212

133 unmodified lines

) ([]BranchPlan, error) {
	cfg = normalizeAllRefs(cfg)
	if cfg.Prune {
		// addPruneCandidates mutates the map, so copy first — the caller's
		// managed map must not be modified (matches BuildReplicationPlans).
		managed = copyManagedTargets(managed)
		addPruneCandidates(managed, targetRefs, cfg)
	}

60 unmodified lines

cfg PlanConfig,
) ([]BranchPlan, error) {
	cfg = normalizeAllRefs(cfg)
	managed = copyManagedTargets(managed)
	if cfg.Prune {
		// Copy before the only mutation so the caller's managed map is left
		// untouched (matches BuildPlans).
		managed = copyManagedTargets(managed)
		addPruneCandidates(managed, targetRefs, cfg)
	}
```

Minternal/planner/planner.go+6/-1

```
75 unmodified lines

76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122

75 unmodified lines

}

// BuildPlans must not mutate the caller's managed map: under --prune it adds
// prune candidates, and those additions must land in a copy.
func TestBuildPlansDoesNotMutateManagedMap(t *testing.T) {
	mainRef := plumbing.NewBranchReferenceName("main")
	staleRef := plumbing.NewBranchReferenceName("stale")
	hash := plumbing.NewHash("1111111111111111111111111111111111111111")

managed := map[plumbing.ReferenceName]ManagedTarget{
		mainRef: {Kind: RefKindBranch, Label: "main"},
	}
	desired := map[plumbing.ReferenceName]DesiredRef{
		mainRef: {Kind: RefKindBranch, Label: "main", SourceRef: mainRef, TargetRef: mainRef, SourceHash: hash},
	}
	targetRefs := map[plumbing.ReferenceName]plumbing.Hash{
		mainRef:  hash, // same hash -> skip, no ancestry walk needed
		staleRef: plumbing.NewHash("2222222222222222222222222222222222222222"),
	}

plans, err := BuildPlans(memory.NewStorage(), desired, targetRefs, managed, PlanConfig{Prune: true})
	if err != nil {
		t.Fatalf("BuildPlans: %v", err)
	}
	// Sanity: the stale ref was planned for deletion (so prune actually ran).
	var sawDelete bool
	for _, p := range plans {
		if p.TargetRef == staleRef && p.Action == ActionDelete {
			sawDelete = true
		}
	}
	if !sawDelete {
		t.Fatalf("expected a delete plan for the stale ref; plans = %+v", plans)
	}

if len(managed) != 1 {
		t.Fatalf("BuildPlans mutated caller's managed map: len = %d, want 1", len(managed))
	}
	if _, leaked := managed[staleRef]; leaked {
		t.Fatalf("prune candidate %s leaked into the caller's managed map", staleRef)
	}
}

func TestPlanReplicationRefOverwritesDivergence(t *testing.T) {
	target := plumbing.NewHash("1111111111111111111111111111111111111111")
	source := plumbing.NewHash("2222222222222222222222222222222222222222")

```

Minternal/planner/planner_test.go+41