From a5c61b054642105463655a752c34230fb8c49978 Mon Sep 17 00:00:00 2001 From: SuperComboGamer Date: Sun, 14 Dec 2025 14:40:34 -0500 Subject: [PATCH] feat: improve terminal creation and resizing logic - Added a debouncing mechanism for terminal creation to prevent rapid requests. - Enhanced terminal resizing with rate limiting and suppression of output during resize to avoid duplicates. - Updated scrollback handling to clear pending output when establishing new WebSocket connections. - Improved stability of terminal fitting logic by ensuring dimensions are stable before fitting. --- .../src/components/views/terminal-view.tsx | 46 ++++++---- .../views/terminal-view/terminal-panel.tsx | 84 ++++++++++++------- apps/server/src/index.ts | 39 +++++++-- apps/server/src/services/terminal-service.ts | 66 ++++++++++++++- 4 files changed, 180 insertions(+), 55 deletions(-) diff --git a/apps/app/src/components/views/terminal-view.tsx b/apps/app/src/components/views/terminal-view.tsx index 25fab530..df52eefe 100644 --- a/apps/app/src/components/views/terminal-view.tsx +++ b/apps/app/src/components/views/terminal-view.tsx @@ -147,6 +147,18 @@ export function TerminalView() { const serverUrl = process.env.NEXT_PUBLIC_SERVER_URL || "http://localhost:3008"; const CREATE_COOLDOWN_MS = 500; // Prevent rapid terminal creation + // Helper to check if terminal creation should be debounced + const canCreateTerminal = (debounceMessage: string): boolean => { + const now = Date.now(); + if (now - lastCreateTimeRef.current < CREATE_COOLDOWN_MS || isCreatingRef.current) { + console.log(debounceMessage); + return false; + } + lastCreateTimeRef.current = now; + isCreatingRef.current = true; + return true; + }; + // Get active tab const activeTab = terminalState.tabs.find(t => t.id === terminalState.activeTabId); @@ -263,14 +275,9 @@ export function TerminalView() { // Create a new terminal session // targetSessionId: the terminal to split (if splitting an existing terminal) const createTerminal = async (direction?: "horizontal" | "vertical", targetSessionId?: string) => { - // Debounce: prevent rapid terminal creation - const now = Date.now(); - if (now - lastCreateTimeRef.current < CREATE_COOLDOWN_MS || isCreatingRef.current) { - console.log("[Terminal] Debounced terminal creation"); + if (!canCreateTerminal("[Terminal] Debounced terminal creation")) { return; } - lastCreateTimeRef.current = now; - isCreatingRef.current = true; try { const headers: Record = { @@ -305,14 +312,9 @@ export function TerminalView() { // Create terminal in new tab const createTerminalInNewTab = async () => { - // Debounce: prevent rapid terminal creation - const now = Date.now(); - if (now - lastCreateTimeRef.current < CREATE_COOLDOWN_MS || isCreatingRef.current) { - console.log("[Terminal] Debounced terminal tab creation"); + if (!canCreateTerminal("[Terminal] Debounced terminal tab creation")) { return; } - lastCreateTimeRef.current = now; - isCreatingRef.current = true; const tabId = addTerminalTab(); try { @@ -424,12 +426,22 @@ export function TerminalView() { return () => window.removeEventListener('keydown', handleKeyDown); }, [terminalState.isUnlocked, terminalState.activeSessionId, shortcuts]); - // Get a stable key for a panel + // Collect all terminal IDs from a panel tree in order + const getTerminalIds = (panel: TerminalPanelContent): string[] => { + if (panel.type === "terminal") { + return [panel.sessionId]; + } + return panel.panels.flatMap(getTerminalIds); + }; + + // Get a STABLE key for a panel - based only on terminal IDs, not tree structure + // This prevents unnecessary remounts when layout structure changes const getPanelKey = (panel: TerminalPanelContent): string => { if (panel.type === "terminal") { return panel.sessionId; } - return `split-${panel.direction}-${panel.panels.map(getPanelKey).join("-")}`; + // Use joined terminal IDs - stable regardless of nesting depth + return `group-${getTerminalIds(panel).join("-")}`; }; // Render panel content recursively @@ -465,10 +477,12 @@ export function TerminalView() { ? panel.size : defaultSizePerPanel; + const panelKey = getPanelKey(panel); return ( - + {index > 0 && ( )} - + {renderPanelContent(panel)} diff --git a/apps/app/src/components/views/terminal-view/terminal-panel.tsx b/apps/app/src/components/views/terminal-view/terminal-panel.tsx index 6fe487b6..3cfd967b 100644 --- a/apps/app/src/components/views/terminal-view/terminal-panel.tsx +++ b/apps/app/src/components/views/terminal-view/terminal-panel.tsx @@ -21,6 +21,9 @@ const MIN_FONT_SIZE = 8; const MAX_FONT_SIZE = 32; const DEFAULT_FONT_SIZE = 14; +// Resize constraints +const RESIZE_DEBOUNCE_MS = 100; // Short debounce for responsive feel + interface TerminalPanelProps { sessionId: string; authToken: string | null; @@ -169,20 +172,41 @@ export function TerminalPanel({ console.warn("[Terminal] WebGL addon not available, falling back to canvas"); } - // Fit terminal to container using requestAnimationFrame for better timing - requestAnimationFrame(() => { - if (fitAddon && terminalRef.current) { - const rect = terminalRef.current.getBoundingClientRect(); - // Only fit if container has valid dimensions - if (rect.width >= 100 && rect.height >= 50) { - try { - fitAddon.fit(); - } catch (err) { - console.error("[Terminal] Initial fit error:", err); - } + // Fit terminal to container - wait for stable dimensions + // Use multiple RAFs to let react-resizable-panels finish layout + let fitAttempts = 0; + const MAX_FIT_ATTEMPTS = 5; + let lastWidth = 0; + let lastHeight = 0; + + const attemptFit = () => { + if (!fitAddon || !terminalRef.current || fitAttempts >= MAX_FIT_ATTEMPTS) return; + + const rect = terminalRef.current.getBoundingClientRect(); + fitAttempts++; + + // Check if dimensions are stable (same as last attempt) and valid + if ( + rect.width === lastWidth && + rect.height === lastHeight && + rect.width > 0 && + rect.height > 0 + ) { + try { + fitAddon.fit(); + } catch (err) { + console.error("[Terminal] Initial fit error:", err); } + return; } - }); + + // Dimensions still changing or too small, try again + lastWidth = rect.width; + lastHeight = rect.height; + requestAnimationFrame(attemptFit); + }; + + requestAnimationFrame(attemptFit); xtermRef.current = terminal; fitAddonRef.current = fitAddon; @@ -299,10 +323,11 @@ export function TerminalPanel({ terminal.write(msg.data); break; case "scrollback": - // Clear terminal before replaying scrollback to prevent duplicates on reconnection - terminal.clear(); - // Replay scrollback buffer (previous terminal output) - if (msg.data) { + // Only process scrollback if there's actual data + // Don't clear if empty - prevents blank terminal issue + if (msg.data && msg.data.length > 0) { + // Use reset() which is more reliable than clear() or escape sequences + terminal.reset(); terminal.write(msg.data); } break; @@ -371,7 +396,7 @@ export function TerminalPanel({ }; }, [sessionId, authToken, wsUrl, isTerminalReady]); - // Handle resize with debouncing and validation + // Handle resize with debouncing const handleResize = useCallback(() => { // Clear any pending resize if (resizeDebounceRef.current) { @@ -382,14 +407,11 @@ export function TerminalPanel({ resizeDebounceRef.current = setTimeout(() => { if (!fitAddonRef.current || !xtermRef.current || !terminalRef.current) return; - // Validate minimum dimensions before resizing const container = terminalRef.current; const rect = container.getBoundingClientRect(); - const MIN_WIDTH = 100; - const MIN_HEIGHT = 50; - if (rect.width < MIN_WIDTH || rect.height < MIN_HEIGHT) { - console.log("[Terminal] Container too small to resize:", rect.width, "x", rect.height); + // Only skip if container has no size at all + if (rect.width <= 0 || rect.height <= 0) { return; } @@ -404,7 +426,7 @@ export function TerminalPanel({ } catch (err) { console.error("[Terminal] Resize error:", err); } - }, 100); // 100ms debounce + }, RESIZE_DEBOUNCE_MS); }, []); // Resize observer @@ -439,12 +461,16 @@ export function TerminalPanel({ if (xtermRef.current && isTerminalReady) { xtermRef.current.options.fontSize = fontSize; // Refit after font size change - if (fitAddonRef.current) { - fitAddonRef.current.fit(); - // Notify server of new dimensions - const { cols, rows } = xtermRef.current; - if (wsRef.current?.readyState === WebSocket.OPEN) { - wsRef.current.send(JSON.stringify({ type: "resize", cols, rows })); + if (fitAddonRef.current && terminalRef.current) { + const rect = terminalRef.current.getBoundingClientRect(); + // Only fit if container has any size + if (rect.width > 0 && rect.height > 0) { + fitAddonRef.current.fit(); + // Notify server of new dimensions + const { cols, rows } = xtermRef.current; + if (wsRef.current?.readyState === WebSocket.OPEN) { + wsRef.current.send(JSON.stringify({ type: "resize", cols, rows })); + } } } } diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index 551c3c3d..0cfa1a5a 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -199,6 +199,16 @@ wss.on("connection", (ws: WebSocket) => { const terminalConnections: Map> = new Map(); // Track last resize dimensions per session to deduplicate resize messages const lastResizeDimensions: Map = new Map(); +// Track last resize timestamp to rate-limit resize operations (prevents resize storm) +const lastResizeTime: Map = new Map(); +const RESIZE_MIN_INTERVAL_MS = 100; // Minimum 100ms between resize operations + +// Clean up resize tracking when sessions actually exit (not just when connections close) +terminalService.onExit((sessionId) => { + lastResizeDimensions.delete(sessionId); + lastResizeTime.delete(sessionId); + terminalConnections.delete(sessionId); +}); // Terminal WebSocket connection handler terminalWss.on( @@ -261,8 +271,8 @@ terminalWss.on( ); // Send scrollback buffer BEFORE subscribing to prevent race condition - // This ensures data isn't sent twice (once in scrollback, once via subscription) - const scrollback = terminalService.getScrollback(sessionId); + // Also clear pending output buffer to prevent duplicates from throttled flush + const scrollback = terminalService.getScrollbackAndClearPending(sessionId); if (scrollback && scrollback.length > 0) { ws.send( JSON.stringify({ @@ -299,21 +309,32 @@ terminalWss.on( break; case "resize": - // Resize terminal with deduplication + // Resize terminal with deduplication and rate limiting if (msg.cols && msg.rows) { - // Check if dimensions are different from last resize + const now = Date.now(); + const lastTime = lastResizeTime.get(sessionId) || 0; const lastDimensions = lastResizeDimensions.get(sessionId); + + // Skip if resized too recently (prevents resize storm during splits) + if (now - lastTime < RESIZE_MIN_INTERVAL_MS) { + break; + } + + // Check if dimensions are different from last resize if ( !lastDimensions || lastDimensions.cols !== msg.cols || lastDimensions.rows !== msg.rows ) { - // Only resize if dimensions changed - terminalService.resize(sessionId, msg.cols, msg.rows); + // Only suppress output on subsequent resizes, not the first one + // The first resize happens on terminal open and we don't want to drop the initial prompt + const isFirstResize = !lastDimensions; + terminalService.resize(sessionId, msg.cols, msg.rows, !isFirstResize); lastResizeDimensions.set(sessionId, { cols: msg.cols, rows: msg.rows, }); + lastResizeTime.set(sessionId, now); } } break; @@ -344,8 +365,10 @@ terminalWss.on( connections.delete(ws); if (connections.size === 0) { terminalConnections.delete(sessionId); - // Clean up resize dimensions tracking when session has no more connections - lastResizeDimensions.delete(sessionId); + // DON'T delete lastResizeDimensions/lastResizeTime here! + // The session still exists, and reconnecting clients need to know + // this isn't the "first resize" to prevent duplicate prompts. + // These get cleaned up when the session actually exits. } } }); diff --git a/apps/server/src/services/terminal-service.ts b/apps/server/src/services/terminal-service.ts index b05d987c..6d8faa7f 100644 --- a/apps/server/src/services/terminal-service.ts +++ b/apps/server/src/services/terminal-service.ts @@ -26,6 +26,8 @@ export interface TerminalSession { scrollbackBuffer: string; // Store recent output for replay on reconnect outputBuffer: string; // Pending output to be flushed flushTimeout: NodeJS.Timeout | null; // Throttle timer + resizeInProgress: boolean; // Flag to suppress scrollback during resize + resizeDebounceTimeout: NodeJS.Timeout | null; // Resize settle timer } export interface TerminalOptions { @@ -213,6 +215,8 @@ export class TerminalService extends EventEmitter { scrollbackBuffer: "", outputBuffer: "", flushTimeout: null, + resizeInProgress: false, + resizeDebounceTimeout: null, }; this.sessions.set(id, session); @@ -239,6 +243,13 @@ export class TerminalService extends EventEmitter { // Forward data events with throttling ptyProcess.onData((data) => { + // Skip ALL output during resize/reconnect to prevent prompt redraw duplication + // This drops both scrollback AND live output during the suppression window + // Without this, prompt redraws from SIGWINCH go to live clients causing duplicates + if (session.resizeInProgress) { + return; + } + // Append to scrollback buffer session.scrollbackBuffer += data; // Trim if too large (keep the most recent data) @@ -246,7 +257,7 @@ export class TerminalService extends EventEmitter { session.scrollbackBuffer = session.scrollbackBuffer.slice(-MAX_SCROLLBACK_SIZE); } - // Buffer output for throttled delivery + // Buffer output for throttled live delivery session.outputBuffer += data; // Schedule flush if not already scheduled @@ -282,18 +293,40 @@ export class TerminalService extends EventEmitter { /** * Resize a terminal session + * @param suppressOutput - If true, suppress output during resize to prevent duplicate prompts. + * Should be false for the initial resize so the first prompt isn't dropped. */ - resize(sessionId: string, cols: number, rows: number): boolean { + resize(sessionId: string, cols: number, rows: number, suppressOutput: boolean = true): boolean { const session = this.sessions.get(sessionId); if (!session) { console.warn(`[Terminal] Session ${sessionId} not found for resize`); return false; } try { + // Only suppress output on subsequent resizes, not the initial one + // This prevents the shell's first prompt from being dropped + if (suppressOutput) { + session.resizeInProgress = true; + if (session.resizeDebounceTimeout) { + clearTimeout(session.resizeDebounceTimeout); + } + } + session.pty.resize(cols, rows); + + // Clear resize flag after a delay (allow prompt to settle) + // 150ms is enough for most prompts - longer causes sluggish feel + if (suppressOutput) { + session.resizeDebounceTimeout = setTimeout(() => { + session.resizeInProgress = false; + session.resizeDebounceTimeout = null; + }, 150); + } + return true; } catch (error) { console.error(`[Terminal] Error resizing session ${sessionId}:`, error); + session.resizeInProgress = false; // Clear flag on error return false; } } @@ -312,6 +345,11 @@ export class TerminalService extends EventEmitter { clearTimeout(session.flushTimeout); session.flushTimeout = null; } + // Clean up resize debounce timeout + if (session.resizeDebounceTimeout) { + clearTimeout(session.resizeDebounceTimeout); + session.resizeDebounceTimeout = null; + } session.pty.kill(); this.sessions.delete(sessionId); console.log(`[Terminal] Session ${sessionId} killed`); @@ -337,6 +375,30 @@ export class TerminalService extends EventEmitter { return session?.scrollbackBuffer || null; } + /** + * Get scrollback buffer and clear pending output buffer to prevent duplicates + * Call this when establishing a new WebSocket connection + * This prevents data that's already in scrollback from being sent again via data callback + */ + getScrollbackAndClearPending(sessionId: string): string | null { + const session = this.sessions.get(sessionId); + if (!session) return null; + + // Clear any pending output that hasn't been flushed yet + // This data is already in scrollbackBuffer + session.outputBuffer = ""; + if (session.flushTimeout) { + clearTimeout(session.flushTimeout); + session.flushTimeout = null; + } + + // NOTE: Don't set resizeInProgress here - it causes blank terminals + // if the shell hasn't output its prompt yet when WebSocket connects. + // The resize() method handles suppression during actual resize events. + + return session.scrollbackBuffer || null; + } + /** * Get all active sessions */