GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||
GeezerVison is showing github.com — Fix berry restore keeping removed descriptors (#1435) by mikolalysenko · Pull Request #1436 · SocketDev/socket-patch Conversation
|
| Issue | Test | Without fix | With fix |
|---|---|---|---|
| #1435 | in_process_redirect::yarn_berry_unwind_drops_a_descriptor_no_entry_declares (rollback and remove <purl>, stale selector undeclared vs declared by a real workspace member) |
fails: restored key "in-proc-redirect@npm:1.0.0, in-proc-redirect@npm:^1.0.0" |
passes |
| #1435 | npm::tests::berry_descriptor_declared_reads_what_the_lock_still_uses (yarn 4/3 spellings, patch: key and dep, alias, scoped namesake, prerelease range) |
n/a (new helper) | passes |
Real-yarn check (yarn 4.9.1, the issue's two-workspace repro with a local
tarball server): with the old two-descriptor key yarn install --immutable fails YN0028 (-"left-pad@npm:1.3.0, left-pad@npm:^1.3.0"
/ +"left-pad@npm:^1.3.0"). With the key this PR writes, it passes.
Local: cargo fmt --all -- --check ok; cargo clippy --workspace --all-features -- -D warnings ok. Suites: socket-patch-core --lib
6357 passed, hosted_inventory 39/39, in_process_redirect 140/143,
in_process_rollback_hosted 42/42, hosted_memory_engine and
hosted_memory_parity all pass. The 7 failures (4 core, 3
in_process_redirect) are chmod-based write-failure tests. They fail the
same way on main here because this sandbox runs as root. A full
cargo test --workspace run ran out of the sandbox's disk while linking,
so I ran these suites instead.
I couldn't run the real-yarn e2e matrix (scripts/yarn-berry-vex-matrix.sh)
here because corepack can't fetch yarn in this sandbox. CI runs it.
I fixed these findings in 3977e12:
referenced (a scoped namesake or areferenced. The change was reverted.workspaces field.Not fixed: the silent fallback when no descriptor matches. That fallback
keeps today's behavior for lock spellings this code doesn't read. A
new warning code there would fire only on shapes nobody has reported.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JbxZ6TGnJBvpiAJkB9N2XZ
Generated by Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
A hosted yarn berry pin routes each descriptor of the patched package through its own root `resolutions` selector. After `yarn remove` in one workspace member (another member still using the package), rollback and remove rebuilt the restored lock key from every selector, putting the removed member's descriptor back. Every `yarn install --immutable` then failed with YN0028 even though the command reported success. The restore now keys the entry only by the descriptors some lock entry still declares (a dependency map, in yarn 4's `npm:` or yarn 2/3's bare spelling, or a `patch:` wrapper). All routed selectors are still dropped. If the lock declares none of them, the old behavior is kept. Fixes #1435 Assisted-by: Claude Code:claude-opus-5-5
Read what the lock still declares once per restore, as a set of exact descriptors from dependency maps and `patch:` wrappers, instead of a substring scan per selector. A scoped namesake (`@acme/left-pad`) or a longer range (`1.3.0-rc.1`) no longer counts as declaring `left-pad@npm:1.3.0`, so its stale selector can't slip back into the restored key. Refs #1435 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 3977e12. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (95f8bed94a)diff --git a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs --- a/crates/socket-patch-core/src/formats/yarn/berry_prune.rs +++ b/crates/socket-patch-core/src/formats/yarn/berry_prune.rs @@ -60,6 +60,44 @@ out } +/// The descriptors (`name@range`) an entry's `dependencies:` and +/// `peerDependencies:` maps name. Yarn keys a lock entry by the union of +/// every descriptor that resolved to it, including peer requests. +pub(crate) fn dependency_and_peer_descriptors(stanza: &str) -> Vec<String> { + let mut out = Vec::new(); + let mut in_deps = false; + for line in stanza.lines().skip(1) { + if let Some(field) = line.strip_prefix(" ").filter(|r| !r.starts_with(' ')) { + in_deps = + field.trim_end() == "dependencies:" || field.trim_end() == "peerDependencies:"; + continue; + } + if !in_deps { + continue; + } + let Some(rest) = line.strip_prefix(" ").filter(|r| !r.starts_with(' ')) else { + continue; + }; + let (name, range) = match rest.strip_prefix('"') { + Some(quoted) => match quoted.split_once("\": ") { + Some((name, range)) => (name, range), + None => continue, + }, + None => match rest.split_once(": ") { + Some((name, range)) => (name, range), + None => continue, + }, + }; + let range = range.trim_end(); + let range = range + .strip_prefix('"') + .and_then(|r| r.strip_suffix('"')) + .unwrap_or(range); + out.push(format!("{name}@{range}")); + } + out +} + /// The descriptors `before` depends on that `after` (the same entry, /// rewritten) no longer does: the edges a rewrite cut. pub(crate) fn cut_descriptors(before: &str, after: &str) -> Vec<String> { diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -1068,13 +1068,13 @@ } /// The descriptors a berry lock's `stanzas` still declare: every -/// entry's `dependencies:` map, plus the descriptor a `patch:` descriptor -/// wraps (`name@patch:name@npm%3A1.3.0#…` wraps `name@npm:1.3.0`), in -/// either a dependency map or a key. Yarn 4 spells a registry range -/// `npm:range`, yarn 2 and 3 a bare `range`; see +/// entry's `dependencies:` and `peerDependencies:` map, plus the +/// descriptor a `patch:` descriptor wraps (`name@patch:name@npm%3A1.3.0#…` +/// wraps `name@npm:1.3.0`), in either a dependency map or a key. Yarn 4 +/// spells a registry range `npm:range`, yarn 2 and 3 a bare `range`; see /// [`berry_descriptor_declared`]. fn berry_declared_descriptors(stanzas: &[String]) -> BTreeSet<String> { - use crate::formats::yarn::berry_prune::dependency_descriptors; + use crate::formats::yarn::berry_prune::dependency_and_peer_descriptors; use crate::formats::yarn::patterns::split_berry_key_patterns; use crate::formats::yarn::stanzas::stanza_key; @@ -1083,7 +1083,7 @@ let keys = stanza_key(stanza) .map(split_berry_key_patterns) .unwrap_or_default(); - for descriptor in dependency_descriptors(stanza) { + for descriptor in dependency_and_peer_descriptors(stanza) { if let Some(inner) = berry_patch_inner(&descriptor) { out.insert(inner); } You can send follow-ups to the cloud agent here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Hosted yarn berry rollback/remove put back a descriptor thatyarn remove deleted from one workspace member, so every yarn install --immutable fails YN0028
There was an error while loading. Please reload this page.