feat(wallet-cli): Add wallet CLI with daemon support#8446
Draft
rekmarks wants to merge 32 commits into
Draft
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This comment was marked as resolved.
This comment was marked as resolved.
Base automatically changed from
rekm/wallet-library-tweaks
to
feat/wallet-library
April 14, 2026 10:50
98a9392 to
1c1a4bf
Compare
Member
Author
|
@SocketSecurity ignore npm/jake@10.9.4 Transitive via |
623f9af to
3fb60ee
Compare
45d013e to
6ec54b9
Compare
6687519 to
8e1bd9d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8e1bd9d. Configure here.
d9daf4f to
88c6f09
Compare
…rocess Adds a daemon that runs a Wallet instance as a detached background process, communicating over JSON-RPC via Unix domain sockets. This mirrors the architecture of kernel-cli's daemon. Infrastructure (src/daemon/): - socket-line: newline-delimited socket I/O - rpc-socket-server: generic Unix socket JSON-RPC server - daemon-client: one-shot JSON-RPC client with retry - daemon-entry: standalone entry point for the spawned process - daemon-spawn: spawns daemon-entry as detached child - stop-daemon: shared stop logic with escalation - wallet-factory: creates configured Wallet from config - utils, paths, types: process utilities and path resolution Commands (src/commands/daemon/): - start: start daemon (--infura-project-id or INFURA_PROJECT_ID env) - stop: stop daemon (RPC shutdown -> SIGTERM -> SIGKILL) - status: check daemon status - purge: stop daemon and delete all state files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9 test files covering all daemon infrastructure: paths, socket-line, utils, rpc-socket-server, daemon-client, stop-daemon, wallet-factory, daemon-entry, and daemon-spawn. 96 tests total. Config changes: - jest.config.js: exclude commands/ from coverage (not yet tested) - eslint.config.mjs: disable n/no-process-env and n/no-sync for wallet-cli test files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… typedoc and JSDoc - Await rm calls in shutdown finally block via Promise.all and in error cleanup path so callers know cleanup is complete - Use rpcErrors.parse() (-32700) for JSON.parse failures per JSON-RPC spec - Update typedoc.json entry points after index.ts removal - Move CONNECTION_TIMEOUT_MS above JSDoc so doc attaches to the function Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject params where the first element is not a string, preventing confusing downstream errors from messenger.call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Log errors in catch blocks instead of silently swallowing them across
daemon-entry, stop-daemon, and rpc-socket-server
- Make shutdown cleanup independent: handle.close(), wallet.destroy(),
and file removal each run in their own try/catch so one failure
doesn't skip the others
- Add default 30s timeout to sendCommand to prevent indefinite hangs
- Use JsonRpcResponse from @metamask/utils as handleRequest return type
- Add shared DaemonStatusInfo type for the getStatus RPC contract
- Change RpcHandler to allow void return, document null params
- Filter expected socket errors (EPIPE/ECONNRESET), log unexpected ones
- Only suppress ENOENT in socket unlink, re-throw other errors
- Clean up socket file (not just PID file) when stopDaemon succeeds
- Add child.on('error') handler in daemon-spawn for spawn failures
- Switch makeLogger from sync appendFileSync to async appendFile
- Remove socketPath from DaemonSpawnConfig, derive from dataDir
- Include error details in status command catch block
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the required --force flag with an interactive y/N confirmation prompt using @inquirer/confirm. The --force flag now skips the prompt instead of being mandatory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced May 4, 2026
The `@metamask/wallet` package is meant to be platform-agnostic, so the Node-only `better-sqlite3` persistence layer moves to the CLI together with its tests. `createWallet` now opens a `KeyValueStore` at `<dataDir>/wallet.db`, hydrates state via `loadState`, wires `subscribeToChanges`, and only imports the SRP on first run (detected by absence of a persisted KeyringController vault). On any post-construction failure the wallet is destroyed before the store closes, and first-run failures also remove the on-disk database so a retry can't latch onto an orphaned partial vault. Closes #8682 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves operational footguns surfaced by review of the daemon code:
- `pingDaemon` returns a discriminated `{ status: 'responsive' | 'absent' |
'unreachable' }` rather than a boolean. Callers can now distinguish "no
daemon" from "wedged daemon"; previously both produced silent
duplicate-spawns or wrong cleanup decisions.
- `ensureDaemon` returns `{ state: 'already-running' | 'started' }` and
refuses to spawn when the socket exists but is unreachable. The `daemon
start` command surfaces the "already running" case rather than silently
pretending new flags took effect.
- `daemon-entry` claims the daemon slot atomically: pre-flight refuses to
start when a responsive daemon owns the socket, then writes the PID file
with `flag: 'wx'` (exclusive create) recording `${pid}\n${startTime}`.
Cleanup paths only remove the PID file when its contents still match.
- `stop-daemon` only signals the recorded PID when we have evidence the
socket existed (responsive or unreachable); for absent sockets we treat
the PID file as stale and skip signalling, removing the PID-reuse
footgun. Post-stop cleanup wraps `rm` in best-effort handlers.
- `ensureDaemon` watches `child.on('exit')` so a daemon that crashes during
startup surfaces a real error pointing at the daemon log instead of a
30-second "did not start" timeout.
- `daemon call` catches socket errors and prints a friendly "daemon is not
running" hint for `ENOENT`/`ECONNREFUSED`.
- `daemon purge` deletes a whitelist of daemon-owned files (`pidPath`,
`socketPath`, `logPath`, `dbPath` + `-wal`/`-shm`) rather than rm'ing
the entire oclif `dataDir`.
- `daemon status` reports the unreachable case distinctly and warns when
the local PID file disagrees with the running daemon's reported PID.
- `isProcessAlive` rethrows unknown errors rather than treating them as
"process is gone".
- `sendCommand` verifies the response id matches the request id.
- Reworded a misleading inline comment and a `// TODO: Delete unsafe
flags` that no longer applied. Fixed a README path inaccuracy.
181 wallet-cli tests pass at 100% line/branch/function coverage.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves issues surfaced by a second review of the daemon hardening:
- Collapse `readPidFile` (utils.ts) and `readPidFromFile` (daemon-entry.ts)
into one function in utils.ts that parses the first line. The previous
commit changed the daemon to write `${pid}\n${startTime}\n` but the
shared reader still did `Number(contents)`, which evaluated to `NaN` —
silently disabling `stopDaemon`'s SIGTERM/SIGKILL fallbacks and the
`daemon status` PID-mismatch warning. Added a round-trip test using
the production format.
- `claimDaemonSlot` now also rm's the PID file in the
`existingPid === undefined` branch. A corrupt/truncated PID file from a
crashed run would otherwise permanently block startup with EEXIST on
the exclusive `wx` write.
- `claimDaemonSlot` refuses to clobber when the socket is `unreachable`
AND the recorded PID is still alive — mirrors `ensureDaemon`'s refusal
so direct invocations of `daemon-entry` (or races against parent ping)
can't orphan a sibling daemon either.
- `daemon purge` no longer aborts when `stopDaemon` returns false on a
daemon that isn't responsive — that's the exact state purge exists to
recover from. It still refuses when a daemon IS responsive.
- `packages/wallet/CHANGELOG.md` adds an `Added` entry for the
`importSecretRecoveryPhrase` export introduced earlier on this branch.
- Tightened JSDoc accuracy on `PingResult` (don't tell callers to refuse
destructive action when stopDaemon legitimately signals on unreachable),
on `stopDaemon` and `ensureDaemon` (replace "socket file exists" with
"ping yielded non-ENOENT"), and on `createWallet` (both `password` and
`srp` are unused on subsequent runs, not only the SRP).
- New tests: EEXIST race, unreachable+alive refusal, corrupt-PID-file
recovery, PID-file format round-trip.
186 wallet-cli tests pass at 100% coverage; 9 wallet tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 3 of review-driven fixes. Closes operational/UX gaps surfaced by the branch-wide review of `6eae8aa17` and tightens the CLI surface. Daemon IPC - `PingResult` adds a `reason: 'refused' | 'timeout' | 'permission' | 'protocol' | 'other'` discriminator so callers can act per failure mode instead of string-sniffing messages. `daemon start` reports a foreign-user daemon distinctly. The unreachable error is normalised to an `Error` instance at the producer so consumers cannot crash on string/object throws. - `startRpcSocketServer` validates incoming JSON via `@metamask/utils.isJsonRpcRequest` instead of casting `JSON.parse`'s output. The per-connection 30 s idle timer is now `unref`'d and cleared on socket close/error so an aborted probe cannot stall shutdown. - Server-side dispatch failures and unexpected socket errors flow through an optional `log` callback. The persistence layer's `subscribeToChanges` accepts the same callback. `daemon-entry.ts` wires both to the daemon log, so failures that previously vanished into `stdio: 'ignore'` now land in `daemon.log`. CLI UX - `daemon stop` reports `Daemon is not running` when there is nothing to stop, instead of returning silently. - `daemon purge` uses a static import for `@inquirer/confirm` so the prompt can be mocked in tests; whitelist deletion now extends to the SQLite `-wal` / `-shm` sidecars. Tests - Un-exclude `commands/` from coverage; add command-level tests for `start`, `stop`, `status`, `purge`, `call` covering the user-visible branches (responsive/absent/unreachable, PID-mismatch warning, refuse vs. proceed in purge, ENOENT/ECONNREFUSED friendly hints, JSON-array validation, TTY vs. piped output, etc.). A shared `runCommand` harness in `src/test/` invokes commands without going through `Config.load`. - New `src/daemon/socket-integration.test.ts` exercises a real Unix socket round trip: success response, error response, methodNotFound, shutdown RPC, concurrent in-flight requests, and pipelined-request rejection. Every other test mocks one side of this boundary; this guards the seams. - `isErrorWithCode` is now duck-typed instead of `instanceof Error` so errno values that crossed a Node-built-in realm boundary (under jest's `--experimental-vm-modules`) still classify correctly. Docs - `packages/wallet-cli/CHANGELOG.md` now lists the package's initial user-facing surface (commands + SQLite persistence) without internal hardening detail. 239 wallet-cli tests pass at 100% line/branch/function coverage; 9 wallet tests pass. Lint, constraints, both changelogs validate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
External review caught issues that the prior local validation missed
because I only ran tests + lint, not `yarn build`.
Build (now passes `yarn build` from the monorepo root)
- call.ts: type rpcParams as `Json[]` instead of `unknown[]` so the
`params` field actually satisfies `JsonRpcParams`.
- purge.ts / prompts.ts: factor the `@inquirer/confirm` dynamic import
out into a small `prompts.ts` wrapper. The previous static import broke
ts-bridge's CJS emit because the package is ESM-only.
- daemon-spawn.ts: rework exitInfo to a tagged container so TS can
re-narrow it after the await boundary.
- persistence.ts: keep the two `@ts-expect-error` directives on the
dynamically-constructed event subscriptions — they were stale under
the workspace-local build (which loses event-type information) but
load-bearing under the root build, where the messenger's full event
union is visible.
Concurrency: write PID before opening the database
- The order was `claimDaemonSlot → createWallet → wx write`. Two
concurrent `daemon start` invocations could both pass the preflight,
both open `<dataDir>/wallet.db`, and both run first-run SRP import
before one lost the wx race. Move the wx write before `createWallet`
so the loser fails fast without touching the DB. Adjust cleanup paths
to handle wallet/store being undefined.
- `claimDaemonSlot` now also refuses to clobber when the recorded PID
is alive AND the socket is absent (e.g. sibling daemon mid-startup
before bind, or socket manually rm'd from under a live daemon).
Symmetric with the unreachable+alive case.
Security: filesystem permissions
- `mkdirSync(dataDir, { recursive: true, mode: 0o700 })` so another
local user cannot traverse into the daemon's data dir.
- `chmod(socketPath, 0o600)` immediately after listen() so the socket
is owner-only. The daemon explicitly exposes the full wallet messenger
to anyone who can connect, so these permissions are the only access
boundary.
Docs
- `packages/wallet/CHANGELOG.md`: link PR #8446 alongside the issue,
matching AGENTS.md's PR-link convention.
241 wallet-cli tests pass at 100% coverage; 9 wallet tests pass.
`yarn build`, `yarn lint:eslint`, `yarn lint:misc:check`,
`yarn constraints`, both `changelog:validate`s all clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- `rpc-socket-server`: gate the handler lookup behind `Object.prototype.hasOwnProperty.call(handlers, method)` so a request with `method: "toString"` (or `"constructor"`, etc.) returns `methodNotFound` instead of resolving to an Object.prototype member. Closes the Cursor Bugbot prototype-method-invocation finding. - CI: `better-sqlite3@12.9.0` only ships prebuilt binaries for Node 20+ (declared `engines.node = "20.x || 22.x || 23.x || 24.x | 25.x"`), so the wallet-cli test job on 18.x fails when `prebuild-install` can't find a matching binary. Mirror the existing exclusion for `@metamask/wallet` in `.github/workflows/lint-build-test.yml`, bump `packages/wallet-cli`'s declared `engines.node` to `>=20`, and update the yarn constraint to allow that single deviation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- stop-daemon: signal the recorded PID when the socket is absent but the process is still alive. Previous behaviour treated the PID file as stale, returned success, and let `purge` wipe the SQLite DB while a live daemon still held handles. Trades a small recycled-PID risk for closing a real DB-corruption window. - daemon-entry: `chmod(dataDir, 0o700)` after `mkdirSync`. The `mode` option is ignored when the directory already exists, so first-run- with-existing-dir installs were leaving the dir at default umask. - Move `prompts.ts` out of `src/commands/daemon/` and into `src/daemon/` so oclif no longer discovers it as a `daemon:prompts` command (visible as a `command daemon:prompts not found` warning in `mm --help`). - CODEOWNERS: list `@MetaMask/ocap-kernel` alongside `@MetaMask/core- platform` for `packages/wallet-cli` to match `teams.json`. - CHANGELOG: drop the issue link from the persistence entry (both packages); AGENTS.md asks for PR links, not issue links. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Introduces an
oclifCLI for@metamask/wallet. It is more or less a port of the Ocap Kernel equivalent.Usage
First,
cd packages/wallet-cli.Then, to start the daemon:
Try calling an available controller action:
Actions params must be specified as a stringified JSON array:
When done, stop the daemon and clean up:
Summary
wallet-clipackage scaffolded with oclif (mmbinary)@metamask/walletinstance as a detached background process, communicating over JSON-RPC via Unix domain socketsstart,stop,status,purge,callcallcommand dispatches arbitrary messenger actions to the running daemon (e.g.mm daemon call AccountsController:listAccounts)Persistence (closes #8682)
The Node-only SQLite persistence layer that previously lived in
@metamask/wallet/persistencemoves to@metamask/wallet-cliso the wallet package stays platform-agnostic. The daemon now persists controller state across restarts.KeyValueStore,loadState, andsubscribeToChangesfrompackages/wallet/src/persistence/topackages/wallet-cli/src/persistence/along with their tests; relocate thebetter-sqlite3+@types/better-sqlite3deps and theprebuild-installblock of the test-prep script../persistencesubpath export from@metamask/wallet(breaking change — captured in its changelog).createWalletnow opens aKeyValueStoreat<dataDir>/wallet.db, hydrates theWalletfromloadState, wiressubscribeToChangesagainst the wallet's messenger +controllerMetadata, and only imports the SRP on first run (detected by absence of a persistedKeyringController.vault).daemon-entry.tscloses the store during shutdown and during startup-failure cleanup (logged, idempotent withwallet.destroy()).subscribeToChanges/importSecretRecoveryPhrase/etc.) the wallet is destroyed before the store closes so persistence handlers unsubscribe cleanly. On a first-run failure the on-disk database (wallet.db+-wal+-shm) is removed so a retry can't latch onto an orphaned partial vault.subscribeToChanges'controllerMetadataparameter is nowReadonly<Record<string, Readonly<StateMetadataConstraint>>>, matchingWallet.controllerMetadataexactly.On subsequent runs the persisted vault is reused and the SRP is unused; the wallet still starts locked (
KeyringController.isUnlockedispersist: false), so unlocking it remains the caller's responsibility — a follow-up will wire that into the daemon.Note
Medium Risk
Adds a new CLI + background daemon that can dispatch arbitrary
@metamask/walletmessenger actions over a local Unix socket, which is operationally/security sensitive if socket permissions or secret handling are misconfigured. Most changes are additive and well-tested, but they introduce new process/spawn/shutdown and IPC behavior.Overview
Introduces a new
@metamask/wallet-clipackage (oclifmmbinary) that can start a detached wallet daemon and interact with it over newline-delimited JSON-RPC on a Unix domain socket.Adds daemon lifecycle and IPC plumbing (
ensureDaemonspawn/polling,startRpcSocketServerrequest handling +shutdown, clientsendCommand/pingDaemon, PID/socket/log management) and CLI commands tostart,stop,status,purge, andcallarbitrary wallet messenger actions (with optional JSON-array params and timeouts).Wires the new package into the monorepo (tsconfig refs, README/dependency graph, CODEOWNERS/teams ownership, ESLint exceptions, yarn workspace/export validation exceptions) and updates
@metamask/walletexports to exposeimportSecretRecoveryPhrasefor CLI wallet bootstrapping.Reviewed by Cursor Bugbot for commit 41a2397. Bugbot is set up for automated code reviews on this repo. Configure here.