Skip to content

Fix open bun issues (#992, #861, #784, #764, #735, #635, #599, #578, #497, #443, #371) - #1009

Merged
Mikola Lysenko (mikolalysenko) merged 69 commits into
mainfrom
agent/fix-bun-open-issues
Oct 9, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 69 commits into
mainfrom
agent/fix-bun-open-issues

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

This fixes all 11 open pm:bun issues. None of them had a fix PR before this one. For each issue a regression test was written that fails on main, and real Bun releases (1.1.x–1.4.2, as each issue names) were used to check behavior. Every fix went through two independent reviews, and a completeness pass afterwards found gaps that the later commits close.

Per issue

#992: hosted revert writes "" into the registry slot. restore_bun_locks always wrote "". Bun releases before 1.3.7 read that as npmjs and ignore bunfig, so custom-registry projects broke. Restores now write the project registry's tarball URL, resolved in Bun's own order:

  • scope, then bunfig [install] registry, then .npmrc, then env, skipping keys that aren't http(s) URLs;
  • "" stays only for npmjs and its aliases;
  • the registry's credentials (token or basic auth) are sent, so private scoped registries work;
  • userinfo in a registry URL (e.g. https://u:${TOKEN}@host/) is moved onto the request's Authorization header and stripped from the base, so it never reaches bun.lock/bun.lockb or warnings (security review finding on aa03736, fixed in 1305ecb; test bun_registry_userinfo_goes_on_the_request_not_the_base).

Covered end to end for rollback, remove, takeover followed by vendor --revert, and scoped registries.

#861: duplicate bun.lockb records point at one vendored tarball, causing EEXIST. On a vendored re-run, a late nested registry duplicate is now folded into the vendored record the way Bun's own hoister places it. The hoister port follows Bun's Tree.zig/Tree.rs, including unresolved optional peers like ws's bufferutil. Where the fold can't be modeled safely, the run warns with vendor_bun_lockb_duplicate_records. Covered by 16 real-Bun 1.3.9/1.4.2 fixtures and a Bun-gated e2e test.

#784: vendored bun.lockb migrated to bun.lock. Revert and rollback now restore the migrated text lock. A superseding re-vendor keeps the pre-vendor original. A same-uuid re-pin, which leaves an entry with both binary- and text-lock records, reverts cleanly. Covered by a real-Bun e2e test for migrate, then re-run/repair, then supersede, then revert.

#764: the hoisted linker keeps patched bytes after a revert. Bun doesn't re-extract a package when its lock entry moves back to the registry copy of the same name@version. Rollback and vendor --revert now emit vendor_bun_reinstall_required / redirect_bun_reinstall_required, which name bun install --force. The advisory is emitted only when an entry was actually restored, and is deduplicated across legs. The GC path forwards only the two reinstall advisories. A --dry-run preview gives the same advisory, so its reinstall note names bun install --force too (Bugbot finding on 5f75908, fixed in 8ddcb3f).

#735: a dangling bun.lock symlink. One stat-based predicate decides which lock is live. Only ENOENT falls back to bun.lockb, matching Bun; ELOOP, ENOTDIR and EACCES keep the text lock shadowing. Since main's #1050 moved the vendored in-use check into discovery, the GC's in-use verdict reaches this predicate through Bun discovery (bun_text_lock_drives). The #735 GC regression test now runs through Discovery::vendor_entry_in_use.

#635: Bun's global store (install.globalStore). Agent apply and rollback now refuse copies in the global store, because those are shared with other projects. The crawl follows the store's links, and vex judges the store copy. Apply's note says why.

#599: isolated linker with orphaned node_modules/.bun entries. Liveness is now judged by what is actually linked: importers, .bun/node_modules, workspace members from bun.lock and package.json, and links through the global store. This filter applies only to the scan and to vex. Apply, rollback and remove can still reach a patched orphan.

#578: perf regression on bun/hosted. The cheap spec match now runs before is_bundled_entry's JSON parse, in rewrite_bun_lock, classify_rewritable and bundled_matches. Bench, same machine, paired, 25 pairs:

scenario main this PR Δ
bun/hosted 165.5 ms 119.1 ms −28.2% [−30.0, −26.2]
bun/rescan 164.5 ms 116.4 ms −28.3% [−31.2, −27.1]

That is within about 5% of cbf1f748, the commit before #472. Requests are unchanged at 127.

#497: URL, file:, git and folder copies. Hosted and vendored runs warn (redirect_bun_non_registry_entry_skipped / vendor_non_registry_entry_skipped), and vex no longer attests while such a copy of the package is in the lock. That covers bun.lock and bun.lockb, including copies whose version the lock doesn't record (pkg.tgz, codeload, git), which vex withholds with patched_ref_unattributable.

#443: relocated Bun global dirs. The global dir now comes from bun pm ls -g, not <bin>/... If that fails, it falls back to Bun's own env order (BUN_INSTALL_GLOBAL_DIR, BUN_INSTALL, XDG_CACHE_HOME, HOME). When the dir can't be determined, the run warns instead of dropping it silently. bunfig globalDir is deliberately not honored, because real Bun 1.4.2 ignores it.

#371: default trust is dropped after a rewire. Hosted and vendored rewires of a package on Bun's built-in default-trusted list now warn (*_default_trust_lost). Adding the package to trustedDependencies automatically would grant trust the user never gave. The warning isn't gated on the Bun version, because nothing in a project reliably pins the Bun that installs it: 1.3.9 and 1.4.2 ignore packageManager, engines.bun and .bun-version.

Merge with main (e647108)

main's #1050 (one discovery verdict for vendored liveness) and #1045 (PurlKey) conflicted with this branch. The branch's npm_flavor::vendored_entry_in_use was dropped in favor of main's Discovery::vendor_entry_in_use. The #735 test was kept and pointed at the new API, using a purl-matched minimist entry. The Bun reinstall dedupe in vendor.rs now keys on PurlKey. Local results on e647108:

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test --workspace --all-features --no-fail-fast: 237 test binaries pass. 13 tests fail only because of the sandbox. 12 of them need a read-only directory to block writes, and the sandbox runs as root, which bypasses that (e.g. covgap_commands_vendor::*_state_write_failure_*, copy_tree::relax_loop_must_not_traverse_symlinked_root). The other one, pipenv_hosted_to_vendored_names_the_unpatched_requirements, needs live pypi.org. None of them touch Bun or the merged code.
  • cargo fmt --check reports diffs only in files this branch and the merge don't touch. Those come from main, and CI has no fmt gate.

Known limits / follow-ups

Fixes #992
Fixes #861
Fixes #784
Fixes #764
Fixes #735
Fixes #635
Fixes #599
Fixes #578
Fixes #497
Fixes #443
Fixes #371

Scan performance (fd75f30)

scan performance failed on 3a45799: bun-isolated/rescan +10.2% wall (bun-isolated/hosted +8.4%). The cost was the #599 orphan walk over node_modules/.bun. strace -c shows the same syscalls as the base, so the reads were not the problem. The time went to two other things:

  • The walk tracked entries by name, hashing and cloning an OsString per entry several times. It now tracks them by index into the store's entry list.
  • It ran one parallel pass per link-graph frontier, plus one more for the scan. Each pass wakes and parks the walk pool for little work (about +12 ms user+sys). The quick walk now reads every entry's listing in one pass, the same reads the scan always made, and follows links in memory. The scan reuses those listings without another pass.

What counts as live is unchanged, and the full re-walk is unchanged. Measured locally against the PR base on the bench fixture: 579M -> 556M instructions (base 524M). The paired socket-patch-bench compare -f '^bun' now gives bun-isolated/hosted +6.3% and /rescan +8.7% wall, +5% CPU (it was +11-14%). Tests: cargo clippy --workspace --all-features -D warnings is clean. cargo test -p socket-patch-core --lib gives 5885 passed and 4 failed; the 4 are permission tests that fail the same way without this change when run as root. The CLI bun suites (e2e_bun_lockb, mode_migration_bun, in_process_vendor_bun_takeover, vendor_eject_bun_lockb) give 100 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NfrapG9DFf76Fy96mocdRS


Note

High Risk
Changes Bun lockfile rewrite/restore, registry credential handling, and post-revert install behavior across many Bun versions—mistakes can break installs or leave unpatched copies in node_modules.

Overview
This PR closes a batch of Bun edge cases around hosted/vendored lock rewrites, upstream restore, and what still runs in node_modules after unwind.

Lock restore & registry semantics (#992, #784): Restoring bun.lock / bun.lockb to upstream no longer always writes "" in the registry slot—it follows Bun’s configured registry and tarball URL (with credential handling kept off the lock bytes). Contract and failure text now tell users to run bun install --force after revert when the hoisted linker would keep patched bytes (#764).

bun.lockb vendoring (#861, #784): Duplicate binary records for the same name@version can be folded via Bun’s hoister when safe; otherwise vendor_bun_lockb_duplicate_records. Migrated text locks participate in revert/rollback paths.

Discovery, apply, and attest (#497, #371, #635, #599, #735, #443): User URL/file:/git copies are skipped or warned and withheld from VEX; default-trusted packages warn after rewire; global-store layouts are refused on agent apply with a clearer note; isolated-linker liveness and dangling bun.lock symlinks are handled; global install dir resolution uses bun pm ls -g with fallbacks.

CLI polish: Rollback/GC/hosted unwind emit redirect_bun_reinstall_required / vendor_bun_reinstall_required and qualify generic reinstall notes; hosted “missing entry” hints ignore redirect_bun_default_trust_lost. CI adds Bun 1.3.14 / 1.4.2 e2e filters; fixtures get .gitattributes for byte-exact Bun locks.

Reviewed by Cursor Bugbot for commit fd75f30. Configure here.


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 7, 2026
Global mode asked `bun pm bin -g` for Bun's global bin dir and assumed
the packages sit at `<bin>/../install/global/node_modules`. Bun moves
its bin dir (BUN_INSTALL_BIN, bunfig globalBinDir) and its global dir
(BUN_INSTALL_GLOBAL_DIR) independently, so with either one set the
guessed dir did not exist, get_global_node_modules_paths dropped it
silently, and `scan -g` reported a clean, empty result while every Bun
global package stayed unpatched.

Ask Bun where the packages are instead: the first line of
`bun pm ls -g` is `<globalDir> node_modules (N[ installed])` on every
Bun from 1.0 to 1.4. When bun can't be asked (not on PATH, timed out,
unreadable answer), follow Bun's own openGlobalDir resolution from the
environment: BUN_INSTALL_GLOBAL_DIR, then $BUN_INSTALL/install/global,
then .bun/install/global under XDG_CACHE_HOME or the home dir.

Verified against real Bun 1.1.45, 1.2.23, 1.3.14 and 1.4.2 on macOS
with BUN_INSTALL_BIN and BUN_INSTALL_GLOBAL_DIR set: `scan -g` now
sends the globally installed package to the patch API in every cell.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Seven call sites each asked "is bun.lock the live lock?" with their own
presence check. The lock inventory, the vendored GC probe, the wired
integrity probe and VEX lockfile discovery used lstat, so a dangling
bun.lock symlink counted as present; hosted and vendored routing used
Path::exists, which follows links. Bun opens bun.lock through symlinks
and falls back to bun.lockb only when the open finds nothing: with Bun
1.2.23 and 1.3.14, `bun install --frozen-lockfile` installs from
bun.lockb beside a dangling bun.lock link. On that tree the inventory
returned no packages and no diagnostic, the GC probe never decided, and
VEX discovery saw no refs, while vendored and hosted mode wired
bun.lockb.

Add lock_inventory::bun_text_lock_drives (stat through links, over a
ProjectView) and bun_binary_lock_drives, and route the inventory, the
live-sibling fallback, wired_vendor_integrity, vendored_entry_in_use,
vendored routing and revert, bun_workspace, the hosted engine, CLI
repair's reference scan and VEX discovery through them. Remove
bun_text_lock_present{,_in} and hosted::engine::bun_lock_present.

A bun.lock directory is not treated like a dangling link: Bun opens it,
fails to read it and ignores both locks ("warn: Ignoring lockfile"), so
bun.lockb is not live. The text lock keeps winning there and its guarded
read refuses, which is what every path already did.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bun_text_lock_drives counted any stat error on bun.lock as "absent",
so bun.lockb was picked as the live lock. Bun's loadFromDir falls back
to bun.lockb only when opening bun.lock fails with ENOENT. Any other
open error (ELOOP from a self-referencing link, ENOTDIR from a link
through a regular file, EACCES from a link into an unreadable
directory) makes Bun print "Ignoring lockfile" and install from
neither lock. Verified with Bun 1.3.14 in a scratch project: a
bun.lock -> bun.lock self-link plus bun.lockb gives "ELOOP: failed to
open lockfile" and a frozen-lockfile refusal, with nothing installed.
The inventory, the GC probe and VEX discovery therefore read refs
from a bun.lockb Bun would not install from. Before #735 these sites
used lstat, which chose the text lock here.

Only io::ErrorKind::NotFound now means absent. Every other error keeps
the text lock shadowing, and its guarded read then refuses or
diagnoses. A unix control test covers an ELOOP self-link and an
ENOTDIR link through a file. It fails on the previous predicate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Hosted rollback, remove and the hosted -> vendored takeover rebuild a
bun.lock registry 4-tuple with an empty registry slot. Bun writes "" only
for a package from registry.npmjs.org and the full tarball URL for any
other registry, and Bun 1.1.39 through 1.3.6 read "" as npmjs whatever
bunfig.toml says. So on a project with a private registry or mirror the
restored lock fetched from npmjs on a cold frozen install: a 404 for a
private package, a silent bypass of the mirror for a public one. The
bun.lockb takeover restore had the same gap: it wrote the default
registry's dist.tarball into the record, which Bun fetches from as is.

Both restores now read the registry Bun resolves each package against
(a scope's .npmrc @scope:registry or bunfig [install.scopes] entry, else
BUN_CONFIG_REGISTRY / NPM_CONFIG_REGISTRY, the .npmrc registry, then
bunfig [install] registry, in Bun's order), fetch the version document
from it through fetch_dists_on as the berry and vlt restores do (#918),
and record its dist.tarball. When that registry can't be read, the
default registry's conventional URL is re-based on it (with the existing
upstream_registry_fallback warning). The text slot stays "" for npmjs,
matching Bun's own prefix test, so projects without a custom registry
restore byte for byte as before.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review follow-ups to the Bun registry-slot restore:

- An npmjs alias as the project registry (registry.yarnpkg.com, or
  http://registry.npmjs.org) had the default document's conventional
  URL re-based onto the alias, so the bun.lock slot carried the alias
  URL where Bun writes "" (its writer prefix-tests the manifest's
  dist.tarball, which those registries advertise on npmjs). Only a
  fallback from a non-default registry (non_default_registry is Some)
  is re-based now, and the slot short-circuits on any npmjs host.
- The env registry lookup took the first non-empty of
  BUN_CONFIG_REGISTRY / NPM_CONFIG_REGISTRY / npm_config_registry and
  then dropped it if it was not a URL; Bun skips a non-http(s) key and
  reads the next one. bun_env_registry now filters inside the search.
- A bunfig [install.scopes] entry with no url (token only) takes the
  configured default registry (.npmrc / bunfig), not the environment,
  matching Bun's `registry.url = base.url`.
- Unit tests no longer read the ambient env registry, and the CLI
  subprocess harnesses strip it, so a shell where npm exports
  npm_config_registry no longer turns the Bun restore tests red.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bun 1.3.14 added a machine-wide store for the isolated linker
(`[install] globalStore = true` or BUN_INSTALL_GLOBAL_STORE=1). Each
node_modules/.bun/<entry> is then a symlink into
<cache>/links/<entry>-<hash>, and every project using that cache links
to the same directory.

socket-patch did not know this layout. patch/shared_store.rs only
recognized pnpm's global virtual store and PDM's cache, so agent apply
wrote through the link into the shared directory and patched every other
project, and a rollback in one project silently unpatched them all. And
the .bun store walk kept only entries that are real directories, so
every globalStore entry was skipped: agent mode reported transitive
dependencies as package_not_installed, and hosted vex attested a pinned
transitive package as not_affected without reading the unpatched copy
that is actually installed.

Add a BunGlobalStore shared-store kind, recognized on the real path
<cache>/links/<name>@<...>-<hex>/node_modules/<name>, so agent apply and
rollback refuse it like the pnpm and PDM stores (#361), naming the store
and how to get a private copy. Have the .bun walk also follow entries
that link into a Bun global store entry, so scan, apply's resolver, the
peer-copy fan-out and vex see this project's copies there. Other links
in .bun are still skipped.

Tested with real Bun 1.3.14: two projects sharing a cache, agent apply
in one now refuses both the direct and the transitive package and the
other project's bytes stay untouched.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Human-mode apply printed "bun layout detected. Copy-on-write will keep
~/.bun/install/cache/ untouched." for every Bun project. With Bun's global
store (install.globalStore / BUN_INSTALL_GLOBAL_STORE, Bun >= 1.3.14) the
installed package dirs are the cache's shared <cache>/links entries, and
apply now refuses them, so the note said the opposite of what happens.

Add crawlers::bun_uses_global_store, which reads the installed tree (a
node_modules/.bun entry linking into <cache>/links/<entry>-<hash>) rather
than bunfig or the env, and print a note naming the shared store and the
globalStore = false remedy in that case. Other Bun projects keep the old
note.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Vendored mode records bun.lockb wiring as binary package snapshots. Bun's
own `bun install --save-text-lockfile` deletes bun.lockb and carries the
vendored tuples into bun.lock, but every unwind path still treated the
entry as binary-only:

- bun_binary::revert failed hard on the missing bun.lockb, so
  `vendor --revert`, `rollback` and the hosted takeover could not unwind
  the vendored wiring.
- carry_forward_wiring only carried an original across a matching
  surface, so a superseding re-vendor on the migrated lock (which records
  no original for our own stale tuple) lost the registry original, and
  revert then left the project patched while reporting success.

When bun.lockb is gone but bun.lock exists, revert now restores each
recorded binary package in bun.lock: every tuple still pointing into the
entry's vendor artifact is rewritten to the registry 4-tuple Bun writes
for the pristine binary record (key, indent, dependency object and comma
kept verbatim; an empty registry slot under https://registry.npmjs.org
and the tarball URL otherwise, as Bun's text writer does). Records whose
originals disagree are left alone as drift. A re-vendor's bun.lock record
whose predecessor only has bun.lockb records gets that registry tuple as
its pre-vendor original.

Real-Bun fixtures (1.2.23 lockb migrated by 1.2.23 and 1.4.2, pristine
and vendored) pin the exact restored bytes; a Bun-gated e2e covers
vendor --revert, rollback and the hosted takeover, and joins the 1.4.2
CI leg.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The migrated-lock revert tests compare restored bun.lock bytes against
real Bun output; a CRLF checkout on Windows would break that comparison.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A vendored re-run on a bun.lock that Bun migrated from bun.lockb re-pins
our own tuple at the same uuid whenever the committed artifact is missing
(fresh clone, `repair` rebuild) or the tuple's digest differs. The new
entry carries a `bun_lock_package` record (its original rebuilt from the
binary snapshot), and the same-uuid wiring union in carry_forward_wiring
also keeps the predecessor's `bun_lockb_package` records, since no surface
matches across kinds. revert_bun_opts routes any entry with binary records
to bun_binary::revert, which rejected the text record as "unexpected
binary wiring file or kind": that counted as drift, so revert exited 0
with bun.lock still vendored and the artifact kept.

In the migrated mode, bun_binary::revert now restores `bun_lock_package`
records in bun.lock through the text revert's own per-record restore
(which also converges silently when a binary record already restored the
line). The binary and workspace-mirror records stay in the entry so mirror
tarballs are still cleaned up. A regression test re-pins the same uuid on
both migrated fixtures (1.2.23 digest-less, 1.4.2 with a changed digest)
and checks the revert restores the pristine bun.lock with no warnings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
After a workspace bun.lockb is vendored, a new dependent of the patched
name@version (a member added later, or `bun add` in a member) makes Bun
write a second, nested registry record of it, because the hoisted record
is now a local tarball. The vendored re-run rewired every matching record
to the same .socket/vendor tarball, leaving two package records with one
resolution - a shape Bun's own writer never produces. On the isolated
linker both map to one node_modules/.bun store directory, so cold frozen
installs failed intermittently with EEXIST (reproduced on Bun 1.3.9 and
1.4.2: 3/8 fresh installs on 1.4.2), while scan/vendor reported success.

The re-run now folds such duplicates into one kept record (the one at
the target tarball, else ours, else the first), the way Bun's re-save
would: their dependency edges resolve to the kept record, the duplicate
rows leave every package column with later IDs (resolution buffer and
meta.id) renumbered, and the hoisting trees are rewritten as Bun
re-hoists them - an edge whose nearest same-name ancestor now holds the
same package is deduplicated and an emptied tree dropped. Bun 1.3.x
re-hoists a frozen binary lock and refuses one whose trees differ
(Lockfile.eql), so merely re-pointing edges, or leaving an orphan record
(which 1.4.2 refuses), is not enough. The buffers are re-laid with Bun's
eight-byte data alignment and the metadata hash updated; the same
workspace normalization as any record edit is applied.

The tree rewrite is exact only where hoisting is predictable, so the
merge is limited to records without dependencies of their own, no peer
edge to them and no bundled edge in the lock; otherwise the records are
rewired as before with a new vendor_bun_lockb_duplicate_records warning.

Tests: real-Bun e2e (new member and `bun add` triggers, 6 cold frozen
installs each, idempotent re-run, revert) on 1.3.9/1.3.14/1.4.2, run in
CI on the 1.3.14 and 1.4.2 legs; hermetic codec and vendor tests over
locks captured from Bun 1.3.9 and 1.4.2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The bun-lockb/late-dependent fixture added for the duplicate-record fix
is walked by the vex discovery golden corpus, but bun-lockb.json had no
entry for it, so committed_fixture_corpus_matches_golden failed.
Regenerated with SOCKET_PATCH_UPDATE_GOLDEN=1; the only change is the
new empty entry for that fixture.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The vex discovery golden corpus walks every bun-lockb fixture directory.
#861 added the golden entry for its late-dependent fixture, but the
bun-lockb/1.2.23-migrated fixture added by the #784 fix had no entry in
bun-lockb.json, so committed_fixture_corpus_matches_golden still failed
on the combined branch. Regenerated with SOCKET_PATCH_UPDATE_GOLDEN=1;
the only change is the new empty entry for that fixture.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Resolve conflicts with #1058 (hosted pin attribution through discovery):

- hosted/engine.rs: keep this branch's removal of bun_lock_present in
  favor of lock_inventory::bun_text_lock_drives; give the branch's
  default-trust test the new RewriteOptions fields.
- lock_inventory/bun.rs: DiskSnapshot's root is private on main, so
  bun_text_lock_drives reaches it through disk_root_reading([BUN_LOCK]),
  which also records the probe in the snapshot's read set (without it
  the gate's overlaid discovery was no longer view-only).
- CLI_CONTRACT.md: keep main's new attribution-gate paragraph and this
  branch's redirect_bun_non_registry_entry_skipped note.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@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.

Stale Bugbot comment from a previous run.

revert_bun_wiring returned success on a dry run before reading
bun.lock, so revert_bun_opts never saw vendor_lockfile_missing or
vendor_lock_entry_removed and a preview advised `bun install --force`
for a revert that restores nothing (the lock gone, or the entry
removed by `bun remove`).

The dry run now replays the lock in memory like the wet run, skipping
only the write and the artifact deletion, so its warnings and the
reinstall advisory match the wet run's.

Found by Bugbot on #1009.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSx4W4Qfb8hsBhw4NEvkv9
Comment thread crates/socket-patch-core/src/patch/redirect/upstream/npm.rs
The bench gate flagged bun-isolated/hosted and /rescan (+12-13% wall
in CI, +15% instructions under callgrind against main). Cut the
per-entry costs this branch added:

- user_tarball_version (#497): run the cheap leaf checks before the
  vendor-path parse, and reject an entry of another package by length
  and '@' before any string work; the hosted rewriter asks this of
  every lock entry per patch.
- loses_default_trust (#371): look the name up in a set built once
  instead of scanning the default trusted list per call (22k calls in
  the bench).
- live_bun_store_entries_sync (#599): look a guessed link target up
  before building its path, and only clone a target that is newly
  reached.

callgrind on the bun-isolated/hosted fixture: 768.7M -> 728.4M
instructions (main: 669.5M).

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSx4W4Qfb8hsBhw4NEvkv9
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

On 111b849, scan performance is red: bun-isolated/hosted came in at +12.0% wall and bun-isolated/rescan at +13.4%, with CPU up about 10%. Every other scenario, including plain bun, is within noise. This is this PR's own regression. The benchmark passed on 46a0dd1 and is red against the current main.

I reproduced it locally and profiled it with callgrind on the bun-isolated/hosted fixture:

build instructions
main 669.5M
111b849 768.7M (+15%)
3a45799 728.4M (+8.8%)

The extra cost came from three places:

3a45799 trims all three: cheap checks run first, the trusted list is built once as a set, and the walk now looks up guessed targets without allocating or cloning. That cuts about 40M instructions. Locally the gate is still red on wall time, at +11–15%. The rest is the #599 walk's frontier-by-frontier .bun traversal: the scan's store listing now waits on several parallel rounds instead of one.

Getting under +10% needs a design call on #599, so I've asked the owner. The options are: skip the walk when every .bun entry is listed in bun.lock (orphans are by definition absent from the lock), restructure the walk into a single parallel pass, or apply performance-regression-accepted.


Generated by Claude Code

The scan-performance gate failed on bun-isolated/rescan (+10.2% wall
in CI) and bun-isolated/hosted (+8.4%). The cause was the #599
orphan walk over node_modules/.bun, not its reads: the syscall
counts match main. Two things cost the time:

- The walk tracked entries by name. It hashed and cloned an OsString
  for each of the 3000 entries, several times over. It now tracks
  them by index into the store's entry list.
- It read one parallel batch per link-graph frontier, plus another
  batch for the scan. Each batch wakes and parks the walk pool for
  little work, which cost about 12 ms of user and system time. The
  quick walk now reads every entry's listing in one parallel pass,
  the same reads the scan always made, and follows the links in
  memory. The scan reuses those listings without another pass.

What counts as live is unchanged. The full re-walk that runs after
a quick walk leaves entries unreached is still read as before.

bun-isolated/rescan on the bench fixture, against the PR base:
instructions 579M -> 556M (base 524M); paired local compare now
+6.3% hosted and +8.7% rescan wall, +5% CPU (was +11-14%).

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@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 fd75f30. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Needs a maintainer decision (burn-down agent, head fd75f30d). CI is green (453 success, 15 skipped) and every other review thread is resolved. One thing blocks the label: the open Bugbot security thread on crates/socket-patch-core/src/patch/redirect/upstream/npm.rs:1156.

Question: when the Bun hosted restore (rollback / remove / vendor, dry runs included) reads bunfig.toml / .npmrc, which $VAR references may it expand into the registry credentials it sends?

  1. None: send only literal tokens. Safest; private registries configured with $NPM_TOKEN fall back to the anonymous read main does today, with an upstream_registry_fallback warning.
  2. An allowlist of conventional token variables (NPM_TOKEN, NODE_AUTH_TOKEN, BUN_AUTH_TOKEN). This blocks $GITHUB_TOKEN and other arbitrary secrets and keeps the common setups working.
  3. Full Bun parity, as the PR does now, documented.

Whichever you pick, an expanded value that ends up in the registry URL's path or query still has to be stripped from the upstream_registry_fallback warning and from the tarball URL written to the lock. The next agent run will do that together with the option you choose. Reply with 1, 2 or 3 on this PR.


Generated by Claude Code

Resolve conflicts with #1008, which moved npm-family vendoring into a shared flow: re-apply this PR's bun additions (default-trust warning #371, non-registry tarball warnings and not-rewritable refusal #497, and bun.lockb duplicate-record folding #861) on top of the new backend structure.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down] Merged main (f3c6313) into this branch as 274c478 to clear the conflicts with #1008. #1008 moved npm-family vendoring into vendor_npm_family, so this PR's Bun additions now sit in the backends:

CLI_CONTRACT.md keeps both sides' rows. Main's #920 rerun test moved back into rebuild_tests, where its fixtures live.

Checked locally: cargo check (core + cli, all targets) is clean. Core lib tests: vendor::bun 140/140, and 5939 of 5943 overall. The 4 failures are chmod-based tests that can't pass when run as root, outside the Bun code. CLI Bun suites are all green: e2e_bun_lockb, in_process_vendor, in_process_vendor_bun_takeover, mode_migration_bun and vendor_eject_bun_lockb.

agent:needs-human stays on: the security thread at npm.rs:1156 still needs option 1, 2 or 3.

bugbot run


Generated by Claude Code

A Bun hosted restore reads the project's bunfig.toml and .npmrc and
sends the registry credentials they configure. It expanded any
$VAR / ${VAR} in them, so a project could name its own registry host
and have socket-patch send it an unrelated secret such as
$GITHUB_TOKEN. Bun itself would send the same on `bun install`, but
socket-patch also runs where Bun does not, so it is an extra layer of
defense: only NPM_TOKEN, NODE_AUTH_TOKEN and BUN_AUTH_TOKEN expand now,
and any other variable expands to nothing (the read falls back to the
anonymous read, as on main).

The maintainer chose the allowlist on the review thread.

Assisted-by: Claude Code:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSx4W4Qfb8hsBhw4NEvkv9
Comment thread crates/socket-patch-core/src/patch/redirect/upstream/npm.rs Outdated
The env allowlist still let an allowed token reach a registry URL's
host, path or query (`https://h/${NPM_TOKEN}/`). That part of the URL
is requested, printed in upstream_registry_fallback and written into
the restored bun.lock / bun.lockb, so the token leaked into all three
(security review on 55cf5b3). A registry URL from bunfig.toml or
.npmrc now expands variables only inside its `user:password@`, which
split_userinfo moves onto the request's Authorization header; a
reference anywhere else expands to nothing, and such a registry falls
back like any other unreadable one.

Assisted-by: Claude Code:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSx4W4Qfb8hsBhw4NEvkv9
…sues

# Conflicts:
#	crates/socket-patch-cli/CLI_CONTRACT.md
#	crates/socket-patch-core/src/hosted/governing_root.rs
#	crates/socket-patch-core/src/patch/redirect/upstream/npm.rs
#	crates/socket-patch-core/src/patch/shared_store.rs
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit 2044d9e Oct 9, 2026
468 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-bun-open-issues branch October 9, 2026 13:21
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
#1009 landed its own HARNESS_TRANSPORT_FAILURE in backtest-bun.py
after this branch was queued. The squash merged cleanly, but the
later assignment shadowed this PR's HTTP 5xx/429 pattern, so
BunTransportRetryTests failed in merge group 87ff737 and every
group stacked behind it.

Keep one definition: the urllib 5xx/429 match plus #1009's anchored
urlopen and Errno connection-reset matches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVVtQNBuPeYaRmV82s6r6V
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

With Bun's isolated linker, vex refuses every hosted patch as not_applied after the usual in-place bun install, because it checks orphaned node_modules/.bun registry entries that Bun never removes (regression from #496) Perf regression: bun/hosted wall +110% (1169ae68, #472) Bun hosted and vendored modes skip a URL or file: tarball copy of the patched package without warning, and vendored vex attests not_affected (the #326 fix covers npm locks only) Global mode misses every Bun global package when BUN_INSTALL_BIN or BUN_INSTALL_GLOBAL_DIR is set: scan -g reports success with nothing found, get -g / vex -g patch and attest nothing On Bun ≥ 1.3.5, hosted and vendored rewiring drops Bun's default trust, so install scripts of patched packages (better-sqlite3, esbuild, sharp…) are silently blocked

4 participants