diff --git a/src/main/browser/browser-guest-ui.ts b/src/main/browser/browser-guest-ui.ts index dd0bdef9b..ca43372bc 100644 --- a/src/main/browser/browser-guest-ui.ts +++ b/src/main/browser/browser-guest-ui.ts @@ -209,6 +209,25 @@ export function setupGuestShortcutForwarding(args: { if (input.type !== 'keyDown') { return } + // Why: resolve the policy action once per keystroke. The history-navigate + // chord (Cmd/Ctrl+Alt+Arrow) is the only allowlisted chord that carries + // Alt and must be handled before the generic modifier-chord gate below, + // which rejects Alt. Every other chord handled further down can reuse + // the same `action` rather than re-running the full predicate chain. + const action = resolveWindowShortcutAction(input, process.platform) + if (action?.type === 'worktreeHistoryNavigate') { + // Why: preventDefault unconditionally — if we cannot resolve the + // renderer (torn-down tab or teardown race), dropping the keystroke + // into the guest's webContents would let Chromium / the guest page + // handle Cmd+Alt+Arrow as their own chord (e.g. guest-side text + // navigation). Consistency with the main-window path is preserved + // only by suppressing the event here too. + event.preventDefault() + const renderer = resolveRenderer(browserTabId) + renderer?.send('ui:worktreeHistoryNavigate', action.direction) + return + } + // Why: browser guests need a broader modifier-chord gate than the main // window because they also forward guest-specific tab shortcuts // (Cmd/Ctrl+T/W/Shift+B/Shift+[ / ]) in addition to the shared allowlist @@ -222,11 +241,6 @@ export function setupGuestShortcutForwarding(args: { return } - // Why: centralizing the shared subset still keeps guest forwarding in - // lockstep with the main window for the chords that must never steal - // readline control input above the terminal. - const action = resolveWindowShortcutAction(input, process.platform) - if (input.code === 'KeyB' && input.shift) { renderer.send('ui:newBrowserTab') } else if (input.code === 'KeyT' && !input.shift) { diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 1bef0dd7b..7be28e67e 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -393,6 +393,14 @@ export function createMainWindow( if (action.type === 'jumpToWorktreeIndex') { // Forward Cmd/Ctrl+1-9 for quick worktree switching mainWindow.webContents.send('ui:jumpToWorktreeIndex', action.index) + return + } + + if (action.type === 'worktreeHistoryNavigate') { + // Why: routed through main so the chord reaches the renderer even when + // a terminal (xterm.js) or a browser guest has focus — both surfaces + // otherwise absorb Arrow keys before the renderer's window listener. + mainWindow.webContents.send('ui:worktreeHistoryNavigate', action.direction) } }) diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index db7c66952..194960dc0 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -618,6 +618,7 @@ export type PreloadApi = { onOpenQuickOpen: (callback: () => void) => () => void onOpenNewWorkspace: (callback: () => void) => () => void onJumpToWorktreeIndex: (callback: (index: number) => void) => () => void + onWorktreeHistoryNavigate: (callback: (direction: 'back' | 'forward') => void) => () => void onNewBrowserTab: (callback: () => void) => () => void onRequestTabCreate: ( callback: (data: { requestId: string; url: string; worktreeId?: string }) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index e295ba8ef..7c95c360f 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1082,6 +1082,14 @@ const api = { ipcRenderer.on('ui:jumpToWorktreeIndex', listener) return () => ipcRenderer.removeListener('ui:jumpToWorktreeIndex', listener) }, + onWorktreeHistoryNavigate: ( + callback: (direction: 'back' | 'forward') => void + ): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, direction: 'back' | 'forward') => + callback(direction) + ipcRenderer.on('ui:worktreeHistoryNavigate', listener) + return () => ipcRenderer.removeListener('ui:worktreeHistoryNavigate', listener) + }, onNewBrowserTab: (callback: () => void): (() => void) => { const listener = (_event: Electron.IpcRendererEvent) => callback() ipcRenderer.on('ui:newBrowserTab', listener) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 2c3a4683c..6d1fb6f1b 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -2,7 +2,7 @@ import { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { DEFAULT_STATUS_BAR_ITEMS, DEFAULT_WORKTREE_CARD_PROPERTIES } from '../../shared/constants' -import { Minimize2, PanelLeft, PanelRight } from 'lucide-react' +import { ChevronLeft, ChevronRight, Minimize2, PanelLeft, PanelRight } from 'lucide-react' import { FOCUS_TERMINAL_PANE_EVENT, TOGGLE_TERMINAL_PANE_EXPAND_EVENT } from '@/constants/terminal' import { syncZoomCSSVar } from '@/lib/ui-zoom' import { toast } from 'sonner' @@ -39,6 +39,10 @@ import { countWorkingAgents, getWorkingAgentsPerWorktree } from './lib/agent-sta import { activateAndRevealWorktree } from './lib/worktree-activation' import { Popover, PopoverTrigger, PopoverContent } from '@/components/ui/popover' import { findWorktreeById, getRepoIdFromWorktreeId } from '@/store/slices/worktree-helpers' +import { + canGoBackWorktreeHistory, + canGoForwardWorktreeHistory +} from '@/store/slices/worktree-nav-history' import { dispatchClearModifierHints } from './hooks/useModifierHint' const isMac = navigator.userAgent.includes('Mac') @@ -126,6 +130,8 @@ function App(): React.JSX.Element { const rightSidebarOpen = useAppStore((s) => s.rightSidebarOpen) const isFullScreen = useAppStore((s) => s.isFullScreen) const settings = useAppStore((s) => s.settings) + const canGoBackWorktree = useAppStore(canGoBackWorktreeHistory) + const canGoForwardWorktree = useAppStore(canGoForwardWorktreeHistory) const titlebarLeftControlsRef = useRef(null) const [collapsedSidebarHeaderWidth, setCollapsedSidebarHeaderWidth] = useState(0) @@ -491,6 +497,33 @@ function App(): React.JSX.Element { if (isEditableTarget(e.target)) { return } + + // Cmd/Ctrl+Alt+Arrow — worktree history back/forward. Handled before the + // `mod && !alt` branch below since this is the one renderer-side shortcut + // that intentionally requires Alt. + if ( + e.altKey && + !e.shiftKey && + (isMac ? e.metaKey && !e.ctrlKey : e.ctrlKey && !e.metaKey) && + (e.code === 'ArrowLeft' || e.code === 'ArrowRight') + ) { + // Why: hidden buttons in non-terminal views mean the shortcut must be + // a no-op there too — navigating worktree history from Settings or + // Tasks is not a meaningful action. + if (activeView !== 'terminal') { + return + } + dispatchClearModifierHints() + e.preventDefault() + const store = useAppStore.getState() + if (e.code === 'ArrowLeft') { + store.goBackWorktree() + } else { + store.goForwardWorktree() + } + return + } + if (!mod) { return } @@ -718,6 +751,44 @@ function App(): React.JSX.Element { ) : null} + {/* Why: Back/Forward navigate worktree-activation history. Only + meaningful while viewing a worktree (terminal view); hidden in + Settings/Tasks/Landing to keep the titlebar compact and the + semantics unambiguous. */} + {activeView === 'terminal' && ( + <> + + + + + + {`Go back (${isMac ? '⌘⌥←' : 'Ctrl+Alt+←'})`} + + + + + + + + {`Go forward (${isMac ? '⌘⌥→' : 'Ctrl+Alt+→'})`} + + + + )} ) diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index ee050ed60..e148017f1 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -8,6 +8,7 @@ import CacheTimer from './CacheTimer' import WorktreeContextMenu from './WorktreeContextMenu' import { SshDisconnectedDialog } from './SshDisconnectedDialog' import { cn } from '@/lib/utils' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { getWorktreeStatus, type WorktreeStatus } from '@/lib/worktree-status' import { getRepoKindLabel, isFolderRepo } from '../../../../shared/repo-kind' import type { Worktree, Repo, PRInfo, IssueInfo } from '../../../../shared/types' @@ -37,8 +38,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ hideRepoBadge, hintNumber }: WorktreeCardProps) { - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) - const setActiveView = useAppStore((s) => s.setActiveView) const openModal = useAppStore((s) => s.openModal) const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch) @@ -159,8 +158,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ }, [repo, isFolder, worktree.linkedIssue, fetchIssue, issueCacheKey, showIssue]) // Stable click handler – ignore clicks that are really text selections. - // Why: if the SSH connection is down, show a reconnect dialog instead of - // activating the worktree — all remote operations would fail anyway. const handleClick = useCallback( (event: React.MouseEvent) => { const selection = window.getSelection() @@ -181,21 +178,15 @@ const WorktreeCard = React.memo(function WorktreeCard({ return } } - if (useAppStore.getState().activeView !== 'terminal') { - // Why: the sidebar remains visible on the tasks page, so clicking a - // real worktree should switch the main pane back to that worktree - // instead of leaving the tasks surface visible. - setActiveView('terminal') - } - // Why: always activate the worktree so the user can see terminal history, - // editor state, etc. even when SSH is disconnected. Show the reconnect - // dialog as a non-blocking overlay rather than a gate. - setActiveWorktree(worktree.id) + // Why: route sidebar clicks through the shared activation path so the + // back/forward stack stays complete for the primary worktree navigation + // surface instead of only recording palette-driven switches. + activateAndRevealWorktree(worktree.id) if (isSshDisconnected) { setShowDisconnectedDialog(true) } }, - [worktree.id, setActiveView, setActiveWorktree, isSshDisconnected] + [worktree.id, isSshDisconnected] ) const handleDoubleClick = useCallback(() => { diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 2421a8f24..f70a84e89 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -18,6 +18,7 @@ import { } from './worktree-list-groups' import { computeVisibleWorktreeIds, setVisibleWorktreeIds } from './visible-worktrees' import { useModifierHint } from '@/hooks/useModifierHint' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' // How long to wait after a sortEpoch bump before actually re-sorting. // Prevents jarring position shifts when background events (AI starting work, @@ -52,7 +53,6 @@ function getWorktreeOptionId(worktreeId: string): string { type VirtualizedWorktreeViewportProps = { rows: Row[] activeWorktreeId: string | null - setActiveWorktree: (worktreeId: string | null) => void groupBy: 'none' | 'repo' | 'pr-status' toggleGroup: (key: string) => void collapsedGroups: Set @@ -69,7 +69,6 @@ type VirtualizedWorktreeViewportProps = { const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewport({ rows, activeWorktreeId, - setActiveWorktree, groupBy, toggleGroup, collapsedGroups, @@ -180,14 +179,16 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } const nextWorktreeId = worktreeRows[nextIndex].worktree.id - setActiveWorktree(nextWorktreeId) + // Why: keyboard cycling between worktrees is still real navigation, so + // it must flow through the same activation helper that records history. + activateAndRevealWorktree(nextWorktreeId) const rowIndex = rows.findIndex((r) => r.type === 'item' && r.worktree.id === nextWorktreeId) if (rowIndex !== -1) { virtualizer.scrollToIndex(rowIndex, { align: 'auto' }) } }, - [rows, activeWorktreeId, setActiveWorktree, virtualizer] + [rows, activeWorktreeId, virtualizer] ) useEffect(() => { @@ -390,7 +391,6 @@ const WorktreeList = React.memo(function WorktreeList() { const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) const repos = useAppStore((s) => s.repos) const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const searchQuery = useAppStore((s) => s.searchQuery) const groupBy = useAppStore((s) => s.groupBy) const sortBy = useAppStore((s) => s.sortBy) @@ -688,7 +688,6 @@ const WorktreeList = React.memo(function WorktreeList() { key={viewportResetKey} rows={rows} activeWorktreeId={selectedSidebarWorktreeId} - setActiveWorktree={setActiveWorktree} groupBy={groupBy} toggleGroup={toggleGroup} collapsedGroups={collapsedGroups} diff --git a/src/renderer/src/components/status-bar/SessionsStatusSegment.tsx b/src/renderer/src/components/status-bar/SessionsStatusSegment.tsx index 87d2702c4..cbbbe48a8 100644 --- a/src/renderer/src/components/status-bar/SessionsStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/SessionsStatusSegment.tsx @@ -7,6 +7,7 @@ import { DropdownMenuTrigger } from '@/components/ui/dropdown-menu' import { useAppStore } from '../../store' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' type DaemonSession = { id: string; cwd: string; title: string } @@ -90,7 +91,6 @@ export function SessionsStatusSegment({ const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId) const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady) - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const setActiveTab = useAppStore((s) => s.setActiveTab) const setActiveView = useAppStore((s) => s.setActiveView) @@ -172,12 +172,15 @@ export function SessionsStatusSegment({ (tabId: string) => { const worktreeId = tabIdToWorktreeId.get(tabId) if (worktreeId) { - setActiveWorktree(worktreeId) + // Why: opening a session from the status bar is another worktree jump, + // so route it through the shared activation path before restoring the + // specific tab inside that worktree. + activateAndRevealWorktree(worktreeId) } setActiveView('terminal') setActiveTab(tabId) }, - [tabIdToWorktreeId, setActiveWorktree, setActiveView, setActiveTab] + [tabIdToWorktreeId, setActiveView, setActiveTab] ) return ( diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts index ce6345ac8..d8a4d2228 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.test.ts @@ -32,6 +32,14 @@ vi.mock('@/lib/language-detect', () => ({ detectLanguage: () => 'plaintext' })) +// Why: the real helper reads worktreesByRepo/activeRepoId/etc. from the store +// and orchestrates side effects that are out of scope for the link-handler +// unit tests. Mock it so these tests only assert on routing (browser tab vs. +// openFile), not on activation internals. +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorktree: vi.fn() +})) + function setPlatform(userAgent: string): void { vi.stubGlobal('navigator', { userAgent }) } diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts index 13552d2a7..21b71f60c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -10,6 +10,7 @@ import { useAppStore } from '@/store' import { getConnectionId } from '@/lib/connection-context' import { absolutePathToFileUri } from '@/components/editor/markdown-internal-links' import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' export type LinkHandlerDeps = { worktreeId: string @@ -52,7 +53,9 @@ function isHtmlFilePath(filePath: string): boolean { function openHtmlFileInBrowser(filePath: string, worktreeId: string): void { const store = useAppStore.getState() if (worktreeId) { - store.setActiveWorktree(worktreeId) + // Why: following an HTML file link changes which worktree is foregrounded, + // so it must record a history visit before opening the browser tab. + activateAndRevealWorktree(worktreeId) } const fileUrl = absolutePathToFileUri(filePath) const title = filePath.split(/[/\\]/).pop() ?? filePath @@ -104,7 +107,10 @@ export function openDetectedFilePath( const store = useAppStore.getState() if (worktreeId) { - store.setActiveWorktree(worktreeId) + // Why: terminal file links can jump across worktrees. Reusing the shared + // activation path keeps those jumps in the same history stack as sidebar + // and palette navigation before the editor opens the destination file. + activateAndRevealWorktree(worktreeId) } store.openFile({ diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index c0193122e..5531548c7 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -151,6 +151,7 @@ describe('useIpcEvents updater integration', () => { onOpenQuickOpen: () => () => {}, onOpenNewWorkspace: () => () => {}, onJumpToWorktreeIndex: () => () => {}, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, @@ -320,6 +321,7 @@ describe('useIpcEvents updater integration', () => { onOpenQuickOpen: () => () => {}, onOpenNewWorkspace: () => () => {}, onJumpToWorktreeIndex: () => () => {}, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, @@ -492,6 +494,7 @@ describe('useIpcEvents browser tab close routing', () => { onOpenQuickOpen: () => () => {}, onOpenNewWorkspace: () => () => {}, onJumpToWorktreeIndex: () => () => {}, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, @@ -657,6 +660,7 @@ describe('useIpcEvents browser tab close routing', () => { onOpenQuickOpen: () => () => {}, onOpenNewWorkspace: () => () => {}, onJumpToWorktreeIndex: () => () => {}, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, @@ -817,6 +821,7 @@ describe('useIpcEvents browser tab close routing', () => { onOpenQuickOpen: () => () => {}, onOpenNewWorkspace: () => () => {}, onJumpToWorktreeIndex: () => () => {}, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, @@ -995,6 +1000,7 @@ describe('useIpcEvents shortcut hint clearing', () => { jumpToWorktreeRef.current = listener return () => {} }, + onWorktreeHistoryNavigate: () => () => {}, onActivateWorktree: () => () => {}, onNewBrowserTab: () => () => {}, onRequestTabCreate: () => () => {}, diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 134062ab3..426373d47 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -108,6 +108,25 @@ export function useIpcEvents(): void { }) ) + unsubs.push( + window.api.ui.onWorktreeHistoryNavigate((direction) => { + dispatchClearModifierHints() + const store = useAppStore.getState() + // Why: mirror the button-visibility rule — worktree history navigation + // is only meaningful in the terminal (worktree) view. Settings/Tasks + // transitions aren't worktree activations and the buttons are hidden, + // so the shortcut no-ops there too. + if (store.activeView !== 'terminal') { + return + } + if (direction === 'back') { + store.goBackWorktree() + } else { + store.goForwardWorktree() + } + }) + ) + unsubs.push( window.api.ui.onToggleStatusBar(() => { const store = useAppStore.getState() diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index 88f4dc559..31623e641 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -3,6 +3,7 @@ import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-t import { buildSetupRunnerCommand } from './setup-runner' import { useAppStore } from '@/store' import { findWorktreeById } from '@/store/slices/worktree-helpers' +import { setWorktreeNavActivator } from '@/store/slices/worktree-nav-history' // Why: issue commands can originate from two sources with different shapes — // (1) a repo-level runner script generated by main (WorktreeSetupLaunch), or @@ -81,6 +82,15 @@ export function activateAndRevealWorktree( // clears unread, bumps dead PTY generations, triggers GitHub refresh state.setActiveWorktree(worktreeId) + // Why: activateAndRevealWorktree always ends in 'terminal' view (step 2), + // and Settings/Tasks transitions do not pass through this function, so no + // view-guard is needed here. The guard skips re-recording when the caller + // is goBackWorktree/goForwardWorktree, which mutate the history index + // directly instead of treating the target as a new visit. + if (!state.isNavigatingHistory) { + state.recordWorktreeVisit(worktreeId) + } + // 4. Ensure a focusable surface exists for externally-created worktrees const primaryTabId = ensureWorktreeHasInitialTerminal( useAppStore.getState(), @@ -184,3 +194,9 @@ export function ensureWorktreeHasInitialTerminal( return terminalTab.id } + +// Why: break the import cycle — the nav-history slice must call +// activateAndRevealWorktree from goBack/goForward, but the slice lives under +// @/store, which activation already imports from. Registering the activator +// at module init here lets the slice call back without importing this file. +setWorktreeNavActivator(activateAndRevealWorktree) diff --git a/src/renderer/src/store/index.ts b/src/renderer/src/store/index.ts index b688177b4..1b4840b6d 100644 --- a/src/renderer/src/store/index.ts +++ b/src/renderer/src/store/index.ts @@ -16,6 +16,7 @@ import { createRateLimitSlice } from './slices/rate-limits' import { createSshSlice } from './slices/ssh' import { createDiffCommentsSlice } from './slices/diffComments' import { createDetectedAgentsSlice } from './slices/detected-agents' +import { createWorktreeNavHistorySlice } from './slices/worktree-nav-history' import { e2eConfig } from '@/lib/e2e-config' export const useAppStore = create()((...a) => ({ @@ -34,7 +35,8 @@ export const useAppStore = create()((...a) => ({ ...createRateLimitSlice(...a), ...createSshSlice(...a), ...createDiffCommentsSlice(...a), - ...createDetectedAgentsSlice(...a) + ...createDetectedAgentsSlice(...a), + ...createWorktreeNavHistorySlice(...a) })) export type { AppState } from './types' diff --git a/src/renderer/src/store/slices/store-session-cascades.test.ts b/src/renderer/src/store/slices/store-session-cascades.test.ts index e7fc86ea6..424d6fcd4 100644 --- a/src/renderer/src/store/slices/store-session-cascades.test.ts +++ b/src/renderer/src/store/slices/store-session-cascades.test.ts @@ -100,6 +100,7 @@ import { createRateLimitSlice } from './rate-limits' import { createSshSlice } from './ssh' import { createDiffCommentsSlice } from './diffComments' import { createDetectedAgentsSlice } from './detected-agents' +import { createWorktreeNavHistorySlice } from './worktree-nav-history' function createTestStore() { return create()((...a) => ({ @@ -118,7 +119,8 @@ function createTestStore() { ...createRateLimitSlice(...a), ...createSshSlice(...a), ...createDiffCommentsSlice(...a), - ...createDetectedAgentsSlice(...a) + ...createDetectedAgentsSlice(...a), + ...createWorktreeNavHistorySlice(...a) })) } diff --git a/src/renderer/src/store/slices/store-test-helpers.ts b/src/renderer/src/store/slices/store-test-helpers.ts index c3881ee85..1ea91ec90 100644 --- a/src/renderer/src/store/slices/store-test-helpers.ts +++ b/src/renderer/src/store/slices/store-test-helpers.ts @@ -24,6 +24,7 @@ import { createRateLimitSlice } from './rate-limits' import { createSshSlice } from './ssh' import { createDiffCommentsSlice } from './diffComments' import { createDetectedAgentsSlice } from './detected-agents' +import { createWorktreeNavHistorySlice } from './worktree-nav-history' export const TEST_REPO = { id: 'repo1', @@ -50,7 +51,8 @@ export function createTestStore() { ...createRateLimitSlice(...a), ...createSshSlice(...a), ...createDiffCommentsSlice(...a), - ...createDetectedAgentsSlice(...a) + ...createDetectedAgentsSlice(...a), + ...createWorktreeNavHistorySlice(...a) })) } diff --git a/src/renderer/src/store/slices/tabs.test.ts b/src/renderer/src/store/slices/tabs.test.ts index 9c5591ce3..82ab193c8 100644 --- a/src/renderer/src/store/slices/tabs.test.ts +++ b/src/renderer/src/store/slices/tabs.test.ts @@ -95,6 +95,7 @@ import { createRateLimitSlice } from './rate-limits' import { createSshSlice } from './ssh' import { createDiffCommentsSlice } from './diffComments' import { createDetectedAgentsSlice } from './detected-agents' +import { createWorktreeNavHistorySlice } from './worktree-nav-history' const WT = 'repo1::/tmp/feature' @@ -115,7 +116,8 @@ function createTestStore() { ...createRateLimitSlice(...a), ...createSshSlice(...a), ...createDiffCommentsSlice(...a), - ...createDetectedAgentsSlice(...a) + ...createDetectedAgentsSlice(...a), + ...createWorktreeNavHistorySlice(...a) })) } diff --git a/src/renderer/src/store/slices/worktree-nav-history.test.ts b/src/renderer/src/store/slices/worktree-nav-history.test.ts new file mode 100644 index 000000000..8a1666aae --- /dev/null +++ b/src/renderer/src/store/slices/worktree-nav-history.test.ts @@ -0,0 +1,221 @@ +import { createStore, type StoreApi } from 'zustand/vanilla' +import { afterEach, describe, expect, it } from 'vitest' +import type { AppState } from '../types' +import type { Worktree } from '../../../../shared/types' +import { + canGoBackWorktreeHistory, + canGoForwardWorktreeHistory, + createWorktreeNavHistorySlice, + setWorktreeNavActivator +} from './worktree-nav-history' + +type MinimalState = Pick< + AppState, + | 'worktreeNavHistory' + | 'worktreeNavHistoryIndex' + | 'isNavigatingHistory' + | 'recordWorktreeVisit' + | 'goBackWorktree' + | 'goForwardWorktree' + | 'worktreesByRepo' +> + +function makeWorktree(id: string): Worktree { + // Only `id` is read by findWorktreeById, which is what the slice uses for + // live-entry checks. Cast covers fields irrelevant to these tests. + return { id } as unknown as Worktree +} + +function createHistoryStore(worktreeIds: string[] = []): StoreApi { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return createStore()((set, get, api) => ({ + worktreesByRepo: { + 'repo-1': worktreeIds.map(makeWorktree) + }, + ...createWorktreeNavHistorySlice( + set as Parameters[0], + get as Parameters[1], + api as Parameters[2] + ) + })) as unknown as StoreApi +} + +describe('worktree-nav-history slice: recordWorktreeVisit', () => { + it('appends new entries and advances the index', () => { + const store = createHistoryStore() + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + store.getState().recordWorktreeVisit('c') + + expect(store.getState().worktreeNavHistory).toEqual(['a', 'b', 'c']) + expect(store.getState().worktreeNavHistoryIndex).toBe(2) + }) + + it('de-dupes only the current entry (A -> B -> A is valid)', () => { + const store = createHistoryStore() + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + store.getState().recordWorktreeVisit('a') + // A repeated activation of the current entry is a no-op + store.getState().recordWorktreeVisit('a') + + expect(store.getState().worktreeNavHistory).toEqual(['a', 'b', 'a']) + expect(store.getState().worktreeNavHistoryIndex).toBe(2) + }) + + it('truncates forward entries when recording from a non-head index', () => { + const store = createHistoryStore(['a', 'b', 'c']) + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + store.getState().recordWorktreeVisit('c') + // Move index back to 'a' (simulate two back presses) + store.setState({ worktreeNavHistoryIndex: 0 }) + // New navigation truncates 'b' and 'c' from the forward stack + store.getState().recordWorktreeVisit('d') + + expect(store.getState().worktreeNavHistory).toEqual(['a', 'd']) + expect(store.getState().worktreeNavHistoryIndex).toBe(1) + }) + + it('caps the history at 50 entries, evicting oldest', () => { + const store = createHistoryStore() + for (let i = 0; i < 60; i++) { + store.getState().recordWorktreeVisit(`w${i}`) + } + const state = store.getState() + expect(state.worktreeNavHistory).toHaveLength(50) + // Oldest 10 are evicted; the head is the most recent. + expect(state.worktreeNavHistory[0]).toBe('w10') + expect(state.worktreeNavHistory[49]).toBe('w59') + expect(state.worktreeNavHistoryIndex).toBe(49) + }) +}) + +describe('worktree-nav-history slice: goBack / goForward', () => { + // Why: reset the module-level activator after every test so a mid-test + // throw cannot leak mock state into sibling tests in the same worker. + afterEach(() => { + setWorktreeNavActivator(null) + }) + + it('moves the index without mutating the history array on success', () => { + const store = createHistoryStore(['a', 'b', 'c']) + // Install activator that simulates a successful activation. + setWorktreeNavActivator(() => ({ primaryTabId: null })) + + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + store.getState().recordWorktreeVisit('c') + + store.getState().goBackWorktree() + expect(store.getState().worktreeNavHistoryIndex).toBe(1) + expect(store.getState().worktreeNavHistory).toEqual(['a', 'b', 'c']) + expect(store.getState().isNavigatingHistory).toBe(false) + + store.getState().goForwardWorktree() + expect(store.getState().worktreeNavHistoryIndex).toBe(2) + }) + + it('leaves the index untouched when activator returns false', () => { + const store = createHistoryStore(['a', 'b', 'c']) + setWorktreeNavActivator(() => false) + + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + + store.getState().goBackWorktree() + expect(store.getState().worktreeNavHistoryIndex).toBe(1) + expect(store.getState().isNavigatingHistory).toBe(false) + }) + + it('skips deleted worktrees when searching for the prev live entry', () => { + // Only 'a' and 'c' remain in worktreesByRepo; 'b' was deleted. + const store = createHistoryStore(['a', 'c']) + const activated: string[] = [] + setWorktreeNavActivator((id) => { + activated.push(id as string) + return { primaryTabId: null } + }) + + store.setState({ + worktreeNavHistory: ['a', 'b', 'c'], + worktreeNavHistoryIndex: 2 + }) + + store.getState().goBackWorktree() + expect(activated).toEqual(['a']) + expect(store.getState().worktreeNavHistoryIndex).toBe(0) + }) + + it('no-ops when the entire direction is dead', () => { + // All prior entries point at deleted worktrees. + const store = createHistoryStore(['c']) + const activated: string[] = [] + setWorktreeNavActivator((id) => { + activated.push(id as string) + return { primaryTabId: null } + }) + + store.setState({ + worktreeNavHistory: ['a', 'b', 'c'], + worktreeNavHistoryIndex: 2 + }) + + store.getState().goBackWorktree() + expect(activated).toEqual([]) + expect(store.getState().worktreeNavHistoryIndex).toBe(2) + }) + + it('two rapid back presses each decrement the index by one', () => { + const store = createHistoryStore(['a', 'b', 'c']) + setWorktreeNavActivator(() => ({ primaryTabId: null })) + + store.getState().recordWorktreeVisit('a') + store.getState().recordWorktreeVisit('b') + store.getState().recordWorktreeVisit('c') + + store.getState().goBackWorktree() + store.getState().goBackWorktree() + + expect(store.getState().worktreeNavHistoryIndex).toBe(0) + expect(store.getState().isNavigatingHistory).toBe(false) + }) +}) + +describe('worktree-nav-history selectors', () => { + it('reports back availability only when a live prior entry exists', () => { + const store = createHistoryStore(['c']) + store.setState({ + worktreeNavHistory: ['a', 'b', 'c'], + worktreeNavHistoryIndex: 2 + }) + + expect(canGoBackWorktreeHistory(store.getState() as AppState)).toBe(false) + + store.setState({ + worktreesByRepo: { + 'repo-1': [makeWorktree('a'), makeWorktree('c')] + } + }) + + expect(canGoBackWorktreeHistory(store.getState() as AppState)).toBe(true) + }) + + it('reports forward availability only when a live next entry exists', () => { + const store = createHistoryStore(['a']) + store.setState({ + worktreeNavHistory: ['a', 'b', 'c'], + worktreeNavHistoryIndex: 0 + }) + + expect(canGoForwardWorktreeHistory(store.getState() as AppState)).toBe(false) + + store.setState({ + worktreesByRepo: { + 'repo-1': [makeWorktree('a'), makeWorktree('c')] + } + }) + + expect(canGoForwardWorktreeHistory(store.getState() as AppState)).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/worktree-nav-history.ts b/src/renderer/src/store/slices/worktree-nav-history.ts new file mode 100644 index 000000000..d7900d9e7 --- /dev/null +++ b/src/renderer/src/store/slices/worktree-nav-history.ts @@ -0,0 +1,170 @@ +import type { StateCreator } from 'zustand' +import type { AppState } from '../types' +import { findWorktreeById } from './worktree-helpers' + +// Why: cap the per-session history so a long-lived workspace with many +// worktree jumps cannot grow the array unbounded. 50 is generous enough +// that the cap is never visible in normal use but small enough that the +// linear skip-deleted scan in goBack/goForward stays trivially cheap. +const MAX_HISTORY = 50 + +export type WorktreeNavHistorySlice = { + // Linear history, oldest -> newest. + worktreeNavHistory: string[] + // Index into worktreeNavHistory; points at the currently-active entry. + // -1 means empty (no worktree ever activated this session). + worktreeNavHistoryIndex: number + // Why: set while goBack/goForward are calling activateAndRevealWorktree so + // the activation path's recordWorktreeVisit step can skip re-recording a + // history-driven navigation. Kept in-store (rather than as a module-level + // mutable) so tests can drive the slice in isolation. + isNavigatingHistory: boolean + + recordWorktreeVisit: (worktreeId: string) => void + goBackWorktree: () => void + goForwardWorktree: () => void +} + +type ActivateFn = (worktreeId: string) => unknown + +// Why: the slice must call activateAndRevealWorktree from goBack/goForward, but +// importing it directly would create a cycle (activation imports the store). +// Install the reference at module init via setWorktreeNavActivator and keep +// the slice itself unaware of the activation module. +let activator: ActivateFn | null = null + +export function setWorktreeNavActivator(fn: ActivateFn | null): void { + activator = fn +} + +export function findPrevLiveWorktreeHistoryIndex(state: AppState): number | null { + for (let i = state.worktreeNavHistoryIndex - 1; i >= 0; i--) { + const id = state.worktreeNavHistory[i] + if (findWorktreeById(state.worktreesByRepo, id)) { + return i + } + } + return null +} + +export function findNextLiveWorktreeHistoryIndex(state: AppState): number | null { + for (let i = state.worktreeNavHistoryIndex + 1; i < state.worktreeNavHistory.length; i++) { + const id = state.worktreeNavHistory[i] + if (findWorktreeById(state.worktreesByRepo, id)) { + return i + } + } + return null +} + +export function canGoBackWorktreeHistory(state: AppState): boolean { + return findPrevLiveWorktreeHistoryIndex(state) !== null +} + +export function canGoForwardWorktreeHistory(state: AppState): boolean { + return findNextLiveWorktreeHistoryIndex(state) !== null +} + +export const createWorktreeNavHistorySlice: StateCreator< + AppState, + [], + [], + WorktreeNavHistorySlice +> = (set, get) => ({ + worktreeNavHistory: [], + worktreeNavHistoryIndex: -1, + isNavigatingHistory: false, + + recordWorktreeVisit: (worktreeId) => { + set((s) => { + // Why: re-activating the same worktree must not pollute history. The + // de-dup applies only to the current entry so that A -> B -> A remains + // a valid stack (user left B, returned to A via the sidebar). + if (s.worktreeNavHistory[s.worktreeNavHistoryIndex] === worktreeId) { + return s + } + + // Truncate any forward entries, then append and advance the index. + const truncated = s.worktreeNavHistory.slice(0, s.worktreeNavHistoryIndex + 1) + truncated.push(worktreeId) + let nextIndex = s.worktreeNavHistoryIndex + 1 + + // Why: cap eviction drops the oldest entries. The index must shift left + // by the same count so it still points at the just-appended current entry. + if (truncated.length > MAX_HISTORY) { + const evict = truncated.length - MAX_HISTORY + truncated.splice(0, evict) + nextIndex = Math.max(0, nextIndex - evict) + } + + return { + worktreeNavHistory: truncated, + worktreeNavHistoryIndex: nextIndex + } + }) + }, + + goBackWorktree: () => { + const state = get() + if (state.worktreeNavHistoryIndex <= 0) { + return + } + const targetIndex = findPrevLiveWorktreeHistoryIndex(state) + if (targetIndex === null) { + return + } + if (!activator) { + // Why: a silent no-op here would mean the back/forward chord simply + // does nothing with no diagnostic. The activator is registered at + // module init by worktree-activation.ts, so a missing activator means + // either test setup forgot to install one or the production import + // graph regressed. + console.warn('goBackWorktree called before worktree activator was registered') + return + } + const targetId = state.worktreeNavHistory[targetIndex] + // Why: capture-and-restore (not force false) so re-entrant navigation + // (e.g. a store subscriber synchronously triggers another goBack) does + // not race on the boolean — the outer call's `finally` restores its own + // prior value rather than clobbering state set by an inner call. + const prevNavigating = get().isNavigatingHistory + set({ isNavigatingHistory: true }) + try { + // Why: activateAndRevealWorktree returns `ActivateAndRevealResult | false`; + // `false` is the only observable failure signal. Advance the index only on + // success so the slice stays consistent with what the user actually sees. + const result = activator(targetId) + if (result !== false) { + set({ worktreeNavHistoryIndex: targetIndex }) + } + } finally { + set({ isNavigatingHistory: prevNavigating }) + } + }, + + goForwardWorktree: () => { + const state = get() + if (state.worktreeNavHistoryIndex >= state.worktreeNavHistory.length - 1) { + return + } + const targetIndex = findNextLiveWorktreeHistoryIndex(state) + if (targetIndex === null) { + return + } + if (!activator) { + console.warn('goForwardWorktree called before worktree activator was registered') + return + } + const targetId = state.worktreeNavHistory[targetIndex] + const prevNavigating = get().isNavigatingHistory + set({ isNavigatingHistory: true }) + try { + const result = activator(targetId) + if (result !== false) { + set({ worktreeNavHistoryIndex: targetIndex }) + } + } finally { + set({ isNavigatingHistory: prevNavigating }) + } + } +}) diff --git a/src/renderer/src/store/types.ts b/src/renderer/src/store/types.ts index ffb821ca5..75abf4b7d 100644 --- a/src/renderer/src/store/types.ts +++ b/src/renderer/src/store/types.ts @@ -14,6 +14,7 @@ import type { RateLimitSlice } from './slices/rate-limits' import type { SshSlice } from './slices/ssh' import type { DiffCommentsSlice } from './slices/diffComments' import type { DetectedAgentsSlice } from './slices/detected-agents' +import type { WorktreeNavHistorySlice } from './slices/worktree-nav-history' export type AppState = RepoSlice & WorktreeSlice & @@ -30,4 +31,5 @@ export type AppState = RepoSlice & RateLimitSlice & SshSlice & DiffCommentsSlice & - DetectedAgentsSlice + DetectedAgentsSlice & + WorktreeNavHistorySlice diff --git a/src/shared/window-shortcut-policy.test.ts b/src/shared/window-shortcut-policy.test.ts index 11584db3f..a7e477b04 100644 --- a/src/shared/window-shortcut-policy.test.ts +++ b/src/shared/window-shortcut-policy.test.ts @@ -103,6 +103,168 @@ describe('resolveWindowShortcutAction', () => { ).toEqual({ type: 'zoom', direction: 'reset' }) }) + it('resolves the worktree-history chord despite carrying Alt', () => { + expect( + resolveWindowShortcutAction( + { + code: 'ArrowLeft', + key: 'ArrowLeft', + meta: true, + control: false, + alt: true, + shift: false + }, + 'darwin' + ) + ).toEqual({ type: 'worktreeHistoryNavigate', direction: 'back' }) + + expect( + resolveWindowShortcutAction( + { + code: 'ArrowRight', + key: 'ArrowRight', + meta: true, + control: false, + alt: true, + shift: false + }, + 'darwin' + ) + ).toEqual({ type: 'worktreeHistoryNavigate', direction: 'forward' }) + + expect( + resolveWindowShortcutAction( + { + code: 'ArrowLeft', + key: 'ArrowLeft', + meta: false, + control: true, + alt: true, + shift: false + }, + 'linux' + ) + ).toEqual({ type: 'worktreeHistoryNavigate', direction: 'back' }) + }) + + it('rejects the history chord when Shift is also held', () => { + expect( + resolveWindowShortcutAction( + { + code: 'ArrowLeft', + key: 'ArrowLeft', + meta: true, + control: false, + alt: true, + shift: true + }, + 'darwin' + ) + ).toBeNull() + }) + + it('leaves Alt+Arrow without a primary modifier untouched (word-nav territory)', () => { + expect( + resolveWindowShortcutAction( + { + code: 'ArrowLeft', + key: 'ArrowLeft', + meta: false, + control: false, + alt: true, + shift: false + }, + 'darwin' + ) + ).toBeNull() + }) + + it('ignores Cmd/Ctrl+Alt combined with ArrowUp or ArrowDown', () => { + // Why: the history predicate explicitly narrows to ArrowLeft/ArrowRight. + // Cmd+Alt+Up / Cmd+Alt+Down must fall through to null so the event + // reaches the renderer/PTTY (e.g. shells / readline). + expect( + resolveWindowShortcutAction( + { + code: 'ArrowUp', + key: 'ArrowUp', + meta: true, + control: false, + alt: true, + shift: false + }, + 'darwin' + ) + ).toBeNull() + + expect( + resolveWindowShortcutAction( + { + code: 'ArrowDown', + key: 'ArrowDown', + meta: false, + control: true, + alt: true, + shift: false + }, + 'linux' + ) + ).toBeNull() + }) + + it('rejects the history chord when the opposite primary modifier is also held', () => { + // Why: Cmd+Ctrl+Alt+Arrow on macOS collides with Mission Control space + // switching; Ctrl+Meta+Alt+Arrow on Linux collides with GNOME workspace + // switching. The app must not intercept either. + expect( + resolveWindowShortcutAction( + { + code: 'ArrowLeft', + key: 'ArrowLeft', + meta: true, + control: true, + alt: true, + shift: false + }, + 'darwin' + ) + ).toBeNull() + + expect( + resolveWindowShortcutAction( + { + code: 'ArrowRight', + key: 'ArrowRight', + meta: true, + control: true, + alt: true, + shift: false + }, + 'linux' + ) + ).toBeNull() + }) + + it('still returns null for other Cmd/Ctrl+Alt combos (not an allowlist escape)', () => { + // Why: regression guard — the history early-return must not swallow + // unrelated primary+alt chords in a way that changes their old null + // result. A future addition that intentionally consumes e.g. Cmd+Alt+KeyT + // must add a new branch explicitly. + expect( + resolveWindowShortcutAction( + { + code: 'KeyB', + key: 'b', + meta: true, + control: false, + alt: true, + shift: false + }, + 'darwin' + ) + ).toBeNull() + }) + it('exposes the shared platform modifier gate used by browser guests', () => { expect( isWindowShortcutModifierChord({ meta: true, control: false, alt: false }, 'darwin') diff --git a/src/shared/window-shortcut-policy.ts b/src/shared/window-shortcut-policy.ts index d5cdeeeac..7c99e7b2b 100644 --- a/src/shared/window-shortcut-policy.ts +++ b/src/shared/window-shortcut-policy.ts @@ -15,13 +15,57 @@ export type WindowShortcutAction = | { type: 'openQuickOpen' } | { type: 'openNewWorkspace' } | { type: 'jumpToWorktreeIndex'; index: number } + | { type: 'worktreeHistoryNavigate'; direction: 'back' | 'forward' } + +function platformPrimaryModifier( + input: Pick, + platform: NodeJS.Platform +): boolean { + return platform === 'darwin' ? Boolean(input.meta) : Boolean(input.control) +} + +function platformOppositeModifier( + input: Pick, + platform: NodeJS.Platform +): boolean { + return platform === 'darwin' ? Boolean(input.control) : Boolean(input.meta) +} export function isWindowShortcutModifierChord( input: Pick, platform: NodeJS.Platform ): boolean { - const modifierPressed = platform === 'darwin' ? input.meta : input.control - return Boolean(modifierPressed) && !input.alt + return platformPrimaryModifier(input, platform) && !input.alt +} + +// Why: worktree history navigation is the first allowlisted chord that +// intentionally carries Alt, so it needs its own predicate. The shared +// isWindowShortcutModifierChord helper deliberately rejects Alt because its +// callers (zoom, sidebar toggles, palette, jump indices) must not steal +// Alt-combinations used by shells and readline. +// +// Why: this predicate also narrows to ArrowLeft/ArrowRight (not just +// "primary+alt") so a future alt-carrying chord added as its own branch in +// resolveWindowShortcutAction is not silently swallowed by the early +// return-null below. Any non-arrow alt combo falls through to the rest of +// the policy, where Alt is rejected by isWindowShortcutModifierChord as +// before. +function isHistoryNavigateChord(input: WindowShortcutInput, platform: NodeJS.Platform): boolean { + // Why: excluding Shift reserves Cmd/Ctrl+Alt+Shift+Arrow for future chords + // (e.g. "close back/forward entry" or cross-stack selection) without + // taking a breaking-change hit on the v1 chord binding. Excluding the + // opposite primary modifier (Ctrl on darwin, Meta on non-darwin) prevents + // Cmd+Ctrl+Alt+Arrow / Win+Ctrl+Alt+Arrow from being mis-classified as + // history navigation — those combinations collide with OS chords + // (macOS Mission Control spaces, GNOME workspace switching) and must + // continue to flow to the OS. + return ( + platformPrimaryModifier(input, platform) && + !platformOppositeModifier(input, platform) && + Boolean(input.alt) && + !input.shift && + (input.code === 'ArrowLeft' || input.code === 'ArrowRight') + ) } function isZoomInShortcut(input: WindowShortcutInput): boolean { @@ -49,6 +93,16 @@ export function resolveWindowShortcutAction( input: WindowShortcutInput, platform: NodeJS.Platform ): WindowShortcutAction | null { + // Why: evaluate the history-navigate chord BEFORE the standard modifier-chord + // gate because that gate rejects Alt. The predicate already narrows to + // ArrowLeft/ArrowRight so only those two codes reach here. + if (isHistoryNavigateChord(input, platform)) { + return { + type: 'worktreeHistoryNavigate', + direction: input.code === 'ArrowLeft' ? 'back' : 'forward' + } + } + if (!isWindowShortcutModifierChord(input, platform)) { return null } diff --git a/tests/e2e/workspace-back-forward-navigation.spec.ts b/tests/e2e/workspace-back-forward-navigation.spec.ts new file mode 100644 index 000000000..4867b0ca4 --- /dev/null +++ b/tests/e2e/workspace-back-forward-navigation.spec.ts @@ -0,0 +1,265 @@ +/** + * E2E tests for the workspace Back / Forward titlebar buttons + their + * Cmd/Ctrl+Alt+Arrow shortcuts. + * + * Covers edge cases that unit tests cannot exercise: + * - Button DOM disabled/hidden states across view transitions. + * - De-dup: re-activating the current worktree does not grow history. + * - Forward-stack truncation after a mid-history activation. + * - Keyboard shortcuts fire the same back/forward path as clicks. + * - Shortcuts no-op in non-terminal views (buttons also hidden there). + */ + +import { test, expect } from './helpers/orca-app' +import type { Page } from '@stablyai/playwright-test' +import { + waitForSessionReady, + waitForActiveWorktree, + getActiveWorktreeId, + getAllWorktreeIds, + ensureTerminalVisible +} from './helpers/store' + +/** + * Record a visit through the same two store calls that + * `activateAndRevealWorktree` makes for the history slice, without having to + * expose the activation helper on window. This mirrors production ordering: + * `setActiveWorktree` first, then `recordWorktreeVisit` (skipped by the real + * helper when `isNavigatingHistory` is true — not relevant for seeding). + */ +async function seedVisit(page: Page, worktreeId: string): Promise { + await page.evaluate((id) => { + // Why: window.__store is typed minimally in runtime-types.ts. At runtime it + // is the full Zustand vanilla store (setState/getState/subscribe), so we + // widen to call the nav-history slice actions directly. + type StoreLike = { + getState: () => { + setActiveWorktree: (worktreeId: string) => void + recordWorktreeVisit: (worktreeId: string) => void + } + } + const store = window.__store as unknown as StoreLike + const state = store.getState() + state.setActiveWorktree(id) + state.recordWorktreeVisit(id) + }, worktreeId) +} + +async function getNavHistorySnapshot(page: Page): Promise<{ history: string[]; index: number }> { + return page.evaluate(() => { + type StoreLike = { + getState: () => { + worktreeNavHistory: string[] + worktreeNavHistoryIndex: number + } + } + const store = window.__store as unknown as StoreLike + const state = store.getState() + return { + history: [...state.worktreeNavHistory], + index: state.worktreeNavHistoryIndex + } + }) +} + +async function resetNavHistory(page: Page): Promise { + await page.evaluate(() => { + type StoreLike = { + setState: (partial: { worktreeNavHistory: string[]; worktreeNavHistoryIndex: number }) => void + } + const store = window.__store as unknown as StoreLike + store.setState({ worktreeNavHistory: [], worktreeNavHistoryIndex: -1 }) + }) +} + +async function getBackButton(page: Page) { + return page.getByRole('button', { name: 'Go back' }) +} + +async function getForwardButton(page: Page) { + return page.getByRole('button', { name: 'Go forward' }) +} + +const isMac = process.platform === 'darwin' +const mod = isMac ? 'Meta' : 'Control' + +test.describe('Workspace Back/Forward Navigation', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + }) + + test('buttons are hidden outside the terminal view', async ({ orcaPage }) => { + await expect(await getBackButton(orcaPage)).toBeVisible() + await expect(await getForwardButton(orcaPage)).toBeVisible() + + // Why: the Back/Forward pair is conditional on `activeView === 'terminal'`. + // Settings, Tasks, and Landing must not render the buttons at all (not just + // disable them) so the titlebar stays compact and the semantics unambiguous. + await orcaPage.evaluate(() => { + window.__store!.getState().openSettingsPage() + }) + + await expect(await getBackButton(orcaPage)).toHaveCount(0) + await expect(await getForwardButton(orcaPage)).toHaveCount(0) + + await orcaPage.evaluate(() => { + window.__store!.getState().setActiveView('terminal') + }) + await expect(await getBackButton(orcaPage)).toBeVisible() + }) + + test('both buttons disabled at cold start with a single history entry', async ({ orcaPage }) => { + // The test fixture already activated a worktree during setup, so one entry + // may or may not exist. Reset the slice to a known empty baseline, then + // record the current worktree as the single entry. + const activeId = await getActiveWorktreeId(orcaPage) + expect(activeId).not.toBeNull() + + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, activeId!) + + const back = await getBackButton(orcaPage) + const forward = await getForwardButton(orcaPage) + await expect(back).toBeDisabled() + await expect(forward).toBeDisabled() + }) + + test('clicking Back and Forward walks the history stack', async ({ orcaPage }) => { + const worktreeIds = await getAllWorktreeIds(orcaPage) + test.skip(worktreeIds.length < 2, 'Need at least two worktrees to exercise back/forward') + const [primaryId, secondaryId] = worktreeIds + + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, primaryId) + await seedVisit(orcaPage, secondaryId) + + const back = await getBackButton(orcaPage) + const forward = await getForwardButton(orcaPage) + await expect(back).toBeEnabled() + await expect(forward).toBeDisabled() + + await back.click() + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { + message: 'Back click did not activate the previous worktree' + }) + .toBe(primaryId) + await expect(back).toBeDisabled() + await expect(forward).toBeEnabled() + + await forward.click() + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { + message: 'Forward click did not re-activate the next worktree' + }) + .toBe(secondaryId) + await expect(forward).toBeDisabled() + }) + + test('re-activating the current worktree is a no-op (dedupe)', async ({ orcaPage }) => { + const activeId = await getActiveWorktreeId(orcaPage) + expect(activeId).not.toBeNull() + + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, activeId!) + await seedVisit(orcaPage, activeId!) + await seedVisit(orcaPage, activeId!) + + const snapshot = await getNavHistorySnapshot(orcaPage) + expect(snapshot.history).toEqual([activeId]) + expect(snapshot.index).toBe(0) + await expect(await getBackButton(orcaPage)).toBeDisabled() + }) + + test('new navigation after going back truncates the forward stack', async ({ orcaPage }) => { + const worktreeIds = await getAllWorktreeIds(orcaPage) + test.skip(worktreeIds.length < 2, 'Need at least two worktrees to exercise forward truncation') + const [primaryId, secondaryId] = worktreeIds + + // Stack: primary -> secondary. Go back to primary, then "activate" primary + // again via a fresh visit (simulating a sidebar click on the same entry + // from mid-history). The current-entry dedupe should kick in, but if we + // instead activate secondary while sitting on primary mid-history, the + // forward entry pointing at secondary must be truncated. + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, primaryId) + await seedVisit(orcaPage, secondaryId) + await (await getBackButton(orcaPage)).click() + await expect.poll(() => getActiveWorktreeId(orcaPage)).toBe(primaryId) + + // Forward button is live — a forward entry exists. + await expect(await getForwardButton(orcaPage)).toBeEnabled() + + // Fresh activation from mid-history. Using secondary again is the simplest + // way to prove truncation happened: after this call, the stack must be + // [primary, secondary] with index=1, so Forward is disabled even though + // there *was* a forward entry moments ago. + await seedVisit(orcaPage, secondaryId) + const snapshot = await getNavHistorySnapshot(orcaPage) + expect(snapshot.history).toEqual([primaryId, secondaryId]) + expect(snapshot.index).toBe(1) + await expect(await getForwardButton(orcaPage)).toBeDisabled() + }) + + test(`${isMac ? 'Cmd' : 'Ctrl'}+Alt+Left/Right shortcuts walk history`, async ({ orcaPage }) => { + const worktreeIds = await getAllWorktreeIds(orcaPage) + test.skip(worktreeIds.length < 2, 'Need at least two worktrees to exercise shortcuts') + const [primaryId, secondaryId] = worktreeIds + + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, primaryId) + await seedVisit(orcaPage, secondaryId) + + // Why: focus body so the window-level keydown capture handler runs without + // an `isEditableTarget` bail-out. The xterm helper textarea is explicitly + // treated as non-editable, but body is the simplest stable target in a + // hidden-window Electron run. + await orcaPage.evaluate(() => document.body.focus()) + + await orcaPage.keyboard.press(`${mod}+Alt+ArrowLeft`) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { + message: `${mod}+Alt+Left did not navigate back` + }) + .toBe(primaryId) + + await orcaPage.keyboard.press(`${mod}+Alt+ArrowRight`) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { + message: `${mod}+Alt+Right did not navigate forward` + }) + .toBe(secondaryId) + }) + + test('shortcut is a no-op in settings view', async ({ orcaPage }) => { + const worktreeIds = await getAllWorktreeIds(orcaPage) + test.skip(worktreeIds.length < 2, 'Need at least two worktrees to exercise settings gating') + const [primaryId, secondaryId] = worktreeIds + + await resetNavHistory(orcaPage) + await seedVisit(orcaPage, primaryId) + await seedVisit(orcaPage, secondaryId) + + // Enter settings. The back shortcut must not change the active worktree, + // matching the view-guard in App.tsx and useIpcEvents.ts. + await orcaPage.evaluate(() => { + window.__store!.getState().openSettingsPage() + }) + await expect + .poll(async () => orcaPage.evaluate(() => window.__store!.getState().activeView)) + .toBe('settings') + + const idBefore = await getActiveWorktreeId(orcaPage) + await orcaPage.evaluate(() => document.body.focus()) + await orcaPage.keyboard.press(`${mod}+Alt+ArrowLeft`) + + // Give any erroneous nav a beat to land, then assert the active worktree + // and the slice index both stayed put. + await orcaPage.waitForTimeout(150) + expect(await getActiveWorktreeId(orcaPage)).toBe(idBefore) + const snapshot = await getNavHistorySnapshot(orcaPage) + expect(snapshot.index).toBe(1) + }) +})