Repository navigation
fix(cli): warn when a profile or flag overrides a set VOLTAGE_* variable - #128
Conversation
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.
…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.
# Conflicts: # CHANGELOG.md
…-list-and-stop' into brandon/dev-2956-voltage-cli-commands-ignore-voltage_-variables-under-a # Conflicts: # CHANGELOG.md
tee8z
left a comment
There was a problem hiding this comment.
Approved at 89b144a. I traced the precedence through sourced_scope: a flag gives Precedence::Flag, and a profile gives Unset for an ambient value, which maps to Profile, so the warning names the right cause in each case. context::ignored_variables feeds both the warning and voltage context, so they cannot disagree. The saves_selection exemption is tested for profiles create and auth import-key, including the --config-dir warning that still applies. Keeping the warning under --quiet matches docs/guides/scripting.md.
Non-blocking: retain matches Precedence::Flag("--config-dir") by its string. variable.name == "VOLTAGE_CONFIG_DIR" would say the same thing without depending on the flag text.
CRAP on this head: 857 of 1069 functions measured, 0 violations. Highest: 18.10 wait_for_payment (unchanged). CI is green.
Fixes DEV-2956
Closes #121
Problem
With
--profile, or with a flag such as--wallet, the CLI ignores a setVOLTAGE_ORGANIZATION_ID,VOLTAGE_ENVIRONMENT_ID,VOLTAGE_WALLET_ID,VOLTAGE_API_KEY, orVOLTAGE_CONFIG_DIR. Onlyvoltage contextsaid so, inignored_variables. Thevoltage-apiskill's helper reads the same names and lets them win, so in one shell the helper and the CLI could act on different wallets without any sign.Behavior
Every command that reads the configuration prints one stderr line per ignored variable, before it prompts or sends a request:
VOLTAGE_ORGANIZATION_ID,VOLTAGE_ENVIRONMENT_ID,VOLTAGE_WALLET_ID--profileis selected, or--org/--env/--walletis given--profile NAME is selected, or--org is given(and so on)VOLTAGE_API_KEY--profileor--account--profile NAME is selectedor--account is givenVOLTAGE_CONFIG_DIR--config-dir--config-dir is given--jsonresults are unchanged. The warning is a stderr line,warning:in the caution color on a terminal, plain when redirected.--quietkeeps it.--quiethides optional progress and notices;docs/guides/scripting.mdsays it never hides warnings, and the offset-pagination deprecation already follows that. A silent override is what this issue is about, so hiding it behind--quietwould bring the problem back for scripts.auth import-keyandprofiles createsave the organization, environment, and account that their flags name, so they do not warn about those flags. They still warn about a profile or--config-dir.scripts/voltage-stagingandscripts/voltage-localnow pass their state directory asVOLTAGE_CONFIG_DIR, not--config-dir. Hints the CLI prints, such as the one afterprofiles create, then do not name a flag the wrapper refuses. feat(cli): print a profile's scope as dotenv lines with context --format env #129 makes the same change, so whichever PR merges second merges cleanly.price,convert,completions, and barevoltageread no scope, so they never warn.Design notes
config::ignored_variablesnow returnsIgnoredVariable { name, precedence }, wherePrecedenceisFlag("--wallet")orProfile, decided from the sameScopeSourcesas before.auth::ignored_api_keyreplacesignores_ambient_api_keythe same way.context::ignored_variablesjoins the two lists.voltage contextmaps it to names (its JSON is unchanged), andSession::openpasses it tocontext::warn_ignoredright after the scope is resolved. One list feeds both, so the warning andvoltage contextcannot disagree.Line::text, which scrubs control characters.error.detailof--jsonerrors. That changes the error contract, so it is left for a follow-up if wanted.Try it on staging
Setup: Try a pull request on staging
The wrapper passes only the
VOLTAGE_*variables in.env, so these steps needVOLTAGE_ORGANIZATION_IDandVOLTAGE_ENVIRONMENT_IDset there. They are optional lines in.env.example. Any valid UUIDs show the warnings; use your real IDs fromvoltage organizations list --account meandvoltage environments list --account me --org ORG_IDso the lists succeed.Expect: no
warning:line.organization_idandenvironment_idsshow sourceenvironment.Expect, before the wallet table:
warning: VOLTAGE_ORGANIZATION_ID is set but ignored because --org is given.voltage profiles create try-it --account me --org ORG_ID --env ENV_ID voltage wallets list --profile try-it --quiet voltage wallets list --profile try-it --json > /dev/null voltage profiles delete try-itExpect:
profiles createprints no warning, because its flags name what it saves.warning: VOLTAGE_… is set but ignored because --profile try-it is selectedline for eachVOLTAGE_*variable in.env,--quietincluded.--json, the warnings are still on stderr, and stdout has only the JSON envelope.Validation
Run on the head after merging
master(9c7bc29):cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo machete --with-metadata,cargo test --locked(202 unit, 150 integration), andpython3 scripts/check-coverage.pyall pass.context: the warning text for a profile, a flag, and--account; the sentence without a profile name; and that a profile name with escape characters is scrubbed (checked on the unstripped line).profile_credentials_override_ambient_key_and_scopechecks the exact stderr under--profile --quiet(organization, environment, and API key, in that order), and that stdout is still the JSON result.a_flag_over_a_set_variable_warns_and_a_used_variable_does_notchecks--orgover a set variable, and no warning when the variable is used.profiles createandauth import-keyprint no warning about their own flags. 9c7bc29 adds a check thatprofiles createstill warns about--config-dirover a setVOLTAGE_CONFIG_DIR, and not about--org.contextwith a usedVOLTAGE_ORGANIZATION_IDprinted no warning.wallets list --orgover a set variable warned.context --profile NAME --quietwarned once per ignored variable on stderr, and stdout had only the result.scripts/crap-report.sh, cargo-llvm-cov 0.8.5, knots 1.16.0): 853 of 1065 functions measured, 0 violations. The highest score is 18.10,wait_for_paymentinsrc/api.rs, which this PR does not change. Changed functions:context::inspect12.01,Session::open6.0,ignored_api_key4.0,config::ignored_variables3.0,ignored_warningandwarn_ignored2.0,saves_selection1.0.Checklist
docs/andCHANGELOG.md(Unreleased) are updated. (docs/guides/login-and-profiles.md,docs/reference/environment.md)docs/development.mdpasses.voltage-pr-reviewskill, and every new error and early-return path has a test.wait_for_payment, unchanged); no function at or above 20.profiles create try-itstaging steps were not run, to avoid changing the shared staging config. The warning is not tested on a real terminal (color) by an automated test; the color comes from the existingimportant_linepath.Stack
Stack #135, layer 3 of 9 (bottom targets
master):--allonorganizations,environments, andprofiles listVOLTAGE_*variablevoltage context --format envvoltage feedbackand panicsvoltage environments createflake.nixwith a KaifileThe layers are chained with merge commits, because history is not rewritten. Use squash and merge, bottom first.