Skip to content

Commit a613b9f

Browse files
committed
feat(simulator): migrate build_sim to session-aware defaults and concise description
docs(session-management): add Tool Description Policy and update plan example test: update build_sim and session-management tests for session-aware preflight and description
1 parent a72960d commit a613b9f

File tree

4 files changed

+115
-149
lines changed

4 files changed

+115
-149
lines changed

docs/session_management_plan.md

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ const publicSchemaObject = baseSchemaObject.omit(
185185

186186
export default {
187187
name: 'build_sim',
188-
description: 'Builds an app for a simulator using session defaults (scheme/project/sim).',
188+
description: 'Builds an app for an iOS simulator.',
189189
schema: publicSchemaObject.shape, // what the MCP client sees
190190
handler: createSessionAwareTool<BuildSimulatorParams>({
191191
internalSchema: buildSimulatorSchema,
@@ -356,11 +356,26 @@ import plugin, { sessionClearDefaultsLogic } from '../session_clear_defaults.ts'
356356
- `npm run lint`
357357
- `npm run test`
358358

359+
### Minimal Changes Policy for Tests (Enforced)
360+
361+
- Only make material, essential edits to tests required by the code change (e.g., new preflight error messages or added/removed fields).
362+
- Do not change sample input values or defaults in tests (e.g., flipping a boolean like `preferXcodebuild`) unless strictly necessary to validate behavior.
363+
- Preserve the original intent and coverage of logic-function tests; keep handler vs logic boundaries intact.
364+
- When session-awareness is added, prefer setting/clearing session defaults around tests rather than altering existing assertions or sample inputs.
365+
366+
### Tool Description Policy (Enforced)
367+
368+
- Keep tool descriptions concise (maximum one short sentence).
369+
- Do not mention session defaults, setup steps, examples, or parameter relationships in descriptions.
370+
- Use clear, imperative phrasing (e.g., "Builds an app for an iOS simulator.").
371+
- Apply consistently across all migrated tools; update any tests that assert `description` to match the concise string only.
372+
359373
## Commit & Review Protocol (Enforced)
360374

361375
At the end of each numbered step above:
362376

363377
1. Ensure all checks pass: `typecheck`, `lint`, `test`.
378+
- Verify tool descriptions comply with the Tool Description Policy (concise, no session-defaults mention).
364379
2. Stage only the files for that step.
365380
3. Prepare a concise commit message focused on the “why”.
366381
4. Request manual review and approval before committing. Do not push.
@@ -408,6 +423,59 @@ Note on commit hooks and selective commits:
408423
- `session-show-defaults {}`
409424
- `session-clear-defaults { "all": true }`
410425

426+
## Manual Testing with mcpli (CLI)
427+
428+
The following commands exercise the session workflow end‑to‑end using the built server.
429+
430+
1) Build the server (required after code changes):
431+
432+
```bash
433+
npm run build
434+
```
435+
436+
2) Discover a scheme (optional helper):
437+
438+
```bash
439+
mcpli --raw list-schemes --projectPath "/Volumes/Developer/XcodeBuildMCP/example_projects/iOS/MCPTest.xcodeproj" -- node build/index.js
440+
```
441+
442+
3) Set the session defaults (project/workspace, scheme, and simulator):
443+
444+
```bash
445+
mcpli --raw session-set-defaults \
446+
--projectPath "/Volumes/Developer/XcodeBuildMCP/example_projects/iOS/MCPTest.xcodeproj" \
447+
--scheme MCPTest \
448+
--simulatorName "iPhone 16" \
449+
-- node build/index.js
450+
```
451+
452+
4) Verify defaults are stored:
453+
454+
```bash
455+
mcpli --raw session-show-defaults -- node build/index.js
456+
```
457+
458+
5) Run a session‑aware tool with zero or minimal args (defaults are merged automatically):
459+
460+
```bash
461+
# Optionally provide a scratch derived data path and a short timeout
462+
mcpli --tool-timeout=60 --raw build-sim --derivedDataPath "/tmp/XBMCP_DD" -- node build/index.js
463+
```
464+
465+
Troubleshooting:
466+
467+
- If you see validation errors like “Missing required session defaults …”, (re)run step 3 with the missing keys.
468+
- If you see connect ECONNREFUSED or the daemon appears flaky:
469+
- Check logs: `mcpli daemon log --since=10m -- node build/index.js`
470+
- Restart daemon: `mcpli daemon restart -- node build/index.js`
471+
- Clean daemon state: `mcpli daemon clean -- node build/index.js` then `mcpli daemon start -- node build/index.js`
472+
- After code changes, always: `npm run build` then `mcpli daemon restart -- node build/index.js`
473+
474+
Notes:
475+
476+
- Public schemas for session‑aware tools intentionally omit session fields (e.g., `scheme`, `projectPath`, `simulatorName`). Provide them once via `session-set-defaults` and then call the tool with zero/minimal flags.
477+
- Use `--tool-timeout=<seconds>` to cap long‑running builds during manual testing.
478+
411479
## Next Steps
412480

