close
Skip to content

[py] Add WebDriver cookie type annotations#17537

Open
adamtheturtle wants to merge 1 commit into
SeleniumHQ:trunkfrom
adamtheturtle:codex/type-webdriver-cookies
Open

[py] Add WebDriver cookie type annotations#17537
adamtheturtle wants to merge 1 commit into
SeleniumHQ:trunkfrom
adamtheturtle:codex/type-webdriver-cookies

Conversation

@adamtheturtle
Copy link
Copy Markdown
Contributor

Summary

Add a shared WebDriver Cookie TypedDict and use it for cookie-related Python APIs.

Details

WebDriver.get_cookies() and get_cookie() previously returned bare dict types, which leaves type checkers with unknown key and value types. This adds a small shared cookie dictionary type and applies it to:

  • WebDriver.get_cookies()
  • WebDriver.get_cookie()
  • WebDriver.add_cookie()
  • API request context cookie synchronization helpers

This keeps cookie typing consistent between WebDriver and the browser-cookie synchronization code.

Validation

  • uv run --with ruff ruff check --config py/pyproject.toml py/selenium/webdriver/remote/webdriver.py py/selenium/webdriver/common/api_request_context.py py/selenium/webdriver/common/cookie.py
  • python3 -m compileall -q py/selenium/webdriver/remote/webdriver.py py/selenium/webdriver/common/api_request_context.py py/selenium/webdriver/common/cookie.py
  • uv run --with-requirements py/requirements.txt python py/run_mypy.py

@selenium-ci selenium-ci added the C-py Python Bindings label May 21, 2026
@adamtheturtle adamtheturtle marked this pull request as ready for review May 21, 2026 09:09
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add WebDriver cookie type annotations with TypedDict

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add shared Cookie TypedDict for WebDriver cookie APIs
• Apply cookie type annotations to get/add cookie methods
• Update API request context to use typed cookies
• Improve type safety for cookie-related operations
Diagram
flowchart LR
  CookieType["Cookie TypedDict<br/>name, value, path, domain, etc."]
  WebDriver["WebDriver Methods<br/>get_cookies, get_cookie, add_cookie"]
  APIContext["APIRequestContext<br/>cookie synchronization"]
  CookieType -- "applied to" --> WebDriver
  CookieType -- "applied to" --> APIContext

Loading

File Changes

1. py/selenium/webdriver/common/cookie.py ✨ Enhancement +31/-0

Create Cookie TypedDict definition

• New file defining Cookie TypedDict with required and optional fields
• Includes name, value, path, domain, secure, httpOnly, expiry, sameSite
• Uses NotRequired for optional cookie attributes

py/selenium/webdriver/common/cookie.py


2. py/selenium/webdriver/remote/webdriver.py ✨ Enhancement +4/-3

Apply Cookie type to WebDriver methods

• Import new Cookie type from common module
• Update get_cookies() return type from list[dict] to list[Cookie]
• Update get_cookie() return type from dict | None to Cookie | None
• Add type annotation to add_cookie() parameter as Cookie
• Add type annotation to get_cookie() name parameter as str

py/selenium/webdriver/remote/webdriver.py


3. py/selenium/webdriver/common/api_request_context.py ✨ Enhancement +11/-9

Apply Cookie type to API request context

• Import Cookie type from common module
• Update _cookie_matches() parameter type from dict to Cookie
• Update _parse_set_cookie() return type from dict to Cookie
• Update _get_cookies_for_request() return type to list[Cookie]
• Update internal cookie storage and initialization to use Cookie type
• Update _IsolatedAPIRequestContext constructor and methods with Cookie types

py/selenium/webdriver/common/api_request_context.py


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 21, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1)

Grey Divider


Action required

1. get_cookies() return type narrowed 📘 Rule violation ⚙ Maintainability
Description
The public WebDriver cookie APIs now use Cookie/list[Cookie] instead of dict/list[dict],
which is not type-compatible for existing typed callers (e.g., list invariance makes
list[Cookie] not assignable to list[dict]). This can force downstream users to change type
annotations/code to satisfy type checkers, violating compatibility expectations for user-facing
APIs.
Code

py/selenium/webdriver/remote/webdriver.py[693]

Evidence
PR Compliance ID 1 requires user-facing APIs remain backward-compatible. The PR changes WebDriver
cookie method type annotations to Cookie/list[Cookie], which is not type-compatible with the
prior dict/list[dict] annotations for many typed callers (e.g., list invariance), potentially
forcing downstream code changes.

AGENTS.md: Maintain API/ABI Compatibility for User-Facing Code Changes
py/selenium/webdriver/remote/webdriver.py[693-700]
py/selenium/webdriver/remote/webdriver.py[702-720]
py/selenium/webdriver/remote/webdriver.py[741-759]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Public cookie-related WebDriver APIs were re-annotated from `dict`/`list[dict]` to `Cookie`/`list[Cookie]`. This is a user-facing interface change that can break downstream static type checking (notably because `list` is invariant), requiring callers to update their types.

## Issue Context
This PR’s goal is better cookie typing, but the compliance requirement is to keep user-facing APIs backward-compatible. Consider using a more compatible annotation strategy (e.g., keeping return/param types as `dict[str, Any]` / `Mapping[str, Any]` in the public API while using `Cookie` internally or via helper casts/types), so existing typed call sites don’t require changes.

