Retry 429 on every request, 502 on reads only

[Ibrahimrahhal]

Why

Scans were showing up in projects several times over from a single corgea scan.

#171 made every API call replay itself on 502 Bad Gateway. A 502 comes from the proxy in front of Corgea rather than from the API, so it says nothing about whether the API handled the request — it is equally the answer for "the request never arrived" and for "the request was processed and the reply was lost coming back". Every write the CLI sends creates something, and three of them create a scan:

• POST /api/v1/start-scan mints a new transfer, so a replay starts a second upload.
• PATCH /api/v1/start-scan/<transfer_id>/ is offset-addressed, so the archive bytes never double — but the chunk that fills the last of Upload-Length is the one that answers with scan_id, and CHUNK_SIZE is 50 MB, so on any repo that zips under that the single chunk is the scan-creating request.
• POST /api/v1/scan-upload takes a whole third-party report.

When the API committed one of those and the proxy lost the reply, sending it again did not finish the first scan. It started another one. With three replays in the schedule, one flaky write became up to four records.

There was never a whole-scan retry, to be clear about the original bug's scope. Retries are per request. It is just that the request being retried was sometimes the one that creates the scan.

The mirror-image gap was that 429 Too Many Requests was not retried at all, so a rate limit failed the whole pipeline.

What changed

The retry decision now takes the status and the method together, because the two statuses worth retrying promise different things about what happened to the request:

	429 Too Many Requests	502 Bad Gateway
What it means	The rate limiter declined the request before the API saw it	The proxy could not get an answer, and cannot say whether the API acted
Was anything created?	No	Unknown
Retried for	Every method, writes included	Reads only

Re-sending after a 429 finishes the same work; re-sending a write after a 502 risks doing it twice. is_gateway_error becomes is_transient_error and covers both, which is also what the source-upload walk wants: it stops early on a 502 because a write is never replayed, and on a 429 because the retries are already spent.

A 429 usually names its own window, and ignoring that means spending the whole schedule inside it and failing anyway, so Retry-After is honored when present. It is capped at two minutes for any one pause, so a header the CLI cannot sanity-check cannot stall a pipeline, and floored at the schedule, so a retry cannot become a hammer. Only the delta-seconds form is read; the HTTP-date form would need the CLI to trust its own clock against the server's.

Writes keep the pre-existing network-error retries (is_connect/is_timeout), where nothing reached Corgea at all.

/code-upload is in fact keyed by run_id and path, so its 502 would be safe to replay on its own merits. It is excluded anyway: a method-level rule is one thing to reason about at every call site, whereas a per-endpoint exemption list is a thing to get wrong later. It can be added deliberately if it turns out to matter.

Also bumps the CLI to 1.14.1.

Evidence

Five scenarios, each run against the pre-fix binary and this one, with a stub that answers 502 after committing (modelling the lost reply a client cannot distinguish) and 429 without committing at all.

retry_policy_502_and_429.log

Reads are unchanged. A 502 on POST /start-scan goes from three transfers to one and on the archive PATCH from three scans to one. A 429 on either goes from failing the scan outright to completing it.

The PATCH/502 figure assumes completing a transfer twice mints two scans. That is an assumption about the API the CLI cannot check, which is the reason not to replay: if completion is keyed to the transfer_id the replay was harmless, and if it is not, it was the bug. Sending it once is correct either way.

Tests

Thirteen unit tests and seven end-to-end. The policy assertions were each checked against a deliberately broken build — retrying 429 only for safe methods, ignoring Retry-After, and dropping its cap — and all three mutations were caught.

retry_policy_tests.log

The end-to-end tests run the real binary against an ordered stub that fails the plan on an unexpected request, so they pin the exact attempt count: a_rate_limited_scan_start_goes_through_on_a_retry proves the rebuilt multipart form is re-sent and the scan completes, while a_rejected_scan_start_is_not_sent_again and a_rejected_archive_chunk_is_not_sent_again prove a 502 on a write is the answer. Both wait tests and the two walk-stop tests are unchanged in intent.

./harness ci is green: strict clippy, format check, dep audit, and 835 tests over the coverage gate.

retry_policy_ci.log

_{To show artifacts inline, enable in settings.}

  

[corgea-security]

high: Offset-addressed report chunks no longer retry 502 responses

This call passes POST, so send_with_retries immediately takes its non-replayable path and never applies GatewayRetries. However, this request is an offset-addressed chunk upload using Upload-Offset and Upload-Length, which the README explicitly identifies as safe to replay. A transient 502 now aborts an otherwise retryable large report upload. Replayability must account for this operation rather than only its HTTP method.

Proof or reproduction:

let response = send_with_retries("chunk", &Method::POST, || post_chunk());
// Stub responses: 502, then 200
// Actual: one request and 502 returned.
// Expected for offset-addressed chunks: two requests and 200 returned.

[cursor]

No merge blockers on 20e0bdf.

Checked the merge-base diff against main (b6b5707...20e0bdf) and the six send_with_retries call sites. is_replayable is method.is_safe() || PATCH, and every site passes the method it actually sends: POST /start-scan, source/report/git-config/scan-upload POSTs, and the archive PATCH that carries Upload-Offset. DebugRequestBuilder::send returns a non-replayable method on the first response, so a cloneable JSON POST cannot pick up the 502 schedule that GET and PATCH still use.

The incident is pinned. send_with_retries_leaves_a_post_at_one_attempt and a_rejected_scan_start_is_not_sent_again fail the stub on a second POST /start-scan. The source walk still stops on the first 502; a_rejected_source_upload_stops_the_whole_upload_walk now expects one /code-upload and reports both files unsent. The wait GETs and blast_upload_replays_an_archive_chunk_the_gateway_rejects still require the old retry counts, so the retries that are safe to keep are still there.

retry_on_network_error (is_connect / is_timeout) still wraps a POST. That path predates this change and is outside the 502-replay bug. rust-tests is green. No prior review threads.

  

_{Sent by Cursor Automation: pr-flow}

[corgea-security]

Automated review risk: 4/5.

The retry policy regresses resilience for offset-addressed scan report chunks by classifying them solely as non-replayable POST requests.

Critical or high-priority changes must be addressed.

Automatic approval was not submitted: automated review found critical or high-priority findings.