Skip to content

Pin Power Pages Playwright MCP runtime - #383

Merged
Priyanshu Agrawal (priyanshu92) merged 19 commits into
mainfrom
users/priyanshu92/pin-playwright-mcp
Aug 11, 2026
Merged

Pin Power Pages Playwright MCP runtime#383
Priyanshu Agrawal (priyanshu92) merged 19 commits into
mainfrom
users/priyanshu92/pin-playwright-mcp

Conversation

@priyanshu92

Copy link
Copy Markdown
Collaborator

Summary

  • pin the runtime-fetched @playwright/mcp package to reviewed version 0.0.78
  • invoke npm's JavaScript npx entrypoint through Node with raw argv and shell: false
  • preserve config paths containing spaces and shell metacharacters without shell quoting
  • handle spawn failures and propagate child exit failures
  • leave MCP root resolution unchanged

Testing

  • POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT=1 node --test plugins/power-pages/scripts/tests/
  • exact-version Playwright MCP CLI smoke test through the approved package feed

- Keep runtime command approvals enabled by default
- Explain credential scope and narrow script allow rules

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- pin the reviewed Playwright MCP runtime package to 0.0.78\n- invoke npx through Node with raw argv and no command shell\n- cover Windows paths, spawn failures, and exit propagation\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- require an absolute host-provided plugin root\n- canonicalize and contain the launcher path\n- cover invalid roots, malicious cwd, and supported hosts\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 6, 2026 11:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Pins the Power Pages Playwright MCP bootstrap to a reviewed @playwright/mcp version and updates the launcher to invoke npx via Node (no shell) so config paths are passed as raw argv (including spaces/metacharacters), with improved error/exit propagation.

Changes:

  • Pin runtime package selection to @playwright/mcp@0.0.78 and remove @latest usage.
  • Launch MCP via node <npx-cli.js> ... with shell: false, preserving raw config paths.
  • Expand tests to validate version pinning, argv behavior, Windows npx-cli.js resolution, and spawn/exit failure handling.
Show a summary per file
File Description
plugins/power-pages/scripts/launch-playwright-mcp.js Pins the MCP package version and changes process launch to node + npx-cli.js with shell: false, plus error/exit handling.
plugins/power-pages/scripts/tests/launch-playwright-mcp.test.js Updates unit tests to assert exact version pinning, raw argv config paths, resolveNpxCli() behavior, and spawn/exit failure scenarios.
plugins/power-pages/scripts/tests/mcp-config.test.js Adjusts bootstrap test strategy to intercept spawn and assert the pinned package/shell:false behavior during MCP server startup.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread plugins/power-pages/scripts/tests/mcp-config.test.js
…eadme-safety-guidance' into users/priyanshu92/fix-power-pages-mcp-root
Preserve the root-resolution security coverage from PR #382 while retaining the exact Playwright MCP pin and shell-free spawn assertions from PR #383.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 6, 2026 11:28
@priyanshu92
Priyanshu Agrawal (priyanshu92) changed the base branch from main to users/priyanshu92/fix-power-pages-mcp-root August 6, 2026 11:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- stub only npx-cli existence probes in the MCP bootstrap preload\n- assert the deterministic npm-resolution seam is exercised\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 6, 2026 11:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

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

  • launch() resolves npxCliPath via a default parameter (npxCliPath = resolveNpxCli()), so if npm isn’t present / npx-cli.js can’t be found, the error is thrown before launch() can use writeError/exitFn. That results in an uncaught exception + stack trace instead of the single-line failure output you added for spawn errors. Consider resolving inside the function body with try/catch and exiting cleanly (still allowing callers to pass npxCliPath explicitly).
function launch({
  browser = detectBrowser(),
  npxCliPath = resolveNpxCli(),
  spawnFn = spawn,
  exitFn = (code) => process.exit(code),
  writeError = (message) => process.stderr.write(message),
} = {}) {
  const child = spawnFn(process.execPath, [npxCliPath, ...buildMcpArgs(browser)], {
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- 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>
Copilot AI review requested due to automatic review settings August 6, 2026 11:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- Guard EACCES/EPERM errors while statting the declared plugin root.
- Guard EACCES/EPERM errors while statting the resolved launcher.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
Preserve guarded root and launcher stat diagnostics while retaining the pinned, shell-free Playwright MCP launcher and deterministic npm-resolution seams.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 11, 2026 07:23
- Classify an exact-parent launcher resolution as escaping the declared plugin root.
- Add deterministic coverage for the exact-parent containment boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Preserve the exact-parent containment boundary fix while retaining the pinned, shell-free Playwright MCP launcher and prior deterministic test seams.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 11, 2026 07:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Inherit current main through the lower stack while preserving the exact Playwright MCP pin, shell-free launcher, npx error handling, and hardened root-resolution coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 11, 2026 07:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (1)

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

  • This still fetches and executes @playwright/mcp at runtime via npx-cli.js without a committed lockfile or other integrity record, which conflicts with the plugin’s secure-coding requirement that runtime packages have an exact version and a committed lock / equivalent integrity boundary (and calls out scripts/launch-playwright-mcp.js specifically). Pinning the version is a good step, but consider switching to a vendored/checked-in artifact or adding a verifiable integrity mechanism (e.g., checked-in tarball + hash verification) so installs don’t execute unverified installer output during a plugin run.
  // Marketplace installs copy only this plugin directory and do not run npm install,
  // so a lockfile would not materialize a local executable. Keep the runtime package
  // immutable, and disable lifecycle scripts while npx prepares the reviewed version.
  return [
    '--yes',
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- require an absolute host-provided plugin root\n- canonicalize and contain the launcher path\n- cover invalid roots, malicious cwd, and supported hosts\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Guard EACCES/EPERM errors while statting the declared plugin root.
- Guard EACCES/EPERM errors while statting the resolved launcher.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
- Classify an exact-parent launcher resolution as escaping the declared plugin root.
- Add deterministic coverage for the exact-parent containment boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
Adopt the rebased root-hardening history and current main while preserving the exact Playwright MCP pin, shell-free launcher, npx error handling, and deterministic review coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Base automatically changed from users/priyanshu92/fix-power-pages-mcp-root to main August 11, 2026 09:39
Record the merged lower-layer history while retaining only the exact Playwright MCP pin, shell-free launcher, npx resolution/error behavior, and their tests as the main-relative PR delta.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@priyanshu92
Priyanshu Agrawal (priyanshu92) merged commit a2f3897 into main Aug 11, 2026
7 checks passed
@priyanshu92
Priyanshu Agrawal (priyanshu92) deleted the users/priyanshu92/pin-playwright-mcp branch August 11, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants