-
Notifications
You must be signed in to change notification settings - Fork 62
feat(remote): a resizable split tree for remote workspaces, with grow/shrink (#1091) #1094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a2fbe3b
4f2d7a5
76e9295
497b3a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<void> { | ||
| const res = await this.fetchImpl(`${this.host.origin}/api/sessions/${encodeURIComponent(sessionId)}`, { | ||
|
Comment on lines
+263
to
+264
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- applicable repository conventions ---'
for f in /tmp/coderabbit-repo-knowledge/openwong2kim-wmux-1cb661f7/*/*.md; do
case "$f" in
*remote*|*daemon*|*ipc*) printf '\n### %s\n' "$f"; head -120 "$f" ;;
esac
done
printf '%s\n' '--- daemon session-delete references ---'
rg -n -C 12 'handleSessionDelete|/api/sessions|mayInput|workspaceId|creator' src/daemon/web/WebTerminalServer.ts src/main/ipc/handlers/remote.handler.ts src/main/remote/RemoteHostClient.ts
printf '%s\n' '--- close-session IPC handler ---'
sed -n '500,545p' src/main/ipc/handlers/remote.handler.tsRepository: openwong2kim/wmux Length of output: 50373 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- exact IPC close handler ---'
sed -n '513,543p' src/main/ipc/handlers/remote.handler.ts
printf '%s\n' '--- exact daemon delete handler and authorization helper ---'
rg -n '^ (private )?(async )?handleSessionDelete|^ (private )?mayInput|handleSessionCreate|listSessions' src/daemon/web/WebTerminalServer.ts
sed -n '1770,1855p' src/daemon/web/WebTerminalServer.ts
printf '%s\n' '--- session identity and lifecycle contract definitions ---'
rg -n -C 8 'interface DaemonSession|type DaemonSession|workspaceId|creator|owner|destroy\\(' src/daemon src/shared -g '*.ts'Repository: openwong2kim/wmux Length of output: 6645 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- session creation and deletion implementation ---'
sed -n '2310,2475p' src/daemon/web/WebTerminalServer.ts
printf '%s\n' '--- mayInput implementation ---'
sed -n '3738,3760p' src/daemon/web/WebTerminalServer.ts
printf '%s\n' '--- lifecycle/session ownership fields ---'
rg -n -C 5 'interface DaemonSession|type DaemonSession|workspaceId|ENV_KEYS\\.WORKSPACE|destroy\\(id|destroy\\(session' src/daemon src/shared -g '*.ts' | head -240Repository: openwong2kim/wmux Length of output: 9668 Authorization Bypass (CWE-862): Missing Authorization Reachability: Internal · Exploitability: Moderate Enforce ownership before deleting a remote session.
🤖 Prompt for AI Agents |
||
| 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<RemoteWorkspacesResponse> { | ||
| const res = await this.fetchImpl(`${this.host.origin}/api/workspaces`, { | ||
| headers: this.authHeaders(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<ReturnType<typeof setTimeout> | 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 ( | ||
| <Group | ||
| groupRef={groupRef} | ||
| orientation={node.direction} | ||
| className="h-full w-full" | ||
| resizeTargetMinimumSize={{ coarse: 37, fine: 16 }} | ||
| onLayoutChanged={handleLayoutChanged} | ||
| > | ||
| {node.children.map((child, i) => ( | ||
| <Fragment key={child.id}> | ||
| {i > 0 && ( | ||
| <Separator | ||
| className={`${node.direction === 'horizontal' ? 'w-px' : 'h-px'} bg-[var(--border-soft)] hover:bg-[var(--accent-blue)] transition-colors`} | ||
| /> | ||
| )} | ||
| <Panel id={child.id} defaultSize={node.sizes?.[i] ?? 100 / node.children.length} minSize={10}> | ||
| <RemotePaneContainer node={child} renderLeaf={renderLeaf} onResize={onResize} /> | ||
| </Panel> | ||
| </Fragment> | ||
| ))} | ||
| </Group> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Moderate
Require HTTPS before sending the host token.
createWorkspacesendsauthHeaders()to${this.host.origin}/api/sessions. Because registered hosts may usehttp:, a network attacker can capture the Bearer token. Reject non-HTTPS origins before persistence and block authenticated requests for existing HTTP records.redirect: 'error'does not protect the initial HTTP request.🤖 Prompt for AI Agents