Fix yarn classic hosted scan lock rescans (#1423)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1423

Summary

A hosted scan of a yarn classic project got about 2.6x slower after #1328. Two
checks that #1328 added re-read the whole yarn.lock once for every patched
package. Each lock is now read once per scan. On the 3000-package / 60-patch
bench, yarn-classic/hosted and yarn-classic/rescan are both about 56%
faster than current main. Rescan is back to its pre-#1328 time.

Root cause

Both are O(patched packages × lock size):

1. Choosing which served tarballs to fetch
   (hosted::engine::yarn_classic_artifact_targets). For each candidate it ran
   classic_locks_registry_copy, which re-ran scan_blocks over the whole lock
   and split every key. It also ran two whole-lock substring searches per
   candidate: one for the pinned URL and one for the name.
2. The Yarn classic: a patch that adds a dependency to the package's own package.json leaves vendored yarn.lock with a dangling dependency (offline frozen install fails, lock churns), and hosted silently installs without it #591 served-dependency check in the yarn classic rewriter
   (classic_deps::unlocked_descriptors). For each patched package it split
   every key of the lock into a fresh BTreeSet, even when the package has no
   dependencies.

Changes

• select_classic_artifacts collects the lock's registry copies
  (classic_registry_copies) once, and only if some candidate still needs
  them. It also collects every URL the lock already pins with a # fragment
  (quoted_fragment_urls) in one lazy pass. The cheap "already pinned" test
  runs first, so a rescan whose grants all carry a sha1 never reads the blocks.
• The rewriter already splits each key once (heads). The Yarn classic: a patch that adds a dependency to the package's own package.json leaves vendored yarn.lock with a dangling dependency (offline frozen install fails, lock churns), and hosted silently installs without it #591 check now
  looks descriptors up in a HashSet<&str> built from those patterns the first
  time a package needs it (classic_deps::unlocked_descriptors_in). The
  vendored path keeps unlocked_descriptors, because it calls it once per
  package.
• Selection is unchanged: the same tarballs are fetched and the same
  descriptors are reported as unlocked.

The history has one reverted experiment: a commit that fetched the tarballs
concurrently, then its revert. Code review found that it could hold up to 32
whole tarballs in memory, and that retries would hit the server together.
The wheel fetch window was designed specifically to avoid both. It also gained
almost nothing on the bench, so it was backed out.

Bench (socket-patch-bench compare --runs 20, same 4 vCPU sandbox, bench fixture from #1422)

Against main (1779317):

scenario	base wall	head wall	Δ wall [95% CI]	requests
yarn-classic/hosted	472.5 ms	204.1 ms	-56.2% [-59.5, -53.9]	187
yarn-classic/rescan	320.8 ms	143.6 ms	-55.8% [-58.1, -53.7]	127
yarn-berry/hosted	225.1 ms	224.6 ms	-0.9% ≈	127
yarn-berry/rescan	187.6 ms	184.1 ms	-1.0% ≈	127

Against the commit before #1328 (f864148):

scenario	base wall	head wall	Δ wall [95% CI]	requests
yarn-classic/hosted	176.3 ms	208.9 ms	+17.7% [+13.5, +22.5]	127 → 187
yarn-classic/rescan	137.1 ms	145.4 ms	+3.5% [-5.4, +13.2] ≈	127

Most of the remaining hosted delta comes from the 60 served-tarball downloads
(plus gunzip and hashing) that #1328 added deliberately, for the #558 sha1 and
the #591 dependency check. #1423 itself calls those downloads intentional, so
this PR leaves them in place. Callgrind on the hosted scan: total instructions
dropped from 1.74G to about 1.1G. yarn_classic_artifact_targets went from
68% inclusive to under 4%, and unlocked_among went from 29% to 0%.

Tests

issue	test	red → green
#1423 (target selection)	hosted::engine::tests::classic_artifact_targets_read_the_lock_blocks_at_most_once counts block reads: 0 on a fully pinned rescan, exactly 1 for 200 unpinned deps	fails (left: 1, right: 0) if the cheap pinned filter moves behind the block read. An earlier timing version of this test took 58 s with per-dep scans and 1.7 s with the fix
#1423 (target selection)	quoted_fragment_urls_reads_each_resolved_url, classic_registry_copy_is_matched_by_block_name_and_source (updated to the set)	new / existing
#1423 (#591 check)	classic_deps::tests::unlocked_descriptors_in_matches_the_block_scan (the set form agrees with the block scan)	new

Commands run locally:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features: 14229 passed, 13 failed. All 13
  also fail on main in this sandbox and none touch yarn code. Ten are
  permission-failure tests that can't fail as root (chmod is ignored), for
  example covgap_commands_vendor::*_state_write_failure_* and
  vendor::pypi_poetry::tests::wire_write_failure_*. The others need
  patches-api.socket.dev, which the proxy blocks, for example
  e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed.
• scripts/yarn-classic-vex-matrix.sh 1.22.22 1.0.2 (real yarn,
  SOCKET_PATCH_YARN_E2E_REQUIRED=1): exit 0, every suite passed (110 PASS
  cells)
• cargo test -p socket-patch-cli --test scan --test hosted_memory_engine --test e2e_redirect_yarn_classic_build --test e2e_vendor_yarn_classic_build --test e2e_vendor_yarn_classic_dev_flow:
  all pass

Review notes (/code-review high, findings not fixed)

• A grant for a package missing from the lock still forces one block read.
  The read is bounded at once per lock. The alternative was a whole-lock
  substring search per dep, which is what this PR removes.
• yarn-berry's berry_lock_locks scans per dep too. That is a different
  backend and not part of this regression; the berry bench is unchanged. Left
  for a separate issue.

Follow-ups

• The 60 served-tarball GETs on a first hosted scan run serially. To make them
  concurrent, they need the wheel window's design: a small cap, a first-attempt
  window, then serial resumption.

🤖 Generated with Claude Code

https://claude.ai/code/session_018G3cdPFQuuBbZahpQSKpoB

---

Generated by Claude Code

[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 9d0c1bc. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review at head 9d0c1bc79c311694009d54f2e818b15c5836cd49.

• CI: 36/36 run checks green, 50 path-skipped, no failures (no main-wide failures).
• Bugbot: reviewed 9d0c1bc, no findings.
• Mergeable, no conflicts; does not touch CHANGELOG.md.

Slack announcement not sent this run (Slack send unavailable); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Removes the 2.6x slowdown in yarn classic hosted scans that #1328 introduced (#1423). Two checks that re-read the whole yarn.lock once per patched package now build their lookup set once per scan, and only when they need it: the registry (name, version) copies, the #-pinned URLs, and the #591 locked-descriptor set.

Risk: low. It refactors the code into precomputed sets and keeps the same predicates (split_key_patterns / classic_key_real_name / classic_copy_source). Scoped packages, multi-descriptor keys, npm: aliases, CRLF locks and the selection order behave the same as before. The one case that changes in theory is an artifact URL that itself contains #, and serve URLs never do. The vendored path is unchanged.

Look here

• engine.rs:1225-1252: select_classic_artifacts, the filter order and the lazy caches
• engine.rs:1260-1273: quoted_fragment_urls, which replaces the per-dep lock.contains("\"{url}#")
• engine.rs:1280-1291: classic_registry_copies
• redirect/mod.rs:4013-4019:`` the descriptor set, built lazily from the already-split heads
• classic_deps.rs:62-99:``` unlocked_descriptors_in

Verified

• Read the full diff against the old code, edge case by edge case.
• cargo test -p socket-patch-core --lib -- yarn hosted classic: 587 passed.
• A new test asserts the lock blocks are read 0 times when every dep is pinned and exactly once for 200+ deps.
• CHANGELOG.md is untouched and there's no debug code (the read counter is test-only).
• CI: ci-ok and clippy are green, 86/86 check runs are passing or skipped, and Bugbot passed. No unresolved threads.

Changes I made: none.

Open questions (non-blocking):

• The quoted_fragment_urls test could add a few cases: a closing quote right before a URL, a CRLF lock, and a URL in a comment line.
• (String, String) is allocated on each registry-copy lookup.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code