Skip to content

[CLI] wallet show is not machine-parseable, same as wallet list - #727

Merged
Codier merged 1 commit into
mainfrom
hulk-linus/api-686-wallet-show-json
Sep 30, 2026
Merged

Codier merged 1 commit into
mainfrom
hulk-linus/api-686-wallet-show-json

Conversation

@hulk-linus

@hulk-linus hulk-linus Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Closes API-686. Follow-up to #584.

Problem

nansen wallet show <name> printed a readable summary through log() and returned undefined, so nansen wallet show main | jq . failed with a parse error and --pretty, --table and --fields did nothing. The redacted default of nansen wallet export <name> had the same problem.

Change

  • wallet show returns showWallet(name). Stdout carries {"success":true,"data":{name, provider, evm, solana, createdAt, isDefault}}, plus privyWalletIds for Privy wallets. Same output in a terminal and a pipe, nothing on stderr, no isTTY switch, per the rule fix: wallet list writes only JSON to stdout (#154) #584 settled on.
  • wallet export <name> without --reveal or --file returns {name, redacted: true, evm: {address}, solana: {address}}. The key fields are left out instead of holding a [REDACTED] placeholder, so nothing key-shaped reaches a caller. It still never decrypts or asks for a password, and still rejects non-local wallets. --reveal and --file are unchanged.
  • The old redacted view also printed how to get the keys (--file, --reveal). That hint is gone from the output; the wallet export description in schema.json (shown by --help) says it.
  • schema.json: returns for wallet show and wallet export; the export description notes the JSON default.
  • AGENTS.md: the operational-commands exception now lists wallet show and redacted wallet export.
  • Changeset: minor, since the output format of both commands changes.

Checks

  • npm test: 109 files, 4503 passed, 9 skipped.
  • npm run lint and npm run mcp:check pass.
  • New tests drive runCLI with isTTY false and true for wallet show and redacted wallet export, and require one parseable envelope with nothing on stderr. Also covered: Privy privyWalletIds, --fields on show, and no key material or 64-hex blob in the export output.
  • Mutation: with src/wallet.js restored to origin/main, all 6 new tests fail; with the fix, they pass.
  • Real CLI with a temp HOME: wallet show main | jq and wallet export main | jq parse with empty stderr, wallet show main --table renders a table, wallet show nope returns {"success":false,...,"code":"SHOW_FAILED"} with exit 1, and --reveal still prints text.

BI

No event affected. Both commands still send the same cli_command_succeeded / cli_command_failed event with the same command path. Returning data sends the success event through the data branch of runCLI, which passes from_cache explicitly; the undefined branch left it at its false default, and nothing on a wallet command reads from the cache, so the properties are identical. The event carries no result data.

AI execution metadata

  • Provider: Anthropic (Claude Code)
  • Model: claude-opus-5-5
  • Thinking level: default (no explicit reasoning override)
  • Token usage: 64 input, 2,195,053 cache read, 86,564 cache write, 13,285 output; 2,294,966 total, as of PR creation. Reasoning output is not reported separately.
  • Attribution: creation-only, combined implementation and orchestration in one session. Independent review usage (Factory droid deepseek-v4-pro, gpt-5.6-sol) is reported separately in the review posts.

🤖 Generated with Claude Code

`wallet show <name>` printed a readable summary through log() and
returned undefined, so `nansen wallet show main | jq .` failed and
--pretty, --table and --fields did nothing. It now returns showWallet(),
following the rule #584 set for `wallet list`: same envelope in a
terminal and a pipe, nothing on stderr, no isTTY switch.

The redacted default of `wallet export` had the same problem. It now
returns the addresses with `redacted: true` and no key fields. The
--reveal and --file paths are unchanged.

Adds `returns` for both commands to schema.json. Tests drive runCLI with
isTTY true and false.

Closes API-686.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nansen-pr-reviewer

Copy link
Copy Markdown

pr-reviewer Summary for #43ae539

✅ No issues found

The code review completed successfully with no findings.

Review effort: 2/5 (Simple)

Summary

This PR extends the machine-parseable JSON envelope pattern (established in PR #584 for wallet list) to wallet show and the redacted default of wallet export. The changes are minimal, correct, and well-tested.

Production changes reviewed:

  • src/wallet.js — show handler drops the log() summary and returns showWallet(name) directly. export redacted branch drops the multi-line log() block and returns a clean {name, redacted, evm: {address}, solana: {address}} object. Both are exactly what the CLI layer needs to emit the standard {"success":true,"data":{...}} envelope.
  • src/schema.json — returns arrays added for both commands; the export description updated. Content is accurate and consistent with the implementation.
  • AGENTS.md — exception list for operational commands updated to include wallet show and redacted wallet export. Matches the new behaviour.
  • .changeset/fix-wallet-show-stdout-json.md — minor bump, matching the precedent set by the wallet list fix (.changeset/fix-wallet-list-stdout-json.md).

Tests reviewed:

  • New for (const isTTY of [false, true]) loops in both test files verify the same JSON envelope appears regardless of terminal state, nothing on stderr, and no key material. Privy privyWalletIds and --fields filtering are also covered for wallet show.
  • The old runList helper in wallet.test.js is cleanly refactored into runWallet with no loss of existing coverage.
  • wallet-export-guard.test.js replaces the old buildWalletCommands-direct test with a runCLI call that mirrors the new contract; existing --reveal, --file, security, and telemetry tests are untouched.

No bugs, logic errors, security issues, or CLAUDE.md/AGENTS.md violations found.


Token usage: 11,074 input, 4,276 output, 796,809 cache read, 51,538 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

@nansen-pr-reviewer nansen-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected

If you have any concerns, please request a manual review.

@Codier Codier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewer A (Factory droid deepseek-v4-pro, max effort) reviewed head 43ae539 and found nothing to fix.

What it checked:

  • npm test: 109 files, 4503 passed, 9 skipped. Lint and mcp:check pass.
  • It copied the base src/wallet.js over the fix: exactly the 6 new tests failed. It also injected a decrypted EVM key into the redacted export result: both redacted-export tests failed. The checkout was clean afterwards.
  • Real CLI with a throwaway HOME: wallet show in default, --pretty, --table, --format csv, --stream and --fields modes gives a valid envelope. Under a pseudo-terminal the JSON was byte-identical, so there is no isTTY branch.
  • It grepped the real EVM and Solana keys against stdout and stderr of redacted wallet export in every output mode, including --fields privateKey: no hits. Redacted export succeeds with a wrong password and with the credential store deleted, so it never decrypts.
  • Privy: wallet show returns privyWalletIds; wallet export still rejects Privy wallets with EXPORT_FAILED.
  • BI: with fetch intercepted, wallet show, redacted wallet export and wallet default (still on the no-output branch) send cli_command_succeeded with the same property keys, from_cache: false and no result data. A missing wallet sends cli_command_failed with SHOW_FAILED. The PR's BI claim holds.
  • Nothing in src/, scripts/ or evals/ parses show/export stdout; internal callers import showWallet/exportWallet directly.

Judgment calls it accepted: dropping the "how to export keys" hint follows the no-stderr, no-isTTY rule, and --help plus the schema description still document both flags. returns on wallet export follows the cache stats precedent. Omitting the key fields, rather than null or [REDACTED], is unambiguous.

Not run: npm run test:privy, which needs real Privy credentials. The Privy path was covered with a synthetic wallet file.

Reviewer B reviews next.

@Codier Codier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewer B (Factory droid gpt-5.6-sol, high effort) independently reviewed head 43ae539 and found nothing to fix. Round A made no changes, so this is the same head.

What it checked:

  • Focused tests: 2 files, 97 passed. Full npm test: 109 files, 4503 passed, 9 skipped. Lint, mcp:check and git diff --check pass.
  • With the base handlers restored, exactly the 6 new tests failed. A mutation that decrypts in the redacted path made both redacted-export guard tests fail.
  • Real CLI: default JSON, --pretty, --table, --format csv, --stream, --fields, missing-wallet errors, and legacy or partial wallet files all behaved. No synthetic key material appeared in any output.
  • BI: base and head telemetry match. Same cli_command_succeeded event and paths, from_cache: false, no result data.
  • No internal caller relies on the old text or the undefined return; direct showWallet/exportWallet callers are unchanged.

The checkout stayed clean.

@Codier Codier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving head 43ae539 on the evidence from two independent reviewers. Both returned clean with no findings, and no code changed between the rounds.

  • Reviewer A (Factory deepseek-v4-pro, max): review
  • Reviewer B (Factory gpt-5.6-sol, high): review

Across both rounds:

  • The full suite passes.
  • The new tests fail against the old handlers.
  • wallet show and redacted wallet export give one JSON envelope on stdout in every output mode. It is the same in a pipe and a pseudo-terminal, with nothing on stderr.
  • No private key material appears in any mode, and redacted export never decrypts.
  • BI telemetry is unchanged.

All CI checks on this head pass. The writer was Claude Opus 5.5; this approval rests on the external reviewers' reports, not the writer's own checks.

@Codier
Codier merged commit 9404c13 into main Sep 30, 2026
19 checks passed
@Codier
Codier deleted the hulk-linus/api-686-wallet-show-json branch September 30, 2026 00:08
@github-actions github-actions Bot mentioned this pull request Sep 29, 2026
kome12 added a commit that referenced this pull request Oct 6, 2026
All three swap e2e groups gated on `wallet list` printing an `EVM:` /
`Solana:` summary, but that command reports data and so prints the
standard JSON envelope on stdout in a terminal and a pipe alike (#584,
extended to `wallet show` and redacted `wallet export` in #727). The
preconditions could therefore never pass, and because each group is
sequential, the whole file was unrunnable.

Parse the envelope through one `walletAddresses()` helper instead of
grepping prose, and take the Solana address from the parsed wallet rather
than a base58 regex over human output.

Pre-existing on main; unrelated to the execution route, but it blocks any
e2e run of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant