Fix child workspace context menus

Co-authored-by: Orca <help@stably.ai>
2026-10-13 00:02:54 +00:00 · 2026-05-15 14:06:59 -07:00
parent 6485f87932
commit 4a66e9f6ca
5 changed files with 272 additions and 107 deletions
@@ -759,7 +759,7 @@ const WorktreeCard = React.memo(function WorktreeCard({
          </div>
        )}

-        {lineageChildren && <div className="mt-1.5 space-y-1">{lineageChildren}</div>}
+        {lineageChildren && <div className="-ml-4 mt-1.5 space-y-1">{lineageChildren}</div>}
      </div>
    </div>
  )
@@ -0,0 +1,61 @@
+import { describe, expect, it } from 'vitest'
+import {
+  shouldIgnoreNestedWorktreeContextMenuScope,
+  shouldSuppressContextMenuFollowUpClick
+} from './WorktreeContextMenu'
+
+describe('shouldIgnoreNestedWorktreeContextMenuScope', () => {
+  it('allows the context menu scope that owns the event target', () => {
+    const currentScope = {} as EventTarget
+    const target = {
+      closest: () => currentScope
+    } as unknown as EventTarget
+
+    expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(false)
+  })
+
+  it('ignores context menu events owned by a nested scope', () => {
+    const currentScope = {} as EventTarget
+    const nestedScope = {} as Element
+    const target = {
+      closest: () => nestedScope
+    } as unknown as EventTarget
+
+    expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(true)
+  })
+
+  it('ignores context menu events from text nodes inside a nested scope', () => {
+    const currentScope = {} as EventTarget
+    const nestedScope = {} as Element
+    const target = {
+      parentElement: {
+        closest: () => nestedScope
+      }
+    } as unknown as EventTarget
+
+    expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(true)
+  })
+
+  it('allows events from unscoped targets', () => {
+    const currentScope = {} as EventTarget
+    const target = {
+      closest: () => null
+    } as unknown as EventTarget
+
+    expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(false)
+  })
+})
+
+describe('shouldSuppressContextMenuFollowUpClick', () => {
+  it('suppresses the click emitted immediately after opening a context menu', () => {
+    expect(shouldSuppressContextMenuFollowUpClick(1_000, 1_050)).toBe(true)
+  })
+
+  it('does not suppress later unrelated clicks', () => {
+    expect(shouldSuppressContextMenuFollowUpClick(1_000, 1_700)).toBe(false)
+  })
+
+  it('does not suppress clicks that predate the context menu timestamp', () => {
+    expect(shouldSuppressContextMenuFollowUpClick(1_000, 999)).toBe(false)
+  })
+})
@@ -1,4 +1,5 @@
-import React, { useCallback, useEffect, useMemo, useState } from 'react'
+/* eslint-disable max-lines -- Why: this menu keeps row targeting, batch actions, and ctrl-click event guards together so nested worktree menus share one event policy. */
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import {
  DropdownMenu,
  DropdownMenuContent,
@@ -42,6 +43,30 @@ type Props = {

 const CLOSE_ALL_CONTEXT_MENUS_EVENT = 'orca-close-all-context-menus'
 const WORKTREE_CONTEXT_MENU_SCOPE_ATTR = 'data-worktree-context-menu-scope'
+const CONTEXT_MENU_CLICK_SUPPRESSION_MS = 500
+
+function shouldIgnoreNestedWorktreeContextMenuScope(
+  currentTarget: EventTarget,
+  target: EventTarget | null
+): boolean {
+  const maybeScopedTarget = target as {
+    closest?: (selector: string) => Element | null
+    parentElement?: { closest?: (selector: string) => Element | null }
+  } | null
+  const scopeSelector = `[${WORKTREE_CONTEXT_MENU_SCOPE_ATTR}]`
+  const closestScope =
+    maybeScopedTarget?.closest?.(scopeSelector) ??
+    maybeScopedTarget?.parentElement?.closest?.(scopeSelector)
+  // Why: lineage child previews live inside the parent card DOM but own their
+  // context menu target. The parent must ignore only those nested scopes.
+  return closestScope != null && closestScope !== currentTarget
+}
+
+function shouldSuppressContextMenuFollowUpClick(contextMenuOpenedAt: number, now: number): boolean {
+  return (
+    now - contextMenuOpenedAt >= 0 && now - contextMenuOpenedAt <= CONTEXT_MENU_CLICK_SUPPRESSION_MS
+  )
+}

 const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
  worktree,
@@ -61,13 +86,13 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
  const isFolder = repo ? isFolderRepo(repo) : false
  const repoMap = useRepoMap()
  const worktreeMap = useWorktreeMap()
-  const activeWorktreeId = useAppStore((s) => s.activeWorktreeId)
  const worktreeLineageById = useAppStore((s) => s.worktreeLineageById)
  const updateWorktreeLineage = useAppStore((s) => s.updateWorktreeLineage)
  const tabsByWorktree = useAppStore((s) => s.tabsByWorktree)
  const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId)
  const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree)
  const deleteStateByWorktreeId = useAppStore((s) => s.deleteStateByWorktreeId)
