镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(cli): name the next command in scope, credential, and login errors - #24

Merged
thebrandonlucas merged 1 commit into
masterfrom
feat/cli-error-hints
Oct 20, 2026
Merged

thebrandonlucas merged 1 commit into
masterfrom
feat/cli-error-hints

Conversation

@thebrandonlucas

@thebrandonlucas thebrandonlucas commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Stack: master ← #24 ← #19 ← #26 ← #27 ← #28 ← #29 ← #30 ← #55 ← #59. #20, #22 and #57 are merged.

Errors now name the next command, not only the problem:

Error Hint
Missing --org, --env, --wallet, or webhook ID The command that lists the IDs, and each way to supply one (flag, VOLTAGE_* variable, profile)
Unknown or ambiguous credential, empty API key, key bound to another scope The saved credential names, or how to add one
Unknown profile The saved profile names, or how to create one
Expired or revoked login (OAuth invalid_grant) voltage logout --local --account NAME, then voltage login --account NAME
organizations list / environments list with an API key Which key was used, and how to select a browser login
HTTP 401 with an account credential voltage auth status, or sign in again
HTTP 403 with an account credential voltage context, to check the organization, environment, and credential
HTTP 401/403 with a checkout session or stream token Check the token in --token-file, VOLTAGE_CHECKOUT_TOKEN, or VOLTAGE_STREAM_TOKEN
quote_required voltage quotes create; a wallet on a BTC line of credit needs no quote

Two changes to how API errors print:

  • Message: a 4xx rejection's message leads with the API's own reason, such as Voltage API returned HTTP 400: Quote is required for payment. A 5xx message gives only the status, and its text stays in the details. The credentials the CLI presented are redacted from the message as well as the details.
  • Human details: the rest of the API error prints as indented field value lines instead of one JSON line. --json still reports the complete response.

Credential hints are attached in CredentialProblem::error, so voltage context and commands share one selection path. Hints can now be built at runtime and are scrubbed like messages, because they can name saved credentials and profiles. docs/reference/exit-codes.md lists them.

The --yes refusal hint from the original plan is dropped: #14 replaced confirmation prompts with --execute, and the not-executed error already names --execute and VOLTAGE_EXECUTE=1.

Changes after review

  • Credential leak fixed: an API reason that echoed the presented key printed it in the message, for example HTTP 400: Bad key <key>. Error::redacted now redacts message too. New integration tests check a 400 whose error.detail echoes the key, in --json and in human output, and a 5xx whose text stays out of the message.
  • 401/403 hints apply only to account credentials. Checkout tokens get their own hint, and a request without a credential gets none.
  • The rejection body is parsed once, in send, into ApiErrorBody::message and ApiErrorBody::hint.
  • The quote_required hint no longer claims to know the wallet: "A wallet on a USD line of credit needs a quote for each payment."
  • New assertions for each hint branch: saved API key with discovery, unknown credential with none saved, bound_elsewhere, empty_api_key, unknown profile with saved profiles, profiles get and profiles delete with an unknown name, the --webhook path hint, and the --env hint with no and with two --env values.
  • docs/reference/exit-codes.md says "Many errors" carry a hint and gives the logout-then-login order.

Open question

Logins are saved under the account's email address by default, so the "Saved credentials: …" hints print saved emails to stderr and in --json error.hint, which can end up in CI logs. This PR leaves that behavior as it is. Is that intended?

Checks

On ebd905a:

  • cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, cargo machete --with-metadata: pass.
  • cargo test --locked: 97 unit and 82 integration tests pass. VOLTAGE_BLESS=1 cargo test --locked leaves the tree clean.
  • python3 scripts/check-coverage.py: pass.
  • CRAP (scripts/crap-report.sh): no function at or above 20. Highest is api::wait_for_payment at 19.66, unchanged from or lower than master (19.66).
  • Skill review: voltage-pr-review and the voltage-backend-engineering Rust checklist, against the stack base. Findings are fixed in this commit.

Not run: the staging and live tutorials, which need credentials.

@thebrandonlucas
thebrandonlucas added this pull request to stack #25 October 13, 2026 18:56
@thebrandonlucas
thebrandonlucas force-pushed the feat/cli-error-hints branch 2 times, most recently from c77c34d to 3e49448 Compare October 19, 2026 13:47
@thebrandonlucas
thebrandonlucas force-pushed the feat/cli-error-hints branch 4 times, most recently from 0d3964a to ed7d51a Compare October 19, 2026 19:19
@thebrandonlucas
thebrandonlucas removed this pull request from stack #25 October 20, 2026 11:59
@thebrandonlucas
thebrandonlucas changed the base branch from feat/cli-interactive-pickers to test/cli-picker-failures October 20, 2026 11:59
@thebrandonlucas
thebrandonlucas added this pull request to stack #58 October 20, 2026 11:59
@thebrandonlucas thebrandonlucas added the human review wanted CI passes; ready for a human reviewer label Oct 20, 2026
@tee8z
tee8z self-requested a review October 20, 2026 12:15

@tee8z tee8z left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good! The robot has a bit of feedback around a few missed hints and error paths.

Specifically:

Most of the robots comments are cases where a hint names a fix that doesn't work for the credential or flags in use (--profile with VOLTAGE_WALLET_ID, API keys with organizations list, voltage login with a saved login), plus two places where human error output now drops information it used to show. Each comment has a suggestion. It read the code but didn't run it.

Comment thread src/config.rs Resolved
Comment thread src/config.rs Resolved
Comment thread src/api.rs Resolved
Comment thread src/api.rs Resolved
Comment thread src/output.rs Resolved
Comment thread src/output.rs Resolved
Comment thread src/auth.rs Resolved
Comment thread src/auth.rs Resolved
Comment thread src/input.rs Resolved
Comment thread tests/cli_integration.rs Resolved
Base automatically changed from test/cli-picker-failures to master October 20, 2026 12:25
Errors for a missing --org, --env, --wallet, or webhook ID now name the
command that lists those IDs and every way to supply one. The organization
and environment listings are marked browser login only, and the wallet
hint says VOLTAGE_WALLET_ID is read only without --profile. An unknown or
ambiguous credential or profile lists the saved names. organizations list
and environments list with an API key say which key was used and how to
select a browser login.

An expired or revoked login (OAuth invalid_grant) says so, at refresh and
at the organization token exchange, and gives the voltage logout --local
and voltage login commands that replace it. The commands quote the account
name for the shell and repeat --auth-url when the login is not on the
default auth service, since that flag is never saved.

API rejections carry a next step, also when the body is not JSON. HTTP 401,
and the HTTP 400 missing_credentials the API returns for a key it does not
recognize, point to voltage auth status; for a login the hint also gives
the commands that sign in again. HTTP 403 points to voltage context. For a checkout session
or stream token, both point to the token in --token-file,
VOLTAGE_CHECKOUT_TOKEN, or VOLTAGE_STREAM_TOKEN. quote_required points to
voltage quotes create and notes that a wallet on a BTC line of credit needs
no quote. Hints can now be built at runtime and are scrubbed like
messages, since they can name saved credentials.

A 4xx rejection's message leads with the API's own reason, such as "Quote
is required for payment", instead of only the HTTP status. A 5xx message
gives only the status, and its text stays in the details. The presented
credentials are redacted from the message as well as the details, so an
API reason that echoes a key cannot print it. Human output shows the rest
of the API error as indented field/value lines rather than a JSON line,
including fields beside the error such as a request ID, and keeps the
reason when the message does not carry it. --json still reports the
complete response. The payments guide shows the quote-then-receive flow a
USD wallet needs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human review wanted CI passes; ready for a human reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants