Sign up
View stargazersSign up

fix(desktop): update browser tab titles without navigation#29452

merged4
main
GitHub fork: voltcrash/fix-desktop-preview-title-updates
opened11d agobyVvoltcrash
2 files changed
View on GitHubTwo-way sync enabledPR last imported 7h ago

Desktop Browser tabs on connected environments kept their old tab and history titles when a loaded page changed document.title without navigating. Include title in the existing report cache/comparison so the new title reaches the remote snapshot and existing UI/history subscribers. Repeated unrelated desktop updates stay deduplicated; the server-driven bypass and failure-report behavior remain unchanged.

Closes #29451.

One obvious report-deduplication bug under CONTRIBUTING.md's small focused-fix exception. No new workflow, product default, or adjacent reliability fix.

Proof: real isolated Linux/Wayland Electron 44.10.0 / Chromium 152.0.7977.169, with a non-primary connected environment and desktop-runtime tab (serverDriven=false). After the fixture retitles without navigation, both runs have guest title Updated title 1, the same URL and one navigation entry. Baseline leaves the snapshot/tab/history at Original title; patched updates all three.

After the title-only changeBaselinePatched
Tab labelStale tab titleUpdated tab title
Project historyStale history titleUpdated history title

Baseline recording, patched recording, and full evidence with transcripts, guest captures and reproduction tools.

Capture limits: main-window images/recordings show actual tab/history UI but the guest rectangle is white; separate native IPC guest PNGs show the page. No composites were made. Each recording has four actual samples over about 2.41s (2.48s encoded), so it proves sampled state transitions, not fluid motion or latency. The fixture's own button is invoked through real webview JavaScript, not a physical OS mouse. Windows/macOS, relay/tunnel and the primary server-native control were not exercised. Native proof used the same preview sources as the final conflict-free rebase onto 8657eecb28.

vp test run apps/web/src/components/preview/usePreviewBridge.test.ts failed before the fix at the missing title-only report, then passed all 3 tests. The regression also covers duplicate suppression, unrelated audibility/controller changes, empty titles and returning to Loading. Final vp run check:changed passed format, lint, both knip checks, affected typechecks, focused tests and React Compiler coverage.

CI retrigger: one empty commit 02446a74ee preserves the complete 1323da0 source tree after an existing PDF readiness assertion failed in CI. The exact test passes headlessly and the prior full suite was green; failure evidence and the single retrigger are documented here.

Reviewed with review-loop: 1 round with Claude Opus 5.5. Reviewer verdict: approved.

Created with GPT-6.1 Sol (xhigh) in the Codex harness through T3 Code.

Fixes pingdotgg/t3code#29451

Checks

23 successful · 16 skipped
All checks passed
75f120d
CcoderabbitaiBot11d ago
Synced from GitHub

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

  • Last reviewed commit: 79d631addbb8c8b4b7bc904b584a0433c0e65a5b
  • Latest commit, not reviewed: 75f120d0db8b1076b1269f1ba2fc4d76d1edd584

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The preview bridge now includes the page title in report deduplication and tracks it with the last reported URL and navigation kind. Tests cover title-only changes, duplicate reports, and updates to unrelated desktop state.

Changes: Preview status reporting

Layer / File(s)Summary
Title-aware status reporting
apps/web/src/components/preview/usePreviewBridge.ts, apps/web/src/components/preview/usePreviewBridge.test.ts		buildReportInput now suppresses a non-failure report only when its navigation kind, URL, and title match the previous report. The bridge stores and resets the last reported title. Tests cover title changes, duplicate reports, and unrelated desktop-state updates.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: External image Minimal · previous review 79d63

This change makes title-only updates in desktop preview tabs reach the snapshot and history while still suppressing duplicate reports. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title "fix(desktop): update browser tab titles without navigation" matches the main change. The summary and objectives show that title-only changes in desktop preview reports now reach the remote …
Description checkPassedThe description follows the template's main sections. It states the problem (stale titles on title-only changes) and the change (adding title to the report cache and comparison). It links issue #29451…
Linked Issues checkPassedIssue #29451 requires that a desktop-runtime Browser tab on a connected non-primary environment update its tab label, remote snapshot, and project history when the page changes document.title withou…
Out of Scope Changes checkPassedThe whole-PR source diff changes only usePreviewBridge.ts in the preview component area, plus its focused test file per the summary. The source change adds title to the existing report cache and com…
Full details: Title check

Explanation

The title "fix(desktop): update browser tab titles without navigation" matches the main change. The summary and objectives show that title-only changes in desktop preview reports now reach the remote snapshot and tab/history subscribers, and the title follows conventional commit style. It is concise and specific.

Full details: Description check

Explanation

The description follows the template's main sections. It states the problem (stale titles on title-only changes) and the change (adding title to the report cache and comparison). It links issue #29451 and explains the scope as a small focused fix. The verification section gives the failing-then-passing focused test, the Linux/Wayland Electron proof, and the items not checked. It omits a few template items. It has no explicit Problem heading, and the conventional commit title is not repeated there. The author did not add an explicit repro-steps section beyond the proof text. Overall it is mostly complete.

Full details: Linked Issues check

Explanation

Issue #29451 requires that a desktop-runtime Browser tab on a connected non-primary environment update its tab label, remote snapshot, and project history when the page changes document.title without navigating. The whole-PR diff (d8f3feff..79d631ad) to apps/web/src/components/preview/usePreviewBridge.ts adds lastReportedTitle state. buildReportInput now suppresses a report only when kind, URL, and title all match the previous report, and status.kind !== "LoadFailed". A title change at the same URL therefore produces a report with the new title. The ref is reset on subscription and updated after each non-bypassed report. The serverDriven bypass and the LoadFailed branch are unchanged. The test file was not re-read at the current head. Its described coverage (title-only change, duplicate suppression, unrelated desktop updates, empty titles, Loading transitions) is consistent with the requirement. The author-reported native proof is not independently verified here.

Full details: Out of Scope Changes check

Explanation

The whole-PR source diff changes only usePreviewBridge.ts in the preview component area, plus its focused test file per the summary. The source change adds title to the existing report cache and comparison, which is the change named in #29451. The only other source-level change is exporting buildReportInput and adding its lastReportedTitle argument, which supports the focused test and does not add behavior. No workflow, product default, retry, or adjacent reliability change appears in the reviewed source diff. The empty CI retrigger commit carries no code change according to the PR description. This was not verified against the Git tree in this review.

Finishing Touches

Generate unit tests (beta)

  • 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

External image Commits

Reviewing files that changed from the base of the PR and between 05ab4ee656265b70438471a33de17cf8d1843780 and 79d631addbb8c8b4b7bc904b584a0433c0e65a5b.

External image Files selected for processing: 1

  • apps/web/src/components/preview/usePreviewBridge.test.ts

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

MmacroscopeappBot11d ago
Synced from GitHub

Approvability

Verdict: Approved at b403596

Macroscope's review found this PR approvable — Browser tab titles now refresh without navigation through the existing preview-status channel. Existing filtering and reporting guards remain, keeping the change limited to title synchronization.

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

Vvoltcrash11d ago
Synced from GitHub

Note

🤖 GPT-6.1 Sol responding on behalf of voltcrash

The manual review rerun was denied because this account lacks repository permissions; it did not run, and no permission bypass was attempted.

The original Macroscope Correctness check completed successfully on b403596 with no issues, and Approvability approved that diff. Test Signal and Effect Service Conventions were skipped while waiting for their CI prerequisite; they did not run.

Current head 1323da0 is a conflict-free rebase. Both changed preview files are identical to b403596 and the subsequent CodeRabbit-reviewed 147a62e. GitHub carries the original approval on the current head, but there are no fresh Macroscope check runs on this head. Review coverage comes from the completed review of the unchanged diff, not from the denied request.

MmacroscopeappBot11d ago
Synced from GitHub

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

Vvoltcrash11d ago
Synced from GitHub

Note

🤖 GPT-6.1 Sol responding on behalf of voltcrash

Test Web failed in the existing PDF reload test at AttachmentFilePreview.browser.test.tsx:2568: after rerender, the viewer was still “Loading file” instead of showing “1 / 3 pages”. The other 5,412 tests passed. Aggregate Check failed because Test Web failed.

The exact case passes on this head with both display variables unset: env -u DISPLAY -u WAYLAND_DISPLAY vp test run apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx -t 'keeps workspace PDF scroll and zoom after reloads' (output). The PDF source/test, render helper and browser config are unchanged from the successful full 5,413-test run on 147a62e; intervening lockfile changes only update the server/provider Cursor SDK.

This supports one CI retrigger for a timing-sensitive PDF readiness assertion, not a claim of a platform outage. The direct workflow rerun was denied repository-admin permissions. One explicitly authorized CI-only empty commit, 02446a74ee44dce56ba32c5e1d224e46dc571fc0, now retriggers checks through the normal fork task-branch push. Its complete Git tree is identical to 1323da0a80fa1beebc1ad7955e31eb60abb27198 (7500aba04c0041bd52ea2e4176fd470791c9f0ba); no PDF source, timeout, retry, workflow, dependency, or permission change was made. This is the single retrigger for this failure. Prior review covers the identical source; the denied Macroscope manual request did not run.

Added 3 commits

  • fix(desktop): report browser title changes without navigation1f1838e
  • test(web): keep the title-only preview report regression case20079e2
  • ci: retrigger checks after PDF preview readiness failure4eb63a5
JJulius Marminge21h ago

The automated pipeline stopped on this pull request and will not merge it:

someone else pushed to voltcrash/fix-desktop-preview-title-updates (head is now 20079e20ef); the pipeline does not build on commits it did not review

It stays open for a person to finish or close.

Added 1 commit

  • Merge remote-tracking branch 'origin/main' into voltcrash/fix-desktop-preview-title-updates75f120d
JJulius Marminge16h ago

Evidence

Both captures use Linux Electron under headless-desktop, light theme, 1600Ă—1000, fresh isolated databases, and identical synthetic project/thread data. In a thread on a connected non-primary environment, loaded the fixture in a desktop-runtime Browser tab, waited for success, clicked Update title, then inspected the tab label, fresh preview.list snapshot, and Recently used entry. The URL stayed unchanged.

BeforeAfter
95cb068c1075f120d0db
Before: page updated but Browser label retains Original titleAfter: page and Browser label show Updated title 1
Before: Recently used retains Original titleAfter: Recently used shows Updated title 1
Before recordingAfter recording
Before snapshot and history observationsAfter snapshot and history observations

Interaction driver · Fixture page · Seed script