Skip to content

refactor(10/12): rewire CLI, daemon, and MCP server boundaries#328

Merged
cameroncooke merged 3 commits intomainfrom
refactor/cli-daemon-mcp-boundaries
Apr 10, 2026
Merged

refactor(10/12): rewire CLI, daemon, and MCP server boundaries#328
cameroncooke merged 3 commits intomainfrom
refactor/cli-daemon-mcp-boundaries

Conversation

@cameroncooke
Copy link
Copy Markdown
Collaborator

Summary

This is PR 10 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 9 (all tool migrations complete). This PR connects the dots -- it rewires the three output boundaries (CLI, daemon, MCP server) to use the render session from PR 3 with the event-based handlers from PRs 5-9.

Architecture: three boundaries, one rendering engine

The rendering pipeline has three consumers, each using the same render session differently:

CLI direct (src/cli/register-tool-commands.ts): Creates a render session with the user's chosen strategy (text or JSON). Wraps emit to write incrementally to stdout as events arrive (streaming output during builds). After the handler completes, calls finalize() to flush any buffered content (grouped diagnostics). Sets exit code from session.isError().

MCP server (src/utils/tool-registry.ts): Creates a render session with text strategy. After the handler completes, calls finalize() to get the complete rendered string, wraps it in a ToolResponse (the MCP protocol type). ToolResponse construction happens here and only here -- it is not exported or used anywhere else in the codebase.

Daemon (src/daemon/daemon-server.ts): Creates a render session in record-only mode. Sends raw events over the wire to the CLI client. The CLI client re-renders locally using its own session, giving the same output as CLI direct mode.

CLI changes

  • output.ts: Deleted printToolResponse(), extractRenderedNextSteps(), and all _meta coordination logic. These were the main sources of complexity in the old rendering pipeline. Kept formatToolList() for --list output.
  • register-tool-commands.ts: Now creates and manages the render session directly. Streaming output is handled by a thin wrapper around session.emit().
  • daemon-client.ts: Updated to receive events from daemon and render locally.

Daemon protocol

  • Protocol version bumped to v2. Wire payload is now { events, attachments?, isError? } instead of pre-rendered content.
  • Old CLI + new daemon (or vice versa) gets a clear version mismatch error with restart instructions.

MCP resources

All MCP resource handlers updated to use the simplified context pattern (10 files). These are simpler than tool handlers since resources don't emit pipeline events.

Other changes

  • src/integrations/xcode-tools-bridge/: Updated to work with the new handler context. Added bridge-tool-result.ts for structured bridge tool results.
  • src/visibility/exposure.ts: Updated exposure predicates for new handler signatures.
  • src/smoke-tests/: Updated test harness for new tool invocation pattern.
  • Various utility files updated for signature changes: CommandExecutor, FileSystemExecutor, execution/, xcode-state-reader, xcode-state-watcher, video-capture, log_capture.

Stack navigation

  • PR 1-9/12: Foundation, utilities, contract, tool migrations
  • PR 10/12 (this PR): CLI, daemon, MCP server boundaries
  • PR 11/12: Manifests, config, docs, examples
  • PR 12/12: Snapshot test fixtures and benchmarks

Test plan

  • npx vitest run passes -- boundary tests updated
  • CLI text output matches expected formatting (manual spot check)
  • CLI JSON output produces valid JSONL
  • MCP server returns correctly structured ToolResponse content
  • Daemon protocol v2 correctly transmits events over wire
  • Smoke tests pass with new tool invocation pattern

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 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Nullish coalescing changes Xcode availability detection logic
    • Changed nullish coalescing operator (??) back to logical OR (||) to properly handle empty strings in Xcode version field chain.
  • ✅ Fixed: Unused outputStyle variable after refactoring
    • Removed the unused outputStyle variable extraction that was left behind after refactoring removed printToolResponse().

Create PR

Or push these changes by commenting:

@cursor push 5001d2a65c
Preview (5001d2a65c)
diff --git a/src/cli/register-tool-commands.ts b/src/cli/register-tool-commands.ts
--- a/src/cli/register-tool-commands.ts
+++ b/src/cli/register-tool-commands.ts
@@ -221,7 +221,6 @@
       const jsonArg = argv.json as string | undefined;
       const profileOverride = argv.profile as string | undefined;
       const outputFormat = (argv.output as OutputFormat) ?? 'text';
-      const outputStyle = (argv.style as OutputStyle) ?? 'normal';
       const socketPath = argv.socket as string;
       const logLevel = argv['log-level'] as string | undefined;
 

diff --git a/src/daemon.ts b/src/daemon.ts
--- a/src/daemon.ts
+++ b/src/daemon.ts
@@ -208,9 +208,9 @@
       return { success: result.success, output: result.output };
     });
     const xcodeAvailable = Boolean(
-      xcodeVersion.version ??
-        xcodeVersion.buildVersion ??
-        xcodeVersion.developerDir ??
+      xcodeVersion.version ||
+        xcodeVersion.buildVersion ||
+        xcodeVersion.developerDir ||
         xcodeVersion.xcodebuildPath,
     );
     const axeVersion = await getAxeVersionMetadata(async (command) => {

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

xcodeVersion.developerDir ||
xcodeVersion.version ??
xcodeVersion.buildVersion ??
xcodeVersion.developerDir ??
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nullish coalescing changes Xcode availability detection logic

Medium Severity

Replacing || with ?? changes the semantics for empty strings. If xcodeVersion.version is "" (common when a command fails), ?? considers it non-nullish and stops evaluation, making Boolean("") return false. With ||, the chain would continue to check buildVersion, developerDir, and xcodebuildPath. This can incorrectly report xcodeAvailable as false even when later fields have valid values.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b91063e. Configure here.

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 bug. The XcodeVersionMetadata fields are typed as string | undefined and are only assigned under result.success guards. The values come from regex captures ((.+) requires 1+ chars) or trim() on successful command output, so they are either undefined or non-empty strings. Empty string is not a realistic case, making ?? and || equivalent here.

@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from b91063e to e6d00fe Compare April 8, 2026 21:29
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from f8a5827 to 23575fb Compare April 8, 2026 21:29
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from fd818c3 to dad2143 Compare April 9, 2026 10:39
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch 2 times, most recently from a3c5515 to f79a1f8 Compare April 9, 2026 10:56
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from dad2143 to 87835ee Compare April 9, 2026 10:56
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from 0cf0796 to 701bb70 Compare April 9, 2026 11:31
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from 7ba6db8 to 34ec659 Compare April 9, 2026 11:31
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from 701bb70 to fb11021 Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from 34ec659 to b3c094d Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from fb11021 to e31bf00 Compare April 9, 2026 12:03
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from b3c094d to 13b58c1 Compare April 9, 2026 12:03
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from 13b58c1 to b1364d7 Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from e31bf00 to 57874da Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from 57874da to f7944fd Compare April 9, 2026 15:15
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from b1364d7 to b0455f7 Compare April 9, 2026 15:15
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from f7944fd to 6249b37 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from b0455f7 to 2f31ed2 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from 6249b37 to 0ae8ed2 Compare April 9, 2026 20:53
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch 2 times, most recently from 62a5be6 to a089cf9 Compare April 10, 2026 07:36
@cameroncooke cameroncooke force-pushed the refactor/migrate-remaining-tools branch from 9886ffc to 9552f12 Compare April 10, 2026 07:36
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.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit a089cf9. Configure here.

Copy link
Copy Markdown
Collaborator Author

cameroncooke commented Apr 10, 2026

Merge activity

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

@cameroncooke cameroncooke changed the base branch from refactor/migrate-remaining-tools to graphite-base/328 April 10, 2026 12:28
@cameroncooke cameroncooke changed the base branch from graphite-base/328 to main April 10, 2026 12:29
@cameroncooke cameroncooke force-pushed the refactor/cli-daemon-mcp-boundaries branch from a089cf9 to 8adb1b2 Compare April 10, 2026 12:30
@cameroncooke cameroncooke merged commit 2063cca into main Apr 10, 2026
11 of 12 checks passed
@cameroncooke cameroncooke deleted the refactor/cli-daemon-mcp-boundaries branch April 10, 2026 12:31
Comment on lines 229 to 234
params.remoteTool,
params.args ?? {},
);
const result: XcodeIdeInvokeResult = { response };
const xcodeResult = toolResponseToDaemonResult(response as ToolResponse);
const result: XcodeIdeInvokeResult = { result: xcodeResult };
return writeFrame(socket, { ...base, result });
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: A type cast from CallToolResult to ToolResponse causes toolResponseToDaemonResult to corrupt structured text output and silently discard image content from Xcode IDE tools.
Severity: HIGH

Suggested Fix

Update toolResponseToDaemonResult to correctly handle a CallToolResult object. This involves adding logic to process its content array, properly handling both text and image items without wrapping them in statusLine events, and removing the incorrect type cast as ToolResponse in daemon-server.ts.

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/daemon/daemon-server.ts#L229-L234

Potential issue: A type mismatch occurs when invoking Xcode IDE tools through the
daemon. The `invokeTool` function returns a `CallToolResult`, but this is incorrectly
cast to a `ToolResponse`. This triggers fallback logic in `toolResponseToDaemonResult`
that is not designed for this type. As a result, any structured text output (like JSON)
is corrupted by being wrapped in status line events, and any image content returned by
the tool is silently discarded. This affects all Xcode IDE tools that return structured
data or images when invoked via the CLI daemon.

Comment on lines +79 to +82
if (verbose) {
childProcess.stdout?.pipe(process.stderr, { end: false });
childProcess.stderr?.pipe(process.stderr, { end: false });
}
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: When using --output raw, the tool's output is buffered but never written to stdout because the result of session.finalize() is discarded. Subprocess output is sent to stderr instead.
Severity: HIGH

Suggested Fix

Capture the return value of session.finalize() and write its contents to process.stdout. This will ensure that the buffered raw output is correctly emitted, aligning its behavior with other output formats like json and text.

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/utils/command.ts#L79-L82

Potential issue: When a command is run with the `--output raw` flag, the tool's output
is not written to stdout as expected. The code creates a text render session that
buffers the output, but the final result from `session.finalize()` is discarded. While
subprocess output is redirected to stderr due to the `XCODEBUILDMCP_VERBOSE=1` flag, the
primary tool output is lost. This prevents users from piping the raw output to other
commands or using it in scripts, as stdout will be empty.

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