fix(cli): omit null fields in screenshot command #256
Conversation
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
The CLI was sending selector: null and path: null in the JSON payload when these optional fields were not provided. While the daemon now accepts null values (via .nullish() in Zod), omitting the fields entirely is cleaner and aligns with the original schema expectation. This is a complementary fix to vercel-labs#236 - the daemon accepts null, but the CLI should not send it in the first place. Related to vercel-labs#238
Contributor
|
@edxeth is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
The CLI was sending
selector: nullandpath: nullin the JSON payload when these optional fields were not provided. While #236 fixed this on the daemon side by changing Zod validation from.optional()to.nullish(), the CLI should not send null values in the first place.This is a complementary fix that makes the protocol cleaner:
{"action": "screenshot", "path": null, "selector": null, "fullPage": false}{"action": "screenshot", "fullPage": false}Changes
cli/src/commands.rsto conditionally includepathandselectorfields only when they have valuesTesting
agent-browser screenshot(no args) - worksagent-browser screenshot /tmp/test.png(with path) - worksagent-browser screenshot @e1(with selector) - worksagent-browser screenshot @e1 /tmp/test.png(both) - worksRelated to #238