chore: check sent messages against the protocol spec in tests

.claude/skills/playwright-roll/SKILL.md

@@ -241,6 +241,8 @@ The script, for `sync` and then `async`:
 1. runs `node scripts/codegen/generate_api.js --flavor <flavor>` (reads the docs from `$PW_SRC_DIR`) straight into `playwright/<flavor>_api/_generated.py`,
 2. invokes `pre-commit run --files` on the generated file.
 
+It then runs `python scripts/codegen/generate_protocol.py`, which reads `packages/protocol/spec/*.yml` from `$PW_SRC_DIR` and writes the command parameter tables to `playwright/_impl/_protocol.py`.
+
 Then run `mypy playwright` and work through the errors in `_generated.py` — each one is a call into `_impl/` that the implementation does not support yet.
 
 To see exactly what a regeneration changed in the public API, compare against the committed file:
@@ -272,6 +274,7 @@ Failure modes and fixes:
 | A kwarg in `_impl/` that the docs don't mention (e.g. legacy name from a doc rename) | Nothing fails: the generator simply never passes it. | If the param is positional with no default, just rename it in `_impl/`. Check `packages/protocol/spec/*.yml` and the server dispatcher — if the wire field renamed too, also update the channel-send dict key. Only use `extraOptions` for genuinely hidden internal kwargs (`default_browser_type`). |
 | `AttributeError: 'X' object has no attribute '_loop'` (or `_dispatcher_fiber`) at first use of a new wrapper class | The non-ChannelOwner wrapper isn't initializing the fields the generated `AsyncBase`/`SyncBase` reads. | In the wrapper's `__init__`, set `self._loop = parent._loop` and `self._dispatcher_fiber = parent._dispatcher_fiber`. |
 | `'NoneType' object has no attribute 'get'` after `send_return_as_dict` | Method's protocol response carries no fields and `send_return_as_dict` returned `None`. | `(result or {}).get(...)`. |
+| A test fails at teardown with `Protocol violations: X.y: params.z: unknown` (or `missing`, or `expected …`) | The client sends a message that the protocol spec does not allow. The driver would drop an unknown parameter without an error. | Fix the send in `_impl/`: rename the key after a wire rename, or remove a client-side option before the send. |
 | Frame/buffer payload arrives as a `str` instead of `bytes` | Protocol `binary` fields cross the wire as base64. | `base64.b64decode(value)` in the impl before exposing. |
 
 After the script settles, run `pre-commit run --all-files` once more to confirm everything is idle.
