Fix four vendored/hosted correctness bugs found while profiling

[Mikola Lysenko (mikolalysenko)]

Four correctness bugs recorded while profiling the vendored and hosted scan
paths, plus the adjudication of an adversarial review of the first pass at
them. This is not a performance change — nothing here is from §2.2 or §5
of the perf plan, and no code was restructured for speed.

Base: origin/main @ 1bfe5326. Every fix is its own commit, each one
reproduced on unmodified main (or, for the three regressions the first
pass introduced, A/B'd against main's own behavior) before being written.

What was wrong, and what changed

1. download.patches came out in HashMap order (acae5db3, b8f08b07)

Bug (BASE vendored JSON, §6). filter_to_installed_releases buckets
every release-variant purl (PyPI ?artifact_id=, RubyGems ?platform=,
Maven ?classifier=) into a HashMap keyed by base purl and then drains
it. That drain is the function's OUTPUT order, which is the order the
download loop walks, so download.patches, apply.patches and the
per-patch stderr lines came out in bucket order: two identical runs of the
same project emitted the same records in different orders.

Change. Sort the multi-variant bases before resolving them (stable
warnings) and sort the kept selection by (purl, uuid) before returning —
matching scan's packages and the agent flow's skip_records. The
--all-releases arm gets the same order so both arms share one contract,
and its docstring no longer claims a verbatim pass-through.

Tests. release_narrowing_keeps_a_stable_purl_order (the narrowing
itself) and download_patches_json_is_purl_ordered (the emitted array).

2. A hosted- or vendored-rewired requirements.txt dropped out of the inventory (11de805c, 45425953)

Bug (req-big, §6). The requirements.txt inventory read only exact ==
pins, so every line this CLI had already rewritten vanished from it. The
hosted wet re-run of a 391-pin requirements.txt reported
packagesWithPatches: 1 instead of 12, having "lost" the eleven packages it
had just wired. uv.lock keeps its [[package]] name/version through the
same rewrite; Pipfile.lock's reader already kept a Socket-written reference
as the package it replaces.

Change. Read both shapes we write, through the same
socket_reference_coords reader Pipfile.lock uses:

• hosted — the PEP 508 direct reference name @ <patch-server url>;
• vendored — a BARE path line,
  `./.socket/vendor/pypi// --hash=sha256:…

  socket-patch vendor: ==`, whose requirement name lives ONLY

  in that comment tag (utils::requirements::vendor_tag, the reader
  vex::discover::pypi_other already uses).

The first commit covered the hosted half only, while both docstrings claimed
the vendored half too; 45425953 closes it. The recovered entry is
discovery-only (resolved: None, integrity: None), so the PATCHED
artifact it points at can never be fetched as a pristine source. A line
whose reference contradicts its own name, and a user's own file/url/path
reference, still stay out.

Tests. The hosted shape and the vendored shape, each beside a == pin
and a user's own wheel, plus a round trip through each writer's own
formatter — the "the two grammars must agree" guard, once per writer.

3. One contentless patch view killed the whole vendored run (a34bb9d0, 7dbc90a1, ec2c1c7d, 5f598436)

Bug (JS-7, §6). The patch view serves blobContent only for the files a
patch CHANGES; a zero-delta file (beforeHash == afterHash) comes back with
hashes and no content. The in-memory vendor stager counted any such view as
a failed fetch, and a single failed fetch made the WHOLE run bail
no_local_source: exit 1, status: "error", zero events, and every other
package in the manifest left unvendored without a word. Reproduced against
the live API with pkg:npm/tar-fs@2.1.1, patch 8ff3e0c7-… — seven
zero-delta fixture files plus one changed package/index.js.

Change, in three parts.

• A zero-delta file needs no blob: the pristine copy already carries the
  patched bytes, which is exactly what verify_file_patch answers
  AlreadyPatched for. needs_blob() is now the one rule, shared by
  covered() (what to fetch) and the fetch loop (whether a view came back
  complete), so the two can never disagree. The §6 example package vendors
  now instead of failing on every run.
• A patch whose content genuinely cannot be obtained is an unsatisfiable
  PACKAGE, like vendor_fetch_failed or redirect_revert_failed: it gets
  its own failed/no_local_source event and the run carries on, in
  vendor, scan/get --mode vendored and repair alike.
• That event now carries the REAL reason — which file the view served with
  no content, a malformed or undecodable blob, or the fetch error — instead
  of the verbatim run-level string "patch artifacts unavailable (offline or
  download failure)", which named two causes that are both false for a view
  served 200. Every human channel in the stager is gated on !--json, so
  the per-package slot is the only explanation a --json consumer gets. The
  fetch loop also collects every contentless file instead of breaking at the
  first one, since patch.files is a HashMap.

The pre-event bail is deliberately kept for the case it was written for —
NOTHING in the manifest can be staged, so there are no per-package events to
report. See "Deliberately not changed" below.

Tests. a_view_whose_only_contentless_files_are_zero_delta_vendors
(red on unmodified main with status: "error"; asserts the vendored
tarball carries the changed file at its patched bytes AND the zero-delta
file at the bytes it always had), contentless_patch_view_fails_only_its_own_package,
every_patch_unstageable_keeps_the_run_level_error, and a mixed-selection
case for scan --mode vendored so the vendored scan's own
Ok(staging_errors || engine_errors) fold is covered, not just vendor's.

4. --vendor-source build downloaded a gem it could never use (25756641, 66a6e4b9)

