Open
Conversation
B1: Remove duplicated authorization code (BAPScope, ScopeProfiles,
MethodScopes, hasScope, parseScopes, createAuthorizationError) from
server-playwright and import from @browseragentprotocol/protocol.
B2: Add Zod input validation schemas for the 10 most critical MCP tool
arguments (navigate, click, type, fill, press, select, hover, element,
activate_page, extract) to catch invalid inputs before they reach the
BAP client.
B3: Fix token not refreshed on reconnect in BAPClient. Store the token
separately from the URL in WebSocketTransport, reconstruct the URL with
the current token on each connect/reconnect, and add updateToken()
method to both WebSocketTransport and BAPClient.
B4: Replace inline require('crypto') and require('fs') in ESM module
server-playwright/server.ts with top-level imports using node: protocol.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all 4 blocking issues identified in code review:
B1: Remove duplicated authorization code from server-playwright --
BAPScope,ScopeProfiles,MethodScopes,hasScope(),parseScopes(), andcreateAuthorizationError()were defined inline inserver-playwright/server.tsinstead of being imported from@browseragentprotocol/protocolwhere the canonical definitions live. Removed ~120 lines of duplicated code and replaced with imports.B2: Add Zod input validation on MCP tool arguments -- Tool arguments were cast with
as string/as numberwithout validation. Added Zod validation schemas for the 10 most critical tools (navigate,click,type,fill,press,select,hover,element,activate_page,extract). Invalid arguments now throw descriptive errors before reaching the BAP client.B3: Fix token not refreshed on reconnect -- The auth token was baked into the WebSocket URL at construction time and never updated on reconnect. Refactored
WebSocketTransportto store the base URL and token separately, rebuilding the URL with the current token on eachconnect()call. AddedupdateToken()method to bothWebSocketTransportandBAPClient.B4: Replace
require()in ESM modules --require('crypto')andrequire('fs')were used inline inserver-playwright/server.ts(an ESM module). Replaced with top-levelimport { timingSafeEqual } from "node:crypto"andimport fs from "node:fs".Changes
packages/server-playwright/src/server.tsrequire()with top-level ESM imports.packages/mcp/src/index.tsvalidateArgs()helper. Applied validation to 10 tool handlers.packages/client/src/index.tsWebSocketTransportto store token separately from URL. AddedupdateToken()to both transport and client.Test plan
pnpm build-- all 7 packages build successfullypnpm typecheck-- all packages pass type checkingpnpm test-- all 420 tests pass (163 protocol + 78 client + 66 mcp + 43 server-playwright + 70 cli)updateToken()works across reconnection