Simplify after fifth review: dedupe, fold tag/other walk, reuse helpers

39055c0→main·

Soph·2mo ago·9 files·+121 added/-180 removed

Fifth review pass surfaced a few real wins after the obvious dedup op opportunities had landed in earlier passes:

• planner.RefPrefixes now takes PlanConfig instead of three positional bools. Five call sites all passed the same triple; the new shape removes the footgun (was-it-includeTags-or-allRefs?).
• BuildDesiredRefs collapses the tag and other-kind walks into one iteration of sourceRefs under AllRefs+IncludeTags, instead of two full traversals. Saves a pass on Gerrit-style repos with many refs/changes/* or refs/notes/*.
• syncer.tallyActions extracted from finalizeCounts; bootstrapWithInputs now uses it instead of an open-coded recount loop and three lines of self-justifying comment.
• syncertest.SetRefAtBranch helper replaces 7+ copies of the resolve-head + SetReference + dual-fatal-check pattern across the AllRefs integration tests.
• syncertest.DenyRefsReport helper replaces 4 copies of the ng-status hook synthesis (3 in syncer integration, 2 in cmd CLI tests).
• The CLI test server's sideband-wrap now respects no-progress, mirroring the syncer test server's writeReceivePackReport behavior.
• Comment trims: normalizeAllRefs, addPruneCandidates, tailPhaseLabel, syncSession.rejections, and four CLI test docstrings.

Net: -59 lines, all tests still green.

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

Sessions

0c2a854b8908View transcript

[?
do we have integration tests?Claude Code·Opus 4.7[1m]·1 step](/content/gh/entireio/git-sync/session/d0406407-d612-489d-b375-1372d062af82#timeline-0c2a854b8908/index.html)

Changes

9

• cmd/git-sync

• Mmain_test.go+22/-44

• internal

• planner

• Mplanner.go+14/-24

• Mplanner_test.go+2/-2

• Mtypes.go+7/-8

• strategy/bootstrap

• Mbootstrap.go+1/-2

• syncer

• Mgit_http_backend_test.go+2/-2

• Mintegration_test.go+15/-83

• Msyncer.go+17/-15

• syncertest

• Mrepo.go+41

// CLI smoke test for --all-refs: covers cobra flag parsing through the full
// sync pipeline so wiring breaks fail at the cmd layer, not just integration.
// Smoke test: cobra flag parsing through the full sync pipeline.
func TestRun_Sync_AllRefsSmokeTest(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 2)

head, err := sourceRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
    if err != nil {
        t.Fatalf("resolve source head: %v", err)
    }
    notesRef := plumbing.ReferenceName("refs/notes/commits")
    if err := sourceRepo.Storer.SetReference(plumbing.NewHashReference(notesRef, head.Hash())); err != nil {
        t.Fatalf("set source notes ref: %v", err)
    }
    head := syncertest.SetRefAtBranch(t, sourceRepo, notesRef, testBranch)

targetRepo, err := git.Init(memory.NewStorage())
    if err != nil {
        t.Fatalf("expected refs/notes/commits on target: %v", err)
    }
    if gotNotes.Hash() != head.Hash() {
        t.Fatalf("target notes hash = %s, want %s", gotNotes.Hash(), head.Hash())
    }
    assertHeadsMatch(t, sourceRepo, targetRepo, testBranch)
}

// replicate must keep strict failure semantics — its contract is "target matches source" — so --all-refs must NOT bundle BestEffort the way it does for sync. A target ng on any ref must surface as a non-nil error. // replicate's --all-refs must not bundle BestEffort the way sync's does. func TestRun_Replicate_AllRefsKeepsStrictFailureOnNg(t *testing.T) { sourceRepo, sourceFS := newSourceRepo(t) makeCommits(t, sourceRepo, sourceFS, 1) firstHead, err := sourceRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true) if err != nil { t.Fatalf("resolve first head: %v", err) } notesRef := plumbing.ReferenceName("refs/notes/commits") if err := sourceRepo.Storer.SetReference(plumbing.NewHashReference(notesRef, firstHead.Hash())); err != nil { t.Fatalf("set source notes ref: %v", err) } syncertest.SetRefAtBranch(t, sourceRepo, notesRef, testBranch)

targetRepo, err := git.Init(memory.NewStorage()) if err != nil { t.Fatalf("failed to initialize target repo: %v", err) }

// Test logic here... }