From 440603df10b418d997b53752ccd96578673b427e Mon Sep 17 00:00:00 2001 From: JunghwanNA <70629228+shaun0927@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:24:42 +0900 Subject: [PATCH 1/2] Keep manual bridge disconnects from re-syncing immediately xcode_tools_bridge_disconnect is documented as a disconnect-and-unregister operation, but the bridge close callback still fed back into the listChanged resync path. That made an explicit manual disconnect behave like a transient reconnect event instead of a stable disconnected state. Suppress listChanged-driven resync while a manual disconnect is in progress, then clear that suppression on the next explicit startup/manual sync. Add a focused regression test for the manager callback flow and record the fix under Unreleased. Constraint: The xcode-ide bridge keeps a live invalidation callback even during explicit disconnect teardown Rejected: Remove the invalidation callback entirely | would break legitimate remote catalog refresh handling Confidence: high Scope-risk: narrow Reversibility: clean Directive: Preserve the semantic split between bridge-disconnect and bridge-sync; disconnect should not implicitly reconnect Tested: npm run format; npm run lint; npm run generate:version; npm run typecheck; npm run build; npm run docs:check; npm test Not-tested: End-to-end interaction with a real Xcode mcpbridge instance outside mocked/unit coverage --- CHANGELOG.md | 6 +- .../__tests__/manager.test.ts | 108 ++++++++++++++++++ .../xcode-tools-bridge/manager.ts | 9 ++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/integrations/xcode-tools-bridge/__tests__/manager.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cfc1e4f9..bb04fbf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - Auto-scope DerivedData per workspace/project path at xcodebuild invocation time when no explicit `derivedDataPath` is configured. Session defaults remain raw, while build/test/app-path commands derive a stable hashed subdirectory under the global DerivedData root from the resolved workspace or project path. Explicit `derivedDataPath` still takes precedence ([#340](https://github.com/getsentry/XcodeBuildMCP/issues/340)). +### Fixed + +- Fixed `xcode_tools_bridge_disconnect` immediately re-syncing proxied tools after a manual disconnect ([#343](https://github.com/getsentry/XcodeBuildMCP/issues/343)). + ## [2.3.2] ### Fixed @@ -449,5 +453,3 @@ Please note that the UI automation features are an early preview and currently i ## [v1.0.1] - 2025-04-02 - Initial release of XcodeBuildMCP - Basic support for building iOS and macOS applications - - diff --git a/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts b/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts new file mode 100644 index 00000000..29691fcd --- /dev/null +++ b/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts @@ -0,0 +1,108 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { registryMocks, buildStatusMock, serviceMocks, onToolCatalogInvalidatedRef } = vi.hoisted( + () => ({ + registryMocks: { + clear: vi.fn(), + getRegisteredCount: vi.fn(() => 0), + sync: vi.fn(() => ({ added: 0, updated: 0, removed: 0, total: 0 })), + }, + buildStatusMock: vi.fn(), + serviceMocks: { + setWorkflowEnabled: vi.fn(), + disconnect: vi.fn(), + getClientStatus: vi.fn(), + getLastError: vi.fn(), + listTools: vi.fn(), + invokeTool: vi.fn(), + }, + onToolCatalogInvalidatedRef: { + current: undefined as (() => void) | undefined, + }, + }), +); + +vi.mock('../registry.ts', () => ({ + XcodeToolsProxyRegistry: vi.fn().mockImplementation(() => registryMocks), +})); + +vi.mock('../core.ts', () => ({ + buildXcodeToolsBridgeStatus: buildStatusMock, + classifyBridgeError: vi.fn(() => 'XCODE_MCP_UNAVAILABLE'), + getMcpBridgeAvailability: vi.fn(), + serializeBridgeTool: vi.fn((tool) => tool), +})); + +vi.mock('../tool-service.ts', () => ({ + XcodeIdeToolService: vi + .fn() + .mockImplementation((options: { onToolCatalogInvalidated?: () => void }) => { + onToolCatalogInvalidatedRef.current = options.onToolCatalogInvalidated; + return serviceMocks; + }), +})); + +import { XcodeToolsBridgeManager } from '../manager.ts'; + +describe('XcodeToolsBridgeManager', () => { + beforeEach(() => { + onToolCatalogInvalidatedRef.current = undefined; + + registryMocks.clear.mockReset(); + registryMocks.getRegisteredCount.mockReset(); + registryMocks.getRegisteredCount.mockReturnValue(0); + registryMocks.sync.mockReset(); + registryMocks.sync.mockReturnValue({ added: 0, updated: 0, removed: 0, total: 0 }); + + buildStatusMock.mockReset(); + buildStatusMock.mockResolvedValue({ + workflowEnabled: true, + bridgeAvailable: false, + bridgePath: null, + xcodeRunning: null, + connected: false, + bridgePid: null, + proxiedToolCount: 0, + lastError: null, + xcodePid: null, + xcodeSessionId: null, + }); + + serviceMocks.setWorkflowEnabled.mockReset(); + serviceMocks.disconnect.mockReset(); + serviceMocks.disconnect.mockImplementation(async () => { + onToolCatalogInvalidatedRef.current?.(); + }); + serviceMocks.getClientStatus.mockReset(); + serviceMocks.getClientStatus.mockReturnValue({ + connected: false, + bridgePid: null, + lastError: null, + }); + serviceMocks.getLastError.mockReset(); + serviceMocks.getLastError.mockReturnValue(null); + serviceMocks.listTools.mockReset(); + serviceMocks.invokeTool.mockReset(); + }); + + it('does not resync on listChanged while a manual disconnect is in progress', async () => { + const server = { + sendToolListChanged: vi.fn(), + } as unknown as McpServer; + + const manager = new XcodeToolsBridgeManager(server); + manager.setWorkflowEnabled(true); + + const syncSpy = vi.spyOn(manager, 'syncTools'); + + await manager.disconnectTool(); + await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(serviceMocks.disconnect).toHaveBeenCalledOnce(); + expect(syncSpy).not.toHaveBeenCalled(); + expect(registryMocks.clear).toHaveBeenCalledOnce(); + expect(server.sendToolListChanged).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/integrations/xcode-tools-bridge/manager.ts b/src/integrations/xcode-tools-bridge/manager.ts index 576d7eaa..8baab603 100644 --- a/src/integrations/xcode-tools-bridge/manager.ts +++ b/src/integrations/xcode-tools-bridge/manager.ts @@ -24,12 +24,16 @@ export class XcodeToolsBridgeManager { private workflowEnabled = false; private lastError: string | null = null; private syncInFlight: Promise | null = null; + private suppressListChangedSync = false; constructor(server: McpServer) { this.server = server; this.registry = new XcodeToolsProxyRegistry(server); this.service = new XcodeIdeToolService({ onToolCatalogInvalidated: (): void => { + if (this.suppressListChangedSync) { + return; + } void this.syncTools({ reason: 'listChanged' }); }, }); @@ -61,6 +65,10 @@ export class XcodeToolsBridgeManager { throw new Error('xcode-ide workflow is not enabled'); } + if (opts.reason !== 'listChanged') { + this.suppressListChangedSync = false; + } + if (this.syncInFlight) return this.syncInFlight; this.syncInFlight = (async (): Promise => { @@ -107,6 +115,7 @@ export class XcodeToolsBridgeManager { } async disconnect(): Promise { + this.suppressListChangedSync = true; this.registry.clear(); this.server.sendToolListChanged(); await this.service.disconnect(); From 62c80dcee95116c3cb7a09bfa992668c6190bccf Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Sun, 26 Apr 2026 09:03:49 +0100 Subject: [PATCH 2/2] test: cover bridge listChanged re-enable after manual sync Add a second test that locks in the suppression-flag clearing contract: a manual sync after disconnect must re-enable listChanged-driven syncs, otherwise the flag silently swallows legitimate catalog-invalidation events from later reconnections. --- .../__tests__/manager.test.ts | 77 +++++++++++++------ 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts b/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts index 29691fcd..c3799992 100644 --- a/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts +++ b/src/integrations/xcode-tools-bridge/__tests__/manager.test.ts @@ -1,27 +1,33 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { Tool } from '@modelcontextprotocol/sdk/types.js'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { registryMocks, buildStatusMock, serviceMocks, onToolCatalogInvalidatedRef } = vi.hoisted( - () => ({ - registryMocks: { - clear: vi.fn(), - getRegisteredCount: vi.fn(() => 0), - sync: vi.fn(() => ({ added: 0, updated: 0, removed: 0, total: 0 })), - }, - buildStatusMock: vi.fn(), - serviceMocks: { - setWorkflowEnabled: vi.fn(), - disconnect: vi.fn(), - getClientStatus: vi.fn(), - getLastError: vi.fn(), - listTools: vi.fn(), - invokeTool: vi.fn(), - }, - onToolCatalogInvalidatedRef: { - current: undefined as (() => void) | undefined, - }, - }), -); +const { + registryMocks, + buildStatusMock, + serviceMocks, + onToolCatalogInvalidatedRef, + getMcpBridgeAvailabilityMock, +} = vi.hoisted(() => ({ + registryMocks: { + clear: vi.fn(), + getRegisteredCount: vi.fn(() => 0), + sync: vi.fn(() => ({ added: 0, updated: 0, removed: 0, total: 0 })), + }, + buildStatusMock: vi.fn(), + serviceMocks: { + setWorkflowEnabled: vi.fn(), + disconnect: vi.fn(), + getClientStatus: vi.fn(), + getLastError: vi.fn(), + listTools: vi.fn(), + invokeTool: vi.fn(), + }, + onToolCatalogInvalidatedRef: { + current: undefined as (() => void) | undefined, + }, + getMcpBridgeAvailabilityMock: vi.fn(), +})); vi.mock('../registry.ts', () => ({ XcodeToolsProxyRegistry: vi.fn().mockImplementation(() => registryMocks), @@ -30,7 +36,7 @@ vi.mock('../registry.ts', () => ({ vi.mock('../core.ts', () => ({ buildXcodeToolsBridgeStatus: buildStatusMock, classifyBridgeError: vi.fn(() => 'XCODE_MCP_UNAVAILABLE'), - getMcpBridgeAvailability: vi.fn(), + getMcpBridgeAvailability: getMcpBridgeAvailabilityMock, serializeBridgeTool: vi.fn((tool) => tool), })); @@ -83,7 +89,11 @@ describe('XcodeToolsBridgeManager', () => { serviceMocks.getLastError.mockReset(); serviceMocks.getLastError.mockReturnValue(null); serviceMocks.listTools.mockReset(); + serviceMocks.listTools.mockResolvedValue([]); serviceMocks.invokeTool.mockReset(); + + getMcpBridgeAvailabilityMock.mockReset(); + getMcpBridgeAvailabilityMock.mockResolvedValue({ available: true, path: '/usr/bin/mcpbridge' }); }); it('does not resync on listChanged while a manual disconnect is in progress', async () => { @@ -105,4 +115,27 @@ describe('XcodeToolsBridgeManager', () => { expect(registryMocks.clear).toHaveBeenCalledOnce(); expect(server.sendToolListChanged).toHaveBeenCalledOnce(); }); + + it('re-enables listChanged-driven syncs after a manual sync follows a disconnect', async () => { + const server = { + sendToolListChanged: vi.fn(), + } as unknown as McpServer; + + const tools: Tool[] = [{ name: 'remote.tool', inputSchema: { type: 'object' } } as Tool]; + serviceMocks.listTools.mockResolvedValue(tools); + + const manager = new XcodeToolsBridgeManager(server); + manager.setWorkflowEnabled(true); + + await manager.disconnectTool(); + await manager.syncTools({ reason: 'manual' }); + + const syncSpy = vi.spyOn(manager, 'syncTools'); + + onToolCatalogInvalidatedRef.current?.(); + await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(syncSpy).toHaveBeenCalledWith({ reason: 'listChanged' }); + }); });