Skip to content

Follow-ups from invoice payment + bulk-return work (PRs #37, #38, #39 / sphere-sdk #405, #413) #40

@vrogojin

Description

@vrogojin

Carry-over from the invoice-payment + bulk-return work landed today (2026-06-06).
The end-to-end story is green — `sphere invoice pay ` (default `--amount`)
correctly covers the remaining across pay/refund cycles, verified by the
accounting soak. These are the smaller follow-ups noted along the way.


1. CLI render bug — `sphere invoice return ` (bulk) shows `amount: "0"`

Symptom. Output of the no-flag form is missing the actual refunded amount:

```
$ sphere invoice return 0000abcd...
Return payment results:
1 refund(s) submitted:
[0] 0 UCT → DIRECT://0000def0… ← should be the refunded amount, not 0
id : 645a1a4f-…
status : submitted
```

Root cause. `src/legacy/legacy-cli.ts` builds the display rows AFTER
calling `returnAllInvoicePayments`. By that point the SDK has already
drained `senderBalances[].netBalance` to zero (that's the whole point of
the refund), so the human-readable amount the CLI emits is the
post-refund value (0), not what was actually sent.

Fix. Capture `getInvoiceStatus().senderBalances` BEFORE the SDK call,
then align with `results[]` by index. Sketch:

```ts
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) {
for (const sb of ca.senderBalances) {
if (sdkOptions.recipient !== undefined && sb.senderAddress !== sdkOptions.recipient) continue;
if (BigInt(sb.netBalance) <= 0n) continue;
plan.push({ recipient: sb.senderAddress, coinId: ca.coin[0], netBalance: sb.netBalance });
}
}
}
const results = await sphere.accounting!.returnAllInvoicePayments(invoiceId, sdkOptions);
// results[i] now aligns with plan[i] — render using plan[i].netBalance
```

Cosmetic only — underlying refund is correct (V6 soak's `bob-return-delta-minus-3`
asserts the token-level delta). Just the display.

Scope. Small (~10 LoC + one regression test).


2. Integration test regex backlog from canonical-UX cutover

Symptom. `npm run test:integration` shows ~63 failed tests across
`test/integration/*.test.ts`. All failures share the same shape:

```
expect(out).toMatch(/Usage:\sinvoice-pay|usage:\sinvoice-pay/i)
→ actual: "Usage: npm run cli -- invoice-pay …"
```

The regex expects no characters between `Usage:` and the command name.
PR #33 (canonical UX) changed the help block to print
`Usage: npm run cli -- …`. The `npm run cli -- ` prefix breaks
the assertion.

Reproducible on main. Confirmed via:

```bash
git checkout main && npm run build && npx vitest run --config vitest.integration.config.ts

63 failed | 126 passed | 42 skipped

```

Scope. Bulk scrub. Either:

  • Update each regex to `/Usage:\s*(?:npm run cli -- )?/i`, OR
  • Add a shared helper `expectUsageHint(out, cmdName)` that all
    integration tests call. Same pattern, one place to fix the next time
    help format moves.

Affected files (per the run output):

  • cli-invoice.integration.test.ts (~10 cases)
  • cli-swap.integration.test.ts (~9 cases)
  • cli-crypto.integration.test.ts (~15 cases)
  • cli-group.integration.test.ts (~8 cases)
  • cli-wallet-lifecycle.integration.test.ts (~3 cases)
  • cli-nametag.integration.test.ts (~3 cases)
  • cli-assets.integration.test.ts (~2 cases)
  • cli-market.integration.test.ts (~3 cases)
  • cli-wallet-profile.integration.test.ts (~few)

3. `--asset-index` and `--target-index` selectors for multi-asset / multi-target invoices

Today the CLI's `invoice pay` and `invoice return` paths hard-code
`targetIndex: 0` and `assetIndex: 0`. The SDK supports both.

When to do this. Defer until a real multi-asset or multi-target
invoice scenario shows up — exposing the selectors without a use case
adds CLI surface area we'd have to maintain forever. The current single-
target single-asset shape is the only one exercised by the accounting
soak and the demo playbook.

If we do build it: `--target-index ` already exists on `invoice pay`
(per PR #37 / commit f1ef6c4). `--asset-index ` and the same flag on
`invoice return` are the gaps.


4. NFT-only `invoice pay` flow

Spec supports NFT requests in invoice targets (`assets[i].nft`) and
`invoice pay` is supposed to handle them, but no scenario in
`manual-test-accounting-roundtrip.sh` exercises this. `invoice return`
already rejects NFTs explicitly with a clear error
("`--amount only applies to coin-asset targets, not NFT`").

When to do this. Once an NFT-issuance / NFT-redeem scenario lands in
the soaks. The corresponding playbook section can demonstrate "Bob's
invoice requests a specific NFT; alice pays by sending it; bob confirms
ownership."


Suggested order

  1. feat: sphere-cli phase 2 — legacy bridge + host DM transport #1 (CLI render fix) — small, isolated, fixes a user-visible cosmetic regression in the bulk-return I just shipped.
  2. feat: integration tests against real public testnet infrastructure #2 (integration test scrub) — moderate but mechanical. Unblocks future PRs from inheriting these red tests.
  3. ci: harden nightly integration workflow — drop artifact upload #3 and fix(host): pre-flight --timeout floor at MIN_TIMEOUT_MS=100 #4 — defer until a real scenario forces them.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions