diff --git a/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts b/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts index d93de34e591..4c72212ee1c 100644 --- a/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts +++ b/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts @@ -19,6 +19,7 @@ import '@testing-library/jest-dom/vitest'; import { render, screen, waitFor } from '@testing-library/svelte'; +import { Terminal } from '@xterm/xterm'; import { get } from 'svelte/store'; import { beforeEach, expect, test, vi } from 'vitest'; @@ -174,7 +175,247 @@ test('terminal active/ restarts connection after stopping and starting a contain await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3)); }); -test('terminal active/ restarts connection after restarting a container', async () => { +test('terminal reconnects via scheduleReconnect when immediate reconnect fails during restart', async () => { + vi.useFakeTimers(); + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Simulate restart: the exec dies while the store still says RUNNING (due to debounce). + // The immediate reconnect in receiveEndCallback fails because the container is mid-restart. + shellInContainerMock.mockRejectedValueOnce(new Error('container is restarting')); + + onEndCallback(); + + // The rejected reconnect triggers scheduleReconnect + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Restore mock for the next call to succeed + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + // Advance time to trigger the scheduled reconnect (2s delay) + await vi.advanceTimersByTimeAsync(2000); + + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3)); + + // Clean up component before restoring real timers to clear internal reconnect timer + renderObject.unmount(); + vi.useRealTimers(); +}); + +test('scheduleReconnect retries when restartTerminal fails inside the timer callback', async () => { + vi.useFakeTimers(); + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Make the next TWO calls fail so scheduleReconnect's own catch path is exercised + shellInContainerMock.mockRejectedValueOnce(new Error('first failure')); + shellInContainerMock.mockRejectedValueOnce(new Error('second failure')); + + // Shell ends → receiveEndCallback → restartTerminal fails (call #2) → scheduleReconnect + onEndCallback(); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Advance timer → scheduleReconnect fires → restartTerminal fails again (call #3) + // → the catch inside scheduleReconnect re-calls scheduleReconnect (lines 80-81) + await vi.advanceTimersByTimeAsync(2000); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3)); + + // Restore mock so the re-scheduled timer succeeds + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + // Advance again → the re-scheduled timer fires → restartTerminal succeeds (call #4) + await vi.advanceTimersByTimeAsync(2000); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(4)); + + renderObject.unmount(); + vi.useRealTimers(); +}); + +test('user input is forwarded to the container via shellInContainerSend', async () => { + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + // Spy on Terminal.open to capture the instance via mock.contexts + const openSpy = vi.spyOn(Terminal.prototype, 'open'); + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + _onEnd: () => void, + ) => { + return sendCallbackId; + }, + ); + vi.mocked(window.shellInContainerSend).mockResolvedValue(undefined); + + render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Retrieve the Terminal instance that called open() + const terminalInstance = openSpy.mock.contexts[0]; + expect(terminalInstance).toBeDefined(); + + // Fire the onData event through xterm's internal emitter to simulate user typing + /* eslint-disable @typescript-eslint/no-explicit-any */ + // biome-ignore lint/suspicious/noExplicitAny: accessing xterm internals for testing + (terminalInstance as any)._core._onData.fire('test input'); + /* eslint-enable @typescript-eslint/no-explicit-any */ + + await waitFor(() => expect(window.shellInContainerSend).toHaveBeenCalledWith(sendCallbackId, 'test input')); + + openSpy.mockRestore(); +}); + +test('receiveEndCallback schedules reconnect when container is not running', async () => { + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Container stops — exec ends while state is not RUNNING. + // receiveEndCallback's else branch calls scheduleReconnect. + container.state = 'EXITED'; + await renderObject.rerender({ container: container, screenReaderMode: true }); + onEndCallback(); + + // No immediate reconnect since container is not RUNNING + expect(shellInContainerMock).toHaveBeenCalledTimes(1); + + // Container comes back — $effect detects EXITED → RUNNING and reconnects + container.state = 'RUNNING'; + await renderObject.rerender({ container: container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); +}); + +test('receiveEndCallback reconnects successfully and resizes terminal', async () => { + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Shell ends while container is running — receiveEndCallback calls restartTerminal + onEndCallback(); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Verify shellInContainerResize was called for the reconnect (not just the initial connection) + expect(window.shellInContainerResize).toHaveBeenCalledTimes(2); +}); + +test('receiveEndCallback reconnect ignores first data chunk to avoid prompt duplication', async () => { const container: ContainerInfoUI = { id: 'myContainer', state: 'RUNNING', @@ -195,41 +436,235 @@ test('terminal active/ restarts connection after restarting a container', async ) => { onDataCallback = onData; onEndCallback = onEnd; - // return a callback id return sendCallbackId; }, ); - // render the component with a terminal const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); - // wait shellInContainerMock is called - await waitFor(() => expect(shellInContainerMock).toHaveBeenCalled()); - - // write some data on the terminal - onDataCallback(Buffer.from('hello\nworld')); - - // wait 1s - await waitFor(() => renderObject.container.querySelector('div[aria-live="assertive"]')); - - // check the content + // Write initial data + onDataCallback(Buffer.from('prompt$ ')); await waitFor(() => { - // search a div having aria-live="assertive" attribute - const terminalLinesLiveRegion = renderObject.container.querySelector('div[aria-live="assertive"]'); - expect(terminalLinesLiveRegion).toHaveTextContent('hello world'); + const region = renderObject.container.querySelector('div[aria-live="assertive"]'); + expect(region).toHaveTextContent('prompt$'); }); - container.state = 'RESTARTING'; + // Shell ends while running — receiveEndCallback routes through restartTerminal, + // which sets ignoreFirstData = true before reopening the shell + onEndCallback(); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + // The reconnected shell sends the initial prompt chunk — it should be ignored + onDataCallback(Buffer.from('prompt$ ')); + + // Send real user output after the ignored chunk + onDataCallback(Buffer.from('ls\nfile1 file2\nprompt$ ')); + + await waitFor(() => { + const region = renderObject.container.querySelector('div[aria-live="assertive"]'); + expect(region).toHaveTextContent('prompt$ ls file1 file2 prompt$'); + }); +}); + +test('$effect schedules reconnect when restartTerminal fails', async () => { + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + _onEnd: () => void, + ) => { + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Container goes to EXITED + container.state = 'EXITED'; await renderObject.rerender({ container: container, screenReaderMode: true }); + // Container returns to RUNNING — $effect fires restartTerminal, but it fails + shellInContainerMock.mockRejectedValueOnce(new Error('not ready yet')); container.state = 'RUNNING'; - await renderObject.rerender({ container: container, screenReaderMode: true }); + // $effect's catch calls scheduleReconnect — wait for the failed attempt + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Restore mock for the scheduled retry to succeed + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + _onEnd: () => void, + ) => { + return sendCallbackId; + }, + ); + + // scheduleReconnect will fire after 2s — wait for it with real timers + await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3), { timeout: 5000 }); +}); + +test('concurrent receiveEndCallback calls do not create duplicate connections', async () => { + vi.useFakeTimers(); + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + let resolveShell: ((id: number) => void) | undefined; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Make the next shellInContainer call hang so we can test the guard + shellInContainerMock.mockImplementation( + () => + new Promise(resolve => { + resolveShell = resolve; + }), + ); + + // First exec dies — receiveEndCallback starts a reconnect (now in flight) onEndCallback(); + // Second exec end fires while the first reconnect is still pending — + // the reconnecting guard prevents a duplicate, but schedules a safety-net reconnect + onEndCallback(); + + // Resolve the pending reconnect + resolveShell?.(sendCallbackId); + + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Only 2 calls so far: initial + one reconnect. The second onEndCallback was blocked + // by the reconnecting guard but scheduled a safety-net timer. expect(shellInContainerMock).toHaveBeenCalledTimes(2); + + // Restore mock for the safety-net timer's reconnect + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + // Advance past the safety-net timer (2s) + await vi.advanceTimersByTimeAsync(2000); + + // The safety-net reconnect fires — call #3 + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3)); + + renderObject.unmount(); + vi.useRealTimers(); +}); + +test('receiveEndCallback during reconnect schedules safety-net retry to prevent freeze', async () => { + vi.useFakeTimers(); + const container: ContainerInfoUI = { + id: 'myContainer', + state: 'RUNNING', + engineId: 'podman', + } as unknown as ContainerInfoUI; + + let onEndCallback: () => void = () => {}; + let resolveShell: ((id: number) => void) | undefined; + + const sendCallbackId = 12345; + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + const renderObject = render(ContainerDetailsTerminal, { container, screenReaderMode: true }); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(1)); + + // Make reconnect hang so we can simulate the race + shellInContainerMock.mockImplementation( + () => + new Promise(resolve => { + resolveShell = resolve; + }), + ); + + // Exec dies → receiveEndCallback → restartTerminal starts (reconnecting = true) + onEndCallback(); + + // The new session's onEnd fires while the reconnect is still in flight — + // this is the race condition: the session being established is already dead. + // With the fix, this schedules a safety-net reconnect instead of being dropped. + onEndCallback(); + + // The in-progress reconnect completes — but the session it established is "dead" + resolveShell?.(sendCallbackId); + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2)); + + // Restore mock so the safety-net reconnect succeeds + shellInContainerMock.mockImplementation( + async ( + _engineId: string, + _containerId: string, + _onData: (data: Buffer) => void, + _onError: (error: string) => void, + onEnd: () => void, + ) => { + onEndCallback = onEnd; + return sendCallbackId; + }, + ); + + // Advance past the 2s safety-net timer + await vi.advanceTimersByTimeAsync(2000); + + // The safety-net reconnect creates a fresh session (call #3) — terminal recovers + await vi.waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(3)); + + renderObject.unmount(); + vi.useRealTimers(); }); test('prompt is not duplicated after restoring terminal from containerTerminals store', async () => { diff --git a/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte b/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte index 7609269a313..7085e0b62c1 100644 --- a/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte +++ b/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte @@ -5,6 +5,7 @@ import { TerminalSettings } from '@podman-desktop/core-api/terminal'; import { EmptyScreen } from '@podman-desktop/ui-svelte'; import { FitAddon } from '@xterm/addon-fit'; import { SerializeAddon } from '@xterm/addon-serialize'; +import type { IDisposable } from '@xterm/xterm'; import { Terminal } from '@xterm/xterm'; import { onDestroy, onMount } from 'svelte'; import { router } from 'tinro'; @@ -29,18 +30,58 @@ let terminalContent: string = ''; let serializeAddon: SerializeAddon; let lastState = $state(''); let containerState = $derived(container.state); +let reconnectTimer: ReturnType | undefined; +let onDataDisposable: IDisposable | undefined; +let reconnecting = false; + +function registerInputHandler(callbackId: number): void { + onDataDisposable?.dispose(); + onDataDisposable = shellTerminal?.onData(data => { + window.shellInContainerSend(callbackId, data).catch((error: unknown) => console.log(String(error))); + }); +} $effect(() => { - if (lastState === 'STARTING' && containerState === 'RUNNING') { - restartTerminal().catch((err: unknown) => console.error('Error restarting terminal', err)); + if (lastState !== '' && lastState !== 'RUNNING' && containerState === 'RUNNING') { + restartTerminal().catch((err: unknown) => { + console.error('Error restarting terminal', err); + scheduleReconnect(); + }); } lastState = container.state; }); async function restartTerminal(): Promise { - ignoreFirstData = true; - await executeShellIntoContainer(); - window.dispatchEvent(new Event('resize')); + if (reconnecting) return; + reconnecting = true; + try { + clearReconnectTimer(); + ignoreFirstData = true; + await executeShellIntoContainer(); + window.dispatchEvent(new Event('resize')); + } finally { + reconnecting = false; + } +} + +function clearReconnectTimer(): void { + if (reconnectTimer) { + clearTimeout(reconnectTimer); + reconnectTimer = undefined; + } +} + +function scheduleReconnect(): void { + if (reconnectTimer) return; + reconnectTimer = setTimeout(() => { + reconnectTimer = undefined; + if (container.state === 'RUNNING') { + restartTerminal().catch((err: unknown) => { + console.error('Error restarting terminal', err); + scheduleReconnect(); + }); + } + }, 2000); } // update current route scheme @@ -63,17 +104,20 @@ function createDataCallback(): (data: Buffer) => void { } function receiveEndCallback(): void { - // need to reopen a new terminal if container is running - if (sendCallbackId && containerState === 'RUNNING') { - window - .shellInContainer(container.engineId, container.id, createDataCallback(), () => {}, receiveEndCallback) - .then(id => { - sendCallbackId = id; - shellTerminal?.onData(async data => { - await window.shellInContainerSend(id, data); - }); - }) - .catch((err: unknown) => console.error(`Error opening terminal for container ${container.id}`, err)); + if (!sendCallbackId) return; + + if (reconnecting) { + scheduleReconnect(); + return; + } + + if (containerState === 'RUNNING') { + restartTerminal().catch((err: unknown) => { + console.error(`Error opening terminal for container ${container.id}`, err); + scheduleReconnect(); + }); + } else { + scheduleReconnect(); } } @@ -91,12 +135,7 @@ async function executeShellIntoContainer(): Promise { receiveEndCallback, ); await window.shellInContainerResize(callbackId, shellTerminal.cols, shellTerminal.rows); - // pass data from xterm to container - shellTerminal?.onData(data => { - window.shellInContainerSend(callbackId, data).catch((error: unknown) => console.log(String(error))); - }); - - // store it + registerInputHandler(callbackId); sendCallbackId = callbackId; } @@ -164,8 +203,9 @@ onMount(async () => { }); onDestroy(() => { + clearReconnectTimer(); + onDataDisposable?.dispose(); terminalContent = serializeAddon.serialize(); - // register terminal for reusing it registerTerminal({ engineId: container.engineId, containerId: container.id,