Skip to content

Commit 9294cd7

Browse files
committed
address PR review feedback
- resolve the default npx CLI path inside launch error handling\n- report missing npm once without spawning\n- preserve explicit path injection and shell-free exact-pin behavior\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 928bd48 commit 9294cd7

2 files changed

Lines changed: 49 additions & 3 deletions

File tree

plugins/power-pages/scripts/launch-playwright-mcp.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,24 @@ function resolveNpxCli({
5757

5858
function launch({
5959
browser = detectBrowser(),
60-
npxCliPath = resolveNpxCli(),
60+
npxCliPath,
61+
resolveNpxCliFn = resolveNpxCli,
6162
spawnFn = spawn,
6263
exitFn = (code) => process.exit(code),
6364
writeError = (message) => process.stderr.write(message),
6465
} = {}) {
65-
const child = spawnFn(process.execPath, [npxCliPath, ...buildMcpArgs(browser)], {
66+
let resolvedNpxCliPath = npxCliPath;
67+
if (resolvedNpxCliPath === undefined) {
68+
try {
69+
resolvedNpxCliPath = resolveNpxCliFn();
70+
} catch (error) {
71+
writeError(`Failed to start Playwright MCP: ${error.message}\n`);
72+
exitFn(1);
73+
return null;
74+
}
75+
}
76+
77+
const child = spawnFn(process.execPath, [resolvedNpxCliPath, ...buildMcpArgs(browser)], {
6678
stdio: 'inherit',
6779
shell: false,
6880
});

plugins/power-pages/scripts/tests/launch-playwright-mcp.test.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,16 @@ test('fullscreen config maximizes the browser and uses the real viewport size',
7878
assert.equal(config.browser.contextOptions.viewport, null);
7979
});
8080

81-
test('launch uses Node and raw argv without a shell', () => {
81+
test('launch preserves an explicit npx CLI path and uses raw argv without a shell', () => {
8282
let spawnCall;
8383
const child = new EventEmitter();
8484

8585
launch({
8686
browser: 'msedge',
8787
npxCliPath: 'C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npx-cli.js',
88+
resolveNpxCliFn() {
89+
assert.fail('explicit npxCliPath must bypass default resolution');
90+
},
8891
spawnFn(command, args, options) {
8992
spawnCall = { command, args, options };
9093
return child;
@@ -113,6 +116,37 @@ test('launch uses Node and raw argv without a shell', () => {
113116
assert.equal(spawnCall.exitCode, 7);
114117
});
115118

119+
test('launch reports missing npm once and does not spawn', () => {
120+
const exits = [];
121+
let spawnCalls = 0;
122+
let stderr = '';
123+
124+
const child = launch({
125+
browser: 'chrome',
126+
resolveNpxCliFn() {
127+
throw new Error('Could not locate npm/bin/npx-cli.js');
128+
},
129+
spawnFn() {
130+
spawnCalls += 1;
131+
return new EventEmitter();
132+
},
133+
exitFn(code) {
134+
exits.push(code);
135+
},
136+
writeError(message) {
137+
stderr += message;
138+
},
139+
});
140+
141+
assert.equal(child, null);
142+
assert.equal(spawnCalls, 0);
143+
assert.deepEqual(exits, [1]);
144+
assert.equal(
145+
stderr,
146+
'Failed to start Playwright MCP: Could not locate npm/bin/npx-cli.js\n',
147+
);
148+
});
149+
116150
test('launch reports spawn errors and exits with failure', () => {
117151
const child = new EventEmitter();
118152
const exits = [];

0 commit comments

Comments
 (0)