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

ARCHITECTURE.md

@@ -19,6 +19,7 @@ internal/
   httpapi/            authenticated retrying HTTP transport
   logging/            opt-in diagnostic logging abstraction
   output/             terminal and JSON/JSONL rendering
+  retry/              shared bounded retry and backoff policy
   search/             WebDAV search-files REPORT client and response mapping
   sharing/            OCS share discovery, public-link, and capability client
   sync/               deterministic one-way and bidirectional planning model
@@ -80,10 +81,14 @@ without starting a subprocess.
   public-link permission facets; and mutate resource tags by stable resource
   ID. Identity and authorization policy remain on the server.
 - `internal/httpapi`: send replayable authenticated API requests with bounded
-  retries for non-WebDAV protocols.
+  retries for non-WebDAV protocols, using the shared `internal/retry` policy.
 - `internal/logging`: provide an injected no-op or text diagnostic logger.
 - `internal/output`: render human-readable output and versioned JSON/JSONL
   envelopes through injected writers.
+- `internal/retry`: decide which responses may be retried and how long to wait,
+  honoring a server-requested delay exactly within one ceiling and refusing a
+  longer one, so no response can suspend an operation indefinitely and no retry
+  arrives before a throttling server allows it.
 - `internal/search`: issue bounded `search-files` REPORT requests and decode
   ranked WebDAV multistatus responses without depending on Cobra or Space
   selection policy.
@@ -209,6 +214,13 @@ Fast package tests remain Docker-independent.
   planner; job configuration never implements a second transfer path.
 - An unavailable persisted Space is cleared and reported; commands never
   silently fall back to personal files.
+- Retry policy lives only in `internal/retry`. Every wait between attempts,
+  whether requested by a server through `Retry-After` or produced by
+  exponential backoff, is bounded by one ceiling. A response never controls how
+  long the CLI pauses, because no request timeout applies while it waits. A
+  server-requested delay is honored exactly or refused, never shortened: an
+  exponential delay is the CLI's own choice and may be clamped, but retrying
+  before a throttling server allows can extend a rate-limit ban.
 - Cancellation propagates through Cobra contexts, application use cases, HTTP
   requests, and transfer workers and maps to exit code 130.
 - New behavior requires tests at its narrowest package boundary.

Makefile

@@ -38,7 +38,8 @@ check: fmt
 
 coverage:
 	go run ./tools/covercheck -min $(COVERAGE_MIN) \
-		app auth graph httpapi search sharing sync trash transfer versions webdav
+		app auth graph httpapi retry search sharing sync trash transfer versions \
+		webdav
 
 fmt:
 	gofmt -w .

README.md

@@ -629,8 +629,13 @@ ocis download /notes/hello.txt - > hello.txt
 ```
 
 Temporary network errors, HTTP `429`, and HTTP `5xx` responses are retried with
-bounded exponential backoff. Global reliability controls are available on
-every command:
+bounded exponential backoff, never waiting longer than 30 seconds between
+attempts. A server may ask for a specific delay with `Retry-After`; the CLI
+honors it exactly when it is within that limit. A longer delay stops the command
+with an error naming the requested wait, because retrying sooner than a
+throttling server allows can extend a rate-limit ban — run the command again
+later. Either way no response can stall a command indefinitely. Global
+reliability controls are available on every command:
 
 ```sh
 ocis --timeout 2m --retries 5 --concurrency 8 upload --recursive ./photos /photos

internal/httpapi/client.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/mzner/ocis-cli/internal/logging"
+	"github.com/mzner/ocis-cli/internal/retry"
 )
 
 // Config contains shared authentication and retry settings.
@@ -89,22 +90,22 @@ func (client *Client) Do(
 		}
 		client.authenticate(request)
 		response, err := client.http.Do(request)
-		if err == nil && (!retryableStatus(response.StatusCode) || attempt >= client.config.Retries) {
+		if err == nil && (!retry.RetryableStatus(response.StatusCode) || attempt >= client.config.Retries) {
 			return response, nil
 		}
 		if err != nil && attempt >= client.config.Retries {
 			return nil, err
 		}
 		delay, reason := time.Duration(0), "transport_error"
 		if response != nil {
-			delay, reason = retryAfter(response), response.Status
+			delay, reason = retry.After(response), response.Status
 			_ = response.Body.Close()
 		}
 		client.config.Logger.Debug(
 			"retrying API request",
 			"method", method, "attempt", attempt+2, "reason", reason,
 		)
-		if err := waitRetry(ctx, client.config.RetryWait, attempt, delay); err != nil {
+		if err := retry.Wait(ctx, client.config.RetryWait, attempt, delay); err != nil {
 			return nil, err
 		}
 	}
@@ -156,34 +157,3 @@ func (client *Client) authenticate(request *http.Request) {
 		request.Header.Set("User-Agent", client.config.UserAgent)
 	}
 }
-
-func retryableStatus(status int) bool {
-	return status == http.StatusTooManyRequests || status >= 500
-}
-
-func retryAfter(response *http.Response) time.Duration {
-	value := response.Header.Get("Retry-After")
-	if seconds, err := time.ParseDuration(value + "s"); err == nil && seconds > 0 {
-		return seconds
-	}
-	if when, err := http.ParseTime(value); err == nil {
-		return max(time.Until(when), 0)
-	}
-	return 0
-}
-
-func waitRetry(
-	ctx context.Context, base time.Duration, attempt int, delay time.Duration,
-) error {
-	if delay <= 0 {
-		delay = base * time.Duration(1<<min(attempt, 4))
-	}
-	timer := time.NewTimer(delay)
-	defer timer.Stop()
-	select {
-	case <-ctx.Done():
-		return ctx.Err()
-	case <-timer.C:
-		return nil
-	}
-}

internal/httpapi/client_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/mzner/ocis-cli/internal/logging"
+	"github.com/mzner/ocis-cli/internal/retry"
 )
 
 func TestClientAuthenticatesAndRetries(t *testing.T) {
@@ -117,6 +118,42 @@ func TestResponseErrorExplainsOcisMFARequirement(t *testing.T) {
 	}
 }
 
