Open into the window's emptied pane beside a split branch in one layout command (dark, layout-owner build only) (#30039)

* Open into the window's emptied root beside a split branch in one layout command (dark, layout-owner build only)

A tab opened into a group the window kept empty reaches main with the
split that places it, now also when the group's neighbour is itself split
(named by the branch's first and last groups) or is another empty group
main lacks. Main makes the group with its tab beside that branch. Switch-off
builds send no such command.

* Comment the refusal of a branch main's tree does not hold (dark)

src/renderer/src/runtime/layout-owner-new-group-split.ts

@@ -1,13 +1,14 @@
 import type { TabGroupLayoutNode } from '../../../shared/tab-types'
+import { removeGroupLayoutLeaf } from '../../../shared/workspace-layout/tab-group-layout-tree'
 import type { NewGroupSplit } from '../../../shared/workspace-layout/workspace-layout-command-types'
 import type { TabsSlice } from '@/store/slices/tabs/tabs-slice-contract'
 
 /**
  * The split that made a just-opened tab's group, when the group holds no other tab. Main stores no
- * empty group, so it lacks that group (the window made it with the tab, or empty a moment before)
- * and makes it with the tab, beside the sibling the window split. None for a workspace's only
- * group, or a sibling that is itself split, which the window's split never makes. `state` may
- * predate the tab (an editor open passes the store from before it).
+ * empty group, so it lacks that group (the window made it with the tab, or empty before) and makes
+ * it with the tab, beside the sibling branch, read off the window's tree without the other empty
+ * groups main lacks too. None for what is then the workspace's only group. `state` may predate
+ * the tab (an editor open passes the store from before it).
  */
 export function newGroupSplitOf(
   state: Pick<TabsSlice, 'groupsByWorktree' | 'layoutByWorktree'>,
@@ -15,12 +16,19 @@ export function newGroupSplitOf(
   groupId: string,
   tabId: string
 ): { split: NewGroupSplit } | null {
-  const group = state.groupsByWorktree[worktreeId]?.find((entry) => entry.id === groupId)
+  const groups = state.groupsByWorktree[worktreeId] ?? []
+  const group = groups.find((entry) => entry.id === groupId)
   const layout = state.layoutByWorktree[worktreeId]
   if (!group || group.tabOrder.some((id) => id !== tabId) || !layout) {
     return null
   }
-  return splitBeside(layout, groupId)
+  let held: TabGroupLayoutNode | null = layout
+  for (const other of groups) {
+    if (held && other.id !== groupId && other.tabOrder.length === 0) {
+      held = removeGroupLayoutLeaf(held, other.id)
+    }
+  }
+  return held ? splitBeside(held, groupId) : null
 }
 
 function splitBeside(node: TabGroupLayoutNode, groupId: string): { split: NewGroupSplit } | null {
@@ -33,10 +41,19 @@ function splitBeside(node: TabGroupLayoutNode, groupId: string): { split: NewGro
     return splitBeside(node.first, groupId) ?? splitBeside(node.second, groupId)
   }
   const sibling = isFirst ? node.second : node.first
-  if (sibling.type !== 'leaf') {
-    return null
-  }
   const sides =
     node.direction === 'horizontal' ? (['left', 'right'] as const) : (['up', 'down'] as const)
-  return { split: { besideGroupId: sibling.groupId, direction: sides[isFirst ? 0 : 1] } }
+  const besideGroupId = edgeGroupId(sibling, 'first')
+  const besideLastGroupId = edgeGroupId(sibling, 'second')
+  return {
+    split: {
+      besideGroupId,
+      ...(besideLastGroupId !== besideGroupId ? { besideLastGroupId } : {}),
+      direction: sides[isFirst ? 0 : 1]
+    }
+  }
+}
+
+function edgeGroupId(node: TabGroupLayoutNode, edge: 'first' | 'second'): string {
+  return node.type === 'leaf' ? node.groupId : edgeGroupId(node[edge], edge)
 }

src/renderer/src/store/slices/store-open-to-side-layout-owner.test.ts

@@ -3,6 +3,7 @@ import type * as AgentStatusModule from '@/lib/agent-status'
 import { createTestStore, makeWorktree, seedStore } from './store-test-helpers'
 import { createStoreCascadesMockApi } from './store-cascades-test-harness'
 import { newGroupSplitOf } from '@/runtime/layout-owner-new-group-split'
+import { recordOpenedAgentSessionTabInLayoutOwner } from '@/runtime/agent-session-tab-layout-owner'
 import type { TabGroupLayoutNode } from '../../../../shared/tab-types'
 import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host'
 import { LayoutApplyParams } from '../../../../shared/rpc-contract/workspace-layout-params'
@@ -81,10 +82,17 @@ function applySent() {
   return commands
 }
 
+/** Groups by id: the detector matches them by id, and main appends a group the window kept. */
+function byId(groups: readonly { id: string; tabOrder: string[] }[]) {
+  return groups
+    .map(({ id, tabOrder }) => ({ id, tabOrder }))
+    .sort((left, right) => left.id.localeCompare(right.id))
+}
+
 function windowView() {
   const state = store.getState()
   return {
-    groups: (state.groupsByWorktree[WT] ?? []).map(({ id, tabOrder }) => ({ id, tabOrder })),
+    groups: byId(state.groupsByWorktree[WT] ?? []),
     tree: splitTree(state.layoutByWorktree[WT])
   }
 }
@@ -92,7 +100,7 @@ function windowView() {
 function mainView() {
   const workspace = main.workspaces[WT]!
   return {
-    groups: workspace.groups.map(({ id, tabOrder }) => ({ id, tabOrder })),
+    groups: byId(workspace.groups),
     tree: splitTree(workspace.groupLayout)
   }
 }
@@ -180,11 +188,72 @@ describe('open to the side in a layout-owner build', () => {
     applySent()
 
     // The window keeps its empty root beside it; main stores no empty group.
-    expect(windowView().groups).toEqual([
-      { id: ROOT, tabOrder: [] },
-      { id: editorGroup, tabOrder: [onlyTabOf(editorGroup)] }
-    ])
-    expect(mainView()).toEqual({ groups: [windowView().groups[1]], tree: null })
+    expect(windowView().groups).toEqual(
+      byId([
+        { id: ROOT, tabOrder: [] },
+        { id: editorGroup, tabOrder: [onlyTabOf(editorGroup)] }
+      ])
+    )
+    expect(mainView()).toEqual({
+      groups: [{ id: editorGroup, tabOrder: [onlyTabOf(editorGroup)] }],
+      tree: null
+    })
+  })
+
+  it.each([
+    ['terminal', () => store.getState().createTab(WT, ROOT)],
+    ['editor', () => openFile('/c.ts', { targetGroupId: ROOT })],
+    [
+      'browser',
+      () => store.getState().createBrowserTab(WT, 'about:blank', { targetGroupId: ROOT })
+    ],
+    [
+      'chat',
+      () => {
+        // As openStructuredAgentSessionProvisionalTab does, on this test's store.
+        const tab = store.getState().createUnifiedTab(WT, 'agent-session', {
+          entityId: 'chat-1',
+          agentSessionAgent: 'codex',
+          targetGroupId: ROOT
+        })
+        recordOpenedAgentSessionTabInLayoutOwner(store.getState(), tab)
+      }
+    ]
+  ])(
+    'records a %s tab opened into the emptied root after its neighbour split again',
+    (_kind, openIntoRoot) => {
+      // The window keeps its emptied root; the splits beside it hold tabs, so main has only those.
+      store.getState().createTab(WT, undefined, undefined, {
+        split: { sourceGroupId: ROOT, splitDirection: 'right' }
+      })
+      const right = store.getState().groupsByWorktree[WT]!.at(-1)!.id
+      store.getState().createTab(WT, undefined, undefined, {
+        split: { sourceGroupId: right, splitDirection: 'down' }
+      })
+      applySent()
+      expect(windowView().groups).toContainEqual({ id: ROOT, tabOrder: [] })
+
+      openIntoRoot()
+      applySent()
+      expect(mainView()).toEqual(windowView())
+    }
+  )
+
+  it('records opens into a split made off another empty split, beside the groups main holds', () => {
+    openFile('/a.ts')
+    const outer = store.getState().createEmptySplitGroup(WT, ROOT, 'right')!
+    const inner = store.getState().createEmptySplitGroup(WT, outer, 'down')!
+    applySent()
+
+    // The inner group's sibling is the outer one, which main lacks until its own first tab.
+    openFile('/b.ts', { targetGroupId: inner })
+    applySent()
+    store.getState().closeEmptyGroup(WT, outer)
+    expect(mainView()).toEqual(windowView())
+    const again = store.getState().createEmptySplitGroup(WT, inner, 'up')!
+    openFile('/c.ts', { targetGroupId: again })
+    applySent()
+    expect(mainView()).toEqual(windowView())
   })
 
   it('sends no split for a tab that joins a group holding others, or the only group', () => {
@@ -210,7 +279,7 @@ describe('open to the side in a layout-owner build', () => {
 })
 
 describe('newGroupSplitOf', () => {
-  it('finds no split beside a sibling that is itself split, so main gets none', () => {
+  it('names a sibling that is itself split by its first and last groups', () => {
     const group = (id: string, tabOrder: string[]) => ({
       id,
       worktreeId: WT,
@@ -234,6 +303,8 @@ describe('newGroupSplitOf', () => {
       }
     }
 
-    expect(newGroupSplitOf(state, WT, 'N', 'n')).toBeNull()
+    expect(newGroupSplitOf(state, WT, 'N', 'n')).toEqual({
+      split: { besideGroupId: 'A', besideLastGroupId: 'M', direction: 'right' }
+    })
   })
 })

src/shared/rpc-contract/workspace-layout-params.ts

@@ -116,6 +116,7 @@ const CloseTabsCommand = z.object({
 const NewGroupSplit = z
   .object({
     besideGroupId: z.string().min(1),
+    besideLastGroupId: z.string().min(1).optional(),
     direction: z.enum(['left', 'right', 'up', 'down'])
   })
   .optional()

src/shared/workspace-layout/tab-group-layout-tree.ts

@@ -143,3 +143,49 @@ export function moveGroupLeafBeside(
     buildSplitNode(besideGroupId, groupId, direction, position)
   )
 }
+
+function holdsGroup(node: TabGroupLayoutNode, groupId: string): boolean {
+  return node.type === 'leaf'
+    ? node.groupId === groupId
+    : holdsGroup(node.first, groupId) || holdsGroup(node.second, groupId)
+}
+
+/**
+ * Moves a group's leaf to `side` of the smallest branch holding both `fromGroupId` and
+ * `toGroupId` (one group's leaf when they are the same); null when no branch holds both.
+ */
+export function moveGroupLeafBesideBranch(
+  root: TabGroupLayoutNode,
+  groupId: string,
+  fromGroupId: string,
+  toGroupId: string,
+  side: 'left' | 'right' | 'up' | 'down'
+): TabGroupLayoutNode | null {
+  const leaf: TabGroupLayoutNode = { type: 'leaf', groupId }
+  const direction = side === 'left' || side === 'right' ? 'horizontal' : 'vertical'
+  const leafFirst = side === 'left' || side === 'up'
+  const besideBranch = (node: TabGroupLayoutNode): TabGroupLayoutNode | null => {
+    if (!holdsGroup(node, fromGroupId) || !holdsGroup(node, toGroupId)) {
+      return null
+    }
+    if (node.type === 'split') {
+      const first = besideBranch(node.first)
+      if (first) {
+        return { ...node, first }
+      }
+      const second = besideBranch(node.second)
+      if (second) {
+        return { ...node, second }
+      }
+    }
+    return {
+      type: 'split',
+      direction,
+      first: leafFirst ? leaf : node,
+      second: leafFirst ? node : leaf,
+      ratio: 0.5
+    }
+  }
+  const rest = removeGroupLayoutLeaf(root, groupId)
+  return rest && besideBranch(rest)
+}

src/shared/workspace-layout/workspace-layout-command-types.ts

@@ -213,9 +213,15 @@ export type GroupSide = 'left' | 'right' | 'up' | 'down'
 
 /**
  * An open into a new group, named by the command's `groupId`, split off `besideGroupId`: the
- * group and its first tab are made in one step, so no empty group is ever stored.
+ * group and its first tab are made in one step, so no empty group is ever stored. With
+ * `besideLastGroupId`, the split is off the branch from `besideGroupId` to it (its first and last
+ * groups), as when a group the window kept empty sits beside a branch split since.
  */
-export type NewGroupSplit = { besideGroupId: string; direction: GroupSide }
+export type NewGroupSplit = {
+  besideGroupId: string
+  besideLastGroupId?: string
+  direction: GroupSide
+}
 
 export type TabTitle =
   | { kind: 'custom' | 'generated'; title: string | null }

src/shared/workspace-layout/workspace-layout-group-commands.ts

@@ -5,7 +5,7 @@
 
 import type { TabGroupLayoutNode } from '../tab-types'
 import { buildHeadlessTabGroupMove, buildHeadlessTabGroupSplit } from './tab-group-moves'
-import { moveGroupLeafBeside } from './tab-group-layout-tree'
+import { moveGroupLeafBeside, moveGroupLeafBesideBranch } from './tab-group-layout-tree'
 import {
   applied,
   findTab,
@@ -173,20 +173,28 @@ export function openInNewGroup(
   if (workspaces.some((entry) => entry.groups.some((group) => group.id === groupId))) {
     return refuse('group_exists')
   }
-  const besideHeld = workspace.groups.some((group) => group.id === split.besideGroupId)
+  const lastGroupId = split.besideLastGroupId ?? split.besideGroupId
+  const besideHeld = [split.besideGroupId, lastGroupId].every((id) =>
+    workspace.groups.some((group) => group.id === id)
+  )
   // Beside a group main lacks: refused, unless main has no group here at all (the window split off
   // its emptied root), where the new group is the only one.
   if (!besideHeld && workspace.groups.length > 0) {
     return refuse('group_not_found')
   }
   const groupLayout = besideHeld
-    ? moveGroupLeafBeside(
+    ? moveGroupLeafBesideBranch(
         workspace.groupLayout ?? { type: 'leaf', groupId: split.besideGroupId },
         groupId,
         split.besideGroupId,
+        lastGroupId,
         split.direction
       )
     : undefined
+  // Both groups held, but no branch of main's tree holds them both.
+  if (besideHeld && !groupLayout) {
+    return refuse('invalid_params')
+  }
   const opened = open(
     withWorkspace(model, command.workspace, {
       ...workspace,

src/shared/workspace-layout/workspace-layout-open-in-new-group.test.ts

@@ -123,6 +123,36 @@ describe('opening into a new split', () => {
     })
   })
 
+  it('splits off a whole branch named by its first and last groups', () => {
+    const { ctx, model } = oneGroup()
+    const opened = build(
+      ctx,
+      [
+        editor('/w/b.ts', { tabId: 'b', groupId: 'g2', split: right }),
+        editor('/w/c.ts', {
+          tabId: 'c',
+          groupId: 'g3',
+          split: { besideGroupId: 'g1', besideLastGroupId: 'g2', direction: 'up' }
+        })
+      ],
+      model
+    )
+
+    expect(opened.model.workspaces[WS]!.groupLayout).toEqual({
+      type: 'split',
+      direction: 'vertical',
+      first: { type: 'leaf', groupId: 'g3' },
+      second: {
+        type: 'split',
+        direction: 'horizontal',
+        first: { type: 'leaf', groupId: 'g1' },
+        second: { type: 'leaf', groupId: 'g2' },
+        ratio: 0.5
+      },
+      ratio: 0.5
+    })
+  })
+
   it('refuses a split it cannot make, and changes nothing', () => {
     const { ctx, model } = oneGroup()
     const refusal = (command: LayoutCommand) => {
@@ -139,6 +169,15 @@ describe('opening into a new split', () => {
         })
       )
     ).toBe('group_not_found')
+    expect(
+      refusal(
+        editor('/w/b.ts', {
+          tabId: 'b',
+          groupId: 'g2',
+          split: { besideGroupId: 'g1', besideLastGroupId: 'missing', direction: 'right' }
+        })
+      )
+    ).toBe('group_not_found')
     // The group id another workspace holds.
     const other = build(
       ctx,