Skip to content

Commit db7877d

Browse files
cameroncookecameroncooke
committed
fix(types): properly type ToolHandler return for test-mode invocations
The tool factory functions already returned a ToolTestResult-shaped object at runtime when called without a ToolHandlerContext (test mode), but lied to TypeScript via `as unknown as void`. This caused 350+ typecheck errors across test files accessing .content and .isError on void. Add ToolHandler overloaded interface that reflects the actual runtime behavior: void with context (production), ToolTestResult without (tests). Update callHandler return type and allText helper to accept broader content types. Fix miscellaneous test type issues (findLast, readonly properties, mock return types).
1 parent 41f3544 commit db7877d

File tree

16 files changed

+125
-106
lines changed

16 files changed

+125
-106
lines changed

‎src/cli/__tests__/register-tool-commands.test.ts‎

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ function createTool(overrides: Partial<ToolDefinition> = {}): ToolDefinition {
2323
scheme: z.string().optional(),
2424
},
2525
stateful: false,
26-
handler: vi.fn(async () => ({
27-
content: [createTextContent('ok')],
28-
isError: false,
29-
})),
26+
handler: vi.fn(async () => {}) as ToolDefinition['handler'],
3027
...overrides,
3128
};
3229
}
@@ -97,10 +94,9 @@ describe('registerToolCommands', () => {
9794
});
9895

9996
it('hydrates required args from the active defaults profile', async () => {
100-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
101-
content: [createTextContent('ok')],
102-
isError: false,
103-
});
97+
const invokeDirect = vi
98+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
99+
.mockResolvedValue(undefined);
104100
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
105101

106102
const tool = createTool();
@@ -126,10 +122,9 @@ describe('registerToolCommands', () => {
126122
it('hydrates required args from the explicit --profile override', async () => {
127123
process.argv = ['node', 'xcodebuildmcp', 'simulator', 'run-tool', '--profile', 'qa'];
128124

129-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
130-
content: [createTextContent('ok')],
131-
isError: false,
132-
});
125+
const invokeDirect = vi
126+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
127+
.mockResolvedValue(undefined);
133128
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
134129

135130
const tool = createTool();
@@ -176,10 +171,9 @@ describe('registerToolCommands', () => {
176171
});
177172

178173
it('hydrates args before daemon-routed invocation', async () => {
179-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
180-
content: [createTextContent('ok')],
181-
isError: false,
182-
});
174+
const invokeDirect = vi
175+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
176+
.mockResolvedValue(undefined);
183177
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
184178

185179
const tool = createTool({ stateful: true });
@@ -199,10 +193,9 @@ describe('registerToolCommands', () => {
199193
});
200194

201195
it('lets explicit args override conflicting defaults before invocation', async () => {
202-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
203-
content: [createTextContent('ok')],
204-
isError: false,
205-
});
196+
const invokeDirect = vi
197+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
198+
.mockResolvedValue(undefined);
206199
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
207200

208201
const tool = createTool({
@@ -250,10 +243,9 @@ describe('registerToolCommands', () => {
250243
});
251244

252245
it('lets --json override configured defaults', async () => {
253-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
254-
content: [createTextContent('ok')],
255-
isError: false,
256-
});
246+
const invokeDirect = vi
247+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
248+
.mockResolvedValue(undefined);
257249
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
258250

259251
const tool = createTool();
@@ -280,10 +272,9 @@ describe('registerToolCommands', () => {
280272
});
281273

282274
it('allows --json to satisfy required arguments', async () => {
283-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
284-
content: [createTextContent('ok')],
285-
isError: false,
286-
});
275+
const invokeDirect = vi
276+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
277+
.mockResolvedValue(undefined);
287278
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
288279

289280
const tool = createTool();
@@ -315,10 +306,9 @@ describe('registerToolCommands', () => {
315306
});
316307

317308
it('allows array args that begin with a dash', async () => {
318-
const invokeDirect = vi.spyOn(DefaultToolInvoker.prototype, 'invokeDirect').mockResolvedValue({
319-
content: [createTextContent('ok')],
320-
isError: false,
321-
});
309+
const invokeDirect = vi
310+
.spyOn(DefaultToolInvoker.prototype, 'invokeDirect')
311+
.mockResolvedValue(undefined);
322312
const stdoutWrite = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
323313

324314
const tool = createTool({

‎src/mcp/tools/debugging/__tests__/debugging-tools.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ const runLogic = async (logic: () => Promise<unknown>) => {
5959
return response as {
6060
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
6161
isError?: boolean;
62-
nextStepParams?: unknown;
62+
nextStepParams?: Record<string, Record<string, unknown> | Record<string, unknown>[]>;
6363
};
6464
}
6565

‎src/mcp/tools/device/__tests__/build_run_device.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ describe('build_run_device tool', () => {
258258
);
259259
expect(completionEvent).toBeDefined();
260260

261-
const detailTree = result.events.findLast(
262-
(event): event is { type: 'detail-tree'; items: Array<{ label: string; value: string }> } =>
263-
event.type === 'detail-tree',
264-
);
261+
const detailTrees = result.events.filter((event) => event.type === 'detail-tree');
262+
const detailTree = detailTrees[detailTrees.length - 1] as
263+
| { type: 'detail-tree'; items: Array<{ label: string; value: string }> }
264+
| undefined;
265265
expect(detailTree).toBeDefined();
266266
expect(detailTree?.items.some((item) => item.label === 'Process ID')).toBe(false);
267267
});

‎src/mcp/tools/macos/__tests__/launch_mac_app.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,7 @@ describe('launch_mac_app plugin', () => {
9696
);
9797

9898
expect(result.isError).toBe(true);
99-
const text = result.content
100-
.filter((i) => i.type === 'text')
101-
.map((i) => i.text)
102-
.join('\n');
99+
const text = allText(result);
103100
expect(text).toContain("File not found: '/path/to/NonExistent.app'");
104101
});
105102
});

‎src/mcp/tools/simulator-management/__tests__/set_sim_location.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
createNoopExecutor,
77
} from '../../../../test-utils/mock-executors.ts';
88
import { schema, handler, set_sim_locationLogic } from '../set_sim_location.ts';
9-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
9+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
1010

1111
const runLogic = async (logic: () => Promise<unknown>) => {
1212
const { result, run } = createMockToolHandlerContext();
@@ -163,7 +163,7 @@ describe('set_sim_location tool', () => {
163163
),
164164
);
165165

166-
expect(result.content[0]?.text).toContain('Latitude must be between -90 and 90 degrees');
166+
expect(allText(result)).toContain('Latitude must be between -90 and 90 degrees');
167167
expect(result.isError).toBe(true);
168168
});
169169

@@ -179,7 +179,7 @@ describe('set_sim_location tool', () => {
179179
),
180180
);
181181

182-
expect(result.content[0]?.text).toContain('Longitude must be between -180 and 180 degrees');
182+
expect(allText(result)).toContain('Longitude must be between -180 and 180 degrees');
183183
expect(result.isError).toBe(true);
184184
});
185185

‎src/mcp/tools/simulator/__tests__/get_sim_app_path.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { sessionStore } from '../../../../utils/session-store.ts';
77
import { schema, handler, get_sim_app_pathLogic } from '../get_sim_app_path.ts';
88
import type { CommandExecutor } from '../../../../utils/CommandExecutor.ts';
99
import { XcodePlatform } from '../../../../types/common.ts';
10-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
10+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
1111

1212
const runLogic = async (logic: () => Promise<unknown>) => {
1313
const { result, run } = createMockToolHandlerContext();
@@ -193,7 +193,7 @@ describe('get_sim_app_path tool', () => {
193193
]);
194194

195195
expect(result.isError).toBeFalsy();
196-
const text = result.content.map((c) => c.text).join('\n');
196+
const text = allText(result);
197197
expect(text).toContain('Get App Path');
198198
expect(text).toContain('MyScheme');
199199
expect(text).toContain('/path/to/workspace.xcworkspace');
@@ -223,7 +223,7 @@ describe('get_sim_app_path tool', () => {
223223
);
224224

225225
expect(result.isError).toBe(true);
226-
const text = result.content.map((c) => c.text).join('\n');
226+
const text = allText(result);
227227
expect(text).toContain('Get App Path');
228228
expect(text).toContain('MyScheme');
229229
expect(text).toContain('Errors (1):');

‎src/mcp/tools/simulator/__tests__/install_app_sim.test.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
import { sessionStore } from '../../../../utils/session-store.ts';
1010
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
1111
import { schema, handler, install_app_simLogic } from '../install_app_sim.ts';
12-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
12+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
1313

1414
const runLogic = async (logic: () => Promise<unknown>) => {
1515
const { result, run } = createMockToolHandlerContext();
@@ -193,10 +193,7 @@ describe('install_app_sim tool', () => {
193193
);
194194

195195
expect(result.isError).toBe(true);
196-
const text = result.content
197-
.filter((i) => i.type === 'text')
198-
.map((i) => i.text)
199-
.join('\n');
196+
const text = allText(result);
200197
expect(text).toContain("File not found: '/path/to/app.app'");
201198
});
202199

@@ -241,7 +238,7 @@ describe('install_app_sim tool', () => {
241238
),
242239
);
243240

244-
const text = result.content.map((c) => c.text).join('\n');
241+
const text = allText(result);
245242
expect(text).toContain('App installed successfully');
246243
expect(text).toContain('test-uuid-123');
247244
expect(result.nextStepParams).toEqual({
@@ -292,7 +289,7 @@ describe('install_app_sim tool', () => {
292289
),
293290
);
294291

295-
const text = result.content.map((c) => c.text).join('\n');
292+
const text = allText(result);
296293
expect(text).toContain('App installed successfully');
297294
expect(text).toContain('test-uuid-123');
298295
expect(result.nextStepParams).toEqual({
@@ -327,7 +324,7 @@ describe('install_app_sim tool', () => {
327324
),
328325
);
329326

330-
const text = result.content.map((c) => c.text).join('\n');
327+
const text = allText(result);
331328
expect(text).toContain('Install app in simulator operation failed');
332329
expect(text).toContain('Install failed');
333330
expect(result.isError).toBe(true);
@@ -351,7 +348,7 @@ describe('install_app_sim tool', () => {
351348
),
352349
);
353350

354-
const text = result.content.map((c) => c.text).join('\n');
351+
const text = allText(result);
355352
expect(text).toContain('Install app in simulator operation failed');
356353
expect(text).toContain('Command execution failed');
357354
expect(result.isError).toBe(true);
@@ -375,7 +372,7 @@ describe('install_app_sim tool', () => {
375372
),
376373
);
377374

378-
const text = result.content.map((c) => c.text).join('\n');
375+
const text = allText(result);
379376
expect(text).toContain('Install app in simulator operation failed');
380377
expect(text).toContain('String error');
381378
expect(result.isError).toBe(true);

‎src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
88
import { sessionStore } from '../../../../utils/session-store.ts';
99
import { schema, handler, stop_app_simLogic } from '../stop_app_sim.ts';
10-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
10+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
1111

1212
const runLogic = async (logic: () => Promise<unknown>) => {
1313
const { result, run } = createMockToolHandlerContext();
@@ -115,7 +115,7 @@ describe('stop_app_sim tool', () => {
115115
),
116116
);
117117

118-
const text = result.content.map((c) => c.text).join('\n');
118+
const text = allText(result);
119119
expect(text).toContain('Stop App');
120120
expect(text).toContain('io.sentry.App');
121121
expect(text).toContain('stopped successfully');
@@ -136,7 +136,7 @@ describe('stop_app_sim tool', () => {
136136
),
137137
);
138138

139-
const text = result.content.map((c) => c.text).join('\n');
139+
const text = allText(result);
140140
expect(text).toContain('Stop App');
141141
expect(text).toContain('io.sentry.App');
142142
expect(text).toContain('stopped successfully');
@@ -160,7 +160,7 @@ describe('stop_app_sim tool', () => {
160160
),
161161
);
162162

163-
const text = result.content.map((c) => c.text).join('\n');
163+
const text = allText(result);
164164
expect(text).toContain('Stop app in simulator operation failed');
165165
expect(text).toContain('Simulator not found');
166166
expect(result.isError).toBe(true);
@@ -181,7 +181,7 @@ describe('stop_app_sim tool', () => {
181181
),
182182
);
183183

184-
const text = result.content.map((c) => c.text).join('\n');
184+
const text = allText(result);
185185
expect(text).toContain('Stop app in simulator operation failed');
186186
expect(text).toContain('Unexpected error');
187187
expect(result.isError).toBe(true);

‎src/mcp/tools/swift-package/__tests__/swift_package_clean.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
} from '../../../../test-utils/mock-executors.ts';
88
import { schema, handler, swift_package_cleanLogic } from '../swift_package_clean.ts';
99
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
10-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
10+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
1111

1212
const runLogic = async (logic: () => Promise<unknown>) => {
1313
const { result, run } = createMockToolHandlerContext();
@@ -113,7 +113,7 @@ describe('swift_package_clean plugin', () => {
113113
);
114114

115115
expect(result.isError).toBeUndefined();
116-
const text = result.content.map((c) => c.text).join('\n');
116+
const text = allText(result);
117117
expect(text).toContain('Swift package cleaned successfully');
118118
});
119119

@@ -133,7 +133,7 @@ describe('swift_package_clean plugin', () => {
133133
);
134134

135135
expect(result.isError).toBeUndefined();
136-
const text = result.content.map((c) => c.text).join('\n');
136+
const text = allText(result);
137137
expect(text).toContain('Swift Package Clean');
138138
expect(text).toContain('Swift package cleaned successfully');
139139
expect(text).toContain('Package cleaned successfully');
@@ -155,7 +155,7 @@ describe('swift_package_clean plugin', () => {
155155
);
156156

157157
expect(result.isError).toBeUndefined();
158-
const text = result.content.map((c) => c.text).join('\n');
158+
const text = allText(result);
159159
expect(text).toContain('Swift Package Clean');
160160
expect(text).toContain('Swift package cleaned successfully');
161161
});
@@ -176,7 +176,7 @@ describe('swift_package_clean plugin', () => {
176176
);
177177

178178
expect(result.isError).toBe(true);
179-
const text = result.content.map((c) => c.text).join('\n');
179+
const text = allText(result);
180180
expect(text).toContain('Swift package clean failed');
181181
expect(text).toContain('Permission denied');
182182
});
@@ -196,7 +196,7 @@ describe('swift_package_clean plugin', () => {
196196
);
197197

198198
expect(result.isError).toBe(true);
199-
const text = result.content.map((c) => c.text).join('\n');
199+
const text = allText(result);
200200
expect(text).toContain('Failed to execute swift package clean');
201201
expect(text).toContain('spawn ENOENT');
202202
});

‎src/mcp/tools/swift-package/__tests__/swift_package_list.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect } from 'vitest';
22
import { schema, handler, swift_package_listLogic } from '../swift_package_list.ts';
3-
import { createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
3+
import { allText, createMockToolHandlerContext } from '../../../../test-utils/test-helpers.ts';
44

55
const runLogic = async (logic: () => Promise<unknown>) => {
66
const { result, run } = createMockToolHandlerContext();
@@ -61,9 +61,7 @@ describe('swift_package_list plugin', () => {
6161
);
6262

6363
expect(result.isError).toBeUndefined();
64-
expect(result.content.map((c) => c.text).join('\n')).toContain(
65-
'No Swift Package processes currently running',
66-
);
64+
expect(allText(result)).toContain('No Swift Package processes currently running');
6765
});
6866

6967
it('should use default executable name and clamp durations to at least one second', async () => {
@@ -89,7 +87,7 @@ describe('swift_package_list plugin', () => {
8987
);
9088

9189
expect(result.isError).toBeUndefined();
92-
const text = result.content.map((c) => c.text).join('\n');
90+
const text = allText(result);
9391
expect(text).toContain('12345');
9492
expect(text).toContain('default');
9593
expect(text).toContain('/test/package');

0 commit comments

Comments
 (0)