Let Go's raw GET retry loop see the Retry-After it already parses

[jeremy]

Closes #795. go/pkg/basecamp/client.go:694 read its server-specified retry delay off a *retryableError. Nothing in the tree ever built one.

$ rg -n "retryableError" go/
go/pkg/basecamp/http.go:115:// retryableError wraps an error with retry metadata.
go/pkg/basecamp/http.go:117:type retryableError struct {
go/pkg/basecamp/http.go:122:func (r *retryableError) Error() string {
go/pkg/basecamp/http.go:126:func (r *retryableError) Unwrap() error {
go/pkg/basecamp/client.go:694:		if re, ok := err.(*retryableError); ok {

The type, its two methods, one type assertion. No &retryableError{. So the branch was unreachable, every 429 landed on the else if apiErr arm, and the loop backed off locally against a server that had named a time. The number was parsed — client.go:847 calls parseRetryAfter — and then spent on a hint string, because Error had nowhere to put it.

Scope: the raw path only. Typed service methods go through the generated client's loop, which reads the header inline and honours it. downloadURL and the rate-limiter hook honour it too. This was Client.Get/GetAll and the escape hatch, which is why conformance never saw it.

Shape: Error.RetryAfter, and retryableError deleted

Of the two candidates in #775's remedy table, only one of them removes a mechanism. Constructing retryableError at the 429 arm keeps two error types flowing through one loop and changes what singleRequest returns to every other caller — including the mutation path's err.(*Error) at client.go:664. Putting the value on Error makes the *Error arm the only arm, and the dead type goes with the branch that was its only reader.

Nothing is lost by deleting it. Unwrap is load-bearing for errors.Is/errors.As only through a value that reaches a caller, and no value ever existed — apidiff confirms it was never public surface either.

The field is populated at both 429 construction sites — ErrRateLimit for the raw path, checkResponse for the generated service layer — so a caller reading err.RetryAfter gets the same answer whichever door it came through. Half-populating it would have been worse than not having it.

What did NOT change: which statuses carry a value

Only the two 429 sites set the field, matching downloadURL and the generated loop. #775 decides whether 503 and the rest join them, and its five-SDK convergence is not this PR's. The code says so at the branch. No ceiling is applied either — #793 is settling that in SPEC.

Red proof

Reverting only the honour hunk — the loop back to delay = c.backoffDelay(attempt), everything else including the tests unchanged — and running go test ./pkg/basecamp/ -run RetryAfter:

    client_retry_after_test.go:127: computed a 1.42974ms retry delay, want 2s from the Retry-After header (the backoff curve here is ~1ms, so this is the backoff, not the server's number)
--- FAIL: TestClient_RetryAfterReplacesBackoff (0.00s)
    client_retry_after_test.go:147: computed a 1.639404ms retry delay for an HTTP-date 90s out, want ~90s
--- FAIL: TestClient_RetryAfterHTTPDateReplacesBackoff (0.00s)
--- PASS: TestClient_RetryAfterAbsentOrUnusableKeepsBackoff (0.00s)
FAIL	github.com/basecamp/basecamp-sdk/go/pkg/basecamp	1.342s
REAL_EXIT=1

Five mutants, five kills. Files restored by cp and verified with diff -q, never git checkout --.

Mutation	Killed by
loop back to backoffDelay unconditionally	RetryAfterReplacesBackoff, …HTTPDate… — 1.4ms vs 2s
if apiErr.RetryAfter > 0 → if true	…AbsentOrUnusableKeepsBackoff — [0s 0s], backoff collapsed
retry wait's select → plain time.Sleep	both probe tests — 2 delays computed where a cancelled request must compute 1
checkResponse drops the field	CheckResponse_CarriesRetryAfter/seconds — 0, want 17
ErrRateLimit drops the field	…ErrorCarriesSeconds — 0, want 42; and …ReplacesBackoff

The tests spend no wall clock and add no seam

The first version of this branch gave Client a retrySleep func(...) field so tests could read the computed delay without sleeping it. The api-compat job from #776 rejects that, and correctly:

Incompatible changes:
- ./pkg/basecamp.Client: old is comparable, new is not

A func field makes the struct non-comparable. Rather than work around the gate, the seam is gone: the loop already logs the computed delay and then fires OnRetry, both before it sleeps. Cancelling from OnRetry returns the wait instantly however long it was, and the delay is read off the "retrying request" log record's delay attribute — an exact value, from observables the SDK already emits, at zero production cost. apidiff on the branch as it stands:

Compatible changes:
- ./pkg/basecamp.Error.RetryAfter: added

No test in this file measures elapsed time as an assertion (#783). The one timing check in retryAfterProbe is a 5s tripwire whose only job is to turn "the wait stopped observing cancellation" into a message instead of a package timeout, and it is one-sided: load can only push a ~2ms measurement toward a bound three orders of magnitude away.

Interruptibility: yes, and now proven

The wait is a select on ctx.Done() and time.After(delay), so a caller can abandon a long server-directed delay — which matters precisely because no ceiling is applied to it. The select is unchanged by this PR; what changed is that it can now be reached holding a five-minute delay instead of a one-second one.

It was previously untested. The obvious test does not work: signalling from the handler and cancelling from a goroutine cancels while the response is still in flight, so the loop takes the network-error path, backs off ~1ms, and returns a wrapped context.Canceled that satisfies every assertion — that version passes against a plain uninterruptible time.Sleep, which is how the mutant survived the first round here. Cancelling from OnRetry is what pins it, and now every case in the file carries that guard.

download.go:205 and the generated loop are the same select; all three honour cancellation.

Generated path: not affected, but not identical either

client.gen.go:5805-5813 reads and honours Retry-After on 429 inline. It never had this defect, go/templates/client.tmpl is untouched, and nothing was regenerated.

Two pre-existing differences, reported rather than fixed:

• It parses delta-seconds only (strconv.Atoi), so SPEC §6 step 2's HTTP-date form is dropped — the same shape Fix SPEC §6's Retry-After parsing in Kotlin and TypeScript, and make the fixture able to fail #781 just closed in Kotlin and TypeScript, in a fourth copy of the algorithm that PR did not reach. The raw path resolves both forms; a test here covers it.
• It adds jitter on top of a server-directed delay. The raw path and downloadURL replace the curve outright.

Neither belongs in a bug fix for a dead branch; both are worth an issue.

SPEC

§5's BasecampError RECORD already listed retry_after, with a Go divergence note saying Go omits it. That note is now false and is rewritten. RequestResult.retry_after is untouched — it stays the hook-facing copy, no longer the only one.

Left alone deliberately: §6's "Go on 429 and 503" in the status-set paragraph. It measures http.go:203, which feeds hooks and the rate limiter, not the sleep — and that paragraph is explicitly #775's.

Verification

LC_ALL=C on all of it: make go-check 0 · go-check-drift 0 · go-check-wrapper-drift 0 · doc-constants-check 0 · conformance-go 0 (all six Retry-After cases PASS) · check-retry-metadata-parity 0 · apidiff -m against origin/main compatible.

Review follow-up: the conversion is clamped

Two reviewers independently found the same defect in the honour hunk, and it is real: Retry-After: 9223372036854775807 parses cleanly on a 64-bit build and time.Duration(n) * time.Second wraps to -1s, so time.After fires at once and the loop spends its whole attempt budget back to back — the newly honoured header turned into a tight retry loop.

Both doors onto Error.RetryAfter now normalize through clampRetryAfterSeconds: parseRetryAfter, which also feeds checkResponse, downloadURL and RequestResult, and the exported ErrRateLimit, which takes a bare int and could therefore also carry a negative value the field's own doc calls invalid.

Over-range that the parser can hold saturates rather than falling back to absent — falling back would compute the ~1ms backoff curve and hammer the peer, the same tight loop by another route — while a digit string the parser's own int64 cannot hold is malformed and falls through to backoff (bf45888bc; the tiers are #793's, the cross-SDK decision #799's). This amends "no ceiling is applied either" above: what is applied is a representability bound, not the policy cap #793 declined. At 2147483647s (~68 years) it rejects nothing a server could sensibly ask for, and SPEC §7 note 4 already carves out bounding against host limits (Swift's UInt64 trap is its worked example). This PR no longer touches that sentence; #793 is the lane that adds Go to it.

Four mutants, four kills: the clamp out of parseRetryAfter (CheckResponse_CarriesRetryAfter/beyond_duration_range), out of ErrRateLimit (ErrRateLimit_NormalizesRetryAfter/{negative, beyond duration range}), out of both — the reported state — (RetryAfterSaturatesAtDurationCeiling, which reports 2 delays ([-1s 2.802726ms]), the tight loop itself), and saturate replaced by treat-as-absent (same test, 1.964215ms, want 2562047h47m16s). Neither single-guard mutant is caught by the end-to-end case, so each clamp carries its own killing test.

The generated client keeps the identical unclamped conversion at client.gen.go:5810 (go/templates/client.tmpl:457), plus an unchecked Atoi feeding NewRateLimitError. Not fixed here — it needs a template change and a regeneration this PR deliberately does not do — and filed as #798 alongside the two other divergences in that loop reported above.

---

Summary by cubic

Honor Retry-After in Go’s raw GET retry loop and make waits cancellation-first. Previously 429s ignored Retry-After and used millisecond backoff; now the loop sleeps the server’s delay (delta-seconds or HTTP-date), rounds HTTP-date up to whole seconds, clamps to 2,147,483,647s, and checks context before waiting.

• Add basecamp.Error.RetryAfter (seconds). ErrRateLimit and checkResponse set it; the raw loop uses it when > 0. Remove dead retryableError.
• Parse delta-seconds as digits-only into int64 and clamp via a shared clampRetryAfterSeconds; HTTP-date converts with integer math and rounds up. Shared by downloadURL and the rate-limiter hook.
• Cancellation-first: both GET and download retry waits check ctx.Err() before the select.
• Generated clients unchanged (seconds-only parse, jitter, no clamp). SPEC now states step-2 “round up” and clears pending markers.

Migration

• Replace any unkeyed basecamp.Error{...} literals with keyed fields; RetryAfter was inserted between Retryable and RequestID.
• When rescheduling from a returned error, use errors.As to extract *basecamp.Error and read RetryAfter. 429 waits can be long; prefer context cancellation or reschedule using RetryAfter.

^{Written for commit 93d1dde. Summary will update on new commits.}

[Copilot]

Pull request overview

Fixes Go’s raw GET retry loop so 429 responses honor parsed Retry-After values.

Changes:

• Adds RetryAfter to structured errors.
• Removes unreachable retryableError handling.
• Adds regression tests and updates the specification.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
SPEC.md	Documents Go retry metadata behavior.
go/pkg/basecamp/http.go	Removes the unused error wrapper.
go/pkg/basecamp/helpers.go	Populates retry metadata for typed services.
go/pkg/basecamp/errors.go	Exposes retry delay on structured errors.
go/pkg/basecamp/client.go	Applies server-directed delays in raw GET retries.
go/pkg/basecamp/client_retry_after_test.go	Tests parsing, propagation, fallback, and cancellation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72050ba6b9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

go/pkg/basecamp/client_retry_after_test.go:320

• This direct constant-to-int conversion also prevents the package tests from compiling on 32-bit targets; the runtime skip cannot protect a compile-time overflow. Derive an expectation bounded by the host int range.

		{name: "beyond duration range", header: "9223372036854775807", want: int(maxRetryAfterSeconds), needsWideInt: true},

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

SPEC.md:840

• This unqualified statement is false for Go's generated client: go/templates/client.tmpl:457 still multiplies an unchecked strconv.Atoi result, which is the overflow tracked in #798 and explicitly left unchanged by this PR. Qualify the bound as applying only to the hand-written Retry-After paths so the SPEC does not promise safety that typed service calls lack.

   86,400s because `UInt64(_:)` on an out-of-range `Double` is a trap, and Go
   saturates its seconds→`time.Duration` conversion at `math.MaxInt64 / time.Second`
   (~292 years) because the product otherwise wraps negative and `time.After` on a
   non-positive duration fires at once — turning a server-directed wait into a tight
   retry loop. Both are *representability* bounds on the conversion, not policy caps

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51546fab1b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

go/pkg/basecamp/client_retry_after_test.go:101

• This 5-second tripwire cannot detect an uninterruptible retry wait: client.Get is called synchronously, so if the wait blocks (especially the ~292-year saturation case), execution never reaches the elapsed-time check and the package test times out instead. Run Get in a goroutine and select between its result and a 5-second timer so the guard can actually fail with the intended diagnostic.

	start := time.Now()
	_, err := client.Get(ctx, "/test.json")
	// Not the assertion — the assertions are all exact equalities below. This
	// is the guard that keeps the whole file honest: if the loop's wait ever
	// stopped observing cancellation, every case here would spend its full

go/pkg/basecamp/client.go:1130

• On 32-bit builds this clamp runs too late for HTTP-date values beyond math.MaxInt seconds (for example, a valid date in 2100): converting the positive float64 to int produces an out-of-range implementation-specific value, which is non-positive with the Go compiler, so the parser returns 0 and the client falls back to millisecond backoff instead of saturating. Clamp the floating-point duration against both the time.Duration ceiling and math.MaxInt before converting it to int; this is distinct from the oversized decimal case, which Atoi rejects on 32-bit.

			return clampRetryAfterSeconds(seconds)

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (4)

go/pkg/basecamp/client.go:1130

• On 32-bit builds, Atoi rejects valid delta-seconds above math.MaxInt before the new clamp can saturate them. For example, Retry-After: 2147483648 falls back to the local backoff instead of clamping to the largest delay the exported int field can represent. Parse in the clamp's int64 domain first; the PR already explicitly supports 386/ARM and bounds the result before converting to int.

	if seconds, err := strconv.Atoi(header); err == nil && seconds > 0 {
		return clampRetryAfterSeconds(int64(seconds))

go/pkg/basecamp/client.go:1146

• Integer division rounds a future HTTP-date down, so the retry can happen almost one second before the server's named time; a date less than one second away becomes zero and is discarded entirely. Kotlin, Swift, and TypeScript all round this conversion up for the same reason. Compute a ceiling in integer arithmetic before clamping.

		if seconds := int64(time.Until(t) / time.Second); seconds > 0 {
			return clampRetryAfterSeconds(seconds)

SPEC.md:838

• This bound is not always math.MaxInt64 / time.Second: clampRetryAfterSeconds also applies math.MaxInt, so Go saturates at about 68 years on 32-bit builds and about 292 years on 64-bit builds. The canonical SPEC should describe both host limits, especially since this PR explicitly keeps 32-bit builds supported.

   hand-written client saturates its seconds→`time.Duration` conversion at
   `math.MaxInt64 / time.Second` (~292 years) because the product otherwise wraps

MIGRATING.md:260

• The new migration claim is architecture-dependent: RetryAfter is an int, and the clamp uses math.MaxInt, so a 32-bit build can represent only about 68 years rather than 292 years. Document both limits so consumers do not rely on a value this public field cannot carry on supported 32-bit targets.

ceiling beyond what a `time.Duration` can represent (over-range values saturate
at ~292 years rather than wrapping negative). Typed service methods, downloads

[jeremy]

Four suppressed Copilot comments — all four taken

No threads to reply in, so they are answered here. All in 48a85d29.

client.go:1130 — Atoi rejects delta-seconds above math.MaxInt before the clamp can saturate them.

Right, and it made the same header mean two different things by target: Retry-After: 2147483648 was honoured on a 64-bit build and fell onto the millisecond backoff curve on a 32-bit one, because Atoi's range is int's. The parse is now strconv.ParseInt(header, 10, 64), which leaves the ceiling where it belongs — in the clamp, which already bounds the result by what the host can hold. A digit string too long for an int64 is still malformed on every target, which keeps the boundary SPEC §16's device parser draws, and the existing digits beyond int range case still pins it. 64-bit behaviour is unchanged.

client.go:1146 — integer division rounds a future date down.

Right, and the claim about the other SDKs checks out: TypeScript Math.ceil, Kotlin (remainingMs + 999) / 1000, Swift .rounded(.up) — and Kotlin's source states the rule as though it were the convention. SPEC never wrote it down, which is why Go, Python and Ruby all truncate.

Go now rounds up (remainder bump on the integer division, so nothing overflows at the saturation boundary), and SPEC §6 step 2 states the rule and its two reasons: a positive remainder must never round to zero, because zero reads as "no usable value" and drops the request onto the backoff curve; and truncating retries before the moment the server named. Red-proved — with the truncating mutant restored:

--- FAIL: TestParseRetryAfter_HTTPDateRoundsUp (0.00s)
    client_retry_after_test.go:313: parseRetryAfter(a date 5s out) = 4, want 5 — a truncated remainder waits less than the server asked, and under a second it rounds to 0 and is discarded
REAL_EXIT=1

Python and Ruby still truncate, and this PR does not touch them — that is #799, which carries the six-SDK table and the SPEC wording. Go was brought into line here only because the line was already being edited; the convergence is not complete and the issue says so.

SPEC.md:838 and MIGRATING.md:260 — the saturation bound is architecture-dependent.

Right on both, and they are the same defect in two documents: RetryAfter is an int, the clamp applies math.MaxInt as well as the Duration ceiling, so the bound is ~292 years only where int is 64 bits and ~68 years where it is 32. SPEC now spells the bound min(math.MaxInt64 / time.Second, math.MaxInt) and names which limit binds where; MIGRATING gives both figures and says not to read the larger one as portable.

Gates after all four: LC_ALL=C make go-check 0 · go-check-drift 0 · go-check-wrapper-drift 0 · doc-constants-check 0 · apidiff -m -incompatible clean (still only Error.RetryAfter: added) · GOOS=linux GOARCH=386 and GOARCH=arm vet 0.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48a85d298d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

MIGRATING.md:263

• SPEC §6 draws that line is inaccurate: §6 only says to parse a positive integer, while #799 records over-range handling as an unresolved cross-SDK decision. Label the int64 fallback as Go-specific rather than normative.

wrapping negative, and that figure does not vary by architecture. A value too
large for the parser's own `int64` is malformed instead and falls through to the
backoff curve, as it always did — SPEC §6 draws that line.

go/pkg/basecamp/client.go:1164

• SPEC §6 does not define these two tiers; it only says to parse a valid positive integer, and #799 explicitly tracks over-range behavior as unresolved. The fallback may remain Go's behavior, but this comment should not claim the SPEC mandates it.

	// A value too large for that int64 is MALFORMED and falls through to step
	// 3's backoff — SPEC §6's first tier — rather than saturating. So is any
	// other unparseable input: ParseInt returns 0 with ErrSyntax, and a
	// negative range error clamps to math.MinInt64, both caught by the `> 0`
	// guard alongside the err check. The second tier, saturation, is for a

go/pkg/basecamp/client.go:1081

• The PR description says Go is added to SPEC §7's host-limit note, but that note still names only Swift. Please record Go's 2,147,483,647-second cross-architecture bound there so the normative documentation matches this new implementation.

// maxRetryAfterSeconds is the largest delta-seconds value this SDK honours:
// 2147483647, ~68 years. It is a REPRESENTABILITY bound taken at the portable
// limit, not a policy ceiling — SPEC §7's "Retry-After is exempt" note already
// carves out exactly this ("implementations may still bound it against host
// limits"), and Swift clamps its own seconds→nanoseconds conversion for the

[jeremy]

Six suppressed Copilot comments across three rounds — four taken, one declined, one already absorbed

Rounds @f56784f (21:27Z), @bf45888 (04:51Z) and @6dd6702 (06:04Z). No threads to reply in, so they are answered here. All in 7c19452.

client.go:729 — the retry wait's select cannot promise "cancellation wins". TAKEN, in both loops.

Right. When both cases are ready Go picks pseudo-randomly, and the loop fires OnRetry immediately before it waits — so a hook that cancels there, with a delay that has already elapsed, saw the timer win about half the time and sent one more request on a dead context. That request fails fast, but what comes back is the transport's wrapping of context.Canceled, and sometimes the loop goes round again to request failed after 3 attempts. A ctx.Err() check now runs before the select, where nothing competes with it. fetchAPIDownload (download.go:205) has the identical select behind the identical OnRetry, so it takes the identical guard; the generated loop's two selects (go/templates/client.tmpl:416,462) have the same shape but no hook fires before them, so the window there is a concurrent cancellation only — noted for #798, not edited, since it is template-and-regenerate work this PR deliberately does not do.

Red-proved, stated honestly: the interleaving cannot be forced — pseudo-random means exactly that — so the contract is asserted the one way it can be. With the context cancelled at OnRetry and a zero backoff so the timer is already ready, the loop must return ctx.Err() itself (identity, not errors.Is, because the transport's error also satisfies errors.Is) having handed the transport exactly one request, on every one of 64 runs. Against the bare select each run is a coin flip; five invocations, five failures, by run 3 at the latest, on both loops:

--- FAIL: TestClient_RetryWaitChecksCancellationBeforeTheTimer (0.00s)
    client_retry_after_test.go:315: run 3: Get returned request failed after 3 attempts: Network error: Get "http://127.0.0.1:52700/test.json": context canceled, want ctx.Err() itself (context.Canceled) — the loop went round again after the caller cancelled at the retry boundary
--- FAIL: TestClient_RetryWaitChecksCancellationBeforeTheTimer (0.00s)
    client_retry_after_test.go:319: run 0: the transport was handed 2 requests, want 1 — a cancellation delivered before the wait must not be followed by another attempt
--- FAIL: TestDownloadURL_RetryWaitChecksCancellationBeforeTheTimer (0.00s)
    download_test.go:836: run 0: DownloadURL returned Network error: Get "http://127.0.0.1:52921/999/blobs/abc/download/file.png": context canceled, want ctx.Err() itself (context.Canceled) — the loop went round again after the caller cancelled at the retry boundary
REAL_EXIT=1

Against the guard it cannot fail — the check runs before there is anything to pick between — and 20× green, plus go test -race -count=20 -run 'RetryAfter|Cancel' clean. This is also what takes the load-sensitivity out of the probe tests: their 1ms backoff cases could lose the same flip under preemption, and now cannot.

client.go:1186 — positive ErrRange should saturate, not fall to backoff. DECLINED, already decided.

This is the reading bf45888b reverted (04:47Z note above), and the reason has not changed: a digit string the parser's own int64 cannot hold is the first of the two tiers #793 states in §6 "Retry-After Honouring" — malformed, step 3 — and #793 names Go as rejecting above its 64-bit integer, so saturating here would falsify that sentence on contact. Accurately bounded: this is Go's behaviour and #793's rule; main's §6 today says only "parse a positive integer", and the cross-SDK over-range decision is open in #799. Which is the next item.

MIGRATING.md:263 and client.go:1164 — "SPEC §6 draws that line" overclaims. TAKEN.

Right: §6's parsing algorithm on main draws no such line, and #799 records over-range as unresolved across the SDKs. Both now say the split is Go's, that it is the two-tier rule #793 states in §6 "Retry-After Honouring" (unrepresentable → malformed, unschedulable → saturate), and that #799 tracks the convergence — worded to be true on either side of #793 landing. The same phrase sat in two test comments; corrected with them. Two more stale comments from the 21:07–21:14Z rounds that never got a note went the same way: the loop and the wait said Retry-After carries "no ceiling" (it carries no policy ceiling; the representability clamp exists), and parseRetryAfter's doc said it clamps to what a Duration can hold, where the constant is the smaller portable bound.

client.go:1081 — the PR description said Go was added to §7's note 4. TAKEN, in the description.

Right, that was true of an earlier revision and 4aa068d0 dropped the hunk — the description kept the claim. Edited in place; the sentence now reads that this PR no longer touches that note and #793 is the lane that adds Go to it (its branch already does), and the same sentence's "~292 years" became the shipped 2147483647s / ~68 years with the tier-1 clause stated. Nothing in the tree changes for this one.

client_retry_after_test.go:279 and :101 (21:27Z round) — one absorbed, one taken.

:101's "~292 years" was already corrected to ~68 in 6dd6702b (its message says so). :279 was not: the far-future HTTP-date test asserted only <= maxRetryAfterSeconds, which any positive value satisfies — including a parser returning an arbitrary "safe" delay instead of saturating. It now asserts equality, which is exact and deterministic because time.Until saturates at the Duration maximum for a year-9999 date, far past the ceiling.

Merge state with #793. wt/lane-spec advanced to 7d2c37f6 since the last note, and its new [PENDING #796 …] paragraph now describes this PR as shipped — ParseInt into int64, over-range malformed, clamp at math.MaxInt32 — so the one-token correction the 04:47Z note asked for is absorbed. git merge-tree --write-tree --messages wt/lane-spec wt/lane-go-retryafter → MT_REAL_EXIT=1, still exactly one CONFLICT (content) in SPEC.md, still the same adjacency hunk: #793 replaces the "This algorithm defines parsing only" paragraph and appends its section; this PR adds the step-2 rounding paragraph immediately above it. Resolution unchanged — take #793's block, keep the rounding paragraph above it. One more thing that rebase owes: #793 now marks its two "Go saturation" sentences [PENDING #796] (the §6 worked example and §7's note 4), because it merges first and its tree does not contain this PR. Those markers come out in this PR's rebase onto main after #793 lands — they are not anything this PR's branch carries today. One more flag for whoever takes #799: its Go row ("saturates at 2147483647s … in the parser") predates bf45888b and should read "rejects above int64, saturates in-range above math.MaxInt32".

Gates, REAL_EXIT read back from each log: go-check 0 · go-check-drift 0 · go-check-wrapper-drift 0 · doc-constants-check 0 · GOOS=linux GOARCH=386 vet 0 · GOARCH=arm vet 0 · apidiff -m -incompatible 0 with empty output, full diff still only Error.RetryAfter: added · go test -race -count=20 -run 'RetryAfter|Cancel' ./pkg/basecamp/... 0.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

MIGRATING.md:284

• The raw GET loop wraps its final *Error with fmt.Errorf after retry exhaustion (client.go:755), even when the attempt cap is one; the new test correctly uses errors.As for this reason. Describing the result as a returned *Error suggests direct access/type assertion, which will fail. Document the required errors.As extraction before reading RetryAfter.

backoff may now hit it. The wait observes cancellation — the loop selects on
`ctx.Done()` — so cancelling is the escape, and `err.RetryAfter` on the returned
`*Error` is there if you would rather reschedule the work yourself.

[jeremy]

One suppressed Copilot comment on 7c1945224, taken in cf2a1e874: MIGRATING.md:284 said err.RetryAfter is on the "returned *Error". Correct — the raw GET loop wraps the final error with fmt.Errorf on exhaustion (client.go:755), even at a cap of one attempt, and this PR's own test uses errors.As for that reason. The paragraph now says so and shows the errors.As extraction. Doc-only; LC_ALL=C make doc-constants-check exit 0.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

[jeremy]

Brought up to main after #793 landed (19896d7f5), as a merge rather than a rebase so the per-commit history stays intact for the squash body — replaying the early commits against #793's final text conflicted at every step in places the final states do not.

92a205115 resolves the one SPEC.md hunk exactly as both PRs said it would: this PR's step-2 rounding paragraph kept, #793's replacement "parsing only" paragraph and new "Retry-After Honouring" section taken beneath it. Nothing else collided.

93d1dde88 clears the three [PENDING #796] markers SPEC carried for this PR (the representability observation at §6, the Go worked-example paragraph, and §7's requirement 4). Each said "until #796 lands, the hand-written path is unclamped and this paragraph describes the contract, not the tree" — true until this merge and false after it, which is exactly the class-A current-value claim this repo's marker discipline exists to keep out of prose. Doing it in the commit whose landing makes them true avoids a window where SPEC is wrong. Wording only; no contract changed.

LC_ALL=C make doc-constants-check and make sync-api-version-check exit 0 on the merged tree.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.