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);
|
const keyboardOverlapRef = useRef(0);
|
||||||
/** Tracks a pending requestAnimationFrame for deferred xterm re-fit. */
|
/** Tracks a pending requestAnimationFrame for deferred xterm re-fit. */
|
||||||
const pendingFitRef = useRef<number | null>(null);
|
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
|
// 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.
|
// 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;
|
const currentSessionId = activeTab.sessionId;
|
||||||
|
|
||||||
// If already initialized for this session, skip
|
// Detect project switch: if projectId changed, invalidate xterm even if sessionId is the same.
|
||||||
if (xtermInitializedRef.current === currentSessionId && xtermRef.current) {
|
// 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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Clean up existing xterm if switching sessions or if DOM was cleared
|
// Clean up existing xterm if switching sessions/projects or if DOM was cleared
|
||||||
if (xtermRef.current && xtermInitializedRef.current !== currentSessionId) {
|
if (xtermRef.current && (xtermInitializedRef.current !== currentSessionId || projectChanged)) {
|
||||||
xtermRef.current.dispose();
|
xtermRef.current.dispose();
|
||||||
xtermRef.current = null;
|
xtermRef.current = null;
|
||||||
fitAddonRef.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();
|
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
|
* - Early message buffering: scrollback, connected, and initial data messages
|
||||||
* are buffered and replayed to subscribers that register after the WebSocket
|
* are buffered and replayed to subscribers that register after the WebSocket
|
||||||
* starts receiving events (e.g. while xterm is still initializing).
|
* 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
|
* @example
|
||||||
* ```tsx
|
* ```tsx
|
||||||
@@ -82,6 +85,26 @@ function createEmptyBuffer(): BufferedMessages {
|
|||||||
export function useTerminal(sessionId: string | null, projectId?: string): UseTerminalReturn {
|
export function useTerminal(sessionId: string | null, projectId?: string): UseTerminalReturn {
|
||||||
const [connectionStatus, setConnectionStatus] = useState<ConnectionStatus>("disconnected");
|
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 wsRef = useRef<WebSocket | null>(null);
|
||||||
const reconnectAttemptsRef = useRef(0);
|
const reconnectAttemptsRef = useRef(0);
|
||||||
const reconnectTimeoutRef = useRef<NodeJS.Timeout | null>(null);
|
const reconnectTimeoutRef = useRef<NodeJS.Timeout | null>(null);
|
||||||
@@ -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(() => {
|
const cleanup = useCallback(() => {
|
||||||
if (reconnectTimeoutRef.current) {
|
if (reconnectTimeoutRef.current) {
|
||||||
clearTimeout(reconnectTimeoutRef.current);
|
clearTimeout(reconnectTimeoutRef.current);
|
||||||
@@ -213,6 +266,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
|||||||
isManualCloseRef.current = false;
|
isManualCloseRef.current = false;
|
||||||
setConnectionStatus("connecting");
|
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
|
// Build WebSocket URL with optional projectId for multi-project support
|
||||||
const protocol = window.location.protocol === "https:" ? "wss:" : "ws:";
|
const protocol = window.location.protocol === "https:" ? "wss:" : "ws:";
|
||||||
let wsUrl = `${protocol}//${window.location.host}/api/terminal/ws?sessionId=${encodeURIComponent(sessionId)}`;
|
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;
|
wsRef.current = ws;
|
||||||
|
|
||||||
ws.onopen = () => {
|
ws.onopen = () => {
|
||||||
|
// Reject stale events from previous context
|
||||||
|
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||||
|
ws.close();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Reset buffer ONLY when connection is established — ensures any
|
// Reset buffer ONLY when connection is established — ensures any
|
||||||
// late-arriving messages from a previous session are discarded and
|
// late-arriving messages from a previous session are discarded and
|
||||||
// the new session's scrollback/data is captured in a fresh buffer.
|
// 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(() => {
|
heartbeatIntervalRef.current = setInterval(() => {
|
||||||
|
// Reject stale heartbeat from previous context
|
||||||
|
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (ws.readyState === WebSocket.OPEN) {
|
if (ws.readyState === WebSocket.OPEN) {
|
||||||
ws.send(JSON.stringify({ type: "ping" }));
|
ws.send(JSON.stringify({ type: "ping" }));
|
||||||
}
|
}
|
||||||
@@ -244,6 +312,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
|||||||
};
|
};
|
||||||
|
|
||||||
ws.onmessage = (event) => {
|
ws.onmessage = (event) => {
|
||||||
|
// Reject stale events from previous context
|
||||||
|
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const msg: WebSocketMessage = JSON.parse(event.data);
|
const msg: WebSocketMessage = JSON.parse(event.data);
|
||||||
const buffer = initialBufferRef.current;
|
const buffer = initialBufferRef.current;
|
||||||
@@ -294,6 +367,11 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
|||||||
};
|
};
|
||||||
|
|
||||||
ws.onclose = (event) => {
|
ws.onclose = (event) => {
|
||||||
|
// Reject stale close events from previous context
|
||||||
|
if (contextVersionRef.current !== contextVersionAtConnect) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
wsRef.current = null;
|
wsRef.current = null;
|
||||||
|
|
||||||
if (heartbeatIntervalRef.current) {
|
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);
|
const delay = INITIAL_RECONNECT_DELAY * Math.pow(2, reconnectAttemptsRef.current - 1);
|
||||||
setConnectionStatus("reconnecting");
|
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(() => {
|
reconnectTimeoutRef.current = setTimeout(() => {
|
||||||
|
// Reject reconnect if context changed while timeout was pending
|
||||||
|
if (contextVersionRef.current !== contextVersionAtSchedule) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (!isManualCloseRef.current) {
|
if (!isManualCloseRef.current) {
|
||||||
connect();
|
connect();
|
||||||
}
|
}
|
||||||
@@ -350,17 +436,28 @@ export function useTerminal(sessionId: string | null, projectId?: string): UseTe
|
|||||||
connect();
|
connect();
|
||||||
}, [cleanup, connect]);
|
}, [cleanup, connect]);
|
||||||
|
|
||||||
// Connect when sessionId changes
|
// Connect when sessionId or projectId changes
|
||||||
|
// Handle context change: close existing WebSocket, cancel timers, reset state
|
||||||
useEffect(() => {
|
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) {
|
if (sessionId) {
|
||||||
connect();
|
connect();
|
||||||
} else {
|
} else {
|
||||||
cleanup();
|
|
||||||
setConnectionStatus("disconnected");
|
setConnectionStatus("disconnected");
|
||||||
}
|
}
|
||||||
|
|
||||||
return cleanup;
|
return cleanup;
|
||||||
}, [sessionId, connect, cleanup]);
|
}, [sessionId, projectId, contextChanged, connect, cleanup, closeWebSocketForContextChange]);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
connectionStatus,
|
connectionStatus,
|
||||||
|
|||||||
Reference in New Issue
Block a user