diff --git a/client.go b/client.go index 463bf5e7..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) } @@ -228,12 +234,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/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/cmd/git-sync-bench/main.go b/cmd/git-sync-bench/main.go index 9f8a0edb..594dbfad 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") } } @@ -450,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)) } diff --git a/cmd/git-sync/main_test.go b/cmd/git-sync/main_test.go index 431c53ce..c0cde587 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(), "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 41bc087d..c0da2fbc 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,9 @@ 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 semantics) or --force-blind (real overwrite)") + } req.Policy.Mode = gitsync.OperationMode(modeValue) req.Policy.Protocol = gitsync.ProtocolMode(protocolVal) @@ -109,7 +113,13 @@ 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.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/docs/usage.md b/docs/usage.md index 6f3b63f1..c9bbfe1e 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,40 @@ 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'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 +`--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 +322,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..2ec9961d 100644 --- a/internal/convert/convert.go +++ b/internal/convert/convert.go @@ -60,7 +60,9 @@ 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, 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 { out[i] = gitproto.PushCommand{ @@ -71,6 +73,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/gitproto/push.go b/internal/gitproto/push.go index 6ed137d8..5c9f2489 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,44 @@ func buildUpdateRequest( return req, hasDelete, hasUpdates, nil } +// 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", +} + +// 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 { + var cs *packp.CommandStatusErr + if !errors.As(err, &cs) { + return err + } + if !IsLeaseFailure(cs.Status) { + 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 +187,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..2205855f 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); !errors.Is(got, err) { + t.Fatalf("unrelated error should pass through unchanged, got %v", got) + } +} 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..fc22fffd 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 @@ -425,6 +426,37 @@ 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. +// +// 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 + } + 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 { @@ -455,11 +487,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 +515,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 +597,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 +726,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 +802,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 @@ -792,6 +832,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 @@ -886,6 +929,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 @@ -927,8 +973,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 +997,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 +1100,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 +1122,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/internal/syncer/syncer_test.go b/internal/syncer/syncer_test.go index 9063d048..7a63c37d 100644 --- a/internal/syncer/syncer_test.go +++ b/internal/syncer/syncer_test.go @@ -35,6 +35,50 @@ 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{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 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 { + 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 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{} 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..2ad0b651 100644 --- a/types.go +++ b/types.go @@ -2,6 +2,8 @@ package gitsync import ( "context" + "errors" + "entire.io/entire/git-sync/internalbridge" ) @@ -93,13 +95,33 @@ 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"` +} + +// 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. diff --git a/unstable/client.go b/unstable/client.go index 94a600f7..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 @@ -264,7 +273,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,