refactor(7/12): migrate device and macOS tools to event-based handlers

[cameroncooke]

Summary

This is PR 7 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 6 (simulator migrations).

Migrates all device and macOS tool handlers to the new event-based handler contract. Same mechanical transformation pattern as PR 6 but for physical device and macOS desktop targets.

Tools migrated (31 files)

Device tools: build_device, build_run_device, get_device_app_path, install_app_device, launch_app_device, list_devices, stop_app_device, test_device, build-settings

macOS tools: build_macos, build_run_macos, get_mac_app_path, launch_mac_app, stop_mac_app, test_macos

Notable changes

• test_device.ts and test_macos.ts were the most complex handlers (~250-280 lines each). They've been significantly simplified by delegating to test-preflight.ts, device-steps.ts/macos-steps.ts, and xcodebuild-pipeline.ts from PR 4. The handlers are now thin orchestrators that emit events.
• Device and macOS build tools pass ctx.emit through to the xcodebuild pipeline for real-time progress streaming.
• stop_app_device.ts and stop_mac_app.ts updated to emit structured events for process termination results.

Stack navigation

• PR 1-5/12: Foundation, utilities, runtime contract
• PR 6/12: Simulator tool migrations
• PR 7/12 (this PR): Device + macOS tool migrations
• PR 8/12: UI automation tool migrations
• PR 9/12: Remaining tool migrations
• PR 10-12/12: Boundaries, config, tests

Test plan

• npx vitest run passes -- all device and macOS tool tests updated
• Build tools stream progress events through the xcodebuild pipeline
• Test tools correctly delegate to test-preflight and platform step modules

[cameroncooke]

• refactor(12/12): add snapshot test fixtures and benchmarks #330
• refactor(11/12): update manifests, config, docs, and example projects #329
• refactor(10/12): rewire CLI, daemon, and MCP server boundaries #328
• refactor(9/12): migrate debugging, swift-package, and remaining tools #327
• refactor(8/12): migrate UI automation tools to event-based handlers #326
• refactor(7/12): migrate device and macOS tools to event-based handlers #325 👈 (View in Graphite)
• refactor(6/12): migrate simulator tools to event-based handlers #324
• refactor(5/12): change handler contract to event-based emission #323
• refactor(4/12): extract build/test utilities, platform steps, and xcodebuild pipeline #322
• refactor(3/12): add rendering engine and output formatting #321
• refactor(2/12): add pipeline event types, parsers, and event builders #320
• refactor(1/12): remove deprecated logging tools #319
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[sentry]

Bug: The use of execSync in device name resolution functions blocks the Node.js event loop, causing the application to hang for up to 10 seconds.
_{Severity: HIGH}

Suggested Fix

Replace the synchronous execSync call in device-name-resolver.ts with an asynchronous alternative, such as exec or spawn from the child_process module. This will prevent the event loop from being blocked while waiting for the external command to complete. Ensure the asynchronous implementation properly handles stdout, stderr, and potential errors.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/mcp/tools/device/build_run_device.ts#L111

Potential issue: The functions `formatDeviceId` and `resolveDeviceName` use `execSync`
to run an external command (`xcrun devicectl list devices`). This is a synchronous,
blocking call that will freeze the Node.js event loop for up to its 10-second timeout.
While a 30-second cache mitigates repeated blocking, the first call to any affected
device tool, or any call after the cache expires, will cause the application to hang.
This behavior violates the project's own coding standards, which ban `execSync` in
production code due to its severe performance implications.

[cameroncooke]

Valid concern, tracked in #333. Low priority given the 10s timeout, 30s TTL cache, and the MCP server's single-request-at-a-time model.

[sentry]

Bug: ctx.nextStepParams is set unconditionally in list_devicesLogic, suggesting follow-up actions even when no devices are found or an error occurs.
_{Severity: MEDIUM}

Suggested Fix

Reintroduce a conditional check before setting ctx.nextStepParams. The assignment should only happen if the availableDevices array is not empty, mirroring the logic that was present before the refactoring.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/mcp/tools/device/list_devices.ts#L342-L345

Potential issue: In the `list_devicesLogic` function, `ctx.nextStepParams` is set
unconditionally. This occurs even when no devices are found or when an error is
encountered during the device listing process. This is a regression from the previous
implementation, where this assignment was guarded by a condition checking for the
existence of available devices. Consequently, the client will be misled into suggesting
follow-up actions like `build_device` when no devices are actually available, degrading
the user experience and potentially causing errors for automated clients.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8f6c4c7. Configure here.}

[cursor]

Unconditional nextStepParams assignment breaks conditional behavior

Medium Severity

ctx.nextStepParams is set unconditionally after buildEvents() completes, but the old code only set nextStepParams when availableDevicesExist was true. This means next-step hints are now provided even when no devices are found or all devices are unavailable, which is misleading. The test at line 219 expects nextStepParams to be undefined, confirming the intended conditional behavior was lost during migration.

Additional Locations (1)

• src/mcp/tools/device/__tests__/list_devices.test.ts#L218-L219

 

^{Reviewed by Cursor Bugbot for commit 8f6c4c7. Configure here.}

[cameroncooke]

Merge activity

• Apr 10, 12:18 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 10, 12:25 PM UTC: Graphite rebased this pull request as part of a merge.
• Apr 10, 12:25 PM UTC: @cameroncooke merged this pull request with Graphite.