Skip to content

Commit 7d0fe2c

Browse files
cameroncookecodex
andcommitted
fix(simulator): Correct refresh signaling and selector notices
Return explicit scheduling status from simulator defaults refresh so MCP bootstrap logs only claim refresh scheduling when one is actually queued. Tighten session default notices so cleared selector messages only appear when a counterpart existed, and changed messages only appear on actual selector changes. Also prefer workspace defaults over project defaults when both are present, remove an unnecessary infer-platform export, and drop emoji from simulator build/run user-facing success strings. Co-Authored-By: Codex <noreply@openai.com>
1 parent bf0a0ee commit 7d0fe2c

7 files changed

Lines changed: 101 additions & 28 deletions

File tree

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ describe('session-set-defaults tool', () => {
125125
expect(current.simulatorId).toBe('RESOLVED-SIM-UUID');
126126
expect(current.simulatorName).toBeUndefined();
127127
expect(result.content[0].text).toContain(
128-
'Cleared simulatorName because simulatorId changed; background resolution will repopulate it.',
128+
'Cleared simulatorName because simulatorId was set; background resolution will repopulate it.',
129129
);
130130
});
131131

@@ -137,10 +137,20 @@ describe('session-set-defaults tool', () => {
137137
expect(current.simulatorName).toBe('iPhone 16');
138138
expect(current.simulatorId).toBeUndefined();
139139
expect(result.content[0].text).toContain(
140-
'Cleared simulatorId because simulatorName changed; background resolution will repopulate it.',
140+
'Cleared simulatorId because simulatorName was set; background resolution will repopulate it.',
141141
);
142142
});
143143

