Skip to content

Commit a421a7f

Browse files
haifeng-li-at-salesforcehaifeng-li-at-salesforce
authored
Merge pull request #148 from haifeng-li-at-salesforce/windowInstall
fix: address the issues related to Windows
2 parents d784bda + 769ead1 commit a421a7f

File tree

10 files changed

+117
-77
lines changed

10 files changed

+117
-77
lines changed

‎packages/mcp-workflow/src/execution/commandRunner.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export class DefaultCommandRunner implements CommandRunner {
8080

8181
const childProcess = spawn(command, args, {
8282
env,
83-
shell: false,
83+
shell: true,
8484
stdio: ['ignore', 'pipe', 'pipe'],
8585
cwd,
8686
});
@@ -192,11 +192,8 @@ export class DefaultCommandRunner implements CommandRunner {
192192
const errorMsg = signal
193193
? `Command terminated by signal: ${signal}`
194194
: `Command failed with exit code: ${code}`;
195-
progressReporter.report(
196-
DefaultCommandRunner.PROGRESS_TOTAL,
197-
DefaultCommandRunner.PROGRESS_TOTAL,
198-
errorMsg
199-
);
195+
// Don't report 100% for failures - keep current progress to indicate incomplete
196+
progressReporter.report(currentProgress, DefaultCommandRunner.PROGRESS_TOTAL, errorMsg);
200197
}
201198
}
202199

