From d02d15aa7a6f19b0d792d2d84086e26e95fef475 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 13:46:54 +0200 Subject: [PATCH 1/7] Split --force into --force-with-lease and --force-blind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous --force was always lease-protected: PlansToPushCommands sent the captured session-start target hash as the push command's expected-old, and receive-pack rejected updates where the target had moved during the run. The name oversold the danger — it never matched git push --force's raw clobber semantics. Replace with two explicit flags matching git push's surface: - --force-with-lease — previous behavior (allow non-FF, send captured target tip as expected-old; server rejects on lease miss). - --force-blind — new path; zero the expected-old for non-delete commands so receive-pack overwrites regardless of current target value. Matches git push --force. The two are mutually exclusive. Legacy --force errors out with a migration hint pointing at both replacements (pre-0.5, no installed script base to preserve). bootstrap and replicate continue to reject force flags entirely. SyncPolicy.Force splits into ForceWithLease + ForceBlind on the public API; syncer.Config grows a ForceAny() helper for the internal "allow non-FF" sense (planner permissiveness). convert.PlansToPushCommands takes a forceBlind bool; incremental/materialized strategies plumb it from cfg, others pass false since bootstrap/replicate reject force. Closes the rename portion of #47. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: a733497c4d7f --- client.go | 13 +++--- cmd/git-sync-bench/main.go | 15 +++++-- cmd/git-sync/main_test.go | 35 +++++++++++++++- cmd/git-sync/syncplan.go | 14 ++++++- docs/usage.md | 40 +++++++++++++++++-- internal/convert/convert.go | 13 +++++- internal/convert/convert_test.go | 10 ++++- internal/planner/planner.go | 4 +- internal/strategy/bootstrap/bootstrap.go | 8 ++-- internal/strategy/incremental/incremental.go | 3 +- .../strategy/materialized/materialized.go | 3 +- internal/strategy/replicate/replicate.go | 4 +- internal/syncer/git_http_backend_test.go | 4 +- internal/syncer/integration_test.go | 24 +++++------ internal/syncer/syncer.go | 33 ++++++++++----- internalbridge/config.go | 16 ++++---- types.go | 21 +++++++--- unstable/client.go | 3 +- unstable/client_test.go | 2 +- 19 files changed, 197 insertions(+), 68 deletions(-) diff --git a/client.go b/client.go index 463bf5e7..ef644416 100644 --- a/client.go +++ b/client.go @@ -228,12 +228,13 @@ func bridgeScope(scope RefScope) internalbridge.RefScope { func bridgePolicy(policy SyncPolicy) internalbridge.SyncPolicy { return internalbridge.SyncPolicy{ - Mode: internalbridge.OperationMode(policy.Mode), - IncludeTags: policy.IncludeTags, - Force: policy.Force, - Prune: policy.Prune, - BestEffort: policy.BestEffort, - Protocol: internalbridge.ProtocolMode(policy.Protocol), + Mode: internalbridge.OperationMode(policy.Mode), + IncludeTags: policy.IncludeTags, + ForceWithLease: policy.ForceWithLease, + ForceBlind: policy.ForceBlind, + Prune: policy.Prune, + BestEffort: policy.BestEffort, + Protocol: internalbridge.ProtocolMode(policy.Protocol), } } diff --git a/cmd/git-sync-bench/main.go b/cmd/git-sync-bench/main.go index 9f8a0edb..34dc5d6a 100644 --- a/cmd/git-sync-bench/main.go +++ b/cmd/git-sync-bench/main.go @@ -106,7 +106,10 @@ func run(ctx context.Context, args []string) error { branches := fs.String("branch", "", "comma-separated branch list; default is all source branches") fs.Var(&mappings, "map", "ref mapping in src:dst form; short names map branches, full refs map exact refs") fs.BoolVar(&cfg.Policy.IncludeTags, "tags", false, "mirror tags") - fs.BoolVar(&cfg.Policy.Force, "force", false, "allow non-fast-forward branch updates and retarget tags") + fs.BoolVar(&cfg.Policy.ForceWithLease, "force-with-lease", false, "allow non-fast-forward branch updates with per-run lease") + fs.BoolVar(&cfg.Policy.ForceBlind, "force-blind", false, "allow non-fast-forward branch updates; overwrite regardless of current target tip") + var legacyForce bool + fs.BoolVar(&legacyForce, "force", false, "removed: pick --force-with-lease or --force-blind") fs.BoolVar(&cfg.Policy.Prune, "prune", false, "delete managed target refs that no longer exist on source") fs.BoolVar(&cfg.Options.CollectStats, "stats", false, "collect transfer statistics") fs.BoolVar(&cfg.Options.MeasureMemory, "measure-memory", true, "sample elapsed time and Go heap usage") @@ -119,6 +122,12 @@ func run(ctx context.Context, args []string) error { if err := fs.Parse(args); err != nil { return fmt.Errorf("parse flags: %w", err) } + if legacyForce { + return usageError("--force has been removed; use --force-with-lease or --force-blind") + } + if cfg.Policy.ForceWithLease && cfg.Policy.ForceBlind { + return usageError("--force-with-lease and --force-blind are mutually exclusive") + } cfg.Policy.Protocol = gitsync.ProtocolMode(benchProtocol) if len(fs.Args()) > 0 { return usageError("unexpected positional arguments") @@ -152,8 +161,8 @@ func run(ctx context.Context, args []string) error { return err } if sc == scenarioBootstrap { - if cfg.Policy.Force || cfg.Policy.Prune { - return usageError("bootstrap benchmarks do not support --force or --prune") + if cfg.Policy.ForceWithLease || cfg.Policy.ForceBlind || cfg.Policy.Prune { + return usageError("bootstrap benchmarks do not support force flags or --prune") } } diff --git a/cmd/git-sync/main_test.go b/cmd/git-sync/main_test.go index 431c53ce..cde390a3 100644 --- a/cmd/git-sync/main_test.go +++ b/cmd/git-sync/main_test.go @@ -557,14 +557,45 @@ func TestRun_Sync_AllRefsWarnsOnNg(t *testing.T) { func TestRun_Replicate_SubcommandRejectsForce(t *testing.T) { err := run(context.Background(), []string{ modeReplicate, + "--force-with-lease", + "http://127.0.0.1:1/source.git", + "http://127.0.0.1:1/target.git", + }) + if err == nil { + t.Fatal("expected replicate --force-with-lease to be rejected") + } + if !strings.Contains(err.Error(), "replicate does not support force flags") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRun_LegacyForceFlagRejected(t *testing.T) { + err := run(context.Background(), []string{ + "sync", "--force", "http://127.0.0.1:1/source.git", "http://127.0.0.1:1/target.git", }) if err == nil { - t.Fatal("expected replicate --force to be rejected") + t.Fatal("expected --force to be rejected") + } + if !strings.Contains(err.Error(), "--force has been removed") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRun_ForceWithLeaseAndBlindAreMutuallyExclusive(t *testing.T) { + err := run(context.Background(), []string{ + "sync", + "--force-with-lease", + "--force-blind", + "http://127.0.0.1:1/source.git", + "http://127.0.0.1:1/target.git", + }) + if err == nil { + t.Fatal("expected --force-with-lease and --force-blind together to be rejected") } - if !strings.Contains(err.Error(), "replicate does not support --force") { + if !strings.Contains(err.Error(), "mutually exclusive") { t.Fatalf("unexpected error: %v", err) } } diff --git a/cmd/git-sync/syncplan.go b/cmd/git-sync/syncplan.go index 41bc087d..166d74b5 100644 --- a/cmd/git-sync/syncplan.go +++ b/cmd/git-sync/syncplan.go @@ -31,6 +31,7 @@ func newSyncLikeCmd(name, short string, dryRun bool, defaultMode gitsync.Operati branches string modeValue = operationModeFlag(defaultOperationMode(defaultMode)) protocolVal = newProtocolFlag() + legacyForce bool req = unstable.SyncRequest{DryRun: dryRun} ) @@ -41,6 +42,12 @@ func newSyncLikeCmd(name, short string, dryRun bool, defaultMode gitsync.Operati SilenceErrors: true, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + if legacyForce { + return errors.New("--force has been removed; use --force-with-lease (previous --force semantics: receive-pack rejects updates where the target moved during the run) or --force-blind (true overwrite, matches `git push --force`)") + } + if req.Policy.ForceWithLease && req.Policy.ForceBlind { + return errors.New("--force-with-lease and --force-blind are mutually exclusive") + } req.Policy.Mode = gitsync.OperationMode(modeValue) req.Policy.Protocol = gitsync.ProtocolMode(protocolVal) @@ -109,7 +116,12 @@ func newSyncLikeCmd(name, short string, dryRun bool, defaultMode gitsync.Operati cmd.Flags().Var(&modeValue, "mode", "operation mode: sync or replicate") } cmd.Flags().BoolVar(&req.Policy.IncludeTags, "tags", false, "mirror tags") - cmd.Flags().BoolVar(&req.Policy.Force, "force", false, "allow non-fast-forward branch updates and retarget tags") + cmd.Flags().BoolVar(&req.Policy.ForceWithLease, "force-with-lease", false, "allow non-fast-forward branch updates and retarget tags; receive-pack rejects updates where the target moved during the run (lease captured at session start)") + cmd.Flags().BoolVar(&req.Policy.ForceBlind, "force-blind", false, "allow non-fast-forward branch updates and retarget tags; overwrite regardless of current target tip (matches `git push --force`)") + cmd.Flags().BoolVar(&legacyForce, "force", false, "removed: pick --force-with-lease (previous semantics) or --force-blind (real overwrite)") + if err := cmd.Flags().MarkHidden("force"); err != nil { + panic(err) + } cmd.Flags().BoolVar(&req.Policy.Prune, "prune", false, "delete managed target refs that no longer exist on source") // Tag inclusion is now handled at the library level (AllRefs implies // it in BuildDesiredRefs). Replicate keeps strict failure semantics — diff --git a/docs/usage.md b/docs/usage.md index 6f3b63f1..d3a42802 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -205,9 +205,9 @@ best-effort completeness against hostile targets. `sync --all-refs` blocks updates to non-branch refs (notes, pulls, custom namespaces) by default — those refs don't generally form fast-forward -chains, so the same `--force` opt-in that retargets tags is required to -update them. `replicate` doesn't run that check; its overwrite contract -covers other-kind refs without `--force`. +chains, so the same `--force-with-lease` opt-in that retargets tags is +required to update them. `replicate` doesn't run that check; its overwrite +contract covers other-kind refs without a force flag. `SyncPolicy.BestEffort` is independent of scope and can be set without `AllRefs` if a library caller wants per-ref warn semantics on a narrower @@ -222,6 +222,38 @@ git-sync sync \ ``` +## Force Updates and the Per-Run Lease + +Non-fast-forward updates and tag retargets are opt-in. git-sync exposes two +flags that mirror `git push`'s force semantics: + +- **`--force-with-lease`** — allow non-fast-forward updates, but include the + target tip captured at session start as the push command's expected-old + value. If another writer moves the target between session start and the + push, receive-pack rejects the update with a "remote ref does not match + expected old value" error and the sync fails without clobbering the racing + write. The lease window is one sync run; git-sync keeps no state between + runs. +- **`--force-blind`** — allow non-fast-forward updates and zero out the + expected-old, telling receive-pack to overwrite regardless of current + target value. Matches `git push --force` semantics. Use this when the + target was edited out-of-band and you intend to overwrite whatever is + there. + +The two flags are mutually exclusive. Without either, divergent or +non-ancestor refs are reported as blocked and the sync exits non-zero +before any push, so the lease check is a second line of defense against +races for users who opt into non-fast-forward updates. + +`bootstrap` and `replicate` do not accept force flags. Bootstrap seeds an +empty target where every ref is a create; replicate is fast-forward-only by +design. + +The pre-0.5 `--force` flag is removed. Its semantics were lease-protected +(it never sent a zero expected-old), so the closest direct replacement is +`--force-with-lease`. `--force-blind` is new behavior with no pre-0.5 +analog. + ## HEAD / Default Branch git-sync surfaces the source's symref HEAD target — the source's default @@ -288,7 +320,7 @@ That means local testing against a dummy GitHub repo can reuse your regular Git - Source-side discovery and fetch can use protocol v2 when supported. Push stays on the existing v1 `receive-pack` path. `--protocol auto` tries v2 first and falls back to v1. `--protocol v2` requires the source to negotiate v2. - Source fetch advertises current target tip hashes as `have`, so reruns download less when source and target already share history. -- Branches are updated only when the target tip is an ancestor of the source tip, unless `--force` is set. Tags are immutable by default. Retargeting an existing tag requires `--force`. With `--prune`, managed target refs that are absent on source are deleted. +- Branches are updated only when the target tip is an ancestor of the source tip, unless `--force-with-lease` or `--force-blind` is set. Tags are immutable by default; retargeting an existing tag requires one of the force flags. With `--prune`, managed target refs that are absent on source are deleted. - If `sync` finds blocked refs, it exits non-zero before pushing anything. - `--stats` adds per-service request, byte, want, have, and command counters to the output. diff --git a/internal/convert/convert.go b/internal/convert/convert.go index ac660793..ba2181f8 100644 --- a/internal/convert/convert.go +++ b/internal/convert/convert.go @@ -60,7 +60,16 @@ func PlansToPushPlans(plans []planner.BranchPlan) []gitproto.PushPlan { } // PlansToPushCommands converts planner BranchPlans directly to gitproto PushCommands. -func PlansToPushCommands(plans []planner.BranchPlan) []gitproto.PushCommand { +// +// When forceBlind is true, the expected-old hash is zeroed for non-delete +// commands. This tells receive-pack to overwrite regardless of the current +// target tip — matching `git push --force` semantics. Delete commands always +// carry the captured target hash so the server can confirm what it is removing. +// +// When forceBlind is false (the default), commands carry the target hash +// captured at session start; receive-pack rejects updates where the target +// moved during the run, providing per-run `--force-with-lease` protection. +func PlansToPushCommands(plans []planner.BranchPlan, forceBlind bool) []gitproto.PushCommand { out := make([]gitproto.PushCommand, len(plans)) for i, p := range plans { out[i] = gitproto.PushCommand{ @@ -71,6 +80,8 @@ func PlansToPushCommands(plans []planner.BranchPlan) []gitproto.PushCommand { } if out[i].Delete { out[i].New = plumbing.ZeroHash + } else if forceBlind { + out[i].Old = plumbing.ZeroHash } } return out diff --git a/internal/convert/convert_test.go b/internal/convert/convert_test.go index f70b5be1..561992d8 100644 --- a/internal/convert/convert_test.go +++ b/internal/convert/convert_test.go @@ -54,7 +54,7 @@ func TestPlansToPushCommands(t *testing.T) { {TargetRef: plumbing.NewBranchReferenceName("old"), TargetHash: oldHash, Action: planner.ActionDelete}, } - got := PlansToPushCommands(plans) + got := PlansToPushCommands(plans, false) want := []gitproto.PushCommand{ {Name: ref, Old: oldHash, New: newHash}, {Name: plumbing.NewBranchReferenceName("old"), Old: oldHash, Delete: true}, @@ -67,4 +67,12 @@ func TestPlansToPushCommands(t *testing.T) { t.Fatalf("got[%d] = %+v, want %+v", i, got[i], want[i]) } } + + gotBlind := PlansToPushCommands(plans, true) + if !gotBlind[0].Old.IsZero() { + t.Fatalf("force-blind update should zero Old, got %v", gotBlind[0].Old) + } + if gotBlind[1].Old != oldHash { + t.Fatalf("force-blind delete should keep target hash, got %v want %v", gotBlind[1].Old, oldHash) + } } diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 00cc790b..cfffc34a 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -330,7 +330,7 @@ func PlanRef(store storer.EncodedObjectStorer, want DesiredRef, targetHash plumb return plan, nil } plan.Action = ActionBlock - plan.Reason = ShortHash(targetHash) + " differs from " + ShortHash(want.SourceHash) + "; use --force to update " + string(want.Kind) + " ref " + want.TargetRef.String() + plan.Reason = ShortHash(targetHash) + " differs from " + ShortHash(want.SourceHash) + "; use --force-with-lease to update " + string(want.Kind) + " ref " + want.TargetRef.String() return plan, nil } @@ -339,7 +339,7 @@ func PlanRef(store storer.EncodedObjectStorer, want DesiredRef, targetHash plumb if errors.Is(err, ErrAncestryDepthExceeded) { // Can't prove fast-forward within depth limit — block with explanation. plan.Action = ActionBlock - plan.Reason = "ancestry check for " + want.TargetRef.String() + " exceeded depth limit; use --force if this is a valid fast-forward" + plan.Reason = "ancestry check for " + want.TargetRef.String() + " exceeded depth limit; use --force-with-lease if this is a valid fast-forward" return plan, nil } return plan, fmt.Errorf("check fast-forward for %s: %w", want.TargetRef, err) diff --git a/internal/strategy/bootstrap/bootstrap.go b/internal/strategy/bootstrap/bootstrap.go index 8d20d3df..61546ddd 100644 --- a/internal/strategy/bootstrap/bootstrap.go +++ b/internal/strategy/bootstrap/bootstrap.go @@ -176,7 +176,7 @@ func Execute(ctx context.Context, p Params, relayReason string) (Result, error) if p.OnPhase != nil { p.OnPhase("pushing pack") } - cmds := convert.PlansToPushCommands(hoistSourceHeadPlan(plans, p.SourceHeadTarget)) + cmds := convert.PlansToPushCommands(hoistSourceHeadPlan(plans, p.SourceHeadTarget), false) pushErr := p.TargetPusher.PushPack(ctx, cmds, packReader) _ = packReader.Close() if pushErr != nil { @@ -486,7 +486,7 @@ func executeBatched( //nolint:maintidx // complex batch logic is inherently bran "target_limit_bytes", p.TargetMaxPack, "calibrated_bytes_per_object", calibratedBytesPerObject) - cmds := convert.PlansToPushCommands(stagePlans) + cmds := convert.PlansToPushCommands(stagePlans, false) observer := newPackStreamObserver(packReader) if selfImposedBudget > 0 { budget := selfImposedBudget @@ -671,7 +671,7 @@ func executeBatched( //nolint:maintidx // complex batch logic is inherently bran packReader, err := p.SourceService.FetchPack(ctx, p.SourceConn, tailDesired, tailTargetRefs) if err != nil { if errors.Is(err, git.NoErrAlreadyUpToDate) { - cmds := convert.PlansToPushCommands(tailPlans) + cmds := convert.PlansToPushCommands(tailPlans, false) if err := p.TargetPusher.PushCommands(ctx, cmds); err != nil { return result, fmt.Errorf("create tail refs after bootstrap: %w", err) } @@ -681,7 +681,7 @@ func executeBatched( //nolint:maintidx // complex batch logic is inherently bran } else { packReader = gitproto.LimitPackReader(packReader, p.MaxPackBytes) packReader = closeOnce(packReader) - cmds := convert.PlansToPushCommands(tailPlans) + cmds := convert.PlansToPushCommands(tailPlans, false) if err := p.TargetPusher.PushPack(ctx, cmds, packReader); err != nil { _ = packReader.Close() return result, fmt.Errorf("push bootstrap tail refs: %w", err) diff --git a/internal/strategy/incremental/incremental.go b/internal/strategy/incremental/incremental.go index bda2dff4..6862ea26 100644 --- a/internal/strategy/incremental/incremental.go +++ b/internal/strategy/incremental/incremental.go @@ -31,6 +31,7 @@ type Params struct { PushPlans []planner.BranchPlan MaxPackBytes int64 Verbose bool + ForceBlind bool CanRelay func(bool, bool, bool, []planner.BranchPlan) (bool, string) CanTagRelay func([]planner.BranchPlan) (bool, string) } @@ -53,7 +54,7 @@ func Execute(ctx context.Context, p Params, cfg planner.PlanConfig) (Result, err if p.TargetPusher == nil { return Result{}, errors.New("incremental strategy requires TargetPusher") } - cmds := convert.PlansToPushCommands(p.PushPlans) + cmds := convert.PlansToPushCommands(p.PushPlans, p.ForceBlind) if ok, reason := canRelay(cfg.Force, cfg.Prune, false, p.PushPlans); ok { desired := convert.DesiredRefsForPlans(p.DesiredRefs, p.PushPlans) packReader, err := p.SourceService.FetchPack(ctx, p.SourceConn, desired, p.TargetRefs) diff --git a/internal/strategy/materialized/materialized.go b/internal/strategy/materialized/materialized.go index 4eb3b319..6e7bb1dd 100644 --- a/internal/strategy/materialized/materialized.go +++ b/internal/strategy/materialized/materialized.go @@ -31,6 +31,7 @@ type Params struct { TargetRefs map[plumbing.ReferenceName]plumbing.Hash PushPlans []planner.BranchPlan MaxObjects int + ForceBlind bool } // DefaultMaxMaterializedObjects is the default safety limit for the materialized fallback path. @@ -115,7 +116,7 @@ func (e *executor) enforceObjectLimit(hashes []plumbing.Hash) error { } func (e *executor) push(hashes []plumbing.Hash) error { - cmds := convert.PlansToPushCommands(e.params.PushPlans) + cmds := convert.PlansToPushCommands(e.params.PushPlans, e.params.ForceBlind) if e.params.TargetPusher == nil { return errors.New("materialized strategy requires TargetPusher") } diff --git a/internal/strategy/replicate/replicate.go b/internal/strategy/replicate/replicate.go index c39161c8..8886ef16 100644 --- a/internal/strategy/replicate/replicate.go +++ b/internal/strategy/replicate/replicate.go @@ -67,7 +67,7 @@ func Execute(ctx context.Context, p Params) (Result, error) { } packReader = gitproto.LimitPackReader(packReader, p.MaxPackBytes) packReader = closeOnce(packReader) - if err := p.TargetPusher.PushPack(ctx, convert.PlansToPushCommands(updatePlans), packReader); err != nil { + if err := p.TargetPusher.PushPack(ctx, convert.PlansToPushCommands(updatePlans, false), packReader); err != nil { _ = packReader.Close() return Result{}, fmt.Errorf("push target refs: %w", err) } @@ -75,7 +75,7 @@ func Execute(ctx context.Context, p Params) (Result, error) { } if len(deletePlans) > 0 { - if err := p.TargetPusher.PushCommands(ctx, convert.PlansToPushCommands(deletePlans)); err != nil { + if err := p.TargetPusher.PushCommands(ctx, convert.PlansToPushCommands(deletePlans, false)); err != nil { return Result{}, fmt.Errorf("delete target refs: %w", err) } } diff --git a/internal/syncer/git_http_backend_test.go b/internal/syncer/git_http_backend_test.go index 088dddc0..cf15e6aa 100644 --- a/internal/syncer/git_http_backend_test.go +++ b/internal/syncer/git_http_backend_test.go @@ -525,7 +525,7 @@ func TestBootstrap_GitHTTPBackendBatchedBranchResume(t *testing.T) { } desired, _, err := planner.BuildDesiredRefs(gitproto.RefHashMap(sourceRefs), planner.PlanConfig{ Branches: cfg.Branches, Mappings: cfg.Mappings, IncludeTags: cfg.IncludeTags, - Force: cfg.Force, Prune: cfg.Prune, + Force: cfg.ForceAny(), Prune: cfg.Prune, }) if err != nil { t.Fatalf("build desired refs: %v", err) @@ -622,7 +622,7 @@ func TestBootstrap_GitHTTPBackendBatchedPlanningTracksBatchLimit(t *testing.T) { } desired, _, err := planner.BuildDesiredRefs(gitproto.RefHashMap(sourceRefs), planner.PlanConfig{ Branches: cfg.Branches, Mappings: cfg.Mappings, IncludeTags: cfg.IncludeTags, - Force: cfg.Force, Prune: cfg.Prune, + Force: cfg.ForceAny(), Prune: cfg.Prune, }) if err != nil { t.Fatalf("build desired refs: %v", err) diff --git a/internal/syncer/integration_test.go b/internal/syncer/integration_test.go index 4d4ceda9..607304e7 100644 --- a/internal/syncer/integration_test.go +++ b/internal/syncer/integration_test.go @@ -163,7 +163,7 @@ func TestRun_IntegrationMaterializedLimitFailsClearly(t *testing.T) { Source: Endpoint{URL: sourceServer.RepoURL()}, Target: Endpoint{URL: targetServer.RepoURL()}, ProtocolMode: protocolModeAuto, - Force: true, + ForceWithLease: true, MaterializedMaxObjects: 1, }) if err == nil { @@ -1956,9 +1956,9 @@ func TestRun_IntegrationTagsPruneAndForce(t *testing.T) { } if _, err := Run(context.Background(), Config{ - Source: Endpoint{URL: sourceServer.RepoURL()}, - Target: Endpoint{URL: targetServer.RepoURL()}, - Force: true, + Source: Endpoint{URL: sourceServer.RepoURL()}, + Target: Endpoint{URL: targetServer.RepoURL()}, + ForceWithLease: true, }); err != nil { t.Fatalf("expected forced sync to succeed: %v", err) } @@ -2659,15 +2659,15 @@ func TestRun_ReplicateRejectsForceAtSessionConstruction(t *testing.T) { // The Force-with-replicate check happens before any network I/O, so the URLs // never get dialed. Using obviously-invalid URLs also asserts that fact. _, err := Run(context.Background(), Config{ - Source: Endpoint{URL: "http://127.0.0.1:1/source.git"}, - Target: Endpoint{URL: "http://127.0.0.1:1/target.git"}, - Mode: modeReplicate, - Force: true, + Source: Endpoint{URL: "http://127.0.0.1:1/source.git"}, + Target: Endpoint{URL: "http://127.0.0.1:1/target.git"}, + Mode: modeReplicate, + ForceWithLease: true, }) if err == nil { t.Fatal("expected replicate+force to be rejected") } - if !strings.Contains(err.Error(), "replicate does not support --force") || + if !strings.Contains(err.Error(), "replicate does not support force flags") || !strings.Contains(err.Error(), "use sync instead") { t.Fatalf("unexpected error: %v", err) } @@ -3324,12 +3324,12 @@ func TestRun_IntegrationAllRefsSyncOtherKindUpdateRequiresForce(t *testing.T) { if notesPlan.Action != ActionBlock { t.Errorf("expected notes ref Action=%s, got %s", ActionBlock, notesPlan.Action) } - if !strings.Contains(notesPlan.Reason, "use --force to update other ref") { + if !strings.Contains(notesPlan.Reason, "use --force-with-lease to update other ref") { t.Errorf("expected clear --force-required reason for other-kind ref, got %q", notesPlan.Reason) } - // Same scenario with --force succeeds. - cfg.Force = true + // Same scenario with --force-with-lease succeeds. + cfg.ForceWithLease = true if _, err := Run(context.Background(), cfg); err != nil { t.Fatalf("force-update of other-kind ref failed: %v", err) } diff --git a/internal/syncer/syncer.go b/internal/syncer/syncer.go index 06a4fbd4..5da0a600 100644 --- a/internal/syncer/syncer.go +++ b/internal/syncer/syncer.go @@ -79,7 +79,8 @@ type Config struct { MeasureMemory bool Progress bool Mode string - Force bool + ForceWithLease bool + ForceBlind bool Prune bool BestEffort bool MaxPackBytes int64 @@ -455,11 +456,16 @@ func planConfig(cfg Config) planner.PlanConfig { IncludeTags: cfg.IncludeTags, AllRefs: cfg.AllRefs, ExcludeRefPrefixes: cfg.ExcludeRefPrefixes, - Force: cfg.Force, + Force: cfg.ForceAny(), Prune: cfg.Prune, } } +// ForceAny reports whether either force flag is set. Used by call sites that +// only care about "is this run allowed to do non-fast-forward updates?" rather +// than the lease-vs-blind distinction. +func (c Config) ForceAny() bool { return c.ForceWithLease || c.ForceBlind } + // needsLocalSourceClosure reports whether the sync must populate the // in-memory store with the full source closure before running. The fetch // is required when: @@ -478,7 +484,7 @@ func needsLocalSourceClosure( desired map[plumbing.ReferenceName]planner.DesiredRef, targetRefs map[plumbing.ReferenceName]plumbing.Hash, ) bool { - if cfg.Force || cfg.Prune { + if cfg.ForceAny() || cfg.Prune { return true } for targetRef, want := range desired { @@ -560,8 +566,11 @@ func newSession(ctx context.Context, cfg Config, needTarget bool) (*syncSession, if _, err := validation.ValidateMappings(cfg.Mappings, cfg.AllRefs); err != nil { return nil, fmt.Errorf("validate mappings: %w", err) } - if cfg.Mode == modeReplicate && cfg.Force { - return nil, errors.New("replicate does not support --force; use sync instead") + if cfg.ForceWithLease && cfg.ForceBlind { + return nil, errors.New("--force-with-lease and --force-blind are mutually exclusive") + } + if cfg.Mode == modeReplicate && cfg.ForceAny() { + return nil, errors.New("replicate does not support force flags; use sync instead") } if needTarget { if err := validation.ValidateEndpoints(cfg.Source.URL, cfg.Target.URL); err != nil { @@ -686,7 +695,7 @@ func (s *syncSession) runSync(ctx context.Context) (Result, error) { } // Check for bootstrap opportunity (before allocating in-memory repo) - if ok, reason := planner.CanBootstrapRelay(s.cfg.Force, s.cfg.Prune, desiredRefs, targetRefMap); ok { + if ok, reason := planner.CanBootstrapRelay(s.cfg.ForceAny(), s.cfg.Prune, desiredRefs, targetRefMap); ok { if s.cfg.DryRun { plans, err := planner.BuildBootstrapPlans(desiredRefs, targetRefMap) if err != nil { @@ -762,9 +771,9 @@ func (s *syncSession) runSync(ctx context.Context) (Result, error) { } if !s.cfg.DryRun && result.Blocked > 0 { - return result, fmt.Errorf("blocked %d ref update(s); rerun with --force where appropriate", result.Blocked) + return result, fmt.Errorf("blocked %d ref update(s); rerun with --force-with-lease (or --force-blind) where appropriate", result.Blocked) } - result.RelayReason = planner.RelayFallbackReason(s.cfg.Force, s.cfg.Prune, s.cfg.DryRun, pushPlans, s.target.policy) + result.RelayReason = planner.RelayFallbackReason(s.cfg.ForceAny(), s.cfg.Prune, s.cfg.DryRun, pushPlans, s.target.policy) if !s.cfg.DryRun { // Try incremental relay first @@ -927,8 +936,8 @@ func (s *syncSession) replicateCanBootstrap(desiredRefs map[plumbing.ReferenceNa // Bootstrap seeds an empty target with relay behavior. func Bootstrap(ctx context.Context, cfg Config) (Result, error) { - if cfg.Force { - return Result{}, errors.New("bootstrap does not support --force") + if cfg.ForceAny() { + return Result{}, errors.New("bootstrap does not support force flags") } if cfg.Prune { return Result{}, errors.New("bootstrap does not support --prune") @@ -951,7 +960,7 @@ func Bootstrap(ctx context.Context, cfg Config) (Result, error) { return Result{}, errors.New("no source refs matched") } - _, reason := planner.CanBootstrapRelay(cfg.Force, cfg.Prune, desiredRefs, s.target.refMap) + _, reason := planner.CanBootstrapRelay(cfg.ForceAny(), cfg.Prune, desiredRefs, s.target.refMap) result, err := bootstrapWithInputs(ctx, s, desiredRefs, s.target.refMap, reason) result.Measurement = s.measurementDone() return result, err @@ -1054,6 +1063,7 @@ func (s *syncSession) executeIncremental( SourceConn: s.sourceConn, SourceService: s.sourceService, TargetPusher: s.target.pusher, DesiredRefs: desiredRefs, TargetRefs: s.target.refMap, PushPlans: pushPlans, MaxPackBytes: s.cfg.MaxPackBytes, + ForceBlind: s.cfg.ForceBlind, CanRelay: func(force, prune, dryRun bool, plans []planner.BranchPlan) (bool, string) { return planner.CanIncrementalRelay(force, prune, dryRun, plans, s.target.policy) }, @@ -1075,6 +1085,7 @@ func (s *syncSession) executeMaterialized( Store: store, SourceConn: s.sourceConn, SourceService: s.sourceService, TargetPusher: s.target.pusher, DesiredRefs: desiredRefs, TargetRefs: s.target.refMap, PushPlans: pushPlans, MaxObjects: s.cfg.MaterializedMaxObjects, + ForceBlind: s.cfg.ForceBlind, }); err != nil { return fmt.Errorf("materialized execute: %w", err) } diff --git a/internalbridge/config.go b/internalbridge/config.go index dd031bd5..f158a978 100644 --- a/internalbridge/config.go +++ b/internalbridge/config.go @@ -47,12 +47,13 @@ type RefScope struct { } type SyncPolicy struct { - Mode OperationMode - IncludeTags bool - Force bool - Prune bool - BestEffort bool - Protocol ProtocolMode + Mode OperationMode + IncludeTags bool + ForceWithLease bool + ForceBlind bool + Prune bool + BestEffort bool + Protocol ProtocolMode } func ProbeConfig(source Endpoint, sourceAuth EndpointAuth, target *Endpoint, targetAuth EndpointAuth, protocol ProtocolMode, includeTags, allRefs, collectStats bool, excludeRefPrefixes []string, httpClient *http.Client) Config { @@ -84,7 +85,8 @@ func SyncConfig(source Endpoint, sourceAuth EndpointAuth, target Endpoint, targe DryRun: dryRun, ShowStats: collectStats, Mode: operationModeString(policy.Mode), - Force: policy.Force, + ForceWithLease: policy.ForceWithLease, + ForceBlind: policy.ForceBlind, Prune: policy.Prune, BestEffort: policy.BestEffort, ProtocolMode: protocolString(policy.Protocol), diff --git a/types.go b/types.go index e0dc72a4..1fdee32c 100644 --- a/types.go +++ b/types.go @@ -93,13 +93,22 @@ type RefScope struct { // SyncPolicy controls high-level sync behavior. BestEffort downgrades per-ref // receive-pack rejections to warnings; pack-level failures remain fatal. +// +// ForceWithLease and ForceBlind both allow non-fast-forward branch updates and +// tag retargets; they differ in how the push command's expected-old value is +// set. ForceWithLease sends the target tip captured at session start, so +// receive-pack rejects updates where the target moved during the run (the +// "lease"). ForceBlind sends a zero expected-old, telling receive-pack to +// overwrite regardless of the current target value — matching `git push +// --force` semantics. The two are mutually exclusive. type SyncPolicy struct { - Mode OperationMode `json:"mode"` - IncludeTags bool `json:"includeTags"` - Force bool `json:"force"` - Prune bool `json:"prune"` - BestEffort bool `json:"bestEffort,omitempty"` - Protocol ProtocolMode `json:"protocol"` + Mode OperationMode `json:"mode"` + IncludeTags bool `json:"includeTags"` + ForceWithLease bool `json:"forceWithLease,omitempty"` + ForceBlind bool `json:"forceBlind,omitempty"` + Prune bool `json:"prune"` + BestEffort bool `json:"bestEffort,omitempty"` + Protocol ProtocolMode `json:"protocol"` } // ProbeRequest inspects source refs and optional target capabilities. diff --git a/unstable/client.go b/unstable/client.go index 94a600f7..928e0566 100644 --- a/unstable/client.go +++ b/unstable/client.go @@ -264,7 +264,8 @@ func (c *Client) buildSyncConfig(ctx context.Context, req SyncRequest) (syncer.C MeasureMemory: req.Options.MeasureMemory, Progress: req.Options.Progress, Mode: operationModeString(req.Policy.Mode), - Force: req.Policy.Force, + ForceWithLease: req.Policy.ForceWithLease, + ForceBlind: req.Policy.ForceBlind, Prune: req.Policy.Prune, BestEffort: req.Policy.BestEffort, MaxPackBytes: req.Options.MaxPackBytes, diff --git a/unstable/client_test.go b/unstable/client_test.go index fe203785..3e10c931 100644 --- a/unstable/client_test.go +++ b/unstable/client_test.go @@ -21,7 +21,7 @@ func TestBuildSyncConfigCarriesAdvancedOptions(t *testing.T) { Source: gitsync.Endpoint{URL: "https://source.example/repo.git", FollowInfoRefsRedirect: true}, Target: gitsync.Endpoint{URL: "https://target.example/repo.git", FollowInfoRefsRedirect: true}, Scope: gitsync.RefScope{Branches: []string{"main"}}, - Policy: gitsync.SyncPolicy{IncludeTags: true, Force: true, Prune: true}, + Policy: gitsync.SyncPolicy{IncludeTags: true, ForceWithLease: true, Prune: true}, DryRun: true, Options: AdvancedOptions{ CollectStats: true, From 322747c896ee8d4078b60a7214fa4e7f316150c3 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 13:47:06 +0200 Subject: [PATCH 2/7] Hint at --force-blind on receive-pack lease failures When a non-BestEffort push gets a CommandStatusErr whose status looks like a lease miss (stale info / fetch first / non-fast-forward / does not match expected old value), wrap it with "target ref X moved or differs from session start; rerun, or use --force-blind to overwrite." The wrapper preserves the underlying CommandStatusErr via fmt.Errorf %w, so callers and tests that inspect the typed error keep working; the hint is purely additive on the error message. Closes the reason-text portion of #47. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 6432aa0b89a7 --- internal/gitproto/push.go | 22 +++++++++++++++++++- internal/gitproto/push_test.go | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/internal/gitproto/push.go b/internal/gitproto/push.go index 6ed137d8..397f3090 100644 --- a/internal/gitproto/push.go +++ b/internal/gitproto/push.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/go-git/go-git/v6/plumbing" "github.com/go-git/go-git/v6/plumbing/format/packfile" @@ -104,6 +105,25 @@ func buildUpdateRequest( return req, hasDelete, hasUpdates, nil } +// annotateLeaseFailure wraps a CommandStatusErr whose status looks like a +// lease failure (target ref moved during the sync, or the captured expected-old +// no longer matches) with a hint pointing users at the retry/override path. +// Other receive-pack errors pass through unchanged. +func annotateLeaseFailure(err error) error { + var cs *packp.CommandStatusErr + if !errors.As(err, &cs) { + return err + } + status := strings.ToLower(cs.Status) + if !strings.Contains(status, "stale info") && + !strings.Contains(status, "fetch first") && + !strings.Contains(status, "non-fast-forward") && + !strings.Contains(status, "does not match") { + return err + } + return fmt.Errorf("%w (target ref %s moved or differs from session start; rerun, or use --force-blind to overwrite)", err, cs.ReferenceName) +} + // sendReceivePack encodes and POSTs a receive-pack request, then decodes the report. func sendReceivePack( ctx context.Context, @@ -148,7 +168,7 @@ func sendReceivePack( } if onRejection == nil { if err := report.Error(); err != nil { - return fmt.Errorf("report-status: %w", err) + return fmt.Errorf("report-status: %w", annotateLeaseFailure(err)) } return nil } diff --git a/internal/gitproto/push_test.go b/internal/gitproto/push_test.go index cfc125a9..6a2cba98 100644 --- a/internal/gitproto/push_test.go +++ b/internal/gitproto/push_test.go @@ -3,6 +3,7 @@ package gitproto import ( "bytes" "context" + "errors" "io" "net/http" "net/http/httptest" @@ -379,3 +380,40 @@ func (r *gatedReadCloser) Close() error { r.closed = true return nil } + +func TestAnnotateLeaseFailureWrapsStaleInfo(t *testing.T) { + cases := []struct { + name string + status string + wrap bool + }{ + {name: "stale info", status: "stale info", wrap: true}, + {name: "stale info with detail", status: "stale info, exp 1234, got abcd", wrap: true}, + {name: "fetch first", status: "fetch first", wrap: true}, + {name: "non-fast-forward", status: "non-fast-forward", wrap: true}, + {name: "does not match expected old", status: "remote ref does not match expected old value", wrap: true}, + {name: "unrelated reason passes through", status: "deny updating a hidden ref", wrap: false}, + {name: "case-insensitive match", status: "Stale Info", wrap: true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + in := &packp.CommandStatusErr{ReferenceName: "refs/heads/main", Status: c.status} + out := annotateLeaseFailure(in) + wrapped := strings.Contains(out.Error(), "moved or differs from session start") + if wrapped != c.wrap { + t.Fatalf("wrap=%v want=%v (err=%q)", wrapped, c.wrap, out) + } + var inner *packp.CommandStatusErr + if !errors.As(out, &inner) || inner.Status != c.status { + t.Fatalf("annotateLeaseFailure must preserve the underlying CommandStatusErr; got %#v", out) + } + }) + } +} + +func TestAnnotateLeaseFailurePassesNonCommandStatusErrors(t *testing.T) { + err := errors.New("network blew up") + if got := annotateLeaseFailure(err); got != err { + t.Fatalf("unrelated error should pass through unchanged, got %v", got) + } +} From 1ce7b69e386c5edad4543d6e5e489b3b72075757 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 14:54:52 +0200 Subject: [PATCH 3/7] Simplify after review - Use cobra's MarkFlagsMutuallyExclusive for the lease/blind pair instead of a hand-rolled RunE check. - Pull lease-failure markers to a package-level var; loop the contains-checks for symmetry and to keep the list in one place. - Trim PlansToPushCommands doc; lease semantics already live on SyncPolicy. - Shorten the legacy --force migration message. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 604f682ccdc6 --- cmd/git-sync/main_test.go | 2 +- cmd/git-sync/syncplan.go | 6 ++---- internal/convert/convert.go | 11 ++--------- internal/gitproto/push.go | 28 ++++++++++++++++++---------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/cmd/git-sync/main_test.go b/cmd/git-sync/main_test.go index cde390a3..c0cde587 100644 --- a/cmd/git-sync/main_test.go +++ b/cmd/git-sync/main_test.go @@ -595,7 +595,7 @@ func TestRun_ForceWithLeaseAndBlindAreMutuallyExclusive(t *testing.T) { if err == nil { t.Fatal("expected --force-with-lease and --force-blind together to be rejected") } - if !strings.Contains(err.Error(), "mutually exclusive") { + if !strings.Contains(err.Error(), "force-with-lease") || !strings.Contains(err.Error(), "force-blind") { t.Fatalf("unexpected error: %v", err) } } diff --git a/cmd/git-sync/syncplan.go b/cmd/git-sync/syncplan.go index 166d74b5..c0da2fbc 100644 --- a/cmd/git-sync/syncplan.go +++ b/cmd/git-sync/syncplan.go @@ -43,10 +43,7 @@ func newSyncLikeCmd(name, short string, dryRun bool, defaultMode gitsync.Operati SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { if legacyForce { - return errors.New("--force has been removed; use --force-with-lease (previous --force semantics: receive-pack rejects updates where the target moved during the run) or --force-blind (true overwrite, matches `git push --force`)") - } - if req.Policy.ForceWithLease && req.Policy.ForceBlind { - return errors.New("--force-with-lease and --force-blind are mutually exclusive") + return errors.New("--force has been removed; use --force-with-lease (previous semantics) or --force-blind (real overwrite)") } req.Policy.Mode = gitsync.OperationMode(modeValue) req.Policy.Protocol = gitsync.ProtocolMode(protocolVal) @@ -122,6 +119,7 @@ func newSyncLikeCmd(name, short string, dryRun bool, defaultMode gitsync.Operati if err := cmd.Flags().MarkHidden("force"); err != nil { panic(err) } + cmd.MarkFlagsMutuallyExclusive("force-with-lease", "force-blind") cmd.Flags().BoolVar(&req.Policy.Prune, "prune", false, "delete managed target refs that no longer exist on source") // Tag inclusion is now handled at the library level (AllRefs implies // it in BuildDesiredRefs). Replicate keeps strict failure semantics — diff --git a/internal/convert/convert.go b/internal/convert/convert.go index ba2181f8..2ec9961d 100644 --- a/internal/convert/convert.go +++ b/internal/convert/convert.go @@ -60,15 +60,8 @@ func PlansToPushPlans(plans []planner.BranchPlan) []gitproto.PushPlan { } // PlansToPushCommands converts planner BranchPlans directly to gitproto PushCommands. -// -// When forceBlind is true, the expected-old hash is zeroed for non-delete -// commands. This tells receive-pack to overwrite regardless of the current -// target tip — matching `git push --force` semantics. Delete commands always -// carry the captured target hash so the server can confirm what it is removing. -// -// When forceBlind is false (the default), commands carry the target hash -// captured at session start; receive-pack rejects updates where the target -// moved during the run, providing per-run `--force-with-lease` protection. +// When forceBlind is true, non-delete commands send a zero expected-old so +// receive-pack overwrites regardless of current target value; see SyncPolicy. func PlansToPushCommands(plans []planner.BranchPlan, forceBlind bool) []gitproto.PushCommand { out := make([]gitproto.PushCommand, len(plans)) for i, p := range plans { diff --git a/internal/gitproto/push.go b/internal/gitproto/push.go index 397f3090..d6f1d52a 100644 --- a/internal/gitproto/push.go +++ b/internal/gitproto/push.go @@ -105,23 +105,31 @@ func buildUpdateRequest( return req, hasDelete, hasUpdates, nil } -// annotateLeaseFailure wraps a CommandStatusErr whose status looks like a -// lease failure (target ref moved during the sync, or the captured expected-old -// no longer matches) with a hint pointing users at the retry/override path. -// Other receive-pack errors pass through unchanged. +// leaseFailureMarkers are receive-pack ng reason substrings that indicate the +// captured target tip didn't match what was on the server at push time. Match +// is case-insensitive. CommandStatusErr.Status is a free-form string in go-git, +// so substring matching is the only option absent upstream sentinels. +var leaseFailureMarkers = []string{ + "stale info", + "fetch first", + "non-fast-forward", + "does not match", +} + +// annotateLeaseFailure wraps a lease-failure CommandStatusErr with a retry/ +// override hint. Other receive-pack errors pass through unchanged. func annotateLeaseFailure(err error) error { var cs *packp.CommandStatusErr if !errors.As(err, &cs) { return err } status := strings.ToLower(cs.Status) - if !strings.Contains(status, "stale info") && - !strings.Contains(status, "fetch first") && - !strings.Contains(status, "non-fast-forward") && - !strings.Contains(status, "does not match") { - return err + for _, marker := range leaseFailureMarkers { + if strings.Contains(status, marker) { + return fmt.Errorf("%w (target ref %s moved or differs from session start; rerun, or use --force-blind to overwrite)", err, cs.ReferenceName) + } } - return fmt.Errorf("%w (target ref %s moved or differs from session start; rerun, or use --force-blind to overwrite)", err, cs.ReferenceName) + return err } // sendReceivePack encodes and POSTs a receive-pack request, then decodes the report. From 583fcf1a346aebcec942dfc71b30f713eae6b802 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 16:24:16 +0200 Subject: [PATCH 4/7] Address force-clarification review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues from review on top of the force-clarification branch: 1. BestEffort silently defeated --force-with-lease. The OnRejection callback installed under BestEffort stored every per-ref ng status for downgrade to a warning, including lease-mismatch statuses. `sync --all-refs --force-with-lease` could then exit successfully after a concurrent target update, contradicting the lease. Export gitproto.IsLeaseFailure and add a syncer.leaseFailureError pass after finalizeCounts. Lease-class rejections (stale info / fetch first / non-fast-forward / does not match) now escalate to a fatal error even with BestEffort on; non-lease rejections continue to downgrade to warnings. 2. Public and unstable APIs accepted invalid force combinations and only rejected them deep in newSession (after auth resolution). Add SyncPolicy.Validate, invoke it from gitsync.SyncRequest.Validate and gitsync.PlanRequest.Validate, and from unstable.Client.Sync, .Plan, .Replicate. The syncer.go check stays as defense-in-depth for callers reaching syncer.Config directly (tests). 3. docs/usage.md described replicate as "fast-forward-only by design"; in fact replicate's contract is source-authoritative overwrite — divergent branches and tags are retargeted unconditionally, which is why force flags are unnecessary rather than disallowed by a gate. Rewrite the sentence. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: eaa899c96ddf --- client.go | 6 ++++++ client_test.go | 14 ++++++++++++++ docs/usage.md | 6 ++++-- internal/gitproto/push.go | 23 +++++++++++++++++------ internal/syncer/syncer.go | 27 +++++++++++++++++++++++++++ internal/syncer/syncer_test.go | 26 ++++++++++++++++++++++++++ types.go | 13 +++++++++++++ unstable/client.go | 9 +++++++++ 8 files changed, 116 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index ef644416..226ce87f 100644 --- a/client.go +++ b/client.go @@ -153,6 +153,9 @@ func (r SyncRequest) Validate() error { if err := validateOperationMode(r.Policy.Mode); err != nil { return err } + if err := r.Policy.Validate(); err != nil { + return err + } if _, err := validation.NormalizeProtocolMode(string(r.Policy.Protocol)); err != nil { return fmt.Errorf("normalize protocol: %w", err) } @@ -172,6 +175,9 @@ func (r PlanRequest) Validate() error { if err := validateOperationMode(r.Policy.Mode); err != nil { return err } + if err := r.Policy.Validate(); err != nil { + return err + } if _, err := validation.NormalizeProtocolMode(string(r.Policy.Protocol)); err != nil { return fmt.Errorf("normalize protocol: %w", err) } diff --git a/client_test.go b/client_test.go index 8f11f1ce..1a183783 100644 --- a/client_test.go +++ b/client_test.go @@ -59,6 +59,20 @@ func TestValidateRequests(t *testing.T) { }).Validate(); err == nil { t.Fatalf("expected duplicate mapping validation error") } + if err := (SyncRequest{ + Source: Endpoint{URL: "https://source.example/repo.git"}, + Target: Endpoint{URL: "https://target.example/repo.git"}, + Policy: SyncPolicy{ForceWithLease: true, ForceBlind: true}, + }).Validate(); err == nil { + t.Fatalf("expected force-with-lease + force-blind to be rejected at the request edge") + } + if err := (SyncRequest{ + Source: Endpoint{URL: "https://source.example/repo.git"}, + Target: Endpoint{URL: "https://target.example/repo.git"}, + Policy: SyncPolicy{Mode: ModeReplicate, ForceWithLease: true}, + }).Validate(); err == nil { + t.Fatalf("expected replicate + force to be rejected at the request edge") + } } func TestClientReturnsAuthProviderErrors(t *testing.T) { diff --git a/docs/usage.md b/docs/usage.md index d3a42802..c9bbfe1e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -246,8 +246,10 @@ before any push, so the lease check is a second line of defense against races for users who opt into non-fast-forward updates. `bootstrap` and `replicate` do not accept force flags. Bootstrap seeds an -empty target where every ref is a create; replicate is fast-forward-only by -design. +empty target where every ref is a create. Replicate's contract is +source-authoritative overwrite: divergent branches and tags are retargeted +against the source unconditionally, so there is no fast-forward gate for a +force flag to opt out of. The pre-0.5 `--force` flag is removed. Its semantics were lease-protected (it never sent a zero expected-old), so the closest direct replacement is diff --git a/internal/gitproto/push.go b/internal/gitproto/push.go index d6f1d52a..5c9f2489 100644 --- a/internal/gitproto/push.go +++ b/internal/gitproto/push.go @@ -116,6 +116,20 @@ var leaseFailureMarkers = []string{ "does not match", } +// IsLeaseFailure reports whether a receive-pack ng reason indicates the +// captured target tip no longer matched at push time. Callers that downgrade +// per-ref rejections to warnings (BestEffort) must still treat these as fatal +// to preserve --force-with-lease semantics. +func IsLeaseFailure(status string) bool { + lowered := strings.ToLower(status) + for _, marker := range leaseFailureMarkers { + if strings.Contains(lowered, marker) { + return true + } + } + return false +} + // annotateLeaseFailure wraps a lease-failure CommandStatusErr with a retry/ // override hint. Other receive-pack errors pass through unchanged. func annotateLeaseFailure(err error) error { @@ -123,13 +137,10 @@ func annotateLeaseFailure(err error) error { if !errors.As(err, &cs) { return err } - status := strings.ToLower(cs.Status) - for _, marker := range leaseFailureMarkers { - if strings.Contains(status, marker) { - return fmt.Errorf("%w (target ref %s moved or differs from session start; rerun, or use --force-blind to overwrite)", err, cs.ReferenceName) - } + if !IsLeaseFailure(cs.Status) { + return err } - return err + return fmt.Errorf("%w (target ref %s moved or differs from session start; rerun, or use --force-blind to overwrite)", err, cs.ReferenceName) } // sendReceivePack encodes and POSTs a receive-pack request, then decodes the report. diff --git a/internal/syncer/syncer.go b/internal/syncer/syncer.go index 5da0a600..ea0b6a2d 100644 --- a/internal/syncer/syncer.go +++ b/internal/syncer/syncer.go @@ -426,6 +426,27 @@ func tallyActions(plans []BranchPlan) (pushed, deleted int) { return pushed, deleted } +// leaseFailureError surfaces receive-pack lease misses as a fatal error even +// when BestEffort would otherwise downgrade them. Without this, a sync with +// both --force-with-lease and --all-refs (which implies BestEffort) would +// silently treat a concurrent target update as a warning, defeating the lease. +func (s *syncSession) leaseFailureError() error { + if len(s.rejections) == 0 { + return nil + } + var refs []string + for name, status := range s.rejections { + if gitproto.IsLeaseFailure(status) { + refs = append(refs, name.String()) + } + } + if len(refs) == 0 { + return nil + } + sort.Strings(refs) + return fmt.Errorf("lease failure on %d ref(s) (%s) — target moved during sync; rerun, or use --force-blind to overwrite", len(refs), strings.Join(refs, ", ")) +} + // applyRejections downgrades plans whose ref was rejected by the target to // ActionWarn and returns the count. func (s *syncSession) applyRejections(plans []BranchPlan) int { @@ -801,6 +822,9 @@ func (s *syncSession) runSync(ctx context.Context) (Result, error) { } s.finalizeCounts(pushPlans, &result) + if err := s.leaseFailureError(); err != nil { + return result, err + } result.Stats = stats.snapshot() result.Measurement = measurementDone() return result, nil @@ -895,6 +919,9 @@ func (s *syncSession) runReplicate(ctx context.Context) (Result, error) { } s.finalizeCounts(pushPlans, &result) + if err := s.leaseFailureError(); err != nil { + return result, err + } result.Stats = s.stats.snapshot() result.Measurement = s.measurementDone() return result, nil diff --git a/internal/syncer/syncer_test.go b/internal/syncer/syncer_test.go index 9063d048..0955f325 100644 --- a/internal/syncer/syncer_test.go +++ b/internal/syncer/syncer_test.go @@ -35,6 +35,32 @@ func TestApplyRejectionsDowngradesAndCarriesReason(t *testing.T) { } } +func TestLeaseFailureErrorEscalatesPastBestEffort(t *testing.T) { + main := plumbing.NewBranchReferenceName("main") + pull := plumbing.ReferenceName("refs/pull/1/head") + + // Non-lease rejection alone: BestEffort handles it as a warning, no fatal. + s := &syncSession{rejections: map[plumbing.ReferenceName]string{ + pull: "deny updating a hidden ref", + }} + if err := s.leaseFailureError(); err != nil { + t.Fatalf("expected nil for non-lease rejection, got %v", err) + } + + // Lease rejection: must escalate, naming the affected ref and the override flag. + s.rejections[main] = "stale info, exp 1234, got abcd" + err := s.leaseFailureError() + if err == nil { + t.Fatal("expected lease failure to escalate past BestEffort") + } + if !strings.Contains(err.Error(), main.String()) { + t.Errorf("expected affected ref in error, got %q", err) + } + if !strings.Contains(err.Error(), "--force-blind") { + t.Errorf("expected migration hint in error, got %q", err) + } +} + func TestApplyRejectionsEmptyMapIsNoOp(t *testing.T) { plans := []BranchPlan{{TargetRef: plumbing.NewBranchReferenceName("main"), Action: ActionUpdate}} s := &syncSession{} diff --git a/types.go b/types.go index 1fdee32c..2ad0b651 100644 --- a/types.go +++ b/types.go @@ -2,6 +2,8 @@ package gitsync import ( "context" + "errors" + "entire.io/entire/git-sync/internalbridge" ) @@ -111,6 +113,17 @@ type SyncPolicy struct { Protocol ProtocolMode `json:"protocol"` } +// Validate enforces SyncPolicy invariants at the request edge. +func (p SyncPolicy) Validate() error { + if p.ForceWithLease && p.ForceBlind { + return errors.New("ForceWithLease and ForceBlind are mutually exclusive") + } + if p.Mode == ModeReplicate && (p.ForceWithLease || p.ForceBlind) { + return errors.New("replicate does not support force flags; use sync instead") + } + return nil +} + // ProbeRequest inspects source refs and optional target capabilities. type ProbeRequest struct { Source Endpoint `json:"source"` diff --git a/unstable/client.go b/unstable/client.go index 928e0566..f635e6b7 100644 --- a/unstable/client.go +++ b/unstable/client.go @@ -136,6 +136,9 @@ func (c *Client) Plan(ctx context.Context, req SyncRequest) (Result, error) { if err := req.Options.Validate(); err != nil { return Result{}, fmt.Errorf("plan: %w", err) } + if err := req.Policy.Validate(); err != nil { + return Result{}, fmt.Errorf("plan: %w", err) + } planReq := req planReq.DryRun = true cfg, err := c.buildSyncConfig(ctx, planReq) @@ -153,6 +156,9 @@ func (c *Client) Sync(ctx context.Context, req SyncRequest) (Result, error) { if err := req.Options.Validate(); err != nil { return Result{}, fmt.Errorf("sync: %w", err) } + if err := req.Policy.Validate(); err != nil { + return Result{}, fmt.Errorf("sync: %w", err) + } cfg, err := c.buildSyncConfig(ctx, req) if err != nil { return Result{}, err @@ -169,6 +175,9 @@ func (c *Client) Replicate(ctx context.Context, req SyncRequest) (Result, error) return Result{}, fmt.Errorf("replicate: %w", err) } req.Policy.Mode = gitsync.ModeReplicate + if err := req.Policy.Validate(); err != nil { + return Result{}, fmt.Errorf("replicate: %w", err) + } cfg, err := c.buildSyncConfig(ctx, req) if err != nil { return Result{}, err From 5349be76df50809e253dac51da9d08790b546d32 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 16:29:14 +0200 Subject: [PATCH 5/7] Scope lease-failure escalation to --force-with-lease The lease-failure marker set in gitproto is intentionally broad to catch phrasing across servers (stale info / fetch first / non-fast-forward / does not match). That's right under --force-with-lease, where the user has explicitly promised "rerun if target moved." But the same server messages can mean ordinary policy rejection on a --force-blind or non-force best-effort run, which BestEffort is allowed to downgrade to warnings. Gate leaseFailureError on cfg.ForceWithLease so the escalation only fires when the lease contract is actually in effect. Other rejection paths follow the BestEffort warn-and-continue contract as documented. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 153a4deb52ba --- internal/syncer/syncer.go | 10 ++++++++++ internal/syncer/syncer_test.go | 22 ++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/syncer/syncer.go b/internal/syncer/syncer.go index ea0b6a2d..fc22fffd 100644 --- a/internal/syncer/syncer.go +++ b/internal/syncer/syncer.go @@ -430,7 +430,17 @@ func tallyActions(plans []BranchPlan) (pushed, deleted int) { // when BestEffort would otherwise downgrade them. Without this, a sync with // both --force-with-lease and --all-refs (which implies BestEffort) would // silently treat a concurrent target update as a warning, defeating the lease. +// +// Scoped to --force-with-lease only. The lease-failure marker set in gitproto +// is intentionally broad to cover phrasing across servers (stale info / fetch +// first / non-fast-forward / does not match), but under --force-blind or +// non-force runs those same messages can mean ordinary policy rejection rather +// than a stale lease, which BestEffort is allowed to downgrade. The contract +// is only made when the user explicitly opts in. func (s *syncSession) leaseFailureError() error { + if !s.cfg.ForceWithLease { + return nil + } if len(s.rejections) == 0 { return nil } diff --git a/internal/syncer/syncer_test.go b/internal/syncer/syncer_test.go index 0955f325..7a63c37d 100644 --- a/internal/syncer/syncer_test.go +++ b/internal/syncer/syncer_test.go @@ -38,16 +38,18 @@ func TestApplyRejectionsDowngradesAndCarriesReason(t *testing.T) { func TestLeaseFailureErrorEscalatesPastBestEffort(t *testing.T) { main := plumbing.NewBranchReferenceName("main") pull := plumbing.ReferenceName("refs/pull/1/head") + leased := Config{ForceWithLease: true} // Non-lease rejection alone: BestEffort handles it as a warning, no fatal. - s := &syncSession{rejections: map[plumbing.ReferenceName]string{ + s := &syncSession{cfg: leased, rejections: map[plumbing.ReferenceName]string{ pull: "deny updating a hidden ref", }} if err := s.leaseFailureError(); err != nil { t.Fatalf("expected nil for non-lease rejection, got %v", err) } - // Lease rejection: must escalate, naming the affected ref and the override flag. + // Lease rejection under --force-with-lease: must escalate, naming the + // affected ref and the override flag. s.rejections[main] = "stale info, exp 1234, got abcd" err := s.leaseFailureError() if err == nil { @@ -61,6 +63,22 @@ func TestLeaseFailureErrorEscalatesPastBestEffort(t *testing.T) { } } +func TestLeaseFailureErrorOnlyAppliesUnderForceWithLease(t *testing.T) { + main := plumbing.NewBranchReferenceName("main") + // The broad markers (non-fast-forward / fetch first) can mean ordinary + // server policy rejection rather than a lease miss. Those rejections must + // stay warnable under BestEffort when --force-with-lease isn't set. + rejections := map[plumbing.ReferenceName]string{ + main: "non-fast-forward", + } + for _, cfg := range []Config{{}, {ForceBlind: true}} { + s := &syncSession{cfg: cfg, rejections: rejections} + if err := s.leaseFailureError(); err != nil { + t.Errorf("cfg=%+v: expected nil (no lease contract), got %v", cfg, err) + } + } +} + func TestApplyRejectionsEmptyMapIsNoOp(t *testing.T) { plans := []BranchPlan{{TargetRef: plumbing.NewBranchReferenceName("main"), Action: ActionUpdate}} s := &syncSession{} From bf2f40242548a3b32d993765fd0d1a867106cff1 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 16:39:21 +0200 Subject: [PATCH 6/7] Use errors.Is in annotateLeaseFailure passthrough test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit errorlint flagged the `!= err` comparison on a wrapped error. The helper currently returns the input untouched on the non-lease branch (so `==` works), but the contract is "pass through unchanged" — switch to errors.Is so the test still passes if the helper ever wraps with a non-lease annotation. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: b1e36ac6c6d5 --- internal/gitproto/push_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gitproto/push_test.go b/internal/gitproto/push_test.go index 6a2cba98..2205855f 100644 --- a/internal/gitproto/push_test.go +++ b/internal/gitproto/push_test.go @@ -413,7 +413,7 @@ func TestAnnotateLeaseFailureWrapsStaleInfo(t *testing.T) { func TestAnnotateLeaseFailurePassesNonCommandStatusErrors(t *testing.T) { err := errors.New("network blew up") - if got := annotateLeaseFailure(err); got != err { + if got := annotateLeaseFailure(err); !errors.Is(got, err) { t.Fatalf("unrelated error should pass through unchanged, got %v", got) } } From aec7011bb7da1773c15f10608fd364ddb2dad9fa Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 13 May 2026 16:53:11 +0200 Subject: [PATCH 7/7] Update bench usage text to match new force flags The parser already rejects --force with a migration hint, but the usageError() string still listed it as a valid flag. Replace with --force-with-lease and --force-blind so the help text matches what the parser accepts. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 98799f8a9225 --- cmd/git-sync-bench/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/git-sync-bench/main.go b/cmd/git-sync-bench/main.go index 34dc5d6a..594dbfad 100644 --- a/cmd/git-sync-bench/main.go +++ b/cmd/git-sync-bench/main.go @@ -459,7 +459,7 @@ func splitCSV(value string) []string { } func usageError(message string) error { - usage := fmt.Sprintf("usage:\n %s --source-url [flags]\n\nflags:\n --scenario bootstrap|sync\n --repeat 3\n --work-dir /tmp/git-sync-bench\n --keep-targets\n --json\n --branch main,release\n --map main:stable\n --tags\n --force\n --prune\n --stats\n --measure-memory\n --max-pack-bytes 104857600\n --target-max-pack-bytes 104857600\n --protocol auto|v1|v2\n -v\n", os.Args[0]) + usage := fmt.Sprintf("usage:\n %s --source-url [flags]\n\nflags:\n --scenario bootstrap|sync\n --repeat 3\n --work-dir /tmp/git-sync-bench\n --keep-targets\n --json\n --branch main,release\n --map main:stable\n --tags\n --force-with-lease\n --force-blind\n --prune\n --stats\n --measure-memory\n --max-pack-bytes 104857600\n --target-max-pack-bytes 104857600\n --protocol auto|v1|v2\n -v\n", os.Args[0]) if message == "" { return errors.New(strings.TrimSpace(usage)) }