Repository navigation
Fix yarn global folder moved by global-folder (#1629) - #1696
Mikola Lysenko (mikolalysenko) merged 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A `yarn global add` install whose global folder was moved with yarn's `global-folder` setting (YARN_GLOBAL_FOLDER, npm_config_global_folder, ~/.yarnrc, ~/.npmrc, ...) to a folder not named `global` looked like a small yarn project, so `socket-patch --update` told the user to run `yarn add` from wherever they were. That edits some other project and leaves the installed copy stale. Self-update now reads the `global-folder` values yarn 1 reads, resolves each path the way yarn 1.22 does, and treats an install inside one of those folders as yarn's global folder: `yarn global add …@latest`. Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the moved yarn global folder fix:
- Accept a `--global-folder` default flag in a .yarnrc.
- Read a .yarnrc value as one token of yarn's rc grammar, so a
trailing `# comment` is dropped and a double-quoted value has its
escapes decoded (Windows paths).
- Decode an .npmrc value like npm's ini parser (inline comments,
quotes, escapes) and expand its ${NAME} references like yarn.
- Read yarn's config only when a yarn install is being classified,
so other install channels never walk the rc files.
Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 9e104c4. Configure here.
|
Ready for review at head
Generated by Claude Code |
|
Burn-down agent: labeled Ready for review.
Generated by Claude Code |
Final review briefWhat it does: Risk: low–medium. It only changes the hint/label for yarn-owned Look here:
Verified: I read the full diff. Locally, Changes I made: none. Open questions: none blocking. The description notes that moving the yarn 1 config resolver out of Auto-merge is armed, so approving sends it straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1629
This closes the variant #1680 left open: a yarn 1 global folder that yarn's
global-foldersetting moved to a folder not namedglobal.Summary
yarn global addwrites apackage.jsonandyarn.lockinto its globalfolder. When
global-foldermoves that folder (for exampleYARN_GLOBAL_FOLDER=/opt/yarn-g), it looks exactly like a small yarn 1project on disk.
socket-patch --updatethen refused withyarn add @socketsecurity/socket-patch@latest. Run from the user's cwd,that edits some other project and leaves the installed copy stale. It now
says
yarn global add @socketsecurity/socket-patch@latest.Root cause
update::channel::is_yarn_global_storerecognized yarn's global folderonly by its default spelling (a
globaldir under ayarnhome, or aglobaldir holding yarn's lock). It never read yarn's config.Fix
patch::redirect::yarnrc::configured_yarn_global_folderslists everyfolder yarn 1 run in the cwd is configured to use as its global folder.
It uses the same layers as the existing
yarn-offline-mirrorresolver,now shared through
RcLayout:YARN_*/npm_config_*env, the cwd's,user's, prefix's and ancestors'
.yarnrc(including a--global-folderdefault flag), and the same set of
.npmrcfiles. Yarn uses only thefirst layer by precedence, but a folder that any layer names was set up
as a yarn global folder, so every layer counts.
.yarnrcvalue is onetoken of yarn's rc grammar: JSON double quotes, a bare word that ends at
a space so a trailing
# commentis dropped, and single quotes throughthe YAML fallback. It resolves against its file's folder. An
.npmrcvalue goes through npm's
inidecoding (quotes,;/#comments,escapes) and yarn's
envReplace(${NAME}; an unset name drops thevalue, as yarn fails to load it). Env and
.npmrcvalues then go throughresolveWithHome:~/is yarn's user home (/usr/local/shareas root),and anything else resolves against the cwd.
ChannelEnvcarries the canonicalized folders. It reads them lazily, onfirst use, so only a yarn-owned
node_modulesinstall walks the rc files.upgrade_hint_forandchannel_label_fortake theChannelEnv, and thetwo CLI call sites pass the one they already build.
I checked every decoding rule in the tests against a real yarn 1.22.22
(
yarn global dir).Not changed (reviewed)
--global-folderpassed only on the command line at install timeleaves no trace in config, so it can't be told apart from a yarn project.
Such installs still get
yarn add.yarn-offline-mirrorreader keeps its existing value parsing. The newtoken reader is used only for
global-folder, so this PR doesn't changemirror behavior.
/code-review highsuggested moving the yarn 1 config resolver out ofpatch/redirectinto a shared module. I kept it next to the mirrorresolver whose file layout it reuses. Moving it is a separate refactor.
hint logic.
Tests (red → green)
YARN_GLOBAL_FOLDER/yarn_global_folder/npm_config_global_folder,~/.yarnrc,~/.npmrcupdate::self_update_channels_e2e::yarn_configured_global_folder_refuses_with_yarn_global_hintyarn add …)update::channel::tests::yarn_configured_global_folder_gets_yarn_global_addyarnrc::tests::global_folder_layers_match_yarn~/, root)yarnrc::tests::global_folder_paths_resolve_like_yarn--global-folder, comments, JSON/YAML quotes,${NAME}, ini)yarnrc::tests::global_folder_values_read_like_yarnLocal:
cargo fmt --all -- --checkandcargo clippy --workspace --all-features -- -D warningsare clean, andcargo test -p socket-patch-cli --all-features --test updatepasses (68/68).cargo test -p socket-patch-core --all-features --libpasses 6704/6708. The 4 failures are permission-denied tests that fail the same way onmainin this sandbox, which runs as root so chmod can't make writes fail:copy_tree,vlt_heal,pypi_poetry,pypi_requirements.cargo test -p socket-patch-cli --all-features --libpasses (933/933). The fullcargo test --workspace --all-featuresbuild needs more disk than this sandbox allows (it ran out twice), so CI runs the rest of the integration suites./code-review highran on the diff. Its findings are fixed in the second commit, except the module move listed under "Not changed".https://claude.ai/code/session_01MFXsCQpS1bvSPpbaoq45V6
Generated by Claude Code