Merge bluebird/code + Browser tabs – bulletproof persistence, skeleton-first navigation, bounded memory by adamleithp · Pull Request #3221 · PostHog/code

adamleithp · 2026-07-07T11:32:46Z

Migrating browser tabs out of bluebird into the merged Code app surfaced persistence bugs and sluggish tab switching. This branch fixes them, with regression tests and live CDP profiling behind each change.

(fix) Tab persistence — nav clicks opened duplicate tabs, switches overwrote other tabs

Stale renderer mirror mis-targeted writes. The route→tab effect makes persistent writes (setTabTarget/openOrFocus) from the renderer mirror, which lagged the server by an IPC round-trip. A navigation landing in that window wrote to the previous active tab (tab contents "copied over") or saw no active tab and opened a duplicate. Every mutation now applies its returned snapshot to the mirror synchronously, and activate/replace also apply optimistically before the round-trip.
Dead history tags dangled focus. Back/forward can replay an entry tagged with a since-closed tab id; persisting it left a dangling activeTabId, after which every nav looked like "no active tab" and opened new tabs. Guarded at three layers: decideTabNavigation validates tags against live tab ids, a new setWindowActiveTab shared primitive rejects unknown/foreign tab ids, and the service refuses to persist them.
appView as a tab identity dimension (home, inbox, agents, skills, MCP servers, command center) so top-level pages dedup and navigate in place instead of spawning tabs; navigation is exhaustiveness-checked so a new page kind can't silently no-op. The incomplete inline blank-tab check in __root.tsx (missed channelId/appView) is replaced with the shared hook.
29 new browser-tabs tests, including a regression suite reproducing each reported symptom end-to-end.

(perf) Tab switching — click to first paint went from ~570ms of frozen UI to a next-frame skeleton

