Repository navigation
Fix yarn .env.yarn ignored in rc expansion (#1611) - #1676
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Yarn 4 loads the files named by injectEnvironmentFiles (.env.yarn by
default) before it expands ${NAME} in .yarnrc.yml, and a value from
those files wins over an exported variable. socket-patch expanded the
references from its own environment only, so a project that picks its
nodeLinker through .env.yarn was misread: vendored mode refused a
node-modules project as Plug'n'Play, and agent mode missed a live PnP
project, skipping the package as not installed with exit 0.
The linker probe, the yarn pnpm-store gate and the hosted upstream
registry reader now expand with the injected files applied. The file
list follows yarn: YARN_INJECT_ENVIRONMENT_FILES, else the nearest rc
file that sets it (relative to that file), else .env.yarn in the
project; an emptied list loads nothing and a later file wins.
Fixes #1611
Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the .env.yarn fix:
- Expand ${NAME} inside .env.yarn values as yarn does, against the
values injected so far and then the environment, so a linker set
through a chained variable resolves the way yarn resolves it.
- Read injectEnvironmentFiles through the same YAML walk as every
other setting, so a file yarn refuses (a list root, two documents,
a doubled BOM) sets no list here either.
- Load the env files only when a ${NAME} is actually expanded, and
reuse the same lookup in the hosted registry reader.
Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
|
Ready for review at Generated by Claude Code |
Final review brief (head
|
Yarn 4 concatenates the injectEnvironmentFiles list of every rc file it reads instead of taking the nearest one: the home rc file's entries load first, then each ancestor's from the farthest to the nearest, each against its own folder. A nearer [] adds nothing and clears nothing. Taking only the nearest list could drop a parent's env file, misreading nodeLinker the same way #1611 did. Checked against yarn 4.9.1. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Final reviewer: disabled auto-merge. My review brief covered d645e11, and 1a3bb28 (rc-list merge review fix) landed after it. There is also one unresolved review thread (crates/socket-patch-core/src/formats/yarn/rc.rs:232). I'll re-review once CI and Bugbot settle and the thread is resolved, then re-arm auto-merge if nothing blocks. Generated by Claude Code |
Yarn 4 stops merging injectEnvironmentFiles across rc files at a list
written as {onConflict: reset, value: [...]} (kept) or hardReset
(dropped too), and a list given as a string replaces the others
outright. Without that, a nearer reset still let a parent folder's
env file pick nodeLinker. The YAML walk now reads the onConflict
wrapper. Checked against yarn 4.9.1.
Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Final review brief (head
|
Three cases where socket-patch picked a different .env file than
yarn 4, so the nodeLinker it worked out could differ:
- A list given as a string now resolves against the folder yarn runs
in, not the rc file's folder, since yarn's string items carry no
source file.
- ${NAME} in an entry now expands with no variable set: yarn reads
this list before it loads any environment, so only the fallback
applies.
- Comma-split items keep their spaces, as yarn's split does.
Checked against yarn 4.9.1.
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 35dbcf1. Configure here.
|
This updates my brief on What it does. socket-patch now expands
Risk: low. This is read-only, and it only matters when Look here
Verified
Changes I made. None. Open questions. None new. The non-blocking ones from the earlier briefs still stand. Auto-merge is armed, so approving sends it straight to the merge queue. Generated by Claude Code |
|
Burn-down agent: re-confirmed Ready for review on the new head (label was first applied at an earlier head).
Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1611
Summary
Yarn 4 loads the dotenv files named by
injectEnvironmentFiles(.env.yarnby default) before it expands
${NAME}in.yarnrc.yml. A value from thosefiles wins over an exported variable. socket-patch expanded the references
from its own process environment only, so it resolved a different
nodeLinkerfrom the one yarn uses:nodeLinker: "${L:-pnp}"+.env.yarnL=node-modules: vendored modefalsely refused a node-modules project with
vendor_yarn_berry_unsupported(exit 1).
nodeLinker: "${L:-node-modules}"+.env.yarnL=pnp: agent mode misseda live PnP project, skipped the package as
package_not_installedandexited 0 without the
yarn_pnp_unsupportedrefusal, so the package stayedunpatched with no warning.
Root cause
YarnEnv::var(the linker probe),formats::yarn::rc::process_setting(theyarn pnpm-store gate in
patch/shared_store.rs) and the hosted upstreamregistry reader (
patch/redirect/upstream/npm.rs) each expanded${NAME}from
std::envalone.Fix
formats::yarn::rc:raw_list_settingreadsinjectEnvironmentFiles(asequence, a flow list or a comma string;
[]empties it) through the sameYAML walk as every scalar setting.
env_file_pathsbuilds the list the wayyarn does:
YARN_INJECT_ENVIRONMENT_FILESwhen set; else every rc file'slist concatenated (home first, then ancestors from the farthest to the
nearest, each entry against its own rc folder, so a nearer
[]clearsnothing; the walk stops at
{onConflict: reset, value: [...]}, beforehardReset, and a string list replaces the others); else.env.yarn?in the folder yarn runs in. As in yarn, a string list's items resolve
against the folder yarn runs in and keep their spaces, and
${…}in anentry expands with no variable set (yarn reads the list before its
environment).
parse_dotenvmirrors thedotenv parser yarn uses (
export, quotes, multi-line values, comments).injected_envexpands each value's${…}against the values injected sofar, then the environment, as yarn does.
YarnEnvgains lazily loadedenv_filesthat win over the environment invar().YarnEnv::for_projectis used byyarn_node_linker, the yarnpnpm-store gate and the upstream registry reader.
process_settingisremoved, so no reader expands from the bare process environment.
${NAME}is actually expanded.I checked every rule against a real yarn 4.9.1 (
yarn config get nodeLinker): a file value overrides an exported one; a later file wins; amissing
?file is skipped;[]adds no files; rc lists concatenate home →farthest → nearest; an rc entry resolves against its rc folder; the default
.env.yarnresolves against the folderyarn runs in, not the lockfile root; and
${…}in a dotenv value expands,including references to earlier injected values. A missing required file makes
yarn crash, so the
?marker carries no meaning here and is dropped.Not changed (reviewed)
ProjectView::Memory, napi hosted engine)still resolves the linker from the repository's rc files with no
variables (
YarnEnv::repository_only). Its file selection doesn't fetch.env.yarn(normally untracked) either, so supporting it means wideninghosted/memory/select.rs. That is out of scope for this CLI fix..env.yarnis a yarn 4 feature, but the lockversion doesn't identify the binary: yarn 4.9.1 applies
.env.yarnto aproject with a
__metadata.version: 6lock. A yarn 2/3 project whose.env.yarnsets the same variable itsnodeLinkerreads is far-fetched.logic, so they need no change.
Tests (red → green)
${L:-pnp}+.env.yarnnode-modules (also custominjectEnvironmentFiles, emptied list still refuses)in_process_vendor::berry_env_file_linker_decides_vendoringvendor_yarn_berry_unsupported).env.yarn(also with a conflicting exported value)e2e_safety_yarn_pnp::pnp_loader_under_env_file_pnp_linker_refuses.env.yarnnode-modules (export, quotes, comment)e2e_safety_yarn_pnp::stale_pnp_loader_under_env_file_linker_appliespkg_managers::tests::env_files_feed_rc_references,env_files_resolve_against_their_rc_folderrc::tests::dotenv_reads_like_yarn,list_settings_read_like_yarn,list_settings_need_a_file_yarn_reads,env_file_paths_follow_precedence,injected_values_expand_like_yarnLocal:
cargo fmt --all -- --checkandcargo clippy --workspace --all-features -- -D warningsare clean.cargo test --workspace --all-features --no-fail-fastpasses except for 10 tests that fail identically onmainin this sandbox and have nothing to do with yarn: 9 permission-denied tests (the sandbox runs as root, so chmod can't make writes fail:repairx2,in_process_redirectx3, corecopy_tree/vlt_heal/pypi_poetry/pypi_requirements) ande2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed(needspatches-api.socket.dev, which the sandbox can't reach). The real-yarn berry e2e suite couldn't run locally because corepack can't reach repo.yarnpkg.com, so CI covers it./code-review highran on the diff. Its findings are fixed in the third commit, except the two listed under "Not changed".https://claude.ai/code/session_01JsUVcdSrpjkK1bKHJrZzq4
Generated by Claude Code