Skip to content

Commit 440603d

Browse files
shaun0927cameroncooke
authored andcommitted
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
1 parent e628bb2 commit 440603d

3 files changed

Lines changed: 121 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323

2424
- 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)).
2525

26+
### Fixed
27+
28+
- Fixed `xcode_tools_bridge_disconnect` immediately re-syncing proxied tools after a manual disconnect ([#343](https://github.com/getsentry/XcodeBuildMCP/issues/343)).
29+
2630
## [2.3.2]
2731

2832
### Fixed
@@ -449,5 +453,3 @@ Please note that the UI automation features are an early preview and currently i
449453
## [v1.0.1] - 2025-04-02
450454
- Initial release of XcodeBuildMCP
451455
- Basic support for building iOS and macOS applications
452-
453-
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
2+
import { beforeEach, describe, expect, it, vi } from 'vitest';
3+
4+
const { registryMocks, buildStatusMock, serviceMocks, onToolCatalogInvalidatedRef } = vi.hoisted(
5+
() => ({
6+
registryMocks: {
7+
clear: vi.fn(),
8+
getRegisteredCount: vi.fn(() => 0),
9+
sync: vi.fn(() => ({ added: 0, updated: 0, removed: 0, total: 0 })),
10+
},
11+
buildStatusMock: vi.fn(),
12+
serviceMocks: {
13+
setWorkflowEnabled: vi.fn(),
14+
disconnect: vi.fn(),
15+
getClientStatus: vi.fn(),
16+
getLastError: vi.fn(),
17+
listTools: vi.fn(),
18+
invokeTool: vi.fn(),
19+
},
20+
onToolCatalogInvalidatedRef: {
21+
current: undefined as (() => void) | undefined,
22+
},
23+
}),
24+
);
25+
26+
vi.mock('../registry.ts', () => ({
27+
XcodeToolsProxyRegistry: vi.fn().mockImplementation(() => registryMocks),
28+
}));
29+
30+
vi.mock('../core.ts', () => ({
31+
buildXcodeToolsBridgeStatus: buildStatusMock,
32+
classifyBridgeError: vi.fn(() => 'XCODE_MCP_UNAVAILABLE'),
33+
getMcpBridgeAvailability: vi.fn(),
34+
serializeBridgeTool: vi.fn((tool) => tool),
35+
}));
36+
37+
vi.mock('../tool-service.ts', () => ({
38+
XcodeIdeToolService: vi
39+
.fn()
40+
.mockImplementation((options: { onToolCatalogInvalidated?: () => void }) => {
41+
onToolCatalogInvalidatedRef.current = options.onToolCatalogInvalidated;
42+
return serviceMocks;
43+
}),
44+
}));
45+
46+
import { XcodeToolsBridgeManager } from '../manager.ts';
47+
48+
describe('XcodeToolsBridgeManager', () => {
49+
beforeEach(() => {
50+
onToolCatalogInvalidatedRef.current = undefined;
51+
52+
registryMocks.clear.mockReset();
53+
registryMocks.getRegisteredCount.mockReset();
54+
registryMocks.getRegisteredCount.mockReturnValue(0);
55+
registryMocks.sync.mockReset();
56+
registryMocks.sync.mockReturnValue({ added: 0, updated: 0, removed: 0, total: 0 });
57+
58+
buildStatusMock.mockReset();
59+
buildStatusMock.mockResolvedValue({
60+
workflowEnabled: true,
61+
bridgeAvailable: false,
62+
bridgePath: null,
63+
xcodeRunning: null,
64+
connected: false,
65+
bridgePid: null,
66+
proxiedToolCount: 0,
67+
lastError: null,
68+
xcodePid: null,
69+
xcodeSessionId: null,
70+
});
71+
72+
serviceMocks.setWorkflowEnabled.mockReset();
73+
serviceMocks.disconnect.mockReset();
74+
serviceMocks.disconnect.mockImplementation(async () => {
75+
onToolCatalogInvalidatedRef.current?.();
76+
});
77+
serviceMocks.getClientStatus.mockReset();
78+
serviceMocks.getClientStatus.mockReturnValue({
79+
connected: false,
80+
bridgePid: null,
81+
lastError: null,
82+
});
83+
serviceMocks.getLastError.mockReset();
84+
serviceMocks.getLastError.mockReturnValue(null);
85+
serviceMocks.listTools.mockReset();
86+
serviceMocks.invokeTool.mockReset();
87+
});
88+
89+
it('does not resync on listChanged while a manual disconnect is in progress', async () => {
90+
const server = {
91+
sendToolListChanged: vi.fn(),
92+
} as unknown as McpServer;
93+
94+
const manager = new XcodeToolsBridgeManager(server);
95+
manager.setWorkflowEnabled(true);
96+
97+
const syncSpy = vi.spyOn(manager, 'syncTools');
98+
99+
await manager.disconnectTool();
100+
await Promise.resolve();
101+
await new Promise((resolve) => setTimeout(resolve, 0));
102+
103+
expect(serviceMocks.disconnect).toHaveBeenCalledOnce();
104+
expect(syncSpy).not.toHaveBeenCalled();
105+
expect(registryMocks.clear).toHaveBeenCalledOnce();
106+
expect(server.sendToolListChanged).toHaveBeenCalledOnce();
107+
});
108+
});

src/integrations/xcode-tools-bridge/manager.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ export class XcodeToolsBridgeManager {
2424
private workflowEnabled = false;
2525
private lastError: string | null = null;
2626
private syncInFlight: Promise<ProxySyncResult> | null = null;
27+
private suppressListChangedSync = false;
2728

2829
constructor(server: McpServer) {
2930
this.server = server;
3031
this.registry = new XcodeToolsProxyRegistry(server);
3132
this.service = new XcodeIdeToolService({
3233
onToolCatalogInvalidated: (): void => {
34+
if (this.suppressListChangedSync) {
35+
return;
36+
}
3337
void this.syncTools({ reason: 'listChanged' });
3438
},
3539
});
@@ -61,6 +65,10 @@ export class XcodeToolsBridgeManager {
6165
throw new Error('xcode-ide workflow is not enabled');
6266
}
6367

68+
if (opts.reason !== 'listChanged') {
69+
this.suppressListChangedSync = false;
70+
}
71+
6472
if (this.syncInFlight) return this.syncInFlight;
6573

6674
this.syncInFlight = (async (): Promise<ProxySyncResult> => {
@@ -107,6 +115,7 @@ export class XcodeToolsBridgeManager {
107115
}
108116

109117
async disconnect(): Promise<void> {
118+
this.suppressListChangedSync = true;
110119
this.registry.clear();
111120
this.server.sendToolListChanged();
112121
await this.service.disconnect();

0 commit comments

Comments
 (0)