From 0cc52e3d71e15112560530294c8dcd08db5306dc Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 20 Sep 2026 19:39:14 -0400 Subject: [PATCH] refactor(terminal): drop the split reveal's second row lookup A split reveal names its parent tab by id, and that hint already resolves through the pty-owner path: with zero holders the hinted row wins whenever it exists under any worktree key. So looking it up again could only ever repeat an answer already in hand. The remaining null case means the hint names no row at all, which still fails the reveal with the same message. --- ...al-reveal-split-owner-worktree-key.test.ts | 38 +++++++++++++++++++ .../src/lib/terminal-reveal-tab-adoption.ts | 10 ++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/hooks/terminal-reveal-split-owner-worktree-key.test.ts b/src/renderer/src/hooks/terminal-reveal-split-owner-worktree-key.test.ts index f182f08aca1..fb70ef0ee8a 100644 --- a/src/renderer/src/hooks/terminal-reveal-split-owner-worktree-key.test.ts +++ b/src/renderer/src/hooks/terminal-reveal-split-owner-worktree-key.test.ts @@ -3,6 +3,10 @@ // whole reveal, so the split pane never appeared. import { describe, expect, it } from 'vitest' import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' +import { + resolveTerminalRevealTabAdoption, + type TerminalRevealAdoptionState +} from '@/lib/terminal-reveal-tab-adoption' import { createHarnessStoreState, loadIpcEventsHarness, @@ -53,6 +57,40 @@ describe('split reveal whose target tab is filed under another worktree key', () expect(layout.ptyIdsByLeafId).toMatchObject({ 'leaf-split': 'pty-split' }) }) + it('adopts the hinted parent as the owner of a brand-new split pty', () => { + // Why this is asserted on its own: it is what lets the split path reuse the adopted row + // instead of looking the hint up a second time. + const state: TerminalRevealAdoptionState = { + tabsByWorktree: { + [OWNER_WORKTREE_ID]: [ + { + id: 'tab-a', + ptyId: null, + worktreeId: OWNER_WORKTREE_ID, + title: 'Terminal 1', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { + 'tab-a': { + root: { type: 'leaf', leafId: 'leaf-a' }, + activeLeafId: 'leaf-a', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-a': 'pty-a' } + } + }, + ptyIdsByTabId: {} + } + + expect( + resolveTerminalRevealTabAdoption(state, { ptyId: 'pty-split', hintTabId: 'tab-a' }) + ).toEqual({ kind: 'adopt', tabId: 'tab-a', via: 'pty-owner' }) + }) + it('still fails a split reveal whose parent row exists under no worktree key', async () => { const storeState = storeWithOwnerFiledElsewhere() const harness = await loadIpcEventsHarness(storeState) diff --git a/src/renderer/src/lib/terminal-reveal-tab-adoption.ts b/src/renderer/src/lib/terminal-reveal-tab-adoption.ts index 411f6c686dd..2bd87f7673d 100644 --- a/src/renderer/src/lib/terminal-reveal-tab-adoption.ts +++ b/src/renderer/src/lib/terminal-reveal-tab-adoption.ts @@ -128,12 +128,10 @@ export function resolveTerminalRevealTarget( const isSplitReveal = Boolean( request.ptyId && request.tabId && request.leafId && request.splitFromLeafId ) - // Why: a split of a new PTY has no owner to adopt, and its parent row can sit under another key. - const splitTargetRow = - isSplitReveal && request.tabId !== undefined ? findTerminalTabRow(state, request.tabId) : null - if (isSplitReveal && !adoptedRow && !splitTargetRow) { + // Why no lookup of its own: the hinted parent is adopted as the pty's owner across every + // worktree key, so a split never needs one — a null row here means the hint names no row at all. + if (isSplitReveal && !adoptedRow) { throw new Error(`Terminal tab ${request.tabId} not found`) } - const ownerRow = adoptedRow ?? splitTargetRow - return { tab: ownerRow?.tab, ownerWorktreeId: ownerRow?.worktreeId ?? request.worktreeId } + return { tab: adoptedRow?.tab, ownerWorktreeId: adoptedRow?.worktreeId ?? request.worktreeId } }