`wallet-cli`: make `sendCommand` retry idempotency-safe before mutating RPC ships

[sirtimid]

Summary

@metamask/wallet-cli's daemon client (sendCommand) automatically retries a request once on transient connection errors. It retries on both ECONNREFUSED and ECONNRESET, and ECONNRESET is not safe to blindly retry for a state-changing request: the daemon may have already received and executed the request before the connection dropped. This must be tightened before any mutating RPC (sign / send / transfer) becomes reachable, or a dropped connection could cause the same action to run twice (e.g. a transaction broadcast twice).

Current behavior

packages/wallet-cli/src/daemon/daemon-client.ts (sendCommand):

try {
  return await attempt();
} catch (error: unknown) {
  if (
    !isErrorWithCode(error, 'ECONNREFUSED') &&
    !isErrorWithCode(error, 'ECONNRESET')
  ) {
    throw error;
  }
  await new Promise((resolve) => setTimeout(resolve, 100));
  return attempt(); // resends the whole request
}

• ECONNREFUSED means the connection was never established — the daemon provably never received the request, so retrying is safe.
• ECONNRESET can occur after the request line was written and the daemon began (or finished) processing it. Retrying then re-sends a request that may already have taken effect.

The request id (randomUUID()) is stable across the retry, but the server does not deduplicate by id, so a second identical request is executed as new.

Why it matters (the failure scenario)

1. CLI sends a mutating request (e.g. "send transaction") to the daemon.
2. The daemon receives it and broadcasts the transaction.
3. The connection resets before the response reaches the CLI (ECONNRESET).
4. The CLI's retry re-sends the same request → the daemon broadcasts it again.
5. Result: a duplicated, non-reversible action.

The technical term: sending value is not idempotent (doing it twice ≠ doing it once), so a naive resend is a double-execution hazard.

Scope — why it's latent today but must be fixed before send lands

Today's sendCommand callers are effectively idempotent, so the bug is currently latent:

• getStatus (ping), status, list — read-only.
• shutdown (stop-daemon) — idempotent.
• unlock → KeyringController:submitPassword — idempotent.

But mm daemon call <Controller>:<method> is a general dispatch surface that can already route mutating actions (e.g. KeyringController:setLocked), and it is the exact path a future TransactionController send/transfer action would travel. The safety guarantee erodes the moment a genuinely non-idempotent action becomes callable. So this is a prerequisite for shipping any send/sign/transfer command, not a standalone bug to fix opportunistically.

Proposed fix (pick one or combine)

• Restrict connection-error retry to ECONNREFUSED only — drop ECONNRESET from the retry condition. Simplest; ECONNREFUSED is the only code that provably means "never received." (ECONNRESET would then surface as an error the caller handles.)
• Gate retry to idempotent methods — add an explicit retryable/idempotent flag to SendCommandOptions; only auto-retry read/idempotent methods (getStatus, listActions, getState, …), never arbitrary call dispatch or send/sign.
• Server-side dedup by request id — have the daemon track recently-seen request ids and return the prior result (or reject) on a duplicate. The id is already stable across the current retry, so this is feasible; heavier, but robust if retry-on-ECONNRESET is desired.

Recommended: restrict to ECONNREFUSED now (small, safe), and introduce the per-method idempotency flag when the send command is designed.

Acceptance criteria

• sendCommand no longer blindly re-sends a request that may have already reached the daemon (i.e. does not auto-retry a state-changing request on ECONNRESET).
• The chosen mechanism is documented in sendCommand's JSDoc (currently states it "retries once … on ECONNREFUSED, ECONNRESET").
• Tests cover: retry still happens for the safe/connect-phase case; no retry (or a documented dedup) for the unsafe case; a case where both attempts fail.
• 100% coverage maintained; build, test, yarn lint:fix, yarn lint, changelog:validate pass.

References

• packages/wallet-cli/src/daemon/daemon-client.ts — sendCommand retry block (~lines 91–102) and the getStatus ping caller.
• Callers: src/daemon/stop-daemon.ts (shutdown), src/commands/daemon/{status,call,list}.ts, src/commands/wallet/unlock.ts.

Owning teams: @MetaMask/core-platform @MetaMask/ocap-kernel.

Related

• wallet-cli: add the mm send-transaction command #9513 — hard prerequisite for: the mm send command (a mutating RPC must not be blindly retried).
• Related daemon transaction work: wallet-cli: make the daemon transaction-capable — consume GasFeeController + headless auto-approval #9512. Siblings in the "enable sending from the CLI" effort: Wire GasFeeController into @metamask/wallet default initialization #9510 (GasFeeController wiring), feat(wallet-cli): wire the TransactionController slot in the daemon wallet #9509 (TransactionController slot).