Extend envutil for bool/string access and remove direct CI bypasses

pkg/cli/add_interactive_orchestrator.go

@@ -10,6 +10,7 @@ import (
 	"charm.land/huh/v2"
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/envutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -70,8 +71,11 @@ type AddInteractiveConfig struct {
 func RunAddInteractive(ctx context.Context, config *AddInteractiveConfig) error {
 	addInteractiveLog.Print("Starting interactive add workflow")
 
-	// Assert this function is not running in automated unit tests or CI
-	if os.Getenv("GO_TEST_MODE") == "true" || os.Getenv("CI") != "" { //nolint:osgetenvlibrary
+	// Assert this function is not running in automated unit tests or CI.
+	// GO_TEST_MODE intentionally uses GetBoolFromEnv so common boolean spellings
+	// are treated consistently across test and automation environments, while
+	// IsRunningInCI centralizes the broader CI environment detection logic.
+	if envutil.GetBoolFromEnv("GO_TEST_MODE", false, addInteractiveLog) || IsRunningInCI() {
 		return errors.New("interactive add cannot be used in automated tests or CI environments")
 	}
 

pkg/cli/add_interactive_workflow.go

@@ -87,7 +87,7 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error
 	}
 
 	// In Codespaces, don't offer to trigger - provide link to Actions page instead
-	if os.Getenv("CODESPACES") == "true" { //nolint:osgetenvlibrary
+	if isRunningInCodespace() {
 		addInteractiveLog.Print("Running in Codespaces, skipping run offer and showing Actions link")
 		fmt.Fprintln(os.Stderr, "")
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running in GitHub Codespaces - please trigger the workflow manually from the Actions page"))

pkg/cli/add_wizard_command.go

@@ -2,7 +2,6 @@ package cli
 
 import (
 	"errors"
-	"os"
 
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
@@ -84,7 +83,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`,
 
 			// add-wizard requires an interactive terminal
 			isTerminal := tty.IsStdoutTerminal()
-			isCIEnv := os.Getenv("CI") != "" //nolint:osgetenvlibrary
+			isCIEnv := IsRunningInCI()
 			addWizardLog.Printf("Terminal check: is_terminal=%v, is_ci=%v", isTerminal, isCIEnv)
 			if !isTerminal || isCIEnv {
 				return errors.New("add-wizard requires an interactive terminal; use 'add' for non-interactive environments")

pkg/cli/codespace.go

@@ -1,10 +1,10 @@
 package cli
 
 import (
-	"os"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/envutil"
 	"github.com/github/gh-aw/pkg/logger"
 )
 
@@ -14,7 +14,7 @@ var codespaceLog = logger.New("cli:codespace")
 // by checking for the CODESPACES environment variable
 func isRunningInCodespace() bool {
 	// GitHub Codespaces sets CODESPACES=true environment variable
-	isCodespace := strings.EqualFold(os.Getenv("CODESPACES"), "true") //nolint:osgetenvlibrary
+	isCodespace := strings.EqualFold(envutil.GetStringFromEnv("CODESPACES", "", codespaceLog), "true")
 	codespaceLog.Printf("Codespace detection: is_codespace=%v", isCodespace)
 	return isCodespace
 }

pkg/cli/interactive.go

@@ -15,6 +15,7 @@ import (
 	"charm.land/huh/v2"
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/envutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/setutil"
 	"github.com/github/gh-aw/pkg/sliceutil"
@@ -63,8 +64,11 @@ func (b *InteractiveWorkflowBuilder) ensureNonTTYScanner(r io.Reader) *bufio.Sca
 func CreateWorkflowInteractively(ctx context.Context, workflowName string, verbose bool, force bool) error {
 	interactiveLog.Printf("Starting interactive workflow creation: workflowName=%s, force=%v", workflowName, force)
 
-	// Assert this function is not running in automated unit tests
-	if os.Getenv("GO_TEST_MODE") == "true" || os.Getenv("CI") != "" { //nolint:osgetenvlibrary
+	// Assert this function is not running in automated unit tests or CI.
+	// GO_TEST_MODE intentionally uses GetBoolFromEnv so common boolean spellings
+	// are treated consistently across test and automation environments, while
+	// IsRunningInCI centralizes the broader CI environment detection logic.
+	if envutil.GetBoolFromEnv("GO_TEST_MODE", false, interactiveLog) || IsRunningInCI() {
 		return errors.New("interactive workflow creation cannot be used in automated tests or CI environments")
 	}
 

pkg/envutil/README.md

@@ -1,12 +1,12 @@
 # envutil Package
 
-> Reads and validates integer-valued environment variables with bounds checking.
+> Reads and validates environment variables with consistent default handling.
 
 ## Overview
 
-The `envutil` package centralizes the pattern of reading integer-valued environment variables, validating them against configured minimum and maximum bounds, and falling back to a default value when the variable is absent or out of range. It emits warning messages to stderr when an invalid value is encountered, following the console formatting conventions of the rest of the codebase.
+The `envutil` package centralizes the pattern of reading environment variables, validating typed values where needed, and falling back to a default value when the variable is absent or invalid. It emits warning messages to stderr when an invalid typed value is encountered, following the console formatting conventions of the rest of the codebase.
 
-The package exposes a single generic helper, `GetIntFromEnv`, which encapsulates the repetitive read-parse-validate-default logic required wherever configurable integer parameters are exposed via environment variables.
+The package exposes helpers for integer, boolean, and string values so callers can avoid scattered `os.Getenv` checks while preserving consistent defaulting and debug logging behavior.
 
 ## Public API
 
@@ -15,6 +15,8 @@ The package exposes a single generic helper, `GetIntFromEnv`, which encapsulates
 | Function | Signature | Description |
 |----------|-----------|-------------|
 | `GetIntFromEnv` | `func(envVar string, defaultValue, minValue, maxValue int, debugLog *logger.Logger) int` | Reads an integer-valued environment variable, validates it, and returns a default when absent or invalid |
+| `GetBoolFromEnv` | `func(envVar string, defaultValue bool, debugLog *logger.Logger) bool` | Reads a boolean-valued environment variable and returns a default when absent or invalid |
+| `GetStringFromEnv` | `func(envVar, defaultValue string, debugLog *logger.Logger) string` | Reads a string-valued environment variable and returns a default when absent |
 
 #### `GetIntFromEnv`
 
@@ -40,6 +42,46 @@ Reads `envVar` from the process environment, parses it as an integer, validates
 - SHOULD emit warnings formatted via `console.FormatWarningMessage` to `os.Stderr` when `debugLog` is `nil`.
 - MAY route warnings through `debugLog.Printf` when `debugLog` is non-nil instead of writing directly to stderr.
 
+#### `GetBoolFromEnv`
+
+```go
+func GetBoolFromEnv(envVar string, defaultValue bool, debugLog *logger.Logger) bool
+```
+
+Reads `envVar` from the process environment, parses it as a boolean using Go's standard `strconv.ParseBool` rules, and returns `defaultValue` when the variable is absent or unparseable.
+
+| Parameter | Type | Description |
+|-----------|------|-------------|
+| `envVar` | `string` | Environment variable name (e.g. `"CI"`) |
+| `defaultValue` | `bool` | Value returned when env var is absent or invalid |
+| `debugLog` | `*logger.Logger` | Optional logger for debug output; pass `nil` to disable |
+
+**Behavioral contract**:
+- MUST return `defaultValue` when the environment variable is not set (empty string).
+- MUST return `defaultValue` and emit a warning when the value cannot be parsed as a boolean.
+- MUST log the accepted value via `debugLog` when `debugLog` is non-nil.
+- SHOULD emit warnings formatted via `console.FormatWarningMessage` to `os.Stderr` when `debugLog` is `nil`.
+- MAY route warnings through `debugLog.Printf` when `debugLog` is non-nil instead of writing directly to stderr.
+
+#### `GetStringFromEnv`
+
+```go
+func GetStringFromEnv(envVar, defaultValue string, debugLog *logger.Logger) string
+```
+
+Reads `envVar` from the process environment and returns `defaultValue` when the variable is absent or empty.
+
+| Parameter | Type | Description |
+|-----------|------|-------------|
+| `envVar` | `string` | Environment variable name (e.g. `"GITHUB_TOKEN"`) |
+| `defaultValue` | `string` | Value returned when env var is absent or empty |
+| `debugLog` | `*logger.Logger` | Optional logger for debug output; pass `nil` to disable |
+
+**Behavioral contract**:
+- MUST return `defaultValue` when the environment variable is not set (empty string).
+- MUST return the environment variable value unchanged when it is non-empty.
+- SHOULD log only that the variable was found, without logging its value, via `debugLog` when `debugLog` is non-nil.
+
 ## Usage Examples
 
 ```go
@@ -53,19 +95,26 @@ var log = logger.New("mypackage:config")
 // Read GH_AW_MAX_CONCURRENT_DOWNLOADS, constrained to [1, 20], default 5
 concurrency := envutil.GetIntFromEnv("GH_AW_MAX_CONCURRENT_DOWNLOADS", 5, 1, 20, log)
 
+// Read CI, defaulting to false when unset or invalid
+isCI := envutil.GetBoolFromEnv("CI", false, log)
+
+// Read GITHUB_TOKEN, defaulting to empty string when unset
+token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", log)
+
 // Suppress debug output by passing nil logger
 timeout := envutil.GetIntFromEnv("GH_AW_TIMEOUT", 60, 1, 3600, nil)
 ```
 
 ## Thread Safety
 
-`GetIntFromEnv` is safe for concurrent use. It holds no shared mutable state; each invocation reads the process environment via `os.Getenv` and operates on function-local variables only.
+`GetIntFromEnv`, `GetBoolFromEnv`, and `GetStringFromEnv` are safe for concurrent use. They hold no shared mutable state; each invocation reads the process environment via `os.Getenv` and operates on function-local variables only.
 
 ## Design Decisions
 
 - Warning messages use `console.FormatWarningMessage` so they render consistently in terminals.
 - All warnings go to `os.Stderr` to avoid polluting structured stdout output.
-- The function handles integers only; floating-point or string env vars should be read directly via `os.Getenv`.
+- Typed helpers use Go standard library parsing rules (`strconv.Atoi`, `strconv.ParseBool`) for predictable behavior.
+- `GetStringFromEnv` logs a message of the form `Using ENV_VAR from environment` when a value is found, never the value itself, so secret-like values can be read without echoing their contents.
 - When `debugLog` is non-nil, warnings are routed through the logger rather than written directly to stderr, allowing callers to control output formatting.
 
 ## Dependencies

pkg/envutil/envutil.go

@@ -10,6 +10,17 @@ import (
 	"github.com/github/gh-aw/pkg/logger"
 )
 
+// warn is an internal helper shared by GetIntFromEnv and GetBoolFromEnv. It
+// routes environment parsing warnings through the provided logger when
+// available, or to stderr using the standard console warning format otherwise.
+func warn(debugLog *logger.Logger, msg string) {
+	if debugLog != nil {
+		debugLog.Printf("WARNING: %s", msg)
+		return
+	}
+	fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
+}
+
 // GetIntFromEnv is a generic helper that reads an integer value from an environment variable,
 // validates it against min/max bounds, and returns a default value if invalid.
 // This follows the configuration helper pattern from pkg/workflow/config_helpers.go.
@@ -19,7 +30,7 @@ import (
 //   - defaultValue: The default value to return if env var is not set or invalid
 //   - minValue: Minimum allowed value (inclusive)
 //   - maxValue: Maximum allowed value (inclusive)
-//   - log: Optional logger for debug output
+//   - debugLog: Optional logger for debug output
 //
 // Returns the parsed integer value, or defaultValue if:
 //   - Environment variable is not set
@@ -28,27 +39,19 @@ import (
 //
 // Invalid values trigger warning messages to stderr, or through the logger if provided.
 func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, debugLog *logger.Logger) int {
-	warn := func(msg string) {
-		if debugLog != nil {
-			debugLog.Printf("WARNING: %s", msg)
-		} else {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg))
-		}
-	}
-
 	envValue := os.Getenv(envVar) //nolint:osgetenvlibrary
 	if envValue == "" {
 		return defaultValue
 	}
 
 	val, err := strconv.Atoi(envValue)
 	if err != nil {
-		warn(fmt.Sprintf("Invalid %s value '%s' (must be a number), using default %d", envVar, envValue, defaultValue))
+		warn(debugLog, fmt.Sprintf("Invalid %s value '%s' (must be a number), using default %d", envVar, envValue, defaultValue))
 		return defaultValue
 	}
 
 	if val < minValue || val > maxValue {
-		warn(fmt.Sprintf("%s value %d is out of bounds (must be %d-%d), using default %d", envVar, val, minValue, maxValue, defaultValue))
+		warn(debugLog, fmt.Sprintf("%s value %d is out of bounds (must be %d-%d), using default %d", envVar, val, minValue, maxValue, defaultValue))
 		return defaultValue
 	}
 
@@ -57,3 +60,56 @@ func GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, debugLog
 	}
 	return val
 }
+
+// GetBoolFromEnv reads a boolean value from an environment variable and returns
+// defaultValue if the variable is not set or cannot be parsed as a boolean.
+//
+// Parameters:
+//   - envVar: The environment variable name (e.g., "CI")
+//   - defaultValue: The default value to return if env var is not set or invalid
+//   - debugLog: Optional logger for debug output
+//
+// Returns the parsed boolean value, or defaultValue if:
+//   - Environment variable is not set
+//   - Value cannot be parsed as a boolean
+//
+// Invalid values trigger warning messages to stderr, or through the logger if provided.
+func GetBoolFromEnv(envVar string, defaultValue bool, debugLog *logger.Logger) bool {
+	envValue := os.Getenv(envVar) //nolint:osgetenvlibrary
+	if envValue == "" {
+		return defaultValue
+	}
+
+	val, err := strconv.ParseBool(envValue)
+	if err != nil {
+		warn(debugLog, fmt.Sprintf("Invalid %s value '%s' (must be a boolean), using default %t", envVar, envValue, defaultValue))
+		return defaultValue
+	}
+
+	if debugLog != nil {
+		debugLog.Printf("Using %s=%t", envVar, val)
+	}
+	return val
+}
+
+// GetStringFromEnv reads a string value from an environment variable and
+// returns defaultValue if the variable is not set.
+//
+// Parameters:
+//   - envVar: The environment variable name (e.g., "GITHUB_TOKEN")
+//   - defaultValue: The default value to return if env var is not set
+//   - debugLog: Optional logger for debug output
+//
+// Returns the environment variable value, or defaultValue when the variable is
+// not set. Empty string values are treated the same as unset variables.
+func GetStringFromEnv(envVar, defaultValue string, debugLog *logger.Logger) string {
+	envValue := os.Getenv(envVar) //nolint:osgetenvlibrary
+	if envValue == "" {
+		return defaultValue
+	}
+
+	if debugLog != nil {
+		debugLog.Printf("Using %s from environment", envVar)
+	}
+	return envValue
+}