Keep basecamp connect running, and starting, through a rate-limited or failing token endpoint

internal/auth/agent.go

@@ -351,14 +351,18 @@ func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.T
 	}
 
 	body, err := io.ReadAll(io.LimitReader(resp.Body, maxAgentTokenBytes+1))
-	if resp.StatusCode == http.StatusTooManyRequests && (err != nil || int64(len(body)) > maxAgentTokenBytes) {
-		// A 429's status and Retry-After are all its hold needs, and a
-		// body that could not be read must not cost the hold.
+	if resp.StatusCode != http.StatusOK && (err != nil || int64(len(body)) > maxAgentTokenBytes) {
+		// A status that arrived is the verdict even when its body could
+		// not be read, or was too large to: a 401 is still a refusal, a
+		// 429 still sets its hold from its status and Retry-After, a 5xx
+		// — a proxy's error page, say — is still retryable.
 		hold, err := m.agentMintRefusal(resp, nil, mint)
 		return nil, hold, err
 	}
 	if err != nil {
-		return nil, nil, wrapOAuthError("minting an agent token", err)
+		// Only a 200 cut short has no verdict — the token never arrived —
+		// and it is the transport failure it is.
+		return nil, nil, transportFailure("minting an agent token", err)
 	}
 	if int64(len(body)) > maxAgentTokenBytes {
 		return nil, nil, output.ErrAPI(resp.StatusCode,
@@ -482,7 +486,13 @@ func (m *Manager) agentMintRefusal(resp *http.Response, body []byte, mint *agent
 		now := m.now()
 		hold := mintHoldFor(mint, resp, detail, code, false, now)
 		if hold != nil && hold.Kind == mintHoldRateLimited {
-			return hold, holdRateLimitError(hold, now, "minting an agent token: "+detail)
+			e := holdRateLimitError(hold, now, "minting an agent token: "+detail)
+			if named := retryAfterSeconds(resp.Header.Get("Retry-After"), now); named > holdWait(hold, now) {
+				// Held for the cap; what the server asked for is kept too,
+				// so a caller can say both.
+				e.Cause = errors.Join(e.Cause, namedWaitError(named))
+			}
+			return hold, e
 		}
 		return hold, statusFailure("minting an agent token: "+detail, resp)
 	}

internal/auth/agent_connect.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 	"time"
 	"unicode"
@@ -766,23 +765,11 @@ func agentConnectBackoff(answer *agentConnectAnswer, interval time.Duration) tim
 // for nothing this can read. The date form is what a proxy in front of the
 // server is as likely to send as the server itself, and reading only the
 // seconds form would answer a mandated wait with the five-second step and
-// earn the next refusal.
+// earn the next refusal. It is read by retryAfterSeconds, the one parser
+// every Retry-After goes through, so a count too large to hold is the cap
+// here too, not a header nothing could read.
 func retryAfter(header http.Header, now time.Time) time.Duration {
-	value := strings.TrimSpace(header.Get("Retry-After"))
-	if value == "" {
-		return 0
-	}
-	if seconds, err := strconv.Atoi(value); err == nil {
-		if seconds <= 0 {
-			return 0
-		}
-		return reportedSeconds(seconds, maxAgentConnectLifetime)
-	}
-	when, err := http.ParseTime(value)
-	if err != nil {
-		return 0
-	}
-	return min(max(when.Sub(now), 0), maxAgentConnectLifetime)
+	return min(time.Duration(retryAfterSeconds(header.Get("Retry-After"), now))*time.Second, maxAgentConnectLifetime)
 }
 
 // agentConnectExpired is the one ending that is nobody's fault: the code

internal/auth/agent_hold.go

@@ -100,6 +100,12 @@ const defaultAgentRateLimitHold = 60 * time.Second
 // the safe direction to fail in.
 const maxAgentMintHold = time.Hour
 
+// MaxServerWait is the longest any wait a server names is waited, whatever
+// the status or the token path: the bound a rate-limit hold is written
+// under, so a held rate limit and every other named wait agree. A connector
+// that waited a server's word for years would be a hang, not a wait.
+const MaxServerWait = maxAgentConnectLifetime
+
 // MintHold is the token endpoint's last refusal of an agent credential's
 // client, remembered so the next mint can answer it without asking again.
 //
@@ -160,13 +166,13 @@ func mintHoldFor(mint *agentMint, resp *http.Response, detail, code string, refu
 }
 
 // rateLimitHold is how long a 429 holds the mint: its Retry-After, or
-// defaultAgentRateLimitHold when it has none, never past maxAgentMintHold.
+// defaultAgentRateLimitHold when it has none, never past MaxServerWait.
 func rateLimitHold(header http.Header, now time.Time) time.Duration {
 	wait := retryAfter(header, now)
 	if wait <= 0 {
 		wait = defaultAgentRateLimitHold
 	}
-	return min(wait, maxAgentMintHold)
+	return min(wait, MaxServerWait)
 }
 
 // ceilUnix is t in Unix seconds, rounded up: a deadline stored in whole
@@ -184,10 +190,16 @@ func holdWait(hold *MintHold, now time.Time) int {
 }
 
 // holdRateLimitError is the rate-limit error for a rate_limited hold,
-// whether the 429 that set it or a later mint it held.
+// whether the 429 that set it or a later mint it held. It carries the
+// hold's wait, read from its deadline, for a caller that reschedules the
+// mint itself (RetryAfter): one sent any sooner is answered by the hold.
 func holdRateLimitError(hold *MintHold, now time.Time, message string) *output.Error {
-	e := output.ErrRateLimit(holdWait(hold, now))
+	wait := holdWait(hold, now)
+	e := output.ErrRateLimit(wait)
 	e.Message = message
+	if wait > 0 {
+		e.Cause = retryAfterError(wait)
+	}
 	return e
 }
 
@@ -209,7 +221,7 @@ func (m *Manager) heldMint(creds *Credentials) error {
 	switch hold.Kind {
 	case mintHoldRateLimited:
 		e := holdRateLimitError(hold, now, fmt.Sprintf("Minting an agent token is held until %s: the token endpoint rate-limited the last attempt (%s)", when, hold.Detail))
-		e.Cause = errMintHeld
+		e.Cause = errors.Join(e.Cause, errMintHeld)
 		return e
 	case mintHoldRefused:
 		msg := "Minting an agent token was refused (" + hold.Detail + ")"

internal/auth/agent_hold_test.go

@@ -155,6 +155,7 @@ func TestARateLimitIsHeldUntilItsRetryAfter(t *testing.T) {
 	require.Error(t, err)
 	assert.Equal(t, output.CodeRateLimit, output.AsError(err).Code)
 	assert.NotContains(t, output.AsError(err).Message, "invalid_client", "a 429 is not a verdict on the client")
+	assert.Equal(t, 300, RetryAfter(err), "the wait is read from the hold's deadline")
 
 	*now = now.Add(299 * time.Second)
 	_, err = m.AccessToken(context.Background())
@@ -167,6 +168,8 @@ func TestARateLimitIsHeldUntilItsRetryAfter(t *testing.T) {
 	assert.Contains(t, held.Message, "the server answered HTTP 429")
 	assert.NotContains(t, held.Message, "invalid_client")
 	assert.Contains(t, held.Hint, "1 seconds")
+	assert.Equal(t, 1, RetryAfter(err), "a held mint carries what is left of the hold")
+	assert.ErrorIs(t, err, errMintHeld)
 	assert.NotContains(t, held.Hint, "--with-client-credentials", "a rate limit is not a refused secret")
 	assert.False(t, errors.Is(err, ErrAgentCredentialRefused), "a rate limit is not a disconnect")
 
@@ -223,6 +226,9 @@ func TestARateLimitHoldIsBounded(t *testing.T) {
 	}{
 		"no Retry-After": {func(time.Time) http.Header { return nil }, defaultAgentRateLimitHold},
 		"an absurd one":  {retryAfter("86400"), maxAgentConnectLifetime},
+		// Too large for an int: still the cap, never the one-minute default
+		// a header nothing could read gets.
+		"an overflowing one": {retryAfter("99999999999999999999999"), MaxServerWait},
 		"an HTTP-date": {func(now time.Time) http.Header {
 			return http.Header{"Retry-After": {now.Add(10 * time.Minute).UTC().Format(http.TimeFormat)}}
 		}, 10 * time.Minute},

internal/auth/auth.go

@@ -1976,3 +1976,9 @@ func (m *Manager) GetStore() *Store {
 func (m *Manager) SetStore(s *Store) {
 	m.store = s
 }
+
+// SetClock sets the time a remembered mint verdict is read and written
+// against, for a caller that drives time itself; nil means time.Now.
+func (m *Manager) SetClock(clock func() time.Time) {
+	m.clock = clock
+}

internal/auth/retry_after_test.go

@@ -0,0 +1,39 @@
+package auth
+
+import (
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// A Retry-After is read in both of its forms, a date rounded up to the
+// second, and a count too large to hold is held at the bound, never read as
+// no wait at all.
+func TestRetryAfterReadsBothForms(t *testing.T) {
+	now := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	for value, want := range map[string]int{
+		"42":                      42,
+		" 42 ":                    42,
+		"0":                       0,
+		"99999999999999999999999": maxRetryAfterSeconds,
+		now.Add(90 * time.Second).Format(http.TimeFormat): 90,
+		now.Add(-time.Minute).Format(http.TimeFormat):     0,
+		"":     0,
+		"-5":   0,
+		"soon": 0,
+	} {
+		assert.Equal(t, want, retryAfterSeconds(value, now), "%q", value)
+	}
+}
+
+// The wait a rate limit named travels with it, whichever form it came in.
+func TestARateLimitCarriesItsWait(t *testing.T) {
+	for _, value := range []string{"90", time.Now().Add(90 * time.Second).UTC().Format(http.TimeFormat)} {
+		err := statusFailure("minting an agent token", &http.Response{StatusCode: http.StatusTooManyRequests, Header: http.Header{"Retry-After": {value}}})
+		assert.InDelta(t, 90, RetryAfter(err), 1, value)
+	}
+	err := statusFailure("minting an agent token", &http.Response{StatusCode: http.StatusTooManyRequests, Header: http.Header{}})
+	assert.Zero(t, RetryAfter(err))
+}

internal/auth/revoke.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -224,14 +225,18 @@ func transportFailure(msg string, cause error) error {
 // statusFailure is a revocation request the server answered with something
 // other than 200, classified the way the SDK classifies any other response:
 // 429 is a rate limit (retryable, with its Retry-After), 507 an account
-// limit (a verdict, not retryable), any other 5xx retryable, the rest final.
+// limit (a verdict, not retryable), any other 5xx retryable (with its
+// Retry-After, when it names one), the rest final.
 func statusFailure(msg string, resp *http.Response) error {
 	switch resp.StatusCode {
 	case http.StatusTooManyRequests:
-		retryAfter, _ := strconv.Atoi(strings.TrimSpace(resp.Header.Get("Retry-After")))
+		retryAfter := retryAfterSeconds(resp.Header.Get("Retry-After"), time.Now())
 		e := output.ErrRateLimit(retryAfter)
 		e.Message = msg
 		e.HTTPStatus = resp.StatusCode
+		if retryAfter > 0 {
+			e.Cause = retryAfterError(retryAfter)
+		}
 		return e
 	case http.StatusInsufficientStorage:
 		e := output.ErrAPI(resp.StatusCode, msg)
@@ -240,10 +245,80 @@ func statusFailure(msg string, resp *http.Response) error {
 	default:
 		e := output.ErrAPI(resp.StatusCode, msg)
 		e.Retryable = resp.StatusCode >= 500 && resp.StatusCode < 600
+		// A 503 may name its wait as surely as a 429; a retryable answer
+		// carries it the same way.
+		if retryAfter := retryAfterSeconds(resp.Header.Get("Retry-After"), time.Now()); e.Retryable && retryAfter > 0 {
+			e.Cause = retryAfterError(retryAfter)
+		}
 		return e
 	}
 }
 
+// retryAfterError is a 429's Retry-After, in seconds, carried as the cause of
+// the rate limit statusFailure builds: output.Error has no field for it, and
+// a caller that reschedules the work itself needs the number, not the hint.
+type retryAfterError int
+
+func (s retryAfterError) Error() string { return fmt.Sprintf("retry after %d seconds", int(s)) }
+
+// maxRetryAfterSeconds bounds a Retry-After to what an int holds on every
+// platform; the SDK applies its own, tighter bound to whatever it is handed.
+const maxRetryAfterSeconds = math.MaxInt32
+
+// retryAfterSeconds is the wait a Retry-After header asks for, in either
+// RFC 9110 form — a count of seconds or an HTTP date, rounded up — and zero
+// when it asks for nothing this can read. A count too large to hold is held
+// at the bound rather than read as no wait at all.
+func retryAfterSeconds(value string, now time.Time) int {
+	value = strings.TrimSpace(value)
+	if seconds, err := strconv.ParseUint(value, 10, 64); err == nil || errors.Is(err, strconv.ErrRange) {
+		if seconds > maxRetryAfterSeconds {
+			return maxRetryAfterSeconds
+		}
+		return int(seconds)
+	}
+	when, err := http.ParseTime(value)
+	if err != nil {
+		return 0
+	}
+	seconds := math.Ceil(when.Sub(now).Seconds())
+	switch {
+	case seconds <= 0:
+		return 0
+	case seconds > maxRetryAfterSeconds:
+		return maxRetryAfterSeconds
+	}
+	return int(seconds)
+}
+
+// namedWaitError is the wait a server asked for, in seconds, when what is
+// waited is less: a 429 held for the cap rather than for all it named.
+type namedWaitError int
+
+func (s namedWaitError) Error() string {
+	return fmt.Sprintf("the server asked to wait %d seconds", int(s))
+}
+
+// NamedWait is the wait, in seconds, the server itself named for err: more
+// than RetryAfter when the wait was capped, the same otherwise.
+func NamedWait(err error) int {
+	var s namedWaitError
+	if errors.As(err, &s) {
+		return int(s)
+	}
+	return RetryAfter(err)
+}
+
+// RetryAfter is the wait, in seconds, a rate-limited answer named, or zero
+// when it named none.
+func RetryAfter(err error) int {
+	var s retryAfterError
+	if errors.As(err, &s) {
+		return int(s)
+	}
+	return 0
+}
+
 // discardGrant revokes a freshly minted credential the login refused to
 // store (a Verify failure). Without it the grant would stay live and
 // full-scope with no local record of it. The refusal is the error the