Skip to content

Commit ec1a1e5

Browse files
cursoragentcursoragent
committed
Fix dependency injection violations and env persistence in config store
- Fixed listAvailableDevices to use injected FileSystemExecutor instead of direct fs imports - Fixed config store to preserve custom env parameter through session defaults refresh - Updated tests to use mock FileSystemExecutor instead of real filesystem operations
1 parent c07a7a8 commit ec1a1e5

File tree

3 files changed

+100
-49
lines changed

3 files changed

+100
-49
lines changed

‎src/cli/commands/__tests__/setup.test.ts‎

Lines changed: 86 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,36 @@
11
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
2-
import { promises as fs } from 'node:fs';
32
import path from 'node:path';
43
import { parse as parseYaml } from 'yaml';
54
import {
65
createMockCommandResponse,
76
createMockFileSystemExecutor,
87
} from '../../../test-utils/mock-executors.ts';
98
import type { CommandExecutor } from '../../../utils/CommandExecutor.ts';
9+
import type { FileSystemExecutor } from '../../../utils/FileSystemExecutor.ts';
1010
import type { Prompter } from '../../interactive/prompts.ts';
1111
import { runSetupWizard } from '../setup.ts';
1212

1313
const cwd = '/repo';
1414
const configPath = path.join(cwd, '.xcodebuildmcp', 'config.yaml');
1515

