Skip to content

Harden preview automation reliability - #4577

Open
Quicksaver wants to merge 86 commits into
pingdotgg:mainfrom
Quicksaver:fix/preview-automation-reliability
Open

Harden preview automation reliability#4577
Quicksaver wants to merge 86 commits into
pingdotgg:mainfrom
Quicksaver:fix/preview-automation-reliability

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jul 26, 2026

Copy link
Copy Markdown

PR id: 4577
PR url: #4577
PR title: Harden preview automation reliability
PR target: pingdotgg/t3code:main
PR head: Quicksaver:fix/preview-automation-reliability

Summary

Hardens collaborative preview automation across the web host, MCP server, shared contracts, and Electron CDP controller so operations remain bounded and recoverable through cold tab startup, background-tab capture, runtime guest replacement, renderer presentation races, and stalled debugger sessions.

New tabs acknowledge creation without waiting on cold renderer readiness, reused tabs retain stable visibility checks, and exact epoch-scoped runtime guest ids keep automation aligned with the intended Electron target. Snapshots preserve semantic page data when raster capture is unavailable, while one-shot background capture stages the retained inactive guest without changing the user's selected tab or focusing the native guest.

What Changed

  • Bounded Electron automation operations, debugger-session initialization, screenshot capture, and recovery within caller deadlines; propagated caller timeouts into click, type, wait, and snapshot operations and detached poisoned sessions without disrupting unrelated queued work.
  • Clamped click, type, press, scroll, and wait desktop inputs to the renderer host budget that remains after target readiness, so control sessions cannot restart the original timeout after the caller-facing request has nearly expired.
  • Propagated the post-readiness deadline into color-scheme changes and recording startup; timed-out appearance changes do not persist late preferences, while recording startup bounds failed-start cleanup and always releases its renderer slot.
  • Re-read current tab state after a bounded appearance command settles and retry against a replacement webview even when detaching the stale guest rejects its command, rechecked resize deadlines after the serialized viewport mutation queue, and bounded recording-stop capture shutdown, MediaRecorder settlement, blob conversion, and artifact persistence. Renderer and desktop recording-stop deadlines retain the slot and captured chunks so finalization can be retried without silently losing the artifact, while an in-flight save promise and a stable validated artifact key prevent duplicate files across renderer or desktop timeout retries.
  • Serialized poisoned-session teardown while the active operation still holds its control permit, made registry removal atomic with debugger detachment, bound timeout cleanup to the exact acquired session so late cleanup cannot detach its replacement, and made operations already queued on a retired semaphore retry before sending a command.
  • Kept renderer and desktop response-grace budgets monotonic across short caller
    timeouts so increasing a requested timeout never reduces its execution window.
  • Made desktop debugger attachment lazy for tabs following the system color scheme and restored explicit color-scheme overrides through a separately bounded recovery path after detached DevTools closes.
  • Added bounded compositor screenshots with hidden capturePage fallback, PNG validation and resizing, structured failure logging, and degraded semantic snapshots with screenshot: null.
  • Clamped primary and fallback screenshot attempts to the remaining control-session
    deadline while reserving time for fallback capture and semantic result
    settlement.
  • Kept healthy debugger sessions attached when exhausted screenshot budget
    skips CDP capture before any command starts.
  • Built one-shot background snapshot presentation on upstream's retained hidden guest and shared capture lifecycle, using reference-counted presentation leases that restore placement, honor operation deadlines, and never select or focus the background tab.
  • Required the epoch-scoped runtime tab id for every staging, readiness, diagnostic, lease, and desktop capture lookup, while retaining the stable server tab id for selection and error reporting.
  • Aborted pending open and snapshot work with PreviewAutomationTargetUnavailableError when a server-epoch change replaces the runtime guest, preventing stale operations from adopting the replacement target.
  • Added deterministic background staging when no panel rectangle exists, stable presentation checks, selection reassertion across hydration races, and typed timeout diagnostics for host, visibility, overlay, and staging failures.
  • Released background presentation promptly when capture has not started, while retaining the lease until an already-started desktop capture settles so response timeouts cannot tear down compositor staging beneath in-flight work.
  • Unified renderer and broker response-grace handling, preserved short caller-supplied execution budgets, used non-throwing remaining budget for best-effort presentation settling, bounded overlay status calls with post-await runtime identity validation, and clamped overlay and navigation polls to their remaining deadlines.
  • Clamped inline visibility polling intervals to the remaining operation budget
    so short open requests do not overshoot their deadline.
  • Rechecked the remaining host deadline immediately before preview creation,
    resize, reveal, and navigation mutations so expired open requests cannot
    start late side effects after settings or session synchronization.
  • Clamped rendered-viewport polling sleeps to the remaining resize deadline and
    revalidated epoch-scoped runtime identity after every awaited guest read.
  • Changed new preview tabs to apply their server snapshot, assigned id, and automation viewport before acknowledging creation; reused tabs continue to wait for the readiness guarantees their existing targets should satisfy.
  • Updated snapshot contracts, preload, IPC, and MCP responses so screenshots are nullable and image content is omitted when capture is unavailable while semantic structured content remains usable.
  • Standardized composite, web, desktop, and server-only development URLs on explicit IPv4 loopback without leaking browser-mode HOST overrides, while preserving IPv6 proxying for explicit IPv6 backend binds.
  • Scoped retained browser-surface subscriptions to each runtime tab so presentation and background-capture updates do not rerender unrelated mounted guests, while thread selection overrides stale surface visibility during background staging.
  • Kept inactive and nearly transparent background-capture guests out of the
    host accessibility tree without affecting CDP accessibility snapshots.
  • Made the pairing route claim later URL-fragment tokens in an already-mounted document once each, remove the secret fragment after capture, and serialize overlapping exchanges while retaining pending UI state until the queue drains.
  • Isolated explicit dev:desktop runs under <base>/userdata/electron so a worktree desktop does not reuse incompatible IndexedDB state from an installed or earlier development profile.
  • Added focused regression coverage for desktop identity and session deadlines, capture fallback, runtime guest replacement, renderer staging and readiness, browser-surface state, pairing fragments, contracts, MCP snapshot responses, and development host configuration.
  • Documented the branch-owned behavior, upstream ownership boundary, current limitations, primary files, focused test command, and development ports in BRANCH_DETAILS.md.

