diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 9666ae98e53..fc2da132f37 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -136,8 +136,7 @@ import { } from '@/runtime/runtime-terminal-inspection' import { clearWebRuntimeTerminalBuffer, - closeWebRuntimeTerminal, - updateWebRuntimePaneLayout + closeWebRuntimeTerminal } from '@/runtime/web-runtime-session' import { armPrimarySelectionNativePasteSuppression, @@ -158,6 +157,10 @@ import { planTerminalLiveLayoutInsertions } from './terminal-live-layout-reconciliation' import type { TerminalQuickCommand, TerminalQuickCommandScope } from '../../../../shared/types' +import { + createRemotePaneLayoutPusher, + type RemotePaneLayoutPusher +} from './remote-pane-layout-push' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { isRuntimeOwnedSshTargetId } from '../../../../shared/execution-host' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id' @@ -512,6 +515,8 @@ function TerminalPane( paneTitlesRef.current = paneTitles const removedTitleLeafIdsRef = useRef>(new Set()) const clearedScrollbackLeafIdsRef = useRef>(new Set()) + const remotePaneLayoutPusherRef = useRef(null) + remotePaneLayoutPusherRef.current ??= createRemotePaneLayoutPusher() const [paneTitleOverlayRects, setPaneTitleOverlayRects] = useState< Record >({}) @@ -1057,13 +1062,7 @@ function TerminalPane( (ptyId) => typeof ptyId === 'string' && isRemoteRuntimePtyId(ptyId) ) if (hasRemotePane) { - void updateWebRuntimePaneLayout({ - worktreeId, - tabId, - root: layout.root, - expandedLeafId: layout.expandedLeafId, - ...(layout.titlesByLeafId ? { titlesByLeafId: layout.titlesByLeafId } : {}) - }) + remotePaneLayoutPusherRef.current?.push({ worktreeId, tabId, layout }) } for (const leafId of currentLeafIds) { clearedScrollbackLeafIds.delete(leafId) diff --git a/src/renderer/src/components/terminal-pane/remote-pane-layout-push.test.ts b/src/renderer/src/components/terminal-pane/remote-pane-layout-push.test.ts new file mode 100644 index 00000000000..d6d777f728c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-pane-layout-push.test.ts @@ -0,0 +1,183 @@ +/** + * Perf regression: layout persists fire on pane-title churn, and every one of + * them used to push a remote-runtime IPC round trip regardless of whether the + * host-visible layout had moved. Counting invocations at the mocked IPC boundary + * pins the before/after: 100 persists with an unchanged layout cost 100 pushes + * before the dedupe and 1 after. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../../../shared/types' + +const updateWebRuntimePaneLayout = vi.fn() +vi.mock('@/runtime/web-runtime-session', () => ({ + updateWebRuntimePaneLayout: (...args: unknown[]) => updateWebRuntimePaneLayout(...args) +})) + +const { createRemotePaneLayoutPusher } = await import('./remote-pane-layout-push') + +const PERSISTS = 100 + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve = (_value: T): void => undefined + const promise = new Promise((complete) => { + resolve = complete + }) + return { promise, resolve } +} + +function makeLayout(overrides: Partial = {}): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + ratio: 0.5, + first: { type: 'leaf', leafId: 'leaf-a' }, + second: { type: 'leaf', leafId: 'leaf-b' } + }, + activeLeafId: 'leaf-a', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-a': 'remote:pty-a', 'leaf-b': 'remote:pty-b' }, + titlesByLeafId: { 'leaf-a': 'build' }, + ...overrides + } +} + +describe('createRemotePaneLayoutPusher', () => { + beforeEach(() => { + updateWebRuntimePaneLayout.mockReset().mockResolvedValue(true) + }) + + it('pushes once across 100 persists of an unchanged layout', () => { + const pusher = createRemotePaneLayoutPusher() + for (let i = 0; i < PERSISTS; i += 1) { + // Fresh object each time: persistLayoutSnapshot re-serializes on every call. + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + } + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(1) + }) + + it('sends the same payload the un-deduped path sent', () => { + const pusher = createRemotePaneLayoutPusher() + const layout = makeLayout() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledWith({ + worktreeId: 'wt-1', + tabId: 'tab-1', + root: layout.root, + expandedLeafId: layout.expandedLeafId, + titlesByLeafId: layout.titlesByLeafId + }) + }) + + it('omits titlesByLeafId when the layout carries no titles', () => { + const pusher = createRemotePaneLayoutPusher() + pusher.push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout({ titlesByLeafId: undefined }) + }) + expect(updateWebRuntimePaneLayout.mock.calls[0][0]).not.toHaveProperty('titlesByLeafId') + }) + + it('pushes again for every host-visible change', () => { + const pusher = createRemotePaneLayoutPusher() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + pusher.push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout({ expandedLeafId: 'leaf-a' }) + }) + pusher.push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout({ + expandedLeafId: 'leaf-a', + titlesByLeafId: { 'leaf-a': 'test' } + }) + }) + pusher.push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout({ + expandedLeafId: 'leaf-a', + titlesByLeafId: { 'leaf-a': 'test' }, + root: { + type: 'split', + direction: 'vertical', + ratio: 0.7, + first: { type: 'leaf', leafId: 'leaf-a' }, + second: { type: 'leaf', leafId: 'leaf-b' } + } + }) + }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(4) + }) + + it('pushes again when a remote pane swaps its pty', () => { + // ptyIdsByLeafId is not in the payload, but a swap means a different host session. + const pusher = createRemotePaneLayoutPusher() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + pusher.push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout({ ptyIdsByLeafId: { 'leaf-a': 'remote:pty-c', 'leaf-b': 'remote:pty-b' } }) + }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(2) + }) + + it('does not carry a cached layout across tabs', () => { + const pusher = createRemotePaneLayoutPusher() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-2', layout: makeLayout() }) + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(3) + }) + + it('does not carry a cached layout across worktrees', () => { + const pusher = createRemotePaneLayoutPusher() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + pusher.push({ worktreeId: 'wt-2', tabId: 'tab-1', layout: makeLayout() }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(2) + }) + + it('retries an unchanged layout after a failed push', async () => { + updateWebRuntimePaneLayout.mockResolvedValueOnce(false) + const pusher = createRemotePaneLayoutPusher() + const input = { worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() } + + pusher.push(input) + await Promise.resolve() + pusher.push(input) + + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(2) + }) + + it('does not let a stale failure invalidate a newer in-flight layout', async () => { + const first = deferred() + const second = deferred() + updateWebRuntimePaneLayout.mockImplementationOnce(() => first.promise) + updateWebRuntimePaneLayout.mockImplementationOnce(() => second.promise) + const pusher = createRemotePaneLayoutPusher() + const changedLayout = makeLayout({ expandedLeafId: 'leaf-a' }) + + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: changedLayout }) + first.resolve(false) + await Promise.resolve() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: changedLayout }) + + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(2) + second.resolve(true) + }) + + it('re-pushes after a remount re-establishes host geometry', () => { + const pusher = createRemotePaneLayoutPusher() + pusher.push({ worktreeId: 'wt-1', tabId: 'tab-1', layout: makeLayout() }) + createRemotePaneLayoutPusher().push({ + worktreeId: 'wt-1', + tabId: 'tab-1', + layout: makeLayout() + }) + expect(updateWebRuntimePaneLayout).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-pane-layout-push.ts b/src/renderer/src/components/terminal-pane/remote-pane-layout-push.ts new file mode 100644 index 00000000000..506bd7ea42f --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-pane-layout-push.ts @@ -0,0 +1,47 @@ +import type { TerminalLayoutSnapshot } from '../../../../shared/types' +import { terminalLayoutEqual } from '@/lib/terminal-layout-equality' +import { updateWebRuntimePaneLayout } from '@/runtime/web-runtime-session' + +export type RemotePaneLayoutPusher = { + push: (input: { worktreeId: string; tabId: string; layout: TerminalLayoutSnapshot }) => void +} + +/** + * Pane geometry is host-authoritative for remote tabs, so persists must push it — but + * persists also fire on pane-title churn, which leaves the host-visible layout untouched. + * Dedupe against the last push so unchanged layouts cost no remote round trip. + */ +export function createRemotePaneLayoutPusher(): RemotePaneLayoutPusher { + let lastAttempt: { + id: number + worktreeId: string + tabId: string + snapshot: TerminalLayoutSnapshot + } | null = null + let nextAttemptId = 0 + return { + push: ({ worktreeId, tabId, layout }) => { + if ( + lastAttempt?.worktreeId === worktreeId && + lastAttempt.tabId === tabId && + terminalLayoutEqual(lastAttempt.snapshot, layout) + ) { + return + } + const attempt = { id: ++nextAttemptId, worktreeId, tabId, snapshot: layout } + lastAttempt = attempt + void updateWebRuntimePaneLayout({ + worktreeId, + tabId, + root: layout.root, + expandedLeafId: layout.expandedLeafId, + ...(layout.titlesByLeafId ? { titlesByLeafId: layout.titlesByLeafId } : {}) + }).then((updated) => { + // Why: a disconnected or timed-out push carried no information, so the next persist must retry it. + if (!updated && lastAttempt?.id === attempt.id) { + lastAttempt = null + } + }) + } + } +} diff --git a/src/renderer/src/lib/terminal-layout-equality.ts b/src/renderer/src/lib/terminal-layout-equality.ts new file mode 100644 index 00000000000..ce11907bed1 --- /dev/null +++ b/src/renderer/src/lib/terminal-layout-equality.ts @@ -0,0 +1,55 @@ +import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode } from '../../../shared/types' + +function sameStringRecord( + a: Readonly> | undefined, + b: Readonly> | undefined +): boolean { + const left = a ?? {} + const right = b ?? {} + const leftKeys = Object.keys(left) + const rightKeys = Object.keys(right) + return ( + leftKeys.length === rightKeys.length && + leftKeys.every( + (key) => Object.prototype.hasOwnProperty.call(right, key) && left[key] === right[key] + ) + ) +} + +export function terminalLayoutNodeEqual( + a: TerminalPaneLayoutNode | null | undefined, + b: TerminalPaneLayoutNode | null | undefined +): boolean { + if (!a || !b) { + return !a && !b + } + if (a.type !== b.type) { + return false + } + if (a.type === 'leaf') { + return b.type === 'leaf' && a.leafId === b.leafId + } + return ( + b.type === 'split' && + a.direction === b.direction && + a.ratio === b.ratio && + terminalLayoutNodeEqual(a.first, b.first) && + terminalLayoutNodeEqual(a.second, b.second) + ) +} + +/** Structural equality over every persisted layout field; drives store/IPC write bailouts. */ +export function terminalLayoutEqual( + a: TerminalLayoutSnapshot | undefined, + b: TerminalLayoutSnapshot +): boolean { + return ( + terminalLayoutNodeEqual(a?.root, b.root) && + (a?.activeLeafId ?? null) === b.activeLeafId && + (a?.expandedLeafId ?? null) === b.expandedLeafId && + sameStringRecord(a?.ptyIdsByLeafId, b.ptyIdsByLeafId) && + sameStringRecord(a?.buffersByLeafId, b.buffersByLeafId) && + sameStringRecord(a?.scrollbackRefsByLeafId, b.scrollbackRefsByLeafId) && + sameStringRecord(a?.titlesByLeafId, b.titlesByLeafId) + ) +} diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index d944272accb..f3447e62643 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -25,13 +25,13 @@ import type { TabGroup, TabGroupLayoutNode, TerminalLayoutSnapshot, - TerminalPaneLayoutNode, TerminalTab } from '../../../shared/types' import type { OpenFile } from '../store/slices/editor' import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id' import { getRemoteRuntimePtyEnvironmentId, toRemoteRuntimePtyId } from './runtime-terminal-stream' import { sanitizeTerminalLayoutPaneTitlesForLabels } from '@/lib/terminal-pane-title-sanitization' +import { terminalLayoutEqual } from '@/lib/terminal-layout-equality' import { normalizeTerminalLayoutPtyOwnership } from '@/components/terminal-pane/terminal-layout-pty-ownership' import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { getReachableRuntimeSessionMirrorTargets } from '@/lib/runtime-session-mirror-targets' @@ -1424,59 +1424,6 @@ function isMirroredCommandCodeTurnBump( ) } -function sameStringRecord( - a: Readonly> | undefined, - b: Readonly> | undefined -): boolean { - const left = a ?? {} - const right = b ?? {} - const leftKeys = Object.keys(left) - const rightKeys = Object.keys(right) - return ( - leftKeys.length === rightKeys.length && - leftKeys.every( - (key) => Object.prototype.hasOwnProperty.call(right, key) && left[key] === right[key] - ) - ) -} - -function terminalLayoutNodeEqual( - a: TerminalPaneLayoutNode | null | undefined, - b: TerminalPaneLayoutNode | null | undefined -): boolean { - if (!a || !b) { - return !a && !b - } - if (a.type !== b.type) { - return false - } - if (a.type === 'leaf') { - return b.type === 'leaf' && a.leafId === b.leafId - } - return ( - b.type === 'split' && - a.direction === b.direction && - a.ratio === b.ratio && - terminalLayoutNodeEqual(a.first, b.first) && - terminalLayoutNodeEqual(a.second, b.second) - ) -} - -function terminalLayoutEqual( - a: TerminalLayoutSnapshot | undefined, - b: TerminalLayoutSnapshot -): boolean { - return ( - terminalLayoutNodeEqual(a?.root, b.root) && - (a?.activeLeafId ?? null) === b.activeLeafId && - (a?.expandedLeafId ?? null) === b.expandedLeafId && - sameStringRecord(a?.ptyIdsByLeafId, b.ptyIdsByLeafId) && - sameStringRecord(a?.buffersByLeafId, b.buffersByLeafId) && - sameStringRecord(a?.scrollbackRefsByLeafId, b.scrollbackRefsByLeafId) && - sameStringRecord(a?.titlesByLeafId, b.titlesByLeafId) - ) -} - function sanitizeRecentTabIds(recent: string[] | undefined, tabOrder: string[]): string[] { if (!recent || recent.length === 0) { return [] diff --git a/src/renderer/src/store/slices/terminal-write-identity-bailouts.test.ts b/src/renderer/src/store/slices/terminal-write-identity-bailouts.test.ts new file mode 100644 index 00000000000..bfd5ab13964 --- /dev/null +++ b/src/renderer/src/store/slices/terminal-write-identity-bailouts.test.ts @@ -0,0 +1,227 @@ +/** + * Perf regression: setCacheTimerStartedAt and setTabLayout must not publish + * state when the write is a no-op. + * + * Both actions previously spread a fresh object and returned it unconditionally, + * so every redundant call produced a new AppState object and woke EVERY zustand + * subscriber — which, with per-pane selectors across all mounted panes, is an + * O(panes) sweep per write. The cadence is real: parked-terminal-byte-watcher + * writes a null cache timer on every agent working/exit/stale-title transition, + * and TerminalPane re-persists its layout on pane-title churn. + * + * These tests count subscriber wakeups on a real store; the pre-fix numbers are + * 1_000 (one per call), the post-fix numbers are 0. + */ +import { describe, expect, it } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../../../shared/types' +import { createTestStore } from './store-test-helpers' + +const REPEATS = 1_000 + +function makeLayout(overrides: Partial = {}): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + ratio: 0.5, + first: { type: 'leaf', leafId: 'leaf-a' }, + second: { type: 'leaf', leafId: 'leaf-b' } + }, + activeLeafId: 'leaf-a', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-a': 'pty-a', 'leaf-b': 'pty-b' }, + titlesByLeafId: { 'leaf-a': 'build' }, + ...overrides + } +} + +describe('setCacheTimerStartedAt identity bailout', () => { + it('wakes zero subscribers across 1,000 null-over-null writes', () => { + const store = createTestStore() + const paneKey = 'tab-1:leaf-a' + store.getState().setCacheTimerStartedAt(paneKey, null) + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + for (let i = 0; i < REPEATS; i += 1) { + store.getState().setCacheTimerStartedAt(paneKey, null) + } + unsubscribe() + + expect(wakeups).toBe(0) + expect(store.getState().cacheTimerByKey[paneKey]).toBeNull() + }) + + it('wakes zero subscribers across 1,000 repeats of the same timestamp', () => { + const store = createTestStore() + const paneKey = 'tab-1:leaf-a' + store.getState().setCacheTimerStartedAt(paneKey, 1_700_000_000_000) + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + for (let i = 0; i < REPEATS; i += 1) { + store.getState().setCacheTimerStartedAt(paneKey, 1_700_000_000_000) + } + unsubscribe() + + expect(wakeups).toBe(0) + }) + + it('still publishes when the timestamp actually changes', () => { + const store = createTestStore() + const paneKey = 'tab-1:leaf-a' + store.getState().setCacheTimerStartedAt(paneKey, null) + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + store.getState().setCacheTimerStartedAt(paneKey, 123) + store.getState().setCacheTimerStartedAt(paneKey, null) + unsubscribe() + + expect(wakeups).toBe(2) + expect(store.getState().cacheTimerByKey[paneKey]).toBeNull() + }) + + it('records the first write for a key even when the value is null', () => { + const store = createTestStore() + const paneKey = 'tab-1:leaf-a' + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + store.getState().setCacheTimerStartedAt(paneKey, null) + unsubscribe() + + expect(wakeups).toBe(1) + expect(paneKey in store.getState().cacheTimerByKey).toBe(true) + }) + + it('still clears a stale :seed sentinel when the pane value is unchanged', () => { + // Without this carve-out the bailout would strand the sentinel and leave a phantom timer. + const store = createTestStore() + const paneKey = 'tab-1:leaf-a' + store.getState().setCacheTimerStartedAt(paneKey, null) + store.setState((s) => ({ cacheTimerByKey: { ...s.cacheTimerByKey, 'tab-1:seed': 42 } })) + + store.getState().setCacheTimerStartedAt(paneKey, null) + + expect('tab-1:seed' in store.getState().cacheTimerByKey).toBe(false) + }) +}) + +describe('setTabLayout identity bailout', () => { + it('wakes zero subscribers across 1,000 structurally identical snapshots', () => { + const store = createTestStore() + store.getState().setTabLayout('tab-1', makeLayout()) + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + for (let i = 0; i < REPEATS; i += 1) { + // Fresh object each iteration: this is what persistLayoutSnapshot produces. + store.getState().setTabLayout('tab-1', makeLayout()) + } + unsubscribe() + + expect(wakeups).toBe(0) + }) + + it('keeps the stored snapshot reference stable when nothing changed', () => { + const store = createTestStore() + store.getState().setTabLayout('tab-1', makeLayout()) + const first = store.getState().terminalLayoutsByTabId['tab-1'] + + store.getState().setTabLayout('tab-1', makeLayout()) + + expect(store.getState().terminalLayoutsByTabId['tab-1']).toBe(first) + }) + + it('publishes when any tracked field changes', () => { + const store = createTestStore() + store.getState().setTabLayout('tab-1', makeLayout()) + + const mutations: Partial[] = [ + { activeLeafId: 'leaf-b' }, + { expandedLeafId: 'leaf-a' }, + { titlesByLeafId: { 'leaf-a': 'test' } }, + { ptyIdsByLeafId: { 'leaf-a': 'pty-a', 'leaf-b': 'pty-c' } }, + { buffersByLeafId: { 'leaf-a': 'scrollback' } }, + { scrollbackRefsByLeafId: { 'leaf-a': 'ref-1' } }, + { + root: { + type: 'split', + direction: 'horizontal', + ratio: 0.5, + first: { type: 'leaf', leafId: 'leaf-a' }, + second: { type: 'leaf', leafId: 'leaf-b' } + } + }, + { + root: { + type: 'split', + direction: 'vertical', + ratio: 0.7, + first: { type: 'leaf', leafId: 'leaf-a' }, + second: { type: 'leaf', leafId: 'leaf-b' } + } + } + ] + + for (const mutation of mutations) { + store.getState().setTabLayout('tab-1', makeLayout()) + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + store.getState().setTabLayout('tab-1', makeLayout(mutation)) + unsubscribe() + expect(wakeups, `expected a publish for ${JSON.stringify(mutation)}`).toBe(1) + } + }) + + it('does not fire pane-ownership transfers for a bailed-out identical layout', () => { + // A duplicate-pty layout normalizes to a transfer; replaying the already-normalized + // snapshot must be inert, since normalization then finds nothing to move. + const store = createTestStore() + const duplicate = makeLayout({ + ptyIdsByLeafId: { 'leaf-a': 'pty-a', 'leaf-b': 'pty-a' } + }) + store.getState().setTabLayout('tab-1', duplicate) + const normalized = store.getState().terminalLayoutsByTabId['tab-1'] + expect(normalized.ptyIdsByLeafId).not.toEqual(duplicate.ptyIdsByLeafId) + + store.getState().markTerminalPaneUnread('tab-1:leaf-a') + const beforeUnread = { ...store.getState().unreadTerminalPanes } + + for (let i = 0; i < REPEATS; i += 1) { + store.getState().setTabLayout('tab-1', { ...normalized }) + } + + expect(store.getState().terminalLayoutsByTabId['tab-1']).toBe(normalized) + expect(store.getState().unreadTerminalPanes).toEqual(beforeUnread) + }) + + it('still deletes the layout on a clearing call, and bails when already absent', () => { + const store = createTestStore() + store.getState().setTabLayout('tab-1', makeLayout()) + + store.getState().setTabLayout('tab-1', null) + expect('tab-1' in store.getState().terminalLayoutsByTabId).toBe(false) + + let wakeups = 0 + const unsubscribe = store.subscribe(() => { + wakeups += 1 + }) + store.getState().setTabLayout('tab-1', null) + unsubscribe() + expect(wakeups).toBe(0) + }) +}) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 8c41cedab6e..d98c8b82079 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -54,6 +54,7 @@ import type { AgentStartedTelemetry } from '../../lib/worktree-activation' import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { forgetAgentHibernationTabOutput } from '@/lib/agent-hibernation-output-activity' import { forgetForegroundTerminalTabs } from '@/lib/foreground-terminal-tabs' +import { terminalLayoutEqual } from '@/lib/terminal-layout-equality' import { forgetAgentStartupDeliveriesForTabs } from '@/lib/agent-startup-delivery-guards' import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers' import { @@ -1266,15 +1267,18 @@ export const createTerminalSlice: StateCreator setCacheTimerStartedAt: (key, ts) => { set((s) => { - const next = { ...s.cacheTimerByKey, [key]: ts } // Why: a real pane write clears any ':seed' sentinel from seedCacheTimersForIdleTabs, avoiding phantom timers when the seed key doesn't match the real pane. const colonIdx = key.indexOf(':') - if (colonIdx !== -1) { - const tabId = key.slice(0, colonIdx) - const suffix = key.slice(colonIdx + 1) - if (suffix !== 'seed') { - delete next[`${tabId}:seed`] - } + const suffix = colonIdx === -1 ? null : key.slice(colonIdx + 1) + const seedKey = colonIdx !== -1 && suffix !== 'seed' ? `${key.slice(0, colonIdx)}:seed` : null + const hasStaleSeed = seedKey !== null && seedKey in s.cacheTimerByKey + // Why: parked-pane watchers replay null-over-null on every working/exit transition; each redundant write runs every subscriber's selector. + if (s.cacheTimerByKey[key] === ts && !hasStaleSeed) { + return s + } + const next = { ...s.cacheTimerByKey, [key]: ts } + if (seedKey !== null) { + delete next[seedKey] } return { cacheTimerByKey: next } }) @@ -3649,20 +3653,27 @@ export const createTerminalSlice: StateCreator setTabLayout: (tabId, layout) => { let ownershipTransfers: ReturnType = [] set((s) => { - const next = { ...s.terminalLayoutsByTabId } - if (layout) { - const normalized = normalizeTerminalLayoutPtyOwnership(layout) - next[tabId] = normalized.snapshot - if (normalized.changed) { - ownershipTransfers = resolveTerminalLayoutPtyOwnershipTransfers( - layout, - normalized.snapshot - ) + if (!layout) { + if (!(tabId in s.terminalLayoutsByTabId)) { + return s } - } else { + const next = { ...s.terminalLayoutsByTabId } delete next[tabId] + return { terminalLayoutsByTabId: next } + } + const normalized = normalizeTerminalLayoutPtyOwnership(layout) + // Resolved before the bailout: normalization can transfer pane ownership even when the stored snapshot is untouched. + if (normalized.changed) { + ownershipTransfers = resolveTerminalLayoutPtyOwnershipTransfers(layout, normalized.snapshot) + } + // Why: pane-title churn re-persists structurally identical snapshots; bailing keeps every pane selector asleep. + const existing = s.terminalLayoutsByTabId[tabId] + if (existing && terminalLayoutEqual(existing, normalized.snapshot)) { + return s + } + return { + terminalLayoutsByTabId: { ...s.terminalLayoutsByTabId, [tabId]: normalized.snapshot } } - return { terminalLayoutsByTabId: next } }) transferNormalizedTerminalLayoutPtyOwnership(get(), tabId, ownershipTransfers) }, diff --git a/tests/e2e/paired-remote-pane-layout-retry.spec.ts b/tests/e2e/paired-remote-pane-layout-retry.spec.ts new file mode 100644 index 00000000000..653cde88add --- /dev/null +++ b/tests/e2e/paired-remote-pane-layout-retry.spec.ts @@ -0,0 +1,210 @@ +import type { Page } from '@stablyai/playwright-test' +import type { RuntimeMobileSessionTabsResult } from '../../src/shared/runtime-types' +import { toWebTerminalSurfaceTabId } from '../../src/shared/terminal-surface-id' +import type { TerminalLayoutSnapshot } from '../../src/shared/types' +import { + launchHeadlessPairedRuntimeHost, + type HeadlessPairedRuntimeHost +} from './helpers/headless-paired-runtime-host' +import { expect, test } from './helpers/orca-app' +import { + launchPairedElectronClient, + type PairedElectronClient +} from './helpers/paired-electron-client' + +async function callEnvironment( + page: Page, + environmentId: string, + method: string, + params: unknown +): Promise { + return page.evaluate( + async ({ environmentId, method, params }) => { + const response = await window.api.runtimeEnvironments.call({ + selector: environmentId, + method, + params + }) + if (!response.ok) { + throw new Error(`${response.error.code}: ${response.error.message}`) + } + return response.result + }, + { environmentId, method, params } + ) as Promise +} + +async function openClientTab(page: Page, worktreeId: string, tabId: string): Promise { + await expect + .poll( + () => + page.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (tab) => tab.id === tabId + ), + { tabId, worktreeId } + ), + { timeout: 60_000, message: `paired client never mirrored host tab ${tabId}` } + ) + .toBe(true) + await page.evaluate( + ({ tabId, worktreeId }) => { + const state = window.__store?.getState() + state?.setActiveView('terminal') + state?.setActiveWorktree(worktreeId) + state?.setActiveTab(tabId) + state?.setActiveTabType('terminal') + }, + { tabId, worktreeId } + ) + await expect + .poll(() => page.evaluate((id) => window.__paneManagers?.has(id) ?? false, tabId), { + timeout: 60_000, + message: `paired client pane for ${tabId} did not mount` + }) + .toBe(true) +} + +async function readHostLayout( + host: HeadlessPairedRuntimeHost, + worktreeId: string, + hostTabId: string +): Promise { + const snapshot = ( + await host.client.call('session.tabs.list', { + worktree: `id:${worktreeId}` + }) + ).result + return ( + snapshot.tabs.find((tab) => tab.type === 'terminal' && tab.parentTabId === hostTabId) + ?.parentLayout ?? null + ) +} + +async function readClientLayout(page: Page, tabId: string): Promise { + return page.evaluate((id) => window.__store?.getState().terminalLayoutsByTabId[id] ?? null, tabId) +} + +async function setPaneTitle(page: Page, title: string): Promise { + const isMac = await page.evaluate(() => navigator.userAgent.includes('Mac')) + await page + .locator('.xterm:visible') + .first() + .click({ + button: isMac ? 'left' : 'right', + position: { x: 40, y: 40 }, + modifiers: isMac ? ['Control'] : [] + }) + await page.getByText('Set Title…', { exact: true }).click() + const titleInput = page.getByRole('textbox', { name: 'Pane title' }) + await expect(titleInput).toBeVisible() + await titleInput.fill(title) + await titleInput.press('Enter') + await expect(titleInput).toHaveCount(0) + await expect(page.getByRole('button', { name: `Edit pane title: ${title}` })).toBeVisible() +} + +test('retries an identical remote pane layout after reconnect', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(240_000) + const title = `Reconnect retry ${Date.now()}` + const host = await launchHeadlessPairedRuntimeHost() + let client: PairedElectronClient | null = null + let observer: PairedElectronClient | null = null + let terminal: string | null = null + + try { + await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) + client = await launchPairedElectronClient(host.offer, testInfo, 'Pane layout retry client') + await expect + .poll( + () => + client?.page.evaluate(() => window.__store?.getState().allWorktrees().length ?? 0) ?? 0, + { timeout: 60_000, message: 'paired client never saw a host worktree' } + ) + .toBeGreaterThan(0) + const worktreeId = await client.page.evaluate( + () => window.__store?.getState().allWorktrees()[0]?.id ?? null + ) + if (!worktreeId) { + throw new Error('Paired client did not receive the host worktree') + } + + const created = await callEnvironment<{ + tab: { parentTabId: string; terminal: string | null } + }>(client.page, client.environmentId, 'session.tabs.createTerminal', { + worktree: `id:${worktreeId}`, + activate: false, + select: false, + navigation: 'caller' + }) + terminal = created.tab.terminal + if (!terminal) { + throw new Error('Host terminal was not created') + } + const hostTabId = created.tab.parentTabId + const webTabId = toWebTerminalSurfaceTabId(hostTabId) + await openClientTab(client.page, worktreeId, webTabId) + + const terminalRoot = client.page.locator(`[data-terminal-tab-id="${webTabId}"]`).first() + await terminalRoot.evaluate((element) => { + element.setAttribute('data-layout-retry-owner', 'original') + }) + await client.page.evaluate(async (selector) => { + await window.api.runtimeEnvironments.disconnect({ selector }) + }, client.environmentId) + + const failedPush = client.page.waitForEvent('console', { + predicate: (message) => + message.type() === 'warning' && + message.text().includes('[web-runtime-session] failed to update pane layout:'), + timeout: 30_000 + }) + await setPaneTitle(client.page, title) + await failedPush + const failedLayout = await readClientLayout(client.page, webTabId) + expect(Object.values(failedLayout?.titlesByLeafId ?? {})).toContain(title) + expect( + Object.values((await readHostLayout(host, worktreeId, hostTabId))?.titlesByLeafId ?? {}) + ).not.toContain(title) + + await expect + .poll( + () => + client.page.evaluate(async (selector) => { + const response = await window.api.runtimeEnvironments.connect({ selector }) + return response.ok + }, client.environmentId), + { timeout: 60_000, message: 'paired client never reconnected to the host runtime' } + ) + .toBe(true) + await expect(terminalRoot).toHaveAttribute('data-layout-retry-owner', 'original') + + await setPaneTitle(client.page, title) + expect(await readClientLayout(client.page, webTabId)).toEqual(failedLayout) + await expect + .poll( + async () => + Object.values( + (await readHostLayout(host, worktreeId, hostTabId))?.titlesByLeafId ?? {} + ).includes(title), + { timeout: 30_000, message: 'headless host never persisted the retried pane layout' } + ) + .toBe(true) + + observer = await launchPairedElectronClient(host.offer, testInfo, 'Pane layout retry observer') + await openClientTab(observer.page, worktreeId, webTabId) + await expect( + observer.page.getByRole('button', { name: `Edit pane title: ${title}` }) + ).toBeVisible({ timeout: 30_000 }) + } finally { + await observer?.dispose() + if (terminal) { + await host.client.call('terminal.closeTab', { terminal }).catch(() => undefined) + } + await client?.dispose() + await host.dispose() + } +})