client: harden retry transport

- Cap drainAndClose to 4KB (prevents blocking on large error bodies)
- Clamp Retry-After to MaxDelay (prevents 24h+ blocks)
- Rename isRetryableStatus → isRetryableServerError, remove dead 429 case
- Fix WithRetry zero-value bug: merge partial config instead of replacing

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: somanshreddy

internal/client/client.go

@@ -35,9 +35,20 @@ func WithHTTPClient(hc *http.Client) Option {
 	return func(c *Client) { c.httpClient = hc }
 }
 
-// WithRetry overrides the default retry configuration.
+// WithRetry merges the given config into the default retry configuration.
+// MaxRetries is always applied (0 is valid — means no retries).
+// BaseDelay and MaxDelay are only applied if positive, so
+// WithRetry(RetryConfig{MaxRetries: 1}) keeps the default delays.
 func WithRetry(config RetryConfig) Option {
-	return func(c *Client) { c.retry = config }
+	return func(c *Client) {
+		c.retry.MaxRetries = config.MaxRetries
+		if config.BaseDelay > 0 {
+			c.retry.BaseDelay = config.BaseDelay
+		}
+		if config.MaxDelay > 0 {
+			c.retry.MaxDelay = config.MaxDelay
+		}
+	}
 }
 
 // WithNoRetry disables retries entirely.

internal/client/retry.go

@@ -64,6 +64,11 @@ func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 		delay := backoffDelay(attempt, t.config)
 		if resp != nil {
 			if retryAfter := parseRetryAfter(resp.Header.Get("Retry-After")); retryAfter > delay {
+				// Clamp Retry-After to MaxDelay — a server sending Retry-After: 86400
+				// shouldn't block the CLI for 24 hours.
+				if t.config.MaxDelay > 0 && retryAfter > t.config.MaxDelay {
+					retryAfter = t.config.MaxDelay
+				}
 				delay = retryAfter
 			}
 			drainAndClose(resp.Body)
@@ -97,13 +102,16 @@ func shouldRetry(req *http.Request, resp *http.Response, err error) bool {
 	if resp.StatusCode == http.StatusTooManyRequests {
 		return true
 	}
-	return isRetryableStatus(resp.StatusCode) && isIdempotent(req.Method)
+	// 5xx server errors — only retry for idempotent methods.
+	return isRetryableServerError(resp.StatusCode) && isIdempotent(req.Method)
 }
 
-func isRetryableStatus(code int) bool {
+// isRetryableServerError returns true for server-side errors worth retrying.
+// 429 (rate limited) is handled separately in shouldRetry — it's always retried
+// regardless of method because the server explicitly rejected without processing.
+func isRetryableServerError(code int) bool {
 	switch code {
-	case http.StatusTooManyRequests,
-		http.StatusInternalServerError,
+	case http.StatusInternalServerError,
 		http.StatusBadGateway,
 		http.StatusServiceUnavailable,
 		http.StatusGatewayTimeout:
@@ -209,6 +217,8 @@ func drainAndClose(body io.ReadCloser) {
 	if body == nil {
 		return
 	}
-	_, _ = io.Copy(io.Discard, body)
+	// Cap the drain to 4KB — response bodies on 429/5xx should be tiny.
+	// A misbehaving server sending a large body won't block the retry loop.
+	_, _ = io.Copy(io.Discard, io.LimitReader(body, 4096))
 	_ = body.Close()
 }