diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 7d57016c36d..3e9dce72b5f 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -67,7 +67,7 @@ func (c *AddInteractiveConfig) selectAIEngineAndKey() error { if opt.EnvVarName != "" { envVar = opt.EnvVarName } - if os.Getenv(envVar) != "" { + if lookupEnv(envVar) != "" { defaultEngine = opt.Value addInteractiveLog.Printf("Found env var %s, recommending engine: %s", envVar, opt.Value) break diff --git a/pkg/cli/outcomes_command.go b/pkg/cli/outcomes_command.go index 304690cd8e7..45ccb7ff133 100644 --- a/pkg/cli/outcomes_command.go +++ b/pkg/cli/outcomes_command.go @@ -173,7 +173,7 @@ func RunOutcomes(config OutcomesConfig) error { // The --outcomes-dir flag takes precedence over the GH_AW_OUTCOMES_DIR env var. outcomesDir := config.OutcomesDir if outcomesDir == "" { - outcomesDir, _ = os.LookupEnv("GH_AW_OUTCOMES_DIR") + outcomesDir = lookupEnv("GH_AW_OUTCOMES_DIR") } if outcomesDir != "" { writeOutcomeJSONL(outcomesDir, config.RunID, reports) diff --git a/pkg/cli/process_env_lookup.go b/pkg/cli/process_env_lookup.go new file mode 100644 index 00000000000..ca4a64e1f28 --- /dev/null +++ b/pkg/cli/process_env_lookup.go @@ -0,0 +1,40 @@ +package cli + +import ( + "os" + "sync" +) + +type envLookupFunc func(string) (string, bool) + +var ( + processEnvLookupMu sync.RWMutex + processEnvLookup envLookupFunc = os.LookupEnv +) + +// SetProcessEnvLookup configures how CLI helpers resolve environment values. +// Passing nil restores the default process environment lookup. +func SetProcessEnvLookup(lookup func(string) (string, bool)) { + processEnvLookupMu.Lock() + defer processEnvLookupMu.Unlock() + if lookup == nil { + processEnvLookup = os.LookupEnv + return + } + processEnvLookup = lookup +} + +func lookupEnv(key string) string { + processEnvLookupMu.RLock() + defer processEnvLookupMu.RUnlock() + // Intentionally ignore the existence flag to preserve os.Getenv semantics: + // missing variables and explicitly empty variables are both treated as "". + value, _ := processEnvLookup(key) + return value +} + +func lookupEnvOk(key string) (string, bool) { + processEnvLookupMu.RLock() + defer processEnvLookupMu.RUnlock() + return processEnvLookup(key) +} diff --git a/pkg/cli/process_env_lookup_test.go b/pkg/cli/process_env_lookup_test.go new file mode 100644 index 00000000000..709c22dc9d8 --- /dev/null +++ b/pkg/cli/process_env_lookup_test.go @@ -0,0 +1,67 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSetProcessEnvLookup(t *testing.T) { + t.Cleanup(func() { SetProcessEnvLookup(nil) }) + + SetProcessEnvLookup(func(key string) (string, bool) { + env := map[string]string{ + "PRESENT": "hello", + "EMPTY": "", + } + v, ok := env[key] + return v, ok + }) + + t.Run("lookupEnv returns value for present key", func(t *testing.T) { + assert.Equal(t, "hello", lookupEnv("PRESENT")) + }) + + t.Run("lookupEnv returns empty string for missing key", func(t *testing.T) { + assert.Empty(t, lookupEnv("MISSING")) + }) + + t.Run("lookupEnv returns empty string for explicitly empty key", func(t *testing.T) { + assert.Empty(t, lookupEnv("EMPTY")) + }) + + t.Run("lookupEnvOk returns value and true for present key", func(t *testing.T) { + val, ok := lookupEnvOk("PRESENT") + assert.Equal(t, "hello", val) + assert.True(t, ok) + }) + + t.Run("lookupEnvOk returns empty string and false for missing key", func(t *testing.T) { + val, ok := lookupEnvOk("MISSING") + assert.Empty(t, val) + assert.False(t, ok) + }) + + t.Run("lookupEnvOk returns empty string and true for explicitly empty key", func(t *testing.T) { + val, ok := lookupEnvOk("EMPTY") + assert.Empty(t, val) + assert.True(t, ok) + }) +} + +func TestSetProcessEnvLookupNilRestoresDefault(t *testing.T) { + t.Cleanup(func() { SetProcessEnvLookup(nil) }) + + SetProcessEnvLookup(func(key string) (string, bool) { + return "overridden", true + }) + + SetProcessEnvLookup(nil) + + // After restoring the default, missing keys should return "" / false. + val, ok := lookupEnvOk("__GH_AW_TEST_KEY_THAT_MUST_NOT_EXIST__") + assert.Empty(t, val) + assert.False(t, ok) +} diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index ef6b1de40c6..8d2bc53b431 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -2,7 +2,6 @@ package workflow import ( "context" - "os" actionpins "github.com/github/gh-aw/pkg/actionpins" "github.com/github/gh-aw/pkg/logger" @@ -228,7 +227,7 @@ func (d *WorkflowData) PinContext() *actionpins.PinContext { // for example from auto-detected git remotes). Mirror setupGHCommand's // (github_cli.go) precedence: GH_HOST wins when present; default host is // only consulted when GH_HOST is absent. - if ghHost := os.Getenv("GH_HOST"); ghHost != "" { + if ghHost := lookupProcessEnv("GH_HOST"); ghHost != "" { if ghHost != "github.com" { workflowDataLog.Print("Non-github.com GH_HOST detected; disabling hardcoded action pin fallback") pinCtx.SkipHardcodedFallback = true