Bug (GEM-4, §6). A bundler path source will not load without the
eval-able stub gemspec rubygems writes into <gem home>/specifications/
when the gem is INSTALLED, and a downloaded .gem carries its gemspec only
as YAML in metadata.gz — which is why the vendoring service converts it
and serves a separate gem-stub-gemspec artifact. So a local build can
never vendor a fetched gem. The auto-fetch rung downloaded the .gem anyway
and only then hit the backend's gem_spec_missing: a wasted registry round
trip on every run, ending in a message whose remedy named the mode that does
not work from there.

Change. Refuse before the fetch, with the same gem_spec_missing code
and a detail that names the real remedy (bundle install, or
--vendor-source=auto). Scoped to the purls a DOWNLOAD would actually
happen for, mirroring fetch_pristine_package's own fetchable filter: an
inventory entry whose integrity is not LockIntegrity::None, and no ledger
entry. The backend keeps its own refusal as the backstop for every other
route into it.

25756641 keyed the gate off mere resolvability
(lookup(inv, purl).is_some() || ledger_entry.is_some()) and so fired in
two cases where main never fetched anything and never failed; 66a6e4b9
is the scope correction. Both are measured A/B against origin/main:

fixture, --vendor-source build	origin/main	25756641	now
not installed, lock verifies it (bundler ≥ 2.6 CHECKSUMS)	1 registry GET, then failed/gem_spec_missing	0 GETs, failed/gem_spec_missing	same as 25756641
not installed, lock does NOT verify it (bundler < 2.6, no CHECKSUMS)	0 GETs, skipped/vendor_fetch_unverifiable + skipped/package_not_installed, exit 1	failed/gem_spec_missing	same as main
already vendored, fresh clone (rm -rf vendor/)	exit 0, success, skipped/vendor_fetched_missing + skipped/already_vendored	exit 1, partialFailure, failed/gem_spec_missing	same as main

The second row matters because CLI_CONTRACT documents it ("Entries the lock
cannot verify are NEVER fetched"), there was no download to save, and the
refusal's remedy could not help: the same fixture under --vendor-source auto still yields the skip pair, because the purl never reaches the gem
backend in any mode. The third is the already-vendored fresh-clone case the
ladder exists for — the gem backend's idempotent hot path re-confirms the
wired lock and needs no stub gemspec of its own.

Tests. build_mode_refuses_a_lockfile_only_gem_before_downloading_it,
auto_mode_still_fetches_a_lockfile_only_gem,
a_gem_that_resolves_from_nowhere_still_reports_not_installed, plus the two
scope guards an_unverifiable_lock_entry_keeps_the_documented_skip_pair and
an_already_vendored_gem_re_runs_green_on_a_fresh_clone (both green on
origin/main, red on 25756641).

Docs (ee41402d)

[Unreleased] → Fixed carries one entry per fix, each naming what a JSON
consumer sees change. CLI_CONTRACT's error-code table gains two facts that
were not readable anywhere:

• no_local_source is reported at TWO levels, and which one arrives depends
  on whether anything else in the manifest staged.
• gem_spec_missing was never in the table at all. Its row states the
  pre-fetch refusal, its exact scope, what is deliberately unaffected, and
  the vendor_fetched_missing warning a build-mode run no longer emits
  because it no longer downloads.

Deliberately not changed

• The whole-run no_local_source bail, including for a one-patch
  manifest. When NOTHING in the manifest can be staged there are no
  per-package events to report, and five suites pin that shape
  (scan_vendor_step_error_e2e, covgap_commands_scan_vendor_flow ×2,
  covgap_commands_get, coverage_fix_vendor_silent_mute_exit,
  in_process_vendor::offline_missing_source_fails). Making the
  per-package path unconditional would mean rewriting those assertions,
  which is a contract change and not this branch's job. The inconsistency is
  real but it is now documented rather than undecided, in both
  CLI_CONTRACT's error-code table and json_envelope's error_code doc.
  Note that the §6 example (tar-fs@2.1.1) no longer reaches this path at
  all: its view is complete once zero-delta files stop demanding content.
• repair's candidate partition has no mixed-manifest test. Its
  unavailable arm is exercised only through single-candidate fixtures.
  scan --mode vendored (the other untested caller) is covered here; the
  repair twin is left as a follow-up.
• Anything from perf-plan-phase3 §2.2 or §5. Note that the gem fix (4)
  is §2.2's X1b behavior for the gem half, which §5.4 holds behind a product
  decision — while GEM-4 is also named in §6, so the mandate is genuinely
  ambiguous. It is a separate commit (plus its separate scope correction) so
  it can be dropped or held for that sign-off without touching the other
  three fixes, and §2.2's stated reason for wanting sign-off — the
  vendor_fetched_missing warning disappearing from JSON — is now recorded
  in CHANGELOG.md and the CLI_CONTRACT error-code table.

Verification

• cargo clippy --workspace --all-targets -- -D warnings: clean.
• cargo test --workspace --no-fail-fast: 8773 passed, 0 failed, 0
  ignored across 254 suites (macOS, CARGO_INCREMENTAL=0). The 15
  docker_e2e_* suites self-skip without docker on this host and report
  zero tests.
• rustfmt run on touched files only — never cargo fmt --all, and note that
  rustfmt <crate>/src/vendor/mod.rs reformats that module's WHOLE tree,
  which is how two unrelated files briefly picked up churn that has been
  taken back out.

🤖 Generated with Claude Code