Why

Preview automation could fail with generic execution errors or timeouts during cold startup, after an offscreen guest stalled CDP initialization, when a background tab lacked a presentable compositor surface, or when a server epoch replaced the runtime guest during an in-flight operation. Screenshot failures could also discard otherwise usable semantic page state, staging a native guest could cover the T3 interface, and isolated desktop development could inherit incompatible persisted browser state.

These changes make automation deadlines and target identity explicit end to end, recover control sessions after stalls, decouple new-tab creation from renderer readiness, isolate development profiles, and preserve useful semantic results even when raster evidence is temporarily unavailable.

Validation

  • vp test run scripts/dev-runner.test.ts apps/desktop/src/preview/Manager.test.ts apps/server/src/mcp/McpHttpServer.test.ts apps/web/src/browser/browserRecording.test.ts apps/web/src/browser/browserSurfaceStore.test.ts apps/web/src/browser/hostedBrowserWebviewStyle.test.ts apps/web/src/browser/browserViewportActions.test.ts apps/web/src/browser/browserViewportLayout.test.ts apps/web/src/browser/previewRuntimeTabId.test.ts apps/web/src/components/auth/PairingRouteSurface.logic.test.ts apps/web/src/components/preview/previewAutomationOpenReadiness.test.ts apps/web/src/components/preview/previewAutomationOverlayReadiness.test.ts apps/web/src/components/preview/previewAutomationPresentation.test.ts apps/web/src/components/preview/previewAutomationRequestConsumer.test.ts apps/web/src/components/preview/previewNavigationReadiness.test.ts apps/web/src/components/preview/previewViewportReadiness.test.ts apps/web/src/components/preview/previewViewportRollback.test.ts packages/contracts/src/ipc.test.ts packages/contracts/src/preview.test.ts passed all 287 tests across 19 files. The two unchanged desktop path-fixture files omitted from this Windows run retain eight documented POSIX-versus-Windows assertion failures.
  • The runtime-id presentation subset passed all 50 tests, including immediate
    open, monotonic short-deadline handling, remaining desktop input budgets,
    non-throwing best-effort settling, bounded overlay status calls, post-status
    runtime replacement rejection, deadline-clamped presentation, overlay,
    navigation, and visibility polling, handled delayed capture rejection, and
    snapshot rejection after a server-epoch replacement.
  • The desktop manager's 59 focused tests verify that timed-out screenshot and
    appearance work detaches cleanly, does not persist a late preference, and
    lets queued evaluation reattach before sending its first command.
  • Desktop, server, contracts, and mobile typechecks completed without type errors. The web typecheck reports the same 16 documented baseline errors, including the pre-existing RegistryContext access in PreviewAutomationHosts.tsx; none are in the follow-up changes.
  • An isolated dev:desktop instance using the worktree user-data override paired successfully and remained usable through repeated renderer/CDP inspection and raster capture; an archive worktree client loaded its seeded project without the previous IndexedDB VersionError.
  • An isolated web client on ports 5744/13784 completed first-navigation pairing, loaded the seeded Preview Reliability thread, and rendered the right-panel surface chooser. The non-Electron client could not exercise the Browser surface.
  • A second isolated web pass delayed the first browser-session exchange, injected another fragment token while it was pending, and verified that the second exchange began only after the first finished before the client loaded the authenticated app.
  • The final review-follow-up isolated stack on ports 5744/13784 paired through Playwright, loaded the React app shell, and reported no browser-console errors. The affected mutation paths remain Electron-only and therefore cannot execute in the non-Electron web client; focused renderer, contracts, server, and desktop tests cover their deadline and cleanup behavior.
  • git diff --check upstream/main...HEAD passed.

