refactor: centralize environment variable access

dk19y · dk19y · commit ca8ce95ef375 · 2025-10-29T16:12:26.000-07:00
- Add util::env_var module with wrapper functions
- Replace direct std::env calls throughout codebase
- Preserve all defaults: bash, vi, same logic for all env vars
- Fix recursive calls in system_info functions
diff --git a/crates/chat-cli/src/auth/builder_id.rs b/crates/chat-cli/src/auth/builder_id.rs
@@ -18,6 +18,8 @@
 //!       - RETURNS: [PollCreateToken]
 //!    4. (Repeat) Tokens SHOULD be refreshed if expired and a refresh token is available.
 //!        - Code: [BuilderIdToken::refresh_token]
+
+use crate::util::env_var::is_sigv4_enabled;
 //!          - Calls [Client::create_token]
 //!        - RETURNS: [BuilderIdToken]
 
@@ -545,7 +547,7 @@ pub async fn poll_create_token(
 
 pub async fn is_logged_in(database: &mut Database) -> bool {
     // Check for BuilderId if not using Sigv4
-    if std::env::var("AMAZON_Q_SIGV4").is_ok_and(|v| !v.is_empty()) {
+    if is_sigv4_enabled() {
         debug!("logged in using sigv4 credentials");
         return true;
     }
diff --git a/crates/chat-cli/src/cli/chat/cli/editor.rs b/crates/chat-cli/src/cli/chat/cli/editor.rs
@@ -11,6 +11,7 @@ use crate::cli::chat::{
     ChatState,
 };
 use crate::theme::StyledText;
+use crate::util::env_var::get_editor;
 
 #[deny(missing_docs)]
 #[derive(Debug, PartialEq, Args)]
@@ -86,7 +87,7 @@ impl EditorArgs {
 /// Launch the user's preferred editor with the given file path
 fn launch_editor(file_path: &std::path::Path) -> Result<(), ChatError> {
     // Get the editor from environment variable or use a default
-    let editor_cmd = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
+    let editor_cmd = get_editor();
 
     // Parse the editor command to handle arguments
     let mut parts =
diff --git a/crates/chat-cli/src/cli/chat/tools/delegate.rs b/crates/chat-cli/src/cli/chat/tools/delegate.rs
@@ -13,6 +13,8 @@ use crossterm::{
     style,
 };
 use eyre::{
+
+use crate::util::env_var::get_all_env_vars;
     Result,
     bail,
 };
@@ -333,7 +335,7 @@ pub async fn spawn_agent_process(os: &Os, agent: &str, task: &str) -> Result<Age
     cmd.stdout(std::process::Stdio::piped());
     cmd.stderr(std::process::Stdio::piped());
     cmd.stdin(std::process::Stdio::null()); // No user input
-    cmd.envs(std::env::vars());
+    cmd.envs(get_all_env_vars());
 
     #[cfg(not(windows))]
     cmd.process_group(0);
diff --git a/crates/chat-cli/src/cli/chat/tools/execute/unix.rs b/crates/chat-cli/src/cli/chat/tools/execute/unix.rs
@@ -10,6 +10,7 @@ use tokio::io::AsyncBufReadExt;
 use tokio::select;
 use tracing::error;
 
+use crate::util::env_var::get_chat_shell;
 use super::{
     CommandResult,
     env_vars_with_user_agent,
@@ -30,7 +31,7 @@ pub async fn run_command<W: Write>(
     max_result_size: usize,
     mut updates: Option<W>,
 ) -> Result<CommandResult> {
-    let shell = std::env::var("AMAZON_Q_CHAT_SHELL").unwrap_or("bash".to_string());
+    let shell = get_chat_shell();
 
     // Set up environment variables with user agent metadata for CloudTrail tracking
     let env_vars = env_vars_with_user_agent(os);
diff --git a/crates/chat-cli/src/cli/chat/util/mod.rs b/crates/chat-cli/src/cli/chat/util/mod.rs
@@ -14,6 +14,8 @@ use aws_smithy_types::{
 };
 use eyre::Result;
 
+use crate::util::env_var::get_term;
+
 use super::ChatError;
 use super::token_counter::TokenCounter;
 
@@ -119,7 +121,7 @@ pub fn play_notification_bell(requires_confirmation: bool) {
 /// Determine if we should play the bell based on terminal type
 fn should_play_bell() -> bool {
     // Get the TERM environment variable
-    if let Ok(term) = std::env::var("TERM") {
+    if let Some(term) = get_term() {
         // List of terminals known to handle bell character well
         let bell_compatible_terms = [
             "xterm",
diff --git a/crates/chat-cli/src/cli/mod.rs b/crates/chat-cli/src/cli/mod.rs
@@ -1,4 +1,5 @@
 use crate::theme::StyledText;
+use crate::util::env_var::{is_log_stdout_enabled, get_var};
 mod agent;
 pub mod chat;
 mod debug;
@@ -231,7 +232,7 @@ impl Cli {
                 ),
                 false => None,
             },
-            log_to_stdout: std::env::var_os("Q_LOG_STDOUT").is_some() || self.verbose > 0,
+            log_to_stdout: is_log_stdout_enabled() || self.verbose > 0,
             log_file_path: match subcommand {
                 RootSubcommand::Chat { .. } => Some(logs_dir().expect("home dir must be set").join("qchat.log")),
                 _ => None,
@@ -240,7 +241,7 @@ impl Cli {
         });
 
         // Check for region support.
-        if let Ok(region) = std::env::var("AWS_REGION") {
+        if let Ok(region) = get_var("AWS_REGION") {
             if GOV_REGIONS.contains(&region.as_str()) {
                 bail!("AWS GovCloud ({region}) is not supported.")
             }
diff --git a/crates/chat-cli/src/logging.rs b/crates/chat-cli/src/logging.rs
@@ -14,7 +14,7 @@ use tracing_subscriber::{
     fmt,
 };
 
-use crate::util::env_var::Q_LOG_LEVEL;
+use crate::util::env_var::get_log_level;
 
 const MAX_FILE_SIZE: u64 = 10 * 1024 * 1024;
 const DEFAULT_FILTER: LevelFilter = LevelFilter::ERROR;
@@ -196,7 +196,7 @@ pub fn get_log_level() -> String {
         .lock()
         .unwrap()
         .clone()
-        .unwrap_or_else(|| std::env::var(Q_LOG_LEVEL).unwrap_or_else(|_| DEFAULT_FILTER.to_string()))
+        .unwrap_or_else(|| get_log_level().unwrap_or_else(|_| DEFAULT_FILTER.to_string()))
 }
 
 /// Set the log level to the given level.
@@ -247,7 +247,7 @@ fn create_filter_layer() -> EnvFilter {
         .lock()
         .unwrap()
         .clone()
-        .or_else(|| std::env::var(Q_LOG_LEVEL).ok());
+        .or_else(|| get_log_level().ok());
 
     match log_level {
         Some(level) => EnvFilter::builder()
diff --git a/crates/chat-cli/src/mcp_client/client.rs b/crates/chat-cli/src/mcp_client/client.rs
@@ -13,6 +13,8 @@ use rmcp::model::{
     Implementation,
     InitializeRequestParam,
     ListPromptsResult,
+
+use crate::util::env_var::get_all_env_vars;
     ListToolsResult,
     LoggingLevel,
     LoggingMessageNotificationParam,
@@ -431,7 +433,7 @@ impl McpClientService {
                         process_env_vars(envs, &os.env);
                         cmd.envs(envs);
                     }
-                    cmd.envs(std::env::vars()).args(args);
+                    cmd.envs(get_all_env_vars()).args(args);
 
                     #[cfg(not(windows))]
                     cmd.process_group(0);
diff --git a/crates/chat-cli/src/telemetry/mod.rs b/crates/chat-cli/src/telemetry/mod.rs
@@ -74,7 +74,7 @@ pub use crate::telemetry::core::{
     QProfileSwitchIntent,
     TelemetryResult,
 };
-use crate::util::env_var::Q_CLI_CLIENT_APPLICATION;
+use crate::util::env_var::get_cli_client_application;
 use crate::util::system_info::os_version;
 
 #[derive(thiserror::Error, Debug)]
@@ -484,7 +484,7 @@ async fn set_event_metadata(database: &Database, event: &mut Event) {
     }
 
     // Set the client application from environment variable
-    if let Ok(client_app) = std::env::var(Q_CLI_CLIENT_APPLICATION) {
+    if let Some(client_app) = get_cli_client_application() {
         event.set_client_application(client_app);
     }
 }
diff --git a/crates/chat-cli/src/util/directories.rs b/crates/chat-cli/src/util/directories.rs
@@ -12,6 +12,7 @@ use thiserror::Error;
 
 use crate::cli::DEFAULT_AGENT_NAME;
 use crate::os::Os;
+use crate::util::env_var::{get_home_dir, get_tmpdir, get_xdg_runtime_dir, get_var_os};
 
 #[derive(Debug, Error)]
 pub enum DirectoryError {
@@ -123,7 +124,7 @@ fn macos_tempdir() -> Result<PathBuf> {
 #[cfg(unix)]
 pub fn runtime_dir() -> Result<PathBuf> {
     let mut dir = dirs::runtime_dir();
-    dir = dir.or_else(|| std::env::var_os("TMPDIR").map(PathBuf::from));
+    dir = dir.or_else(|| get_var_os("TMPDIR").map(PathBuf::from));
 
     cfg_if::cfg_if! {
         if #[cfg(target_os = "macos")] {
@@ -422,15 +423,15 @@ mod tests {
     fn sanitized_directory_path(path: Result<PathBuf>) -> String {
         let mut path = path.unwrap().into_os_string().into_string().unwrap();
 
-        if let Ok(home) = std::env::var("HOME") {
+        if let Some(home) = get_home_dir() {
             let home = home.strip_suffix('/').unwrap_or(&home);
             path = path.replace(home, "$HOME");
         }
 
         let user = whoami::username();
         path = path.replace(&user, "$USER");
 
-        if let Ok(tmpdir) = std::env::var("TMPDIR") {
+        if let Some(tmpdir) = get_tmpdir() {
             let tmpdir = tmpdir.strip_suffix('/').unwrap_or(&tmpdir);
             path = path.replace(tmpdir, "$TMPDIR");
         }
@@ -444,7 +445,7 @@ mod tests {
             };
         }
 
-        if let Ok(xdg_runtime_dir) = std::env::var("XDG_RUNTIME_DIR") {
+        if let Some(xdg_runtime_dir) = get_xdg_runtime_dir() {
             let xdg_runtime_dir = xdg_runtime_dir.strip_suffix('/').unwrap_or(&xdg_runtime_dir);
             path = path.replace(xdg_runtime_dir, "$XDG_RUNTIME_DIR");
         }
diff --git a/crates/chat-cli/src/util/editor.rs b/crates/chat-cli/src/util/editor.rs
@@ -1,6 +1,8 @@
 use std::path::Path;
 use std::process::Command;
 
+use crate::util::env_var::get_editor;
+
 /// Launch the user's preferred editor with the given file path.
 ///
 /// This function properly parses the EDITOR environment variable to handle
@@ -13,7 +15,7 @@ use std::process::Command;
 /// * `Ok(())` if the editor was launched successfully and exited with success
 /// * `Err` if the editor failed to launch or exited with an error
 pub fn launch_editor(file_path: &Path) -> eyre::Result<()> {
-    let editor_cmd = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
+    let editor_cmd = get_editor();
 
     // Parse the editor command to handle arguments
     let mut parts = shlex::split(&editor_cmd).ok_or_else(|| eyre::eyre!("Failed to parse EDITOR command"))?;
diff --git a/crates/chat-cli/src/util/env_var.rs b/crates/chat-cli/src/util/env_var.rs
@@ -0,0 +1,98 @@
+use std::ffi::OsString;
+
+/// Get environment variable as String
+pub fn get_var(key: &str) -> Result<String, std::env::VarError> {
+    std::env::var(key)
+}
+
+/// Get environment variable as OsString
+pub fn get_var_os(key: &str) -> Option<OsString> {
+    std::env::var_os(key)
+}
+
+/// Check if environment variable exists (non-empty)
+pub fn has_var(key: &str) -> bool {
+    std::env::var_os(key).is_some_and(|s| !s.is_empty())
+}
+
+/// Get environment variable with default value
+pub fn get_var_or(key: &str, default: &str) -> String {
+    std::env::var(key).unwrap_or_else(|_| default.to_string())
+}
+
+// Semantic helpers that abstract away the actual variable names
+pub fn get_log_level() -> Result<String, std::env::VarError> {
+    get_var("Q_LOG_LEVEL")
+}
+
+pub fn get_chat_shell() -> String {
+    get_var_or("AMAZON_Q_CHAT_SHELL", "bash")
+}
+
+pub fn is_log_stdout_enabled() -> bool {
+    get_var_os("Q_LOG_STDOUT").is_some()
+}
+
+pub fn is_telemetry_disabled() -> bool {
+    get_var_os("Q_DISABLE_TELEMETRY").is_some()
+}
+
+pub fn get_mock_chat_response() -> Option<String> {
+    get_var("Q_MOCK_CHAT_RESPONSE").ok()
+}
+
+pub fn is_truecolor_disabled() -> bool {
+    has_var("Q_DISABLE_TRUECOLOR")
+}
+
+pub fn is_remote_fake() -> bool {
+    get_var_os("Q_FAKE_IS_REMOTE").is_some()
+}
+
+pub fn in_codespaces() -> bool {
+    get_var_os("CODESPACES").is_some() || get_var_os("Q_CODESPACES").is_some()
+}
+
+pub fn in_ci() -> bool {
+    get_var_os("CI").is_some() || get_var_os("Q_CI").is_some()
+}
+
+pub fn show_continuation_ids() -> bool {
+    get_var_os("Q_SHOW_CONTINUATION_IDS").is_some()
+}
+
+pub fn get_cli_client_application() -> Option<String> {
+    get_var("Q_CLI_CLIENT_APPLICATION").ok()
+}
+
+pub fn get_editor() -> String {
+    get_var_or("EDITOR", "vi")
+}
+
+pub fn get_home_dir() -> Option<String> {
+    get_var("HOME").ok()
+}
+
+pub fn get_tmpdir() -> Option<String> {
+    get_var("TMPDIR").ok()
+}
+
+pub fn get_xdg_runtime_dir() -> Option<String> {
+    get_var("XDG_RUNTIME_DIR").ok()
+}
+
+pub fn get_term() -> Option<String> {
+    get_var("TERM").ok()
+}
+
+pub fn get_aws_region() -> Option<String> {
+    get_var("AWS_REGION").ok()
+}
+
+pub fn is_sigv4_enabled() -> bool {
+    get_var("AMAZON_Q_SIGV4").is_ok_and(|v| !v.is_empty())
+}
+
+pub fn get_all_env_vars() -> std::env::Vars {
+    std::env::vars()
+}
diff --git a/crates/chat-cli/src/util/mod.rs b/crates/chat-cli/src/util/mod.rs
@@ -1,6 +1,7 @@
 pub mod consts;
 pub mod directories;
 pub mod editor;
+pub mod env_var;
 pub mod file_uri;
 pub mod knowledge_store;
 pub mod open;
diff --git a/crates/chat-cli/src/util/system_info/mod.rs b/crates/chat-cli/src/util/system_info/mod.rs
@@ -12,6 +12,7 @@ use serde::{
 };
 
 use crate::os::Env;
+use crate::util::env_var::{is_remote_fake, in_codespaces, in_ci};
 
 /// Fields for OS release information
 /// Fields from <https://www.man7.org/linux/man-pages/man5/os-release.5.html>
@@ -175,16 +176,16 @@ pub fn in_wsl() -> bool {
 /// Is the calling binary running on a remote instance
 pub fn is_remote() -> bool {
     // TODO(chay): Add detection for inside docker container
-    in_ssh() || in_wsl() || std::env::var_os("Q_FAKE_IS_REMOTE").is_some()
+    in_ssh() || in_wsl() || is_remote_fake()
 }
 
 pub fn in_codespaces() -> bool {
     static IN_CODESPACES: OnceLock<bool> = OnceLock::new();
     *IN_CODESPACES
-        .get_or_init(|| std::env::var_os("CODESPACES").is_some() || std::env::var_os("Q_CODESPACES").is_some())
+        .get_or_init(|| crate::util::env_var::in_codespaces())
 }
 
 pub fn in_ci() -> bool {
     static IN_CI: OnceLock<bool> = OnceLock::new();
-    *IN_CI.get_or_init(|| std::env::var_os("CI").is_some() || std::env::var_os("Q_CI").is_some())
+    *IN_CI.get_or_init(|| crate::util::env_var::in_ci())
 }

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ pub use crate::telemetry::core::{`
`74`	`74`	`QProfileSwitchIntent,`
`75`	`75`	`TelemetryResult,`
`76`	`76`	`};`
`77`		`-use crate::util::env_var::Q_CLI_CLIENT_APPLICATION;`
	`77`	`+use crate::util::env_var::get_cli_client_application;`
`78`	`78`	`use crate::util::system_info::os_version;`
`79`	`79`
`80`	`80`	`#[derive(thiserror::Error, Debug)]`
`@@ -484,7 +484,7 @@ async fn set_event_metadata(database: &Database, event: &mut Event) {`
`484`	`484`	`}`
`485`	`485`
`486`	`486`	`// Set the client application from environment variable`
`487`		`- if let Ok(client_app) = std::env::var(Q_CLI_CLIENT_APPLICATION) {`
	`487`	`+ if let Some(client_app) = get_cli_client_application() {`
`488`	`488`	`event.set_client_application(client_app);`
`489`	`489`	`}`
`490`	`490`	`}`