Skip to content

Honor HTTP-date Retry-After on vendor-service retries through api::retry (#677) - #889

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
arch-refactor/677-vendor-retry-after
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
arch-refactor/677-vendor-retry-after

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #677

Summary

Vendor-service retries (the package-reference POST, the archive GET, the capped artifact GET and a resumed deferred GET) now read Retry-After through api::retry::parse_retry_after and draw their jitter from the seeded api::retry::jitter_sample, on the client's RetryHooks. The private, weaker copies in api/client.rs are deleted.

Why

Register row C15 (child 1 of tracking #676), living document doc/07-infra-agent.md ("Three retry systems in one module"). Leverage: B 0 (no separate bug issue), U 1 (child 2 of #676, which merges the retry loops, needs this), D ≈2 (retry_after_secs, jitter_sample()), R L. Score ≈4. It was the best candidate that no open PR overlaps.

What changed

Size (git diff --stat)

  • api/client.rs: production +36 / −36 (net 0 after rustfmt wrapping; the two helpers are gone), tests +173 / −2.
  • Port: production +10 / −22 across jvm_jar.rs and sidecars/maven.rs, plus 1 line in the test ratchet.

Behavior

  • A vendor 429/5xx whose Retry-After is an HTTP-date now waits until that date, still capped at max_delay (4 s by default). Before this, it fell back to the 400 ms → 4 s backoff.
  • Jitter is drawn from the per-process seed. The range is unchanged (±25%).
  • Nothing else changes: same retryable status set (vendor_status_retryable), same attempt counts and request sequences.

Test evidence

  • New every_vendor_retry_loop_honors_an_http_date_retry_after: all four former callers wait exactly 2 s on Retry-After: <date 2 s ahead>. New an_http_date_retry_after_is_capped_at_max_delay covers the cap. New vendor_backoff_jitter_replays_from_the_hook_seed: the same seed gives the same waits, another seed gives different ones, and every wait stays within ±25% of nominal.
  • Red→green: with the hint temporarily switched back to the old delta-seconds parse, the first two tests fail. With the shared parser, they pass.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test -p socket-patch-core --lib: 5249 passed. The 4 known root-only failures remain (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_…).
  • cargo test -p socket-patch-cli --all-features --test covgap_commands_vendor --test cli_scan_silent --test covgap_commands_get: all pass except 3 tests that need a read-only directory and fail when run as root in the sandbox (*_state_write_failure_* in covgap_commands_vendor; they chmod a directory). These pass in CI.

Risk

Low. One file of production code. The vendor retry suites (vendor_retry_tests, vendor_prefetch tests) pass with unchanged request sequences.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HegUKEf6caV6QASRt7z8iX


Note

Low Risk
Retry timing and jitter change for vendor HTTP calls only; behavior is tightened (HTTP-date support) with broad test coverage and unchanged retry status sets.

Overview
Vendor-service retries (package-reference POST, archive GET, capped artifact GET, and resumed deferred downloads) now use the shared api::retry path for Retry-After and backoff timing instead of local helpers.

Retry-After is parsed via parse_retry_after on the client’s RetryHooks clock, so HTTP-date values are honored (still capped at VendorRetryPolicy::max_delay), not only delta-seconds. Backoff waits use hooks.sleep; jitter comes from the seeded retry_jitter keyed by request (constant for the reference POST, URL for downloads). Local retry_after_secs and unseeded jitter_sample() are removed; hint logic is centralized in vendor_retry_hint.

JVM jar patching and Maven sidecars switch inline SHA-1/SHA-256 hex helpers to utils::digest::{sha1_hex_of, sha256_hex_of}, with a small test ratchet update. New wiremock tests cover HTTP-date Retry-After across all four retry loops, cap behavior, and deterministic jitter from the hook seed.

Reviewed by Cursor Bugbot for commit 1d752aa. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Oct 5, 2026
A vendor-service 429 or 503 that sent Retry-After as an HTTP-date
was retried on the 400 ms backoff, because the vendor path kept its
own delta-seconds-only parser. Vendor retries now read Retry-After
through api::retry::parse_retry_after on the client's RetryHooks
clock (still capped at VendorRetryPolicy::max_delay), and draw their
±25% jitter from the seeded api::retry::jitter_sample, keyed by
request and attempt, and wait on the hooks' sleep.

The private retry_after_secs and jitter_sample copies in client.rs
are deleted. New tests run the reference POST, the archive GET, the
capped artifact GET and a resumed deferred GET through the shared
parser, and pin the cap and the seeded jitter.

Assisted-by: Claude Code:claude-opus-5-5
#646 landed inline sha1/sha256 computations in patch/jvm_jar.rs,
patch/sidecars/maven.rs and crawlers/gradle_cache.rs after the
utils::digest ratchet, so production_digests_go_through_the_helpers
fails on main. jvm_jar's private sha1_hex/sha256_hex copies and the
Maven sidecar's inline sha1 now call the shared helpers; gradle_cache.rs,
which an open PR also edits, joins the pending list for now. Hashes are
byte-identical.

Ported from #876 so this PR's CI is not red on the base-red ratchet.
(cherry picked from commit 28d4d52)

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 5, 2026 20:21
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 5, 2026
Assisted-by: Claude Code:claude-opus-5-5

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1d752aa. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] CI on 1d752aa was hit by a runner loss, not by a failure in this PR. At about 20:45Z the runners received a shutdown signal (coverage log: "The runner has received a shutdown signal", exit 143, in the middle of e2e_vendor_pypi_build with every test passing up to that point). About 45 jobs across CI, npm, pnpm, Benchmarks and Audit GHA ended cancelled, and none ended with a test failure. I re-ran the cancelled jobs once in Audit GHA Workflows, Benchmarks, npm hosted/vendored compatibility and pnpm hosted compatibility. The main CI run is queued again. The PR #889 workflow run refuses a re-run (403 "cannot be retried"), so it needs the next push or a maintainer re-run. Locally: clippy is clean, and socket-patch-core --lib passes apart from the 4 known root-only tests. The digest ratchet that is red on main is ported here from #876. Handing this PR to the burn-down routine.


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 5, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 1d752aa (1d752aa49bd5f19f31428b849a7ded8392ab9db9).

  • CI: 402/402 workflow checks green on the head commit (6 skipped by matrix rule), after re-running jobs the GitHub Actions runner outage cancelled. No test failed. The only non-green entries are 2 CodeQL default-setup Analyze jobs that GitHub cancelled during the outage, and GitHub doesn't allow re-running them ("This workflow run cannot be retried").
  • Bugbot: reviewed 1d752aa with no new issues, and no review threads are open.
  • Mergeable against main, with no conflicts.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 6, 2026
No slot free (#876, #886, #889 ready); main and ranking unchanged.

Assisted-by: Claude Code:claude-opus-5-5

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vendor-service retries ignore an HTTP-date Retry-After: fold the vendor Retry-After parser and jitter onto api::retry

3 participants