From 4b8c59c4002b5682bb3eab5d23486bae1a74e254 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:41:44 -0700 Subject: [PATCH] fix(terminal): a split right after a paired-server tab opens starts on the server (#33078) * fix(terminal): a split of a paired-server pane still attaching starts on the server A pane of a server tab whose own attach is in flight has no transport id, so its split fell back to a local split. Only the server starts terminals in its tabs, so the new pane stayed blank. Split from the terminal the server already published for the pane, or wait for it. * fix(terminal): tell the user when a held split never starts, and keep focus where they went A split held for the server's source terminal now gives up when the pane, its tab or the pairing goes, toasts when the server never publishes it, and lands without focus if the user moved on. * fix(terminal): a held split runs only on the server the gesture targeted Check the workspace still belongs to that server before reading the source binding, and accept only a binding that names it, so a hydration swapping both cannot redirect the split. * refactor(terminal): route an unattached server pane's split straight to the server Drop the held split. The split uses the terminal the server published for the source pane, and when there is none yet it says so at once instead of queueing a gesture that needs its own fences. * fix(terminal): a server-tab pane never splits locally Refuse every split the server cannot take for a pane of its tab, and say whether the host is disconnected or the source terminal has not started yet. --- ...inal-pane-split-with-inherited-cwd.test.ts | 83 +++++++++++++++++-- .../terminal-pane-split-with-inherited-cwd.ts | 23 +++-- src/renderer/src/i18n/locales/en.json | 1 + src/renderer/src/i18n/locales/es.json | 1 + src/renderer/src/i18n/locales/fr.json | 1 + src/renderer/src/i18n/locales/ja.json | 1 + src/renderer/src/i18n/locales/ko.json | 1 + src/renderer/src/i18n/locales/pt.json | 1 + src/renderer/src/i18n/locales/zh.json | 1 + .../web-runtime-split-source-pty.test.ts | 61 ++++++++++++++ .../runtime/web-runtime-split-source-pty.ts | 56 +++++++++++++ ...ed-seeded-terminal-immediate-split.spec.ts | 67 +++++++++++++++ 12 files changed, 282 insertions(+), 15 deletions(-) create mode 100644 src/renderer/src/runtime/web-runtime-split-source-pty.test.ts create mode 100644 src/renderer/src/runtime/web-runtime-split-source-pty.ts create mode 100644 tests/e2e/paired-seeded-terminal-immediate-split.spec.ts diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts index 40b549126fd..39b300de69d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts @@ -6,13 +6,24 @@ import { splitTerminalPaneWithInheritedCwd } from './terminal-pane-split-with-in import { createDeferred } from './pty-connection-test-async' import type { splitPaneInMain, splitServerPane } from './terminal-pane-split-in-main' -const mocks = vi.hoisted(() => ({ - layoutOwnerIsMain: false, - recordCreatedTerminalPaneSplit: vi.fn(), - resolveSplitCwd: vi.fn(), - splitPaneInMain: vi.fn(), - splitServerPane: vi.fn(), - splitWebRuntimeTerminal: vi.fn() +const mocks = vi.hoisted(() => { + return { + layoutOwnerIsMain: false, + recordCreatedTerminalPaneSplit: vi.fn(), + resolveSplitCwd: vi.fn(), + splitPaneInMain: vi.fn(), + splitServerPane: vi.fn(), + splitWebRuntimeTerminal: vi.fn(), + readHostPublishedSplitSourcePtyId: vi.fn<() => string | null>(), + isHostTabPane: vi.fn<() => boolean>(), + notifyHostTabSplitRefused: vi.fn() + } +}) + +vi.mock('@/runtime/web-runtime-split-source-pty', () => ({ + readHostPublishedSplitSourcePtyId: mocks.readHostPublishedSplitSourcePtyId, + isHostTabPane: mocks.isHostTabPane, + notifyHostTabSplitRefused: mocks.notifyHostTabSplitRefused })) vi.mock('../../../../shared/layout-owner-switch', () => ({ @@ -67,6 +78,64 @@ describe('splitTerminalPaneWithInheritedCwd', () => { mocks.resolveSplitCwd.mockReset() mocks.splitWebRuntimeTerminal.mockReset() mocks.splitWebRuntimeTerminal.mockReturnValue(false) + mocks.readHostPublishedSplitSourcePtyId.mockReset().mockReturnValue(null) + mocks.isHostTabPane.mockReset().mockReturnValue(false) + mocks.notifyHostTabSplitRefused.mockReset() + }) + + describe('a pane of a paired server tab whose own attach has not landed', () => { + const HOST_TAB = 'web-terminal-host-tab-1' + const HOST_PTY = 'remote:env-1@@terminal-1' + const SOURCE = { worktreeId: 'worktree-1', tabId: HOST_TAB, leafId: 'leaf-1' } + + function splitHostTabPane(splitPane: ReturnType, serverTakesSplit = true): void { + mocks.isHostTabPane.mockReturnValue(true) + mocks.splitWebRuntimeTerminal.mockImplementation( + (ptyId: string | null) => serverTakesSplit && Boolean(ptyId) + ) + splitTerminalPaneWithInheritedCwd({ + worktreeId: 'worktree-1', + tabId: HOST_TAB, + manager: makeManager(splitPane), + // No transport yet: the pane's attach has not landed. + paneTransports: new Map(), + paneCwdMap: new Map(), + fallbackCwd: '/fallback', + pane: LAYOUT_OWNER_SOURCE_PANE, + direction: 'vertical', + source: 'keyboard' + }) + } + + it('splits on the server from the terminal the server published for it', () => { + mocks.readHostPublishedSplitSourcePtyId.mockReturnValue(HOST_PTY) + const splitPane = vi.fn() + + splitHostTabPane(splitPane) + + expect(mocks.splitWebRuntimeTerminal).toHaveBeenCalledWith( + HOST_PTY, + 'vertical', + 'keyboard', + SOURCE + ) + expect(splitPane).not.toHaveBeenCalled() + }) + + // A pane split here has no server terminal, and nothing would ever start one. + it.each([ + ['no terminal yet', null], + // e.g. the session to that server is not active. + ['a terminal the server cannot take a split for', HOST_PTY] + ])('refuses rather than splitting here with %s', (_case, published) => { + mocks.readHostPublishedSplitSourcePtyId.mockReturnValue(published) + const splitPane = vi.fn(() => ({ id: 2 })) + + splitHostTabPane(splitPane, false) + + expect(splitPane).not.toHaveBeenCalled() + expect(mocks.notifyHostTabSplitRefused).toHaveBeenCalledOnce() + }) }) it.each(['keyboard', 'context_menu'] as const)( diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts index 381465436d8..d3d78fad9a7 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts @@ -1,6 +1,11 @@ import type { TerminalPaneSplitSource } from '../../../../shared/feature-education-telemetry' import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager' import { splitWebRuntimeTerminal } from '@/runtime/web-runtime-terminal-actions' +import { + isHostTabPane, + notifyHostTabSplitRefused, + readHostPublishedSplitSourcePtyId +} from '@/runtime/web-runtime-split-source-pty' import type { PtyTransport } from './pty-transport-types' import { resolveSplitCwd, type PaneCwdMap } from './resolve-split-cwd' import { recordCreatedTerminalPaneSplit } from './terminal-pane-split-completion' @@ -26,14 +31,12 @@ type SplitArgs = { } export function splitTerminalPaneWithInheritedCwd(args: SplitArgs): void { - const ptyId = args.paneTransports.get(args.pane.id)?.getPtyId() ?? null - if ( - splitWebRuntimeTerminal(ptyId, args.direction, args.source, { - worktreeId: args.worktreeId, - tabId: args.tabId, - leafId: args.pane.leafId - }) - ) { + const sourcePane = { worktreeId: args.worktreeId, tabId: args.tabId, leafId: args.pane.leafId } + // Why the host's binding: a pane whose attach is still in flight has no transport id yet. + const ptyId = + args.paneTransports.get(args.pane.id)?.getPtyId() ?? + readHostPublishedSplitSourcePtyId(sourcePane) + if (splitWebRuntimeTerminal(ptyId, args.direction, args.source, sourcePane)) { return } // Switch point: main writes the split and starts its terminal; main's frame draws the pane. A @@ -54,6 +57,10 @@ export function splitTerminalPaneWithInheritedCwd(args: SplitArgs): void { return } } + if (isHostTabPane(sourcePane)) { + notifyHostTabSplitRefused(sourcePane) + return + } splitLocally(args, () => splitCwd(args, ptyId)) } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a79e1bef9e3..66fc5676179 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -18792,6 +18792,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "The workspace is not connected to a remote Orca host.", + "splitSourceUnpublished": "The pane didn’t split: its terminal hasn’t started on the host yet.", "terminalCreateUnconfirmed": "Orca couldn’t confirm that the new terminal opened on the host." }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 3afd4c23074..055f5bb9ec0 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -18511,6 +18511,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "El espacio de trabajo no está conectado a un host Orca remoto.", + "splitSourceUnpublished": "El panel no se dividió: su terminal aún no se ha iniciado en el host.", "terminalCreateUnconfirmed": "Orca no pudo confirmar que la nueva terminal se abrió en el host." }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 9c78c89d314..ded65607d61 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -18651,6 +18651,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "L'espace de travail n'est pas connecté à un hôte Orca distant.", + "splitSourceUnpublished": "Le volet n'a pas été divisé : son terminal n'a pas encore démarré sur l'hôte.", "terminalCreateUnconfirmed": "Orca n'a pas pu confirmer que le nouveau terminal s'est ouvert sur l'hôte." }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 4f35ee4a440..eecdc39af6d 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -18510,6 +18510,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "このワークスペースは、リモートの Orca ホストに接続されていません。", + "splitSourceUnpublished": "ペインを分割できませんでした: ホスト上でターミナルがまだ起動していません。", "terminalCreateUnconfirmed": "Orca は、ホスト上で新規ターミナルが開いたことを確認できませんでした。" }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 33b04edc432..dc332e230d8 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -18510,6 +18510,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "워크스페이스가 원격 Orca 호스트에 연결되어 있지 않습니다.", + "splitSourceUnpublished": "창을 분할하지 못했습니다: 호스트에서 터미널이 아직 시작되지 않았습니다.", "terminalCreateUnconfirmed": "Orca가 호스트에서 새 터미널이 열렸는지 확인할 수 없습니다." }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/pt.json b/src/renderer/src/i18n/locales/pt.json index 2a20accbb9c..2f316a217f7 100644 --- a/src/renderer/src/i18n/locales/pt.json +++ b/src/renderer/src/i18n/locales/pt.json @@ -1038,6 +1038,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "O espaço de trabalho não está conectado a um host Orca remoto.", + "splitSourceUnpublished": "O painel não foi dividido: o terminal ainda não foi iniciado no host.", "terminalCreateUnconfirmed": "O Orca não conseguiu confirmar que o novo terminal foi aberto no host." }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 3af40d5080e..60f4a3b54d1 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -18510,6 +18510,7 @@ }, "webRuntimeSession": { "remoteHostDisconnected": "工作区未连接到远程 Orca 主机。", + "splitSourceUnpublished": "窗格未拆分：其终端尚未在主机上启动。", "terminalCreateUnconfirmed": "Orca 无法确认新终端是否在主机上打开。" }, "gitlabJobTraceClient": { diff --git a/src/renderer/src/runtime/web-runtime-split-source-pty.test.ts b/src/renderer/src/runtime/web-runtime-split-source-pty.test.ts new file mode 100644 index 00000000000..b8d15eb1612 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-split-source-pty.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + notifyHostTabSplitRefused, + readHostPublishedSplitSourcePtyId +} from './web-runtime-split-source-pty' + +const TAB = 'web-terminal-host-tab-1' +const SOURCE = { worktreeId: 'worktree-1', tabId: TAB, leafId: 'leaf-1' } + +const mocks = vi.hoisted(() => { + const owner: { environmentId: string | null; transport: string } = { + environmentId: 'env-1', + transport: 'ready' + } + return { owner, toastError: vi.fn() } +}) + +vi.mock('../store', () => ({ + useAppStore: { + getState: () => ({ + terminalLayoutsByTabId: { + 'web-terminal-host-tab-1': { ptyIdsByLeafId: { 'leaf-1': 'remote:env-1@@terminal-1' } } + }, + runtimeStatusByEnvironmentId: new Map([ + ['env-1', { snapshot: { transport: mocks.owner.transport } }] + ]) + }) + } +})) +vi.mock('../lib/resolve-owner', () => ({ + runtimeEnvironmentIdUnlessTwin: () => mocks.owner.environmentId +})) +vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) + +describe('host-tab split source', () => { + beforeEach(() => { + mocks.owner.environmentId = 'env-1' + mocks.owner.transport = 'ready' + mocks.toastError.mockReset() + }) + + it('reads the terminal the workspace’s server published for the pane', () => { + expect(readHostPublishedSplitSourcePtyId(SOURCE)).toBe('remote:env-1@@terminal-1') + }) + + // A split sent from it would run on a server the workspace no longer belongs to. + it('ignores a binding naming another server', () => { + mocks.owner.environmentId = 'env-2' + expect(readHostPublishedSplitSourcePtyId(SOURCE)).toBeNull() + }) + + it.each([ + ['still starting', 'ready', 'its terminal hasn’t started'], + ['disconnected', 'disconnected', 'not connected to a remote Orca host'] + ])('tells the user why the split was refused while the server is %s', (_c, transport, text) => { + mocks.owner.transport = transport + notifyHostTabSplitRefused(SOURCE) + expect(mocks.toastError).toHaveBeenCalledWith(expect.stringContaining(text), expect.anything()) + }) +}) diff --git a/src/renderer/src/runtime/web-runtime-split-source-pty.ts b/src/renderer/src/runtime/web-runtime-split-source-pty.ts new file mode 100644 index 00000000000..afcc4a70a3d --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-split-source-pty.ts @@ -0,0 +1,56 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' +import { isWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' +import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id' +import { runtimeEnvironmentIdUnlessTwin } from '../lib/resolve-owner' +import { useAppStore } from '../store' +import type { WebRuntimeSplitSource } from './web-runtime-split-focus' + +/** + * The terminal the workspace's server published for a pane of one of its tabs, before this pane's + * own attach lands. A binding naming any other server is not this pane's terminal. + */ +export function readHostPublishedSplitSourcePtyId(source: WebRuntimeSplitSource): string | null { + if (!isWebTerminalSurfaceTabId(source.tabId)) { + return null + } + const state = useAppStore.getState() + const ptyId = state.terminalLayoutsByTabId[source.tabId]?.ptyIdsByLeafId?.[source.leafId] + const environmentId = runtimeEnvironmentIdUnlessTwin(state, source.worktreeId) + return ptyId && environmentId && parseRemoteRuntimePtyId(ptyId)?.environmentId === environmentId + ? ptyId + : null +} + +/** A pane of a paired server's tab: only that server starts terminals in it. */ +export function isHostTabPane(source: WebRuntimeSplitSource): boolean { + return ( + isWebTerminalSurfaceTabId(source.tabId) && + runtimeEnvironmentIdUnlessTwin(useAppStore.getState(), source.worktreeId) !== null + ) +} + +/** + * Why a server-tab pane did not split. Only the server starts terminals in its tabs, so splitting + * here instead would leave a blank pane. + */ +export function notifyHostTabSplitRefused(source: WebRuntimeSplitSource): void { + const state = useAppStore.getState() + const environmentId = runtimeEnvironmentIdUnlessTwin(state, source.worktreeId) + const disconnected = + !environmentId || + state.runtimeStatusByEnvironmentId.get(environmentId)?.snapshot?.transport === 'disconnected' + toast.error( + disconnected + ? translate( + 'auto.runtime.webRuntimeSession.remoteHostDisconnected', + 'The workspace is not connected to a remote Orca host.' + ) + : translate( + 'auto.runtime.webRuntimeSession.splitSourceUnpublished', + 'The pane didn’t split: its terminal hasn’t started on the host yet.' + ), + // Why an id: repeated presses must not stack toasts. + { id: 'web-runtime-host-tab-split-refused' } + ) +} diff --git a/tests/e2e/paired-seeded-terminal-immediate-split.spec.ts b/tests/e2e/paired-seeded-terminal-immediate-split.spec.ts new file mode 100644 index 00000000000..7366192ac92 --- /dev/null +++ b/tests/e2e/paired-seeded-terminal-immediate-split.spec.ts @@ -0,0 +1,67 @@ +import type { RuntimeTerminalListResult } from '../../src/shared/runtime-types' +import { expect, test } from './helpers/orca-app' +import { launchHeadlessPairedRuntimeHost } from './helpers/headless-paired-runtime-host' +import { launchPairedElectronClient } from './helpers/paired-electron-client' +import { waitForBoundPanes } from './helpers/terminal-layout-journeys' +import { splitActiveTerminalPane } from './helpers/terminal' + +const PANE = '.pane[data-leaf-id]:visible' + +// A split issued while the desktop's seeded first tab is still settling must still give the new +// pane a live server terminal, not a blank pane. +test('a split right after the first terminal of an empty paired-server workspace binds both panes', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(180_000) + const host = await launchHeadlessPairedRuntimeHost() + try { + await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) + const desktop = await launchPairedElectronClient(host.offer, testInfo, 'seeded split') + try { + const page = desktop.page + let worktreeId = '' + await expect + .poll( + async () => + (worktreeId = await page.evaluate( + (repo) => + window.__store + ?.getState() + .allWorktrees() + .find((worktree) => worktree.path === repo)?.id ?? '', + testRepoPath + )), + { timeout: 60_000 } + ) + .not.toBe('') + await page.evaluate((id) => window.__store?.getState().setActiveWorktree(id), worktreeId) + // Split the moment the seeded pane mounts, before its terminal has attached. + await expect(page.locator(PANE)).toHaveCount(1, { timeout: 60_000 }) + await splitActiveTerminalPane(page, 'vertical') + await waitForBoundPanes(page, 2) + + const leafIds = await page + .locator(PANE) + .evaluateAll((panes) => panes.map((pane) => pane.getAttribute('data-leaf-id') ?? '')) + await expect + .poll( + async () => { + const { terminals } = ( + await host.client.call('terminal.list', { + worktree: `id:${worktreeId}` + }) + ).result + return leafIds.every((leafId) => + terminals.some((terminal) => terminal.leafId === leafId) + ) + }, + { timeout: 30_000, message: 'a split pane has no server terminal' } + ) + .toBe(true) + } finally { + await desktop.dispose() + } + } finally { + await host.dispose() + } +})