Skip to content

Commit 518a84f

Browse files
Fix yarn .env.yarn ignored in rc expansion (#1611) (#1676)
* Start fix for #1611 Assisted-by: Claude Code:claude-opus-5-5 * Read .env.yarn when expanding yarnrc variables 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 * Document .env.yarn in the yarn linker rules Assisted-by: Claude Code:claude-opus-5-5 * Match yarn on .env.yarn values and list parsing 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 * Merge injectEnvironmentFiles across rc files 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 * Honour onConflict reset in env file lists 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 * Read env file lists the way yarn parses them 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3114a28 commit 518a84f

8 files changed

Lines changed: 831 additions & 21 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_safety_yarn_pnp.rs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,68 @@ fn stale_pnp_loader_under_yaml_spelled_linker_applies() {
13041304
}
13051305
}
13061306

1307+
/// #1611: yarn 4 expands `${NAME}` in `.yarnrc.yml` with the variables of
1308+
/// the project's `.env.yarn` applied, a file value winning over an
1309+
/// exported one. A live loader whose linker `.env.yarn` turns into `pnp`
1310+
/// is refused instead of exiting 0 `package_not_installed`, even when the
1311+
/// shell exports a non-PnP value for the same variable.
1312+
#[test]
1313+
fn pnp_loader_under_env_file_pnp_linker_refuses() {
1314+
for env in [&[][..], &[("SOCKET_PATCH_T1611_L", "node-modules")][..]] {
1315+
let dir = tempfile::tempdir().unwrap();
1316+
make_yarn_berry_project(dir.path());
1317+
std::fs::write(
1318+
dir.path().join(".yarnrc.yml"),
1319+
"nodeLinker: \"${SOCKET_PATCH_T1611_L:-node-modules}\"\n",
1320+
)
1321+
.unwrap();
1322+
std::fs::write(dir.path().join(".env.yarn"), "SOCKET_PATCH_T1611_L=pnp\n").unwrap();
1323+
let index = stage_applicable_package(dir.path());
1324+
let (code, stdout, _) = run_with_env(dir.path(), &["apply", "--json"], env);
1325+
assert_eq!(code, 1, "{env:?}: {stdout}");
1326+
let envelope = parse_json_envelope(&stdout);
1327+
assert_eq!(
1328+
envelope_error_code(&envelope),
1329+
Some("yarn_pnp_unsupported"),
1330+
"{env:?}"
1331+
);
1332+
assert_eq!(std::fs::read(&index).unwrap(), ORIGINAL_BYTES, "{env:?}");
1333+
}
1334+
}
1335+
1336+
/// #1611: the other direction. `.env.yarn` turns `${NAME:-pnp}` into
1337+
/// `node-modules`, so a stale loader is disowned and apply patches the
1338+
/// installed copy.
1339+
#[test]
1340+
fn stale_pnp_loader_under_env_file_linker_applies() {
1341+
let dir = tempfile::tempdir().unwrap();
1342+
let cwd = dir.path();
1343+
std::fs::write(
1344+
cwd.join("package.json"),
1345+
r#"{"name":"migrated","version":"0.0.0","private":true}"#,
1346+
)
1347+
.unwrap();
1348+
std::fs::write(cwd.join("yarn.lock"), "__metadata:\n version: 8\n").unwrap();
1349+
std::fs::write(cwd.join(".pnp.js"), b"// stale Yarn 2 loader\n").unwrap();
1350+
std::fs::write(
1351+
cwd.join(".yarnrc.yml"),
1352+
"nodeLinker: \"${SOCKET_PATCH_T1611_L:-pnp}\"\n",
1353+
)
1354+
.unwrap();
1355+
std::fs::write(
1356+
cwd.join(".env.yarn"),
1357+
"# yarn injects this\nexport SOCKET_PATCH_T1611_L='node-modules'\n",
1358+
)
1359+
.unwrap();
1360+
let index = stage_applicable_package(cwd);
1361+
let (code, stdout, stderr) = run(cwd, &["apply", "--json"]);
1362+
assert_eq!(
1363+
code, 0,
1364+
"a stale loader must not refuse.\nstdout:\n{stdout}\nstderr:\n{stderr}"
1365+
);
1366+
assert_eq!(std::fs::read(&index).unwrap(), PATCHED_BYTES);
1367+
}
1368+
13071369
/// #1129: pnpm's `node-linker=pnp` with a custom `modulesDir` keeps its
13081370
/// store in `<modulesDir>/.pnpm`, not `node_modules/.pnpm`. The layout
13091371
/// detector reads the same modules dirs as the crawler, so the tree is

