INF-1671/fix: clamp and floor Retry-After in twoctl backoffDelay#2
Merged
brtkwr merged 1 commit intoJun 16, 2026
Merged
Conversation
Treat the Retry-After header as a lower bound clamped to maxBackoff rather than an override. A hostile or misconfigured Retry-After: 999999 no longer parks the CLI for days, and Retry-After: 0 after a rate-limit reset no longer collapses into a zero-delay retry storm. Mirrors the twoadm-cli httpx hardening so the two CLIs keep the same retry semantics (INF-1314).
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Puvendhan
approved these changes
Jun 16, 2026
brtkwr
deleted the
brtkwr-two/inf-1671-twoctl-clamp-and-floor-retry-after
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
backoffDelayininternal/httpx/client.gonow treatsRetry-Afteras a lower bound clamped tomaxBackoff, instead of honouring it literally. It always computes the jittered exponential backoff and usesmax(backoff, retryAfter), withretryAfterclamped tomaxBackoff.Extracted two helpers (
jitteredBackoff,retryAfter) so the policy reads clearly.Why
Two latent bugs in the previous literal handling:
Retry-After: 999999made the CLI sleep ~11.5 days between retries (bounded only by a per-command context deadline, which not every command sets).Retry-After: 0-> zero-delay retry. After a fleet-wide rate-limit reset,Retry-After: 0collapsed backoff to ~0, producing a thundering herd and burning the retry budget instantly.twoadm-cli's
internal/httpxalready guards both edges; this brings twoctl in line so the two CLIs keep the same retry semantics (the "same retry policy semantics across both ctls" line of INF-1314).Acceptance criteria
Retry-After: 999999-> delay== maxBackoff(no multi-day sleep). ✅TestBackoffDelayClampsLargeRetryAfterRetry-After: 0-> delay>= baseBackoff(no zero-delay storm). ✅TestBackoffDelayFloorsZeroRetryAfterRetry-After: 7-> still ~7s (below cap, unchanged). ✅ existingTestBackoffDelayHonoursRetryAfterRetry-Afternever shrinks our backoff below the cap at high attempts. ✅TestBackoffDelayRetryAfterIsLowerBoundNotCeilingScope
Stayed within the ticket boundary: only
backoffDelayand its tests changed. HTTP-date form ofRetry-After(twoadm-cli parses it, twoctl is seconds-only) is left as a possible follow-up, noted on the ticket.Verification
go test ./...green (full module, including the pre-existingRetry-After: 0fast-path test).gofmt/go vetclean,go build ./...ok. Run in the cloud sandbox with Go 1.26.3.Fixes INF-1671
Refs INF-1314