‎packages/mcp-workflow/tests/execution/commandRunner.test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('DefaultCommandRunner', () => {
112112
env: expect.objectContaining({
113113
...process.env,
114114
}),
115-
shell: false,
115+
shell: true,
116116
stdio: ['ignore', 'pipe', 'pipe'],
117117
cwd: undefined,
118118
});
@@ -431,8 +431,9 @@ describe('DefaultCommandRunner', () => {
431431

432432
await promise;
433433

434+
// On failure, progress stays at current (0) to indicate incomplete
434435
expect(mockProgressReporter.report).toHaveBeenCalledWith(
435-
100,
436+
0,
436437
100,
437438
'Command failed with exit code: 1'
438439
);
@@ -454,8 +455,9 @@ describe('DefaultCommandRunner', () => {
454455

455456
await promise;
456457

458+
// On signal termination, progress stays at current (0) to indicate incomplete
457459
expect(mockProgressReporter.report).toHaveBeenCalledWith(
458-
100,
460+
0,
459461
100,
460462
'Command terminated by signal: SIGTERM'
461463
);

‎packages/mobile-native-mcp-server/src/execution/build/android/buildCommandFactory.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,27 @@ const ANDROID_PROGRESS_PATTERNS: ProgressPattern[] = [
4646
{ pattern: /> Task :app:assemble(Debug|Release)/g, weight: 5 },
4747
];
4848

49+
/**
50+
* Default check if running on Windows.
51+
*/
52+
export const isWindows = process.platform === 'win32';
53+
4954
/**
5055
* Android build command factory.
5156
* Creates Gradle build commands and parses Gradle output for progress.
5257
*/
5358
export class AndroidBuildCommandFactory implements BuildCommandFactory {
59+
private readonly isWindows: boolean;
60+
61+
constructor(isWindows: boolean) {
62+
this.isWindows = isWindows;
63+
}
64+
5465
create(params: BuildCommandParams): Command {
66+
const gradlew = this.isWindows ? 'gradlew.bat' : './gradlew';
5567
return {
56-
executable: 'sh',
57-
args: ['-c', `cd "${params.projectPath}" && ./gradlew assemble`],
68+
executable: gradlew,
69+
args: ['assemble'],
5870
env: process.env,
5971
cwd: params.projectPath,
6072
};

‎packages/mobile-native-mcp-server/src/execution/build/buildExecutor.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { PlatformEnum } from '../../common/schemas.js';
1515
import { TempDirectoryManager } from '../../common.js';
1616
import { BuildCommandFactory } from './types.js';
1717
import { iOSBuildCommandFactory } from './ios/buildCommandFactory.js';
18-
import { AndroidBuildCommandFactory } from './android/buildCommandFactory.js';
18+
import { AndroidBuildCommandFactory, isWindows } from './android/buildCommandFactory.js';
1919

2020
/**
2121
* Result of build execution
@@ -54,6 +54,12 @@ export interface BuildExecutor {
5454
): Promise<BuildExecutionResult>;
5555
}
5656

57+
/**
58+
* Default timeout for build commands: 30 minutes.
59+
* Builds (especially first-time Android/iOS builds) can take significant time.
60+
*/
61+
const BUILD_TIMEOUT_MS = 30 * 60 * 1000;
62+
5763
/**
5864
* Default implementation of BuildExecutor.
5965
*/
@@ -69,7 +75,7 @@ export class DefaultBuildExecutor implements BuildExecutor {
6975
this.tempDirManager = tempDirManager;
7076
this.logger = logger ?? createComponentLogger('BuildExecutor');
7177
this.iosFactory = new iOSBuildCommandFactory();
72-
this.androidFactory = new AndroidBuildCommandFactory();
78+
this.androidFactory = new AndroidBuildCommandFactory(isWindows);
7379
}
7480

7581
async execute(
@@ -101,6 +107,7 @@ export class DefaultBuildExecutor implements BuildExecutor {
101107
const result = await this.commandRunner.execute(command.executable, command.args, {
102108
env: command.env,
103109
cwd: command.cwd,
110+
timeout: BUILD_TIMEOUT_MS,
104111
progressParser: (output: string, currentProgress: number) =>
105112
factory.parseProgress(output, currentProgress),
106113
progressReporter,

‎packages/mobile-native-mcp-server/src/workflow/nodes/checkPlatformSetup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class PlatformCheckNode extends BaseNode<State> {
8585

8686
try {
8787
this.logger.debug(`Executing command (pre-execution)`, { command });
88-
const output = execSync(command, { encoding: 'utf-8', timeout: 20000 });
88+
const output = execSync(command, { encoding: 'utf-8', timeout: 120000 });
8989

9090
const platformCheckResult = this.parsePlatformCheckOutput(output, command);
9191

‎packages/mobile-native-mcp-server/src/workflow/nodes/checkPluginSetup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class PluginCheckNode extends BaseNode<State> {
126126
): Partial<State> {
127127
try {
128128
// Pipe "y" to automatically answer the trust prompt
129-
const installCommand = `echo "y" | sf plugins install ${config.name}${config.installTag ?? ''}`;
129+
const installCommand = `echo y | sf plugins install ${config.name}${config.installTag ?? ''}`;
130130
const operationVerb = operation === 'install' ? 'Installing' : 'Upgrading';
131131
this.logger.debug(`${operationVerb} plugin`, {
132132
command: installCommand,

‎packages/mobile-native-mcp-server/tests/execution/build/android/buildCommandFactory.test.ts‎

Lines changed: 65 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -10,76 +10,92 @@ import { AndroidBuildCommandFactory } from '../../../../src/execution/build/andr
1010
import { PROGRESS_COMPLETE } from '../../../../src/execution/build/types.js';
1111

1212
describe('AndroidBuildCommandFactory', () => {
13-
let factory: AndroidBuildCommandFactory;
13+
describe('create', () => {
14+
describe('Unix/macOS', () => {
15+
let factory: AndroidBuildCommandFactory;
1416

15-
beforeEach(() => {
16-
factory = new AndroidBuildCommandFactory();
17-
});
17+
beforeEach(() => {
18+
// Pass false to indicate Unix/macOS
19+
factory = new AndroidBuildCommandFactory(false);
20+
});
1821

19-
describe('create', () => {
20-
it('should create command with correct executable', () => {
21-
const params = {
22-
projectPath: '/path/to/project',
23-
projectName: 'TestApp',
24-
buildOutputDir: '/output',
25-
};
22+
it('should create command with correct executable', () => {
23+
const params = {
24+
projectPath: '/path/to/project',
25+
projectName: 'TestApp',
26+
buildOutputDir: '/output',
27+
};
2628

27-
const command = factory.create(params);
29+
const command = factory.create(params);
2830

29-
expect(command.executable).toBe('sh');
30-
expect(command.args).toContain('-c');
31-
});
31+
expect(command.executable).toBe('./gradlew');
32+
expect(command.args).toEqual(['assemble']);
33+
});
3234

33-
it('should include project path in command', () => {
34-
const params = {
35-
projectPath: '/custom/path',
36-
projectName: 'TestApp',
37-
buildOutputDir: '/output',
38-
};
35+
it('should set cwd to project path', () => {
36+
const params = {
37+
projectPath: '/path/to/project',
38+
projectName: 'TestApp',
39+
buildOutputDir: '/output',
40+
};
3941

40-
const command = factory.create(params);
42+
const command = factory.create(params);
4143

42-
expect(command.args[1]).toContain('/custom/path');
43-
});
44+
expect(command.cwd).toBe('/path/to/project');
45+
});
4446

45-
it('should include gradlew assemble command', () => {
46-
const params = {
47-
projectPath: '/path/to/project',
48-
projectName: 'TestApp',
49-
buildOutputDir: '/output',
50-
};
47+
it('should use process.env for environment', () => {
48+
const params = {
49+
projectPath: '/path/to/project',
50+
projectName: 'TestApp',
51+
buildOutputDir: '/output',
52+
};
5153

52-
const command = factory.create(params);
54+
const command = factory.create(params);
5355

54-
expect(command.args[1]).toContain('./gradlew assemble');
56+
expect(command.env).toBe(process.env);
57+
});
5558
});
5659

57-
it('should set cwd to project path', () => {
58-
const params = {
59-
projectPath: '/path/to/project',
60-
projectName: 'TestApp',
61-
buildOutputDir: '/output',
62-
};
60+
describe('Windows', () => {
61+
let factory: AndroidBuildCommandFactory;
62+
63+
beforeEach(() => {
64+
// Pass true to indicate Windows
65+
factory = new AndroidBuildCommandFactory(true);
66+
});
6367

64-
const command = factory.create(params);
68+
it('should create command with gradlew.bat executable', () => {
69+
const params = {
70+
projectPath: 'C:\\path\\to\\project',
71+
projectName: 'TestApp',
72+
buildOutputDir: 'C:\\output',
73+
};
6574

66-
expect(command.cwd).toBe('/path/to/project');
67-
});
75+
const command = factory.create(params);
6876

69-
it('should use process.env for environment', () => {
70-
const params = {
71-
projectPath: '/path/to/project',
72-
projectName: 'TestApp',
73-
buildOutputDir: '/output',
74-
};
77+
expect(command.executable).toBe('gradlew.bat');
78+
expect(command.args).toEqual(['assemble']);
79+
});
80+
81+
it('should set cwd to project path', () => {
82+
const params = {
83+
projectPath: 'C:\\path\\to\\project',
84+
projectName: 'TestApp',
85+
buildOutputDir: 'C:\\output',
86+
};
7587

76-
const command = factory.create(params);
88+
const command = factory.create(params);
7789

78-
expect(command.env).toBe(process.env);
90+
expect(command.cwd).toBe('C:\\path\\to\\project');
91+
});
7992
});
8093
});
8194

8295
describe('parseProgress', () => {
96+
// parseProgress doesn't depend on platform, so we can use either
97+
const factory = new AndroidBuildCommandFactory(false);
98+
8399
describe('initialization phase', () => {
84100
it('should track Gradle daemon startup', () => {
85101
const output = 'Starting a Gradle Daemon (subsequent builds will be faster)';

‎packages/mobile-native-mcp-server/tests/execution/build/buildExecutor.test.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,14 @@ describe('DefaultBuildExecutor', () => {
9090

9191
expect(mockCommandRunner.execute).toHaveBeenCalled();
9292
const callArgs = vi.mocked(mockCommandRunner.execute).mock.calls[0];
93-
expect(callArgs[0]).toBe('sh');
94-
expect(callArgs[1][1]).toContain('./gradlew assemble');
93+
94+
// Platform-aware assertions: Windows uses 'gradlew.bat', Unix uses './gradlew'
95+
if (process.platform === 'win32') {
96+
expect(callArgs[0]).toBe('gradlew.bat');
97+
} else {
98+
expect(callArgs[0]).toBe('./gradlew');
99+
}
100+
expect(callArgs[1]).toEqual(['assemble']);
95101
});
96102

97103
it('should return successful result when build succeeds', async () => {

‎packages/mobile-native-mcp-server/tests/workflow/nodes/checkPlatformSetup.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe('PlatformCheckNode', () => {
127127
expect(result.workflowFatalErrorMessages).toBeUndefined();
128128
expect(mockExecSync).toHaveBeenCalledWith(
129129
'sf force lightning local setup -p ios -l 17.0 --json',
130-
{ encoding: 'utf-8', timeout: 20000 }
130+
{ encoding: 'utf-8', timeout: 120000 }
131131
);
132132
});
133133

@@ -209,7 +209,7 @@ describe('PlatformCheckNode', () => {
209209
expect(result.workflowFatalErrorMessages).toBeUndefined();
210210
expect(mockExecSync).toHaveBeenCalledWith(
211211
'sf force lightning local setup -p android -l 35 --json',
212-
{ encoding: 'utf-8', timeout: 20000 }
212+
{ encoding: 'utf-8', timeout: 120000 }
213213
);
214214
});
215215

@@ -1096,7 +1096,7 @@ describe('PlatformCheckNode', () => {
10961096
});
10971097

10981098
describe('execute() - Command Timeout Configuration', () => {
1099-
it('should set timeout to 20000ms', () => {
1099+
it('should set timeout to 120000ms', () => {
11001100
const inputState = createTestState({
11011101
platform: 'iOS',
11021102
});
@@ -1114,7 +1114,7 @@ describe('PlatformCheckNode', () => {
11141114

11151115
expect(mockExecSync).toHaveBeenCalledWith(expect.any(String), {
11161116
encoding: 'utf-8',
1117-
timeout: 20000,
1117+
timeout: 120000,
11181118
});
11191119
});
11201120
});

‎packages/mobile-native-mcp-server/tests/workflow/nodes/checkPluginSetup.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,15 @@ describe('PluginCheckNode', () => {
252252

253253
expect(result.validPluginSetup).toBe(true);
254254
expect(mockExecSync).toHaveBeenCalledWith(
255-
'echo "y" | sf plugins install sfdx-mobilesdk-plugin@alpha',
255+
'echo y | sf plugins install sfdx-mobilesdk-plugin@alpha',
256256
{
257257
encoding: 'utf-8',
258258
timeout: 3 * 60 * 1000,
259259
maxBuffer: 2 * 1024 * 1024,
260260
}
261261
);
262262
expect(mockExecSync).toHaveBeenCalledWith(
263-
'echo "y" | sf plugins install @salesforce/lwc-dev-mobile',
263+
'echo y | sf plugins install @salesforce/lwc-dev-mobile',
264264
{
265265
encoding: 'utf-8',
266266
timeout: 3 * 60 * 1000,
@@ -373,7 +373,7 @@ describe('PluginCheckNode', () => {
373373

374374
expect(result.validPluginSetup).toBe(true);
375375
expect(mockExecSync).toHaveBeenCalledWith(
376-
'echo "y" | sf plugins install sfdx-mobilesdk-plugin@alpha',
376+
'echo y | sf plugins install sfdx-mobilesdk-plugin@alpha',
377377
{
378378
encoding: 'utf-8',
379379
timeout: 3 * 60 * 1000,
@@ -1038,7 +1038,7 @@ describe('PluginCheckNode', () => {
10381038
});
10391039
});
10401040

1041-
it('should set timeout to 10000ms for install command', () => {
1041+
it('should set timeout to 3 minutes for install command', () => {
10421042
const inputState = createTestState({});
10431043

10441044
mockExecSync.mockImplementationOnce(() => {
@@ -1056,7 +1056,7 @@ describe('PluginCheckNode', () => {
10561056
node.execute(inputState);
10571057

10581058
expect(mockExecSync).toHaveBeenCalledWith(
1059-
'echo "y" | sf plugins install sfdx-mobilesdk-plugin@alpha',
1059+
'echo y | sf plugins install sfdx-mobilesdk-plugin@alpha',
10601060
{
10611061
encoding: 'utf-8',
10621062
timeout: 3 * 60 * 1000,
@@ -1065,7 +1065,7 @@ describe('PluginCheckNode', () => {
10651065
);
10661066
});
10671067

1068-
it('should set timeout to 10000ms for update command', () => {
1068+
it('should set timeout to 3 minutes for update command', () => {
10691069
const inputState = createTestState({});
10701070

10711071
mockExecSync.mockReturnValueOnce(
@@ -1086,7 +1086,7 @@ describe('PluginCheckNode', () => {
10861086
node.execute(inputState);
10871087

10881088
expect(mockExecSync).toHaveBeenCalledWith(
1089-
'echo "y" | sf plugins install sfdx-mobilesdk-plugin@alpha',
1089+
'echo y | sf plugins install sfdx-mobilesdk-plugin@alpha',
10901090
{
10911091
encoding: 'utf-8',
10921092
timeout: 3 * 60 * 1000,

0 commit comments

Comments
 (0)