diff --git a/src/main/persistence/terminal-topology/terminal-layout-apply.test.ts b/src/main/persistence/terminal-topology/terminal-layout-apply.test.ts index 07d9b5bcd3a..5c3f9906fdd 100644 --- a/src/main/persistence/terminal-topology/terminal-layout-apply.test.ts +++ b/src/main/persistence/terminal-topology/terminal-layout-apply.test.ts @@ -76,6 +76,53 @@ describe('Store.applyLayoutCommand (layout.apply)', () => { expect(layout?.ptyIdsByLeafId).toEqual({ [LEAF]: 'pty-1' }) }) + it('records a window’s tab close durably, so a late start of it is refused', async () => { + const { store, directory } = await storeWithOneTab() + const revision = (key: Store) => + Object.values(key.getWorkspaceSession().terminalTopologyRevisionByRepoId ?? {})[0] ?? 0 + const revisionBefore = revision(store) + + const reply = await store.applyLayoutCommand({ + type: 'closeTabs', + workspace: WORKTREE, + tabIds: [TAB], + force: true, + recordClose: 'user' + }) + + expect(reply).toEqual({ ok: true, result: { closed: [TAB], refused: [] } }) + const session = (await reopenTopologyStore(store, directory)).getWorkspaceSession() + expect(session.tabsByWorktree[WORKTREE] ?? []).toEqual([]) + expect(session.closedTerminalTabTombstonesByTabId?.[TAB]).toMatchObject({ + worktreeId: WORKTREE, + reason: 'user' + }) + // A stale window save cannot bring the tab back. + expect(revision(store)).toBeGreaterThan(revisionBefore) + }) + + it('prunes the partition’s expired close records when it records one, as today', async () => { + const { store } = await storeWithOneTab() + store.setWorkspaceSession({ + ...store.getWorkspaceSession(), + closedTerminalTabTombstonesByTabId: { + 'tab-old': { worktreeId: 'repo-other::/x', closedAt: 1, reason: 'user' } + } + }) + + await store.applyLayoutCommand({ + type: 'closeTabs', + workspace: WORKTREE, + tabIds: [TAB], + force: true, + recordClose: 'cleanup' + }) + + expect( + Object.keys(store.getWorkspaceSession().closedTerminalTabTombstonesByTabId ?? {}) + ).toEqual([TAB]) + }) + it('writes a folder workspace’s split', async () => { const { store } = await storeWithOneTab(undefined, FIXTURE_FOLDER_WORKTREE_ID) diff --git a/src/main/persistence/terminal-topology/terminal-pane-admission.ts b/src/main/persistence/terminal-topology/terminal-pane-admission.ts index fc4f46213ee..472f59b2af2 100644 --- a/src/main/persistence/terminal-topology/terminal-pane-admission.ts +++ b/src/main/persistence/terminal-topology/terminal-pane-admission.ts @@ -19,6 +19,7 @@ import { paneKeyOf } from '../../../shared/workspace-layout/workspace-layout-mod import { saveWorkspaceLayout } from '../../../shared/workspace-layout/workspace-layout-save' import { patchStoredSession } from '../../../shared/workspace-layout/workspace-layout-record-patch' import { followRemoteSessionAfterPaneClose } from './pane-close-remote-session' +import { pruneClosedTerminalTabTombstones } from '../../../shared/closed-terminal-tab-tombstones' /** A new tab, or a split of an existing pane, with the ids its terminal will run under. */ export type TerminalPaneAdmission = @@ -67,7 +68,9 @@ export function applyCommandToSession( const loaded = loadWorkspaceLayout(hostId, session, nameBasedLoadContext()) return applyToSession(loaded, command, (after) => { const patched = patchStoredSession(session, saveWorkspaceLayout(loaded), after) - return patched === session ? null : followRemoteSessionAfterPaneClose(command, session, patched) + return patched === session + ? null + : followRemoteSessionAfterPaneClose(command, session, prunedOnRecord(command, patched)) }) } @@ -88,3 +91,19 @@ function applyToSession( } : { value: { ok: false, code: applied.code }, session: null } } + +/** A recorded close prunes the partition's close records, as today's close does. */ +function prunedOnRecord( + command: LayoutCommand, + session: WorkspaceSessionState +): WorkspaceSessionState { + return command.type === 'closeTabs' && command.recordClose + ? { + ...session, + closedTerminalTabTombstonesByTabId: pruneClosedTerminalTabTombstones( + session.closedTerminalTabTombstonesByTabId, + Date.now() + ) + } + : session +} diff --git a/src/main/runtime/rpc/methods/workspace-layout-apply.test.ts b/src/main/runtime/rpc/methods/workspace-layout-apply.test.ts index 5b87c4a4db3..8c5cc1d0a58 100644 --- a/src/main/runtime/rpc/methods/workspace-layout-apply.test.ts +++ b/src/main/runtime/rpc/methods/workspace-layout-apply.test.ts @@ -160,6 +160,24 @@ describe('layout.apply', () => { ).toBe(false) }) + it('accepts the close of one tab with the window’s reason, but no force or refusals', async () => { + const apply = (await registeredMethods(true)).find((method) => method.name === 'layout.apply') + const close = { type: 'closeTabs', workspace: WORKTREE, recordClose: 'user' } + + expect( + apply?.params?.parse({ + command: { ...close, tabIds: ['tab-1'], force: false, refusePinned: true } + }) + ).toEqual({ command: { ...close, tabIds: ['tab-1'] } }) + expect(apply?.params?.safeParse({ command: { ...close, tabIds: ['a', 'b'] } }).success).toBe( + false + ) + expect( + apply?.params?.safeParse({ command: { ...close, tabIds: ['a'], recordClose: 'pty-exit' } }) + .success + ).toBe(false) + }) + it('applies group sizes to the workspace’s routed partition', async () => { const { runtime, applyLayoutCommand } = runtimeHolding('ssh:target-1') const ratios = { diff --git a/src/main/runtime/runtime-layout-apply.test.ts b/src/main/runtime/runtime-layout-apply.test.ts index 4d727fceb24..bc2a229521b 100644 --- a/src/main/runtime/runtime-layout-apply.test.ts +++ b/src/main/runtime/runtime-layout-apply.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { applyWindowLayoutCommand } from './runtime-layout-apply' +import { applyWindowLayoutCommand, type WindowLayoutCommand } from './runtime-layout-apply' import type { RuntimeStore } from './runtime-store-contract' +import type { LayoutApplyOutcome } from '../../shared/workspace-layout/workspace-layout-command-types' const mocks = vi.hoisted(() => ({ markAgentLaunchesClosedByUser: vi.fn() })) vi.mock('../agent-launch/agent-launch-pane-attachment', () => ({ @@ -10,7 +11,10 @@ vi.mock('../agent-launch/agent-launch-pane-attachment', () => ({ const CLOSE = { type: 'closePane', workspace: 'wt-1', tabId: 'tab-1', leafId: 'leaf-1' } as const function storeWith() { - const applyLayoutCommand = vi.fn(async () => ({ ok: true as const, result: {} })) + const applyLayoutCommand = vi.fn(async (): Promise => ({ + ok: true, + result: {} + })) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the apply path reads only applyLayoutCommand. const store = { applyLayoutCommand } as unknown as RuntimeStore return { store, applyLayoutCommand } @@ -60,6 +64,64 @@ describe('applyWindowLayoutCommand', () => { expect(applyLayoutCommand).toHaveBeenCalledWith(ratios, ['ssh:target-1']) }) + it('records the window’s tab close in every holder, forced, stopping no terminal', async () => { + const { store, applyLayoutCommand } = storeWith() + const close: WindowLayoutCommand = { + type: 'closeTabs', + workspace: 'wt-1', + tabIds: ['tab-1'], + recordClose: 'user' + } + + await applyWindowLayoutCommand(store, () => ['local', 'runtime:env-1', 'ssh:target-1'], close) + + expect(applyLayoutCommand).toHaveBeenCalledWith({ ...close, force: true }, [ + 'local', + 'ssh:target-1' + ]) + expect(mocks.markAgentLaunchesClosedByUser).toHaveBeenCalledWith('wt-1', { + kind: 'tab', + tabId: 'tab-1' + }) + }) + + it('marks a user close’s launches by the terminal’s id, not its tab-bar entry’s', async () => { + const { store } = storeWith() + const session = { unifiedTabs: { 'wt-1': [{ id: 'entry-1', entityId: 'term-1' }] } } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the close reads only unifiedTabs. + const withSession = { ...store, getWorkspaceSession: () => session } as unknown as RuntimeStore + const close: WindowLayoutCommand = { + type: 'closeTabs', + workspace: 'wt-1', + tabIds: ['entry-1'], + recordClose: 'user' + } + + await applyWindowLayoutCommand(withSession, () => ['local'], close) + + expect(mocks.markAgentLaunchesClosedByUser).toHaveBeenCalledWith('wt-1', { + kind: 'tab', + tabId: 'term-1' + }) + }) + + it('leaves a cleanup close’s launches alone, and a workspace main lacks to today’s close', async () => { + const { store, applyLayoutCommand } = storeWith() + applyLayoutCommand.mockResolvedValueOnce({ ok: true, result: { alreadyClosed: true } }) + const close: WindowLayoutCommand = { + type: 'closeTabs', + workspace: 'wt-1', + tabIds: ['tab-1'], + recordClose: 'cleanup' + } + + await expect(applyWindowLayoutCommand(store, () => ['local'], close)).resolves.toEqual({ + ok: false, + code: 'workspace_not_found' + }) + expect(mocks.markAgentLaunchesClosedByUser).not.toHaveBeenCalled() + }) + it('refuses a tab a paired server routes, touching nothing', async () => { const { store, applyLayoutCommand } = storeWith() diff --git a/src/main/runtime/runtime-layout-apply.ts b/src/main/runtime/runtime-layout-apply.ts index 1af71db4cc3..cf418a8cd44 100644 --- a/src/main/runtime/runtime-layout-apply.ts +++ b/src/main/runtime/runtime-layout-apply.ts @@ -7,7 +7,8 @@ import { markAgentLaunchesClosedByUser } from '../agent-launch/agent-launch-pane import type { RuntimeStore } from './runtime-store-contract' /** A window's layout command: one on a tab, which names its partitions, or on the group tree. */ -export type WindowLayoutCommand = LayoutCommand & ({ tabId: string } | { type: 'setGroupRatios' }) +export type WindowLayoutCommand = LayoutCommand & + ({ tabId: string } | { type: 'setGroupRatios' } | { type: 'closeTabs'; tabIds: [string] }) /** * `layout.apply`: the window's command on the partitions holding its tab, routed partition first; @@ -19,7 +20,9 @@ export async function applyWindowLayoutCommand( hostIdsFor: (worktreeId: string, tabId: string | null) => ExecutionHostId[], command: WindowLayoutCommand ): Promise { - const hostIds = hostIdsFor(command.workspace, 'tabId' in command ? command.tabId : null) + const routedTabId = + 'tabId' in command ? command.tabId : command.type === 'closeTabs' ? command.tabIds[0] : null + const hostIds = hostIdsFor(command.workspace, routedTabId) // Why: a paired server's workspaces keep today's mirrored layout until that server owns it. if (!hostIds[0] || parseExecutionHostId(hostIds[0])?.kind === 'runtime') { return { ok: false, code: 'host_not_owned' } @@ -27,15 +30,31 @@ export async function applyWindowLayoutCommand( if (!store.applyLayoutCommand) { throw new Error('layout_store_unavailable') } + // Why every holder: a legacy relay reattach can leave one tab in two partitions. + const holders = hostIds.filter((hostId) => parseExecutionHostId(hostId)?.kind !== 'runtime') + if (command.type === 'closeTabs') { + // The window already confirmed, removed the tab and stopped its terminals: main forces the + // close, records it, and stops nothing (a terminal another tab shares must live on). + if (command.recordClose === 'user') { + // Launches go by the terminal's own id, which a legacy tab-bar entry's can differ from. + const tabId = + store + .getWorkspaceSession?.(hostIds[0]) + ?.unifiedTabs?.[command.workspace]?.find((entry) => entry.id === command.tabIds[0]) + ?.entityId ?? command.tabIds[0] + markAgentLaunchesClosedByUser(command.workspace, { kind: 'tab', tabId }) + } + const reply = await store.applyLayoutCommand({ ...command, force: true }, holders) + // Why: a workspace main lacks records nothing; today's close records it in the routed partition. + return reply.ok && reply.result.alreadyClosed + ? { ok: false, code: 'workspace_not_found' } + : reply + } if (command.type !== 'closePane') { return store.applyLayoutCommand(command, [hostIds[0]]) } const { workspace, tabId, leafId } = command // A launch still starting in the pane the user closed stops, and says so (as the legacy close). markAgentLaunchesClosedByUser(workspace, { kind: 'pane', tabId, leafId }) - // Why every holder: a legacy relay reattach can leave one tab in two partitions. - return store.applyLayoutCommand( - { ...command, userPaneClose: true }, - hostIds.filter((hostId) => parseExecutionHostId(hostId)?.kind !== 'runtime') - ) + return store.applyLayoutCommand({ ...command, userPaneClose: true }, holders) } diff --git a/src/renderer/src/components/tab-group/tab-group-layout-owner-commands.ts b/src/renderer/src/components/tab-group/tab-group-layout-owner-commands.ts index 83444856738..8c709e9a17d 100644 --- a/src/renderer/src/components/tab-group/tab-group-layout-owner-commands.ts +++ b/src/renderer/src/components/tab-group/tab-group-layout-owner-commands.ts @@ -5,7 +5,8 @@ import { useAppStore } from '@/store' import type { TabSplitDirection } from '@/store/slices/tabs' -import { sendLayoutCommandToOwner, type WindowLayoutCommand } from '@/runtime/layout-owner-command' +import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import type { WindowLayoutCommand } from '@/runtime/layout-apply-send' import { moveGroupLeafBeside } from '../../../../shared/workspace-layout/tab-group-layout-tree' import { stableJson } from '../../../../shared/workspace-layout/workspace-layout-load-report' import { isPaneColumnSplitDropNoOp } from '@/store/slices/pane-column-split-drop-no-op' diff --git a/src/renderer/src/runtime/layout-apply-send.ts b/src/renderer/src/runtime/layout-apply-send.ts new file mode 100644 index 00000000000..ba6f2e54eb6 --- /dev/null +++ b/src/renderer/src/runtime/layout-apply-send.ts @@ -0,0 +1,91 @@ +// Store-free so store slices can send; `layout-owner-command` reads the store for other callers. +import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' +import type { TabGroupLayoutNode } from '../../../shared/tab-types' +import type { TerminalPaneLayoutNode } from '../../../shared/terminal-tab-types' +import type { PaneSide } from '../../../shared/workspace-layout/terminal-pane-tree' +import type { + GroupSide, + LayoutApplyReply, + LayoutCommandResult, + LayoutRefusalCode +} from '../../../shared/workspace-layout/workspace-layout-command-types' + +/** The commands a window sends `layout.apply`; main mints any new id. */ +export type WindowLayoutCommand = + | { type: 'splitPane'; tabId: string; leafId: string; direction: 'vertical' | 'horizontal' } + | { type: 'closePane'; tabId: string; leafId: string } + | { type: 'renamePane'; tabId: string; leafId: string; title: string | null } + | { type: 'equalizePanes'; tabId: string } + | { type: 'movePane'; tabId: string; leafId: string; targetLeafId: string; side: PaneSide } + | { type: 'setPaneRatios'; tabId: string; root: TerminalPaneLayoutNode } + | { type: 'moveTab'; tabId: string; toGroupId: string; index: number } + | { type: 'splitGroup'; tabId: string; besideGroupId: string; direction: GroupSide } + | { type: 'setGroupRatios'; groupLayout: TabGroupLayoutNode } + | { type: 'closeTabs'; tabIds: [string]; recordClose: 'user' | 'cleanup' } + +export type LayoutApplyHandlers = { + apply: (result: LayoutCommandResult) => void + asToday: () => void + refused?: (code: LayoutRefusalCode) => void +} + +/** + * A layout command in a layout-owner build: main applies it first, then `apply` shows main's + * result. `asToday` runs instead when main cannot act on it: a paired server holds the tab + * (#18202), main's copy lacks the workspace, tab, pane or group, or the call threw. `refused` on + * any other refusal. False when a paired server owns the workspace's layout; the caller acts as + * today. + */ +export function sendLayoutApply( + state: Parameters[0], + worktreeId: string, + command: WindowLayoutCommand, + handlers: LayoutApplyHandlers +): boolean { + // Why: the same owner test that mirrors a workspace's tabs from a paired server; a focused + // server must not claim a workspace whose row names no owner. + if (getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId)) { + return false + } + void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { + command: { ...command, workspace: worktreeId } + }) + .then( + (reply) => { + if (reply.ok) { + handlers.apply(reply.result) + } else if (reply.code === 'host_not_owned' || mainLacksTarget(reply.code)) { + // Why lacking: main's copy lags the window's here (the window still saves its own). + handlers.asToday() + } else { + console.warn(`[layout-owner] ${command.type} refused:`, reply.code) + handlers.refused?.(reply.code) + } + }, + (error: unknown) => { + // Why as today: a thrown write is rolled back. An indeterminate disk write is not, and may + // leave a pane main minted; accepted until main is the only writer. + console.warn(`[layout-owner] ${command.type} failed:`, errorMessage(error)) + handlers.asToday() + } + ) + // A throwing handler is not a failed call; it must not run the fallback too. + .catch((error: unknown) => { + console.warn(`[layout-owner] ${command.type} handler threw:`, errorMessage(error)) + }) + return true +} + +function mainLacksTarget(code: LayoutRefusalCode): boolean { + return ( + code === 'workspace_not_found' || + code === 'tab_not_found' || + code === 'pane_not_found' || + code === 'group_not_found' + ) +} + +function errorMessage(error: unknown): unknown { + return error instanceof Error ? error.message : error +} diff --git a/src/renderer/src/runtime/layout-owner-command.ts b/src/renderer/src/runtime/layout-owner-command.ts index e6d1972333a..da05e87b6c9 100644 --- a/src/renderer/src/runtime/layout-owner-command.ts +++ b/src/renderer/src/runtime/layout-owner-command.ts @@ -1,87 +1,15 @@ import { useAppStore } from '@/store' -import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' -import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' -import type { TabGroupLayoutNode } from '../../../shared/tab-types' -import type { TerminalPaneLayoutNode } from '../../../shared/terminal-tab-types' -import type { PaneSide } from '../../../shared/workspace-layout/terminal-pane-tree' -import type { - GroupSide, - LayoutApplyReply, - LayoutCommandResult, - LayoutRefusalCode -} from '../../../shared/workspace-layout/workspace-layout-command-types' +import { + sendLayoutApply, + type LayoutApplyHandlers, + type WindowLayoutCommand +} from './layout-apply-send' -/** The commands a window sends `layout.apply`; main mints any new id. */ -export type WindowLayoutCommand = - | { type: 'splitPane'; tabId: string; leafId: string; direction: 'vertical' | 'horizontal' } - | { type: 'closePane'; tabId: string; leafId: string } - | { type: 'renamePane'; tabId: string; leafId: string; title: string | null } - | { type: 'equalizePanes'; tabId: string } - | { type: 'movePane'; tabId: string; leafId: string; targetLeafId: string; side: PaneSide } - | { type: 'setPaneRatios'; tabId: string; root: TerminalPaneLayoutNode } - | { type: 'moveTab'; tabId: string; toGroupId: string; index: number } - | { type: 'splitGroup'; tabId: string; besideGroupId: string; direction: GroupSide } - | { type: 'setGroupRatios'; groupLayout: TabGroupLayoutNode } - -/** - * A layout command in a layout-owner build: main applies it first, then `apply` shows main's - * result. `asToday` runs instead when main cannot act on it: a paired server holds the tab - * (#18202), main's copy lacks the workspace, tab, pane or group, or the call threw. `refused` on - * any other refusal. False when a paired server owns the workspace's layout; the caller acts as - * today. - */ +/** `sendLayoutApply` against the current store. */ export function sendLayoutCommandToOwner( worktreeId: string, command: WindowLayoutCommand, - handlers: { - apply: (result: LayoutCommandResult) => void - asToday: () => void - refused?: (code: LayoutRefusalCode) => void - } + handlers: LayoutApplyHandlers ): boolean { - // Why: the same owner test that mirrors a workspace's tabs from a paired server; a focused - // server must not claim a workspace whose row names no owner. - if (getExplicitRuntimeEnvironmentIdForWorktree(useAppStore.getState(), worktreeId)) { - return false - } - void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { - command: { ...command, workspace: worktreeId } - }) - .then( - (reply) => { - if (reply.ok) { - handlers.apply(reply.result) - } else if (reply.code === 'host_not_owned' || mainLacksTarget(reply.code)) { - // Why lacking: main's copy lags the window's here (the window still saves its own). - handlers.asToday() - } else { - console.warn(`[layout-owner] ${command.type} refused:`, reply.code) - handlers.refused?.(reply.code) - } - }, - (error: unknown) => { - // Why as today: a thrown write is rolled back. An indeterminate disk write is not, and may - // leave a pane main minted; accepted until main is the only writer. - console.warn(`[layout-owner] ${command.type} failed:`, errorMessage(error)) - handlers.asToday() - } - ) - // A throwing handler is not a failed call; it must not run the fallback too. - .catch((error: unknown) => { - console.warn(`[layout-owner] ${command.type} handler threw:`, errorMessage(error)) - }) - return true -} - -function mainLacksTarget(code: LayoutRefusalCode): boolean { - return ( - code === 'workspace_not_found' || - code === 'tab_not_found' || - code === 'pane_not_found' || - code === 'group_not_found' - ) -} - -function errorMessage(error: unknown): unknown { - return error instanceof Error ? error.message : error + return sendLayoutApply(useAppStore.getState(), worktreeId, command, handlers) } diff --git a/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.test.ts b/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.test.ts new file mode 100644 index 00000000000..ef51fb31186 --- /dev/null +++ b/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { sendLayoutApply } from '@/runtime/layout-apply-send' +import { recordTerminalTabCloseThroughLayoutOwner } from './terminal-tab-close-layout-owner' + +const mocks = vi.hoisted(() => ({ + send: vi.fn(), + commit: vi.fn() +})) + +vi.mock('@/runtime/layout-apply-send', () => ({ sendLayoutApply: mocks.send })) +vi.mock('./terminal-surface-close-intent', () => ({ commitTerminalSurfaceClose: mocks.commit })) + +const WT = 'repo1::/path/wt1' +const STATE = { + unifiedTabsByWorktree: { + [WT]: [ + { + id: 'entry-legacy', + entityId: 'term-legacy', + contentType: 'terminal' as const, + groupId: 'g1', + worktreeId: WT, + label: 'Terminal', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } +} + +describe('recordTerminalTabCloseThroughLayoutOwner', () => { + beforeEach(() => { + mocks.send.mockReset() + mocks.commit.mockReset() + mocks.send.mockReturnValue(true) + }) + + it('names the tab by its tab-bar entry and keeps the window’s reason', () => { + expect(recordTerminalTabCloseThroughLayoutOwner(STATE, WT, 'term-legacy', 'cleanup')).toBe(true) + + expect(mocks.send).toHaveBeenCalledWith( + STATE, + WT, + { type: 'closeTabs', tabIds: ['entry-legacy'], recordClose: 'cleanup' }, + expect.anything() + ) + expect(mocks.commit).not.toHaveBeenCalled() + }) + + it('records the close as today when main cannot', () => { + recordTerminalTabCloseThroughLayoutOwner(STATE, WT, 'term-new', 'user') + expect(mocks.send.mock.calls[0]![2]).toMatchObject({ tabIds: ['term-new'] }) + + mocks.send.mock.calls[0]![3].asToday() + + expect(mocks.commit).toHaveBeenCalledWith(WT, { kind: 'tab', tabId: 'term-new' }, 'user') + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.ts b/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.ts new file mode 100644 index 00000000000..1c76b462569 --- /dev/null +++ b/src/renderer/src/store/terminals/terminal-tab-close-layout-owner.ts @@ -0,0 +1,32 @@ +import type { AppState } from '../types' +import type { WorktreeRuntimeOwnerState } from '@/lib/worktree-runtime-owner-state' +import { sendLayoutApply } from '@/runtime/layout-apply-send' +import { commitTerminalSurfaceClose } from './terminal-surface-close-intent' + +/** + * A layout-owner build's record of a terminal tab this window closed: main records it through + * `layout.apply`, or today's close intent does when main cannot. False when a paired server owns + * the workspace's layout; the caller records it as today. + */ +export function recordTerminalTabCloseThroughLayoutOwner( + state: WorktreeRuntimeOwnerState & Pick, + worktreeId: string, + tabId: string, + reason: 'user' | 'cleanup' +): boolean { + // Main's layout names a tab by its tab-bar entry, which a legacy terminal's id can differ from. + const entryId = + state.unifiedTabsByWorktree[worktreeId]?.find( + (entry) => entry.contentType === 'terminal' && entry.entityId === tabId + )?.id ?? tabId + const keep = (): void => {} + return sendLayoutApply( + state, + worktreeId, + { type: 'closeTabs', tabIds: [entryId], recordClose: reason }, + { + apply: keep, + asToday: () => commitTerminalSurfaceClose(worktreeId, { kind: 'tab', tabId }, reason) + } + ) +} diff --git a/src/renderer/src/store/terminals/terminal-tab-close.ts b/src/renderer/src/store/terminals/terminal-tab-close.ts index e2ae97e5177..c1daffc0b74 100644 --- a/src/renderer/src/store/terminals/terminal-tab-close.ts +++ b/src/renderer/src/store/terminals/terminal-tab-close.ts @@ -14,6 +14,8 @@ import { import type { TerminalSlice, TerminalStoreGet, TerminalStoreSet } from './terminal-state' import { startTerminalTabProviderRetirement } from './terminal-tab-close-providers' import { commitTerminalSurfaceClose } from './terminal-surface-close-intent' +import { recordTerminalTabCloseThroughLayoutOwner } from './terminal-tab-close-layout-owner' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' import { omitUnverifiedPtyLossTabIds } from './terminal-unverified-pty-loss' import { removePaneKeysByTabPrefix } from '../slices/agent-status-pane-keyed-records' import { removeTabsAttention } from '../slices/workspace-attention/subject-attention-transitions' @@ -245,7 +247,13 @@ export function createTerminalTabCloseActions( }) ) if (intentReason && closingWorktreeId && opts?.remoteCloseOwnedByHost !== true) { - commitTerminalSurfaceClose(closingWorktreeId, { kind: 'tab', tabId }, intentReason) + // Switch point: main records the close through layout.apply. + if ( + !ORCA_LAYOUT_OWNER_IS_MAIN || + !recordTerminalTabCloseThroughLayoutOwner(get(), closingWorktreeId, tabId, intentReason) + ) { + commitTerminalSurfaceClose(closingWorktreeId, { kind: 'tab', tabId }, intentReason) + } } // Why only a user close: it is the explicit abandon. Other closes keep the drafts until their // workspace is removed in Orca; drafts have no budget that retires them. diff --git a/src/shared/rpc-contract/workspace-layout-params.ts b/src/shared/rpc-contract/workspace-layout-params.ts index d2eaa9e1fb0..3c2d78eaa0f 100644 --- a/src/shared/rpc-contract/workspace-layout-params.ts +++ b/src/shared/rpc-contract/workspace-layout-params.ts @@ -81,6 +81,14 @@ const SetGroupRatiosCommand = z.object({ groupLayout: TerminalOrphanGroupLayout }) +/** The window's close of one tab it already closed; main records it, the window's reason kept. */ +const CloseTabsCommand = z.object({ + type: z.literal('closeTabs'), + workspace: OnPane.workspace, + tabIds: z.tuple([z.string().min(1)]), + recordClose: z.enum(['user', 'cleanup']) +}) + /** * A window's layout command (`layout.apply`, registered only in a layout-owner build). The window * names what it acts on; main mints every new id, so no new id is accepted here. @@ -95,6 +103,7 @@ export const LayoutApplyParams = z.object({ SetPaneRatiosCommand, MoveTabCommand, SplitGroupCommand, - SetGroupRatiosCommand + SetGroupRatiosCommand, + CloseTabsCommand ]) }) diff --git a/src/shared/workspace-layout/workspace-layout-command-types.ts b/src/shared/workspace-layout/workspace-layout-command-types.ts index a9461bc09d4..e3ea3d10a0d 100644 --- a/src/shared/workspace-layout/workspace-layout-command-types.ts +++ b/src/shared/workspace-layout/workspace-layout-command-types.ts @@ -58,6 +58,11 @@ export type LayoutCommand = refusePinned?: boolean /** Editor tabs whose view reported an unsaved draft (rule 10). */ dirtyTabIds?: string[] + /** + * Records each terminal tab closed, or not listed, as closed for this reason, so a start + * that commits later is refused. Main sets it on a window's close. + */ + recordClose?: 'user' | 'cleanup' }) | (On & { type: 'moveTab'; tabId: string; toGroupId: string; index: number }) | (On & { type: 'splitGroup'; tabId: string; besideGroupId: string; direction: GroupSide }) diff --git a/src/shared/workspace-layout/workspace-layout-commands.ts b/src/shared/workspace-layout/workspace-layout-commands.ts index afc337cbd32..5ca6d213d61 100644 --- a/src/shared/workspace-layout/workspace-layout-commands.ts +++ b/src/shared/workspace-layout/workspace-layout-commands.ts @@ -68,7 +68,7 @@ function applyCommand( case 'closePane': return closePane(model, command) case 'closeTabs': - return closeTabs(model, command) + return closeTabs(model, command, context) case 'moveTab': return moveTab(model, command) case 'splitGroup': diff --git a/src/shared/workspace-layout/workspace-layout-record-patch.test.ts b/src/shared/workspace-layout/workspace-layout-record-patch.test.ts index 275a789dff4..54f0b2196f8 100644 --- a/src/shared/workspace-layout/workspace-layout-record-patch.test.ts +++ b/src/shared/workspace-layout/workspace-layout-record-patch.test.ts @@ -24,6 +24,13 @@ function commandsFor(model: WorkspaceLayoutModel): LayoutCommand[] { for (const [workspace, layout] of Object.entries(model.workspaces)) { commands.push({ type: 'createTerminalTab', workspace }) commands.push({ type: 'openEditorTab', workspace, fileId: '/new.md', contentType: 'editor' }) + commands.push({ + type: 'closeTabs', + workspace, + tabIds: ['tab-not-listed'], + force: true, + recordClose: 'cleanup' + }) const lastGroup = layout.groups.at(-1) if (layout.groupLayout?.type === 'split') { commands.push({ @@ -34,6 +41,13 @@ function commandsFor(model: WorkspaceLayoutModel): LayoutCommand[] { } for (const tab of layout.tabs) { commands.push({ type: 'closeTabs', workspace, tabIds: [tab.id], force: true }) + commands.push({ + type: 'closeTabs', + workspace, + tabIds: [tab.id], + force: true, + recordClose: 'user' + }) commands.push({ type: 'renameTab', workspace, @@ -215,8 +229,15 @@ describe('patchStoredSession', () => { ...changedRecords(diffStoredSession(before, after)), ...loadForms ]) + // A map the stored partition lacks is reported whole on one side, by record on the other. + const wholeMaps = commanded.includes('session/closedTerminalTabTombstonesByTabId') + ? ['closedTab/'] + : [] expect( - written.filter((record) => !commanded.includes(record)), + written.filter( + (record) => + !commanded.includes(record) && !wholeMaps.some((table) => record.startsWith(table)) + ), `${label} commanded=${commanded.join(',')}` ).toEqual([]) } @@ -313,6 +334,29 @@ describe('patchStoredSession', () => { }) }) + it('does not write a sibling the load supplied when a close only renumbers it', () => { + const profile = LEGACY_WORKSPACE_LAYOUT_PROFILES.find( + (candidate) => candidate.name === 'row without a tab-bar entry, still bound to a terminal' + )! + const stored = profile.build() + const loaded = loadWorkspaceLayout(profile.hostId, stored, nameBasedLoadContext()) + const [workspace] = Object.keys(stored.tabsByWorktree) + const result = applyLayoutCommand( + loaded.layout, + { type: 'closeTabs', workspace: workspace!, tabIds: ['tab-a'], force: true }, + CONTEXT + ) + if (!result.ok) { + throw new Error(result.code) + } + + const after = saveWorkspaceLayout({ ...loaded, layout: result.model }) + const patched = patchStoredSession(stored, saveWorkspaceLayout(loaded), after) + + expect(after.unifiedTabs?.[workspace!]?.map((entry) => entry.entityId)).toEqual(['tab-b']) + expect(patched.unifiedTabs?.[workspace!] ?? []).toEqual([]) + }) + it('keeps a legacy partition’s unrelated records as stored when a pane splits', () => { const profile = LEGACY_WORKSPACE_LAYOUT_PROFILES.find( (candidate) => candidate.name === 'session from before the tab bar, with an open file' diff --git a/src/shared/workspace-layout/workspace-layout-record-patch.ts b/src/shared/workspace-layout/workspace-layout-record-patch.ts index 2ede8f357d1..f0a8fe823fa 100644 --- a/src/shared/workspace-layout/workspace-layout-record-patch.ts +++ b/src/shared/workspace-layout/workspace-layout-record-patch.ts @@ -97,6 +97,12 @@ function patchList(stored: Json[], before: Json[], after: Json[], idOf: (record: const find = (list: Json[], id: unknown) => list.find((record) => idOf(record) === id) const result = [...stored] const ids = new Set([...before.map(idOf), ...after.map(idOf)]) + const kept = (list: Json[], other: Json[]) => { + const otherIds = new Set(other.map(idOf)) + return list.map(idOf).filter((id) => otherIds.has(id)) + } + // Records kept their relative order: sort orders changed only to close a removal's gap. + const renumberedOnly = sameJson(kept(before, after), kept(after, before)) for (const id of ids) { const old = find(before, id) const next = find(after, id) @@ -104,6 +110,17 @@ function patchList(stored: Json[], before: Json[], after: Json[], idOf: (record: continue } const index = result.findIndex((record) => idOf(record) === id) + // A record the load supplied that the command only renumbered stays unwritten (a close's + // siblings); writing it whole would add it where the stored partition has none. + if ( + index === -1 && + renumberedOnly && + old && + next && + sameJson({ ...old, sortOrder: 0 }, { ...next, sortOrder: 0 }) + ) { + continue + } if (!next) { if (index !== -1) { result.splice(index, 1) diff --git a/src/shared/workspace-layout/workspace-layout-tab-commands.ts b/src/shared/workspace-layout/workspace-layout-tab-commands.ts index 3ab42a7a9ed..1590fc62b06 100644 --- a/src/shared/workspace-layout/workspace-layout-tab-commands.ts +++ b/src/shared/workspace-layout/workspace-layout-tab-commands.ts @@ -21,6 +21,7 @@ import type { } from './workspace-layout-command-types' import { paneKeyOf, + type LayoutClosedTab, type LayoutContentTab, type LayoutTerminalTab, type WorkspaceLayout, @@ -98,15 +99,21 @@ export function closeTab( return withWorkspace(model, key, next) } -export function closeTabs(model: WorkspaceLayoutModel, command: CommandOf<'closeTabs'>): Applied { +export function closeTabs( + model: WorkspaceLayoutModel, + command: CommandOf<'closeTabs'>, + context?: LayoutContext +): Applied { let next = model const closed: string[] = [] + const recorded: string[] = [] const refused: RefusedClose[] = [] const stopPtyIds: string[] = [] for (const tabId of command.tabIds) { const tab = findTab(next.workspaces[command.workspace]!, tabId) if (!tab) { closed.push(tabId) + recorded.push(tabId) continue } if (!command.force && command.refusePinned && tab.isPinned) { @@ -120,10 +127,33 @@ export function closeTabs(model: WorkspaceLayoutModel, command: CommandOf<'close stopPtyIds.push(...boundPtyIds(next.workspaces[command.workspace]!, tab)) next = closeTab(next, command.workspace, tabId) closed.push(tabId) + if (tab.kind === 'terminal') { + recorded.push(tab.entityId) + } + } + if (command.recordClose && context && recorded.length > 0) { + next = recordClosedTerminalTabs(next, command.workspace, recorded, { + reason: command.recordClose, + closedAt: context.now() + }) } return applied(next, { closed, refused }, { stopPtyIds }) } +function recordClosedTerminalTabs( + model: WorkspaceLayoutModel, + key: string, + tabIds: readonly string[], + record: LayoutClosedTab +): WorkspaceLayoutModel { + const workspace = model.workspaces[key]! + const closedTerminalTabs = { ...workspace.closedTerminalTabs } + for (const tabId of tabIds) { + closedTerminalTabs[tabId] = record + } + return withWorkspace(model, key, { ...workspace, closedTerminalTabs }) +} + export function renameTab(model: WorkspaceLayoutModel, command: CommandOf<'renameTab'>): Applied { const tab = findTab(model.workspaces[command.workspace]!, command.tabId) if (!tab) {