144+
it('does not claim simulatorName was cleared when none existed', async () => {
145+
sessionStore.setDefaults({ simulatorId: 'RESOLVED-SIM-UUID' });
146+
const result = await sessionSetDefaultsLogic(
147+
{ simulatorId: 'RESOLVED-SIM-UUID' },
148+
createContext(),
149+
);
150+
151+
expect(result.content[0].text).not.toContain('Cleared simulatorName');
152+
});
153+
144154
it('should not fail when simulatorName cannot be resolved immediately', async () => {
145155
const contextWithFailingExecutor = {
146156
executor: vi.fn().mockImplementation(async (command: string[]) => {

src/mcp/tools/session-management/session_set_defaults.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -75,33 +75,43 @@ export async function sessionSetDefaultsLogic(
7575
}
7676

7777
const selectorProvided = hasSimulatorId || hasSimulatorName;
78+
const simulatorIdChanged = hasSimulatorId && nextParams.simulatorId !== current.simulatorId;
79+
const simulatorNameChanged =
80+
hasSimulatorName && nextParams.simulatorName !== current.simulatorName;
81+
7882
if (hasSimulatorId && hasSimulatorName) {
7983
// Both provided - keep both, simulatorId takes precedence for tools
8084
notices.push(
8185
'Both simulatorId and simulatorName were provided; simulatorId will be used by tools.',
8286
);
8387
} else if (hasSimulatorId && !hasSimulatorName) {
8488
toClear.add('simulatorName');
85-
notices.push(
86-
'Cleared simulatorName because simulatorId changed; background resolution will repopulate it.',
87-
);
88-
notices.push(
89-
`Set simulatorId to "${nextParams.simulatorId}". Simulator name and platform refresh scheduled in background.`,
90-
);
89+
if (current.simulatorName !== undefined) {
90+
notices.push(
91+
'Cleared simulatorName because simulatorId was set; background resolution will repopulate it.',
92+
);
93+
}
94+
if (simulatorIdChanged) {
95+
notices.push(
96+
`Set simulatorId to "${nextParams.simulatorId}". Simulator name and platform refresh scheduled in background.`,
97+
);
98+
}
9199
} else if (hasSimulatorName && !hasSimulatorId) {
92100
toClear.add('simulatorId');
93-
notices.push(
94-
'Cleared simulatorId because simulatorName changed; background resolution will repopulate it.',
95-
);
96-
notices.push(
97-
`Set simulatorName to "${nextParams.simulatorName}". Simulator ID and platform refresh scheduled in background.`,
98-
);
101+
if (current.simulatorId !== undefined) {
102+
notices.push(
103+
'Cleared simulatorId because simulatorName was set; background resolution will repopulate it.',
104+
);
105+
}
106+
if (simulatorNameChanged) {
107+
notices.push(
108+
`Set simulatorName to "${nextParams.simulatorName}". Simulator ID and platform refresh scheduled in background.`,
109+
);
110+
}
99111
}
100112

101113
if (selectorProvided) {
102-
const selectorChanged =
103-
(hasSimulatorId && nextParams.simulatorId !== current.simulatorId) ||
104-
(hasSimulatorName && nextParams.simulatorName !== current.simulatorName);
114+
const selectorChanged = simulatorIdChanged || simulatorNameChanged;
105115
if (selectorChanged) {
106116
toClear.add('simulatorPlatform');
107117
notices.push('Cleared simulatorPlatform because simulator selector changed.');

src/mcp/tools/simulator/build_run_sim.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ export async function build_run_simLogic(
468468
}
469469

470470
// --- Success ---
471-
log('info', `${platformName} simulator build & run succeeded.`);
471+
log('info', `${platformName} simulator build & run succeeded.`);
472472

473473
const target = params.simulatorId
474474
? `simulator UUID '${params.simulatorId}'`
@@ -480,7 +480,7 @@ export async function build_run_simLogic(
480480
content: [
481481
{
482482
type: 'text',
483-
text: `${platformName} simulator build and run succeeded for scheme ${params.scheme} from ${sourceType} ${sourcePath} targeting ${target}.\n\nThe app (${bundleId}) is now running in the ${platformName} Simulator.\nIf you don't see the simulator window, it may be hidden behind other windows. The Simulator app should be open.`,
483+
text: `${platformName} simulator build and run succeeded for scheme ${params.scheme} from ${sourceType} ${sourcePath} targeting ${target}.\n\nThe app (${bundleId}) is now running in the ${platformName} Simulator.\nIf you don't see the simulator window, it may be hidden behind other windows. The Simulator app should be open.`,
484484
},
485485
],
486486
nextSteps: [

src/runtime/bootstrap-runtime.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,18 @@ export interface BootstrapRuntimeResult {
3333
notices: string[];
3434
}
3535

36-
function hydrateSessionDefaultsForMcp(defaults: Partial<SessionDefaults> | undefined): void {
36+
/**
37+
* Returns true when defaults were hydrated and a background simulator refresh was scheduled.
38+
*/
39+
function hydrateSessionDefaultsForMcp(defaults: Partial<SessionDefaults> | undefined): boolean {
3740
const hydratedDefaults = { ...(defaults ?? {}) };
3841
if (Object.keys(hydratedDefaults).length === 0) {
39-
return;
42+
return false;
4043
}
4144

4245
sessionStore.setDefaults(hydratedDefaults);
4346
const revision = sessionStore.getRevision();
44-
scheduleSimulatorDefaultsRefresh({
47+
return scheduleSimulatorDefaultsRefresh({
4548
expectedRevision: revision,
4649
reason: 'startup-hydration',
4750
persist: true,
@@ -72,8 +75,7 @@ export async function bootstrapRuntime(
7275

7376
const config = getConfig();
7477

75-
if (opts.runtime === 'mcp') {
76-
hydrateSessionDefaultsForMcp(config.sessionDefaults);
78+
if (opts.runtime === 'mcp' && hydrateSessionDefaultsForMcp(config.sessionDefaults)) {
7779
log('info', '[Session] Hydrated MCP session defaults; simulator metadata refresh scheduled.');
7880
}
7981

src/utils/__tests__/infer-platform.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,45 @@ describe('inferPlatform', () => {
195195
]);
196196
});
197197

198+
it('prefers workspace defaults when both projectPath and workspacePath are present in session defaults', async () => {
199+
sessionStore.setDefaults({
200+
projectPath: '/tmp/Test.xcodeproj',
201+
workspacePath: '/tmp/Test.xcworkspace',
202+
scheme: 'WatchScheme',
203+
});
204+
205+
const callHistory: string[][] = [];
206+
const mockExecutor: CommandExecutor = async (command) => {
207+
callHistory.push(command);
208+
209+
if (command[0] === 'xcrun') {
210+
return createMockCommandResponse({
211+
success: true,
212+
output: JSON.stringify({ devices: {} }),
213+
});
214+
}
215+
216+
return createMockCommandResponse({
217+
success: true,
218+
output: 'SDKROOT = watchsimulator',
219+
});
220+
};
221+
222+
const result = await inferPlatform({ simulatorId: 'SIM-UUID' }, mockExecutor);
223+
224+
expect(result.platform).toBe(XcodePlatform.watchOSSimulator);
225+
expect(result.source).toBe('build-settings');
226+
expect(callHistory).toHaveLength(2);
227+
expect(callHistory[1]).toEqual([
228+
'xcodebuild',
229+
'-showBuildSettings',
230+
'-scheme',
231+
'WatchScheme',
232+
'-workspace',
233+
'/tmp/Test.xcworkspace',
234+
]);
235+
});
236+
198237
it('defaults to iOS when simulator and build-settings inference both fail', async () => {
199238
const mockExecutor: CommandExecutor = async (command) => {
200239
if (command[0] === 'xcrun') {

src/utils/infer-platform.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ function isSimulatorPlatform(value: unknown): value is SimulatorPlatform {
8787
return SIMULATOR_PLATFORMS.includes(value as SimulatorPlatform);
8888
}
8989

90-
export function inferSimulatorSelectorForTool(params: {
90+
function inferSimulatorSelectorForTool(params: {
9191
simulatorId?: string;
9292
simulatorName?: string;
9393
sessionDefaults?: Partial<SessionDefaults>;
@@ -155,11 +155,21 @@ function resolveProjectFromSession(params: InferPlatformParams): {
155155
scheme?: string;
156156
} {
157157
const defaults = params.sessionDefaults ?? sessionStore.getAll();
158+
const hasExplicitProjectPath = params.projectPath !== undefined;
159+
const hasExplicitWorkspacePath = params.workspacePath !== undefined;
158160
const projectPath =
159161
params.projectPath ?? (params.workspacePath ? undefined : defaults.projectPath);
160162
const workspacePath =
161163
params.workspacePath ?? (params.projectPath ? undefined : defaults.workspacePath);
162164

165+
if (projectPath && workspacePath && !hasExplicitProjectPath && !hasExplicitWorkspacePath) {
166+
return {
167+
projectPath: undefined,
168+
workspacePath,
169+
scheme: params.scheme ?? defaults.scheme,
170+
};
171+
}
172+
163173
return {
164174
projectPath,
165175
workspacePath,

src/utils/simulator-defaults-refresh.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,21 @@ function shouldSkipBackgroundRefresh(): boolean {
2323

2424
export function scheduleSimulatorDefaultsRefresh(
2525
options: ScheduleSimulatorDefaultsRefreshOptions,
26-
): void {
26+
): boolean {
2727
const hasSelector = options.simulatorId != null || options.simulatorName != null;
2828
if (!hasSelector) {
29-
return;
29+
return false;
3030
}
3131

3232
if (shouldSkipBackgroundRefresh()) {
33-
return;
33+
return false;
3434
}
3535

3636
setTimeout(() => {
3737
void refreshSimulatorDefaults(options);
3838
}, 0);
39+
40+
return true;
3941
}
4042

4143
async function refreshSimulatorDefaults(

0 commit comments

Comments
 (0)