From 6165eaf8d9ac6cf98f228a402f1e7b4369e82b71 Mon Sep 17 00:00:00 2001 From: Blomios Date: Thu, 30 Jul 2026 09:36:05 +0200 Subject: [PATCH] =?UTF-8?q?fix(terminals):=20d=C3=A9tecte=20la=20stabilit?= =?UTF-8?q?=C3=A9=20r=C3=A9elle=20du=20fit=20terminal=20au=20remount?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L'ancien correctif (traîne de refits à nombre de frames fixe, MOUNT_SETTLE_REFIT_FRAMES) restait insuffisant : une cellule CLI pouvait rester mal dimensionnée après un changement de projet/layout si l'agent écrivait en arrière-plan pendant la fenêtre de stabilisation, nécessitant un resize manuel pour corriger l'affichage. TerminalView détecte désormais la stabilité réelle du fit (dimensions inchangées sur des mesures successives) au lieu de s'appuyer sur un nombre de frames fixe avant de rendre la main au ResizeObserver. Co-Authored-By: Claude Sonnet 5 --- .../features/terminals/TerminalView.test.tsx | 75 ++++++++++++++++-- .../src/features/terminals/TerminalView.tsx | 76 ++++++++++++------- 2 files changed, 120 insertions(+), 31 deletions(-) diff --git a/frontend/src/features/terminals/TerminalView.test.tsx b/frontend/src/features/terminals/TerminalView.test.tsx index 6e60038..90bc728 100644 --- a/frontend/src/features/terminals/TerminalView.test.tsx +++ b/frontend/src/features/terminals/TerminalView.test.tsx @@ -455,10 +455,11 @@ describe("TerminalView — visible launch-failure surface (ticket #14 F3)", () = fitSpy.mockRestore(); }); - it("keeps fitting across a keyed project remount until the new cell settles", async () => { + it("keeps fitting across a keyed project remount until the new cell is actually stable", async () => { // ProjectsView keys LayoutGrid by project. A project switch therefore - // unmounts/remounts TerminalView, so the mount path itself must keep a - // short tail of fits alive while the new cell geometry settles. + // unmounts/remounts TerminalView, so the mount path itself must fit until + // the new cell's real geometry has stabilized, not for a guessed count of + // frames. const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); const firstHandle = makeHandle({ sessionId: "project-1" }); const secondHandle = makeHandle({ sessionId: "project-2" }); @@ -489,11 +490,11 @@ describe("TerminalView — visible launch-failure surface (ticket #14 F3)", () = await waitFor(() => expect(open).toHaveBeenCalledTimes(2)); setTerminalBoxSize(520, 260); - for (let i = 0; i < 8; i += 1) { + for (let i = 0; i < 3; i += 1) { await new Promise((resolve) => requestAnimationFrame(resolve)); } - expect(fitSpy.mock.calls.length).toBeGreaterThan(5); + expect(fitSpy.mock.calls.length).toBeGreaterThanOrEqual(2); await waitFor(() => expect(secondHandle.resize).toHaveBeenCalled()); expect(firstHandle.close).not.toHaveBeenCalled(); expect(secondHandle.close).not.toHaveBeenCalled(); @@ -501,6 +502,70 @@ describe("TerminalView — visible launch-failure surface (ticket #14 F3)", () = fitSpy.mockRestore(); }); + it("refits a late remount after scrollback while the cell was absent", async () => { + // Regression for project/layout switch: the agent can keep writing while + // its view is unmounted. On reattach, scrollback is repainted immediately, + // but the new container may stay 0x0 longer than the old fixed remount + // frame tail. The view must keep retrying until a real size appears. + const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); + const handle = makeHandle({ sessionId: "late-remount-1" }); + const reattach = vi.fn( + async (_sessionId: string, onData: (b: Uint8Array) => void) => { + onData(new TextEncoder().encode("agent wrote while absent\r\n")); + return { + handle, + scrollback: new TextEncoder().encode("retained scrollback\r\n"), + } satisfies ReattachResult; + }, + ); + const open = vi.fn(async () => makeHandle({ sessionId: "should-not-open" })); + + renderView(new MockTerminalGateway(), "/cwd", { + sessionId: "late-remount-1", + open, + reattach, + }); + + await waitFor(() => expect(reattach).toHaveBeenCalledTimes(1)); + expect(open).not.toHaveBeenCalled(); + fitSpy.mockClear(); + + // Stay zero-sized past the previous fixed remount tail. + for (let i = 0; i < 16; i += 1) { + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + expect(fitSpy).not.toHaveBeenCalled(); + expect(handle.resize).not.toHaveBeenCalled(); + + setTerminalBoxSize(560, 280); + + await waitFor(() => expect(fitSpy).toHaveBeenCalled()); + await waitFor(() => expect(handle.resize).toHaveBeenCalled()); + expect(handle.close).not.toHaveBeenCalled(); + + fitSpy.mockRestore(); + }); + + it("continues refitting while remount geometry keeps changing, then stops after stability", async () => { + const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); + const handle = makeHandle({ sessionId: "moving-remount-1" }); + const open = vi.fn(async () => handle); + + renderView(new MockTerminalGateway(), "/cwd", { open }); + await waitFor(() => expect(open).toHaveBeenCalledTimes(1)); + + for (let i = 0; i < 16; i += 1) { + setTerminalBoxSize(420 + i, 220); + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + + expect(fitSpy.mock.calls.length).toBeGreaterThan(12); + setTerminalBoxSize(520, 260); + await waitFor(() => expect(handle.resize).toHaveBeenCalled()); + + fitSpy.mockRestore(); + }); + it("does not refit when refitSignal is left undefined (no-op for callers that don't pass it)", async () => { const fitSpy = vi.spyOn(FitAddon.prototype, "fit"); const open = vi.fn(async () => makeHandle({ sessionId: "no-signal-1" })); diff --git a/frontend/src/features/terminals/TerminalView.tsx b/frontend/src/features/terminals/TerminalView.tsx index 6a2e8b9..d373cfd 100644 --- a/frontend/src/features/terminals/TerminalView.tsx +++ b/frontend/src/features/terminals/TerminalView.tsx @@ -191,7 +191,7 @@ export function TerminalView({ // Holds the mounted instance's `refit` closure so the `refitSignal` effect // below (a separate effect, since it must NOT re-run/reopen the terminal on // every parent render) can trigger it without depending on `cwd`'s effect. - const refitRef = useRef<((settleFrames?: number) => void) | null>(null); + const refitRef = useRef<((maxFrames?: number) => void) | null>(null); useEffect(() => { const container = containerRef.current; @@ -229,7 +229,6 @@ export function TerminalView({ let lastRows = term.rows; let lastCols = term.cols; let hasUsefulFit = false; - let settleFramesRemaining = 0; // Keystroke → PTY path. The agent cell is a **native terminal** // (ARCHITECTURE §20): keystrokes reach the PTY exactly like a plain shell. @@ -363,34 +362,51 @@ export function TerminalView({ // doesn't spin forever. // A successful fit can also land on a non-zero but still intermediate box // during project/layout switches, split/merge commits, re-attach, and OS - // minimize/restore. Keep a small coalesced tail of fits on following frames - // so the final settled geometry is pushed automatically without requiring a - // manual resize. This stays bounded and preserves the rows/cols-changed - // guard before touching the PTY. - const SETTLE_REFIT_FRAMES = 4; - // Project switches remount LayoutGrid/TerminalView by key. The new cell can - // report a non-zero but still intermediate box for more than the ordinary - // transition tail, so the mount path keeps fitting a little longer. - const MOUNT_SETTLE_REFIT_FRAMES = 12; - const MAX_ZERO_SIZE_RETRIES = MOUNT_SETTLE_REFIT_FRAMES; - let zeroSizeRetries = 0; + // minimize/restore. Instead of guessing a fixed number of follow-up frames, + // keep fitting until the measured DOM box and resulting xterm grid stay + // stable for consecutive frames. A hard frame cap keeps genuinely unstable + // or never-laid-out containers bounded. + const STABLE_REFIT_FRAMES = 2; + const SETTLE_REFIT_MAX_FRAMES = 24; + const MOUNT_REFIT_MAX_FRAMES = 60; + let refitFramesRemaining = 0; + let stableFrames = 0; + let lastStabilityKey = ""; const refit = () => { rafId = 0; if (disposed) return; - if (container.clientWidth === 0 || container.clientHeight === 0) { - if (zeroSizeRetries < MAX_ZERO_SIZE_RETRIES) { - zeroSizeRetries += 1; + + const consumeFrame = () => { + refitFramesRemaining = Math.max(0, refitFramesRemaining - 1); + }; + const scheduleNextIfNeeded = () => { + if (refitFramesRemaining > 0) { rafId = requestAnimationFrame(refit); } + }; + + const width = container.clientWidth; + const height = container.clientHeight; + if (width === 0 || height === 0) { + stableFrames = 0; + lastStabilityKey = ""; + consumeFrame(); + scheduleNextIfNeeded(); return; } - zeroSizeRetries = 0; + try { fit.fit(); } catch { return; } - if (term.rows <= 0 || term.cols <= 0) return; + if (term.rows <= 0 || term.cols <= 0) { + stableFrames = 0; + lastStabilityKey = ""; + consumeFrame(); + scheduleNextIfNeeded(); + return; + } const isFirstUsefulFit = !hasUsefulFit; if (isFirstUsefulFit) { @@ -406,23 +422,31 @@ export function TerminalView({ resizeHandleToCurrentGeometry(); } - if (settleFramesRemaining > 0) { - settleFramesRemaining -= 1; - rafId = requestAnimationFrame(refit); + const stabilityKey = `${width}x${height}:${term.rows}x${term.cols}`; + if (stabilityKey === lastStabilityKey) { + stableFrames += 1; + } else { + lastStabilityKey = stabilityKey; + stableFrames = 1; } + + consumeFrame(); + if (stableFrames < STABLE_REFIT_FRAMES) scheduleNextIfNeeded(); }; - const scheduleRefit = (settleFrames = 0) => { - settleFramesRemaining = Math.max(settleFramesRemaining, settleFrames); + const scheduleRefit = (maxFrames = SETTLE_REFIT_MAX_FRAMES) => { + refitFramesRemaining = Math.max(refitFramesRemaining, maxFrames); + stableFrames = 0; + lastStabilityKey = ""; if (!rafId) rafId = requestAnimationFrame(refit); }; - const scheduleSettledRefit = () => scheduleRefit(SETTLE_REFIT_FRAMES); + const scheduleSettledRefit = () => scheduleRefit(SETTLE_REFIT_MAX_FRAMES); const scheduleVisibleRefit = () => { if (document.visibilityState === "hidden") return; scheduleSettledRefit(); }; const ro = new ResizeObserver(() => scheduleRefit()); ro.observe(container); - scheduleRefit(MOUNT_SETTLE_REFIT_FRAMES); + scheduleRefit(MOUNT_REFIT_MAX_FRAMES); window.addEventListener("resize", scheduleSettledRefit); window.addEventListener("focus", scheduleSettledRefit); window.addEventListener("pageshow", scheduleSettledRefit); @@ -468,7 +492,7 @@ export function TerminalView({ // logic. useEffect(() => { if (refitSignal === undefined) return; - refitRef.current?.(4); + refitRef.current?.(24); }, [refitSignal]); const showNetworkBanner =