From e01df6138cfe220cbee0951f3af19e162f8a30ff Mon Sep 17 00:00:00 2001 From: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Date: Sat, 1 Aug 2026 22:37:52 -0400 Subject: [PATCH] fix(desktop): harden mounted terminal lifecycle Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta --- .../terminal/TerminalBootstrap.test.mjs | 142 ++++++++++++++---- .../features/terminal/TerminalBootstrap.tsx | 48 +++--- 2 files changed, 142 insertions(+), 48 deletions(-) diff --git a/desktop/src/features/terminal/TerminalBootstrap.test.mjs b/desktop/src/features/terminal/TerminalBootstrap.test.mjs index 20d0e2ec2..514fdb29c 100644 --- a/desktop/src/features/terminal/TerminalBootstrap.test.mjs +++ b/desktop/src/features/terminal/TerminalBootstrap.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { after, before, test } from "node:test"; +import { after, afterEach, before, test } from "node:test"; import { JSDOM } from "jsdom"; @@ -10,6 +10,9 @@ const callbacks = new Map(); const calls = []; let nextCallback = 1; let channel; +let resizeCallback; +let canvasWidth = 840; +let attachResolver = null; before(async () => { Object.assign(globalThis, { @@ -29,6 +32,9 @@ before(async () => { removeEventListener() {}, }); dom.window.ResizeObserver = class { + constructor(callback) { + resizeCallback = callback; + } observe() {} disconnect() {} }; @@ -44,9 +50,9 @@ before(async () => { bottom: 408, height: 408, left: 0, - right: 840, + right: canvasWidth, top: 0, - width: 840, + width: canvasWidth, x: 0, y: 0, toJSON() {}, @@ -66,10 +72,22 @@ before(async () => { calls.push({ command, args }); if (command === "terminal_attach") { channel = args.onFrame; - return Promise.resolve({ + const response = { sessionId: "session-1", subscriptionId: "subscription-1", viewport: { columns: 100, generation: 0, screenLines: 24 }, + }; + return attachResolver + ? new Promise((resolve) => { + attachResolver = () => resolve(response); + }) + : Promise.resolve(response); + } + if (command === "terminal_resize") { + return Promise.resolve({ + columns: args.columns, + generation: args.columns, + screenLines: args.rows, }); } return Promise.resolve(); @@ -86,6 +104,11 @@ before(async () => { }); after(() => dom.window.close()); +afterEach(() => { + calls.length = 0; + canvasWidth = 840; + attachResolver = null; +}); function emit(message, index = 0) { const id = Number(channel.toJSON().slice("__CHANNEL__:".length)); @@ -93,26 +116,30 @@ function emit(message, index = 0) { } test("mounted bootstrap passes GUI context and ACKs only after consuming a frame", async () => { - const { createElement } = await import("react"); + const { StrictMode, createElement } = await import("react"); const { act, render, waitFor } = await import("@testing-library/react"); const { ThemeProvider } = await import("@/shared/theme/ThemeProvider"); const { TerminalBootstrap } = await import("./TerminalBootstrap.tsx"); render( createElement( - ThemeProvider, + StrictMode, null, - createElement("div", { - className: "buzz-huddle-app-surface", - tabIndex: -1, - }), - createElement(TerminalBootstrap, { - channelId: "channel-1", - channelName: "general", - npub: "npub1owner", - relayUrl: "wss://relay.example", - threadId: "thread-1", - }), + createElement( + ThemeProvider, + null, + createElement("div", { + className: "buzz-huddle-app-surface", + tabIndex: -1, + }), + createElement(TerminalBootstrap, { + channelId: "channel-1", + channelName: "general", + npub: "npub1owner", + relayUrl: "wss://relay.example", + threadId: "thread-1", + }), + ), ), ); @@ -132,20 +159,21 @@ test("mounted bootstrap passes GUI context and ACKs only after consuming a frame threadId: "thread-1", }); + const frameMessage = { + type: "frame", + payload: { + bracketedPaste: true, + cursor: { column: 0, line: 0, visible: true }, + focusReporting: true, + full: true, + rows: [], + sequence: 7, + subscriptionId: "subscription-1", + viewport: { columns: 100, generation: 0, screenLines: 24 }, + }, + }; await act(async () => { - emit({ - type: "frame", - payload: { - bracketedPaste: true, - cursor: { column: 0, line: 0, visible: true }, - focusReporting: true, - full: true, - rows: [], - sequence: 7, - subscriptionId: "subscription-1", - viewport: { columns: 100, generation: 0, screenLines: 24 }, - }, - }); + emit(frameMessage); }); await waitFor(() => assert.ok(calls.some(({ command }) => command === "terminal_ack")), @@ -158,4 +186,58 @@ test("mounted bootstrap passes GUI context and ACKs only after consuming a frame subscriptionId: "subscription-1", }, ); + + await act(async () => { + emit(frameMessage, 1); + }); + assert.equal( + calls.filter(({ command }) => command === "terminal_ack").length, + 1, + ); +}); + +test("resize during attach declares only the newest viewport ready", async () => { + const { createElement } = await import("react"); + const { act, render, waitFor } = await import("@testing-library/react"); + const { ThemeProvider } = await import("@/shared/theme/ThemeProvider"); + const { TerminalBootstrap } = await import("./TerminalBootstrap.tsx"); + + attachResolver = () => {}; + const view = render( + createElement( + ThemeProvider, + null, + createElement("div", { + className: "buzz-huddle-app-surface", + tabIndex: -1, + }), + createElement(TerminalBootstrap, { + channelId: "channel-1", + channelName: "general", + npub: "npub1owner", + relayUrl: "wss://relay.example", + threadId: null, + }), + ), + ); + await waitFor(() => assert.equal(typeof attachResolver, "function")); + + canvasWidth = 1_008; + act(() => resizeCallback()); + await act(async () => attachResolver()); + await waitFor(() => + assert.ok( + calls.some(({ command }) => command === "terminal_viewport_ready"), + ), + ); + + const readyCalls = calls.filter( + ({ command }) => command === "terminal_viewport_ready", + ); + assert.equal(readyCalls.at(-1).args.viewport.columns, 120); + assert.equal( + readyCalls.every(({ args }) => args.viewport.columns === 120), + true, + ); + view.unmount(); }); diff --git a/desktop/src/features/terminal/TerminalBootstrap.tsx b/desktop/src/features/terminal/TerminalBootstrap.tsx index 1cf535822..1649b067a 100644 --- a/desktop/src/features/terminal/TerminalBootstrap.tsx +++ b/desktop/src/features/terminal/TerminalBootstrap.tsx @@ -71,7 +71,7 @@ export function TerminalBootstrap({ const [sessions, setSessions] = React.useState([]); const [activeKey, setActiveKey] = React.useState(null); const [available, setAvailable] = React.useState(() => isTauri()); - const acknowledgedFramesRef = React.useRef(new Set()); + const acknowledgedSequenceRef = React.useRef(new Map()); const sessionsRef = React.useRef(sessions); sessionsRef.current = sessions; @@ -147,14 +147,17 @@ export function TerminalBootstrap({ update((session) => ({ ...session, connection })); if (sizeRef.current !== size) { const currentSize = sizeRef.current; - return connection - .resize( - currentSize.columns, - currentSize.rows, - currentSize.pixelWidth, - currentSize.pixelHeight, - ) - .then((viewport) => connection.viewportReady(viewport)); + resizeChainRef.current = resizeChainRef.current + .then(async () => { + const viewport = await connection.resize( + currentSize.columns, + currentSize.rows, + currentSize.pixelWidth, + currentSize.pixelHeight, + ); + await connection.viewportReady(viewport); + }) + .catch(fail); } }) .catch((error) => { @@ -167,15 +170,15 @@ export function TerminalBootstrap({ if (available && context && sessions.length === 0) createSession(); }, [available, context, createSession, sessions.length]); - React.useEffect( - () => () => { + React.useEffect(() => { + mountedRef.current = true; + return () => { mountedRef.current = false; for (const session of sessionsRef.current) { void session.connection?.detach().catch(report); } - }, - [], - ); + }; + }, []); const active = sessions.find((session) => session.key === activeKey) ?? null; const send = (operation: Promise | undefined) => operation?.catch(fail); @@ -232,11 +235,20 @@ export function TerminalBootstrap({ (session) => session.delivery?.frame === frame, )?.delivery; if (!delivery) return; - const deliveryKey = `${delivery.frame.subscriptionId}:${delivery.frame.sequence}`; - if (acknowledgedFramesRef.current.has(deliveryKey)) return; - acknowledgedFramesRef.current.add(deliveryKey); + const { sequence, subscriptionId } = delivery.frame; + const lastAcknowledged = + acknowledgedSequenceRef.current.get(subscriptionId) ?? -1; + if (sequence <= lastAcknowledged) return; + acknowledgedSequenceRef.current.set(subscriptionId, sequence); delivery.acknowledge().catch((error) => { - acknowledgedFramesRef.current.delete(deliveryKey); + if ( + acknowledgedSequenceRef.current.get(subscriptionId) === sequence + ) { + acknowledgedSequenceRef.current.set( + subscriptionId, + lastAcknowledged, + ); + } fail(error); }); }}