Proof

No additional proof artifacts are included. Product-native hidden, non-selected preview_snapshot capture is not covered end to end in the isolated worktree desktop because the controlled web client is non-Electron and agent preview tools may remain attached to the installed T3 Code host; focused automated coverage exercises the target identity, deadline, capture, presentation, pairing, and contract behavior.

🤖 Generated by GPT-5 in Codex via T3 Code


Note

High Risk
Changes span security-adjacent automation (CDP, screenshots, recording artifacts), core desktop preview infrastructure, and nullable snapshot contracts that all MCP/web consumers must handle.

Overview
This PR makes collaborative preview automation bounded and recoverable across the renderer, MCP layer, contracts, and Electron PreviewManager.

Desktop CDP and snapshots: Automation operations run under monotonic execution budgets with response grace; control sessions initialize lazily, detach on timeout without breaking unrelated queued work, and support stale-session retry. Snapshots prefer CDP compositor capture (with a target-bridge path for background tabs), fall back to capturePage, validate PNGs, and return semantic data with screenshot: null when raster capture fails. Color scheme, recording start/stop, and artifact save accept deadlines; saves use an idempotency key for retries.

Web renderer: Reference-counted background presentation stages nearly transparent guests for one-shot snapshots without focusing native UI; surface subscriptions are scoped per runtime tab. The automation consumer maps broker stalls to PreviewAutomationTimeoutError and clamps polling and bridge calls to remaining budgets. Open/readiness policies distinguish new vs reused tabs and abort on runtime guest replacement.

Contracts / MCP: Snapshot screenshots are nullable; MCP omits image content when capture is missing. Tool handlers propagate timeoutMs for press, scroll, appearance, and recording.

Other: T3CODE_DESKTOP_USER_DATA_DIR isolates worktree Electron profiles; /pair claims tokens from later hash changes with a serialized submission queue; BRANCH_DETAILS.md documents behavior and verification.

Reviewed by Cursor Bugbot for commit b584620. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Harden preview automation reliability with execution budgets, background snapshots, and deadline-bounded recording

  • Introduces per-request execution budgets in the preview automation host (PreviewAutomationHost) and request consumer, rejecting with PreviewAutomationHostDeadlineExceededError when the budget is exhausted instead of awaiting indefinitely.
  • Adds CDP-based screenshot capture for both foreground and background (target-specific) snapshots in the desktop PreviewManager, replacing wc.capturePage; snapshot results may now carry a null screenshot when neither capture path succeeds.
  • Bounds all automation operations (color scheme, press, scroll, start/stop/save recording, snapshot) end-to-end with propagated timeoutMs from MCP tools through IPC handlers to the desktop bridge and native operations.
  • Adds idempotency key support and retry-safe state retention to stopBrowserRecording/finalizeBrowserRecording so stop-deadline failures preserve captured chunks for a subsequent retry.
  • Extracts overlay readiness, presentation, and viewport readiness into dedicated modules (previewAutomationOverlayReadiness, previewAutomationPresentation, previewViewportReadiness) with strict deadline-clamped polling loops.
  • Adds background capture staging to HostedBrowserWebview via reference-counted leases in browserSurfaceStore, rendering a near-transparent webview above the active surface without intercepting pointer events.
  • Risk: DesktopPreviewBridge interface signatures are extended with new required parameters; implementors must update setColorScheme, startScreencast, stopScreencast, saveScreencast, and snapshot.

Macroscope summarized b584620.

Report background presentation deadlines as typed timeouts.
Bound background leases and preserve short request timeouts.

Isolate queued CDP timeouts and handle hidden empty tabs.
Preserve control deadlines and screenshot recovery.

Default legacy snapshot IPC and scope loopback host overrides.
- Release background capture leases when staged operations time out
- Keep desktop wait operations inside caller timeout budgets
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 185b40ad-56d5-4f55-86be-cfc4ca41d6c4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 26, 2026
Comment thread apps/web/src/browser/browserSurfaceStore.ts Outdated
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx Outdated
Comment thread apps/desktop/src/preview/Manager.ts Outdated
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx Outdated
Comment thread apps/web/src/components/preview/previewAutomationPresentation.ts
@macroscopeapp

macroscopeapp Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

Diff is too large for automated approval analysis. A human reviewer should evaluate this PR.

You can customize Macroscope's approvability policy. Learn more.

- Clamp background capture staging to current renderer bounds
- Share host deadlines and pass snapshot budgets through desktop IPC
- Retain presentation leases until timed-out captures settle
- Add focused regression coverage for all review findings
@Quicksaver

Copy link
Copy Markdown
Author

Follow-up to Macroscope’s approvability summary: all five inline findings were addressed in 27d0818, each thread now has a concrete resolution reply, and all review threads are resolved. The feature-focused suite passes with 11 files and 119 tests; targeted lint and affected-package typechecks also pass.

Comment thread apps/web/src/components/preview/previewAutomationPresentation.ts Outdated
Comment thread apps/web/src/components/preview/previewAutomationPresentation.ts
- Use selected panel state for background capture
- Bound compositor frames and release stalled staging leases
- Cover panel-switch and paused-frame regressions
@Quicksaver

Copy link
Copy Markdown
Author

Follow-up to Macroscope's approvability summary: the two later inline findings were addressed in bfe907c and both threads are resolved. Background capture now follows selected panel state, paused compositor-frame waits release staging before desktop capture begins, and the feature-focused suite passes with 11 files and 121 tests.

- Preserve upstream attribution for single-origin browser dev
- Document optional preferred ports and collision handling
…ion-reliability

# Conflicts:
#	apps/desktop/src/preview/Manager.test.ts
#	apps/desktop/src/preview/Manager.ts
#	apps/server/src/mcp/toolkits/preview/tools.ts
#	apps/web/src/browser/HostedBrowserWebview.tsx
#	apps/web/src/browser/browserSurfaceStore.ts
#	apps/web/src/browser/hostedBrowserWebviewStyle.test.ts
#	apps/web/src/browser/hostedBrowserWebviewStyle.ts
#	apps/web/src/components/preview/PreviewAutomationHosts.tsx
#	apps/web/src/components/preview/previewAutomationOpenReadiness.test.ts
#	apps/web/src/components/preview/previewAutomationOpenReadiness.ts
- Preserve namespaced active surface identifiers across inline previews
- Keep timeout diagnostics consistent for hidden and competing surfaces
- Restore complete behavior-level branch documentation
T3 Verification and others added 15 commits August 8, 2026 12:26
- preserve bounded preview automation around upstream changes
- adopt normalized preview action results and registration safeguards

🤖 Co-authored by GPT-5 in Codex via Codex
…ion-reliability

# Conflicts:
#	apps/web/src/components/preview/previewAutomationOpenReadiness.ts
#	packages/contracts/src/ipc.ts
- resolve browser defaults once per automation open request
- pass one presentation decision into reused-tab readiness
- document merged defaults and focused verification results

🤖 Co-authored by GPT-5 in Codex via T3 Code
- hydrate browser defaults before reading the open request session
- prevent reused-tab opens from mixing server epochs

🤖 Co-authored by GPT-5 in Codex via T3 Code
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UI consistency review: one accessibility-semantics finding on the new background-capture staging path in HostedBrowserWebview.tsx. Everything else in scope (shared Input/Button usage in PairingRouteSurface.tsx, inline wrapper geometry kept in hostedBrowserWebviewStyle.ts, no global CSS or primitive overrides) looks consistent.

Posted via Macroscope — UI Consistency

Comment thread apps/web/src/browser/HostedBrowserWebview.tsx Outdated
- reject expired open mutations before starting side effects
- clamp viewport polling and revalidate runtime identity
- hide inactive capture guests from host assistive technology
- document focused tests and integrated verification limits

🤖 Co-authored by GPT-5 in Codex via T3 Code
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx
Comment thread apps/web/src/components/preview/PreviewAutomationHosts.tsx
Comment thread apps/desktop/src/preview/Manager.ts
Comment thread apps/desktop/src/preview/Manager.ts Outdated
Comment thread apps/web/src/browser/browserRecording.ts Outdated
Comment thread apps/web/src/browser/browserRecording.ts
Comment thread apps/web/src/browser/browserRecording.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6716ed7. Configure here.

Comment thread apps/desktop/src/preview/Manager.ts
Comment thread apps/web/src/browser/browserRecording.ts
- Bound failed recording-start cleanup and always release its slot
- Retry appearance changes when a replacement rejects the stale command
- Reuse validated artifact keys across desktop save timeout retries

🤖 Co-authored by GPT-5 in Codex via T3 Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant