feat(mcp): default Patchright backend for Playwright MCP#1265
feat(mcp): default Patchright backend for Playwright MCP#1265atxtechbro wants to merge 1 commit intomainfrom
Conversation
…m so require('playwright') resolves to Patchright\n- Add wrapper 'patchright-playwright-mcp' to set NODE_PATH and run @playwright/mcp\n- Point mcp/mcp.json Playwright server to the wrapper\n- Provide setup script to install Patchright locally\n\nCloses #1263
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
There was a problem hiding this comment.
I've reviewed the changes for defaulting the Playwright MCP to use Patchright backend. Here's the summary of findings:
Key Concerns:
- Security: The use of
execwith unchecked arguments in the MCP wrapper script needs attention - Error Handling: Several places would benefit from more robust error handling, particularly in the setup script and shim
- Configuration: The mcp.json could use versioning for better maintainability
The overall approach of using a shim for Playwright resolution is solid, but these improvements would make the implementation more robust and maintainable. Please address the security concern as a priority.
|
|
||
| # Execute the community Playwright MCP server | ||
| exec npx @playwright/mcp@latest "$@" | ||
|
|
There was a problem hiding this comment.
🛑 [Security Concern]: The script is using exec with "$@" which could potentially allow command injection if not properly sanitized1. Consider adding input validation for the arguments before passing them to exec.
Footnotes
-
CWE-78: OS Command Injection - https://cwe.mitre.org/data/definitions/78.html ↩
| fi | ||
|
|
||
| echo "Installing patchright into $SHIM_DIR ..." | ||
| (cd "$SHIM_DIR" && npm install patchright --silent) |
There was a problem hiding this comment.
Consider adding error handling for the npm install command to provide more informative error messages if the installation fails.
| @@ -1,13 +1,11 @@ | |||
| { | |||
There was a problem hiding this comment.
Consider adding a version field to the configuration to help with future compatibility checks and migrations.
Summary
Default the Playwright MCP server to use an undetectable backend by shimming
require('playwright')to Patchright, while continuing to run the community@playwright/mcpserver.What changed
mcp/patchright-shim/node_modules/playwright/index.jsresolves Playwright to Patchright.mcp/servers/patchright-playwright-mcpsetsNODE_PATHto the shim (opt‑out withUSE_PATCHRIGHT=0|false) and executesnpx @playwright/mcp@latest.mcp/mcp.jsonnow points the Playwright server to the wrapper command.mcp/setup-patchright-shim.shinstalls thepatchrightpackage locally in the shim.Usage
USE_PATCHRIGHT=0(orfalse) before launching clients.Notes
NODE_PATH.Follow‑ups (optional)
npx patchright install chromium.mcp/servers/patchright-playwright-mcp.Closes #1263