From 44f01462098fdffa0c3b34a32c441cd2d6e00682 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 29 Jun 2026 17:24:41 +0200 Subject: [PATCH 01/12] test(wallet-cli): add subprocess daemon lifecycle e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an end-to-end suite that spawns the built `mm` CLI as real child processes and drives the full daemon lifecycle over the Unix socket — the gap left by the in-process suites (`socket-integration.test.ts` exercises the transport in-realm; `wallet-factory.e2e.test.ts` constructs a `Wallet` in-process). Covers: `start` (and the already-running guard on a second `start`), `call` returning the SRP-derived account, `status`, `stop`, persistence across a restart (the resume path: the wallet comes back locked rather than re-importing the SRP), `purge`, and the owner-only socket (0600) / data dir (0700). Because it needs the built `dist/` and the native better-sqlite3 addon, it runs as its own jest project (`jest.config.e2e.js`) via a new `test:e2e` script and is excluded from the fast unit `test` run and its 100%-coverage gate. A dedicated `test-wallet-cli-e2e` CI job (Node 20.x and 24.x) builds the dependency subtree and runs it; the per-package `test-*` matrix can't host it because it runs against source with no build. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/lint-build-test.yml | 30 +++ packages/wallet-cli/README.md | 6 + packages/wallet-cli/jest.config.e2e.js | 29 +++ packages/wallet-cli/jest.config.js | 16 +- packages/wallet-cli/package.json | 1 + .../src/daemon/lifecycle.daemon-e2e.test.ts | 233 ++++++++++++++++++ 6 files changed, 313 insertions(+), 2 deletions(-) create mode 100644 packages/wallet-cli/jest.config.e2e.js create mode 100644 packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 306446104fb..efa8cafa418 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -222,3 +222,33 @@ jobs: echo "Working tree dirty at end of job" exit 1 fi + + # The wallet-cli daemon e2e spawns the BUILT `mm` CLI and the native + # better-sqlite3 addon as real child processes, so it needs its dependency + # subtree built first and cannot run in the per-package `test-*` matrix above + # (which runs against source with no build). It has its own job here. Runs on + # the package's engine floor (20.x) and latest (24.x); better-sqlite3 needs + # Node >=20. + test-wallet-cli-e2e: + name: Test wallet-cli daemon e2e (${{ matrix.node-version }}) + runs-on: ubuntu-latest + needs: prepare + strategy: + matrix: + node-version: [20.x, 24.x] + steps: + - name: Checkout and setup environment + uses: MetaMask/action-checkout-and-setup@v2 + with: + is-high-risk-environment: false + node-version: ${{ matrix.node-version }} + - name: Build wallet-cli and its dependencies + run: yarn workspaces foreach --topological-dev --recursive --from '@metamask/wallet-cli' run build + - run: yarn workspace @metamask/wallet-cli run test:e2e + - name: Require clean working directory + shell: bash + run: | + if ! git diff --exit-code; then + echo "Working tree dirty at end of job" + exit 1 + fi diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index 29ac82275b1..6be52efeae1 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -55,6 +55,12 @@ Or invoke `prebuild-install` directly from the monorepo root (where `better-sqli cd node_modules/better-sqlite3 && node ../.bin/prebuild-install ``` +## Testing + +Unit tests run with `yarn workspace @metamask/wallet-cli test`. + +The subprocess end-to-end suite (`*.daemon-e2e.test.ts`) spawns the built `mm` CLI and the native `better-sqlite3` addon as real processes, so it is kept out of the unit run and its coverage gate. Build the workspace dependencies first (`yarn build` from the repo root), then run it with `yarn workspace @metamask/wallet-cli test:e2e`. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/wallet-cli/jest.config.e2e.js b/packages/wallet-cli/jest.config.e2e.js new file mode 100644 index 00000000000..b6764581282 --- /dev/null +++ b/packages/wallet-cli/jest.config.e2e.js @@ -0,0 +1,29 @@ +/* + * Jest configuration for the subprocess daemon e2e suite (`*.daemon-e2e.test.ts`). + * + * Kept separate from `jest.config.js` because this suite spawns the BUILT `mm` + * CLI and the native `better-sqlite3` addon as real child processes: it must + * stay out of the fast unit `test` run and must not be held to that run's + * 100%-coverage gate (subprocess work is invisible to in-process coverage). + * Run it with `yarn test:e2e`. + */ + +const merge = require('deepmerge'); + +const baseConfig = require('../../jest.config.packages'); + +module.exports = merge(baseConfig, { + displayName: 'wallet-cli:e2e', + + // Only the subprocess e2e suite; the default config runs everything else. + testMatch: ['**/*.daemon-e2e.test.ts'], + + // Coverage is meaningless here — the work happens in spawned processes — so + // collecting it would only report the e2e harness as uncovered source. + collectCoverage: false, + + // The CLI runs in a normal Node process with the Web Crypto globals, so this + // suite needs neither the `jest.environment.js` polyfill nor any coverage + // threshold. + testEnvironment: 'node', +}); diff --git a/packages/wallet-cli/jest.config.js b/packages/wallet-cli/jest.config.js index bc5f44e07f4..d4509b236d7 100644 --- a/packages/wallet-cli/jest.config.js +++ b/packages/wallet-cli/jest.config.js @@ -21,8 +21,20 @@ module.exports = merge(baseConfig, { // The test harness in `src/test/` is exercised by the command tests but // not all of its error/edge branches are worth driving directly — it's - // production code's test infrastructure, not production code itself. - coveragePathIgnorePatterns: ['.*/src/test/.*'], + // production code's test infrastructure, not production code itself. The + // daemon e2e is skipped from this run (below), so it never executes here and + // must also be excluded from coverage, or it would report as 0%-covered + // source. + coveragePathIgnorePatterns: [ + '.*/src/test/.*', + '.*\\.daemon-e2e\\.test\\.ts$', + ], + + // The subprocess daemon e2e (`*.daemon-e2e.test.ts`) spawns the built CLI and + // the native sqlite addon; it has its own config (`jest.config.e2e.js`, run + // via `yarn test:e2e`) and must not run in — or be coverage-gated by — the + // fast unit suite. + testPathIgnorePatterns: ['/node_modules/', '\\.daemon-e2e\\.test\\.ts$'], // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { diff --git a/packages/wallet-cli/package.json b/packages/wallet-cli/package.json index aaa4f06e1c6..696de1b92e6 100644 --- a/packages/wallet-cli/package.json +++ b/packages/wallet-cli/package.json @@ -39,6 +39,7 @@ "test:prepare": "./scripts/install-binaries.sh", "test": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", + "test:e2e": "yarn build && yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.e2e.js", "test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose", "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, diff --git a/packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts new file mode 100644 index 00000000000..68af8b8d512 --- /dev/null +++ b/packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts @@ -0,0 +1,233 @@ +import { spawn } from 'node:child_process'; +import { mkdtemp, rm, stat } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { getDaemonPaths } from './paths'; +import { isProcessAlive, readPidFile } from './utils'; + +// Subprocess-level lifecycle test for the `mm daemon` command suite. Unlike the +// in-process suites (`socket-integration.test.ts` exercises the transport in +// the test realm; `wallet-factory.e2e.test.ts` constructs a real `Wallet` +// in-process), this spawns the BUILT `mm` CLI as a child process against a temp +// data directory and drives the real `start → call → status/stop/purge` +// lifecycle over the Unix socket. It needs `dist/` and the native +// `better-sqlite3` addon, so it runs only via `yarn test:e2e` (its own jest +// config), excluded from the fast unit `test` run and its 100%-coverage gate. +// +// Offline-safe: the daemon's startup neither fetches feature flags +// (RemoteFeatureFlagController only fetches in `updateRemoteFeatureFlags`) nor +// looks up the network (NetworkController's `init` is synchronous), and the +// only action called here, `KeyringController:getState`, is local. + +// A valid 12-word BIP-39 mnemonic — the same fixtures the in-process e2e uses. +const TEST_SRP = 'test test test test test test test test test test test ball'; +const TEST_PASSWORD = 'testpass'; +// NetworkController requires a project ID but is never reached over the network +// here, so any well-formed-looking value works. +const TEST_INFURA_PROJECT_ID = '00000000000000000000000000000000'; + +const ADDRESS_REGEX = /^0x[0-9a-fA-F]{40}$/u; + +const BIN_PATH = join(__dirname, '..', '..', 'bin', 'run.mjs'); + +// Each step (spawn the CLI, construct a real Wallet, run PBKDF2 key derivation +// for the first-run SRP import) is slow; give the whole lifecycle room. +const STEP_TIMEOUT_MS = 60_000; + +type RunResult = { code: number | null; stdout: string; stderr: string }; + +/** + * Run the built `mm` CLI as a child process and capture its output. + * + * `NODE_OPTIONS` is stripped from the child environment so the parent jest + * run's `--experimental-vm-modules` flag does not leak an ExperimentalWarning + * onto the CLI's stderr. + * + * @param args - CLI arguments (e.g. `['daemon', 'start']`). + * @param dataDir - Data directory to point the CLI at (via `MM_DATA_DIR`). + * @returns The exit code and captured stdout/stderr. + */ +async function runMm(args: string[], dataDir: string): Promise { + const env = { ...process.env }; + delete env.NODE_OPTIONS; + + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [BIN_PATH, ...args], { + env: { + ...env, + MM_DATA_DIR: dataDir, + INFURA_PROJECT_ID: TEST_INFURA_PROJECT_ID, + MM_WALLET_PASSWORD: TEST_PASSWORD, + MM_WALLET_SRP: TEST_SRP, + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => (stdout += chunk.toString())); + child.stderr.on('data', (chunk) => (stderr += chunk.toString())); + child.on('error', reject); + child.on('close', (code) => resolve({ code, stdout, stderr })); + }); +} + +/** + * Call a messenger action on the running daemon and parse its JSON result. + * + * @param action - The messenger action name. + * @param dataDir - Data directory the daemon is using. + * @returns The parsed result object. + */ +async function callAction( + action: string, + dataDir: string, +): Promise> { + const result = await runMm(['daemon', 'call', action], dataDir); + expect(result.code).toBe(0); + return JSON.parse(result.stdout.trim()); +} + +/** + * Whether a path exists, without throwing on absence. + * + * @param path - The path to check. + * @returns True if `stat` succeeds. + */ +async function exists(path: string): Promise { + try { + await stat(path); + return true; + } catch { + return false; + } +} + +/** + * Guarantee no daemon is left running and remove the temp data directory, + * regardless of how a test ended. Kills by the recorded PID directly (rather + * than going through `mm daemon stop`) so a wedged daemon cannot block cleanup. + * + * @param dataDir - The temp data directory to tear down. + */ +async function cleanup(dataDir: string): Promise { + const { pidPath } = getDaemonPaths(dataDir); + const pid = await readPidFile(pidPath).catch(() => undefined); + if (pid !== undefined && isProcessAlive(pid)) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + // Already gone — nothing to clean up. + } + } + await rm(dataDir, { recursive: true, force: true }); +} + +describe('mm daemon lifecycle (subprocess e2e)', () => { + let dataDir: string; + + beforeEach(async () => { + dataDir = await mkdtemp(join(tmpdir(), 'mm-e2e-')); + }); + + afterEach(async () => { + await cleanup(dataDir); + }); + + it( + 'starts, reports already-running, answers call & status, then stops', + async () => { + const start = await runMm(['daemon', 'start'], dataDir); + expect(start.code).toBe(0); + expect(start.stdout).toMatch(/Daemon running\. Socket:/u); + + // A second start finds the responsive daemon and leaves it untouched. + const restart = await runMm(['daemon', 'start'], dataDir); + expect(restart.code).toBe(0); + expect(restart.stdout).toMatch(/already running/iu); + + // First run imports the SRP, so the wallet is unlocked and exposes the + // derived account. + const keyringState = await callAction( + 'KeyringController:getState', + dataDir, + ); + expect(keyringState.isUnlocked).toBe(true); + const keyrings = keyringState.keyrings as { accounts: string[] }[]; + expect(keyrings[0]?.accounts[0]).toMatch(ADDRESS_REGEX); + + const status = await runMm(['daemon', 'status'], dataDir); + expect(status.code).toBe(0); + expect(status.stdout).toMatch( + /Daemon is running\. PID: \d+, Uptime: \d+s/u, + ); + + // The socket holds an unlocked wallet and the data dir holds the vault, so + // both must be owner-only (the only access-control boundary). The low 3 + // octal digits of `mode` are the permission bits. + const paths = getDaemonPaths(dataDir); + const socketMode = (await stat(paths.socketPath)).mode + .toString(8) + .slice(-3); + const dirMode = (await stat(dataDir)).mode.toString(8).slice(-3); + expect(socketMode).toBe('600'); + expect(dirMode).toBe('700'); + + const stop = await runMm(['daemon', 'stop'], dataDir); + expect(stop.code).toBe(0); + + const statusAfterStop = await runMm(['daemon', 'status'], dataDir); + expect(statusAfterStop.code).toBe(0); + expect(statusAfterStop.stdout).toMatch(/not running/iu); + }, + STEP_TIMEOUT_MS, + ); + + it( + 'resumes the persisted vault across a restart instead of re-importing', + async () => { + await runMm(['daemon', 'start'], dataDir); + const firstRun = await callAction('KeyringController:getState', dataDir); + expect(firstRun.isUnlocked).toBe(true); + const firstKeyrings = firstRun.keyrings as { accounts: string[] }[]; + expect(firstKeyrings[0]?.accounts[0]).toMatch(ADDRESS_REGEX); + + await runMm(['daemon', 'stop'], dataDir); + + // The on-disk database survives the stop. + const { dbPath } = getDaemonPaths(dataDir); + expect(await exists(dbPath)).toBe(true); + + await runMm(['daemon', 'start'], dataDir); + + // On the second start the persisted vault is found, so first-run SRP + // import is skipped and the wallet resumes LOCKED — a re-import would + // have left it unlocked. This is the observable signature of the + // `hasPersistedKeyring` resume path. + const resumed = await callAction('KeyringController:getState', dataDir); + expect(resumed.isUnlocked).toBe(false); + expect(typeof resumed.vault).toBe('string'); + + await runMm(['daemon', 'stop'], dataDir); + }, + STEP_TIMEOUT_MS, + ); + + it( + 'purges all daemon state', + async () => { + await runMm(['daemon', 'start'], dataDir); + + const purge = await runMm(['daemon', 'purge', '--force'], dataDir); + expect(purge.code).toBe(0); + expect(purge.stdout).toMatch(/All daemon state deleted/u); + + const paths = getDaemonPaths(dataDir); + expect(await exists(paths.dbPath)).toBe(false); + expect(await exists(paths.socketPath)).toBe(false); + expect(await exists(paths.pidPath)).toBe(false); + }, + STEP_TIMEOUT_MS, + ); +}); From 372de87f20888c6964f92c8877b85583de30fbc0 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 29 Jun 2026 22:55:34 +0200 Subject: [PATCH 02/12] move e2e tests --- eslint.config.mjs | 1 + .../{src/daemon => tests}/lifecycle.daemon-e2e.test.ts | 6 +++--- packages/wallet-cli/tsconfig.json | 6 ++---- 3 files changed, 6 insertions(+), 7 deletions(-) rename packages/wallet-cli/{src/daemon => tests}/lifecycle.daemon-e2e.test.ts (97%) diff --git a/eslint.config.mjs b/eslint.config.mjs index 3cab21fa9c7..c87252b6894 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -321,6 +321,7 @@ const config = createConfig([ { files: [ 'packages/wallet-cli/src/**/*.test.{js,ts}', + 'packages/wallet-cli/tests/**/*.{js,ts}', 'packages/platform-api-docs/**/*.{js,ts}', ], rules: { diff --git a/packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts similarity index 97% rename from packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts rename to packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts index 68af8b8d512..3e34db1b6ac 100644 --- a/packages/wallet-cli/src/daemon/lifecycle.daemon-e2e.test.ts +++ b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts @@ -3,8 +3,8 @@ import { mkdtemp, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { getDaemonPaths } from './paths'; -import { isProcessAlive, readPidFile } from './utils'; +import { getDaemonPaths } from '../src/daemon/paths'; +import { isProcessAlive, readPidFile } from '../src/daemon/utils'; // Subprocess-level lifecycle test for the `mm daemon` command suite. Unlike the // in-process suites (`socket-integration.test.ts` exercises the transport in @@ -29,7 +29,7 @@ const TEST_INFURA_PROJECT_ID = '00000000000000000000000000000000'; const ADDRESS_REGEX = /^0x[0-9a-fA-F]{40}$/u; -const BIN_PATH = join(__dirname, '..', '..', 'bin', 'run.mjs'); +const BIN_PATH = join(__dirname, '..', 'bin', 'run.mjs'); // Each step (spawn the CLI, construct a real Wallet, run PBKDF2 key derivation // for the first-run SRP import) is slow; give the whole lifecycle room. diff --git a/packages/wallet-cli/tsconfig.json b/packages/wallet-cli/tsconfig.json index 7a5498120f9..5f3efa9c6fe 100644 --- a/packages/wallet-cli/tsconfig.json +++ b/packages/wallet-cli/tsconfig.json @@ -1,9 +1,7 @@ { "extends": "../../tsconfig.packages.json", "compilerOptions": { - "baseUrl": "./", - "outDir": "./dist", - "rootDir": "./src" + "baseUrl": "./" }, "references": [ { "path": "../base-controller/tsconfig.json" }, @@ -11,5 +9,5 @@ { "path": "../storage-service/tsconfig.json" }, { "path": "../wallet/tsconfig.json" } ], - "include": ["../../types", "./bin", "./src"] + "include": ["../../types", "./bin", "./src", "./tests"] } From 0d7edc4d690baaf3d042089980f818455842da57 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 30 Jun 2026 13:08:00 +0200 Subject: [PATCH 03/12] chore: simplify comment --- .github/workflows/lint-build-test.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index efa8cafa418..b7ed1da6da2 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -225,10 +225,7 @@ jobs: # The wallet-cli daemon e2e spawns the BUILT `mm` CLI and the native # better-sqlite3 addon as real child processes, so it needs its dependency - # subtree built first and cannot run in the per-package `test-*` matrix above - # (which runs against source with no build). It has its own job here. Runs on - # the package's engine floor (20.x) and latest (24.x); better-sqlite3 needs - # Node >=20. + # subtree built first and cannot run in the per-package `test-*` matrix above. test-wallet-cli-e2e: name: Test wallet-cli daemon e2e (${{ matrix.node-version }}) runs-on: ubuntu-latest From 82d2ccc2a5e7423ccaa656aaf04faa86a96873ff Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 30 Jun 2026 13:28:30 +0200 Subject: [PATCH 04/12] fix: show logs and report dead daemon as stopped --- .../src/daemon/daemon-spawn.test.ts | 26 +++++++++- .../wallet-cli/src/daemon/daemon-spawn.ts | 19 +++++-- .../wallet-cli/src/daemon/stop-daemon.test.ts | 17 +++++++ packages/wallet-cli/src/daemon/stop-daemon.ts | 28 ++++++++--- .../tests/lifecycle.daemon-e2e.test.ts | 50 ++++++++++++++++--- 5 files changed, 120 insertions(+), 20 deletions(-) diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts index 353dbf20328..5d693286bb0 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { closeSync, existsSync, openSync } from 'node:fs'; import { pingDaemon } from './daemon-client'; import { ensureDaemon } from './daemon-spawn'; @@ -13,9 +13,15 @@ jest.mock('./paths'); const mockSpawn = jest.mocked(spawn); const mockExistsSync = jest.mocked(existsSync); +const mockOpenSync = jest.mocked(openSync); +const mockCloseSync = jest.mocked(closeSync); const mockPingDaemon = jest.mocked(pingDaemon); const mockGetDaemonPaths = jest.mocked(getDaemonPaths); +// Arbitrary fd handed back by the mocked `openSync` so tests can assert it is +// wired into the child's stdio and later closed in the parent. +const LOG_FD = 7; + const CONFIG: DaemonSpawnConfig = { dataDir: '/tmp/data', infuraProjectId: 'test-key', @@ -79,6 +85,7 @@ describe('ensureDaemon', () => { logPath: '/tmp/test.log', dbPath: '/tmp/wallet.db', }); + mockOpenSync.mockReturnValue(LOG_FD); setupSpawnMock(); }); @@ -128,7 +135,7 @@ describe('ensureDaemon', () => { ['/pkg/dist/daemon/daemon-entry.mjs'], expect.objectContaining({ detached: true, - stdio: 'ignore', + stdio: ['ignore', 'ignore', LOG_FD], env: expect.objectContaining({ MM_DAEMON_DATA_DIR: '/tmp/data', MM_DAEMON_SOCKET_PATH: '/tmp/test.sock', @@ -141,6 +148,21 @@ describe('ensureDaemon', () => { ); }); + it('redirects the daemon stderr to its log file and closes the parent fd', async () => { + mockPingDaemon + .mockResolvedValueOnce(ABSENT) + .mockResolvedValueOnce(RESPONSIVE); + mockExistsSync.mockReturnValue(true); + + await ensureDaemon(CONFIG); + + expect(mockOpenSync).toHaveBeenCalledWith('/tmp/test.log', 'a'); + const spawnOptions = mockSpawn.mock.calls[0][2] as { stdio: unknown }; + expect(spawnOptions.stdio).toStrictEqual(['ignore', 'ignore', LOG_FD]); + // The child dups the fd, so the parent must close its own copy. + expect(mockCloseSync).toHaveBeenCalledWith(LOG_FD); + }); + it('returns started when the spawned daemon becomes responsive', async () => { mockPingDaemon .mockResolvedValueOnce(ABSENT) diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.ts b/packages/wallet-cli/src/daemon/daemon-spawn.ts index ff6f429d0ad..e401ceaa96f 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.ts @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { closeSync, existsSync, openSync } from 'node:fs'; import { join } from 'node:path'; import { pingDaemon } from './daemon-client'; @@ -38,7 +38,7 @@ export type EnsureDaemonResult = { export async function ensureDaemon( config: DaemonSpawnConfig, ): Promise { - const { socketPath } = getDaemonPaths(config.dataDir); + const { socketPath, logPath } = getDaemonPaths(config.dataDir); const initialPing = await pingDaemon(socketPath); if (initialPing.status === 'responsive') { @@ -63,9 +63,18 @@ export async function ensureDaemon( const { entryPath, args } = resolveEntryPoint(config.packageRoot); + // Redirect the daemon's stderr into its log file rather than discarding it. + // The daemon is detached, so anything it writes to stderr — the top-level + // `Daemon fatal: ...` line, an uncaught stack trace, or a native + // `better-sqlite3` abort — would otherwise vanish, leaving a daemon that dies + // after startup completely undiagnosable (e.g. a `daemon stop` that then + // finds a stale socket and a dead PID). `stdout` stays ignored: structured + // status already goes through the file logger. The child dups the fd on + // spawn, so the parent closes its own copy immediately. + const logFd = openSync(logPath, 'a'); const child = spawn(process.execPath, [...args, entryPath], { detached: true, - stdio: 'ignore', + stdio: ['ignore', 'ignore', logFd], env: { ...process.env, MM_DAEMON_DATA_DIR: config.dataDir, @@ -75,6 +84,10 @@ export async function ensureDaemon( MM_WALLET_SRP: config.srp, }, }); + // The child has dup'd the fd into its own stderr; the parent no longer needs + // its copy. `spawn` reports runtime failures via the 'error' event rather + // than throwing synchronously, so closing here is safe on the success path. + closeSync(logFd); type ExitInfo = { code: number | null; signal: NodeJS.Signals | null }; const exitInfo: { value: ExitInfo | null } = { value: null }; diff --git a/packages/wallet-cli/src/daemon/stop-daemon.test.ts b/packages/wallet-cli/src/daemon/stop-daemon.test.ts index 5c45f09c2bb..bf7a956ca72 100644 --- a/packages/wallet-cli/src/daemon/stop-daemon.test.ts +++ b/packages/wallet-cli/src/daemon/stop-daemon.test.ts @@ -50,6 +50,23 @@ describe('stopDaemon', () => { expect(mockSendSignal).not.toHaveBeenCalled(); }); + it('cleans up a stale socket and PID file when the socket is unreachable but the process is dead', async () => { + mockReadPidFile.mockResolvedValue(123); + mockPingDaemon.mockResolvedValue(UNREACHABLE); + mockIsProcessAlive.mockReturnValue(false); + + const result = await stopDaemon('/tmp/test.sock', '/tmp/test.pid'); + + // A daemon that crashed leaves a connectable-but-dead socket and its PID + // file behind; the daemon is gone, so report success and clear both. + expect(result).toBe(true); + expect(mockRm).toHaveBeenCalledWith('/tmp/test.pid', { force: true }); + expect(mockRm).toHaveBeenCalledWith('/tmp/test.sock', { force: true }); + // The recorded PID is dead, so never signal it — it may have been recycled. + expect(mockSendSignal).not.toHaveBeenCalled(); + expect(mockSendCommand).not.toHaveBeenCalled(); + }); + it('signals the recorded PID when the socket is absent but the process is still alive', async () => { mockReadPidFile.mockResolvedValue(123); mockPingDaemon.mockResolvedValue(ABSENT); diff --git a/packages/wallet-cli/src/daemon/stop-daemon.ts b/packages/wallet-cli/src/daemon/stop-daemon.ts index f62e8d77744..5f0036cd029 100644 --- a/packages/wallet-cli/src/daemon/stop-daemon.ts +++ b/packages/wallet-cli/src/daemon/stop-daemon.ts @@ -7,12 +7,23 @@ import { isProcessAlive, readPidFile, sendSignal, waitFor } from './utils'; * Stop the daemon via a `shutdown` RPC call. Falls back to PID + SIGTERM if * the socket is unresponsive, and escalates to SIGKILL if SIGTERM is ignored. * - * Signals are sent when EITHER the socket was observed (`responsive` or - * `unreachable`) OR the recorded PID is still alive on its own. The - * socket-absent + alive-PID branch trades a small risk of signalling a + * Resolution order when a live daemon is present: + * 1. If the socket is responsive, request a graceful `shutdown` over it. + * 2. If the recorded PID is still alive, escalate to SIGTERM. + * 3. ...then SIGKILL. + * + * Signals (steps 2-3) are only ever sent against a PID that is observed alive. + * The socket-absent + alive-PID branch trades a small risk of signalling a * recycled PID for the larger risk of leaving an orphan daemon holding the - * SQLite database — which `daemon purge` would otherwise wipe out from - * under it. + * SQLite database — which `daemon purge` would otherwise wipe out from under + * it. + * + * When the socket is NOT responsive AND the recorded PID is dead (or there is + * no PID file), there is no live daemon: a lingering socket or PID file is + * stale leftovers from a daemon that already exited — typically one that + * crashed without running its own cleanup. Those files are removed and the + * stop is reported as successful, rather than failing on a daemon that is + * already gone. * * @param socketPath - The daemon socket path. * @param pidPath - The daemon PID file path. @@ -30,9 +41,12 @@ export async function stopDaemon( ping.status === 'responsive' || ping.status === 'unreachable'; const processAlive = pid !== undefined && isProcessAlive(pid); - if (!socketObserved && !processAlive) { - // No live daemon evidence. Just remove the stale PID file if any. + if (ping.status !== 'responsive' && !processAlive) { + // No live daemon: the socket is not answering and the recorded PID (if + // any) is dead. Remove any stale socket/PID files left behind by a daemon + // that already exited and report success. await cleanupFile(pidPath, 'PID file', log); + await cleanupFile(socketPath, 'socket file', log); return true; } diff --git a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts index 3e34db1b6ac..72adf8f5547 100644 --- a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts +++ b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process'; -import { mkdtemp, rm, stat } from 'node:fs/promises'; +import { mkdtemp, readFile, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -73,6 +73,40 @@ async function runMm(args: string[], dataDir: string): Promise { }); } +/** + * Assert that a CLI invocation exited 0. + * + * On failure this throws an error embedding the captured stdout/stderr AND the + * daemon's own log file. The harness otherwise discards all three when only the + * exit code is asserted, which leaves a CI-only failure (one we cannot + * reproduce locally) impossible to diagnose from the run output. The daemon log + * is the only window into the spawned process: it records `Shutting down + * (...)`, `handle.close() failed`, `Daemon fatal: ...`, etc. + * + * @param step - Human-readable label for the CLI step (e.g. `daemon stop`). + * @param result - The captured run result. + * @param dataDir - Data directory the daemon is using (to locate its log). + */ +async function expectSuccessfulRun( + step: string, + result: RunResult, + dataDir: string, +): Promise { + if (result.code === 0) { + return; + } + const { logPath } = getDaemonPaths(dataDir); + const daemonLog = await readFile(logPath, 'utf-8').catch( + (error: unknown) => ``, + ); + throw new Error( + `Expected \`mm ${step}\` to exit 0 but it exited ${String(result.code)}.\n` + + `=== stdout ===\n${result.stdout}\n` + + `=== stderr ===\n${result.stderr}\n` + + `=== ${logPath} ===\n${daemonLog}\n`, + ); +} + /** * Call a messenger action on the running daemon and parse its JSON result. * @@ -85,7 +119,7 @@ async function callAction( dataDir: string, ): Promise> { const result = await runMm(['daemon', 'call', action], dataDir); - expect(result.code).toBe(0); + await expectSuccessfulRun(`daemon call ${action}`, result, dataDir); return JSON.parse(result.stdout.trim()); } @@ -139,12 +173,12 @@ describe('mm daemon lifecycle (subprocess e2e)', () => { 'starts, reports already-running, answers call & status, then stops', async () => { const start = await runMm(['daemon', 'start'], dataDir); - expect(start.code).toBe(0); + await expectSuccessfulRun('daemon start', start, dataDir); expect(start.stdout).toMatch(/Daemon running\. Socket:/u); // A second start finds the responsive daemon and leaves it untouched. const restart = await runMm(['daemon', 'start'], dataDir); - expect(restart.code).toBe(0); + await expectSuccessfulRun('daemon start (restart)', restart, dataDir); expect(restart.stdout).toMatch(/already running/iu); // First run imports the SRP, so the wallet is unlocked and exposes the @@ -158,7 +192,7 @@ describe('mm daemon lifecycle (subprocess e2e)', () => { expect(keyrings[0]?.accounts[0]).toMatch(ADDRESS_REGEX); const status = await runMm(['daemon', 'status'], dataDir); - expect(status.code).toBe(0); + await expectSuccessfulRun('daemon status', status, dataDir); expect(status.stdout).toMatch( /Daemon is running\. PID: \d+, Uptime: \d+s/u, ); @@ -175,10 +209,10 @@ describe('mm daemon lifecycle (subprocess e2e)', () => { expect(dirMode).toBe('700'); const stop = await runMm(['daemon', 'stop'], dataDir); - expect(stop.code).toBe(0); + await expectSuccessfulRun('daemon stop', stop, dataDir); const statusAfterStop = await runMm(['daemon', 'status'], dataDir); - expect(statusAfterStop.code).toBe(0); + await expectSuccessfulRun('daemon status (after stop)', statusAfterStop, dataDir); expect(statusAfterStop.stdout).toMatch(/not running/iu); }, STEP_TIMEOUT_MS, @@ -220,7 +254,7 @@ describe('mm daemon lifecycle (subprocess e2e)', () => { await runMm(['daemon', 'start'], dataDir); const purge = await runMm(['daemon', 'purge', '--force'], dataDir); - expect(purge.code).toBe(0); + await expectSuccessfulRun('daemon purge --force', purge, dataDir); expect(purge.stdout).toMatch(/All daemon state deleted/u); const paths = getDaemonPaths(dataDir); From d393d47a84e04160a92ab391900081527e07ce19 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 30 Jun 2026 13:41:31 +0200 Subject: [PATCH 05/12] lint --- packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts index 72adf8f5547..f17aec07e9d 100644 --- a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts +++ b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts @@ -212,7 +212,11 @@ describe('mm daemon lifecycle (subprocess e2e)', () => { await expectSuccessfulRun('daemon stop', stop, dataDir); const statusAfterStop = await runMm(['daemon', 'status'], dataDir); - await expectSuccessfulRun('daemon status (after stop)', statusAfterStop, dataDir); + await expectSuccessfulRun( + 'daemon status (after stop)', + statusAfterStop, + dataDir, + ); expect(statusAfterStop.stdout).toMatch(/not running/iu); }, STEP_TIMEOUT_MS, From 393604f81f27220f02fbddabb39bc3ac6434a892 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 30 Jun 2026 14:08:47 +0200 Subject: [PATCH 06/12] fix closing socket --- .../wallet-cli/src/daemon/socket-line.test.ts | 18 ++++++++++++++++++ packages/wallet-cli/src/daemon/socket-line.ts | 14 +++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/wallet-cli/src/daemon/socket-line.test.ts b/packages/wallet-cli/src/daemon/socket-line.test.ts index 0fc460f765a..400d2b80617 100644 --- a/packages/wallet-cli/src/daemon/socket-line.test.ts +++ b/packages/wallet-cli/src/daemon/socket-line.test.ts @@ -38,6 +38,24 @@ describe('writeLine', () => { await expect(writeLine(socket, 'hello')).rejects.toThrow('write failed'); }); + it('keeps an error listener for the trailing event a failed write also emits', async () => { + const socket = createMockSocket(); + const writeError = Object.assign(new Error('EPIPE'), { code: 'EPIPE' }); + (socket.write as jest.Mock).mockImplementation( + (_data: string, callback: (e?: Error) => void) => callback(writeError), + ); + + await expect(writeLine(socket, 'hello')).rejects.toThrow('EPIPE'); + + // A failed write surfaces both via the callback (above) AND as a separate + // 'error' event. A listener must remain so that emission is handled; + // otherwise Node throws "Unhandled 'error' event" and crashes the process. + expect(socket.listenerCount('error')).toBe(1); + expect(() => socket.emit('error', writeError)).not.toThrow(); + // Once it fires, the listener detaches itself. + expect(socket.listenerCount('error')).toBe(0); + }); + it('rejects when the socket emits an error before the write completes', async () => { const socket = createMockSocket(); // Never invoke the write callback; the failure arrives via the 'error' event. diff --git a/packages/wallet-cli/src/daemon/socket-line.ts b/packages/wallet-cli/src/daemon/socket-line.ts index 6959a792b76..dc96580cbd5 100644 --- a/packages/wallet-cli/src/daemon/socket-line.ts +++ b/packages/wallet-cli/src/daemon/socket-line.ts @@ -18,12 +18,20 @@ export async function writeLine(socket: Socket, line: string): Promise { socket.once('error', onError); socket.write(`${line}\n`, (error) => { - socket.removeListener('error', onError); if (error) { + // A failed write (e.g. EPIPE when the peer closed mid-write) is + // delivered BOTH here AND as a separate 'error' event on the socket. + // Leave `onError` attached so that event still has a handler — + // detaching it here would let Node treat the emission as unhandled and + // crash the whole process, even though this rejection is caught + // upstream. `onError` rejects idempotently and detaches itself when the + // event arrives; rejecting here settles the promise even if it never + // does. reject(error); - } else { - resolve(); + return; } + socket.removeListener('error', onError); + resolve(); }); }); } From 7c0b7dffa35a15da7ebf88ce6a8e5e5cb81ce47c Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 30 Jun 2026 14:34:07 +0200 Subject: [PATCH 07/12] fix write log bug --- .../src/daemon/daemon-spawn.test.ts | 27 +++++++++++++++++++ .../wallet-cli/src/daemon/daemon-spawn.ts | 7 +++++ 2 files changed, 34 insertions(+) diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts index 5d693286bb0..93e6c916456 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts @@ -3,18 +3,21 @@ import { closeSync, existsSync, openSync } from 'node:fs'; import { pingDaemon } from './daemon-client'; import { ensureDaemon } from './daemon-spawn'; +import { ensureOwnerOnlyDirectory } from './data-dir'; import { getDaemonPaths } from './paths'; import type { DaemonSpawnConfig } from './types'; jest.mock('node:child_process'); jest.mock('node:fs'); jest.mock('./daemon-client'); +jest.mock('./data-dir'); jest.mock('./paths'); const mockSpawn = jest.mocked(spawn); const mockExistsSync = jest.mocked(existsSync); const mockOpenSync = jest.mocked(openSync); const mockCloseSync = jest.mocked(closeSync); +const mockEnsureOwnerOnlyDirectory = jest.mocked(ensureOwnerOnlyDirectory); const mockPingDaemon = jest.mocked(pingDaemon); const mockGetDaemonPaths = jest.mocked(getDaemonPaths); @@ -86,6 +89,7 @@ describe('ensureDaemon', () => { dbPath: '/tmp/wallet.db', }); mockOpenSync.mockReturnValue(LOG_FD); + mockEnsureOwnerOnlyDirectory.mockResolvedValue(undefined); setupSpawnMock(); }); @@ -163,6 +167,29 @@ describe('ensureDaemon', () => { expect(mockCloseSync).toHaveBeenCalledWith(LOG_FD); }); + it('creates the data directory before opening the log file', async () => { + mockPingDaemon + .mockResolvedValueOnce(ABSENT) + .mockResolvedValueOnce(RESPONSIVE); + mockExistsSync.mockReturnValue(true); + // The log lives inside the data directory; opening it before the directory + // exists would throw ENOENT on a fresh `MM_DATA_DIR`, so the directory must + // be created first. + const order: string[] = []; + mockEnsureOwnerOnlyDirectory.mockImplementation(async () => { + order.push('ensureDir'); + }); + mockOpenSync.mockImplementation(() => { + order.push('openLog'); + return LOG_FD; + }); + + await ensureDaemon(CONFIG); + + expect(mockEnsureOwnerOnlyDirectory).toHaveBeenCalledWith('/tmp/data'); + expect(order).toStrictEqual(['ensureDir', 'openLog']); + }); + it('returns started when the spawned daemon becomes responsive', async () => { mockPingDaemon .mockResolvedValueOnce(ABSENT) diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.ts b/packages/wallet-cli/src/daemon/daemon-spawn.ts index e401ceaa96f..157ddebb4e9 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.ts @@ -3,6 +3,7 @@ import { closeSync, existsSync, openSync } from 'node:fs'; import { join } from 'node:path'; import { pingDaemon } from './daemon-client'; +import { ensureOwnerOnlyDirectory } from './data-dir'; import { getDaemonPaths } from './paths'; import type { DaemonSpawnConfig } from './types'; @@ -63,6 +64,12 @@ export async function ensureDaemon( const { entryPath, args } = resolveEntryPoint(config.packageRoot); + // Create (and lock down) the data directory here, before opening the log + // file below. The daemon entry also does this, but that runs only once the + // child is spawned: opening the log first would fail with ENOENT on a fresh + // data directory that does not exist yet. + await ensureOwnerOnlyDirectory(config.dataDir); + // Redirect the daemon's stderr into its log file rather than discarding it. // The daemon is detached, so anything it writes to stderr — the top-level // `Daemon fatal: ...` line, an uncaught stack trace, or a native From 0e84fe69a763bdb63afd79ba9bb1ef8519b9cb31 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 1 Jul 2026 11:20:18 +0200 Subject: [PATCH 08/12] refactor(wallet-cli): address daemon review feedback - Rename `fd` to `file descriptor` throughout the daemon spawn code and tests - Narrow `stopDaemon` stale-cleanup to `absent`/`refused` sockets only: a `permission`/`timeout`/`protocol` socket may be a wedged or foreign-user daemon, so it is no longer deleted or reported as a successful stop - Add a compile-time exhaustiveness guard to `ensureDaemon`'s ping handling so spawning is reachable only for a positive `absent` result - Replace the two mutable `{ value: T | null }` boxes with a single `StartupOutcome` discriminated union - Simplify/de-duplicate daemon comments; use `as unknown as ChildProcess` in the spawn mocks; add tests for the new branches Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/daemon/daemon-spawn.test.ts | 86 +++++++++++++---- .../wallet-cli/src/daemon/daemon-spawn.ts | 96 ++++++++++--------- .../wallet-cli/src/daemon/socket-line.test.ts | 19 +++- packages/wallet-cli/src/daemon/socket-line.ts | 13 +-- .../wallet-cli/src/daemon/stop-daemon.test.ts | 24 ++++- packages/wallet-cli/src/daemon/stop-daemon.ts | 14 +-- .../tests/lifecycle.daemon-e2e.test.ts | 6 +- 7 files changed, 172 insertions(+), 86 deletions(-) diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts index 93e6c916456..7d884b7f2ee 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.test.ts @@ -1,4 +1,5 @@ import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; import { closeSync, existsSync, openSync } from 'node:fs'; import { pingDaemon } from './daemon-client'; @@ -21,9 +22,9 @@ const mockEnsureOwnerOnlyDirectory = jest.mocked(ensureOwnerOnlyDirectory); const mockPingDaemon = jest.mocked(pingDaemon); const mockGetDaemonPaths = jest.mocked(getDaemonPaths); -// Arbitrary fd handed back by the mocked `openSync` so tests can assert it is -// wired into the child's stdio and later closed in the parent. -const LOG_FD = 7; +// Arbitrary file descriptor handed back by the mocked `openSync` so tests can +// assert it is wired into the child's stdio and later closed in the parent. +const LOG_FILE_DESCRIPTOR = 7; const CONFIG: DaemonSpawnConfig = { dataDir: '/tmp/data', @@ -74,7 +75,7 @@ function setupSpawnMock(): SpawnMock { listeners.get('exit')?.(code, signal); }, }; - mockSpawn.mockReturnValue(result as never); + mockSpawn.mockReturnValue(result as unknown as ChildProcess); return result; } @@ -88,7 +89,7 @@ describe('ensureDaemon', () => { logPath: '/tmp/test.log', dbPath: '/tmp/wallet.db', }); - mockOpenSync.mockReturnValue(LOG_FD); + mockOpenSync.mockReturnValue(LOG_FILE_DESCRIPTOR); mockEnsureOwnerOnlyDirectory.mockResolvedValue(undefined); setupSpawnMock(); }); @@ -139,7 +140,7 @@ describe('ensureDaemon', () => { ['/pkg/dist/daemon/daemon-entry.mjs'], expect.objectContaining({ detached: true, - stdio: ['ignore', 'ignore', LOG_FD], + stdio: ['ignore', 'ignore', LOG_FILE_DESCRIPTOR], env: expect.objectContaining({ MM_DAEMON_DATA_DIR: '/tmp/data', MM_DAEMON_SOCKET_PATH: '/tmp/test.sock', @@ -152,7 +153,7 @@ describe('ensureDaemon', () => { ); }); - it('redirects the daemon stderr to its log file and closes the parent fd', async () => { + it('redirects the daemon stderr to its log file and closes the parent file descriptor', async () => { mockPingDaemon .mockResolvedValueOnce(ABSENT) .mockResolvedValueOnce(RESPONSIVE); @@ -162,9 +163,23 @@ describe('ensureDaemon', () => { expect(mockOpenSync).toHaveBeenCalledWith('/tmp/test.log', 'a'); const spawnOptions = mockSpawn.mock.calls[0][2] as { stdio: unknown }; - expect(spawnOptions.stdio).toStrictEqual(['ignore', 'ignore', LOG_FD]); - // The child dups the fd, so the parent must close its own copy. - expect(mockCloseSync).toHaveBeenCalledWith(LOG_FD); + expect(spawnOptions.stdio).toStrictEqual([ + 'ignore', + 'ignore', + LOG_FILE_DESCRIPTOR, + ]); + expect(mockCloseSync).toHaveBeenCalledWith(LOG_FILE_DESCRIPTOR); + }); + + it('propagates a log-file open failure without spawning', async () => { + mockPingDaemon.mockResolvedValue(ABSENT); + mockExistsSync.mockReturnValue(true); + mockOpenSync.mockImplementation(() => { + throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); + }); + + await expect(ensureDaemon(CONFIG)).rejects.toThrow('EACCES'); + expect(mockSpawn).not.toHaveBeenCalled(); }); it('creates the data directory before opening the log file', async () => { @@ -172,16 +187,15 @@ describe('ensureDaemon', () => { .mockResolvedValueOnce(ABSENT) .mockResolvedValueOnce(RESPONSIVE); mockExistsSync.mockReturnValue(true); - // The log lives inside the data directory; opening it before the directory - // exists would throw ENOENT on a fresh `MM_DATA_DIR`, so the directory must - // be created first. + // The log lives inside the data dir, so the dir must be created first (else + // openSync ENOENTs on a fresh dir). const order: string[] = []; mockEnsureOwnerOnlyDirectory.mockImplementation(async () => { order.push('ensureDir'); }); mockOpenSync.mockImplementation(() => { order.push('openLog'); - return LOG_FD; + return LOG_FILE_DESCRIPTOR; }); await ensureDaemon(CONFIG); @@ -275,7 +289,10 @@ describe('ensureDaemon', () => { } }, ); - mockSpawn.mockReturnValue({ unref: jest.fn(), on } as never); + mockSpawn.mockReturnValue({ + unref: jest.fn(), + on, + } as unknown as ChildProcess); jest.useFakeTimers(); const promise = ensureDaemon(CONFIG); @@ -316,7 +333,10 @@ describe('ensureDaemon', () => { } }, ); - mockSpawn.mockReturnValue({ unref: jest.fn(), on } as never); + mockSpawn.mockReturnValue({ + unref: jest.fn(), + on, + } as unknown as ChildProcess); jest.useFakeTimers(); const promise = ensureDaemon(CONFIG); @@ -332,6 +352,35 @@ describe('ensureDaemon', () => { expect((thrownError as Error).message).toContain('/tmp/test.log'); }); + it('reports the spawn error when the child both errors and exits', async () => { + mockPingDaemon.mockResolvedValue(ABSENT); + mockExistsSync.mockReturnValue(true); + const on = jest.fn( + (event: string, handler: (...args: unknown[]) => void) => { + if (event === 'error') { + handler(new Error('spawn ENOENT')); + } + if (event === 'exit') { + handler(1, null); + } + }, + ); + mockSpawn.mockReturnValue({ + unref: jest.fn(), + on, + } as unknown as ChildProcess); + + jest.useFakeTimers(); + const promise = ensureDaemon(CONFIG); + const rejection = promise.catch((thrown: unknown) => thrown); + await jest.advanceTimersByTimeAsync(200); + + const thrownError = await rejection; + expect((thrownError as Error).message).toContain( + 'Failed to spawn daemon process', + ); + }); + it('writes spawn errors to stderr', async () => { mockPingDaemon .mockResolvedValueOnce(ABSENT) @@ -346,7 +395,10 @@ describe('ensureDaemon', () => { } }, ); - mockSpawn.mockReturnValue({ unref: jest.fn(), on } as never); + mockSpawn.mockReturnValue({ + unref: jest.fn(), + on, + } as unknown as ChildProcess); await ensureDaemon(CONFIG); errorHandler?.(new Error('spawn ENOENT')); diff --git a/packages/wallet-cli/src/daemon/daemon-spawn.ts b/packages/wallet-cli/src/daemon/daemon-spawn.ts index 157ddebb4e9..5f740943f17 100644 --- a/packages/wallet-cli/src/daemon/daemon-spawn.ts +++ b/packages/wallet-cli/src/daemon/daemon-spawn.ts @@ -42,46 +42,49 @@ export async function ensureDaemon( const { socketPath, logPath } = getDaemonPaths(config.dataDir); const initialPing = await pingDaemon(socketPath); - if (initialPing.status === 'responsive') { - return { state: 'already-running', socketPath }; - } - if (initialPing.status === 'unreachable') { - if (initialPing.reason === 'permission') { + switch (initialPing.status) { + case 'responsive': + return { state: 'already-running', socketPath }; + case 'unreachable': + if (initialPing.reason === 'permission') { + throw new Error( + `Refusing to start: the socket at ${socketPath} is owned by another user. ` + + `Choose a different data directory (MM_DAEMON_DATA_DIR) or remove the socket manually. ` + + `(${initialPing.error.message})`, + ); + } throw new Error( - `Refusing to start: the socket at ${socketPath} is owned by another user. ` + - `Choose a different data directory (MM_DAEMON_DATA_DIR) or remove the socket manually. ` + + `Refusing to start: a daemon socket already exists at ${socketPath} but is unresponsive. ` + + `Run \`mm daemon stop\` (or \`mm daemon purge\`) before starting a new daemon. ` + `(${initialPing.error.message})`, ); + case 'absent': + break; + /* istanbul ignore next -- exhaustiveness guard; unreachable for the current PingResult union */ + default: { + const exhaustiveCheck: never = initialPing; + throw new Error( + `Unexpected daemon ping status: ${String(exhaustiveCheck)}`, + ); } - throw new Error( - `Refusing to start: a daemon socket already exists at ${socketPath} but is unresponsive. ` + - `Run \`mm daemon stop\` (or \`mm daemon purge\`) before starting a new daemon. ` + - `(${initialPing.error.message})`, - ); } process.stderr.write('Starting daemon...\n'); const { entryPath, args } = resolveEntryPoint(config.packageRoot); - // Create (and lock down) the data directory here, before opening the log - // file below. The daemon entry also does this, but that runs only once the - // child is spawned: opening the log first would fail with ENOENT on a fresh - // data directory that does not exist yet. + // Create the data directory before opening the log file inside it. The daemon + // entry also does this, but only after spawn — opening the log first would + // ENOENT on a fresh data directory. await ensureOwnerOnlyDirectory(config.dataDir); - // Redirect the daemon's stderr into its log file rather than discarding it. - // The daemon is detached, so anything it writes to stderr — the top-level - // `Daemon fatal: ...` line, an uncaught stack trace, or a native - // `better-sqlite3` abort — would otherwise vanish, leaving a daemon that dies - // after startup completely undiagnosable (e.g. a `daemon stop` that then - // finds a stale socket and a dead PID). `stdout` stays ignored: structured - // status already goes through the file logger. The child dups the fd on - // spawn, so the parent closes its own copy immediately. - const logFd = openSync(logPath, 'a'); + // Redirect the detached daemon's stderr to its log file rather than + // discarding it, so a crash after startup stays diagnosable. stdout stays + // ignored — structured status goes through the file logger. + const logFileDescriptor = openSync(logPath, 'a'); const child = spawn(process.execPath, [...args, entryPath], { detached: true, - stdio: ['ignore', 'ignore', logFd], + stdio: ['ignore', 'ignore', logFileDescriptor], env: { ...process.env, MM_DAEMON_DATA_DIR: config.dataDir, @@ -91,40 +94,45 @@ export async function ensureDaemon( MM_WALLET_SRP: config.srp, }, }); - // The child has dup'd the fd into its own stderr; the parent no longer needs - // its copy. `spawn` reports runtime failures via the 'error' event rather - // than throwing synchronously, so closing here is safe on the success path. - closeSync(logFd); + // The child dup'd the file descriptor into its stderr, so drop the parent's + // copy. Safe on the success path: `spawn` reports failures via the 'error' + // event, not a synchronous throw. + closeSync(logFileDescriptor); + + type StartupOutcome = + | { kind: 'pending' } + | { kind: 'error'; error: Error } + | { kind: 'exited'; code: number | null; signal: NodeJS.Signals | null }; - type ExitInfo = { code: number | null; signal: NodeJS.Signals | null }; - const exitInfo: { value: ExitInfo | null } = { value: null }; // A failed spawn (bad interpreter, EACCES, ENOENT) emits 'error' and may - // never emit 'exit'. Capture it so the readiness loop can surface the real - // cause immediately instead of hanging for the full timeout. - const spawnError: { value: Error | null } = { value: null }; + // never emit 'exit', so 'error' is recorded first and not overwritten by a + // later 'exit' — the loop surfaces the real cause instead of hanging. + const outcome: { current: StartupOutcome } = { current: { kind: 'pending' } }; child.on('error', (error: Error) => { process.stderr.write(`Failed to spawn daemon process: ${String(error)}\n`); - spawnError.value = error; + outcome.current = { kind: 'error', error }; }); child.on('exit', (code, signal) => { - exitInfo.value = { code, signal }; + if (outcome.current.kind === 'pending') { + outcome.current = { kind: 'exited', code, signal }; + } }); child.unref(); for (let i = 0; i < MAX_POLLS; i++) { await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS)); - if (spawnError.value !== null) { + const settled = outcome.current; + if (settled.kind === 'error') { throw new Error( - `Failed to spawn daemon process: ${spawnError.value.message}. ` + - `Check the daemon log at ${getDaemonPaths(config.dataDir).logPath}.`, + `Failed to spawn daemon process: ${settled.error.message}. ` + + `Check the daemon log at ${logPath}.`, ); } - if (exitInfo.value !== null) { - const { code, signal } = exitInfo.value; + if (settled.kind === 'exited') { throw new Error( - `Daemon process exited during startup (code=${String(code)}, signal=${String(signal)}). ` + - `Check the daemon log at ${getDaemonPaths(config.dataDir).logPath}.`, + `Daemon process exited during startup (code=${String(settled.code)}, signal=${String(settled.signal)}). ` + + `Check the daemon log at ${logPath}.`, ); } const ping = await pingDaemon(socketPath); diff --git a/packages/wallet-cli/src/daemon/socket-line.test.ts b/packages/wallet-cli/src/daemon/socket-line.test.ts index 400d2b80617..acb02e4a234 100644 --- a/packages/wallet-cli/src/daemon/socket-line.test.ts +++ b/packages/wallet-cli/src/daemon/socket-line.test.ts @@ -47,9 +47,8 @@ describe('writeLine', () => { await expect(writeLine(socket, 'hello')).rejects.toThrow('EPIPE'); - // A failed write surfaces both via the callback (above) AND as a separate - // 'error' event. A listener must remain so that emission is handled; - // otherwise Node throws "Unhandled 'error' event" and crashes the process. + // A failed write also emits a trailing 'error' event; a listener must + // survive to handle it, or Node crashes with "Unhandled 'error' event". expect(socket.listenerCount('error')).toBe(1); expect(() => socket.emit('error', writeError)).not.toThrow(); // Once it fires, the listener detaches itself. @@ -124,6 +123,20 @@ describe('readLine', () => { ); }); + it('settles once and cleans up when end is followed by close', async () => { + const socket = createMockSocket(); + const promise = readLine(socket); + + socket.emit('end'); + socket.emit('close'); + + await expect(promise).rejects.toThrow( + 'Socket closed before response received', + ); + expect(socket.listenerCount('end')).toBe(0); + expect(socket.listenerCount('close')).toBe(0); + }); + it('rejects after timeout when no complete line received', async () => { jest.useFakeTimers(); const socket = createMockSocket(); diff --git a/packages/wallet-cli/src/daemon/socket-line.ts b/packages/wallet-cli/src/daemon/socket-line.ts index dc96580cbd5..e4c00505890 100644 --- a/packages/wallet-cli/src/daemon/socket-line.ts +++ b/packages/wallet-cli/src/daemon/socket-line.ts @@ -19,14 +19,11 @@ export async function writeLine(socket: Socket, line: string): Promise { socket.write(`${line}\n`, (error) => { if (error) { - // A failed write (e.g. EPIPE when the peer closed mid-write) is - // delivered BOTH here AND as a separate 'error' event on the socket. - // Leave `onError` attached so that event still has a handler — - // detaching it here would let Node treat the emission as unhandled and - // crash the whole process, even though this rejection is caught - // upstream. `onError` rejects idempotently and detaches itself when the - // event arrives; rejecting here settles the promise even if it never - // does. + // A failed write (e.g. EPIPE) is delivered BOTH here AND as a later + // 'error' event. Leave `onError` attached to handle that event — + // detaching here would let Node treat it as unhandled and crash the + // process. This reject settles the promise; `onError` detaches itself + // when/if the event arrives. reject(error); return; } diff --git a/packages/wallet-cli/src/daemon/stop-daemon.test.ts b/packages/wallet-cli/src/daemon/stop-daemon.test.ts index bf7a956ca72..daaa96cd5cf 100644 --- a/packages/wallet-cli/src/daemon/stop-daemon.test.ts +++ b/packages/wallet-cli/src/daemon/stop-daemon.test.ts @@ -50,15 +50,15 @@ describe('stopDaemon', () => { expect(mockSendSignal).not.toHaveBeenCalled(); }); - it('cleans up a stale socket and PID file when the socket is unreachable but the process is dead', async () => { + it('cleans up a stale socket and PID file when connections are refused and the process is dead', async () => { mockReadPidFile.mockResolvedValue(123); mockPingDaemon.mockResolvedValue(UNREACHABLE); mockIsProcessAlive.mockReturnValue(false); const result = await stopDaemon('/tmp/test.sock', '/tmp/test.pid'); - // A daemon that crashed leaves a connectable-but-dead socket and its PID - // file behind; the daemon is gone, so report success and clear both. + // A crashed daemon leaves a refused socket and PID file behind; the daemon + // is gone, so report success and clear both. expect(result).toBe(true); expect(mockRm).toHaveBeenCalledWith('/tmp/test.pid', { force: true }); expect(mockRm).toHaveBeenCalledWith('/tmp/test.sock', { force: true }); @@ -67,6 +67,22 @@ describe('stopDaemon', () => { expect(mockSendCommand).not.toHaveBeenCalled(); }); + it('does not delete the socket or report success when the socket is unreachable for a non-refused reason and the process is dead', async () => { + mockReadPidFile.mockResolvedValue(123); + mockPingDaemon.mockResolvedValue({ + status: 'unreachable', + reason: 'permission', + error: new Error('EACCES'), + }); + mockIsProcessAlive.mockReturnValue(false); + + const result = await stopDaemon('/tmp/test.sock', '/tmp/test.pid'); + + expect(result).toBe(false); + expect(mockRm).not.toHaveBeenCalled(); + expect(mockSendSignal).not.toHaveBeenCalled(); + }); + it('signals the recorded PID when the socket is absent but the process is still alive', async () => { mockReadPidFile.mockResolvedValue(123); mockPingDaemon.mockResolvedValue(ABSENT); @@ -133,6 +149,7 @@ describe('stopDaemon', () => { expect(result).toBe(true); expect(mockSendSignal).toHaveBeenCalledWith(123, 'SIGTERM'); + expect(mockRm).toHaveBeenCalledWith('/tmp/test.sock', { force: true }); }); it('falls through to SIGTERM when graceful shutdown times out', async () => { @@ -187,6 +204,7 @@ describe('stopDaemon', () => { const result = await stopDaemon('/tmp/test.sock', '/tmp/test.pid'); expect(result).toBe(true); expect(mockSendSignal).toHaveBeenCalledWith(123, 'SIGKILL'); + expect(mockRm).toHaveBeenCalledWith('/tmp/test.sock', { force: true }); }); it('returns false when all strategies fail', async () => { diff --git a/packages/wallet-cli/src/daemon/stop-daemon.ts b/packages/wallet-cli/src/daemon/stop-daemon.ts index 5f0036cd029..3ffcdde8392 100644 --- a/packages/wallet-cli/src/daemon/stop-daemon.ts +++ b/packages/wallet-cli/src/daemon/stop-daemon.ts @@ -4,8 +4,7 @@ import { pingDaemon, sendCommand } from './daemon-client'; import { isProcessAlive, readPidFile, sendSignal, waitFor } from './utils'; /** - * Stop the daemon via a `shutdown` RPC call. Falls back to PID + SIGTERM if - * the socket is unresponsive, and escalates to SIGKILL if SIGTERM is ignored. + * Stop the daemon, preferring a graceful shutdown. * * Resolution order when a live daemon is present: * 1. If the socket is responsive, request a graceful `shutdown` over it. @@ -41,10 +40,13 @@ export async function stopDaemon( ping.status === 'responsive' || ping.status === 'unreachable'; const processAlive = pid !== undefined && isProcessAlive(pid); - if (ping.status !== 'responsive' && !processAlive) { - // No live daemon: the socket is not answering and the recorded PID (if - // any) is dead. Remove any stale socket/PID files left behind by a daemon - // that already exited and report success. + // Only `absent` and `refused` prove no live daemon; `permission`/`timeout`/ + // `protocol` may be a wedged or foreign daemon, so those fall through. + const socketProvenGone = + ping.status === 'absent' || + (ping.status === 'unreachable' && ping.reason === 'refused'); + + if (socketProvenGone && !processAlive) { await cleanupFile(pidPath, 'PID file', log); await cleanupFile(socketPath, 'socket file', log); return true; diff --git a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts index f17aec07e9d..398afe574b7 100644 --- a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts +++ b/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts @@ -77,11 +77,7 @@ async function runMm(args: string[], dataDir: string): Promise { * Assert that a CLI invocation exited 0. * * On failure this throws an error embedding the captured stdout/stderr AND the - * daemon's own log file. The harness otherwise discards all three when only the - * exit code is asserted, which leaves a CI-only failure (one we cannot - * reproduce locally) impossible to diagnose from the run output. The daemon log - * is the only window into the spawned process: it records `Shutting down - * (...)`, `handle.close() failed`, `Daemon fatal: ...`, etc. + * daemon's own log file. * * @param step - Human-readable label for the CLI step (e.g. `daemon stop`). * @param result - The captured run result. From 02d713bca495ecccb2b3efbb9be9de33bf69083a Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 1 Jul 2026 12:00:07 +0200 Subject: [PATCH 09/12] test(wallet-cli): key the subprocess e2e suite off the tests/ folder Generalize the e2e jest config to discriminate by folder instead of the feature-specific `daemon-e2e` filename suffix, so future subprocess e2e tests just drop into `tests/` with no config change. - jest.config.e2e.js: match `roots: ['/tests']` instead of `**/*.daemon-e2e.test.ts` - jest.config.js: ignore `/tests/`; drop the redundant daemon-e2e coverage exclusion (coverage is collected from `./src/**` only) - Rename lifecycle.daemon-e2e.test.ts -> lifecycle.e2e.test.ts - README: describe the suite by folder The in-process `wallet-factory.e2e.test.ts` stays in `src/` (unit suite): it needs the Web-Crypto polyfill env and is coverage-visible, unlike the subprocess suite. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/README.md | 2 +- packages/wallet-cli/jest.config.e2e.js | 11 +++++----- packages/wallet-cli/jest.config.js | 21 +++++++------------ ...emon-e2e.test.ts => lifecycle.e2e.test.ts} | 0 4 files changed, 14 insertions(+), 20 deletions(-) rename packages/wallet-cli/tests/{lifecycle.daemon-e2e.test.ts => lifecycle.e2e.test.ts} (100%) diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index 6be52efeae1..ef6c44b847e 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -59,7 +59,7 @@ cd node_modules/better-sqlite3 && node ../.bin/prebuild-install Unit tests run with `yarn workspace @metamask/wallet-cli test`. -The subprocess end-to-end suite (`*.daemon-e2e.test.ts`) spawns the built `mm` CLI and the native `better-sqlite3` addon as real processes, so it is kept out of the unit run and its coverage gate. Build the workspace dependencies first (`yarn build` from the repo root), then run it with `yarn workspace @metamask/wallet-cli test:e2e`. +The subprocess end-to-end suite (in `tests/`) spawns the built `mm` CLI and the native `better-sqlite3` addon as real processes, so it is kept out of the unit run and its coverage gate. Build the workspace dependencies first (`yarn build` from the repo root), then run it with `yarn workspace @metamask/wallet-cli test:e2e`. ## Contributing diff --git a/packages/wallet-cli/jest.config.e2e.js b/packages/wallet-cli/jest.config.e2e.js index b6764581282..aff6a9372c3 100644 --- a/packages/wallet-cli/jest.config.e2e.js +++ b/packages/wallet-cli/jest.config.e2e.js @@ -1,8 +1,8 @@ /* - * Jest configuration for the subprocess daemon e2e suite (`*.daemon-e2e.test.ts`). + * Jest configuration for the subprocess e2e suite in `tests/`. * - * Kept separate from `jest.config.js` because this suite spawns the BUILT `mm` - * CLI and the native `better-sqlite3` addon as real child processes: it must + * Kept separate from `jest.config.js` because these suites spawn the BUILT `mm` + * CLI and the native `better-sqlite3` addon as real child processes: they must * stay out of the fast unit `test` run and must not be held to that run's * 100%-coverage gate (subprocess work is invisible to in-process coverage). * Run it with `yarn test:e2e`. @@ -15,8 +15,9 @@ const baseConfig = require('../../jest.config.packages'); module.exports = merge(baseConfig, { displayName: 'wallet-cli:e2e', - // Only the subprocess e2e suite; the default config runs everything else. - testMatch: ['**/*.daemon-e2e.test.ts'], + // Every test under `tests/` is a subprocess e2e; the default config runs + // everything in `src/`. + roots: ['/tests'], // Coverage is meaningless here — the work happens in spawned processes — so // collecting it would only report the e2e harness as uncovered source. diff --git a/packages/wallet-cli/jest.config.js b/packages/wallet-cli/jest.config.js index d4509b236d7..568fedaec47 100644 --- a/packages/wallet-cli/jest.config.js +++ b/packages/wallet-cli/jest.config.js @@ -21,20 +21,13 @@ module.exports = merge(baseConfig, { // The test harness in `src/test/` is exercised by the command tests but // not all of its error/edge branches are worth driving directly — it's - // production code's test infrastructure, not production code itself. The - // daemon e2e is skipped from this run (below), so it never executes here and - // must also be excluded from coverage, or it would report as 0%-covered - // source. - coveragePathIgnorePatterns: [ - '.*/src/test/.*', - '.*\\.daemon-e2e\\.test\\.ts$', - ], - - // The subprocess daemon e2e (`*.daemon-e2e.test.ts`) spawns the built CLI and - // the native sqlite addon; it has its own config (`jest.config.e2e.js`, run - // via `yarn test:e2e`) and must not run in — or be coverage-gated by — the - // fast unit suite. - testPathIgnorePatterns: ['/node_modules/', '\\.daemon-e2e\\.test\\.ts$'], + // production code's test infrastructure, not production code itself. + coveragePathIgnorePatterns: ['.*/src/test/.*'], + + // The subprocess e2e suite lives in `tests/` and has its own config + // (`jest.config.e2e.js`, run via `yarn test:e2e`); it spawns the built CLI + // and must not run in the fast unit suite. + testPathIgnorePatterns: ['/node_modules/', '/tests/'], // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { diff --git a/packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts b/packages/wallet-cli/tests/lifecycle.e2e.test.ts similarity index 100% rename from packages/wallet-cli/tests/lifecycle.daemon-e2e.test.ts rename to packages/wallet-cli/tests/lifecycle.e2e.test.ts From ee51c0b2d0fc848968e6d9a9d60d882a747f8f2d Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 1 Jul 2026 12:04:11 +0200 Subject: [PATCH 10/12] test(wallet-cli): rename wallet-factory e2e to integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wallet-factory.e2e.test.ts` constructs a real `Wallet` in-process (no CLI, no subprocess, no socket boundary), so it is an integration test — the same category as `socket-integration.test.ts`, not an end-to-end test. Rename it to match that convention; the only true e2e is `lifecycle.e2e.test.ts`, which spawns the built CLI. - Rename wallet-factory.e2e.test.ts -> wallet-factory-integration.test.ts - Update the reference in lifecycle.e2e.test.ts's header Co-Authored-By: Claude Opus 4.8 (1M context) --- ...t-factory.e2e.test.ts => wallet-factory-integration.test.ts} | 0 packages/wallet-cli/tests/lifecycle.e2e.test.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/wallet-cli/src/daemon/{wallet-factory.e2e.test.ts => wallet-factory-integration.test.ts} (100%) diff --git a/packages/wallet-cli/src/daemon/wallet-factory.e2e.test.ts b/packages/wallet-cli/src/daemon/wallet-factory-integration.test.ts similarity index 100% rename from packages/wallet-cli/src/daemon/wallet-factory.e2e.test.ts rename to packages/wallet-cli/src/daemon/wallet-factory-integration.test.ts diff --git a/packages/wallet-cli/tests/lifecycle.e2e.test.ts b/packages/wallet-cli/tests/lifecycle.e2e.test.ts index 398afe574b7..9db668dfc7c 100644 --- a/packages/wallet-cli/tests/lifecycle.e2e.test.ts +++ b/packages/wallet-cli/tests/lifecycle.e2e.test.ts @@ -8,7 +8,7 @@ import { isProcessAlive, readPidFile } from '../src/daemon/utils'; // Subprocess-level lifecycle test for the `mm daemon` command suite. Unlike the // in-process suites (`socket-integration.test.ts` exercises the transport in -// the test realm; `wallet-factory.e2e.test.ts` constructs a real `Wallet` +// the test realm; `wallet-factory-integration.test.ts` constructs a real `Wallet` // in-process), this spawns the BUILT `mm` CLI as a child process against a temp // data directory and drives the real `start → call → status/stop/purge` // lifecycle over the Unix socket. It needs `dist/` and the native From 02285d486e4d9344d728fe3e6e83d7ffb4adfac9 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 1 Jul 2026 15:41:57 +0200 Subject: [PATCH 11/12] ci(wallet-cli): align e2e job with test matrix conventions Use action-checkout-and-setup@v3 with persist-credentials: false and add 22.x to the daemon e2e node-version matrix, matching the rest of the test jobs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/lint-build-test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index b7ed1da6da2..30f4ebc0fec 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -232,12 +232,13 @@ jobs: needs: prepare strategy: matrix: - node-version: [20.x, 24.x] + node-version: [20.x, 22.x, 24.x] steps: - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@v2 + uses: MetaMask/action-checkout-and-setup@v3 with: is-high-risk-environment: false + persist-credentials: false node-version: ${{ matrix.node-version }} - name: Build wallet-cli and its dependencies run: yarn workspaces foreach --topological-dev --recursive --from '@metamask/wallet-cli' run build From 56d3cf9e86438c933ec2ccceb08c20bd6231139b Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 1 Jul 2026 16:02:46 +0200 Subject: [PATCH 12/12] chore: remove build step from test:e2e command --- packages/wallet-cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wallet-cli/package.json b/packages/wallet-cli/package.json index 696de1b92e6..a1123ccb00d 100644 --- a/packages/wallet-cli/package.json +++ b/packages/wallet-cli/package.json @@ -39,7 +39,7 @@ "test:prepare": "./scripts/install-binaries.sh", "test": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", - "test:e2e": "yarn build && yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.e2e.js", + "test:e2e": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.e2e.js", "test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose", "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" },