Skip to content

fix(web,server): browser tabs follow the default appearance setting - #16983

Open
ilyaliao wants to merge 1 commit into
pingdotgg:mainfrom
ilyaliao:fix/browser-theme-light-mode
Open

ilyaliao wants to merge 1 commit into
pingdotgg:mainfrom
ilyaliao:fix/browser-theme-light-mode

Conversation

@ilyaliao

@ilyaliao ilyaliao commented Oct 7, 2026 •

Copy link
Copy Markdown

Problem

In the desktop app, a browser tab always renders light, even when Settings sets the default browser appearance to Dark or to System with a dark app theme. To reproduce, put macOS in Dark mode, set both the app theme and the default browser appearance to System, then open github.com in a new browser tab. The page renders light. The tab should open at the configured appearance, and a System tab should follow the app theme.

There are two causes. First, PreviewOpenInput has no colorScheme, so server tabs never receive browserDefaultAppearance. The desktop applies that default only to its own webview tabs, through browserDefaultTabState. Second, the desktop renders server tabs in its own webview, and the server drives that page through Playwright connectOverCDP. Playwright sets prefers-color-scheme: light on attach. ServerTab.colorScheme started as "system", so applyRendering treated a System tab as already applied and never cleared that override.

Change

  • PreviewOpenInput gains an optional colorScheme. PreviewManager.open writes it into the new tab's snapshot.
  • openPreviewSession, openUrlInPreview, and openTerminalLinkInPreview send defaults.appearance when they open a server tab.
  • ServerTab.colorScheme starts as null, so the first applyRendering always calls emulateMedia. For System, that call clears Playwright's light override, and the page follows the desktop's theme again.
  • A popup that a tab opens uses the same appearance as that tab.

Scope and approval

This is a small fix for an obvious bug, so it has no prior issue. The default browser appearance setting already exists and works for desktop tabs. This change makes server tabs honor the same setting and adds no new setting or behavior. The regression likely came with the server-run browser in #15328, which opens tabs with only viewport and profileId. #16701 reports the same gap for the default browser profile.

Verification

  • A new test in ServerBrowser.test.ts checks two things. A tab opened with colorScheme: "dark" gets emulateMedia({ colorScheme: "dark" }). A System tab that the desktop renders gets emulateMedia({ colorScheme: null }). Without the server change the test failed (Tests 1 failed). With the change, ServerBrowser.test.ts and Manager.test.ts pass (Tests 47 passed (47)).
  • I reproduced the root cause with real Electron. An Electron window with nativeTheme.themeSource = "dark" measured light after connectOverCDP and dark after emulateMedia({ colorScheme: null }).
  • tsc --noEmit passes for packages/contracts, apps/server, and apps/web. Lint and format pass on the changed files.
  • In the desktop dev app, a new tab with the default set to Dark renders dark. With System, the tab follows the app theme and switches when the app theme changes.
Before After
before.mp4
CleanShot.2026-10-08.at.7.09.55.AM.mp4

Recorded on macOS in Dark mode, with the app theme and the default browser appearance both set to System.

Two cases are not covered. A headless server tab streamed to a web client still renders light under System. I measured that headless Chromium on a dark Mac reports light even with no override, so the server has no system theme to follow. Tabs that an agent opens with preview_open do not get the client's default, because that setting lives on the client. In the desktop app, those tabs now follow the app theme.

Model and harness: Claude Opus 5.5 in Claude Code, run through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 7, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This changes the effective default appearance of server-backed browser tabs and popup tabs, including how System clears Playwright’s light override. The implementation is narrow and backward-compatible, but the user-visible default behavior warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 1cc43e3d-0145-46b9-8987-2a64b36cf509
📥 Commits

Reviewing files that changed from the base of the PR and between 300f7f9 and d74cf37.

📒 Files selected for processing (7)
  • apps/server/src/preview/Manager.ts
  • apps/server/src/preview/ServerBrowser.test.ts
  • apps/server/src/preview/ServerBrowser.ts
  • apps/web/src/browser/openFileInPreview.ts
  • apps/web/src/components/preview/openPreviewSession.ts
  • apps/web/src/components/preview/openTerminalLinkInPreview.ts
  • packages/contracts/src/preview.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.


📝 Walkthrough

Walkthrough

Preview creation now passes an optional color scheme to server tabs. Server tab snapshots retain supplied schemes, and browser rendering applies the requested appearance.

Changes

Preview appearance

Layer / File(s) Summary
Define and pass preview appearance
packages/contracts/src/preview.ts, apps/web/src/browser/openFileInPreview.ts, apps/web/src/components/preview/openPreviewSession.ts, apps/web/src/components/preview/openTerminalLinkInPreview.ts
PreviewOpenInput accepts an optional colorScheme. Web preview entry points pass the configured appearance when a runtime is available.
Apply appearance to server tabs
apps/server/src/preview/Manager.ts, apps/server/src/preview/ServerBrowser.ts, apps/server/src/preview/ServerBrowser.test.ts
Server tab snapshots retain supplied schemes. Tabs start with no applied scheme, and popups pass a non-null scheme. Tests check dark emulation and resetting emulation to null.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to d74cf

Preview opens remain compatible with older servers, though those servers will not apply the new appearance setting. No merge-blocking issue is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d74cf

Appearance choices remain narrowly constrained and do not expand access or ownership. However, failure while applying the initial appearance can leave surviving browser resources outside normal tab cleanup and limits. This is a limited failure-containment risk; no new authorization bypass or data-access expansion was identified.

Retained concerns

  • Low · reliability · inferred: Initial appearance application now introduces a failure point for every omitted/system server-tab creation. If emulation rejects while browser resources remain live, creation exits before registration without closing those resources. Retries can allocate additional pages, while tab limits and idle cleanup do not account for the abandoned resources. The cleanup gap predates this PR, but the new unconditional initial call broadens its exposure.
Security review details

Security Blast Radius

  • inferred — Normal effects are page-scoped appearance changes, including inherited popup appearance. Conditional resource accumulation from failed initialization could affect the browser host within one server instance. The inspected evidence does not demonstrate cross-tenant access, privilege gain, or an attacker-controlled failure trigger.

Trust Boundaries and Controls

  • observed — The preview-open RPC retains its existing handler and declared-scope authorization path. The appearance field does not replace thread, URL, runtime, profile, or ownership inputs. Popup rejection still closes pages when the opener is closing or the agent tab limit is reached.

Resilience and Maintainability Implications

  • observed — Normal tab teardown and idle sweeping operate on registered tabs. Context disposal at server shutdown provides eventual cleanup for managed headless contexts, but it does not supply per-attempt rollback when initial rendering fails before registration.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Approvability ✅ Passed PASS. The PR is a focused browser appearance bug fix across seven files. It does not change the product setting default; it applies the existing browserDefaultAppearance to server tabs. The `package…
Title check ✅ Passed The title clearly and concisely describes the main change: browser tabs now follow the configured default appearance setting.
Description check ✅ Passed The description includes all required sections and provides the problem, implementation details, scope justification, focused tests, manual verification, screenshots, limitations, and agent informatio…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 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