fix(cli)(#40): bulk-return amount display + integration test scrub (#41)

Closes follow-ups #1 and #2 from issue #40.

Item #1 — `sphere invoice return <id>` (bulk) shows actual refunded
amount instead of `amount: "0"`. The bug: `getInvoiceStatus()` ran
AFTER `returnAllInvoicePayments()`, by which point the SDK had drained
`senderBalances[].netBalance` to 0 (refunds had been issued), so every
row in the rendered output showed 0. Fix: snapshot status BEFORE the
SDK call, mirror the SDK's iteration filters so `plan[i] ↔ results[i]`.

Static-analysis pin in `legacy-cli-ux.test.ts` asserts the call
ordering doesn't regress (first `await getInvoiceStatus(` must precede
first `await returnAllInvoicePayments(` inside the invoice-return
case).

Item #2 — integration test backlog from PR #33 canonical-UX cutover.
Down from 63 → 3 failures (remaining 3 are unrelated regressions,
called out below).

  • `expectUsageHint(out, cmdName, positional?)` helper in
    `test/integration/helpers.ts`: tolerates the new
    `Usage: npm run cli -- <cmd>` prefix so the next help-format
    change is a one-line fix. All ~13 call sites updated.
  • JSON-shape stdout captures (`/"directAddress": "..."/` etc.)
    updated to canonical-UX labelled-block form
    (`/directAddress\s*:\s*.../`) across 9 test files.
  • `--asset "1000000 UCT"` quoted single-string form (rejected by
    the canonical asset-pair guard) replaced with two-positional form
    in cli-invoice/cli-swap.
  • `expect.toMatch(/Asset not found/)` → case-insensitive — the
    error string is lowercased by `failWithHelp` now. Same for
    `No invoice found matching prefix:`.
  • `crypto decrypt` now accepts BOTH bare base64 (the new
    canonical-UX `crypto encrypt` output) and JSON-quoted base64
    (legacy `--json` callers). Restores the
    `encrypt foo pw | decrypt - pw` roundtrip.
  • Added `swap-ping` HELP_TEXT block and completion entry (the
    `failWithHelp('swap-ping', ...)` calls were already there;
    only the help registration was missing). Moved swap-ping into
    `SWAP_SUBCOMMANDS` and dropped the obsolete
    "no help available (HELP_TEXT gap pin)" test.

Items #3 (`--asset-index` / `--target-index`) and #4 (NFT-only
`invoice pay`) deferred per the issue's suggested order.

Out of scope, remaining integration failures (3):

  • `cli-multiaddress`: `tokens/` subdir ENOENT after `payments
    switch 1` — likely Profile (OrbitDB) cutover relocating the
    per-address token store off `.sphere-cli/tokens/`. Separate
    bug; not Usage/UX cutover.
  • `cli-wallet-lifecycle`: `sphere clear --yes` succeeds, but
    `sphere status` afterwards auto-generates a new wallet and
    reports it instead of "No wallet found". Real CLI/SDK
    regression; needs a separate ticket.
  • `cli-wallet-lifecycle`: `init --nametag` flake — testnet
    nametag mint sometimes doesn't complete inside the timeout.
    Locally green when the mint succeeds; the regex now correctly
    matches the canonical UX `nametag : @<name>` form.

Unit tests (127) all green. Type-check + lint clean.

Co-authored-by: Vladimir Rogojin <vrogojin@blockyinnovations.com>

Author: vrogojin

src/legacy/legacy-cli-ux.test.ts

@@ -127,3 +127,52 @@ describe('issue #32 — UX consistency', () => {
     expect(SOURCE).toMatch(/args\.includes\('--help'\)\s*\|\|\s*args\.includes\('-h'\)/);
   });
 });
