Skip to content

Commit dac75d6

Browse files
cameroncookecodex
andcommitted
ref(tests): Remove direct handler fallback path
Require tool handlers to receive an explicit ToolHandlerContext instead of supporting a no-context test response path. Update unit tests to use the shared callHandler helper so tests provide context explicitly. Remove the env-driven MCP next-step fallback test because MCP output should be verified through the MCP boundary, not by forcing runtime state in handler scaffolding. Document the no-fallback rule in AGENTS.md and CLAUDE.md. Co-Authored-By: Codex <codex@openai.com>
1 parent 48c9078 commit dac75d6

50 files changed

Lines changed: 255 additions & 359 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ ESM TypeScript project (`type: module`). Key layers:
4747
- **NEVER use inline imports** - no `await import("./foo.js")`, no `import("pkg").Type` in type positions, no dynamic imports for types. Always use standard top-level imports.
4848
- NEVER remove or downgrade code to fix type errors from outdated dependencies; upgrade the dependency instead
4949
- Always ask before removing functionality or code that appears to be intentional
50+
- Do not add fallback behavior by default. If required context, configuration, runtime state, or dependencies are missing, fail loudly and fix the caller/setup instead of silently switching to an alternate path. Add a fallback only when explicitly requested or when it is a documented product requirement.
5051
- Follow TypeScript best practices
5152

5253
## Import Conventions

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- **NEVER use inline imports** - no `await import("./foo.js")`, no `import("pkg").Type` in type positions, no dynamic imports for types. Always use standard top-level imports.
77
- NEVER remove or downgrade code to fix type errors from outdated dependencies; upgrade the dependency instead
88
- Always ask before removing functionality or code that appears to be intentional
9+
- Do not add fallback behavior by default. If required context, configuration, runtime state, or dependencies are missing, fail loudly and fix the caller/setup instead of silently switching to an alternate path. Add a fallback only when explicitly requested or when it is a documented product requirement.
910
- Follow TypeScript best practices
1011

1112
## Commands

src/mcp/tools/coverage/__tests__/get_coverage_report.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
get_coverage_reportLogic,
1212
createGetCoverageReportExecutor,
1313
} from '../get_coverage_report.ts';
14-
import { allText, runLogic, runToolLogic } from '../../../../test-utils/test-helpers.ts';
14+
import { allText, runLogic, runToolLogic, callHandler} from '../../../../test-utils/test-helpers.ts';
1515

1616

1717

