Skip to content

Commit 75962e0

Browse files
Fix bun.lockb shared bundled pin being unmanageable (#1243) (#1247)
* Start fix for #1243 Assisted-by: Claude Code:claude-opus-5-5 * Keep bun.lockb shared bundled pins manageable Bun 1.2+ keeps one bun.lockb record for a version that is installed both from the registry and bundled inside a parent's tarball. The hosted scan wires that record for the regular install, but discovery treated it like a bundled-only record and dropped its ref. So `list`, `remove` and `rollback` refused the pin as contested, and the hosted to vendored takeover failed with vendor_lock_entry_not_found. Discovery now classifies a shared record as the regular install and records its version as a bundled copy, so the ref is shadowed: still never attested in VEX (the bundled copy stays unpatched), but visible to every command that manages hosted pins, as the text bun.lock already is. Fixes #1243 Assisted-by: Claude Code:claude-opus-5-5 * Test a hosted pin on a shared bundled bun.lockb record Bun 1.2+ e2e: a root that depends on minimist@1.2.2 and on a local parent that bundles its own minimist@1.2.2. After the hosted scan, `list` must name the pin (it exited 1 with hosted_wiring_contested), the online takeover must vendor over it (it failed with vendor_lock_entry_not_found), `vendor --revert` must restore the original bytes, and `rollback` must refuse it with the checkout remedy like any binary hosted pin. Older Bun keeps no shared record, so the leg skips there. Refs #1243 Assisted-by: Claude Code:claude-opus-5-5 * Run the shared bundled bun.lockb test on Bun 1.4 The Bun compatibility backtest runs every `native_binary_` test and expects exactly three to pass, so the new #1243 test's name broke all three `binary` legs. Rename it out of that prefix, and add it to the Bun 1.4.2 e2e_bun_lockb leg: the 1.0 and 1.1 legs keep no shared record and skip it, so without this no CI leg ran it for real. Refs #1243 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f2335d3 commit 75962e0

3 files changed

Lines changed: 206 additions & 4 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,11 +1109,13 @@ jobs:
11091109
- {os: ubuntu-latest, suite: e2e_bun_lockb, bun: '1.1.45', test_filter: --include-ignored}
11101110
# Bun >= 1.4 migrating a hosted workspace bun.lockb to bun.lock
11111111
# (#803; its reader must be 1.4+) and a vendored one, then
1112-
# reverting it (#784; skipped by the < 1.2 readers above). Both
1112+
# reverting it (#784; skipped by the < 1.2 readers above), and a
1113+
# hosted pin on a record Bun 1.2+ shares between a regular and a
1114+
# bundled install (#1243; also skipped below 1.2). Both
11131115
# 1.4.2 and 1.3.14 also run the isolated-linker vendored re-run
11141116
# after a late dependent (#861); 1.3 re-hoists a frozen binary lock
11151117
# and refuses one whose trees changed.
1116-
- {os: ubuntu-latest, suite: e2e_bun_lockb, bun: '1.4.2', test_filter: --include-ignored text_migration workspace_late_dependent}
1118+
- {os: ubuntu-latest, suite: e2e_bun_lockb, bun: '1.4.2', test_filter: --include-ignored text_migration workspace_late_dependent binary_shared_bundled_record_hosted_pin_is_managed}
11171119
- {os: ubuntu-latest, suite: e2e_bun_lockb, bun: '1.3.14', test_filter: --include-ignored workspace_late_dependent}
11181120
# Real-vlt capstones (DESIGN §8.4): wiremock patch service and a local
11191121
# npm registry fed from npmjs, driven by the pinned vlt release

‎crates/socket-patch-cli/tests/e2e_bun_lockb.rs‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,16 @@ impl Fixture {
385385
)
386386
.unwrap();
387387
}
388+
if shape == "bundled" {
389+
// REGRESSION (#1243): a local parent that bundles its own
390+
// minimist@1.2.2 beside the root's registry minimist@1.2.2.
391+
std::fs::write(
392+
project.join("bparent-1.0.0.tgz"),
393+
bundling_parent_tgz("minimist", "1.2.2"),
394+
)
395+
.unwrap();
396+
package["dependencies"]["bparent"] = json!("file:./bparent-1.0.0.tgz");
397+
}
388398
if shape == "extensions" {
389399
package["dependencies"]["consumer"] = json!("workspace:*");
390400
package["dependencies"]["git-number"] = json!("github:jonschlinkert/is-number#7.0.0");
@@ -665,6 +675,44 @@ impl Fixture {
665675
}
666676
}
667677

