[build-tools] Let updateEnv override existing values (#3901)

## Summary
- Change `BuildContext.updateEnv` so incoming values override existing env values.
- Add a regression test covering overrides and clearing via undefined.

## Why
This makes `updateEnv` behave like an update operation and lets callers intentionally refresh or clear existing environment entries.

No idea how it existed so long like this. I guess the values we update with don't overlap with the values we already have that often.

## Test Plan

CI should pass.

Author: sjchmiela

packages/build-tools/src/__tests__/context.test.ts

@@ -19,10 +19,12 @@ jest.mock('fs-extra');
 jest.mock('../datadog', () => ({
   Datadog: {
     distribution: jest.fn(),
+    log: jest.fn(),
   },
 }));
 
 const datadogDistributionMock = jest.mocked(Datadog.distribution);
+const datadogLogMock = jest.mocked(Datadog.log);
 
 describe('BuildContext', () => {
   beforeEach(() => {
@@ -139,6 +141,77 @@ describe('BuildContext', () => {
     });
   });
 
+  it('overwrites existing environment variables when updating env', async () => {
+    await vol.promises.mkdir('/workingdir/eas-environment-secrets/', { recursive: true });
+
+    const ctx = new BuildContext(
+      {
+        triggeredBy: BuildTrigger.GIT_BASED_INTEGRATION,
+        secrets: {},
+      } as Job,
+      {
+        env: {
+          __API_SERVER_URL: 'http://api.expo.test',
+          EXISTING_ENV: 'old-value',
+          REMOVED_ENV: 'old-value',
+        },
+        workingdir: '/workingdir',
+        logger: createMockLogger(),
+        logBuffer: { getLogs: () => [], getPhaseLogs: () => [] },
+        uploadArtifact: jest.fn(),
+      }
+    );
+
+    ctx.updateEnv({
+      EXISTING_ENV: 'new-value',
+      REMOVED_ENV: undefined,
+      NEW_ENV: 'new-env-value',
+    });
+
+    expect(ctx.env.EXISTING_ENV).toBe('new-value');
+    expect(ctx.env.REMOVED_ENV).toBeUndefined();
+    expect(ctx.env.NEW_ENV).toBe('new-env-value');
+    const logMessage = datadogLogMock.mock.calls[0][0];
+    expect(logMessage).toContain('BuildContext.updateEnv merge order produced different env');
+    expect(logMessage).toContain('different_env_names=');
+    expect(logMessage).toContain('EXISTING_ENV');
+    expect(logMessage).toContain('REMOVED_ENV');
+    expect(JSON.stringify(datadogLogMock.mock.calls)).not.toContain('old-value');
+    expect(JSON.stringify(datadogLogMock.mock.calls)).not.toContain('new-value');
+    expect(JSON.stringify(datadogLogMock.mock.calls)).not.toContain('new-env-value');
+  });
+
+  it('logs when updateEnv merge order produces the same env without values', async () => {
+    await vol.promises.mkdir('/workingdir/eas-environment-secrets/', { recursive: true });
+
+    const ctx = new BuildContext(
+      {
+        triggeredBy: BuildTrigger.GIT_BASED_INTEGRATION,
+        secrets: {},
+      } as Job,
+      {
+        env: {
+          __API_SERVER_URL: 'http://api.expo.test',
+          EXISTING_ENV: 'same-value',
+        },
+        workingdir: '/workingdir',
+        logger: createMockLogger(),
+        logBuffer: { getLogs: () => [], getPhaseLogs: () => [] },
+        uploadArtifact: jest.fn(),
+      }
+    );
+
+    ctx.updateEnv({
+      EXISTING_ENV: 'same-value',
+      UNSET_ENV: undefined,
+    });
+
+    expect(datadogLogMock).toHaveBeenCalledWith(
+      'BuildContext.updateEnv merge order produced same env'
+    );
+    expect(JSON.stringify(datadogLogMock.mock.calls)).not.toContain('same-value');
+  });
+
   it('emits build phase duration metrics for successful build phases', async () => {
     const ctx = createTestBuildContext({ platform: Platform.IOS });
 

packages/build-tools/src/context.ts

@@ -72,6 +72,41 @@ export interface BuildContextOptions {
 
 export class SkipNativeBuildError extends Error {}
 
+function logEnvComparison({
+  oldMergeOrderEnv,
+  newMergeOrderEnv,
+}: {
+  oldMergeOrderEnv: Env;
+  newMergeOrderEnv: Env;
+}): void {
+  const envNames = new Set([...Object.keys(oldMergeOrderEnv), ...Object.keys(newMergeOrderEnv)]);
+  const sameEnvNames: string[] = [];
+  const differentEnvNames: string[] = [];
+
+  for (const key of envNames) {
+    const hasOldValue = key in oldMergeOrderEnv;
+    const hasNewValue = key in newMergeOrderEnv;
+    if (hasOldValue === hasNewValue && oldMergeOrderEnv[key] === newMergeOrderEnv[key]) {
+      sameEnvNames.push(key);
+    } else {
+      differentEnvNames.push(key);
+    }
+  }
+
+  if (differentEnvNames.length === 0) {
+    Datadog.log('BuildContext.updateEnv merge order produced same env');
+    return;
+  }
+
+  Datadog.log(
+    `BuildContext.updateEnv merge order produced different env: compared_env_count=${
+      envNames.size
+    } different_env_count=${differentEnvNames.length} different_env_names=${differentEnvNames.join(
+      ','
+    )} same_env_count=${sameEnvNames.length} same_env_names=${sameEnvNames.join(',')}`
+  );
+}
+
 export class BuildContext<TJob extends Job = Job> {
   public readonly workingdir: string;
   public logger: bunyan;
@@ -247,11 +282,24 @@ export class BuildContext<TJob extends Job = Job> {
         'Updating environment variables is only allowed when build was triggered by a git-based integration.'
       );
     }
-    this._env = {
+
+    const oldMergeOrderEnv: Env = {
       ...env,
       ...this._env,
       __EAS_BUILD_ENVS_DIR: this.buildEnvsDirectory,
     };
+    const newMergeOrderEnv: Env = {
+      ...this._env,
+      ...env,
+      __EAS_BUILD_ENVS_DIR: this.buildEnvsDirectory,
+    };
+
+    logEnvComparison({
+      oldMergeOrderEnv,
+      newMergeOrderEnv,
+    });
+
+    this._env = newMergeOrderEnv;
     this._env.PATH = this._env.PATH
       ? [this.buildExecutablesDirectory, this._env.PATH].join(':')
       : this.buildExecutablesDirectory;