Skip to content

Clearer force params#53

Merged
Soph merged 7 commits into
mainfrom
soph/force-clarification
May 14, 2026
Merged

Clearer force params#53
Soph merged 7 commits into
mainfrom
soph/force-clarification

Conversation

@Soph
Copy link
Copy Markdown
Contributor

@Soph Soph commented May 13, 2026 •
edited by cursor Bot
Loading

Clarify force semantics: --force-with-lease and --force-blind

Closes #47.

The old --force was always lease-protected — it sent the captured session-start target tip as the push command's expected-old, so 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.

Replace it with two explicit flags that mirror git's surface:

  • --force-with-lease — previous behavior (allow non-FF, captured tip as expected-old; server rejects on lease miss).
  • --force-blind — new path (zero expected-old, overwrite regardless of current target value; matches git push --force).

The two flags are mutually exclusive. Legacy --force errors out with a migration hint. bootstrap and replicate continue to reject force flags entirely. Pre-0.5, so hard switch — no silent semantics flip.

On the API side, SyncPolicy.Force splits into ForceWithLease + ForceBlind. convert.PlansToPushCommands takes a forceBlind bool; sync's incremental and materialized strategies plumb it through.

Also: when receive-pack returns a lease-failure ng (stale info / fetch first / non-fast-forward / does not match), the error is wrapped with a "target ref X moved or differs from session start; rerun, or use --force-blind to overwrite" hint.

Notes

  • Docs: new "Force Updates and the Per-Run Lease" section in docs/usage.md.
  • CHANGELOG left untouched — to be updated at release time.
  • Tests cover both convert modes, legacy --force rejection, mutual exclusion, replicate-with-force rejection, and the lease-failure annotation.

Note

Medium Risk
Changes force-push behavior and CLI/API semantics, including a new blind overwrite path that can clobber target refs if misused. Touches core sync planning/push code paths, though guarded by mutual-exclusion checks and expanded tests.

Overview
Replaces the single --force knob with two explicit options: --force-with-lease (previous behavior) and --force-blind (true overwrite by sending a zero expected-old), enforcing mutual exclusivity and rejecting the legacy --force flag with migration guidance.

Plumbs the new policy fields through public/unstable API types and internal syncer config (ForceAny() for callers that only need “force enabled”), and updates strategies to generate push commands accordingly (blind mode zeroes Old on non-delete updates).

Improves user feedback: planner/block messages now reference --force-with-lease, blocked-sync guidance mentions both flags, and receive-pack lease-related ng errors are annotated with a rerun/--force-blind hint. Docs and tests are updated to cover the new flags and rejection cases (replicate/bootstrap force rejection, legacy flag rejection, and mutual exclusion).

Reviewed by Cursor Bugbot for commit 1ce7b69. Configure here.

Soph and others added 3 commits May 13, 2026 13:46
@Soph @claude
Split --force into --force-with-lease and --force-blind
d02d15a 
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) <noreply@anthropic.com>
Entire-Checkpoint: a733497c4d7f
@Soph @claude
Hint at --force-blind on receive-pack lease failures
322747c 
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) <noreply@anthropic.com>
Entire-Checkpoint: 6432aa0b89a7
@Soph @claude
Simplify after review
1ce7b69 
- 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) <noreply@anthropic.com>
Entire-Checkpoint: 604f682ccdc6
nodo
nodo previously approved these changes May 13, 2026
View reviewed changes
Soph and others added 3 commits May 13, 2026 16:24
@Soph @claude
Address force-clarification review findings
583fcf1 
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) <noreply@anthropic.com>
Entire-Checkpoint: eaa899c96ddf
@Soph @claude
Scope lease-failure escalation to --force-with-lease
5349be7 
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) <noreply@anthropic.com>
Entire-Checkpoint: 153a4deb52ba
@Soph @claude
Use errors.Is in annotateLeaseFailure passthrough test
bf2f402 
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) <noreply@anthropic.com>
Entire-Checkpoint: b1e36ac6c6d5
@Soph @claude
Update bench usage text to match new force flags
aec7011 
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) <noreply@anthropic.com>
Entire-Checkpoint: 98799f8a9225
@Soph Soph merged commit 64a8d6d into main May 14, 2026
3 checks passed
@Soph Soph deleted the soph/force-clarification branch May 14, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nodo nodo nodo approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Surface force-with-lease semantics; consider --force-blind opt-out

2 participants

@Soph @nodo