api/client.rs downloads a served artifact through two near-copy GETs and hand-writes four retry loops under two retry policies #1869

[agent] Filed by the scheduled architecture audit (audit-core). Register: register.

Kind: refactor. Register ID: A27.

Problem

1. Two GETs of a grant-tokenized serve URL.

• download_vendor_archive_once L1683-L1776 is behind the vendor archive and the crate-internal download_artifact(url) (gem stub gemspec).
• download_artifact_capped_once L2851-L2937 is behind PatchApi::download_artifact(url, max_bytes), the hosted and in-memory engine.

Both do the same steps: the scheme check, the send timeout, 404/410 → miss, 408 → still building, the retry hint plus classify_auth_error, then read_capped_typed under the body timeout. They have drifted:

The two functions share the name download_artifact, but a cap breach comes back as a different error variant from each.

2. Four hand-written retry loops. They are send_json_request L567, request_vendor_references L1532, download_vendor_archive_from L1642 and download_artifact_capped L2827. The last three have the same shape: loop { match once() { (Err, Some(after)) if attempt < attempts => backoff; attempt += 1 } }.

The archive download also sits behind a chain of one-line wrappers: download_vendor_archive → download_vendor_archive_retrying → download_vendor_archive_from(url, 1).

3. Two retry policies that disagree. One is ApiRetryPolicy, for the JSON calls. The other is VendorRetryPolicy, for the vendoring service and artifact downloads.

Executed on main @ 5ca5623, both tests pass:

• api::retry::tests::delay_caps_retry_after_and_jitters_backoff_into_the_upper_half pins delay(1, Some(120 s)) == None;
• api::client::vendor_retry_tests::delay_is_exponential_jittered_and_capped pins delay(1, Some(60 s)) == 4 s.

So a vendoring-service 429 that says Retry-After: 60 is retried 4 s later. The JSON path's own module doc says such a retry "would only be refused again". The JSON path and the vendor path are retry loops in the same file that give opposite answers here.

Proposed change

1. Use one serve_get_once(url, max_bytes, noun) for both GETs, and map its outcome once. Pick one error variant for a cap breach. ApiError::Other is what PatchApi callers see today, and the archive path does not retry a cap breach either, so the variant change is not observable in retries.
2. Use one generic retry_attempts(policy, key, once) helper for the three vendor loops, and delete download_vendor_archive, download_vendor_archive_retrying and vendor_download_retry.
3. Leave the policy values alone in this PR. The Retry-After-over-cap rule is a behavior change; propose it in the PR description, or file it as a follow-up.

Size

1 file (api/client.rs), about −150 lines.

Acceptance criteria

• Behavior is unchanged apart from the unified error text and variant named in step 1. Update the tests that pin the old text in the same PR.
• vendor_retry_tests::*, trait_download_enforces_the_callers_byte_cap, download_artifact_*, first_attempt_settles_or_defers_like_download_artifact, resuming_a_deferral_spends_one_budget_not_two, tests/api_retry_e2e.rs and tests/api_timeout_e2e.rs pass.
• download_artifact_first_attempt / download_artifact_resuming still spend one budget across a split.