GeezerVison is showing github.com — Honor HTTP-date Retry-After on vendor-service retries through api::retry (#677) by mikolalysenko · Pull Request #889 · SocketDev/socket-patch
Conversation
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
- New
ApiClient::vendor_retry_hint(status, headers): retryable status → parse_retry_after(headers, hooks.now_unix_secs()). It replaces the three vendor_status_retryable(..).then(|| retry_after_secs(..)) sites.
vendor_backoff(key, retry, retry_after) takes jitter from retry_jitter(hooks.jitter_seed, key, retry), keyed by the archive or artifact URL, or by a constant key for the reference POST. It waits on hooks.sleep, which is tokio's sleep by default. VendorRetryPolicy::delay still maps the sample onto ±25% and caps at max_delay.
- Deleted:
retry_after_secs (delta-seconds only) and jitter_sample() (unseeded RandomState).
- Ported from Bound registry downloads by ApiTimeouts instead of a 60 s total deadline (#872) #876 (commit
28d4d52, cherry-picked unchanged): production_digests_go_through_the_helpers fails on main because Full Gradle support in agent, hosted and vendored modes #646 added inline JVM digests. This is the same change Bound registry downloads by ApiTimeouts instead of a 60 s total deadline (#872) #876 carries, so it becomes a no-op once that PR lands.
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 aligns with existing api::retry semantics and is covered by new tests.
Overview
Vendor-service retry backoff now goes through the shared api::retry helpers instead of local copies in client.rs.
Retry-After is parsed with parse_retry_after (delta-seconds and HTTP-date), using RetryHooks::now_unix_secs, so a date-based header is honored up to max_delay. Retry hints are centralized in vendor_retry_hint for package-reference POST, archive GET, and capped artifact GET paths.
Backoff jitter comes from retry_jitter(hooks.jitter_seed, key, retry)—a fixed key for the reference POST, the URL for downloads—and waits use hooks.sleep rather than hard-coded tokio::time::sleep. The old retry_after_secs and unseeded jitter_sample helpers are removed.
New tests assert HTTP-date Retry-After (2 s wait) across all four retry loops, cap behavior, and deterministic jitter from the hook seed.
Reviewed by Cursor Bugbot for commit 5077cab. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
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
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Oct 7, 2026
* Start fix for #424
Assisted-by: Claude Code:claude-opus-5-5
* Add failing tests for apply failures in --json
scan --mode agent --json and get --json report a failed nested apply
as failed: 0 with the patch listed as added and no error text. These
tests pin the expected envelope: the patch record carries
action: failed, errorCode and error, and failed counts it.
Refs #424
Assisted-by: Claude Code:claude-opus-5-5
* Report apply failures in scan/get --json
When scan --mode agent or get downloads a patch and the in-place apply
then fails, the --json output said failed: 0, listed the patch as
added and carried no error, so automation reading the JSON could not
tell what went wrong. Only the exit code and status hinted at it.
The nested apply now hands its failures back to the caller instead of
just a pass/fail flag. Each patch that failed to apply is reported as
action: failed with the same errorCode/error pair that apply --json
prints (apply_failed or package_not_installed), failed counts it, and
applied counts only patches that really applied. A failure that no
single patch explains (unreadable manifest, yarn PnP refusal, missing
patch sources) is reported as a top-level errorCode/error.
Fixes #424
Assisted-by: Claude Code:claude-opus-5-5
* Keep uninstalled patches as warnings in --json
When one patch fails to apply, apply only warns about other patches
that have no installed copy. The JSON report now matches that: those
patches are reported as package_not_installed failures only when
nothing else failed the run. Adds unit tests for the failure
collection.
Refs #424
Assisted-by: Claude Code:claude-opus-5-5
* Check composer/gem docker sync via the manifest
The composer and gem docker e2e scripts checked that scan's JSON said
"action": "added". In these fixtures scan's own in-place apply fails
(the later apply --force patches the file), and scan --json now
reports that failure on the patch record (#424). So "added" was only
there because of the bug. Check instead that the patch was recorded in
.socket/manifest.json, which is what "synced" means here.
Refs #424
Assisted-by: Claude Code:claude-opus-5-5
* Count only patches apply really applied
The --json apply failure report could blame the wrong patch and miscount
applied:
- a failure on one PyPI release variant was pinned on a selected
sibling variant that applied fine, via a base-purl fallback;
- applied was "selected minus failed", so a selected patch that was
never installed (only a warning next to a real failure) still counted
as applied;
- get <uuid> zeroed applied whenever any other manifest patch failed,
and its extra failure records had no uuid.
The nested apply now also reports which package keys it patched, and the
envelope counts applied from that. A failure only marks records it
covers: the same purl, or an unqualified key covering its variants.
Refs #424
Assisted-by: Claude Code:claude-opus-5-5
* Add the Gradle and Maven inline digests to the pending list
The digest guard test (#865) fails on main. Gradle support landed with
inline sha256/sha1 computations in crawlers/gradle_cache.rs,
patch/jvm_jar.rs and patch/sidecars/maven.rs, and the guard's pending
list doesn't name them. List them as pending so CI is green until they
move onto the utils::digest helpers. Open PRs #876 and #889 add only
gradle_cache.rs.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HjNH36TbmyXCpJPw3EyBZB
---------
Co-authored-by: Claude <[email protected]>
Main landed the #878/#876 digest-helper change for jvm_jar.rs and the
inline-digest ratchet in utils/digest.rs, so take main's version of both
files and drop this branch's ported copy; the PR now only changes the
vendor retry path in api/client.rs.
Co-Authored-By: Claude <[email protected]>
gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs no longer compute
digests inline on main (they go through utils::digest), so
production_digests_go_through_the_helpers fails on the stale list. The
test's own doc says to drop a file once it moves onto the helpers.
Co-Authored-By: Claude <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 7, 2026
Mikola Lysenko (mikolalysenko)
pushed a commit
that referenced
this pull request
Oct 7, 2026
main is red on production_digests_go_through_the_helpers: #690
moved gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs onto the
utils::digest helpers, but PENDING_INLINE_DIGESTS still lists them,
and the ratchet fails on a stale entry. Same three-line change as
#889 and #980; it no-ops once main carries it.
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 5077cab. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 7, 2026
|
[agent] Ready for review at 5077cab (merged as 835601b while CI was finishing).
- Merged origin/main into the branch after Fix main CI red on stale digest pending-list entries #1016 landed. The merge was clean: this branch's
PENDING_INLINE_DIGESTS change was identical to Fix main CI red on stale digest pending-list entries #1016's, so the diff vs main is now only api/client.rs.
- Local checks:
production_digests_go_through_the_helpers and the 128 api::client tests pass, and rustfmt is clean. Local cargo clippy --workspace --all-features -D warnings on macOS stopped on an unused unix_default variable in crawlers/python_crawler.rs. This PR doesn't touch that file, and the CI clippy job passed.
- CI on 5077cab: 232 pass, 4 skipped, 0 failing.
- Bugbot: success on 5077cab, no open threads.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
arch-refactor
PR opened by the scheduled architecture refactor routine
refactor
Structural change: duplicated code or logic, missing abstraction, layering, dead code
Milestone
No milestone
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.
|