feat(FN-1021): stabilize terminal first-open bootstrap and WebSocket recovery
- Harden TerminalModal bootstrap to handle invalid/expired sessions on first open - Add WebSocket reconnection logic for terminal sessions that become invalid mid-stream - Create useTerminal hook with robust session lifecycle management - Create useTerminalSessions hook for multi-session terminal coordination - Add comprehensive tests for TerminalModal, useTerminal, and useTerminalSessions - Document terminal first-open reliability behavior in dashboard README
This commit is contained in:
@@ -427,6 +427,123 @@ describe("useTerminalSessions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("replacing active tab session (invalid session recovery)", () => {
|
||||
it("swaps sessionId on active tab without killing old session", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
mockCreateTerminalSession
|
||||
.mockResolvedValueOnce({ sessionId: "session-1", shell: "/bin/bash", cwd: "/project" })
|
||||
.mockResolvedValueOnce({ sessionId: "session-replacement", shell: "/bin/bash", cwd: "/project" });
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-1");
|
||||
});
|
||||
|
||||
const tabId = result.current.activeTab!.id;
|
||||
|
||||
await act(async () => {
|
||||
await result.current.replaceActiveTabSession();
|
||||
});
|
||||
|
||||
// Should NOT kill the old session (it's already gone from server)
|
||||
expect(mockKillPtyTerminalSession).not.toHaveBeenCalled();
|
||||
|
||||
// Tab should still exist with same ID but new sessionId
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
expect(result.current.activeTab?.id).toBe(tabId);
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-replacement");
|
||||
});
|
||||
|
||||
it("sets bootstrapError when replacement session creation fails", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
mockCreateTerminalSession
|
||||
.mockResolvedValueOnce({ sessionId: "session-1", shell: "/bin/bash", cwd: "/project" })
|
||||
.mockRejectedValueOnce(new Error("Server unreachable"));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.replaceActiveTabSession();
|
||||
});
|
||||
|
||||
// Error should be set so UI can show retry
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("Server unreachable");
|
||||
});
|
||||
|
||||
// Tab should still exist with the old sessionId
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-1");
|
||||
});
|
||||
|
||||
it("does nothing when no active tab exists", async () => {
|
||||
// Set up a scenario where tabs are empty and isReady is true but no auto-create happened yet
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
// Make auto-create hang so no tab is created
|
||||
mockCreateTerminalSession.mockReturnValue(new Promise(() => {}));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
// Wait for isReady to be true (list completes) but tabs are empty (create pending)
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// replaceActiveTabSession should be a no-op with no active tab
|
||||
await act(async () => {
|
||||
await result.current.replaceActiveTabSession();
|
||||
});
|
||||
|
||||
// No additional createTerminalSession calls beyond the pending auto-create
|
||||
expect(mockCreateTerminalSession).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("clears bootstrapError on successful replacement after failure", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
mockCreateTerminalSession
|
||||
.mockResolvedValueOnce({ sessionId: "session-1", shell: "/bin/bash", cwd: "/project" })
|
||||
.mockRejectedValueOnce(new Error("Temporary failure"))
|
||||
.mockResolvedValueOnce({ sessionId: "session-recovered", shell: "/bin/bash", cwd: "/project" });
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
});
|
||||
|
||||
// First replacement fails
|
||||
await act(async () => {
|
||||
await result.current.replaceActiveTabSession();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("Temporary failure");
|
||||
});
|
||||
|
||||
// Second replacement succeeds
|
||||
await act(async () => {
|
||||
await result.current.replaceActiveTabSession();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeNull();
|
||||
});
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-recovered");
|
||||
});
|
||||
});
|
||||
|
||||
describe("localStorage persistence", () => {
|
||||
it("persists tabs to localStorage", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
|
||||
@@ -186,6 +186,90 @@ describe("useTerminal", () => {
|
||||
expect(result.current.connectionStatus).toBe("disconnected");
|
||||
});
|
||||
|
||||
describe("onSessionInvalid callback", () => {
|
||||
it("fires onSessionInvalid callbacks when WebSocket closes with code 4004", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
const onSessionInvalid = vi.fn();
|
||||
|
||||
act(() => {
|
||||
result.current.onSessionInvalid(onSessionInvalid);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(4004);
|
||||
});
|
||||
|
||||
expect(onSessionInvalid).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does NOT fire onSessionInvalid for close code 4000", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
const onSessionInvalid = vi.fn();
|
||||
|
||||
act(() => {
|
||||
result.current.onSessionInvalid(onSessionInvalid);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(4000);
|
||||
});
|
||||
|
||||
expect(onSessionInvalid).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT fire onSessionInvalid for normal close codes", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
const onSessionInvalid = vi.fn();
|
||||
|
||||
act(() => {
|
||||
result.current.onSessionInvalid(onSessionInvalid);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(1000);
|
||||
});
|
||||
|
||||
expect(onSessionInvalid).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("unsubscribes correctly", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
const onSessionInvalid = vi.fn();
|
||||
|
||||
const unsub = result.current.onSessionInvalid(onSessionInvalid);
|
||||
|
||||
// Unsubscribe
|
||||
act(() => {
|
||||
unsub();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(4004);
|
||||
});
|
||||
|
||||
// Should NOT have been called after unsubscribe
|
||||
expect(onSessionInvalid).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fires for multiple subscribers", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
const cb1 = vi.fn();
|
||||
const cb2 = vi.fn();
|
||||
|
||||
act(() => {
|
||||
result.current.onSessionInvalid(cb1);
|
||||
result.current.onSessionInvalid(cb2);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(4004);
|
||||
});
|
||||
|
||||
expect(cb1).toHaveBeenCalledTimes(1);
|
||||
expect(cb2).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("early message buffering", () => {
|
||||
it("replays buffered scrollback to late subscribers", () => {
|
||||
const { result } = renderHook(() => useTerminal("test-session-123"));
|
||||
|
||||
@@ -19,6 +19,13 @@ export interface UseTerminalReturn {
|
||||
onScrollback: (callback: (data: string) => void) => () => void;
|
||||
/** Manually reconnect */
|
||||
reconnect: () => void;
|
||||
/**
|
||||
* Register a callback for session-invalid events.
|
||||
* Fires when the WebSocket closes with code 4004 (session-not-found),
|
||||
* meaning the server no longer recognizes the session. The caller should
|
||||
* create a new session rather than attempting reconnect.
|
||||
*/
|
||||
onSessionInvalid: (callback: () => void) => () => void;
|
||||
}
|
||||
|
||||
interface WebSocketMessage {
|
||||
@@ -86,6 +93,7 @@ export function useTerminal(sessionId: string | null): UseTerminalReturn {
|
||||
const onExitCallbacksRef = useRef<Set<(exitCode: number) => void>>(new Set());
|
||||
const onConnectCallbacksRef = useRef<Set<(info: { shell: string; cwd: string }) => void>>(new Set());
|
||||
const onScrollbackCallbacksRef = useRef<Set<(data: string) => void>>(new Set());
|
||||
const onSessionInvalidCallbacksRef = useRef<Set<() => void>>(new Set());
|
||||
|
||||
// Buffer for initial messages received before subscribers are registered.
|
||||
// This ensures scrollback, connected info, and early shell output are
|
||||
@@ -135,6 +143,17 @@ export function useTerminal(sessionId: string | null): UseTerminalReturn {
|
||||
return () => onScrollbackCallbacksRef.current.delete(callback);
|
||||
}, []);
|
||||
|
||||
/**
|
||||
* Register a callback for session-invalid events.
|
||||
* Fires when the server closes the WebSocket with code 4004, indicating
|
||||
* the session no longer exists. Unlike transient disconnects, this is a
|
||||
* permanent condition that requires creating a new session to recover.
|
||||
*/
|
||||
const onSessionInvalid = useCallback((callback: () => void) => {
|
||||
onSessionInvalidCallbacksRef.current.add(callback);
|
||||
return () => onSessionInvalidCallbacksRef.current.delete(callback);
|
||||
}, []);
|
||||
|
||||
// Send input to terminal
|
||||
const sendInput = useCallback((data: string) => {
|
||||
const ws = wsRef.current;
|
||||
@@ -288,6 +307,13 @@ export function useTerminal(sessionId: string | null): UseTerminalReturn {
|
||||
// Don't reconnect for certain close codes
|
||||
if (event.code === 4000 || event.code === 4004) {
|
||||
setConnectionStatus("disconnected");
|
||||
|
||||
// Code 4004 means the server doesn't recognize the session — it's
|
||||
// permanently invalid. Notify subscribers so they can create a new
|
||||
// session rather than retrying the stale one.
|
||||
if (event.code === 4004) {
|
||||
onSessionInvalidCallbacksRef.current.forEach((cb) => cb());
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -342,5 +368,6 @@ export function useTerminal(sessionId: string | null): UseTerminalReturn {
|
||||
onConnect,
|
||||
onScrollback,
|
||||
reconnect,
|
||||
onSessionInvalid,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -50,6 +50,14 @@ interface UseTerminalSessionsReturn {
|
||||
restartActiveTab: () => Promise<void>;
|
||||
/** Retry bootstrap after a creation failure. Clears error and re-attempts auto-create. */
|
||||
retryBootstrap: () => void;
|
||||
/**
|
||||
* Replace the active tab's session with a fresh server session.
|
||||
* Called when the WebSocket reports the current session is invalid (code 4004).
|
||||
* Unlike restartActiveTab, this does NOT kill the old session (it's already
|
||||
* gone from the server) and does NOT reset xterm state — it only swaps the
|
||||
* sessionId so the next WebSocket connect targets the new session.
|
||||
*/
|
||||
replaceActiveTabSession: () => Promise<void>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -393,6 +401,46 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
);
|
||||
}, [tabs]);
|
||||
|
||||
/**
|
||||
* Replace the active tab's session with a fresh server session.
|
||||
* Called when the WebSocket reports the current session is invalid (code 4004).
|
||||
*
|
||||
* Unlike restartActiveTab:
|
||||
* - Does NOT kill the old session (it's already gone from the server).
|
||||
* - Does NOT clear xterm or reset exit state — TerminalModal handles that.
|
||||
* - Only swaps the sessionId so useTerminal reconnects to the new session.
|
||||
*
|
||||
* If session creation fails, the bootstrap error is set so the user can
|
||||
* retry via the error UI.
|
||||
*/
|
||||
const replaceActiveTabSession = useCallback(async (): Promise<void> => {
|
||||
// Read the active tab directly from the derived value.
|
||||
// Use a local snapshot since the async createTerminalSession may
|
||||
// cause re-renders that change tabs state.
|
||||
const currentActiveTab = tabs.find((t) => t.isActive);
|
||||
if (!currentActiveTab) return;
|
||||
|
||||
try {
|
||||
const session = await createTerminalSession();
|
||||
|
||||
setTabs((currentTabs) =>
|
||||
currentTabs.map((tab) =>
|
||||
tab.id === currentActiveTab.id
|
||||
? { ...tab, sessionId: session.sessionId }
|
||||
: tab
|
||||
)
|
||||
);
|
||||
setBootstrapError(null);
|
||||
} catch (err) {
|
||||
if (!isRelativeUrlFetchError(err)) {
|
||||
console.error(err);
|
||||
}
|
||||
const message =
|
||||
err instanceof Error ? err.message : typeof err === "string" ? err : "Failed to create terminal session";
|
||||
setBootstrapError(message);
|
||||
}
|
||||
}, [tabs]);
|
||||
|
||||
// Derive active tab
|
||||
const activeTab = tabs.find((tab) => tab.isActive) ?? null;
|
||||
|
||||
@@ -420,5 +468,6 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
updateTabTitle,
|
||||
restartActiveTab,
|
||||
retryBootstrap,
|
||||
replaceActiveTabSession,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user