From 6f976f6a2b90b2c218a93f2dca480764d8da6ce5 Mon Sep 17 00:00:00 2001
From: James McMeeking <james.mcmeeking@joingelt.com>
Date: Thu, 2 Apr 2026 13:03:01 +0100
Subject: [PATCH] Address PR comments

---
 .../rush/createRushPackageManager.js          | 30 ++++++++-----
 .../src/packagemanager/rush/runRushCommand.js | 44 +++++++++++--------
 .../rush/runRushCommand.spec.js               | 18 ++++++++
 3 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/packages/safe-chain/src/packagemanager/rush/createRushPackageManager.js b/packages/safe-chain/src/packagemanager/rush/createRushPackageManager.js
index 1a4aebb..16c5815 100644
--- a/packages/safe-chain/src/packagemanager/rush/createRushPackageManager.js
+++ b/packages/safe-chain/src/packagemanager/rush/createRushPackageManager.js
@@ -22,23 +22,29 @@ async function scanRushAddCommand(args) {
     return [];
   }
 
-  const packageSpecs = extractRushAddPackageSpecs(args);
+  const parsedSpecs = extractRushAddPackageSpecs(args)
+    .map((spec) => parsePackageSpec(spec))
+    .filter((spec) => spec !== null);
+
+  const resolvedVersions = await Promise.all(
+    parsedSpecs.map(async (parsed) => {
+      const exactVersion = await resolvePackageVersion(parsed.name, parsed.version);
+      return {
+        parsed,
+        exactVersion,
+      };
+    }),
+  );
+
   const changes = [];