## Fix Focus Areas
- py/selenium/webdriver/remote/webdriver.py[693-703]
- py/selenium/webdriver/remote/webdriver.py[741-742]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Cookie missing required keys 🐞 Bug ≡ Correctness
Description
Cookie is declared with TypedDict(total=False), making name and value optional and allowing
{} to type-check as a Cookie. This defeats the goal of cookie typing and can let invalid cookies
flow into WebDriver.add_cookie(), which requires name/value and will fail at runtime when
given incomplete data.
Code

py/selenium/webdriver/common/cookie.py[R21-31]

Evidence
The Cookie type is defined with total=False, which makes all keys optional. This is relied on by
_parse_set_cookie() returning {} while annotated as returning Cookie; and it conflicts with
WebDriver.add_cookie()'s contract that name and value are required.

py/selenium/webdriver/common/cookie.py[21-31]
py/selenium/webdriver/common/api_request_context.py[156-216]
py/selenium/webdriver/remote/webdriver.py[741-748]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Cookie` is currently defined as `TypedDict(total=False)`, which makes **all** keys optional (including `name` and `value`). This allows invalid cookies (including `{}`) to type-check, undermining the purpose of introducing the shared cookie type.

## Issue Context
- `WebDriver.add_cookie()` documents that `name` and `value` are required.
- `_parse_set_cookie()` currently returns `{}` on parse failure while being annotated to return `Cookie`, which only type-checks because `Cookie` has no required keys.

## Fix Focus Areas
- py/selenium/webdriver/common/cookie.py[21-31]
- py/selenium/webdriver/common/api_request_context.py[156-216]
- py/selenium/webdriver/remote/webdriver.py[741-760]

## Proposed fix
1. Make `Cookie` require `name` and `value` by removing `total=False` (default `total=True`) and keeping the other keys as `NotRequired[...]`.
2. Update `_parse_set_cookie()` to return `Cookie | None` (or introduce a separate permissive type like `ParsedCookie` / `PartialCookie`) and adjust callers to handle the non-cookie case without relying on `{}` being a `Cookie`.
3. Ensure any code path calling `add_cookie()` only passes a `Cookie` that includes `name` and `value`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

self.execute(Command.REFRESH)

def get_cookies(self) -> list[dict]:
def get_cookies(self) -> list[Cookie]:
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.

Action required

1. get_cookies() return type narrowed 📘 Rule violation ⚙ Maintainability

The public WebDriver cookie APIs now use Cookie/list[Cookie] instead of dict/list[dict],
which is not type-compatible for existing typed callers (e.g., list invariance makes
list[Cookie] not assignable to list[dict]). This can force downstream users to change type
annotations/code to satisfy type checkers, violating compatibility expectations for user-facing
APIs.
Agent Prompt
## Issue description
Public cookie-related WebDriver APIs were re-annotated from `dict`/`list[dict]` to `Cookie`/`list[Cookie]`. This is a user-facing interface change that can break downstream static type checking (notably because `list` is invariant), requiring callers to update their types.

## Issue Context
This PR’s goal is better cookie typing, but the compliance requirement is to keep user-facing APIs backward-compatible. Consider using a more compatible annotation strategy (e.g., keeping return/param types as `dict[str, Any]` / `Mapping[str, Any]` in the public API while using `Cookie` internally or via helper casts/types), so existing typed call sites don’t require changes.

## Fix Focus Areas
- py/selenium/webdriver/remote/webdriver.py[693-703]
- py/selenium/webdriver/remote/webdriver.py[741-742]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +21 to +31
class Cookie(TypedDict, total=False):
"""A WebDriver cookie dictionary."""

name: str
value: str
path: NotRequired[str]
domain: NotRequired[str]
secure: NotRequired[bool]
httpOnly: NotRequired[bool]
expiry: NotRequired[int]
sameSite: NotRequired[str]
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.

Action required

2. Cookie missing required keys 🐞 Bug ≡ Correctness

Cookie is declared with TypedDict(total=False), making name and value optional and allowing
{} to type-check as a Cookie. This defeats the goal of cookie typing and can let invalid cookies
flow into WebDriver.add_cookie(), which requires name/value and will fail at runtime when
given incomplete data.
Agent Prompt
## Issue description
`Cookie` is currently defined as `TypedDict(total=False)`, which makes **all** keys optional (including `name` and `value`). This allows invalid cookies (including `{}`) to type-check, undermining the purpose of introducing the shared cookie type.

## Issue Context
- `WebDriver.add_cookie()` documents that `name` and `value` are required.
- `_parse_set_cookie()` currently returns `{}` on parse failure while being annotated to return `Cookie`, which only type-checks because `Cookie` has no required keys.

## Fix Focus Areas
- py/selenium/webdriver/common/cookie.py[21-31]
- py/selenium/webdriver/common/api_request_context.py[156-216]
- py/selenium/webdriver/remote/webdriver.py[741-760]

## Proposed fix
1. Make `Cookie` require `name` and `value` by removing `total=False` (default `total=True`) and keeping the other keys as `NotRequired[...]`.
2. Update `_parse_set_cookie()` to return `Cookie | None` (or introduce a separate permissive type like `ParsedCookie` / `PartialCookie`) and adjust callers to handle the non-cookie case without relying on `{}` being a `Cookie`.
3. Ensure any code path calling `add_cookie()` only passes a `Cookie` that includes `name` and `value`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants