Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(frontend): Disable terminal stdin if the runtime is starting up #5625

Merged
merged 4 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions frontend/__tests__/components/terminal/terminal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,6 @@ import { vi, describe, afterEach, it, expect } from "vitest";
import { Command, appendInput, appendOutput } from "#/state/command-slice";
import Terminal from "#/components/features/terminal/terminal";

global.ResizeObserver = vi.fn().mockImplementation(() => ({
observe: vi.fn(),
disconnect: vi.fn(),
}));

const mockTerminal = {
open: vi.fn(),
write: vi.fn(),
writeln: vi.fn(),
dispose: vi.fn(),
onKey: vi.fn(),
attachCustomKeyEventHandler: vi.fn(),
loadAddon: vi.fn(),
};

vi.mock("@xterm/xterm", async (importOriginal) => ({
...(await importOriginal<typeof import("@xterm/xterm")>()),
Terminal: vi.fn().mockImplementation(() => mockTerminal),
}));

const renderTerminal = (commands: Command[] = []) =>
renderWithProviders(<Terminal secrets={[]} />, {
preloadedState: {
Expand All @@ -34,6 +14,26 @@ const renderTerminal = (commands: Command[] = []) =>
});

describe.skip("Terminal", () => {
global.ResizeObserver = vi.fn().mockImplementation(() => ({
observe: vi.fn(),
disconnect: vi.fn(),
}));

const mockTerminal = {
open: vi.fn(),
write: vi.fn(),
writeln: vi.fn(),
dispose: vi.fn(),
onKey: vi.fn(),
attachCustomKeyEventHandler: vi.fn(),
loadAddon: vi.fn(),
};

vi.mock("@xterm/xterm", async (importOriginal) => ({
...(await importOriginal<typeof import("@xterm/xterm")>()),
Terminal: vi.fn().mockImplementation(() => mockTerminal),
}));

afterEach(() => {
vi.clearAllMocks();
});
Expand Down
7 changes: 2 additions & 5 deletions frontend/__tests__/hooks/use-terminal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { ReactNode } from "react";
import { useTerminal } from "#/hooks/use-terminal";
import { Command } from "#/state/command-slice";


interface TestTerminalComponentProps {
commands: Command[];
secrets: string[];
Expand All @@ -15,7 +14,7 @@ function TestTerminalComponent({
commands,
secrets,
}: TestTerminalComponentProps) {
const ref = useTerminal(commands, secrets);
const ref = useTerminal({ commands, secrets, disabled: false });
return <div ref={ref} />;
}

Expand All @@ -24,9 +23,7 @@ interface WrapperProps {
}

function Wrapper({ children }: WrapperProps) {
return (
<div>{children}</div>
)
return <div>{children}</div>;
}

describe("useTerminal", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {
useWsClient,
WsClientProviderStatus,
} from "#/context/ws-client-provider";
import { cn } from "#/utils/utils";

export function TerminalStatusLabel() {
const { status } = useWsClient();

return (
<div className="flex items-center gap-2">
<div
className={cn(
"w-2 h-2 rounded-full",
status === WsClientProviderStatus.ACTIVE && "bg-green-500",
status !== WsClientProviderStatus.ACTIVE &&
"bg-red-500 animate-pulse",
)}
/>
Terminal
</div>
);
}
13 changes: 11 additions & 2 deletions frontend/src/components/features/terminal/terminal.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
import { useSelector } from "react-redux";
import { RootState } from "#/store";
import { useTerminal } from "#/hooks/use-terminal";

import "@xterm/xterm/css/xterm.css";
import {
useWsClient,
WsClientProviderStatus,
} from "#/context/ws-client-provider";

interface TerminalProps {
secrets: string[];
}

function Terminal({ secrets }: TerminalProps) {
const { status } = useWsClient();
const { commands } = useSelector((state: RootState) => state.cmd);
const ref = useTerminal(commands, secrets);

const ref = useTerminal({
commands,
secrets,
disabled: status !== WsClientProviderStatus.ACTIVE,
});

return (
<div className="h-full p-2 min-h-0">
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/layout/container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from "react";
import { NavTab } from "./nav-tab";

interface ContainerProps {
label?: string;
label?: React.ReactNode;
labels?: {
label: string | React.ReactNode;
to: string;
Expand Down
86 changes: 56 additions & 30 deletions frontend/src/hooks/use-terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,23 @@ import { useWsClient } from "#/context/ws-client-provider";
The reason for this is that the hook exposes a ref that requires a DOM element to be rendered.
*/

export const useTerminal = (
commands: Command[] = [],
secrets: string[] = [],
) => {
interface UseTerminalConfig {
commands: Command[];
secrets: string[];
disabled: boolean;
}

const DEFAULT_TERMINAL_CONFIG: UseTerminalConfig = {
commands: [],
secrets: [],
disabled: false,
};

export const useTerminal = ({
commands,
secrets,
disabled,
}: UseTerminalConfig = DEFAULT_TERMINAL_CONFIG) => {
const { send } = useWsClient();
const terminal = React.useRef<Terminal | null>(null);
const fitAddon = React.useRef<FitAddon | null>(null);
Expand Down Expand Up @@ -85,36 +98,12 @@ export const useTerminal = (
terminal.current = createTerminal();
fitAddon.current = new FitAddon();

let resizeObserver: ResizeObserver;
let commandBuffer = "";
let resizeObserver: ResizeObserver | null = null;

if (ref.current) {
/* Initialize the terminal in the DOM */
initializeTerminal();

terminal.current.write("$ ");
terminal.current.onKey(({ key, domEvent }) => {
if (domEvent.key === "Enter") {
handleEnter(commandBuffer);
commandBuffer = "";
} else if (domEvent.key === "Backspace") {
if (commandBuffer.length > 0) {
commandBuffer = handleBackspace(commandBuffer);
}
} else {
// Ignore paste event
if (key.charCodeAt(0) === 22) {
return;
}
commandBuffer += key;
terminal.current?.write(key);
}
});
terminal.current.attachCustomKeyEventHandler((event) =>
pasteHandler(event, (text) => {
commandBuffer += text;
}),
);

/* Listen for resize events */
resizeObserver = new ResizeObserver(() => {
Expand All @@ -125,7 +114,7 @@ export const useTerminal = (

return () => {
terminal.current?.dispose();
resizeObserver.disconnect();
resizeObserver?.disconnect();
};
}, []);

Expand All @@ -152,5 +141,42 @@ export const useTerminal = (
}
}, [commands]);

React.useEffect(() => {
if (terminal.current) {
let commandBuffer = "";

if (!disabled) {
terminal.current.onKey(({ key, domEvent }) => {
if (domEvent.key === "Enter") {
handleEnter(commandBuffer);
commandBuffer = "";
} else if (domEvent.key === "Backspace") {
if (commandBuffer.length > 0) {
commandBuffer = handleBackspace(commandBuffer);
}
} else {
// Ignore paste event
if (key.charCodeAt(0) === 22) {
return;
}
commandBuffer += key;
terminal.current?.write(key);
}
});

terminal.current.attachCustomKeyEventHandler((event) =>
pasteHandler(event, (text) => {
commandBuffer += text;
}),
);
} else {
terminal.current.onKey((e) => {
e.domEvent.preventDefault();
e.domEvent.stopPropagation();
});
}
}
}, [disabled, terminal]);

return ref;
};
6 changes: 5 additions & 1 deletion frontend/src/routes/_oh.app/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { useConversationConfig } from "#/hooks/query/use-conversation-config";
import { Container } from "#/components/layout/container";
import Security from "#/components/shared/modals/security/security";
import { CountBadge } from "#/components/layout/count-badge";
import { TerminalStatusLabel } from "#/components/features/terminal/terminal-status-label";

function App() {
const { token, gitHubToken } = useAuth();
Expand Down Expand Up @@ -101,7 +102,10 @@ function App() {
</Container>
{/* Terminal uses some API that is not compatible in a server-environment. For this reason, we lazy load it to ensure
* that it loads only in the client-side. */}
<Container className="h-1/3 overflow-scroll" label="Terminal">
<Container
className="h-1/3 overflow-scroll"
label={<TerminalStatusLabel />}
>
<React.Suspense fallback={<div className="h-full" />}>
<Terminal secrets={secrets} />
</React.Suspense>
Expand Down
Loading