root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(FN-842): fix new-tab xterm lifecycle for immediate output rendering

Browse Source 
- Fix xterm terminal lifecycle so new tabs render output immediately instead of showing blank
- Refactor terminal management: move open/close logic from AgentsView to AgentDetailView per-tab
- Simplify AgentsView and AgentListModal to delegate terminal creation to individual agent tabs
- Update TerminalModal to handle xterm init/resize lifecycle correctly
- Add regression tests for new-tab behavior while modal is already open
- Update README terminal section with new-tab rendering behavior docs
This commit is contained in:
gsxdsm
2026-04-04 06:28:02 -07:00
parent dd4f0994ae
commit c1f5e4bb4f
4 changed files with 341 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
packages/dashboard/app/components/TerminalModal.tsx
View File

@@ -257,8 +257,12 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
// Subscribe to terminal data.
// Depends on `xtermReady` so subscriptions are established after the
// async xterm initialization completes and xtermRef.current is set.
// Depends on `activeTab?.sessionId` (not just `activeTab?.id`) so that
// creating a new tab triggers rebinding to the new session's WebSocket
// callbacks. Without sessionId, the effect would miss session switches
// that happen within the same modal session.
useEffect(() => {
if (!xtermReady || !xtermRef.current) return;
if (!xtermReady || !xtermRef.current || !activeTab) return;
const unsubData = onData((data) => {
xtermRef.current?.write(data);
@@ -270,9 +274,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
const unsubConnect = onConnect((info) => {
// Update tab title with shell name
if (activeTab) {
updateTabTitle(activeTab.id, info.shell.split("/").pop() || info.shell);
}
updateTabTitle(activeTab.id, info.shell.split("/").pop() || info.shell);
});
const unsubExit = onExit((code) => {
@@ -286,7 +288,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
unsubConnect();
unsubExit();
};
}, [xtermReady, activeTab?.id, onData, onScrollback, onConnect, onExit, updateTabTitle]);
}, [xtermReady, activeTab?.sessionId, activeTab?.id, onData, onScrollback, onConnect, onExit, updateTabTitle]);
// Run initial command when connected.
// Tracks the last command that was sent so that a new command provided
@@ -534,12 +536,19 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
<span>Starting terminal...</span>
</div>
)}
{/* Use key to force remount on session change */}
{/*
Always render the xterm container (no display:none) so that
terminal.open() can measure its dimensions even during a tab switch.
The loading overlay (position: absolute) visually covers it until
xterm is ready. Use key={sessionId} to force a clean DOM remount
when switching tabs — this prevents stale xterm state from the
previous session.
*/}
<div
key={activeTab?.sessionId}
ref={terminalRef}
className="terminal-xterm"
data-testid="terminal-xterm"
style={isLoading ? { display: "none" } : undefined}
/>
</div>

323
packages/dashboard/app/components/__tests__/TerminalModal.test.tsx
View File

@@ -345,7 +345,7 @@ describe("TerminalModal", () => {
expect(mockTerminalInstance.open).toHaveBeenCalledWith(terminalDiv);
});
it("xterm container is hidden while loading", async () => {
it("xterm container is rendered (visible under loading overlay) while loading", async () => {
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
isReady: false,
@@ -354,12 +354,15 @@ describe("TerminalModal", () => {
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
await waitFor(() => {
// The xterm container is always rendered (no display:none) so that
// terminal.open() can measure dimensions even during a tab switch.
// The loading overlay visually covers it.
const xtermDiv = screen.getByTestId("terminal-xterm");
expect(xtermDiv.style.display).toBe("none");
expect(xtermDiv.style.display).toBe("");
});
});
it("xterm container becomes visible when ready", async () => {
it("xterm container remains rendered when ready", async () => {
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
await waitFor(() => {
@@ -820,6 +823,320 @@ describe("TerminalModal — mobile layout contract", () => {
});
});
// --- New-tab regression tests ---
describe("TerminalModal — new tab while modal open", () => {
const mockOnClose = vi.fn();
const mockSendInput = vi.fn();
const mockResize = vi.fn();
const mockReconnect = vi.fn();
const createMockTerminalState = (overrides = {}) => ({
connectionStatus: "disconnected" 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,
...overrides,
});
beforeEach(() => {
vi.clearAllMocks();
mockCreateTerminalSession.mockResolvedValue({
sessionId: "test-session-123",
shell: "/bin/bash",
cwd: "/project",
});
mockKillPtyTerminalSession.mockResolvedValue({ killed: true });
mockUseTerminal.mockReturnValue(createMockTerminalState());
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
});
});
afterEach(() => {
vi.restoreAllMocks();
});
/**
* Regression: creating a new tab while the modal is open must initialize
* xterm for the new session immediately — no close/reopen required.
*/
it("initializes xterm for the new tab without closing and reopening the modal", async () => {
// Start with one tab
const firstTab = {
id: "tab-1",
sessionId: "session-1",
title: "Terminal 1",
isActive: true,
createdAt: Date.now(),
};
const { result, rerender } = renderWithTabs([firstTab], firstTab);
// Wait for initial xterm to be created
await waitFor(() => {
expect(mockTerminalInstance.open).toHaveBeenCalledTimes(1);
});
// Now simulate creating a new tab — the sessions hook updates state
const newTab = {
id: "tab-2",
sessionId: "session-2",
title: "Terminal 2",
isActive: true,
createdAt: Date.now(),
};
const deactivatedFirstTab = { ...firstTab, isActive: false };
// Update the mock to return the new tab state
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs: [deactivatedFirstTab, newTab],
activeTab: newTab,
});
// The useTerminal hook is called with the new sessionId
mockUseTerminal.mockReturnValue(
createMockTerminalState({ connectionStatus: "connected" })
);
// Re-render to pick up the new tab
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} />
);
// xterm should be reinitialized for the new session
await waitFor(() => {
expect(mockTerminalInstance.open).toHaveBeenCalledTimes(2);
});
// Loading state should clear — no loading overlay present
await waitFor(() => {
expect(screen.queryByTestId("terminal-loading")).toBeNull();
});
});
/**
* Regression: output from the new tab's session must be delivered to xterm
* via write(), not silently dropped.
*/
it("delivers output from new tab session to xterm write()", async () => {
let capturedDataCallback: ((data: string) => void) | null = null;
let capturedScrollbackCallback: ((data: string) => void) | null = null;
const mockOnData = vi.fn((cb: (data: string) => void) => {
capturedDataCallback = cb;
return vi.fn();
});
const mockOnScrollback = vi.fn((cb: (data: string) => void) => {
capturedScrollbackCallback = cb;
return vi.fn();
});
const newTab = {
id: "tab-2",
sessionId: "session-2",
title: "Terminal 2",
isActive: true,
createdAt: Date.now(),
};
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs: [
{ id: "tab-1", sessionId: "session-1", title: "Terminal 1", isActive: false, createdAt: Date.now() },
newTab,
],
activeTab: newTab,
});
mockUseTerminal.mockReturnValue(
createMockTerminalState({
connectionStatus: "connected",
onData: mockOnData,
onScrollback: mockOnScrollback,
})
);
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
// Wait for xterm to initialize and subscriptions to be established
await waitFor(() => {
expect(mockTerminalInstance.open).toHaveBeenCalled();
});
await waitFor(() => {
expect(mockOnData).toHaveBeenCalled();
expect(mockOnScrollback).toHaveBeenCalled();
});
// Clear any previous write calls from buffered replay
mockTerminalInstance.write.mockClear();
// Simulate output arriving for the new tab's session
act(() => {
if (capturedScrollbackCallback) {
capturedScrollbackCallback("user@host:~$ ");
}
if (capturedDataCallback) {
capturedDataCallback("ls\r\n");
}
});
// xterm must receive the output via write()
expect(mockTerminalInstance.write).toHaveBeenCalledWith("user@host:~$ ");
expect(mockTerminalInstance.write).toHaveBeenCalledWith("ls\r\n");
});
/**
* Regression: subscriptions must be established for the new session,
* not stuck on the prior tab's session. When the active session changes,
* the subscription effect must re-run with the new sessionId.
*/
it("establishes subscriptions for the new session after tab creation", async () => {
const mockOnData1 = vi.fn(() => vi.fn());
const mockOnScrollback1 = vi.fn(() => vi.fn());
const mockOnConnect1 = vi.fn(() => vi.fn());
const mockOnExit1 = vi.fn(() => vi.fn());
// First tab's terminal state
const firstTabState = createMockTerminalState({
connectionStatus: "connected",
onData: mockOnData1,
onScrollback: mockOnScrollback1,
onConnect: mockOnConnect1,
onExit: mockOnExit1,
});
const firstTab = {
id: "tab-1",
sessionId: "session-1",
title: "Terminal 1",
isActive: true,
createdAt: Date.now(),
};
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs: [firstTab],
activeTab: firstTab,
});
mockUseTerminal.mockReturnValue(firstTabState);
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} />
);
// Wait for initial subscriptions to be established for session-1
await waitFor(() => {
expect(mockOnData1).toHaveBeenCalled();
expect(mockOnScrollback1).toHaveBeenCalled();
});
// Now create a new tab — new session
const mockOnData2 = vi.fn(() => vi.fn());
const mockOnScrollback2 = vi.fn(() => vi.fn());
const mockOnConnect2 = vi.fn(() => vi.fn());
const mockOnExit2 = vi.fn(() => vi.fn());
const secondTabState = createMockTerminalState({
connectionStatus: "connected",
onData: mockOnData2,
onScrollback: mockOnScrollback2,
onConnect: mockOnConnect2,
onExit: mockOnExit2,
});
const newTab = {
id: "tab-2",
sessionId: "session-2",
title: "Terminal 2",
isActive: true,
createdAt: Date.now(),
};
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs: [{ ...firstTab, isActive: false }, newTab],
activeTab: newTab,
});
mockUseTerminal.mockReturnValue(secondTabState);
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} />
);
// Subscriptions should be re-established for session-2
await waitFor(() => {
expect(mockOnData2).toHaveBeenCalled();
expect(mockOnScrollback2).toHaveBeenCalled();
expect(mockOnConnect2).toHaveBeenCalled();
expect(mockOnExit2).toHaveBeenCalled();
});
});
/**
* Regression: the xterm container must not have display:none when switching
* tabs, so that terminal.open() can always measure container dimensions.
*/
it("xterm container has no display:none during tab switch re-initialization", async () => {
const firstTab = {
id: "tab-1",
sessionId: "session-1",
title: "Terminal 1",
isActive: true,
createdAt: Date.now(),
};
const { rerender } = renderWithTabs([firstTab], firstTab);
// Wait for initial xterm
await waitFor(() => {
expect(mockTerminalInstance.open).toHaveBeenCalled();
});
// Switch to new tab
const newTab = {
id: "tab-2",
sessionId: "session-2",
title: "Terminal 2",
isActive: true,
createdAt: Date.now(),
};
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs: [{ ...firstTab, isActive: false }, newTab],
activeTab: newTab,
});
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} />
);
// The xterm container should never have display:none
await waitFor(() => {
const xtermDiv = screen.getByTestId("terminal-xterm");
expect(xtermDiv.style.display).not.toBe("none");
});
});
// Helper to render with specific tabs
function renderWithTabs(tabs: typeof defaultTab[], activeTab: typeof defaultTab) {
mockUseTerminalSessions.mockReturnValue({
...defaultSessionState,
tabs,
activeTab,
});
mockUseTerminal.mockReturnValue(createMockTerminalState());
return render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
}
});
// --- Virtual keyboard overlap handling ---
describe("TerminalModal — virtual keyboard overlap handling", () => {
const mockOnClose = vi.fn();