Conversation
|
Greptile SummaryThis PR adds Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant V3
participant ObserveHandler
participant LLM
participant ActHandler
Caller->>V3: observe(instruction, { variables })
V3->>ObserveHandler: ObserveHandlerParams { instruction, variables }
ObserveHandler->>LLM: buildObserveSystemPrompt(userInstructions, supportedActions, variables)<br/>System prompt exposes only %variableName% keys — no values
LLM-->>ObserveHandler: Action[] with %placeholder% tokens in arguments
ObserveHandler-->>V3: Action[] (placeholders preserved)
V3-->>Caller: Action[] (e.g. [{ method: "fill", arguments: ["%username%"] }])
Note over Caller: Caller validates returned actions
Caller->>V3: act(emailAction, { variables })
V3->>ActHandler: substituteVariables(%username% → "user@example.com")
ActHandler-->>Caller: ActResult (real value used only at execution time)
Last reviewed commit: 10dc220 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| variables: z | ||
| .record(z.string(), z.string()) | ||
| .optional() | ||
| .meta({ | ||
| description: | ||
| "Variables to substitute into observed action arguments using %variableName% placeholders", | ||
| example: { username: "john_doe" }, | ||
| }), |
There was a problem hiding this comment.
Misleading schema description — observe doesn't substitute variables
The description says "Variables to substitute into observed action arguments", but observe() doesn't substitute values — it tells the model to return %variableName% placeholders. Actual substitution happens later in act(). A more accurate description would be something like "Variables whose names are exposed to the model so it returns %variableName% placeholders in action arguments instead of literal values".
Also note this schema accepts only z.record(z.string(), z.string()) (flat strings), while the public ObserveOptions.variables type accepts rich Variables (including { value, description? } objects). flattenVariables is called client-side to reconcile this, but a developer calling the server API directly with a rich object would get a schema validation error without a clear reason.
There was a problem hiding this comment.
yeah this part is crucial:
Also note this schema accepts only z.record(z.string(), z.string()) (flat strings), while the public ObserveOptions.variables type accepts rich Variables (including { value, description? } objects). flattenVariables is called client-side to reconcile this, but a developer calling the server API directly with a rich object would get a schema validation error without a clear reason.
There was a problem hiding this comment.
Agreed. observe() does not substitute values; it only exposes variable names so the model returns %variableName% placeholders in action arguments. Actual
substitution happens later in act(). I updated the schema description to reflect that and added coverage for both SDK flattening and direct server requests with
rich variables.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
2 issues found across 18 files
Confidence score: 4/5
- This PR looks safe to merge with minimal risk: both findings are low-to-moderate severity quality issues rather than likely functional breakages in core runtime behavior.
- Most severe issue: in
packages/core/examples/observe_variables_login.ts, PlaywrightgotousestimeoutMs(invalid) instead oftimeout, so the intended 30s limit is ignored; this is user-facing because example code is likely to be copied. - In
packages/core/lib/prompt.ts, prompt assembly can produce a double period (..) when variables are present, which is minor but can degrade prompt polish/readability. - Pay close attention to
packages/core/examples/observe_variables_login.tsandpackages/core/lib/prompt.ts- fix the invalid Playwright timeout option and the punctuation duplication in generated prompts.
Prompt for AI agents (unresolved 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/prompt.ts">
<violation number="1" location="packages/core/lib/prompt.ts:137">
P2: Double period in the generated prompt when variables are provided. `variablesString` ends with `.` and the template appends another `.` right after, producing `...literal value.. When choosing...` after whitespace collapse. Remove the trailing period from `variablesString` or from the template literal to avoid the stutter.</violation>
</file>
<file name="packages/core/examples/observe_variables_login.ts">
<violation number="1" location="packages/core/examples/observe_variables_login.ts:59">
P2: `timeoutMs` is not a valid Playwright `goto` option — it will be silently ignored. Use `timeout` instead so the 30 s limit actually takes effect. Since users are likely to copy this example, the typo is worth fixing.</violation>
</file>
Linked issue analysis
Linked issue: STG-1438: Add variables to observe
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add variables support to observe in the public SDK types (ObserveOptions) | ObserveOptions and schema now include variables |
| ✅ | Add variables param to internal ObserveHandlerParams and handler signature | ObserveHandlerParams and observe() accept variables |
| ✅ | Thread variables through local SDK path (v3.ts) into handler params | V3 passes options?.variables into handler params |
| ✅ | Thread variables into inference.observe() signature and calls | inference.observe now accepts and forwards variables |
| ✅ | Update prompt builder to show available variable names to model | buildObserveSystemPrompt now includes variablesString guidance |
| ✅ | Make model return %variableName% placeholders (prompt instructs so) | Prompt instructs placeholder usage and inference path preserves them |
| ✅ | Preserve placeholders in returned Action[] (no literal secrets leaked) | Test asserts observed action arguments remain %username% |
| ✅ | Add observe variable support to hosted/API path and schemas | API ObserveOptionsSchema now accepts variables and server tests added |
| ✅ | Flatten rich variable values to existing wire format before sending API request | StagehandAPIClient flattens variables via flattenVariables |
| ✅ | Update internal fillForm tool to pass variables into observe() | fillFormTool now includes variables in observeOptions |
| ✅ | Ensure fillFormTool forwarding to v3.observe is tested | Unit test verifies fillFormTool passes variables to v3.observe |
| ✅ | Add docs for observe({ variables }) and validate-then-act login flow | Docs updated with variables sections and examples |
| ✅ | Add example demonstrating placeholder-based login planning and act() execution | Example file added showing validate-then-act with variables |
| ✅ | Add unit tests covering public ObserveOptions type support | public-types.test updated to include Variables in ObserveOptions |
| ✅ | Add unit tests verifying observe variable forwarding into inference/prompting | Test asserts observeInference called with variables |
| ✅ | Add unit tests for API client observe variable serialization | api-client-observe-variables.test asserts flattened payload |
| ✅ | Add integration tests for observe request schemas in v3 and v4 server tests | Integration tests post observe with variables and assert success |
Architecture diagram
sequenceDiagram
participant User as User Script
participant SDK as Stagehand SDK (V3)
participant API as API Client / Server
participant Handler as ObserveHandler
participant Prompt as Prompt Builder
participant LLM as LLM (Inference)
Note over User,LLM: Variable-aware Observation Flow (Secure Login Pattern)
User->>SDK: observe(instruction, { variables })
opt If Hosted/Remote Execution
SDK->>API: observe(instruction, { variables })
API->>API: NEW: flattenVariables()
Note right of API: Converts rich objects {value, desc} <br/>to simple strings for wire format
end
SDK->>Handler: observe(params)
Handler->>Handler: Capture DOM & Accessibility Tree
Handler->>Prompt: NEW: buildObserveSystemPrompt(..., variables)
Prompt->>Prompt: Extract variable keys (e.g., "username")
Note right of Prompt: Injects instruction: "When an action needs a <br/>sensitive value, return %variableName% placeholder"
Handler->>LLM: inference.observe(prompt, snapshot)
LLM-->>Handler: Return Action[] (with %placeholders%)
Handler-->>SDK: Return observed actions
SDK-->>User: Return actions (e.g., [{ method: "fill", args: ["%password%"] }])
Note over User: Security Boundary: Real secrets never sent to LLM
User->>User: CHANGED: Validate observed actions <br/>contain expected placeholders
alt Happy Path: Validation Success
User->>SDK: act(action, { variables })
SDK->>SDK: CHANGED: Substitute %placeholder% with real value
SDK->>Handler: Perform browser action with secret
else Unhappy Path: Malicious/Unexpected Action
User->>User: Abort execution (Safety check)
end
opt Internal Tooling: fillForm
SDK->>SDK: fillFormTool.execute(fields, variables)
SDK->>Handler: NEW: observe(instruction, { variables })
Note right of SDK: fillForm now uses variable-aware <br/>observation to find form fields
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "Model configuration object or model name string (e.g., 'openai/gpt-5-nano')", | ||
| }), | ||
| variables: z | ||
| .record(z.string(), z.string()) |
There was a problem hiding this comment.
wrong shape, shouldnt it be the new variables shape that supports description or flat?
There was a problem hiding this comment.
Valid point. The earlier version only accepted Record<string, string>, but ObserveOptions.variables uses the shared Variables shape. I updated the wire schema/
OpenAPI to accept flat primitives and { value, description? } objects so direct API callers match the SDK surface too.
| variables: z | ||
| .record(z.string(), z.string()) | ||
| .optional() | ||
| .meta({ | ||
| description: | ||
| "Variables to substitute into observed action arguments using %variableName% placeholders", | ||
| example: { username: "john_doe" }, | ||
| }), |
There was a problem hiding this comment.
yeah this part is crucial:
Also note this schema accepts only z.record(z.string(), z.string()) (flat strings), while the public ObserveOptions.variables type accepts rich Variables (including { value, description? } objects). flattenVariables is called client-side to reconcile this, but a developer calling the server API directly with a rich object would get a schema validation error without a clear reason.
✱ Stainless preview buildsThis PR will update the
|
|
@pirate lint / test passing after feedback addressed! |
3 participants
why
act()already supports variables, butobserve()did not. That made safety-sensitive flows like login automation harder to use correctly: callers could inspectobserve()results before executing them, but then had to guess which returned action should receive each secret value before calling
act().This change brings variable support to
observe()so it can return placeholder-backed actions like%username%and%password%. That preserves the existing safe pattern of:observe()candidate actionsact()the validated actions with real variable values at execution timewhat changed
variables?: Variablessupport toobserve()in the public SDK types and internal handler params.observevariables through the local SDK path, inference layer, and prompt builder.%variableName%placeholders in action arguments instead of literal sensitive values.fillFormtool to pass variables intoobserve()as well asact().observe({ variables })and the validate-then-act login flow.packages/core/examples/observe_variables_login.tsshowing placeholder-based login planning withobserve(), explicit validation, and execution viaact().test plan
ObserveOptionstype supportfillFormforwarding variables toobserve()pnpm --filter @browserbasehq/stagehand exec vitest run --config /tmp/stagehand-vitest-source.config.mjs tests/unit/public-api/public-types.test.ts tests/unit/agent-execution- model.test.ts tests/unit/timeout-handlers.test.ts tests/unit/api-client-observe-variables.test.tspackages/core/lib/v3/launch/browserbase.tsand existing server test environment/type-resolutionfailures.