Skip to content

Commit 140c26f

Browse files
meqtMacclaude
andcommitted
fix(simctl): Address PR review feedback
- Fix publicSchemaObject to not omit non-session-default params - Capture new simulator UDID from simctl clone/create stdout - Allow shutdownFirst for delete_sims with target=all - Add tests for clone, create, and delete tools Co-Authored-By: deepseek v4 pro <noreply@anthropic.com>
1 parent e3b5dce commit 140c26f

6 files changed

Lines changed: 235 additions & 29 deletions

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { schema, clone_simsLogic } from '../clone_sims.ts';
3+
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
4+
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
5+
6+
describe('clone_sims tool', () => {
7+
describe('Plugin Structure', () => {
8+
it('should expose schema', () => {
9+
expect(schema).toBeDefined();
10+
});
11+
});
12+
13+
describe('Happy path', () => {
14+
it('clones a simulator and captures the new UDID', async () => {
15+
const newUdid = '00000000-0000-0000-0000-000000000001';
16+
const mock = createMockExecutor({ success: true, output: `${newUdid}\n` });
17+
const res = await runLogic(() =>
18+
clone_simsLogic({ sourceSimulatorId: '00000000-0000-0000-0000-000000000000' }, mock),
19+
);
20+
expect(res.isError).toBeFalsy();
21+
const text = allText(res);
22+
expect(text).toContain('Simulator cloned successfully');
23+
});
24+
25+
it('clones with a custom name', async () => {
26+
const mock = createMockExecutor({ success: true, output: 'UUID1\n' });
27+
const res = await runLogic(() =>
28+
clone_simsLogic(
29+
{
30+
sourceSimulatorId: '00000000-0000-0000-0000-000000000000',
31+
newName: 'My Clone',
32+
},
33+
mock,
34+
),
35+
);
36+
expect(res.isError).toBeFalsy();
37+
});
38+
});
39+
40+
describe('Failure path', () => {
41+
it('returns failure when clone fails', async () => {
42+
const mock = createMockExecutor({ success: false, error: 'No such device' });
43+
const res = await runLogic(() =>
44+
clone_simsLogic({ sourceSimulatorId: '00000000-0000-0000-0000-000000000000' }, mock),
45+
);
46+
expect(res.isError).toBe(true);
47+
const text = allText(res);
48+
expect(text).toContain('Clone simulator failed');
49+
expect(text).toContain('No such device');
50+
});
51+
});
52+
});
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { schema, create_simLogic } from '../create_sim.ts';
3+
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
4+
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
5+
6+
describe('create_sim tool', () => {
7+
describe('Plugin Structure', () => {
8+
it('should expose schema', () => {
9+
expect(schema).toBeDefined();
10+
});
11+
});
12+
13+
describe('Happy path', () => {
14+
it('creates a simulator and captures the new UDID', async () => {
15+
const newUdid = '00000000-0000-0000-0000-000000000001';
16+
const mock = createMockExecutor({ success: true, output: `${newUdid}\n` });
17+
const res = await runLogic(() =>
18+
create_simLogic(
19+
{
20+
name: 'Test Sim',
21+
deviceType: 'iPhone 17',
22+
runtime: 'iOS 26.4',
23+
},
24+
mock,
25+
),
26+
);
27+
expect(res.isError).toBeFalsy();
28+
const text = allText(res);
29+
expect(text).toContain('Simulator created successfully');
30+
});
31+
});
32+
33+
describe('Failure path', () => {
34+
it('returns failure when create fails', async () => {
35+
const mock = createMockExecutor({ success: false, error: 'Invalid device type' });
36+
const res = await runLogic(() =>
37+
create_simLogic(
38+
{
39+
name: 'Bad Sim',
40+
deviceType: 'NonExistent',
41+
runtime: 'iOS 99',
42+
},
43+
mock,
44+
),
45+
);
46+
expect(res.isError).toBe(true);
47+
const text = allText(res);
48+
expect(text).toContain('Create simulator failed');
49+
expect(text).toContain('Invalid device type');
50+
});
51+
});
52+
});
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { schema, delete_simsLogic } from '../delete_sims.ts';
3+
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
4+
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
5+
6+
describe('delete_sims tool', () => {
7+
describe('Plugin Structure', () => {
8+
it('should expose schema', () => {
9+
expect(schema).toBeDefined();
10+
});
11+
});
12+
13+
describe('Happy path', () => {
14+
it('deletes a simulator by UDID', async () => {
15+
const mock = createMockExecutor({ success: true, output: 'OK' });
16+
const res = await runLogic(() =>
17+
delete_simsLogic({ target: '00000000-0000-0000-0000-000000000000' }, mock),
18+
);
19+
expect(res.isError).toBeFalsy();
20+
const text = allText(res);
21+
expect(text).toContain('Simulator(s) deleted successfully');
22+
});
23+
24+
it('deletes all simulators', async () => {
25+
const mock = createMockExecutor({ success: true, output: 'OK' });
26+
const res = await runLogic(() => delete_simsLogic({ target: 'all' }, mock));
27+
expect(res.isError).toBeFalsy();
28+
});
29+
30+
it('deletes unavailable simulators', async () => {
31+
const mock = createMockExecutor({ success: true, output: 'OK' });
32+
const res = await runLogic(() => delete_simsLogic({ target: 'unavailable' }, mock));
33+
expect(res.isError).toBeFalsy();
34+
});
35+
});
36+
37+
describe('Shutdown first', () => {
38+
it('shuts down before deleting when shutdownFirst=true', async () => {
39+
const calls: any[] = [];
40+
const exec = async (cmd: string[]) => {
41+
calls.push(cmd);
42+
return { success: true, output: 'OK', error: '', process: { pid: 1 } as any };
43+
};
44+
const res = await runLogic(() =>
45+
delete_simsLogic(
46+
{ target: '00000000-0000-0000-0000-000000000000', shutdownFirst: true },
47+
exec as any,
48+
),
49+
);
50+
expect(calls).toEqual([
51+
['xcrun', 'simctl', 'shutdown', '00000000-0000-0000-0000-000000000000'],
52+
['xcrun', 'simctl', 'delete', '00000000-0000-0000-0000-000000000000'],
53+
]);
54+
expect(res.isError).toBeFalsy();
55+
});
56+
57+
it('shuts down all before deleting when target=all', async () => {
58+
const calls: any[] = [];
59+
const exec = async (cmd: string[]) => {
60+
calls.push(cmd);
61+
return { success: true, output: 'OK', error: '', process: { pid: 1 } as any };
62+
};
63+
const res = await runLogic(() =>
64+
delete_simsLogic({ target: 'all', shutdownFirst: true }, exec as any),
65+
);
66+
expect(calls).toEqual([
67+
['xcrun', 'simctl', 'shutdown', 'all'],
68+
['xcrun', 'simctl', 'delete', 'all'],
69+
]);
70+
expect(res.isError).toBeFalsy();
71+
});
72+
73+
it('skips shutdown when target=unavailable', async () => {
74+
const calls: any[] = [];
75+
const exec = async (cmd: string[]) => {
76+
calls.push(cmd);
77+
return { success: true, output: 'OK', error: '', process: { pid: 1 } as any };
78+
};
79+
const res = await runLogic(() =>
80+
delete_simsLogic({ target: 'unavailable', shutdownFirst: true }, exec as any),
81+
);
82+
expect(calls).toEqual([['xcrun', 'simctl', 'delete', 'unavailable']]);
83+
expect(res.isError).toBeFalsy();
84+
});
85+
86+
it('does not shut down when shutdownFirst is not set', async () => {
87+
const calls: any[] = [];
88+
const exec = async (cmd: string[]) => {
89+
calls.push(cmd);
90+
return { success: true, output: 'OK', error: '', process: { pid: 1 } as any };
91+
};
92+
await runLogic(() =>
93+
delete_simsLogic({ target: '00000000-0000-0000-0000-000000000000' }, exec as any),
94+
);
95+
expect(calls).toEqual([
96+
['xcrun', 'simctl', 'delete', '00000000-0000-0000-0000-000000000000'],
97+
]);
98+
});
99+
});
100+
101+
describe('Failure path', () => {
102+
it('returns failure when delete fails', async () => {
103+
const mock = createMockExecutor({ success: false, error: 'Unable to delete' });
104+
const res = await runLogic(() =>
105+
delete_simsLogic({ target: '00000000-0000-0000-0000-000000000000' }, mock),
106+
);
107+
expect(res.isError).toBe(true);
108+
const text = allText(res);
109+
expect(text).toContain('Failed to delete simulator');
110+
expect(text).toContain('Unable to delete');
111+
});
112+
});
113+
});

