Skip to content

fix(cli): omit undefined screenshot options - #2528

Draft
shrey150 wants to merge 1 commit into
v4-spike-cli-actionsfrom
v4-spike-cli-page-runtime
Draft

fix(cli): omit undefined screenshot options#2528
shrey150 wants to merge 1 commit into
v4-spike-cli-actionsfrom
v4-spike-cli-page-runtime

Conversation

@shrey150

@shrey150 shrey150 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • omit absent screenshot options instead of serializing explicit undefined values onto the V4 wire
  • preserve the existing screenshot command syntax, file/base64 result shapes, timeout, and populated option behavior
  • add a regression test for both default and fully populated screenshot requests

This is the next draft in the CLI migration stack and is based on #2526.

CLI contract changes

Surface Contract in this draft
screenshot syntax and result JSON Unchanged
Omitted screenshot flags No longer appear as explicit undefined properties in the V4 page request
Populated screenshot flags Continue mapping to animations, caret, clip, fullPage, quality, and type
Default timeout Remains 10 seconds

E2E Test Matrix

Command / flow Observed output Confidence / sufficiency
BROWSE_DAEMON_DIR=<temp> pnpm --filter browse test:unit 24 files passed; 367 tests passed Proves the screenshot regression and the existing CLI command/daemon contracts
pnpm check Formatting, lint, and typecheck completed; 9/9 tasks succeeded Proves the exact stacked source passes the repository gate; lint emitted warnings but no errors
Local built CLI: open about:blank with --local, run screenshot --base64, decode the result, then stop Screenshot returned a non-empty 3,881-byte PNG and cleanup succeeded Exercises the real local browser, launch-loaded extension, V4 screenshot RPC, CLI base64 output, and daemon lifecycle end to end

No Changeset is included because this remains part of the unreleased V4 spike stack.


Summary by cubic

Omit undefined screenshot options so V4 page requests only include flags that are set. Screenshot syntax, output (file/base64), and the 10s timeout remain unchanged.

  • Bug Fixes
    • Include animations, caret, clip, fullPage, quality, and type only when provided; omit them otherwise.
    • Added regression test to verify minimal { timeout: 10_000 } calls and a fully populated request.

Written for commit db7b4ca. Summary will update on new commits.

Review in cubic

@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: db7b4ca

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant