Add setInputFiles - #2559
Merged
Merged
Conversation
|
Contributor
Author
Contributor
|
@monadoid I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 30 files
Architecture diagram
sequenceDiagram
participant SDK as SDK Client (TS/Py/Go)
participant FileSys as Local Filesystem
participant Normalizer as FileNormalizer
participant RPC as RPC Client
participant Server as Stagehand Server
participant Runtime as StagehandRuntime
participant Understudy as Understudy Locator
participant Browser as Browser (CDP Session)
Note over SDK,Browser: File Upload Flow via locator.setInputFiles
SDK->>SDK: User calls setInputFiles(pathOrPayload)
SDK->>FileSys: Read file (stat, readFile) for path inputs
FileSys-->>SDK: file buffer + metadata
SDK->>Normalizer: normalizeFileInput()
Normalizer->>Normalizer: Convert to InputFilePayload (base64 data, name, mimeType, lastModified)
alt Empty array passed
Normalizer->>Normalizer: Return empty payload list (clear selection)
end
Normalizer-->>SDK: InputFilePayload[]
SDK->>RPC: send("locator.set_input_files", {page_id, selector, files: [...]})
RPC->>Server: JSON-RPC request (method: "locator.set_input_files")
Server->>Runtime: locatorSetInputFiles(params)
Runtime->>Runtime: Resolve locator from pageId + selector
Runtime->>Runtime: Decode base64 → Uint8Array buffer
Runtime->>Understudy: setInputFiles([{name, mimeType, buffer, lastModified}])
Understudy->>Browser: Resolve objectId for input selector
Understudy->>Browser: CDP Input.dispatchFileInput / setFileInputFiles
Browser-->>Understudy: File(s) attached to input element
Understudy-->>Runtime: void (success)
Runtime-->>Server: { set: true }
Server-->>RPC: JSON-RPC response
RPC-->>SDK: { set: true }
SDK-->>SDK: Resolve Promise/await
Note over Understudy,Browser: For empty files: clear selection via CDP
Understudy->>Browser: DOM.setFileInputFiles with empty array (or file payload injection)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
monadoid
marked this pull request as ready for review
August 3, 2026 22:06
Contributor
There was a problem hiding this comment.
13 issues found across 33 files
Confidence score: 3/5
- The highest-risk gap is around instrumentation coverage for the new upload API:
locator.set_input_filesappears not to be fully enforced by the flow-logger contract inpackages/protocol/tests/protocol/schema-registry.test-d.ts, so calls could bypass expected tracing/observability and make debugging or policy enforcement harder — instrument all newly exposed page/locator methods and lock it with schema-registry checks. - Behavior changed in
packages/server/understudy/locator.tswithout integration coverage, andpackages/server/tests/stagehand-clients.test.tscurrently misseslocator.nth(...).setInputFiles(...)visibility, so routing/clearing regressions can slip through undetected — add server integration tests for the new clearing semantics and nth-path call assertions. - Large upload paths in
packages/sdk-ts/src/fileUpload.tsandpackages/server/runtime.tsdo avoidable parallel read/encode/decode work, which can spike memory/CPU and increase timeout/OOM risk on big payloads — switch to sequential or bounded-concurrency reads and avoid redundant base64 transforms across RPC/runtime boundaries. - Cross-SDK edge handling is still inconsistent:
packages/sdk-go/locator.gocan bypass the 50 MiB limit if files change afterStat,packages/sdk-python/src/stagehand/file_upload.pycan leak raw OS read errors, andpackages/sdk-ts/src/fileUpload.tscan emit invalid negativelastModifiedfor pre-1970 files — add post-read size validation, normalize timestamps, and map local file-read failures to stable SDK error types.
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/server/understudy/locator.ts">
<violation number="1" location="packages/server/understudy/locator.ts:99">
P2: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
This PR changes the server-side clearing behavior for `locator.setInputFiles` (empty array case) to route through `assignFilesViaPayloadInjection`. That changed code path should be covered by a server integration test in `packages/server/tests/` so regressions in the clearing branch are caught at the RPC/runtime boundary. Consider adding an empty-array variant to the existing `locator.set_input_files` handler tests.</violation>
</file>
<file name="packages/server/tests/stagehand-clients.test.ts">
<violation number="1" location="packages/server/tests/stagehand-clients.test.ts:562">
P3: Calls through `locator.nth(...).setInputFiles(...)` are not observable by this fake, so an nth-specific routing regression can pass without being detected. Sharing the call recorder across `nth()` clones or adding an assertion against the returned clone would make this new test double cover the supported nth path.</violation>
</file>
<file name="packages/sdk-ts/src/fileUpload.ts">
<violation number="1" location="packages/sdk-ts/src/fileUpload.ts:18">
P2: Large multi-file uploads can cause avoidable memory spikes because all files are read and encoded concurrently. Processing entries sequentially (or with bounded concurrency) keeps resource usage predictable.</violation>
<violation number="2" location="packages/sdk-ts/src/fileUpload.ts:36">
P3: Local files with a pre-1970 modification time cannot be uploaded: `Math.trunc(fileStat.mtimeMs)` can remain negative, while the `InputFilePayload` protocol rejects negative `lastModified` values. Normalizing this generated metadata to the protocol's valid range (or omitting it when invalid) would keep path uploads working.</violation>
<violation number="3" location="packages/sdk-ts/src/fileUpload.ts:41">
P3: The `instanceof Uint8Array` check is redundant because both branches produce the same value. Simplifying to a single `Buffer.from(file.buffer)` keeps this conversion path easier to read.</violation>
</file>
<file name="packages/protocol/tests/protocol/schema-registry.test-d.ts">
<violation number="1" location="packages/protocol/tests/protocol/schema-registry.test-d.ts:89">
P2: Custom agent: **Ensure all public methods added to the stagehand class, agent, or understudy (page, locator, etc.) interfaces are properly instrumented with the flowLogger**
The new `locator.set_input_files` browser action is currently untracked. Both the public TypeScript `Locator.setInputFiles` wrapper and the server understudy locator implementation perform the upload without a flowLogger decorator or manual flowLogger call, even though Rule 3 requires significant `locator.*` actions to be traced. Adding the equivalent flowLogger instrumentation to the new locator method would keep file uploads visible in logging and span tracking.</violation>
</file>
<file name="packages/protocol/schemas.ts">
<violation number="1" location="packages/protocol/schemas.ts:1788">
P3: The new 50 MiB decoded-size guard is not covered by the protocol tests: `object-model-protocol.test.ts` only exercises three small padding cases and malformed base64. A regression in the encoded-length calculation or the refine boundary could therefore accept an oversized payload (or reject a valid 50 MiB payload) without failing CI. Focused cases for exactly 50 MiB and 50 MiB + 1 byte would make this contract executable.
(Based on your team's feedback about unit tests for new behavior.)</violation>
</file>
<file name="packages/sdk-go/locator.go">
<violation number="1" location="packages/sdk-go/locator.go:245">
P2: The 50 MiB client guard can be bypassed when a file grows between `Stat` and `os.ReadFile`, because size is only checked pre-read. A post-read length check (or bounded read) would keep limit enforcement consistent.</violation>
<violation number="2" location="packages/sdk-go/locator.go:249">
P3: Uploading a valid local file with a pre-1970 modification time fails at the RPC boundary because `UnixMilli()` can produce a negative `last_modified`, while the protocol only accepts nonnegative values. Treat this optional metadata as absent (or return a local validation error) when it is negative, and apply the same validation to `FileInput.LastModified`.</violation>
</file>
<file name="packages/docs/v4/reference/locator.mdx">
<violation number="1" location="packages/docs/v4/reference/locator.mdx:332">
P2: Custom agent: **Stagehand docs prose guide**
The new `setInputFiles()` docs use passive voice ('Relative paths are resolved...' and 'Files are serialized...'), which violates the active-voice requirement in the Stagehand docs prose guide. Please rewrite these sentences with the actor performing the action. For example:
- 'Relative paths are resolved on the SDK caller's machine.' → 'The SDK resolves relative paths on the caller's machine.'
- 'Files are serialized in memory and limited to 50 MiB each.' → 'The SDK serializes each file in memory and limits it to 50 MiB.'
The same passive wording should also be updated in the Python `set_input_files()` section.</violation>
</file>
<file name="packages/sdk-python/src/stagehand/file_upload.py">
<violation number="1" location="packages/sdk-python/src/stagehand/file_upload.py:36">
P2: Unreadable or race-deleted files can currently bubble raw OS exceptions from file reads, which makes `set_input_files()` error behavior inconsistent for callers. Wrapping local-file stat/read in `OSError` handling and re-raising the existing `ValueError` message would keep failures deterministic.</violation>
<violation number="2" location="packages/sdk-python/src/stagehand/file_upload.py:45">
P3: Invalid sequence entries currently fail with `AttributeError` when `_normalize_file` accesses `file.name`, which is hard to diagnose from API usage. An explicit `FilePayload` type check before field access would return a clear `ValueError` for unsupported item types.</violation>
</file>
<file name="packages/server/runtime.ts">
<violation number="1" location="packages/server/runtime.ts:725">
P2: Large uploads do extra CPU/memory work because RPC base64 payloads are decoded in runtime and then encoded again in locator injection. Consider passing already-encoded payloads through this path (or adding a base64-aware locator helper) to avoid the double conversion for up-to-50 MiB files.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User Code (TS/Py/Go)
participant SDK as SDK (Normalization)
participant FS as Local Filesystem
participant Protocol as JSON-RPC (Protocol)
participant Server as Stagehand Server
participant Browser as Browser (Understudy)
Note over User,SDK: File Preparation Phase
User->>SDK: NEW: setInputFiles(files)
alt If input is file path
SDK->>FS: resolve() & stat()
FS-->>SDK: file metadata (mtime, size)
opt File > 50MiB
SDK-->>User: Throw Range/Value Error
end
SDK->>FS: readFile()
FS-->>SDK: bytes
else If input is payload/buffer
SDK->>SDK: Validate buffer size
end
Note over SDK,Protocol: Serialization Phase
SDK->>SDK: NEW: Base64 encode data
SDK->>Protocol: Request: locator.set_input_files
Note right of Protocol: Payload: name, mimeType, data (b64), lastModified
Note over Protocol,Server: Server Routing Phase
Protocol->>Server: RPCRouter: route request
Server->>Server: locatorController: setInputFiles()
Note over Server,Browser: Execution Phase
Server->>Server: NEW: StagehandRuntime: Decode Base64 to bytes
alt Files array not empty
Server->>Browser: UnderstudyLocator: setInputFiles(normalized)
Browser->>Browser: CDP: DOM.setFileInputFiles
else Files array empty (Clear)
Server->>Browser: UnderstudyLocator: setInputFiles([])
Browser->>Browser: NEW: Payload Injection (Clear Selection)
end
Browser-->>Server: void
Server-->>Protocol: Response: { "set": true }
Protocol-->>SDK: Response
SDK-->>User: Resolve/Success
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
# Conflicts: # packages/sdk-go/internal/extensionassets/stagehand-extension.zip # packages/sdk-python/tests/test_rpc_client.py
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
# Conflicts: # packages/sdk-go/internal/extensionassets/stagehand-extension.zip
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
# Conflicts: # packages/sdk-go/internal/extensionassets/stagehand-extension.zip
# Conflicts: # packages/docs/tests/sdk-reference.test.ts # packages/protocol/schema-registry.ts # packages/protocol/stagehand.v4.json # packages/sdk-go/internal/extensionassets/stagehand-extension.zip
Member
|
@monadoid could we add one bb smoke test for this? just to confirm there is no remote browser weirdness. maybe in |
seanmcguire12
approved these changes
Aug 4, 2026
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.
Summary
add file input upload support to all three SDKs.
Supports local paths, multiple files, in-memory payloads, and clearing the selection. Also adds runnable, model-free examples and end-to-end coverage.
Summary by cubic
Adds cross-SDK file uploads on locators:
setInputFiles(TS),set_input_files(Python), andSetInputFiles(Go). Supports local paths, in-memory payloads, multiple files, and clearing; the protocol enforces a strict per-file 50 MiB decoded limit.locator.setInputFiles(files)withFileInput/FilePayloadnormalization (base64 transport, size checks, non-negativelastModified); types exported; example, integration, and unit tests plus a Browserbase smoke test for remote upload and clearing.locator.set_input_files(files)withFileInput/FilePayloaddataclass normalization; omits unset metadata in RPC; example and tests added; types exported fromstagehand.locator.SetInputFiles(ctx, files...)withFilePath/FileData; reads, validates, and base64-encodes files; example and tests added.locator.set_input_files;InputFilePayload(base64data, optionalmimeType/lastModified) with decoded-size enforcement; router/controller/runtime wiring added; server decodes to bytes and forwards to the runtime locator; empty arrays clear via payload injection.Written for commit ef13797. Summary will update on new commits.