gitproto: classify target-ref moves by extracting CommandStatusErr robustly

06be847→main·

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

asRefRejectedError and annotateLeaseFailure used errors.As against a *packp.CommandStatusErr (pointer) target, but go-git returns CommandStatusErr BY VALUE from ReportStatus.Error() (value receiver, constructed by value in report_status.go). A pointer target never matches a value in the error chain, so on the real receive-pack path the errors.As fell through, no *RefRejectedError was built, moved was never set, and errors.Is(err, ErrTargetRefMoved) was always false. Every live concurrent target-ref rejection ("remote ref has changed") passed through unclassified — the typed-rejection feature was inert in production. The same bug silently disabled the lease-failure hint.

Extract the error through a shared commandStatusErr helper that accepts BOTH the value form (today's go-git) and a *CommandStatusErr, since errors.As is exact about value-vs-pointer and the form is not part of go-git's (alpha) API contract — so a future switch can't silently regress classification to "every rejection unclassified".

The existing unit tests masked the original bug by feeding a pointer &packp.CommandStatusErr{} as input, which the pointer target did match — not how go-git hands the error over. Make the table tests build input via go-git's real (&CommandStatus{}).Error() (value), add TestAsRefRejectedError_RealReportStatusPath drive a real ReportStatus.Error() end to end; fails loud on a deeper type change and TestAsRefRejectedError_ToleratesPointerCommandStatusErr (pins the pointer form).

Bug was in the unreleased typed-rejection feature (#71); never shipped tagged.

Changes

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.

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.)