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
This commit is contained in:
@@ -251,6 +251,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
const [fontSize, setFontSize] = useState<number>(() => readInitialTerminalFontSize());
|
||||
const [showShortcuts, setShowShortcuts] = useState(false);
|
||||
const [stickyModifier, setStickyModifier] = useState<null | "ctrl" | "alt">(null);
|
||||
const [pendingInitialCommandGeneration, setPendingInitialCommandGeneration] = useState(0);
|
||||
|
||||
const terminalRef = useRef<HTMLDivElement>(null);
|
||||
const modalRef = useRef<HTMLDivElement>(null);
|
||||
@@ -259,6 +260,10 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
const xtermRef = useRef<XTerm | null>(null);
|
||||
const fitAddonRef = useRef<ITerminalAddon | null>(null);
|
||||
const hasInitialCommandRun = useRef<string | false>(false);
|
||||
const initialCommandAtOpenRef = useRef<string | null>(null);
|
||||
const latestInitialCommandRef = useRef<string | undefined>(initialCommand);
|
||||
const pendingInitialCommandRef = useRef<{ command: string; sessionId: string } | null>(null);
|
||||
const creatingInitialCommandTabRef = useRef(false);
|
||||
const xtermInitializedRef = useRef<string | false>(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) {
|
||||
|
||||
@@ -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(<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />);
|
||||
|
||||
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(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} />
|
||||
);
|
||||
|
||||
rerender(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
|
||||
);
|
||||
|
||||
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(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
|
||||
);
|
||||
|
||||
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(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
|
||||
);
|
||||
|
||||
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(
|
||||
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
|
||||
);
|
||||
@@ -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(),
|
||||
};
|
||||
}),
|
||||
}));
|
||||
});
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user