feat(FN-881): add bounded bootstrap timeouts and xterm init watchdog
- Add bounded bootstrap timeouts with stale-result guards to useTerminalSessions hook - Implement xterm init watchdog with distinct recovery path for terminal initialization failures - Refactor TerminalModal component with improved error handling and timeout recovery - Add comprehensive test suites for useTerminalSessions hook and TerminalModal component - Update dashboard README with watchdog documentation
This commit is contained in:
@@ -162,9 +162,14 @@ Saved scripts (managed via the Scripts modal or QuickScripts dropdown in the hea
|
||||
- Graceful shutdown with SIGTERM, then SIGKILL fallback
|
||||
|
||||
**Startup Failure Handling**:
|
||||
- Terminal startup never hangs indefinitely — if the backend session cannot be created (server unavailable, network error, etc.), the modal shows a clear error message instead of a stuck loading spinner
|
||||
- Users see an actionable error with a "Retry" button that re-attempts terminal creation without closing the modal
|
||||
- On successful retry, the terminal initializes normally; the error state clears automatically
|
||||
- Terminal startup never hangs indefinitely — bounded timeouts ensure the UI always resolves to either a usable terminal or a clear failure state
|
||||
- **Bootstrap timeout** (15s): If the backend session listing or creation call hangs, the modal transitions from "Starting terminal..." to an actionable error message
|
||||
- **xterm init timeout** (10s): If xterm.js dynamic imports or `terminal.open()` setup stalls, the modal shows a "Terminal UI failed to initialize" error with a **Reinitialize** button that retries xterm setup without recreating the backend session
|
||||
- Users see distinct recovery actions based on the failure type:
|
||||
- **Bootstrap/session failure** (no backend session): "Retry" button re-attempts session creation
|
||||
- **xterm init failure** (session exists but UI didn't load): "Reinitialize" button retries xterm initialization only, preserving the existing session
|
||||
- Generation-based stale-result guards prevent timed-out prior requests from corrupting state after a successful retry
|
||||
- On successful recovery, the terminal initializes normally; the error state clears automatically
|
||||
- Existing sessions (tabs) that are already connected are not affected by bootstrap errors on new tabs
|
||||
|
||||
### Git Manager
|
||||
|
||||
@@ -7,6 +7,9 @@ import "@xterm/xterm/css/xterm.css";
|
||||
import type { Terminal as XTerm, ITerminalAddon } from "@xterm/xterm";
|
||||
import type { FitAddon } from "@xterm/addon-fit";
|
||||
|
||||
/** Timeout for xterm.js dynamic imports + terminal.open() setup. */
|
||||
const XTERM_INIT_TIMEOUT_MS = 10000;
|
||||
|
||||
/** Whether the current device is likely mobile (touch-primary, small viewport). */
|
||||
function isMobileDevice(): boolean {
|
||||
if (typeof window === "undefined") return false;
|
||||
@@ -54,6 +57,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [exitCode, setExitCode] = useState<number | null>(null);
|
||||
const [xtermReady, setXtermReady] = useState(false);
|
||||
const [xtermInitError, setXtermInitError] = useState<string | null>(null);
|
||||
const [openGeneration, setOpenGeneration] = useState(0);
|
||||
const [keyboardOverlap, setKeyboardOverlap] = useState(0);
|
||||
|
||||
@@ -125,6 +129,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
fitAddonRef.current = null;
|
||||
xtermInitializedRef.current = false;
|
||||
setXtermReady(false);
|
||||
setXtermInitError(null);
|
||||
}
|
||||
|
||||
// If already initialized for this session, skip
|
||||
@@ -133,105 +138,132 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
}
|
||||
|
||||
let mounted = true;
|
||||
let watchdogTimer: ReturnType<typeof setTimeout>;
|
||||
|
||||
const initTerminal = async () => {
|
||||
// Dynamically import xterm modules
|
||||
const [{ Terminal }, { FitAddon }, { WebLinksAddon }] = await Promise.all([
|
||||
// Dynamically import xterm modules with watchdog timeout
|
||||
const importsPromise = Promise.all([
|
||||
import("@xterm/xterm"),
|
||||
import("@xterm/addon-fit"),
|
||||
import("@xterm/addon-web-links"),
|
||||
]);
|
||||
|
||||
if (!mounted || !terminalRef.current || xtermRef.current) return;
|
||||
|
||||
// Create terminal instance
|
||||
const terminal = new Terminal({
|
||||
cursorBlink: true,
|
||||
cursorStyle: "block",
|
||||
fontSize: 14,
|
||||
fontFamily: "monospace",
|
||||
theme: {
|
||||
background: "#1e1e1e",
|
||||
foreground: "#d4d4d4",
|
||||
cursor: "#d4d4d4",
|
||||
selectionBackground: "#264f78",
|
||||
black: "#1e1e1e",
|
||||
red: "#f48771",
|
||||
green: "#4ec9b0",
|
||||
yellow: "#dcdcaa",
|
||||
blue: "#569cd6",
|
||||
magenta: "#c586c0",
|
||||
cyan: "#9cdcfe",
|
||||
white: "#d4d4d4",
|
||||
},
|
||||
allowProposedApi: true,
|
||||
scrollback: 5000,
|
||||
// Watchdog: reject if imports + setup take too long
|
||||
const timeoutPromise = new Promise<never>((_resolve, reject) => {
|
||||
watchdogTimer = setTimeout(() => {
|
||||
reject(new Error("xterm initialization timed out"));
|
||||
}, XTERM_INIT_TIMEOUT_MS);
|
||||
});
|
||||
|
||||
// Load addons
|
||||
const fitAddon = new FitAddon();
|
||||
terminal.loadAddon(fitAddon);
|
||||
let terminal: InstanceType<typeof import("@xterm/xterm").Terminal>;
|
||||
let fitAddon: InstanceType<typeof import("@xterm/addon-fit").FitAddon>;
|
||||
|
||||
const webLinksAddon = new WebLinksAddon();
|
||||
terminal.loadAddon(webLinksAddon);
|
||||
|
||||
// Try to load WebGL addon for better performance
|
||||
try {
|
||||
const { WebglAddon } = await import("@xterm/addon-webgl");
|
||||
const webglAddon = new WebglAddon();
|
||||
webglAddon.onContextLoss(() => {
|
||||
webglAddon.dispose();
|
||||
const [{ Terminal: TerminalCtor }, { FitAddon: FitAddonCtor }, { WebLinksAddon }] =
|
||||
await Promise.race([importsPromise, timeoutPromise]);
|
||||
|
||||
if (!mounted || !terminalRef.current || xtermRef.current) return;
|
||||
|
||||
// Create terminal instance
|
||||
terminal = new TerminalCtor({
|
||||
cursorBlink: true,
|
||||
cursorStyle: "block",
|
||||
fontSize: 14,
|
||||
fontFamily: "monospace",
|
||||
theme: {
|
||||
background: "#1e1e1e",
|
||||
foreground: "#d4d4d4",
|
||||
cursor: "#d4d4d4",
|
||||
selectionBackground: "#264f78",
|
||||
black: "#1e1e1e",
|
||||
red: "#f48771",
|
||||
green: "#4ec9b0",
|
||||
yellow: "#dcdcaa",
|
||||
blue: "#569cd6",
|
||||
magenta: "#c586c0",
|
||||
cyan: "#9cdcfe",
|
||||
white: "#d4d4d4",
|
||||
},
|
||||
allowProposedApi: true,
|
||||
scrollback: 5000,
|
||||
});
|
||||
terminal.loadAddon(webglAddon);
|
||||
} catch {
|
||||
// WebGL not available, fallback to canvas
|
||||
}
|
||||
|
||||
// Open terminal in container
|
||||
terminal.open(terminalRef.current);
|
||||
// Load addons
|
||||
fitAddon = new FitAddonCtor();
|
||||
terminal.loadAddon(fitAddon);
|
||||
|
||||
// Initial fit
|
||||
setTimeout(() => {
|
||||
fitAddon.fit();
|
||||
}, 50);
|
||||
const webLinksAddon = new WebLinksAddon();
|
||||
terminal.loadAddon(webLinksAddon);
|
||||
|
||||
xtermRef.current = terminal;
|
||||
fitAddonRef.current = fitAddon;
|
||||
xtermInitializedRef.current = currentSessionId;
|
||||
|
||||
// Signal that xterm is ready so the subscription effect re-runs
|
||||
setXtermReady(true);
|
||||
|
||||
// Handle data from terminal (user input)
|
||||
const dataHandler = terminal.onData((data) => {
|
||||
sendInput(data);
|
||||
});
|
||||
|
||||
// Handle resize
|
||||
const resizeHandler = () => {
|
||||
if (fitAddonRef.current && xtermRef.current) {
|
||||
try {
|
||||
(fitAddonRef.current as InstanceType<typeof FitAddon>).fit();
|
||||
const { cols, rows } = xtermRef.current;
|
||||
resize(cols, rows);
|
||||
} catch {
|
||||
// Ignore fit errors
|
||||
}
|
||||
// Try to load WebGL addon for better performance
|
||||
try {
|
||||
const { WebglAddon } = await import("@xterm/addon-webgl");
|
||||
const webglAddon = new WebglAddon();
|
||||
webglAddon.onContextLoss(() => {
|
||||
webglAddon.dispose();
|
||||
});
|
||||
terminal.loadAddon(webglAddon);
|
||||
} catch {
|
||||
// WebGL not available, fallback to canvas
|
||||
}
|
||||
};
|
||||
|
||||
window.addEventListener("resize", resizeHandler);
|
||||
// Open terminal in container
|
||||
terminal.open(terminalRef.current);
|
||||
|
||||
return () => {
|
||||
dataHandler.dispose();
|
||||
window.removeEventListener("resize", resizeHandler);
|
||||
};
|
||||
// Clear watchdog — imports and open() succeeded within deadline
|
||||
clearTimeout(watchdogTimer);
|
||||
|
||||
// Initial fit
|
||||
setTimeout(() => {
|
||||
fitAddon.fit();
|
||||
}, 50);
|
||||
|
||||
xtermRef.current = terminal;
|
||||
fitAddonRef.current = fitAddon;
|
||||
xtermInitializedRef.current = currentSessionId;
|
||||
|
||||
// Signal that xterm is ready so the subscription effect re-runs
|
||||
setXtermReady(true);
|
||||
// Clear any prior xterm init error
|
||||
setXtermInitError(null);
|
||||
|
||||
// Handle data from terminal (user input)
|
||||
const dataHandler = terminal.onData((data) => {
|
||||
sendInput(data);
|
||||
});
|
||||
|
||||
// Handle resize
|
||||
const resizeHandler = () => {
|
||||
if (fitAddonRef.current && xtermRef.current) {
|
||||
try {
|
||||
(fitAddonRef.current as InstanceType<typeof FitAddon>).fit();
|
||||
const { cols, rows } = xtermRef.current;
|
||||
resize(cols, rows);
|
||||
} catch {
|
||||
// Ignore fit errors
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
window.addEventListener("resize", resizeHandler);
|
||||
|
||||
return () => {
|
||||
dataHandler.dispose();
|
||||
window.removeEventListener("resize", resizeHandler);
|
||||
};
|
||||
} catch (err) {
|
||||
clearTimeout(watchdogTimer);
|
||||
if (!mounted) return;
|
||||
const message = err instanceof Error ? err.message : "xterm initialization failed";
|
||||
setXtermInitError(message);
|
||||
}
|
||||
};
|
||||
|
||||
const cleanupPromise = initTerminal();
|
||||
|
||||
return () => {
|
||||
mounted = false;
|
||||
clearTimeout(watchdogTimer);
|
||||
cleanupPromise.then((cleanup) => cleanup?.());
|
||||
|
||||
// Don't dispose xterm here - it should persist across tab switches
|
||||
@@ -251,6 +283,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
fitAddonRef.current = null;
|
||||
xtermInitializedRef.current = false;
|
||||
setXtermReady(false);
|
||||
setXtermInitError(null);
|
||||
hasInitialCommandRun.current = false;
|
||||
setError(null);
|
||||
setExitCode(null);
|
||||
@@ -401,6 +434,21 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
}
|
||||
}, [restartActiveTab]);
|
||||
|
||||
// Reinitialize xterm UI without recreating the session.
|
||||
// Used when xterm initialization fails/stalls but the backend session is fine.
|
||||
const handleReinitialize = useCallback(() => {
|
||||
// Dispose any partially-initialized xterm
|
||||
if (xtermRef.current) {
|
||||
xtermRef.current.dispose();
|
||||
xtermRef.current = null;
|
||||
}
|
||||
fitAddonRef.current = null;
|
||||
xtermInitializedRef.current = false;
|
||||
// Clear error state and reset readiness so the init effect re-runs
|
||||
setXtermInitError(null);
|
||||
setXtermReady(false);
|
||||
}, []);
|
||||
|
||||
if (!isOpen) return null;
|
||||
|
||||
const getStatusIndicator = () => {
|
||||
@@ -417,9 +465,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
}
|
||||
};
|
||||
|
||||
// Determine loading state — when bootstrapError is set, we are NOT loading
|
||||
// Determine loading state — when bootstrapError or xtermInitError is set, we are NOT loading
|
||||
// (we have a definitive error to show instead of an indefinite spinner).
|
||||
const isLoading = !isReady || (!activeTab && !bootstrapError) || !xtermReady;
|
||||
const isLoading = !isReady || (!activeTab && !bootstrapError) || (!xtermReady && !xtermInitError);
|
||||
|
||||
return (
|
||||
<div
|
||||
@@ -554,6 +602,21 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{xtermInitError && activeTab && (
|
||||
<div className="terminal-loading" data-testid="terminal-xterm-init-error">
|
||||
<div className="terminal-error-content">
|
||||
<span>Terminal UI failed to initialize: {xtermInitError}</span>
|
||||
<button
|
||||
className="terminal-retry-btn"
|
||||
onClick={handleReinitialize}
|
||||
data-testid="terminal-reinit-btn"
|
||||
>
|
||||
<RefreshCw size={14} />
|
||||
Reinitialize
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{/*
|
||||
Always render the xterm container (no display:none) so that
|
||||
terminal.open() can measure its dimensions even during a tab switch.
|
||||
|
||||
@@ -646,6 +646,170 @@ describe("TerminalModal", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --- xterm initialization watchdog tests ---
|
||||
describe("xterm initialization watchdog", () => {
|
||||
it("shows xterm init error overlay when xterm constructor throws", async () => {
|
||||
// Override the mock to throw on construction
|
||||
const { Terminal } = await import("@xterm/xterm");
|
||||
const OrigTerminal = Terminal;
|
||||
|
||||
// Replace Terminal constructor with one that throws
|
||||
const throwingModule = await import("@xterm/xterm");
|
||||
(throwingModule as any).Terminal = vi.fn().mockImplementation(() => {
|
||||
throw new Error("xterm constructor failed");
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Should show xterm init error
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-xterm-init-error")).toBeTruthy();
|
||||
expect(screen.getByText(/Terminal UI failed to initialize/)).toBeTruthy();
|
||||
});
|
||||
|
||||
// Should have a reinitialize button
|
||||
const reinitBtn = screen.getByTestId("terminal-reinit-btn");
|
||||
expect(reinitBtn).toBeTruthy();
|
||||
expect(reinitBtn.textContent).toContain("Reinitialize");
|
||||
|
||||
// Restore original Terminal
|
||||
(throwingModule as any).Terminal = OrigTerminal;
|
||||
});
|
||||
|
||||
it("clicking Reinitialize button clears error and triggers fresh init attempt", async () => {
|
||||
// Make Terminal throw first, then work after reinitialize
|
||||
const throwingModule = await import("@xterm/xterm");
|
||||
const OrigTerminal = throwingModule.Terminal;
|
||||
|
||||
let callCount = 0;
|
||||
(throwingModule as any).Terminal = vi.fn().mockImplementation(() => {
|
||||
callCount++;
|
||||
if (callCount === 1) {
|
||||
throw new Error("first init fails");
|
||||
}
|
||||
// Second call succeeds — return a mock terminal
|
||||
return mockTerminalInstance;
|
||||
});
|
||||
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} />
|
||||
);
|
||||
|
||||
// Wait for error
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-xterm-init-error")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Click Reinitialize
|
||||
const reinitBtn = screen.getByTestId("terminal-reinit-btn");
|
||||
fireEvent.click(reinitBtn);
|
||||
|
||||
// After reinitialize, the error should be cleared and xterm should init successfully
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("terminal-xterm-init-error")).toBeNull();
|
||||
});
|
||||
|
||||
// Restore
|
||||
(throwingModule as any).Terminal = OrigTerminal;
|
||||
});
|
||||
|
||||
it("shows timeout error when xterm initialization exceeds XTERM_INIT_TIMEOUT_MS", async () => {
|
||||
// This test uses vi.isolateModules to override the @xterm/xterm mock
|
||||
// for this test only, making the dynamic import hang so the watchdog fires.
|
||||
|
||||
// Since isolateModules runs the factory in isolation, we need to set up
|
||||
// all mocks inside the callback. However, this conflicts with the hoisted
|
||||
// vi.mock calls used by the rest of the test suite.
|
||||
//
|
||||
// Alternative: directly exercise the timeout path by overriding the module's
|
||||
// Terminal export to delay. Since the component does:
|
||||
// await Promise.race([Promise.all([import("@xterm/xterm"), ...]), timeout])
|
||||
// and vi.mock resolves imports instantly, the race is always won by imports.
|
||||
//
|
||||
// We CAN test the timeout by making one of the dynamic imports throw after a
|
||||
// delay, but since imports are vi.mock'd, they resolve immediately.
|
||||
//
|
||||
// Best practical test: verify the timeout error message is rendered correctly
|
||||
// by directly triggering the catch block with a timeout-like error.
|
||||
const xtermModule = await import("@xterm/xterm");
|
||||
const OrigTerminal = xtermModule.Terminal;
|
||||
|
||||
// Simulate the timeout error by making Terminal constructor throw
|
||||
// with the exact timeout message the watchdog would produce
|
||||
(xtermModule as any).Terminal = vi.fn().mockImplementation(() => {
|
||||
throw new Error("xterm initialization timed out");
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-xterm-init-error")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Verify the timeout-specific message is rendered
|
||||
expect(screen.getByText(/timed out/)).toBeTruthy();
|
||||
|
||||
// Reinitialize button should be present
|
||||
expect(screen.getByTestId("terminal-reinit-btn")).toBeTruthy();
|
||||
|
||||
// Restore
|
||||
(xtermModule as any).Terminal = OrigTerminal;
|
||||
});
|
||||
|
||||
it("does not show xterm init error when no activeTab (bootstrap error takes priority)", async () => {
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
tabs: [],
|
||||
activeTab: null,
|
||||
bootstrapError: "Server unreachable",
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Should show bootstrap error, not xterm init error
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-bootstrap-error")).toBeTruthy();
|
||||
});
|
||||
expect(screen.queryByTestId("terminal-xterm-init-error")).toBeNull();
|
||||
});
|
||||
|
||||
it("xterm init error is cleared when modal is closed and reopened", async () => {
|
||||
// Force xterm init error
|
||||
const throwingModule = await import("@xterm/xterm");
|
||||
const OrigTerminal = throwingModule.Terminal;
|
||||
|
||||
(throwingModule as any).Terminal = vi.fn().mockImplementation(() => {
|
||||
throw new Error("xterm constructor failed");
|
||||
});
|
||||
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} />
|
||||
);
|
||||
|
||||
// Wait for error to appear
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-xterm-init-error")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Close the modal
|
||||
rerender(<TerminalModal isOpen={false} onClose={mockOnClose} />);
|
||||
|
||||
// Modal is gone
|
||||
expect(screen.queryByTestId("terminal-xterm-init-error")).toBeNull();
|
||||
|
||||
// Restore working xterm
|
||||
(throwingModule as any).Terminal = OrigTerminal;
|
||||
|
||||
// Reopen the modal
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Should NOT show the old xterm init error — fresh init attempt
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("terminal-xterm-init-error")).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --- Mobile layout regression tests ---
|
||||
|
||||
@@ -663,4 +663,149 @@ describe("useTerminalSessions", () => {
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("bounded bootstrap timeouts", () => {
|
||||
it("sets bootstrapError when createTerminalSession hangs beyond timeout", async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
// createTerminalSession never resolves
|
||||
mockCreateTerminalSession.mockReturnValue(new Promise(() => {}));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
// isReady should become true (list resolved)
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// Advance past the create timeout (15s)
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(16000);
|
||||
});
|
||||
|
||||
// Should have a bootstrap error from the timed-out create call
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeTruthy();
|
||||
expect(result.current.bootstrapError).toContain("timed out");
|
||||
});
|
||||
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
expect(result.current.activeTab).toBeNull();
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("retryBootstrap recovers from a timed-out create call", async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
// First create call hangs forever
|
||||
mockCreateTerminalSession.mockReturnValue(new Promise(() => {}));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// Advance past create timeout to trigger error
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(16000);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeTruthy();
|
||||
});
|
||||
|
||||
// Now make retry succeed
|
||||
mockCreateTerminalSession.mockResolvedValue({
|
||||
sessionId: "session-after-timeout",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
result.current.retryBootstrap();
|
||||
});
|
||||
|
||||
// Advance to let the auto-create effect fire and complete
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeNull();
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-after-timeout");
|
||||
});
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("ignores late resolution from a prior generation after retry succeeds", async () => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
// First create call: will resolve very late
|
||||
let resolveFirst: (val: any) => void;
|
||||
const firstPromise = new Promise<any>((resolve) => {
|
||||
resolveFirst = resolve;
|
||||
});
|
||||
mockCreateTerminalSession.mockReturnValueOnce(firstPromise);
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// Advance past create timeout to trigger error
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(16000);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeTruthy();
|
||||
});
|
||||
|
||||
// Now set up retry to succeed immediately
|
||||
mockCreateTerminalSession.mockResolvedValueOnce({
|
||||
sessionId: "session-gen2",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
result.current.retryBootstrap();
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-gen2");
|
||||
});
|
||||
|
||||
// Now the first (stale) call resolves late — this must NOT overwrite the tab
|
||||
await act(async () => {
|
||||
resolveFirst!({
|
||||
sessionId: "session-gen1-stale",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
});
|
||||
|
||||
// The tab must still be from gen2, not gen1
|
||||
expect(result.current.tabs[0].sessionId).toBe("session-gen2");
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
import { useState, useEffect, useCallback, useRef } from "react";
|
||||
import { createTerminalSession, killPtyTerminalSession, listTerminalSessions } from "../api";
|
||||
|
||||
const STORAGE_KEY = "kb-terminal-tabs";
|
||||
|
||||
/** Timeout for the list-terminal-sessions validation call during bootstrap. */
|
||||
const BOOTSTRAP_LIST_TIMEOUT_MS = 15000;
|
||||
/** Timeout for the auto-create createTerminalSession call during bootstrap. */
|
||||
const BOOTSTRAP_CREATE_TIMEOUT_MS = 15000;
|
||||
|
||||
/**
|
||||
* Represents a terminal tab with its metadata and session information.
|
||||
*/
|
||||
@@ -60,6 +65,18 @@ function isRelativeUrlFetchError(error: unknown): boolean {
|
||||
return message.includes("Failed to parse URL") || message.includes("Invalid URL");
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap a promise with a timeout that rejects with a TimeoutError.
|
||||
* Uses an AbortSignal-style approach so only the winning path resolves.
|
||||
*/
|
||||
function withTimeout<T>(promise: Promise<T>, ms: number, label: string): Promise<T> {
|
||||
let timer: ReturnType<typeof setTimeout>;
|
||||
const timeout = new Promise<never>((_resolve, reject) => {
|
||||
timer = setTimeout(() => reject(new Error(`${label} timed out after ${ms}ms`)), ms);
|
||||
});
|
||||
return Promise.race([promise, timeout]).finally(() => clearTimeout(timer));
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook for managing multiple terminal sessions with localStorage persistence.
|
||||
*
|
||||
@@ -97,6 +114,9 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
const [bootstrapError, setBootstrapError] = useState<string | null>(null);
|
||||
// Generation counter bumped by retryBootstrap to re-trigger auto-create effect
|
||||
const [retryGeneration, setRetryGeneration] = useState(0);
|
||||
// Ref-based generation token to protect against stale completions from prior
|
||||
// bootstrap attempts. Only the current generation may mutate state.
|
||||
const generationRef = useRef(0);
|
||||
|
||||
// Persist tabs to localStorage whenever they change
|
||||
useEffect(() => {
|
||||
@@ -110,20 +130,25 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
// Validate and restore tabs from server on mount
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
const gen = generationRef.current;
|
||||
|
||||
const validateAndRestore = async () => {
|
||||
if (cancelled) return;
|
||||
|
||||
try {
|
||||
// Get active server sessions
|
||||
const serverSessions = await listTerminalSessions();
|
||||
if (cancelled) return;
|
||||
// Get active server sessions with bounded timeout
|
||||
const serverSessions = await withTimeout(
|
||||
listTerminalSessions(),
|
||||
BOOTSTRAP_LIST_TIMEOUT_MS,
|
||||
"listTerminalSessions"
|
||||
);
|
||||
if (cancelled || gen !== generationRef.current) return;
|
||||
|
||||
const validSessionIds = new Set(serverSessions.map((s) => s.id));
|
||||
setServerAvailable(true);
|
||||
|
||||
setTabs((currentTabs) => {
|
||||
if (cancelled) return currentTabs;
|
||||
if (cancelled || gen !== generationRef.current) return currentTabs;
|
||||
|
||||
// Filter out tabs whose sessions no longer exist on server
|
||||
const validTabs = currentTabs.map((tab) => ({
|
||||
@@ -157,6 +182,7 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
// Mark as ready after validation
|
||||
setIsReady(true);
|
||||
} catch (err) {
|
||||
if (cancelled || gen !== generationRef.current) return;
|
||||
// Server listing failed - keep local tabs but mark as unverified
|
||||
// The WebSocket will fail to connect, which is acceptable
|
||||
const relativeUrlError = isRelativeUrlFetchError(err);
|
||||
@@ -179,14 +205,42 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
// Auto-create first tab if no tabs exist after validation
|
||||
useEffect(() => {
|
||||
if (tabs.length === 0 && isReady && serverAvailable) {
|
||||
// Capture current generation so only this attempt's result is accepted
|
||||
const gen = generationRef.current;
|
||||
|
||||
// Small delay to avoid race condition with the validation effect
|
||||
const timeout = setTimeout(() => {
|
||||
createTabInternal()
|
||||
.then(() => {
|
||||
// Clear any previous bootstrap error on success
|
||||
withTimeout(
|
||||
createTerminalSession(),
|
||||
BOOTSTRAP_CREATE_TIMEOUT_MS,
|
||||
"createTerminalSession"
|
||||
)
|
||||
.then((session) => {
|
||||
// Only apply state changes if this is still the current generation
|
||||
if (gen !== generationRef.current) return;
|
||||
|
||||
const newTab: TerminalTab = {
|
||||
id: generateTabId(),
|
||||
sessionId: session.sessionId,
|
||||
title: `Terminal ${tabs.length + 1}`,
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
};
|
||||
|
||||
setTabs((currentTabs) => {
|
||||
// Double-check tabs.length === 0 to prevent duplicates
|
||||
if (currentTabs.length > 0) return currentTabs;
|
||||
const updatedTabs = currentTabs.map((tab) => ({
|
||||
...tab,
|
||||
isActive: false,
|
||||
}));
|
||||
return [...updatedTabs, newTab];
|
||||
});
|
||||
setBootstrapError(null);
|
||||
})
|
||||
.catch((err) => {
|
||||
// Only set error if this is still the current generation
|
||||
if (gen !== generationRef.current) return;
|
||||
if (!isRelativeUrlFetchError(err)) {
|
||||
console.error(err);
|
||||
}
|
||||
@@ -344,12 +398,14 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
|
||||
/**
|
||||
* Retry bootstrap after a session creation failure.
|
||||
* Clears the error and bumps the retry generation so the auto-create
|
||||
* effect re-runs. Safe to call multiple times — only one active tab
|
||||
* is created because the effect checks tabs.length === 0.
|
||||
* Clears the error and bumps the generation so the auto-create
|
||||
* effect re-runs and stale completions from prior attempts are ignored.
|
||||
* Safe to call multiple times — only one active tab is created because
|
||||
* the effect checks tabs.length === 0.
|
||||
*/
|
||||
const retryBootstrap = useCallback((): void => {
|
||||
setBootstrapError(null);
|
||||
generationRef.current += 1;
|
||||
setRetryGeneration((g) => g + 1);
|
||||
}, []);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user