+  const contextMenuOpenedAtRef = useRef<number | null>(null)
  const activeContextWorktrees = menuOpen ? contextWorktrees : selectedWorktrees
  const isMultiContext = activeContextWorktrees.length > 1
  const sleepableWorktrees = useMemo(
@@ -109,8 +134,6 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
  )
  const validParentWorktreeId = lineageInfo.state === 'valid' ? lineageInfo.parent.id : null
  const hasAnyContextLineage = activeContextWorktrees.some((item) => worktreeLineageById[item.id])
-  const canGroupUnderActive =
-    activeWorktreeId != null && activeContextWorktrees.every((item) => item.id !== activeWorktreeId)

  useEffect(() => {
    const closeMenu = (): void => setMenuOpen(false)
@@ -231,46 +254,53 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
    }
  }, [validParentWorktreeId])

-  const handleGroupUnderActive = useCallback(() => {
-    if (!activeWorktreeId) {
-      return
-    }
-    void Promise.all(
-      activeContextWorktrees.map((item) =>
-        updateWorktreeLineage(item.id, { parentWorktreeId: activeWorktreeId })
-      )
-    )
-  }, [activeContextWorktrees, activeWorktreeId, updateWorktreeLineage])
-
  const handleRemoveParentLink = useCallback(() => {
    void Promise.all(
      activeContextWorktrees.map((item) => updateWorktreeLineage(item.id, { noParent: true }))
    )
  }, [activeContextWorktrees, updateWorktreeLineage])

