fix(FN-868): add terminal bootstrap error handling with retry and improve workflow step manager
- Add bootstrap error state and retry logic to useTerminalSessions hook to prevent indefinite 'Starting terminal' hang - Add error/retry UI states in TerminalModal component with CSS styling - Add tests for useTerminalSessions bootstrap error and retry behavior - Refactor WorkflowStepManager with template support and improved UX - Extract CSS from inline styles to stylesheet for terminal and workflow components - Document terminal startup error handling and non-hanging guarantee in dashboard README
This commit is contained in:
@@ -160,6 +160,12 @@ Saved scripts (managed via the Scripts modal or QuickScripts dropdown in the hea
|
||||
- Sessions can be restarted when shell exits
|
||||
- Graceful shutdown with SIGTERM, then SIGKILL fallback
|
||||
|
||||
**Startup Failure Handling**:
|
||||
- Terminal startup never hangs indefinitely — if the backend session cannot be created (server unavailable, network error, etc.), the modal shows a clear error message instead of a stuck loading spinner
|
||||
- Users see an actionable error with a "Retry" button that re-attempts terminal creation without closing the modal
|
||||
- On successful retry, the terminal initializes normally; the error state clears automatically
|
||||
- Existing sessions (tabs) that are already connected are not affected by bootstrap errors on new tabs
|
||||
|
||||
### Git Manager
|
||||
The Git Manager provides comprehensive repository visualization and management directly from the web UI. Access it via the Git Branch icon button in the header (desktop: inline with other utility buttons, mobile: in the overflow menu).
|
||||
- **Safety Validation**: Dangerous commands (rm -rf /, etc.) are automatically blocked
|
||||
|
||||
@@ -97,11 +97,13 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
tabs,
|
||||
activeTab,
|
||||
isReady,
|
||||
bootstrapError,
|
||||
createTab,
|
||||
closeTab,
|
||||
setActiveTab,
|
||||
updateTabTitle,
|
||||
restartActiveTab
|
||||
restartActiveTab,
|
||||
retryBootstrap,
|
||||
} = useTerminalSessions();
|
||||
|
||||
// Get the WebSocket connection for the active session
|
||||
@@ -415,8 +417,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
}
|
||||
};
|
||||
|
||||
// Determine loading state
|
||||
const isLoading = !isReady || !activeTab || !xtermReady;
|
||||
// Determine loading state — when bootstrapError is set, we are NOT loading
|
||||
// (we have a definitive error to show instead of an indefinite spinner).
|
||||
const isLoading = !isReady || (!activeTab && !bootstrapError) || !xtermReady;
|
||||
|
||||
return (
|
||||
<div
|
||||
@@ -530,12 +533,27 @@ export function TerminalModal({ isOpen, onClose, initialCommand }: TerminalModal
|
||||
|
||||
{/* Terminal container */}
|
||||
<div className="terminal-container" data-testid="terminal-container">
|
||||
{isLoading && (
|
||||
{isLoading && !bootstrapError && (
|
||||
<div className="terminal-loading" data-testid="terminal-loading">
|
||||
<div className="terminal-spinner" />
|
||||
<span>Starting terminal...</span>
|
||||
</div>
|
||||
)}
|
||||
{bootstrapError && !activeTab && (
|
||||
<div className="terminal-loading" data-testid="terminal-bootstrap-error">
|
||||
<div className="terminal-error-content">
|
||||
<span>Failed to start terminal: {bootstrapError}</span>
|
||||
<button
|
||||
className="terminal-retry-btn"
|
||||
onClick={retryBootstrap}
|
||||
data-testid="terminal-retry-btn"
|
||||
>
|
||||
<RefreshCw size={14} />
|
||||
Retry
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
{/*
|
||||
Always render the xterm container (no display:none) so that
|
||||
terminal.open() can measure its dimensions even during a tab switch.
|
||||
|
||||
@@ -76,11 +76,13 @@ const defaultSessionState = {
|
||||
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(),
|
||||
};
|
||||
|
||||
describe("TerminalModal", () => {
|
||||
@@ -143,6 +145,104 @@ describe("TerminalModal", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("shows error with retry button when bootstrap fails instead of stuck loading", async () => {
|
||||
const mockRetryBootstrap = vi.fn();
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
tabs: [],
|
||||
activeTab: null,
|
||||
bootstrapError: "Server unreachable",
|
||||
retryBootstrap: mockRetryBootstrap,
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Should NOT show the loading spinner
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("terminal-loading")).toBeNull();
|
||||
});
|
||||
|
||||
// Should show the bootstrap error state with retry button
|
||||
expect(screen.getByTestId("terminal-bootstrap-error")).toBeTruthy();
|
||||
expect(screen.getByText(/Failed to start terminal: Server unreachable/)).toBeTruthy();
|
||||
|
||||
const retryBtn = screen.getByTestId("terminal-retry-btn");
|
||||
expect(retryBtn).toBeTruthy();
|
||||
expect(retryBtn.textContent).toContain("Retry");
|
||||
});
|
||||
|
||||
it("retry button calls retryBootstrap from the hook", async () => {
|
||||
const mockRetryBootstrap = vi.fn();
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
tabs: [],
|
||||
activeTab: null,
|
||||
bootstrapError: "Connection refused",
|
||||
retryBootstrap: mockRetryBootstrap,
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
const retryBtn = screen.getByTestId("terminal-retry-btn");
|
||||
fireEvent.click(retryBtn);
|
||||
|
||||
expect(mockRetryBootstrap).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears error state and shows terminal after successful retry", async () => {
|
||||
const mockRetryBootstrap = vi.fn();
|
||||
|
||||
// Start with error state
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
tabs: [],
|
||||
activeTab: null,
|
||||
bootstrapError: "Server unreachable",
|
||||
retryBootstrap: mockRetryBootstrap,
|
||||
});
|
||||
|
||||
const { rerender } = render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Error state should be shown
|
||||
expect(screen.getByTestId("terminal-bootstrap-error")).toBeTruthy();
|
||||
|
||||
// Simulate successful retry — hook updates state
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
tabs: [defaultTab],
|
||||
activeTab: defaultTab,
|
||||
bootstrapError: null,
|
||||
retryBootstrap: mockRetryBootstrap,
|
||||
});
|
||||
|
||||
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Error state should be gone
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("terminal-bootstrap-error")).toBeNull();
|
||||
});
|
||||
|
||||
// Loading spinner should also be gone (xterm will init)
|
||||
// The loading overlay will disappear after xterm initializes
|
||||
expect(screen.queryByTestId("terminal-loading")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not show bootstrap error when activeTab exists (recovered state)", async () => {
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
bootstrapError: "Previous error",
|
||||
tabs: [defaultTab],
|
||||
activeTab: defaultTab,
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
// Bootstrap error should NOT show because activeTab exists
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("terminal-bootstrap-error")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("shows tabs when multiple sessions exist", async () => {
|
||||
mockUseTerminalSessions.mockReturnValue({
|
||||
...defaultSessionState,
|
||||
|
||||
@@ -506,4 +506,161 @@ describe("useTerminalSessions", () => {
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("bootstrap failure and retry", () => {
|
||||
it("sets bootstrapError when createTerminalSession fails during auto-create", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
mockCreateTerminalSession.mockRejectedValue(new Error("Server unreachable"));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
// Should become ready (validation passed)
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// Should have a bootstrap error (auto-create failed)
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("Server unreachable");
|
||||
});
|
||||
|
||||
// No tabs should be created
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
expect(result.current.activeTab).toBeNull();
|
||||
});
|
||||
|
||||
it("sets bootstrapError with fallback message for non-Error throws", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
mockCreateTerminalSession.mockRejectedValue("string error");
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("string error");
|
||||
});
|
||||
});
|
||||
|
||||
it("clears bootstrapError and creates tab on retryBootstrap after failure", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
|
||||
// First attempt fails
|
||||
mockCreateTerminalSession.mockRejectedValueOnce(new Error("Connection refused"));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("Connection refused");
|
||||
});
|
||||
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
|
||||
// Retry succeeds
|
||||
mockCreateTerminalSession.mockResolvedValueOnce({
|
||||
sessionId: "session-retry",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
result.current.retryBootstrap();
|
||||
});
|
||||
|
||||
// Error should be cleared
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBeNull();
|
||||
});
|
||||
|
||||
// Tab should be created
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
expect(result.current.activeTab?.sessionId).toBe("session-retry");
|
||||
});
|
||||
});
|
||||
|
||||
it("retryBootstrap does not create duplicate tabs", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
mockCreateTerminalSession.mockResolvedValue({
|
||||
sessionId: "session-1",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
// Wait for initial tab creation
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
});
|
||||
|
||||
expect(result.current.bootstrapError).toBeNull();
|
||||
|
||||
// Call retryBootstrap when there's already a tab (no error state)
|
||||
act(() => {
|
||||
result.current.retryBootstrap();
|
||||
});
|
||||
|
||||
// Should still have exactly one tab (effect checks tabs.length === 0)
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("bootstrapError remains null when createTerminalSession succeeds", async () => {
|
||||
localStorageMock.getItem.mockReturnValue(null);
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
mockCreateTerminalSession.mockResolvedValue({
|
||||
sessionId: "session-1",
|
||||
shell: "/bin/bash",
|
||||
cwd: "/project",
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.tabs.length).toBe(1);
|
||||
});
|
||||
|
||||
expect(result.current.bootstrapError).toBeNull();
|
||||
});
|
||||
|
||||
it("sets bootstrapError when session creation fails after restoring stale tabs", async () => {
|
||||
// Stored tabs that are stale (don't exist on server)
|
||||
const storedTabs = [
|
||||
{
|
||||
id: "tab-1",
|
||||
sessionId: "session-stale",
|
||||
title: "bash",
|
||||
isActive: true,
|
||||
createdAt: Date.now(),
|
||||
},
|
||||
];
|
||||
localStorageMock.getItem.mockReturnValue(JSON.stringify(storedTabs));
|
||||
|
||||
// Server has no sessions
|
||||
mockListTerminalSessions.mockResolvedValue([]);
|
||||
// Auto-create fails
|
||||
mockCreateTerminalSession.mockRejectedValue(new Error("Internal server error"));
|
||||
|
||||
const { result } = renderHook(() => useTerminalSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.isReady).toBe(true);
|
||||
});
|
||||
|
||||
// Stale tabs should be removed, auto-create should fail
|
||||
await waitFor(() => {
|
||||
expect(result.current.bootstrapError).toBe("Internal server error");
|
||||
});
|
||||
|
||||
expect(result.current.tabs.length).toBe(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,6 +31,8 @@ interface UseTerminalSessionsReturn {
|
||||
activeTab: TerminalTab | null;
|
||||
/** Whether sessions have been validated and restored from server */
|
||||
isReady: boolean;
|
||||
/** Error during bootstrap/session creation, or null if no error */
|
||||
bootstrapError: string | null;
|
||||
/** Creates a new tab with a fresh server session */
|
||||
createTab: () => Promise<TerminalTab>;
|
||||
/** Closes a specific tab (kills server session) */
|
||||
@@ -41,6 +43,8 @@ interface UseTerminalSessionsReturn {
|
||||
updateTabTitle: (tabId: string, title: string) => void;
|
||||
/** Restarts the active tab's session with a new PTY session */
|
||||
restartActiveTab: () => Promise<void>;
|
||||
/** Retry bootstrap after a creation failure. Clears error and re-attempts auto-create. */
|
||||
retryBootstrap: () => void;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -89,6 +93,10 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
// Track whether validation has completed
|
||||
const [isReady, setIsReady] = useState(false);
|
||||
const [serverAvailable, setServerAvailable] = useState(true);
|
||||
// Track bootstrap creation failure so callers can show error/retry UI
|
||||
const [bootstrapError, setBootstrapError] = useState<string | null>(null);
|
||||
// Generation counter bumped by retryBootstrap to re-trigger auto-create effect
|
||||
const [retryGeneration, setRetryGeneration] = useState(0);
|
||||
|
||||
// Persist tabs to localStorage whenever they change
|
||||
useEffect(() => {
|
||||
@@ -173,15 +181,23 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
if (tabs.length === 0 && isReady && serverAvailable) {
|
||||
// Small delay to avoid race condition with the validation effect
|
||||
const timeout = setTimeout(() => {
|
||||
createTabInternal().catch((err) => {
|
||||
if (!isRelativeUrlFetchError(err)) {
|
||||
console.error(err);
|
||||
}
|
||||
});
|
||||
createTabInternal()
|
||||
.then(() => {
|
||||
// Clear any previous bootstrap error on success
|
||||
setBootstrapError(null);
|
||||
})
|
||||
.catch((err) => {
|
||||
if (!isRelativeUrlFetchError(err)) {
|
||||
console.error(err);
|
||||
}
|
||||
const message =
|
||||
err instanceof Error ? err.message : typeof err === "string" ? err : "Failed to create terminal session";
|
||||
setBootstrapError(message);
|
||||
});
|
||||
}, 0);
|
||||
return () => clearTimeout(timeout);
|
||||
}
|
||||
}, [isReady, serverAvailable, tabs.length]); // Run when ready or when tabs become empty
|
||||
}, [isReady, serverAvailable, tabs.length, retryGeneration]); // Run when ready or when tabs become empty
|
||||
|
||||
/**
|
||||
* Internal create tab function (used for auto-creation and user-initiated creation)
|
||||
@@ -326,14 +342,27 @@ export function useTerminalSessions(): UseTerminalSessionsReturn {
|
||||
// Derive active tab
|
||||
const activeTab = tabs.find((tab) => tab.isActive) ?? null;
|
||||
|
||||
/**
|
||||
* Retry bootstrap after a session creation failure.
|
||||
* Clears the error and bumps the retry generation so the auto-create
|
||||
* effect re-runs. Safe to call multiple times — only one active tab
|
||||
* is created because the effect checks tabs.length === 0.
|
||||
*/
|
||||
const retryBootstrap = useCallback((): void => {
|
||||
setBootstrapError(null);
|
||||
setRetryGeneration((g) => g + 1);
|
||||
}, []);
|
||||
|
||||
return {
|
||||
tabs,
|
||||
activeTab,
|
||||
isReady,
|
||||
bootstrapError,
|
||||
createTab,
|
||||
closeTab,
|
||||
setActiveTab,
|
||||
updateTabTitle,
|
||||
restartActiveTab,
|
||||
retryBootstrap,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -7325,6 +7325,38 @@ body {
|
||||
font-size: 13px;
|
||||
}
|
||||
|
||||
.terminal-error-content {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: center;
|
||||
gap: var(--space-md, 12px);
|
||||
}
|
||||
|
||||
.terminal-error-content span {
|
||||
color: var(--error, #f48771);
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
.terminal-retry-btn {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: var(--space-xs, 4px);
|
||||
padding: 6px 12px;
|
||||
background: var(--card);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius);
|
||||
color: var(--text);
|
||||
font-size: 12px;
|
||||
font-weight: 500;
|
||||
cursor: pointer;
|
||||
transition: all var(--transition-fast);
|
||||
}
|
||||
|
||||
.terminal-retry-btn:hover {
|
||||
background: var(--card-hover);
|
||||
border-color: var(--text-muted);
|
||||
}
|
||||
|
||||
/* === Terminal Modal Mobile Responsive === */
|
||||
@media (max-width: 768px) {
|
||||
.terminal-modal {
|
||||
|
||||
Reference in New Issue
Block a user