fix: launch emulator detached so the 5s start timeout doesn't kill it

[holloandris]

Summary

EmulatorAdapter.start() launches the emulator with runEmulator([...], { timeoutMs: 5000 }) and a comment saying it's "Expected to 'timeout' as emulator runs forever." But ProcessRunner.run() uses execa(..., { timeout }), and execa's timeout SIGTERMs the child process when it fires — it doesn't just stop awaiting. Because the emulator is a normal (non-detached) child of that execa call, it gets killed ~5s after launch, long before a cold boot completes (30–60s).

On hosts where the emulator launcher hasn't already forked a detached qemu within those 5s (reproducible on macOS / Apple Silicon), the emulator dies and emulator-device list then reports running: [] — start appears to "succeed" but no emulator survives.

Fix

Launch the emulator fully detached, with no killing timeout:

• Add ProcessRunner.runDetached() / runEmulatorDetached() using spawn(cmd, args, { detached: true, stdio: "ignore" }).unref().
• Call runEmulatorDetached() from EmulatorAdapter.start() instead of the timeout-bounded runEmulator().

The emulator now outlives the MCP process and boots normally. The existing start() registration/diff logic and the -no-snapshot-load / -no-boot-anim flags are unchanged.

Test Plan

• npm run validate passes (build + lint + complexity + 669 tests + contracts).
• Updated tests/adapters/emulator.test.ts to mock runEmulatorDetached.
• Verified manually on macOS/Apple Silicon: before the fix, emulator-device start left running: [] within ~5s; after, the emulator stays up and reaches sys.boot_completed=1, and the full install → launch → UI-drive loop works end to end.

Checklist

• Tests pass (npm run validate)
• If this completes a roadmap item: README.md "Future Roadmap" → "Current Features" — N/A (bug fix)
• If this adds new planned work: Added to README.md "Future Roadmap" — N/A
• Design doc status updated (if applicable) — N/A

[greptile-apps]

Greptile Summary

This PR fixes a real emulator lifecycle bug: EmulatorAdapter.start() previously called runEmulator() with a 5 s execa timeout, which SIGTERMs the child when it fires — killing the emulator before a cold boot completes. The fix introduces runDetached() / runEmulatorDetached() using spawn({ detached: true, stdio: "ignore" }).unref() so the emulator process outlives the MCP parent.

• process-runner.ts: adds runDetached() with a correctly-wired error event handler (prevents uncaughtException on ENOENT/EACCES) and runEmulatorDetached() that resolves the SDK path before fire-and-forgetting.
• emulator.ts: start() swaps the timeout-bounded runEmulator() call for await runEmulatorDetached(); detection/diff logic and cold-boot flags are unchanged.
• Tests: emulator.test.ts and process-runner.test.ts updated to cover the new detached path and the error-event handling.

Confidence Score: 5/5

The change is safe to merge: it correctly detaches the emulator process so it outlives the MCP parent, and the previously-flagged unhandled error event is properly handled before unref().

The fix is targeted and well-reasoned. The new runDetached() path reuses the existing validateCommand guard, attaches the error listener before calling unref(), and lets getEmulatorPath() failures propagate naturally. The detection/diff logic in start() is unchanged, tests cover both the happy path and ENOENT handling, and the build + 669-test suite passes.

No files require special attention.

Important Files Changed

