Skip to content

refactor(6/12): migrate simulator tools to event-based handlers#324

Open
cameroncooke wants to merge 4 commits intorefactor/runtime-handler-contractfrom
refactor/migrate-simulator-tools
Open

refactor(6/12): migrate simulator tools to event-based handlers#324
cameroncooke wants to merge 4 commits intorefactor/runtime-handler-contractfrom
refactor/migrate-simulator-tools

Conversation

@cameroncooke
Copy link
Copy Markdown
Collaborator

Summary

This is PR 6 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 5 (handler contract change).

Migrates all simulator and simulator-management tool handlers from the old return toolResponse([...]) pattern to the new ctx.emit(event) pattern introduced in PR 5. This is the largest tool migration PR because simulators are the primary development target.

Tools migrated (36 files)

Simulator tools: boot_sim, build_sim, build_run_sim, get_sim_app_path, install_app_sim, launch_app_sim, list_sims, open_sim, record_sim_video, stop_app_sim, test_sim, screenshot

Simulator management tools: erase_sims, reset_sim_location, set_sim_appearance, set_sim_location, sim_statusbar

Pattern of change

Each handler follows the same mechanical transformation:

```typescript
// Before
async function handler(params) {
const result = await buildForSimulator(params);
return toolResponse([header('Build', [...]), statusLine('success', '...')]);
}

// After
async function handler(params, ctx) {
const result = await buildForSimulator(params, ctx);
ctx.emit(header('Build', [...]));
ctx.emit(statusLine('success', '...'));
}
```

Build/test tools additionally pass ctx.emit to the xcodebuild pipeline so events stream in real-time during compilation. test_sim delegates to simulator-test-execution.ts and test-preflight.ts from PR 4.

Supporting changes

  • simulator-utils.ts: Updated to work with the new handler context
  • simulator-resolver.ts: Simplified resolver that integrates with the step modules from PR 4

Stack navigation

  • PR 1-5/12: Foundation, utilities, runtime contract
  • PR 6/12 (this PR): Simulator tool migrations
  • PR 7/12: 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 simulator tool tests updated
  • Each tool handler emits the expected event sequence
  • Build/test tools stream progress events during xcodebuild execution

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Malformed JSON mock embeds command args in device array
    • Removed the incorrectly placed '-derivedDataPath' and DERIVED_DATA_DIR strings from the devices array in both test cases, restoring valid simctl JSON structure.

Create PR

Or push these changes by commenting:

@cursor push 4dad9e6f53
Preview (4dad9e6f53)
diff --git a/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts b/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts
--- a/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts
+++ b/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts
@@ -194,8 +194,6 @@
                     state: 'Booted',
                     isAvailable: true,
                   },
-                  '-derivedDataPath',
-                  DERIVED_DATA_DIR,
                 ],
               },
             }),
@@ -277,8 +275,6 @@
                     state: 'Booted',
                     isAvailable: true,
                   },
-                  '-derivedDataPath',
-                  DERIVED_DATA_DIR,
                 ],
               },
             }),

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Reviewed by Cursor Bugbot for commit daf00b3. Configure here.

@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 089f6a3 to fa1ece2 Compare April 8, 2026 21:29
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from daf00b3 to d9e9216 Compare April 8, 2026 21:29
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from fa1ece2 to e6e44bb Compare April 9, 2026 07:49
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from d9e9216 to 90f6699 Compare April 9, 2026 07:49
Comment on lines 113 to +122
`Inferred simulator platform for tests: ${inferred.platform} (source: ${inferred.source})`,
);

return handleTestLogic(
const ctx = getHandlerContext();

const simulatorResolution = await resolveSimulatorIdOrName(
executor,
params.simulatorId,
params.simulatorName,
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new simulator resolution logic uses a strict, case-sensitive match, a regression from xcodebuild's more flexible matching, which may cause existing test setups to fail.
Severity: MEDIUM

Suggested Fix

To restore the previous, more flexible behavior, consider passing the simulator name directly to xcodebuild instead of pre-resolving it with resolveSimulatorIdOrName. Alternatively, update resolveSimulatorIdOrName to perform a case-insensitive match to better align with user expectations and xcodebuild's functionality.

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/simulator/test_sim.ts#L113-L122

Potential issue: The new pre-flight simulator resolution in `test_sim.ts` uses
`resolveSimulatorIdOrName`, which performs a strict, case-sensitive name match against
available simulators. This is a regression from the previous behavior where the name was
passed directly to `xcodebuild`, which has more flexible resolution logic (e.g.,
case-insensitivity). This change can cause tests to fail for users who relied on
`xcodebuild`'s more permissive matching, as their simulator names may no longer resolve
correctly if they don't match the `simctl` output exactly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a regression. The pre-existing determineSimulatorUuid in simulator-utils.ts:94 uses identical strict equality (d.name === params.simulatorName). Both old and new code paths match the same way. xcodebuild itself expects exact simulator names as returned by simctl list devices, so case-insensitive matching would be non-standard.

@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch 2 times, most recently from 5e38f0a to c802970 Compare April 9, 2026 11:31
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 9396aff to fef58c3 Compare April 9, 2026 11:31
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from c802970 to f5b95e6 Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from fef58c3 to 03324df Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from f5b95e6 to 3a2b3da Compare April 9, 2026 12:03
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch 2 times, most recently from 589573e to 886a46f Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from 3a2b3da to 9f921b2 Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 886a46f to a42228b Compare April 9, 2026 15:15
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch 2 times, most recently from 25b8d8d to 8d84711 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from a42228b to 696acb9 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 696acb9 to 0384076 Compare April 9, 2026 20:50
The simulatorId parameter was not being passed to inferPlatform, causing
platform detection to fall back to scheme-based inference even when a
specific simulator UUID was provided. This broke non-iOS simulator
detection (e.g. watchOS, tvOS) when specified by UUID.
Replace 12 identical local runLogic definitions with the shared import
from test-helpers.ts.
@cameroncooke cameroncooke force-pushed the refactor/migrate-simulator-tools branch from 8d84711 to a7d936d Compare April 9, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant