diff --git a/src/renderer/src/runtime/layout-owner-new-group-split.ts b/src/renderer/src/runtime/layout-owner-new-group-split.ts index 63a49e01915..4b04e43518e 100644 --- a/src/renderer/src/runtime/layout-owner-new-group-split.ts +++ b/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, @@ -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) } diff --git a/src/renderer/src/store/slices/store-open-to-side-layout-owner.test.ts b/src/renderer/src/store/slices/store-open-to-side-layout-owner.test.ts index 621fbd24838..b4ec778c8fc 100644 --- a/src/renderer/src/store/slices/store-open-to-side-layout-owner.test.ts +++ b/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' } + }) }) }) diff --git a/src/shared/rpc-contract/workspace-layout-params.ts b/src/shared/rpc-contract/workspace-layout-params.ts index 7df8da33d25..01fcca18532 100644 --- a/src/shared/rpc-contract/workspace-layout-params.ts +++ b/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() diff --git a/src/shared/workspace-layout/tab-group-layout-tree.ts b/src/shared/workspace-layout/tab-group-layout-tree.ts index fe9ae87651a..585d644c08c 100644 --- a/src/shared/workspace-layout/tab-group-layout-tree.ts +++ b/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) +} diff --git a/src/shared/workspace-layout/workspace-layout-command-types.ts b/src/shared/workspace-layout/workspace-layout-command-types.ts index 0cfa7754d61..8455c49d895 100644 --- a/src/shared/workspace-layout/workspace-layout-command-types.ts +++ b/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 } diff --git a/src/shared/workspace-layout/workspace-layout-group-commands.ts b/src/shared/workspace-layout/workspace-layout-group-commands.ts index f9295797829..6dcf24fc9fb 100644 --- a/src/shared/workspace-layout/workspace-layout-group-commands.ts +++ b/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, diff --git a/src/shared/workspace-layout/workspace-layout-open-in-new-group.test.ts b/src/shared/workspace-layout/workspace-layout-open-in-new-group.test.ts index 493172604b2..23b0652087c 100644 --- a/src/shared/workspace-layout/workspace-layout-open-in-new-group.test.ts +++ b/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,