diff --git a/changelog.d/1094.md b/changelog.d/1094.md new file mode 100644 index 000000000..ad50d1e22 --- /dev/null +++ b/changelog.d/1094.md @@ -0,0 +1,3 @@ +### Added + +- **A workspace bootstrapped on a remote host now has a real, resizable split tree — parity with a local workspace, not a fixed mirror grid.** "New workspace on this host" (#1067) used to leave you with a grid sized purely from the pane count; "Split right"/"Split down" now let you place a new pane exactly where you want it, drag the divider to resize, and close any one pane without disturbing the rest (#1091). diff --git a/src/main/ipc/handlers/__tests__/remote.handler.test.ts b/src/main/ipc/handlers/__tests__/remote.handler.test.ts index 60d308de0..1e0779d99 100644 --- a/src/main/ipc/handlers/__tests__/remote.handler.test.ts +++ b/src/main/ipc/handlers/__tests__/remote.handler.test.ts @@ -110,6 +110,7 @@ function fakeClient(host: RemoteHost) { write: vi.fn(async () => undefined), listWorkspaces: vi.fn(async (): Promise => ({ workspaces: [] })), createWorkspace: vi.fn(async (): Promise<{ sessionId: string }> => ({ sessionId: 'web-1' })), + closeSession: vi.fn(async (): Promise => undefined), onMeta: vi.fn((cb: (e: RemoteMetaEvent) => void) => { metaCbs.push(cb); }), onResize: vi.fn((cb: (e: RemoteResizeEvent) => void) => { resizeCbs.push(cb); }), onData: vi.fn((cb: (e: RemoteDataEvent) => void) => { dataCbs.push(cb); }), @@ -127,6 +128,7 @@ function fakeClient(host: RemoteHost) { write: ReturnType; listWorkspaces: ReturnType; createWorkspace: ReturnType; + closeSession: ReturnType; emitMeta: (e: RemoteMetaEvent) => void; emitResize: (e: RemoteResizeEvent) => void; emitData: (e: RemoteDataEvent) => void; @@ -535,6 +537,87 @@ describe('remote.handler — workspaceCreate (#1001)', () => { }); }); +describe('remote.handler — workspacePaneAdd (#1091)', () => { + const host: RemoteHost = { id: 'h1', label: 'box', origin: 'https://box:9600', token: 't', addedAt: 0 }; + + it('grows an already-live workspace and returns the new sessionId — same call as workspaceCreate', async () => { + const store = fakeStore([host]); + const client = fakeClient(host); + client.createWorkspace.mockResolvedValueOnce({ sessionId: 'web-2' }); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never, clientFactory: () => client }); + + const res = await getHandler(IPC.REMOTE_WORKSPACE_PANE_ADD)({}, 'h1', 'ws-1') as { ok: true; sessionId: string }; + + expect(res).toEqual({ ok: true, sessionId: 'web-2' }); + expect(client.createWorkspace).toHaveBeenCalledWith('ws-1', undefined); + }); + + it('forwards cwd when given', async () => { + const store = fakeStore([host]); + const client = fakeClient(host); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never, clientFactory: () => client }); + + await getHandler(IPC.REMOTE_WORKSPACE_PANE_ADD)({}, 'h1', 'ws-1', '/repo'); + + expect(client.createWorkspace).toHaveBeenCalledWith('ws-1', '/repo'); + }); + + it('maps a client rejection to {ok:false} rather than throwing', async () => { + const store = fakeStore([host]); + const client = fakeClient(host); + client.createWorkspace.mockRejectedValueOnce(new Error('daemon busy')); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never, clientFactory: () => client }); + + const res = await getHandler(IPC.REMOTE_WORKSPACE_PANE_ADD)({}, 'h1', 'ws-1') as { ok: false; error: string }; + + expect(res).toEqual({ ok: false, error: 'daemon busy' }); + }); + + it('reports unknown host without touching a client', async () => { + const store = fakeStore([]); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never }); + + const res = await getHandler(IPC.REMOTE_WORKSPACE_PANE_ADD)({}, 'missing', 'ws-1') as { ok: boolean; error?: string }; + + expect(res).toEqual({ ok: false, error: 'unknown host' }); + }); +}); + +describe('remote.handler — sessionClose (#1091)', () => { + const host: RemoteHost = { id: 'h1', label: 'box', origin: 'https://box:9600', token: 't', addedAt: 0 }; + + it('closes a session on the given host', async () => { + const store = fakeStore([host]); + const client = fakeClient(host); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never, clientFactory: () => client }); + + const res = await getHandler(IPC.REMOTE_SESSION_CLOSE)({}, 'h1', 'sess-1') as { ok: true }; + + expect(res).toEqual({ ok: true }); + expect(client.closeSession).toHaveBeenCalledWith('sess-1'); + }); + + it('maps a client rejection to {ok:false} rather than throwing', async () => { + const store = fakeStore([host]); + const client = fakeClient(host); + client.closeSession.mockRejectedValueOnce(new Error('daemon busy')); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never, clientFactory: () => client }); + + const res = await getHandler(IPC.REMOTE_SESSION_CLOSE)({}, 'h1', 'sess-1') as { ok: false; error: string }; + + expect(res).toEqual({ ok: false, error: 'daemon busy' }); + }); + + it('reports unknown host without touching a client', async () => { + const store = fakeStore([]); + registerRemoteHandlers({ store: store as never, attachments: fakeAttachments() as never }); + + const res = await getHandler(IPC.REMOTE_SESSION_CLOSE)({}, 'missing', 'sess-1') as { ok: boolean; error?: string }; + + expect(res).toEqual({ ok: false, error: 'unknown host' }); + }); +}); + describe('remote.handler — pane attach/detach/write push routing', () => { const host: RemoteHost = { id: 'h1', label: 'box', origin: 'https://box:9600', token: 't', addedAt: 0 }; diff --git a/src/main/ipc/handlers/remote.handler.ts b/src/main/ipc/handlers/remote.handler.ts index 4e5e0dde9..78d96ec7c 100644 --- a/src/main/ipc/handlers/remote.handler.ts +++ b/src/main/ipc/handlers/remote.handler.ts @@ -483,6 +483,55 @@ export function registerRemoteHandlers(deps: RegisterRemoteHandlersDeps): () => } })); + // Add a pane to a workspace that ALREADY has a live pane on this host + // (#1091 — parity with a local workspace's "add pane"). Same server call + // as REMOTE_WORKSPACE_CREATE (rejectWorkspaceId accepts a known-live id + // just as readily as an operator-minted new one); kept as its own IPC name + // so the renderer's intent ("grow this workspace") reads distinctly from + // "bootstrap a brand-new one". + ipcMain.removeHandler(IPC.REMOTE_WORKSPACE_PANE_ADD); + ipcMain.handle(IPC.REMOTE_WORKSPACE_PANE_ADD, wrapHandler(IPC.REMOTE_WORKSPACE_PANE_ADD, + async ( + _e: IpcMainInvokeEvent, + hostId: unknown, + workspaceId: unknown, + cwd?: unknown, + ): Promise<{ ok: true; sessionId: string } | { ok: false; error: string }> => { + const id = assertString(hostId, 'hostId'); + const wsId = assertString(workspaceId, 'workspaceId'); + const safeCwd = cwd === undefined ? undefined : assertString(cwd, 'cwd'); + const client = getOrCreateClient(id); + if (!client) return { ok: false, error: 'unknown host' }; + try { + const { sessionId } = await client.createWorkspace(wsId, safeCwd); + return { ok: true, sessionId }; + } catch (err) { + return { ok: false, error: err instanceof Error ? err.message : String(err) }; + } + })); + + // Close a pane on a workspace this client bootstrapped (#1091). See + // RemoteHostClient.closeSession for the reasoning behind allowing this one + // destroy call. + ipcMain.removeHandler(IPC.REMOTE_SESSION_CLOSE); + ipcMain.handle(IPC.REMOTE_SESSION_CLOSE, wrapHandler(IPC.REMOTE_SESSION_CLOSE, + async ( + _e: IpcMainInvokeEvent, + hostId: unknown, + sessionId: unknown, + ): Promise<{ ok: true } | { ok: false; error: string }> => { + const id = assertString(hostId, 'hostId'); + const sid = assertString(sessionId, 'sessionId'); + const client = getOrCreateClient(id); + if (!client) return { ok: false, error: 'unknown host' }; + try { + await client.closeSession(sid); + return { ok: true }; + } catch (err) { + return { ok: false, error: err instanceof Error ? err.message : String(err) }; + } + })); + // Attach descriptors — the persistence half of "attachments survive a // reload". Deliberately independent of the SSE attach lifecycle below: the // reload teardown in installSenderCleanup still kills every live stream (a @@ -599,6 +648,8 @@ export function registerRemoteHandlers(deps: RegisterRemoteHandlersDeps): () => ipcMain.removeHandler(IPC.REMOTE_HOSTS_REMOVE); ipcMain.removeHandler(IPC.REMOTE_WORKSPACES_LIST); ipcMain.removeHandler(IPC.REMOTE_WORKSPACE_CREATE); + ipcMain.removeHandler(IPC.REMOTE_WORKSPACE_PANE_ADD); + ipcMain.removeHandler(IPC.REMOTE_SESSION_CLOSE); ipcMain.removeHandler(IPC.REMOTE_ATTACHMENTS_LIST); ipcMain.removeHandler(IPC.REMOTE_ATTACHMENTS_ADD); ipcMain.removeHandler(IPC.REMOTE_ATTACHMENTS_REMOVE); diff --git a/src/main/remote/RemoteHostClient.ts b/src/main/remote/RemoteHostClient.ts index f152f0405..e8df78f86 100644 --- a/src/main/remote/RemoteHostClient.ts +++ b/src/main/remote/RemoteHostClient.ts @@ -6,8 +6,12 @@ // relying on EventSource's query-string token, and it drives the pane state // for Task 5's IPC handler instead of a DOM terminal. // -// This class is observer + input only: it never calls a destroy/delete -// endpoint on the remote host. +// Mostly observer + input: it does not touch anyone else's existing panes. +// The one exception is `closeSession` (#1091) — closing a pane belonging to +// a workspace THIS client's operator credential bootstrapped in the first +// place (#1067), which is a local-workspace-close-button equivalent, not a +// mirror doing something to someone else's session. See that method's own +// doc comment for the full reasoning. import * as crypto from 'crypto'; import type { @@ -241,6 +245,44 @@ export class RemoteHostClient implements RemotePaneEvents { return { sessionId }; } + /** + * Close a pane this client (or the desktop app on our behalf) created on + * this host (#1091). This is the one destroy/delete call this class makes + * — a deliberate, narrow exception to the class-level "observer + input + * only" rule above, which was written for the older attach-to-an- + * EXISTING-workspace-to-watch-it use case, where destroying someone else's + * pane from a mirror would be a real surprise. #1067 added a second use + * case — bootstrapping a brand-new workspace on a paired host to actually + * work in it — and a workspace you just created has no "someone else" to + * surprise: closing one of its own panes is exactly what a local + * workspace's close button does, so a remote one needs the same capability + * to reach parity. `DELETE /api/sessions/:id` (`WebTerminalServer. + * handleSessionDelete`) already gates this on `mayInput`, same as create, + * so no server change was needed. + */ + async closeSession(sessionId: string): Promise { + const res = await this.fetchImpl(`${this.host.origin}/api/sessions/${encodeURIComponent(sessionId)}`, { + method: 'DELETE', + headers: this.authHeaders(), + redirect: 'error', + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + // 404 is treated as success: the pane is gone either way, and a caller + // racing a poll-driven pane-list refresh against its own close click + // must not surface an error for a session that already disappeared. + if (!res.ok && res.status !== 404) { + let message = `closeSession failed: HTTP ${res.status}`; + try { + const parsed = (await res.json()) as { error?: string; detail?: string }; + if (parsed?.detail) message = parsed.detail; + else if (parsed?.error) message = parsed.error; + } catch { + /* body wasn't JSON — fall back to the generic message */ + } + throw new Error(message); + } + } + async listWorkspaces(): Promise { const res = await this.fetchImpl(`${this.host.origin}/api/workspaces`, { headers: this.authHeaders(), diff --git a/src/main/remote/__tests__/RemoteHostClient.test.ts b/src/main/remote/__tests__/RemoteHostClient.test.ts index df3427494..2cbccb44b 100644 --- a/src/main/remote/__tests__/RemoteHostClient.test.ts +++ b/src/main/remote/__tests__/RemoteHostClient.test.ts @@ -128,6 +128,65 @@ describe('RemoteHostClient', () => { }); }); + describe('closeSession (#1091)', () => { + it('DELETEs /api/sessions/:id with the operator Bearer token', async () => { + const fetchImpl = vi.fn(async (_url: string, _init?: RequestInit) => ({ + ok: true, + status: 204, + }) as unknown as Response); + const client = new RemoteHostClient(host, fetchImpl as unknown as typeof fetch); + + await client.closeSession('sess-1'); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + const [url, init] = fetchImpl.mock.calls[0]; + expect(url).toBe(`${host.origin}/api/sessions/sess-1`); + expect(init?.method).toBe('DELETE'); + expect((init?.headers as Record)?.Authorization).toBe(`Bearer ${host.token}`); + expect(init?.redirect).toBe('error'); + expect(init?.signal).toBeInstanceOf(AbortSignal); + }); + + it('URL-encodes the session id', async () => { + const fetchImpl = vi.fn(async (_url: string, _init?: RequestInit) => ({ ok: true, status: 204 }) as unknown as Response); + const client = new RemoteHostClient(host, fetchImpl as unknown as typeof fetch); + + await client.closeSession('sess/weird id'); + + const [url] = fetchImpl.mock.calls[0]; + expect(url).toBe(`${host.origin}/api/sessions/${encodeURIComponent('sess/weird id')}`); + }); + + it('treats a 404 as success — the pane is gone either way', async () => { + const fetchImpl = vi.fn(async () => ({ ok: false, status: 404 }) as unknown as Response); + const client = new RemoteHostClient(host, fetchImpl as unknown as typeof fetch); + + await expect(client.closeSession('sess-1')).resolves.toBeUndefined(); + }); + + it('rejects with the daemon-supplied detail on a non-OK, non-404 response', async () => { + const fetchImpl = vi.fn(async () => ({ + ok: false, + status: 500, + json: async () => ({ error: 'destroy-failed', detail: 'daemon busy' }), + }) as unknown as Response); + const client = new RemoteHostClient(host, fetchImpl as unknown as typeof fetch); + + await expect(client.closeSession('sess-1')).rejects.toThrow('daemon busy'); + }); + + it('rejects with a generic message when the error body is not JSON', async () => { + const fetchImpl = vi.fn(async () => ({ + ok: false, + status: 500, + json: async () => { throw new Error('not json'); }, + }) as unknown as Response); + const client = new RemoteHostClient(host, fetchImpl as unknown as typeof fetch); + + await expect(client.closeSession('sess-1')).rejects.toThrow('closeSession failed: HTTP 500'); + }); + }); + describe('listWorkspaces', () => { it('sends Authorization: Bearer and parses the body', async () => { const fetchImpl = vi.fn(async (_url: string, _init?: RequestInit) => { diff --git a/src/preload/preload.ts b/src/preload/preload.ts index 4a8bbd1d4..f719a89eb 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -1389,6 +1389,14 @@ document.addEventListener('DOMContentLoaded', () => { ipcRenderer.invoke(IPC.REMOTE_WORKSPACE_CREATE, hostId, workspaceId, cwd) as Promise< { ok: true; sessionId: string } | { ok: false; error: string } >, + workspacePaneAdd: (hostId: string, workspaceId: string, cwd?: string) => + ipcRenderer.invoke(IPC.REMOTE_WORKSPACE_PANE_ADD, hostId, workspaceId, cwd) as Promise< + { ok: true; sessionId: string } | { ok: false; error: string } + >, + sessionClose: (hostId: string, sessionId: string) => + ipcRenderer.invoke(IPC.REMOTE_SESSION_CLOSE, hostId, sessionId) as Promise< + { ok: true } | { ok: false; error: string } + >, attachmentsList: () => ipcRenderer.invoke(IPC.REMOTE_ATTACHMENTS_LIST) as Promise, attachmentsAdd: (descriptor: RemoteAttachmentDescriptor) => diff --git a/src/renderer/components/Remote/RemotePaneContainer.tsx b/src/renderer/components/Remote/RemotePaneContainer.tsx new file mode 100644 index 000000000..165bb8b3d --- /dev/null +++ b/src/renderer/components/Remote/RemotePaneContainer.tsx @@ -0,0 +1,61 @@ +import { Fragment, useCallback, useRef, type ReactNode } from 'react'; +import { Panel, Group, Separator, useGroupRef } from 'react-resizable-panels'; +import type { Layout } from 'react-resizable-panels'; +import type { RemotePaneLeaf, RemotePaneNode } from './remotePaneTree'; + +interface RemotePaneContainerProps { + node: RemotePaneNode; + renderLeaf: (leaf: RemotePaneLeaf) => ReactNode; + onResize: (branchId: string, sizes: number[]) => void; +} + +/** + * Recursive resizable-split renderer for a remote workspace's pane tree + * (#1091) — the same `react-resizable-panels` primitives (`Group`/`Panel`/ + * `Separator`) the local `PaneContainer` uses, so drag-to-resize behaves + * identically. Deliberately simpler than `PaneContainer`: this tree only + * changes from THIS component's own resize/split/close actions (never from + * an external write racing a live drag), so there is no need for + * `PaneContainer`'s programmatic-vs-user layout reconciliation — a + * structural change (split/close) always mounts a fresh branch id, which + * naturally gets a fresh `Group` with the right `defaultSize`s. + */ +export default function RemotePaneContainer({ node, renderLeaf, onResize }: RemotePaneContainerProps) { + const groupRef = useGroupRef(); + const debounceRef = useRef | undefined>(undefined); + + const handleLayoutChanged = useCallback((layout: Layout) => { + if (node.type !== 'branch') return; + const branch = node; + const sizes = branch.children.map((c) => layout[c.id] ?? 100 / branch.children.length); + if (debounceRef.current) clearTimeout(debounceRef.current); + debounceRef.current = setTimeout(() => onResize(branch.id, sizes), 200); + }, [node, onResize]); + + if (node.type === 'leaf') { + return <>{renderLeaf(node)}; + } + + return ( + + {node.children.map((child, i) => ( + + {i > 0 && ( + + )} + + + + + ))} + + ); +} diff --git a/src/renderer/components/Remote/RemoteWorkspaceView.tsx b/src/renderer/components/Remote/RemoteWorkspaceView.tsx index 758ae8634..768aeb468 100644 --- a/src/renderer/components/Remote/RemoteWorkspaceView.tsx +++ b/src/renderer/components/Remote/RemoteWorkspaceView.tsx @@ -1,21 +1,42 @@ -import { useEffect, useRef, useState, type CSSProperties } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useT } from '../../hooks/useT'; +import { useStore } from '../../stores'; import type { AttachedRemoteWorkspace } from '../../stores/slices/remoteWorkspacesSlice'; import type { RemotePaneSummary } from '../../../shared/remoteHosts'; import RemoteMirrorTerminal from './RemoteMirrorTerminal'; +import RemotePaneContainer from './RemotePaneContainer'; +import { + type RemotePaneNode, + applySizes, + leafIds, + reconcile, + removeLeaf, + splitLeaf, +} from './remotePaneTree'; + +/** + * A bare leaf render (no Group/Panel) at the SAME position a branch would + * render (Group -> Panel -> leaf) is a different React element TYPE — React + * unmounts and remounts the whole subtree on that transition, tearing down + * and re-attaching a survivor's live SSE mirror for no reason (a live + * regression: collapsing a 2-pane split down to 1 pane by closing the other + * one detached AND re-attached the survivor). Wrapping every render in a + * trivial single-child "branch" keeps the outer element type constant across + * the 1-pane <-> N-pane transition, at the render boundary only — the STORED + * tree still uses a bare leaf for 1 pane, which keeps splitLeaf/removeLeaf's + * own invariants (and their tests) simple. + */ +function wrapForRender(node: RemotePaneNode): RemotePaneNode { + if (node.type === 'branch') return node; + return { id: `${node.id}-solo`, type: 'branch', direction: 'horizontal', children: [node] }; +} /** Bounded reads rule: never open more than this many concurrent SSE mirrors - * for one workspace — a "+N more panes" note covers the rest. */ + * for one workspace — a "+N more panes" note covers the rest. Applied by + * capping which session ids ever enter the split tree (`reconcile` below), + * not by capping the tree's own rendering. */ const MAX_MIRRORS = 6; -/** 1 → full, 2 → columns, 3-4 → 2×2, 5-6 → 3×2 (brief-specified layout). */ -function gridStyle(count: number): CSSProperties { - if (count <= 1) return { gridTemplateColumns: '1fr', gridTemplateRows: '1fr' }; - if (count === 2) return { gridTemplateColumns: '1fr 1fr', gridTemplateRows: '1fr' }; - if (count <= 4) return { gridTemplateColumns: '1fr 1fr', gridTemplateRows: '1fr 1fr' }; - return { gridTemplateColumns: '1fr 1fr 1fr', gridTemplateRows: '1fr 1fr' }; -} - /** One pane cell: owns the paneAttach call (and its resulting attachId), the * shell/cwd caption, and the mirror terminal itself. Attach is idempotent in * main, so a StrictMode double-effect here is harmless. @@ -25,11 +46,16 @@ function gridStyle(count: number): CSSProperties { * the SAME remote sessionIds, so nothing in the pane list changes and this * effect would never fire again — the mirror would sit blank forever with no * visible error. The epoch is bumped by the store whenever `stale` clears. */ -function PaneCell({ hostId, pane, readOnly, attachEpoch }: { +function PaneCell({ hostId, pane, readOnly, attachEpoch, onClose, closeLabel }: { hostId: string; pane: RemotePaneSummary; readOnly: boolean; attachEpoch: number | undefined; + /** Undefined when this pane can't be closed from here — read-only hosts + * (#1091, #1067's parity ask): closing panes is spawn's mirror image, so + * it goes through `mayInput` too, same as add. */ + onClose?: (sessionId: string) => void; + closeLabel: string; }) { const [attachId, setAttachId] = useState(null); const [error, setError] = useState(undefined); @@ -79,11 +105,25 @@ function PaneCell({ hostId, pane, readOnly, attachEpoch }: { return (
- {pane.shell ?? pane.sessionId.slice(0, 8)} - {pane.cwd ? ` — ${pane.cwd}` : ''} + + {pane.shell ?? pane.sessionId.slice(0, 8)} + {pane.cwd ? ` — ${pane.cwd}` : ''} + + {onClose && ( + + )}
@@ -93,7 +133,9 @@ function PaneCell({ hostId, pane, readOnly, attachEpoch }: { } /** - * Grid of one attached remote workspace's panes. Mount lifecycle is + * Resizable split view of one attached remote workspace's panes (#1091 — + * replaced the earlier fixed mirror grid with a real, user-controlled split + * tree, parity with a local workspace). Mount lifecycle is * hidden-but-alive — WorkspaceCenter renders one of these per attached * remote workspace and toggles display:none by activeRemoteKey, the same * technique WorkspaceViewport uses for local workspaces. Unmounting on every @@ -114,6 +156,35 @@ export default function RemoteWorkspaceView({ workspace }: { workspace: Attached // at mount — the view stays mounted for the app session per the // hidden-but-alive rule above) is that probe. const [allowInput, setAllowInput] = useState(undefined); + const [pending, setPending] = useState(false); + const [actionError, setActionError] = useState(undefined); + // The split tree (#1091). Local-only, rebuilt from `workspace.panes` by the + // reconcile effect below — see remotePaneTree.ts's header comment for why + // this is a separate structure from both the server pane list and the + // local-workspace pane tree. + const [layout, setLayout] = useState(null); + // Which leaf a split targets. Falls back to the tree's first leaf when + // nothing has been clicked yet (e.g. right after the very first pane + // attaches) so "Split" always has somewhere to act on. + const [activeLeafId, setActiveLeafId] = useState(null); + + const visibleSessionIds = useMemo( + () => workspace.panes.slice(0, MAX_MIRRORS).map((p) => p.sessionId), + [workspace.panes], + ); + const visibleSessionIdsKey = visibleSessionIds.join(','); + + // Keyed on visibleSessionIdsKey, not visibleSessionIds: the array is + // recreated every render, the key is stable unless the actual ids change — + // same discipline PaneContainer's childIdKey uses. + useEffect(() => { + setLayout((prev) => reconcile(prev, visibleSessionIds)); + }, [visibleSessionIdsKey]); + + useEffect(() => { + if (activeLeafId && layout && leafIds(layout).includes(activeLeafId)) return; + setActiveLeafId(layout ? leafIds(layout)[0] ?? null : null); + }, [layout, activeLeafId]); useEffect(() => { let cancelled = false; @@ -127,12 +198,72 @@ export default function RemoteWorkspaceView({ workspace }: { workspace: Attached return () => { cancelled = true; }; }, [workspace.hostId]); - const visiblePanes = workspace.panes.slice(0, MAX_MIRRORS); + // Grow/shrink the SAME workspace on the remote host (#1091 — parity with a + // local workspace's add/close pane). Both apply the result straight to + // `remoteWorkspaces` via `setRemoteWorkspacePanes`'s merge — an add appends + // the new sessionId (mergePaneSets keeps anything present in the next list), + // a close is a filter of the current list — rather than waiting up to + // POLL_INTERVAL_MS for `useRemoteAttachmentsLifecycle`'s own refetch to + // notice, so the workspace visibly grows/shrinks the moment the request + // that did it succeeds. + const handleAddPane = useCallback(async (direction: 'horizontal' | 'vertical') => { + const remote = window.electronAPI?.remote; + if (!remote || pending) return; + setPending(true); + setActionError(undefined); + try { + const res = await remote.workspacePaneAdd(workspace.hostId, workspace.workspaceId); + if (!res.ok) { + setActionError(t('remote.addPaneFailed')); + return; + } + const nextPanes: RemotePaneSummary[] = [...workspace.panes, { sessionId: res.sessionId }]; + useStore.getState().setRemoteWorkspacePanes(workspace.key, nextPanes); + setLayout((prev) => { + if (!prev) return { id: res.sessionId, type: 'leaf' }; + const target = activeLeafId && leafIds(prev).includes(activeLeafId) ? activeLeafId : leafIds(prev)[0]; + return target ? splitLeaf(prev, target, res.sessionId, direction) : prev; + }); + setActiveLeafId(res.sessionId); + } catch { + setActionError(t('remote.addPaneFailed')); + } finally { + setPending(false); + } + }, [workspace.hostId, workspace.workspaceId, workspace.key, workspace.panes, pending, activeLeafId, t]); + + const handleClosePane = useCallback(async (sessionId: string) => { + const remote = window.electronAPI?.remote; + if (!remote) return; + setActionError(undefined); + try { + const res = await remote.sessionClose(workspace.hostId, sessionId); + if (!res.ok) { + setActionError(t('remote.closePaneFailed')); + return; + } + const nextPanes = workspace.panes.filter((p) => p.sessionId !== sessionId); + useStore.getState().setRemoteWorkspacePanes(workspace.key, nextPanes); + setLayout((prev) => (prev ? removeLeaf(prev, sessionId) : prev)); + } catch { + setActionError(t('remote.closePaneFailed')); + } + }, [workspace.hostId, workspace.key, workspace.panes, t]); + + const handleResize = useCallback((branchId: string, sizes: number[]) => { + setLayout((prev) => (prev ? applySizes(prev, branchId, sizes) : prev)); + }, []); + + const readOnly = allowInput === false; const hiddenCount = Math.max(0, workspace.panes.length - MAX_MIRRORS); + // A read-only host has no mayInput grant server-side either — add/close + // would just 403, so don't offer them (same gate handleSessionCreate and + // handleSessionDelete apply on WebTerminalServer). + const canManagePanes = !readOnly; return (
- {allowInput === false && ( + {readOnly && (
)} + {actionError && ( +
setActionError(undefined)} + > + {actionError} +
+ )}
- {visiblePanes.map((pane) => ( - { + const pane = workspace.panes.find((p) => p.sessionId === leaf.id); + if (!pane) return null; + return ( +
setActiveLeafId(leaf.id)} + style={leaf.id === activeLeafId ? { outline: '1px solid var(--accent-blue)', outlineOffset: '-1px' } : undefined} + > + +
+ ); + }} /> - ))} + )}
{hiddenCount > 0 && (
{t('remote.morePanes', { count: hiddenCount })}
)} + {canManagePanes && ( +
+ + +
+ )}
); } diff --git a/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.attach.test.tsx b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.attach.test.tsx index 0a8b539d7..03107efd3 100644 --- a/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.attach.test.tsx +++ b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.attach.test.tsx @@ -19,6 +19,16 @@ vi.mock('../../../hooks/useT', () => ({ useT: () => (k: string) => k })); (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +// #1091 follow-up: RemoteWorkspaceView now renders panes through +// RemotePaneContainer's react-resizable-panels Group, which probes +// ResizeObserver on mount — same stub PaneContainer.moveSizes.test.tsx uses. +class ResizeObserverStub { + observe(): void { /* layout reflow is irrelevant under jsdom */ } + unobserve(): void { /* no-op */ } + disconnect(): void { /* no-op */ } +} +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver ??= ResizeObserverStub; + let container: HTMLDivElement; let root: Root; let paneAttach: ReturnType; diff --git a/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.paneManagement.test.tsx b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.paneManagement.test.tsx new file mode 100644 index 000000000..7d3d6e51a --- /dev/null +++ b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.paneManagement.test.tsx @@ -0,0 +1,167 @@ +// @vitest-environment jsdom +// +// #1091 — add/close a pane on a remote workspace, the parity ask from #1067: +// a workspace bootstrapped on a remote host should grow/shrink like a local +// one, not sit as a fixed mirror grid. Both actions go through the store's +// real setRemoteWorkspacePanes (mergePaneSets), asserted via useStore.getState() +// rather than a re-render of this isolated component — WorkspaceCenter is the +// one that re-renders it with a fresh `workspace` prop in the real app. +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import RemoteWorkspaceView from '../RemoteWorkspaceView'; +import { useStore } from '../../../stores'; +import type { AttachedRemoteWorkspace } from '../../../stores/slices/remoteWorkspacesSlice'; + +vi.mock('../RemoteMirrorTerminal', () => ({ + default: ({ attachId }: { attachId: string | null }) => + React.createElement('div', { 'data-attach-id': attachId ?? '' }), +})); + +vi.mock('../../../hooks/useT', () => ({ useT: () => (k: string) => k })); + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +// #1091 follow-up: RemoteWorkspaceView now renders panes through +// RemotePaneContainer's react-resizable-panels Group, which probes +// ResizeObserver on mount — same stub PaneContainer.moveSizes.test.tsx uses. +class ResizeObserverStub { + observe(): void { /* layout reflow is irrelevant under jsdom */ } + unobserve(): void { /* no-op */ } + disconnect(): void { /* no-op */ } +} +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver ??= ResizeObserverStub; + +let container: HTMLDivElement; +let root: Root; +let workspacePaneAdd: ReturnType; +let sessionClose: ReturnType; + +function installElectronApi(opts: { + addImpl?: () => Promise; + closeImpl?: () => Promise; + allowInput?: boolean; +} = {}): void { + workspacePaneAdd = vi.fn(opts.addImpl ?? (async () => ({ ok: true as const, sessionId: 's-new' }))); + sessionClose = vi.fn(opts.closeImpl ?? (async () => ({ ok: true as const }))); + (window as unknown as { electronAPI: unknown }).electronAPI = { + remote: { + paneAttach: vi.fn(async () => ({ ok: true as const, attachId: 'att-1' })), + paneDetach: vi.fn(async () => undefined), + hostsList: vi.fn(async () => [ + { id: 'h1', label: 'office-mac', origin: 'https://x', addedAt: 0, allowInput: opts.allowInput ?? true }, + ]), + workspacePaneAdd, + sessionClose, + }, + }; +} + +function workspace(overrides: Partial = {}): AttachedRemoteWorkspace { + return { + key: 'h1:ws-1', + hostId: 'h1', + hostLabel: 'office-mac', + workspaceId: 'ws-1', + name: 'Remote WS', + panes: [{ sessionId: 's1' }], + ...overrides, + }; +} + +function render(w: AttachedRemoteWorkspace): void { + act(() => { root.render(React.createElement(RemoteWorkspaceView, { workspace: w })); }); +} + +async function settle(): Promise { + await act(async () => { + for (let i = 0; i < 6; i += 1) await Promise.resolve(); + }); +} + +beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + // A clean slate per test — this suite asserts against the real store. + useStore.setState({ remoteWorkspaces: [workspace()] } as never); +}); + +afterEach(() => { + act(() => { root.unmount(); }); + container.remove(); + vi.restoreAllMocks(); +}); + +describe('RemoteWorkspaceView — add/close a pane (#1091)', () => { + it('add pane calls workspacePaneAdd with (hostId, workspaceId) and appends the new session to the store', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + const addButton = container.querySelector('button:not([title])') as HTMLButtonElement; + expect(addButton).not.toBeNull(); + act(() => { addButton.click(); }); + await settle(); + + expect(workspacePaneAdd).toHaveBeenCalledTimes(1); + expect(workspacePaneAdd).toHaveBeenCalledWith('h1', 'ws-1'); + + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === 'h1:ws-1'); + expect(entry?.panes.map((p) => p.sessionId)).toEqual(['s1', 's-new']); + }); + + it('close pane calls sessionClose with (hostId, sessionId) and removes it from the store', async () => { + installElectronApi(); + render(workspace({ panes: [{ sessionId: 's1' }, { sessionId: 's2' }] })); + await settle(); + + const closeButtons = container.querySelectorAll('button[title="remote.closePane"]'); + expect(closeButtons.length).toBe(2); + act(() => { (closeButtons[0] as HTMLButtonElement).click(); }); + await settle(); + + expect(sessionClose).toHaveBeenCalledTimes(1); + expect(sessionClose).toHaveBeenCalledWith('h1', 's1'); + + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === 'h1:ws-1'); + expect(entry?.panes.map((p) => p.sessionId)).toEqual(['s2']); + }); + + it('a failed add surfaces an error and does not touch the store', async () => { + installElectronApi({ addImpl: async () => ({ ok: false as const, error: 'boom' }) }); + render(workspace()); + await settle(); + + const addButton = container.querySelector('button:not([title])') as HTMLButtonElement; + act(() => { addButton.click(); }); + await settle(); + + expect(container.textContent).toContain('remote.addPaneFailed'); + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === 'h1:ws-1'); + expect(entry?.panes.map((p) => p.sessionId)).toEqual(['s1']); + }); + + it('a failed close surfaces an error and does not touch the store', async () => { + installElectronApi({ closeImpl: async () => ({ ok: false as const, error: 'boom' }) }); + render(workspace()); + await settle(); + + const closeButton = container.querySelector('button[title="remote.closePane"]') as HTMLButtonElement; + act(() => { closeButton.click(); }); + await settle(); + + expect(container.textContent).toContain('remote.closePaneFailed'); + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === 'h1:ws-1'); + expect(entry?.panes.map((p) => p.sessionId)).toEqual(['s1']); + }); + + it('read-only host (mayInput false) offers neither add nor close', async () => { + installElectronApi({ allowInput: false }); + render(workspace()); + await settle(); + + expect(container.querySelector('button[title="remote.closePane"]')).toBeNull(); + expect(container.textContent).not.toContain('remote.addPane'); + }); +}); diff --git a/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.splitTree.test.tsx b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.splitTree.test.tsx new file mode 100644 index 000000000..1c68600ff --- /dev/null +++ b/src/renderer/components/Remote/__tests__/RemoteWorkspaceView.splitTree.test.tsx @@ -0,0 +1,202 @@ +// @vitest-environment jsdom +// +// #1091 follow-up — the flat mirror grid from #1094 is replaced by a real, +// user-controlled split tree (RemotePaneContainer, backed by the same +// react-resizable-panels primitives the local workspace uses). This suite +// proves the RENDER TREE follows split/close actions; see +// PaneContainer.moveSizes.test.tsx's own header comment for what jsdom can +// and cannot prove about the panels library (no real drag-resize here, +// by the same construction). +// +// `RemoteWorkspaceView` takes `workspace` as a plain prop — in the real app, +// `WorkspaceCenter` re-renders it with a fresh object on every store change +// (it subscribes to `remoteWorkspaces` itself). This harness has no +// WorkspaceCenter, so `rerenderFromStore` plays that part explicitly after +// each action, the same way a real store-driven re-render would. +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import RemoteWorkspaceView from '../RemoteWorkspaceView'; +import { useStore } from '../../../stores'; +import type { AttachedRemoteWorkspace } from '../../../stores/slices/remoteWorkspacesSlice'; + +vi.mock('../RemoteMirrorTerminal', () => ({ + default: ({ attachId }: { attachId: string | null }) => + React.createElement('div', { 'data-attach-id': attachId ?? '' }), +})); + +vi.mock('../../../hooks/useT', () => ({ useT: () => (k: string) => k })); + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +class ResizeObserverStub { + observe(): void { /* layout reflow is irrelevant under jsdom */ } + unobserve(): void { /* no-op */ } + disconnect(): void { /* no-op */ } +} +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver ??= ResizeObserverStub; + +const KEY = 'h1:ws-1'; + +let container: HTMLDivElement; +let root: Root; +let seq = 0; + +function installElectronApi(): void { + (window as unknown as { electronAPI: unknown }).electronAPI = { + remote: { + paneAttach: vi.fn(async () => ({ ok: true as const, attachId: 'att-1' })), + paneDetach: vi.fn(async () => undefined), + hostsList: vi.fn(async () => [ + { id: 'h1', label: 'office-mac', origin: 'https://x', addedAt: 0, allowInput: true }, + ]), + workspacePaneAdd: vi.fn(async () => { + seq += 1; + return { ok: true as const, sessionId: `s-new-${seq}` }; + }), + sessionClose: vi.fn(async () => ({ ok: true as const })), + }, + }; +} + +function workspace(overrides: Partial = {}): AttachedRemoteWorkspace { + return { + key: KEY, + hostId: 'h1', + hostLabel: 'office-mac', + workspaceId: 'ws-1', + name: 'Remote WS', + panes: [{ sessionId: 's1' }], + ...overrides, + }; +} + +function render(w: AttachedRemoteWorkspace): void { + act(() => { root.render(React.createElement(RemoteWorkspaceView, { workspace: w })); }); +} + +/** Re-renders with the CURRENT store snapshot for this key — what + * WorkspaceCenter does on every store change in the real app. */ +function rerenderFromStore(): void { + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === KEY); + if (!entry) throw new Error('workspace missing from store'); + render(entry); +} + +async function settle(): Promise { + await act(async () => { + for (let i = 0; i < 6; i += 1) await Promise.resolve(); + }); +} + +function mirrorCellCount(): number { + return container.querySelectorAll('[data-attach-id]').length; +} + +beforeEach(() => { + seq = 0; + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + useStore.setState({ remoteWorkspaces: [workspace()] } as never); +}); + +afterEach(() => { + act(() => { root.unmount(); }); + container.remove(); + vi.restoreAllMocks(); +}); + +describe('RemoteWorkspaceView — split tree (#1091)', () => { + it('renders a single mirror cell for a one-pane workspace, no split yet', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + expect(mirrorCellCount()).toBe(1); + }); + + it('"Split right" grows the workspace and renders a second mirror cell', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + const splitRight = container.querySelector('button:not([title])') as HTMLButtonElement; + act(() => { splitRight.click(); }); + await settle(); + rerenderFromStore(); + await settle(); + + expect(mirrorCellCount()).toBe(2); + const entry = useStore.getState().remoteWorkspaces.find((w) => w.key === KEY); + expect(entry?.panes.map((p) => p.sessionId)).toEqual(['s1', 's-new-1']); + }); + + it('"Split down" also grows to two cells, via the titled button', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + const splitDown = container.querySelector('button[title="remote.splitDown"]') as HTMLButtonElement; + act(() => { splitDown.click(); }); + await settle(); + rerenderFromStore(); + await settle(); + + expect(mirrorCellCount()).toBe(2); + }); + + it('closing a pane after a split leaves exactly one mirror cell', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + const splitRight = container.querySelector('button:not([title])') as HTMLButtonElement; + act(() => { splitRight.click(); }); + await settle(); + rerenderFromStore(); + await settle(); + expect(mirrorCellCount()).toBe(2); + + const closeButtons = container.querySelectorAll('button[title="remote.closePane"]'); + expect(closeButtons.length).toBe(2); + act(() => { (closeButtons[0] as HTMLButtonElement).click(); }); + await settle(); + rerenderFromStore(); + await settle(); + + expect(mirrorCellCount()).toBe(1); + }); + + it('two splits in a row produce three mirror cells', async () => { + installElectronApi(); + render(workspace()); + await settle(); + + const splitRight = () => container.querySelector('button:not([title])') as HTMLButtonElement; + act(() => { splitRight().click(); }); + await settle(); + rerenderFromStore(); + await settle(); + act(() => { splitRight().click(); }); + await settle(); + rerenderFromStore(); + await settle(); + + expect(mirrorCellCount()).toBe(3); + }); + + it('reconciles a pane closed elsewhere (poll refresh) out of the tree', async () => { + installElectronApi(); + render(workspace({ panes: [{ sessionId: 's1' }, { sessionId: 's2' }] })); + await settle(); + expect(mirrorCellCount()).toBe(2); + + // Simulate the poll dropping s2 without going through this view's own + // close action — e.g. it was closed from a different client. + render(workspace({ panes: [{ sessionId: 's1' }] })); + await settle(); + + expect(mirrorCellCount()).toBe(1); + }); +}); diff --git a/src/renderer/components/Remote/__tests__/remotePaneTree.test.ts b/src/renderer/components/Remote/__tests__/remotePaneTree.test.ts new file mode 100644 index 000000000..890b4f255 --- /dev/null +++ b/src/renderer/components/Remote/__tests__/remotePaneTree.test.ts @@ -0,0 +1,171 @@ +// #1091 — pure split-tree logic for remote workspaces. Deliberately its own +// tree, independent of the local pane tree (shared/types.ts Pane) — these +// tests exist so that independence stays true without ever touching a local +// workspace fixture. +import { describe, it, expect } from 'vitest'; +import { + applySizes, + findLeaf, + leafIds, + reconcile, + removeLeaf, + splitLeaf, + type RemotePaneNode, +} from '../remotePaneTree'; + +const leaf = (id: string): RemotePaneNode => ({ id, type: 'leaf' }); + +describe('leafIds / findLeaf', () => { + it('returns a single id for a bare leaf', () => { + expect(leafIds(leaf('a'))).toEqual(['a']); + }); + + it('collects every leaf id under nested branches', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), { id: 'b2', type: 'branch', direction: 'vertical', children: [leaf('b'), leaf('c')] }], + }; + expect(leafIds(tree)).toEqual(['a', 'b', 'c']); + expect(findLeaf(tree, 'c')?.id).toBe('c'); + expect(findLeaf(tree, 'zzz')).toBeNull(); + }); +}); + +describe('splitLeaf', () => { + it('turns a bare leaf into a two-child branch with even sizes', () => { + const result = splitLeaf(leaf('a'), 'a', 'b', 'horizontal'); + expect(result.type).toBe('branch'); + if (result.type !== 'branch') throw new Error('unreachable'); + expect(result.direction).toBe('horizontal'); + expect(result.children.map((c) => c.id)).toEqual(['a', 'b']); + expect(result.sizes).toEqual([50, 50]); + }); + + it('splits the correct leaf deep in a tree, leaving siblings untouched', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b')], + }; + const result = splitLeaf(tree, 'b', 'c', 'vertical'); + if (result.type !== 'branch') throw new Error('unreachable'); + expect(leafIds(result.children[0])).toEqual(['a']); + const second = result.children[1]; + if (second.type !== 'branch') throw new Error('expected b to have split'); + expect(second.direction).toBe('vertical'); + expect(leafIds(second)).toEqual(['b', 'c']); + }); + + it('is a no-op when the target id is not in the tree', () => { + const tree = leaf('a'); + expect(splitLeaf(tree, 'missing', 'new', 'horizontal')).toEqual(tree); + }); +}); + +describe('removeLeaf', () => { + it('returns null when removing the only leaf', () => { + expect(removeLeaf(leaf('a'), 'a')).toBeNull(); + }); + + it('collapses a two-child branch into its surviving sibling', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b')], + }; + expect(removeLeaf(tree, 'a')).toEqual(leaf('b')); + }); + + it('collapses a three-way branch to a two-child one without collapsing further', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b'), leaf('c')], + }; + const result = removeLeaf(tree, 'b'); + if (result === null || result.type !== 'branch') throw new Error('expected a surviving branch'); + expect(leafIds(result)).toEqual(['a', 'c']); + }); + + it('is a no-op when the target id is not in the tree', () => { + const tree = leaf('a'); + expect(removeLeaf(tree, 'missing')).toEqual(tree); + }); +}); + +describe('applySizes', () => { + it('writes sizes onto the branch with the matching id, leaves others alone', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b')], + sizes: [50, 50], + }; + const result = applySizes(tree, 'b1', [30, 70]); + if (result.type !== 'branch') throw new Error('unreachable'); + expect(result.sizes).toEqual([30, 70]); + }); + + it('is a no-op for a bare leaf', () => { + expect(applySizes(leaf('a'), 'b1', [1, 2])).toEqual(leaf('a')); + }); +}); + +describe('reconcile', () => { + it('builds a bare leaf from a null tree with one pane id', () => { + expect(reconcile(null, ['a'])).toEqual(leaf('a')); + }); + + it('stays null when there are no pane ids', () => { + expect(reconcile(null, [])).toBeNull(); + }); + + it('appends a new pane as a sibling branch when one already exists', () => { + const result = reconcile(leaf('a'), ['a', 'b']); + if (result === null || result.type !== 'branch') throw new Error('expected a new branch'); + expect(leafIds(result)).toEqual(['a', 'b']); + }); + + it('preserves an existing user-built split shape when its ids are still all wanted', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'vertical', + children: [leaf('a'), leaf('b')], + sizes: [30, 70], + }; + expect(reconcile(tree, ['a', 'b'])).toEqual(tree); + }); + + it('drops a leaf whose session closed elsewhere, collapsing the branch', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b')], + }; + expect(reconcile(tree, ['b'])).toEqual(leaf('b')); + }); + + it('drops every leaf and returns null when the pane list goes empty', () => { + expect(reconcile(leaf('a'), [])).toBeNull(); + }); + + it('adds and drops in the same pass', () => { + const tree: RemotePaneNode = { + id: 'b1', + type: 'branch', + direction: 'horizontal', + children: [leaf('a'), leaf('b')], + }; + const result = reconcile(tree, ['a', 'c']); + expect(result).not.toBeNull(); + expect(new Set(leafIds(result as RemotePaneNode))).toEqual(new Set(['a', 'c'])); + }); +}); diff --git a/src/renderer/components/Remote/remotePaneTree.ts b/src/renderer/components/Remote/remotePaneTree.ts new file mode 100644 index 000000000..4e30b0c1c --- /dev/null +++ b/src/renderer/components/Remote/remotePaneTree.ts @@ -0,0 +1,125 @@ +/** + * Split-tree layout for one remote workspace's panes (#1091 follow-up to + * #1094's flat mirror grid — the fixed 6-cell grid never let a user choose + * how panes are arranged or resize them, unlike a local workspace's own + * split/resize). + * + * Deliberately independent of both: + * - `AttachedRemoteWorkspace.panes` (server truth: which sessions exist — + * `reconcile` below is the only bridge from that list into this tree) + * - the LOCAL pane tree (`shared/types.ts` `Pane`, `paneSlice.ts`) — a leaf + * here holds a remote sessionId, not a local PTY. Zero shared code, zero + * risk to local workspaces if this file has a bug. + * + * This tree is intentionally NOT persisted: it lives in `RemoteWorkspaceView` + * component state only (rebuilt via `reconcile` from `workspace.panes` on + * every change), matching the existing "panes are always re-fetched, never + * restored from disk" rule for remote workspaces. + */ + +export interface RemotePaneLeaf { + id: string; // sessionId + type: 'leaf'; +} + +export interface RemotePaneBranch { + id: string; + type: 'branch'; + direction: 'horizontal' | 'vertical'; + children: RemotePaneNode[]; + sizes?: number[]; +} + +export type RemotePaneNode = RemotePaneLeaf | RemotePaneBranch; + +let branchCounter = 0; +function newBranchId(): string { + branchCounter += 1; + return `remote-branch-${branchCounter}`; +} + +export function leafIds(node: RemotePaneNode): string[] { + if (node.type === 'leaf') return [node.id]; + return node.children.flatMap(leafIds); +} + +export function findLeaf(node: RemotePaneNode, id: string): RemotePaneLeaf | null { + if (node.type === 'leaf') return node.id === id ? node : null; + for (const child of node.children) { + const found = findLeaf(child, id); + if (found) return found; + } + return null; +} + +/** Split `targetId`'s leaf into a branch holding the old leaf plus a new one + * for `newId`, in the given direction. A no-op if `targetId` isn't in the + * tree (the pane it pointed at may have just closed under the user). */ +export function splitLeaf( + root: RemotePaneNode, + targetId: string, + newId: string, + direction: 'horizontal' | 'vertical', +): RemotePaneNode { + if (root.type === 'leaf') { + if (root.id !== targetId) return root; + return { + id: newBranchId(), + type: 'branch', + direction, + children: [{ id: targetId, type: 'leaf' }, { id: newId, type: 'leaf' }], + sizes: [50, 50], + }; + } + return { ...root, children: root.children.map((c) => splitLeaf(c, targetId, newId, direction)) }; +} + +/** Remove one leaf. A branch left with a single child collapses into it — + * same discipline a local workspace's pane-close uses. Returns null when + * the whole tree was that one leaf. */ +export function removeLeaf(root: RemotePaneNode, targetId: string): RemotePaneNode | null { + if (root.type === 'leaf') return root.id === targetId ? null : root; + const nextChildren = root.children + .map((c) => removeLeaf(c, targetId)) + .filter((c): c is RemotePaneNode => c !== null); + if (nextChildren.length === 0) return null; + if (nextChildren.length === 1) return nextChildren[0]; + if (nextChildren.length === root.children.length) return root; // nothing removed + return { ...root, children: nextChildren, sizes: undefined }; +} + +/** Write fresh drag-resize sizes onto the branch with this id. A no-op if the + * branch already restructured out from under an in-flight resize. */ +export function applySizes(root: RemotePaneNode, branchId: string, sizes: number[]): RemotePaneNode { + if (root.type === 'leaf') return root; + if (root.id === branchId) return { ...root, sizes }; + return { ...root, children: root.children.map((c) => applySizes(c, branchId, sizes)) }; +} + +/** Bring the tree in line with the server's current pane id list: drop + * leaves for sessions that closed elsewhere, append a leaf for any session + * the tree doesn't have yet (as a new root-level sibling — the user can + * split it into place). `paneIds` order matters only for first construction. */ +export function reconcile(root: RemotePaneNode | null, paneIds: readonly string[]): RemotePaneNode | null { + let next = root; + const wanted = new Set(paneIds); + + if (next) { + for (const id of leafIds(next)) { + if (!wanted.has(id)) next = next ? removeLeaf(next, id) : null; + } + } + + const known = next ? new Set(leafIds(next)) : new Set(); + for (const id of paneIds) { + if (known.has(id)) continue; + if (!next) { + next = { id, type: 'leaf' }; + } else if (next.type === 'leaf') { + next = { id: newBranchId(), type: 'branch', direction: 'horizontal', children: [next, { id, type: 'leaf' }] }; + } else { + next = { ...next, children: [...next.children, { id, type: 'leaf' }], sizes: undefined }; + } + } + return next; +} diff --git a/src/renderer/i18n/locales/en.ts b/src/renderer/i18n/locales/en.ts index 5c255272e..455349ec6 100644 --- a/src/renderer/i18n/locales/en.ts +++ b/src/renderer/i18n/locales/en.ts @@ -1765,6 +1765,12 @@ export const en = { 'remote.addFailed': 'Could not add host — check the app logs', 'remote.workspacesFailed': 'Could not load workspaces — check the app logs', 'remote.morePanes': '+{count} more panes', + 'remote.addPane': 'Add pane', + 'remote.splitRight': 'Split right', + 'remote.splitDown': 'Split down', + 'remote.closePane': 'Close pane', + 'remote.addPaneFailed': 'Could not add a pane — check the app logs', + 'remote.closePaneFailed': 'Could not close the pane — check the app logs', 'remote.mirrorDescription': 'Mirror a workspace from another wmux', 'remote.labelOptional': 'Label (optional)', 'remote.paneCount': '{count} panes', diff --git a/src/renderer/i18n/locales/pl.ts b/src/renderer/i18n/locales/pl.ts index 4df595fda..526134adb 100644 --- a/src/renderer/i18n/locales/pl.ts +++ b/src/renderer/i18n/locales/pl.ts @@ -1773,6 +1773,12 @@ export const pl = { 'remote.addFailed': 'Nie udało się dodać hosta — sprawdź logi aplikacji', 'remote.workspacesFailed': 'Nie udało się wczytać obszarów roboczych — sprawdź logi aplikacji', 'remote.morePanes': '+{count} więcej paneli', + 'remote.addPane': 'Dodaj panel', + 'remote.splitRight': 'Podziel w prawo', + 'remote.splitDown': 'Podziel w dół', + 'remote.closePane': 'Zamknij panel', + 'remote.addPaneFailed': 'Nie udało się dodać panelu — sprawdź logi aplikacji', + 'remote.closePaneFailed': 'Nie udało się zamknąć panelu — sprawdź logi aplikacji', 'remote.mirrorDescription': 'Lustrzane odbicie obszaru roboczego z innego wmuxa', 'remote.labelOptional': 'Etykieta (opcjonalna)', 'remote.paneCount': '{count} paneli', diff --git a/src/renderer/i18n/locales/zh.ts b/src/renderer/i18n/locales/zh.ts index 4794b6aa2..127b04880 100644 --- a/src/renderer/i18n/locales/zh.ts +++ b/src/renderer/i18n/locales/zh.ts @@ -1564,6 +1564,12 @@ export const zh = { 'remote.addFailed': '无法添加主机 — 请检查应用日志', 'remote.workspacesFailed': '无法加载工作区 — 请检查应用日志', 'remote.morePanes': '+{count} 个更多面板', + 'remote.addPane': '添加面板', + 'remote.splitRight': '向右拆分', + 'remote.splitDown': '向下拆分', + 'remote.closePane': '关闭面板', + 'remote.addPaneFailed': '无法添加面板 — 请检查应用日志', + 'remote.closePaneFailed': '无法关闭面板 — 请检查应用日志', 'remote.mirrorDescription': '镜像另一个 wmux 中的工作区', 'remote.labelOptional': '标签（可选）', 'remote.paneCount': '{count} 个面板', diff --git a/src/shared/constants.ts b/src/shared/constants.ts index a68712f4c..cdc5f32e6 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -567,6 +567,16 @@ export const IPC = { // as an operator-authenticated caller (WebTerminalServer.rejectWorkspaceId's // one exception). See RemoteHostClient.createWorkspace. REMOTE_WORKSPACE_CREATE: 'remote:workspace:create', + // Add another pane to an ALREADY-live remote workspace (#1091) — the same + // POST /api/sessions call as REMOTE_WORKSPACE_CREATE, just with a + // workspaceId that already has a live pane under it (rejectWorkspaceId + // accepts either shape). Kept as its own channel so main-process test + // fixtures and renderer intent stay distinct from "bootstrap a new one". + REMOTE_WORKSPACE_PANE_ADD: 'remote:workspace:pane:add', + // Close a pane on a remote workspace THIS client bootstrapped (#1091). See + // RemoteHostClient.closeSession for why this is the one destroy call this + // bridge makes. + REMOTE_SESSION_CLOSE: 'remote:session:close', // Persisted attach descriptors (see RemoteAttachmentsStore). The renderer's // remote-workspace slice is memory-only, so these are what survive a reload // and an app restart; panes are never stored, only re-fetched. diff --git a/src/shared/electron.d.ts b/src/shared/electron.d.ts index bf7f5e922..29aef8bf0 100644 --- a/src/shared/electron.d.ts +++ b/src/shared/electron.d.ts @@ -223,6 +223,16 @@ declare global { workspaceCreate: (hostId: string, workspaceId: string, cwd?: string) => Promise< { ok: true; sessionId: string } | { ok: false; error: string } >; + /** Add another pane to a workspace that already has a live pane on + * this host (#1091) — parity with a local workspace's "add pane". */ + workspacePaneAdd: (hostId: string, workspaceId: string, cwd?: string) => Promise< + { ok: true; sessionId: string } | { ok: false; error: string } + >; + /** Close a pane on a workspace this client bootstrapped (#1091). See + * RemoteHostClient.closeSession for why this destroy call is safe. */ + sessionClose: (hostId: string, sessionId: string) => Promise< + { ok: true } | { ok: false; error: string } + >; /** Persisted attach descriptors — read on renderer boot to restore * the attachments a reload/restart wiped out of the memory-only * slice. Panes are never part of a descriptor: they are re-fetched