From 3f6e1c6e407f258a4101ff913ffd40c6a77a9c37 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 9 Jun 2026 23:03:51 -0700 Subject: [PATCH] FN-6169: open script commands in a new terminal tab Open fresh tabs for follow-up script launches instead of reusing the current terminal. - track whether an initial command arrived with modal open versus after the terminal was already active - create a new terminal tab for later script commands, wait for that tab to become active, then send the command there - expand TerminalModal coverage for first-open versus follow-up script execution and clarify modal-manager script handoff expectations Files changed: packages/dashboard/app/components/TerminalModal.tsx | 80 ++++++++++- .../components/__tests__/TerminalModal.test.tsx | 150 ++++++++++++++++++--- .../app/hooks/__tests__/useModalManager.test.ts | 4 +- 3 files changed, 207 insertions(+), 27 deletions(-) Fusion-Task-Id: FN-6169 Fusion-Task-Lineage: a7e57a9b-dc3a-432f-bff1-195b62555852 --- .../app/components/TerminalModal.tsx | 84 +++++++++- .../__tests__/TerminalModal.test.tsx | 150 +++++++++++++++--- .../hooks/__tests__/useModalManager.test.ts | 4 +- 3 files changed, 209 insertions(+), 29 deletions(-) diff --git a/packages/dashboard/app/components/TerminalModal.tsx b/packages/dashboard/app/components/TerminalModal.tsx index 5f68cafc0f..4bb5652723 100644 --- a/packages/dashboard/app/components/TerminalModal.tsx +++ b/packages/dashboard/app/components/TerminalModal.tsx @@ -251,6 +251,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te const [fontSize, setFontSize] = useState(() => readInitialTerminalFontSize()); const [showShortcuts, setShowShortcuts] = useState(false); const [stickyModifier, setStickyModifier] = useState(null); + const [pendingInitialCommandGeneration, setPendingInitialCommandGeneration] = useState(0); const terminalRef = useRef(null); const modalRef = useRef(null); @@ -259,6 +260,10 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te const xtermRef = useRef(null); const fitAddonRef = useRef(null); const hasInitialCommandRun = useRef(false); + const initialCommandAtOpenRef = useRef(null); + const latestInitialCommandRef = useRef(initialCommand); + const pendingInitialCommandRef = useRef<{ command: string; sessionId: string } | null>(null); + const creatingInitialCommandTabRef = useRef(false); const xtermInitializedRef = useRef(false); const resizeRef = useRef<((cols: number, rows: number) => void) | null>(null); // Latest sendInput, kept in a ref so the xterm.onData listener bound at @@ -280,6 +285,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te // current mobile keyboard state without forcing the init effect to re-run. keyboardOverlapRef.current = keyboardOverlap; fontSizeRef.current = fontSize; + latestInitialCommandRef.current = initialCommand; /** * Fit xterm and publish cols/rows for a specific terminal session. @@ -321,6 +327,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te // effect re-evaluates after a close/reopen cycle (deps may be identical). useEffect(() => { if (isOpen) { + initialCommandAtOpenRef.current = latestInitialCommandRef.current ?? null; setOpenGeneration((g) => g + 1); } }, [isOpen]); @@ -769,6 +776,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te setXtermReady(false); setXtermInitError(null); hasInitialCommandRun.current = false; + initialCommandAtOpenRef.current = null; + pendingInitialCommandRef.current = null; + creatingInitialCommandTabRef.current = false; setError(null); setExitCode(null); setShowShortcuts(false); @@ -826,16 +836,76 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te // Tracks the last command that was sent so that a new command provided // while the terminal is already open (e.g., running a different script) // will be executed immediately without requiring a modal close/reopen. + // Commands provided with a fresh modal open use the auto-created active tab; + // later commands create a new tab first so an existing terminal is not + // interrupted or overwritten by script output. // Depends on openGeneration so the command re-fires after close/reopen. useEffect(() => { - if (connectionStatus === "connected" && initialCommand && hasInitialCommandRun.current !== initialCommand && activeTab) { - hasInitialCommandRun.current = initialCommand; - // Small delay to let shell initialize - setTimeout(() => { - sendInput(initialCommand + "\n"); - }, 500); + if (connectionStatus !== "connected" || !initialCommand || !activeTab) { + return; } - }, [connectionStatus, initialCommand, sendInput, activeTab, openGeneration]); + + if (hasInitialCommandRun.current === initialCommand) { + return; + } + + const pendingCommand = pendingInitialCommandRef.current; + if (pendingCommand?.command === initialCommand || creatingInitialCommandTabRef.current) { + return; + } + + hasInitialCommandRun.current = initialCommand; + + const commandArrivedWithThisOpen = + hasInitialCommandRun.current === initialCommand && + initialCommandAtOpenRef.current === initialCommand; + + if (commandArrivedWithThisOpen) { + setTimeout(() => { + sendInputRef.current(initialCommand + "\n"); + }, 500); + return; + } + + creatingInitialCommandTabRef.current = true; + void createTab() + .then((newTab) => { + pendingInitialCommandRef.current = { + command: initialCommand, + sessionId: newTab.sessionId, + }; + setPendingInitialCommandGeneration((generation) => generation + 1); + }) + .catch((err) => { + const message = getErrorMessage(err); + setError(t("terminal.createScriptTabError", "Failed to create terminal tab for script: {{message}}", { message })); + if (hasInitialCommandRun.current === initialCommand) { + hasInitialCommandRun.current = false; + } + }) + .finally(() => { + creatingInitialCommandTabRef.current = false; + }); + }, [connectionStatus, initialCommand, activeTab, createTab, openGeneration, t]); + + useEffect(() => { + const pendingCommand = pendingInitialCommandRef.current; + if ( + connectionStatus !== "connected" || + !activeTab || + !pendingCommand || + pendingCommand.sessionId !== activeTab.sessionId + ) { + return; + } + + pendingInitialCommandRef.current = null; + const timeout = setTimeout(() => { + sendInputRef.current(pendingCommand.command + "\n"); + }, 500); + + return () => clearTimeout(timeout); + }, [connectionStatus, activeTab?.sessionId, pendingInitialCommandGeneration]); useEffect(() => { if (!xtermReady || !xtermRef.current) { diff --git a/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx b/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx index ef22148499..f8de390a1b 100644 --- a/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx @@ -45,20 +45,26 @@ const mockTerminalInstance = { }; vi.mock("@xterm/xterm", () => ({ - Terminal: vi.fn(() => mockTerminalInstance), + Terminal: vi.fn(function TerminalMock() { + return mockTerminalInstance; + }), })); vi.mock("@xterm/addon-fit", () => ({ - FitAddon: vi.fn(() => ({ - fit: mockFitAddonFit, - dispose: vi.fn(), - })), + FitAddon: vi.fn(function FitAddonMock() { + return { + fit: mockFitAddonFit, + dispose: vi.fn(), + }; + }), })); vi.mock("@xterm/addon-web-links", () => ({ - WebLinksAddon: vi.fn(() => ({ - dispose: vi.fn(), - })), + WebLinksAddon: vi.fn(function WebLinksAddonMock() { + return { + dispose: vi.fn(), + }; + }), })); vi.mock("@xterm/addon-webgl", () => { @@ -128,7 +134,24 @@ describe("TerminalModal", () => { ...overrides, }); - beforeEach(() => { + beforeEach(async () => { + const xtermModule = await import("@xterm/xterm"); + const fitAddonModule = await import("@xterm/addon-fit"); + const webLinksAddonModule = await import("@xterm/addon-web-links"); + vi.mocked(xtermModule.Terminal).mockImplementation(function TerminalMock() { + return mockTerminalInstance; + } as never); + vi.mocked(fitAddonModule.FitAddon).mockImplementation(function FitAddonMock() { + return { + fit: mockFitAddonFit, + dispose: vi.fn(), + }; + } as never); + vi.mocked(webLinksAddonModule.WebLinksAddon).mockImplementation(function WebLinksAddonMock() { + return { + dispose: vi.fn(), + }; + } as never); vi.clearAllMocks(); terminalKeyEventHandler = null; mockFitAddonFit.mockClear(); @@ -155,6 +178,7 @@ describe("TerminalModal", () => { }); afterEach(() => { + vi.useRealTimers(); vi.restoreAllMocks(); }); @@ -924,16 +948,22 @@ describe("TerminalModal", () => { }); } - it("sends initialCommand to terminal when connected", async () => { + it("sends initialCommand to the existing auto-created terminal on first open", async () => { vi.useFakeTimers(); + const mockCreateTab = vi.fn(); mockUseTerminal.mockReturnValue( createMockTerminalState({ connectionStatus: "connected" }) ); + mockUseTerminalSessions.mockReturnValue({ + ...defaultSessionState, + createTab: mockCreateTab, + }); try { render(); await flushInitialCommandDelay(); + expect(mockCreateTab).not.toHaveBeenCalled(); expect(mockSendInput).toHaveBeenCalledWith("npm run build\n"); } finally { vi.useRealTimers(); @@ -970,11 +1000,74 @@ describe("TerminalModal", () => { } }); - it("sends a new initialCommand when it changes while terminal is open", async () => { + it("creates a new tab before sending an initialCommand that arrives while terminal is already open", async () => { vi.useFakeTimers(); + const scriptTab = { + id: "tab-script", + sessionId: "script-session-456", + title: "Terminal 2", + isActive: true, + createdAt: Date.now(), + }; + const mockCreateTab = vi.fn().mockResolvedValue(scriptTab); mockUseTerminal.mockReturnValue( createMockTerminalState({ connectionStatus: "connected" }) ); + mockUseTerminalSessions.mockReturnValue({ + ...defaultSessionState, + createTab: mockCreateTab, + }); + + try { + const { rerender } = render( + + ); + + rerender( + + ); + + await act(async () => {}); + expect(mockCreateTab).toHaveBeenCalledTimes(1); + expect(mockSendInput).not.toHaveBeenCalledWith("pnpm test\n"); + + mockUseTerminalSessions.mockReturnValue({ + ...defaultSessionState, + tabs: [{ ...defaultTab, isActive: false }, scriptTab], + activeTab: scriptTab, + createTab: mockCreateTab, + }); + rerender( + + ); + + await flushInitialCommandDelay(); + expect(mockSendInput).toHaveBeenCalledWith("pnpm test\n"); + expect(mockCreateTab.mock.invocationCallOrder[0]).toBeLessThan( + mockSendInput.mock.invocationCallOrder.at(-1) ?? Number.MAX_SAFE_INTEGER, + ); + } finally { + vi.useRealTimers(); + } + }); + + it("creates a new tab before sending a changed initialCommand while terminal remains open", async () => { + vi.useFakeTimers(); + const scriptTab = { + id: "tab-script", + sessionId: "script-session-456", + title: "Terminal 2", + isActive: true, + createdAt: Date.now(), + }; + const mockCreateTab = vi.fn().mockResolvedValue(scriptTab); + mockUseTerminal.mockReturnValue( + createMockTerminalState({ connectionStatus: "connected" }) + ); + mockUseTerminalSessions.mockReturnValue({ + ...defaultSessionState, + createTab: mockCreateTab, + }); try { const { rerender } = render( @@ -984,7 +1077,20 @@ describe("TerminalModal", () => { await flushInitialCommandDelay(); expect(mockSendInput).toHaveBeenCalledWith("npm run build\n"); - // Change the command (e.g., user runs a different script) + rerender( + + ); + + await act(async () => {}); + expect(mockCreateTab).toHaveBeenCalledTimes(1); + expect(mockSendInput).not.toHaveBeenCalledWith("pnpm test\n"); + + mockUseTerminalSessions.mockReturnValue({ + ...defaultSessionState, + tabs: [{ ...defaultTab, isActive: false }, scriptTab], + activeTab: scriptTab, + createTab: mockCreateTab, + }); rerender( ); @@ -1038,7 +1144,7 @@ describe("TerminalModal", () => { // Replace Terminal constructor with one that throws const throwingModule = await import("@xterm/xterm"); - (throwingModule as any).Terminal = vi.fn().mockImplementation(() => { + (throwingModule as any).Terminal = vi.fn(function ThrowingTerminalMock() { throw new Error("xterm constructor failed"); }); @@ -1069,7 +1175,7 @@ describe("TerminalModal", () => { const OrigTerminal = throwingModule.Terminal; let callCount = 0; - (throwingModule as any).Terminal = vi.fn().mockImplementation(() => { + (throwingModule as any).Terminal = vi.fn(function ReinitializingTerminalMock() { callCount++; if (callCount === 1) { throw new Error("first init fails"); @@ -1119,7 +1225,7 @@ describe("TerminalModal", () => { const xtermModule = await import("@xterm/xterm"); const OrigTerminal = xtermModule.Terminal; - (xtermModule as any).Terminal = vi.fn().mockImplementation(() => { + (xtermModule as any).Terminal = vi.fn(function ThrowingTerminalMock() { throw new Error("xterm constructor failed"); }); @@ -1161,7 +1267,7 @@ describe("TerminalModal", () => { // 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(() => { + (xtermModule as any).Terminal = vi.fn(function TimeoutTerminalMock() { throw new Error("xterm initialization timed out"); }); @@ -1203,7 +1309,7 @@ describe("TerminalModal", () => { const throwingModule = await import("@xterm/xterm"); const OrigTerminal = throwingModule.Terminal; - (throwingModule as any).Terminal = vi.fn().mockImplementation(() => { + (throwingModule as any).Terminal = vi.fn(function ThrowingTerminalMock() { throw new Error("xterm constructor failed"); }); @@ -1248,10 +1354,12 @@ describe("TerminalModal", () => { // Mock WebGL addon to track if it's loaded vi.mock("@xterm/addon-webgl", () => ({ - WebglAddon: vi.fn(() => ({ - onContextLoss: vi.fn(), - dispose: vi.fn(), - })), + WebglAddon: vi.fn(function WebglAddonMock() { + return { + onContextLoss: vi.fn(), + dispose: vi.fn(), + }; + }), })); }); diff --git a/packages/dashboard/app/hooks/__tests__/useModalManager.test.ts b/packages/dashboard/app/hooks/__tests__/useModalManager.test.ts index 6befbc10de..04fcb5e711 100644 --- a/packages/dashboard/app/hooks/__tests__/useModalManager.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useModalManager.test.ts @@ -100,7 +100,7 @@ describe("useModalManager", () => { expect(result.current.planningResumeSessionId).toBe("plan-1"); }); - it("keeps script-to-terminal handoff inside runScript", async () => { + it("runScript sets terminalInitialCommand and opens the terminal modal", async () => { const { result } = renderHook(() => useModalManager({ projectId: "proj_1", planningSessions: [] }), ); @@ -109,6 +109,8 @@ describe("useModalManager", () => { result.current.openScripts(); }); expect(result.current.scriptsOpen).toBe(true); + expect(result.current.terminalOpen).toBe(false); + expect(result.current.terminalInitialCommand).toBeUndefined(); await act(async () => { await result.current.runScript("build", "pnpm build");