Skip to content

Commit de2a554

Browse files
committed
refactor: centralize environment variable access
- Add util/env_var.rs module with semantic helpers - Replace direct std::env calls across codebase - Improve maintainability and testability of env var usage
1 parent a275492 commit de2a554

File tree

18 files changed

+139
-39
lines changed

18 files changed

+139
-39
lines changed

crates/chat-cli/src/api_client/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ pub struct ApiClient {
100100

101101
impl ApiClient {
102102
pub async fn new(
103-
env: &Env,
103+
_env: &Env,
104104
fs: &Fs,
105105
database: &mut Database,
106106
// endpoint is only passed here for list_profiles where it needs to be called for each region
@@ -138,7 +138,7 @@ impl ApiClient {
138138
model_cache: Arc::new(RwLock::new(None)),
139139
};
140140

141-
if let Ok(json) = env.get("Q_MOCK_CHAT_RESPONSE") {
141+
if let Some(json) = crate::util::env_var::get_mock_chat_response() {
142142
this.set_mock_output(serde_json::from_str(fs.read_to_string(json).await.unwrap().as_str()).unwrap());
143143
}
144144

@@ -148,7 +148,7 @@ impl ApiClient {
148148
// If SIGV4_AUTH_ENABLED is true, use Q developer client
149149
let mut streaming_client = None;
150150
let mut sigv4_streaming_client = None;
151-
match env.get("AMAZON_Q_SIGV4").is_ok() {
151+
match crate::util::env_var::is_sigv4_enabled() {
152152
true => {
153153
let credentials_chain = CredentialsChain::new().await;
154154
if let Err(err) = credentials_chain.provide_credentials().await {

crates/chat-cli/src/auth/builder_id.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ use crate::database::{
6363
Database,
6464
Secret,
6565
};
66+
use crate::util::env_var::is_sigv4_enabled;
6667

6768
#[derive(Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
6869
pub enum OAuthFlow {
@@ -545,7 +546,7 @@ pub async fn poll_create_token(
545546

546547
pub async fn is_logged_in(database: &mut Database) -> bool {
547548
// Check for BuilderId if not using Sigv4
548-
if std::env::var("AMAZON_Q_SIGV4").is_ok_and(|v| !v.is_empty()) {
549+
if is_sigv4_enabled() {
549550
debug!("logged in using sigv4 credentials");
550551
return true;
551552
}

crates/chat-cli/src/aws_common/user_agent_override_interceptor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl Intercept for UserAgentOverrideInterceptor {
125125
}
126126
}
127127

128-
fn apply_additional_metadata(env: &Env, ua: &mut AwsUserAgent) {
128+
fn apply_additional_metadata(_env: &Env, ua: &mut AwsUserAgent) {
129129
let ver = format!("{VERSION_HEADER}/{VERSION_VALUE}");
130130
match AdditionalMetadata::new(clean_metadata(&ver)) {
131131
Ok(md) => {
@@ -134,7 +134,7 @@ fn apply_additional_metadata(env: &Env, ua: &mut AwsUserAgent) {
134134
Err(err) => panic!("Failed to parse version: {err}"),
135135
};
136136

137-
if let Ok(val) = env.get(AWS_TOOLING_USER_AGENT) {
137+
if let Some(val) = crate::util::env_var::get_aws_tooling_user_agent() {
138138
match AdditionalMetadata::new(clean_metadata(&val)) {
139139
Ok(md) => {
140140
ua.add_additional_metadata(md);

crates/chat-cli/src/cli/chat/cli/editor.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::cli::chat::{
1111
ChatState,
1212
};
1313
use crate::theme::StyledText;
14+
use crate::util::env_var::get_editor;
1415

1516
#[deny(missing_docs)]
1617
#[derive(Debug, PartialEq, Args)]
@@ -86,7 +87,7 @@ impl EditorArgs {
8687
/// Launch the user's preferred editor with the given file path
8788
fn launch_editor(file_path: &std::path::Path) -> Result<(), ChatError> {
8889
// Get the editor from environment variable or use a default
89-
let editor_cmd = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
90+
let editor_cmd = get_editor();
9091

9192
// Parse the editor command to handle arguments
9293
let mut parts =

crates/chat-cli/src/cli/chat/tools/delegate.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use crate::cli::{
4141
};
4242
use crate::os::Os;
4343
use crate::theme::StyledText;
44+
use crate::util::env_var::get_all_env_vars;
4445

4546
/// Launch and manage async agent processes. Delegate tasks to agents that run independently in
4647
/// background.
@@ -333,7 +334,7 @@ pub async fn spawn_agent_process(os: &Os, agent: &str, task: &str) -> Result<Age
333334
cmd.stdout(std::process::Stdio::piped());
334335
cmd.stderr(std::process::Stdio::piped());
335336
cmd.stdin(std::process::Stdio::null()); // No user input
336-
cmd.envs(std::env::vars());
337+
cmd.envs(get_all_env_vars());
337338

338339
#[cfg(not(windows))]
339340
cmd.process_group(0);

crates/chat-cli/src/cli/chat/tools/execute/unix.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use super::{
1616
format_output,
1717
};
1818
use crate::os::Os;
19+
use crate::util::env_var::get_chat_shell;
1920

2021
/// Run a bash command on Unix systems.
2122
/// # Arguments
@@ -30,7 +31,7 @@ pub async fn run_command<W: Write>(
3031
max_result_size: usize,
3132
mut updates: Option<W>,
3233
) -> Result<CommandResult> {
33-
let shell = std::env::var("AMAZON_Q_CHAT_SHELL").unwrap_or("bash".to_string());
34+
let shell = get_chat_shell();
3435

3536
// Set up environment variables with user agent metadata for CloudTrail tracking
3637
let env_vars = env_vars_with_user_agent(os);

crates/chat-cli/src/cli/chat/tools/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,9 +444,9 @@ fn format_path(cwd: impl AsRef<Path>, path: impl AsRef<Path>) -> String {
444444
.unwrap_or(path.as_ref().to_string_lossy().to_string())
445445
}
446446

447-
fn supports_truecolor(os: &Os) -> bool {
447+
fn supports_truecolor(_os: &Os) -> bool {
448448
// Simple override to disable truecolor since shell_color doesn't use Context.
449-
!os.env.get("Q_DISABLE_TRUECOLOR").is_ok_and(|s| !s.is_empty())
449+
!crate::util::env_var::is_truecolor_disabled()
450450
&& shell_color::get_color_support().contains(shell_color::ColorSupport::TERM24BIT)
451451
}
452452

@@ -514,7 +514,7 @@ pub fn queue_function_result(result: &str, updates: &mut impl Write, is_error: b
514514

515515
/// Helper function to set up environment variables with user agent metadata for CloudTrail tracking
516516
pub fn env_vars_with_user_agent(os: &Os) -> std::collections::HashMap<String, String> {
517-
let mut env_vars: std::collections::HashMap<String, String> = std::env::vars().collect();
517+
let mut env_vars: std::collections::HashMap<String, String> = crate::util::env_var::get_all_env_vars().collect();
518518

519519
// Set up additional metadata for the AWS CLI user agent
520520
let user_agent_metadata_value = format!(

crates/chat-cli/src/cli/chat/util/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use eyre::Result;
1616

1717
use super::ChatError;
1818
use super::token_counter::TokenCounter;
19+
use crate::util::env_var::get_term;
1920

2021
pub fn truncate_safe(s: &str, max_bytes: usize) -> &str {
2122
if s.len() <= max_bytes {
@@ -119,7 +120,7 @@ pub fn play_notification_bell(requires_confirmation: bool) {
119120
/// Determine if we should play the bell based on terminal type
120121
fn should_play_bell() -> bool {
121122
// Get the TERM environment variable
122-
if let Ok(term) = std::env::var("TERM") {
123+
if let Some(term) = get_term() {
123124
// List of terminals known to handle bell character well
124125
let bell_compatible_terms = [
125126
"xterm",

crates/chat-cli/src/cli/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
use crate::theme::StyledText;
2+
use crate::util::env_var::{
3+
get_aws_region,
4+
is_log_stdout_enabled,
5+
};
26
mod agent;
37
pub mod chat;
48
mod debug;
@@ -231,7 +235,7 @@ impl Cli {
231235
),
232236
false => None,
233237
},
234-
log_to_stdout: std::env::var_os("Q_LOG_STDOUT").is_some() || self.verbose > 0,
238+
log_to_stdout: is_log_stdout_enabled() || self.verbose > 0,
235239
log_file_path: match subcommand {
236240
RootSubcommand::Chat { .. } => Some(logs_dir().expect("home dir must be set").join("qchat.log")),
237241
_ => None,
@@ -240,7 +244,7 @@ impl Cli {
240244
});
241245

242246
// Check for region support.
243-
if let Ok(region) = std::env::var("AWS_REGION") {
247+
if let Ok(region) = get_aws_region() {
244248
if GOV_REGIONS.contains(&region.as_str()) {
245249
bail!("AWS GovCloud ({region}) is not supported.")
246250
}

crates/chat-cli/src/cli/settings.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use crossterm::style::Stylize;
1010
use eyre::{
1111
Result,
1212
WrapErr,
13-
bail,
1413
};
1514
use globset::Glob;
1615
use serde_json::json;
@@ -190,12 +189,11 @@ impl SettingsArgs {
190189
match self.cmd {
191190
Some(SettingsSubcommands::Open) => {
192191
let file = directories::settings_path().context("Could not get settings path")?;
193-
if let Ok(editor) = os.env.get("EDITOR") {
194-
tokio::process::Command::new(editor).arg(file).spawn()?.wait().await?;
195-
Ok(ExitCode::SUCCESS)
196-
} else {
197-
bail!("The EDITOR environment variable is not set")
198-
}
192+
193+
let editor =
194+
crate::util::env_var::try_get_editor().context("The EDITOR environment variable is not set")?;
195+
tokio::process::Command::new(editor).arg(file).spawn()?.wait().await?;
196+
Ok(ExitCode::SUCCESS)
199197
},
200198
Some(SettingsSubcommands::List { all, format, state }) => {
201199
if state {

0 commit comments

Comments
 (0)