diff --git a/evaluation/benchmarks/swe_bench/README.md b/evaluation/benchmarks/swe_bench/README.md index 08ec3427e6c5..4758a6616c81 100644 --- a/evaluation/benchmarks/swe_bench/README.md +++ b/evaluation/benchmarks/swe_bench/README.md @@ -204,7 +204,7 @@ Then, in a separate Python environment with `streamlit` library, you can run the ```bash # Make sure you are inside the cloned `evaluation` repo conda activate streamlit # if you follow the optional conda env setup above -streamlit app.py --server.port 8501 --server.address 0.0.0.0 +streamlit run app.py --server.port 8501 --server.address 0.0.0.0 ``` Then you can access the SWE-Bench trajectory visualizer at `localhost:8501`. diff --git a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx index 749bc6c48de0..e07eb25a3d00 100644 --- a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx +++ b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx @@ -19,9 +19,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); const expectedDate = `${formatTimeDelta(new Date("2021-10-01T12:00:00Z"))} ago`; @@ -33,20 +33,20 @@ describe("ConversationCard", () => { within(card).getByText(expectedDate); }); - it("should render the repo if available", () => { + it("should render the selectedRepository if available", () => { const { rerender } = render( , ); expect( - screen.queryByTestId("conversation-card-repo"), + screen.queryByTestId("conversation-card-selected-repository"), ).not.toBeInTheDocument(); rerender( @@ -54,13 +54,13 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo="org/repo" - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository="org/selectedRepository" + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); - screen.getByTestId("conversation-card-repo"); + screen.getByTestId("conversation-card-selected-repository"); }); it("should call onClick when the card is clicked", async () => { @@ -70,9 +70,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -89,9 +89,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -114,9 +114,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -131,21 +131,21 @@ describe("ConversationCard", () => { expect(onDelete).toHaveBeenCalled(); }); - test("clicking the repo should not trigger the onClick handler", async () => { + test("clicking the selectedRepository should not trigger the onClick handler", async () => { const user = userEvent.setup(); render( , ); - const repo = screen.getByTestId("conversation-card-repo"); - await user.click(repo); + const selectedRepository = screen.getByTestId("conversation-card-selected-repository"); + await user.click(selectedRepository); expect(onClick).not.toHaveBeenCalled(); }); @@ -156,9 +156,9 @@ describe("ConversationCard", () => { , ); @@ -180,9 +180,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -202,9 +202,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -221,9 +221,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -239,19 +239,19 @@ describe("ConversationCard", () => { }); describe("state indicator", () => { - it("should render the 'cold' indicator by default", () => { + it("should render the 'STOPPED' indicator by default", () => { render( , ); - screen.getByTestId("cold-indicator"); + screen.getByTestId("STOPPED-indicator"); }); it("should render the other indicators when provided", () => { @@ -260,15 +260,15 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" - state="warm" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" + status="RUNNING" />, ); - expect(screen.queryByTestId("cold-indicator")).not.toBeInTheDocument(); - screen.getByTestId("warm-indicator"); + expect(screen.queryByTestId("STOPPED-indicator")).not.toBeInTheDocument(); + screen.getByTestId("RUNNING-indicator"); }); }); }); diff --git a/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx b/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx index 049181c05dba..0e35d4799c05 100644 --- a/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx +++ b/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx @@ -175,7 +175,7 @@ describe("ConversationPanel", () => { // Ensure the conversation is renamed expect(updateUserConversationSpy).toHaveBeenCalledWith("3", { - name: "Conversation 1 Renamed", + title: "Conversation 1 Renamed", }); }); diff --git a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx index 40d0ea4a48bc..1e80607301b0 100644 --- a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx +++ b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import { renderWithProviders } from "test-utils"; import { createRoutesStub } from "react-router"; import { Sidebar } from "#/components/features/sidebar/sidebar"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; const renderSidebar = () => { const RouterStub = createRoutesStub([ @@ -18,7 +18,7 @@ const renderSidebar = () => { }; describe("Sidebar", () => { - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should have the conversation panel open by default", () => { renderSidebar(); @@ -26,7 +26,7 @@ describe("Sidebar", () => { }, ); - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should toggle the conversation panel", async () => { const user = userEvent.setup(); diff --git a/frontend/__tests__/components/shared/modals/settings/runtime-size-selector.test.tsx b/frontend/__tests__/components/shared/modals/settings/runtime-size-selector.test.tsx new file mode 100644 index 000000000000..e607c6f026f4 --- /dev/null +++ b/frontend/__tests__/components/shared/modals/settings/runtime-size-selector.test.tsx @@ -0,0 +1,35 @@ +import { screen } from "@testing-library/react"; +import { describe, it, expect } from "vitest"; +import { renderWithProviders } from "test-utils"; +import { RuntimeSizeSelector } from "#/components/shared/modals/settings/runtime-size-selector"; + +const renderRuntimeSizeSelector = () => + renderWithProviders(); + +describe("RuntimeSizeSelector", () => { + it("should show both runtime size options", () => { + renderRuntimeSizeSelector(); + // The options are in the hidden select element + const select = screen.getByRole("combobox", { hidden: true }); + expect(select).toHaveValue("1"); + expect(select).toHaveDisplayValue("1x (2 core, 8G)"); + expect(select.children).toHaveLength(3); // Empty option + 2 size options + }); + + it("should show the full description text for disabled options", async () => { + renderRuntimeSizeSelector(); + + // Click the button to open the dropdown + const button = screen.getByRole("button", { + name: "1x (2 core, 8G) SETTINGS_FORM$RUNTIME_SIZE_LABEL", + }); + button.click(); + + // Wait for the dropdown to open and find the description text + const description = await screen.findByText( + "Runtime sizes over 1 are disabled by default, please contact contact@all-hands.dev to get access to larger runtimes.", + ); + expect(description).toBeInTheDocument(); + expect(description).toHaveClass("whitespace-normal", "break-words"); + }); +}); diff --git a/frontend/__tests__/components/shared/modals/settings/settings-form.test.tsx b/frontend/__tests__/components/shared/modals/settings/settings-form.test.tsx new file mode 100644 index 000000000000..e373fdfb3e4f --- /dev/null +++ b/frontend/__tests__/components/shared/modals/settings/settings-form.test.tsx @@ -0,0 +1,45 @@ +import { screen, fireEvent } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; +import { renderWithProviders } from "test-utils"; +import { createRoutesStub } from "react-router"; +import { DEFAULT_SETTINGS } from "#/services/settings"; +import { SettingsForm } from "#/components/shared/modals/settings/settings-form"; +import OpenHands from "#/api/open-hands"; + +describe("SettingsForm", () => { + const getConfigSpy = vi.spyOn(OpenHands, "getConfig"); + getConfigSpy.mockResolvedValue({ + APP_MODE: "saas", + GITHUB_CLIENT_ID: "123", + POSTHOG_CLIENT_KEY: "123", + }); + + const RouterStub = createRoutesStub([ + { + Component: () => ( + {}} + /> + ), + path: "/", + }, + ]); + + it("should not show runtime size selector by default", () => { + renderWithProviders(); + expect(screen.queryByText("Runtime Size")).not.toBeInTheDocument(); + }); + + it("should show runtime size selector when advanced options are enabled", async () => { + renderWithProviders(); + const advancedSwitch = screen.getByRole("switch", { + name: "SETTINGS_FORM$ADVANCED_OPTIONS_LABEL", + }); + fireEvent.click(advancedSwitch); + await screen.findByText("SETTINGS_FORM$RUNTIME_SIZE_LABEL"); + }); +}); diff --git a/frontend/__tests__/routes/_oh.app.test.tsx b/frontend/__tests__/routes/_oh.app.test.tsx index 2addbc5fe604..e034c7476545 100644 --- a/frontend/__tests__/routes/_oh.app.test.tsx +++ b/frontend/__tests__/routes/_oh.app.test.tsx @@ -5,7 +5,7 @@ import { screen, waitFor } from "@testing-library/react"; import toast from "react-hot-toast"; import App from "#/routes/_oh.app/route"; import OpenHands from "#/api/open-hands"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; describe("App", () => { const RouteStub = createRoutesStub([ @@ -35,7 +35,7 @@ describe("App", () => { await screen.findByTestId("app-route"); }); - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should call endSession if the user does not have permission to view conversation", async () => { const errorToastSpy = vi.spyOn(toast, "error"); @@ -59,10 +59,10 @@ describe("App", () => { getConversationSpy.mockResolvedValue({ conversation_id: "9999", - lastUpdated: "", - name: "", - repo: "", - state: "cold", + last_updated_at: "", + title: "", + selected_repository: "", + status: "STOPPED", }); const { rerender } = renderWithProviders( , diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c047943b2403..140667fbd2bf 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -52,7 +52,7 @@ "@playwright/test": "^1.49.1", "@react-router/dev": "^7.1.1", "@tailwindcss/typography": "^0.5.15", - "@tanstack/eslint-plugin-query": "^5.62.9", + "@tanstack/eslint-plugin-query": "^5.62.15", "@testing-library/jest-dom": "^6.6.1", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", @@ -5376,11 +5376,10 @@ } }, "node_modules/@tanstack/eslint-plugin-query": { - "version": "5.62.9", - "resolved": "https://registry.npmjs.org/@tanstack/eslint-plugin-query/-/eslint-plugin-query-5.62.9.tgz", - "integrity": "sha512-F3onhTcpBj7zQDo0NVtZwZQKRFx8BwpSabMJybl9no3+dFHUurvNMrH5M/6KNpkdDCf3zyHWadruZL6636B8Fw==", + "version": "5.62.15", + "resolved": "https://registry.npmjs.org/@tanstack/eslint-plugin-query/-/eslint-plugin-query-5.62.15.tgz", + "integrity": "sha512-24BHoF3LIzyptjrZXc1IpaISno+fhVD3zWWso/HPSB+ZVOyOXoiQSQc2K362T13JKJ07EInhHi1+KyNoRzCCfQ==", "dev": true, - "license": "MIT", "dependencies": { "@typescript-eslint/utils": "^8.18.1" }, diff --git a/frontend/package.json b/frontend/package.json index 41d5a8418729..b5245109f0c4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -79,7 +79,7 @@ "@playwright/test": "^1.49.1", "@react-router/dev": "^7.1.1", "@tailwindcss/typography": "^0.5.15", - "@tanstack/eslint-plugin-query": "^5.62.9", + "@tanstack/eslint-plugin-query": "^5.62.15", "@testing-library/jest-dom": "^6.6.1", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", diff --git a/frontend/src/api/open-hands.ts b/frontend/src/api/open-hands.ts index 1534be879ca5..08dec1ebe258 100644 --- a/frontend/src/api/open-hands.ts +++ b/frontend/src/api/open-hands.ts @@ -9,6 +9,7 @@ import { GetVSCodeUrlResponse, AuthenticateResponse, Conversation, + ResultSet, } from "./open-hands.types"; import { openHands } from "./open-hands-axios"; import { ApiSettings } from "#/services/settings"; @@ -222,8 +223,10 @@ class OpenHands { } static async getUserConversations(): Promise { - const { data } = await openHands.get("/api/conversations"); - return data; + const { data } = await openHands.get>( + "/api/conversations?limit=9", + ); + return data.results; } static async deleteUserConversation(conversationId: string): Promise { @@ -232,9 +235,9 @@ class OpenHands { static async updateUserConversation( conversationId: string, - conversation: Partial>, + conversation: Partial>, ): Promise { - await openHands.put(`/api/conversations/${conversationId}`, conversation); + await openHands.patch(`/api/conversations/${conversationId}`, conversation); } static async createConversation( diff --git a/frontend/src/api/open-hands.types.ts b/frontend/src/api/open-hands.types.ts index c17d2016816d..263bfb781162 100644 --- a/frontend/src/api/open-hands.types.ts +++ b/frontend/src/api/open-hands.types.ts @@ -1,4 +1,4 @@ -import { ProjectState } from "#/components/features/conversation-panel/conversation-state-indicator"; +import { ProjectStatus } from "#/components/features/conversation-panel/conversation-state-indicator"; export interface ErrorResponse { error: string; @@ -62,8 +62,13 @@ export interface AuthenticateResponse { export interface Conversation { conversation_id: string; - name: string; - repo: string | null; - lastUpdated: string; - state: ProjectState; + title: string; + selected_repository: string | null; + last_updated_at: string; + status: ProjectStatus; +} + +export interface ResultSet { + results: T[]; + next_page_id: string | null; } diff --git a/frontend/src/components/features/context-menu/context-menu.tsx b/frontend/src/components/features/context-menu/context-menu.tsx index d4d47708da4b..8350391a7b55 100644 --- a/frontend/src/components/features/context-menu/context-menu.tsx +++ b/frontend/src/components/features/context-menu/context-menu.tsx @@ -18,7 +18,7 @@ export function ContextMenu({
    {children}
diff --git a/frontend/src/components/features/conversation-panel/conversation-card.tsx b/frontend/src/components/features/conversation-panel/conversation-card.tsx index bba98607bb23..c10e90f005ba 100644 --- a/frontend/src/components/features/conversation-panel/conversation-card.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-card.tsx @@ -2,7 +2,7 @@ import React from "react"; import { formatTimeDelta } from "#/utils/format-time-delta"; import { ConversationRepoLink } from "./conversation-repo-link"; import { - ProjectState, + ProjectStatus, ConversationStateIndicator, } from "./conversation-state-indicator"; import { ContextMenu } from "../context-menu/context-menu"; @@ -13,20 +13,20 @@ interface ProjectCardProps { onClick: () => void; onDelete: () => void; onChangeTitle: (title: string) => void; - name: string; - repo: string | null; - lastUpdated: string; // ISO 8601 - state?: ProjectState; + title: string; + selectedRepository: string | null; + lastUpdatedAt: string; // ISO 8601 + status?: ProjectStatus; } export function ConversationCard({ onClick, onDelete, onChangeTitle, - name, - repo, - lastUpdated, - state = "cold", + title, + selectedRepository, + lastUpdatedAt, + status = "STOPPED", }: ProjectCardProps) { const [contextMenuVisible, setContextMenuVisible] = React.useState(false); const inputRef = React.useRef(null); @@ -38,7 +38,13 @@ export function ConversationCard({ inputRef.current!.value = trimmed; } else { // reset the value if it's empty - inputRef.current!.value = name; + inputRef.current!.value = title; + } + }; + + const handleKeyUp = (event: React.KeyboardEvent) => { + if (event.key === "Enter") { + event.currentTarget.blur(); } }; @@ -55,47 +61,45 @@ export function ConversationCard({
-
+
- + { event.stopPropagation(); setContextMenuVisible((prev) => !prev); }} /> - {contextMenuVisible && ( - - - Delete - - - )}
- {repo && ( + {contextMenuVisible && ( + + + Delete + + + )} + {selectedRepository && ( e.stopPropagation()} /> )}

- +

); diff --git a/frontend/src/components/features/conversation-panel/conversation-panel.tsx b/frontend/src/components/features/conversation-panel/conversation-panel.tsx index 673d0454a9c4..a7c9ebd54d12 100644 --- a/frontend/src/components/features/conversation-panel/conversation-panel.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-panel.tsx @@ -63,7 +63,7 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { if (oldTitle !== newTitle) updateConversation({ id: conversationId, - conversation: { name: newTitle }, + conversation: { title: newTitle }, }); }; @@ -75,7 +75,7 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { return (
{location.pathname.startsWith("/conversation") && ( @@ -103,12 +103,12 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { onClick={() => handleClickCard(project.conversation_id)} onDelete={() => handleDeleteProject(project.conversation_id)} onChangeTitle={(title) => - handleChangeTitle(project.conversation_id, project.name, title) + handleChangeTitle(project.conversation_id, project.title, title) } - name={project.name} - repo={project.repo} - lastUpdated={project.lastUpdated} - state={project.state} + title={project.title} + selectedRepository={project.selected_repository} + lastUpdatedAt={project.last_updated_at} + status={project.status} /> ))} diff --git a/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx b/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx index c2e527c2a6f9..28480277c3f4 100644 --- a/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx @@ -1,21 +1,21 @@ interface ConversationRepoLinkProps { - repo: string; + selectedRepository: string; onClick?: (event: React.MouseEvent) => void; } export function ConversationRepoLink({ - repo, + selectedRepository, onClick, }: ConversationRepoLinkProps) { return ( - {repo} + {selectedRepository} ); } diff --git a/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx b/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx index b42a13606def..c0132c2496d8 100644 --- a/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx @@ -1,39 +1,25 @@ import ColdIcon from "./state-indicators/cold.svg?react"; -import CoolingIcon from "./state-indicators/cooling.svg?react"; -import FinishedIcon from "./state-indicators/finished.svg?react"; import RunningIcon from "./state-indicators/running.svg?react"; -import WaitingIcon from "./state-indicators/waiting.svg?react"; -import WarmIcon from "./state-indicators/warm.svg?react"; type SVGIcon = React.FunctionComponent>; -export type ProjectState = - | "cold" - | "cooling" - | "finished" - | "running" - | "waiting" - | "warm"; +export type ProjectStatus = "RUNNING" | "STOPPED"; -const INDICATORS: Record = { - cold: ColdIcon, - cooling: CoolingIcon, - finished: FinishedIcon, - running: RunningIcon, - waiting: WaitingIcon, - warm: WarmIcon, +const INDICATORS: Record = { + STOPPED: ColdIcon, + RUNNING: RunningIcon, }; interface ConversationStateIndicatorProps { - state: ProjectState; + status: ProjectStatus; } export function ConversationStateIndicator({ - state, + status, }: ConversationStateIndicatorProps) { - const StateIcon = INDICATORS[state]; + const StateIcon = INDICATORS[status]; return ( -
+
); diff --git a/frontend/src/components/features/sidebar/sidebar.tsx b/frontend/src/components/features/sidebar/sidebar.tsx index 3d2e9a3e60b0..112375650487 100644 --- a/frontend/src/components/features/sidebar/sidebar.tsx +++ b/frontend/src/components/features/sidebar/sidebar.tsx @@ -1,6 +1,6 @@ import React from "react"; import { useLocation } from "react-router"; -import FolderIcon from "#/icons/docs.svg?react"; +import { FaListUl } from "react-icons/fa"; import { useAuth } from "#/context/auth-context"; import { useGitHubUser } from "#/hooks/query/use-github-user"; import { useIsAuthed } from "#/hooks/query/use-is-authed"; @@ -16,8 +16,7 @@ import { SettingsModal } from "#/components/shared/modals/settings/settings-moda import { useSettingsUpToDate } from "#/context/settings-up-to-date-context"; import { useSettings } from "#/hooks/query/use-settings"; import { ConversationPanel } from "../conversation-panel/conversation-panel"; -import { cn } from "#/utils/utils"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; export function Sidebar() { const location = useLocation(); @@ -32,9 +31,18 @@ export function Sidebar() { const [settingsModalIsOpen, setSettingsModalIsOpen] = React.useState(false); const [startNewProjectModalIsOpen, setStartNewProjectModalIsOpen] = React.useState(false); - const [conversationPanelIsOpen, setConversationPanelIsOpen] = React.useState( - MULTI_CONVO_UI_IS_ENABLED, - ); + const [conversationPanelIsOpen, setConversationPanelIsOpen] = + React.useState(false); + const conversationPanelRef = React.useRef(null); + + const handleClick = (event: MouseEvent) => { + const conversationPanel = conversationPanelRef.current; + if (conversationPanelIsOpen && conversationPanel) { + if (!conversationPanel.contains(event.target as Node)) { + setConversationPanelIsOpen(false); + } + } + }; React.useEffect(() => { // If the github token is invalid, open the account settings modal again @@ -43,6 +51,13 @@ export function Sidebar() { } }, [user.isError]); + React.useEffect(() => { + document.addEventListener("click", handleClick); + return () => { + document.removeEventListener("click", handleClick); + }; + }, [conversationPanelIsOpen]); + const handleAccountSettingsModalClose = () => { // If the user closes the modal without connecting to GitHub, // we need to log them out to clear the invalid token from the @@ -77,16 +92,17 @@ export function Sidebar() { /> )} setSettingsModalIsOpen(true)} /> - {MULTI_CONVO_UI_IS_ENABLED && ( + {MULTI_CONVERSATION_UI && ( )} @@ -97,6 +113,7 @@ export function Sidebar() { {conversationPanelIsOpen && (
+ + + + ); +} diff --git a/frontend/src/components/shared/modals/settings/settings-form.tsx b/frontend/src/components/shared/modals/settings/settings-form.tsx index 7aba4856e227..8390c237a66f 100644 --- a/frontend/src/components/shared/modals/settings/settings-form.tsx +++ b/frontend/src/components/shared/modals/settings/settings-form.tsx @@ -21,6 +21,9 @@ import { ModalBackdrop } from "../modal-backdrop"; import { ModelSelector } from "./model-selector"; import { useSaveSettings } from "#/hooks/mutation/use-save-settings"; +import { RuntimeSizeSelector } from "./runtime-size-selector"; +import { useConfig } from "#/hooks/query/use-config"; + interface SettingsFormProps { disabled?: boolean; settings: Settings; @@ -40,6 +43,7 @@ export function SettingsForm({ }: SettingsFormProps) { const { mutateAsync: saveSettings } = useSaveSettings(); const endSession = useEndSession(); + const { data: config } = useConfig(); const location = useLocation(); const { t } = useTranslation(); @@ -97,6 +101,8 @@ export function SettingsForm({ posthog.capture("settings_saved", { LLM_MODEL: newSettings.LLM_MODEL, LLM_API_KEY: newSettings.LLM_API_KEY ? "SET" : "UNSET", + REMOTE_RUNTIME_RESOURCE_FACTOR: + newSettings.REMOTE_RUNTIME_RESOURCE_FACTOR, }); }; @@ -122,6 +128,8 @@ export function SettingsForm({ } }; + const isSaasMode = config?.APP_MODE === "saas"; + return (
- {showAdvancedOptions && ( - - )} - {showAdvancedOptions && ( <> + + + {isSaasMode && ( + + )} + +const isOpenHandsEvent = (event: unknown): event is OpenHandsParsedEvent => typeof event === "object" && event !== null && "id" in event && @@ -15,10 +18,26 @@ const isOpenHandsMessage = (event: unknown): event is OpenHandsParsedEvent => "message" in event && "timestamp" in event; -const isAgentStateChangeObservation = ( +const isUserMessage = ( event: OpenHandsParsedEvent, -): event is AgentStateChangeObservation => - "observation" in event && event.observation === "agent_state_changed"; +): event is UserMessageAction => + "source" in event && + "type" in event && + event.source === "user" && + event.type === "message"; + +const isAssistantMessage = ( + event: OpenHandsParsedEvent, +): event is AssistantMessageAction => + "source" in event && + "type" in event && + event.source === "agent" && + event.type === "message"; + +const isMessageAction = ( + event: OpenHandsParsedEvent, +): event is UserMessageAction | AssistantMessageAction => + isUserMessage(event) || isAssistantMessage(event); export enum WsClientProviderStatus { CONNECTED, @@ -43,16 +62,13 @@ const WsClientContext = React.createContext({ interface WsClientProviderProps { conversationId: string; - ghToken: string | null; } export function WsClientProvider({ - ghToken, conversationId, children, }: React.PropsWithChildren) { const sioRef = React.useRef(null); - const ghTokenRef = React.useRef(ghToken); const [status, setStatus] = React.useState( WsClientProviderStatus.DISCONNECTED, ); @@ -74,7 +90,7 @@ export function WsClientProvider({ } function handleMessage(event: Record) { - if (isOpenHandsMessage(event) && !isAgentStateChangeObservation(event)) { + if (isOpenHandsEvent(event) && isMessageAction(event)) { messageRateHandler.record(new Date().getTime()); } setEvents((prevEvents) => [...prevEvents, event]); @@ -100,6 +116,10 @@ export function WsClientProvider({ setStatus(WsClientProviderStatus.DISCONNECTED); } + React.useEffect(() => { + lastEventRef.current = null; + }, [conversationId]); + React.useEffect(() => { if (!conversationId) { throw new Error("No conversation ID provided"); @@ -118,9 +138,6 @@ export function WsClientProvider({ sio = io(baseUrl, { transports: ["websocket"], - auth: { - github_token: ghToken || undefined, - }, query, }); sio.on("connect", handleConnect); @@ -130,7 +147,6 @@ export function WsClientProvider({ sio.on("disconnect", handleDisconnect); sioRef.current = sio; - ghTokenRef.current = ghToken; return () => { sio.off("connect", handleConnect); @@ -139,7 +155,7 @@ export function WsClientProvider({ sio.off("connect_failed", handleError); sio.off("disconnect", handleDisconnect); }; - }, [ghToken, conversationId]); + }, [conversationId]); React.useEffect( () => () => { diff --git a/frontend/src/hooks/query/get-conversation-permissions.ts b/frontend/src/hooks/query/get-conversation-permissions.ts index 83002a0ec206..721c2f38d79d 100644 --- a/frontend/src/hooks/query/get-conversation-permissions.ts +++ b/frontend/src/hooks/query/get-conversation-permissions.ts @@ -1,11 +1,11 @@ import { useQuery } from "@tanstack/react-query"; import OpenHands from "#/api/open-hands"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; export const useUserConversation = (cid: string | null) => useQuery({ queryKey: ["user", "conversation", cid], queryFn: () => OpenHands.getConversation(cid!), - enabled: MULTI_CONVO_UI_IS_ENABLED && !!cid, + enabled: MULTI_CONVERSATION_UI && !!cid, retry: false, }); diff --git a/frontend/src/hooks/query/use-settings.ts b/frontend/src/hooks/query/use-settings.ts index f6e12e33e185..a4d430ccf88d 100644 --- a/frontend/src/hooks/query/use-settings.ts +++ b/frontend/src/hooks/query/use-settings.ts @@ -18,6 +18,8 @@ const getSettingsQueryFn = async () => { CONFIRMATION_MODE: apiSettings.confirmation_mode, SECURITY_ANALYZER: apiSettings.security_analyzer, LLM_API_KEY: apiSettings.llm_api_key, + REMOTE_RUNTIME_RESOURCE_FACTOR: + apiSettings.remote_runtime_resource_factor, }; } diff --git a/frontend/src/hooks/use-maybe-migrate-settings.ts b/frontend/src/hooks/use-maybe-migrate-settings.ts index 26892c9745d7..4f5bbd4712a0 100644 --- a/frontend/src/hooks/use-maybe-migrate-settings.ts +++ b/frontend/src/hooks/use-maybe-migrate-settings.ts @@ -3,8 +3,8 @@ import React from "react"; import { useSettingsUpToDate } from "#/context/settings-up-to-date-context"; import { - DEFAULT_SETTINGS, getCurrentSettingsVersion, + DEFAULT_SETTINGS, getLocalStorageSettings, } from "#/services/settings"; import { useSaveSettings } from "./mutation/use-save-settings"; diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index e3b3472103e6..389b3d87b4f8 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -763,6 +763,20 @@ "fr": "jours", "tr": "gün önce" }, + "SETTINGS_FORM$RUNTIME_SIZE_LABEL": { + "en": "Runtime Settings", + "zh-CN": "运行时设置", + "de": "Laufzeiteinstellungen", + "ko-KR": "런타임 설정", + "no": "Kjøretidsinnstillinger", + "zh-TW": "運行時設定", + "it": "Impostazioni Runtime", + "pt": "Configurações de Runtime", + "es": "Configuración de Runtime", + "ar": "إعدادات وقت التشغيل", + "fr": "Paramètres d'exécution", + "tr": "Çalışma Zamanı Ayarları" + }, "CONFIGURATION$SETTINGS_NEED_UPDATE_MESSAGE": { "en": "We've changed some settings in the latest update. Take a minute to review.", "de": "Mit dem letzten Update haben wir ein paar Einstellungen geändert. Bitte kontrollieren Ihre Einstellungen.", diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index 0ad90741247d..37894e2da38e 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -17,26 +17,30 @@ const userPreferences = { const conversations: Conversation[] = [ { conversation_id: "1", - name: "My New Project", - repo: null, - lastUpdated: new Date().toISOString(), - state: "running", + title: "My New Project", + selected_repository: null, + last_updated_at: new Date().toISOString(), + status: "RUNNING", }, { conversation_id: "2", - name: "Repo Testing", - repo: "octocat/hello-world", + title: "Repo Testing", + selected_repository: "octocat/hello-world", // 2 days ago - lastUpdated: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(), - state: "cold", + last_updated_at: new Date( + Date.now() - 2 * 24 * 60 * 60 * 1000, + ).toISOString(), + status: "STOPPED", }, { conversation_id: "3", - name: "Another Project", - repo: "octocat/earth", + title: "Another Project", + selected_repository: "octocat/earth", // 5 days ago - lastUpdated: new Date(Date.now() - 5 * 24 * 60 * 60 * 1000).toISOString(), - state: "finished", + last_updated_at: new Date( + Date.now() - 5 * 24 * 60 * 60 * 1000, + ).toISOString(), + status: "STOPPED", }, ]; @@ -182,8 +186,11 @@ export const handlers = [ http.get("/api/options/config", () => HttpResponse.json({ APP_MODE: "oss" })), - http.get("/api/conversations", async () => - HttpResponse.json(Array.from(CONVERSATIONS.values())), + http.get("/api/conversations?limit=9", async () => + HttpResponse.json({ + results: Array.from(CONVERSATIONS.values()), + next_page_id: null, + }), ), http.delete("/api/conversations/:conversationId", async ({ params }) => { @@ -197,7 +204,7 @@ export const handlers = [ return HttpResponse.json(null, { status: 404 }); }), - http.put( + http.patch( "/api/conversations/:conversationId", async ({ params, request }) => { const { conversationId } = params; @@ -207,10 +214,10 @@ export const handlers = [ if (conversation) { const body = await request.json(); - if (typeof body === "object" && body?.name) { + if (typeof body === "object" && body?.title) { CONVERSATIONS.set(conversationId, { ...conversation, - name: body.name, + title: body.title, }); return HttpResponse.json(null, { status: 200 }); } @@ -224,10 +231,10 @@ export const handlers = [ http.post("/api/conversations", () => { const conversation: Conversation = { conversation_id: (Math.random() * 100).toString(), - name: "New Conversation", - repo: null, - lastUpdated: new Date().toISOString(), - state: "warm", + title: "New Conversation", + selected_repository: null, + last_updated_at: new Date().toISOString(), + status: "RUNNING", }; CONVERSATIONS.set(conversation.conversation_id, conversation); diff --git a/frontend/src/routes/_oh.app/route.tsx b/frontend/src/routes/_oh.app/route.tsx index 73c50f5d8a7d..ab3384f951ec 100644 --- a/frontend/src/routes/_oh.app/route.tsx +++ b/frontend/src/routes/_oh.app/route.tsx @@ -34,7 +34,7 @@ import { useUserConversation } from "#/hooks/query/get-conversation-permissions" import { CountBadge } from "#/components/layout/count-badge"; import { TerminalStatusLabel } from "#/components/features/terminal/terminal-status-label"; import { useSettings } from "#/hooks/query/use-settings"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; function AppContent() { const { gitHubToken } = useAuth(); @@ -73,7 +73,7 @@ function AppContent() { ); React.useEffect(() => { - if (MULTI_CONVO_UI_IS_ENABLED && isFetched && !conversation) { + if (MULTI_CONVERSATION_UI && isFetched && !conversation) { toast.error( "This conversation does not exist, or you do not have permission to access it.", ); @@ -175,7 +175,7 @@ function AppContent() { } return ( - +
{renderMain()}
diff --git a/frontend/src/services/settings.ts b/frontend/src/services/settings.ts index b42d7f1042fc..bf2cfa8e9754 100644 --- a/frontend/src/services/settings.ts +++ b/frontend/src/services/settings.ts @@ -8,6 +8,7 @@ export type Settings = { LLM_API_KEY: string | null; CONFIRMATION_MODE: boolean; SECURITY_ANALYZER: string; + REMOTE_RUNTIME_RESOURCE_FACTOR: number; }; export type ApiSettings = { @@ -18,6 +19,7 @@ export type ApiSettings = { llm_api_key: string | null; confirmation_mode: boolean; security_analyzer: string; + remote_runtime_resource_factor: number; }; export const DEFAULT_SETTINGS: Settings = { @@ -28,6 +30,7 @@ export const DEFAULT_SETTINGS: Settings = { LLM_API_KEY: null, CONFIRMATION_MODE: false, SECURITY_ANALYZER: "", + REMOTE_RUNTIME_RESOURCE_FACTOR: 1, }; export const getCurrentSettingsVersion = () => { @@ -66,6 +69,8 @@ export const getLocalStorageSettings = (): Settings => { LLM_API_KEY: llmApiKey || DEFAULT_SETTINGS.LLM_API_KEY, CONFIRMATION_MODE: confirmationMode || DEFAULT_SETTINGS.CONFIRMATION_MODE, SECURITY_ANALYZER: securityAnalyzer || DEFAULT_SETTINGS.SECURITY_ANALYZER, + REMOTE_RUNTIME_RESOURCE_FACTOR: + DEFAULT_SETTINGS.REMOTE_RUNTIME_RESOURCE_FACTOR, }; }; @@ -73,3 +78,8 @@ export const getLocalStorageSettings = (): Settings => { * Get the default settings */ export const getDefaultSettings = (): Settings => DEFAULT_SETTINGS; + +/** + * Get the current settings, either from local storage or defaults + */ +export const getSettings = (): Settings => getLocalStorageSettings(); diff --git a/frontend/src/utils/constants.ts b/frontend/src/utils/constants.ts deleted file mode 100644 index 1cc654131b6d..000000000000 --- a/frontend/src/utils/constants.ts +++ /dev/null @@ -1 +0,0 @@ -export const MULTI_CONVO_UI_IS_ENABLED = false; diff --git a/frontend/src/utils/feature-flags.ts b/frontend/src/utils/feature-flags.ts new file mode 100644 index 000000000000..8cdf711aadf5 --- /dev/null +++ b/frontend/src/utils/feature-flags.ts @@ -0,0 +1,15 @@ +function loadFeatureFlag( + flagName: string, + defaultValue: boolean = false, +): boolean { + try { + const stringValue = + localStorage.getItem(`FEATURE_${flagName}`) || defaultValue.toString(); + const value = !!JSON.parse(stringValue); + return value; + } catch (e) { + return defaultValue; + } +} + +export const MULTI_CONVERSATION_UI = loadFeatureFlag("MULTI_CONVERSATION_UI"); diff --git a/openhands/core/config/sandbox_config.py b/openhands/core/config/sandbox_config.py index a91c8b06a32a..d0785a640a34 100644 --- a/openhands/core/config/sandbox_config.py +++ b/openhands/core/config/sandbox_config.py @@ -58,7 +58,7 @@ class SandboxConfig: runtime_startup_env_vars: dict[str, str] = field(default_factory=dict) browsergym_eval_env: str | None = None platform: str | None = None - close_delay: int = 15 + close_delay: int = 900 remote_runtime_resource_factor: int = 1 enable_gpu: bool = False diff --git a/openhands/events/stream.py b/openhands/events/stream.py index 63e464410643..e58f90e79d9c 100644 --- a/openhands/events/stream.py +++ b/openhands/events/stream.py @@ -1,9 +1,10 @@ import asyncio +import queue import threading from concurrent.futures import ThreadPoolExecutor from datetime import datetime from enum import Enum -from queue import Queue +from functools import partial from typing import Callable, Iterable from openhands.core.logger import openhands_logger as logger @@ -61,12 +62,19 @@ class EventStream: _subscribers: dict[str, dict[str, Callable]] _cur_id: int = 0 _lock: threading.Lock + _queue: queue.Queue[Event] + _queue_thread: threading.Thread + _queue_loop: asyncio.AbstractEventLoop | None + _thread_loops: dict[str, dict[str, asyncio.AbstractEventLoop]] - def __init__(self, sid: str, file_store: FileStore, num_workers: int = 1): + def __init__(self, sid: str, file_store: FileStore): self.sid = sid self.file_store = file_store - self._queue: Queue[Event] = Queue() + self._stop_flag = threading.Event() + self._queue: queue.Queue[Event] = queue.Queue() self._thread_pools: dict[str, dict[str, ThreadPoolExecutor]] = {} + self._thread_loops: dict[str, dict[str, asyncio.AbstractEventLoop]] = {} + self._queue_loop = None self._queue_thread = threading.Thread(target=self._run_queue_loop) self._queue_thread.daemon = True self._queue_thread.start() @@ -91,9 +99,54 @@ def __post_init__(self) -> None: if id >= self._cur_id: self._cur_id = id + 1 - def _init_thread_loop(self): + def _init_thread_loop(self, subscriber_id: str, callback_id: str): loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) + if subscriber_id not in self._thread_loops: + self._thread_loops[subscriber_id] = {} + self._thread_loops[subscriber_id][callback_id] = loop + + def close(self): + self._stop_flag.set() + if self._queue_thread.is_alive(): + self._queue_thread.join() + + subscriber_ids = list(self._subscribers.keys()) + for subscriber_id in subscriber_ids: + callback_ids = list(self._subscribers[subscriber_id].keys()) + for callback_id in callback_ids: + self._clean_up_subscriber(subscriber_id, callback_id) + + def _clean_up_subscriber(self, subscriber_id: str, callback_id: str): + if subscriber_id not in self._subscribers: + logger.warning(f'Subscriber not found during cleanup: {subscriber_id}') + return + if callback_id not in self._subscribers[subscriber_id]: + logger.warning(f'Callback not found during cleanup: {callback_id}') + return + if ( + subscriber_id in self._thread_loops + and callback_id in self._thread_loops[subscriber_id] + ): + loop = self._thread_loops[subscriber_id][callback_id] + try: + loop.stop() + loop.close() + except Exception as e: + logger.warning( + f'Error closing loop for {subscriber_id}/{callback_id}: {e}' + ) + del self._thread_loops[subscriber_id][callback_id] + + if ( + subscriber_id in self._thread_pools + and callback_id in self._thread_pools[subscriber_id] + ): + pool = self._thread_pools[subscriber_id][callback_id] + pool.shutdown() + del self._thread_pools[subscriber_id][callback_id] + + del self._subscribers[subscriber_id][callback_id] def _get_filename_for_id(self, id: int) -> str: return get_conversation_event_filename(self.sid, id) @@ -176,7 +229,8 @@ def get_latest_event_id(self) -> int: def subscribe( self, subscriber_id: EventStreamSubscriber, callback: Callable, callback_id: str ): - pool = ThreadPoolExecutor(max_workers=1, initializer=self._init_thread_loop) + initializer = partial(self._init_thread_loop, subscriber_id, callback_id) + pool = ThreadPoolExecutor(max_workers=1, initializer=initializer) if subscriber_id not in self._subscribers: self._subscribers[subscriber_id] = {} self._thread_pools[subscriber_id] = {} @@ -198,7 +252,7 @@ def unsubscribe(self, subscriber_id: EventStreamSubscriber, callback_id: str): logger.warning(f'Callback not found during unsubscribe: {callback_id}') return - del self._subscribers[subscriber_id][callback_id] + self._clean_up_subscriber(subscriber_id, callback_id) def add_event(self, event: Event, source: EventSource): if hasattr(event, '_id') and event.id is not None: @@ -217,13 +271,20 @@ def add_event(self, event: Event, source: EventSource): self._queue.put(event) def _run_queue_loop(self): - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - loop.run_until_complete(self._process_queue()) + self._queue_loop = asyncio.new_event_loop() + asyncio.set_event_loop(self._queue_loop) + try: + self._queue_loop.run_until_complete(self._process_queue()) + finally: + self._queue_loop.close() async def _process_queue(self): - while should_continue(): - event = self._queue.get() + while should_continue() and not self._stop_flag.is_set(): + event = None + try: + event = self._queue.get(timeout=0.1) + except queue.Empty: + continue for key in sorted(self._subscribers.keys()): callbacks = self._subscribers[key] for callback_id in callbacks: diff --git a/openhands/runtime/utils/bash.py b/openhands/runtime/utils/bash.py index 84c5d0265957..70a24e2189f7 100644 --- a/openhands/runtime/utils/bash.py +++ b/openhands/runtime/utils/bash.py @@ -1,6 +1,7 @@ import os import re import time +import traceback import uuid from enum import Enum @@ -23,11 +24,11 @@ def split_bash_commands(commands): return [''] try: parsed = bashlex.parse(commands) - except bashlex.errors.ParsingError as e: + except (bashlex.errors.ParsingError, NotImplementedError): logger.debug( f'Failed to parse bash commands\n' f'[input]: {commands}\n' - f'[warning]: {e}\n' + f'[warning]: {traceback.format_exc()}\n' f'The original command will be returned as is.' ) # If parsing fails, return the original commands @@ -143,9 +144,13 @@ def visit_node(node): remaining = command[last_pos:] parts.append(remaining) return ''.join(parts) - except bashlex.errors.ParsingError: - # Fallback if parsing fails - logger.warning(f'Failed to parse command: {command}') + except (bashlex.errors.ParsingError, NotImplementedError): + logger.debug( + f'Failed to parse bash commands for special characters escape\n' + f'[input]: {command}\n' + f'[warning]: {traceback.format_exc()}\n' + f'The original command will be returned as is.' + ) return command diff --git a/openhands/server/auth.py b/openhands/server/auth.py index 4b3fccdda7f1..b695cff89eab 100644 --- a/openhands/server/auth.py +++ b/openhands/server/auth.py @@ -1,9 +1,14 @@ import jwt +from fastapi import Request from jwt.exceptions import InvalidTokenError from openhands.core.logger import openhands_logger as logger +def get_user_id(request: Request) -> int: + return getattr(request.state, 'github_user_id', 0) + + def get_sid_from_token(token: str, jwt_secret: str) -> str: """Retrieves the session id from a JWT token. diff --git a/openhands/server/listen_socket.py b/openhands/server/listen_socket.py index 4bd7b8071960..4bb81aad6676 100644 --- a/openhands/server/listen_socket.py +++ b/openhands/server/listen_socket.py @@ -1,6 +1,6 @@ from urllib.parse import parse_qs -from github import Github +import jwt from socketio.exceptions import ConnectionRefusedError from openhands.core.logger import openhands_logger as logger @@ -18,7 +18,6 @@ from openhands.server.session.manager import ConversationDoesNotExistError from openhands.server.shared import config, openhands_config, session_manager, sio from openhands.server.types import AppMode -from openhands.utils.async_utils import call_sync_from_async @sio.event @@ -31,20 +30,20 @@ async def connect(connection_id: str, environ, auth): logger.error('No conversation_id in query params') raise ConnectionRefusedError('No conversation_id in query params') - github_token = '' + user_id = -1 if openhands_config.app_mode != AppMode.OSS: - user_id = '' - if auth and 'github_token' in auth: - github_token = auth['github_token'] - with Github(github_token) as g: - gh_user = await call_sync_from_async(g.get_user) - user_id = gh_user.id + cookies_str = environ.get('HTTP_COOKIE', '') + cookies = dict(cookie.split('=', 1) for cookie in cookies_str.split('; ')) + signed_token = cookies.get('github_auth', '') + if not signed_token: + logger.error('No github_auth cookie') + raise ConnectionRefusedError('No github_auth cookie') + decoded = jwt.decode(signed_token, config.jwt_secret, algorithms=['HS256']) + user_id = decoded['github_user_id'] logger.info(f'User {user_id} is connecting to conversation {conversation_id}') - conversation_store = await ConversationStoreImpl.get_instance( - config, github_token - ) + conversation_store = await ConversationStoreImpl.get_instance(config, user_id) metadata = await conversation_store.get_metadata(conversation_id) if metadata.github_user_id != user_id: logger.error( @@ -54,7 +53,7 @@ async def connect(connection_id: str, environ, auth): f'User {user_id} is not allowed to join conversation {conversation_id}' ) - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance(config, user_id) settings = await settings_store.load() if not settings: diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 0d375229a50b..235b5801f24d 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -4,11 +4,11 @@ from fastapi import APIRouter, Body, Request from fastapi.responses import JSONResponse -from github import Github from pydantic import BaseModel from openhands.core.logger import openhands_logger as logger from openhands.events.stream import EventStreamSubscriber +from openhands.server.auth import get_user_id from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager @@ -21,7 +21,6 @@ from openhands.utils.async_utils import ( GENERAL_TIMEOUT, call_async_from_sync, - call_sync_from_async, wait_all, ) @@ -43,10 +42,9 @@ async def new_conversation(request: Request, data: InitSessionRequest): using the returned conversation ID """ logger.info('Initializing new conversation') - github_token = data.github_token or '' logger.info('Loading settings') - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance(config, get_user_id(request)) settings = await settings_store.load() logger.info('Settings loaded') @@ -54,11 +52,14 @@ async def new_conversation(request: Request, data: InitSessionRequest): if settings: session_init_args = {**settings.__dict__, **session_init_args} + github_token = getattr(request.state, 'github_token', '') session_init_args['github_token'] = github_token session_init_args['selected_repository'] = data.selected_repository conversation_init_data = ConversationInitData(**session_init_args) logger.info('Loading conversation store') - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) logger.info('Conversation store loaded') conversation_id = uuid.uuid4().hex @@ -67,18 +68,11 @@ async def new_conversation(request: Request, data: InitSessionRequest): conversation_id = uuid.uuid4().hex logger.info(f'New conversation ID: {conversation_id}') - user_id = '' - if data.github_token: - logger.info('Fetching Github user ID') - with Github(data.github_token) as g: - gh_user = await call_sync_from_async(g.get_user) - user_id = gh_user.id - logger.info(f'Saving metadata for conversation {conversation_id}') await conversation_store.save_metadata( ConversationMetadata( conversation_id=conversation_id, - github_user_id=user_id, + github_user_id=get_user_id(request), selected_repository=data.selected_repository, ) ) @@ -90,9 +84,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): try: event_stream.subscribe( EventStreamSubscriber.SERVER, - _create_conversation_update_callback( - data.github_token or '', conversation_id - ), + _create_conversation_update_callback(get_user_id(request), conversation_id), UPDATED_AT_CALLBACK_ID, ) except ValueError: @@ -107,8 +99,9 @@ async def search_conversations( page_id: str | None = None, limit: int = 20, ) -> ConversationInfoResultSet: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) conversation_metadata_result_set = await conversation_store.search(page_id, limit) conversation_ids = set( conversation.conversation_id @@ -134,8 +127,9 @@ async def search_conversations( async def get_conversation( conversation_id: str, request: Request ) -> ConversationInfo | None: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) try: metadata = await conversation_store.get_metadata(conversation_id) is_running = await session_manager.is_agent_loop_running(conversation_id) @@ -149,8 +143,9 @@ async def get_conversation( async def update_conversation( request: Request, conversation_id: str, title: str = Body(embed=True) ) -> bool: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) metadata = await conversation_store.get_metadata(conversation_id) if not metadata: return False @@ -164,15 +159,16 @@ async def delete_conversation( conversation_id: str, request: Request, ) -> bool: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) try: await conversation_store.get_metadata(conversation_id) except FileNotFoundError: return False is_running = await session_manager.is_agent_loop_running(conversation_id) if is_running: - return False + await session_manager.close_session(conversation_id) await conversation_store.delete_metadata(conversation_id) return True @@ -189,6 +185,7 @@ async def _get_conversation_info( conversation_id=conversation.conversation_id, title=title, last_updated_at=conversation.last_updated_at, + created_at=conversation.created_at, selected_repository=conversation.selected_repository, status=ConversationStatus.RUNNING if is_running @@ -204,21 +201,21 @@ async def _get_conversation_info( def _create_conversation_update_callback( - github_token: str, conversation_id: str + user_id: int, conversation_id: str ) -> Callable: def callback(*args, **kwargs): call_async_from_sync( _update_timestamp_for_conversation, GENERAL_TIMEOUT, - github_token, + user_id, conversation_id, ) return callback -async def _update_timestamp_for_conversation(github_token: str, conversation_id: str): - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) +async def _update_timestamp_for_conversation(user_id: int, conversation_id: str): + conversation_store = await ConversationStoreImpl.get_instance(config, user_id) conversation = await conversation_store.get_metadata(conversation_id) conversation.last_updated_at = datetime.now() await conversation_store.save_metadata(conversation) diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 4fcdb42f02ba..ca45c142ff1c 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -2,6 +2,7 @@ from fastapi.responses import JSONResponse from openhands.core.logger import openhands_logger as logger +from openhands.server.auth import get_user_id from openhands.server.settings import Settings from openhands.server.shared import config, openhands_config from openhands.storage.conversation.conversation_store import ConversationStore @@ -19,9 +20,10 @@ @app.get('/settings') async def load_settings(request: Request) -> Settings | None: - github_token = getattr(request.state, 'github_token', '') or '' try: - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance( + config, get_user_id(request) + ) settings = await settings_store.load() if not settings: return JSONResponse( @@ -45,17 +47,23 @@ async def store_settings( request: Request, settings: Settings, ) -> JSONResponse: - github_token = '' - if hasattr(request.state, 'github_token'): - github_token = request.state.github_token try: - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance( + config, get_user_id(request) + ) existing_settings = await settings_store.load() if existing_settings: # LLM key isn't on the frontend, so we need to keep it if unset if settings.llm_api_key is None: settings.llm_api_key = existing_settings.llm_api_key + + # Update sandbox config with new settings + if settings.remote_runtime_resource_factor is not None: + config.sandbox.remote_runtime_resource_factor = ( + settings.remote_runtime_resource_factor + ) + await settings_store.store(settings) return JSONResponse( diff --git a/openhands/server/session/agent_session.py b/openhands/server/session/agent_session.py index 91e3921596a5..9f761a9298be 100644 --- a/openhands/server/session/agent_session.py +++ b/openhands/server/session/agent_session.py @@ -131,6 +131,8 @@ async def _close(self): f'Waited too long for initialization to finish before closing session {self.sid}' ) break + if self.event_stream is not None: + self.event_stream.close() if self.controller is not None: end_state = self.controller.get_state() end_state.save_to_session(self.sid, self.file_store) diff --git a/openhands/server/session/conversation.py b/openhands/server/session/conversation.py index 11fdb22d8632..14aa1363e63b 100644 --- a/openhands/server/session/conversation.py +++ b/openhands/server/session/conversation.py @@ -43,4 +43,6 @@ async def connect(self): await self.runtime.connect() async def disconnect(self): + if self.event_stream: + self.event_stream.close() asyncio.create_task(call_sync_from_async(self.runtime.close)) diff --git a/openhands/server/session/manager.py b/openhands/server/session/manager.py index 60b5bd2675af..c7577cc1b558 100644 --- a/openhands/server/session/manager.py +++ b/openhands/server/session/manager.py @@ -156,6 +156,10 @@ async def _process_message(self, message: dict): flag = self._has_remote_connections_flags.get(sid) if flag: flag.set() + elif message_type == 'close_session': + sid = data['sid'] + if sid in self._local_agent_loops_by_sid: + await self._on_close_session(sid) elif message_type == 'session_closing': # Session closing event - We only get this in the event of graceful shutdown, # which can't be guaranteed - nodes can simply vanish unexpectedly! @@ -197,6 +201,7 @@ async def attach_to_conversation(self, sid: str) -> Conversation | None: await c.connect() except AgentRuntimeUnavailableError as e: logger.error(f'Error connecting to conversation {c.sid}: {e}') + await c.disconnect() return None end_time = time.time() logger.info( @@ -419,7 +424,7 @@ async def disconnect_from_session(self, connection_id: str): if should_continue(): asyncio.create_task(self._cleanup_session_later(sid)) else: - await self._close_session(sid) + await self._on_close_session(sid) async def _cleanup_session_later(self, sid: str): # Once there have been no connections to a session for a reasonable period, we close it @@ -451,10 +456,22 @@ async def _cleanup_session(self, sid: str) -> bool: json.dumps({'sid': sid, 'message_type': 'session_closing'}), ) - await self._close_session(sid) + await self._on_close_session(sid) return True - async def _close_session(self, sid: str): + async def close_session(self, sid: str): + session = self._local_agent_loops_by_sid.get(sid) + if session: + await self._on_close_session(sid) + + redis_client = self._get_redis_client() + if redis_client: + await redis_client.publish( + 'oh_event', + json.dumps({'sid': sid, 'message_type': 'close_session'}), + ) + + async def _on_close_session(self, sid: str): logger.info(f'_close_session:{sid}') # Clear up local variables diff --git a/openhands/server/settings.py b/openhands/server/settings.py index e78694c6ca32..57c879e49d45 100644 --- a/openhands/server/settings.py +++ b/openhands/server/settings.py @@ -15,3 +15,4 @@ class Settings: llm_model: str | None = None llm_api_key: str | None = None llm_base_url: str | None = None + remote_runtime_resource_factor: int | None = None diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index 2a09322574bf..1f0b41ea6c87 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -40,7 +40,5 @@ async def search( @classmethod @abstractmethod - async def get_instance( - cls, config: AppConfig, token: str | None - ) -> ConversationStore: + async def get_instance(cls, config: AppConfig, user_id: int) -> ConversationStore: """Get a store for the user represented by the token given""" diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 0e8983c01405..622b72f4bbcf 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -90,13 +90,15 @@ def get_conversation_metadata_filename(self, conversation_id: str) -> str: return get_conversation_metadata_filename(conversation_id) @classmethod - async def get_instance(cls, config: AppConfig, token: str | None): + async def get_instance( + cls, config: AppConfig, user_id: int + ) -> FileConversationStore: file_store = get_file_store(config.file_store, config.file_store_path) return FileConversationStore(file_store) def _sort_key(conversation: ConversationMetadata) -> str: - last_updated_at = conversation.last_updated_at - if last_updated_at: - return last_updated_at.isoformat() # YYYY-MM-DDTHH:MM:SS for sorting + created_at = conversation.created_at + if created_at: + return created_at.isoformat() # YYYY-MM-DDTHH:MM:SS for sorting return '' diff --git a/openhands/storage/data_models/conversation_info.py b/openhands/storage/data_models/conversation_info.py index cda7a2653bff..52464ec30f85 100644 --- a/openhands/storage/data_models/conversation_info.py +++ b/openhands/storage/data_models/conversation_info.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import datetime from openhands.storage.data_models.conversation_status import ConversationStatus @@ -13,3 +13,4 @@ class ConversationInfo: last_updated_at: datetime | None = None status: ConversationStatus = ConversationStatus.STOPPED selected_repository: str | None = None + created_at: datetime = field(default_factory=datetime.now) diff --git a/openhands/storage/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py index 21c84be99eba..e75bbf21f8d5 100644 --- a/openhands/storage/data_models/conversation_metadata.py +++ b/openhands/storage/data_models/conversation_metadata.py @@ -1,11 +1,12 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import datetime @dataclass class ConversationMetadata: conversation_id: str - github_user_id: int | str + github_user_id: int selected_repository: str | None title: str | None = None last_updated_at: datetime | None = None + created_at: datetime = field(default_factory=datetime.now) diff --git a/openhands/storage/settings/file_settings_store.py b/openhands/storage/settings/file_settings_store.py index 413376759c8d..c8703b304c11 100644 --- a/openhands/storage/settings/file_settings_store.py +++ b/openhands/storage/settings/file_settings_store.py @@ -30,6 +30,6 @@ async def store(self, settings: Settings): await call_sync_from_async(self.file_store.write, self.path, json_str) @classmethod - async def get_instance(cls, config: AppConfig, token: str | None): + async def get_instance(cls, config: AppConfig, user_id: int) -> FileSettingsStore: file_store = get_file_store(config.file_store, config.file_store_path) return FileSettingsStore(file_store) diff --git a/openhands/storage/settings/settings_store.py b/openhands/storage/settings/settings_store.py index 6e369b8f9739..a371720600ac 100644 --- a/openhands/storage/settings/settings_store.py +++ b/openhands/storage/settings/settings_store.py @@ -21,5 +21,5 @@ async def store(self, settings: Settings): @classmethod @abstractmethod - async def get_instance(cls, config: AppConfig, token: str | None) -> SettingsStore: + async def get_instance(cls, config: AppConfig, user_id: int) -> SettingsStore: """Get a store for the user represented by the token given""" diff --git a/tests/runtime/test_bash.py b/tests/runtime/test_bash.py index b97cdbc9266b..75f9085815fa 100644 --- a/tests/runtime/test_bash.py +++ b/tests/runtime/test_bash.py @@ -153,6 +153,20 @@ def test_multiple_multiline_commands(temp_dir, runtime_cls, run_as_openhands): _close_test_runtime(runtime) +def test_complex_commands(temp_dir, runtime_cls): + cmd = """count=0; tries=0; while [ $count -lt 3 ]; do result=$(echo "Heads"); tries=$((tries+1)); echo "Flip $tries: $result"; if [ "$result" = "Heads" ]; then count=$((count+1)); else count=0; fi; done; echo "Got 3 heads in a row after $tries flips!";""" + + runtime = _load_runtime(temp_dir, runtime_cls) + try: + obs = _run_cmd_action(runtime, cmd) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + assert obs.exit_code == 0, 'The exit code should be 0.' + assert 'Got 3 heads in a row after 3 flips!' in obs.content + + finally: + _close_test_runtime(runtime) + + def test_no_ps2_in_output(temp_dir, runtime_cls, run_as_openhands): """Test that the PS2 sign is not added to the output of a multiline command.""" runtime = _load_runtime(temp_dir, runtime_cls, run_as_openhands) diff --git a/tests/runtime/test_stress_docker_runtime.py b/tests/runtime/test_stress_docker_runtime.py new file mode 100644 index 000000000000..e7d196d48207 --- /dev/null +++ b/tests/runtime/test_stress_docker_runtime.py @@ -0,0 +1,35 @@ +"""Stress tests for the DockerRuntime, which connects to the ActionExecutor running in the sandbox.""" + +import pytest +from conftest import TEST_IN_CI, _close_test_runtime, _load_runtime + +from openhands.core.logger import openhands_logger as logger +from openhands.events.action import CmdRunAction + + +@pytest.mark.skipif( + TEST_IN_CI, + reason='This test should only be run locally, not in CI.', +) +def test_stress_docker_runtime(temp_dir, runtime_cls, repeat=1): + runtime = _load_runtime(temp_dir, runtime_cls) + + action = CmdRunAction( + command='sudo apt-get update && sudo apt-get install -y stress-ng' + ) + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + assert obs.exit_code == 0 + + for _ in range(repeat): + # run stress-ng stress tests for 5 minutes + # FIXME: this would make Docker daemon die, even though running this + # command on its own in the same container is fine + action = CmdRunAction(command='stress-ng --all 1 -t 5m') + action.timeout = 600 + logger.info(action, extra={'msg_type': 'ACTION'}) + obs = runtime.run_action(action) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + + _close_test_runtime(runtime) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 709cb4cae65f..91731a601d6d 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -28,8 +28,9 @@ def _patch_store(): 'title': 'Some Conversation', 'selected_repository': 'foobar', 'conversation_id': 'some_conversation_id', - 'github_user_id': 'github_user', - 'last_updated_at': '2025-01-01T00:00:00', + 'github_user_id': 12345, + 'created_at': '2025-01-01T00:00:00', + 'last_updated_at': '2025-01-01T00:01:00', } ), ) @@ -55,7 +56,8 @@ async def test_search_conversations(): ConversationInfo( conversation_id='some_conversation_id', title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) @@ -73,7 +75,8 @@ async def test_get_conversation(): expected = ConversationInfo( conversation_id='some_conversation_id', title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) @@ -105,7 +108,8 @@ async def test_update_conversation(): expected = ConversationInfo( conversation_id='some_conversation_id', title='New Title', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) diff --git a/tests/unit/test_manager.py b/tests/unit/test_manager.py index c2a61104a864..144f79f9f491 100644 --- a/tests/unit/test_manager.py +++ b/tests/unit/test_manager.py @@ -286,7 +286,7 @@ async def test_cleanup_session_connections(): } ) - await session_manager._close_session('session1') + await session_manager._on_close_session('session1') remaining_connections = session_manager.local_connection_id_to_session_id assert 'conn1' not in remaining_connections diff --git a/tests/unit/test_settings_api.py b/tests/unit/test_settings_api.py new file mode 100644 index 000000000000..a8e52a239010 --- /dev/null +++ b/tests/unit/test_settings_api.py @@ -0,0 +1,85 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from fastapi.testclient import TestClient + +from openhands.core.config.sandbox_config import SandboxConfig +from openhands.server.app import app +from openhands.server.settings import Settings + + +@pytest.fixture +def test_client(): + # Mock the middleware that adds github_token + class MockMiddleware: + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if scope['type'] == 'http': + scope['state'] = {'github_token': 'test-token'} + await self.app(scope, receive, send) + + # Replace the middleware + app.middleware_stack = None # Clear existing middleware + app.add_middleware(MockMiddleware) + + return TestClient(app) + + +@pytest.fixture +def mock_settings_store(): + with patch('openhands.server.routes.settings.SettingsStoreImpl') as mock: + store_instance = MagicMock() + mock.get_instance = AsyncMock(return_value=store_instance) + store_instance.load = AsyncMock() + store_instance.store = AsyncMock() + yield store_instance + + +@pytest.mark.asyncio +async def test_settings_api_runtime_factor(test_client, mock_settings_store): + # Mock the settings store to return None initially (no existing settings) + mock_settings_store.load.return_value = None + + # Test data with remote_runtime_resource_factor + settings_data = { + 'language': 'en', + 'agent': 'test-agent', + 'max_iterations': 100, + 'security_analyzer': 'default', + 'confirmation_mode': True, + 'llm_model': 'test-model', + 'llm_api_key': None, + 'llm_base_url': 'https://test.com', + 'remote_runtime_resource_factor': 2, + } + + # The test_client fixture already handles authentication + + # Make the POST request to store settings + response = test_client.post('/api/settings', json=settings_data) + assert response.status_code == 200 + + # Verify the settings were stored with the correct runtime factor + stored_settings = mock_settings_store.store.call_args[0][0] + assert stored_settings.remote_runtime_resource_factor == 2 + + # Mock settings store to return our settings for the GET request + mock_settings_store.load.return_value = Settings(**settings_data) + + # Make a GET request to retrieve settings + response = test_client.get('/api/settings') + assert response.status_code == 200 + assert response.json()['remote_runtime_resource_factor'] == 2 + + # Verify that the sandbox config gets updated when settings are loaded + with patch('openhands.server.shared.config') as mock_config: + mock_config.sandbox = SandboxConfig() + response = test_client.get('/api/settings') + assert response.status_code == 200 + + # Verify that the sandbox config was updated with the new value + mock_settings_store.store.assert_called() + stored_settings = mock_settings_store.store.call_args[0][0] + assert stored_settings.remote_runtime_resource_factor == 2