413481
Would you like me to proceed with Phase 1–3 implementation (store + session tools + middleware), then migrate a first tool (build_sim) and run the test suite?

src/mcp/tools/session-management/__tests__/index.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ describe('session-management workflow metadata', () => {
1919
});
2020

2121
it('should have correct description', () => {
22-
expect(workflow.description).toBe('Manage session defaults for XcodeBuildMCP tools.');
22+
expect(workflow.description).toBe(
23+
'Manage session defaults for projectPath/workspacePath, scheme, configuration, simulatorName/simulatorId, deviceId, useLatestOS and arch. These defaults are required by many tools and must be set before attempting to call tools that would depend on these values.',
24+
);
2325
});
2426

2527
it('should have correct platforms array', () => {

src/mcp/tools/simulator/__tests__/build_sim.test.ts

Lines changed: 19 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,153 +1,64 @@
11
import { describe, it, expect, beforeEach } from 'vitest';
22
import { z } from 'zod';
33
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
4+
import { sessionStore } from '../../../../utils/session-store.ts';
45

56
// Import the plugin and logic function
67
import buildSim, { build_simLogic } from '../build_sim.ts';
78

89
describe('build_sim tool', () => {
9-
// Only clear any remaining mocks if needed
10+
beforeEach(() => {
11+
sessionStore.clear();
12+
});
1013

1114
describe('Export Field Validation (Literal)', () => {
1215
it('should have correct name', () => {
1316
expect(buildSim.name).toBe('build_sim');
1417
});
1518

1619
it('should have correct description', () => {
17-
expect(buildSim.description).toBe(
18-
"Builds an app from a project or workspace for a specific simulator by UUID or name. Provide exactly one of projectPath or workspacePath, and exactly one of simulatorId or simulatorName. IMPORTANT: Requires either projectPath or workspacePath, plus scheme and either simulatorId or simulatorName. Example: build_sim({ projectPath: '/path/to/MyProject.xcodeproj', scheme: 'MyScheme', simulatorName: 'iPhone 16' })",
19-
);
20+
expect(buildSim.description).toBe('Builds an app for an iOS simulator.');
2021
});
2122

2223
it('should have handler function', () => {
2324
expect(typeof buildSim.handler).toBe('function');
2425
});
2526

26-
it('should have correct schema with required and optional fields', () => {
27+
it('should have correct public schema (only non-session fields)', () => {
2728
const schema = z.object(buildSim.schema);
2829

29-
// Valid inputs - workspace
30-
expect(
31-
schema.safeParse({
32-
workspacePath: '/path/to/workspace',
33-
scheme: 'MyScheme',
34-
simulatorName: 'iPhone 16',
35-
}).success,
36-
).toBe(true);
30+
// Public schema should allow empty input
31+
expect(schema.safeParse({}).success).toBe(true);
3732

38-
// Valid inputs - project
33+
// Valid public inputs
3934
expect(
4035
schema.safeParse({
41-
projectPath: '/path/to/project.xcodeproj',
42-
scheme: 'MyScheme',
43-
simulatorName: 'iPhone 16',
44-
}).success,
45-
).toBe(true);
46-
47-
expect(
48-
schema.safeParse({
49-
workspacePath: '/path/to/workspace',
50-
scheme: 'MyScheme',
51-
simulatorName: 'iPhone 16',
52-
configuration: 'Release',
5336
derivedDataPath: '/path/to/derived',
5437
extraArgs: ['--verbose'],
55-
useLatestOS: true,
5638
preferXcodebuild: false,
5739
}).success,
5840
).toBe(true);
5941

60-
// Invalid inputs - missing required fields
61-
// Note: simulatorId/simulatorName are optional at schema level, XOR validation at runtime
62-
expect(
63-
schema.safeParse({
64-
workspacePath: '/path/to/workspace',
65-
scheme: 'MyScheme',
66-
}).success,
67-
).toBe(true); // Schema validation passes, runtime XOR validation would catch missing simulator fields
68-
69-
expect(
70-
schema.safeParse({
71-
workspacePath: '/path/to/workspace',
72-
simulatorName: 'iPhone 16',
73-
}).success,
74-
).toBe(false);
75-
76-
expect(
77-
schema.safeParse({
78-
scheme: 'MyScheme',
79-
simulatorName: 'iPhone 16',
80-
}).success,
81-
).toBe(true); // Base schema allows both fields optional, XOR validation happens at handler level
82-
83-
// Invalid types
84-
expect(
85-
schema.safeParse({
86-
workspacePath: 123,
87-
scheme: 'MyScheme',
88-
simulatorName: 'iPhone 16',
89-
}).success,
90-
).toBe(false);
91-
92-
expect(
93-
schema.safeParse({
94-
workspacePath: '/path/to/workspace',
95-
scheme: 123,
96-
simulatorName: 'iPhone 16',
97-
}).success,
98-
).toBe(false);
99-
100-
expect(
101-
schema.safeParse({
102-
workspacePath: '/path/to/workspace',
103-
scheme: 'MyScheme',
104-
simulatorName: 123,
105-
}).success,
106-
).toBe(false);
107-
});
108-
109-
it('should validate XOR constraint between projectPath and workspacePath', () => {
110-
const schema = z.object(buildSim.schema);
111-
112-
// Both projectPath and workspacePath provided - should be invalid
113-
expect(
114-
schema.safeParse({
115-
projectPath: '/path/to/project.xcodeproj',
116-
workspacePath: '/path/to/workspace',
117-
scheme: 'MyScheme',
118-
simulatorName: 'iPhone 16',
119-
}).success,
120-
).toBe(true); // Schema validation passes, but handler validation will catch this
121-
122-
// Neither provided - should be invalid
123-
expect(
124-
schema.safeParse({
125-
scheme: 'MyScheme',
126-
simulatorName: 'iPhone 16',
127-
}).success,
128-
).toBe(true); // Schema validation passes, but handler validation will catch this
42+
// Invalid types on public inputs
43+
expect(schema.safeParse({ derivedDataPath: 123 }).success).toBe(false);
44+
expect(schema.safeParse({ extraArgs: [123] }).success).toBe(false);
45+
expect(schema.safeParse({ preferXcodebuild: 'yes' }).success).toBe(false);
12946
});
13047
});
13148

13249
describe('Parameter Validation', () => {
13350
it('should handle missing both projectPath and workspacePath', async () => {
134-
const mockExecutor = createMockExecutor({ success: true, output: 'Build succeeded' });
135-
136-
// Since we use XOR validation, this should fail at the handler level
13751
const result = await buildSim.handler({
13852
scheme: 'MyScheme',
13953
simulatorName: 'iPhone 16',
14054
});
14155

14256
expect(result.isError).toBe(true);
143-
expect(result.content[0].text).toContain('Parameter validation failed');
144-
expect(result.content[0].text).toContain('Either projectPath or workspacePath is required');
57+
expect(result.content[0].text).toContain('Missing required session defaults');
58+
expect(result.content[0].text).toContain('Provide a project or workspace');
14559
});
14660

14761
it('should handle both projectPath and workspacePath provided', async () => {
148-
const mockExecutor = createMockExecutor({ success: true, output: 'Build succeeded' });
149-
150-
// Since we use XOR validation, this should fail at the handler level
15162
const result = await buildSim.handler({
15263
projectPath: '/path/to/project.xcodeproj',
15364
workspacePath: '/path/to/workspace',
@@ -188,19 +99,14 @@ describe('build_sim tool', () => {
18899
});
189100

190101
it('should handle missing scheme parameter', async () => {
191-
const mockExecutor = createMockExecutor({ success: true, output: 'Build succeeded' });
192-
193-
// Since we removed manual validation, this test now checks that Zod validation works
194-
// by testing the typed tool handler through the default export
195102
const result = await buildSim.handler({
196103
workspacePath: '/path/to/workspace',
197104
simulatorName: 'iPhone 16',
198105
});
199106

200107
expect(result.isError).toBe(true);
201-
expect(result.content[0].text).toContain('Parameter validation failed');
202-
expect(result.content[0].text).toContain('scheme');
203-
expect(result.content[0].text).toContain('Required');
108+
expect(result.content[0].text).toContain('Missing required session defaults');
109+
expect(result.content[0].text).toContain('scheme is required');
204110
});
205111

206112
it('should handle empty scheme parameter', async () => {
@@ -229,17 +135,14 @@ describe('build_sim tool', () => {
229135
});
230136

231137
it('should handle missing both simulatorId and simulatorName', async () => {
232-
const mockExecutor = createMockExecutor({ success: true, output: 'Build succeeded' });
233-
234-
// Should fail with XOR validation
235138
const result = await buildSim.handler({
236139
workspacePath: '/path/to/workspace',
237140
scheme: 'MyScheme',
238141
});
239142

240143
expect(result.isError).toBe(true);
241-
expect(result.content[0].text).toContain('Parameter validation failed');
242-
expect(result.content[0].text).toContain('Either simulatorId or simulatorName is required');
144+
expect(result.content[0].text).toContain('Missing required session defaults');
145+
expect(result.content[0].text).toContain('Provide simulatorId or simulatorName');
243146
});
244147

245148
it('should handle both simulatorId and simulatorName provided', async () => {

src/mcp/tools/simulator/build_sim.ts

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { executeXcodeBuildCommand } from '../../../utils/build/index.ts';
1212
import { ToolResponse, XcodePlatform } from '../../../types/common.ts';
1313
import type { CommandExecutor } from '../../../utils/execution/index.ts';
1414
import { getDefaultCommandExecutor } from '../../../utils/execution/index.ts';
15+
import { createSessionAwareTool } from '../../../utils/typed-tool-factory.ts';
1516
import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts';
1617

1718
// Unified schema: XOR between projectPath and workspacePath, and XOR between simulatorId and simulatorName
@@ -135,37 +136,29 @@ export async function build_simLogic(
135136
return _handleSimulatorBuildLogic(processedParams, executor);
136137
}
137138

139+
// Public schema = internal minus session-managed fields
140+
const publicSchemaObject = baseSchemaObject.omit({
141+
projectPath: true,
142+
workspacePath: true,
143+
scheme: true,
144+
configuration: true,
145+
simulatorId: true,
146+
simulatorName: true,
147+
useLatestOS: true,
148+
} as const);
149+
138150
export default {
139151
name: 'build_sim',
140-
description:
141-
"Builds an app from a project or workspace for a specific simulator by UUID or name. Provide exactly one of projectPath or workspacePath, and exactly one of simulatorId or simulatorName. IMPORTANT: Requires either projectPath or workspacePath, plus scheme and either simulatorId or simulatorName. Example: build_sim({ projectPath: '/path/to/MyProject.xcodeproj', scheme: 'MyScheme', simulatorName: 'iPhone 16' })",
142-
schema: baseSchemaObject.shape, // MCP SDK compatibility
143-
handler: async (args: Record<string, unknown>): Promise<ToolResponse> => {
144-
try {
145-
// Runtime validation with XOR constraints
146-
const validatedParams = buildSimulatorSchema.parse(args);
147-
return await build_simLogic(validatedParams, getDefaultCommandExecutor());
148-
} catch (error) {
149-
if (error instanceof z.ZodError) {
150-
// Format validation errors in a user-friendly way
151-
const errorMessages = error.errors.map((e) => {
152-
const path = e.path.length > 0 ? `${e.path.join('.')}` : 'root';
153-
return `${path}: ${e.message}`;
154-
});
155-
156-
return {
157-
content: [
158-
{
159-
type: 'text',
160-
text: `Parameter validation failed. Invalid parameters:\n${errorMessages.join('\n')}`,
161-
},
162-
],
163-
isError: true,
164-
};
165-
}
166-
167-
// Re-throw unexpected errors
168-
throw error;
169-
}
170-
},
152+
description: 'Builds an app for an iOS simulator.',
153+
schema: publicSchemaObject.shape, // MCP SDK compatibility (public inputs only)
154+
handler: createSessionAwareTool<BuildSimulatorParams>({
155+
internalSchema: buildSimulatorSchema as unknown as z.ZodType<BuildSimulatorParams>,
156+
logicFunction: build_simLogic,
157+
getExecutor: getDefaultCommandExecutor,
158+
requirements: [
159+
{ allOf: ['scheme'], message: 'scheme is required' },
160+
{ oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' },
161+
{ oneOf: ['simulatorId', 'simulatorName'], message: 'Provide simulatorId or simulatorName' },
162+
],
163+
}),
171164
};

0 commit comments

Comments
 (0)