diff --git a/README.md b/README.md index cc37520..637f08b 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ The design is **consumer-agnostic**: the core handles protocol, tooling, and kno ## Installation -As a project dependency (the CLI is available via `npx mm` or `yarn mm`): +As a project dependency, run the CLI through your package manager. It does not put a bare `mm` command on your shell `PATH`: ```bash yarn add @metamask/client-mcp-core @@ -87,6 +87,16 @@ npm install -g @metamask/client-mcp-core The global CLI can target any project via `--project` or `MM_PROJECT` (see [Project Targeting](#project-targeting)). +Use the invocation that matches your installation: + +```bash +# Project dependency +yarn mm launch + +# Global installation +mm launch +``` + ## Getting Started Consuming this package requires two things: a **daemon entry point** and a **configuration file**. @@ -133,7 +143,7 @@ export default { }; ``` -The `daemon` field tells the CLI where the daemon entry point lives. The `runtime` field specifies the TypeScript runner (defaults to `tsx`). +The `daemon` field tells the CLI where the daemon entry point lives. The `runtime` field supports `tsx` (the default) and `node`. For `tsx`, the CLI resolves the project's installed `tsx/cli` module and launches it through Node. The CLI uses [cosmiconfig](https://github.com/cosmiconfig/cosmiconfig) for config discovery, so you can also use `mm-client-cli.config.js`, `.mm-client-clirc.json`, or other supported formats. @@ -146,6 +156,8 @@ mm click e3 # interact using a11y refs mm cleanup --shutdown # stop browser and daemon ``` +Use `yarn mm` in place of `mm` when the CLI is installed as a project dependency. + If running from outside the project directory (e.g., a parent folder containing multiple repos): ```bash diff --git a/src/cli/mm.test.ts b/src/cli/mm.test.ts index c1da26a..e661c51 100644 --- a/src/cli/mm.test.ts +++ b/src/cli/mm.test.ts @@ -3,8 +3,8 @@ /* eslint-disable n/no-sync */ /* eslint-disable require-atomic-updates */ import { cosmiconfig } from 'cosmiconfig'; -import { existsSync } from 'node:fs'; import * as fs from 'node:fs/promises'; +import { createRequire } from 'node:module'; import * as path from 'node:path'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { MockInstance } from 'vitest'; @@ -52,11 +52,6 @@ vi.mock('node:child_process', () => ({ }), })); -vi.mock('node:fs', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, existsSync: vi.fn(() => true) }; -}); - vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal(); return { @@ -67,6 +62,10 @@ vi.mock('node:fs/promises', async (importOriginal) => { }; }); +vi.mock('node:module', () => ({ + createRequire: vi.fn(), +})); + vi.mock('../server/daemon-state.js', () => ({ readDaemonState: vi.fn(async () => null), isDaemonAlive: vi.fn(async () => false), @@ -87,6 +86,8 @@ vi.mock('cosmiconfig', () => ({ let exitSpy: MockInstance; let stderrSpy: MockInstance; let stdoutSpy: MockInstance; +const mockCreateRequire = vi.mocked(createRequire); +const mockResolveRuntime = vi.fn(); // eslint-disable-next-line vitest/require-top-level-describe beforeEach(() => { @@ -101,6 +102,10 @@ beforeEach(() => { }) as never); stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); stdoutSpy = vi.spyOn(process.stdout, 'write').mockReturnValue(true); + mockResolveRuntime.mockReturnValue('/mock/worktree/node_modules/tsx/cli.mjs'); + mockCreateRequire.mockReturnValue({ + resolve: mockResolveRuntime, + } as never); }); // eslint-disable-next-line vitest/require-top-level-describe @@ -675,20 +680,61 @@ describe('printHelp', () => { describe('resolveRuntime', () => { it('returns node for node runtime', () => { - expect(resolveRuntime('/root', 'node')).toBe('node'); + const result = resolveRuntime('/root', 'node'); + expect(result).toStrictEqual({ + command: 'node', + getArgs: expect.any(Function), + }); + expect(result.getArgs('./daemon.ts')).toStrictEqual(['./daemon.ts']); }); - it('returns bin path when runtime exists', () => { - vi.mocked(existsSync).mockReturnValue(true); + it('runs tsx through Node using the project-local CLI module', () => { const result = resolveRuntime('/root', 'tsx'); - expect(result).toBe(path.join('/root', 'node_modules', '.bin', 'tsx')); + expect(result).toStrictEqual({ + command: process.execPath, + getArgs: expect.any(Function), + }); + expect(mockCreateRequire).toHaveBeenCalledWith( + path.join('/root', 'package.json'), + ); + expect(mockResolveRuntime).toHaveBeenCalledWith('tsx/cli'); + expect(result.getArgs('./daemon.ts')).toStrictEqual([ + '/mock/worktree/node_modules/tsx/cli.mjs', + './daemon.ts', + ]); }); - it('exits when runtime binary not found', () => { - vi.mocked(existsSync).mockReturnValue(false); + it('runs tsx without a shell for Windows project paths containing spaces', () => { + mockResolveRuntime.mockReturnValue( + 'C:\\Users\\Jane Doe\\project\\node_modules\\tsx\\dist\\cli.mjs', + ); + + const runtime = resolveRuntime('C:\\Users\\Jane Doe\\project', 'tsx'); + + expect(runtime).toStrictEqual({ + command: process.execPath, + getArgs: expect.any(Function), + }); + expect(runtime.getArgs('test/e2e/daemon.ts')).toStrictEqual([ + 'C:\\Users\\Jane Doe\\project\\node_modules\\tsx\\dist\\cli.mjs', + 'test/e2e/daemon.ts', + ]); + }); + + it('exits when tsx is not installed in the project', () => { + mockResolveRuntime.mockImplementation(() => { + throw new Error('Cannot find module'); + }); + expect(() => resolveRuntime('/root', 'tsx')).toThrowError('process.exit'); expect(stderrSpy).toHaveBeenCalledWith( - expect.stringContaining("Runtime 'tsx' not found"), + expect.stringContaining("Runtime 'tsx' is not installed in /root"), + ); + }); + + it('throws for an invalid runtime value', () => { + expect(() => resolveRuntime('/root', 'invalid' as never)).toThrowError( + 'Unsupported runtime', ); }); }); @@ -803,6 +849,21 @@ describe('readDaemonConfig', () => { expect(result.runtime).toBe('tsx'); }); + it('exits when the configured runtime is unsupported', async () => { + mockSearch.mockResolvedValue({ + config: { daemon: './daemon.ts', runtime: 'swc-node' }, + filepath: '/mock/worktree/mm-client-cli.config.ts', + isEmpty: false, + }); + + await expect(readDaemonConfig('/mock/worktree')).rejects.toThrowError( + 'process.exit', + ); + expect(stderrSpy).toHaveBeenCalledWith( + "Error: Unsupported runtime 'swc-node'. Supported runtimes are 'node' and 'tsx'.\n", + ); + }); + it('exits when no config file is found', async () => { mockSearch.mockResolvedValueOnce(null); @@ -2614,12 +2675,14 @@ describe('waitForDaemon', () => { vi.useFakeTimers(); const promise = waitForDaemon('/root').catch((error: Error) => error); - for (let i = 0; i < 55; i++) { + for (let i = 0; i < 155; i++) { await vi.advanceTimersByTimeAsync(200); } const result = await promise; expect(result).toBeInstanceOf(Error); - expect((result as Error).message).toContain('Daemon failed to start'); + expect((result as Error).message).toBe( + 'Daemon failed to start within 30 seconds', + ); vi.useRealTimers(); }); }); @@ -2736,6 +2799,86 @@ describe('main', () => { process.argv = origArgv; }); + it('resolves a relative extension path from the worktree root', async () => { + const { readDaemonState, isDaemonAlive, isDaemonVersionMatch } = + await import('../server/daemon-state.js'); + const mockState = { + port: 3000, + pid: 123, + nonce: 'abc', + startedAt: '2024-01-01', + version: '1.0.0', + subPorts: { anvil: 8545, fixture: 8546, mock: 8547 }, + }; + vi.mocked(readDaemonState).mockResolvedValueOnce(mockState); + vi.mocked(isDaemonAlive).mockResolvedValueOnce(true); + vi.mocked(isDaemonVersionMatch).mockReturnValueOnce(true); + + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ ok: true, result: 'launched' }), + } as Response); + + const originalProject = process.env.MM_PROJECT; + process.env.MM_PROJECT = '/mock/worktree'; + const origArgv = process.argv; + process.argv = ['node', 'mm', 'launch', '--extension-path', 'dist/chrome']; + + await main(); + + expect(globalThis.fetch).toHaveBeenCalledWith( + 'http://127.0.0.1:3000/launch', + expect.objectContaining({ + body: JSON.stringify({ + extensionPath: path.resolve('/mock/worktree', 'dist/chrome'), + }), + }), + ); + + process.argv = origArgv; + if (originalProject === undefined) { + delete process.env.MM_PROJECT; + } else { + process.env.MM_PROJECT = originalProject; + } + }); + + it('preserves an absolute extension path', async () => { + const { readDaemonState, isDaemonAlive, isDaemonVersionMatch } = + await import('../server/daemon-state.js'); + const mockState = { + port: 3000, + pid: 123, + nonce: 'abc', + startedAt: '2024-01-01', + version: '1.0.0', + subPorts: { anvil: 8545, fixture: 8546, mock: 8547 }, + }; + vi.mocked(readDaemonState).mockResolvedValueOnce(mockState); + vi.mocked(isDaemonAlive).mockResolvedValueOnce(true); + vi.mocked(isDaemonVersionMatch).mockReturnValueOnce(true); + + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ ok: true, result: 'launched' }), + } as Response); + + const extensionPath = path.resolve('/custom-extension'); + const origArgv = process.argv; + process.argv = ['node', 'mm', 'launch', '--extension-path', extensionPath]; + + await main(); + + expect(globalThis.fetch).toHaveBeenCalledWith( + 'http://127.0.0.1:3000/launch', + expect.objectContaining({ + body: JSON.stringify({ extensionPath }), + }), + ); + + process.argv = origArgv; + }); + it('routes cleanup command through discoverDaemon', async () => { const { readDaemonState, isDaemonAlive, isDaemonVersionMatch } = await import('../server/daemon-state.js'); @@ -2893,7 +3036,6 @@ describe('handleServe', () => { vi.mocked(readDaemonState).mockResolvedValueOnce(null); - vi.mocked(existsSync).mockReturnValue(true); mockSearch.mockResolvedValueOnce({ config: { daemon: './daemon.ts', runtime: 'node' }, filepath: '/root/mm-client-cli.config.ts', @@ -2943,7 +3085,6 @@ describe('handleServe', () => { vi.mocked(readDaemonState).mockResolvedValueOnce(staleState); vi.mocked(isDaemonAlive).mockResolvedValueOnce(false); - vi.mocked(existsSync).mockReturnValue(true); mockSearch.mockResolvedValueOnce({ config: { daemon: './d.ts', runtime: 'node' }, filepath: '/root/mm-client-cli.config.ts', @@ -3148,7 +3289,6 @@ describe('autoStartDaemon', () => { vi.mocked(acquireStartupLock).mockResolvedValueOnce(true); vi.mocked(readDaemonState).mockResolvedValueOnce(null); - vi.mocked(existsSync).mockReturnValue(true); mockSearch.mockResolvedValueOnce({ config: { daemon: './daemon.ts', runtime: 'node' }, filepath: '/root/mm-client-cli.config.ts', diff --git a/src/cli/mm.ts b/src/cli/mm.ts index d558df4..c4a5e60 100644 --- a/src/cli/mm.ts +++ b/src/cli/mm.ts @@ -1,8 +1,8 @@ #!/usr/bin/env node import { cosmiconfig } from 'cosmiconfig'; import { execSync, spawn } from 'node:child_process'; -import { existsSync } from 'node:fs'; import * as fs from 'node:fs/promises'; +import { createRequire } from 'node:module'; import * as path from 'node:path'; import pkg from '../../package.json'; @@ -28,10 +28,13 @@ const COMMAND_TIMEOUTS_MS: Record = { const AUTO_START_COMMANDS = new Set(['launch', 'serve']); const DAEMON_POLL_INTERVAL_MS = 200; -const DAEMON_POLL_MAX_ATTEMPTS = 50; // 50 * 200ms = 10s +const DAEMON_START_TIMEOUT_MS = 30_000; +const DAEMON_POLL_MAX_ATTEMPTS = + DAEMON_START_TIMEOUT_MS / DAEMON_POLL_INTERVAL_MS; const SEND_MAX_RETRIES = 3; const SEND_RETRY_BASE_DELAY_MS = 200; const CONFIG_MODULE_NAME = 'mm-client-cli'; +const SUPPORTED_RUNTIMES = new Set(['node', 'tsx']); /** * Configuration shape for mm-client-cli config files. @@ -40,13 +43,18 @@ const CONFIG_MODULE_NAME = 'mm-client-cli'; export type MmClientCliConfig = { /** Path to the daemon entry point (TypeScript or JavaScript file). */ daemon: string; - /** TypeScript runner to use. Defaults to 'tsx'. */ - runtime?: string; + /** Runtime used to start the daemon. Defaults to 'tsx'. */ + runtime?: 'node' | 'tsx'; }; type DaemonConfig = { daemonPath: string; - runtime: string; + runtime: 'node' | 'tsx'; +}; + +type RuntimeCommand = { + command: string; + getArgs: (daemonPath: string) => string[]; }; /** @@ -167,6 +175,12 @@ export async function main(): Promise { if (command === 'launch') { const launchArgs = parseLaunchArgs(args.slice(1)); + if (typeof launchArgs.extensionPath === 'string') { + launchArgs.extensionPath = path.resolve( + worktreeRoot, + launchArgs.extensionPath, + ); + } await sendRequest(daemonState.port, 'POST', '/launch', launchArgs); return; } @@ -1181,13 +1195,17 @@ export async function autoStartDaemon( } const config = await readDaemonConfig(worktreeRoot); - const runtimeBin = resolveRuntime(worktreeRoot, config.runtime); - - const child = spawn(runtimeBin, [config.daemonPath], { - detached: true, - stdio: ['ignore', 'ignore', 'ignore'], - cwd: worktreeRoot, - }); + const runtimeCommand = resolveRuntime(worktreeRoot, config.runtime); + + const child = spawn( + runtimeCommand.command, + runtimeCommand.getArgs(config.daemonPath), + { + detached: true, + stdio: ['ignore', 'ignore', 'ignore'], + cwd: worktreeRoot, + }, + ); child.unref(); return await waitForDaemon(worktreeRoot); @@ -1219,14 +1237,18 @@ export async function handleServe( } const config = await readDaemonConfig(worktreeRoot); - const runtimeBin = resolveRuntime(worktreeRoot, config.runtime); + const runtimeCommand = resolveRuntime(worktreeRoot, config.runtime); if (background) { - const child = spawn(runtimeBin, [config.daemonPath], { - detached: true, - stdio: ['ignore', 'ignore', 'ignore'], - cwd: worktreeRoot, - }); + const child = spawn( + runtimeCommand.command, + runtimeCommand.getArgs(config.daemonPath), + { + detached: true, + stdio: ['ignore', 'ignore', 'ignore'], + cwd: worktreeRoot, + }, + ); child.unref(); const state = await waitForDaemon(worktreeRoot); @@ -1236,10 +1258,14 @@ export async function handleServe( return; } - const child = spawn(runtimeBin, [config.daemonPath], { - stdio: 'inherit', - cwd: worktreeRoot, - }); + const child = spawn( + runtimeCommand.command, + runtimeCommand.getArgs(config.daemonPath), + { + stdio: 'inherit', + cwd: worktreeRoot, + }, + ); await new Promise((resolve) => { child.on('exit', (code) => { @@ -1353,32 +1379,55 @@ export async function readDaemonConfig( process.exit(1); } + const runtime = config.runtime ?? 'tsx'; + if (!SUPPORTED_RUNTIMES.has(runtime)) { + process.stderr.write( + `Error: Unsupported runtime '${runtime}'. Supported runtimes are 'node' and 'tsx'.\n`, + ); + process.exit(1); + } + return { daemonPath: config.daemon, - runtime: config.runtime ?? 'tsx', + runtime, }; } /** - * Resolves the runtime binary path for spawning the daemon. + * Resolves the supported runtime command for spawning the daemon. * * @param worktreeRoot - The git worktree root directory. * @param runtime - The runtime name from configuration. - * @returns The absolute path to the runtime binary. + * @returns Spawn metadata for the runtime binary. */ -export function resolveRuntime(worktreeRoot: string, runtime: string): string { +export function resolveRuntime( + worktreeRoot: string, + runtime: 'node' | 'tsx', +): RuntimeCommand { if (runtime === 'node') { - return 'node'; + return { command: 'node', getArgs: (daemonPath) => [daemonPath] }; } - const binPath = path.join(worktreeRoot, 'node_modules', '.bin', runtime); - if (!existsSync(binPath)) { - process.stderr.write( - `Error: Runtime '${runtime}' not found at ${binPath}. Install it or set "mm.runtime" in package.json.\n`, - ); - process.exit(1); + if (runtime === 'tsx') { + try { + const requireFromProject = createRequire( + path.join(worktreeRoot, 'package.json'), + ); + const tsxCli = requireFromProject.resolve('tsx/cli'); + + return { + command: process.execPath, + getArgs: (daemonPath) => [tsxCli, daemonPath], + }; + } catch { + process.stderr.write( + `Error: Runtime 'tsx' is not installed in ${worktreeRoot}. Install it or set "mm.runtime" in package.json.\n`, + ); + process.exit(1); + } } - return binPath; + + throw new Error('Unsupported runtime'); } /** @@ -1397,7 +1446,9 @@ export async function waitForDaemon( return state; } } - throw new Error('Daemon failed to start within 10 seconds'); + throw new Error( + `Daemon failed to start within ${DAEMON_START_TIMEOUT_MS / 1000} seconds`, + ); } /** diff --git a/src/server/daemon-state.test.ts b/src/server/daemon-state.test.ts index f8a26be..869e96e 100644 --- a/src/server/daemon-state.test.ts +++ b/src/server/daemon-state.test.ts @@ -132,11 +132,11 @@ describe('daemon-state', () => { expect(acquired).toBe(false); }); - it('reclaims a stale lock by age', async () => { + it('reclaims an old lock without a valid pid', async () => { const lockPath = path.join(tmpDir, '.mm-server.lock'); const staleTime = new Date(Date.now() - 31_000); - await fs.writeFile(lockPath, `${process.pid}\n`); + await fs.writeFile(lockPath, 'not-a-pid\n'); await fs.utimes(lockPath, staleTime, staleTime); const acquired = await acquireStartupLock(tmpDir); @@ -145,24 +145,35 @@ describe('daemon-state', () => { expect(await fs.readFile(lockPath, 'utf-8')).toBe(`${process.pid}\n`); }); - it('reclaims a stale lock for a dead pid', async () => { + it('does not reclaim an old lock held by a live process', async () => { const lockPath = path.join(tmpDir, '.mm-server.lock'); + const staleTime = new Date(Date.now() - 31_000); - await fs.writeFile(lockPath, '999999\n'); + await fs.writeFile(lockPath, `${process.pid}\n`); + await fs.utimes(lockPath, staleTime, staleTime); - const acquired = await acquireStartupLock(tmpDir); + expect(await acquireStartupLock(tmpDir)).toBe(false); + }); - expect(acquired).toBe(true); - expect(await fs.readFile(lockPath, 'utf-8')).toBe(`${process.pid}\n`); + it('reclaims an old lock with a malformed pid', async () => { + const lockPath = path.join(tmpDir, '.mm-server.lock'); + const staleTime = new Date(Date.now() - 31_000); + + await fs.writeFile(lockPath, `${process.pid}invalid\n`); + await fs.utimes(lockPath, staleTime, staleTime); + + expect(await acquireStartupLock(tmpDir)).toBe(true); }); - it('returns false when stale lock check errors', async () => { - await fs.writeFile(path.join(tmpDir, '.mm-server.lock'), '12345\n'); - await fs.chmod(path.join(tmpDir, '.mm-server.lock'), 0o000); + it('reclaims a stale lock for a dead pid', async () => { + const lockPath = path.join(tmpDir, '.mm-server.lock'); + + await fs.writeFile(lockPath, '999999\n'); const acquired = await acquireStartupLock(tmpDir); - expect(acquired).toBe(false); + expect(acquired).toBe(true); + expect(await fs.readFile(lockPath, 'utf-8')).toBe(`${process.pid}\n`); }); it('throws when lock creation fails with a non-EEXIST error', async () => { diff --git a/src/server/daemon-state.ts b/src/server/daemon-state.ts index d3cfe9a..941fd53 100644 --- a/src/server/daemon-state.ts +++ b/src/server/daemon-state.ts @@ -124,7 +124,8 @@ export function generateNonce(): string { /** * Acquires an exclusive startup lock for the worktree. * Uses O_CREAT | O_EXCL to atomically create the lock file — if it already - * exists, checks whether the lock is stale (dead PID or older than 30s) + * exists, checks whether the lock is stale (a dead PID or an old lock without + * a valid PID) * and reclaims it if so. * * @param worktreeRoot - Absolute path to the git worktree root. @@ -161,7 +162,7 @@ export async function acquireStartupLock( * Checks whether a lock file is stale by examining PID liveness and file age. * * @param lockPath - Absolute path to the lock file. - * @returns true if the lock holder is dead or the file is older than LOCK_STALE_MS. + * @returns true if the lock holder is dead or the lock is old without a valid PID. */ async function isLockStale(lockPath: string): Promise { try { @@ -170,13 +171,9 @@ async function isLockStale(lockPath: string): Promise { fs.stat(lockPath), ]); - const ageMs = Date.now() - stat.mtimeMs; - if (ageMs > LOCK_STALE_MS) { - return true; - } - - const pid = parseInt(content.trim(), 10); - if (!isNaN(pid)) { + const pidText = content.trim(); + if (/^[1-9]\d*$/u.test(pidText)) { + const pid = Number(pidText); try { process.kill(pid, 0); return false; @@ -185,7 +182,7 @@ async function isLockStale(lockPath: string): Promise { } } - return false; + return Date.now() - stat.mtimeMs > LOCK_STALE_MS; } catch { return false; } diff --git a/src/tools/interaction.test.ts b/src/tools/interaction.test.ts index 7dc2d98..0d16c0c 100644 --- a/src/tools/interaction.test.ts +++ b/src/tools/interaction.test.ts @@ -511,6 +511,9 @@ describe('interaction', () => { it('types text into element by CSS selector', async () => { const locator = createMockLocator(); const context = createMockContext(); + const nowSpy = vi.spyOn(Date, 'now'); + nowSpy.mockReturnValueOnce(1000); + nowSpy.mockReturnValueOnce(1000); vi.spyOn(discoveryModule, 'waitForTarget').mockResolvedValue( locator as any,