Fix unbounded registry lookups in upstream restore (#1220)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1220

Summary

A rollback or remove of hosted pins, and the hosted → vendored takeover and eject, re-resolve each pin's upstream entry from its registry. With many pins, every lookup went out at once: 40 pins meant 40 requests in flight to registry.npmjs.org, PyPI, crates.io, the Go proxy or Packagist. The registry client has no 429 handling, so a rate-limited lookup got its pin refused. Under a tight RLIMIT_NOFILE the burst could also fail with EMFILE. Now at most registry_concurrency() lookups are in flight: 4, or 1 under a tight descriptor limit.

Root cause

The five restorers (npm/vlt, PyPI, cargo, Go, Composer) start every pin's lookup together with join_all. UpstreamClient sent each request on a plain registry client with no cap, and utils::concurrent::registry_concurrency() had no caller.

Fix

The cap lives in one place. UpstreamClient holds a tokio::sync::Semaphore sized by registry_concurrency(), and every request it sends takes a slot from send to the end of its body. That covers JSON GETs and downloads (crates index, Go zip/mod, nupkg, berry archive checks). The issue offered this or five ordered_concurrent call sites; I took the client-level one so nested lookups and any future fan-out are covered too. The join_all folds don't change, so restore output stays byte-identical.

Permits never nest. Only get_json_authorized and download take one, and neither calls another client method while holding it, so a cap of 1 can't deadlock.

The cap also covers the project's private registry or mirror and the patch service's upstream checksum metadata (npm_berry_checksum), because both go through this client. The field doc says so.

Tests (red → green)

Issue	Test	Before the fix	After
#1220: npm restore with 40 pins	patch::redirect::upstream::npm::tests::restore_lookups_never_exceed_the_registry_cap (a local server counts peak in-flight requests across keep-alive connections)	FAILED: peak 40 > cap 4	ok: peak ≤ 4, still concurrent
#1220: tight fd limit (cap 1)	patch::redirect::upstream::client::tests::a_cap_of_one_sends_lookups_one_at_a_time	FAILED: left: 10, right: 1	ok: peak 1

The npm test drives fetch_dists_on, which every npm-family restorer uses. The other formats go through the same client methods, so the client-level test covers them. That's the option the issue's acceptance criteria allow.

Local checks

• cargo fmt --all -- --check: clean.
• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --all-features --no-fail-fast: 6391 passed in the lib, and every integration test passed. The 4 lib failures are the root-only chmod tests (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_rolls_back…). They also fail on main in this sandbox, which runs as uid 0, and other agent PRs list the same ones.
• CLI suites that run the restore: in_process_vendor_{bun,npm_v1,pnpm,pypi}_takeover and mode_migration_{npm,pypi,cargo,vlt,bun} all pass. in_process_redirect: 140 passed, and 3 failed. All three are chmod write-failure tests (partial_lockfile_write_failure…, redirect_json_mode_write_failures…, vlt::scan_redirect_vlt_heal_invalidation_failure_warns), the same root-only class. CI runs as non-root.
• No wrapper changes (npm/, pypi/, gem/): they only dispatch to the binary.

CI: green on 0ba9f7e (ci-ok success). Bugbot: no findings on 0ba9f7e.

/code-review high: findings not fixed here

• get_json_authorized reads JSON bodies with no size cap, and its errors carry the raw URL (userinfo included) where download redacts. Both predate this PR and are separate from the concurrency cap. Worth a follow-up issue.
• Concurrent cache misses for the same key each send a request. This also predates the PR. A duplicate now waits for a slot instead of bursting. A shared in-flight future per key would remove it, but that's a cache redesign outside this fix.
• Fixed: the flaky exact-peak assertion (now an overlap check with a longer delay), the requests() accessor, and the field doc's scope.

Per-issue checklist

• Upstream restore sends every public-registry lookup at once; the registry_concurrency cap has no caller #1220: 40-pin restore never exceeds registry_concurrency(): restore_lookups_never_exceed_the_registry_cap
• Upstream restore sends every public-registry lookup at once; the registry_concurrency cap has no caller #1220: tight fd limit gives peak 1: a_cap_of_one_sends_lookups_one_at_a_time
• Upstream restore sends every public-registry lookup at once; the registry_concurrency cap has no caller #1220: restore output unchanged: the existing upstream unit tests and the takeover/mode-migration suites pass
• Upstream restore sends every public-registry lookup at once; the registry_concurrency cap has no caller #1220: registry_concurrency has a production caller, and clippy shows no dead-code allowance

🤖 Generated with Claude Code

https://claude.ai/code/session_01DFfqPEjeRuqL9sPPtsLH2n

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 0ba9f7e. Configure here.}