‎crates/socket-patch-cli/tests/in_process_vendor.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,51 @@ async fn berry_lock_only_pnp_project_refused_up_front() {
15201520
}
15211521
}
15221522

1523+
/// #1611: yarn 4 loads the project's `injectEnvironmentFiles` (`.env.yarn`
1524+
/// by default) before it expands `${NAME}` in `.yarnrc.yml`, so a linker
1525+
/// that a `.env.yarn` variable turns into `node-modules` is not PnP:
1526+
/// vendor wires the project instead of refusing it. An emptied
1527+
/// `injectEnvironmentFiles` loads nothing, and the fallback (`pnp`) is
1528+
/// refused as before.
1529+
#[tokio::test]
1530+
async fn berry_env_file_linker_decides_vendoring() {
1531+
const RC: &str = "nodeLinker: \"${SOCKET_PATCH_T1611_VL:-pnp}\"\nenableGlobalCache: false\n";
1532+
for (label, rc_extra, file, vendors) in [
1533+
("default .env.yarn", "", ".env.yarn", true),
1534+
(
1535+
"custom file",
1536+
"injectEnvironmentFiles: [\".env.custom\"]\n",
1537+
".env.custom",
1538+
true,
1539+
),
1540+
(
1541+
"emptied list",
1542+
"injectEnvironmentFiles: []\n",
1543+
".env.yarn",
1544+
false,
1545+
),
1546+
] {
1547+
let tmp = tempfile::tempdir().unwrap();
1548+
let root = tmp.path();
1549+
stage_berry_project(root, BERRY_WIN_PKG, &berry_win_lock());
1550+
std::fs::write(root.join(".yarnrc.yml"), format!("{RC}{rc_extra}")).unwrap();
1551+
std::fs::write(root.join(file), "SOCKET_PATCH_T1611_VL=node-modules\n").unwrap();
1552+
let (code, env) = vendor_cli(root, &[]);
1553+
let wired = std::fs::read_to_string(root.join("yarn.lock"))
1554+
.unwrap()
1555+
.contains(".socket/vendor/npm/");
1556+
if vendors {
1557+
assert_eq!(code, 0, "{label}: {env:#}");
1558+
assert_eq!(env["summary"]["applied"], 1, "{label}: {env:#}");
1559+
assert!(wired, "{label}: wired");
1560+
} else {
1561+
assert_eq!(code, 1, "{label}: {env:#}");
1562+
find_event(&env, "failed", Some("vendor_yarn_berry_unsupported"));
1563+
assert!(!wired, "{label}: not wired");
1564+
}
1565+
}
1566+
}
1567+
15231568
/// Mount the hosted-mode API (batch discovery, per-package search, a granted
15241569
/// reference carrying the yarn-berry-zip checksum, the patch view) for the
15251570
/// berry takeover legs. Returns the hosted tarball URL.

‎crates/socket-patch-core/src/crawlers/pkg_managers.rs‎

Lines changed: 165 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,20 @@ pub(crate) struct YarnEnv {
176176
/// The variables `${NAME}` references read; `None` reads this
177177
/// process's environment.
178178
pub vars: Option<std::collections::BTreeMap<String, String>>,
179+
/// The variables yarn 4 injects from the project's
180+
/// `injectEnvironmentFiles` ([`Self::with_env_files`]): they win over
181+
/// `vars`, as yarn applies them over its environment (#1611).
182+
pub env_files: EnvFiles,
183+
}
184+
185+
/// The dotenv files a yarn run injects, read on the first `${NAME}`
186+
/// lookup: most runs expand none.
187+
#[derive(Debug, Clone, Default)]
188+
pub(crate) struct EnvFiles {
189+
/// The folder yarn runs in and `YARN_INJECT_ENVIRONMENT_FILES`; `None`
190+
/// injects nothing.
191+
run: Option<(PathBuf, Option<String>)>,
192+
vars: std::sync::OnceLock<std::collections::BTreeMap<String, String>>,
179193
}
180194

181195
impl YarnEnv {
@@ -189,9 +203,58 @@ impl YarnEnv {
189203
.unwrap_or_else(|| ".yarnrc.yml".to_string()),
190204
home: crate::utils::fs::home_dir(),
191205
vars: None,
206+
env_files: Default::default(),
192207
}
193208
}
194209

210+
/// [`Self::current`] for a yarn run in `project_root`: with the
211+
/// variables of the project's `injectEnvironmentFiles` (#1611).
212+
pub(crate) fn for_project(project_root: &Path) -> Self {
213+
let env_list = yarn_env_key(std::env::vars_os(), "YARN_INJECT_ENVIRONMENT_FILES", true);
214+
Self::current().with_env_files(project_root, env_list.as_deref())
215+
}
216+
217+
/// This environment with the variables yarn 4 injects for a run in
218+
/// `project_root`: the dotenv files `injectEnvironmentFiles` names
219+
/// (`env_list` when the environment sets it, else the nearest rc file
220+
/// at or above the project that sets it, else the home rc file's,
221+
/// else `.env.yarn`), a later file winning.
222+
pub(crate) fn with_env_files(mut self, project_root: &Path, env_list: Option<&str>) -> Self {
223+
let start =
224+
std::path::absolute(project_root).unwrap_or_else(|_| project_root.to_path_buf());
225+
self.env_files = EnvFiles {
226+
run: Some((start, env_list.map(str::to_string))),
227+
vars: Default::default(),
228+
};
229+
self
230+
}
231+
232+
/// The variables the env files inject, read once.
233+
fn injected(&self) -> &std::collections::BTreeMap<String, String> {
234+
self.env_files.vars.get_or_init(|| {
235+
let Some((start, env_list)) = &self.env_files.run else {
236+
return Default::default();
237+
};
238+
let read = |path: &Path| crate::utils::fs::read_regular_to_string_sync(path).ok();
239+
let mut rcs: Vec<(PathBuf, String)> = start
240+
.ancestors()
241+
.filter_map(|dir| Some((dir.to_path_buf(), read(&dir.join(&self.rc_filename))?)))
242+
.collect();
243+
if let Some(home) = &self.home {
244+
if !rcs.iter().any(|(dir, _)| dir == home) {
245+
if let Some(rc) = read(&home.join(&self.rc_filename)) {
246+
rcs.push((home.clone(), rc));
247+
}
248+
}
249+
}
250+
let rcs: Vec<(&Path, &str)> =
251+
rcs.iter().map(|(d, t)| (d.as_path(), t.as_str())).collect();
252+
let base_var = |name: &str| self.base_var(name);
253+
let paths = crate::formats::yarn::rc::env_file_paths(env_list.as_deref(), &rcs, start);
254+
crate::formats::yarn::rc::injected_env(&paths, &read, &base_var)
255+
})
256+
}
257+
195258
/// The repository alone, not the host it is scanned on: no
196259
/// environment override, no home rc file, and no variables, so a
197260
/// `${NAME}` reference takes its fallback or fails closed. For a view
@@ -202,10 +265,21 @@ impl YarnEnv {
202265
rc_filename: ".yarnrc.yml".to_string(),
203266
home: None,
204267
vars: Some(Default::default()),
268+
env_files: Default::default(),
205269
}
206270
}
207271

208-
fn var(&self, name: &str) -> Option<String> {
272+
/// The value yarn expands `${name}` to: an injected variable, else
273+
/// one of `vars`.
274+
pub(crate) fn var(&self, name: &str) -> Option<String> {
275+
if let Some(value) = self.injected().get(name) {
276+
return Some(value.clone());
277+
}
278+
self.base_var(name)
279+
}
280+
281+
/// A variable of `vars`, or of this process's environment.
282+
fn base_var(&self, name: &str) -> Option<String> {
209283
match &self.vars {
210284
Some(vars) => vars.get(name).cloned(),
211285
None => std::env::var(name).ok(),
@@ -274,7 +348,7 @@ pub(crate) fn rc_node_linker(rc: &str, env: &YarnEnv) -> Option<String> {
274348
/// is `YARN_RC_FILENAME` when set. `None` when nothing sets it: yarn berry
275349
/// then uses its default linker, `pnp`.
276350
pub fn yarn_node_linker(project_root: &Path) -> Option<String> {
277-
yarn_node_linker_in(project_root, &YarnEnv::current())
351+
yarn_node_linker_in(project_root, &YarnEnv::for_project(project_root))
278352
}
279353

280354
pub(crate) fn yarn_node_linker_in(project_root: &Path, env: &YarnEnv) -> Option<String> {
@@ -708,9 +782,98 @@ mod tests {
708782
rc_filename: ".yarnrc.yml".into(),
709783
home: None,
710784
vars: Some(Default::default()),
785+
env_files: Default::default(),
711786
}
712787
}
713788

789+
/// #1611: yarn 4 expands `${…}` with `injectEnvironmentFiles` applied
790+
/// (`.env.yarn` by default), a file's value winning over an exported
791+
/// one; an rc file or `YARN_INJECT_ENVIRONMENT_FILES` renames or
792+
/// empties the list, and a later file wins over an earlier one.
793+
#[test]
794+
fn env_files_feed_rc_references() {
795+
let linker = |rc_extra: &str, files: &[(&str, &str)], env_list: Option<&str>| {
796+
let d = tempfile::tempdir().unwrap();
797+
let rc = format!("nodeLinker: \"${{L:-pnp}}\"\n{rc_extra}");
798+
std::fs::write(d.path().join(".yarnrc.yml"), rc).unwrap();
799+
for (name, text) in files {
800+
std::fs::write(d.path().join(name), text).unwrap();
801+
}
802+
let mut env = bare_env(None);
803+
env.vars = Some([("L".to_string(), "pnpm".to_string())].into());
804+
let env = env.with_env_files(d.path(), env_list);
805+
yarn_node_linker_in(d.path(), &env)
806+
};
807+
let nm = Some("node-modules".to_string());
808+
// No file: the exported variable.
809+
assert_eq!(linker("", &[], None).as_deref(), Some("pnpm"));
810+
// `.env.yarn` overrides it.
811+
assert_eq!(linker("", &[(".env.yarn", "L=node-modules\n")], None), nm);
812+
// An rc list replaces the default; `[]` loads nothing.
813+
let custom = [(".env.yarn", "L=pnp\n"), (".env.ci", "L=node-modules\n")];
814+
assert_eq!(
815+
linker("injectEnvironmentFiles: [.env.ci]\n", &custom, None),
816+
nm
817+
);
818+
assert_eq!(
819+
linker("injectEnvironmentFiles: []\n", &custom, None).as_deref(),
820+
Some("pnpm")
821+
);
822+
// A later file wins; a missing optional one is skipped.
823+
assert_eq!(
824+
linker(
825+
"injectEnvironmentFiles:\n - .env.yarn\n - .env.missing?\n - .env.ci\n",
826+
&custom,
827+
None
828+
),
829+
nm
830+
);
831+
// The environment's list beats the rc file's.
832+
assert_eq!(
833+
linker("injectEnvironmentFiles: []\n", &custom, Some(".env.ci")),
834+
nm
835+
);
836+
}
837+
838+
/// #1611: an rc file's relative `injectEnvironmentFiles` entries
839+
/// resolve against its own folder, and every rc file's list loads, the
840+
/// nearest last.
841+
#[test]
842+
fn env_files_resolve_against_their_rc_folder() {
843+
let w = tempfile::tempdir().unwrap();
844+
std::fs::write(
845+
w.path().join(".yarnrc.yml"),
846+
"nodeLinker: \"${L:-pnp}\"\ninjectEnvironmentFiles: [env/linker]\n",
847+
)
848+
.unwrap();
849+
std::fs::create_dir_all(w.path().join("env")).unwrap();
850+
std::fs::write(w.path().join("env/linker"), "L=node-modules\n").unwrap();
851+
let proj = w.path().join("packages/app");
852+
std::fs::create_dir_all(&proj).unwrap();
853+
let env = bare_env(None).with_env_files(&proj, None);
854+
assert_eq!(
855+
yarn_node_linker_in(&proj, &env).as_deref(),
856+
Some("node-modules")
857+
);
858+
// Yarn concatenates every rc file's list: a nearer `[]` clears
859+
// nothing (yarn 4.9.1 resolves `node-modules` here)...
860+
std::fs::write(proj.join(".yarnrc.yml"), "injectEnvironmentFiles: []\n").unwrap();
861+
let env = bare_env(None).with_env_files(&proj, None);
862+
assert_eq!(
863+
yarn_node_linker_in(&proj, &env).as_deref(),
864+
Some("node-modules")
865+
);
866+
// ...and a nearer file loads last, so its value wins.
867+
std::fs::write(
868+
proj.join(".yarnrc.yml"),
869+
"injectEnvironmentFiles: [.env.app]\n",
870+
)
871+
.unwrap();
872+
std::fs::write(proj.join(".env.app"), "L=pnpm\n").unwrap();
873+
let env = bare_env(None).with_env_files(&proj, None);
874+
assert_eq!(yarn_node_linker_in(&proj, &env).as_deref(), Some("pnpm"));
875+
}
876+
714877
/// yarn reads the home folder's rc file after every project-side one,
715878
/// and `YARN_RC_FILENAME` renames the rc file it looks for everywhere.
716879
#[test]

0 commit comments

Comments
 (0)