[feat]: add cookie related getters & setters#1681
Conversation
🦋 Changeset detectedLatest commit: e6e3e6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Greptile SummaryAdded native cookie management to Stagehand's V3 API with Key highlights:
The PR resolves the gap in Stagehand's API by providing direct cookie access without requiring raw CDP commands or page-level JavaScript injection. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User calls context.addCookies] --> B[normalizeCookieParams validates input]
B --> C{Valid?}
C -->|No| D[Throw CookieValidationError]
C -->|Yes| E[Convert to CDP format via toCdpCookieParam]
E --> F[Batch send via Storage.setCookies]
F --> G{CDP Success?}
G -->|No| H[Throw CookieSetError]
G -->|Yes| I[Cookies set successfully]
J[User calls context.cookies] --> K[Fetch via Storage.getCookies]
K --> L[Map CDP cookies to Cookie type]
L --> M{URLs provided?}
M -->|Yes| N[filterCookies by domain/path/secure]
M -->|No| O[Return all cookies]
N --> O
P[User calls context.clearCookies] --> Q{Filter options?}
Q -->|No| R[Atomic Storage.clearCookies]
Q -->|Yes| S[Fetch all cookies via context.cookies]
S --> T[Filter with cookieMatchesFilter]
T --> U{Any to delete?}
U -->|No| V[Skip operation]
U -->|Yes| W[Storage.clearCookies wipes all]
W --> X{Any to keep?}
X -->|Yes| Y[Re-add kept cookies via Storage.setCookies]
X -->|No| Z[Done]
Y --> Z
Last reviewed commit: 810b7c6 |
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 3/5
- The cookie validation in
packages/core/lib/v3/understudy/cookies.tsmisses thesecureundefined case, which can allowsameSite: "None"cookies without secure to slip through and behave unexpectedly for users. - The cookie clearing flow in
packages/core/lib/v3/understudy/context.tsis O(N) roundtrips with a race window; this can leave cookies behind or impact performance when no filter is provided. - These are medium-severity, user-impacting behaviors, so there’s some risk despite the changes being localized.
- Pay close attention to
packages/core/lib/v3/understudy/cookies.tsandpackages/core/lib/v3/understudy/context.ts- cookie validation and clearing behavior.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/understudy/cookies.ts">
<violation number="1" location="packages/core/lib/v3/understudy/cookies.ts:123">
P1: Bug: `copy.secure === false` misses the `undefined` case. When a user sets `sameSite: "None"` with `domain`+`path` but omits `secure`, this validation is skipped because `undefined === false` is `false`. Use `!copy.secure` to also catch `undefined`, matching the stated intent of preventing silent CDP failures.</violation>
</file>
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:892">
P2: When no filter is provided, use `Network.clearBrowserCookies` instead of fetching all cookies and deleting them one by one. The current approach makes O(N) CDP roundtrips and still has a race condition (cookies set between the fetch and the deletion loop will be missed). `Network.clearBrowserCookies` is a single atomic CDP call.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User
participant V3 as Stagehand (V3)
participant Ctx as V3Context
participant Util as Cookie Utils
participant Browser as Browser (CDP)
Note over User, Browser: Cookie Management Flow
User->>V3: NEW: addCookies(cookies)
V3->>Ctx: addCookies(cookies)
Ctx->>Util: NEW: normalizeCookieParams()
Note right of Util: Validates url OR domain/path<br/>Enforces sameSite=None + secure
Util-->>Ctx: Normalized CookieParams
loop Each Cookie
Ctx->>Browser: Network.setCookie
Browser-->>Ctx: { success: boolean }
alt NEW: Browser rejected cookie
Ctx-->>User: Throw Error (Explicit failure)
end
end
Ctx-->>User: Done
User->>V3: NEW: cookies(urls?)
V3->>Ctx: cookies(urls)
Ctx->>Browser: Network.getAllCookies
Browser-->>Ctx: Protocol.Network.Cookie[]
Ctx->>Util: NEW: filterCookies(cookies, urls)
Note right of Util: Matches domain, path, and<br/>secure protocol requirements
Util-->>Ctx: Filtered Cookie[]
Ctx-->>User: Cookie[]
User->>V3: NEW: clearCookies(options?)
V3->>Ctx: clearCookies(options)
Ctx->>Ctx: cookies()
alt Selective Clear (options provided)
loop Each Cookie
Ctx->>Util: NEW: cookieMatchesFilter(cookie, options)
Util-->>Ctx: boolean
opt Match Found
Ctx->>Browser: CHANGED: Network.deleteCookies
end
end
else Clear All
Ctx->>Browser: Network.deleteCookies (All)
end
Ctx-->>User: Done
Note over User, Browser: Storage Persistence Flow
User->>Ctx: NEW: storageState()
Ctx->>Ctx: cookies()
Ctx-->>User: { cookies: Cookie[] }
User->>Ctx: NEW: setStorageState(state)
Ctx->>Ctx: clearCookies()
Ctx->>Util: Filter expired cookies
Ctx->>Ctx: addCookies(validCookies)
Ctx-->>User: Context Restored
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 6 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant User as User/Script
participant SDK as V3Context
participant Helpers as Cookie Helpers
participant CDP as Browser (CDP)
Note over User,CDP: NEW: Cookie Management Flow
rect rgb(23, 37, 84)
Note right of User: Get Cookies
User->>SDK: cookies(urls?)
SDK->>CDP: Network.getAllCookies
CDP-->>SDK: List of Protocol.Network.Cookie
SDK->>Helpers: filterCookies(mappedCookies, urlList)
Helpers-->>SDK: Filtered Cookie[]
SDK-->>User: Return cookies
end
rect rgb(5, 46, 22)
Note right of User: Add Cookies
User->>SDK: addCookies(CookieParam[])
SDK->>Helpers: NEW: normalizeCookieParams()
Note over Helpers: Validates url/domain pairs<br/>Enforce Secure for SameSite=None
Helpers-->>SDK: Normalized Params
loop For each cookie
SDK->>CDP: Network.setCookie(params)
CDP-->>SDK: { success: boolean }
alt NEW: Browser Rejected (success: false)
SDK-->>User: Throw Error (Validation/CDP failure)
end
end
SDK-->>User: Success
end
rect rgb(69, 26, 3)
Note right of User: Clear Cookies
User->>SDK: clearCookies(options?)
alt Options contains name/domain/path filter
SDK->>SDK: internal: cookies()
SDK->>Helpers: NEW: cookieMatchesFilter()
loop For matching cookies
SDK->>CDP: Network.deleteCookies({ name, domain, path })
end
else No filters (Clear All)
SDK->>CDP: CHANGED: Network.clearBrowserCookies (Atomic wipe)
end
CDP-->>SDK: Done
SDK-->>User: Success
end
classDef highlight fill:#172554,stroke:#333,stroke-width:2px;
class SDK,Helpers highlight;
3284e7d to
b222fd9
Compare
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:856">
P2: Stale JSDoc: the comment says "we check the CDP success flag" but the implementation now uses try/catch on `Storage.setCookies`, which doesn't return a success flag. Update the comment to reflect the actual error-handling mechanism (exception-based, not flag-based).</violation>
<violation number="2" location="packages/core/lib/v3/understudy/context.ts:900">
P2: Race condition: selective `clearCookies` now nukes all cookies then re-adds keepers, creating a window where all cookies (including ones to keep) are temporarily deleted. If the page sets cookies between the clear and re-add, those will be permanently lost. Consider documenting this limitation on the public API or exploring whether `Network.deleteCookies` can be used as a fallback for targeted deletes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
98a1da8 to
f88cced
Compare
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/tests/cookies.spec.ts">
<violation number="1" location="packages/core/lib/v3/tests/cookies.spec.ts:154">
P2: Test navigates to the same URL instead of a different path. The URL on line 157 is identical to `BASE_URL`, so this test doesn't verify what the description claims (cookie persistence across navigation to a *different* path). Consider navigating to a genuinely different path, e.g. the root of the domain.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 3/5
- There is a concrete risk of partial cookie persistence in
packages/core/lib/v3/understudy/context.tsif one cookie fails mid-loop, which could leave inconsistent state for users. - The same loop also performs O(N) CDP roundtrips, which could impact performance and makes the change less predictable under load.
- Pay close attention to
packages/core/lib/v3/understudy/context.ts- cookie batching/rollback behavior and per-cookie CDP calls.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:879">
P2: Partial application on error: cookies are sent one-at-a-time, so if cookie N fails, cookies 1…N-1 are already persisted with no rollback. This also causes O(N) CDP roundtrips. Consider batching all cookies in a single `Storage.setCookies` call (as `clearCookies` already does for re-adding kept cookies) for both atomicity and performance.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User Code
participant Context as V3Context
participant Helper as Cookie Helper
participant CDP as Browser (CDP)
Note over User,CDP: NEW: Cookie Management Flow
rect rgb(23, 37, 84)
Note right of User: context.addCookies(cookies)
User->>Context: addCookies(cookies)
Context->>Helper: NEW: normalizeCookieParams()
Helper->>Helper: Validate (URL vs Domain/Path)
Helper->>Helper: NEW: Enforce sameSite:None + Secure
alt Validation Failed
Helper-->>Context: CookieValidationError
Context-->>User: Error
else Valid
Helper-->>Context: Normalized Cookies
loop for each cookie
Context->>CDP: CHANGED: Storage.setCookies
CDP-->>Context: Result
end
Context-->>User: void
end
end
rect rgb(5, 46, 22)
Note right of User: context.cookies(urls)
User->>Context: cookies(urls)
Context->>CDP: Storage.getCookies
CDP-->>Context: raw cookies
Context->>Helper: NEW: filterCookies(urlList)
Helper-->>Context: matched cookies
Context-->>User: Cookie[]
end
rect rgb(69, 26, 3)
Note right of User: context.clearCookies(options)
User->>Context: clearCookies(options)
alt No filter (Selective clearing is false)
Context->>CDP: Storage.clearCookies (Atomic)
CDP-->>Context: ok
else With filters (Selective clearing)
Context->>Context: Fetch current cookies (see above)
Context->>Helper: NEW: cookieMatchesFilter()
Helper-->>Context: toKeep[]
Context->>CDP: Storage.clearCookies (Wipe all)
opt toKeep.length > 0
Context->>CDP: Storage.setCookies(toKeep)
end
end
Context-->>User: void
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 10 files
Confidence score: 3/5
- Potential user-impacting risk: in
packages/core/lib/v3/understudy/context.ts, a failure inStorage.setCookiesafterStorage.clearCookiescan permanently drop cookies meant to be retained. - Overall risk is moderate because this is a concrete failure path in cookie handling, though it may be mitigated with error handling.
- Pay close attention to
packages/core/lib/v3/understudy/context.ts- cookie clear/add sequence can lose data if re-add fails.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:923">
P2: If `Storage.setCookies` fails after `Storage.clearCookies` succeeds, all cookies that should have been kept are permanently lost. Consider wrapping the re-add in a try/catch that logs a warning or rethrows with a clear message about the partial failure, so callers know the state is now inconsistent.</violation>
<violation number="2" location="packages/core/lib/v3/understudy/context.ts:936">
P2: The cookie-to-CDP-param mapping is duplicated between `addCookies` and `clearCookies`. Extract a shared helper (e.g., `toCdpCookieParam`) in `cookies.ts` to avoid divergence if the mapping logic changes.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User / Automation
participant Context as V3Context
participant Utils as Cookie Helpers (Internal)
participant CDP as Browser (CDP Session)
Note over User,CDP: NEW: Cookie Management Flow
rect rgb(23, 37, 84)
Note right of User: context.cookies(urls?)
User->>Context: cookies(urls)
Context->>CDP: Storage.getCookies
CDP-->>Context: Protocol.Network.Cookie[]
Context->>Utils: filterCookies(mapped, urls)
Note over Utils: Matches domain/path/secure<br/>attributes against URL(s)
Utils-->>Context: Cookie[]
Context-->>User: Cookie[]
end
rect rgb(69, 26, 3)
Note right of User: context.addCookies(cookies)
User->>Context: addCookies(CookieParam[])
Context->>Utils: normalizeCookieParams()
alt NEW: Invalid Configuration
Note over Utils: e.g. sameSite="None" without secure: true
Utils-->>Context: throw CookieValidationError
Context-->>User: Error
else Valid Configuration
Utils-->>Context: Normalized CookieParam[]
Context->>CDP: Storage.setCookies(cdpCookies)
alt CDP Rejection
CDP-->>Context: Error response
Context-->>User: throw CookieSetError
else Success
CDP-->>Context: { success: true }
Context-->>User: void
end
end
end
rect rgb(5, 46, 22)
Note right of User: context.clearCookies(options?)
User->>Context: clearCookies(options)
alt NEW: Selective Clear (name/domain/path filters)
Context->>Context: cookies() [Fetch current state]
Context->>Utils: Filter cookies to keep (cookieMatchesFilter)
Context->>CDP: Storage.clearCookies [Atomic Wipe]
opt Has cookies to preserve
Context->>CDP: Storage.setCookies [Re-add non-matching]
end
else NEW: Global Clear (no options)
Context->>CDP: Storage.clearCookies [Atomic Wipe]
end
CDP-->>Context: OK
Context-->>User: void
end
classDef highlight fill:#1e293b,stroke:#333,stroke-width:2px;
class User,Context,Utils,CDP highlight;
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
1 similar comment
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 10 files
Confidence score: 3/5
- Node ESM runtime risk:
packages/core/tests/cookies.test.tsuses relative imports without.js, which can fail under strict ESM resolution. - Behavioral inconsistency in
packages/core/lib/v3/understudy/cookies.ts: malformed URLs throw a rawTypeErrorinstead ofCookieValidationError, which could leak unexpected errors. - Some risk of data loss in
packages/core/lib/v3/understudy/context.tswhereclearCookiesdrops CDP-specific metadata during the clear/restore cycle. - Pay close attention to
packages/core/tests/cookies.test.ts,packages/core/lib/v3/understudy/cookies.ts,packages/core/lib/v3/understudy/context.ts- ESM import resolution, error handling consistency, and cookie metadata preservation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/cookies.ts">
<violation number="1" location="packages/core/lib/v3/understudy/cookies.ts:79">
P2: Malformed URL strings passed to `new URL()` will throw a raw `TypeError` instead of the `CookieValidationError` used for all other validation failures. Wrap the `new URL(copy.url)` call in a try-catch to provide a consistent error type and a clear message (e.g., `Cookie "<name>" has an invalid url: "<url>"`).
The same concern applies to `filterCookies` where `new URL(u)` is also unguarded.</violation>
</file>
<file name="packages/core/tests/cookies.test.ts">
<violation number="1" location="packages/core/tests/cookies.test.ts:6">
P2: All four relative imports are missing the `.js` extension, breaking the codebase convention used by every other test file. This can cause runtime failures under Node ESM resolution (which requires explicit extensions).
(Based on your team's feedback about requiring explicit .js extensions on all relative ESM import specifiers.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/core/lib/v3/understudy/context.ts">
<violation number="1" location="packages/core/lib/v3/understudy/context.ts:920">
P2: Filtered `clearCookies` silently strips CDP-specific cookie metadata (e.g., `priority`, `sourceScheme`, `sourcePort`, `partitionKey`) from non-matching cookies during the clear-and-restore cycle. Consider storing the raw CDP cookie objects for re-adding, or pass through the original `Protocol.Network.Cookie` shape to `Storage.setCookies` to preserve all attributes.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User Script
participant Context as V3Context
participant Utils as Cookie Helpers (cookies.ts)
participant Conn as CDP Connection
participant Browser as Chrome/CDP
Note over User,Browser: Cookie Management Flow
rect rgb(23, 37, 84)
Note right of User: NEW: context.cookies(urls?)
User->>Context: cookies(["https://example.com"])
Context->>Conn: send("Storage.getCookies")
Conn->>Browser: Storage.getCookies
Browser-->>Conn: Protocol.Network.Cookie[]
Conn-->>Context: Raw CDP Cookies
Context->>Utils: filterCookies(mapped, urls)
Utils->>Utils: Match domain, path, & secure flag
Context-->>User: Cookie[]
end
rect rgb(5, 46, 22)
Note right of User: NEW: context.addCookies(cookies)
User->>Context: addCookies(CookieParam[])
Context->>Utils: normalizeCookieParams(cookies)
alt Invalid: sameSite="None" AND secure=false
Utils-->>Context: Throw CookieValidationError
Context-->>User: Error: Browsers require secure for None
else Valid
Utils-->>Context: Normalized params (derived domain/path)
Context->>Conn: send("Storage.setCookies", { cookies })
Conn->>Browser: Storage.setCookies
alt CDP Success
Browser-->>Context: OK
Context-->>User: void
else CDP Reject
Browser-->>Context: Error
Context-->>User: Throw CookieSetError
end
end
end
rect rgb(69, 26, 3)
Note right of User: NEW: context.clearCookies(options?)
User->>Context: clearCookies(options)
alt No filter (Clear All)
Context->>Conn: send("Storage.clearCookies")
Conn->>Browser: Storage.clearCookies
else Selective Clear (With Filter)
Context->>Context: cookies() [Internal Fetch]
Context->>Utils: cookieMatchesFilter(current, options)
Note over Context,Utils: Identify cookies to keep vs. remove
Context->>Conn: send("Storage.clearCookies")
Conn->>Browser: Storage.clearCookies
opt Has cookies to keep
Context->>Conn: send("Storage.setCookies", { toKeep })
Conn-->>Browser: Re-insert non-matching cookies
end
end
Context-->>User: void
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| const current = await this.cookies(); | ||
| const toKeep = current.filter((c) => !cookieMatchesFilter(c, options!)); |
There was a problem hiding this comment.
P2: Filtered clearCookies silently strips CDP-specific cookie metadata (e.g., priority, sourceScheme, sourcePort, partitionKey) from non-matching cookies during the clear-and-restore cycle. Consider storing the raw CDP cookie objects for re-adding, or pass through the original Protocol.Network.Cookie shape to Storage.setCookies to preserve all attributes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/understudy/context.ts, line 920:
<comment>Filtered `clearCookies` silently strips CDP-specific cookie metadata (e.g., `priority`, `sourceScheme`, `sourcePort`, `partitionKey`) from non-matching cookies during the clear-and-restore cycle. Consider storing the raw CDP cookie objects for re-adding, or pass through the original `Protocol.Network.Cookie` shape to `Storage.setCookies` to preserve all attributes.</comment>
<file context>
@@ -823,4 +838,106 @@ export class V3Context {
+ }
+
+ const current = await this.cookies();
+ const toKeep = current.filter((c) => !cookieMatchesFilter(c, options!));
+
+ if (toKeep.length === current.length) return;
</file context>
There was a problem hiding this comment.
this should be fine for now since none of these fields are settable by users. this can be fixed when we add support for partitionKey
There was a problem hiding this comment.
Got it, thanks for the context.
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 3/5
- Potential user-visible cookie scoping bug in
packages/core/lib/v3/understudy/cookies.ts: path matching usesstartsWithwithout the RFC 6265 boundary check, so/foomay incorrectly match/foobar. - Score reflects a concrete behavior deviation (severity 6/10) that could affect request routing/session handling, though limited in scope to cookie path matching.
- Pay close attention to
packages/core/lib/v3/understudy/cookies.ts- cookie path matching needs an RFC 6265 boundary check.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/cookies.ts">
<violation number="1" location="packages/core/lib/v3/understudy/cookies.ts:35">
P2: Cookie path matching does not enforce path-boundary check per RFC 6265 §5.1.4. A cookie with `path: "/foo"` will incorrectly match URL `http://example.com/foobar` because `startsWith` doesn't verify the next character is `/` (or end-of-string). This causes `cookies(urls)` to return cookies that the browser would not actually send.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Context as V3Context
participant Helper as Cookie Utils
participant Browser as Browser (CDP)
Note over Client, Browser: NEW: Cookie Management Flow
rect rgb(23, 37, 84)
Note right of Client: Get Cookies
Client->>Context: cookies(urls?)
Context->>Browser: NEW: Storage.getCookies
Browser-->>Context: Protocol.Network.Cookie[]
Context->>Helper: NEW: filterCookies(mapped, urls)
Helper-->>Client: Cookie[]
end
rect rgb(5, 46, 22)
Note right of Client: Add Cookies
Client->>Context: addCookies(cookies)
Context->>Helper: NEW: normalizeCookieParams()
alt NEW: Invalid params (e.g. SameSite:None and Secure:false)
Helper-->>Client: throw CookieValidationError
else Valid params
Helper-->>Context: Normalized CookieParam[]
Context->>Browser: NEW: Storage.setCookies
alt CDP Error
Browser-->>Client: throw CookieSetError
else Success
Browser-->>Client: void
end
end
end
rect rgb(69, 26, 3)
Note right of Client: Clear Cookies
Client->>Context: clearCookies(options?)
alt NEW: No options (Atomic Clear)
Context->>Browser: Storage.clearCookies
else NEW: Selective Clear (Filter provided)
Context->>Context: cookies() [Internal Fetch]
Context->>Helper: NEW: cookieMatchesFilter()
Context->>Browser: Storage.clearCookies [Wipe context]
opt Non-matching cookies exist
Context->>Browser: Storage.setCookies [Re-add kept cookies]
end
end
Context-->>Client: void
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 4/5
- Potential user-facing gap in secure-cookie handling:
packages/core/lib/v3/understudy/cookies.tsonly treatslocalhostas secure and may incorrectly drop secure cookies for loopback IPs like127.0.0.1or::1. - Issue is moderate severity and localized, suggesting the PR is likely safe to merge with minor risk for local dev/testing scenarios.
- Pay close attention to
packages/core/lib/v3/understudy/cookies.ts- secure-context detection for loopback IPs.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/understudy/cookies.ts">
<violation number="1" location="packages/core/lib/v3/understudy/cookies.ts:45">
P2: Secure-context check only handles `localhost`, missing loopback IPs (`127.0.0.1`, `::1`). Browsers treat all loopback addresses as secure contexts, so secure cookies should also be returned when filtering by `http://127.0.0.1/...` or `http://[::1]/...` URLs.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User/Test
participant Ctx as V3Context
participant Util as Cookie Helpers
participant CDP as CDPSession (conn)
participant Browser as Browser Storage
Note over User, Browser: NEW: Cookie Management Flow
rect rgb(20, 30, 50)
Note right of User: context.cookies(urls?)
User->>Ctx: cookies(urls)
Ctx->>CDP: Storage.getCookies
CDP->>Browser: Request all cookies
Browser-->>CDP: Network.Cookie[]
CDP-->>Ctx: { cookies }
Ctx->>Util: filterCookies(mapped, urls)
Note over Util: Matches domain, path,<br/>and secure protocol
Util-->>Ctx: Filtered Cookie[]
Ctx-->>User: Cookie[]
end
rect rgb(30, 20, 50)
Note right of User: context.addCookies(cookies)
User->>Ctx: addCookies(CookieParam[])
Ctx->>Util: normalizeCookieParams(cookies)
alt Invalid Params
Util-->>Ctx: throw CookieValidationError
Ctx-->>User: throw error
else Valid
Util-->>Ctx: NormalizedParams[]
end
Ctx->>CDP: Storage.setCookies({ cookies: cdpParams })
CDP->>Browser: Apply cookie batch
alt CDP Rejection
CDP-->>Ctx: error
Ctx-->>User: throw CookieSetError
else Success
CDP-->>Ctx: ok
Ctx-->>User: void
end
end
rect rgb(20, 50, 30)
Note right of User: context.clearCookies(options?)
User->>Ctx: clearCookies(options)
alt No Filter (options empty)
Ctx->>CDP: Storage.clearCookies
CDP->>Browser: Wipe all cookies
else Selective Clear (options.name/domain/path)
Ctx->>Ctx: cookies() [Fetch all current]
Ctx->>Util: cookieMatchesFilter(cookie, options)
Util-->>Ctx: cookiesToKeep[]
Ctx->>CDP: Storage.clearCookies
Note over Ctx, CDP: Necessary because CDP Storage domain<br/>lacks targeted delete support
opt Has cookies to keep
Ctx->>CDP: Storage.setCookies(cookiesToKeep)
end
end
CDP-->>User: Success
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 4/5
- This PR looks safe to merge, with only a release-metadata concern around semantic versioning rather than functional behavior.
- The changeset in
.changeset/fast-buses-kneel.mdmarks new public API methods (cookies,addCookies,clearCookies) as a patch, but semver suggests a minor bump to avoid misleading consumers. - Pay close attention to
.changeset/fast-buses-kneel.md- ensure the version bump matches the new API surface.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".changeset/fast-buses-kneel.md">
<violation number="1" location=".changeset/fast-buses-kneel.md:2">
P2: Adding new public API methods (`cookies`, `addCookies`, `clearCookies`) is a new feature, not a bug fix. Per semver, this should be a `minor` bump rather than `patch`.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User/Test
participant Context as V3Context
participant Utils as cookies.ts (Logic)
participant CDP as Browser (CDP Storage)
Note over User,CDP: NEW: Cookie Retrieval Flow
User->>Context: cookies(urls?)
Context->>CDP: Storage.getCookies
CDP-->>Context: Protocol.Network.Cookie[]
Context->>Utils: filterCookies(mapped, urlList)
Utils-->>Context: Filtered Cookie[]
Context-->>User: Cookie[]
Note over User,CDP: NEW: Cookie Addition Flow
User->>Context: addCookies(CookieParam[])
Context->>Utils: normalizeCookieParams(cookies)
alt Invalid (e.g. SameSite: None without Secure)
Utils-->>Context: throw CookieValidationError
Context-->>User: Error
else Valid
Utils-->>Context: Normalized params
Context->>CDP: Storage.setCookies
alt CDP Rejection
CDP-->>Context: Error
Context-->>User: throw CookieSetError
else Success
CDP-->>Context: void
Context-->>User: void
end
end
Note over User,CDP: NEW: Selective Cookie Clearing Flow
User->>Context: clearCookies(options)
alt No filters provided
Context->>CDP: Storage.clearCookies (Atomic Wipe)
else Selective delete (options.name/domain/path)
Context->>Context: cookies() (Fetch current)
Context->>Utils: cookieMatchesFilter()
Note right of Utils: Identify cookies to KEEP
Context->>CDP: Storage.clearCookies (Total Wipe)
opt Has cookies to keep
Context->>CDP: Storage.setCookies (Re-add non-matching)
end
end
Context-->>User: void
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
seems ok but we should really provide a unified API that sets all of these together, cookies alone are not enough for sites like banks/social media:
- cookies
- session storage
- local storage
- indexed db
This is why playwright pushes people towards storageState() and auth.json not cookieState() / cookies.txt:
Created ticket here: https://linear.app/browserbase/issue/STG-1425
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @browserbasehq/stagehand@3.1.0 ### Minor Changes - [#1681](#1681) [`e3db9aa`](e3db9aa) Thanks [@tkattkat](https://github.com/tkattkat)! - Add cookie management APIs: `context.addCookies()`, `context.clearCookies()`, & `context.cookies()` - [#1672](#1672) [`b65756e`](b65756e) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add boolean keepAlive parameter to allow for configuring whether the browser should be closed when stagehand.close() is called. - [#1708](#1708) [`176d420`](176d420) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add context.setExtraHTTPHeaders() - [#1611](#1611) [`8a3c066`](8a3c066) Thanks [@monadoid](https://github.com/monadoid)! - Using `mode` enum instead of old `cua` boolean in openapi spec ### Patch Changes - [#1683](#1683) [`7584f3e`](7584f3e) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix: include shadow DOM in .count() & .nth() & support xpath predicates - [#1644](#1644) [`1e1c9c1`](1e1c9c1) Thanks [@monadoid](https://github.com/monadoid)! - Fix unhandled CDP detaches by returning the original sendCDP promise - [#1729](#1729) [`6bef890`](6bef890) Thanks [@shrey150](https://github.com/shrey150)! - fix: support Claude 4.6 (Opus and Sonnet) in CUA mode by using the correct `computer_20251124` tool version and `computer-use-2025-11-24` beta header - [#1647](#1647) [`ffd4b33`](ffd4b33) Thanks [@tkattkat](https://github.com/tkattkat)! - Fix [Agent] - Address bug causing issues with continuing a conversation from past messages in dom mode - [#1614](#1614) [`677bff5`](677bff5) Thanks [@miguelg719](https://github.com/miguelg719)! - Enforce <number>-<number> regex validation on act/observe for elementId - [#1580](#1580) [`65ff464`](65ff464) Thanks [@tkattkat](https://github.com/tkattkat)! - Add unified variables support across act and agent with a single VariableValue type - [#1666](#1666) [`101bcf2`](101bcf2) Thanks [@Kylejeong2](https://github.com/Kylejeong2)! - add support for codex models - [#1728](#1728) [`0a94301`](0a94301) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - handle potential race condition on `.close()` when using the Stagehand API - [#1664](#1664) [`b27c04d`](b27c04d) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fixes issue with context.addInitScript() where scripts were not being applied to out of process iframes (OOPIFs), and popup pages with same process iframes (SPIFs) - [#1632](#1632) [`afbd08b`](afbd08b) Thanks [@pirate](https://github.com/pirate)! - Remove automatic `.env` loading via `dotenv`. If your app relies on `.env` files, install `dotenv` and load it explicitly in your code: ```ts import dotenv from "dotenv"; dotenv.config({ path: ".env" }); ``` - [#1624](#1624) [`0e8d569`](0e8d569) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix issue where screenshot masks were not being applied to dialog elements - [#1596](#1596) [`ff0f979`](ff0f979) Thanks [@tkattkat](https://github.com/tkattkat)! - Update usage/metrics handling in agent - [#1631](#1631) [`2d89d2b`](2d89d2b) Thanks [@miguelg719](https://github.com/miguelg719)! - Add right and middle click support to act and observe - [#1697](#1697) [`aac9a19`](aac9a19) Thanks [@shrey150](https://github.com/shrey150)! - fix: support `<frame>` elements in XPath frame boundary detection so `act()` works on legacy `<frameset>` pages - [#1692](#1692) [`06de50f`](06de50f) Thanks [@shrey150](https://github.com/shrey150)! - fix: skip piercer injection for chrome-extension:// and other non-HTML targets - [#1613](#1613) [`aa4d981`](aa4d981) Thanks [@miguelg719](https://github.com/miguelg719)! - SupportedUnderstudyAction Enum validation for 'method' on act/observe inference - [#1652](#1652) [`18b1e3b`](18b1e3b) Thanks [@miguelg719](https://github.com/miguelg719)! - Add support for gemini 3 flash and pro in hybrid/cua agent - [#1706](#1706) [`957d82b`](957d82b) Thanks [@chrisreadsf](https://github.com/chrisreadsf)! - Add GLM to prompt-based JSON fallback for models without native structured output support - [#1633](#1633) [`22e371a`](22e371a) Thanks [@tkattkat](https://github.com/tkattkat)! - Add warning when incorrect models are used with agents hybrid mode - [#1673](#1673) [`d29b91f`](d29b91f) Thanks [@miguelg719](https://github.com/miguelg719)! - Add multi-region support for Stagehand API with region-specific endpoints - [#1695](#1695) [`7b4f817`](7b4f817) Thanks [@tkattkat](https://github.com/tkattkat)! - Fix: zod bug when pinning zod to v3 and using structured output in agent - [#1609](#1609) [`3f9ca4d`](3f9ca4d) Thanks [@miguelg719](https://github.com/miguelg719)! - Add SupportedUnderstudyActions to observe system prompt - [#1581](#1581) [`49ead1e`](49ead1e) Thanks [@sameelarif](https://github.com/sameelarif)! - **Server-side caching is now available.** When running `env: "BROWSERBASE"`, Stagehand automatically caches `act()`, `extract()`, and `observe()` results server-side — repeated calls with the same inputs return instantly without consuming LLM tokens. Caching is enabled by default and can be disabled via `serverCache: false` on the Stagehand instance or per individual call. Check out the [browserbase blog](https://www.browserbase.com/blog/stagehand-caching) for more details. - [#1642](#1642) [`3673369`](3673369) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix issue where scripts added via context.addInitScripts() were not being injected into new pages that were opened via popups (eg, clicking a link that opens a new page) and/or calling context.newPage(url) - [#1735](#1735) [`c465e87`](c465e87) Thanks [@monadoid](https://github.com/monadoid)! - Supports request header authentication with connectToMCPServer - [#1705](#1705) [`ae533e4`](ae533e4) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - include error cause in UnderstudyCommandException - [#1636](#1636) [`ea33052`](ea33052) Thanks [@miguelg719](https://github.com/miguelg719)! - Include executionModel on the AgentConfigSchema - [#1679](#1679) [`5764ede`](5764ede) Thanks [@shrey150](https://github.com/shrey150)! - fix issue where locator.count() was not working with xpaths that have attribute predicates - [#1646](#1646) [`f09b184`](f09b184) Thanks [@miguelg719](https://github.com/miguelg719)! - Add user-agent to CDP connections - [#1637](#1637) [`a7d29de`](a7d29de) Thanks [@miguelg719](https://github.com/miguelg719)! - Improve error and warning message for legacy model format - [#1685](#1685) [`d334399`](d334399) Thanks [@tkattkat](https://github.com/tkattkat)! - Bump ai sdk & google provider version - [#1662](#1662) [`44416da`](44416da) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix issue where locator.fill() was not working on elements that require direct value setting - [#1612](#1612) [`bdd8b4e`](bdd8b4e) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - fix issue where screenshot mask was only being applied to the first element that the locator resolved to. masks now apply to all matching elements. ## @browserbasehq/stagehand-server@3.6.0 ### Minor Changes - [#1611](#1611) [`8a3c066`](8a3c066) Thanks [@monadoid](https://github.com/monadoid)! - Using `mode` enum instead of old `cua` boolean in openapi spec ### Patch Changes - [#1604](#1604) [`4753078`](4753078) Thanks [@miguelg719](https://github.com/miguelg719)! - Enable bedrock - [#1636](#1636) [`ea33052`](ea33052) Thanks [@miguelg719](https://github.com/miguelg719)! - Include executionModel on the AgentConfigSchema - [#1602](#1602) [`22a0502`](22a0502) Thanks [@miguelg719](https://github.com/miguelg719)! - Include vertex as a supported provider - Updated dependencies \[[`7584f3e`](7584f3e), [`1e1c9c1`](1e1c9c1), [`6bef890`](6bef890), [`ffd4b33`](ffd4b33), [`677bff5`](677bff5), [`65ff464`](65ff464), [`101bcf2`](101bcf2), [`0a94301`](0a94301), [`b27c04d`](b27c04d), [`afbd08b`](afbd08b), [`e3db9aa`](e3db9aa), [`0e8d569`](0e8d569), [`ff0f979`](ff0f979), [`2d89d2b`](2d89d2b), [`aac9a19`](aac9a19), [`06de50f`](06de50f), [`aa4d981`](aa4d981), [`18b1e3b`](18b1e3b), [`957d82b`](957d82b), [`b65756e`](b65756e), [`22e371a`](22e371a), [`d29b91f`](d29b91f), [`7b4f817`](7b4f817), [`176d420`](176d420), [`3f9ca4d`](3f9ca4d), [`8a3c066`](8a3c066), [`49ead1e`](49ead1e), [`3673369`](3673369), [`c465e87`](c465e87), [`ae533e4`](ae533e4), [`ea33052`](ea33052), [`5764ede`](5764ede), [`f09b184`](f09b184), [`a7d29de`](a7d29de), [`d334399`](d334399), [`44416da`](44416da), [`bdd8b4e`](bdd8b4e)]: - @browserbasehq/stagehand@3.1.0 ## @browserbasehq/stagehand-evals@1.1.8 ### Patch Changes - Updated dependencies \[[`7584f3e`](7584f3e), [`1e1c9c1`](1e1c9c1), [`6bef890`](6bef890), [`ffd4b33`](ffd4b33), [`677bff5`](677bff5), [`65ff464`](65ff464), [`101bcf2`](101bcf2), [`0a94301`](0a94301), [`b27c04d`](b27c04d), [`afbd08b`](afbd08b), [`e3db9aa`](e3db9aa), [`0e8d569`](0e8d569), [`ff0f979`](ff0f979), [`2d89d2b`](2d89d2b), [`aac9a19`](aac9a19), [`06de50f`](06de50f), [`aa4d981`](aa4d981), [`18b1e3b`](18b1e3b), [`957d82b`](957d82b), [`b65756e`](b65756e), [`22e371a`](22e371a), [`d29b91f`](d29b91f), [`7b4f817`](7b4f817), [`176d420`](176d420), [`3f9ca4d`](3f9ca4d), [`8a3c066`](8a3c066), [`49ead1e`](49ead1e), [`3673369`](3673369), [`c465e87`](c465e87), [`ae533e4`](ae533e4), [`ea33052`](ea33052), [`5764ede`](5764ede), [`f09b184`](f09b184), [`a7d29de`](a7d29de), [`d334399`](d334399), [`44416da`](44416da), [`bdd8b4e`](bdd8b4e)]: - @browserbasehq/stagehand@3.1.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Why
Browser automation workflows frequently need to manage authentication state, persist sessions across runs, and handle cookie-based features. Currently, users have no direct API to read, write, or clear cookies in Stagehand, forcing workarounds through raw CDP commands or page-level JavaScript injection.
What Changed
Cookie Management APIs
Added cookie management methods that mirror the familiar Playwright BrowserContext API:
On
contextinstance:context.cookies(urls?)— Get all browser cookies, optionally filtered by URL(s)context.addCookies(cookies)— Set one or more cookies in the browser contextcontext.clearCookies(options?)— Clear all cookies or selectively by name/domain/path (supports RegExp)Types Added
Cookie— Cookie object returned by the browserCookieParam— Parameters for setting a cookie (supportsurlORdomain+path)ClearCookieOptions— Filter options for selective cookie clearingTest Plan
Cookie Management
context.cookies()returns all cookies when no URLs providedcontext.cookies(url)filters cookies by single URLcontext.cookies([url1, url2])filters cookies by multiple URLscontext.addCookies([...])successfully sets cookiescontext.addCookies()validates cookie params (requires url OR domain+path)context.addCookies()rejectssameSite: "None"withoutsecure: truecontext.addCookies()throws on browser rejection (checks CDP success flag)context.clearCookies()clears all cookies atomically via single CDP callcontext.clearCookies({ name: "session" })clears by exact namecontext.clearCookies({ domain: /\.example\.com/ })clears by regex