16-
async function writeMockDeviceList(jsonPath: string): Promise<void> {
17-
await fs.writeFile(
18-
jsonPath,
19-
JSON.stringify({
20-
result: {
21-
devices: [
22-
{
23-
identifier: 'DEVICE-1',
24-
visibilityClass: 'Default',
25-
connectionProperties: {
26-
pairingState: 'paired',
27-
tunnelState: 'connected',
28-
},
29-
deviceProperties: {
30-
name: 'Cam iPhone',
31-
platformIdentifier: 'com.apple.platform.iphoneos',
32-
},
33-
},
34-
],
16+
const mockDeviceListJson = JSON.stringify({
17+
result: {
18+
devices: [
19+
{
20+
identifier: 'DEVICE-1',
21+
visibilityClass: 'Default',
22+
connectionProperties: {
23+
pairingState: 'paired',
24+
tunnelState: 'connected',
25+
},
26+
deviceProperties: {
27+
name: 'Cam iPhone',
28+
platformIdentifier: 'com.apple.platform.iphoneos',
29+
},
3530
},
36-
}),
37-
);
38-
}
31+
],
32+
},
33+
});
3934

4035
function createTestPrompter(): Prompter {
4136
return {
@@ -72,6 +67,7 @@ describe('setup command', () => {
7267

7368
it('exports a setup wizard that writes config selections', async () => {
7469
let storedConfig = '';
70+
const deviceListFiles = new Map<string, string>();
7571

7672
const fs = createMockFileSystemExecutor({
7773
existsSync: (targetPath) => targetPath === configPath && storedConfig.length > 0,
@@ -90,22 +86,31 @@ describe('setup command', () => {
9086
return [];
9187
},
9288
readFile: async (targetPath) => {
93-
if (targetPath !== configPath) {
94-
throw new Error(`Unexpected read path: ${targetPath}`);
89+
if (targetPath === configPath) {
90+
return storedConfig;
9591
}
96-
return storedConfig;
92+
if (deviceListFiles.has(targetPath)) {
93+
return deviceListFiles.get(targetPath)!;
94+
}
95+
throw new Error(`Unexpected read path: ${targetPath}`);
9796
},
9897
writeFile: async (targetPath, content) => {
99-
if (targetPath !== configPath) {
100-
throw new Error(`Unexpected write path: ${targetPath}`);
98+
if (targetPath === configPath) {
99+
storedConfig = content;
100+
return;
101101
}
102-
storedConfig = content;
102+
deviceListFiles.set(targetPath, content);
103103
},
104+
mkdtemp: async (prefix: string) => {
105+
return `${prefix}123456`;
106+
},
107+
tmpdir: () => '/tmp',
108+
rm: async () => {},
104109
});
105110

106111
const executor: CommandExecutor = async (command) => {
107112
if (command[0] === 'xcrun' && command[1] === 'devicectl') {
108-
await writeMockDeviceList(command[5]);
113+
await fs.writeFile(command[5], mockDeviceListJson);
109114
return createMockCommandResponse({
110115
success: true,
111116
output: '',
@@ -172,6 +177,7 @@ describe('setup command', () => {
172177
it('shows debug-gated workflows when existing config enables debug', async () => {
173178
let storedConfig = 'schemaVersion: 1\ndebug: true\n';
174179
let offeredWorkflowIds: string[] = [];
180+
const deviceListFiles = new Map<string, string>();
175181

176182
const fs = createMockFileSystemExecutor({
177183
existsSync: (targetPath) => targetPath === configPath && storedConfig.length > 0,
@@ -190,22 +196,31 @@ describe('setup command', () => {
190196
return [];
191197
},
192198
readFile: async (targetPath) => {
193-
if (targetPath !== configPath) {
194-
throw new Error(`Unexpected read path: ${targetPath}`);
199+
if (targetPath === configPath) {
200+
return storedConfig;
195201
}
196-
return storedConfig;
202+
if (deviceListFiles.has(targetPath)) {
203+
return deviceListFiles.get(targetPath)!;
204+
}
205+
throw new Error(`Unexpected read path: ${targetPath}`);
197206
},
198207
writeFile: async (targetPath, content) => {
199-
if (targetPath !== configPath) {
200-
throw new Error(`Unexpected write path: ${targetPath}`);
208+
if (targetPath === configPath) {
209+
storedConfig = content;
210+
return;
201211
}
202-
storedConfig = content;
212+
deviceListFiles.set(targetPath, content);
213+
},
214+
mkdtemp: async (prefix: string) => {
215+
return `${prefix}123456`;
203216
},
217+
tmpdir: () => '/tmp',
218+
rm: async () => {},
204219
});
205220

206221
const executor: CommandExecutor = async (command) => {
207222
if (command[0] === 'xcrun' && command[1] === 'devicectl') {
208-
await writeMockDeviceList(command[5]);
223+
await fs.writeFile(command[5], mockDeviceListJson);
209224
return createMockCommandResponse({
210225
success: true,
211226
output: '',
@@ -297,6 +312,8 @@ describe('setup command', () => {
297312
});
298313

299314
it('outputs MCP config JSON when format is mcp-json', async () => {
315+
const deviceListFiles = new Map<string, string>();
316+
300317
const fs = createMockFileSystemExecutor({
301318
existsSync: () => false,
302319
stat: async () => ({ isDirectory: () => true, mtimeMs: 0 }),
@@ -313,13 +330,25 @@ describe('setup command', () => {
313330

314331
return [];
315332
},
316-
readFile: async () => '',
317-
writeFile: async () => {},
333+
readFile: async (targetPath) => {
334+
if (deviceListFiles.has(targetPath)) {
335+
return deviceListFiles.get(targetPath)!;
336+
}
337+
return '';
338+
},
339+
writeFile: async (targetPath, content) => {
340+
deviceListFiles.set(targetPath, content);
341+
},
342+
mkdtemp: async (prefix: string) => {
343+
return `${prefix}123456`;
344+
},
345+
tmpdir: () => '/tmp',
346+
rm: async () => {},
318347
});
319348

320349
const executor: CommandExecutor = async (command) => {
321350
if (command[0] === 'xcrun' && command[1] === 'devicectl') {
322-
await writeMockDeviceList(command[5]);
351+
await fs.writeFile(command[5], mockDeviceListJson);
323352
return createMockCommandResponse({
324353
success: true,
325354
output: '',
@@ -474,6 +503,8 @@ describe('setup command', () => {
474503
});
475504

476505
it('collects a device default without requiring simulator selection when only device-dependent workflows are enabled', async () => {
506+
const deviceListFiles = new Map<string, string>();
507+
477508
const fs = createMockFileSystemExecutor({
478509
existsSync: () => false,
479510
stat: async () => ({ isDirectory: () => true, mtimeMs: 0 }),
@@ -490,8 +521,20 @@ describe('setup command', () => {
490521

491522
return [];
492523
},
493-
readFile: async () => '',
494-
writeFile: async () => {},
524+
readFile: async (targetPath) => {
525+
if (deviceListFiles.has(targetPath)) {
526+
return deviceListFiles.get(targetPath)!;
527+
}
528+
return '';
529+
},
530+
writeFile: async (targetPath, content) => {
531+
deviceListFiles.set(targetPath, content);
532+
},
533+
mkdtemp: async (prefix: string) => {
534+
return `${prefix}123456`;
535+
},
536+
tmpdir: () => '/tmp',
537+
rm: async () => {},
495538
});
496539

497540
const executor: CommandExecutor = async (command) => {
@@ -500,7 +543,7 @@ describe('setup command', () => {
500543
}
501544

502545
if (command[0] === 'xcrun' && command[1] === 'devicectl') {
503-
await writeMockDeviceList(command[5]);
546+
await fs.writeFile(command[5], mockDeviceListJson);
504547
return createMockCommandResponse({
505548
success: true,
506549
output: '',

‎src/cli/commands/setup.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import type { Argv } from 'yargs';
2-
import { promises as fs } from 'node:fs';
3-
import { tmpdir } from 'node:os';
42
import path from 'node:path';
53
import * as clack from '@clack/prompts';
64
import { getDefaultCommandExecutor, getDefaultFileSystemExecutor } from '../../utils/command.ts';
@@ -543,8 +541,12 @@ function parseXctraceDevices(output: string): SetupDevice[] {
543541
return listed;
544542
}
545543

546-
async function listAvailableDevices(executor: CommandExecutor): Promise<SetupDevice[]> {
547-
const jsonPath = path.join(tmpdir(), `xcodebuildmcp-setup-devices-${Date.now()}.json`);
544+
async function listAvailableDevices(
545+
executor: CommandExecutor,
546+
fs: FileSystemExecutor,
547+
): Promise<SetupDevice[]> {
548+
const tmpDir = await fs.mkdtemp(path.join(fs.tmpdir(), 'xcodebuildmcp-setup-devices-'));
549+
const jsonPath = path.join(tmpDir, 'devices.json');
548550

549551
try {
550552
const result = await executor(
@@ -564,7 +566,7 @@ async function listAvailableDevices(executor: CommandExecutor): Promise<SetupDev
564566
} catch {
565567
// Fall back to xctrace below.
566568
} finally {
567-
await fs.unlink(jsonPath).catch(() => {});
569+
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
568570
}
569571

570572
const fallbackResult = await executor(
@@ -595,6 +597,7 @@ function getDefaultDeviceIndex(devices: SetupDevice[], existingDeviceId?: string
595597
async function selectDevice(opts: {
596598
existingDeviceId?: string;
597599
executor: CommandExecutor;
600+
fs: FileSystemExecutor;
598601
prompter: Prompter;
599602
isTTY: boolean;
600603
quietOutput: boolean;
@@ -604,7 +607,7 @@ async function selectDevice(opts: {
604607
quietOutput: opts.quietOutput,
605608
startMessage: 'Loading devices...',
606609
stopMessage: 'Devices loaded.',
607-
task: () => listAvailableDevices(opts.executor),
610+
task: () => listAvailableDevices(opts.executor, opts.fs),
608611
});
609612

610613
if (devices.length === 0) {
@@ -725,6 +728,7 @@ async function collectSetupSelection(
725728
? await selectDevice({
726729
existingDeviceId: existing.deviceId,
727730
executor: deps.executor,
731+
fs: deps.fs,
728732
prompter: deps.prompter,
729733
isTTY,
730734
quietOutput: deps.quietOutput,

‎src/utils/config-store.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ type ConfigStoreState = {
6565
fs?: FileSystemExecutor;
6666
overrides?: RuntimeConfigOverrides;
6767
fileConfig?: ProjectConfig;
68+
env?: NodeJS.ProcessEnv;
6869
resolved: ResolvedRuntimeConfig;
6970
};
7071

@@ -365,6 +366,7 @@ function refreshResolvedSessionFields(): void {
365366
storeState.resolved.sessionDefaults = resolveSessionDefaults({
366367
overrides: storeState.overrides,
367368
fileConfig: storeState.fileConfig,
369+
env: storeState.env,
368370
});
369371
storeState.resolved.sessionDefaultsProfiles = resolveSessionDefaultsProfiles({
370372
overrides: storeState.overrides,
@@ -576,6 +578,7 @@ export async function initConfigStore(opts: {
576578
storeState.cwd = opts.cwd;
577579
storeState.fs = opts.fs;
578580
storeState.overrides = opts.overrides;
581+
storeState.env = opts.env;
579582

580583
let fileConfig: ProjectConfig | undefined;
581584
let found = false;
@@ -676,5 +679,6 @@ export function __resetConfigStoreForTests(): void {
676679
storeState.fs = undefined;
677680
storeState.overrides = undefined;
678681
storeState.fileConfig = undefined;
682+
storeState.env = undefined;
679683
storeState.resolved = { ...DEFAULT_CONFIG };
680684
}

0 commit comments

Comments
 (0)