Merge pull request #73 from entireio/fix/ref-rejected-value-target

12e0ac6 → main

nodo · 1mo ago · 3 files · +93 added / -11 removed

Changes

• MCHANGELOG.md +5/-1
• internal/gitproto
  • Mpush.go +27/-5
  • Mpush_test.go +61/-5

8 unmodified lines

### Added

- Typed push-rejection errors on the public API. `Sync` / `Replicate` now report a `*RefRejectedError` (carrying the rejected `Ref` and the raw server `Reason`) for per-ref receive-pack `ng` statuses, reachable with `errors.As`. Rejections that are unambiguous concurrent target-ref moves — entire-server's compare-and-swap rejection (`remote ref has changed`) and git's `--force-with-lease` lease miss (`stale info`) — additionally satisfy `errors.Is(err, ErrTargetRefMoved)`. This lets embedders distinguish a benign racing concurrent push (retryable) from a genuine push failure without substring-matching the free-form error message. Ambiguous markers (`non-fast-forward` / `fetch first`) are deliberately excluded from the move classification so a real "needs `--force`" rejection is not masked. The `ForceWithLease` lease-failure escalation (raised even under `BestEffort`) also satisfies `errors.Is(err, ErrTargetRefMoved)`, though it is not itself a `*RefRejectedError` — prefer `errors.Is` over `errors.As` when you only need the cause. The error message and the underlying `*packp.CommandStatusErr` are preserved unchanged, so existing checks keep working.
- Typed push-rejection errors on the public API. `Sync` / `Replicate` now report a `*RefRejectedError` (carrying the rejected `Ref` and the raw server `Reason`) for per-ref receive-pack `ng` statuses, reachable with `errors.As`. Rejections that are unambiguous concurrent target-ref moves — entire-server's compare-and-swap rejection (`remote ref has changed`) and git's `--force-with-lease` lease miss (`stale info`) — additionally satisfy `errors.Is(err, ErrTargetRefMoved)`. This lets embedders distinguish a benign racing concurrent push (retryable) from a genuine push failure without substring-matching the free-form error message. Ambiguous markers (`non-fast-forward` / `fetch first`) are deliberately excluded from the move classification so a real "needs `--force`" rejection is not masked. The `ForceWithLease` lease-failure escalation (raised even under `BestEffort`) also satisfies `errors.Is(err, ErrTargetRefMoved)`, though it is not itself a `*RefRejectedError` — prefer `errors.Is` over `errors.As` when you only need the cause. The error message and the underlying value-typed `packp.CommandStatusErr` are preserved unchanged (reach it with a value `errors.As` target), so existing checks keep working.

### Fixed

- Concurrent target-ref rejections are now actually classified — `errors.Is(err, ErrTargetRefMoved)` and `errors.As(err, *RefRejectedError)` match on the real push path. go-git returns `packp.CommandStatusErr` **by value** from `ReportStatus.Error()`, but `asRefRejectedError` / `annotateLeaseFailure` used a `*packp.CommandStatusErr` (pointer) `errors.As` target, which never matches a value in the chain — so every live receive-pack `ng` status passed through unclassified and `ErrTargetRefMoved` was never reported. Both now use a value target, and a regression test drives a real `ReportStatus.Error()` end to end so a pointer-vs-value relapse fails CI. (Bug in the unreleased typed-rejection feature above — never shipped in a tagged release.)

## [0.6.0] - 2026-06-03

```plaintext
MCHANGELOG.md +5/-1

128 unmodified lines

// 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) {
cs, ok := commandStatusErr(err)
if !ok {
return err
}
if !IsLeaseFailure(cs.Status) {
return false
}
}
}

// commandStatusErr extracts go-git's per-ref CommandStatusErr from err's chain.
// errors.As is EXACT about value-vs-pointer, and the form go-git uses is not
// part of its API contract: today report.Error() returns the error BY VALUE
// (value receiver Error(), constructed by value in report_status.go), but go-git
// is an alpha dependency and could switch to a *CommandStatusErr. A target that
// only matches one form would silently stop classifying every rejection if the
// other showed up — the exact failure a pointer-only target caused before. So we
// try the value form first (the current reality) and fall back to the pointer
// form. TestAsRefRejectedError_RealReportStatusPath drives go-git's real
// report.Error() so a deeper type change still fails loud in CI.
func commandStatusErr(err error) (packp.CommandStatusErr, bool) {
var byVal packp.CommandStatusErr
if errors.As(err, &byVal) {
return byVal, true
}
var byPtr *packp.CommandStatusErr
if errors.As(err, &byPtr) && byPtr != nil {
return *byPtr, true
}
return packp.CommandStatusErr{}, false
}

// asRefRejectedError wraps a target receive-pack report-status "ng" error in
// a typed *RefRejectedError so callers can branch on errors.As /
// errors.Is(err, ErrTargetRefMoved) instead of substring-matching the free-form
// reason themselves. Inputs that are not a per-ref command status (e.g. an
// unpack-status error) pass through unchanged. The input is preserved via Unwrap,
// so the message and any errors.As(*packp.CommandStatusErr) check are unchanged.
// so the message and the underlying packp.CommandStatusErr stay reachable.
func asRefRejectedError(err error) error {
var cs *packp.CommandStatusErr
if !errors.As(err, &cs) {
cs, ok := commandStatusErr(err)
if !ok {
return err
}
return &RefRejectedError{