fix: avoid overwriting existing env vars (#583)

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>

Author: exKAZUu

packages/shared-lib-node/src/env.ts

@@ -82,35 +82,35 @@ export function readEnvironmentVariables(
     console.info('Reading env files:', envPaths.join(', '));
   }
 
-  const envPathAndEnvVarCountPairs: [string, string[]][] = [];
+  const envPathAndLoadedEnvVarNames: [string, string[]][] = [];
   const envVars: Record<string, string> = {};
   for (const envPath of envPaths) {
     const keys: string[] = [];
     for (const [key, value] of Object.entries(readEnvFile(path.join(cwd, envPath)))) {
-      if (!(key in envVars)) {
+      if (!(key in envVars) && !(key in process.env)) {
         envVars[key] = value;
         keys.push(key);
       }
     }
-    envPathAndEnvVarCountPairs.push([envPath, keys]);
+    envPathAndLoadedEnvVarNames.push([envPath, keys]);
     if (argv.verbose && keys.length > 0) {
       console.info(`Read ${keys.length} environment variables from ${envPath}`);
     }
   }
   if (!argv.verbose) {
     console.info(
-      `Read: ${envPathAndEnvVarCountPairs.map(([envPath, keys]) => `${envPath} (${keys.join(', ')})`).join(', ')}`
+      `Read: ${envPathAndLoadedEnvVarNames.map(([envPath, keys]) => `${envPath} (${keys.join(', ')})`).join(', ')}`
     );
   }
 
   if (argv.checkEnv) {
     const exampleKeys = Object.keys(readEnvFile(path.join(cwd, argv.checkEnv)));
-    const missingKeys = exampleKeys.filter((key) => !(key in envVars));
+    const missingKeys = exampleKeys.filter((key) => !(key in envVars) && !(key in process.env));
     if (missingKeys.length > 0) {
       throw new Error(`Missing environment variables in [${envPaths.join(', ')}]: [${missingKeys.join(', ')}]`);
     }
   }
-  return [expand({ parsed: envVars, processEnv: {} }).parsed ?? envVars, envPathAndEnvVarCountPairs];
+  return [expand({ parsed: envVars, processEnv: {} }).parsed ?? envVars, envPathAndLoadedEnvVarNames];
 }
 
 /**
@@ -121,7 +121,11 @@ export function readAndApplyEnvironmentVariables(
   cwd: string
 ): Record<string, string | undefined> {
   const [envVars] = readEnvironmentVariables(argv, cwd);
-  Object.assign(process.env, envVars);
+  for (const [key, value] of Object.entries(envVars)) {
+    if (!(key in process.env)) {
+      process.env[key] = value;
+    }
+  }
   return envVars;
 }
 

packages/shared-lib-node/test/env.test.ts

@@ -1,13 +1,28 @@
-import { beforeEach, describe, expect, it } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
 
-import { readAndApplyEnvironmentVariables } from '../src/env.js';
+import { readAndApplyEnvironmentVariables, readEnvironmentVariables } from '../src/env.js';
 
-describe('readAndApplyEnvironmentVariables()', () => {
-  beforeEach(() => {
-    process.env.WB_ENV = '';
-    process.env.NODE_ENV = '';
-  });
+const originalEnv = { ...process.env };
+
+beforeEach(() => {
+  process.env.WB_ENV = '';
+  process.env.NODE_ENV = '';
+  // Clear env vars that could affect env loading behavior in tests.
+
+  delete process.env.PORT;
+
+  delete process.env.NAME;
+});
+
+afterEach(() => {
+  for (const key of Object.keys(process.env)) {
+    // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+    delete process.env[key];
+  }
+  Object.assign(process.env, originalEnv);
+});
 
+describe('readAndApplyEnvironmentVariables()', () => {
   it('should load no env vars with empty options', () => {
     const envVars = readAndApplyEnvironmentVariables({}, 'test/fixtures/app1');
     expect(envVars).toEqual({});
@@ -44,4 +59,32 @@ describe('readAndApplyEnvironmentVariables()', () => {
     );
     expect(envVars).toEqual({ ENV: 'test2', PORT: '4002', NAME: 'app2' });
   });
+
+  it('should not overwrite existing process.env values', () => {
+    process.env.PORT = '9999';
+    process.env.NAME = 'override';
+    const envVars = readAndApplyEnvironmentVariables({ autoCascadeEnv: true }, 'test/fixtures/app1');
+    expect(envVars).toEqual({ ENV: 'development1' });
+    expect(process.env.ENV).toBe('development1');
+    expect(process.env.PORT).toBe('9999');
+    expect(process.env.NAME).toBe('override');
+  });
+});
+
+describe('readEnvironmentVariables()', () => {
+  it('should skip existing process.env values in env vars and env files list', () => {
+    process.env.PORT = '9999';
+    process.env.NAME = 'override';
+    const [envVars, envPathAndLoadedEnvVarNames] = readEnvironmentVariables(
+      { autoCascadeEnv: true },
+      'test/fixtures/app1'
+    );
+    expect(envVars).toEqual({ ENV: 'development1' });
+    expect(envPathAndLoadedEnvVarNames).toEqual([
+      ['.env.development', ['ENV']],
+      ['.env', []],
+    ]);
+    expect(process.env.PORT).toBe('9999');
+    expect(process.env.NAME).toBe('override');
+  });
 });