678+
/// A `bparent@1.0.0` tarball that declares `bundleDependencies: [name]`
679+
/// and ships its own `name@version` under `node_modules/`.
680+
fn bundling_parent_tgz(name: &str, version: &str) -> Vec<u8> {
681+
let manifest = json!({"name":"bparent", "version":"1.0.0",
682+
"dependencies":{name: version}, "bundleDependencies":[name]});
683+
let bundled = json!({"name":name, "version":version, "main":"index.js"});
684+
let files = [
685+
("package/package.json".to_string(), manifest.to_string()),
686+
(
687+
"package/index.js".to_string(),
688+
format!("module.exports = require({name:?});\n"),
689+
),
690+
(
691+
format!("package/node_modules/{name}/package.json"),
692+
bundled.to_string(),
693+
),
694+
(
695+
format!("package/node_modules/{name}/index.js"),
696+
"module.exports = 'bundled';\n".to_string(),
697+
),
698+
];
699+
let mut builder = tar::Builder::new(flate2::write::GzEncoder::new(
700+
Vec::new(),
701+
flate2::Compression::default(),
702+
));
703+
for (path, body) in &files {
704+
let mut header = tar::Header::new_gnu();
705+
header.set_size(body.len() as u64);
706+
header.set_mode(0o644);
707+
header.set_mtime(0);
708+
header.set_cksum();
709+
builder
710+
.append_data(&mut header, path, body.as_bytes())
711+
.unwrap();
712+
}
713+
builder.into_inner().unwrap().finish().unwrap()
714+
}
715+
668716
fn make_tgz_from_installed(pkg_dir: &Path, replaced_index: &[u8]) -> Vec<u8> {
669717
let pkg_dir = pkg_dir
670718
.canonicalize()
@@ -1031,6 +1079,115 @@ async fn native_binary_hosted_vendored_takeover_roundtrip() {
10311079
fixture.frozen("rolled-back", &fixture.original, "minimist");
10321080
}
10331081

1082+
/// REGRESSION (#1243): Bun 1.2+ keeps ONE `bun.lockb` record for a
1083+
/// version installed both from the registry and bundled inside a parent's
1084+
/// tarball. The hosted scan wires that record for the regular install
1085+
/// (warning that the bundled copy stays unpatched); the pin it wrote must
1086+
/// then be listed and unwound like any other: `list` succeeds, the online
1087+
/// hosted → vendored takeover restores the registry record and vendors over
1088+
/// it, and `vendor --revert` gives back the pre-hosted bytes exactly.
1089+
#[tokio::test(flavor = "multi_thread")]
1090+
#[serial_test::serial]
1091+
async fn binary_shared_bundled_record_hosted_pin_is_managed() {
1092+
let Some(fixture) = Fixture::new("bundled") else {
1093+
return;
1094+
};
1095+
let server = MockServer::start().await;
1096+
mock_api(&server, &fixture, "minimist").await;
1097+
let project = &fixture.project;
1098+
let uri = server.uri();
1099+
1100+
let hosted = scan(project, &server, "hosted", &[]);
1101+
assert_eq!(hosted["redirect"]["redirected"], 1, "hosted scan: {hosted}");
1102+
let text = hosted.to_string();
1103+
let shared =
1104+
text.contains("redirect_bun_bundled_instance_skipped") && text.contains("also bundled");
1105+
if !shared {
1106+
// Bun < 1.2 records no bundled flag on the regular record; that
1107+
// shape is the ordinary hosted pin the other tests cover.
1108+
eprintln!("SKIP #1243 leg: this Bun keeps no shared bundled record: {hosted}");
1109+
return;
1110+
}
1111+
assert_ne!(fixture.lock(), fixture.original_lock);
1112+
1113+
// `--patch-server-url`: the mock serves the hosted artifact, so name
1114+
// it the hosted origin (as the vendor run below does).
1115+
let (code, listed) = cli_code(project, &["list", "--patch-server-url", &uri]);
1116+
assert_eq!(
1117+
code, 0,
1118+
"list must accept the hosted pin it wrote: {listed}"
1119+
);
1120+
assert!(
1121+
!listed.to_string().contains("hosted_wiring_contested"),
1122+
"{listed}"
1123+
);
1124+
assert!(
1125+
listed.to_string().contains(PURL),
1126+
"list names the hosted pin: {listed}"
1127+
);
1128+
1129+
// The npm registry's version document for minimist@1.2.2, served
1130+
// locally (see native_binary_hosted_vendored_takeover_roundtrip).
1131+
let integrity = "sha512-rIqbOrKb8GJmx/5bc2M0QchhUouMXSpd1RTclXsB41JdL+VtnojfaJR+h7F9k18/4kHUsBFgk80Uk+q569vjPA==";
1132+
Mock::given(method("GET"))
1133+
.and(path("/minimist/1.2.2"))
1134+
.respond_with(ResponseTemplate::new(200).set_body_json(json!({"dist": {
1135+
"tarball": "https://registry.npmjs.org/minimist/-/minimist-1.2.2.tgz",
1136+
"integrity": integrity}})))
1137+
.mount(&server)
1138+
.await;
1139+
fixture.stage();
1140+
let taken_over = cli_env(
1141+
project,
1142+
&[
1143+
"vendor",
1144+
"--patch-server-url",
1145+
&uri,
1146+
"--vendor-source",
1147+
"service",
1148+
],
1149+
&[("SOCKET_NPM_REGISTRY", &uri)],
1150+
);
1151+
assert_eq!(
1152+
taken_over["summary"]["applied"], 1,
1153+
"online vendor over the shared hosted pin: {taken_over}"
1154+
);
1155+
assert!(
1156+
taken_over["events"].as_array().is_some_and(|events| events
1157+
.iter()
1158+
.any(|e| e["errorCode"] == "vendor_takeover_reverted_redirect")),
1159+
"the takeover is reported: {taken_over}"
1160+
);
1161+
assert!(
1162+
!taken_over
1163+
.to_string()
1164+
.contains("vendor_lock_entry_not_found"),
1165+
"{taken_over}"
1166+
);
1167+
let vendor_lock = fixture.lock();
1168+
assert!(
1169+
!vendor_lock.windows(uri.len()).any(|w| w == uri.as_bytes()),
1170+
"no hosted URL is left in bun.lockb"
1171+
);
1172+
1173+
let reverted = cli(project, &["vendor", "--revert", "--offline"]);
1174+
assert_eq!(
1175+
fixture.lock(),
1176+
fixture.original_lock,
1177+
"the revert restores the pre-hosted bytes exactly: {reverted}"
1178+
);
1179+
1180+
// `rollback` of the same pin refuses it as any binary hosted pin is
1181+
// refused (the checkout remedy), not as contested wiring.
1182+
let hosted = scan(project, &server, "hosted", &[]);
1183+
assert_eq!(
1184+
hosted["redirect"]["redirected"], 1,
1185+
"hosted again: {hosted}"
1186+
);
1187+
rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server);
1188+
assert_eq!(fixture.lock(), fixture.original_lock);
1189+
}
1190+
10341191
#[tokio::test(flavor = "multi_thread")]
10351192
#[serial_test::serial]
10361193
async fn native_binary_scan_vendored() {

‎crates/socket-patch-core/src/vex/discover/bun.rs‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,31 @@ impl Bundled {
230230
}
231231
}
232232

233+
/// Record a `bun.lockb` record Bun shares between a regular and a
234+
/// bundled install: it is classified as the regular install, so a ref
235+
/// it makes is kept for [`Bundled::contest`] to shadow (never attested,
236+
/// but still a pin list / rollback / remove / the vendored takeover can
237+
/// unwind), and its `name@version` is recorded as a bundled copy.
238+
fn share(&mut self, ctx: &DiscoverCtx<'_>, file: &str, entry: Entry<'_>, out: &mut Discovery) {
239+
let (label, name) = (entry.label.to_string(), entry.name);
240+
let recorded = entry.recorded_version;
241+
let mut alone = Discovery::default();
242+
classify(ctx, file, entry, &mut alone);
243+
out.diagnostics.extend(alone.diagnostics);
244+
let mut purls: Vec<String> = alone.elsewhere.into_iter().map(|e| e.purl).collect();
245+
for r in alone.refs {
246+
purls.push(r.purl.clone());
247+
out.push(r);
248+
}
249+
if purls.is_empty() {
250+
purls.extend(recorded.and_then(|version| npm_purl(name, version)));
251+
}
252+
for purl in purls {
253+
out.resolved_elsewhere(file, Some(purl.clone()));
254+
self.copies.entry(purl).or_insert_with(|| label.clone());
255+
}
256+
}
257+
233258
/// Withdraw every ref of `file` whose `name@version` a bundled copy in
234259
/// the same lock also installs: that copy stays unpatched beside it.
235260
fn contest(&self, file: &str, out: &mut Discovery) {
@@ -302,9 +327,13 @@ async fn extract_binary(ctx: &DiscoverCtx<'_>, out: &mut Discovery) {
302327
// A record some bundled edge reaches installs (also) as a copy
303328
// unpacked from that parent's tarball. Bun keeps ONE record for a
304329
// regular and a bundled install of the same version, so even a
305-
// record a regular edge also reaches is never attested.
306-
if p.bundled {
330+
// record a regular edge also reaches is never attested; its ref is
331+
// still the pin the hosted writer wired for that regular install
332+
// (#1243), so it is shadowed rather than dropped.
333+
if p.bundled_only {
307334
bundled.record(ctx, BUN_LOCKB, classified, out);
335+
} else if p.bundled {
336+
bundled.share(ctx, BUN_LOCKB, classified, out);
308337
} else {
309338
let user_tarball =
310339
p.version.is_none() && user_tarball_version(&p.name, &p.resolution).is_some();
@@ -1725,6 +1754,20 @@ mod tests {
17251754
"{shape}: {:?}",
17261755
diag_codes(&out)
17271756
);
1757+
// REGRESSION (#1243): a record Bun shares with a regular install
1758+
// is the pin the hosted writer wired for that install, so it is
1759+
// shadowed (visible to list / rollback / remove / the vendored
1760+
// takeover), like the text lock's regular entry beside a bundled
1761+
// one. A record only bundled edges reach wires nothing.
1762+
let shadowed: Vec<_> = out
1763+
.shadowed
1764+
.iter()
1765+
.map(|r| (r.purl.as_str(), r.uuid.as_str()))
1766+
.collect();
1767+
match shape {
1768+
"both" => assert_eq!(shadowed, [("pkg:npm/is-number@7.0.0", UUID_A)], "{shape}"),
1769+
_ => assert!(shadowed.is_empty(), "{shape}: {shadowed:?}"),
1770+
}
17281771
}
17291772
}
17301773

0 commit comments

Comments
 (0)