Filename	Overview
src/services/process-runner.ts	Adds runDetached() and runEmulatorDetached(); error event is handled before unref(), validateCommand guard is reused, and getEmulatorPath() failures correctly propagate as a rejected Promise to the caller.
src/adapters/emulator.ts	start() now calls runEmulatorDetached() instead of the timeout-killed runEmulator(); the 2 s registration delay and AVD-diff detection logic are unchanged.
tests/adapters/emulator.test.ts	Mock updated from runEmulator (rejected) to runEmulatorDetached (resolved); no test coverage gaps for the changed path.
tests/services/process-runner.test.ts	New runDetached tests cover both happy path and ENOENT error-event handling; 50 ms timer is tight but should be reliable in CI.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant EmulatorAdapter
    participant ProcessRunner
    participant OS as OS (spawn)
    participant Emulator as emulator process

    Caller->>EmulatorAdapter: start(avdName)
    EmulatorAdapter->>ProcessRunner: getRunningEmulatorIds() [snapshot]
    ProcessRunner-->>EmulatorAdapter: existingIds[]

    EmulatorAdapter->>ProcessRunner: runEmulatorDetached(args)
    ProcessRunner->>ProcessRunner: getEmulatorPath()
    ProcessRunner->>OS: spawn(emulatorPath, args, detached+stdio-ignore)
    OS-->>ProcessRunner: ChildProcess
    ProcessRunner->>ProcessRunner: child.on(error, warn)
    ProcessRunner->>ProcessRunner: child.unref()
    ProcessRunner-->>EmulatorAdapter: void (returns immediately)

    Note over Emulator: boots independently, outlives MCP process

    EmulatorAdapter->>EmulatorAdapter: wait 2s
    EmulatorAdapter->>ProcessRunner: getRunningEmulatorIds() [diff]
    ProcessRunner-->>EmulatorAdapter: currentIds[]
    EmulatorAdapter->>EmulatorAdapter: "newIds = currentIds - existingIds"
    EmulatorAdapter-->>Caller: emulatorId

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant EmulatorAdapter
    participant ProcessRunner
    participant OS as OS (spawn)
    participant Emulator as emulator process

    Caller->>EmulatorAdapter: start(avdName)
    EmulatorAdapter->>ProcessRunner: getRunningEmulatorIds() [snapshot]
    ProcessRunner-->>EmulatorAdapter: existingIds[]

    EmulatorAdapter->>ProcessRunner: runEmulatorDetached(args)
    ProcessRunner->>ProcessRunner: getEmulatorPath()
    ProcessRunner->>OS: spawn(emulatorPath, args, detached+stdio-ignore)
    OS-->>ProcessRunner: ChildProcess
    ProcessRunner->>ProcessRunner: child.on(error, warn)
    ProcessRunner->>ProcessRunner: child.unref()
    ProcessRunner-->>EmulatorAdapter: void (returns immediately)

    Note over Emulator: boots independently, outlives MCP process

    EmulatorAdapter->>EmulatorAdapter: wait 2s
    EmulatorAdapter->>ProcessRunner: getRunningEmulatorIds() [diff]
    ProcessRunner-->>EmulatorAdapter: currentIds[]
    EmulatorAdapter->>EmulatorAdapter: "newIds = currentIds - existingIds"
    EmulatorAdapter-->>Caller: emulatorId

Loading

_{Reviews (3): Last reviewed commit: "fix: handle spawn 'error' in runDetached..." | Re-trigger Greptile}

[greptile-apps]

Unhandled spawn 'error' event can crash the process

If spawn() fails to exec the command (e.g. ENOENT when the emulator binary is missing or the path returned by getEmulatorPath() is wrong), Node.js emits an 'error' event on the returned ChildProcess. Because no listener is attached before child.unref(), Node.js re-throws it as an uncaughtException, which terminates the MCP process — turning a configuration problem into a hard crash with no useful error message to the caller.

Suggested change

	const child = spawn(command, args, {
	detached: true,
	stdio: "ignore",
	});
	child.unref();
	const child = spawn(command, args, {
	detached: true,
	stdio: "ignore",
	});
	child.on("error", () => {
	// Detached process — ignore exec errors (e.g. ENOENT) to prevent
	// an unhandled 'error' event from crashing the MCP process.
	});
	child.unref();

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/services/process-runner.ts
Line: 113-117

Comment:
**Unhandled spawn `'error'` event can crash the process**

If `spawn()` fails to exec the command (e.g. `ENOENT` when the emulator binary is missing or the path returned by `getEmulatorPath()` is wrong), Node.js emits an `'error'` event on the returned `ChildProcess`. Because no listener is attached before `child.unref()`, Node.js re-throws it as an `uncaughtException`, which terminates the MCP process — turning a configuration problem into a hard crash with no useful error message to the caller.

```suggestion
    const child = spawn(command, args, {
      detached: true,
      stdio: "ignore",
    });
    child.on("error", () => {
      // Detached process — ignore exec errors (e.g. ENOENT) to prevent
      // an unhandled 'error' event from crashing the MCP process.
    });
    child.unref();
```

How can I resolve this? If you propose a fix, please make it concise.

[holloandris]

Thanks for the review — good catch on the P1. Fixed in 0fa52b4:

• runDetached() now attaches a child.on("error", …) handler before unref(), so an ENOENT/EACCES (wrong/missing binary path) no longer becomes an unhandled 'error' → uncaughtException that kills the MCP.
• Rather than swallow it silently, the handler logs via the project logger (logger.warn), so the reason is still visible. The failure remains surfaced to the caller — EmulatorAdapter.start() throws EMULATOR_START_FAILED when no device appears — so a misconfig is now a clean error instead of a hard crash.
• Added runDetached tests: happy path + a missing-binary case asserting it doesn't throw and the error is logged.

npm run validate passes (build + lint + complexity + 671 tests + contracts).

[thecombatwombat]

@greptileai lets get this PR reviewed again with the latest changes