From 65a3e3ec0facbe4d906dcc3750cce764c1f06a62 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 08:23:15 +0000 Subject: [PATCH 1/5] Start fix for #1570 Assisted-by: Claude Code:claude-opus-5-5 From fb67a5431aec78e4320fde4160a83032c787330c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 08:45:08 +0000 Subject: [PATCH 2/5] Split vlt lock node ids once per scan Hosted vlt scans split every vlt-lock.json node id (decode, semver and registry checks) once per patched package, three times over, and the artifact preflight parsed the whole lock again for each one. On a 1500-package lock with 30 patches that made vlt the slowest npm-family scan per package. The hosted lock now indexes its nodes once, by registry identity and by slot [1] name, and the preflight checks existing pins against the lock it already parsed. vlt/hosted and vlt/rescan get ~37% faster on the bench with the same results. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/vlt.rs | 285 ++++++++++++++---- .../src/patch/redirect/vlt_preflight.rs | 4 +- 2 files changed, 221 insertions(+), 68 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/vlt.rs b/crates/socket-patch-core/src/patch/redirect/vlt.rs index 331c31703..5e6b4a8d3 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt.rs @@ -8,7 +8,7 @@ //! commas and re-lays flags and trailing slots on later saves, so the //! revert ([`revert_vlt_slots`]) works slot by slot. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use serde_json::{Map, Value}; @@ -55,10 +55,82 @@ fn lock_unsupported(detail: &str) -> RewriteWarning { } } -/// A lock that passed the lock-level parse, with its nodes section located. +/// A lock that passed the lock-level parse, with its nodes section located +/// and its node ids split once. pub(super) struct HostedLock { parsed: ParsedLock, nodes: Option, + index: NodeIndex, +} + +/// A node whose id splits, with its slot [1]. +struct IndexedNode { + id: String, + dep_id: DepId, + slot1: Option, +} + +/// The lock's nodes with their ids split once per scan rather than once per +/// override (#1570). Positions point into `nodes`, which keeps the lock's +/// key order; a node whose id does not split is left out, as it matches no +/// override. +#[derive(Default)] +struct NodeIndex { + nodes: Vec, + /// Registry nodes by `(name, version)`, from any registry segment. + registry: HashMap<(String, String), Vec>, + /// Git, file and remote nodes by slot [1]. + non_registry: HashMap>, +} + +impl NodeIndex { + fn new(nodes: Option<&Map>) -> Self { + let mut index = NodeIndex::default(); + for (id, tuple) in nodes.into_iter().flatten() { + let Some(dep_id) = split_dep_id(id) else { + continue; + }; + let at = index.nodes.len(); + let slot1 = tuple.get(1).and_then(Value::as_str).map(str::to_string); + match dep_id.kind { + DepIdKind::Registry => { + if let Some((name, version)) = dep_id.registry_identity() { + let key = (name.to_string(), version.to_string()); + index.registry.entry(key).or_default().push(at); + } + } + DepIdKind::Git | DepIdKind::File | DepIdKind::Remote => { + if let Some(slot1) = &slot1 { + index + .non_registry + .entry(slot1.clone()) + .or_default() + .push(at); + } + } + DepIdKind::Workspace => {} + } + index.nodes.push(IndexedNode { + id: id.clone(), + dep_id, + slot1, + }); + } + index + } + + fn at<'a>(&'a self, map: Option<&'a Vec>) -> impl Iterator { + map.into_iter().flatten().map(|&i| &self.nodes[i]) + } + + fn registry(&self, name: &str, version: &str) -> impl Iterator { + self.at(self.registry.get(&(name.to_string(), version.to_string()))) + } + + /// The git, file and remote nodes whose slot [1] is `name`. + fn non_registry(&self, name: &str) -> impl Iterator { + self.at(self.non_registry.get(name)) + } } pub(super) fn parse_hosted_lock(text: &str) -> Result { @@ -87,57 +159,57 @@ pub(super) fn parse_hosted_lock(text: &str) -> Result>, -) -> Option { - let dep_id = split_dep_id(id)?; - (dep_id.registry_identity()? == (name, version)) - .then(|| is_default_registry(&dep_id.first, options)) + let index = NodeIndex::new(parsed.nodes()); + Ok(HostedLock { + parsed, + nodes, + index, + }) } /// The default-registry and the foreign registry node ids of /// `name@version`, in the lock's key order. fn partition_instances<'a>( - nodes: &'a Map, + lock: &'a HostedLock, name: &str, version: &str, - options: Option<&Map>, ) -> (Vec<&'a str>, Vec<&'a str>) { + let options = lock.parsed.options(); let mut defaults = Vec::new(); let mut foreign = Vec::new(); - for id in nodes.keys() { - match registry_instance(id, name, version, options) { - Some(true) => defaults.push(id.as_str()), - Some(false) => foreign.push(id.as_str()), - None => {} + for node in lock.index.registry(name, version) { + if is_default_registry(&node.dep_id.first, options) { + defaults.push(node.id.as_str()); + } else { + foreign.push(node.id.as_str()); } } (defaults, foreign) } +/// The git, remote and local-directory node ids that also install +/// `name@version`, in the lock's key order. +fn outside_instances<'a>(lock: &'a HostedLock, name: &str, version: &str) -> Vec<&'a str> { + lock.index + .non_registry(name) + .filter(|node| { + let slot1 = node.slot1.as_deref().unwrap_or_default(); + installs_outside_registry(&node.dep_id, slot1, name, version) + }) + .map(|node| node.id.as_str()) + .collect() +} + /// The default-registry node ids of `dep` in a lock that passed the /// lock-level parse. pub(super) fn default_instances<'a>(lock: &'a HostedLock, dep: &DepOverride) -> Vec<&'a str> { - let Some(nodes) = lock.parsed.nodes() else { - return Vec::new(); - }; - partition_instances(nodes, &full_name(dep), &dep.version, lock.parsed.options()).0 + partition_instances(lock, &full_name(dep), &dep.version).0 } /// Does `dep` have a node of socket-patch's vendored vlt shape in a /// lock that passed the lock-level parse? pub(super) fn has_vendored_instance(lock: &HostedLock, dep: &DepOverride) -> bool { - lock.parsed - .nodes() - .is_some_and(|nodes| has_vendored_node(nodes, &full_name(dep), &dep.version)) + has_vendored_node(lock, &full_name(dep), &dep.version) } /// The lock-level warnings that say vlt may discard the lock's pins: @@ -231,15 +303,10 @@ fn lock_level_warnings( files: &BTreeMap, bun_lockb_present: bool, ) -> Vec { - let ids: Vec = lock - .parsed - .nodes() - .map(|nodes| nodes.keys().filter_map(|id| split_dep_id(id)).collect()) - .unwrap_or_default(); let mut warnings: Vec = discarding_lock_codes( lock.parsed.version, lock.parsed.options(), - ids.iter(), + lock.index.nodes.iter().map(|node| &node.dep_id), files.get(VLT_CONFIG).map(String::as_str), ) .into_iter() @@ -267,14 +334,10 @@ fn npm_purl(name: &str, version: &str) -> String { } /// A `file` node of socket-patch's vendored vlt shape for `name@version`. -fn has_vendored_node(nodes: &Map, name: &str, version: &str) -> bool { - nodes.iter().any(|(id, tuple)| { - let slot1 = tuple.get(1).and_then(Value::as_str); - split_dep_id(id).is_some_and(|dep_id| { - dep_id.kind == DepIdKind::File - && slot1 == Some(name) - && parse_vendored_path(&dep_id.first, name).is_some_and(|p| p.version == version) - }) +fn has_vendored_node(lock: &HostedLock, name: &str, version: &str) -> bool { + lock.index.non_registry(name).any(|node| { + node.dep_id.kind == DepIdKind::File + && parse_vendored_path(&node.dep_id.first, name).is_some_and(|p| p.version == version) }) } @@ -318,15 +381,26 @@ pub(super) fn every_instance_pinned( ) -> Option<()> { let json: Value = serde_json::from_str(text).ok()?; let nodes = json.get("nodes")?.as_object()?; - ids.iter() - .all(|id| { - let tuple = nodes.get(*id).and_then(Value::as_array); - tuple.is_some_and(|t| { - t.get(2).and_then(Value::as_str) == Some(sha512) - && t.get(3).and_then(Value::as_str) == Some(url) - }) + instances_pinned(nodes, ids, sha512, url).then_some(()) +} + +/// Does every node of `ids` carry the patched sha512 and URL? +fn instances_pinned(nodes: &Map, ids: &[&str], sha512: &str, url: &str) -> bool { + ids.iter().all(|id| { + let tuple = nodes.get(*id).and_then(Value::as_array); + tuple.is_some_and(|t| { + t.get(2).and_then(Value::as_str) == Some(sha512) + && t.get(3).and_then(Value::as_str) == Some(url) }) - .then_some(()) + }) +} + +/// Did an earlier run pin every one of `ids`? Reads the lock as parsed, +/// not its text again. +pub(super) fn already_pinned(lock: &HostedLock, ids: &[&str], sha512: &str, url: &str) -> bool { + lock.parsed + .nodes() + .is_some_and(|nodes| instances_pinned(nodes, ids, sha512, url)) } struct Splice { @@ -356,11 +430,8 @@ fn rewrite_dep( ) -> bool { let name = full_name(dep); let version = dep.version.as_str(); - let options = lock.parsed.options(); - let empty = Map::new(); - let nodes = lock.parsed.nodes().unwrap_or(&empty); - let (defaults, foreign) = partition_instances(nodes, &name, version, options); + let (defaults, foreign) = partition_instances(lock, &name, version); if !foreign.is_empty() { result.vlt_foreign_uuids.insert(dep.patch_uuid.clone()); result.warnings.push(RewriteWarning { @@ -372,15 +443,7 @@ fn rewrite_dep( ), }); } - let outside: Vec<&str> = nodes - .iter() - .filter(|(id, tuple)| { - let slot1 = tuple.get(1).and_then(Value::as_str).unwrap_or_default(); - split_dep_id(id) - .is_some_and(|dep_id| installs_outside_registry(&dep_id, slot1, &name, version)) - }) - .map(|(id, _)| id.as_str()) - .collect(); + let outside = outside_instances(lock, &name, version); if !outside.is_empty() { result.vlt_foreign_uuids.insert(dep.patch_uuid.clone()); result.warnings.push(RewriteWarning { @@ -404,7 +467,7 @@ fn rewrite_dep( return false; }; if defaults.is_empty() { - let warning = if has_vendored_node(nodes, &name, version) { + let warning = if has_vendored_node(lock, &name, version) { RewriteWarning { code: "redirect_vlt_entry_vendored".into(), detail: format!( @@ -724,6 +787,96 @@ mod tests { assert!(result.confirmed_vlt_uuids.contains("uuid-left-pad")); } + /// The index answers every per-override question exactly as a scan + /// of every node does: same ids, same key order, same eras. + #[test] + fn the_node_index_matches_a_scan_of_every_node() { + let uuid = "123e4567-e89b-12d3-a456-426614174000"; + let lock = format!( + "{{\n \"lockfileVersion\": 1,\n \"options\": {{\"registries\": {{\"corp\": \"https://corp.example/\"}}}},\n \"nodes\": {{\n{}\n }}\n}}\n", + [ + format!("\"~corp~left-pad@1.3.0\": [0,\"left-pad\",\"{REG_SHA}\"]"), + format!("\"{ID}~peer.1\": [0,\"left-pad\",\"{REG_SHA}\"]"), + format!("\"·npm·left-pad@1.3.0\": [0,\"left-pad\",\"{REG_SHA}\"]"), + format!("\"~npm~ms@2.1.3\": [0,\"ms\",\"{REG_SHA}\"]"), + format!("\"{ID}\": [0,\"left-pad\",\"{REG_SHA}\"]"), + format!("\"~npm~left-pad@1.2.0\": [0,\"left-pad\",\"{REG_SHA}\"]"), + "\"git~github:a+b~abc\": [0,\"left-pad\"]".into(), + "\"remote~https_c++x.example+left-pad-1.3.0.tgz\": [0,\"left-pad\"]".into(), + "\"remote~https_c++x.example+left-pad-1.2.0.tgz\": [0,\"left-pad\"]".into(), + "\"file~packages+lp\": [0,\"left-pad\"]".into(), + "\"file~packages+other\": [0,\"ms\"]".into(), + format!("\"file~.socket+vendor+npm+{uuid}+left-pad-1.3.0+node__modules+left-pad\": [0,\"left-pad\"]"), + format!("\"file~.socket+vendor+npm+{uuid}+ms-2.1.3+node__modules+ms\": [0,\"ms\"]"), + "\"workspace~left-pad\": [0,\"left-pad\"]".into(), + "\"not-a-dep-id\": [0,\"left-pad\"]".into(), + ] + .map(|line| format!(" {line}")) + .join(",\n") + ); + let hosted = parse_hosted_lock(&lock).unwrap(); + let nodes = hosted.parsed.nodes().unwrap(); + let options = hosted.parsed.options(); + let scan = |name: &str, version: &str| { + let mut defaults = Vec::new(); + let mut foreign = Vec::new(); + let mut outside = Vec::new(); + let mut vendored = false; + for (id, tuple) in nodes { + let Some(dep_id) = split_dep_id(id) else { + continue; + }; + let slot1 = tuple.get(1).and_then(Value::as_str).unwrap_or_default(); + if dep_id.registry_identity() == Some((name, version)) { + if is_default_registry(&dep_id.first, options) { + defaults.push(id.as_str()); + } else { + foreign.push(id.as_str()); + } + } + if installs_outside_registry(&dep_id, slot1, name, version) { + outside.push(id.as_str()); + } + vendored |= dep_id.kind == DepIdKind::File + && slot1 == name + && parse_vendored_path(&dep_id.first, name) + .is_some_and(|p| p.version == version); + } + (defaults, foreign, outside, vendored) + }; + let indexed = |name: &str, version: &str| { + let (defaults, foreign) = partition_instances(&hosted, name, version); + let outside = outside_instances(&hosted, name, version); + ( + defaults, + foreign, + outside, + has_vendored_node(&hosted, name, version), + ) + }; + for (name, version) in [ + ("left-pad", "1.3.0"), + ("left-pad", "1.2.0"), + ("ms", "2.1.3"), + ("absent", "1.0.0"), + ] { + assert_eq!( + indexed(name, version), + scan(name, version), + "{name}@{version}" + ); + } + let (defaults, foreign, outside, vendored) = indexed("left-pad", "1.3.0"); + assert_eq!( + defaults, + [format!("{ID}~peer.1").as_str(), "·npm·left-pad@1.3.0", ID] + ); + assert_eq!(foreign, ["~corp~left-pad@1.3.0"]); + assert_eq!(outside.len(), 3, "{outside:?}"); + assert!(vendored); + assert_eq!(hosted.index.nodes.len(), nodes.len() - 1); + } + #[test] fn residual_gate_refuses_when_the_parsed_node_is_not_the_spliced_line() { let entry = format!(" \"{ID}\": [0,\"left-pad\",\"{REG_SHA}\"]"); diff --git a/crates/socket-patch-core/src/patch/redirect/vlt_preflight.rs b/crates/socket-patch-core/src/patch/redirect/vlt_preflight.rs index 6cbe7479d..6482882e7 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt_preflight.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt_preflight.rs @@ -209,8 +209,8 @@ pub fn preflight_scope( if ids.is_empty() && !vendored { return None; } - let already_pinned = !vendored - && vlt::every_instance_pinned(text, &ids, sha512, &dep.artifact_url).is_some(); + let already_pinned = + !vendored && vlt::already_pinned(&lock, &ids, sha512, &dep.artifact_url); Some(PreflightDep { patch_uuid: dep.patch_uuid.clone(), artifact_url: dep.artifact_url.clone(), From cacca7d90939153eb1db6f63ae164b677c174964 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 08:54:49 +0000 Subject: [PATCH 3/5] Skip unrelated vlt nodes in the pin check After splicing a patch into vlt-lock.json, the hosted rewrite re-reads the lock to confirm every instance carries the patched slots. It built the whole document as JSON for every patched package, which was a third of a 1500-package vlt scan. The check still parses every byte, so it refuses exactly what it did before, but it keeps only the nodes it is checking. vlt/hosted is now 47% and vlt/rescan 50% faster than main on the bench. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/vlt.rs | 150 +++++++++++++++++- 1 file changed, 147 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/vlt.rs b/crates/socket-patch-core/src/patch/redirect/vlt.rs index 5e6b4a8d3..dc0befb3d 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt.rs @@ -379,9 +379,108 @@ pub(super) fn every_instance_pinned( sha512: &str, url: &str, ) -> Option<()> { - let json: Value = serde_json::from_str(text).ok()?; - let nodes = json.get("nodes")?.as_object()?; - instances_pinned(nodes, ids, sha512, url).then_some(()) + let nodes = parse_nodes_of(text, ids)?; + instances_pinned(&nodes, ids, sha512, url).then_some(()) +} + +/// The nodes of `ids` in `text`'s `nodes` object, as `serde_json::Value` +/// would read them (the last `nodes` and the last duplicate key win). +/// Every byte is still parsed, so a document `Value` refuses is refused +/// here too, but the other nodes are skipped instead of built: the gate +/// runs once per override, and building a 1500-node lock each time was +/// most of a hosted vlt scan (#1570). `None` when `text` is not a JSON +/// object or its `nodes` is not an object. +fn parse_nodes_of(text: &str, ids: &[&str]) -> Option> { + use serde::de::{DeserializeSeed, IgnoredAny, MapAccess, Visitor}; + use std::fmt; + + struct Lock<'a>(&'a [&'a str]); + struct Nodes<'a>(&'a [&'a str]); + + impl<'de> Visitor<'de> for Lock<'_> { + type Value = Option>; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("a JSON object") + } + + fn visit_map>(self, mut map: A) -> Result { + let mut nodes = None; + while let Some(key) = map.next_key::()? { + if key == "nodes" { + nodes = map.next_value_seed(Nodes(self.0))?; + } else { + map.next_value::()?; + } + } + Ok(nodes) + } + } + + impl<'de> DeserializeSeed<'de> for Nodes<'_> { + type Value = Option>; + + fn deserialize>(self, d: D) -> Result { + d.deserialize_any(self) + } + } + + impl<'de> Visitor<'de> for Nodes<'_> { + type Value = Option>; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("any JSON value") + } + + fn visit_map>(self, mut map: A) -> Result { + let mut nodes = Map::new(); + while let Some(key) = map.next_key::()? { + if self.0.contains(&key.as_str()) { + nodes.insert(key, map.next_value::()?); + } else { + map.next_value::()?; + } + } + Ok(Some(nodes)) + } + + fn visit_seq>( + self, + mut seq: A, + ) -> Result { + while seq.next_element::()?.is_some() {} + Ok(None) + } + + fn visit_bool(self, _: bool) -> Result { + Ok(None) + } + + fn visit_i64(self, _: i64) -> Result { + Ok(None) + } + + fn visit_u64(self, _: u64) -> Result { + Ok(None) + } + + fn visit_f64(self, _: f64) -> Result { + Ok(None) + } + + fn visit_str(self, _: &str) -> Result { + Ok(None) + } + + fn visit_unit(self) -> Result { + Ok(None) + } + } + + let mut de = serde_json::Deserializer::from_str(text); + let nodes = serde::Deserializer::deserialize_map(&mut de, Lock(ids)).ok()?; + de.end().ok()?; + nodes } /// Does every node of `ids` carry the patched sha512 and URL? @@ -877,6 +976,51 @@ mod tests { assert_eq!(hosted.index.nodes.len(), nodes.len() - 1); } + /// The gate's skipping parse reads exactly the nodes a full + /// `serde_json::Value` parse would, and refuses what it refuses. + #[test] + fn the_gate_parse_matches_a_full_value_parse() { + let ids = ["a", "b"]; + let full = |text: &str| -> Option> { + let json: Value = serde_json::from_str(text).ok()?; + let nodes = json.get("nodes")?.as_object()?; + Some( + nodes + .iter() + .filter(|(id, _)| ids.contains(&id.as_str())) + .map(|(id, tuple)| (id.clone(), tuple.clone())) + .collect(), + ) + }; + for text in [ + r#"{"nodes": {"a": [0, "a", "s", "u"], "c": [0, "c"], "b": 1}}"#, + r#"{"lockfileVersion": 1, "options": {"x": [1.5, -2, null, true, {"y": "z"}]}, "nodes": {"a": [0, "a"]}, "edges": {"e": "f"}}"#, + r#"{"nodes": {"a": [0, "old"]}, "nodes": {"a": [0, "new"]}}"#, + r#"{"nodes": {"a": [0, "first"], "a": [0, "last"]}}"#, + r#"{"nodes": {"a": [0, "a"]}, "nodes": []}"#, + r#"{"nodes": [], "nodes": {"b": [0, "b"]}}"#, + r#"{"nodes": {"a": [0, "a"]}}"#, + r#"{"nodes": "x"}"#, + r#"{"nodes": null}"#, + r#"{"nodes": 7}"#, + r#"{"nodes": -7.5}"#, + r#"{"nodes": false}"#, + r#"{"other": {}}"#, + r#"{}"#, + " {\"nodes\": {}}\r\n", + r#"{"nodes": {"a": [0, "a"]}, "edges": {"e": }}"#, + r#"{"nodes": {"a": [0, "a"]}} trailing"#, + r#"{"nodes": {"a": [0, "a"],}}"#, + r#"{"nodes": {"a": [0, "a"]}, "x": "\q"}"#, + r#"{"nodes": {"a": [0, "a"]}"#, + r#"[{"nodes": {}}]"#, + r#""nodes""#, + "", + ] { + assert_eq!(parse_nodes_of(text, &ids), full(text), "{text}"); + } + } + #[test] fn residual_gate_refuses_when_the_parsed_node_is_not_the_spliced_line() { let entry = format!(" \"{ID}\": [0,\"left-pad\",\"{REG_SHA}\"]"); From 3eaf7aac5cff953e3eb9098960e770b733f72c93 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 09:24:21 +0000 Subject: [PATCH 4/5] Say what the pin check's skipping parse skips Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/redirect/vlt.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/vlt.rs b/crates/socket-patch-core/src/patch/redirect/vlt.rs index dc0befb3d..6cc6e11b8 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt.rs @@ -385,11 +385,13 @@ pub(super) fn every_instance_pinned( /// The nodes of `ids` in `text`'s `nodes` object, as `serde_json::Value` /// would read them (the last `nodes` and the last duplicate key win). -/// Every byte is still parsed, so a document `Value` refuses is refused -/// here too, but the other nodes are skipped instead of built: the gate -/// runs once per override, and building a 1500-node lock each time was -/// most of a hosted vlt scan (#1570). `None` when `text` is not a JSON -/// object or its `nodes` is not an object. +/// The whole document is still parsed, so broken JSON anywhere is refused, +/// but every other value is skipped instead of built: the gate runs once +/// per override, and building a 1500-node lock each time was most of a +/// hosted vlt scan (#1570). Skipping does not range-check numbers or pair +/// `\u` surrogates; the text it reads is a lock `sniff_lock` already read +/// whole, with only node lines re-rendered. `None` when `text` is not a +/// JSON object or its `nodes` is not an object. fn parse_nodes_of(text: &str, ids: &[&str]) -> Option> { use serde::de::{DeserializeSeed, IgnoredAny, MapAccess, Visitor}; use std::fmt; From c4aa9f1b2a79aa3c57d76b34c31b6f823e7ff8e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 09:47:03 +0000 Subject: [PATCH 5/5] Keep the vlt pin check private to the rewrite Its skipping parse is only sound on a lock the rewrite already read, so no other module should call it. Also stop copying each node's slot [1] into the index: the lookup key already is that name. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/vlt.rs | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/vlt.rs b/crates/socket-patch-core/src/patch/redirect/vlt.rs index 6cc6e11b8..0159d2e57 100644 --- a/crates/socket-patch-core/src/patch/redirect/vlt.rs +++ b/crates/socket-patch-core/src/patch/redirect/vlt.rs @@ -63,11 +63,10 @@ pub(super) struct HostedLock { index: NodeIndex, } -/// A node whose id splits, with its slot [1]. +/// A node whose id splits. struct IndexedNode { id: String, dep_id: DepId, - slot1: Option, } /// The lock's nodes with their ids split once per scan rather than once per @@ -91,7 +90,6 @@ impl NodeIndex { continue; }; let at = index.nodes.len(); - let slot1 = tuple.get(1).and_then(Value::as_str).map(str::to_string); match dep_id.kind { DepIdKind::Registry => { if let Some((name, version)) = dep_id.registry_identity() { @@ -100,12 +98,9 @@ impl NodeIndex { } } DepIdKind::Git | DepIdKind::File | DepIdKind::Remote => { - if let Some(slot1) = &slot1 { - index - .non_registry - .entry(slot1.clone()) - .or_default() - .push(at); + if let Some(slot1) = tuple.get(1).and_then(Value::as_str) { + let entry = index.non_registry.entry(slot1.to_string()); + entry.or_default().push(at); } } DepIdKind::Workspace => {} @@ -113,7 +108,6 @@ impl NodeIndex { index.nodes.push(IndexedNode { id: id.clone(), dep_id, - slot1, }); } index @@ -190,12 +184,10 @@ fn partition_instances<'a>( /// The git, remote and local-directory node ids that also install /// `name@version`, in the lock's key order. fn outside_instances<'a>(lock: &'a HostedLock, name: &str, version: &str) -> Vec<&'a str> { + // `non_registry(name)` holds only nodes whose slot [1] is `name`. lock.index .non_registry(name) - .filter(|node| { - let slot1 = node.slot1.as_deref().unwrap_or_default(); - installs_outside_registry(&node.dep_id, slot1, name, version) - }) + .filter(|node| installs_outside_registry(&node.dep_id, name, name, version)) .map(|node| node.id.as_str()) .collect() } @@ -373,14 +365,10 @@ fn instance_line( } /// The residual gate: re-parsed, every instance carries the patched slots. -pub(super) fn every_instance_pinned( - text: &str, - ids: &[&str], - sha512: &str, - url: &str, -) -> Option<()> { - let nodes = parse_nodes_of(text, ids)?; - instances_pinned(&nodes, ids, sha512, url).then_some(()) +/// `text` must be a lock [`parse_hosted_lock`] read, with only node lines +/// re-rendered (see [`parse_nodes_of`]). +fn every_instance_pinned(text: &str, ids: &[&str], sha512: &str, url: &str) -> bool { + parse_nodes_of(text, ids).is_some_and(|nodes| instances_pinned(&nodes, ids, sha512, url)) } /// The nodes of `ids` in `text`'s `nodes` object, as `serde_json::Value` @@ -640,7 +628,7 @@ fn rewrite_dep( candidate[splice.line].clone_from(&splice.text); } let ids: Vec<&str> = located.iter().map(|(_, id)| *id).collect(); - if every_instance_pinned(&candidate.join("\n"), &ids, sha512, &dep.artifact_url).is_none() { + if !every_instance_pinned(&candidate.join("\n"), &ids, sha512, &dep.artifact_url) { unsupported_key(result, dep, ids[0]); return false; }