Puppeteer screenshot feature#4634
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9812a8af5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export type ScreenshotPrerenderArgs = { | ||
| realm: string; | ||
| url: string; | ||
| auth: string; | ||
| format: 'isolated' | 'embedded'; | ||
| }; |
There was a problem hiding this comment.
Thread screenshot priority through prerender call chain
/_screenshot-card enqueues with userInitiatedPriority, but the screenshot prerender API drops that signal because ScreenshotPrerenderArgs has no priority field (and the task call therefore cannot forward jobInfo.priority). In busy environments (for example while indexer traffic is active), screenshot renders will compete at default priority and can be delayed behind background work, unlike runCommand which explicitly forwards priority to prerender queues.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds the Phase 1 backend plumbing for a queue-backed “screenshot a card render” flow, wiring realm-server → queue/worker → prerender-server → Puppeteer page.screenshot() and returning a JSON:API envelope containing base64 PNG output.
Changes:
- Introduces a new
screenshot-cardqueue job + worker task that checks permissions and callsprerenderer.prerenderScreenshot(...). - Adds a new realm-server endpoint
POST /_screenshot-cardplus tests for request validation and handler/job wiring. - Extends the prerender-server with a
/prerender-screenshotroute and Puppeteer capture implementation.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/worker.ts | Registers the new screenshot-card job type with the worker queue. |
| packages/runtime-common/tasks/screenshot-card.ts | Implements the worker task: permission checks + calls prerenderer.prerenderScreenshot. |
| packages/runtime-common/tasks/index.ts | Exports the new screenshot task. |
| packages/runtime-common/jobs/screenshot-card.ts | Adds the enqueue helper + timeout/concurrency group for screenshot jobs. |
| packages/runtime-common/index.ts | Introduces screenshot prerender request/response types and extends the Prerenderer interface. |
| packages/realm-server/routes.ts | Wires POST /_screenshot-card into the realm-server router behind JWT middleware. |
| packages/realm-server/handlers/handle-screenshot-card.ts | Implements the endpoint handler: validates JSON:API body, enqueues job, returns result. |
| packages/realm-server/prerender/utils.ts | Adds Puppeteer capture helper captureScreenshot() (settle + screenshot + error detection). |
| packages/realm-server/prerender/render-runner.ts | Adds captureScreenshotAttempt to drive the screenshot render flow on a pooled page. |
| packages/realm-server/prerender/prerenderer.ts | Exposes prerenderScreenshot() on the prerender-server’s local Prerenderer implementation. |
| packages/realm-server/prerender/prerender-app.ts | Adds /prerender-screenshot route parsing + dispatch to prerenderer.prerenderScreenshot. |
| packages/realm-server/prerender/remote-prerenderer.ts | Adds remote client support for calling /prerender-screenshot. |
| packages/realm-server/tests/screenshot-card-test.ts | Adds handler-level tests that assert the job enqueue shape and response forwarding. |
| packages/realm-server/tests/server-endpoints/screenshot-card-endpoint-test.ts | Adds endpoint validation tests (auth required, required attrs, invalid JSON/format). |
| packages/realm-server/tests/index.ts | Registers the new tests in the realm-server test suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let result = await prerenderer.prerenderScreenshot({ | ||
| realm: normalizedRealmURL, | ||
| url: cardId, | ||
| auth, | ||
| format, | ||
| }); |
| let rawAffinityValue = attrs.affinityValue; | ||
| let rawFormat = attrs.format; | ||
| let renderOptions = parseRenderOptions(attrs); | ||
| let formatIsValid = rawFormat === 'isolated' || rawFormat === 'embedded'; | ||
| let missing = missingAttrs([ | ||
| { value: rawUrl, name: 'url' }, | ||
| { value: rawRealm, name: 'realm' }, | ||
| { value: rawAuth, name: 'auth' }, | ||
| { | ||
| value: rawAffinityType === 'realm' ? rawAffinityType : undefined, | ||
| name: 'affinityType', | ||
| }, | ||
| { value: rawAffinityValue, name: 'affinityValue' }, | ||
| { | ||
| value: formatIsValid ? rawFormat : undefined, | ||
| name: 'format', | ||
| }, | ||
| ]); | ||
| return { | ||
| args: | ||
| missing.length > 0 | ||
| ? undefined | ||
| : { | ||
| affinityType: 'realm', | ||
| affinityValue: rawAffinityValue as string, | ||
| realm: rawRealm as string, | ||
| url: rawUrl as string, | ||
| auth: rawAuth as string, | ||
| format: rawFormat as 'isolated' | 'embedded', | ||
| renderOptions, | ||
| }, |
| const { page, reused, launchMs, waits, pageId, release } = | ||
| await this.#getPageForAffinity(affinityKey, auth, 'file', signal); | ||
| const poolInfo: PoolInfo = { |
There was a problem hiding this comment.
this is a new capability. in order to make sure that your prerender requests are prioritized above background reindexing, you should thread thru the job priority. this should already be happening for commands, modules, and indexing. so use those examples.
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 3h 57m 19s ⏱️ + 1h 56m 15s Results for commit 42f73fc. ± Comparison against earlier commit f11f64b. For more details on these errors, see this check. Realm Server Test Results 1 files ± 0 1 suites ±0 18m 3s ⏱️ -9s Results for commit 42f73fc. ± Comparison against earlier commit f11f64b. |
|
UPDATE: you are actually getting the page correctly that respects the queuing and perf considerations |
|
Also, I know that this is triggered from a command, but it seems like this could really benefit from realm affinity such that you don't land on a cold loader or a cold store when you are trying to take a snapshot. i don't think (at least today) that the user auth for the realm affinity would be different than the user auth for the user affinity when you are doing a screenshot, since for private realms the only user that can access the realm is the user that created it. so this is probably ok. for in the future when we have shared realms this is probably not quite right... |
| ); | ||
|
|
||
| const { page, reused, launchMs, waits, pageId, release } = | ||
| await this.#getPageForAffinity( |
There was a problem hiding this comment.
ah, nvm about my comment about not using the queue. this is the correct way to get a page that is mindful of the prerender queue 👍
Card Screenshot Infrastructure
End-to-end screenshot capture for any Boxel card. Open the new
Screenshot Card Demo in the experiments realm, link a card, pick
isolatedorembedded, click Take Screenshot — a fully-paintedPNG is rendered through the existing Puppeteer prerender pool, written
into the card's own realm under
Screenshots/, and shown inline.What's in the PR
POST /_screenshot-cardon therealm-server. Queue-backed, mirroring
/_run-command. Returns base64PNG + dimensions.
ScreenshotCardCommandcallablein-process or via
/_run-command. Writes the PNG to the card's realmvia
WriteBinaryFileCommand. Hard-fails if the caller can't writethere.
ScreenshotCardDemoinpackages/experiments-realm— the harness that drives the full flowwith one button.
Phase 4 (wiring into
listing-create) lives in a separate repo / PR.How to test
prerender-server, prerender-manager, worker, host).
Screenshot Card Demoinstance.
Author/alice-enwunderorImageDefPlayground/mango-demo).isolatedorembeddedfrom thedropdown.
Expected: the result section appears with a realm URL and an inline
PNG preview that fully shows the card content — no spinners, no empty
image placeholders. The file lands at
packages/experiments-realm/Screenshots/<slug>-<uuid8>.png.If you want to hit the API directly:
formatmust be"isolated"or"embedded".runAsis derived fromthe JWT.
Automated tests
10/10 each. The server-endpoints test must run alongside another
server-endpoints test (shares cached realm setup).
Notes for reviewers
captureScreenshotinpackages/realm-server/prerender/utils.ts calls
__waitForRenderLoadStability()(existing) and a newwaitForImagePaint()that waits for<img>elements, CSSbackground-imageURLs, anddocument.fonts.readybefore capture.Without the second wait, avatars and thumbnails would be missing in
the PNG.
/_screenshot-card),prerender-manager (
/prerender-screenshotproxy), prerender-server(
/prerender-screenshotroute). Easy to forget the manager.imageDefUrl: string, notlinksTo(ImageDef)— wetried the cleaner relationship form, but the framework's
link-resolution lifecycle on a freshly-written PNG took a path the
auth service worker didn't cover (manifested as "Missing
Authorization header"). Plain
<img src=URL>goes through the SWreliably (same path that makes
ImageDefPlayground/mango.pngrender). Cleaner relationship is a follow-up.
ScreenshotCardCommandwrites the PNG intothe card's own realm. If the caller lacks write access there it
throws; no silent fallback.
screenshot:<realmURL>,mirroring
command:<realmURL>for run-command.