src/mcp/tools/simulator-management/clone_sims.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,9 @@ const internalSchemaObject = z.object({
3030
type CloneSimsParams = z.infer<typeof internalSchemaObject>;
3131
type CloneSimsResult = SimulatorActionResultDomainResult;
3232

33-
const publicSchemaObject = z.strictObject(
34-
baseSchemaObject.omit({
35-
sourceSimulatorId: true,
36-
newName: true,
37-
} as const).shape,
38-
);
39-
4033
function createCloneSimsResult(params: {
4134
sourceSimulatorId: string;
35+
clonedSimulatorId?: string;
4236
didError: boolean;
4337
error?: string;
4438
diagnosticMessage?: string;
@@ -58,7 +52,7 @@ function createCloneSimsResult(params: {
5852
? { diagnostics: createBasicDiagnostics({ errors: [params.diagnosticMessage] }) }
5953
: {}),
6054
artifacts: {
61-
simulatorId: params.sourceSimulatorId,
55+
simulatorId: params.clonedSimulatorId ?? params.sourceSimulatorId,
6256
},
6357
};
6458
}
@@ -93,8 +87,10 @@ export function createCloneSimsExecutor(
9387
});
9488
}
9589

90+
const clonedSimulatorId = result.output.trim();
9691
return createCloneSimsResult({
9792
sourceSimulatorId: params.sourceSimulatorId,
93+
clonedSimulatorId,
9894
didError: false,
9995
});
10096
} catch (error) {
@@ -128,15 +124,18 @@ export async function clone_simsLogic(
128124
return;
129125
}
130126

127+
const newSimulatorId = result.artifacts?.simulatorId ?? params.sourceSimulatorId;
131128
ctx.nextStepParams = {
132-
boot_sim: { simulatorId: params.sourceSimulatorId },
129+
boot_sim: { simulatorId: newSimulatorId },
133130
open_sim: {},
131+
install_app_sim: { simulatorId: newSimulatorId, appPath: 'PATH_TO_YOUR_APP' },
132+
launch_app_sim: { simulatorId: newSimulatorId, bundleId: 'YOUR_APP_BUNDLE_ID' },
134133
list_sims: {},
135134
};
136135
}
137136

138137
export const schema = getSessionAwareToolSchemaShape({
139-
sessionAware: publicSchemaObject,
138+
sessionAware: baseSchemaObject,
140139
legacy: baseSchemaObject,
141140
});
142141

src/mcp/tools/simulator-management/create_sim.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,11 @@ const internalSchemaObject = z.object({
3939
type CreateSimParams = z.infer<typeof internalSchemaObject>;
4040
type CreateSimResult = SimulatorActionResultDomainResult;
4141

42-
const publicSchemaObject = z.strictObject(
43-
baseSchemaObject.omit({
44-
name: true,
45-
deviceType: true,
46-
runtime: true,
47-
} as const).shape,
48-
);
49-
5042
function createCreateSimResult(params: {
5143
name: string;
5244
deviceType: string;
5345
runtime: string;
46+
simulatorId?: string;
5447
didError: boolean;
5548
error?: string;
5649
diagnosticMessage?: string;
@@ -71,6 +64,7 @@ function createCreateSimResult(params: {
7164
...(params.diagnosticMessage
7265
? { diagnostics: createBasicDiagnostics({ errors: [params.diagnosticMessage] }) }
7366
: {}),
67+
...(params.simulatorId ? { artifacts: { simulatorId: params.simulatorId } } : {}),
7468
};
7569
}
7670

@@ -105,10 +99,12 @@ export function createCreateSimExecutor(
10599
});
106100
}
107101

102+
const simulatorId = result.output.trim();
108103
return createCreateSimResult({
109104
name: params.name,
110105
deviceType: params.deviceType,
111106
runtime: params.runtime,
107+
simulatorId,
112108
didError: false,
113109
});
114110
} catch (error) {
@@ -144,15 +140,16 @@ export async function create_simLogic(
144140
return;
145141
}
146142

143+
const newSimulatorId = result.artifacts?.simulatorId;
147144
ctx.nextStepParams = {
148-
boot_sim: {},
145+
boot_sim: newSimulatorId ? { simulatorId: newSimulatorId } : {},
149146
open_sim: {},
150147
list_sims: {},
151148
};
152149
}
153150

154151
export const schema = getSessionAwareToolSchemaShape({
155-
sessionAware: publicSchemaObject,
152+
sessionAware: baseSchemaObject,
156153
legacy: baseSchemaObject,
157154
});
158155

src/mcp/tools/simulator-management/delete_sims.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,6 @@ const internalSchemaObject = z.object({
3535
type DeleteSimsParams = z.infer<typeof internalSchemaObject>;
3636
type DeleteSimsResult = SimulatorActionResultDomainResult;
3737

38-
const publicSchemaObject = z.strictObject(
39-
baseSchemaObject.omit({
40-
target: true,
41-
shutdownFirst: true,
42-
} as const).shape,
43-
);
44-
4538
function createDeleteSimsResult(params: {
4639
target: string;
4740
didError: boolean;
@@ -80,7 +73,7 @@ export function createDeleteSimsExecutor(
8073
try {
8174
const target = params.target;
8275

83-
if (params.shutdownFirst && target !== 'all' && target !== 'unavailable') {
76+
if (params.shutdownFirst && target !== 'unavailable') {
8477
try {
8578
await executor(
8679
['xcrun', 'simctl', 'shutdown', target],
@@ -153,7 +146,7 @@ export async function delete_simsLogic(
153146
}
154147

155148
export const schema = getSessionAwareToolSchemaShape({
156-
sessionAware: publicSchemaObject,
149+
sessionAware: baseSchemaObject,
157150
legacy: baseSchemaObject,
158151
});
159152

0 commit comments

Comments
 (0)