refactor(4/12): extract build/test utilities, platform steps, and xcodebuild pipeline by cameroncooke · Pull Request #322 · getsentry/XcodeBuildMCP

cameroncooke · 2026-04-08T21:09:28Z

Summary

This is PR 4 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 3 (rendering engine).

Extracts shared build and test logic from monolithic utility files into focused, single-responsibility modules. This is prep work for the tool handler migrations (PRs 6-9) -- the extracted modules provide the building blocks that tool handlers will call instead of duplicating logic inline.

What was extracted and why

Build preflight (build-preflight.ts): Validates build parameters (project path, scheme, destination) before invoking xcodebuild. Previously scattered across individual tool handlers with subtle inconsistencies.

Test preflight (test-preflight.ts): Validates test parameters, resolves test plans, and checks for common misconfiguration. Extracted from the ~500-line test tool handlers where each platform (sim/device/macOS) had its own copy.

Platform step modules: Each platform's build/test/run workflow was a monolithic function. Now decomposed into composable steps:

simulator-steps.ts: Boot simulator, install app, launch app, capture logs
device-steps.ts: Prepare device, install, launch
macos-steps.ts: Build, launch, capture logs
app-path-resolver.ts: Locate built .app bundles in DerivedData (shared across all platforms)
device-name-resolver.ts: Resolve device UDID to display name

Xcodebuild pipeline (xcodebuild-pipeline.ts): The core build/test execution engine. Accepts an emit callback from the handler context, streams parsed events during execution, and returns structured state. This replaces the old pattern where the pipeline owned renderers directly.

Xcodebuild output (xcodebuild-output.ts): Constructs the final event sequence from pipeline state (summary, detail trees, diagnostics). Separated from the pipeline itself so the same output logic works for both real builds and test replay.

Supporting extractions: derived-data-path.ts, log-paths.ts, xcodebuild-log-capture.ts, swift-test-discovery.ts, xcresult-test-failures.ts, simulator-test-execution.ts, tool-error-handling.ts.

Deleted modules

capabilities.ts, validation/index.ts, test-result-content.ts, workflow-selection.ts -- functionality moved into the extracted modules or no longer needed with the new architecture.

Changes to existing modules

build-utils.ts: Slimmed down significantly. Warning suppression, content consolidation, and build-specific logic moved to dedicated modules.
test-common.ts: Simplified to a thin coordination layer that delegates to test-preflight.ts and swift-test-discovery.ts.

Stack navigation

PR 1-3/12: Foundation (logging removal, event types, rendering engine)
PR 4/12 (this PR): Build/test utility extraction, platform steps, xcodebuild pipeline
PR 5/12: Runtime handler contract and tool invoker
PR 6-9/12: Tool migrations (these use the extracted modules)
PR 10-12/12: Boundaries, config, tests

Test plan

npx vitest run passes -- tests for each extracted module
Build preflight correctly rejects invalid schemes/destinations
Pipeline streams events through emit callback during execution
Xcodebuild output constructs correct event sequences from pipeline state

src/utils/device-name-resolver.ts

src/utils/simulator-steps.ts

src/utils/test-common.ts

src/utils/xcresult-test-failures.ts

src/utils/xcodebuild-pipeline.ts

src/utils/test-common.ts

src/utils/xcodebuild-pipeline.ts

src/utils/xcodebuild-log-capture.ts

src/utils/xcodebuild-output.ts

src/utils/device-name-resolver.ts

src/utils/simulator-test-execution.ts

cursor · 2026-04-09T08:08:50Z

src/utils/device-name-resolver.ts

@@ -0,0 +1,69 @@
+import { execSync } from 'node:child_process';
+import { readFileSync, unlinkSync } from 'node:fs';


Direct child_process imports bypass dependency injection

Medium Severity

Several new modules import execSync, execFileSync, spawn, readFileSync, and unlinkSync directly from node:child_process and node:fs instead of using the injected CommandExecutor and FileSystemExecutor. The project architecture rules flag new child_process or fs imports as critical DI violations, since all shell commands must flow through CommandExecutor and filesystem access through FileSystemExecutor.

Additional Locations (2)

