Drop unused defaultOperationMode name param, lock in planner copy

5b7223c→main·Soph·3mo ago·2 files·+52 added/-2 removed

Two small cleanups flagged during review:

• defaultOperationMode took a 'name' argument that was only used by a redundant branch (plan-vs-other that resolved to the same default). The function's real input is the pinned mode, so drop the name parameter and add a comment explaining why an empty defaultMode falls through to ModeSync: that case is 'plan', which accepts --mode.

• BuildReplicationPlans already defensively copies the caller's managed map before adding prune-eligible orphans, but nothing tested the non-mutation guarantee. Add TestBuildReplicationPlansDoesNotMutate-Managed so a future refactor can't regress the footgun by dropping the copy.

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

Sessions

dd63b36b9c0dView transcript

Changes

2

• cmd/git-sync

  • Mmain.go+5/-2

• internal/planner

• Mplanner_test.go+47

71 unmodified lines

modeValue := operationModeFlag(defaultOperationMode(defaultMode))
if name == "plan" {
    fs.Var(&modeValue, "mode", "operation mode: sync or replicate")
}

func defaultOperationMode(name string, defaultMode gitsync.OperationMode) operationMode {
    // defaultOperationMode returns the starting value for the --mode flag.
    // Subcommands that pin a mode (sync, replicate) pass it in; plan passes ""
    // and gets sync as the default, letting --mode override it.
    ...
}

func TestBuildReplicationPlansDoesNotMutateManaged(t *testing.T) { // BuildReplicationPlans inserts prune-eligible orphan refs into a local // copy of managed. Regression guard: it must not mutate the caller's map. orphan := plumbing.NewBranchReferenceName("stale") main := plumbing.NewBranchReferenceName("main") managed := map[plumbing.ReferenceName]ManagedTarget{ main: {Kind: RefKindBranch, Label: "main"}, } desired := map[plumbing.ReferenceName]DesiredRef{ main: { Kind: RefKindBranch, Label: "main", SourceRef: main, TargetRef: main, SourceHash: plumbing.NewHash("2222222222222222222222222222222222222222"), }, } targetRefs := map[plumbing.ReferenceName]plumbing.Hash{ main: plumbing.NewHash("1111111111111111111111111111111111111111"), orphan: plumbing.NewHash("3333333333333333333333333333333333333333"), }

plans, err := BuildReplicationPlans(desired, targetRefs, managed, PlanConfig{Prune: true}) if err != nil { t.Fatalf("BuildReplicationPlans: %v", err) }

// The returned plans should include the orphan delete... var sawDelete bool for _, p := range plans { if p.TargetRef == orphan && p.Action == ActionDelete { sawDelete = true } } if !sawDelete { t.Fatalf("expected prune delete for orphan, got plans=%+v", plans) }

// ...but the caller's managed map must remain unchanged. if len(managed) != 1 { t.Fatalf("caller's managed map was mutated: %+v", managed) } if _, ok := managed[orphan]; ok { t.Fatalf("orphan leaked into caller's managed map") } }

func TestValidateMappingsRejectsDuplicateTargets(t *testing.T) { _, err := validation.ValidateMappings([]RefMapping{ {Source: "main", Target: "stable"}, }) }