-  return (
-    <>
-      <div
-        className="relative"
-        onContextMenuCapture={(event) => {
-          const target = event.target
-          if (
-            target instanceof Element &&
-            target.closest(`[${WORKTREE_CONTEXT_MENU_SCOPE_ATTR}]`)
-          ) {
-            return
-          }
-          event.preventDefault()
-          window.dispatchEvent(new Event(CLOSE_ALL_CONTEXT_MENUS_EVENT))
-          setContextWorktrees(onContextMenuSelect?.(event) ?? selectedWorktrees)
-          const bounds = event.currentTarget.getBoundingClientRect()
-          setMenuPoint({ x: event.clientX - bounds.left, y: event.clientY - bounds.top })
-          setMenuOpen(true)
-        }}
-      >
-        {children}
-      </div>
+  const suppressOpeningPointerEvent = useCallback((event: React.SyntheticEvent) => {
+    const contextMenuOpenedAt = contextMenuOpenedAtRef.current
+    if (
+      contextMenuOpenedAt == null ||
+      !shouldSuppressContextMenuFollowUpClick(contextMenuOpenedAt, Date.now())
+    ) {
+      if (contextMenuOpenedAt != null) {
+        contextMenuOpenedAtRef.current = null
+      }
+      return
+    }
+    // Why: macOS ctrl-click can release over the just-opened menu, selecting
+    // the item under the cursor unless the opening pointer sequence is ignored.
+    event.preventDefault()
+    event.stopPropagation()
+    if (event.type === 'click') {
+      contextMenuOpenedAtRef.current = null
+    }
+  }, [])

+  return (
+    <div
+      className="relative"
+      {...{ [WORKTREE_CONTEXT_MENU_SCOPE_ATTR]: 'worktree' }}
+      onContextMenuCapture={(event) => {
+        if (shouldIgnoreNestedWorktreeContextMenuScope(event.currentTarget, event.target)) {
+          return
+        }
+        event.preventDefault()
+        contextMenuOpenedAtRef.current = Date.now()
+        window.dispatchEvent(new Event(CLOSE_ALL_CONTEXT_MENUS_EVENT))
+        setContextWorktrees(onContextMenuSelect?.(event) ?? selectedWorktrees)
+        const bounds = event.currentTarget.getBoundingClientRect()
+        setMenuPoint({ x: event.clientX - bounds.left, y: event.clientY - bounds.top })
+        setMenuOpen(true)
+      }}
+      onClickCapture={(event) => {
+        suppressOpeningPointerEvent(event)
+      }}
+    >
+      {children}
      <DropdownMenu open={menuOpen} onOpenChange={setMenuOpen} modal={false}>
        <DropdownMenuTrigger asChild>
          <button
@@ -280,7 +310,14 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
            style={{ left: menuPoint.x, top: menuPoint.y }}
          />
        </DropdownMenuTrigger>
-        <DropdownMenuContent className={cn('w-52', contentClassName)} sideOffset={0} align="start">
+        <DropdownMenuContent
+          className={cn('w-52', contentClassName)}
+          sideOffset={0}
+          align="start"
+          onPointerUpCapture={suppressOpeningPointerEvent}
+          onMouseUpCapture={suppressOpeningPointerEvent}
+          onClickCapture={suppressOpeningPointerEvent}
+        >
          {!isMultiContext && (
            <>
              <DropdownMenuItem onSelect={handleOpenInFinder} disabled={isDeleting}>
@@ -324,17 +361,10 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
                <Workflow className="size-3.5" />
                Open Parent Workspace
              </DropdownMenuItem>
-              <DropdownMenuItem
-                onSelect={handleGroupUnderActive}
-                disabled={isDeleting || !canGroupUnderActive}
-              >
-                <Workflow className="size-3.5" />
-                Group under Active Workspace
-              </DropdownMenuItem>
              {lineage && (
                <DropdownMenuItem onSelect={handleRemoveParentLink} disabled={isDeleting}>
                  <Unlink className="size-3.5" />
-                  Remove Parent Link
+                  Remove from Parent
                </DropdownMenuItem>
              )}
              <DropdownMenuSeparator />
@@ -342,17 +372,10 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
          )}
          {isMultiContext && (
            <>
-              <DropdownMenuItem
-                onSelect={handleGroupUnderActive}
-                disabled={deletingContext || !canGroupUnderActive}
-              >
-                <Workflow className="size-3.5" />
-                Group under Active Workspace
-              </DropdownMenuItem>
              {hasAnyContextLineage && (
                <DropdownMenuItem onSelect={handleRemoveParentLink} disabled={deletingContext}>
                  <Unlink className="size-3.5" />
-                  Remove Parent Links
+                  Remove from Parent
                </DropdownMenuItem>
              )}
              <DropdownMenuSeparator />
@@ -403,9 +426,14 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
          </DropdownMenuItem>
        </DropdownMenuContent>
      </DropdownMenu>
-    </>
+    </div>
  )
 })

 export default WorktreeContextMenu
-export { CLOSE_ALL_CONTEXT_MENUS_EVENT, WORKTREE_CONTEXT_MENU_SCOPE_ATTR }
+export {
+  CLOSE_ALL_CONTEXT_MENUS_EVENT,
+  WORKTREE_CONTEXT_MENU_SCOPE_ATTR,
+  shouldSuppressContextMenuFollowUpClick,
+  shouldIgnoreNestedWorktreeContextMenuScope
+}
@@ -1,7 +1,7 @@
 /* eslint-disable max-lines */
 import React, { useMemo, useCallback, useRef, useState, useEffect, useLayoutEffect } from 'react'
 import { useVirtualizer } from '@tanstack/react-virtual'
-import { ChevronDown, CircleX, Plus } from 'lucide-react'
+import { ChevronDown, CircleX, Plus, Workflow } from 'lucide-react'
 import { useAppStore } from '@/store'
 import {
  getAllWorktreesFromState,
@@ -40,6 +40,7 @@ import {
 } from './visible-worktrees'
 import { activateAndRevealWorktree } from '@/lib/worktree-activation'
 import { useRepoHeaderDrag } from './repo-header-drag'
+import WorktreeContextMenu from './WorktreeContextMenu'
 import {
  areWorktreeSelectionsEqual,
  getWorktreeSelectionIntent,
@@ -78,7 +79,7 @@ function getWorktreeOptionId(worktreeId: string): string {
  return `worktree-list-option-${encodeURIComponent(worktreeId)}`
 }

-const LINEAGE_INDENT = 30
+const LINEAGE_INDENT = 24

 type VirtualizedWorktreeViewportProps = {
  rows: Row[]
@@ -671,52 +672,106 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
            return (
              <div
                key={child.worktree.id}
-                id={getWorktreeOptionId(child.worktree.id)}
-                role="option"
-                aria-selected={selectedWorktreeIds.has(child.worktree.id)}
-                aria-current={isActive ? 'page' : undefined}
-                className={cn(
-                  'flex cursor-pointer items-start gap-2 rounded-md border border-transparent px-2 py-1.5 transition-colors',
-                  isActive
-                    ? 'border-black/[0.015] bg-black/[0.08] shadow-[0_1px_2px_rgba(0,0,0,0.04)] dark:border-border/40 dark:bg-white/[0.10] dark:shadow-[0_1px_2px_rgba(0,0,0,0.03)]'
-                    : 'hover:bg-sidebar-accent/40'
-                )}
-                onClick={handleClick}
-                onDoubleClick={(event) => event.stopPropagation()}
+                style={{ paddingLeft: `${Math.max(0, child.depth - 1) * LINEAGE_INDENT}px` }}
              >
-                <span className="mt-[5px] size-2 shrink-0 rounded-full bg-emerald-500" />
-                <div className="min-w-0 flex-1">
-                  <div className="truncate text-[12px] leading-tight text-foreground">
-                    {child.worktree.displayName}
-                  </div>
-                  <div className="mt-1 flex min-w-0 items-center gap-1.5">
-                    {child.repo && groupBy !== 'repo' ? (
-                      <span className="flex h-[16px] shrink-0 items-center gap-1.5 rounded-[4px] border border-border bg-accent px-1.5 text-[10px] font-semibold leading-none text-foreground dark:bg-accent/50 dark:border-border/60">
-                        <span
-                          className="size-1.5 rounded-full"
-                          style={{ backgroundColor: child.repo.badgeColor }}
-                        />
-                        <span className="max-w-[6rem] truncate lowercase">
-                          {child.repo.displayName}
-                        </span>
-                      </span>
-                    ) : null}
-                    <span className="truncate text-[10.5px] leading-none text-muted-foreground">
-                      {branchDisplayName(child.worktree.branch)}
+                <WorktreeContextMenu
+                  worktree={child.worktree}
+                  selectedWorktrees={selectedWorktrees}
+                  onContextMenuSelect={(event) => onContextMenuSelect(event, child.worktree)}
+                >
+                  <div
+                    id={getWorktreeOptionId(child.worktree.id)}
+                    role="option"
+                    aria-selected={selectedWorktreeIds.has(child.worktree.id)}
+                    aria-current={isActive ? 'page' : undefined}
+                    className={cn(
+                      'flex cursor-pointer items-start gap-1.5 rounded-md border border-transparent px-2 py-1.5 transition-colors',
+                      isActive
+                        ? 'border-black/[0.015] bg-black/[0.08] shadow-[0_1px_2px_rgba(0,0,0,0.04)] dark:border-border/40 dark:bg-white/[0.10] dark:shadow-[0_1px_2px_rgba(0,0,0,0.03)]'
+                        : 'hover:bg-sidebar-accent/40'
+                    )}
+                    onClick={handleClick}
+                    onDoubleClick={(event) => event.stopPropagation()}
+                  >
+                    <span className="mt-[2px] flex w-4 shrink-0 justify-center pt-[2px]">
+                      <span className="size-2 rounded-full bg-emerald-500" />
                    </span>
-                  </div>
-                  {child.worktree.linkedIssue || child.worktree.comment ? (
-                    <div className="mt-1.5 truncate text-[10.5px] leading-tight text-muted-foreground">
-                      {child.worktree.linkedIssue ? (
-                        <span className="font-medium text-foreground/80">
-                          #{child.worktree.linkedIssue}
+                    <div className="min-w-0 flex-1">
+                      <div className="truncate text-[12px] leading-tight text-foreground">
+                        {child.worktree.displayName}
+                      </div>
+                      <div className="mt-1 flex min-w-0 items-center gap-1.5">
+                        {child.repo && groupBy !== 'repo' ? (
+                          <span className="flex h-[16px] shrink-0 items-center gap-1.5 rounded-[4px] border border-border bg-accent px-1.5 text-[10px] font-semibold leading-none text-foreground dark:bg-accent/50 dark:border-border/60">
+                            <span
+                              className="size-1.5 rounded-full"
+                              style={{ backgroundColor: child.repo.badgeColor }}
+                            />
+                            <span className="max-w-[6rem] truncate lowercase">
+                              {child.repo.displayName}
+                            </span>
+                          </span>
+                        ) : null}
+                        <span className="truncate text-[10.5px] leading-none text-muted-foreground">
+                          {branchDisplayName(child.worktree.branch)}
                        </span>
+                      </div>
+                      {child.worktree.linkedIssue || child.worktree.comment ? (
+                        <div className="mt-1.5 truncate text-[10.5px] leading-tight text-muted-foreground">
+                          {child.worktree.linkedIssue ? (
+                            <span className="font-medium text-foreground/80">
+                              #{child.worktree.linkedIssue}
+                            </span>
+                          ) : null}
+                          {child.worktree.linkedIssue && child.worktree.comment ? '  ' : null}
+                          {child.worktree.comment}
+                        </div>
+                      ) : null}
+                      {child.lineageChildCount > 0 && child.lineageGroupKey ? (
+                        <div className="mt-1.5 flex min-w-0 justify-start">
+                          <Tooltip>
+                            <TooltipTrigger asChild>
+                              <Button
+                                type="button"
+                                variant="ghost"
+                                size="xs"
+                                className="h-[18px] max-w-[8rem] gap-1 rounded-md border border-sidebar-border bg-sidebar px-1.5 text-[10px] font-medium leading-none text-muted-foreground shadow-none hover:bg-sidebar-accent hover:text-foreground focus-visible:ring-1 focus-visible:ring-sidebar-ring"
+                                aria-label={`${child.lineageCollapsed ? 'Show' : 'Hide'} ${
+                                  child.lineageChildCount
+                                } child ${
+                                  child.lineageChildCount === 1 ? 'workspace' : 'workspaces'
+                                }`}
+                                aria-expanded={!child.lineageCollapsed}
+                                onClick={(event) => {
+                                  event.preventDefault()
+                                  event.stopPropagation()
+                                  toggleGroup(child.lineageGroupKey!)
+                                }}
+                              >
+                                <Workflow className="size-2.5" />
+                                <span className="truncate">
+                                  {child.lineageChildCount}{' '}
+                                  {child.lineageChildCount === 1 ? 'child' : 'children'}
+                                </span>
+                                <ChevronDown
+                                  className={cn(
+                                    'size-2.5 transition-transform',
+                                    child.lineageCollapsed && '-rotate-90'
+                                  )}
+                                />
+                              </Button>
+                            </TooltipTrigger>
+                            <TooltipContent side="right" sideOffset={8}>
+                              {child.lineageCollapsed
+                                ? 'Show child workspaces'
+                                : 'Hide child workspaces'}
+                            </TooltipContent>
+                          </Tooltip>
+                        </div>
                      ) : null}
-                      {child.worktree.linkedIssue && child.worktree.comment ? '  ' : null}
-                      {child.worktree.comment}
                    </div>
-                  ) : null}
-                </div>
+                  </div>
+                </WorktreeContextMenu>
              </div>
            )
          }
@@ -33,6 +33,9 @@ async function seedLineageScenario(page: Page): Promise<LineageScenario> {
    }

    const [parent, child] = worktrees
+    if (!parent.instanceId || !child.instanceId) {
+      throw new Error('Worktree lineage E2E needs instance-stamped worktrees')
+    }
    store.setState((current) => ({
      worktreesByRepo: Object.fromEntries(
        Object.entries(current.worktreesByRepo).map(([repoId, repoWorktrees]) => [
@@ -47,7 +50,19 @@ async function seedLineageScenario(page: Page): Promise<LineageScenario> {
            return worktree
          })
        ])
-      )
+      ),
+      worktreeLineageById: {
+        ...current.worktreeLineageById,
+        [child.id]: {
+          worktreeId: child.id,
+          worktreeInstanceId: child.instanceId,
+          parentWorktreeId: parent.id,
+          parentWorktreeInstanceId: parent.instanceId,
+          origin: 'manual',
+          capture: { source: 'manual-action', confidence: 'explicit' },
+          createdAt: Date.now()
+        }
+      }
    }))

    store.getState().setActiveWorktree(parent.id)
@@ -61,9 +76,7 @@ test.describe('Worktree Lineage', () => {
    await waitForActiveWorktree(orcaPage)
  })

-  test('groups a child under the active workspace from the sidebar context menu', async ({
-    orcaPage
-  }) => {
+  test('renders existing child lineage in the sidebar', async ({ orcaPage }) => {
    const { parentId, childId } = await seedLineageScenario(orcaPage)
    const parentRow = worktreeOption(orcaPage, parentId)
    const childRow = worktreeOption(orcaPage, childId)
@@ -73,9 +86,6 @@ test.describe('Worktree Lineage', () => {
    await expect(parentRow).toHaveAttribute('aria-current', 'page')

    await expect(childRow).toContainText('E2E lineage child')
-    await childRow.click({ button: 'right' })
-    await orcaPage.getByRole('menuitem', { name: 'Group under Active Workspace' }).click()
-
    const childToggle = parentRow.getByRole('button', { name: 'Hide 1 child workspace' })
    await expect(childToggle).toBeVisible({ timeout: 10_000 })
    await expect(childRow).toBeVisible()
@@ -102,5 +112,16 @@ test.describe('Worktree Lineage', () => {
    await childToggle.click()
    await expect(parentRow.getByRole('button', { name: 'Show 1 child workspace' })).toBeVisible()
    await expect(childRow).toBeHidden()
+
+    await parentRow.getByRole('button', { name: 'Show 1 child workspace' }).click()
+    await childRow.click({ button: 'right' })
+    await orcaPage.getByRole('menuitem', { name: 'Remove from Parent' }).click()
+    await expect(parentRow.getByRole('button', { name: /child workspace/ })).toHaveCount(0)
+    await expect(childRow).toBeVisible()
+
+    await parentRow.click({ button: 'right' })
+    await expect(
+      orcaPage.getByRole('menuitem', { name: 'Group under Active Workspace' })
+    ).toHaveCount(0)
  })
 })