fix(desktop): preview snapshots no longer stall on Chromium's legacy capture path#32415

open3

main

fix/issue-32410

View on GitHubTwo-way sync enabledPR last imported 7h ago

Problem

Desktop Browser snapshots intermittently fail with Preview automation snapshot timed out after 15000ms and can keep failing until the tab is replaced. The desktop browser host keeps a preview tab offscreen and makes it paintable only while Page.captureScreenshot is pending; on that path Chromium's legacy CDP capture can stop answering. Playwright turns on CDPScreenshotNewSurface whenever it launches Chromium or Electron itself, but T3 launches Electron on its own and attaches over CDP, so the desktop app never gets it.

Reproduced on main (83a3674ffd) on Linux x86_64 under Xvfb with the reporter's native probe, which wires a real Electron <webview> through T3's createCdpRelayConnection and calls the real ServerBrowserPage.snapshot 100 times: every run stalled within the first 5 snapshots (with --disable-gpu and without) and failed with The page was too busy to capture a snapshot in time… after 10 s.

Change

Before Electron is ready, desktop startup appends --enable-features=CDPScreenshotNewSurface on every platform. The value is merged into any existing --enable-features list, because Chromium keeps only the last value of a features switch (the same reason the Linux --disable-features merge exists). Nothing is added when the feature is already enabled or when the user turned it off with --disable-features.

Scope and approval

Fixes pingdotgg/t3code#32410

Verification

vp test run apps/desktop/src/app/DesktopPreReadyPlatform.test.ts apps/desktop/src/app/DesktopEarlyElectronStartup.test.ts: 29 passed. The new tests fail on main: on darwin, win32 and linux startup leaves no feature (expected [] to have the same members as [ 'CDPScreenshotNewSurface' ]), and an existing --enable-features list is not extended. A third test covers the --disable-features opt-out.
vp run check:changed: format, lint, knip, typecheck and tests pass.
Native probe, Electron 44.10.1 under Xvfb, --no-sandbox, startup through the real DesktopPreReadyPlatform.make of each side:

Side	Startup `enable-features`	Runs	Result
Before (main 83a3674ffd)	(none)	3	stalled at snapshot 4, 3, 3; each failed after ~10 s with the busy-page error, with 3.6–3.8 s captures before it
After (79958c1eb6)	`CDPScreenshotNewSurface`	3	100/100 snapshots each, current pixels and accessibility text verified, median 63–64 ms, max 155 ms
After, launched with `--enable-features=WebRTCPipeWireCapturer`	`WebRTCPipeWireCapturer,CDPScreenshotNewSurface`	1	100/100, max 139 ms

Driver: the reporter's probe gist (build.mjs, host.html, reproduce.cjs), run unchanged with PROBE_PRODUCTION=1.

Before transcript, run 1 (main)

1	{"event":"startup","features":""}
2	{"event":"snapshot","iteration":0,"feature":false,"ms":121,"size":[500,300],"rgb":[255,0,0],"tree":true}
3	{"event":"snapshot","iteration":1,"feature":false,"ms":84,"size":[500,300],"rgb":[0,0,255],"tree":true}
4	{"event":"snapshot","iteration":2,"feature":false,"ms":3594,"size":[500,300],"rgb":[255,0,0],"tree":true}
5	{"event":"snapshot","iteration":3,"feature":false,"ms":82,"size":[500,300],"rgb":[0,0,255],"tree":true}
6	{"event":"failure","iteration":4,"feature":false,"ms":10010,"message":"The page was too busy to capture a snapshot in time. Wait for it to settle and take another snapshot."}

Independent run on a physical Linux x64 GNOME/Wayland desktop (GPU and sandbox on), reported in this PR's comments: with no feature enabled, the same probe stalled at snapshot 12 after 10,011 ms with the busy-page error; with CDPScreenshotNewSurface enabled through DesktopPreReadyPlatform.make, 100/100 passed. That after-run used the equivalent change in pingdotgg/t3code#32412, not this head.

Not checked: physical macOS and Windows hosts, and a minimized main window. The runtime proof covers Linux/X11; other platforms are covered by the unit tests and by Playwright enabling the same feature by default on every platform.

AI-assisted; verified with the tests and tools listed above.

Checks

22 successful · 11 skipped · 1 neutral

All checks passed

Added 2 commits12h ago

fix(desktop): preview snapshots no longer stall on Chromium's legacy capture path79958c1
test(desktop): reproduce #32410 CDP screenshots without a fresh capture surfacefdf304b

