From 9a02e8de875d7f1fc11cf7207c76a0cc1dbcb207 Mon Sep 17 00:00:00 2001
From: Abhisek Datta <abhisek.datta@gmail.com>
Date: Sat, 13 Jun 2026 09:56:43 +0530
Subject: [PATCH] Respect explicitly set CI environment variable (#336)

* fix(proxy): do not override explicitly-set CI env var

pmg forces CI=true for non-interactive (non-PTY) proxy runs so package
managers behave non-interactively. This clobbered a CI value the user set
explicitly (e.g. CI=false on a build server), changing downstream tool
behavior unexpectedly.

Only inject CI=true when CI is not already present in the environment,
preserving the user's intent. mergeEnv override semantics are left intact
since other overrides (HTTP_PROXY, etc.) must clobber.

Fixes #335

* test(proxy): snapshot/restore CI env explicitly in override test

Address review feedback: make the unset-CI subtest's intent explicit by
snapshotting the original CI value, unsetting it for the test, and
restoring it in t.Cleanup instead of relying on t.Setenv cleanup.

---------

Co-authored-by: Claude <noreply@anthropic.com>
---
 internal/flows/proxy_flow.go          | 12 ++++++++++-
 internal/flows/proxy_flow_env_test.go | 29 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go
index 82e8c8c..394640b 100644
--- a/internal/flows/proxy_flow.go
+++ b/internal/flows/proxy_flow.go
@@ -207,7 +207,7 @@ func (f *proxyFlow) Run(ctx context.Context, args []string, parsedCmd *packagema
 		DryRun:             cfg.DryRun,
 		Mode:               runner.ExecutionModeAuto,
 		EnvOverrides:       f.setupEnvForProxy(proxyAddr, caCertPath),
-		DirectEnvOverrides: []string{"CI=true"},
+		DirectEnvOverrides: ciEnvOverride(),
 		BeforeDirectRun: func() error {
 			log.Debugf("Executing proxy for non interactive TTY")
 
@@ -412,6 +412,16 @@ func (f *proxyFlow) createAndStartProxyServer(
 	return proxyServer, proxyAddr, nil
 }
 
+// ciEnvOverride forces CI=true for non-interactive runs so package managers
+// behave non-interactively. It respects an explicitly set CI value (including
+// CI=false) so we don't clobber the user's intent. See issue #335.
+func ciEnvOverride() []string {
+	if _, ok := os.LookupEnv("CI"); ok {
+		return nil
+	}
+	return []string{"CI=true"}
+}
+
 func (f *proxyFlow) setupEnvForProxy(proxyAddr, caCertPath string) []string {
 	proxyURL := fmt.Sprintf("http://%s", proxyAddr)
 
diff --git a/internal/flows/proxy_flow_env_test.go b/internal/flows/proxy_flow_env_test.go
index 2fb9dca..bcc3e76 100644
--- a/internal/flows/proxy_flow_env_test.go
+++ b/internal/flows/proxy_flow_env_test.go
@@ -1,10 +1,12 @@
 package flows
 
 import (
+	"os"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func envToMap(env []string) map[string]string {
@@ -38,3 +40,30 @@ func TestSetupEnvForProxyConfiguresYarn(t *testing.T) {
 	assert.Equal(t, caCertPath, env["YARN_HTTPS_CA_FILE_PATH"],
 		"yarn ignores NODE_EXTRA_CA_CERTS; YARN_HTTPS_CA_FILE_PATH is required to trust the MITM CA")
 }
+
+// TestCIEnvOverride proves the fix for #335: pmg forces CI=true for
+// non-interactive runs but must not clobber a CI value the user set
+// explicitly (e.g. CI=false on a build server).
+func TestCIEnvOverride(t *testing.T) {
+	t.Run("sets CI=true when unset", func(t *testing.T) {
+		original, hadCI := os.LookupEnv("CI")
+		require.NoError(t, os.Unsetenv("CI"))
+		t.Cleanup(func() {
+			if hadCI {
+				require.NoError(t, os.Setenv("CI", original))
+			}
+		})
+
+		assert.Equal(t, []string{"CI=true"}, ciEnvOverride())
+	})
+
+	t.Run("does not override explicitly set CI", func(t *testing.T) {
+		t.Setenv("CI", "false")
+		assert.Nil(t, ciEnvOverride())
+	})
+
+	t.Run("does not override CI set to empty", func(t *testing.T) {
+		t.Setenv("CI", "")
+		assert.Nil(t, ciEnvOverride())
+	})
+}