Skip to content

feat(browser): support local-only screenshots - #122

Closed
MagMueller wants to merge 1 commit into
mainfrom
screenshot-context
Closed

feat(browser): support local-only screenshots#122
MagMueller wants to merge 1 commit into
mainfrom
screenshot-context

Conversation

@MagMueller

@MagMueller MagMueller commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add attachToContext?: false to Page.captureScreenshot calls inside browser_execute
  • continue returning screenshot bytes to the snippet while excluding opted-out screenshots from model attachments
  • document the opt-out in the browser tool description

Test plan

  • full browser-execute.test.ts against local headless Chrome: 8 passed
  • bun typecheck from packages/bcode-browser
  • repository pre-push typecheck (16 packages)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Re-trigger cubic

@MagMueller
MagMueller force-pushed the screenshot-context branch from ee058d8 to 2543d59 Compare July 28, 2026 18:55
@MagMueller MagMueller changed the title fix(browser): recover oversized screenshot context feat(browser): support local-only screenshots Jul 28, 2026
@Alezander9

Copy link
Copy Markdown
Member

Thanks Magnus — the need here is real and not promptable away, which puts it in a different category from #112/#119. An agent genuinely cannot opt out of screenshot attachment today, and its only escapes are worse (Runtime.evaluate + html2canvas, or Page.printToPDF). So this was a real gap.

We're taking a different route for it in #124 — documentation of a real CDP mechanism rather than new surface. Reasoning below, plus one finding from your implementation that's worth knowing regardless.

Why not the CDP param

attachToContext isn't a Page.captureScreenshot field. The real param list is six entries (format, quality, clip, fromSurface, captureBeyondViewport, optimizeForSpeed). More to the point, the thing it controls doesn't exist in CDP at any layer — Chrome has no notion of who consumes the bytes. "Attachment" is entirely a BrowserCode concept living above the protocol, so a CDP param object is the wrong place to express it.

That matters more than usual here because the skill actively teaches the agent that its CDP priors are valid: "The full surface (652 commands) is the Chrome DevTools Protocol", and "For unknown param shapes, call with {} and inspect the thrown CdpError." One invented field on one method undercuts exactly that, and an agent that internalizes it will write it against a real CDP client and be wrong.

The bug worth knowing about

Independent of the design question: the opt-out is carried by object identity. The Proxy destructures attachToContext off, puts the resulting cdpParams in a WeakSet, and the tap checks localOnlyScreenshotParams.has(p). That works only because _call currently happens to pass through the identical object reference.

Any future change that clones or normalizes params — a retry, a param validator, a logging pass — silently breaks it, and it breaks in the direction of attaching the screenshot you asked to keep private. Worth avoiding that pattern generally: correlate on data, not on pointer identity, especially when the failure mode is a silent privacy/context leak rather than an error.

Also, browser-execute.txt claims attachToContext: false "does not save it", but BCODE_SCREENSHOT_DIR writing is untouched and the PR's own test asserts the file lands on disk.

Why not a tool parameter either

That was our first counter-proposal and we talked ourselves out of it. browser_execute is deliberately code + description (+ a rarely-touched timeout). A tool parameter is eagerly loaded into every context on every call, forever — a permanent tax on all users to serve a rare workflow. Wrong price.

What we did instead

The tap subscribes to onCallResult, which fires only from _call's resolve path (session.ts:252-256) — the command/response channel. CDP events route through a separate listener list (session.ts:273-277) and never reach it. So Page.startScreencastPage.screencastFrame gives base64 pixels that are structurally unattachable, using nothing but real protocol.

#124 documents that in skills/browser-execute/SKILL.md, next to the auto-attach explanation. 15 lines in a lazily-loaded doc instead of a permanent field in an eagerly-loaded schema — and it generalizes past og:image to any bulk-capture case, which was the stronger motivation once we looked for it.

Verified against Chrome 150 --headless=new before writing it down: a frame arrives in 7ms on a fully static idle page (no nudge needed), frames produce zero attachments, and setDeviceMetricsOverride controls exact frame dimensions. Two protocol details also fell out that are recorded in #124 — ack is unnecessary for navigation-paced capture but required for continuous capture (3 frames/3s unacked vs 178 acked), and the event's sessionId is not a frame counter despite the protocol calling it "Frame number".

Closing in favor of #124. The default stays auto-attach — you were right that it's the common case and it saves a read call nearly every time.

@Alezander9 Alezander9 closed this Jul 29, 2026
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.

2 participants