Skip to content

fix(web): agent tabs in background threads open in the desktop browser - #19919

Merged
juliusmarminge merged 7 commits into
pingdotgg:mainfrom
MahirAbbas:fix/bg-thread-preview-webview
Oct 20, 2026
Merged

juliusmarminge merged 7 commits into
pingdotgg:mainfrom
MahirAbbas:fix/bg-thread-preview-webview

Conversation

@MahirAbbas

@MahirAbbas MahirAbbas commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Agent-created preview tabs in background threads had no desktop host until their chat mounted. The desktop host now discovers threads from the primary environment's preview events and mounts the existing per-thread session sync. Chat and desktop share that sync, including list reconciliation and tab closure. The shared preview event subscription also delivers each event separately to the atom, so a batch cannot hide an earlier thread open or tab close.

This finishes @MahirAbbas's original contribution, preserves the contributor's commits, and merges main at ab6be13d43 to resolve the conflict.

Verification at 68c1401c:

  • Replaced the obsolete standalone hook test with one behavior case in the existing browser suite: discovery, list reconciliation, and close without mounting chat. Removing desktop discovery makes it fail.
  • 92 focused tests pass across seven files: 40 client-runtime tests, six browser tests, and 46 web unit tests. One added runtime regression verifies all opens and closes from RPC batches; it fails before the delivery fix (39 pass / one fail) and passes after. Client-runtime and web typechecks, targeted lint, formatting, and diff checks pass.
  • Two full-diff review rounds with Codex and Claude. Round one found batched event loss; the shared subscription fix and focused failing-before regression address it. Both reviewers found no remaining code blockers in round two and independently passed the 40 runtime tests.
  • Native Electron 44.4.5 on headless Linux/Xvfb ran the production desktop host, hosted webview, session sync, and CDP relay with synthetic environment/IPC boundaries. With discovery disabled, the same event creates zero webviews. At this head it creates one webview and attaches it with chat absent; a duplicate event keeps one, CDP returns the guest title, and close removes the webview and detaches it. Before output; after output.

Before — discovery disabled in the production host, with otherwise matching fixture/data/viewport:

Before: no desktop guest for the background event

After — reviewed head:

After: one attached desktop guest without mounted chat

Limits: this native integration fixture is not the packaged T3 desktop app. It substitutes transport, settings/permissions, and unexercised UI boundaries; it does not prove the full server fallback transition or CPU reduction. Previously existing tabs still need a subsequent event or chat mount for discovery, and discovered thread syncs live for the desktop host's lifetime. A close for an undiscovered preexisting thread can still leave old cached tabs after chat unmounts; this predates this fix and is outside its discovery scope. macOS, Windows, iOS, Android, packaged Electron, and remote/relay/tunnel flows were not verified. Web and mobile do not mount this Electron-only host; provider adapters and wire contracts are unchanged.

Maintainer work: Codex harness, gpt-6.1-sol; fresh tests: Codex gpt-6-astra; reviews: Codex gpt-6-sol and Claude claude-opus-5-5.

The desktop only loaded preview tabs for threads whose chat was on
screen. A tab an agent opened in a background thread never got a
webview, so after 10 s the server fell back to its headless browser,
whose software-rendered GPU process can use several cores.

The always-mounted browser host now follows the environment's preview
events and starts the existing per-thread sync for each thread that
opens a tab, so the desktop renders it.

Refs pingdotgg#18165, pingdotgg#16901

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 23 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a09cbff6-364a-40c3-bd70-37a851797006
📥 Commits

Reviewing files that changed from the base of the PR and between 68c1401 and 5a16f17.

📒 Files selected for processing: 5
  • apps/web/src/browser/ElectronBrowserHost.tsx
  • apps/web/src/components/preview/previewSessionSync.browser.test.ts
  • apps/web/src/components/preview/usePreviewSession.ts
  • packages/client-runtime/src/state/preview.ts
  • packages/client-runtime/src/state/runtime.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d693419d-3897-46ab-91ac-a54011a65fc0
📥 Commits

Reviewing files that changed from the base of the PR and between a357bf8 and 68c1401.

📒 Files selected for processing: 5
  • apps/web/src/browser/ElectronBrowserHost.tsx
  • apps/web/src/components/preview/previewSessionSync.browser.test.ts
  • apps/web/src/components/preview/usePreviewSession.ts
  • packages/client-runtime/src/state/preview.ts
  • packages/client-runtime/src/state/runtime.test.ts

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

📝 Walkthrough

The runtime publishes preview events individually. In Electron, the host starts environment-scoped synchronization for threads discovered in preview events.

Changes: Desktop preview sessions

Layer / File(s) Summary
Publish preview events individually
packages/client-runtime/src/state/preview.ts, packages/client-runtime/src/state/runtime.test.ts
The preview subscription rechunks its stream so each event is published separately. Runtime tests check that batched opened and closed events arrive in order.
Synchronize desktop preview sessions
apps/web/src/components/preview/usePreviewSession.ts, apps/web/src/browser/ElectronBrowserHost.tsx, apps/web/src/components/preview/previewSessionSync.browser.test.ts
The desktop hook mounts scoped preview synchronization for new eligible thread IDs in Electron. ElectronBrowserHost calls the hook with the primary environment ID. Browser tests cover opened, list, and closed events.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ElectronBrowserHost
  participant useDesktopHostedPreviewSessions
  participant PreviewEventsSubscription
  participant PerThreadPreviewSync
  ElectronBrowserHost->>useDesktopHostedPreviewSessions: Passes primaryEnvironmentId
  useDesktopHostedPreviewSessions->>PreviewEventsSubscription: Subscribes to environment events
  PreviewEventsSubscription->>useDesktopHostedPreviewSessions: Publishes individual preview events
  useDesktopHostedPreviewSessions->>PerThreadPreviewSync: Mounts sync for a new eligible thread ID
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 68c14

Background desktop preview attachment has no identified merge-blocking issue; normal checks remain appropriate.

Security architecture risk: 🔵 Low · up to 68c14

Background previews can now start without opening their chats. Existing environment scoping and preview permissions remain in place, and no permission bypass was established. Cleanup across environment changes and packaged desktop behavior remain partly unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The effective expansion is desktop hosting for event-discovered background threads in the primary environment, using existing browser profiles and desktop-tab mechanisms. It is not merely a rendering change, but the inspected path does not establish new authority over another environment or service.

Trust Boundaries and Controls

  • observed — Server WebSocket setup authenticates the session and installs RPC scope authorization. Preview list and event subscription require orchestration-read scope; preview mutation methods require preview-operation scope. Independently, desktop rendering requires preview-operation permission for the session's environment. These inspected controls are unchanged from the comparison base.

Resilience and Maintainability Implications

  • inferred — Retained state across discovery disposal is a lifecycle uncertainty, not an established authorization bypass. The store's retention predates this PR, and the existing host filter excludes server-runtime sessions outside the current primary environment. Permission checks also remain tied to the original environment. These controls limit the security interpretation of stale-state scenarios.

Hardening Proposals

  • proposed — Define and verify the intended retained-session policy across primary-environment replacement, clearing the primary environment, and reconnecting after missed close events. Validate subscription disposal and externally visible webview removal together, without assuming retained snapshots must always be deleted.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: opening agent tabs from background threads in the desktop browser.
Description check Passed The description explains the problem, implementation, verification, limitations, and affected platforms. It is mostly complete, although it does not explicitly document maintainer approval or explain …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, verification, limitations, and affected platforms. It is mostly complete, although it does not explicitly document maintainer approval or explain why the fix qualifies without prior approval.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one. I will integrate current main, rebuild the focused background preview discovery path, and verify desktop attachment before pushing the reviewed update to this branch.

NikitaMGrimm added a commit to NikitaMGrimm/t3code that referenced this pull request Oct 19, 2026
# Conflicts:
#	apps/web/src/browser/ElectronBrowserHost.tsx
#	packages/client-runtime/src/state/runtime.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 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.

2 participants