Fix vlt 1.3 brotli lock nodes being refused (#372)

crates/socket-patch-cli/src/commands/vex_consumed.rs

@@ -715,8 +715,11 @@ mod tests {
             None,
         )
         .await;
-        assert_eq!(installed_again, installed);
-        let (paths, calls) = tracked_npm_hosted(&common, &installed_again).await;
+        // Since #605 the name-keyed resolver probes bundled trees itself, so
+        // it already returns the aliases and the nested store's peers. Feed
+        // the earlier, alias-free set to keep exercising alias expansion;
+        // the resolver's own set is checked against the same result below.
+        let (paths, calls) = tracked_npm_hosted(&common, &installed).await;
         assert_eq!(calls.len(), 1);
         let mut inputs = calls[0].clone();
         inputs.sort();
@@ -738,6 +741,9 @@ mod tests {
                 .len(),
             paths.len()
         );
+        let (mut resolved, _) = tracked_npm_hosted(&common, &installed_again).await;
+        resolved.sort();
+        assert_eq!(resolved, expected, "the resolver's own copy set");
     }
 
     #[cfg(unix)]
@@ -768,14 +774,19 @@ mod tests {
             None,
         )
         .await;
-        assert!(installed.is_empty(), "{installed:?}");
-        let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await;
+        // Since #605 the name-keyed resolver reaches the alias and its
+        // sibling peers on its own. An alias-only set (what an alias-blind
+        // resolver returns) must still expand to the same copies.
+        let (mut paths, calls) = tracked_npm_hosted(&common, &HashMap::new()).await;
         assert_eq!(calls, vec![vec![alias.clone()]]);
         let mut expected = peers;
         expected.push(alias);
         paths.sort();
         expected.sort();
         assert_eq!(paths, expected);
+        let (mut resolved, _) = tracked_npm_hosted(&common, &installed).await;
+        resolved.sort();
+        assert_eq!(resolved, expected, "the resolver's own copy set");
     }
 
     #[cfg(unix)]

crates/socket-patch-core/src/patch/redirect/upstream/vlt.rs

@@ -28,9 +28,9 @@ use serde_json::{Map, Value};
 use super::npm::{by_uuid, fetch_dists, read_or_refuse, refuse_all_in};
 use super::{Ctx, FormatResult, HostedPin, View};
 use crate::vendor::vlt_lock_text::{
-    default_registry_alias, entry_text, is_default_registry, nodes_block, parse_node_line,
-    registry_base, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
-    split_lines, DepIdEra, DepIdKind, LockSniff,
+    brotli_for_slot3, default_registry_alias, entry_text, is_default_registry, nodes_block,
+    parse_node_line, registry_base, render_entry_line, render_tuple_with_slots, sniff_lock,
+    split_dep_id, split_lines, DepIdEra, DepIdKind, LockSniff,
 };
 
 /// One hosted node line to restore.
@@ -269,6 +269,7 @@ pub(crate) async fn restore(
             };
             let tuple = render_tuple_with_slots(
                 &line.entry.elems,
+                brotli_for_slot3(slot3.as_deref()),
                 Some(&json(&integrity)),
                 slot3.as_deref(),
             );

crates/socket-patch-core/src/patch/redirect/vlt.rs

@@ -17,10 +17,10 @@ use crate::constants::npm_family::{
     BUN_LOCK, BUN_LOCKB, NPM_LOCKS, PNPM_LOCK, VLT_CONFIG, VLT_HIDDEN_LOCK_REL, VLT_LOCK,
 };
 use crate::vendor::vlt_lock_text::{
-    entry_text, installs_outside_registry, is_default_registry, is_registry_url_segment,
-    nodes_block, parse_node_entry_text, parse_node_line, parse_vendored_path, render_entry_line,
-    render_tuple_with_slots, sniff_lock, split_dep_id, split_lines, DepId, DepIdKind, LockSniff,
-    NodeEntry, ParsedLock, SectionSpan,
+    brotli_for_slot3, entry_text, has_brotli_flag, installs_outside_registry, is_default_registry,
+    is_registry_url_segment, nodes_block, parse_node_entry_text, parse_node_line,
+    parse_vendored_path, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
+    split_lines, DepId, DepIdKind, LockSniff, NodeEntry, ParsedLock, SectionSpan,
 };
 
 /// The ledger kind of a hosted vlt node splice.
@@ -490,10 +490,18 @@ fn rewrite_dep(
     for &(idx, id) in &located {
         let line = parse_node_line(&lines[idx]).expect("instance_line parsed this line");
         let elems = &line.entry.elems;
-        if elems.len() >= 4 && elems[2] == s2 && elems[3] == s3 {
+        // The hosted artifact is a `.tgz`, so a node that resolved vlt
+        // 1.3's `.tar.br` alternate drops the brotli bit with its URL, as
+        // `vlt install` would save it (#372).
+        let brotli = brotli_for_slot3(Some(&s3));
+        if elems.len() >= 4
+            && elems[2] == s2
+            && elems[3] == s3
+            && has_brotli_flag(elems[0]) == brotli
+        {
             continue;
         }
-        let tuple = render_tuple_with_slots(elems, Some(&s2), Some(&s3));
+        let tuple = render_tuple_with_slots(elems, brotli, Some(&s2), Some(&s3));
         let new_text = entry_text(id, &tuple);
         let extra = split_dep_id(id).and_then(|dep_id| dep_id.extra);
         splices.push(Splice {
@@ -668,6 +676,7 @@ pub fn carried_pin_original(fresh: &FileEdit, old: &FileEdit) -> Option<Value> {
     }
     let tuple = render_tuple_with_slots(
         &fresh_original.elems,
+        has_brotli_flag(old_original.elems[0]),
         old_original.slot(2).filter(|s| *s != "null"),
         old_original.slot(3).filter(|s| *s != "null"),
     );
@@ -986,4 +995,52 @@ mod tests {
         assert_eq!(carried_pin_original(&relocked, &old), None);
     }
 
+    /// #372: vlt 1.3 records the brotli bit (4) on a node that resolved
+    /// the registry's `.tar.br` alternate. Such a lock is canonical; the
+    /// pin points the node at the hosted `.tgz`, so it clears bit 4 (and
+    /// keeps dev/optional), which is what `vlt install` saves for it.
+    #[test]
+    fn a_vlt_1_3_brotli_lock_is_pinned_with_the_brotli_bit_cleared() {
+        const BR_URL: &str = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tar.br";
+        let peer = "~npm~left-pad@1.3.0~peer.1";
+        let lock = lock_with(&[
+            &format!("\"{ID}\": [4,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
+            &format!("\"{peer}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
+            "\"~npm~other@1.0.0\": [5,\"other\",\"sha512-O==\"]",
+        ]);
+        assert!(preflight_vlt_hosted(&files(&[(VLT_LOCK, &lock)])).is_ok());
+
+        let result = rewrite(&lock, &[dep("left-pad", "1.3.0", Some(SHA))]);
+        assert!(codes(&result).is_empty(), "{:?}", result.warnings);
+        assert!(result.confirmed_vlt_uuids.contains("uuid-left-pad"));
+        let written = &result.files[VLT_LOCK];
+        assert!(written.contains(&format!("\"{ID}\": [0,\"left-pad\",\"{SHA}\",\"{URL}\"]")));
+        assert!(written.contains(&format!("\"{peer}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]")));
+        assert!(written.contains("\"~npm~other@1.0.0\": [5,\"other\",\"sha512-O==\"]"));
+        let originals: Vec<_> = result.edits.iter().map(|e| e.original.clone()).collect();
+        assert!(originals.contains(&Some(Value::String(format!(
+            "\"{ID}\": [4,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"
+        )))));
+
+        // A re-run over its own pin is a no-op.
+        let again = rewrite(written, &[dep("left-pad", "1.3.0", Some(SHA))]);
+        assert!(again.edits.is_empty(), "{:?}", again.edits);
+
+        // Superseding a carried brotli pin restores the brotli bit with
+        // the pristine `.tar.br` slots.
+        let old = vlt_edit(
+            &format!("\"{peer}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
+            &format!("\"{peer}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]"),
+        );
+        let carried = vlt_edit(
+            &format!("\"{ID}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]"),
+            &format!("\"{ID}\": [2,\"left-pad\",\"sha512-P2==\",\"u2\"]"),
+        );
+        assert_eq!(
+            carried_pin_original(&carried, &old),
+            Some(Value::String(format!(
+                "\"{ID}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"
+            )))
+        );
+    }
 }

crates/socket-patch-core/src/patch/redirect/vlt_heal.rs

@@ -194,9 +194,11 @@ pub fn lock_flags(lock_text: &str, dep_id: &str) -> Option<u64> {
 /// (flags 1 or 3) as a skipped optional dependency, so every release from
 /// 0.0.0-32 to 1.2.0 leaves it missing (its importer link dangling) unless
 /// the same install also reinstalls a non-optional node; `vlt ci` restores
-/// it. Unknown flags are not guaranteed either.
+/// it. Unknown flags are not guaranteed either. vlt 1.3's brotli bit (4)
+/// only says which artifact the node fetches, so a brotli prod or dev node
+/// (4 or 6) is reinstalled like any other (#372).
 pub fn reinstalls_after_removal(flags: Option<u64>) -> bool {
-    matches!(flags, Some(0 | 2))
+    flags.is_some_and(|flags| flags <= 7 && flags & 1 == 0)
 }
 
 /// vlt's `isDepID` path-safety rule: the id is used as one path segment.
@@ -1080,7 +1082,10 @@ mod tests {
     fn only_prod_and_dev_nodes_are_reinstalled_after_removal() {
         assert!(reinstalls_after_removal(Some(0)));
         assert!(reinstalls_after_removal(Some(2)));
-        for flags in [Some(1), Some(3), Some(4), None] {
+        // #372: vlt 1.3's brotli bit leaves a prod or dev node reinstalled.
+        assert!(reinstalls_after_removal(Some(4)));
+        assert!(reinstalls_after_removal(Some(6)));
+        for flags in [Some(1), Some(3), Some(5), Some(7), Some(8), None] {
             assert!(!reinstalls_after_removal(flags), "{flags:?}");
         }
     }

crates/socket-patch-core/src/vendor/vlt_lock.rs

@@ -40,12 +40,12 @@ use super::state::{
     WiringRecord,
 };
 use super::vlt_lock_text::{
-    edges_block, entry_text, file_dep_id, installs_outside_registry, is_default_registry,
-    is_importer_dep_id, is_registry_url_segment, nodes_block, parse_edge_entry_text,
-    parse_edge_line, parse_node_entry_text, parse_node_line, parse_vendored_dir_path,
-    render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id, split_lines,
-    vendored_dir_rel, vlt_collate, vlt_edge_cmp, DepIdEra, DepIdKind, LockSniff, ParsedLock,
-    SectionSpan,
+    brotli_for_slot3, edges_block, entry_text, file_dep_id, has_brotli_flag,
+    installs_outside_registry, is_default_registry, is_importer_dep_id, is_registry_url_segment,
+    nodes_block, parse_edge_entry_text, parse_edge_line, parse_node_entry_text, parse_node_line,
+    parse_vendored_dir_path, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
+    split_lines, vendored_dir_rel, vlt_collate, vlt_edge_cmp, DepIdEra, DepIdKind, LockSniff,
+    ParsedLock, SectionSpan,
 };
 use super::{RevertOpts, RevertOutcome, VendorOutcome, VendorWarning};
 
@@ -855,8 +855,13 @@ fn plan_wiring(
             NOT_CANONICAL.to_string(),
         )
     })?;
-    let file_tuple =
-        render_tuple_with_slots(&node_entry.elems, Some("null"), Some(&json_string(rel)));
+    let rel_slot = json_string(rel);
+    let file_tuple = render_tuple_with_slots(
+        &node_entry.elems,
+        brotli_for_slot3(Some(&rel_slot)),
+        Some("null"),
+        Some(&rel_slot),
+    );
     let new_node = Entry {
         key: file_id.clone(),
         value: file_tuple,
@@ -1645,6 +1650,7 @@ fn revert_node(staged: &mut Staged, rec: &WiringRecord) -> Step {
         }
         let tuple = render_tuple_with_slots(
             &live.elems,
+            has_brotli_flag(original.elems[0]),
             original.slot(2).filter(|s| *s != "null"),
             original.slot(3).filter(|s| *s != "null"),
         );
@@ -2981,6 +2987,35 @@ mod tests {
         assert!(fx.root.join(&entry.artifact.path).exists());
     }
 
+    /// #372: vlt 1.3 sets the brotli bit (4) on nodes that resolved the
+    /// registry's `.tar.br` alternate. Vendoring wires such a lock (the
+    /// vendored directory is no Brotli tarball, so the wired node drops
+    /// bit 4) and the revert puts the pristine brotli entry back.
+    #[tokio::test]
+    async fn a_vlt_1_3_brotli_lock_is_vendored_and_reverted_exactly() {
+        let brotli = basic_lock()
+            .replace("[0,\"a\",", "[6,\"a\",")
+            .replace(
+                "[0,\"left-pad\",\"sha512-REG==\",\"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz\"]",
+                "[4,\"left-pad\",\"sha512-REG==\",\"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tar.br\"]",
+            );
+        assert!(brotli.contains("[4,\"left-pad\""));
+        let fx = fx(&brotli, &[(PACKAGE_JSON, ROOT_PKG)]).await;
+        let (entry, _) = entry_of(run(&fx, UUID, false).await);
+        let rel = format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0/node_modules/left-pad");
+        let wired = read(&fx, VLT_LOCK).await;
+        assert!(
+            wired.contains(&format!("[0,\"left-pad\",null,\"{rel}\"]")),
+            "{wired}"
+        );
+        assert!(wired.contains("[6,\"a\",\"sha512-A==\"]"), "{wired}");
+
+        let out = revert_vlt_opts(&entry, &fx.root, RevertOpts::new(false)).await;
+        assert!(out.success && out.warnings.is_empty(), "{out:?}");
+        assert_eq!(read(&fx, VLT_LOCK).await, brotli);
+        assert_eq!(read(&fx, PACKAGE_JSON).await, ROOT_PKG);
+    }
+
     #[tokio::test]
     async fn revert_after_the_user_already_undid_the_wiring_only_removes_the_artifact() {
         let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await;