fix(desktop): don't offer Rename on canonical Bot Chat tabs
Fixes #124857
This commit is contained in:
committed by
brooklyn!
parent
9b17e80970
commit
ac9a850eb9
@@ -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<typeof workspaceSessionRenameable>[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}
|
||||
|
||||
@@ -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(
|
||||
<SessionContextMenu onDelete={vi.fn()} renameable={false} sessionId="bot-chat" title="Bot Chat">
|
||||
<button aria-label="Session row" type="button">
|
||||
Row
|
||||
</button>
|
||||
</SessionContextMenu>
|
||||
)
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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:<id>` 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 ? (
|
||||
<RenameSessionDialog
|
||||
currentTitle={title}
|
||||
onOpenChange={setRenameOpen}
|
||||
@@ -539,7 +552,7 @@ function useSessionActions({
|
||||
profile={profile}
|
||||
sessionId={sessionId}
|
||||
/>
|
||||
)
|
||||
) : 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.
|
||||
|
||||
@@ -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())
|
||||
|
||||
|
||||
@@ -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 ?? ''}`
|
||||
|
||||
Reference in New Issue
Block a user