fix(retry): bound every server-controlled retry delay

[mzner]

Problem

A retried response could dictate how long the CLI paused.

internal/httpapi/client.go and internal/webdav/client.go each parsed Retry-After and slept for exactly that duration with no ceiling. Retry-After: 86400 suspended a single retry for 24 hours.

--timeout does not recover from this. It is the per-request http.Client.Timeout, not a deadline for the whole operation, and no request is in flight while the retry loop waits. A hostile or misconfigured server could hang any command indefinitely, with nothing on screen to distinguish it from a crash. The exponential backoff had the same gap: a large --retries with a long base wait grew without bound.

Confirmed by measurement — the capped path now waits 30s where the previous code would have slept 86400s.

Why it was in two places

The policy existed as three functions (retryableStatus, retryAfter, waitRetry) duplicated verbatim across internal/httpapi and internal/webdav. A ceiling added to one copy would silently have left the other unbounded.

Fix

• New internal/retry package owns the policy: which statuses may be retried, and how long to wait.
• MaxDelay = 30s bounds every wait, from Retry-After or from backoff.
• An excessive hint is clamped rather than discarded, so throttling is still respected — the retry just happens no later than the ceiling.
• Both transports and the TUS upload path now call the shared implementation.

Retry-After parsing

Parsing now follows RFC 9110's two defined forms. The previous implementation appended "s" and called time.ParseDuration, which:

• accepted Go duration syntax the spec does not define (5m3 became 5m3s), and
• rejected values outright that it arguably should not (1h).

Delta-seconds now parses as an integer. An out-of-range value saturates rather than failing, so an absurd hint is capped instead of being read as "no delay requested" — failing toward a bounded retry, not toward none.

Tests

internal/retry covers the ceiling, both header forms, unusable values, whitespace, and backoff growth (96% statement coverage). One test at each transport boundary proves Retry-After is routed through the bounded policy.

The ceiling itself is asserted against the pure functions rather than end to end, because observing a 30-second wait through an HTTP round trip means a test that sleeps for 30 seconds. That tradeoff is noted in both boundary tests.

Verification

go test ./..., go test -race, go vet, gofmt, and golangci-lint all clean. make coverage passes on all 12 gated packages, including the new one.

Docs

ARCHITECTURE.md gains the package entry and a design rule ("Retry policy lives only in internal/retry"), since this is a cross-cutting invariant new adapters must preserve. README documents the 30-second bound as user-visible behavior.

---

Follow-up from review

A second commit (27ce6aa) changes what happens when a server asks for a delay beyond the ceiling.

Clamping respects throttling in the wrong direction. A server that asks for a long wait is usually rate-limiting, and a request sent before the requested delay expires can worsen the throttling or extend a ban — so clamping Retry-After: 86400 to 30s means hammering the exact server that asked to be left alone. Neither a 24-hour sleep nor an early retry is acceptable, so the operation now stops with an actionable error:

server asked to retry after 24h0m0s, which exceeds the 30s limit; run the
command again later

A delay the CLI chose for itself is still simply clamped — the ceiling only refuses server-requested delays.

retry.Delay and retry.Wait gained an error return carrying a typed *retry.DelayTooLongError, so the refusal lives in the shared policy and all three call sites (httpapi, webdav, TUS) inherit it with no changes. After no longer saturates at MaxDelay, which would have hidden an excessive hint from the new check; it saturates at math.MaxInt64 and the ceiling decides. Exit-code behaviour is unchanged — both the old 429 error and the new typed error map to general failure.

A test at the webdav boundary asserts the refusal is prompt: one attempt, errors.As matches the typed error, and elapsed time stays under MaxDelay.