diff --git a/packages/core/src/browser-tabs/browserTabsStore.ts b/packages/core/src/browser-tabs/browserTabsStore.ts index c2985cec3a..a3b3a5ba94 100644 --- a/packages/core/src/browser-tabs/browserTabsStore.ts +++ b/packages/core/src/browser-tabs/browserTabsStore.ts @@ -11,9 +11,25 @@ interface BrowserTabsState { setSnapshot: (snapshot: TabsSnapshot) => void; } +// True when two snapshots carry the same value. Reference check first (the +// common no-op), then a structural compare. Stringify can only yield a false +// *mismatch* (e.g. key-order drift), never a false match — so it can bail on a +// redundant write but never skip a real change, which would strand a stale +// mirror. +function snapshotsEqual(a: TabsSnapshot, b: TabsSnapshot): boolean { + return a === b || JSON.stringify(a) === JSON.stringify(b); +} + export const browserTabsStore = createStore((set) => ({ snapshot: { windows: [], tabs: [] }, - setSnapshot: (snapshot) => set({ snapshot }), + // Skip redundant writes: every tab mutation's onSuccess re-applies the + // authoritative snapshot even when an optimistic write already set an equal + // value. Without this guard that echo re-renders every subscriber and re-runs + // the navigation effect (which depends on this snapshot) for no change. + setSnapshot: (snapshot) => + set((prev) => + snapshotsEqual(prev.snapshot, snapshot) ? prev : { snapshot }, + ), })); export const getTabsSnapshot = () => browserTabsStore.getState().snapshot; 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..955f63a8a7 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, }); }); @@ -398,6 +404,157 @@ describe("decideTabNavigation", () => { }); }); +describe("decideTabNavigation: dedup against existing tabs (windowTabs)", () => { + const identity = { + dashboardId: null, + taskId: null, + channelId: null, + channelSection: null, + appView: null, + }; + + it("activates the existing tab instead of replacing the active tab's target (would-be duplicate)", () => { + // A rapid switch whose history stamp was lost arrives looking like an in-tab + // nav: active tab A, but the route identifies tab B (already open). Without + // the dedup this replaces A's target → two tabs on c2/artifacts. With it, B + // is focused. + expect( + decideTabNavigation({ + historyTabId: "tab-a", + serverActiveTabId: "tab-a", + windowTabs: [ + { id: "tab-a", ...identity, channelId: "c1" }, + { + id: "tab-b", + ...identity, + channelId: "c2", + channelSection: "artifacts", + }, + ], + activeTab: { + id: "tab-a", + dashboardId: null, + taskId: null, + channelId: "c1", + }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: "c2", + routeChannelSection: "artifacts", + }), + ).toEqual({ type: "activate", tabId: "tab-b" }); + }); + + it("activates an existing matching tab instead of opening a duplicate (no active tab)", () => { + expect( + decideTabNavigation({ + historyTabId: null, + serverActiveTabId: null, + windowTabs: [{ id: "tab-b", ...identity, channelId: "c2" }], + activeTab: null, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: "c2", + }), + ).toEqual({ type: "activate", tabId: "tab-b" }); + }); + + it("does NOT jump when the active tab already shows the route, even if a duplicate exists", () => { + // Two tabs share an identity (a pre-existing duplicate). The active one + // already shows the route → stamp/noop. Jumping to the other duplicate + // would ping-pong between them forever (Maximum update depth exceeded). + expect( + decideTabNavigation({ + historyTabId: "tab-x", + serverActiveTabId: "tab-x", + windowTabs: [ + { + id: "tab-x", + ...identity, + channelId: "c1", + channelSection: "artifacts", + }, + { + id: "tab-y", + ...identity, + channelId: "c1", + channelSection: "artifacts", + }, + ], + activeTab: { + id: "tab-x", + dashboardId: null, + taskId: null, + channelId: "c1", + channelSection: "artifacts", + }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: "c1", + routeChannelSection: "artifacts", + }), + ).toEqual({ type: "stamp", stampTabId: "tab-x" }); + }); + + it("fills a blank active tab (fresh + tab) even when the route is open elsewhere", () => { + // Cmd+T lands the new blank tab on #me. If a #me tab is already open, the + // dedup must NOT steal the navigation to it — that would strand the blank + // tab forever. The blank active tab means "fill me". + expect( + decideTabNavigation({ + historyTabId: "tab-blank", + serverActiveTabId: "tab-blank", + windowTabs: [ + { id: "tab-blank", ...identity }, + { id: "tab-me", ...identity, channelId: "me-ch" }, + ], + activeTab: { id: "tab-blank", dashboardId: null, taskId: null }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: "me-ch", + }), + ).toEqual({ + type: "replace", + tabId: "tab-blank", + dashboardId: null, + taskId: null, + channelId: "me-ch", + channelSection: null, + appView: null, + stampTabId: "tab-blank", + }); + }); + + it("still replaces for a genuine in-tab nav to a target no other tab holds", () => { + expect( + decideTabNavigation({ + historyTabId: "tab-a", + serverActiveTabId: "tab-a", + windowTabs: [{ id: "tab-a", ...identity, channelId: "c1" }], + activeTab: { + id: "tab-a", + dashboardId: null, + taskId: null, + channelId: "c1", + }, + routeDashboardId: null, + routeTaskId: null, + routeChannelId: "c1", + routeChannelSection: "artifacts", + }), + ).toEqual({ + type: "replace", + tabId: "tab-a", + dashboardId: null, + taskId: null, + channelId: "c1", + channelSection: "artifacts", + appView: null, + stampTabId: "tab-a", + }); + }); +}); + function openChannel(s: TabsSnapshot, windowId: string, channelId: string) { return openOrFocusTab(s, { windowId, @@ -601,3 +758,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..4d2b0d61b9 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,24 @@ 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 tabs with their identities. When a navigation's route matches + * an existing tab that isn't the active one, we activate that tab instead of + * replacing the active tab's target (which would duplicate it) or opening a + * second copy. This also self-heals a rapid tab switch whose history stamp + * was lost: it arrives looking like an in-tab nav, but the route still + * identifies the intended tab, so we focus it rather than corrupt the active + * tab. When omitted, this dedup is skipped (legacy behaviour). + */ + windowTabs?: readonly (TabIdentity & { id: 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 +477,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 +486,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 +498,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,23 +522,51 @@ export function decideTabNavigation(input: { taskId: routeTaskId, channelId: routeChannelId, channelSection: routeChannelSection, + appView: routeAppView, }; - if (!routeDashboardId && !routeTaskId && !routeChannelId) { + if (!routeDashboardId && !routeTaskId && !routeChannelId && !routeAppView) { return { type: "noop" }; } - if ( - activeTab && - !sameIdentity( + const activeMatchesRoute = + !!activeTab && + sameIdentity( { dashboardId: activeTab.dashboardId, taskId: activeTab.taskId, channelId: activeTab.channelId ?? null, channelSection: activeTab.channelSection ?? null, + appView: activeTab.appView ?? null, }, routeIdentity, - ) - ) { + ); + + // A blank active tab is a fresh `+` tab waiting for its first target: the + // navigation is "fill me", never a switch — so the dedup below must not + // steal it (activating another tab would strand the blank forever). + const activeIsBlank = + !!activeTab && + activeTab.dashboardId == null && + activeTab.taskId == null && + (activeTab.channelId ?? null) == null && + (activeTab.appView ?? null) == null; + + // The route already lives in another tab → focus it instead of replacing the + // active tab's target (which would leave two tabs on the same identity) or + // opening a duplicate. Also recovers a rapid switch whose history tag was + // lost: the intended tab is still identified by the route. Only when the + // active tab does NOT already show the route — otherwise, if a duplicate tab + // already exists, we'd bounce between the two identical tabs forever. + if (!activeMatchesRoute && !activeIsBlank) { + const existingMatch = input.windowTabs?.find( + (t) => t.id !== activeTab?.id && sameIdentity(t, routeIdentity), + ); + if (existingMatch) { + return { type: "activate", tabId: existingMatch.id }; + } + } + + if (activeTab && !activeMatchesRoute) { return { type: "replace", tabId: activeTab.id, @@ -476,6 +574,7 @@ export function decideTabNavigation(input: { taskId: routeTaskId, channelId: routeChannelId, channelSection: routeChannelSection, + appView: routeAppView, stampTabId: serverActiveTabId, }; } @@ -486,6 +585,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/AGENTS.md b/packages/ui/src/features/browser-tabs/AGENTS.md index be8706feb2..46e3681401 100644 --- a/packages/ui/src/features/browser-tabs/AGENTS.md +++ b/packages/ui/src/features/browser-tabs/AGENTS.md @@ -50,8 +50,17 @@ The feature is deliberately split so the rules are portable and testable: - **this folder (`@posthog/ui`)** — `BrowserTabStrip` (container; mounted in the Channels title bar in `router/routes/__root.tsx`), `TabStrip` (presentational), `BlankTabView` (the new-tab placeholder), `TaskTabIcon` (sidebar-parity status - icon for task tabs), the client facade, and the boot contribution that seeds + - subscribes the store. + icon for task tabs), the client facade, the boot contribution that seeds + + subscribes the store, and **`tabsSync.ts` — the local-first sync policy**: + every operation applies its shared pure transform to the renderer mirror + synchronously (interactions are instant; new tabs mint their id client-side + so no navigation ever waits on IPC), server writes are background persistence, + and while any write is in flight remote snapshot pushes are dropped because + they may predate newer local state. If a push was dropped, the renderer + re-fetches the authoritative snapshot after the write batch settles so a real + mutation from another window is retained; otherwise the last settling write + applies its returned snapshot. This makes rapid tab switching race-free + without losing cross-window updates. One source of truth: any window mutates → service writes sqlite + emits → every window's store updates. No window talks to another directly. The same shape ports @@ -192,11 +201,14 @@ differ. Desktop ships first. index for a couple of frames **after** the URL has already left `/website` (the `__root` Outlet un-suppresses on the way to `/website/$channelId` before the matched leaf settles), and that stale render must not redirect. -- **Closing the last tab writes the snapshot synchronously.** `handleClose` - calls `browserTabsStore.setSnapshot(next)` before navigating to `/website`. - The store otherwise lags a subscription round-trip, so the index would render - against the still-has-tabs snapshot and redirect (re-opening a tab) before the - empty strip arrives. +- **All writes are local-first (`tabsSync.ts`).** Close/open/new/reorder apply + their shared transform to the mirror and navigate in the same tick; the + `/website` index therefore always renders against post-mutation state and + can't redirect (re-opening a tab) mid-flight. Mutation results and + subscription pushes are never applied while writes are in flight — only the + last settle reconciles. Don't add a mutation `onSuccess` that calls + `setSnapshot`; route new writes through `applyLocalTransform` + + `persistWrite`. ## Testing diff --git a/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx b/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx index e42714f57f..072256de4a 100644 --- a/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx +++ b/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx @@ -1,20 +1,46 @@ -import { HashIcon } from "@phosphor-icons/react"; -import { browserTabsStore } from "@posthog/core/browser-tabs/browserTabsStore"; +import { + BrainIcon, + HashIcon, + HouseIcon, + PlugsConnectedIcon, + RobotIcon, + SquaresFourIcon, + TrayIcon, +} from "@phosphor-icons/react"; import { useHostTRPC } from "@posthog/host-router/react"; import { + closeTab as closeTabLocal, + closeTabs as closeTabsLocal, decideTabNavigation, + newBlankTab as newBlankTabLocal, + openOrFocusTab as openOrFocusLocal, + PROJECT_BLUEBIRD_FLAG, + primaryWindow, setTabOrder, + setTabTarget as setTabTargetLocal, + setWindowActiveTab, type TabsSnapshot, } from "@posthog/shared"; import { channelSectionFor } from "@posthog/ui/features/canvas/channelSections"; import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; -import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; +import { + type Channel, + useChannelMutations, + useChannels, +} from "@posthog/ui/features/canvas/hooks/useChannels"; import { useDashboard, useDashboards, } from "@posthog/ui/features/canvas/hooks/useDashboards"; +import { PERSONAL_CHANNEL_NAME } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; +import { SHORTCUTS } from "@posthog/ui/features/command/keyboard-shortcuts"; +import { useFeatureFlag } from "@posthog/ui/features/feature-flags/useFeatureFlag"; +import { usePanelLayoutStore } from "@posthog/ui/features/panels/panelLayoutStore"; +import { getLeafPanel } from "@posthog/ui/features/panels/panelStoreHelpers"; +import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; 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 +48,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, @@ -32,6 +59,7 @@ import { usePinnedTabsStore } from "./pinnedTabsStore"; import { TabStrip, type TabView } from "./TabStrip"; import { TaskTabIcon } from "./TaskTabIcon"; import { useTabReorderStore } from "./tabReorderStore"; +import { applyLocalTransform, persistWrite, readMirror } from "./tabsSync"; import { useTabsSnapshot } from "./useBrowserTabs"; /** The active tab id is carried in router history state so back/forward replay @@ -51,6 +79,13 @@ declare module "@tanstack/history" { const canvasInfo = new Map(); const taskInfo = new Map(); +// Dedupe concurrent #me provisioning. The folder-creation endpoint isn't +// server-side idempotent, and the new-tab path is on Cmd+T (trivially +// double-fired/held) — so two landings racing before the first create's cache +// update lands could each create a "me" folder. One in-flight create is shared +// across callers until it settles. +let personalChannelInFlight: Promise | null = null; + /** Bounded insert (most-recent kept) so the caches don't grow unbounded over a * long session. */ const MAX_CACHE_ENTRIES = 200; @@ -63,8 +98,19 @@ function remember(map: Map, key: string, value: V): void { } } -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 = { @@ -73,8 +119,39 @@ type TabRef = { 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,8 +166,28 @@ 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(); + const { createChannel } = useChannelMutations(); + // Whether the channels surface is live — the same gate the sidebar uses. This + // (not the current route) decides a new tab's default: with channels on a + // fresh tab opens #me, otherwise the Code new-task screen. Keying off the + // route would leave the behaviour stale right after the toggle flips. + const bluebirdEnabled = useFeatureFlag( + PROJECT_BLUEBIRD_FLAG, + import.meta.env.DEV, + ); + const channelsEnabled = + useSidebarStore((s) => s.channelsEnabled) && bluebirdEnabled; // The active channel sub-section (artifacts/history/context) is the // route segment after the channelId. Null when on the channel home or a @@ -101,6 +198,12 @@ export function BrowserTabStrip() { return channelSectionFor(seg)?.key ?? null; }, [pathname, params.channelId]); + // Local-first sync (see tabsSync.ts): every operation applies its shared + // pure transform to the mirror synchronously via applyLocalTransform, then + // persists in the background via persistWrite. The mutations below are pure + // transport — their returned snapshots are handled by persistWrite's + // last-settle reconcile, never applied directly, so a stale echo can't + // rewind the mirror mid-interaction. const openOrFocus = useMutation( trpc.browserTabs.openOrFocus.mutationOptions(), ); @@ -132,13 +235,16 @@ export function BrowserTabStrip() { const win = primaryWindow(snapshot); const windowId = win?.id; - const activeTab = win?.activeTabId - ? snapshot.tabs.find((t) => t.id === win.activeTabId) - : undefined; // The history state flips the instant you navigate, while the server snapshot // round-trips — so prefer it for "which tab is active" to avoid a one-step lag - // in the highlight and the name. - const activeTabId = historyTabId ?? win?.activeTabId ?? null; + // in the highlight and the name. Validate it against the live tab list first: + // back/forward can replay an entry tagged with a since-closed tab, and a dead + // id here would blank the strip highlight and point Cmd+W at a tab that no + // longer exists (the navigation effect heals the tag, but asynchronously). + const historyTabIsLive = + !!historyTabId && snapshot.tabs.some((t) => t.id === historyTabId); + const activeTabId = + (historyTabIsLive ? historyTabId : null) ?? win?.activeTabId ?? null; // Names feed the tab labels. The channel canvas list + all-tasks list cover // most tabs; a direct fetch of the *current route's* canvas/task (warm cache @@ -176,71 +282,150 @@ export function BrowserTabStrip() { // decideTabNavigation) and apply it: focus a tab, replace the active tab's // target in place, open a tab, and/or stamp the history entry with the tab it // belongs to so back/forward can replay it. + // + // Keyed on the LOCATION only — the route is the command stream; the mirror is + // state this effect reconciles against, read fresh via readMirror() rather + // than subscribed to. Running on mirror changes is actively wrong under + // local-first sync: a handler moves the mirror BEFORE it navigates (e.g. the + // + tab appends and focuses a blank tab), and an effect run in that gap sees + // the OLD location's tag disagree with the new mirror focus and "activates" + // the stale tab — yanking focus back and mis-targeting the follow-up + // navigation as an in-tab replace of the wrong tab. useEffect(() => { if (!windowId) return; const stamp = (tabId: string) => { const loc = router.history.location; + // Already tagged — skip the replace so history entries and router + // subscribers don't churn. + 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 mirror = readMirror(); + const mirrorWin = primaryWindow(mirror); + const mirrorTabs = mirror.tabs.filter((t) => t.windowId === windowId); + const mirrorActive = mirrorWin?.activeTabId + ? mirrorTabs.find((t) => t.id === mirrorWin.activeTabId) + : undefined; const decision = decideTabNavigation({ historyTabId: historyTabId ?? null, - serverActiveTabId: win?.activeTabId ?? null, - activeTab: activeTab + // 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: mirrorTabs.map((t) => t.id), + // Identities of this window's tabs, so a navigation to a target already + // open in another tab focuses it instead of duplicating it (and a rapid + // switch whose history stamp was lost self-heals to the right tab). + windowTabs: mirrorTabs.map((t) => ({ + id: t.id, + dashboardId: t.dashboardId, + taskId: t.taskId, + channelId: t.channelId, + channelSection: t.channelSection, + appView: t.appView, + })), + serverActiveTabId: mirrorWin?.activeTabId ?? null, + activeTab: mirrorActive ? { - id: activeTab.id, - dashboardId: activeTab.dashboardId, - taskId: activeTab.taskId, - channelId: activeTab.channelId, - channelSection: activeTab.channelSection, + id: mirrorActive.id, + dashboardId: mirrorActive.dashboardId, + taskId: mirrorActive.taskId, + channelId: mirrorActive.channelId, + channelSection: mirrorActive.channelSection, + appView: mirrorActive.appView, } : null, routeDashboardId: params.dashboardId ?? null, routeTaskId: params.taskId ?? null, routeChannelId: params.channelId ?? null, routeChannelSection, + routeAppView, }); switch (decision.type) { - case "activate": - setActiveTab.mutate({ windowId, tabId: decision.tabId }); + case "activate": { + // Focus in the mirror synchronously; persist in the background. + applyLocalTransform((s) => + setWindowActiveTab(s, windowId, decision.tabId), + ); + void persistWrite(() => + setActiveTab.mutateAsync({ windowId, tabId: decision.tabId }), + ); + // Heal the history tag to the tab we're activating. Normally it already + // matches (a tagged switch), so `stamp` no-ops. When the dedup path + // activated an existing tab the route pointed at (a switch whose stamp + // was lost), the entry still carries the STALE tab — left unhealed, the + // first branch above would re-activate it next render and ping-pong with + // the dedup (a "Maximum update depth exceeded" loop). Stamping breaks it. + stamp(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, + }; + // Synchronous local apply keeps re-entrant runs (and the /website index + // redirect guard) from ever seeing the pre-navigation target. + applyLocalTransform((s) => + setTabTargetLocal(s, { ...target, now: Date.now }), + ); + void persistWrite(() => setTabTarget.mutateAsync(target)); if (decision.stampTabId) stamp(decision.stampTabId); break; - case "open": - openOrFocus.mutate({ + } + case "open": { + const input = { windowId, dashboardId: decision.dashboardId, taskId: decision.taskId, channelId: decision.channelId, channelSection: decision.channelSection, + appView: decision.appView, + }; + // Mint the id here so the local apply and the persisted state agree on + // it; openOrFocusLocal may instead dedup-focus an existing tab, in + // which case the minted id goes unused (identically on the server). + const mintedId = crypto.randomUUID(); + let openedTabId: string = mintedId; + applyLocalTransform((s) => { + const result = openOrFocusLocal(s, { + ...input, + makeId: () => mintedId, + now: Date.now, + }); + openedTabId = result.tabId; + return result.snapshot; }); - if (decision.stampTabId) stamp(decision.stampTabId); + void persistWrite(() => + openOrFocus.mutateAsync({ ...input, tabId: mintedId }), + ); + // Stamp the entry with the tab that now owns this route. + stamp(openedTabId); break; + } case "stamp": stamp(decision.stampTabId); break; } }, [ + // windowId flips once when the boot seed lands — that run adopts the + // initial route. Everything else here is location; mirror state is read + // fresh inside, deliberately NOT a dependency (see the comment above). windowId, historyTabId, - win?.activeTabId, params.channelId, params.dashboardId, params.taskId, routeChannelSection, - activeTab, - openOrFocus.mutate, - setTabTarget.mutate, - setActiveTab.mutate, + routeAppView, + openOrFocus.mutateAsync, + setTabTarget.mutateAsync, + setActiveTab.mutateAsync, router, ]); @@ -295,6 +480,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 +519,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 +546,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 +560,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 +587,40 @@ 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; + default: { + // Exhaustiveness guard: a new AppView value fails to compile here + // until its canonical route is wired above — so the tab-target set + // (union + APP_VIEW_META) and this navigation can't drift apart. + const _exhaustive: never = tab.appView; + return _exhaustive; + } + } } 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,22 +632,25 @@ 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, or — when the last tab was closed — to the + // flag's default landing (#me / new-task), never the /website index (which + // would redirect to channels[0], re-opening a random channel tab). 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 landOnDefault(); }; + // Close applies locally and navigates to the survivor in the same tick — the + // /website index therefore always renders against the post-close snapshot + // and can't redirect (re-opening a tab) mid-flight. const handleClose = (tabId: string) => { - close.mutate({ tabId }, { onSuccess: applyCloseResult }); + const next = applyLocalTransform((s) => closeTabLocal(s, tabId).snapshot); + applyCloseResult(next); + void persistWrite(() => close.mutateAsync({ tabId })); }; // Unpinning re-homes the tab at the front of the unpinned block. Apply the @@ -422,13 +661,8 @@ export function BrowserTabStrip() { togglePinned(tabId); if (!wasPinned || !windowId) return; const order = frontOfUnpinnedOrder(snapshot, windowId, tabId, pinnedTabIds); - browserTabsStore - .getState() - .setSnapshot(setTabOrder(snapshot, windowId, order)); - setOrder.mutate( - { windowId, tabIds: order }, - { onSuccess: (next) => browserTabsStore.getState().setSnapshot(next) }, - ); + applyLocalTransform((s) => setTabOrder(s, windowId, order)); + void persistWrite(() => setOrder.mutateAsync({ windowId, tabIds: order })); }; // Bulk closes operate on the strip's *displayed* order (pinned-first) and @@ -436,9 +670,12 @@ export function BrowserTabStrip() { // always survives) takes focus if the active tab was among those closed. const handleCloseMany = (tabIds: string[], anchorTabId: string) => { if (tabIds.length === 0) return; - closeMany.mutate( - { tabIds, focusTabId: anchorTabId }, - { onSuccess: applyCloseResult }, + const next = applyLocalTransform((s) => + closeTabsLocal(s, tabIds, anchorTabId), + ); + applyCloseResult(next); + void persistWrite(() => + closeMany.mutateAsync({ tabIds, focusTabId: anchorTabId }), ); }; @@ -473,6 +710,105 @@ export function BrowserTabStrip() { ); }; + // The default landing, keyed off the channels toggle (not the current route, + // which lags a toggle flip): #me when channels are on, the Code new-task + // screen otherwise. Deliberately never routes through the /website index, + // which would redirect to channels[0]. `tabId` (a fresh blank tab) fills that + // tab in place; without one (last tab closed) the navigation opens a new tab. + const landOnDefault = (tabId?: string) => { + const state = tabId ? (prev: object) => ({ ...prev, tabId }) : undefined; + if (!channelsEnabled) { + navigate({ to: "/code", state }); + return; + } + // #me is provisioned lazily the first time (same bridge the sidebar's #me + // row uses); fall back to the new-task screen if it can't be created. + void (async () => { + try { + const existing = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); + if (!existing && !personalChannelInFlight) { + personalChannelInFlight = createChannel( + PERSONAL_CHANNEL_NAME, + ).finally(() => { + personalChannelInFlight = null; + }); + } + const folder = existing ?? (await personalChannelInFlight); + if (!folder) return; + navigate({ + to: "/website/$channelId", + params: { channelId: folder.id }, + state, + }); + } catch { + navigate({ to: "/code", state }); + } + })(); + }; + + // New tab is fully local: mint the id here, append the blank tab to the + // mirror and navigate in the same tick (no IPC wait), then persist with the + // same id so the durable state matches. The service is idempotent on the + // minted id, so a replay can't append a duplicate. + const handleNewTab = () => { + if (!windowId) return; + const tabId = crypto.randomUUID(); + applyLocalTransform( + (s) => + newBlankTabLocal(s, { windowId, makeId: () => tabId, now: Date.now }) + .snapshot, + ); + landOnDefault(tabId); + void persistWrite(() => newBlankTab.mutateAsync({ windowId, tabId })); + }; + + // 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 }, + ); + + // With channels on, Cmd/Ctrl+1-9 switches to the Nth browser tab (in the + // displayed, pinned-first order) instead of the Nth sidebar task. The global + // task-switch handler yields via the same channelsEnabled gate, so exactly one + // owner fires. Mirror its pure-ctrl guard: ctrl+1-9 is the editor-panel tab + // switcher (SWITCH_TAB), so leave ctrl-only presses to it. + useHotkeys( + SHORTCUTS.SWITCH_TASK, + (event, handler) => { + if (event.ctrlKey && !event.metaKey) return; + const key = handler.keys?.[0]; + if (!key) return; + const tab = tabs[Number.parseInt(key, 10) - 1]; + if (tab) handleSelect(tab.id); + }, + { + enableOnFormTags: true, + enableOnContentEditable: true, + preventDefault: true, + enabled: channelsEnabled, + }, + [tabs, handleSelect], + ); + 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/BrowserTabsDnd.tsx b/packages/ui/src/features/browser-tabs/BrowserTabsDnd.tsx index 4d2e202741..810523c7c6 100644 --- a/packages/ui/src/features/browser-tabs/BrowserTabsDnd.tsx +++ b/packages/ui/src/features/browser-tabs/BrowserTabsDnd.tsx @@ -7,6 +7,7 @@ import { type ReactNode, useRef } from "react"; import { reorderWithinGroup, storedOrderIds } from "./displayOrder"; import { usePinnedTabsStore } from "./pinnedTabsStore"; import { useTabReorderStore } from "./tabReorderStore"; +import { applyLocalTransform, persistWrite } from "./tabsSync"; function sameOrder(a: string[], b: string[]): boolean { return a.length === b.length && a.every((id, i) => id === b[i]); @@ -86,14 +87,12 @@ export function BrowserTabsDndProvider({ children }: { children: ReactNode }) { const snapshot = browserTabsStore.getState().snapshot; const win = primaryWindow(snapshot); if (!win) return; - // Optimistically apply so the strip doesn't flit back to the mirror's - // pre-drop order for a frame; then persist to the host. - browserTabsStore - .getState() - .setSnapshot(setTabOrder(snapshot, win.id, order)); - setOrder.mutate( - { windowId: win.id, tabIds: order }, - { onSuccess: (next) => browserTabsStore.getState().setSnapshot(next) }, + // Apply locally so the strip doesn't flit back to the mirror's pre-drop + // order for a frame; persist through the tabsSync gate so the echo can't + // rewind a newer write. + applyLocalTransform((s) => setTabOrder(s, win.id, order)); + void persistWrite(() => + setOrder.mutateAsync({ windowId: win.id, tabIds: order }), ); }); }; diff --git a/packages/ui/src/features/browser-tabs/browser-tabs-events.contribution.ts b/packages/ui/src/features/browser-tabs/browser-tabs-events.contribution.ts index 3fc460e9d3..42c4cf2b59 100644 --- a/packages/ui/src/features/browser-tabs/browser-tabs-events.contribution.ts +++ b/packages/ui/src/features/browser-tabs/browser-tabs-events.contribution.ts @@ -1,14 +1,17 @@ -import { browserTabsStore } from "@posthog/core/browser-tabs/browserTabsStore"; import type { Contribution } from "@posthog/di/contribution"; import { inject, injectable } from "inversify"; import { BROWSER_TABS_CLIENT, type BrowserTabsClient, } from "./browserTabsClient"; +import { applyRemoteSnapshot, registerSnapshotFetcher } from "./tabsSync"; /** * Seeds the renderer tab snapshot at startup and keeps it live via the * snapshot-change subscription, so a mutation in any window is reflected here. + * Applied through the tabsSync gate: pushes are dropped while this window has + * writes in flight, so an echo of our own mutation can't rewind newer local + * state (see tabsSync.ts). */ @injectable() export class BrowserTabsEventsContribution implements Contribution { @@ -20,21 +23,24 @@ export class BrowserTabsEventsContribution implements Contribution { ) {} start(): void { - const { setSnapshot } = browserTabsStore.getState(); + // Lets tabsSync re-pull the authoritative snapshot after a FAILED write + // (a failed mutation emits no snapshotChange, so nothing else reconciles). + registerSnapshotFetcher(() => this.client.getSnapshot()); void this.client .getSnapshot() - .then((snapshot) => setSnapshot(snapshot)) + .then((snapshot) => applyRemoteSnapshot(snapshot)) .catch(() => undefined); // Replace any prior handle so a repeated start() can't leak a subscription. this.subscription?.unsubscribe(); this.subscription = this.client.onSnapshotChange({ - onData: (snapshot) => setSnapshot(snapshot), + onData: (snapshot) => applyRemoteSnapshot(snapshot), }); } stop(): void { + registerSnapshotFetcher(null); this.subscription?.unsubscribe(); this.subscription = null; } diff --git a/packages/ui/src/features/browser-tabs/browserTabsClient.ts b/packages/ui/src/features/browser-tabs/browserTabsClient.ts index cc6c9f964f..eaf4df383d 100644 --- a/packages/ui/src/features/browser-tabs/browserTabsClient.ts +++ b/packages/ui/src/features/browser-tabs/browserTabsClient.ts @@ -20,14 +20,22 @@ export interface BrowserTabsClient { taskId: string | null; channelId: string | null; channelSection?: string | null; + appView?: string | null; + /** Renderer-minted id for a tab this call may create (local-first sync). */ + tabId?: string; + }): Promise; + newBlankTab(input: { + windowId: string; + /** Renderer-minted id (see openOrFocus.tabId). */ + tabId?: string; }): Promise; - newBlankTab(input: { windowId: string }): Promise; setTabTarget(input: { tabId: string; dashboardId: string | null; 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/browser-tabs/tabsSync.test.ts b/packages/ui/src/features/browser-tabs/tabsSync.test.ts new file mode 100644 index 0000000000..a5a90a96f7 --- /dev/null +++ b/packages/ui/src/features/browser-tabs/tabsSync.test.ts @@ -0,0 +1,192 @@ +import { browserTabsStore } from "@posthog/core/browser-tabs/browserTabsStore"; +import type { TabsSnapshot } from "@posthog/shared"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + applyLocalTransform, + applyRemoteSnapshot, + persistWrite, + registerSnapshotFetcher, +} from "./tabsSync"; + +function snap(tabIds: string[]): TabsSnapshot { + return { + windows: [ + { + id: "w1", + isPrimary: true, + bounds: null, + activeTabId: tabIds[0] ?? null, + }, + ], + tabs: tabIds.map((id, i) => ({ + id, + windowId: "w1", + dashboardId: null, + taskId: null, + channelId: `c-${id}`, + channelSection: null, + appView: null, + position: (i + 1) * 1000, + createdAt: i, + lastActiveAt: i, + })), + }; +} + +const current = () => browserTabsStore.getState().snapshot; + +// Resolvable promise helper so tests control settle order. +function deferred() { + let resolve!: (v: T) => void; + let reject!: (e: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe("tabsSync", () => { + beforeEach(() => { + browserTabsStore.getState().setSnapshot(snap(["a"])); + registerSnapshotFetcher(null); + }); + + it("applyLocalTransform writes the mirror synchronously", () => { + applyLocalTransform(() => snap(["a", "b"])); + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + }); + + it("drops remote snapshots while a write is in flight (stale echo)", async () => { + const d = deferred(); + const settled = persistWrite(() => d.promise); + registerSnapshotFetcher(async () => snap(["a", "b"])); + + applyLocalTransform(() => snap(["a", "b"])); + // Echo of an older state arrives mid-flight — must not rewind the mirror. + applyRemoteSnapshot(snap(["a"])); + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + + d.resolve(snap(["a", "b"])); + await settled; + await Promise.resolve(); + }); + + it("re-pulls changes from another window after a local write settles", async () => { + const d = deferred(); + const fetched = deferred(); + registerSnapshotFetcher(() => fetched.promise); + const settled = persistWrite(() => d.promise); + + applyLocalTransform(() => snap(["a", "b"])); + applyRemoteSnapshot(snap(["a", "b", "remote"])); + + d.resolve(snap(["a", "b"])); + await settled; + fetched.resolve(snap(["a", "b", "remote"])); + await fetched.promise; + await Promise.resolve(); + + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b", "remote"]); + }); + + it("does not overwrite a newer live push with a reconciliation fetch", async () => { + const write = deferred(); + const fetched = deferred(); + registerSnapshotFetcher(() => fetched.promise); + const settled = persistWrite(() => write.promise); + + applyRemoteSnapshot(snap(["a", "remote"])); + write.resolve(snap(["a"])); + await settled; + + applyRemoteSnapshot(snap(["a", "remote", "newer"])); + fetched.resolve(snap(["a", "remote"])); + await fetched.promise; + await Promise.resolve(); + + expect(current().tabs.map((t) => t.id)).toEqual(["a", "remote", "newer"]); + }); + + it("applies remote snapshots when idle", () => { + applyRemoteSnapshot(snap(["a", "b", "c"])); + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b", "c"]); + }); + + it("only the LAST settling write applies its server snapshot", async () => { + const d1 = deferred(); + const d2 = deferred(); + const p1 = persistWrite(() => d1.promise); + const p2 = persistWrite(() => d2.promise); + applyLocalTransform(() => snap(["a", "b"])); + + // First write settles while the second is still in flight: its (older) + // snapshot must NOT be applied. + d1.resolve(snap(["a"])); + await p1; + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + + // Last write settles: its snapshot is authoritative and applies. + d2.resolve(snap(["a", "b"])); + await p2; + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + }); + + it("keeps the optimistic state when a write fails, and unblocks remote applies", async () => { + const d = deferred(); + const settled = persistWrite(() => d.promise); + applyLocalTransform(() => snap(["a", "b"])); + + d.reject(new Error("ipc down")); + await settled; // must not throw + + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + // Gate reopened: the next remote snapshot reconciles. + applyRemoteSnapshot(snap(["a", "b", "c"])); + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b", "c"]); + }); + + it("re-pulls the authoritative snapshot when the last in-flight write fails", async () => { + // A failed mutation commits nothing server-side and emits no push, so the + // failed-last-write path must reconcile via the registered fetcher. + const fetched = deferred(); + registerSnapshotFetcher(() => fetched.promise); + + const d = deferred(); + const settled = persistWrite(() => d.promise); + applyLocalTransform(() => snap(["a", "b"])); // optimistic, never committed + + d.reject(new Error("ipc down")); + await settled; + + fetched.resolve(snap(["a"])); // server truth: the write never landed + await fetched.promise; + await Promise.resolve(); // let the .then apply + expect(current().tabs.map((t) => t.id)).toEqual(["a"]); + }); + + it("skips the failure re-pull when a newer write is already in flight", async () => { + const fetched = deferred(); + registerSnapshotFetcher(() => fetched.promise); + + const d1 = deferred(); + const d2 = deferred(); + const p1 = persistWrite(() => d1.promise); + + d1.reject(new Error("ipc down")); + await p1; // failure re-pull kicked off (was last in flight) + + // A newer write starts before the re-pull resolves: its settle owns + // reconciliation, so the stale re-pull must not apply. + const p2 = persistWrite(() => d2.promise); + applyLocalTransform(() => snap(["a", "b"])); + fetched.resolve(snap(["a"])); + await fetched.promise; + await Promise.resolve(); + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + + d2.resolve(snap(["a", "b"])); + await p2; + expect(current().tabs.map((t) => t.id)).toEqual(["a", "b"]); + }); +}); diff --git a/packages/ui/src/features/browser-tabs/tabsSync.ts b/packages/ui/src/features/browser-tabs/tabsSync.ts new file mode 100644 index 0000000000..a838fd67b4 --- /dev/null +++ b/packages/ui/src/features/browser-tabs/tabsSync.ts @@ -0,0 +1,120 @@ +import { browserTabsStore } from "@posthog/core/browser-tabs/browserTabsStore"; +import type { TabsSnapshot } from "@posthog/shared"; + +/** + * Local-first sync policy for the browser-tabs mirror. + * + * Every tab operation applies its shared pure transform to the renderer mirror + * synchronously (the UI renders from the mirror, so interactions are instant), + * then persists to the main process in the background. Because the renderer + * and the service run the SAME transforms in the SAME order, the mirror and + * the durable snapshot converge. + * + * The hazard this module exists to prevent: the main process echoes every + * commit back (the mutation's return value and the snapshotChange fan-out). + * Under rapid input, the echo of write N arrives AFTER the local apply of + * write N+1; applying it would rewind the mirror, and the navigation effect + * would re-decide against stale state and misfire persistent writes (the + * historical "tab targets swap / titles flicker" corruption). So while any + * local write is in flight, remote snapshots are dropped. If a push arrives, + * the renderer re-fetches the authoritative snapshot after the write batch + * settles because it may contain another window's mutation. Otherwise the last + * write response reconciles the mirror directly. + */ +let inFlight = 0; +let remoteSnapshotVersion = 0; +let needsAuthoritativeReconcile = false; + +// Authoritative-snapshot fetcher, registered once at boot by the events +// contribution (tabsSync can't reach the injected BrowserTabsClient itself). +// Used to reconcile after a failed write or a dropped remote snapshot. +let fetchAuthoritative: (() => Promise) | null = null; + +export function registerSnapshotFetcher( + fetch: (() => Promise) | null, +): void { + fetchAuthoritative = fetch; +} + +function reconcileAuthoritative(): void { + const versionAtRequest = remoteSnapshotVersion; + void fetchAuthoritative?.() + .then((server) => { + if (inFlight === 0 && remoteSnapshotVersion === versionAtRequest) { + browserTabsStore.getState().setSnapshot(server); + } + }) + .catch(() => undefined); +} + +/** Read the mirror's current snapshot (non-reactive; for event handlers and + * effects that must see the latest state without subscribing to it). */ +export function readMirror(): TabsSnapshot { + return browserTabsStore.getState().snapshot; +} + +/** Synchronously apply a pure transform to the mirror (the optimistic write). */ +export function applyLocalTransform( + transform: (snapshot: TabsSnapshot) => TabsSnapshot, +): TabsSnapshot { + const store = browserTabsStore.getState(); + const next = transform(store.snapshot); + store.setSnapshot(next); + return next; +} + +/** + * Persist a local write to the main process. Fire-and-forget from the caller's + * perspective: the UI has already moved via applyLocalTransform. Only the last + * settling write applies its server snapshot unless a remote push was dropped, + * in which case an authoritative fetch reconciles cross-window state. Failed + * writes are swallowed and reconciled through the same fetch path. + */ +export async function persistWrite( + write: () => Promise, +): Promise { + inFlight++; + let serverSnapshot: TabsSnapshot | null = null; + try { + serverSnapshot = await write(); + } catch { + needsAuthoritativeReconcile = true; + } finally { + inFlight--; + if (inFlight === 0) { + if (needsAuthoritativeReconcile) { + needsAuthoritativeReconcile = false; + reconcileAuthoritative(); + } else if (serverSnapshot) { + // Over Electron IPC the last-settling write is also the last-issued + // write (single FIFO channel, synchronous service handlers). A transport + // that can reorder responses will need a sequence guard here. + browserTabsStore.getState().setSnapshot(serverSnapshot); + } + } + } +} + +/** + * Apply a snapshot pushed from the main process (boot seed, or a mutation made + * by another window). A push received during a local write is dropped and + * replaced by an authoritative fetch when the write batch settles. + */ +export function applyRemoteSnapshot(snapshot: TabsSnapshot): void { + remoteSnapshotVersion++; + if (inFlight > 0) { + // This may be an echo of our own write, or a real mutation from another + // window. Re-pull once the local write batch settles so neither case can + // rewind or strand the mirror. + needsAuthoritativeReconcile = true; + return; + } + browserTabsStore.getState().setSnapshot(snapshot); +} + +// Dev-only inspection handle so the live mirror can be dumped from the console +// (and by agent-browser during dogfooding). No-op in production builds. +if (import.meta.env.DEV) { + (globalThis as { __tabsMirror?: () => TabsSnapshot }).__tabsMirror = () => + browserTabsStore.getState().snapshot; +} 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/canvas/components/FeedbackModal.test.tsx b/packages/ui/src/features/canvas/components/FeedbackModal.test.tsx index 9edc78fc8f..a986d183c5 100644 --- a/packages/ui/src/features/canvas/components/FeedbackModal.test.tsx +++ b/packages/ui/src/features/canvas/components/FeedbackModal.test.tsx @@ -26,7 +26,6 @@ describe("FeedbackModal", () => { }); it.each([ - { mode: "leaving" as const, expected: "Skip", missing: "Cancel" }, { mode: "posthog-web" as const, expected: "Skip", missing: "Cancel" }, { mode: "feedback" as const, expected: "Cancel", missing: "Skip" }, ])( @@ -77,7 +76,7 @@ describe("FeedbackModal", () => { it("finishes without capturing when skipped", async () => { const user = userEvent.setup(); - const onFinished = renderModal("leaving"); + const onFinished = renderModal("posthog-web"); await user.click(screen.getByRole("button", { name: "Skip" })); diff --git a/packages/ui/src/features/canvas/components/FeedbackModal.tsx b/packages/ui/src/features/canvas/components/FeedbackModal.tsx index 143cce1daf..16ddffbee9 100644 --- a/packages/ui/src/features/canvas/components/FeedbackModal.tsx +++ b/packages/ui/src/features/canvas/components/FeedbackModal.tsx @@ -18,7 +18,7 @@ import { import { captureSurveyResponse } from "@posthog/ui/shell/analytics"; import { useState } from "react"; -export type FeedbackModalMode = "feedback" | "leaving" | "posthog-web"; +export type FeedbackModalMode = "feedback" | "posthog-web"; /** Title + prompt shown for each way the modal can be opened. */ const MODAL_COPY: Record = @@ -28,11 +28,6 @@ const MODAL_COPY: Record = prompt: "How's the Channels experience? Tell us what's working and what you'd change.", }, - leaving: { - title: "Before you go back to Code", - prompt: - "How's the Channels experience? Tell us what's working and what you'd change.", - }, "posthog-web": { title: "Before you head to PostHog web", prompt: "Why are you going back to PostHog web?", @@ -40,7 +35,7 @@ const MODAL_COPY: Record = }; export interface FeedbackModalProps { - /** `null` closes the modal. `"feedback"` shows a Cancel button; the navigation-intercept modes (`"leaving"`, `"posthog-web"`) show a Skip button. */ + /** `null` closes the modal. `"feedback"` shows a Cancel button; the navigation-intercept mode (`"posthog-web"`) shows a Skip button. */ mode: FeedbackModalMode | null; /** Called after the response is submitted, and when the modal is skipped/cancelled/dismissed. */ onFinished: () => void; @@ -50,8 +45,8 @@ export interface FeedbackModalProps { * Feedback modal for the Channels space. Submitting records the text as a * PostHog survey response (see {@link FEEDBACK_SURVEY_ID}) along with where the * modal was opened from. The secondary button reads "Skip" when the modal - * intercepts a navigation (`"leaving"` / `"posthog-web"`) and "Cancel" when - * opened directly by "Leave feedback". + * intercepts a navigation (`"posthog-web"`) and "Cancel" when opened directly + * by "Leave feedback". */ export function FeedbackModal({ mode, onFinished }: FeedbackModalProps) { const open = mode !== null; diff --git a/packages/ui/src/features/canvas/components/channelsSidebarStore.ts b/packages/ui/src/features/canvas/components/channelsSidebarStore.ts index 5cd78418e8..c376b91bb1 100644 --- a/packages/ui/src/features/canvas/components/channelsSidebarStore.ts +++ b/packages/ui/src/features/canvas/components/channelsSidebarStore.ts @@ -4,3 +4,23 @@ export const useChannelsSidebarStore = createSidebarStore({ name: "channels-sidebar", defaultWidth: 240, }); + +// One-time migration: the unified layout replaced the Code sidebar (whose +// width persisted under "sidebar-storage") with this store. A user who never +// used the Channels space has no "channels-sidebar" entry yet — adopt their +// old Code width instead of silently resetting them to the default. Once this +// store persists (any set), the migration never runs again. +try { + if ( + typeof window !== "undefined" && + window.localStorage.getItem("channels-sidebar") === null + ) { + const legacy = window.localStorage.getItem("sidebar-storage"); + const width: unknown = legacy ? JSON.parse(legacy)?.state?.width : null; + if (typeof width === "number" && Number.isFinite(width) && width > 0) { + useChannelsSidebarStore.getState().setWidth(width); + } + } +} catch { + // localStorage may be unavailable or hold malformed JSON; the default is fine. +} diff --git a/packages/ui/src/features/canvas/feedbackSurvey.ts b/packages/ui/src/features/canvas/feedbackSurvey.ts index d99c89f08c..77941bf786 100644 --- a/packages/ui/src/features/canvas/feedbackSurvey.ts +++ b/packages/ui/src/features/canvas/feedbackSurvey.ts @@ -19,6 +19,5 @@ export const FEEDBACK_SURVEY_SOURCE_QUESTION_ID = // MUST stay in sync with the survey's single_choice options. export const FEEDBACK_SOURCE_BY_MODE = { feedback: "Generic (Leave feedback button)", - leaving: "Going back to Code", "posthog-web": "Visiting PostHog web", } as const; diff --git a/packages/ui/src/features/canvas/stores/freeformChatStore.ts b/packages/ui/src/features/canvas/stores/freeformChatStore.ts index 3c4cce603a..011196f808 100644 --- a/packages/ui/src/features/canvas/stores/freeformChatStore.ts +++ b/packages/ui/src/features/canvas/stores/freeformChatStore.ts @@ -10,6 +10,14 @@ const log = logger.scope("freeform-edit-store"); // versions fall off past this cap. const MAX_VERSIONS = 50; +// Thread retention: each thread holds up to MAX_VERSIONS whole-document copies, +// and threads for every canvas ever visited would otherwise stay resident for +// the app's lifetime (browser tabs make visiting many canvases cheap). Keep the +// most recently used few; an evicted thread reseeds from the saved record on +// the next visit — the working copy autosaves on every committed edit, so only +// an in-progress version *browse* position is lost. +const MAX_THREADS = 8; + // Working copy of a freeform canvas's source + edit history. Generation no // longer streams in-process — it runs as a dedicated task that publishes the // result into the canvas's saved record (see useGenerateFreeformCanvas). This @@ -45,6 +53,9 @@ export const EMPTY_FREEFORM_THREAD: FreeformThreadState = { interface FreeformChatStore { threads: Record; + /** MRU access order for eviction, oldest first. Store state (not a module + * closure) so devtools/tests see it and HMR can't desync it from `threads`. */ + threadOrder: string[]; /** Seed a thread from a saved record (only if the thread is still empty). * The templateId is recorded regardless so a generation gets the right prompt. */ @@ -90,16 +101,47 @@ function dashboardIdOf(threadId: string): string { } export const useFreeformChatStore = create()((set, get) => { + // Every patch refreshes a thread's recency; eviction runs only from the + // mount-time seeding paths (ensureCode / syncFromRecord) so an edit + // mid-session never drops another thread out from under a mounted view + // racing a save. + const touch = (threadId: string) => { + set((s) => ({ + threadOrder: [...s.threadOrder.filter((id) => id !== threadId), threadId], + })); + }; + + const evictExcessThreads = () => { + // Walk oldest-first, skipping (not aborting on) threads with a save in + // flight — an abort would let one slow autosave at the front block the + // cap for every thread behind it. + let excess = get().threadOrder.length - MAX_THREADS; + for (const oldest of [...get().threadOrder]) { + if (excess <= 0) break; + if (get().threads[oldest]?.isSaving) continue; + excess--; + set((s) => { + const { [oldest]: _evicted, ...rest } = s.threads; + return { + threads: rest, + threadOrder: s.threadOrder.filter((id) => id !== oldest), + }; + }); + } + }; + const patch = ( threadId: string, fn: (prev: FreeformThreadState) => FreeformThreadState, - ) => + ) => { + touch(threadId); set((s) => ({ threads: { ...s.threads, [threadId]: fn(s.threads[threadId] ?? EMPTY_FREEFORM_THREAD), }, })); + }; // Autosave the current code + history to the backend, toggling isSaving so the // toolbar can show a spinner. Never throws. @@ -135,8 +177,11 @@ export const useFreeformChatStore = create()((set, get) => { return { threads: {}, + threadOrder: [], ensureCode: (threadId, record) => { + touch(threadId); + evictExcessThreads(); const cur = get().threads[threadId]; // Once the thread has code, only refresh cheap metadata (templateId / // context) — never clobber the working copy. syncFromRecord handles @@ -158,6 +203,8 @@ export const useFreeformChatStore = create()((set, get) => { }, syncFromRecord: (threadId, record) => { + touch(threadId); + evictExcessThreads(); const cur = get().threads[threadId]; // Empty working copy → just seed. if (!cur || (!cur.code && cur.versions.length === 0)) { @@ -259,6 +306,10 @@ export const useFreeformChatStore = create()((set, get) => { }, revert: (threadId) => { + // Guard against an evicted/never-seeded thread: patch() would otherwise + // materialize EMPTY_FREEFORM_THREAD and persist() would then save + // code:"" over the real record — a data-loss path. + if (!get().threads[threadId]) return; // Adopt the version being viewed: drop everything after it so it becomes // the head, then autosave. patch(threadId, (prev) => { diff --git a/packages/ui/src/features/code-editor/hooks/useFileContent.ts b/packages/ui/src/features/code-editor/hooks/useFileContent.ts index a13018806c..37f598b067 100644 --- a/packages/ui/src/features/code-editor/hooks/useFileContent.ts +++ b/packages/ui/src/features/code-editor/hooks/useFileContent.ts @@ -1,6 +1,12 @@ import { useHostTRPC } from "@posthog/host-router/react"; import { useQuery } from "@tanstack/react-query"; +// File bodies (and especially base64 blobs) are the largest payloads in the +// query cache; the default 5-minute gcTime keeps every file viewed across +// every task resident simultaneously. Drop unobserved entries quickly — a +// re-open re-reads from local disk, which is cheap. +const FILE_CONTENT_GC_MS = 30 * 1000; + export function useRepoFileContent( repoPath: string, filePath: string, @@ -13,6 +19,7 @@ export function useRepoFileContent( { enabled, staleTime: Number.POSITIVE_INFINITY, + gcTime: FILE_CONTENT_GC_MS, }, ), ); @@ -26,6 +33,7 @@ export function useAbsoluteFileContent(filePath: string, enabled: boolean) { { enabled, staleTime: Number.POSITIVE_INFINITY, + gcTime: FILE_CONTENT_GC_MS, }, ), ); @@ -39,6 +47,7 @@ export function useFileAsBase64(filePath: string, enabled: boolean) { { enabled, staleTime: Number.POSITIVE_INFINITY, + gcTime: FILE_CONTENT_GC_MS, }, ), ); 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..2d912da5ba 100644 --- a/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx +++ b/packages/ui/src/features/sidebar/components/ProjectSwitcher.tsx @@ -21,7 +21,6 @@ import { AutocompleteItem, AutocompleteList, AutocompleteStatus, - Button, DropdownMenu, DropdownMenuContent, DropdownMenuGroup, @@ -55,17 +54,11 @@ import { openExternalUrl } from "@posthog/ui/shell/openExternal"; import { isMac } from "@posthog/ui/utils/platform"; import { getPostHogUrl } from "@posthog/ui/utils/urls"; import { Avatar, Box } from "@radix-ui/themes"; -import { ChevronDownIcon, ChevronRightIcon } from "lucide-react"; +import { ChevronRightIcon } from "lucide-react"; import { type ReactNode, useMemo, useState } from "react"; -// `item` (default) is the two-line user/project card used in the Code sidebar. -// `button` is a compact bordered button showing just the project name + a -// chevron, used as the Channels-space header. -export function ProjectSwitcher({ - triggerVariant = "item", -}: { - triggerVariant?: "item" | "button"; -} = {}) { +// The two-line user/project card used at the bottom of the sidebar. +export function ProjectSwitcher() { const [popoverOpen, setPopoverOpen] = useState(false); const currentOrgId = useAuthStateValue((state) => state.currentOrgId); @@ -177,58 +170,37 @@ export function ProjectSwitcher({ return ( - + + + {currentProject?.name ?? "No project selected"} - - - - ) : ( - - - - {currentProject?.name ?? "No project selected"} - - - {currentUser?.email ?? "No email"} - - - - - - - ) + + + {currentUser?.email ?? "No email"} + + + + + + } /> {currentUser ? ( - + {currentUser.first_name && ( @@ -543,10 +515,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..5dd3b59233 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"; @@ -157,7 +174,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