feat(FN-1765): merge fusion/fn-1765
This commit is contained in:
@@ -161,6 +161,8 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
const keyboardOverlapRef = useRef(0);
|
||||
/** Tracks a pending requestAnimationFrame for deferred xterm re-fit. */
|
||||
const pendingFitRef = useRef<number | null>(null);
|
||||
/** Tracks the previous projectId to detect project switches and invalidate xterm. */
|
||||
const previousProjectIdRef = useRef<string | undefined>(projectId);
|
||||
|
||||
// Keep the latest keyboard overlap in a ref so async xterm setup can read
|
||||
// current mobile keyboard state without forcing the init effect to re-run.
|
||||
@@ -308,13 +310,20 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
|
||||
const currentSessionId = activeTab.sessionId;
|
||||
|
||||
// If already initialized for this session, skip
|
||||
if (xtermInitializedRef.current === currentSessionId && xtermRef.current) {
|
||||
// Detect project switch: if projectId changed, invalidate xterm even if sessionId is the same.
|
||||
// This ensures xterm content from the previous project is not displayed in the new project.
|
||||
const projectChanged = previousProjectIdRef.current !== projectId;
|
||||
if (projectChanged) {
|
||||
previousProjectIdRef.current = projectId;
|
||||
}
|
||||
|
||||
// If already initialized for this session AND project hasn't changed, skip
|
||||
if (xtermInitializedRef.current === currentSessionId && xtermRef.current && !projectChanged) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Clean up existing xterm if switching sessions or if DOM was cleared
|
||||
if (xtermRef.current && xtermInitializedRef.current !== currentSessionId) {
|
||||
// Clean up existing xterm if switching sessions/projects or if DOM was cleared
|
||||
if (xtermRef.current && (xtermInitializedRef.current !== currentSessionId || projectChanged)) {
|
||||
xtermRef.current.dispose();
|
||||
xtermRef.current = null;
|
||||
fitAddonRef.current = null;
|
||||
|
||||
@@ -3603,3 +3603,251 @@ describe("TerminalModal — xterm focus initialization (FN-1602)", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --- FN-1765: Project-context propagation ---
|
||||
describe("TerminalModal — project-context propagation (FN-1765)", () => {
|
||||
const mockOnClose = vi.fn();
|
||||
const mockSendInput = vi.fn();
|
||||
const mockResize = vi.fn();
|
||||
const mockReconnect = vi.fn();
|
||||
|
||||
const createMockTerminalState = (overrides = {}) => ({
|
||||
connectionStatus: "connected" as const,
|
||||
sendInput: mockSendInput,
|
||||
resize: mockResize,
|
||||
onData: vi.fn(() => vi.fn()),
|
||||
onExit: vi.fn(() => vi.fn()),
|
||||
onConnect: vi.fn(() => vi.fn()),
|
||||
onScrollback: vi.fn(() => vi.fn()),
|
||||
reconnect: mockReconnect,
|
||||
onSessionInvalid: vi.fn(() => vi.fn()),
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const defaultTab = {
|
||||
id: "tab-1",
|
||||
sessionId: "session-1",
|
||||
title: "bash",
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockTerminalInstance.open.mockClear();
|
||||
mockTerminalInstance.dispose.mockClear();
|
||||
mockTerminalInstance.clear.mockClear();
|
||||
mockUseTerminal.mockReturnValue(createMockTerminalState());
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
tabs: [defaultTab],
|
||||
activeTab: defaultTab,
|
||||
isReady: true,
|
||||
bootstrapError: null,
|
||||
createTab: vi.fn(),
|
||||
closeTab: vi.fn(),
|
||||
setActiveTab: vi.fn(),
|
||||
updateTabTitle: vi.fn(),
|
||||
restartActiveTab: vi.fn(),
|
||||
retryBootstrap: vi.fn(),
|
||||
replaceActiveTabSession: vi.fn(),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("passes projectId to useTerminal hook", async () => {
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-123" />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith("session-1", "proj-123");
|
||||
});
|
||||
});
|
||||
|
||||
it("passes undefined projectId to useTerminal when not provided", async () => {
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith("session-1", undefined);
|
||||
});
|
||||
});
|
||||
|
||||
it("re-invokes useTerminal with new projectId when projectId prop changes", async () => {
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-A" />
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith("session-1", "proj-A");
|
||||
});
|
||||
|
||||
// Simulate project switch
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-B" />);
|
||||
|
||||
await waitFor(() => {
|
||||
// useTerminal should be called with the new projectId
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith(expect.any(String), "proj-B");
|
||||
});
|
||||
});
|
||||
|
||||
it("disposes xterm when projectId changes", async () => {
|
||||
// Project A has session-1
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
tabs: [defaultTab],
|
||||
activeTab: defaultTab,
|
||||
isReady: true,
|
||||
bootstrapError: null,
|
||||
createTab: vi.fn(),
|
||||
closeTab: vi.fn(),
|
||||
setActiveTab: vi.fn(),
|
||||
updateTabTitle: vi.fn(),
|
||||
restartActiveTab: vi.fn(),
|
||||
retryBootstrap: vi.fn(),
|
||||
replaceActiveTabSession: vi.fn(),
|
||||
});
|
||||
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-A" />
|
||||
);
|
||||
|
||||
// Wait for initial xterm to be created
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.open).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Clear mock to track disposal separately
|
||||
mockTerminalInstance.dispose.mockClear();
|
||||
|
||||
// Project B has a different session (simulating project-scoped sessions)
|
||||
const projBSession = {
|
||||
id: "tab-1",
|
||||
sessionId: "session-2",
|
||||
title: "zsh",
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
};
|
||||
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
tabs: [projBSession],
|
||||
activeTab: projBSession,
|
||||
isReady: true,
|
||||
bootstrapError: null,
|
||||
createTab: vi.fn(),
|
||||
closeTab: vi.fn(),
|
||||
setActiveTab: vi.fn(),
|
||||
updateTabTitle: vi.fn(),
|
||||
restartActiveTab: vi.fn(),
|
||||
retryBootstrap: vi.fn(),
|
||||
replaceActiveTabSession: vi.fn(),
|
||||
});
|
||||
|
||||
// Switch project
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-B" />);
|
||||
|
||||
// xterm should be disposed when project changes (different session triggers cleanup)
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.dispose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// New xterm should be initialized for the new project's session
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.open).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("uses fresh useTerminal session for new project after project switch", async () => {
|
||||
// Initial project A
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-A" />
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith("session-1", "proj-A");
|
||||
});
|
||||
|
||||
// Simulate project B having different sessions
|
||||
const projBTab = {
|
||||
id: "tab-1",
|
||||
sessionId: "session-2", // Different session for project B
|
||||
title: "zsh",
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
};
|
||||
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
tabs: [projBTab],
|
||||
activeTab: projBTab,
|
||||
isReady: true,
|
||||
bootstrapError: null,
|
||||
createTab: vi.fn(),
|
||||
closeTab: vi.fn(),
|
||||
setActiveTab: vi.fn(),
|
||||
updateTabTitle: vi.fn(),
|
||||
restartActiveTab: vi.fn(),
|
||||
retryBootstrap: vi.fn(),
|
||||
replaceActiveTabSession: vi.fn(),
|
||||
});
|
||||
|
||||
// Switch to project B
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-B" />);
|
||||
|
||||
// useTerminal should be called with the new project's session
|
||||
await waitFor(() => {
|
||||
expect(mockUseTerminal).toHaveBeenCalledWith("session-2", "proj-B");
|
||||
});
|
||||
});
|
||||
|
||||
it("does not dispose xterm when projectId stays the same but session changes", async () => {
|
||||
const { rerender } = render(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-A" />
|
||||
);
|
||||
|
||||
// Wait for initial xterm to be created
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.open).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Clear mock to track disposal
|
||||
mockTerminalInstance.dispose.mockClear();
|
||||
|
||||
// Create a new tab (session change, but same project)
|
||||
const newTab = {
|
||||
id: "tab-2",
|
||||
sessionId: "session-2",
|
||||
title: "zsh",
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
};
|
||||
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
tabs: [
|
||||
{ ...defaultTab, isActive: false },
|
||||
newTab,
|
||||
],
|
||||
activeTab: newTab,
|
||||
isReady: true,
|
||||
bootstrapError: null,
|
||||
createTab: vi.fn(),
|
||||
closeTab: vi.fn(),
|
||||
setActiveTab: vi.fn(),
|
||||
updateTabTitle: vi.fn(),
|
||||
restartActiveTab: vi.fn(),
|
||||
retryBootstrap: vi.fn(),
|
||||
replaceActiveTabSession: vi.fn(),
|
||||
});
|
||||
|
||||
// Switch tab (not project)
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} projectId="proj-A" />);
|
||||
|
||||
// xterm should be disposed for tab switch
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.dispose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// New xterm should be initialized for the new session
|
||||
await waitFor(() => {
|
||||
expect(mockTerminalInstance.open).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -687,4 +687,301 @@ describe("useTerminal", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
});
|
||||
|
||||
describe("stale-context isolation", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("closes old WebSocket when projectId changes", () => {
|
||||
const { rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
const oldWs = MockWebSocket.instances[0];
|
||||
const oldWsCloseSpy = vi.spyOn(oldWs, "close");
|
||||
|
||||
// Change projectId
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
// Old WebSocket should be closed
|
||||
expect(oldWsCloseSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stale WebSocket onopen does not update status when context changed", () => {
|
||||
const { rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
const oldWs = MockWebSocket.instances[0];
|
||||
|
||||
// Change projectId - this closes old ws
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
// Simulate the OLD WebSocket opening (stale event)
|
||||
act(() => {
|
||||
oldWs.emitOpen();
|
||||
});
|
||||
|
||||
// The stale ws onopen should not affect the new context's status
|
||||
// We need to verify that the new WebSocket (proj-B) is the one that matters
|
||||
expect(MockWebSocket.instances).toHaveLength(2);
|
||||
const newWs = MockWebSocket.instances[1];
|
||||
|
||||
// The new ws should be connecting, not connected (since we haven't opened it yet)
|
||||
// Wait for the new ws to be created
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(0);
|
||||
});
|
||||
});
|
||||
|
||||
it("stale WebSocket onmessage does not call callbacks when context changed", () => {
|
||||
const { rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Register callback BEFORE context change
|
||||
const onData = vi.fn();
|
||||
const { result } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) => {
|
||||
const hook = useTerminal(sessionId, projectId);
|
||||
return hook;
|
||||
},
|
||||
{
|
||||
initialProps: { sessionId: "test-session-123", projectId: "proj-A" },
|
||||
wrapper: ({ children }) => children,
|
||||
},
|
||||
);
|
||||
|
||||
// Use a simpler approach: test within a single hook instance
|
||||
const { result: singleResult } = renderHook(() => {
|
||||
const hook = useTerminal("test-session-123", "proj-A");
|
||||
return hook;
|
||||
});
|
||||
|
||||
const dataCallback = vi.fn();
|
||||
singleResult.current.onData(dataCallback);
|
||||
|
||||
// Change projectId - this closes old ws and increments context version
|
||||
singleResult.current; // access to ensure re-render
|
||||
const { rerender: rerenderSingle } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Change projectId
|
||||
rerenderSingle({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
const oldWs = MockWebSocket.instances[0];
|
||||
|
||||
// Simulate the OLD WebSocket receiving a message (stale event)
|
||||
act(() => {
|
||||
oldWs.emitMessage({ type: "data", data: "stale data" });
|
||||
});
|
||||
|
||||
// The stale message should NOT call the callback
|
||||
expect(dataCallback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stale WebSocket reconnect timeout is ignored when context changed", () => {
|
||||
const { rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Open and then close the WebSocket to trigger reconnect timeout
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitOpen();
|
||||
MockWebSocket.instances[0].emitClose(1006); // Unexpected close triggers reconnect
|
||||
});
|
||||
|
||||
// Wait for reconnect to be scheduled
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
|
||||
// Now change projectId before reconnect fires
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
// Advance past when reconnect would have fired
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(2000);
|
||||
});
|
||||
|
||||
// At this point:
|
||||
// - ws1 was created (original)
|
||||
// - ws1 close triggered reconnect
|
||||
// - reconnect timeout fired and created ws2 (for stale context A)
|
||||
// - context changed to B
|
||||
// - closeWebSocketForContextChange closed ws1 and ws2
|
||||
// - connect() created ws3 (for new context B)
|
||||
// The stale ws2 reconnect should NOT have created another instance
|
||||
expect(MockWebSocket.instances).toHaveLength(3);
|
||||
|
||||
// The final ws should be for project B
|
||||
expect(MockWebSocket.instances[2].url).toContain("projectId=proj-B");
|
||||
});
|
||||
|
||||
it("resets connection status on context change", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Connect
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitOpen();
|
||||
});
|
||||
|
||||
expect(result.current.connectionStatus).toBe("connected");
|
||||
|
||||
// Change projectId
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
// Status should be reset to disconnected/connecting
|
||||
// (disconnected initially, then connecting for the new context)
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(0);
|
||||
});
|
||||
|
||||
expect(result.current.connectionStatus).toBe("connecting");
|
||||
});
|
||||
|
||||
it("only reconnects to active context (new projectId)", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Open the first WebSocket
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitOpen();
|
||||
});
|
||||
|
||||
// Close to trigger reconnect
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitClose(1006);
|
||||
});
|
||||
|
||||
// Wait for reconnect timeout to fire
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(1000);
|
||||
});
|
||||
|
||||
// At this point:
|
||||
// - ws1 was closed
|
||||
// - onclose scheduled reconnect
|
||||
// - reconnect timeout fired and called connect()
|
||||
// - connect() closed ws1 and created ws2 (proj-A)
|
||||
|
||||
// Change projectId - this closes ws2 and creates ws3 (proj-B)
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
|
||||
// Wait for effects
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(0);
|
||||
});
|
||||
|
||||
// Final ws should be for project B
|
||||
const finalWs = MockWebSocket.instances[MockWebSocket.instances.length - 1];
|
||||
expect(finalWs.url).toContain("projectId=proj-B");
|
||||
|
||||
// Open the final ws
|
||||
act(() => {
|
||||
finalWs.emitOpen();
|
||||
});
|
||||
|
||||
// Status should be connected
|
||||
expect(result.current.connectionStatus).toBe("connected");
|
||||
});
|
||||
|
||||
it("clears buffer on context change", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ projectId }: { projectId?: string }) => useTerminal("test-session-123", projectId),
|
||||
{ initialProps: { projectId: "proj-A" as string | undefined } },
|
||||
);
|
||||
|
||||
// Send some data before context change
|
||||
act(() => {
|
||||
MockWebSocket.instances[0].emitMessage({ type: "scrollback", data: "buffered data" });
|
||||
});
|
||||
|
||||
// Register a callback that would receive the buffer
|
||||
const scrollbackCallback = vi.fn();
|
||||
result.current.onScrollback(scrollbackCallback);
|
||||
|
||||
// Change projectId - this should clear the buffer
|
||||
rerender({ projectId: "proj-B" });
|
||||
|
||||
// Wait for new context
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(0);
|
||||
});
|
||||
|
||||
// Register a new callback on the NEW context
|
||||
const newScrollbackCallback = vi.fn();
|
||||
result.current.onScrollback(newScrollbackCallback);
|
||||
|
||||
// No buffered data should be delivered because the buffer was cleared
|
||||
expect(newScrollbackCallback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("handles rapid context switches correctly", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
// Rapid switches: A -> B -> C
|
||||
// Each switch closes old ws and creates new ws
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-B" });
|
||||
rerender({ sessionId: "test-session-123", projectId: "proj-C" });
|
||||
|
||||
// Wait for effects
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(0);
|
||||
});
|
||||
|
||||
// Each rerender creates a new ws (close old + connect new)
|
||||
// ws1 (proj-A), ws2 (proj-B), ws3 (proj-C)
|
||||
expect(MockWebSocket.instances).toHaveLength(3);
|
||||
expect(MockWebSocket.instances[2].url).toContain("projectId=proj-C");
|
||||
});
|
||||
|
||||
it("sessionId change also triggers context cleanup", () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ sessionId, projectId }: { sessionId: string | null; projectId?: string }) =>
|
||||
useTerminal(sessionId, projectId),
|
||||
{ initialProps: { sessionId: "test-session-123", projectId: "proj-A" } },
|
||||
);
|
||||
|
||||
const oldWs = MockWebSocket.instances[0];
|
||||
const oldWsCloseSpy = vi.spyOn(oldWs, "close");
|
||||
|
||||
// Change sessionId only
|
||||
rerender({ sessionId: "test-session-456", projectId: "proj-A" });
|
||||
|
||||
// Old WebSocket should be closed
|
||||
expect(oldWsCloseSpy).toHaveBeenCalled();
|
||||
|
||||
// New WebSocket should be created with new sessionId
|
||||
expect(MockWebSocket.instances).toHaveLength(2);
|
||||
expect(MockWebSocket.instances[1].url).toContain("sessionId=test-session-456");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -66,6 +66,9 @@ function createEmptyBuffer(): BufferedMessages {
|
||||
* - Early message buffering: scrollback, connected, and initial data messages
|
||||
* are buffered and replayed to subscribers that register after the WebSocket
|
||||
* starts receiving events (e.g. while xterm is still initializing).
|
||||
* - Project-context isolation: stale WebSocket callbacks from prior project/session
|
||||
* contexts cannot update current UI state. Uses context version guards to reject
|
||||
* events from outdated connections.
|
||||
*
|
||||
* @example
|
||||
* ```tsx
|
||||
@@ -81,6 +84,26 @@ function createEmptyBuffer(): BufferedMessages {
|
||||
*/
|
||||
export function useTerminal(sessionId: string | null, projectId?: string): UseTerminalReturn {
|
||||
const [connectionStatus, setConnectionStatus] = useState<ConnectionStatus>("disconnected");
|
||||
|
||||
// Track context version to detect stale WebSocket callbacks after project/session switches.
|
||||
// Incremented whenever projectId or sessionId changes, invalidating any callbacks
|
||||
// from WebSocket connections that belong to the previous context.
|
||||
const contextVersionRef = useRef(0);
|
||||
|
||||
// Track previous values to detect context changes
|
||||
const previousSessionIdRef = useRef<string | null>(sessionId);
|
||||
const previousProjectIdRef = useRef<string | undefined>(projectId);
|
||||
|
||||
// Detect context change: either projectId or sessionId changed
|
||||
const contextChanged =
|
||||
previousSessionIdRef.current !== sessionId ||
|
||||
previousProjectIdRef.current !== projectId;
|
||||
|
||||
if (contextChanged) {
|
||||
previousSessionIdRef.current = sessionId;
|
||||
previousProjectIdRef.current = projectId;
|
||||
contextVersionRef.current++;
|
||||
}
|
||||
|
||||
const wsRef = useRef<WebSocket | null>(null);
|
||||
const reconnectAttemptsRef = useRef(0);
|
||||
@@ -170,7 +193,37 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
}
|
||||
}, []);
|
||||
|
||||
// Cleanup function
|
||||
// Internal cleanup for context changes: closes WebSocket WITHOUT marking as
|
||||
// manual close, so onclose handler doesn't interfere with the context transition.
|
||||
// Does NOT reset isManualCloseRef to preserve the flag for the calling context.
|
||||
const closeWebSocketForContextChange = useCallback(() => {
|
||||
if (reconnectTimeoutRef.current) {
|
||||
clearTimeout(reconnectTimeoutRef.current);
|
||||
reconnectTimeoutRef.current = null;
|
||||
}
|
||||
|
||||
if (heartbeatIntervalRef.current) {
|
||||
clearInterval(heartbeatIntervalRef.current);
|
||||
heartbeatIntervalRef.current = null;
|
||||
}
|
||||
|
||||
if (wsRef.current) {
|
||||
// Remove listeners to prevent stale onclose from interfering
|
||||
// with the context transition
|
||||
wsRef.current.onopen = null;
|
||||
wsRef.current.onmessage = null;
|
||||
wsRef.current.onclose = null;
|
||||
wsRef.current.onerror = null;
|
||||
wsRef.current.close();
|
||||
wsRef.current = null;
|
||||
}
|
||||
|
||||
// Clear buffers on context change to prevent stale replay
|
||||
initialBufferRef.current = createEmptyBuffer();
|
||||
}, []);
|
||||
|
||||
// Cleanup function (used for unmount and manual reconnect)
|
||||
// Marks the close as intentional so onclose handler doesn't reconnect.
|
||||
const cleanup = useCallback(() => {
|
||||
if (reconnectTimeoutRef.current) {
|
||||
clearTimeout(reconnectTimeoutRef.current);
|
||||
@@ -213,6 +266,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
isManualCloseRef.current = false;
|
||||
setConnectionStatus("connecting");
|
||||
|
||||
// Capture the context version at connection start. Stale callbacks from
|
||||
// previous project/session contexts will be rejected by comparing against
|
||||
// the current contextVersionRef.current value.
|
||||
const contextVersionAtConnect = contextVersionRef.current;
|
||||
|
||||
// Build WebSocket URL with optional projectId for multi-project support
|
||||
const protocol = window.location.protocol === "https:" ? "wss:" : "ws:";
|
||||
let wsUrl = `${protocol}//${window.location.host}/api/terminal/ws?sessionId=${encodeURIComponent(sessionId)}`;
|
||||
@@ -224,6 +282,12 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
wsRef.current = ws;
|
||||
|
||||
ws.onopen = () => {
|
||||
// Reject stale events from previous context
|
||||
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||
ws.close();
|
||||
return;
|
||||
}
|
||||
|
||||
// Reset buffer ONLY when connection is established — ensures any
|
||||
// late-arriving messages from a previous session are discarded and
|
||||
// the new session's scrollback/data is captured in a fresh buffer.
|
||||
@@ -237,6 +301,10 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
}
|
||||
|
||||
heartbeatIntervalRef.current = setInterval(() => {
|
||||
// Reject stale heartbeat from previous context
|
||||
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||
return;
|
||||
}
|
||||
if (ws.readyState === WebSocket.OPEN) {
|
||||
ws.send(JSON.stringify({ type: "ping" }));
|
||||
}
|
||||
@@ -244,6 +312,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
};
|
||||
|
||||
ws.onmessage = (event) => {
|
||||
// Reject stale events from previous context
|
||||
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const msg: WebSocketMessage = JSON.parse(event.data);
|
||||
const buffer = initialBufferRef.current;
|
||||
@@ -294,6 +367,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
};
|
||||
|
||||
ws.onclose = (event) => {
|
||||
// Reject stale close events from previous context
|
||||
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||
return;
|
||||
}
|
||||
|
||||
wsRef.current = null;
|
||||
|
||||
if (heartbeatIntervalRef.current) {
|
||||
@@ -331,7 +409,15 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
const delay = INITIAL_RECONNECT_DELAY * Math.pow(2, reconnectAttemptsRef.current - 1);
|
||||
setConnectionStatus("reconnecting");
|
||||
|
||||
// Capture the version at reconnect scheduling time to detect if context
|
||||
// changed while the timeout was pending
|
||||
const contextVersionAtSchedule = contextVersionRef.current;
|
||||
|
||||
reconnectTimeoutRef.current = setTimeout(() => {
|
||||
// Reject reconnect if context changed while timeout was pending
|
||||
if (contextVersionRef.current !== contextVersionAtSchedule) {
|
||||
return;
|
||||
}
|
||||
if (!isManualCloseRef.current) {
|
||||
connect();
|
||||
}
|
||||
@@ -350,17 +436,28 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
||||
connect();
|
||||
}, [cleanup, connect]);
|
||||
|
||||
// Connect when sessionId changes
|
||||
// Connect when sessionId or projectId changes
|
||||
// Handle context change: close existing WebSocket, cancel timers, reset state
|
||||
useEffect(() => {
|
||||
// If context changed, perform cleanup before connecting to new context
|
||||
if (contextChanged) {
|
||||
// Use internal cleanup that doesn't mark as manual close,
|
||||
// allowing proper context transition without stale onclose interference
|
||||
closeWebSocketForContextChange();
|
||||
|
||||
// Reset transient state
|
||||
reconnectAttemptsRef.current = 0;
|
||||
setConnectionStatus("disconnected");
|
||||
}
|
||||
|
||||
if (sessionId) {
|
||||
connect();
|
||||
} else {
|
||||
cleanup();
|
||||
setConnectionStatus("disconnected");
|
||||
}
|
||||
|
||||
return cleanup;
|
||||
}, [sessionId, connect, cleanup]);
|
||||
}, [sessionId, projectId, contextChanged, connect, cleanup, closeWebSocketForContextChange]);
|
||||
|
||||
return {
|
||||
connectionStatus,
|
||||
|
||||
Reference in New Issue
Block a user