diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index ea927f4a7a..550b8e38ca 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -29,7 +29,7 @@ import { formatRefValue } from '@/components/assistant-ui/directive-text' import { CenteredThreadSpinner } from '@/components/assistant-ui/thread/status' import { findGroupOfPane } from '@/components/pane-shell/tree/model' import { $layoutTree, closeTreePane, moveTreePane, setTreeGroupTabStrip } from '@/components/pane-shell/tree/store' -import { $workspaceOwnerLabels, workspaceOwnerTitle } from '@/components/pane-shell/workspace-scope' +import { $workspaceOwnerLabels, workspaceOwnerTitle, workspaceSessionRenameable } from '@/components/pane-shell/workspace-scope' import { Button } from '@/components/ui/button' import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { transcribeAudio } from '@/hermes' @@ -56,6 +56,7 @@ import { import { isSessionRemovalPending } from '@/store/session-removal' import { requestForSessionProfile } from '@/store/session-request-router' import { + $botChatScopes, $sessionStates, $sessionTileDelegateRevision, $sessionTiles, @@ -672,6 +673,16 @@ function tileTitle(storedSessionId: string): string { return stored ? sessionTitle(stored) : explicit || NEW_SESSION_TITLE } +/** The tile's workspace scope — the same fields `workspaceOwnerTitle` reads, + * so the rename gate agrees with the caption's notion of a bot tab. */ +function tileWorkspaceScope(storedSessionId: string): Parameters[0] { + const tile = $sessionTiles.get().find(candidate => candidate.storedSessionId === storedSessionId) + + return tile + ? { workspaceMode: tile.workspaceMode, workspaceTabTitle: tile.workspaceTabTitle } + : ($botChatScopes.get()[storedSessionId] ?? {}) +} + /** The tab's CAPTION: a bot chat's owner name over the canonical stored title * (#99152). The menu keeps `tileTitle` — rename/delete show the real row. */ function tileCaption(storedSessionId: string): string { @@ -763,16 +774,20 @@ export function stackSessionTilesIntoMain(): void { * updates in other sessions) — for a context menu that's almost never open. * Same class as the TreeGroup fix (#72245): derive narrowly, bail out unless * the derived values change. */ -function useTileMenuRow(storedSessionId: string): { pinId: string; profile?: string; title: string } { - const cache = useRef<{ key: string; value: { pinId: string; profile?: string; title: string } } | null>(null) +function useTileMenuRow(storedSessionId: string): { pinId: string; profile?: string; renameable: boolean; title: string } { + const cache = useRef< + { key: string; value: { pinId: string; profile?: string; renameable: boolean; title: string } } | null + >(null) const subscribe = useCallback((onChange: () => void) => { const offSessions = $sessions.listen(onChange) const offTree = $projectTree.listen(onChange) + const offTiles = $sessionTiles.listen(onChange) return () => { offSessions() offTree() + offTiles() } }, []) @@ -781,10 +796,11 @@ function useTileMenuRow(storedSessionId: string): { pinId: string; profile?: str const pinId = stored ? sessionPinId(stored) : storedSessionId const title = tileTitle(storedSessionId) const profile = stored?.profile - const key = `${pinId}\u0000${title}\u0000${profile ?? ''}` + const renameable = workspaceSessionRenameable(tileWorkspaceScope(storedSessionId)) + const key = `${pinId}\u0000${title}\u0000${profile ?? ''}\u0000${renameable}` if (cache.current?.key !== key) { - cache.current = { key, value: { pinId, profile, title } } + cache.current = { key, value: { pinId, profile, renameable, title } } } return cache.current.value @@ -812,7 +828,7 @@ export function SessionTabMenu({ /** Layout-tree pane id — powers the Close-others/right/all verbs. */ tabPaneId: string }) { - const { pinId, profile, title } = useTileMenuRow(storedSessionId) + const { pinId, profile, renameable, title } = useTileMenuRow(storedSessionId) const pinnedSessionIds = useStore($pinnedSessionIds) const pinned = pinnedSessionIds.includes(pinId) @@ -827,6 +843,7 @@ export function SessionTabMenu({ onPin={() => (pinned ? unpinSession(pinId) : pinSession(pinId))} pinned={pinned} profile={profile} + renameable={renameable} sessionId={storedSessionId} surface="tab" tabPaneId={tabPaneId} diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx index a329f07578..9c96e36887 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx @@ -312,4 +312,32 @@ describe('SessionActionsMenu', () => { expect(await screen.findByText('Session deleted')).toBeTruthy() expect(onDelete).toHaveBeenCalledTimes(1) }) + + // A canonical Bot Chat tab must not offer Rename: the write can never reach + // the caption it names (the caption is the roster label) and the backend + // guard refuses it anyway — the old flow toasted success over a no-op + // (#124857). The item is omitted, not disabled, so the menu shows only + // verbs whose result the user can observe. + it('omits Rename (and never mounts its dialog) when renameable is false', async () => { + const { unmount } = render( + + + + ) + + const row = screen.getByRole('button', { name: 'Session row' }) + fireEvent.contextMenu(row) + + await screen.findByRole('menu') + expect(screen.queryByRole('menuitem', { name: /rename/i })).toBeNull() + // The other identity verbs stay available — only Rename is gated. + expect(screen.getByRole('menuitem', { name: /^pin$/i })).toBeTruthy() + + // No rename dialog is mounted anywhere (portals included): the verb is + // unreachable even programmatically, not just hidden from pointer users. + expect(screen.queryByRole('dialog')).toBeNull() + unmount() + }) }) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index c56b4b4655..a28db645e0 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -119,6 +119,12 @@ interface SessionActions { /** TAB surfaces: the session is already a tab, so "Open in new tab" is * nonsense there — sidebar rows/dropdowns keep it. */ surface?: 'row' | 'tab' + /** May this session be renamed? False for a canonical Bot Chat tab: its + * exact title is the bot's identity (the backend guard refuses a user + * rename and the caption never reads the stored title anyway — #124857), + * so the Rename item and dialog are omitted instead of toasting success + * over a no-op. Mirrors how onPin/onBranch are gated. */ + renameable?: boolean /** The tab's layout-tree pane id (`session-tile:` or `workspace`) — enables * the Close-others / to-the-right / all tab verbs. Tab surfaces only. */ tabPaneId?: string @@ -200,6 +206,7 @@ function useSessionActions({ onDelete, onClose, onHideTabBar, + renameable = true, surface = 'row', tabPaneId }: SessionActions) { @@ -287,19 +294,25 @@ function useSessionActions({ : []) ] - // IDENTITY — name/mark/reference the session. + // IDENTITY — name/mark/reference the session. Rename is omitted (not + // disabled) for a session whose title is not its name — a canonical Bot + // Chat — so the menu never offers a verb whose result the user cannot see. const identityItems: ActionItemSpec[] = [ - spec({ - disabled: !sessionId, - icon: 'edit', - label: r.rename, - onSelect: () => { - triggerHaptic('selection') - // Keep focus off the row trigger so it lands in the dialog input. - suppressCloseFocusRef.current = true - setRenameOpen(true) - } - }), + ...(renameable + ? [ + spec({ + disabled: !sessionId, + icon: 'edit', + label: r.rename, + onSelect: () => { + triggerHaptic('selection') + // Keep focus off the row trigger so it lands in the dialog input. + suppressCloseFocusRef.current = true + setRenameOpen(true) + } + }) + ] + : []), spec({ disabled: !onPin, icon: 'pin', @@ -531,7 +544,7 @@ function useSessionActions({ ) - const renameDialog = ( + const renameDialog = renameable ? ( - ) + ) : null // Consumed once per close: when rename was the action that closed the menu, // block Radix's focus-restore to the trigger so the dialog input keeps focus. diff --git a/apps/desktop/src/components/pane-shell/workspace-scope.test.ts b/apps/desktop/src/components/pane-shell/workspace-scope.test.ts index ac24747480..ed3048d7a7 100644 --- a/apps/desktop/src/components/pane-shell/workspace-scope.test.ts +++ b/apps/desktop/src/components/pane-shell/workspace-scope.test.ts @@ -11,7 +11,8 @@ import { resolveRememberedActivePane, setWorkspaceOwnerLabel, setWorkspaceScope, - workspaceOwnerTitle + workspaceOwnerTitle, + workspaceSessionRenameable } from './workspace-scope' afterEach(() => { @@ -69,6 +70,25 @@ describe('workspace owner title', () => { }) }) +describe('workspace session renameable', () => { + // Same discriminator as the caption: only the bot workspace's OWN canonical + // tab (workspaceMode 'bots' + a registered canonical tab title) loses the + // verb. A session that merely happens to be open in Bot Mode keeps it, and + // plain Sessions tabs are untouched (#124857). + it('gates Rename on the canonical bot-chat scope exactly where the caption takes over', () => { + const botChat = { workspaceMode: 'bots' as const, workspaceOwnerKey: 'bot:alpha', workspaceTabTitle: 'Bot Chat' } + + expect(workspaceSessionRenameable(botChat)).toBe(false) + // A `+` side thread under the same bot has no canonical tab title. + expect(workspaceSessionRenameable({ workspaceMode: 'bots' })).toBe(true) + // A Sessions tab titled the same way is not a bot chat. + expect(workspaceSessionRenameable({ workspaceMode: 'sessions', workspaceTabTitle: 'Bot Chat' })).toBe(true) + // No scope at all (a plain row/tile): renameable. + expect(workspaceSessionRenameable(undefined)).toBe(true) + expect(workspaceSessionRenameable({})).toBe(true) + }) +}) + describe('remembered active panes', () => { beforeEach(() => resetRememberedActivePanes()) diff --git a/apps/desktop/src/components/pane-shell/workspace-scope.ts b/apps/desktop/src/components/pane-shell/workspace-scope.ts index 4b0972265c..beede2c7e4 100644 --- a/apps/desktop/src/components/pane-shell/workspace-scope.ts +++ b/apps/desktop/src/components/pane-shell/workspace-scope.ts @@ -73,6 +73,18 @@ export function workspaceOwnerTitle( return $workspaceOwnerLabels.get()[scope.workspaceOwnerKey] ?? title } +/** May the user rename this session? A canonical Bot Chat's exact title is its + * identity — Bot Mode re-resolves it by name on every open and the backend's + * #95397 guard refuses a user rename — so surfaces that show one must not + * offer the verb (the write can never reach the caption it names). Only a bot + * workspace's OWN canonical tab is non-renameable; a plain session that merely + * happens to be open in Bot Mode keeps the verb. */ +export function workspaceSessionRenameable( + scope: { workspaceMode?: WorkspaceMode; workspaceTabTitle?: string } | undefined +): boolean { + return !(scope?.workspaceMode === 'bots' && scope.workspaceTabTitle) +} + /** One key for window-local active-pane memory. Owner keys stay opaque. */ export function workspaceScopeKey(mode: WorkspaceMode, ownerKey: string | null): string { return mode === 'sessions' ? 'sessions' : `bots:${ownerKey ?? ''}`