src/utils/xcresult-test-failures.ts#L1

src/utils/simulator-steps.ts#L1-L3

^{Triggered by project rule: Bugbot Review Guide for XcodeBuildMCP}

^{Reviewed by Cursor Bugbot for commit 379222d. Configure here.}

The DI guidelines have been updated in this PR (76c9848) to clarify that CommandExecutor/FileSystemExecutor DI is required for MCP tool logic orchestrating complex, long-running processes with sub-processes (e.g. xcodebuild), where standard vitest mocking produces race conditions. Standalone utility modules with simple, short-lived commands (like xcrun devicectl list here) may use direct imports and standard vitest mocking. See updated docs in BUGBOT.md, TESTING.md, CODE_QUALITY.md, CONTRIBUTING.md, and ARCHITECTURE.md.

src/utils/xcodebuild-output.ts

src/utils/device-name-resolver.ts

src/utils/build-utils.ts

cameroncooke · 2026-04-09T09:07:37Z

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

src/utils/device-name-resolver.ts

cursor · 2026-04-09T09:44:55Z

src/utils/simulator-steps.ts

+        '--level=debug',
+        '--predicate',
+        `subsystem == "${bundleId}"`,
+      ],


OSLog predicate allows bundle ID injection

Low Severity

In startOsLogStream, the bundleId is interpolated directly into a predicate string (subsystem == "${bundleId}") without sanitization. A bundleId containing a double-quote character could break the predicate syntax or alter its semantics. While bundleId typically comes from app metadata, the value flows from user-provided parameters through simctl launch output.

^{Triggered by project rule: Bugbot Review Guide for XcodeBuildMCP}

^{Reviewed by Cursor Bugbot for commit 9d23583. Configure here.}

Out of scope for this PR. Bundle IDs follow Apple's reverse-DNS format and cannot contain double-quote characters. The predicate is passed as an array argument to spawn (not shell-interpolated), so there is no injection vector here.

src/utils/xcodebuild-output.ts

src/utils/device-steps.ts

src/utils/device-name-resolver.ts

…d pipeline

The module-level device name cache was never invalidated, causing stale device names in long-running server processes. Now expires after 30 seconds so it refreshes across tool invocations while avoiding repeated devicectl shell-outs within a single invocation.

Adds a shared runLogic function to test-helpers.ts that wraps tool logic execution in a mock handler context and returns a ToolResponse- shaped result. This will replace the identical helper duplicated across 49 test files in subsequent PRs.

cameroncooke · 2026-04-09T21:51:10Z

src/utils/test-common.ts

    'info',
    `Starting test run for scheme ${params.scheme} on platform ${params.platform} (internal)`,
  );
+  const ctx = getHandlerContext();


This is a Graphite stack artifact. getHandlerContext is imported from typed-tool-factory.ts where it will be defined in a later PR in this 12-PR stack. CodeQL analyzes each PR in isolation and flags it as undefined, but it will resolve when the full stack merges. Not actionable on this branch.

cameroncooke · 2026-04-09T21:51:11Z

src/utils/tool-error-handling.ts

+  try {
+    return await run();
+  } catch (error) {
+    const message = toErrorMessage(error);


This is a Graphite stack artifact. toErrorMessage will be exported from errors.ts in a later PR in this 12-PR stack. CodeQL analyzes each PR in isolation and flags it as undefined, but it will resolve when the full stack merges. Not actionable on this branch.

cameroncooke · 2026-04-09T21:51:12Z

src/utils/xcodebuild-pipeline.ts

+    options.emit ??
+    (() => {
+      try {
+        return getHandlerContext().emit;


This is a Graphite stack artifact. getHandlerContext will be exported from typed-tool-factory.ts in a later PR in this 12-PR stack. CodeQL analyzes each PR in isolation and flags it as undefined, but it will resolve when the full stack merges. Not actionable on this branch.

cameroncooke · 2026-04-09T21:51:13Z

src/test-utils/test-helpers.ts

+    ctx,
+    result: resultObj,
+    run: async <T>(fn: () => Promise<T>): Promise<T> => {
+      return handlerContextStorage.run(ctx, fn);


This is a Graphite stack artifact. handlerContextStorage will be exported from typed-tool-factory.ts in a later PR in this 12-PR stack. CodeQL analyzes each PR in isolation and flags it as undefined, but it will resolve when the full stack merges. Not actionable on this branch.

cameroncooke · 2026-04-09T21:51:14Z

src/utils/xcodebuild-pipeline.ts

+      try {
+        return getHandlerContext().emit;
+      } catch {
+        return handlerContextStorage.getStore()?.emit;


This is a Graphite stack artifact. handlerContextStorage will be exported from typed-tool-factory.ts in a later PR in this 12-PR stack. CodeQL analyzes each PR in isolation and flags it as undefined, but it will resolve when the full stack merges. Not actionable on this branch.

src/utils/__tests__/xcodebuild-pipeline.test.ts

src/utils/build-utils.ts

…guidelines - Add missing packageCachePath to PlatformBuildOptions interface - Remove unused vi import and result variable in pipeline test - Restore responses/index.ts barrel and processToolResponse function deleted in PR 3, which broke docs:check and downstream imports - Update DI documentation to clarify that CommandExecutor/FileSystemExecutor is required for complex process orchestration (xcodebuild) but not for simple utility modules

src/utils/responses/next-steps-renderer.ts

…sponse

cursor

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

There are 3 total unresolved issues (including 2 from previous reviews).

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

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

cursor · 2026-04-09T21:53:24Z

src/utils/responses/next-steps-renderer.ts

+  const hasTextContent = response.content.some((item) => item.type === 'text');
+  if (!hasTextContent && nextStepsSection) {
+    processedContent.push({ type: 'text', text: nextStepsSection.trim() });
+  }


Next steps silently dropped when text isn't last content item

Medium Severity

processToolResponse only appends next steps to the last content item if it's a text item (item.type === 'text' && index === response.content.length - 1). When a response has text followed by non-text items (e.g. [text, image]), the text item at index 0 doesn't match the last-index check, and the image at the last index doesn't match the type check. The hasTextContent guard then prevents adding a new text block. Result: next steps are silently dropped whenever text content exists but isn't the final item.

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

cameroncooke · 2026-04-10T12:18:57Z

Merge activity

Apr 10, 12:18 PM UTC: A user started a stack merge that includes this pull request via Graphite.
Apr 10, 12:19 PM UTC: @cameroncooke merged this pull request with Graphite.

sentry bot reviewed Apr 8, 2026

View reviewed changes

src/utils/device-name-resolver.ts Show resolved Hide resolved

cursor bot reviewed Apr 8, 2026

View reviewed changes

src/utils/device-name-resolver.ts Show resolved Hide resolved

src/utils/simulator-steps.ts Show resolved Hide resolved

src/utils/test-common.ts Show resolved Hide resolved

src/utils/xcresult-test-failures.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 344aeba to f7616d2 Compare April 8, 2026 21:29

cameroncooke force-pushed the refactor/rendering-engine branch from d00ff56 to 40b05c5 Compare April 8, 2026 21:29

sentry bot reviewed Apr 8, 2026

View reviewed changes

src/utils/xcodebuild-pipeline.ts Show resolved Hide resolved

cursor bot reviewed Apr 8, 2026

View reviewed changes

src/utils/test-common.ts Show resolved Hide resolved

src/utils/xcodebuild-pipeline.ts Show resolved Hide resolved

src/utils/xcodebuild-log-capture.ts Show resolved Hide resolved

src/utils/xcodebuild-output.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/rendering-engine branch from 40b05c5 to df81928 Compare April 9, 2026 07:49

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from f7616d2 to 7b1a15a Compare April 9, 2026 07:49

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/device-name-resolver.ts Show resolved Hide resolved

src/utils/simulator-test-execution.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/rendering-engine branch from df81928 to a0f22a7 Compare April 9, 2026 07:59

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 7b1a15a to 379222d Compare April 9, 2026 07:59

cursor bot reviewed Apr 9, 2026

View reviewed changes

cameroncooke force-pushed the refactor/rendering-engine branch from a0f22a7 to b7f35d4 Compare April 9, 2026 08:45

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 379222d to 7ffc681 Compare April 9, 2026 08:45

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/device-name-resolver.ts Show resolved Hide resolved

src/utils/build-utils.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 7ffc681 to 9d23583 Compare April 9, 2026 09:33

cameroncooke force-pushed the refactor/rendering-engine branch from b7f35d4 to 67d1e46 Compare April 9, 2026 09:33

cursor bot reviewed Apr 9, 2026

View reviewed changes

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from a36e4d5 to c0d77e5 Compare April 9, 2026 12:03

cameroncooke force-pushed the refactor/rendering-engine branch from 2ac1bbb to 9b5d25f Compare April 9, 2026 12:03

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/xcodebuild-output.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/rendering-engine branch from 9b5d25f to 96962ec Compare April 9, 2026 14:43

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from c0d77e5 to 7315fff Compare April 9, 2026 14:43

cameroncooke force-pushed the refactor/rendering-engine branch from 96962ec to 77c7b0b Compare April 9, 2026 15:15

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 7315fff to 80b323f Compare April 9, 2026 15:15

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/device-steps.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/rendering-engine branch from 77c7b0b to a173ad8 Compare April 9, 2026 15:40

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 80b323f to 3126be4 Compare April 9, 2026 15:40

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/device-name-resolver.ts Show resolved Hide resolved

cameroncooke force-pushed the refactor/rendering-engine branch from 27c8907 to 6716c5b Compare April 9, 2026 19:26

cameroncooke changed the base branch from refactor/rendering-engine to graphite-base/322 April 9, 2026 20:49

cameroncooke added 2 commits April 9, 2026 20:50

refactor: extract build/test utilities, platform steps, and xcodebuil…

5fa2c95

…d pipeline

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 3126be4 to 09a2d7d Compare April 9, 2026 20:50

cameroncooke force-pushed the graphite-base/322 branch from a173ad8 to 200ac9c Compare April 9, 2026 20:50

graphite-app bot changed the base branch from graphite-base/322 to main April 9, 2026 20:50

refactor: add shared runLogic test helper

bee0b39

Adds a shared runLogic function to test-helpers.ts that wraps tool logic execution in a mock handler context and returns a ToolResponse- shaped result. This will replace the identical helper duplicated across 49 test files in subsequent PRs.

cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 09a2d7d to bee0b39 Compare April 9, 2026 20:50

github-code-quality bot found potential problems Apr 9, 2026

View reviewed changes

sentry bot reviewed Apr 9, 2026

View reviewed changes

src/utils/build-utils.ts Show resolved Hide resolved

cursor bot reviewed Apr 9, 2026

View reviewed changes

src/utils/responses/next-steps-renderer.ts Outdated Show resolved Hide resolved

cameroncooke mentioned this pull request Apr 9, 2026

Migrate consumers off responses/index.ts barrel and remove it #332

Open

fix: add newline separator before next steps section in processToolRe…

d279a9b

…sponse

cursor bot reviewed Apr 9, 2026

View reviewed changes

cameroncooke merged commit 8ad1a78 into main Apr 10, 2026
11 of 12 checks passed

cameroncooke deleted the refactor/build-test-utility-extraction branch April 10, 2026 12:19

		@@ -0,0 +1,69 @@
		import { execSync } from 'node:child_process';
		import { readFileSync, unlinkSync } from 'node:fs';

Uh oh!

Conversation

cameroncooke commented Apr 8, 2026

Summary

What was extracted and why

Deleted modules

Changes to existing modules

Stack navigation

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot Apr 9, 2026

Choose a reason for hiding this comment

Direct child_process imports bypass dependency injection

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameroncooke commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cursor bot Apr 9, 2026

Choose a reason for hiding this comment

OSLog predicate allows bundle ID injection

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

cameroncooke Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Apr 9, 2026

Choose a reason for hiding this comment

Next steps silently dropped when text isn't last content item

Uh oh!

cameroncooke commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Direct `child_process` imports bypass dependency injection

cameroncooke commented Apr 9, 2026 •

edited

Loading

cameroncooke commented Apr 10, 2026 •

edited

Loading