@@ -323,7 +326,7 @@ All keys use docs names: `Class.member` or `Class.member.arg`, with the upstream
 - **A cluster of rules on the same class is a smell.** If you're about to add five entries for `Foo.*`, you're almost certainly looking at a class that needs to be implemented. Implement the whole thing once and the entries disappear.
 - **Watch for revert pairs in the same range.** 1.59 added and reverted `Browser.isRemote` (#39613 / #39620) inside the same release. Walking chronologically lets you skip the add when you see the revert later.
 - **Watch for rename-revert pairs.** 1.59 had `Locator.normalize` → `Locator.toCode` (#39648) → `Locator.normalize` (#39754). Final state wins; only port the last.
-- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the channel-send dict key in `_impl/` must change. Suppressing the doc-side mismatch is hiding a real bug — the previous Python code is silently sending an unknown field that the new server ignores.
+- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the channel-send dict key in `_impl/` must change. Suppressing the doc-side mismatch is hiding a real bug — the previous Python code is silently sending an unknown field that the new server ignores. The test suite reports such a field as a protocol violation, but only in tests that send it.
 - **TypedDicts beat `Dict[str, X]` for any structured return.** When the docs describe a return as `[Object]` with named fields, name the struct for the generator, which writes its `TypedDict` into `_api_structures.py`, and re-export it from both public `__init__.py` files. Zero runtime cost (it's still a `dict`), and the docstrings expand its fields from the docs.
 - **Positional renames are free.** A param with no default before any `*` separator is positional-or-keyword in Python, but realistic call sites pass it positionally. Renaming such a param doesn't break callers.
 - **The "Backport changes" GitHub issue can be misleading.** In the 1.59 roll its checkboxes were all marked `[x]` with annotations like "✅ IMPLEMENTED", but several of those features had not actually been merged into the Python port. Trust the `docs/src/api/` walk over the issue.

.pre-commit-config.yaml

@@ -22,7 +22,7 @@ repos:
     rev: v1.17.0
     hooks:
       - id: mypy
-        additional_dependencies: [types-pyOpenSSL==24.1.0.20240722]
+        additional_dependencies: [types-pyOpenSSL==24.1.0.20240722, types-PyYAML==6.0.12.20260906]
   - repo: https://github.com/pycqa/flake8
     rev: 7.3.0
     hooks:

CLAUDE.md

@@ -12,6 +12,7 @@ Python bindings for [Playwright](https://playwright.dev). The Python client talk
 - `playwright/async_api/_generated.py`, `playwright/sync_api/_generated.py`, and the public TypedDicts in `playwright/_impl/_api_structures.py` — **auto-generated** from the upstream API docs. Never edit by hand; rerun `./scripts/update_api.sh` after a roll or after changing `scripts/codegen/`.
 - `scripts/codegen/generate_api.js` — the generator. A Node.js script that loads the upstream doc parser (`utils/doclint/api_parser.js`) from `$PW_SRC_DIR`, filters the docs for Python, and writes the whole public API: signatures, types, docstrings, event overloads, the calls into `_impl/`, and the TypedDicts. It does not read `_impl/`.
 - `scripts/codegen/python_rules.js` — the Python facts that the docs do not contain (positional options, struct `total=`, hidden members and options). Policy: the shipped Python API wins; each entry is a candidate for an upstream docs fix, and the generator fails on entries that no longer match anything. Every entry needs a one-line rationale comment above it.
+- `playwright/_impl/_protocol.py` — **auto-generated** by `scripts/codegen/generate_protocol.py` (run by `./scripts/update_api.sh`) from the upstream protocol spec: the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
 - `scripts/codegen/compare_api.py` — compares two generated files (signatures, annotations, call bodies, docstrings) and lists every difference. Use it to check that a generator change does not change the API.
 - `tests/async/`, `tests/sync/` — pytest suites. Most new tests are added to the async file with a sync mirror.
 - `DRIVER_VERSION` — the single source of truth for which Playwright release the driver is assembled from (one line, the `playwright-core` npm version, e.g. `1.61.0`, no `v` prefix). Read by `setup.py`, `scripts/build_driver.py`, and CI. The wheel build downloads `playwright-core` at this version from npm plus the matching Node.js binary and assembles the per-platform bundles — no source build. The version is baked into the staged bundle filenames (`driver/playwright-<version>-<suffix>.zip`), so it doubles as the build cache key.

local-requirements.txt

@@ -11,6 +11,8 @@ pytest==9.0.3
 pytest-asyncio==1.4.0
 pytest-rerunfailures==16.7
 pytest-timeout==2.4.0
+PyYAML==6.0.3
 service_identity==24.2.0
 twisted==25.5.0
 types-pyOpenSSL==24.1.0.20240722
+types-PyYAML==6.0.12.20260906

playwright/_impl/_browser.py

@@ -275,11 +275,12 @@ async def start_tracing(
         categories: Sequence[str] = None,
     ) -> None:
         params = locals_to_params(locals())
+        # The client writes the trace to `path` in stop_tracing().
+        params.pop("path", None)
         if page:
             params["page"] = page._channel
         if path:
             self._cr_tracing_path = str(path)
-            params["path"] = str(path)
         await self._channel.send("startTracing", None, params)
 
     async def stop_tracing(self) -> bytes:

playwright/_impl/_connection.py

@@ -15,7 +15,6 @@
 import asyncio
 import collections.abc
 import contextvars
-import datetime
 import inspect
 import random
 import string
@@ -49,6 +48,7 @@
     create_task_and_ignore_exception,
     parse_error,
 )
+from playwright._impl._protocol_check import check_message, is_enabled
 from playwright._impl._transport import Transport
 
 if TYPE_CHECKING:
@@ -326,6 +326,7 @@ def __init__(
         self._local_utils: Optional["LocalUtils"] = local_utils
         self._tracing_count = 0
         self._closed_error: Optional[Exception] = None
+        self._check_protocol = is_enabled()
 
     @property
     def local_utils(self) -> "LocalUtils":
@@ -427,14 +428,12 @@ def _send_message_to_server(
             if frames
             else None
         )
-        metadata = {
-            "wallTime": int(datetime.datetime.now().timestamp() * 1000),
-            "apiName": stack_trace_information["apiName"],
+        metadata: Dict[str, Any] = {
             "internal": not stack_trace_information["apiName"],
             "timeout": timeout,
         }
         if location:
-            metadata["location"] = location  # type: ignore
+            metadata["location"] = location
         title = stack_trace_information["title"]
         if title:
             metadata["title"] = title
@@ -445,6 +444,14 @@ def _send_message_to_server(
             "params": self._replace_channels_with_guids(params),
             "metadata": metadata,
         }
+        if self._check_protocol:
+            check_message(
+                object._type,
+                method,
+                message["params"],
+                metadata,
+                self._type_of_guid,
+            )
         if self._tracing_count > 0 and frames and object._guid != "localUtils":
             self.local_utils.add_stack_to_tracing_no_reply(id, frames)
 
@@ -453,6 +460,10 @@ def _send_message_to_server(
 
         return callback
 
+    def _type_of_guid(self, guid: str) -> Optional[str]:
+        object = self._objects.get(guid)
+        return object._type if object else None
+
     async def _abort(
         self, object: ChannelOwner, callback: ProtocolCallback, reason: str
     ) -> None:

playwright/_impl/_frame.py

@@ -650,7 +650,9 @@ async def _fill(
         force: bool = None,
         title: str = None,
     ) -> None:
-        await self._channel.send("fill", self._timeout, locals_to_params(locals()))
+        params = locals_to_params(locals())
+        params.pop("title", None)
+        await self._channel.send("fill", self._timeout, params, title=title)
 
     def locator(
         self,
@@ -938,6 +940,7 @@ async def wait_for_function(
         if isinstance(polling, str) and polling != "raf":
             raise Error(f"Unknown polling option: {polling}")
         params = locals_to_params(locals())
+        params.pop("polling", None)
         params["arg"] = serialize_argument(arg)
         if polling is not None and polling != "raf":
             params["pollingInterval"] = polling

playwright/_impl/_helper.py

@@ -31,14 +31,12 @@
     Optional,
     Pattern,
     Set,
-    Tuple,
     TypedDict,
     TypeVar,
     Union,
     cast,
     overload,
 )
-from urllib.parse import ParseResult, urljoin, urlparse, urlunparse
 
 from playwright._impl._errors import (
     Error,
@@ -47,16 +45,15 @@
     is_target_closed_error,
     rewrite_error,
 )
-from playwright._impl._glob import glob_to_regex_pattern
 from playwright._impl._greenlets import RouteGreenlet
 from playwright._impl._internal_structures import NameValue
 from playwright._impl._str_utils import escape_regex_flags
+from playwright._impl._url_match import URLMatch, url_matches
 
 if TYPE_CHECKING:  # pragma: no cover
     from playwright._impl._internal_structures import HeadersArray
     from playwright._impl._network import Request, Response, Route, WebSocketRoute
 
-URLMatch = Union[str, Pattern[str], Callable[[str], bool]]
 URLMatchRequest = Union[
     str, Pattern[str], Callable[["Request"], Union[bool, Awaitable[bool]]]
 ]
@@ -153,133 +150,6 @@ class FrameNavigatedEvent(TypedDict):
 Env = Dict[str, Union[str, float, bool]]
 
 
-def url_matches(
-    base_url: Optional[str],
-    url_string: str,
-    match: Optional[URLMatch],
-    websocket_url: bool = None,
-) -> bool:
-    if not match:
-        return True
-    if isinstance(match, str):
-        match = re.compile(
-            resolve_glob_to_regex_pattern(base_url, match, websocket_url)
-        )
-    if isinstance(match, Pattern):
-        return bool(match.search(url_string))
-    return match(url_string)
-
-
-def resolve_glob_to_regex_pattern(
-    base_url: Optional[str], glob: str, websocket_url: bool = None
-) -> str:
-    if websocket_url:
-        base_url = to_websocket_base_url(base_url)
-    glob = resolve_glob_base(base_url, glob)
-    return glob_to_regex_pattern(glob)
-
-
-def to_websocket_base_url(base_url: Optional[str]) -> Optional[str]:
-    if base_url is not None and re.match(r"^https?://", base_url):
-        base_url = re.sub(r"^http", "ws", base_url)
-    return base_url
-
-
-def resolve_glob_base(base_url: Optional[str], match: str) -> str:
-    if match[0] == "*":
-        return match
-
-    token_map: Dict[str, str] = {}
-
-    def map_token(original: str, replacement: str) -> str:
-        if len(original) == 0:
-            return ""
-        token_map[replacement] = original
-        return replacement
-
-    # Escaped `\\?` behaves the same as `?` in our glob patterns.
-    match = match.replace(r"\\?", "?")
-    # Special case about: URLs as they are not relative to base_url
-    if (
-        match.startswith("about:")
-        or match.startswith("data:")
-        or match.startswith("chrome:")
-        or match.startswith("edge:")
-        or match.startswith("file:")
-    ):
-        # about: and data: URLs are not relative to base_url, so we return them as is.
-        return match
-    # Glob symbols may be escaped in the URL and some of them such as ? affect resolution,
-    # so we replace them with safe components first.
-    processed_parts = []
-    for index, token in enumerate(match.split("/")):
-        if token in (".", "..", ""):
-            processed_parts.append(token)
-            continue
-        # Handle special case of http*://, note that the new schema has to be
-        # a web schema so that slashes are properly inserted after domain.
-        if index == 0 and token.endswith(":"):
-            # Replace any pattern with http:
-            if "*" in token or "{" in token:
-                processed_parts.append(map_token(token, "http:"))
-            else:
-                # Preserve explicit schema as is as it may affect trailing slashes after domain.
-                processed_parts.append(token)
-            continue
-        question_index = token.find("?")
-        if question_index == -1:
-            processed_parts.append(map_token(token, f"$_{index}_$"))
-        else:
-            new_prefix = map_token(token[:question_index], f"$_{index}_$")
-            new_suffix = map_token(token[question_index:], f"?$_{index}_$")
-            processed_parts.append(new_prefix + new_suffix)
-
-    relative_path = "/".join(processed_parts)
-    resolved, case_insensitive_part = resolve_base_url(base_url, relative_path)
-
-    for token, original in token_map.items():
-        normalize = case_insensitive_part and token in case_insensitive_part
-        resolved = resolved.replace(
-            token, original.lower() if normalize else original, 1
-        )
-
-    return resolved
-
-
-def resolve_base_url(
-    base_url: Optional[str], given_url: str
-) -> Tuple[str, Optional[str]]:
-    try:
-        url = nodelike_urlparse(
-            urljoin(base_url if base_url is not None else "", given_url)
-        )
-        resolved = urlunparse(url)
-        # Schema and domain are case-insensitive.
-        hostname_port = (
-            url.hostname or ""
-        )  # can't use parsed.netloc because it includes userinfo (username:password)
-        if url.port:
-            hostname_port += f":{url.port}"
-        case_insensitive_prefix = f"{url.scheme}://{hostname_port}"
-        return resolved, case_insensitive_prefix
-    except Exception:
-        return given_url, None
-
-
-def nodelike_urlparse(url: str) -> ParseResult:
-    parsed = urlparse(url, allow_fragments=True)
-
-    # https://url.spec.whatwg.org/#special-scheme
-    is_special_url = parsed.scheme in ["http", "https", "ws", "wss", "ftp", "file"]
-    if is_special_url:
-        # special urls have a list path, list paths are serialized as follows: https://url.spec.whatwg.org/#url-path-serializer
-        # urllib diverges, so we patch it here
-        if parsed.path == "":
-            parsed = parsed._replace(path="/")
-
-    return parsed
-
-
 class HarLookupResult(TypedDict, total=False):
     action: Literal["error", "redirect", "fulfill", "noentry"]
     message: Optional[str]

playwright/_impl/_input.py

@@ -70,9 +70,9 @@ async def _click(
         clickCount: int = None,
         title: str = None,
     ) -> None:
-        await self._channel.send(
-            "mouseClick", None, locals_to_params(locals()), title=title
-        )
+        params = locals_to_params(locals())
+        params.pop("title", None)
+        await self._channel.send("mouseClick", None, params, title=title)
 
     async def click(
         self,