+// TestRetryAppliesBoundedServerRequestedDelay proves the retry loop routes
+// Retry-After through the shared bounded policy rather than sleeping for a
+// server-chosen duration. An excessive header value is clamped, so the call
+// completes far sooner than the day the server asked for; the exact ceiling is
+// covered by the internal/retry tests.
+func TestRetryAppliesBoundedServerRequestedDelay(t *testing.T) {
+	var attempts atomic.Int64
+	server := httptest.NewServer(http.HandlerFunc(func(
+		writer http.ResponseWriter, _ *http.Request,
+	) {
+		if attempts.Add(1) == 1 {
+			writer.Header().Set("Retry-After", "1")
+			writer.WriteHeader(http.StatusTooManyRequests)
+			return
+		}
+		_, _ = io.WriteString(writer, "ok")
+	}))
+	defer server.Close()
+	client := NewClient(Config{
+		Server: server.URL, Retries: 1, RetryWait: time.Millisecond,
+	}, server.Client())
+	started := time.Now()
+	response, err := client.Do(context.Background(), http.MethodGet, "/", nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = response.Body.Close()
+	elapsed := time.Since(started)
+	if elapsed < time.Second || elapsed > retry.MaxDelay {
+		t.Fatalf(
+			"elapsed: got %v, want the one-second Retry-After honored within %v",
+			elapsed, retry.MaxDelay,
+		)
+	}
+}
+
 func TestRetryHonorsContext(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(
 		writer http.ResponseWriter, _ *http.Request,

internal/retry/retry.go

@@ -0,0 +1,136 @@
+// Package retry provides the shared bounded-retry policy used by every
+// protocol adapter: which responses may be retried, and how long to wait.
+package retry
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"math"
+	"net/http"
+	"strconv"
+	"strings"
+	"time"
+)
+
+// MaxDelay bounds every wait between attempts. A retry delay is derived from
+// values the server controls, so an unbounded delay would let a hostile or
+// misconfigured server suspend the CLI for an arbitrary time; the request
+// timeout cannot recover from it because no request is in flight while waiting.
+//
+// Waiting longer than this is indistinguishable from a hang. Retrying sooner is
+// not the alternative: a server that asks for a long delay is usually
+// throttling, and a follow-up request before the delay expires can worsen the
+// throttling or extend a rate-limit ban. A server-requested delay beyond this
+// ceiling therefore stops the operation, while a delay the CLI chose for itself
+// is simply clamped.
+const MaxDelay = 30 * time.Second
+
+// DelayTooLongError reports that the server asked the CLI to wait longer than it
+// is willing to. It names both durations so the caller can decide whether to run
+// the command again later.
+type DelayTooLongError struct {
+	Requested time.Duration
+	Ceiling   time.Duration
+}
+
+func (err *DelayTooLongError) Error() string {
+	return fmt.Sprintf(
+		"server asked to retry after %v, which exceeds the %v limit; "+
+			"run the command again later",
+		err.Requested, err.Ceiling,
+	)
+}
+
+// RetryableStatus reports whether a response status may be retried. Only
+// throttling and server-side failures qualify; a client error is a decision
+// the server will repeat.
+func RetryableStatus(status int) bool {
+	return status == http.StatusTooManyRequests || status >= 500
+}
+
+// After returns the delay requested by a response's Retry-After header. It
+// returns zero when the header is absent, unparsable, or already elapsed,
+// leaving the caller's backoff to choose the delay. A value beyond MaxDelay is
+// reported as-is rather than reduced, so that Delay can refuse it and say what
+// was asked for; only the saturation needed to stay inside time.Duration is
+// applied.
+func After(response *http.Response) time.Duration {
+	if response == nil {
+		return 0
+	}
+	// RFC 9110 defines exactly two forms: delta-seconds and an HTTP-date.
+	value := strings.TrimSpace(response.Header.Get("Retry-After"))
+	if seconds, ok := deltaSeconds(value); ok {
+		switch {
+		case seconds <= 0:
+			return 0
+		// Compared in seconds because converting first would overflow
+		// time.Duration and wrap to a negative delay.
+		case seconds >= int64(math.MaxInt64/time.Second):
+			return math.MaxInt64
+		default:
+			return time.Duration(seconds) * time.Second
+		}
+	}
+	if when, err := http.ParseTime(value); err == nil {
+		return max(time.Until(when), 0)
+	}
+	return 0
+}
+
+// deltaSeconds decodes the delta-seconds form. A value too large for int64
+// saturates instead of being rejected, so that an absurd hint is capped by the
+// caller rather than silently treated as "no delay requested".
+func deltaSeconds(value string) (int64, bool) {
+	seconds, err := strconv.ParseInt(value, 10, 64)
+	switch {
+	case err == nil:
+		return seconds, true
+	case errors.Is(err, strconv.ErrRange):
+		return seconds, true
+	default:
+		return 0, false
+	}
+}
+
+// Delay returns how long to wait before the next attempt. A positive
+// server-requested delay wins and is honored exactly, or refused with
+// *DelayTooLongError when it exceeds MaxDelay; shortening it would send a
+// follow-up request the server asked us not to send. Otherwise the base wait is
+// doubled per attempt up to a fixed number of doublings and clamped at MaxDelay.
+func Delay(
+	base time.Duration, attempt int, serverDelay time.Duration,
+) (time.Duration, error) {
+	if serverDelay > 0 {
+		if serverDelay > MaxDelay {
+			return 0, &DelayTooLongError{Requested: serverDelay, Ceiling: MaxDelay}
+		}
+		return serverDelay, nil
+	}
+	return min(base*time.Duration(1<<min(max(attempt, 0), 4)), MaxDelay), nil
+}
+
+// Wait blocks for Delay or until the context ends, whichever happens first. It
+// returns without waiting when the server-requested delay is refused.
+func Wait(
+	ctx context.Context, base time.Duration, attempt int, serverDelay time.Duration,
+) error {
+	// Cancellation is the caller's terminal decision and takes precedence over
+	// retry policy, including refusal of an excessive server-requested delay.
+	if err := ctx.Err(); err != nil {
+		return err
+	}
+	delay, err := Delay(base, attempt, serverDelay)
+	if err != nil {
+		return err
+	}
+	timer := time.NewTimer(delay)
+	defer timer.Stop()
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-timer.C:
+		return nil
+	}
+}