MmacroscopeappBotreviewed12h ago

All clear

^{Posted via Macroscope — Effect Service Conventions}

MmacroscopeappBotreviewed · 1 inline thread12h ago

apps/desktop/src/app/DesktopPreReadyPlatform.test.tslines 150–150Outdated · 79958c1Resolved by Julius Marminge

CcoderabbitaiBot12h ago

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Startup resolves CDPScreenshotNewSurface against existing enable and disable feature values. Pre-ready launch arguments include the resolved enable switch when applicable. Tests cover macOS, Windows, and Linux.

Changes: CDP screenshot feature switch

Layer / File(s)	Summary
Feature switch resolution `apps/desktop/src/app/DesktopEarlyElectronStartup.ts`	The resolver preserves existing feature entries. It returns `null` if the feature is already enabled or disabled; otherwise, it adds the feature to the enable value.
Pre-ready launch integration and tests `apps/desktop/src/app/DesktopPreReadyPlatform.ts`, `apps/desktop/src/app/DesktopPreReadyPlatform.test.ts`	Pre-ready startup appends the resolved enable switch when present. Tests cover three platforms, retained enable entries, and an existing disable entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

pingdotgg/t3code#23022 — The issue describes an offscreen guest capture stall. This change enables a different CDP screenshot path.
pingdotgg/t3code#23514 — The issue reports an unbounded desktop Page.captureScreenshot. This change alters the CDP screenshot path; deadlines and queue recovery are not included.

Possibly related PRs

pingdotgg/t3code#32412 — Both changes enable CDPScreenshotNewSurface before Electron is ready.
pingdotgg/t3code#28991 — It changes pre-ready configuration and boot timing, which affect when this change applies the screenshot switch.
pingdotgg/t3code#24133 — Both changes address desktop preview screenshot stalls, using different capture-path changes.

Suggested reviewers: maria-rcks

Merge Risk: External image Minimal · up to ab788

No actionable merge-blocking issue was found in the startup switch change.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title describes the desktop snapshot stall addressed by the change.
Linked Issues check	Passed	[`#32410`] Pre-ready startup enables `CDPScreenshotNewSurface` and preserves existing feature flags and explicit opt-outs. The current summary reports cross-platform enablement tests and tests for flag …
Out of Scope Changes check	Passed	[`#32410`] The startup changes and tests support the snapshot fix and preserve existing feature settings. No unrelated changes are identified in the whole-PR summary.
Description check	Passed	The description is mostly complete. It explains the problem, change, linked issue, verification results, and test limits. It does not state whether the issue was triaged or identify the AI model and h…

Full details: Linked Issues check

Explanation

[#32410] Pre-ready startup enables CDPScreenshotNewSurface and preserves existing feature flags and explicit opt-outs. The current summary reports cross-platform enablement tests and tests for flag preservation and opt-out. The author reports a successful native snapshot probe. The incremental diff was unavailable; the current whole-PR evidence supports the assessment.

Full details: Description check

Explanation

The description is mostly complete. It explains the problem, change, linked issue, verification results, and test limits. It does not state whether the issue was triaged or identify the AI model and harness.

Finishing Touches

Generate docstrings

Commit to this branch
Create a new PR

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-32410
File: .coderabbit.config.ts
Commit: ab78833
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: `0f7ce523-fe75-4ecf-b390-d9c40e57b88e`

External image Commits

Reviewing files that changed from the base of the PR and between 79958c1eb6e297c7edc5529119bc597d5de0199a and ab7883384e93103b127db38b744f50a0699cec56.

External image Files selected for processing: 1

`apps/desktop/src/app/DesktopPreReadyPlatform.test.ts`

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

MmacroscopeappBot12h ago

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — Desktop startup now enables Chromium's newer screenshot capture path by default to prevent preview snapshot stalls, while respecting explicit disables. Adopting this product-default change requires human review.

Notes:

No code objects were reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

Added 1 commit12h ago

test(desktop): run the enable-features merge case on one platformab78833

Sshivam12h ago

[GPT-6.1 Sol][Codex] on behalf of shivam.

Independent #32410 findings from corex's physical Linux x64 desktop, GNOME/Wayland, Electron 44.10.1, Playwright Core 1.60.0. Normal GPU rendering and sandbox remained enabled. The native probe used an embedded webview, the real createCdpRelayConnection and ServerBrowserPage.snapshot, with separate profile/state directories for each side. Its window used showInactive().

This tested the earlier fix in https://github.com/pingdotgg/t3code/pull/32412 at 4fa36c54ae2e0d0e30db7983d4da716cfb358e3f, based on 83a3674ffd84ffb33de768210c806cb1ac53fd80, not this PR's head. With production startup bypassed and no enabled feature, 12 captures succeeded, then iteration 12 stalled for 10,011 ms and returned the busy-page error. Executing that fix's real DesktopPreReadyPlatform.make enabled CDPScreenshotNewSurface; all 100 captures passed checks for changing PNG pixels and current accessibility labels. I inspected the final blue image with button text Frame 99.

Retained transcript excerpts:

json

1	{"event":"failure","iteration":12,"feature":false,"ms":10011,"message":"The page was too busy to capture a snapshot in time. Wait for it to settle and take another snapshot."}
2	{"event":"startup","features":"CDPScreenshotNewSurface"}
3	{"event":"snapshot","iteration":99,"feature":false,"ms":45,"size":[500,300],"rgb":[0,0,255],"tree":true}

The per-capture feature:false field is the unused manual override, not the effective feature setting. vp test run apps/desktop/src/app/DesktopPreReadyPlatform.test.ts also passed all seven tests at the earlier head. Fresh Codex review found no blocking code findings there.

Limits: the probe bypasses DesktopBrowserHost and renderer IPC, so this is native capture-path evidence rather than a full packaged-app pass. Minimized windows and physical macOS/Windows were not tested. No tests were run against #32415. Test processes exited and temporary artifacts were cleaned up. Handing this job over to you; no duplicate PR or further changes.

WWladefantreviewed12h ago

Separate nonauthor exact-head source review — T3 desktop capture startup, 7 Nov 2026 (Refs pingdotgg/t3code#32410 #32412 #32415; downstream Wladefant/superboard#1777 #1779 #1782 #1798 #1899).

Candidate: OPEN, unmerged ab7883384e93103b127db38b744f50a0699cec56 -> main@83a3674ffd84ffb33de768210c806cb1ac53fd80; 3 paths (DesktopEarlyElectronStartup.ts, DesktopPreReadyPlatform.ts, DesktopPreReadyPlatform.test.ts), author juliusmarminge. This is review only; no source custody or merge claim.

Review: I read the complete three-file delta and relevant exact-head startup code and existing Linux feature-switch tests. resolveEnableFeaturesSwitch appends CDPScreenshotNewSurface into the existing enable-features list, avoids duplicate/parameterized entries via featureListNames, and preserves an explicit disable-features opt-out. It runs inside the synchronous pre-ready Electron setup on Windows, macOS and Linux, after any Linux-specific disable-feature adjustment. The old early-portal/password-store handling and credentials/session logic are not changed. The tests cover all three OS startup choices, existing enabled features, explicit disable and startup ordering. Compared #32412: this head retains caller-requested opt-out, unlike its unconditional append. No additional in-scope P1/P2 source defect demonstrated by this delta. This is not proof of physical host behavior.

Actual CI, not just PR text: unfiltered all-event exact-head Actions inventory returned 22 runs. The relevant automatic CI run https://github.com/pingdotgg/t3code/actions/runs/42358732829 completed SUCCESS but checked out the generated PR merge candidate e1310c3b77bf95070fe048f24648a2fa0fc2fcf7 = this exact source head merged into the recorded base, not the raw head checkout. Its canonical Test job #128362403634 reports apps/desktop/src/app/DesktopPreReadyPlatform.test.ts 10/10, DesktopEarlyElectronStartup.test.ts 19/19, desktop package 1,032/1,032 tests; Build/Typecheck, Lint and Check succeeded. Server shard Test steps, Test Web, Release Smoke, Rust and physical/macOS-preview workflows were SKIPPED where applicable, not passed. Native Linux Xvfb and separate GNOME/Wayland 100-snapshot evidence is author/reporter-provided, not independently executed in this review; the separate GNOME proof used #32412's head. No physical macOS/Windows/minimized-window acceptance or packaged integration run claimed.

Disposition: Source-level COMMENT / no new P1/P2; explicit upstream Macroscope Needs Human Approval for the default Chromium-feature choice remains a product gate, plus maintainers' normal landing rules. Do not treat this comment as a human approval or a fleet host/session/consent proof. No CI dispatch, host action or merge here.