From 39666d1a45f59f68506194dfdee38a0a0e7702a8 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:39:17 -0500 Subject: [PATCH 1/3] fix(cli): warn when a profile or flag overrides a set VOLTAGE_* variable Under --profile, or with a flag such as --wallet, commands ignored set VOLTAGE_* scope, key, and directory variables without saying so; only `voltage context` listed them. The voltage-api skill's helper lets the same variables win, so the two tools could reach different wallets in one shell. Every command that reads the configuration now prints one stderr warning per ignored variable before it prompts or sends a request, from the same list `voltage context` reports. --quiet keeps it, as it keeps other warnings; stdout is unchanged. --- CHANGELOG.md | 1 + docs/guides/login-and-profiles.md | 7 ++- docs/reference/environment.md | 2 +- src/auth.rs | 20 ++++-- src/config.rs | 47 +++++++++++--- src/context.rs | 100 ++++++++++++++++++++++++++++-- src/startup.rs | 8 ++- tests/cli_integration.rs | 40 +++++++++++- 8 files changed, 201 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..2851189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index cfeeeb0..cebe279 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -62,7 +62,12 @@ An API key cannot list organizations or environments, so with a key the CLI offe ## Profiles -A profile bundles an organization, an environment, and a credential, and ignores the scope variables above and `VOLTAGE_API_KEY`; flags still override its scope for one command. +A profile bundles an organization, an environment, and a credential, and ignores the scope variables above and `VOLTAGE_API_KEY`; flags still override its scope for one command. Other tools, such as the `voltage-api` agent skill's helper, read the same variables and let them win, so each command names on stderr every variable it ignores, even with `--quiet`: + +```console +$ VOLTAGE_WALLET_ID=WALLET_ID voltage wallets list --profile staging +warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected +``` ```sh voltage profiles create staging --org ORG_ID --env ENV_ID --account work diff --git a/docs/reference/environment.md b/docs/reference/environment.md index 375d3a4..a491ff7 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -1,6 +1,6 @@ # Environment variables -A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. `voltage context` lists every variable that is set but ignored, and where each value it uses came from. +A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `voltage context` lists the same variables, and where each value it uses came from. | Variable | Used for | Flag that wins | |---|---|---| diff --git a/src/auth.rs b/src/auth.rs index b053228..468514c 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -10,8 +10,8 @@ use crate::{ cli::{GlobalFlags, ImportKeyFlags, LoginFlags}, config::{ AUTH_URL, Account, AccountKind, Credential, CredentialProblem, CredentialStore, - InputSource, LOCK_TIMEOUT, Login, Scope, ScopeSources, Settings, Source, name_in_use, - read_secret, shell_word, unreadable_name, + IgnoredVariable, InputSource, LOCK_TIMEOUT, Login, Precedence, Scope, ScopeSources, + Settings, Source, name_in_use, read_secret, shell_word, unreadable_name, }, error::{CredentialRejection, CredentialSources, ErrorDetail, ResponseLimit}, secret::Secret, @@ -576,9 +576,19 @@ pub(crate) fn environment_key_sources( }) } -/// `VOLTAGE_API_KEY` is set, but a profile or account selection takes precedence over it. -pub(crate) fn ignores_ambient_api_key(flags: &GlobalFlags) -> bool { - std::env::var_os("VOLTAGE_API_KEY").is_some() && ambient_api_key(flags).is_none() +/// `VOLTAGE_API_KEY` when it is set but a profile or account selection takes precedence. +pub(crate) fn ignored_api_key(flags: &GlobalFlags) -> Option { + if std::env::var_os("VOLTAGE_API_KEY").is_none() || ambient_api_key(flags).is_some() { + return None; + } + Some(IgnoredVariable { + name: "VOLTAGE_API_KEY", + precedence: if flags.profile.is_some() { + Precedence::Profile + } else { + Precedence::Flag("--account") + }, + }) } /// The credential a command would use, chosen without reading a saved secret. diff --git a/src/config.rs b/src/config.rs index 1887d2e..b49cf0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -162,18 +162,47 @@ impl CredentialProblem { } } -/// `VOLTAGE_*` variables that are set but supply nothing, because a flag or the selected -/// profile takes precedence. Kept beside the precedence rules that decide it. -pub(crate) fn ignored_variables(sources: &ScopeSources, config_dir: Source) -> Vec<&'static str> { +/// What takes precedence over a `VOLTAGE_*` variable that is set. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum Precedence { + /// The named flag supplies the value. + Flag(&'static str), + /// The selected profile is a complete configuration, so ambient variables supply nothing, + /// even a value the profile lacks. + Profile, +} + +/// A `VOLTAGE_*` variable that is set but supplies nothing. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) struct IgnoredVariable { + pub(crate) name: &'static str, + pub(crate) precedence: Precedence, +} + +/// `VOLTAGE_*` scope and directory variables that are set but supply nothing, because a flag +/// or the selected profile takes precedence. Kept beside the precedence rules that decide it. +pub(crate) fn ignored_variables( + sources: &ScopeSources, + config_dir: Source, +) -> Vec { [ - ("VOLTAGE_ORGANIZATION_ID", sources.org), - ("VOLTAGE_ENVIRONMENT_ID", sources.envs), - ("VOLTAGE_WALLET_ID", sources.wallet), - ("VOLTAGE_CONFIG_DIR", config_dir), + ("VOLTAGE_ORGANIZATION_ID", "--org", sources.org), + ("VOLTAGE_ENVIRONMENT_ID", "--env", sources.envs), + ("VOLTAGE_WALLET_ID", "--wallet", sources.wallet), + ("VOLTAGE_CONFIG_DIR", "--config-dir", config_dir), ] .into_iter() - .filter(|(name, source)| std::env::var_os(name).is_some() && *source != Source::Environment) - .map(|(name, _)| name) + .filter(|(name, _, source)| std::env::var_os(name).is_some() && *source != Source::Environment) + .map(|(name, flag, source)| IgnoredVariable { + name, + // A set variable loses only to its flag or to a profile, which leaves it `Profile` + // or, for a value the profile lacks, `Unset`. + precedence: if source == Source::Flag { + Precedence::Flag(flag) + } else { + Precedence::Profile + }, + }) .collect() } diff --git a/src/context.rs b/src/context.rs index ff95348..d8244db 100644 --- a/src/context.rs +++ b/src/context.rs @@ -9,8 +9,10 @@ use crate::{ auth::{self, Rejected, Selected}, cli::GlobalFlags, config::{ - self, API_URL, AUTH_URL, AccountKind, CredentialProblem, CredentialStore, Settings, Source, + self, API_URL, AUTH_URL, AccountKind, CredentialProblem, CredentialStore, IgnoredVariable, + Precedence, ScopeSources, Settings, Source, }, + style::{Line, Role}, }; use serde::Serialize; use uuid::Uuid; @@ -98,14 +100,52 @@ pub struct Context { ignored_variables: Vec<&'static str>, } +/// Every `VOLTAGE_*` variable that is set but supplies nothing, because a flag or the selected +/// profile takes precedence. Commands warn with this list and `voltage context` reports it, so +/// the two cannot disagree. +pub(crate) fn ignored_variables( + global: &GlobalFlags, + sources: &ScopeSources, + config_dir: Source, +) -> Vec { + let mut ignored = config::ignored_variables(sources, config_dir); + ignored.extend(auth::ignored_api_key(global)); + ignored +} + +/// Warn on stderr about each ignored variable. Another tool can read the same variables with +/// the opposite precedence, so a silent override can send two tools in one shell to different +/// wallets. `--quiet` keeps the warning for that reason; stdout is unchanged. +pub(crate) fn warn_ignored(global: &GlobalFlags, ignored: &[IgnoredVariable]) { + let terminal = global.terminal(); + for variable in ignored { + terminal.important_line(ignored_warning(variable, global.profile.as_deref())); + } +} + +/// `warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected`. +fn ignored_warning(variable: &IgnoredVariable, profile: Option<&str>) -> Line { + let line = Line::default() + .paint(Role::Caution, "warning:") + .text(&format!(" {} is set but ignored because ", variable.name)); + match (variable.precedence, profile) { + (Precedence::Profile, Some(profile)) => { + line.text("--profile ").text(profile).text(" is selected") + } + // Only a selected profile yields `Profile`; this keeps the sentence whole regardless. + (Precedence::Profile, None) => line.text("a profile is selected"), + (Precedence::Flag(flag), _) => line.text(flag).text(" is given"), + } +} + /// Resolve the context with the same precedence every command uses. pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { let (scope, sources) = settings.sourced_scope(global.scope_selection())?; let (_, config_dir_source) = config::sourced_directory(global.config_dir.clone())?; - let mut ignored_variables = config::ignored_variables(&sources, config_dir_source); - if auth::ignores_ambient_api_key(global) { - ignored_variables.push("VOLTAGE_API_KEY"); - } + let ignored_variables = ignored_variables(global, &sources, config_dir_source) + .into_iter() + .map(|ignored| ignored.name) + .collect(); let (selected, credential_problem) = match auth::select(settings, &scope, global) { Ok(selected) => (Some(selected), None), Err(Rejected { problem, selected }) => (selected, Some(problem)), @@ -208,3 +248,53 @@ pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { ignored_variables, }) } + +#[cfg(test)] +mod tests { + use super::*; + + fn shown(name: &'static str, precedence: Precedence, profile: Option<&str>) -> String { + let ignored = IgnoredVariable { name, precedence }; + anstream::adapter::strip_str(ignored_warning(&ignored, profile).as_str()).to_string() + } + + #[test] + fn a_warning_names_the_variable_and_what_took_its_place() { + assert_eq!( + shown("VOLTAGE_WALLET_ID", Precedence::Profile, Some("staging")), + "warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected" + ); + assert_eq!( + shown("VOLTAGE_WALLET_ID", Precedence::Flag("--wallet"), None), + "warning: VOLTAGE_WALLET_ID is set but ignored because --wallet is given" + ); + assert_eq!( + shown("VOLTAGE_API_KEY", Precedence::Flag("--account"), None), + "warning: VOLTAGE_API_KEY is set but ignored because --account is given" + ); + } + + #[test] + fn a_profile_warning_without_a_profile_name_stays_a_sentence() { + assert_eq!( + shown("VOLTAGE_API_KEY", Precedence::Profile, None), + "warning: VOLTAGE_API_KEY is set but ignored because a profile is selected" + ); + } + + #[test] + fn a_profile_name_cannot_write_control_characters_to_the_terminal() { + let ignored = IgnoredVariable { + name: "VOLTAGE_WALLET_ID", + precedence: Precedence::Profile, + }; + // Unstripped, so an escape the name smuggled in would still be here. + let line = ignored_warning(&ignored, Some("evil\u{1b}]0;x\u{7}")); + assert!( + line.as_str() + .ends_with(" is set but ignored because --profile evil ]0;x is selected"), + "{:?}", + line.as_str() + ); + } +} diff --git a/src/startup.rs b/src/startup.rs index ab3a316..7dcd094 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -177,8 +177,14 @@ impl<'a> Session<'a> { global: &'a GlobalFlags, submission: &'a SubmissionState, ) -> Result { - let settings = Settings::open(config::directory(global.config_dir.clone())?)?; + let (dir, dir_source) = config::sourced_directory(global.config_dir.clone())?; + let settings = Settings::open(dir)?; let (mut scope, sources) = settings.sourced_scope(global.scope_selection())?; + // Before any prompt or request, so the warning precedes what the command does. + context::warn_ignored( + global, + &context::ignored_variables(global, &sources, dir_source), + ); let credential_sources = auth::environment_key_sources(global, &sources); let resolver = Resolver { settings: &settings, diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..42a24af 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -934,12 +934,20 @@ async fn profile_credentials_override_ambient_key_and_scope() { ok(json!([])), ) .await; - cli(dir.path(), &server) + let profiled = cli(dir.path(), &server) .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) - .args(["wallets", "list", "--profile", "stage"]) + .args(["wallets", "list", "--profile", "stage", "--quiet"]) .assert() .success(); + // Each ignored variable is named on stderr, even under --quiet; stdout stays the result. + assert_eq!( + stderr(&profiled), + "warning: VOLTAGE_ORGANIZATION_ID is set but ignored because --profile stage is selected\n\ + warning: VOLTAGE_ENVIRONMENT_ID is set but ignored because --profile stage is selected\n\ + warning: VOLTAGE_API_KEY is set but ignored because --profile stage is selected\n" + ); + assert_eq!(json_stdout(&profiled)["data"], json!([])); cli(dir.path(), &server) .args(["wallets", "list", "--profile", "stage", "--env", RESOURCE]) .assert() @@ -947,6 +955,34 @@ async fn profile_credentials_override_ambient_key_and_scope() { server.verify().await; } +#[tokio::test] +async fn a_flag_over_a_set_variable_warns_and_a_used_variable_does_not() { + let (server, dir) = fixture().await; + respond( + &server, + route("GET", wallets_path(ORG)).and(header("x-api-key", ACCOUNT_KEY)), + ok(json!([])), + 2, + ) + .await; + let flagged = cli(dir.path(), &server) + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .args(["wallets", "list", "--org", ORG]) + .assert() + .success(); + assert_eq!( + stderr(&flagged), + "warning: VOLTAGE_ORGANIZATION_ID is set but ignored because --org is given\n" + ); + let ambient = cli(dir.path(), &server) + .env("VOLTAGE_ORGANIZATION_ID", ORG) + .args(["wallets", "list"]) + .assert() + .success(); + assert_eq!(stderr(&ambient), ""); + server.verify().await; +} + #[tokio::test(flavor = "multi_thread")] async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { let (server, dir) = fixture().await; From 174456c8dc278f6d26c4963a858ae17cc7ec02cb Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 28 Oct 2026 05:09:59 -0500 Subject: [PATCH 2/3] fix(cli): keep import-key and profiles create quiet about flags they save Both commands save the organization, environment, and account their flags name, so warning that those flags override VOLTAGE_* variables was noise. They still warn about a profile or --config-dir. The staging and local wrappers now pass their directory as VOLTAGE_CONFIG_DIR instead of --config-dir, so the hint after `profiles create` no longer names a flag the wrappers refuse. --- CHANGELOG.md | 2 +- docs/development.md | 2 +- docs/reference/environment.md | 2 +- scripts/voltage-local | 5 +++-- scripts/voltage-staging | 5 +++-- src/startup.rs | 32 +++++++++++++++++++++++++++----- tests/cli_integration.rs | 9 ++++++++- 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2851189..d03f6e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased -- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. +- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. `auth import-key` and `profiles create`, which save what their flags name, do not warn about the flags they were given. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/development.md b/docs/development.md index d7aca10..452e68e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -133,7 +133,7 @@ build when they disagree. `docs/commands.md` is generated; regenerate it with API-key runs, the optional `VOLTAGE_API_KEY`, `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, and `VOLTAGE_WALLET_ID` from the repository's private `.env` (copy `.env.example`, fill it in, `chmod 600 .env`) without evaluating it as shell code, - keeps CLI state under `.work/staging/cli`, and refuses `--config-dir`, `--api-url`, and + keeps CLI state under `.work/staging/cli` through `VOLTAGE_CONFIG_DIR`, and refuses `--config-dir`, `--api-url`, and `--auth-url` so a staging command cannot target another deployment. A leading `--build` works as it does for `voltage-local`, but checks `.env` before it builds when a command follows; `--build` alone needs no `.env`. Browser login and diff --git a/docs/reference/environment.md b/docs/reference/environment.md index a491ff7..840f2b6 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -1,6 +1,6 @@ # Environment variables -A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `voltage context` lists the same variables, and where each value it uses came from. +A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `auth import-key` and `profiles create` save the scope and account their flags name, so they warn only about a profile or `--config-dir`. `voltage context` lists the same variables, and where each value it uses came from. | Variable | Used for | Flag that wins | |---|---|---| diff --git a/scripts/voltage-local b/scripts/voltage-local index 6d8395a..8ab33d8 100755 --- a/scripts/voltage-local +++ b/scripts/voltage-local @@ -24,8 +24,9 @@ if build: if status or not args: sys.exit(status) binary = root / 'target/debug/voltage' +# A variable rather than --config-dir, so hints the CLI prints do not name a refused flag. +env['VOLTAGE_CONFIG_DIR'] = str(root / '.work/local-demo/cli') if not binary.is_file(): sys.exit('Build the development CLI first: ./scripts/voltage-local --build') -os.execve(binary, [str(binary), '--config-dir', str(root / '.work/local-demo/cli'), - '--auth-url', 'http://127.0.0.1:8081/api/v1', +os.execve(binary, [str(binary), '--auth-url', 'http://127.0.0.1:8081/api/v1', '--api-url', 'http://127.0.0.1:1/disabled-api', *args], env) diff --git a/scripts/voltage-staging b/scripts/voltage-staging index f2ae052..d0387a5 100755 --- a/scripts/voltage-staging +++ b/scripts/voltage-staging @@ -100,6 +100,9 @@ if build: env = {key: value for key, value in os.environ.items() if not key.startswith("VOLTAGE_")} env.update({key: value for key, value in loaded.items() if key in CLI_ENV}) +# The directory is a variable rather than --config-dir, so hints the CLI prints, such as +# the one after `profiles create`, do not name a flag this wrapper refuses. +env["VOLTAGE_CONFIG_DIR"] = str(root / ".work/staging/cli") binary = root / "target/debug/voltage" if not binary.is_file(): sys.exit("Build the development CLI first: ./scripts/voltage-staging --build") @@ -108,8 +111,6 @@ os.execve( binary, [ str(binary), - "--config-dir", - str(root / ".work/staging/cli"), "--auth-url", loaded["VOLTAGE_STAGING_AUTH_URL"], "--api-url", diff --git a/src/startup.rs b/src/startup.rs index 7dcd094..8ba36b3 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -8,7 +8,7 @@ use crate::{ self, AuthCommand, Command, ConvertFlags, GlobalFlags, Invocation, LocalCommand, ParseFailure, PriceFlags, ProfileCommand, }, - config::{self, Profile, Scope, Settings}, + config::{self, Precedence, Profile, Scope, Settings}, context, error::{CredentialSources, ErrorKind}, greeting, @@ -158,6 +158,22 @@ async fn execute(invocation: Invocation, submission: &SubmissionState) -> Result } } +/// Commands that save the organization, environment, and account their flags name. A flag +/// over a set variable is what such a command is for, so only the configuration directory, +/// which decides where it saves, still warns. +fn saves_selection(command: &Command) -> bool { + matches!( + command, + Command::Local( + LocalCommand::Auth { + command: AuthCommand::ImportKey(_), + } | LocalCommand::Profiles { + command: ProfileCommand::Create { .. }, + } + ) + ) +} + /// One command once its configuration is read and its scope is complete: what the flags /// asked for, the saved settings, the scope, and what the command may have changed. /// Commands run from here; what they call receives only the parts it uses. @@ -180,11 +196,17 @@ impl<'a> Session<'a> { let (dir, dir_source) = config::sourced_directory(global.config_dir.clone())?; let settings = Settings::open(dir)?; let (mut scope, sources) = settings.sourced_scope(global.scope_selection())?; + let mut ignored = context::ignored_variables(global, &sources, dir_source); + if saves_selection(command) { + ignored.retain(|variable| { + matches!( + variable.precedence, + Precedence::Profile | Precedence::Flag("--config-dir") + ) + }); + } // Before any prompt or request, so the warning precedes what the command does. - context::warn_ignored( - global, - &context::ignored_variables(global, &sources, dir_source), - ); + context::warn_ignored(global, &ignored); let credential_sources = auth::environment_key_sources(global, &sources); let resolver = Resolver { settings: &settings, diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 42a24af..0cfcbd9 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -1000,6 +1000,8 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { ]; let created = cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) .args(create) .assert() .success(); @@ -1007,6 +1009,8 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { json_stdout(&created)["data"], json!({"profile":"stage","created":true}) ); + // The flags name what the profile saves, so the variables they override are no surprise. + assert_eq!(stderr(&created), ""); cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") .args(create) @@ -1338,7 +1342,8 @@ async fn imported_keys_bind_their_scope_and_report_status_without_secrets() { ENV, ]; let imported = cli(dir.path(), &server) - .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) .args(import) .args(["--credential-store", "file"]) .write_stdin(" imported-secret \n") @@ -1348,6 +1353,8 @@ async fn imported_keys_bind_their_scope_and_report_status_without_secrets() { json_stdout(&imported)["data"], json!({"account":"staging-key","imported":true}) ); + // The flags name what the key is bound to, so the variables they override are no surprise. + assert_eq!(stderr(&imported), ""); let settings = Settings::open(dir.path().to_path_buf()).unwrap(); let account = &settings.config.accounts["staging-key"]; assert_eq!(account.kind, AccountKind::ApiKey); From 9c7bc297c2b6941b0c542e42b6fd8bc35a88828b Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:53:58 -0600 Subject: [PATCH 3/3] test(cli): check profiles create still warns about an overridden config directory --- tests/cli_integration.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 2f56a07..f78db8a 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -1012,11 +1012,23 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { ); // The flags name what the profile saves, so the variables they override are no surprise. assert_eq!(stderr(&created), ""); - cli(dir.path(), &server) + // The directory decides where it saves, so a --config-dir over the variable still warns. + let duplicate = cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_CONFIG_DIR", dir.path().join("unused")) .args(create) .assert() .code(2); + let warnings: Vec<_> = stderr(&duplicate) + .lines() + .filter(|line| line.starts_with("warning:")) + .map(str::to_owned) + .collect(); + assert_eq!( + warnings, + ["warning: VOLTAGE_CONFIG_DIR is set but ignored because --config-dir is given"] + ); let listed = cli(dir.path(), &server) .args(["profiles", "list"]) .assert()