+
+describe('issue #40 item #1 — bulk-return amount display', () => {
+  // Static-analysis pin for the bulk-refund render bug:
+  //
+  //   `sphere invoice return <id>` (no flags / --recipient form) used to
+  //   call `getInvoiceStatus()` AFTER `returnAllInvoicePayments()`. By
+  //   that point the SDK had drained `senderBalances[].netBalance` to 0
+  //   (refunds had been issued), so every refund row in the rendered
+  //   output showed `amount: "0"`. Fix: snapshot status BEFORE the SDK
+  //   call so we render the actual refunded amounts.
+  //
+  //   Pin: inside the `case 'invoice-return':` block, the first
+  //   `getInvoiceStatus(` call must appear BEFORE the first
+  //   `returnAllInvoicePayments(` call. Source ordering reflects
+  //   execution ordering here — both are top-level `await`s, no
+  //   conditional skips between them.
+
+  it('getInvoiceStatus is called BEFORE returnAllInvoicePayments in invoice-return', () => {
+    const caseStart = SOURCE.indexOf("case 'invoice-return':");
+    expect(caseStart, "case 'invoice-return': not found").toBeGreaterThan(-1);
+
+    // Locate end of case — next top-level `case '...'` or `default:` token.
+    const after = SOURCE.slice(caseStart + 1);
+    const nextCaseRel = after.search(/\n {6}case '[^']+':|\n {6}default:/);
+    const caseEnd = nextCaseRel >= 0 ? caseStart + 1 + nextCaseRel : SOURCE.length;
+    const block = SOURCE.slice(caseStart, caseEnd);
+
+    // Match the actual `await sphere.accounting!.<method>(` call sites,
+    // not bare `<method>(` (would also match comment references like
+    // "iterates `getInvoiceStatus().senderBalances` internally").
+    const statusCallRe   = /await\s+sphere\.accounting!?\.getInvoiceStatus\s*\(/;
+    const returnAllRe    = /await\s+sphere\.accounting!?\.returnAllInvoicePayments\s*\(/;
+    const statusMatch    = statusCallRe.exec(block);
+    const returnAllMatch = returnAllRe.exec(block);
+
+    expect(
+      statusMatch,
+      'getInvoiceStatus() call missing from invoice-return — bulk-refund render needs the pre-refund snapshot.',
+    ).not.toBeNull();
+    expect(
+      returnAllMatch,
+      'returnAllInvoicePayments() call missing from invoice-return — Form B/C delegates to the SDK.',
+    ).not.toBeNull();
+    expect(
+      statusMatch!.index,
+      'getInvoiceStatus() must be called BEFORE returnAllInvoicePayments() so renderer shows pre-refund amounts (issue #40 item #1).',
+    ).toBeLessThan(returnAllMatch!.index);
+  });
+});

src/legacy/legacy-cli.ts

@@ -1709,6 +1709,18 @@ const COMMAND_HELP: Record<string, CommandHelp> = {
       'Accepts full 64-char swap ID or a unique prefix (min 4 chars).',
     ],
   },
+  'swap-ping': {
+    usage: 'swap-ping <@nametag_or_address>',
+    description: 'Ping an escrow service to verify it is online and reachable via the configured transport.',
+    examples: [
+      'npm run cli -- swap-ping @escrow',
+      'npm run cli -- swap-ping DIRECT://0000abcd...',
+    ],
+    notes: [
+      'Accepts an @nametag, DIRECT://… address, or any other identifier swap-propose accepts.',
+      'On success, prints the escrow\'s pong response (transport pubkey, capabilities).',
+    ],
+  },
 
   'swap-reject': {
     usage: 'swap-reject <swap_id_or_prefix> [reason]',
@@ -3533,9 +3545,21 @@ async function main(): Promise<void> {
       case 'decrypt': {
         const [, encrypted, password] = args;
         if (!encrypted || !password) {
-          failWithHelp('decrypt', 'missing required <encrypted-json> <password> arguments');
+          failWithHelp('decrypt', 'missing required <encrypted> <password> arguments');
+        }
+        // Accept both shapes: bare base64 (what `crypto encrypt` now
+        // emits under canonical UX) and JSON-quoted base64 (legacy
+        // wrapper, kept for backward compat with scripts that pipe
+        // `encrypt --json` output). Trying JSON.parse first preserves
+        // the legacy roundtrip; falling back to the raw string makes
+        // the natural `crypto encrypt | xargs crypto decrypt` pipeline
+        // work without manual quoting.
+        let encryptedData: string;
+        if (encrypted.startsWith('"') && encrypted.endsWith('"')) {
+          encryptedData = JSON.parse(encrypted) as string;
+        } else {
+          encryptedData = encrypted;
         }
-        const encryptedData = JSON.parse(encrypted);
         const result = decrypt(encryptedData, password);
         console.log(result);
         break;
@@ -4961,38 +4985,51 @@ async function main(): Promise<void> {
           if (explicitRecipient) {
             sdkOptions.recipient = await resolveRecipientToDirect(sphere, explicitRecipient, 'invoice-return');
           }
+
+          // Snapshot per-sender balances BEFORE the SDK call. The SDK drains
+          // `senderBalances[].netBalance` as it refunds, so re-reading status
+          // afterwards produces `amount: 0` rows (issue #40 item #1). Mirror
+          // the SDK's iteration order + filters so plan[i] ↔ results[i].
+          const statusBefore = await sphere.accounting!.getInvoiceStatus(invoiceId);
+          const plan: Array<{ recipient: string; coinId: string; netBalance: string }> = [];
+          for (const target of statusBefore.targets) {
+            for (const ca of target.coinAssets) {
+              const [coinId] = ca.coin;
+              for (const sb of ca.senderBalances) {
+                let bal: bigint;
+                try {
+                  bal = BigInt(sb.netBalance);
+                } catch {
+                  continue;
+                }
+                if (bal <= 0n) continue;
+                if (sdkOptions.recipient !== undefined && sb.senderAddress !== sdkOptions.recipient) continue;
+                plan.push({ recipient: sb.senderAddress, coinId, netBalance: sb.netBalance });
+              }
+            }
+          }
+
           const results = await sphere.accounting!.returnAllInvoicePayments(invoiceId, sdkOptions);
           if (results.length === 0) {
             const reason = explicitRecipient
               ? `no refundable balance for recipient "${explicitRecipient}" on invoice ${invoiceId.slice(0, 16)}…`
               : `no refundable balance found on invoice ${invoiceId.slice(0, 16)}… — nothing to return`;
             failWithHelp('invoice-return', reason);
           }
-          // The SDK returns TransferResult per row but doesn't echo back the
-          // (recipient, amount, coin) tuple — re-attach those from status so
-          // the human renderer can show what got refunded where.
-          const status = await sphere.accounting!.getInvoiceStatus(invoiceId);
+
           const refundRows: Array<Record<string, unknown>> = [];
-          let cursor = 0;
-          for (const target of status.targets) {
-            for (const ca of target.coinAssets) {
-              const [coinId] = ca.coin;
-              const { decimals } = resolveCoin(coinId);
-              for (const sb of ca.senderBalances) {
-                // Skip rows that returnAllInvoicePayments wouldn't have
-                // refunded (zero balance, or recipient filter).
-                if (sdkOptions.recipient !== undefined && sb.senderAddress !== sdkOptions.recipient) continue;
-                if (cursor >= results.length) break;
-                const result = results[cursor++];
-                refundRows.push({
-                  id: result.id,
-                  status: result.status,
-                  recipient: sb.senderAddress,
-                  amount: toHumanReadable(sb.netBalance, decimals),
-                  coin: coinId,
-                });
-              }
-            }
+          const rowCount = Math.min(plan.length, results.length);
+          for (let i = 0; i < rowCount; i++) {
+            const row = plan[i];
+            const result = results[i];
+            const { decimals } = resolveCoin(row.coinId);
+            refundRows.push({
+              id: result.id,
+              status: result.status,
+              recipient: row.recipient,
+              amount: toHumanReadable(row.netBalance, decimals),
+              coin: row.coinId,
+            });
           }
           formatOutput(
             { refunds: refundRows } as Record<string, unknown>,
@@ -5787,6 +5824,7 @@ function getCompletionCommands(): CompletionCommand[] {
     { name: 'swap-accept', description: 'Accept a swap deal', flags: ['--deposit', '--no-wait'] },
     { name: 'swap-status', description: 'Show swap status', flags: ['--query-escrow'] },
     { name: 'swap-deposit', description: 'Deposit into a swap' },
+    { name: 'swap-ping', description: 'Ping an escrow service' },
     { name: 'swap-reject', description: 'Reject a swap proposal' },
     { name: 'swap-cancel', description: 'Cancel a swap' },
     { name: 'market-post', description: 'Post a market intent', flags: ['--type', '--category', '--price', '--currency', '--location', '--contact', '--expires'] },

test/integration/cli-assets.integration.test.ts

@@ -45,6 +45,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import {
   createSphereEnv,
   destroySphereEnv,
+  expectUsageHint,
   runSphere,
   integrationSkip,
   type SphereEnv,
@@ -92,8 +93,7 @@ describe('sphere-cli — asset-info arg validation (offline)', () => {
     // "did I type the right command" probe offline-fast for users.
     const r = runSphere(env, ['payments', 'asset-info'], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(/Usage:\s*asset-info\s*<symbol\|name\|coinId>/i);
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, 'asset-info', '<symbol|name|coinId>');
   });
 });
 
@@ -175,7 +175,10 @@ describe.skipIf(integrationSkip)(
       const r = runSphere(env, ['payments', 'asset-info', 'NOT_A_REAL_TOKEN_ZZZ'], { timeoutMs: 120_000 });
       expect(r.status).not.toBe(0);
       const out = `${r.stdout}\n${r.stderr}`;
-      expect(out).toMatch(/Asset not found/);
+      // Case-insensitive because failWithHelp lowercases its prose
+      // ("asset not found: ...") — the asserted invariant is the
+      // negative-lookup verdict reaching the user, not the casing.
+      expect(out).toMatch(/asset not found/i);
     }, 180_000);
   },
 );

test/integration/cli-crypto.integration.test.ts

@@ -37,6 +37,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import {
   createSphereEnv,
   destroySphereEnv,
+  expectUsageHint,
   runSphere,
   type SphereEnv,
 } from './helpers.js';
@@ -113,8 +114,7 @@ describe('sphere-cli — crypto/util arg validation (offline)', () => {
   ])('`sphere crypto %s` with no args prints usage and exits non-zero', (sub, legacyName) => {
     const r = runSphere(env, ['crypto', sub], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(new RegExp(`Usage:\\s*${legacyName}|usage:\\s*${legacyName}`, 'i'));
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, legacyName);
   });
 
   it.each([
@@ -124,17 +124,15 @@ describe('sphere-cli — crypto/util arg validation (offline)', () => {
   ])('`sphere util %s` with no args prints usage and exits non-zero', (sub, legacyName) => {
     const r = runSphere(env, ['util', sub], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(new RegExp(`Usage:\\s*${legacyName}|usage:\\s*${legacyName}`, 'i'));
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, legacyName);
   });
 
   it('`sphere crypto encrypt foo` (missing password) prints usage and exits non-zero', () => {
     // encrypt + decrypt require TWO positionals — missing the second
     // also hits the pre-getSphere() guard.
     const r = runSphere(env, ['crypto', 'encrypt', 'foo'], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(/Usage:\s*encrypt|usage:\s*encrypt/i);
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, 'encrypt');
   });
 });
 
@@ -187,17 +185,21 @@ describe('sphere-cli — crypto behaviour (offline)', () => {
   it('`sphere crypto validate-key <hex>` accepts a valid private key', () => {
     const r = runSphere(env, ['crypto', 'validate-key', TEST_PRIVKEY]);
     expect(r.status).toBe(0);
-    // Output is JSON: {"valid":true,"length":64}
-    expect(r.stdout).toMatch(/"valid":\s*true/);
-    expect(r.stdout).toMatch(/"length":\s*64/);
+    // Canonical-UX output is human-friendly labelled blocks
+    //   `  valid  : true`
+    //   `  length : 64`
+    // Pad-aware regex (zero-or-more whitespace around `:`) so future
+    // alignment tweaks don't rip these out.
+    expect(r.stdout).toMatch(/valid\s*:\s*true/);
+    expect(r.stdout).toMatch(/length\s*:\s*64/);
   });
 
   it('`sphere crypto validate-key not-hex` rejects an invalid key', () => {
     // Per the help text: "Exits with code 0 if valid, 1 if invalid."
-    // So we expect non-zero exit AND a "valid":false JSON.
+    // So we expect non-zero exit AND `valid : false` in the output.
     const r = runSphere(env, ['crypto', 'validate-key', 'not-hex']);
     expect(r.status).not.toBe(0);
-    expect(r.stdout).toMatch(/"valid":\s*false/);
+    expect(r.stdout).toMatch(/valid\s*:\s*false/);
   });
 
   it('`sphere crypto hex-to-wif` produces a stable WIF for the test private key', () => {
@@ -274,17 +276,17 @@ describe('sphere-cli — util behaviour (offline)', () => {
   });
 
   it('`sphere crypto encrypt` / `decrypt` roundtrips a string with a password', () => {
-    // Pin the AES envelope: encrypt produces a JSON-quoted base64 blob
+    // Pin the AES envelope: encrypt produces a base64 blob
     // ("U2FsdGVkX1+..."), decrypt restores the original plaintext.
     // Failure mode pinned by a regression: a change in the cipher,
     // salt format, or PBKDF2 iteration count would either fail the
     // decrypt or yield a different plaintext.
     //
-    // IMPORTANT: decrypt's first positional MUST be the JSON-quoted
-    // form ("U2Fsd..."), not the bare base64. The handler runs
-    // JSON.parse on argv[1] to extract the string. Stripping the
-    // surrounding quotes here would make decrypt fail with
-    // "Unexpected token 'U', ... is not valid JSON".
+    // Canonical UX: `crypto encrypt` emits bare base64 (no JSON
+    // wrapping) so `crypto encrypt foo pw | crypto decrypt - pw`
+    // pipelines naturally. `crypto decrypt` accepts both the bare form
+    // AND the JSON-quoted legacy form, so scripts piping `--json`
+    // output continue to work.
     const plaintext = 'integration-test-secret';
     const password = 'p4ssw0rd-test';
     const enc = runSphere(env, ['crypto', 'encrypt', plaintext, password]);
@@ -293,9 +295,9 @@ describe('sphere-cli — util behaviour (offline)', () => {
     expect(ciphertext.length).toBeGreaterThan(2);
     // OpenSSL-compatible AES envelope starts with the magic "Salted__"
     // header, base64-encoded as "U2FsdGVkX1" (zero-pad). Pin the prefix
-    // (inside the quote) so a switch to a non-OpenSSL-compatible scheme
-    // breaks compat-hungry downstream tooling.
-    expect(ciphertext).toMatch(/^"U2FsdGVkX1/);
+    // so a switch to a non-OpenSSL-compatible scheme breaks
+    // compat-hungry downstream tooling.
+    expect(ciphertext).toMatch(/^U2FsdGVkX1/);
 
     const dec = runSphere(env, ['crypto', 'decrypt', ciphertext, password]);
     expect(dec.status).toBe(0);

test/integration/cli-dm.integration.test.ts

@@ -48,9 +48,9 @@ describe.skipIf(integrationSkip)('sphere-cli integration — DM round-trip (real
       throw new Error('wallet init failed; cannot proceed with DM tests');
     }
 
-    // Identity JSON is emitted as pretty-printed JSON inside the init output.
-    // Extract directAddress with a lenient regex (order-independent of other fields).
-    const match = init.stdout.match(/"directAddress":\s*"(DIRECT:\/\/[0-9a-fA-F]+)"/);
+    // Canonical UX init emits `  directAddress : DIRECT://...` as a
+    // labelled line inside the identity block.
+    const match = init.stdout.match(/directAddress\s*:\s*(DIRECT:\/\/[0-9a-fA-F]+)/);
     if (!match) throw new Error(`directAddress not found in init output:\n${init.stdout}`);
     directAddress = match[1]!;
   }, 180_000);

test/integration/cli-group.integration.test.ts

@@ -39,6 +39,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import {
   createSphereEnv,
   destroySphereEnv,
+  expectUsageHint,
   runSphere,
   type SphereEnv,
 } from './helpers.js';
@@ -108,16 +109,14 @@ describe('sphere-cli — group arg validation (offline)', () => {
   ])('`sphere group %s` with no args prints usage and exits non-zero', (sub, legacyName) => {
     const r = runSphere(env, ['group', sub], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(new RegExp(`Usage:\\s*${legacyName}|usage:\\s*${legacyName}`, 'i'));
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, legacyName);
   });
 
   it('`sphere group send <groupId>` (missing message) prints usage and exits non-zero', () => {
     // group-send requires TWO positionals; missing the second also
     // hits the pre-getSphere() guard.
     const r = runSphere(env, ['group', 'send', '00deadbeef'], { timeoutMs: 15_000 });
     expect(r.status).not.toBe(0);
-    const out = `${r.stdout}\n${r.stderr}`;
-    expect(out).toMatch(/Usage:\s*group-send|usage:\s*group-send/i);
+    expectUsageHint(`${r.stdout}\n${r.stderr}`, 'group-send');
   });
 });