From 5807f766a40d4068f5a948837d2428ec95c93b6b Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Tue, 7 Jul 2026 09:43:40 +0100 Subject: [PATCH 01/15] fix(browser-tabs): make tab persistence bulletproof across app views 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 --- packages/shared/src/analytics-events.ts | 1 + packages/shared/src/browser-tabs-schemas.ts | 7 + packages/shared/src/browser-tabs.test.ts | 560 +++++++++ packages/shared/src/browser-tabs.ts | 82 +- packages/shared/src/index.ts | 1 + .../features/browser-tabs/BrowserTabStrip.tsx | 295 ++++- .../browser-tabs/browserTabsClient.ts | 2 + .../browser-tabs/displayOrder.test.ts | 1 + .../canvas/components/ChannelsList.tsx | 7 +- .../canvas/components/ChannelsSidebar.tsx | 282 ++--- .../features/command/keyboard-shortcuts.ts | 11 +- .../sidebar/components/MainSidebar.tsx | 51 - .../sidebar/components/ProjectSwitcher.tsx | 17 +- .../sidebar/components/SidebarContent.tsx | 39 - .../sidebar/components/SidebarMenu.tsx | 36 +- .../sidebar/components/SidebarNavSection.tsx | 62 +- .../sidebar/components/items/ActivityItem.tsx | 41 + .../ui/src/features/sidebar/sidebarStore.ts | 11 + packages/ui/src/router/routes/__root.tsx | 309 +++-- .../{HeaderRow.tsx => ContentHeader.tsx} | 115 +- .../src/db/migrations/0019_add_app_view.sql | 1 + .../src/db/migrations/meta/0019_snapshot.json | 1023 +++++++++++++++++ .../src/db/migrations/meta/_journal.json | 9 +- .../repositories/browser-tabs-repository.ts | 2 + packages/workspace-server/src/db/schema.ts | 3 + .../src/services/browser-tabs/schemas.ts | 2 + .../src/services/browser-tabs/service.ts | 17 +- 27 files changed, 2333 insertions(+), 654 deletions(-) delete mode 100644 packages/ui/src/features/sidebar/components/MainSidebar.tsx delete mode 100644 packages/ui/src/features/sidebar/components/SidebarContent.tsx create mode 100644 packages/ui/src/features/sidebar/components/items/ActivityItem.tsx rename packages/ui/src/shell/{HeaderRow.tsx => ContentHeader.tsx} (67%) create mode 100644 packages/workspace-server/src/db/migrations/0019_add_app_view.sql create mode 100644 packages/workspace-server/src/db/migrations/meta/0019_snapshot.json diff --git a/packages/shared/src/analytics-events.ts b/packages/shared/src/analytics-events.ts index b3914781d7..0ede9a0460 100644 --- a/packages/shared/src/analytics-events.ts +++ b/packages/shared/src/analytics-events.ts @@ -840,6 +840,7 @@ export type ChannelsSurface = export type ChannelActionType = | "enter_space" | "leave_space" + | "toggle_channels" | "leave_feedback" | "nav_click" | "open_channel" diff --git a/packages/shared/src/browser-tabs-schemas.ts b/packages/shared/src/browser-tabs-schemas.ts index 2fceacaa96..c7f36d3329 100644 --- a/packages/shared/src/browser-tabs-schemas.ts +++ b/packages/shared/src/browser-tabs-schemas.ts @@ -25,6 +25,13 @@ export const browserTabSchema = z.object({ * blank). Pairs with `channelId`: the two together identify a channel tab. */ channelSection: z.string().nullable().default(null), + /** + * Top-level app page this tab shows (`inbox` / `agents` / `skills` / + * `mcp-servers` / `command-center` / `home`). Null for a canvas / task / + * channel / blank tab. These pages have no channel, task, or dashboard id, so + * this is what lets them be a real tab target (label + restore-on-refocus). + */ + appView: z.string().nullable().default(null), /** Gap-spaced ordering key within a window. Reindexed on collision. */ position: z.number(), /** diff --git a/packages/shared/src/browser-tabs.test.ts b/packages/shared/src/browser-tabs.test.ts index b65ca6f3f0..df65f0c1a4 100644 --- a/packages/shared/src/browser-tabs.test.ts +++ b/packages/shared/src/browser-tabs.test.ts @@ -11,6 +11,7 @@ import { primaryWindowHasNoTabs, setTabOrder, setTabTarget, + setWindowActiveTab, } from "./browser-tabs"; import type { TabsSnapshot } from "./browser-tabs-schemas"; @@ -269,6 +270,7 @@ describe("decideTabNavigation", () => { taskId: null, channelId: "c1", channelSection: null, + appView: null, stampTabId: "tab-a", }); }); @@ -292,6 +294,7 @@ describe("decideTabNavigation", () => { taskId: null, channelId: "c1", channelSection: null, + appView: null, stampTabId: "tab-a", }); }); @@ -309,6 +312,7 @@ describe("decideTabNavigation", () => { taskId: null, channelId: "c1", channelSection: null, + appView: null, stampTabId: null, }); }); @@ -336,6 +340,7 @@ describe("decideTabNavigation", () => { taskId: null, channelId: "c1", channelSection: "artifacts", + appView: null, stampTabId: "tab-a", }); }); @@ -353,6 +358,7 @@ describe("decideTabNavigation", () => { taskId: null, channelId: "c1", channelSection: "history", + appView: null, stampTabId: null, }); }); @@ -601,3 +607,557 @@ describe("primaryWindow", () => { expect(primaryWindow(s)?.id).toBe("w1"); }); }); + +function openAppView(s: TabsSnapshot, windowId: string, appView: string) { + return openOrFocusTab(s, { + windowId, + dashboardId: null, + taskId: null, + channelId: null, + appView, + makeId, + now, + }); +} + +describe("setWindowActiveTab", () => { + it("focuses a tab that exists in the window", () => { + const a = open(snapshot(), "w1", "dash-a"); + const b = open(a.snapshot, "w1", "dash-b"); + const next = setWindowActiveTab(b.snapshot, "w1", a.tabId); + expect(next.windows[0].activeTabId).toBe(a.tabId); + }); + + it("clears focus with null (landing state)", () => { + const a = open(snapshot(), "w1", "dash-a"); + const next = setWindowActiveTab(a.snapshot, "w1", null); + expect(next.windows[0].activeTabId).toBeNull(); + }); + + it("ignores a tab id that does not exist (dead history tag)", () => { + const a = open(snapshot(), "w1", "dash-a"); + const next = setWindowActiveTab(a.snapshot, "w1", "closed-long-ago"); + expect(next).toBe(a.snapshot); + expect(next.windows[0].activeTabId).toBe(a.tabId); + }); + + it("ignores a tab that belongs to another window", () => { + const base = snapshot({ + windows: [ + { id: "w1", isPrimary: true, bounds: null, activeTabId: null }, + { id: "w2", isPrimary: false, bounds: null, activeTabId: null }, + ], + }); + const foreign = open(base, "w2", "dash-z"); + const next = setWindowActiveTab(foreign.snapshot, "w1", foreign.tabId); + expect(next).toBe(foreign.snapshot); + expect(next.windows[0].activeTabId).toBeNull(); + }); + + it("ignores an unknown window", () => { + const a = open(snapshot(), "w1", "dash-a"); + expect(setWindowActiveTab(a.snapshot, "w-nope", null)).toBe(a.snapshot); + }); + + it("keeps snapshot identity when the tab is already active", () => { + const a = open(snapshot(), "w1", "dash-a"); + expect(setWindowActiveTab(a.snapshot, "w1", a.tabId)).toBe(a.snapshot); + }); + + it("a tab closed then re-activated by a stale id never dangles", () => { + // The persistence-bug shape: close a tab, then a back/forward replay tries + // to focus its id. The active tab must survive untouched — a dangling + // activeTabId makes every later navigation open a new tab. + const a = open(snapshot(), "w1", "dash-a"); + const b = open(a.snapshot, "w1", "dash-b"); + const closed = closeTab(b.snapshot, b.tabId).snapshot; + const next = setWindowActiveTab(closed, "w1", b.tabId); + expect(next).toBe(closed); + expect(next.windows[0].activeTabId).toBe(a.tabId); + expect(next.tabs.some((t) => t.id === next.windows[0].activeTabId)).toBe( + true, + ); + }); +}); + +describe("decideTabNavigation: dead history tags (back/forward over closed tabs)", () => { + const base = { + historyTabId: null as string | null, + serverActiveTabId: null as string | null, + activeTab: null as { + id: string; + dashboardId: string | null; + taskId: string | null; + } | null, + routeDashboardId: null as string | null, + routeTaskId: null as string | null, + routeChannelId: null as string | null, + }; + + it("does NOT activate a tagged tab that no longer exists — replays in the active tab", () => { + // Back onto an entry whose tab was closed: fall through to the route and + // replace the active tab, instead of persisting a dangling activeTabId. + expect( + decideTabNavigation({ + ...base, + historyTabId: "tab-closed", + windowTabIds: ["tab-a", "tab-b"], + serverActiveTabId: "tab-a", + activeTab: { id: "tab-a", dashboardId: "old", taskId: null }, + routeDashboardId: "from-history", + routeChannelId: "c1", + }), + ).toEqual({ + type: "replace", + tabId: "tab-a", + dashboardId: "from-history", + taskId: null, + channelId: "c1", + channelSection: null, + appView: null, + stampTabId: "tab-a", + }); + }); + + it("opens a tab for a dead tag when nothing is active", () => { + expect( + decideTabNavigation({ + ...base, + historyTabId: "tab-closed", + windowTabIds: [], + serverActiveTabId: null, + routeDashboardId: "d1", + routeChannelId: "c1", + }), + ).toEqual({ + type: "open", + dashboardId: "d1", + taskId: null, + channelId: "c1", + channelSection: null, + appView: null, + stampTabId: null, + }); + }); + + it("re-stamps the entry with the live active tab when the route already matches", () => { + expect( + decideTabNavigation({ + ...base, + historyTabId: "tab-closed", + windowTabIds: ["tab-a"], + serverActiveTabId: "tab-a", + activeTab: { id: "tab-a", dashboardId: "same", taskId: null }, + routeDashboardId: "same", + routeChannelId: null, + }), + ).toEqual({ type: "stamp", stampTabId: "tab-a" }); + }); + + it("still activates a live tagged tab (windowTabIds provided)", () => { + expect( + decideTabNavigation({ + ...base, + historyTabId: "tab-b", + windowTabIds: ["tab-a", "tab-b"], + serverActiveTabId: "tab-a", + }), + ).toEqual({ type: "activate", tabId: "tab-b" }); + }); + + it("trusts the tag when windowTabIds is omitted (legacy callers)", () => { + expect( + decideTabNavigation({ + ...base, + historyTabId: "tab-b", + serverActiveTabId: "tab-a", + }), + ).toEqual({ type: "activate", tabId: "tab-b" }); + }); +}); + +describe("decideTabNavigation: app-view tabs (Inbox, Command center, …)", () => { + const base = { + historyTabId: null as string | null, + serverActiveTabId: null as string | null, + activeTab: null, + routeDashboardId: null as string | null, + routeTaskId: null as string | null, + routeChannelId: null as string | null, + }; + + it("replaces the active tab in place on an untagged nav to an app view", () => { + // The reported bug: clicking a nav item (Inbox, Command center, …) must + // navigate IN the current tab, not open a new one. + expect( + decideTabNavigation({ + ...base, + serverActiveTabId: "tab-a", + activeTab: { + id: "tab-a", + dashboardId: "dash-1", + taskId: null, + }, + routeAppView: "inbox", + }), + ).toEqual({ + type: "replace", + tabId: "tab-a", + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: "inbox", + stampTabId: "tab-a", + }); + }); + + it("a blank tab absorbs the first app view clicked (new-tab page keeps the URL)", () => { + expect( + decideTabNavigation({ + ...base, + serverActiveTabId: "tab-blank", + activeTab: { + id: "tab-blank", + dashboardId: null, + taskId: null, + }, + routeAppView: "command-center", + }), + ).toEqual({ + type: "replace", + tabId: "tab-blank", + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: "command-center", + stampTabId: "tab-blank", + }); + }); + + it("only stamps when the active tab already shows the app view", () => { + expect( + decideTabNavigation({ + ...base, + serverActiveTabId: "tab-a", + activeTab: { + id: "tab-a", + dashboardId: null, + taskId: null, + appView: "inbox", + }, + routeAppView: "inbox", + }), + ).toEqual({ type: "stamp", stampTabId: "tab-a" }); + }); + + it("switching between app views replaces in place (no duplicate tab)", () => { + expect( + decideTabNavigation({ + ...base, + serverActiveTabId: "tab-a", + activeTab: { + id: "tab-a", + dashboardId: null, + taskId: null, + appView: "inbox", + }, + routeAppView: "skills", + }), + ).toEqual({ + type: "replace", + tabId: "tab-a", + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: "skills", + stampTabId: "tab-a", + }); + }); + + it("opens a tab for an app view when nothing is active", () => { + expect( + decideTabNavigation({ + ...base, + routeAppView: "agents", + }), + ).toEqual({ + type: "open", + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: "agents", + stampTabId: null, + }); + }); +}); + +describe("openOrFocusTab: app-view identity", () => { + it("dedups the same app view instead of opening a second tab", () => { + const first = openAppView(snapshot(), "w1", "inbox"); + const second = openAppView(first.snapshot, "w1", "inbox"); + expect(second.opened).toBe(false); + expect(second.tabId).toBe(first.tabId); + expect(second.snapshot.tabs).toHaveLength(1); + }); + + it("treats different app views as distinct tabs", () => { + const inbox = openAppView(snapshot(), "w1", "inbox"); + const skills = openAppView(inbox.snapshot, "w1", "skills"); + expect(skills.opened).toBe(true); + expect(skills.snapshot.tabs).toHaveLength(2); + }); + + it("an app-view tab and a blank tab are distinct identities", () => { + const blank = newBlankTab(snapshot(), { windowId: "w1", makeId, now }); + const inbox = openAppView(blank.snapshot, "w1", "inbox"); + expect(inbox.opened).toBe(true); + expect(inbox.snapshot.tabs).toHaveLength(2); + }); +}); + +describe("setTabTarget: app views", () => { + it("points a blank tab at an app view and back to blank", () => { + const blank = newBlankTab(snapshot(), { windowId: "w1", makeId, now }); + const withView = setTabTarget(blank.snapshot, { + tabId: blank.tabId, + dashboardId: null, + taskId: null, + channelId: null, + appView: "command-center", + now, + }); + expect(withView.tabs[0].appView).toBe("command-center"); + expect(activeTabIsBlank(withView)).toBe(false); + + const backToBlank = setTabTarget(withView, { + tabId: blank.tabId, + dashboardId: null, + taskId: null, + channelId: null, + now, + }); + expect(backToBlank.tabs[0].appView).toBeNull(); + expect(activeTabIsBlank(backToBlank)).toBe(true); + }); + + it("clears the app view when the tab navigates to a canvas", () => { + const inbox = openAppView(snapshot(), "w1", "inbox"); + const next = setTabTarget(inbox.snapshot, { + tabId: inbox.tabId, + dashboardId: "dash-1", + taskId: null, + channelId: "c1", + now, + }); + const tab = next.tabs.find((t) => t.id === inbox.tabId); + expect(tab?.appView).toBeNull(); + expect(tab?.dashboardId).toBe("dash-1"); + }); +}); + +describe("regression: the reported tab-persistence bugs", () => { + // Three tabs; the third is active (this is the persisted boot state in the + // report: "my third tab is taking the URL of any URL I try on tab 1/2"). + function threeTabs() { + const t1 = open(snapshot(), "w1", "dash-1"); + const t2 = open(t1.snapshot, "w1", "dash-2"); + const t3 = open(t2.snapshot, "w1", "dash-3"); + return { s: t3.snapshot, ids: [t1.tabId, t2.tabId, t3.tabId] as const }; + } + + it("a navigation after a tab switch writes to the switched tab, not the previous one", () => { + const { s, ids } = threeTabs(); + const [t1, , t3] = ids; + expect(s.windows[0].activeTabId).toBe(t3); + + // User clicks tab 1 in the strip → the entry is tagged t1 → activate. + const clickTab1 = decideTabNavigation({ + historyTabId: t1, + windowTabIds: ids, + serverActiveTabId: t3, + activeTab: null, + routeDashboardId: "dash-1", + routeTaskId: null, + routeChannelId: "c1", + }); + expect(clickTab1).toEqual({ type: "activate", tabId: t1 }); + + // The strip applies the focus to its mirror synchronously (the fix): the + // next decision must see t1 active, NOT the stale t3. + const afterSwitch = setWindowActiveTab(s, "w1", t1); + expect(afterSwitch.windows[0].activeTabId).toBe(t1); + + // User clicks a nav item (untagged navigation). It must replace t1. + const activeTab = afterSwitch.tabs.find((t) => t.id === t1); + const navToInbox = decideTabNavigation({ + historyTabId: null, + windowTabIds: ids, + serverActiveTabId: t1, + activeTab: activeTab + ? { + id: activeTab.id, + dashboardId: activeTab.dashboardId, + taskId: activeTab.taskId, + channelId: activeTab.channelId, + channelSection: activeTab.channelSection, + appView: activeTab.appView, + } + : null, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: null, + routeAppView: "inbox", + }); + expect(navToInbox.type).toBe("replace"); + if (navToInbox.type !== "replace") throw new Error("unreachable"); + expect(navToInbox.tabId).toBe(t1); + + // Apply the write: only t1 changed; t3 keeps its canvas. + const applied = setTabTarget(afterSwitch, { + tabId: navToInbox.tabId, + dashboardId: navToInbox.dashboardId, + taskId: navToInbox.taskId, + channelId: navToInbox.channelId, + channelSection: navToInbox.channelSection, + appView: navToInbox.appView, + now, + }); + expect(applied.tabs.find((t) => t.id === t1)?.appView).toBe("inbox"); + expect(applied.tabs.find((t) => t.id === t3)?.dashboardId).toBe("dash-3"); + expect(applied.tabs.find((t) => t.id === ids[1])?.dashboardId).toBe( + "dash-2", + ); + }); + + it("switching tabs never rewrites the target tab's contents", () => { + const { s, ids } = threeTabs(); + const [t1, t2] = ids; + // Switch t3 → t2 → t1: pure focus changes. + let cur = setWindowActiveTab(s, "w1", t2); + cur = setWindowActiveTab(cur, "w1", t1); + expect(cur.tabs.find((t) => t.id === t1)?.dashboardId).toBe("dash-1"); + expect(cur.tabs.find((t) => t.id === t2)?.dashboardId).toBe("dash-2"); + expect(cur.tabs.find((t) => t.id === ids[2])?.dashboardId).toBe("dash-3"); + // Tab records are untouched by focus changes — same array identity. + expect(cur.tabs).toBe(s.tabs); + }); + + it("back over a closed tab's entry cannot dangle focus and flood new tabs", () => { + const { s, ids } = threeTabs(); + const [t1, , t3] = ids; + // Close t1, then replay a history entry tagged with it. + const closed = closeTabs(s, [t1]); + const live = closed.tabs.map((t) => t.id); + const decision = decideTabNavigation({ + historyTabId: t1, + windowTabIds: live, + serverActiveTabId: closed.windows[0].activeTabId, + activeTab: (() => { + const active = closed.tabs.find( + (t) => t.id === closed.windows[0].activeTabId, + ); + return active + ? { + id: active.id, + dashboardId: active.dashboardId, + taskId: active.taskId, + channelId: active.channelId, + channelSection: active.channelSection, + appView: active.appView, + } + : null; + })(), + routeDashboardId: "dash-1", + routeTaskId: null, + routeChannelId: "c1", + }); + // Never "activate" the dead id — the route replays in the active tab. + expect(decision.type).toBe("replace"); + if (decision.type !== "replace") throw new Error("unreachable"); + expect(live).toContain(decision.tabId); + expect(decision.tabId).toBe(t3); + + // And even a hostile setActiveTab with the dead id is a validated no-op. + expect(setWindowActiveTab(closed, "w1", t1)).toBe(closed); + }); + + it("a new blank tab stays blank until the user navigates, then keeps that URL", () => { + const withTabs = open(snapshot(), "w1", "dash-1"); + const blank = newBlankTab(withTabs.snapshot, { + windowId: "w1", + makeId, + now, + }); + expect(activeTabIsBlank(blank.snapshot)).toBe(true); + + // The landing route is a noop — nothing may rewrite the blank tab. + const onLanding = decideTabNavigation({ + historyTabId: blank.tabId, + windowTabIds: blank.snapshot.tabs.map((t) => t.id), + serverActiveTabId: blank.tabId, + activeTab: { + id: blank.tabId, + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: null, + }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: null, + routeAppView: null, + }); + expect(onLanding).toEqual({ type: "noop" }); + + // First click (Command center) replaces the blank tab in place… + const firstNav = decideTabNavigation({ + historyTabId: blank.tabId, + windowTabIds: blank.snapshot.tabs.map((t) => t.id), + serverActiveTabId: blank.tabId, + activeTab: { + id: blank.tabId, + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: null, + }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: null, + routeAppView: "command-center", + }); + expect(firstNav.type).toBe("replace"); + if (firstNav.type !== "replace") throw new Error("unreachable"); + expect(firstNav.tabId).toBe(blank.tabId); + + const applied = setTabTarget(blank.snapshot, { + tabId: firstNav.tabId, + dashboardId: firstNav.dashboardId, + taskId: firstNav.taskId, + channelId: firstNav.channelId, + channelSection: firstNav.channelSection, + appView: firstNav.appView, + now, + }); + // …and the other tab keeps its canvas untouched. + expect(applied.tabs.find((t) => t.id === blank.tabId)?.appView).toBe( + "command-center", + ); + expect(applied.tabs.find((t) => t.id === withTabs.tabId)?.dashboardId).toBe( + "dash-1", + ); + }); +}); + +describe("activeTabIsBlank: app views", () => { + it("is false when the active tab shows an app view", () => { + const t = openAppView(snapshot(), "w1", "inbox"); + expect(activeTabIsBlank(t.snapshot)).toBe(false); + }); +}); diff --git a/packages/shared/src/browser-tabs.ts b/packages/shared/src/browser-tabs.ts index d7248a74eb..148b936126 100644 --- a/packages/shared/src/browser-tabs.ts +++ b/packages/shared/src/browser-tabs.ts @@ -44,7 +44,11 @@ export function activeTabIsBlank(snapshot: TabsSnapshot): boolean { if (!w?.activeTabId) return false; const t = snapshot.tabs.find((x) => x.id === w.activeTabId); return ( - !!t && t.dashboardId == null && t.taskId == null && t.channelId == null + !!t && + t.dashboardId == null && + t.taskId == null && + t.channelId == null && + t.appView == null ); } @@ -72,6 +76,28 @@ function setActiveTab( }; } +/** + * Focus a tab in a window, validating the target: the tab must exist and live + * in that window, otherwise the snapshot is returned unchanged. A `null` tabId + * clears focus (the landing state). This is the persistence-safe primitive — + * history entries can carry ids of tabs closed since (back/forward replay), and + * blindly persisting such an id leaves the window with a dangling activeTabId, + * after which every navigation looks like "no active tab" and opens a new tab. + */ +export function setWindowActiveTab( + snapshot: TabsSnapshot, + windowId: string, + tabId: string | null, +): TabsSnapshot { + if (tabId !== null) { + const tab = snapshot.tabs.find((t) => t.id === tabId); + if (!tab || tab.windowId !== windowId) return snapshot; + } + const window = snapshot.windows.find((w) => w.id === windowId); + if (!window || window.activeTabId === tabId) return snapshot; + return setActiveTab(snapshot, windowId, tabId); +} + /** What a tab points at: a canvas, a task, or neither (blank). */ export type TabTarget = { dashboardId: string | null; @@ -89,6 +115,7 @@ export type TabIdentity = { taskId: string | null; channelId: string | null; channelSection: string | null; + appView: string | null; }; function sameIdentity(a: TabIdentity, b: TabIdentity): boolean { @@ -96,7 +123,8 @@ function sameIdentity(a: TabIdentity, b: TabIdentity): boolean { a.dashboardId === b.dashboardId && a.taskId === b.taskId && a.channelId === b.channelId && - a.channelSection === b.channelSection + a.channelSection === b.channelSection && + a.appView === b.appView ); } @@ -111,16 +139,24 @@ export function openOrFocusTab( windowId: string; channelId: string | null; channelSection?: string | null; + appView?: string | null; makeId: IdFactory; now: Clock; }, ): OpenTabResult { const { windowId, dashboardId, taskId, channelId, makeId, now } = input; const channelSection = input.channelSection ?? null; + const appView = input.appView ?? null; const existing = snapshot.tabs.find( (t) => t.windowId === windowId && - sameIdentity(t, { dashboardId, taskId, channelId, channelSection }), + sameIdentity(t, { + dashboardId, + taskId, + channelId, + channelSection, + appView, + }), ); if (existing) { const ts = now(); @@ -143,6 +179,7 @@ export function openOrFocusTab( taskId, channelId, channelSection, + appView, makeId, now, }); @@ -154,6 +191,7 @@ function appendTab( windowId: string; channelId: string | null; channelSection?: string | null; + appView?: string | null; makeId: IdFactory; now: Clock; }, @@ -169,6 +207,7 @@ function appendTab( taskId, channelId, channelSection: input.channelSection ?? null, + appView: input.appView ?? null, position: lastPos + POSITION_GAP, scrollState: null, createdAt: ts, @@ -212,6 +251,7 @@ export function setTabTarget( tabId: string; channelId: string | null; channelSection?: string | null; + appView?: string | null; now: Clock; }, ): TabsSnapshot { @@ -228,6 +268,7 @@ export function setTabTarget( taskId: input.taskId, channelId: input.channelId, channelSection: input.channelSection ?? null, + appView: input.appView ?? null, lastActiveAt: ts, } : t, @@ -391,6 +432,7 @@ export type TabNavDecision = taskId: string | null; channelId: string | null; channelSection: string | null; + appView: string | null; stampTabId: string | null; } | { @@ -399,6 +441,7 @@ export type TabNavDecision = taskId: string | null; channelId: string | null; channelSection: string | null; + appView: string | null; stampTabId: string | null; } | { type: "stamp"; stampTabId: string } @@ -407,6 +450,14 @@ export type TabNavDecision = export function decideTabNavigation(input: { /** tabId carried in the current history entry, if any. */ historyTabId: string | null; + /** + * Ids of the tabs that currently exist in this window. A history entry can + * be tagged with a tab that has since been closed (back/forward replays the + * entry); such a dead tag must NOT activate — it falls through and the route + * decides (in-tab replace / open / stamp), which also re-stamps the entry + * with a live tab. When omitted, tags are trusted (legacy behaviour). + */ + windowTabIds?: readonly string[]; /** The window's active tab id from the server snapshot (lags history). */ serverActiveTabId: string | null; /** The active tab record, if one exists. */ @@ -416,6 +467,7 @@ export function decideTabNavigation(input: { taskId: string | null; channelId?: string | null; channelSection?: string | null; + appView?: string | null; } | null; /** Canvas in the current route, if any. */ routeDashboardId: string | null; @@ -424,6 +476,8 @@ export function decideTabNavigation(input: { routeChannelId: string | null; /** Channel sub-section in the current route, if any. */ routeChannelSection?: string | null; + /** Top-level app page in the current route, if any. */ + routeAppView?: string | null; }): TabNavDecision { const { historyTabId, @@ -434,13 +488,19 @@ export function decideTabNavigation(input: { routeChannelId, } = input; const routeChannelSection = input.routeChannelSection ?? null; + const routeAppView = input.routeAppView ?? null; // Tagged entry for a DIFFERENT tab → a tab switch or a back/forward replay. - // Focus it (this is how "back returns to the previous tab" resolves). When - // the tag equals the active tab we must NOT stop here: a plain navigation - // (e.g. the sidebar) inherits the active tab's tag, so an in-tab nav arrives - // tagged with the active tab — fall through and decide from the route. - if (historyTabId && historyTabId !== serverActiveTabId) { + // Focus it (this is how "back returns to the previous tab" resolves). Two + // guards: (1) the tagged tab must still exist — back/forward can replay an + // entry whose tab was closed, and activating a dead id persists a dangling + // activeTabId (every nav then opens a new tab); (2) when the tag equals the + // active tab we must NOT stop here: an in-tab nav can arrive tagged with the + // active tab — fall through and decide from the route. + const historyTabIsLive = + !!historyTabId && + (input.windowTabIds ? input.windowTabIds.includes(historyTabId) : true); + if (historyTabId && historyTabIsLive && historyTabId !== serverActiveTabId) { return { type: "activate", tabId: historyTabId }; } @@ -452,8 +512,9 @@ export function decideTabNavigation(input: { taskId: routeTaskId, channelId: routeChannelId, channelSection: routeChannelSection, + appView: routeAppView, }; - if (!routeDashboardId && !routeTaskId && !routeChannelId) { + if (!routeDashboardId && !routeTaskId && !routeChannelId && !routeAppView) { return { type: "noop" }; } @@ -465,6 +526,7 @@ export function decideTabNavigation(input: { taskId: activeTab.taskId, channelId: activeTab.channelId ?? null, channelSection: activeTab.channelSection ?? null, + appView: activeTab.appView ?? null, }, routeIdentity, ) @@ -476,6 +538,7 @@ export function decideTabNavigation(input: { taskId: routeTaskId, channelId: routeChannelId, channelSection: routeChannelSection, + appView: routeAppView, stampTabId: serverActiveTabId, }; } @@ -486,6 +549,7 @@ export function decideTabNavigation(input: { taskId: routeTaskId, channelId: routeChannelId, channelSection: routeChannelSection, + appView: routeAppView, stampTabId: serverActiveTabId, }; } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index c37a45f941..dea20119dd 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -30,6 +30,7 @@ export { primaryWindowHasNoTabs, setTabOrder, setTabTarget, + setWindowActiveTab, type TabNavDecision, type TabTarget, } from "./browser-tabs"; diff --git a/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx b/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx index e42714f57f..b612accb50 100644 --- a/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx +++ b/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx @@ -1,9 +1,19 @@ -import { HashIcon } from "@phosphor-icons/react"; +import { + BrainIcon, + HashIcon, + HouseIcon, + PlugsConnectedIcon, + RobotIcon, + SquaresFourIcon, + TrayIcon, +} from "@phosphor-icons/react"; import { browserTabsStore } from "@posthog/core/browser-tabs/browserTabsStore"; import { useHostTRPC } from "@posthog/host-router/react"; import { decideTabNavigation, setTabOrder, + setTabTarget as setTabTargetLocal, + setWindowActiveTab, type TabsSnapshot, } from "@posthog/shared"; import { channelSectionFor } from "@posthog/ui/features/canvas/channelSections"; @@ -13,8 +23,12 @@ import { useDashboard, useDashboards, } from "@posthog/ui/features/canvas/hooks/useDashboards"; +import { SHORTCUTS } from "@posthog/ui/features/command/keyboard-shortcuts"; +import { usePanelLayoutStore } from "@posthog/ui/features/panels/panelLayoutStore"; +import { getLeafPanel } from "@posthog/ui/features/panels/panelStoreHelpers"; import { taskDetailQuery } from "@posthog/ui/features/tasks/queries"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; +import { useAppView } from "@posthog/ui/router/useAppView"; import { useMutation, useQuery } from "@tanstack/react-query"; import { useNavigate, @@ -22,7 +36,8 @@ import { useRouter, useRouterState, } from "@tanstack/react-router"; -import { useEffect, useMemo } from "react"; +import { type ReactNode, useEffect, useMemo } from "react"; +import { useHotkeys } from "react-hotkeys-hook"; import { frontOfUnpinnedOrder, partitionPinnedFirst, @@ -67,14 +82,60 @@ function primaryWindow(snapshot: TabsSnapshot) { return snapshot.windows.find((w) => w.isPrimary) ?? snapshot.windows[0]; } +// True when the open task's focused editor panel has a closeable active tab. +// Cmd+W is inner-first: it closes that editor tab (handled by +// usePanelKeyboardShortcuts) before it closes the browser tab. +function taskHasCloseableEditorTab(taskId: string | undefined): boolean { + if (!taskId) return false; + const layout = usePanelLayoutStore.getState().getLayout(taskId); + const panelId = layout?.focusedPanelId; + if (!panelId || !layout?.panelTree) return false; + const panel = getLeafPanel(layout.panelTree, panelId); + const activeTab = panel?.content.tabs.find( + (t) => t.id === panel.content.activeTabId, + ); + return !!activeTab && activeTab.closeable !== false; +} + type TabRef = { id: string; dashboardId: string | null; taskId: string | null; channelId: string | null; channelSection: string | null; + appView: string | null; +}; + +// The top-level app pages that can be a tab. Keyed by useAppView's view.type; +// each maps to its canonical route (a task/canvas/channel tab has its own +// route, these don't) plus the strip's label + icon. +type AppView = + | "home" + | "inbox" + | "agents" + | "skills" + | "mcp-servers" + | "command-center"; + +const APP_VIEW_META: Record = { + home: { label: "Home", icon: }, + inbox: { label: "Inbox", icon: }, + agents: { label: "Agents", icon: }, + skills: { label: "Skills", icon: }, + "mcp-servers": { + label: "MCP servers", + icon: , + }, + "command-center": { + label: "Command center", + icon: , + }, }; +function isAppView(value: string): value is AppView { + return value in APP_VIEW_META; +} + export function BrowserTabStrip() { const snapshot = useTabsSnapshot(); const navigate = useNavigate(); @@ -89,6 +150,15 @@ export function BrowserTabStrip() { select: (s) => s.location.state.tabId, }); const pathname = useRouterState({ select: (s) => s.location.pathname }); + // Tabs work in both spaces: channel-scoped tabs live under /website, while a + // plain task tab (no channel) belongs to the Code experience. The space + // decides where a task/blank tab navigates. + const inChannels = pathname.startsWith("/website"); + // Top-level app pages (Inbox, Agents, Skills, MCP servers, Command Center, + // Home) are tab targets too. useAppView normalizes both the /code routes and + // their /website mirrors to the same view.type, so a tab survives either space. + const view = useAppView(); + const routeAppView: AppView | null = isAppView(view.type) ? view.type : null; const { channels } = useChannels(); @@ -101,21 +171,43 @@ export function BrowserTabStrip() { return channelSectionFor(seg)?.key ?? null; }, [pathname, params.channelId]); - const openOrFocus = useMutation( - trpc.browserTabs.openOrFocus.mutationOptions(), - ); - const newBlankTab = useMutation( - trpc.browserTabs.newBlankTab.mutationOptions(), - ); - const setTabTarget = useMutation( - trpc.browserTabs.setTabTarget.mutationOptions(), - ); - const close = useMutation(trpc.browserTabs.close.mutationOptions()); - const closeMany = useMutation(trpc.browserTabs.closeMany.mutationOptions()); - const setOrder = useMutation(trpc.browserTabs.setOrder.mutationOptions()); - const setActiveTab = useMutation( - trpc.browserTabs.setActiveTab.mutationOptions(), - ); + // Every tab mutation returns the fresh authoritative snapshot. Apply it to + // the renderer mirror synchronously: the snapshot-change subscription also + // delivers it, but only after an IPC round-trip, and the navigation effect + // below makes *persistent writes* (setTabTarget/openOrFocus) from the mirror. + // A stale mirror mis-targets those writes — the classic symptom is a + // navigation replacing some other tab's contents, or opening a duplicate tab + // because the mirror still says "no active tab". + const applySnapshot = (next: TabsSnapshot) => + browserTabsStore.getState().setSnapshot(next); + const openOrFocus = useMutation({ + ...trpc.browserTabs.openOrFocus.mutationOptions(), + onSuccess: applySnapshot, + }); + const newBlankTab = useMutation({ + ...trpc.browserTabs.newBlankTab.mutationOptions(), + onSuccess: applySnapshot, + }); + const setTabTarget = useMutation({ + ...trpc.browserTabs.setTabTarget.mutationOptions(), + onSuccess: applySnapshot, + }); + const close = useMutation({ + ...trpc.browserTabs.close.mutationOptions(), + onSuccess: applySnapshot, + }); + const closeMany = useMutation({ + ...trpc.browserTabs.closeMany.mutationOptions(), + onSuccess: applySnapshot, + }); + const setOrder = useMutation({ + ...trpc.browserTabs.setOrder.mutationOptions(), + onSuccess: applySnapshot, + }); + const setActiveTab = useMutation({ + ...trpc.browserTabs.setActiveTab.mutationOptions(), + onSuccess: applySnapshot, + }); const pinnedTabIds = usePinnedTabsStore((s) => s.pinnedTabIds); const togglePinned = usePinnedTabsStore((s) => s.togglePinned); @@ -180,12 +272,22 @@ export function BrowserTabStrip() { if (!windowId) return; const stamp = (tabId: string) => { const loc = router.history.location; + // Already tagged — skip the replace. The effect re-runs on every + // snapshot broadcast, so an unguarded replace would churn history (and + // retrigger router subscribers) once per broadcast. + if ((loc.state as { tabId?: string }).tabId === tabId) return; // Use the full href (always a string); reconstructing from pathname + // search crashes because search is parsed to an object at runtime. router.history.replace(loc.href, { ...(loc.state as object), tabId }); }; const decision = decideTabNavigation({ historyTabId: historyTabId ?? null, + // Validates history tags: back/forward can replay an entry tagged with a + // closed tab; activating that dead id would persist a dangling + // activeTabId, after which every nav "opens" (no active tab found). + windowTabIds: snapshot.tabs + .filter((t) => t.windowId === windowId) + .map((t) => t.id), serverActiveTabId: win?.activeTabId ?? null, activeTab: activeTab ? { @@ -194,27 +296,46 @@ export function BrowserTabStrip() { taskId: activeTab.taskId, channelId: activeTab.channelId, channelSection: activeTab.channelSection, + appView: activeTab.appView, } : null, routeDashboardId: params.dashboardId ?? null, routeTaskId: params.taskId ?? null, routeChannelId: params.channelId ?? null, routeChannelSection, + routeAppView, }); switch (decision.type) { case "activate": + // Optimistically focus in the mirror before the round-trip: an + // untagged navigation racing this window would otherwise decide + // against the PREVIOUS active tab and replace its contents. + browserTabsStore + .getState() + .setSnapshot(setWindowActiveTab(snapshot, windowId, decision.tabId)); setActiveTab.mutate({ windowId, tabId: decision.tabId }); break; - case "replace": - setTabTarget.mutate({ + case "replace": { + const target = { tabId: decision.tabId, dashboardId: decision.dashboardId, taskId: decision.taskId, channelId: decision.channelId, channelSection: decision.channelSection, - }); + appView: decision.appView, + }; + // Same optimistic apply: keep the mirror consistent with the write so + // re-entrant runs (and the /website index redirect guard) never see + // the pre-navigation target. + browserTabsStore + .getState() + .setSnapshot( + setTabTargetLocal(snapshot, { ...target, now: Date.now }), + ); + setTabTarget.mutate(target); if (decision.stampTabId) stamp(decision.stampTabId); break; + } case "open": openOrFocus.mutate({ windowId, @@ -222,6 +343,7 @@ export function BrowserTabStrip() { taskId: decision.taskId, channelId: decision.channelId, channelSection: decision.channelSection, + appView: decision.appView, }); if (decision.stampTabId) stamp(decision.stampTabId); break; @@ -237,7 +359,11 @@ export function BrowserTabStrip() { params.dashboardId, params.taskId, routeChannelSection, + routeAppView, activeTab, + // The tab LIST feeds windowTabIds (dead-tag validation); activeTab alone + // doesn't change when an inactive tab closes in another window. + snapshot, openOrFocus.mutate, setTabTarget.mutate, setActiveTab.mutate, @@ -295,6 +421,7 @@ export function BrowserTabStrip() { const dashId = isActive ? (params.dashboardId ?? null) : t.dashboardId; const channelId = isActive ? (params.channelId ?? null) : t.channelId; const section = isActive ? routeChannelSection : t.channelSection; + const appView = isActive ? routeAppView : t.appView; const channel = channelName(channelId); if (taskId) { const task = findTask(taskId); @@ -333,6 +460,16 @@ export function BrowserTabStrip() { pinned, }; } + // A top-level app page (Inbox, Agents, Skills, …). + if (appView && isAppView(appView)) { + return { + id: t.id, + label: APP_VIEW_META[appView].label, + icon: APP_VIEW_META[appView].icon, + channelName: null, + pinned, + }; + } return { id: t.id, label: "New tab", channelName: null, pinned }; }); }, [ @@ -350,6 +487,7 @@ export function BrowserTabStrip() { params.dashboardId, params.taskId, routeChannelSection, + routeAppView, ]); // Navigate to a tab, tagging the history entry with its id so the switch is @@ -363,6 +501,13 @@ export function BrowserTabStrip() { params: { channelId: tab.channelId, taskId: tab.taskId }, state, }); + } else if (tab.taskId) { + // A channel-less task tab — the Code task detail route. + navigate({ + to: "/code/tasks/$taskId", + params: { taskId: tab.taskId }, + state, + }); } else if (tab.dashboardId && tab.channelId) { navigate({ to: "/website/$channelId/dashboards/$dashboardId", @@ -383,8 +528,33 @@ export function BrowserTabStrip() { } else { navigate({ to: "/website/$channelId", params, state }); } + } else if (tab.appView && isAppView(tab.appView)) { + // A top-level app page — back to its canonical route (literal `to` per + // case so the router types stay checked). + switch (tab.appView) { + case "home": + navigate({ to: "/code/home", state }); + break; + case "inbox": + navigate({ to: "/code/inbox", state }); + break; + case "agents": + navigate({ to: "/code/agents", state }); + break; + case "skills": + navigate({ to: "/skills", state }); + break; + case "mcp-servers": + navigate({ to: "/mcp-servers", state }); + break; + case "command-center": + navigate({ to: "/command-center", state }); + break; + } } else { - navigate({ to: "/website", state }); + // Blank / landing tab: park on the space's home — the channels index, or + // the Code new-task screen. + navigate({ to: inChannels ? "/website" : "/code", state }); } }; @@ -396,18 +566,17 @@ export function BrowserTabStrip() { goToTab(tab); }; - // Apply a post-close snapshot to the store synchronously before navigating. - // The store otherwise lags a subscription round-trip, so the /website index - // would render against the still-has-tabs snapshot and redirect to the first - // channel (re-opening a tab) before the empty strip arrives. + // Navigate to the close's survivor. The mutation-level onSuccess has already + // applied `next` to the mirror (mutation callbacks run after option-level + // ones), so the /website index renders against the post-close snapshot and + // can't redirect to the first channel (re-opening a tab) mid-flight. const applyCloseResult = (next: TabsSnapshot) => { - browserTabsStore.getState().setSnapshot(next); const w = primaryWindow(next); const active = w?.activeTabId ? next.tabs.find((t) => t.id === w.activeTabId) : null; if (active) goToTab(active); - else navigate({ to: "/website" }); + else navigate({ to: inChannels ? "/website" : "/code" }); }; const handleClose = (tabId: string) => { @@ -425,10 +594,7 @@ export function BrowserTabStrip() { browserTabsStore .getState() .setSnapshot(setTabOrder(snapshot, windowId, order)); - setOrder.mutate( - { windowId, tabIds: order }, - { onSuccess: (next) => browserTabsStore.getState().setSnapshot(next) }, - ); + setOrder.mutate({ windowId, tabIds: order }); }; // Bulk closes operate on the strip's *displayed* order (pinned-first) and @@ -473,6 +639,52 @@ export function BrowserTabStrip() { ); }; + const handleNewTab = () => { + if (!windowId) return; + newBlankTab.mutate( + { windowId }, + { + onSuccess: (next) => { + const w = primaryWindow(next); + if (w?.activeTabId) { + goToTab({ + id: w.activeTabId, + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: null, + }); + } + }, + }, + ); + }; + + // Cmd/Ctrl+T opens a new browser tab. Bound here (not globally) so it only + // fires where the strip is mounted; the new-task shortcut owns Cmd/Ctrl+N. + useHotkeys( + SHORTCUTS.NEW_TAB, + (e) => { + e.preventDefault(); + handleNewTab(); + }, + { enableOnFormTags: true, enableOnContentEditable: true }, + ); + + // Cmd/Ctrl+W closes the active browser tab. Always preventDefault so Electron + // doesn't close the window, but defer to the task's editor panel when it has a + // closeable tab (inner-first) — that handler closes the editor tab instead. + useHotkeys( + SHORTCUTS.CLOSE_TAB, + (e) => { + e.preventDefault(); + if (taskHasCloseableEditorTab(params.taskId)) return; + if (activeTabId) handleClose(activeTabId); + }, + { enableOnFormTags: true, enableOnContentEditable: true }, + ); + return ( { - if (!windowId) return; - newBlankTab.mutate( - { windowId }, - { - onSuccess: (next) => { - const w = primaryWindow(next); - if (w?.activeTabId) { - goToTab({ - id: w.activeTabId, - dashboardId: null, - taskId: null, - channelId: null, - channelSection: null, - }); - } - }, - }, - ); - }} + onNewTab={handleNewTab} /> ); } diff --git a/packages/ui/src/features/browser-tabs/browserTabsClient.ts b/packages/ui/src/features/browser-tabs/browserTabsClient.ts index cc6c9f964f..16672c39bd 100644 --- a/packages/ui/src/features/browser-tabs/browserTabsClient.ts +++ b/packages/ui/src/features/browser-tabs/browserTabsClient.ts @@ -20,6 +20,7 @@ export interface BrowserTabsClient { taskId: string | null; channelId: string | null; channelSection?: string | null; + appView?: string | null; }): Promise; newBlankTab(input: { windowId: string }): Promise; setTabTarget(input: { @@ -28,6 +29,7 @@ export interface BrowserTabsClient { taskId: string | null; channelId: string | null; channelSection?: string | null; + appView?: string | null; }): Promise; close(tabId: string): Promise; setActiveTab(input: { diff --git a/packages/ui/src/features/browser-tabs/displayOrder.test.ts b/packages/ui/src/features/browser-tabs/displayOrder.test.ts index 1c1ea7fefe..5ac2c00249 100644 --- a/packages/ui/src/features/browser-tabs/displayOrder.test.ts +++ b/packages/ui/src/features/browser-tabs/displayOrder.test.ts @@ -16,6 +16,7 @@ function tab(id: string, position: number): BrowserTab { taskId: null, channelId: null, channelSection: null, + appView: null, position, scrollState: null, createdAt: 0, diff --git a/packages/ui/src/features/canvas/components/ChannelsList.tsx b/packages/ui/src/features/canvas/components/ChannelsList.tsx index b66444f566..504703c5cc 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -32,7 +32,6 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, MenuLabel, - Separator, Tooltip, TooltipContent, TooltipProvider, @@ -562,11 +561,7 @@ export function ChannelsList() { // One shared provider groups every row tooltip so that once one shows, // moving to the next row reveals its tooltip instantly (no re-delay). - - - - - + {starred.length > 0 && ( diff --git a/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx b/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx index afb691ff93..5a1a0f3219 100644 --- a/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx @@ -1,190 +1,70 @@ -import { - BellIcon, - BrainIcon, - GearSixIcon, - HouseIcon, - RobotIcon, - SquaresFourIcon, - TrayIcon, -} from "@phosphor-icons/react"; -import { countUnseenActivity } from "@posthog/core/canvas/mentionActivity"; -import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; -import { HOME_TAB_FLAG } from "@posthog/shared/constants"; +import { ArchiveIcon } from "@phosphor-icons/react"; +import { Separator } from "@posthog/quill"; +import { PROJECT_BLUEBIRD_FLAG } from "@posthog/shared"; +import { useArchivedTaskIds } from "@posthog/ui/features/archive/useArchivedTaskIds"; +import { SidebarUsageBar } from "@posthog/ui/features/billing/SidebarUsageBar"; import { ChannelsList } from "@posthog/ui/features/canvas/components/ChannelsList"; import { useChannelsSidebarStore } from "@posthog/ui/features/canvas/components/channelsSidebarStore"; -import { useMentionActivity } from "@posthog/ui/features/canvas/hooks/useMentionActivity"; -import { useActivitySeenStore } from "@posthog/ui/features/canvas/stores/activitySeenStore"; import { useFeatureFlag } from "@posthog/ui/features/feature-flags/useFeatureFlag"; -import { openSettings } from "@posthog/ui/features/settings/hooks/useOpenSettings"; -import { SidebarCountBadge } from "@posthog/ui/features/sidebar/components/items/SidebarCountBadge"; +import { useOnboardingStore } from "@posthog/ui/features/onboarding/onboardingStore"; import { ProjectSwitcher } from "@posthog/ui/features/sidebar/components/ProjectSwitcher"; -import { SidebarItem } from "@posthog/ui/features/sidebar/components/SidebarItem"; +import { SidebarMenu } from "@posthog/ui/features/sidebar/components/SidebarMenu"; +import { SidebarNavSection } from "@posthog/ui/features/sidebar/components/SidebarNavSection"; import { UpdateBanner } from "@posthog/ui/features/sidebar/components/UpdateBanner"; +import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; +import { useWorkspaces } from "@posthog/ui/features/workspace/useWorkspace"; import { ResizableSidebar } from "@posthog/ui/primitives/ResizableSidebar"; -import { - navigateToActivity, - navigateToAgents, - navigateToCanvas, - navigateToInbox, - navigateToSkills, - navigateToWebsiteHome, -} from "@posthog/ui/router/navigationBridge"; -import { useAppView } from "@posthog/ui/router/useAppView"; -import { track } from "@posthog/ui/shell/analytics"; +import { navigateToArchived } from "@posthog/ui/router/navigationBridge"; import { Box, Flex } from "@radix-ui/themes"; -import { useRouterState } from "@tanstack/react-router"; -import { useMemo } from "react"; +import { useDeferredValue, useEffect } from "react"; -// Fire a nav_click event, then run the destination's navigation. -function trackNav(navTarget: string, navigate: () => void) { - track(ANALYTICS_EVENTS.CHANNEL_ACTION, { - action_type: "nav_click", - surface: "nav", - nav_target: navTarget, - }); - navigate(); -} - -// Non-canvas /website mirrors (Home, Files, etc.) — used to tell whether the -// current /website route is a canvas surface (channels index / a channel / a -// dashboard) so the Canvas nav item highlights only there. -const NON_CANVAS_WEBSITE_PREFIXES = [ - "/website/home", - "/website/activity", - "/website/skills", - "/website/mcp-servers", - "/website/command-center", -]; - -// The Activity nav row with its unread-mentions dot. Its own component so the -// mentions query only mounts once here. -function ActivityNavItem({ isActive }: { isActive: boolean }) { - const { items } = useMentionActivity(); - const lastSeenAt = useActivitySeenStore((s) => s.lastSeenAt); - const unseen = useMemo( - () => countUnseenActivity(items, lastSeenAt), - [items, lastSeenAt], - ); - return ( - } - label={ - - Activity - - - } - isActive={isActive} - onClick={() => trackNav("activity", navigateToActivity)} - /> - ); -} - -// The global nav brought over from the Code app — a single icon+label row each, -// no rail. Home points at the /website/home mirror so it stays in the Channels -// space (same shared HomeView, channels chrome kept); the other rows are -// app-wide destinations that leave the Channels space for the Code view. The -// channel tree below is channel browsing. -function ChannelsNav() { - const view = useAppView(); - const homeTabEnabled = useFeatureFlag(HOME_TAB_FLAG); - // Active on the canvas surfaces: the channels index, a channel, or a canvas — - // any /website route that isn't one of the cross-app mirrors above. - const isCanvasActive = useRouterState({ - select: (s) => { - const path = s.location.pathname; - return ( - path.startsWith("/website") && - !NON_CANVAS_WEBSITE_PREFIXES.some((p) => path.startsWith(p)) - ); - }, - }); - return ( - - {homeTabEnabled && ( - - } - label="Home" - isActive={view.type === "home"} - onClick={() => trackNav("home", navigateToWebsiteHome)} - /> - )} - - - } - label="Global Inbox" - isActive={view.type === "inbox"} - onClick={() => trackNav("inbox", navigateToInbox)} - /> - - } - label="Canvas" - isActive={isCanvasActive} - onClick={() => trackNav("canvas", navigateToCanvas)} - /> - - } - label="Agents" - isActive={view.type === "agents"} - onClick={() => trackNav("agents", navigateToAgents)} - /> - - } - label="Files" - isActive={view.type === "skills"} - onClick={() => trackNav("files", navigateToSkills)} - /> - - ); -} - -// The Channels-space sidebar: a single column owning the whole left pane. Top to -// bottom — workspace switcher, global nav, the channel tree, then Settings -// pinned to the bottom. There is no app rail in this space; the nav rows above -// are the cross-app navigation. +// The unified app sidebar (Code merged into the Bluebird chrome). Top to +// bottom: workspace switcher, the merged global nav, the "Enable channels" +// opt-in, then the body — the task list by default, swapped for the channel +// tree once channels are enabled — and Settings pinned to the bottom. export function ChannelsSidebar() { const width = useChannelsSidebarStore((state) => state.width); const setWidth = useChannelsSidebarStore((state) => state.setWidth); const isResizing = useChannelsSidebarStore((state) => state.isResizing); const setIsResizing = useChannelsSidebarStore((state) => state.setIsResizing); + // Cmd+B collapses the sidebar (via useSidebarStore.open, toggled globally in + // GlobalEventHandlers / the command menu). Auto-open once the user has + // finished onboarding or has any workspace, matching the retired MainSidebar — + // so a brand-new user sees the welcome screen without the sidebar beside it. + const open = useSidebarStore((s) => s.open); + const setOpenAuto = useSidebarStore((s) => s.setOpenAuto); + const hasCompletedOnboarding = useOnboardingStore( + (s) => s.hasCompletedOnboarding, + ); + const { data: workspaces = {}, isFetched: workspacesFetched } = + useWorkspaces(); + useEffect(() => { + if (!workspacesFetched) return; + setOpenAuto(hasCompletedOnboarding || Object.keys(workspaces).length > 0); + }, [workspacesFetched, workspaces, hasCompletedOnboarding, setOpenAuto]); + + // Channels stay behind project-bluebird: the toggle only appears where the + // canvas backend is wired, and a persisted "on" is ignored when the flag is + // off so the sidebar can't strand a user on an unsupported feature. + const bluebirdEnabled = useFeatureFlag( + PROJECT_BLUEBIRD_FLAG, + import.meta.env.DEV, + ); + const channelsEnabled = + useSidebarStore((s) => s.channelsEnabled) && bluebirdEnabled; + // The Switch (in SidebarNavSection) reads the live value and flips instantly. + // Swapping the sidebar body mounts a heavy tree (ChannelsList: the channels + // query + a provider-laden row per channel), so defer that decision: the + // urgent commit keeps the current body and paints the toggle, then the tree + // mounts in a follow-up non-blocking render. + const bodyChannelsEnabled = useDeferredValue(channelsEnabled); + + const archivedTaskIds = useArchivedTaskIds(); + return ( - {/* Workspace switcher — a compact bordered button. The title bar above - provides the window-drag region and stoplight clearance. */} - - - + {/* The nav owns the "Enable channels" toggle + Canvas rows (gated by + the same flag), so this section carries the whole merged nav. */} + - {/* The global nav links stay pinned below the switcher; only the channel - tree scrolls when it overflows. */} - - - - + {/* Body: the channel tree when channels are on, otherwise the task + list. Each owns its own scroll region. Gated on the deferred value so + the toggle paints before this heavy swap. */} + {bodyChannelsEnabled ? ( + <> + + + + + + ) : ( + + + + )} + + + {/* Archived is a task-list affordance — hidden while channels are on, + since the body then shows the channel tree, not tasks. */} + {!channelsEnabled && archivedTaskIds.size > 0 && ( + + + + )} - {/* Settings pinned to the bottom. Settings is a full-page route, so this - leaves the Channels space rather than highlighting in place. */} - - } - label="Settings" - onClick={() => trackNav("settings", () => openSettings())} - /> + {/* Workspace switcher pinned to the bottom. Its dropdown carries the + Settings entry, so there's no separate Settings row. */} + + diff --git a/packages/ui/src/features/command/keyboard-shortcuts.ts b/packages/ui/src/features/command/keyboard-shortcuts.ts index 94279b6490..a384557310 100644 --- a/packages/ui/src/features/command/keyboard-shortcuts.ts +++ b/packages/ui/src/features/command/keyboard-shortcuts.ts @@ -2,7 +2,8 @@ import { isMac } from "@posthog/ui/utils/platform"; export const SHORTCUTS = { COMMAND_MENU: "mod+k", - NEW_TASK: "mod+n,mod+t", + NEW_TASK: "mod+n", + NEW_TAB: "mod+t", SETTINGS: "mod+,", SHORTCUTS_SHEET: "mod+/", GO_BACK: "mod+[", @@ -52,7 +53,13 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ keys: "mod+n", description: "New task", category: "general", - alternateKeys: "mod+t", + }, + { + id: "new-tab", + keys: SHORTCUTS.NEW_TAB, + description: "New tab", + category: "navigation", + context: "Channels", }, { id: "command-menu", diff --git a/packages/ui/src/features/sidebar/components/MainSidebar.tsx b/packages/ui/src/features/sidebar/components/MainSidebar.tsx deleted file mode 100644 index 08a453433e..0000000000 --- a/packages/ui/src/features/sidebar/components/MainSidebar.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import { useOnboardingStore } from "@posthog/ui/features/onboarding/onboardingStore"; -import { Sidebar } from "@posthog/ui/features/sidebar/components/Sidebar"; -import { SidebarContent } from "@posthog/ui/features/sidebar/components/SidebarContent"; -import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; -import { useTaskSelectionStore } from "@posthog/ui/features/sidebar/taskSelectionStore"; -import { useWorkspaces } from "@posthog/ui/features/workspace/useWorkspace"; -import { Box } from "@radix-ui/themes"; -import { useEffect } from "react"; - -function isEditableTarget(target: EventTarget | null): boolean { - if (!(target instanceof HTMLElement)) return false; - if (target.isContentEditable) return true; - const tag = target.tagName; - return tag === "INPUT" || tag === "TEXTAREA" || tag === "SELECT"; -} - -export function MainSidebar() { - const { data: workspaces = {}, isFetched } = useWorkspaces(); - const hasCompletedOnboarding = useOnboardingStore( - (state) => state.hasCompletedOnboarding, - ); - const setOpenAuto = useSidebarStore((state) => state.setOpenAuto); - - useEffect(() => { - if (isFetched) { - const workspaceCount = Object.keys(workspaces).length; - setOpenAuto(hasCompletedOnboarding || workspaceCount > 0); - } - }, [isFetched, workspaces, hasCompletedOnboarding, setOpenAuto]); - - useEffect(() => { - const handler = (e: KeyboardEvent) => { - if (e.key !== "Escape") return; - if (isEditableTarget(e.target)) return; - const { selectedTaskIds, clearSelection } = - useTaskSelectionStore.getState(); - if (selectedTaskIds.length === 0) return; - clearSelection(); - }; - window.addEventListener("keydown", handler); - return () => window.removeEventListener("keydown", handler); - }, []); - - return ( - - - - - - ); -} diff --git a/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx b/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx index 8d94609e42..44d0e10256 100644 --- a/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx +++ b/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx @@ -195,7 +195,7 @@ export function ProjectSwitcher({ size="xs" className="border-border hover:bg-fill-hover aria-expanded:bg-fill-active" > - + {currentProject?.name ?? "No project selected"} @@ -214,21 +214,14 @@ export function ProjectSwitcher({ {currentUser ? ( - + {currentUser.first_name && ( @@ -543,10 +536,10 @@ function SearchableFlyout({ fits below either trigger row, so the popup itself never grows a second scrollbar. */} 5 ? "h-40" : "max-h-40"} pt-1`} + className={`${items.length > 5 ? "h-40" : "max-h-40"} p-0 pb-0`} > {(section: FlyoutSection) => ( - + {(item: FlyoutItem) => ( { - const archivedTaskIds = useArchivedTaskIds(); - return ( - - - - - - - {archivedTaskIds.size > 0 && ( - - - - )} - - - - - ); -}; diff --git a/packages/ui/src/features/sidebar/components/SidebarMenu.tsx b/packages/ui/src/features/sidebar/components/SidebarMenu.tsx index 20d5828315..29c274b45f 100644 --- a/packages/ui/src/features/sidebar/components/SidebarMenu.tsx +++ b/packages/ui/src/features/sidebar/components/SidebarMenu.tsx @@ -1,7 +1,6 @@ import { findGroupFolder } from "@posthog/core/sidebar/groupTasks"; import { isTaskActivelyRunning } from "@posthog/core/sidebar/taskRunning"; import { useHostTRPCClient } from "@posthog/host-router/react"; -import { Separator } from "@posthog/quill"; import type { Task } from "@posthog/shared/types"; import { archiveTasksImperative, @@ -35,12 +34,18 @@ import { useQueryClient } from "@tanstack/react-query"; import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { ArchiveRunningTaskDialog } from "./ArchiveRunningTaskDialog"; import { SidebarItem } from "./SidebarItem"; -import { SidebarNavSection } from "./SidebarNavSection"; import { TaskListView } from "./TaskListView"; import { TasksHeader } from "./TasksHeader"; const log = logger.scope("sidebar-menu"); +function isEditableTarget(target: EventTarget | null): boolean { + if (!(target instanceof HTMLElement)) return false; + if (target.isContentEditable) return true; + const tag = target.tagName; + return tag === "INPUT" || tag === "TEXTAREA" || tag === "SELECT"; +} + function SidebarMenuComponent() { const hostClient = useHostTRPCClient(); const archiveCacheKeys = useArchiveCacheKeys(); @@ -104,6 +109,21 @@ function SidebarMenuComponent() { taskTitle: string; } | null>(null); + // Escape clears any bulk task selection (moved here from the retired + // MainSidebar so it survives with the task list in the unified sidebar). + useEffect(() => { + const handler = (e: KeyboardEvent) => { + if (e.key !== "Escape") return; + if (isEditableTarget(e.target)) return; + const { selectedTaskIds, clearSelection } = + useTaskSelectionStore.getState(); + if (selectedTaskIds.length === 0) return; + clearSelection(); + }; + window.addEventListener("keydown", handler); + return () => window.removeEventListener("keydown", handler); + }, []); + const selectedTaskIds = useTaskSelectionStore((s) => s.selectedTaskIds); const toggleTaskSelection = useTaskSelectionStore( (s) => s.toggleTaskSelection, @@ -430,18 +450,6 @@ function SidebarMenuComponent() { id="side-bar-menu" className="flex min-h-0 flex-col" > - {/* Derive the command-center count from data SidebarMenu already holds, - so the nested nav section doesn't open its own task subscription. */} - - taskId != null && taskMap.has(taskId) ? count + 1 : count, - 0, - )} - /> - - -
diff --git a/packages/ui/src/features/sidebar/components/SidebarNavSection.tsx b/packages/ui/src/features/sidebar/components/SidebarNavSection.tsx index 77162f0759..831a766446 100644 --- a/packages/ui/src/features/sidebar/components/SidebarNavSection.tsx +++ b/packages/ui/src/features/sidebar/components/SidebarNavSection.tsx @@ -1,3 +1,7 @@ +import { HashIcon } from "@phosphor-icons/react"; +import { Badge, Switch } from "@posthog/quill"; +import { PROJECT_BLUEBIRD_FLAG } from "@posthog/shared"; +import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; import { HOME_TAB_FLAG } from "@posthog/shared/constants"; import { useCommandCenterStore } from "@posthog/ui/features/command-center/commandCenterStore"; import { useFeatureFlag } from "@posthog/ui/features/feature-flags/useFeatureFlag"; @@ -6,6 +10,7 @@ import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; import { useSpendAnalysisEnabled } from "@posthog/ui/features/usage/useSpendAnalysisEnabled"; import { + navigateToActivity, navigateToAgents, navigateToCommandCenter, navigateToHome, @@ -20,9 +25,11 @@ import { } from "@posthog/ui/router/navigationBridge"; import { useAppView } from "@posthog/ui/router/useAppView"; import { openTaskInput } from "@posthog/ui/router/useOpenTask"; +import { track } from "@posthog/ui/shell/analytics"; import { useCommandMenuStore } from "@posthog/ui/shell/commandMenuStore"; import { Box, Flex } from "@radix-ui/themes"; import { useRouterState } from "@tanstack/react-router"; +import { ActivityItem } from "./items/ActivityItem"; import { AgentsItem } from "./items/AgentsItem"; import { CommandCenterItem } from "./items/CommandCenterItem"; import { HomeItem } from "./items/HomeItem"; @@ -57,6 +64,15 @@ export function SidebarNavSection({ const view = useAppView(); const homeTabEnabled = useFeatureFlag(HOME_TAB_FLAG); const usageEnabled = useSpendAnalysisEnabled(); + // Channels stay behind project-bluebird: the "Enable channels" nav row (and + // the Canvas row it reveals) only appear where the canvas backend is wired. + const bluebirdEnabled = useFeatureFlag( + PROJECT_BLUEBIRD_FLAG, + import.meta.env.DEV, + ); + const channelsEnabled = + useSidebarStore((s) => s.channelsEnabled) && bluebirdEnabled; + const setChannelsEnabled = useSidebarStore((s) => s.setChannelsEnabled); // When this section renders inside the Channels space, the destinations that // have a /website mirror stay in that space; everything else (and the whole @@ -81,6 +97,7 @@ export function SidebarNavSection({ const isHomeActive = view.type === "task-input" || view.type === "task-pending"; const isHomeViewActive = view.type === "home"; + const isActivityActive = view.type === "activity"; const isInboxActive = view.type === "inbox"; const isAgentsActive = view.type === "agents"; const isCommandCenterActive = view.type === "command-center"; @@ -133,6 +150,18 @@ export function SidebarNavSection({ )} + {/* Activity (the mentions feed) is a /website surface, so it only appears + where the canvas backend is wired — same gate as the Channels toggle + below. */} + {bluebirdEnabled && ( + + + + )} + @@ -157,7 +186,7 @@ export function SidebarNavSection({ - + )} + + {/* "Channels" is a toggle laid out as a nav row: the # label and Alpha + badge on the left, a Switch on the right. It flips the channels + feature rather than routing — enabling it reveals the Canvas row + below and swaps the sidebar body to the channel tree. A