From c647bb98a9ae2c972ccab143b24eec4d2281ac73 Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:23:11 +0800 Subject: [PATCH] fix(web): break the terminal resize loop properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous 'stabilize useMutation results' fix addressed one cause of /shell/resize being spammed (the mutation object identity), but not the actual root cause: a closed loop between xterm's onResize callback, ResizeObserver, and the backend. The full loop: 1. ResizeObserver fires 2. fitAddon.fit() runs 3. fit() calls terminal.resize(cols, rows) 4. terminal.onResize(cb) fires the listener 5. cb calls resize.mutate() — POSTs to /shell/resize 6. server calls pty.Setsize 7. bash receives SIGWINCH and redraws the prompt 8. the new output writes to the terminal, which can shift a scrollbar / padding by a few pixels 9. ResizeObserver fires again -> repeat Fixes applied to TerminalPanel's terminal-init useEffect: - Drop the terminal.onResize subscription entirely. Per the xterm + FitAddon pattern, the right way to push size to the backend is: from the ResizeObserver path, call fit(), then read terminal.cols/rows synchronously and call resize.mutate(). - Dedupe by (cols, rows) — if the size hasn't changed since the last reported size, skip the mutation. - Dedupe the ResizeObserver callback by container getBoundingClientRect — sub-pixel changes (e.g. a scrollbar appearing) won't trigger a redundant fit(). - Depend on the stable sendResize function (the destructured mutate from useMutation) rather than the whole resize object, so unrelated render churn can't tear down and re-create the terminal. - Run an initial fit() at mount so the backend learns the real size before the user starts typing. The 100ms debounce was a band-aid that didn't actually break the loop — it only slowed it down. --- web/src/components/terminal/TerminalPanel.tsx | 114 ++++++++++++------ 1 file changed, 79 insertions(+), 35 deletions(-) diff --git a/web/src/components/terminal/TerminalPanel.tsx b/web/src/components/terminal/TerminalPanel.tsx index 1f413d0..2174f40 100644 --- a/web/src/components/terminal/TerminalPanel.tsx +++ b/web/src/components/terminal/TerminalPanel.tsx @@ -29,6 +29,9 @@ export function TerminalPanel({ workspaceId, shellId }: TerminalPanelProps) { const restart = useShellRestart(workspaceId); const resize = useShellResize(workspaceId, shellId); + // Destructure the mutate function so the effect can depend on the + // stable function reference alone, not the whole mutation result. + const sendResize = resize.mutate; // Initialize xterm once and keep it alive across WS status changes. useEffect(() => { @@ -37,8 +40,6 @@ export function TerminalPanel({ workspaceId, shellId }: TerminalPanelProps) { let terminal: Terminal | undefined; let fitAddon: FitAddon | undefined; - let debounceRef: number | undefined; - let removeResize: (() => void) | undefined; try { terminal = new Terminal({ @@ -62,19 +63,6 @@ export function TerminalPanel({ workspaceId, shellId }: TerminalPanelProps) { terminal.loadAddon(fitAddon); terminal.open(container); - removeResize = terminal.onResize(({ cols, rows }) => { - if (!workspaceId || !shellId) return; - if (cols <= 0 || rows <= 0) return; - if (debounceRef !== undefined) { - window.clearTimeout(debounceRef); - } - debounceRef = window.setTimeout(() => { - debounceRef = undefined; - if (!workspaceId || !shellId) return; - resize.mutate({ cols, rows }); - }, 100); - }).dispose; - terminalRef.current = terminal; fitAddonRef.current = fitAddon; } catch { @@ -84,46 +72,102 @@ export function TerminalPanel({ workspaceId, shellId }: TerminalPanelProps) { return; } + // --- Resize loop: fit() -> terminal.resize() -> onResize() -> mutate. + // + // Listening to terminal.onResize to push the new size to the backend + // creates a closed loop: + // fit() -> terminal.resize(cols, rows) -> onResize(cb) fires + // -> cb calls resize.mutate() -> server calls pty.Setsize + // -> bash receives SIGWINCH and redraws -> new output via WS + // -> terminal.write(...) may change layout (scrollbar/padding) by + // a few pixels -> ResizeObserver fires again -> fit() again. + // + // The right pattern (per the xterm + FitAddon docs): only call + // fit() from the ResizeObserver, then read terminal.cols/rows + // synchronously and push to the backend. Skip when the size hasn't + // changed. Skip when the container's getBoundingClientRect hasn't + // changed (so sub-pixel oscillation doesn't trigger). + + const lastReportedSize = { cols: 0, rows: 0 }; + const lastRect = { width: -1, height: -1 }; let rafId: number | undefined; + + const reportSize = () => { + if (!workspaceId || !shellId) return; + const cols = terminal!.cols; + const rows = terminal!.rows; + if (cols <= 0 || rows <= 0) return; + if (cols === lastReportedSize.cols && rows === lastReportedSize.rows) { + return; + } + lastReportedSize.cols = cols; + lastReportedSize.rows = rows; + sendResize({ cols, rows }); + }; + + const doFit = () => { + const rect = container.getBoundingClientRect(); + // Skip if the container's outer rect didn't actually move. This + // stops ResizeObserver firings triggered by sub-pixel internal + // changes (e.g. scrollbar appearance) from feeding fit(). + if ( + rect.width === lastRect.width && + rect.height === lastRect.height + ) { + return; + } + lastRect.width = rect.width; + lastRect.height = rect.height; + + try { + fitAddon!.fit(); + } catch { + // fit() throws when the container has zero dimensions; ignore + // and wait for the next ResizeObserver tick. + return; + } + // Push the new size to the backend immediately, deduped by cols/rows. + reportSize(); + }; + const scheduleFit = () => { if (rafId !== undefined) return; rafId = requestAnimationFrame(() => { rafId = undefined; - try { - fitAddon?.fit(); - } catch { - // fit() throws when the container has zero dimensions; ignore - // and wait for the next ResizeObserver tick. - } + doFit(); }); }; const resizeObserver = new ResizeObserver(scheduleFit); resizeObserver.observe(container); - const handleResize = () => { + const handleWindowResize = () => { scheduleFit(); }; - window.addEventListener("resize", handleResize); + window.addEventListener("resize", handleWindowResize); + + // Run an initial fit so the backend knows the size as soon as the + // terminal mounts. Without this, the user would type into a default + // 80x24 terminal and the first line of output would wrap. + scheduleFit(); return () => { if (rafId !== undefined) { cancelAnimationFrame(rafId); } - if (debounceRef !== undefined) { - window.clearTimeout(debounceRef); - debounceRef = undefined; - } - removeResize?.(); resizeObserver.disconnect(); - window.removeEventListener("resize", handleResize); - terminal?.dispose(); - fitAddon?.dispose(); - terminalRef.current = null; - fitAddonRef.current = null; - }; - }, [resize, workspaceId, shellId]); + window.removeEventListener("resize", handleWindowResize); + terminal?.dispose(); + fitAddon?.dispose(); + terminalRef.current = null; + fitAddonRef.current = null; + }; + // Depend on `resize.mutate` (the stable function from useMutation) + // rather than the whole `resize` object, so this effect doesn't + // re-run on every render if anything else in the mutation result + // changes by reference. + }, [sendResize, workspaceId, shellId]); // Reset per-workspace state and wire the active socket to xterm. useEffect(() => {