Profiled over CDP: loaders were synchronous, so the router committed navigation inside one main-thread block while the destination's heavy mount (chat-thread measurement effects, canvas grid) ran — the old route stayed frozen the whole time.
New yieldToPaint() single-frame loader + per-route-kind pendingComponent skeletons (task detail, canvas, channel pages, app pages), applied via a withRouteSkeleton() factory. The click now commits the URL, moves the tab highlight, and paints a skeleton before the heavy mount runs behind it.
Router: defaultPendingMinMs: 0 (no artificial skeleton hold) and defaultPreloadStaleTime: 0 (hover-preload can't satisfy the loader and silently reintroduce the freeze).

(perf) Memory — bounded per-visit caches

Freeform canvas threads (up to 50 whole-document version copies each) capped at 8 MRU; eviction skips threads with saves in flight and lives in store state (devtools/HMR-safe). revert() now guards against evicted threads — previously it could persist an empty canvas over real content.
File-content queries (full bodies + base64 blobs, staleTime: Infinity) bounded to 30s gcTime after unmount instead of the 5-minute default.
Terminal parking bounding was attempted and reverted. An LRU cap on parked xterm instances (and a follow-on display:none parking tweak) froze the app when leaving a tab whose terminal was on screen — term.dispose() on switch synchronously re-rendered/serialized. Reverted to main's terminal behavior; a future attempt needs a teardown path that doesn't do synchronous work on switch.

(migration)

browser_tabs.app_view column (migration 0018) backing the new tab identity dimension.
One-time localStorage adoption of the legacy Code sidebar width (sidebar-storage) into the unified channels-sidebar store, so users' resized width survives the merge instead of resetting to 240px.

(ui) Sidebar-merge regressions restored

Title-bar-left width transition matched to the sidebar's curve again, suppressed during drag (it snapped while the sidebar animated).
Legacy enter_space/leave_space analytics re-fired from the Channels toggle (its merged-world successor) so space-adoption dashboards stay continuous.
Skeletons use @posthog/quill primitives per the design-system rule.

Verification

Full shared + ui test suites pass; typecheck + biome clean across shared, ui, workspace-server.
Live app driven over CDP throughout: frame-level screencasts confirm the skeleton paints mid-switch; the terminal-freeze regression was caught and reverted the same way.

Created with PostHog Code

trunk-io · 2026-07-07T11:32:50Z

✨ Submitted to Merge by @adamleithp. It will be added to the merge queue once all branch protection rules pass and there are no merge conflicts with the target branch. See more details here.

greptile-apps · 2026-07-07T11:40:15Z

Comments Outside Diff (1)

.claude/skills/quill-code/scripts/sync-quill.sh, line 130 (link)

md5 -q is macOS-only

md5 -q is a BSD (macOS) flag that prints the hash without the filename. On Linux the command is md5sum, which outputs <hash> <filename> and has no -q option — the script fails here on any Linux system. A portable one-liner: HASH="$(md5sum "$RAW" 2>/dev/null | cut -c1-8 || md5 -q "$RAW" | cut -c1-8)".

_{Reviews (1): Last reviewed commit: "fix(review): harden tab/canvas caches an..." | Re-trigger Greptile}

github-actions · 2026-07-07T11:45:17Z

React Doctor found 1 issue in 1 file · 1 warning.

1 warning

src/router/routeSkeletons.tsx

⚠️ src/router/routeSkeletons.tsx:29 Non-component export in component file only-export-components

_{Reviewed by React Doctor for commit b693feb.}

stamphog

Gates denied: PR contains database migrations and is too large (3573 lines, 49 files across multiple areas) for automated review. Migrations require human oversight.

Migrating tabs out of bluebird surfaced three persistence bugs, all traced to writes computed from stale renderer state: - Apply every tab mutation's returned snapshot to the renderer mirror synchronously, plus optimistic activate/replace application, so a navigation racing the IPC round-trip can never target the previous tab (nav clicks opened duplicate tabs; tab switches overwrote other tabs' contents). - Guard decideTabNavigation against dead history tags (windowTabIds) and validate setActiveTab through a new setWindowActiveTab shared primitive so a closed tab's id replayed via back/forward can't persist a dangling activeTabId. - Replace the incomplete inline blank-tab check in __root.tsx with useActiveTabIsBlank (it missed channelId/appView). Adds appView as a tab identity dimension (migration 0017) so top-level pages (home, inbox, agents, skills, mcp-servers, command-center) get proper tab dedup and in-place navigation, plus 29 new browser-tabs tests including a regression suite for each reported bug. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Tab switches looked ignored for up to a second: route loaders are all synchronous, so the router committed navigation in one main-thread block with the OLD route frozen on screen through the destination's expensive mount (task detail's chat thread measurement effects, terminal focus, canvas grid — profiled at 400-800ms of layout/render work). Give heavy routes a single-frame `yieldToPaint()` loader (double rAF — one guaranteed painted frame) plus a per-route-kind `pendingComponent` skeleton (task detail, canvas, channel pages, top-level app pages). The click now commits the URL, moves the tab highlight, and paints the skeleton before the heavy mount runs behind it. Router config: `defaultPendingMinMs: 0` so skeletons aren't held for the 500ms default minimum, and `defaultPreloadStaleTime: 0` so a hover preload can't satisfy the loader and skip the pending paint (which would silently reintroduce the frozen-screen path for sidebar links). Parent layouts (/code/inbox, /code/agents) only pend while entering — child navigations keep the match in `success` and reload in the background, so no skeleton flash on inner navigation. Verified over CDP screencast against the live app: skeleton frame paints mid-switch, destination replaces it after mount. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Every task visited in a browser tab parked its xterm on unmount — terminal buffers, DOM subtree, and addons kept alive forever in the hidden parking container. Measured over CDP: xterm count grew monotonically with each distinct task visited (~3MB heap per task, GC-stable), the dominant tab-driven heap growth on the way to the 1.5GB budget. Keep at most 4 parked terminals; evict the least-recently-parked by destroying it. Safe because detach already serializes scrollback into the terminal store (restored via initialState on recreate), the shell session keeps running server-side and reattaches by id, and a parked terminal receives no live output anyway — the data subscription lives in the mounted Terminal component. Re-measured: xterm count plateaus at the cap across 12 task visits; residual growth drops to ~1MB/task (React Query's 5-minute gc window plus small per-task view stores). Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

…time Two more unbounded per-visit retainers behind tab-driven heap growth: - freeformChatStore kept a thread (canvas source + up to 50 whole-document version copies) for every canvas ever visited, for the app's lifetime. Cap at the 8 most-recently-used threads; eviction only runs from the mount-time seeding paths and skips threads with a save in flight. An evicted thread reseeds from the saved record on the next visit — every committed edit autosaves, so only an in-progress version-browse position is lost. - File-content queries (staleTime: Infinity) held full file bodies and base64 blobs under the default 5-minute gcTime, keeping every file viewed across every task resident simultaneously. Bound them to 30s after unmount; re-opens re-read from local disk. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Fixes from a review pass over the tab persistence + performance work: - freeformChatStore: eviction now skips (rather than aborts on) threads with a save in flight, so one slow autosave can't block the cap for every thread behind it; threadOrder moved into store state (visible to devtools/tests, HMR can't desync it from threads); revert() guards against evicted/never-seeded threads — patch() would materialize an empty thread and persist() would save code:"" over the real record. - BrowserTabStrip: activeTabId validates the history-state tab id against the live tab list before preferring it (a dead id from back/forward blanked the strip highlight and pointed Cmd+W at a nonexistent tab); optimistic activate/replace writes derive from the store's current snapshot instead of the effect closure's, so they can't regress the mirror past a mutation onSuccess; local primaryWindow duplicate replaced with the @posthog/shared export. - Route skeletons: swapped @radix-ui/themes Skeleton for @posthog/quill (quill-first rule); ChannelSkeleton/AppPageSkeleton share one ListPageSkeleton silhouette; new withRouteSkeleton(skeleton) factory replaces the pendingComponent + loader boilerplate copy-pasted across 12 route files. - Sidebar merge regressions: adopt the legacy Code sidebar width (sidebar-storage) into channels-sidebar the first time the unified store persists, instead of resetting users to the 240px default; restore the title-bar-left width transition with drag-time suppression (it snapped while the sidebar animated); re-fire the legacy enter_space/leave_space analytics from the Channels toggle so space-adoption dashboards stay continuous. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

The migration meta files were hand-built during a rebase conflict resolution (jq output + manual edit) and didn't match Biome's formatter, failing the quality check. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Adding a new AppView tab target now fails to compile in goToTab until its canonical route is wired, instead of silently falling through with no navigation. Closes the one drift gap in the appView tab-identity surface (APP_VIEW_META is already Record-forced, and shared/schema/DB treat appView as an opaque string). Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

Tab switches went sluggish (up to ~2s of frozen UI) once parked terminals started being torn down on switch. CPU profile pinned it: ~3.9s in `get offsetWidth`, from xterm's renderRows reading offsetWidth per row inside the WebGL-addon / term dispose path. The parking container used visibility:hidden (+ 0x0), which still participates in layout — so every per-row offsetWidth read during dispose forced a full-document reflow. Switch it to display:none: a display:none subtree has no layout box, so those reads return 0 with zero reflow. Parked terminals render nothing and refit on re-attach, so losing the layout box while parked is harmless. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

…king The LRU eviction (cap parked terminals, destroy least-recently-used) made term.dispose() run on tab switches, which froze the app when leaving a tab whose terminal was on screen. The display:none parking change was a follow-on perf fix for that same dispose path. Both are reverted to main's terminal behavior — verified the freeze is gone. Terminal memory bounding is dropped from this PR; if we revisit it, it needs a teardown path that doesn't synchronously re-render/serialize on switch. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

New tabs and the last-tab-close landing were routing through the /website index, which redirects to channels[0] — so a fresh tab (or an emptied strip) opened a random first channel. Land on a single flag-aware default instead, keyed off the channels toggle (not the current route, which lags a flip): - channels on -> the private #me channel (provisioned lazily) - channels off -> the Code new-task screen Unified new-tab (handleNewTab) and close-landing (applyCloseResult) through one landOnDefault() helper that never touches the channels index. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

adamleithp · 2026-07-09T10:31:21Z

@k11kirky pushed a rebase + a few fixes on this branch:

Rebased onto main (was 45 behind) and resolved the conflicts:

Migration index collision — both branches minted 0018. Kept main's 0018_add_pr_urls, renumbered ours to 0019_add_app_view, and rebuilt 0019_snapshot.json on top of main's 0018 (so it carries both pr_urls + app_view), with the prevId chain + journal fixed.
sidebarStore — kept both sides' independent fields (taskTypeFilter from main, channelsEnabled from us).
ChannelsSidebar — took our unified Bluebird chrome. Main had added the Activity nav item into the old ChannelsNav, which we'd replaced with SidebarNavSection — so that was the "missing activity". Ported it back in as a new ActivityItem (bell + unread-mentions badge), sitting under the Channels toggle and gated on channelsEnabled (only shows when channels are on).

New-tab / last-tab-close default (BrowserTabStrip): both were routing through the /website index → channels[0], so a fresh tab or an emptied strip opened a random first channel. Unified them through one landOnDefault() keyed off the channels toggle (not the current route, which lags a flip):

channels on → the private #me channel (provisioned lazily)
channels off → the Code new-task screen
never touches the channels index, so no more random-channel landing.

Typecheck + biome green across ui / core / workspace-server. Heads-up: this was a force-push (history rewritten by the rebase).

k11kirky

Re-reviewed against the latest 2 commits (dd65e61b, 15e6e46f). The new landOnDefault() fix (default new tab / last-tab-close to #me/new-task instead of the /website index) is a real, well-targeted fix for a genuine bug, and I traced the resulting decideTabNavigation flow end-to-end — it correctly fills the blank tab. Two notes on it plus my earlier findings are inline below. Nothing else material changed; the ChannelsSidebar.tsx/SidebarNavSection.tsx diff is just the merge-conflict resolution re-homing the pre-existing "Activity" nav item under the new unified nav (correctly gated on channelsEnabled).

k11kirky · 2026-07-09T13:37:44Z

+              // this toggle is its successor. Keep firing the legacy
+              // enter/leave events so space-adoption dashboards stay continuous.
+              track(ANALYTICS_EVENTS.CHANNEL_ACTION, {
+                action_type: checked ? "enter_space" : "leave_space",


The merged Channels toggle re-fires legacy enter_space/leave_space analytics with only surface: "nav", silently dropping the title_bar and header_button surface values the old Exit button and BluebirdButton used to send (both removed by this PR). Any dashboard/breakdown by surface will see title_bar/header_button stop appearing entirely after this ships — a shape-change in historical analytics data, not quite the "stay continuous" the comment above claims.

Good catch. Leaving this as a follow-up — whether to keep re-firing the legacy enter_space/leave_space events at all, and whether to preserve the old title_bar/header_button surface values, is an analytics data-shape decision I don't want to make silently in this PR. Will raise it separately.

k11kirky · 2026-07-09T13:37:44Z


 const log = logger.scope("sidebar-menu");

+function isEditableTarget(target: EventTarget | null): boolean {


isEditableTarget is restored verbatim from the deleted MainSidebar.tsx as a fresh local copy — at least an 8th independent copy of the same editable-target check in this codebase (AgentBuilderDockLayout.tsx, HomeView.tsx, useInboxReportListSelection.ts, useAutoFocusOnTyping.ts, PlanApprovalSelector.tsx, useActionSelectorState.ts, SpaceSwitcher.tsx), with no shared helper in @posthog/shared or @posthog/ui. Pre-existing duplication, not introduced here, but this was a natural point to extract one.

Agreed it's the natural extraction point, but since it's pre-existing duplication spanning ~8 files and out of scope for this PR, I'll do the shared-helper extraction as a separate follow-up.

…s, drop dead code - browserTabsStore.setSnapshot bails on value-equal snapshots so a mutation's authoritative echo doesn't re-render every subscriber and re-run the nav effect when an optimistic write already set the same value. - Extract applyOptimistic() for the nav effect's activate/replace optimistic mirror writes instead of hand-inlining the same read-transform-set twice. - Dedupe concurrent #me provisioning in landOnDefault via a shared in-flight promise — a double-fired Cmd+T can't create two "me" folders. - Delete the dead ProjectSwitcher "button" trigger variant (no callers left). - Remove the now-unreachable FeedbackModal "leaving" mode + its test case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

greptile-apps Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread packages/workspace-server/src/db/migrations/0018_add_app_view.sql Outdated

adamleithp force-pushed the ux/merge-code-bluebird branch from fe25ae3 to 9a4f1c8 Compare July 7, 2026 11:44

adamleithp added Stamphog This will request an autostamp by stamphog on small changes trunk-merge-queue-submit Adding this label to a pull request enqueues it, and removing this label dequeues it labels Jul 7, 2026

stamphog Bot reviewed Jul 7, 2026

View reviewed changes

stamphog Bot removed the Stamphog This will request an autostamp by stamphog on small changes label Jul 7, 2026

adamleithp force-pushed the ux/merge-code-bluebird branch from 8efdc80 to ac6cf06 Compare July 7, 2026 12:23

adamleithp changed the title ~~Browser tabs: bulletproof persistence, skeleton-first navigation, bounded memory~~ Merge bluebird/code + Browser tabs – bulletproof persistence, skeleton-first navigation, bounded memory Jul 7, 2026

adamleithp and others added 12 commits July 9, 2026 10:29

chore(migrations): add trailing newline to 0018_add_app_view.sql

e92f8f8

Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c

conflicts, add back activity

dd65e61

adamleithp force-pushed the ux/merge-code-bluebird branch from 0e413d4 to 15e6e46 Compare July 9, 2026 10:30

k11kirky reviewed Jul 9, 2026

View reviewed changes

adamleithp and others added 2 commits July 9, 2026 22:03

completely rebuilt tabs, beautiful

b693feb


		const log = logger.scope("sidebar-menu");

		function isEditableTarget(target: EventTarget \| null): boolean {

Uh oh!

Conversation

adamleithp commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(fix) Tab persistence — nav clicks opened duplicate tabs, switches overwrote other tabs

(perf) Tab switching — click to first paint went from ~570ms of frozen UI to a next-frame skeleton

(perf) Memory — bounded per-visit caches

(migration)

(ui) Sidebar-merge regressions restored

Verification

Uh oh!

trunk-io Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments Outside Diff (1)

Uh oh!

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stamphog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

adamleithp commented Jul 9, 2026

Uh oh!

k11kirky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

k11kirky Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

adamleithp Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

k11kirky Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

adamleithp Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adamleithp commented Jul 7, 2026 •

edited

Loading

trunk-io Bot commented Jul 7, 2026 •

edited

Loading

greptile-apps Bot commented Jul 7, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading