From 4f24bbd10c897205d2a6234f173b913ccbacdf3e Mon Sep 17 00:00:00 2001 From: BlitzOS Upstream Prep Date: Wed, 2 Sep 2026 00:28:34 +0000 Subject: [PATCH] feat(components): let a host contribute tabs to the session tab strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host embedding the session viewer may own surfaces that belong beside the conversation rather than in a second tab strip of their own. This fills a hole that already exists rather than cutting a new one: `SessionTabBar` already carries `viewerTabs`, `activeViewerTabId`, `onViewerTabSelect` and `onViewerTabClose`, the `viewer` arm of `SortableItemData` is implemented, and the one production call site passes `variant="session"` and no viewer tabs at all. `variant="viewer"` is declared and cannot be used, because `parentSession` is required by a strip that variant tells not to draw. `SessionTabBar`: - `ViewerTabItem.type` gains `'custom'` and the item gains an optional `icon`, so the bar draws and sorts a host tab exactly like a file or diff tab without knowing what is behind it. - `parentSession` becomes optional, and the two places that read it are guarded, so `variant="viewer"` is finally usable. `SessionDetail` gains six props: `surfaceTabs`, `activeSurfaceTabId`, `onSurfaceTabSelect`, `onSurfaceTabClose`, `onSessionTabSelect` and `onSessionMissing`. The host owns the list, the selection and both verbs; the page owns the drawing and the layout, and tells the host when its own selection has taken the view back. Two of those need saying: - An active host tab hides every conversation surface, so the host's selection has to end when the page selects a conversation tab — and that selection is `useState`, per parent session, not in the URL. Ten call sites move it, so the notification is a wrapper around the setter rather than a call at each site, and not an effect on the value: an effect fires on a CHANGE, and the click that most needs the call changes nothing, because the parent tab is already selected while a host tab covers it. The three writers that keep the raw setter are corrections rather than selections (the render-time session-switch reset and the two `?tab=` syncs), and their signature difference — an updater, not an id — makes the exclusion structural. - `SessionDetail` returns above the tab strip on the not-found branch, and that return takes every host tab with it. `onSessionMissing` is how the host hears it. The call sits above the once-per-session-id analytics gate deliberately: what a host does with this is move an address, and an address can come back. It is read from a ref so a fresh host closure does not re-run the effect. `content` is a `ReactNode` mounted inline and hidden, not a portal host: React remounts a portal whose container identity changes, so the container swap a ref-callback host was meant to avoid happens anyway. The memo that maps `surfaceTabs` sits beside `viewerTabItems`, above every early return, because this component returns early below it. The mobile drawer is deliberately not changed: `MobileSessionTabSheet` keeps its own kind enum, and the props are inert there. With every new prop absent, both components render exactly what they rendered before, and no existing call site passes one. Model: claude-opus-5[1m] --- .../components/sessions/session-detail.tsx | 163 +++++++++++++++++- .../components/sessions/session-tab-bar.tsx | 29 +++- 2 files changed, 174 insertions(+), 18 deletions(-) diff --git a/packages/components/src/components/sessions/session-detail.tsx b/packages/components/src/components/sessions/session-detail.tsx index cf08bfa3a..a064e32dc 100644 --- a/packages/components/src/components/sessions/session-detail.tsx +++ b/packages/components/src/components/sessions/session-detail.tsx @@ -87,7 +87,16 @@ import { } from '@/components/terminal/terminal-controller'; import { isElectronRenderer } from '@/lib/electron'; import { sidebarCollapsedAtom } from '@/atoms/sidebar-state'; -import { memo, useCallback, useEffect, useMemo, useRef, useState, type CSSProperties } from 'react'; +import { + memo, + useCallback, + useEffect, + useMemo, + useRef, + useState, + type CSSProperties, + type ReactNode, +} from 'react'; import { useDocumentTitle } from '@/hooks/use-document-title'; import { useTabStatus, type TabStatus } from '@/hooks/use-tab-status'; import { @@ -654,6 +663,25 @@ const TerminalDockToggleButton = memo(function TerminalDockToggleButton() { ); }); +/** Stable empty default, so the memos that read `surfaceTabs` do not see a new + * array identity on every render of a page that contributes none. */ +const EMPTY_SURFACE_TABS: readonly SessionSurfaceTab[] = []; + +/** One tab the HOST contributes to this session's tab strip. + * + * A host that embeds this page may own surfaces of its own that belong beside + * the conversation rather than in a second strip. The page draws the tab and + * lays the content out; the host owns the list, the selection and both verbs. */ +export interface SessionSurfaceTab { + /** Unique across this strip. Must not collide with a session id. */ + id: string; + label: string; + icon?: ReactNode; + /** Rendered as a peer of the chat surfaces: mounted always, hidden when + * another tab is active. */ + content: ReactNode; +} + /** * Session detail page component. * Displays the chat interface for a single session. @@ -664,12 +692,47 @@ const SessionDetail = ({ urlPrNumber, urlBrowser, onMobileBack, + surfaceTabs = EMPTY_SURFACE_TABS, + activeSurfaceTabId = null, + onSurfaceTabSelect, + onSurfaceTabClose, + onSessionTabSelect, + onSessionMissing, }: { sessionId: SessionId; urlTab?: string; urlPrNumber?: number; urlBrowser?: boolean; onMobileBack?: () => void; + /** Host-contributed tabs, drawn after the session tabs. Empty by default, so + * the strip and the surfaces are exactly what they were without them. */ + surfaceTabs?: readonly SessionSurfaceTab[]; + /** Which host tab is selected, or `null` when a session tab is. */ + activeSurfaceTabId?: string | null; + onSurfaceTabSelect?: (tabId: string) => void; + onSurfaceTabClose?: (tabId: string) => void; + /** + * This page moved its own tab selection to a CONVERSATION tab. + * + * The host owns `activeSurfaceTabId` and this page owns the conversation + * selection, so a host tab stays selected — and keeps covering the + * conversation — until the host is told to drop it. Nothing else can tell it: + * the conversation selection is local state and it is not in the URL. + * + * Fired with the tab id the page selected, so a host that draws its own + * chrome can follow the selection rather than only clear its own. + */ + onSessionTabSelect?: (tabId: string) => void; + /** + * This page has no session to draw: the id it was given is not in the + * workspace, so it renders the not-found card and returns BEFORE the tab + * strip. + * + * A host that contributes tabs loses all of them at that return, including + * the one the member is looking at, so it has to be told: the strip is gone + * and whatever the host had selected in it needs another home. + */ + onSessionMissing?: (sessionId: string) => void; }) => { const { t } = useTranslation(); const router = useRouter(); @@ -753,9 +816,39 @@ const SessionDetail = ({ const [mobileFileViewerTabId, setMobileFileViewerTabId] = useState(null); const [mobileFileViewerOpen, setMobileFileViewerOpen] = useState(false); const mobileFilesBrowserRef = useRef(null); - const [activeTabSessionIdRaw, setActiveTabSessionId] = useState( + const [activeTabSessionIdRaw, setActiveTabSessionIdState] = useState( () => initialTabState.activeTabSessionId ); + const onSessionTabSelectRef = useRef(onSessionTabSelect); + onSessionTabSelectRef.current = onSessionTabSelect; + // Seam patch 5 hunk 20 reads this from an effect whose dependency list is + // upstream's; a ref keeps a fresh host closure from re-running that effect. + const onSessionMissingRef = useRef(onSessionMissing); + onSessionMissingRef.current = onSessionMissing; + /** + * THE ONE PLACE THIS PAGE SELECTS A CONVERSATION TAB (seam patch 5 hunk 17). + * + * Ten call sites move `activeTabSessionIdRaw` — the strip click, the `+`, a + * close, a restore, a fork, a mention navigation, the next/previous cycle, a + * promoted draft, the browser panel, and the URL sync — and a host that + * contributed tabs has to hear about every one of them: an active host tab + * hides all the conversation surfaces, so a selection the host never learns + * about leaves its tab covering the conversation the user just asked for. + * + * A wrapper rather than a notification at each site, and rather than an + * effect on the value: an effect would only fire on a CHANGE, and the click + * that most needs the call does not change anything — the parent tab is + * already `activeTabSessionId` while a host tab covers it. + * + * The three writers that do NOT come through here take the raw setter, and + * each is a CORRECTION rather than a selection: the session-switch reset + * (which runs during render, where a host callback may not), and the two URL + * syncs (which re-assert what `?tab=` already says). + */ + const setActiveTabSessionId = useCallback((tabId: string) => { + setActiveTabSessionIdState(tabId); + onSessionTabSelectRef.current?.(tabId); + }, []); const [localStateSessionId, setLocalStateSessionId] = useState(sessionId); const [commentReferenceKeysBySession, setCommentReferenceKeysBySession] = useState< Record @@ -944,7 +1037,9 @@ const SessionDetail = ({ setMobileDiffState(null); setMobileFilesBrowserOpen(false); setFileProviderRequestedByInteraction(false); - setActiveTabSessionId(nextInitialTabState.activeTabSessionId); + // The raw setter: this runs during RENDER, where a host callback may not, + // and a session switch is the host's own navigation anyway. + setActiveTabSessionIdState(nextInitialTabState.activeTabSessionId); } const setDraftTabs = useCallback( @@ -2582,13 +2677,16 @@ const SessionDetail = ({ setActiveViewerTabId(null); } if (urlSyncAction.kind === 'activate-session') { - setActiveTabSessionId((prev) => + // The raw setter: `?tab=` is a correction of a selection that already + // happened, not a new one, and it fires whenever the parsed value's + // identity changes. + setActiveTabSessionIdState((prev) => prev === urlSyncAction.sessionId ? prev : urlSyncAction.sessionId ); return; } - setActiveTabSessionId((prev) => (prev === sessionId ? prev : sessionId)); + setActiveTabSessionIdState((prev) => (prev === sessionId ? prev : sessionId)); }, [isMobile, parsedUrlTab, sessionId]); const resolveDiffFilePaths = useCallback( @@ -3386,8 +3484,25 @@ const SessionDetail = ({ return openedSidebarTabs.filter((tabId) => availableTabIds.has(tabId)); }, [openedSidebarTabs, sidePanelFixedOptions]); + // Host tabs, in the shape the strip draws a non-session tab in. Memoized on + // the host's own list, so a page that contributes none hands `SessionTabBar` + // the same empty array on every render and its `memo` still holds. + const surfaceTabItems = useMemo( + () => + surfaceTabs.map((tab) => ({ + id: tab.id, + type: 'custom' as const, + label: tab.label, + icon: tab.icon, + })), + [surfaceTabs] + ); + // Shared viewer metadata powers the mobile switcher and desktop side-panel tabs. - const viewerTabItems: ViewerTabItem[] = useMemo( + // Narrowed to this page's own viewer kinds: the side panel and the mobile + // switcher have no row for a host tab, and host tabs travel in + // `surfaceTabItems` instead, which goes to the tab strip alone. + const viewerTabItems: (ViewerTabItem & { type: 'file' | 'diff' })[] = useMemo( () => viewerTabs.map((tab) => ({ id: tab.id, @@ -4304,6 +4419,11 @@ const SessionDetail = ({ } if (sessionPresenceState === 'not-found') { + // Seam patch 5 hunk 20. ABOVE the once-per-session analytics gate on + // purpose: the host has to hear this every time the page settles on + // not-found, because what it does with it is move an address, and an + // address can come back. + onSessionMissingRef.current?.(sessionId); if (!fireDetailNotFoundOnce(sessionId)) { return; } @@ -5502,7 +5622,7 @@ const SessionDetail = ({ lives in the context strip above the composer and in the "…" menu. */ const tabBar = ( 0 ? 'mixed' : 'session'} parentSession={activeSession} childSessions={visibleChildSessions} draftTabs={draftTabs} @@ -5510,6 +5630,10 @@ const SessionDetail = ({ activeTabSessionId={activeTabSessionId} onTabSelect={handleSessionTabSelect} onNewTab={handleNewTab} + viewerTabs={surfaceTabItems} + activeViewerTabId={activeSurfaceTabId} + onViewerTabSelect={onSurfaceTabSelect} + onViewerTabClose={onSurfaceTabClose} onTabRename={handleTabRename} onTabClose={handleTabClose} archivedChildSessions={archivedChildSessions} @@ -5577,6 +5701,14 @@ const SessionDetail = ({ }; }; + // An active HOST tab deselects every conversation surface, the same rule + // `hasActiveViewerTab` applies to the strip. Null when the host contributes + // none, so the surfaces below read exactly what they read before. + const activeChatSurfaceId = + activeSurfaceTabId !== null && surfaceTabs.some((tab) => tab.id === activeSurfaceTabId) + ? null + : activeTabSessionId; + const desktopChatSurfaces = ( {[activeSession, ...visibleChildSessions].map((tabSession) => { - const isActive = tabSession.id === activeTabSessionId; + const isActive = tabSession.id === activeChatSurfaceId; const externalHistoryRefresh = externalHistoryRefreshBySessionId[tabSession.id]; const externalHistoryProviderLabel = externalHistoryRefresh ? getExternalHistoryProviderLabel(externalHistoryRefresh.provider) @@ -5618,7 +5750,7 @@ const SessionDetail = ({ ); })} {draftTabs.map((draft) => { - const isActive = draft.id === activeTabSessionId; + const isActive = draft.id === activeChatSurfaceId; return (
); })} + {surfaceTabs.map((tab) => { + const isActive = tab.id === activeSurfaceTabId; + return ( +
+ {tab.content} +
+ ); + })} ); diff --git a/packages/components/src/components/sessions/session-tab-bar.tsx b/packages/components/src/components/sessions/session-tab-bar.tsx index 8c6ec9d6d..5cbe297e5 100644 --- a/packages/components/src/components/sessions/session-tab-bar.tsx +++ b/packages/components/src/components/sessions/session-tab-bar.tsx @@ -1,4 +1,4 @@ -import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { memo, useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'; import { Plus, Loader2, X, History, Undo2, Pin, FileDiff } from 'lucide-react'; import { cn } from '@/lib/utils'; import { getSessionLaunchConfigLegacyFields, type SessionId, type SessionMeta } from '@lody/shared'; @@ -39,11 +39,18 @@ import { startSessionMentionDrag, } from '@/lib/session-mention-drag'; -/** A viewer tab item (file or diff) displayed in the tab bar. */ +/** A viewer tab item (file or diff) displayed in the tab bar. + * + * `custom` is for a tab a HOST contributes: the bar draws and sorts it exactly + * like a file or a diff tab, and the host supplies both the glyph ({@link icon}) + * and the content, so the bar needs to know nothing about what is behind it. */ export interface ViewerTabItem { id: string; - type: 'file' | 'diff'; + type: 'file' | 'diff' | 'custom'; label: string; + /** Drawn in place of the file/diff glyph. Required in practice for a + * `custom` tab, which has no file path to derive an icon from. */ + icon?: ReactNode; filePath?: string; dirty?: boolean; saving?: boolean; @@ -55,7 +62,10 @@ type MaybePromiseVoid = void | Promise; interface SessionTabBarProps { variant?: SessionTabBarVariant; - parentSession: SessionMeta; + /** The session the strip is rooted in. Optional because `variant="viewer"` + * draws no session tabs at all, so a host using that variant has no session + * to name — requiring it is what stops the declared variant being usable. */ + parentSession?: SessionMeta; childSessions: SessionMeta[]; draftTabs: DraftSessionTab[]; archivedChildSessions: SessionMeta[]; @@ -463,11 +473,11 @@ function ViewerTabContent({ }} > - {tab.type === 'file' && tab.filePath ? ( + {tab.icon ?? (tab.type === 'file' && tab.filePath ? ( ) : ( - )} + ))} {saveStateLabel ? ( (showSessionTabs ? [parentSession.id, ...sortableIds] : sortableIds), - [parentSession.id, showSessionTabs, sortableIds] + () => + showSessionTabs && parentSession ? [parentSession.id, ...sortableIds] : sortableIds, + [parentSession, showSessionTabs, sortableIds] ); const activeTabId = showViewerTabs && activeViewerTabId @@ -762,7 +773,7 @@ export const SessionTabBar = memo(function SessionTabBar({ paddingLeft={variant === 'session' ? 4 : 8} paddingRight={8} > - {showSessionTabs && ( + {showSessionTabs && parentSession && (