Address adversarial review: only the target's own size verdict is terminal

Commit

nodo4d ago

Two independent reviews found the same pair of defects, and both were real.

A deadline was treated as a size verdict. isBatchableTargetPushError covers 408/504 as well as body limits, so a single gateway timeout on an indivisible checkpoint — zero bytes sent, no size evidence whatsoever — returned the permanent sentinel. Once the worker honours it, one target rolling restart could permafail every large bootstrap in flight. Terminal now requires either a parsed body-limit rejection or a relaxed attempt that actually exceeded the limit the target announced; a deadline stays retryable and heals next delivery.

And the relaxed retry never really asked the target. It kept the aborter armed at 95% of the announced limit, with the projection path live, so a pack in the top 5% self-aborted and — relaxedRetried being latched — was reported as the target's verdict. Worse for the case this exists to serve: a front-loaded giant-blob pack makes the early bytes-per-object average overshoot, so a pack comfortably under the limit could abort at a fraction of it and be called permanent. That is strictly worse than the retryable failure it replaced, and it made errors.go's "always carries a verdict from the target" false. The relaxed attempt now cuts only once it has genuinely exceeded the announced limit, where a rejection is certain rather than predicted, and drops the projection entirely. Arming widened to >= so the 95%-to-100% gain is available when the ratchet had equalised the two.

Route facts moved ahead of checkpoint planning: the commit-graph fetch is the likeliest failure for exactly the repos that batch, and setting them after it reported those failures as one-shot bootstraps — the thing the previous commit claimed to fix. unstable.Client.Bootstrap no longer discards the populated result either; the method actually named "bootstrap" was the one throwing it away.

The review's most useful half was mutation-proving that several pieces of this change had no coverage at all: both AnnouncedTargetLimit capture sites (the feature's only production sources — nothing configures it), the route facts, and the syncer error-path carry could each be deleted with a green suite. All are now covered, and every new test here was confirmed to fail with the thing it protects removed.

Entire-Checkpoint: 01M1BWBNT1VDSYSJ6YW0D1EW0D

Checkpoints

Review git-sync PR #118 Size Failures

Claude CodeOpus 5.[1m]
View session
Checkpoint 1