[STG-1427] caching threshold configuration#1732
Open
sameelarif wants to merge 20 commits intomainfrom
Open
Conversation
…rif/stg-1427-caching-threshold-configuration-client-side
🦋 Changeset detectedLatest commit: e12d4b1 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 |
Contributor
Greptile SummaryThis PR adds server-side caching configuration to the Stagehand SDK, allowing users to control caching behavior at both the instance level (
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User calls act/extract/observe<br>with options.serverCache"] --> B{"Method-level<br>serverCache set?"}
B -- Yes --> C{"typeof serverCache"}
C -- "boolean" --> D["Use boolean directly"]
C -- "{ threshold }" --> E["Cache enabled + custom threshold"]
B -- No --> F{"Constructor-level<br>serverCache type?"}
F -- "boolean" --> G["Use constructor boolean"]
F -- "object (per-method)" --> H{"Method key exists<br>in object?"}
H -- Yes --> I["Use per-method setting"]
H -- No --> J["Default: cache enabled"]
F -- "undefined" --> J
D --> K{"Resolved to false?"}
G --> K
I --> K
J --> K
E --> L["Send cacheThreshold<br>in request body"]
L --> K
K -- Yes --> M["Add browserbase-cache-bypass<br>header = true"]
K -- No --> N["No bypass header<br>(caching active)"]
M --> O["API Request"]
N --> O
O --> P["Read browserbase-cache-status<br>response header"]
P --> Q["Attach cacheStatus<br>to ActResult/ExtractResult"]
Last reviewed commit: 3552ff0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| variables?: Stagehand.Variables; | ||
| timeout?: number; | ||
| page?: Stagehand.AnyPage; | ||
| serverCache?: boolean; |
Contributor
There was a problem hiding this comment.
Type mismatch will fail tests
The expected type here declares serverCache?: boolean, but the actual ActOptions type in methods.ts:25 defines serverCache?: boolean | { threshold: number }. Since toEqualTypeOf performs exact type matching, this test will fail. The same issue applies to ExtractOptions (line 155) and ObserveOptions (line 169).
Suggested change
| serverCache?: boolean; | |
| serverCache?: boolean | { threshold: number }; |
| timeout?: number; | ||
| selector?: string; | ||
| page?: Stagehand.AnyPage; | ||
| serverCache?: boolean; |
Contributor
There was a problem hiding this comment.
Same type mismatch as ActOptions
Should match the actual ExtractOptions.serverCache type.
Suggested change
| serverCache?: boolean; | |
| serverCache?: boolean | { threshold: number }; |
| timeout?: number; | ||
| selector?: string; | ||
| page?: Stagehand.AnyPage; | ||
| serverCache?: boolean; |
Contributor
There was a problem hiding this comment.
Same type mismatch as ActOptions
Should match the actual ObserveOptions.serverCache type.
Suggested change
| serverCache?: boolean; | |
| serverCache?: boolean | { threshold: number }; |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 10 files
Confidence score: 3/5
- Missing integration tests for new caching behavior in
packages/core/lib/v3/api.tsincreases regression risk for the Stagehand REST API client/server changes. - Hidden
__extractionIncompletehook inpackages/core/lib/v3/handlers/extractHandler.tsexposes internals instead of using the event bus, which could lead to unintended API surface and maintenance issues. - Several API consistency/doc gaps (LaunchDarkly reference in
packages/core/lib/v3/types/public/api.ts, observe return type missingcacheStatusinpackages/core/lib/v3/types/public/methods.ts) suggest some user-facing confusion but are fixable. - Pay close attention to
packages/core/lib/v3/api.ts,packages/core/lib/v3/handlers/extractHandler.ts,packages/core/lib/v3/types/public/api.ts,packages/core/lib/v3/types/public/methods.ts- ensure tests and public API surface stay consistent.
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/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:876">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
No integration tests cover the new caching behavior introduced in the API client. This PR adds `cacheThreshold` to request bodies, a `browserbase-cache-bypass` request header, and `browserbase-cache-status` response header handling — all of which modify the wire format between client and server. Per the rule, breaking changes to `packages/core/lib/v3/api.ts` must be covered by at least one integration test under `packages/server/test/**`.
At minimum, tests should verify:
- The `browserbase-cache-bypass: true` header is sent when `serverCache` is `false` (instance-level and method-level override)
- `cacheThreshold` is included in act/extract/observe request bodies when provided
- `cacheStatus` is correctly attached to act/extract results from the `browserbase-cache-status` response header
(Based on your team's feedback about preferring unit tests for new behavior.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/core/lib/v3/handlers/extractHandler.ts">
<violation number="1" location="packages/core/lib/v3/handlers/extractHandler.ts:239">
P1: Custom agent: **Don't allow exposing internals of stagehand library via hidden callback hooks, instead force event-based hooking via stagehand.bus EventEmitter**
This introduces a hidden `__extractionIncomplete` property on the return value as an undocumented hook for the cloud cache layer to detect incomplete extractions. This is the exact anti-pattern the team wants to avoid — exposing library internals via hidden `__`-prefixed mechanisms for external code to consume.
Instead, signal incomplete extractions through a well-defined event-based interface (e.g., `this.bus.emit("extraction_incomplete", { output })`) so the stagehand-api cache layer can subscribe cleanly via `stagehand.bus.on("extraction_incomplete", ...)`. This keeps the coupling explicit, discoverable, and consistent with the team's preferred architecture.</violation>
<violation number="2" location="packages/core/lib/v3/handlers/extractHandler.ts:239">
P2: When a non-object schema is used (e.g., `z.string()`, `z.number()`), `output` is unwrapped to a primitive value on line 224–225, and the `typeof output === "object"` guard here correctly avoids a runtime error—but it also means the `__extractionIncomplete` signal is silently lost for all primitive extraction results. The cache layer would have no way to distinguish a complete vs. incomplete primitive result and may persist incomplete data.
Consider an alternative signaling mechanism that works for all output types (e.g., a `WeakMap`, a wrapper return type, or returning a `{ data, metadata }` envelope).</violation>
</file>
<file name="packages/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:438">
P2: The description references "LaunchDarkly flag" — an internal infrastructure detail that shouldn't be exposed in a public API schema. API consumers don't need to know about the feature-flag provider; a description like *"Custom cache threshold override. Minimum hit count before cached results are returned."* conveys the same intent without leaking internals.
(Based on your team's feedback about avoiding exposing internal types as public APIs.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/core/lib/v3/types/public/methods.ts">
<violation number="1" location="packages/core/lib/v3/types/public/methods.ts:25">
P2: Inconsistency: `serverCache` option is added to `ObserveOptions` but there is no `cacheStatus` in the observe return type. The `cacheStatus` field was added to `ActResult` and `ExtractResult` but not to the observe result, so callers using `observe({ serverCache: true })` have no way to determine if the result was served from cache.</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as Client Application
participant V3 as Stagehand SDK (V3)
participant Client as StagehandAPIClient
participant BB_API as Browserbase Server API
participant Handler as ExtractHandler (Internal)
Note over App, BB_API: Configuration & Request Resolution
App->>V3: act / extract / observe (options.serverCache)
V3->>V3: NEW: resolveServerCacheBoolean()<br/>(Prioritize call-level over global config)
V3->>V3: NEW: resolveServerCacheThreshold()<br/>(Extract custom threshold value)
V3->>Client: Call method with resolved cache params
Client->>Client: NEW: Prepare headers & body
opt serverCache is false
Note right of Client: Set 'browserbase-cache-bypass: true'
end
Client->>BB_API: POST request (JSON body includes cacheThreshold)
Note over BB_API: Server processes request<br/>using cacheThreshold & bypass header
BB_API-->>Client: Response Headers (NEW: browserbase-cache-status: HIT|MISS)
loop SSE Stream
BB_API-->>Client: data: { type: "system", status: "finished", result: {...} }
end
Note over Client, Handler: Result Post-Processing
opt Method is "extract"
Handler->>Handler: NEW: If result incomplete, set<br/>__extractionIncomplete: true
Note right of Handler: Signals server-side cache layer<br/>to skip persistence
end
Client->>Client: NEW: attachCacheStatus()
Note right of Client: Maps header/SSE data to result object
Client-->>V3: Result (with cacheStatus field)
V3-->>App: Result (e.g. { success: true, cacheStatus: "HIT" })
Note over App, V3: Logging
V3->>V3: Log: "act cache status: HIT (threshold: 2)" (NEW)
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| StagehandResponseBodyError, | ||
| StagehandResponseParseError, | ||
| StagehandServerError, | ||
| ExperimentalNotConfiguredError, |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test
No integration tests cover the new caching behavior introduced in the API client. This PR adds cacheThreshold to request bodies, a browserbase-cache-bypass request header, and browserbase-cache-status response header handling — all of which modify the wire format between client and server. Per the rule, breaking changes to packages/core/lib/v3/api.ts must be covered by at least one integration test under packages/server/test/**.
At minimum, tests should verify:
- The
browserbase-cache-bypass: trueheader is sent whenserverCacheisfalse(instance-level and method-level override) cacheThresholdis included in act/extract/observe request bodies when providedcacheStatusis correctly attached to act/extract results from thebrowserbase-cache-statusresponse header
(Based on your team's feedback about preferring unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/api.ts, line 876:
<comment>No integration tests cover the new caching behavior introduced in the API client. This PR adds `cacheThreshold` to request bodies, a `browserbase-cache-bypass` request header, and `browserbase-cache-status` response header handling — all of which modify the wire format between client and server. Per the rule, breaking changes to `packages/core/lib/v3/api.ts` must be covered by at least one integration test under `packages/server/test/**`.
At minimum, tests should verify:
- The `browserbase-cache-bypass: true` header is sent when `serverCache` is `false` (instance-level and method-level override)
- `cacheThreshold` is included in act/extract/observe request bodies when provided
- `cacheStatus` is correctly attached to act/extract results from the `browserbase-cache-status` response header
(Based on your team's feedback about preferring unit tests for new behavior.) </comment>
<file context>
@@ -741,6 +871,12 @@ export class StagehandAPIClient {
};
+
+ // Add cache bypass header if caching is disabled
+ if (!this.shouldUseCache(serverCache)) {
+ defaultHeaders["browserbase-cache-bypass"] = "true";
+ }
</file context>
| // (e.g., the stagehand-api cache layer) can skip persisting it. | ||
| // The property is non-enumerable so it is invisible to JSON.stringify | ||
| // and never exposed to clients. | ||
| if (!completed && output !== null && typeof output === "object") { |
Contributor
There was a problem hiding this comment.
P2: When a non-object schema is used (e.g., z.string(), z.number()), output is unwrapped to a primitive value on line 224–225, and the typeof output === "object" guard here correctly avoids a runtime error—but it also means the __extractionIncomplete signal is silently lost for all primitive extraction results. The cache layer would have no way to distinguish a complete vs. incomplete primitive result and may persist incomplete data.
Consider an alternative signaling mechanism that works for all output types (e.g., a WeakMap, a wrapper return type, or returning a { data, metadata } envelope).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/handlers/extractHandler.ts, line 239:
<comment>When a non-object schema is used (e.g., `z.string()`, `z.number()`), `output` is unwrapped to a primitive value on line 224–225, and the `typeof output === "object"` guard here correctly avoids a runtime error—but it also means the `__extractionIncomplete` signal is silently lost for all primitive extraction results. The cache layer would have no way to distinguish a complete vs. incomplete primitive result and may persist incomplete data.
Consider an alternative signaling mechanism that works for all output types (e.g., a `WeakMap`, a wrapper return type, or returning a `{ data, metadata }` envelope).</comment>
<file context>
@@ -232,6 +232,18 @@ export class ExtractHandler {
+ // (e.g., the stagehand-api cache layer) can skip persisting it.
+ // The property is non-enumerable so it is invisible to JSON.stringify
+ // and never exposed to clients.
+ if (!completed && output !== null && typeof output === "object") {
+ Object.defineProperty(output, "__extractionIncomplete", {
+ value: true,
</file context>
miguelg719
changed the base branch from
main
to
sameelarif/stg-1182-cache-bypass-from-sdk
miguelg719
changed the base branch from
sameelarif/stg-1182-cache-bypass-from-sdk
to
main
…rif/stg-1427-caching-threshold-configuration-client-side
Contributor
There was a problem hiding this comment.
2 issues found across 4 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/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:376">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
The `observe()` method now returns `ObserveResult` (with `cacheStatus`) instead of `Action[]`, and `attachCacheStatus` was extended to stamp `cacheStatus` on observe results. These are breaking behavioral changes to the API client, but no integration test under `packages/server/test` covers the new cache status surfacing or `cacheThreshold` parameter for observe. Per the rule, breaking changes to `packages/core/lib/v3/api.ts` must be covered by integration tests. Add tests in `packages/server/test/integration/v3/observe.test.ts` that verify (1) `cacheStatus` is present/correct on observe responses and (2) the `cacheThreshold` field is accepted and handled correctly.</violation>
</file>
<file name="packages/core/lib/v3/types/public/methods.ts">
<violation number="1" location="packages/core/lib/v3/types/public/methods.ts:93">
P2: `Action[] & { cacheStatus }` intersection is fragile: `cacheStatus` is silently dropped by `JSON.stringify()`, array spread, `.map()`, `.filter()`, and other common array operations, since they only preserve indexed elements. Consider wrapping the result in an object (e.g., `{ actions: Action[], cacheStatus?: ... }`) or using a subclass/helper to avoid this gotcha for API consumers.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }: ClientObserveParameters): Promise<Action[]> { | ||
| // Strip non-serializable `page` from options before wire serialization | ||
| cacheThreshold, | ||
| }: ClientObserveParameters): Promise<ObserveResult> { |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test
The observe() method now returns ObserveResult (with cacheStatus) instead of Action[], and attachCacheStatus was extended to stamp cacheStatus on observe results. These are breaking behavioral changes to the API client, but no integration test under packages/server/test covers the new cache status surfacing or cacheThreshold parameter for observe. Per the rule, breaking changes to packages/core/lib/v3/api.ts must be covered by integration tests. Add tests in packages/server/test/integration/v3/observe.test.ts that verify (1) cacheStatus is present/correct on observe responses and (2) the cacheThreshold field is accepted and handled correctly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/api.ts, line 376:
<comment>The `observe()` method now returns `ObserveResult` (with `cacheStatus`) instead of `Action[]`, and `attachCacheStatus` was extended to stamp `cacheStatus` on observe results. These are breaking behavioral changes to the API client, but no integration test under `packages/server/test` covers the new cache status surfacing or `cacheThreshold` parameter for observe. Per the rule, breaking changes to `packages/core/lib/v3/api.ts` must be covered by integration tests. Add tests in `packages/server/test/integration/v3/observe.test.ts` that verify (1) `cacheStatus` is present/correct on observe responses and (2) the `cacheThreshold` field is accepted and handled correctly.</comment>
<file context>
@@ -372,7 +373,7 @@ export class StagehandAPIClient {
frameId,
cacheThreshold,
- }: ClientObserveParameters): Promise<Action[]> {
+ }: ClientObserveParameters): Promise<ObserveResult> {
// Strip non-serializable `page` and client-only `serverCache` from options before wire serialization
let wireOptions: Api.ObserveRequest["options"];
</file context>
| serverCache?: boolean | { threshold: number }; | ||
| } | ||
|
|
||
| export type ObserveResult = Action[] & { cacheStatus?: "HIT" | "MISS" }; |
Contributor
There was a problem hiding this comment.
P2: Action[] & { cacheStatus } intersection is fragile: cacheStatus is silently dropped by JSON.stringify(), array spread, .map(), .filter(), and other common array operations, since they only preserve indexed elements. Consider wrapping the result in an object (e.g., { actions: Action[], cacheStatus?: ... }) or using a subclass/helper to avoid this gotcha for API consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/types/public/methods.ts, line 93:
<comment>`Action[] & { cacheStatus }` intersection is fragile: `cacheStatus` is silently dropped by `JSON.stringify()`, array spread, `.map()`, `.filter()`, and other common array operations, since they only preserve indexed elements. Consider wrapping the result in an object (e.g., `{ actions: Action[], cacheStatus?: ... }`) or using a subclass/helper to avoid this gotcha for API consumers.</comment>
<file context>
@@ -90,6 +90,8 @@ export interface ObserveOptions {
serverCache?: boolean | { threshold: number };
}
+export type ObserveResult = Action[] & { cacheStatus?: "HIT" | "MISS" };
+
export enum V3FunctionName {
</file context>
…uration-client-side
miguelg719
reviewed
Feb 24, 2026
|
|
||
| <V3Banner /> | ||
|
|
||
| <Note> |
Collaborator
There was a problem hiding this comment.
docs should now include the reference section per method and constructor
miguelg719
reviewed
Feb 24, 2026
| }; | ||
|
|
||
| return this.execute<Action[]>({ | ||
| return this.execute<ObserveResult>({ |
Collaborator
There was a problem hiding this comment.
why is this type changing
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
2 participants
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.
why
what changed
test plan
Summary by cubic
Adds client-side controls for server-side caching with per-method threshold overrides and cache bypass. Surfaces cache status on act/extract/observe results and updates docs to meet STG-1427.
Written for commit e12d4b1. Summary will update on new commits. Review in cubic