@@ -99,7 +99,7 @@ describe('get_coverage_report', () => {
9999
__setTestCommandExecutorOverride(mockExecutor);
100100
__setTestFileSystemExecutorOverride(missingFs);
101101

102-
const result = await handler({ xcresultPath: '/tmp/missing.xcresult', showFiles: false });
102+
const result = await callHandler(handler, { xcresultPath: '/tmp/missing.xcresult', showFiles: false });
103103

104104
expect(result.isError).toBe(true);
105105
const text = allText(result);
@@ -120,7 +120,7 @@ describe('get_coverage_report', () => {
120120
__setTestCommandExecutorOverride(mockExecutor);
121121
__setTestFileSystemExecutorOverride(existingFs);
122122

123-
const result = await handler({ xcresultPath: '/tmp/test.xcresult' });
123+
const result = await callHandler(handler, { xcresultPath: '/tmp/test.xcresult' });
124124

125125
expect(result.isError).toBeUndefined();
126126
expect(commands).toHaveLength(1);

src/mcp/tools/coverage/__tests__/get_file_coverage.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
get_file_coverageLogic,
1616
createGetFileCoverageExecutor,
1717
} from '../get_file_coverage.ts';
18-
import { allText, runLogic, runToolLogic } from '../../../../test-utils/test-helpers.ts';
18+
import { allText, runLogic, runToolLogic, callHandler} from '../../../../test-utils/test-helpers.ts';
1919

2020

2121

@@ -98,7 +98,7 @@ describe('get_file_coverage', () => {
9898
__setTestCommandExecutorOverride(mockExecutor);
9999
__setTestFileSystemExecutorOverride(missingFs);
100100

101-
const result = await handler({
101+
const result = await callHandler(handler, {
102102
xcresultPath: '/tmp/missing.xcresult',
103103
file: 'ViewModel.swift',
104104
showLines: false,
@@ -123,7 +123,7 @@ describe('get_file_coverage', () => {
123123
__setTestCommandExecutorOverride(mockExecutor);
124124
__setTestFileSystemExecutorOverride(existingFs);
125125

126-
const result = await handler({ xcresultPath: '/tmp/test.xcresult', file: 'ViewModel.swift' });
126+
const result = await callHandler(handler, { xcresultPath: '/tmp/test.xcresult', file: 'ViewModel.swift' });
127127

128128
expect(result.isError).toBeUndefined();
129129
expect(commands).toHaveLength(1);

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ import {
5050
handler as variablesHandler,
5151
debug_variablesLogic,
5252
} from '../debug_variables.ts';
53-
import { allText, runLogic, runToolLogic } from '../../../../test-utils/test-helpers.ts';
53+
import {
54+
allText,
55+
runLogic,
56+
runToolLogic,
57+
callHandler,
58+
} from '../../../../test-utils/test-helpers.ts';
5459

5560
function createMockBackend(overrides: Partial<DebuggerBackend> = {}): DebuggerBackend {
5661
return {
@@ -125,7 +130,7 @@ describe('debug_attach_sim', () => {
125130

126131
describe('Handler Requirements', () => {
127132
it('should return error when no session defaults for simulator', async () => {
128-
const result = await attachHandler({
133+
const result = await callHandler(attachHandler, {
129134
bundleId: 'com.test.app',
130135
});
131136

@@ -143,7 +148,7 @@ describe('debug_attach_sim', () => {
143148
bundleId: 'com.default.app',
144149
});
145150

146-
const result = await attachHandler({ pid: 1234 });
151+
const result = await callHandler(attachHandler, { pid: 1234 });
147152

148153
expect(result.isError).toBeFalsy();
149154
const text = result.content[0].text;
@@ -155,7 +160,7 @@ describe('debug_attach_sim', () => {
155160
__setTestDebuggerToolContextOverride(createTestContext());
156161
sessionStore.setDefaults({ simulatorId: 'test-sim-uuid' });
157162

158-
const result = await attachHandler({
163+
const result = await callHandler(attachHandler, {
159164
pid: 1234,
160165
bundleId: 'com.test.app',
161166
});

src/mcp/tools/device/__tests__/build_device.test.ts

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@ import { describe, it, expect, beforeEach } from 'vitest';
22
import { computeScopedDerivedDataPath } from '../../../../utils/derived-data-path.ts';
33
import * as z from 'zod';
44
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
5-
import { expectPendingBuildResponse, runToolLogic } from '../../../../test-utils/test-helpers.ts';
5+
import {
6+
expectPendingBuildResponse,
7+
runToolLogic,
8+
callHandler,
9+
} from '../../../../test-utils/test-helpers.ts';
610
import { schema, handler, buildDeviceLogic } from '../build_device.ts';
711
import { sessionStore } from '../../../../utils/session-store.ts';
8-
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
9-
10-
const runHandlerWithExecutor = handler as unknown as (
11-
args: Record<string, unknown>,
12-
executor: CommandExecutor,
13-
) => Promise<{ isError?: boolean }>;
14-
1512
function createSpyExecutor(): {
1613
commandCalls: Array<{ args: string[]; logPrefix?: string }>;
1714
executor: ReturnType<typeof createMockExecutor>;
@@ -58,7 +55,7 @@ describe('build_device plugin', () => {
5855

5956
describe('XOR Validation', () => {
6057
it('should error when neither projectPath nor workspacePath provided', async () => {
61-
const result = await handler({
58+
const result = await callHandler(handler, {
6259
scheme: 'MyScheme',
6360
});
6461

@@ -68,7 +65,7 @@ describe('build_device plugin', () => {
6865
});
6966

7067
it('should error when both projectPath and workspacePath provided', async () => {
71-
const result = await handler({
68+
const result = await callHandler(handler, {
7269
projectPath: '/path/to/MyProject.xcodeproj',
7370
workspacePath: '/path/to/MyProject.xcworkspace',
7471
scheme: 'MyScheme',
@@ -82,7 +79,7 @@ describe('build_device plugin', () => {
8279

8380
describe('Parameter Validation (via Handler)', () => {
8481
it('should return Zod validation error for missing scheme', async () => {
85-
const result = await handler({
82+
const result = await callHandler(handler, {
8683
projectPath: '/path/to/MyProject.xcodeproj',
8784
});
8885

@@ -92,7 +89,7 @@ describe('build_device plugin', () => {
9289
});
9390

9491
it('should return Zod validation error for invalid parameter types', async () => {
95-
const result = await handler({
92+
const result = await callHandler(handler, {
9693
projectPath: 123, // Should be string
9794
scheme: 'MyScheme',
9895
});
@@ -220,26 +217,18 @@ describe('build_device plugin', () => {
220217
scheme: 'MyScheme',
221218
});
222219

223-
const result = await runHandlerWithExecutor({ platform: 'tvOS' }, spy.executor);
224-
225-
expect(result.isError).toBeUndefined();
226-
expect(spy.commandCalls).toHaveLength(1);
227-
expect(spy.commandCalls[0].args).toContain('generic/platform=tvOS');
228-
expect(spy.commandCalls[0].logPrefix).toBe('tvOS Device Build');
229-
});
230-
231-
it('should normalize simulator session platforms for device builds', async () => {
232-
const spy = createSpyExecutor();
233-
234-
sessionStore.setDefaults({
235-
projectPath: '/path/to/MyProject.xcodeproj',
236-
scheme: 'MyScheme',
237-
platform: 'tvOS Simulator',
238-
});
239-
240-
const result = await runHandlerWithExecutor({}, spy.executor);
220+
const { result } = await runToolLogic(() =>
221+
buildDeviceLogic(
222+
{
223+
projectPath: '/path/to/MyProject.xcodeproj',
224+
scheme: 'MyScheme',
225+
platform: 'tvOS',
226+
},
227+
spy.executor,
228+
),
229+
);
241230

242-
expect(result.isError).toBeUndefined();
231+
expect(result.isError()).toBe(false);
243232
expect(spy.commandCalls).toHaveLength(1);
244233
expect(spy.commandCalls[0].args).toContain('generic/platform=tvOS');
245234
expect(spy.commandCalls[0].logPrefix).toBe('tvOS Device Build');

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import {
55
createMockFileSystemExecutor,
66
createMockExecutor,
77
} from '../../../../test-utils/mock-executors.ts';
8-
import { runToolLogic, type MockToolHandlerResult } from '../../../../test-utils/test-helpers.ts';
8+
import {
9+
runToolLogic,
10+
type MockToolHandlerResult,
11+
callHandler,
12+
} from '../../../../test-utils/test-helpers.ts';
913
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
1014
import { sessionStore } from '../../../../utils/session-store.ts';
1115
import { schema, handler, build_run_deviceLogic } from '../build_run_device.ts';
@@ -50,11 +54,14 @@ describe('build_run_device tool', () => {
5054

5155
describe('Handler Requirements', () => {
5256
it('requires scheme + deviceId and project/workspace via handler', async () => {
53-
const missingAll = await handler({});
57+
const missingAll = await callHandler(handler, {});
5458
expect(missingAll.isError).toBe(true);
5559
expect(missingAll.content[0].text).toContain('Provide scheme and deviceId');
5660

57-
const missingSource = await handler({ scheme: 'MyApp', deviceId: 'DEVICE-UDID' });
61+
const missingSource = await callHandler(handler, {
62+
scheme: 'MyApp',
63+
deviceId: 'DEVICE-UDID',
64+
});
5865
expect(missingSource.isError).toBe(true);
5966
expect(missingSource.content[0].text).toContain('Provide a project or workspace');
6067
});

src/mcp/tools/device/__tests__/get_device_app_path.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
createGetDeviceAppPathExecutor,
1313
} from '../get_device_app_path.ts';
1414
import { sessionStore } from '../../../../utils/session-store.ts';
15-
import { runLogic } from '../../../../test-utils/test-helpers.ts';
15+
import { runLogic, callHandler } from '../../../../test-utils/test-helpers.ts';
1616

1717
describe('get_device_app_path plugin', () => {
1818
beforeEach(() => {
@@ -41,7 +41,7 @@ describe('get_device_app_path plugin', () => {
4141

4242
describe('XOR Validation', () => {
4343
it('should error when neither projectPath nor workspacePath provided', async () => {
44-
const result = await handler({
44+
const result = await callHandler(handler, {
4545
scheme: 'MyScheme',
4646
});
4747
expect(result.isError).toBe(true);
@@ -50,7 +50,7 @@ describe('get_device_app_path plugin', () => {
5050
});
5151

5252
it('should error when both projectPath and workspacePath provided', async () => {
53-
const result = await handler({
53+
const result = await callHandler(handler, {
5454
projectPath: '/path/to/project.xcodeproj',
5555
workspacePath: '/path/to/workspace.xcworkspace',
5656
scheme: 'MyScheme',
@@ -63,7 +63,7 @@ describe('get_device_app_path plugin', () => {
6363

6464
describe('Handler Requirements', () => {
6565
it('should require scheme when missing', async () => {
66-
const result = await handler({
66+
const result = await callHandler(handler, {
6767
projectPath: '/path/to/project.xcodeproj',
6868
});
6969
expect(result.isError).toBe(true);
@@ -74,7 +74,7 @@ describe('get_device_app_path plugin', () => {
7474
it('should require project or workspace when scheme default exists', async () => {
7575
sessionStore.setDefaults({ scheme: 'MyScheme' });
7676

77-
const result = await handler({});
77+
const result = await callHandler(handler, {});
7878
expect(result.isError).toBe(true);
7979
expect(result.content[0].text).toContain('Provide a project or workspace');
8080
});

src/mcp/tools/device/__tests__/install_app_device.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as z from 'zod';
33
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
44
import { schema, handler, install_app_deviceLogic } from '../install_app_device.ts';
55
import { sessionStore } from '../../../../utils/session-store.ts';
6-
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
6+
import { allText, runLogic, callHandler } from '../../../../test-utils/test-helpers.ts';
77

88
describe('install_app_device plugin', () => {
99
beforeEach(() => {
@@ -12,7 +12,7 @@ describe('install_app_device plugin', () => {
1212

1313
describe('Handler Requirements', () => {
1414
it('should require deviceId when session defaults are missing', async () => {
15-
const result = await handler({
15+
const result = await callHandler(handler, {
1616
appPath: '/path/to/test.app',
1717
});
1818

src/mcp/tools/device/__tests__/launch_app_device.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
} from '../../../../test-utils/mock-executors.ts';
77
import { schema, handler, launch_app_deviceLogic } from '../launch_app_device.ts';
88
import { sessionStore } from '../../../../utils/session-store.ts';
9-
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
9+
import { allText, runLogic, callHandler } from '../../../../test-utils/test-helpers.ts';
1010

1111
describe('launch_app_device plugin (device-shared)', () => {
1212
beforeEach(() => {
@@ -36,7 +36,7 @@ describe('launch_app_device plugin (device-shared)', () => {
3636

3737
describe('Handler Requirements', () => {
3838
it('should require deviceId and bundleId when not provided', async () => {
39-
const result = await handler({});
39+
const result = await callHandler(handler, {});
4040

4141
expect(result.isError).toBe(true);
4242
expect(result.content[0].text).toContain('Missing required session defaults');

0 commit comments

Comments
 (0)