fix(terminal): gate the developer menu behind Option and unblock manual parking (#11091)

The Developer submenu shipped visible on every worktree right-click, and its
Park terminal action always refused with "These terminals cannot be parked
safely."

- reveal the Developer submenu only when Option/Alt is held at right-click,
  captured at open time so it can't shift rows mid-menu
- stop a settled pendingActivationSpawn tag from refusing a manual park: first
  activation stamps it on every tab and only a fresh updateTabPtyId consumes it,
  so a reattached tab kept it forever
- resolve the single leaf of a rootless layout for parked watcher coverage, so a
  workspace whose panes never mounted is no longer permanently uncoverable
- restore the !isVisible park guard dropped in #11016, which let the workspace
  being viewed unmount its own terminals
- split manual-park eligibility out of the automatic cold-park policy module
This commit is contained in:
Neil
2026-07-28 17:15:40 -07:00
committed by Brennan Benson
parent bf27227437
commit 7cc453e68b
11 changed files with 356 additions and 59 deletions
+7 -2
View File
@@ -1101,7 +1101,9 @@ function Terminal(): React.JSX.Element | null {
const shouldMeasureHiddenWorktree =
!isVisible && measurableBackgroundWorktreeIdsRef.current.has(workspace.id)
const parked =
!shouldMeasureHiddenWorktree && effectiveParkedTerminalWorktreeIds.has(workspace.id)
!isVisible &&
!shouldMeasureHiddenWorktree &&
effectiveParkedTerminalWorktreeIds.has(workspace.id)
if (parked) {
for (const tab of tabs) {
const activityTerminalPortal = findActivityTerminalPortal(activityTerminalPortals, {
@@ -2120,7 +2122,9 @@ function Terminal(): React.JSX.Element | null {
const shouldMeasureHiddenWorktree =
!isVisible && measurableBackgroundWorktreeIdsRef.current.has(workspace.id)
const shouldColdParkTerminalPanes =
!shouldMeasureHiddenWorktree && effectiveParkedTerminalWorktreeIds.has(workspace.id)
!isVisible &&
!shouldMeasureHiddenWorktree &&
effectiveParkedTerminalWorktreeIds.has(workspace.id)
return (
<WorktreeSplitSurface
key={`tab-groups-${workspace.id}`}
@@ -2170,6 +2174,7 @@ function Terminal(): React.JSX.Element | null {
const shouldMeasureHiddenWorktree =
!isVisible && measurableBackgroundWorktreeIdsRef.current.has(workspace.id)
const shouldColdParkTerminalPanes =
!isVisible &&
!shouldMeasureHiddenWorktree &&
effectiveParkedTerminalWorktreeIds.has(workspace.id)
return (
@@ -12,10 +12,33 @@ import {
hasWorktreeParentLink,
isWorktreeParentPickerDisabled,
planWorkspaceStatusAssignment,
selectMenuScopedMap
selectMenuScopedMap,
shouldRevealWorktreeDeveloperMenu
} from './WorktreeContextMenu'
import type { Worktree, WorktreeLineage, WorkspaceStatusDefinition } from '../../../../shared/types'
describe('shouldRevealWorktreeDeveloperMenu', () => {
it('stays hidden for an ordinary right-click', () => {
expect(
shouldRevealWorktreeDeveloperMenu({ developerMenuRevealed: false, isMultiContext: false })
).toBe(false)
})
it('reveals when Option/Alt was held at open time', () => {
expect(
shouldRevealWorktreeDeveloperMenu({ developerMenuRevealed: true, isMultiContext: false })
).toBe(true)
})
// Why: the parking action targets one workspace, so it must not appear for a
// multi-select context even with the modifier held.
it('stays hidden for a multi-workspace selection', () => {
expect(
shouldRevealWorktreeDeveloperMenu({ developerMenuRevealed: true, isMultiContext: true })
).toBe(false)
})
})
describe('selectMenuScopedMap (delete-teardown re-render guard)', () => {
// Why: the closed menu wrapper must stay inert to delete teardown's high-churn
// set()s. The guard is referential stability — when closed, the selector must
@@ -104,6 +104,16 @@ export function selectMenuScopedMap<T>(menuOpen: boolean, live: T, empty: T): T
return menuOpen ? live : empty
}
// Why: the Developer submenu is hidden by default and revealed only by holding
// Option/Alt at right-click. altKey is the same physical key on every platform
// (Option on macOS, Alt on Windows/Linux), so no platform branch is needed.
export function shouldRevealWorktreeDeveloperMenu(args: {
developerMenuRevealed: boolean
isMultiContext: boolean
}): boolean {
return args.developerMenuRevealed && !args.isMultiContext
}
export function hasWorktreeParentLink(
worktree: Worktree,
lineageById: AppState['worktreeLineageById'],
@@ -331,6 +341,11 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
const repo = useRepoById(worktree.repoId)
const deleteState = useAppStore((s) => s.deleteStateByWorktreeId[worktree.id])
const [menuOpen, setMenuOpen] = useState(false)
// Why: the Developer submenu is a power-user affordance, so it is revealed by
// holding Option/Alt at right-click — captured at open time (like the Help
// menu's admin options) so the submenu can't appear or vanish mid-menu and
// shift the rows under the pointer.
const [developerMenuRevealed, setDeveloperMenuRevealed] = useState(false)
const [menuPoint, setMenuPoint] = useState({ x: 0, y: 0 })
const [contextWorktrees, setContextWorktrees] = useState<readonly Worktree[]>(
effectiveSelectedWorktrees
@@ -463,6 +478,10 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
const setMenuOpenState = useCallback(
(open: boolean) => {
setMenuOpen(open)
if (!open) {
// Why: the reveal is per-open, so a later plain right-click can't inherit it.
setDeveloperMenuRevealed(false)
}
onOpenChange?.(open)
},
[onOpenChange]
@@ -731,6 +750,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
event.preventDefault()
contextMenuOpenedAtRef.current = Date.now()
window.dispatchEvent(new Event(CLOSE_ALL_CONTEXT_MENUS_EVENT))
setDeveloperMenuRevealed(event.altKey)
setContextWorktrees(onContextMenuSelect?.(event) ?? effectiveSelectedWorktrees)
const bounds = event.currentTarget.getBoundingClientRect()
setMenuPoint({ x: event.clientX - bounds.left, y: event.clientY - bounds.top })
@@ -925,7 +945,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
</>
) : null}
{!isMultiContext ? (
{shouldRevealWorktreeDeveloperMenu({ developerMenuRevealed, isMultiContext }) ? (
<>
<WorktreeDeveloperMenu worktreeId={worktree.id} disabled={isDeleting} />
<DropdownMenuSeparator />
@@ -0,0 +1,139 @@
// @vitest-environment happy-dom
/**
* Why a render test on top of shouldRevealWorktreeDeveloperMenu's unit tests:
* the shipped bug was the submenu rendering unconditionally, i.e. a WIRING
* defect. These assert the modifier actually reaches the reveal state through
* the real right-click handler.
*/
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import type { ReactNode } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { Worktree } from '../../../../shared/types'
vi.mock('@/store', () => {
const state = {
tabsByWorktree: {},
ptyIdsByTabId: {},
browserTabsByWorktree: {},
deleteStateByWorktreeId: {},
worktreeLineageById: {},
workspaceLineageByChildKey: {},
workspaceStatuses: [],
projectGroups: [],
settings: null,
updateWorktreeMeta: vi.fn(),
setWorktreesPinnedAndReveal: vi.fn(),
openModal: vi.fn(),
createProjectGroup: vi.fn(),
moveProjectToGroup: vi.fn(),
deleteFolderWorkspace: vi.fn(),
setActiveWorktree: vi.fn()
}
return {
useAppStore: Object.assign((selector: (value: typeof state) => unknown) => selector(state), {
getState: () => state
})
}
})
vi.mock('@/store/selectors', () => ({
useAllWorktrees: () => [],
useRepoById: () => null,
useRepoMap: () => new Map(),
useWorktreeMap: () => new Map()
}))
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
vi.mock('@/components/ui/tooltip', () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: () => null,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
// Why: Radix portals/submenus need real layout; a passthrough keeps the test on
// the reveal decision rather than menu mechanics.
vi.mock('@/components/ui/dropdown-menu', () => {
const passthrough = ({ children }: { children?: ReactNode }) => <>{children}</>
return {
DropdownMenu: passthrough,
DropdownMenuContent: passthrough,
DropdownMenuItem: passthrough,
DropdownMenuLabel: passthrough,
DropdownMenuRadioGroup: passthrough,
DropdownMenuRadioItem: passthrough,
DropdownMenuSeparator: () => null,
DropdownMenuSub: passthrough,
DropdownMenuSubContent: passthrough,
DropdownMenuSubTrigger: passthrough,
DropdownMenuTrigger: passthrough
}
})
vi.mock('./WorktreeOpenInMenu', () => ({ WorktreeOpenInSubMenu: () => null }))
vi.mock('./ProjectGroupNameDialog', () => ({ ProjectGroupNameDialog: () => null }))
vi.mock('./WorktreeParentPickerPopover', () => ({ WorktreeParentPickerPopover: () => null }))
vi.mock('@/lib/worktree-activation', () => ({ activateAndRevealWorktree: vi.fn() }))
vi.mock('./delete-worktree-flow', () => ({
runWorktreeBatchDelete: vi.fn(),
runWorktreeDelete: vi.fn()
}))
vi.mock('./sleep-worktree-flow', () => ({ runSleepWorktrees: vi.fn() }))
const WorktreeContextMenu = (await import('./WorktreeContextMenu')).default
function makeWorktree(): Worktree {
return {
id: 'repo-1::/repo/wt',
repoId: 'repo-1',
path: '/repo/wt',
displayName: 'wt',
branch: 'wt',
head: 'abc123',
isBare: false,
isMainWorktree: false,
comment: '',
linkedIssue: null,
linkedPR: null,
linkedLinearIssue: null,
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 0,
lastActivityAt: 0
} as Worktree
}
// Why: assert on text content — the passthrough menu mock renders each label as
// a bare text node beside its icon, which getByText's element matching skips.
function openContextMenu(altKey: boolean): string {
const { container } = render(
<WorktreeContextMenu worktree={makeWorktree()}>
<div data-testid="card">card</div>
</WorktreeContextMenu>
)
fireEvent.contextMenu(screen.getByTestId('card'), { altKey })
return container.textContent ?? ''
}
describe('Developer submenu reveal', () => {
afterEach(() => {
cleanup()
vi.clearAllMocks()
})
it('hides the Developer submenu on a plain right-click', () => {
const text = openContextMenu(false)
expect(text).toContain('Sleep')
expect(text).not.toContain('Developer')
expect(text).not.toContain('Park terminal')
})
it('reveals the Developer submenu when Option/Alt is held', () => {
const text = openContextMenu(true)
expect(text).toContain('Developer')
expect(text).toContain('Park terminal')
})
})
@@ -0,0 +1,63 @@
import { describe, expect, it } from 'vitest'
import { canManuallyParkTerminalWorktreeRenderers } from './manual-terminal-worktree-park-eligibility'
describe('canManuallyParkTerminalWorktreeRenderers', () => {
const base = {
worktreeId: 'repo::/worktree',
terminalTabs: [{ id: 'tab-1', ptyId: 'repo::/worktree@@session-1' }],
pendingStartupByTabId: {},
parkingEnabled: true,
hasLivePty: () => true
}
it('bypasses visibility timing for snapshot-backed local terminals', () => {
expect(canManuallyParkTerminalWorktreeRenderers(base)).toBe(true)
})
// Why: first activation stamps pendingActivationSpawn on every tab and only a
// fresh updateTabPtyId consumes it, so a reattached tab keeps it forever —
// that residue must not permanently refuse a manual park.
it('ignores a settled activation-spawn tag once the tab has a live PTY', () => {
const terminalTabs = [
{ id: 'tab-1', ptyId: 'repo::/worktree@@session-1', pendingActivationSpawn: true as const }
]
expect(canManuallyParkTerminalWorktreeRenderers({ ...base, terminalTabs })).toBe(true)
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
terminalTabs,
hasLivePty: () => false
})
).toBe(false)
})
it('still refuses a multi-pane activation spawn that has not finished', () => {
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
terminalTabs: [
{ id: 'tab-1', ptyId: 'repo::/worktree@@session-1', pendingActivationSpawn: 2 }
],
hasLivePty: () => false
})
).toBe(false)
})
it('preserves safety gates for empty, pending, remote, and disabled terminals', () => {
expect(canManuallyParkTerminalWorktreeRenderers({ ...base, terminalTabs: [] })).toBe(false)
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
pendingStartupByTabId: { 'tab-1': ['echo', 'pending'] }
})
).toBe(false)
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
terminalTabs: [{ id: 'tab-1', ptyId: 'ssh:ssh-1@@pty-1' }]
})
).toBe(false)
expect(canManuallyParkTerminalWorktreeRenderers({ ...base, parkingEnabled: false })).toBe(false)
})
})
@@ -0,0 +1,56 @@
/**
* Eligibility for the on-demand ("Park terminal") path.
*
* Why split from terminal-hidden-view-parking: that module owns the automatic
* cold-park policy — the hysteresis clock, hot-retain cap, and recheck
* deadlines. Manual parking reuses its safety gates but deliberately bypasses
* every time-based one, so the divergence lives here instead of accruing
* `nowMs: 0` special cases inside the policy module.
*/
import {
canParkTerminalWorktreeRenderers,
type ColdParkableTerminalTab
} from './terminal-hidden-view-parking'
// Why: pendingActivationSpawn is sidebar-sort suppression, not a spawn-in-flight
// signal. First activation stamps it on every tab and only a fresh
// updateTabPtyId consumes it, so a tab whose PTY was reattached (not respawned)
// keeps it forever. The automatic path can wait it out; a manual park must not
// be refused by that residue, so drop it wherever the tab already has a live
// PTY — the only trustworthy "spawn settled" signal (tab.ptyId is a wake hint).
function withoutSettledActivationSpawn(
tabs: readonly ColdParkableTerminalTab[],
hasLivePty: (tabId: string) => boolean
): ColdParkableTerminalTab[] {
return tabs.map((tab) => {
if (!tab.pendingActivationSpawn || !hasLivePty(tab.id)) {
return tab
}
const { pendingActivationSpawn: _settled, ...rest } = tab
return rest
})
}
export function canManuallyParkTerminalWorktreeRenderers(args: {
worktreeId: string
terminalTabs: readonly ColdParkableTerminalTab[]
pendingStartupByTabId: Readonly<Record<string, unknown>>
parkingEnabled: boolean
hasLivePty: (tabId: string) => boolean
}): boolean {
return (
args.terminalTabs.length > 0 &&
canParkTerminalWorktreeRenderers({
...args,
terminalTabs: withoutSettledActivationSpawn(args.terminalTabs, args.hasLivePty),
// Why: the user asked for this park explicitly, so skip the hidden-duration
// hysteresis the automatic path uses — but keep every safety gate below it.
isVisible: false,
shouldMeasureHiddenWorktree: false,
hasActivityTerminalPortal: false,
hiddenSinceMs: 0,
nowMs: 0,
coldParkDelayMs: 0
})
)
}
@@ -7,7 +7,6 @@ import {
TERMINAL_TAB_HOT_RETAIN_MS,
TERMINAL_WORKTREE_HOT_RETAIN_MS,
TERMINAL_WORKTREE_PARK_DELAY_MS,
canManuallyParkTerminalWorktreeRenderers,
canParkTerminalTabRenderer,
canParkTerminalWorktreeRenderers,
getTerminalTabColdParkRecheckDelayMs,
@@ -182,36 +181,6 @@ describe('canParkTerminalTabRenderer', () => {
})
})
describe('canManuallyParkTerminalWorktreeRenderers', () => {
const base = {
worktreeId: 'repo::/worktree',
terminalTabs: [{ id: 'tab-1', ptyId: 'repo::/worktree@@session-1' }],
pendingStartupByTabId: {},
parkingEnabled: true
}
it('bypasses visibility timing for snapshot-backed local terminals', () => {
expect(canManuallyParkTerminalWorktreeRenderers(base)).toBe(true)
})
it('preserves safety gates for empty, pending, remote, and disabled terminals', () => {
expect(canManuallyParkTerminalWorktreeRenderers({ ...base, terminalTabs: [] })).toBe(false)
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
pendingStartupByTabId: { 'tab-1': ['echo', 'pending'] }
})
).toBe(false)
expect(
canManuallyParkTerminalWorktreeRenderers({
...base,
terminalTabs: [{ id: 'tab-1', ptyId: 'ssh:ssh-1@@pty-1' }]
})
).toBe(false)
expect(canManuallyParkTerminalWorktreeRenderers({ ...base, parkingEnabled: false })).toBe(false)
})
})
describe('selectColdParkedTerminalWorktrees', () => {
const nowMs = 500_000
@@ -107,26 +107,6 @@ export function canParkTerminalWorktreeRenderers(args: {
})
}
export function canManuallyParkTerminalWorktreeRenderers(args: {
worktreeId: string
terminalTabs: readonly ColdParkableTerminalTab[]
pendingStartupByTabId: Readonly<Record<string, unknown>>
parkingEnabled: boolean
}): boolean {
return (
args.terminalTabs.length > 0 &&
canParkTerminalWorktreeRenderers({
...args,
isVisible: false,
shouldMeasureHiddenWorktree: false,
hasActivityTerminalPortal: false,
hiddenSinceMs: 0,
nowMs: 0,
coldParkDelayMs: 0
})
)
}
export function canParkTerminalTabRenderer(args: {
worktreeId: string
terminalTab: TerminalTabColdParkCandidate
@@ -764,6 +764,36 @@ describe('fallbackParkedPaneCandidates', () => {
).toEqual([{ ptyId: PTY_ID, paneId: 7, leafId: LEAF_ID, drivesTabTitle: true }])
})
// Why: a tab that never mounted a pane persists a rootless layout. Walking
// only `root` yielded zero candidates, so watcher coverage refused it and a
// manual park could never succeed for a workspace the user had not visited.
it('resolves the single leaf of a rootless layout', () => {
expect(
fallbackParkedPaneCandidates({ id: TAB_ID, ptyId: PTY_ID }, {
terminalLayoutsByTabId: {
[TAB_ID]: {
root: null,
activeLeafId: LEAF_ID,
expandedLeafId: null,
ptyIdsByLeafId: { [LEAF_ID]: PTY_ID }
}
},
runtimePaneTitlesByTabId: {}
} as never)
).toEqual([{ ptyId: PTY_ID, paneId: -1, leafId: LEAF_ID, drivesTabTitle: true }])
})
it('returns nothing for a rootless layout with no resolvable leaf', () => {
expect(
fallbackParkedPaneCandidates({ id: TAB_ID, ptyId: PTY_ID }, {
terminalLayoutsByTabId: {
[TAB_ID]: { root: null, activeLeafId: null, expandedLeafId: null }
},
runtimePaneTitlesByTabId: {}
} as never)
).toEqual([])
})
it('maps split leaves to layout PTYs with collision-free negative pane ids', () => {
expect(
fallbackParkedPaneCandidates({ id: TAB_ID, ptyId: PTY_ID }, {
@@ -9,7 +9,10 @@ import { isTerminalLeafId } from '../../../../shared/stable-pane-id'
import type { TerminalTab } from '../../../../shared/types'
import { useAppStore } from '@/store'
import { closeTerminalTab } from '../terminal/terminal-tab-actions'
import { collectLeafIdsInOrder } from './terminal-layout-leaf-ids'
import {
collectLeafIdsInOrder,
resolveRootlessTerminalLayoutLeafId
} from './terminal-layout-leaf-ids'
import { detachTerminalLayoutLeaf } from './terminal-layout-leaf-detach'
import { subscribeToPtyExit } from './pty-dispatcher'
import { discardPreHandlerPtyState } from './pty-pre-handler-buffer'
@@ -54,7 +57,14 @@ export function fallbackParkedPaneCandidates(
state: ParkedPaneFallbackState
): ParkedTerminalPaneCapture[] {
const layout = state.terminalLayoutsByTabId[tab.id]
const leafIds = collectLeafIdsInOrder(layout?.root)
// Why: a tab that never mounted a pane persists a rootless layout, so there is
// no root to walk. replayTerminalLayout resolves the single leaf for that same
// shape — match it here, or such a tab is permanently uncoverable and can
// never park.
const rootLeafIds = collectLeafIdsInOrder(layout?.root)
const rootlessLeafId = layout ? resolveRootlessTerminalLayoutLeafId(layout) : null
const leafIds =
rootLeafIds.length > 0 ? rootLeafIds : rootlessLeafId !== null ? [rootlessLeafId] : []
if (leafIds.length === 0) {
return []
}
@@ -2,13 +2,14 @@ import { useCallback, useEffect, useState } from 'react'
import { toast } from 'sonner'
import { useAppStore } from '@/store'
import { translate } from '@/i18n/i18n'
import { tabHasLivePty } from '@/lib/tab-has-live-pty'
import {
MANUAL_TERMINAL_WORKTREE_PARK_EVENT,
takeAllPendingManualTerminalWorktreeParks,
takePendingManualTerminalWorktreePark,
type ManualTerminalWorktreeParkDetail
} from '@/lib/manual-terminal-worktree-parking'
import { canManuallyParkTerminalWorktreeRenderers } from './terminal-hidden-view-parking'
import { canManuallyParkTerminalWorktreeRenderers } from './manual-terminal-worktree-park-eligibility'
import { canWatcherCoverParkedTerminalTab } from './terminal-parked-tab-watchers'
function addWorktreeId(current: ReadonlySet<string>, worktreeId: string): ReadonlySet<string> {
@@ -53,7 +54,8 @@ export function useManualTerminalWorktreeParking(args: {
worktreeId,
terminalTabs,
pendingStartupByTabId: state.pendingStartupByTabId,
parkingEnabled: state.settings?.terminalHiddenViewParking !== false
parkingEnabled: state.settings?.terminalHiddenViewParking !== false,
hasLivePty: (tabId) => tabHasLivePty(state.ptyIdsByTabId, tabId)
}) && terminalTabs.every((tab) => canWatcherCoverParkedTerminalTab(worktreeId, tab))
if (!canPark) {
toast.warning(