-
-  for (const spec of packageSpecs) {
-    const parsed = parsePackageSpec(spec);
-    if (!parsed) {
-      continue;
-    }
-
-    const exactVersion = await resolvePackageVersion(parsed.name, parsed.version);
-    if (!exactVersion) {
+  for (const resolved of resolvedVersions) {
+    if (!resolved.exactVersion) {
       continue;
     }
 
     changes.push({
-      name: parsed.name,
-      version: exactVersion,
+      name: resolved.parsed.name,
+      version: resolved.exactVersion,
       type: "add",
     });
   }
diff --git a/packages/safe-chain/src/packagemanager/rush/runRushCommand.js b/packages/safe-chain/src/packagemanager/rush/runRushCommand.js
index ebc3bf1..f6ba3cc 100644
--- a/packages/safe-chain/src/packagemanager/rush/runRushCommand.js
+++ b/packages/safe-chain/src/packagemanager/rush/runRushCommand.js
@@ -8,8 +8,9 @@ import { reportCommandExecutionFailure } from "../_shared/commandErrors.js";
  */
 export async function runRushCommand(args) {
   try {
-    const env = mergeSafeChainProxyEnvironmentVariables(process.env);
-    normalizeProxyEnvironmentVariables(env);
+    const env = normalizeProxyEnvironmentVariables(
+      mergeSafeChainProxyEnvironmentVariables(process.env),
+    );
 
     const result = await safeSpawn("rush", args, {
       stdio: "inherit",
@@ -27,37 +28,44 @@ export async function runRushCommand(args) {
  * lowercase or npm/yarn-specific environment variables.
  *
  * @param {Record<string, string>} env
+ * @returns {Record<string, string>}
  */
 function normalizeProxyEnvironmentVariables(env) {
-  if (env.HTTPS_PROXY && !env.HTTP_PROXY) {
-    env.HTTP_PROXY = env.HTTPS_PROXY;
+  const normalized = {
+    ...env,
+  };
+
+  if (normalized.HTTPS_PROXY && !normalized.HTTP_PROXY) {
+    normalized.HTTP_PROXY = normalized.HTTPS_PROXY;
   }
 
-  if (env.HTTP_PROXY && !env.http_proxy) {
-    env.http_proxy = env.HTTP_PROXY;
+  if (normalized.HTTP_PROXY && !normalized.http_proxy) {
+    normalized.http_proxy = normalized.HTTP_PROXY;
   }
 
-  if (env.HTTPS_PROXY && !env.https_proxy) {
-    env.https_proxy = env.HTTPS_PROXY;
+  if (normalized.HTTPS_PROXY && !normalized.https_proxy) {
+    normalized.https_proxy = normalized.HTTPS_PROXY;
   }
 
-  if (env.HTTP_PROXY && !env.npm_config_proxy) {
-    env.npm_config_proxy = env.HTTP_PROXY;
+  if (normalized.HTTP_PROXY && !normalized.npm_config_proxy) {
+    normalized.npm_config_proxy = normalized.HTTP_PROXY;
   }
 
-  if (env.HTTPS_PROXY && !env.npm_config_https_proxy) {
-    env.npm_config_https_proxy = env.HTTPS_PROXY;
+  if (normalized.HTTPS_PROXY && !normalized.npm_config_https_proxy) {
+    normalized.npm_config_https_proxy = normalized.HTTPS_PROXY;
   }
 
-  if (env.HTTP_PROXY && !env.NPM_CONFIG_PROXY) {
-    env.NPM_CONFIG_PROXY = env.HTTP_PROXY;
+  if (normalized.HTTP_PROXY && !normalized.NPM_CONFIG_PROXY) {
+    normalized.NPM_CONFIG_PROXY = normalized.HTTP_PROXY;
   }
 
-  if (env.HTTPS_PROXY && !env.NPM_CONFIG_HTTPS_PROXY) {
-    env.NPM_CONFIG_HTTPS_PROXY = env.HTTPS_PROXY;
+  if (normalized.HTTPS_PROXY && !normalized.NPM_CONFIG_HTTPS_PROXY) {
+    normalized.NPM_CONFIG_HTTPS_PROXY = normalized.HTTPS_PROXY;
   }
 
-  if (env.HTTPS_PROXY && !env.YARN_HTTPS_PROXY) {
-    env.YARN_HTTPS_PROXY = env.HTTPS_PROXY;
+  if (normalized.HTTPS_PROXY && !normalized.YARN_HTTPS_PROXY) {
+    normalized.YARN_HTTPS_PROXY = normalized.HTTPS_PROXY;
   }
+
+  return normalized;
 }
diff --git a/packages/safe-chain/src/packagemanager/rush/runRushCommand.spec.js b/packages/safe-chain/src/packagemanager/rush/runRushCommand.spec.js
index 97676e4..b21087e 100644
--- a/packages/safe-chain/src/packagemanager/rush/runRushCommand.spec.js
+++ b/packages/safe-chain/src/packagemanager/rush/runRushCommand.spec.js
@@ -5,11 +5,13 @@ describe("runRushCommand", () => {
   let runRushCommand;
   let safeSpawnMock;
   let mergeCalls;
+  let mergeResultEnv;
   let nextSpawnStatus;
   let nextSpawnError;
 
   beforeEach(async () => {
     mergeCalls = [];
+    mergeResultEnv = null;
     nextSpawnStatus = 0;
     nextSpawnError = null;
     safeSpawnMock = mock.fn(async () => {
@@ -32,6 +34,10 @@ describe("runRushCommand", () => {
       namedExports: {
         mergeSafeChainProxyEnvironmentVariables: (env) => {
           mergeCalls.push(env);
+          if (mergeResultEnv) {
+            return mergeResultEnv;
+          }
+
           return {
             ...env,
             HTTPS_PROXY: "http://localhost:8080",
@@ -96,4 +102,16 @@ describe("runRushCommand", () => {
 
     assert.strictEqual(res.status, 1);
   });
+
+  it("does not mutate merged env object", async () => {
+    mergeResultEnv = {
+      HTTPS_PROXY: "http://localhost:8080",
+    };
+
+    await runRushCommand(["install"]);
+
+    assert.deepStrictEqual(mergeResultEnv, {
+      HTTPS_PROXY: "http://localhost:8080",
+    });
+  });
 });