From 8e75f251085b6014e8fb9baae0deaf642d5acd90 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Wed, 12 Nov 2025 22:59:45 +0700 Subject: [PATCH 001/108] feat(frontend): implement new task tracker interface (#11692) --- ...task-tracking-observation-content.test.tsx | 54 +++++++--------- .../task-tracking-event-message.tsx | 27 +------- .../task-tracking-observation-content.tsx | 6 -- .../chat/task-tracking/result-section.tsx | 21 ------ .../features/chat/task-tracking/task-item.tsx | 64 ++++++++++++------- .../chat/task-tracking/task-list-section.tsx | 27 ++++---- .../get-event-content.tsx | 19 +++++- .../finish-event-message.tsx | 13 ++-- .../generic-event-message-wrapper.tsx | 7 ++ .../v1/chat/task-tracking/task-item.tsx | 56 ++++++++++++++++ .../chat/task-tracking/task-list-section.tsx | 33 ++++++++++ .../task-tracking-observation-content.tsx | 23 +++++++ frontend/src/i18n/declaration.ts | 1 + frontend/src/i18n/translation.json | 16 +++++ frontend/src/icons/loading.svg | 3 + frontend/src/icons/u-check-circle.svg | 3 + frontend/src/icons/u-circle.svg | 3 + 17 files changed, 249 insertions(+), 127 deletions(-) delete mode 100644 frontend/src/components/features/chat/task-tracking/result-section.tsx create mode 100644 frontend/src/components/v1/chat/task-tracking/task-item.tsx create mode 100644 frontend/src/components/v1/chat/task-tracking/task-list-section.tsx create mode 100644 frontend/src/components/v1/chat/task-tracking/task-tracking-observation-content.tsx create mode 100644 frontend/src/icons/loading.svg create mode 100644 frontend/src/icons/u-check-circle.svg create mode 100644 frontend/src/icons/u-circle.svg diff --git a/frontend/__tests__/components/features/chat/task-tracking-observation-content.test.tsx b/frontend/__tests__/components/features/chat/task-tracking-observation-content.test.tsx index e44c33ca7d93..5db3942aa28b 100644 --- a/frontend/__tests__/components/features/chat/task-tracking-observation-content.test.tsx +++ b/frontend/__tests__/components/features/chat/task-tracking-observation-content.test.tsx @@ -8,10 +8,11 @@ vi.mock("react-i18next", () => ({ useTranslation: () => ({ t: (key: string) => { const translations: Record = { - "TASK_TRACKING_OBSERVATION$TASK_LIST": "Task List", - "TASK_TRACKING_OBSERVATION$TASK_ID": "ID", - "TASK_TRACKING_OBSERVATION$TASK_NOTES": "Notes", - "TASK_TRACKING_OBSERVATION$RESULT": "Result", + TASK_TRACKING_OBSERVATION$TASK_LIST: "Task List", + TASK_TRACKING_OBSERVATION$TASK_ID: "ID", + TASK_TRACKING_OBSERVATION$TASK_NOTES: "Notes", + TASK_TRACKING_OBSERVATION$RESULT: "Result", + COMMON$TASKS: "Tasks", }; return translations[key] || key; }, @@ -61,19 +62,26 @@ describe("TaskTrackingObservationContent", () => { it("renders task list when command is 'plan' and tasks exist", () => { render(); - expect(screen.getByText("Task List (3 items)")).toBeInTheDocument(); + expect(screen.getByText("Tasks")).toBeInTheDocument(); expect(screen.getByText("Implement feature A")).toBeInTheDocument(); expect(screen.getByText("Fix bug B")).toBeInTheDocument(); expect(screen.getByText("Deploy to production")).toBeInTheDocument(); }); it("displays correct status icons and badges", () => { - render(); + const { container } = render( + , + ); + + // Status is represented by icons, not text. Verify task items are rendered with their titles + // which indicates the status icons are present (status affects icon rendering) + expect(screen.getByText("Implement feature A")).toBeInTheDocument(); + expect(screen.getByText("Fix bug B")).toBeInTheDocument(); + expect(screen.getByText("Deploy to production")).toBeInTheDocument(); - // Check for status text (the icons are emojis) - expect(screen.getByText("todo")).toBeInTheDocument(); - expect(screen.getByText("in progress")).toBeInTheDocument(); - expect(screen.getByText("done")).toBeInTheDocument(); + // Verify task items are present (they contain the status icons) + const taskItems = container.querySelectorAll('[data-name="item"]'); + expect(taskItems).toHaveLength(3); }); it("displays task IDs and notes", () => { @@ -84,14 +92,9 @@ describe("TaskTrackingObservationContent", () => { expect(screen.getByText("ID: task-3")).toBeInTheDocument(); expect(screen.getByText("Notes: This is a test task")).toBeInTheDocument(); - expect(screen.getByText("Notes: Completed successfully")).toBeInTheDocument(); - }); - - it("renders result section when content exists", () => { - render(); - - expect(screen.getByText("Result")).toBeInTheDocument(); - expect(screen.getByText("Task tracking operation completed successfully")).toBeInTheDocument(); + expect( + screen.getByText("Notes: Completed successfully"), + ).toBeInTheDocument(); }); it("does not render task list when command is not 'plan'", () => { @@ -105,7 +108,7 @@ describe("TaskTrackingObservationContent", () => { render(); - expect(screen.queryByText("Task List")).not.toBeInTheDocument(); + expect(screen.queryByText("Tasks")).not.toBeInTheDocument(); }); it("does not render task list when task list is empty", () => { @@ -119,17 +122,6 @@ describe("TaskTrackingObservationContent", () => { render(); - expect(screen.queryByText("Task List")).not.toBeInTheDocument(); - }); - - it("does not render result section when content is empty", () => { - const eventWithoutContent = { - ...mockEvent, - content: "", - }; - - render(); - - expect(screen.queryByText("Result")).not.toBeInTheDocument(); + expect(screen.queryByText("Tasks")).not.toBeInTheDocument(); }); }); diff --git a/frontend/src/components/features/chat/event-message-components/task-tracking-event-message.tsx b/frontend/src/components/features/chat/event-message-components/task-tracking-event-message.tsx index 785305333c63..cd6ff59a0549 100644 --- a/frontend/src/components/features/chat/event-message-components/task-tracking-event-message.tsx +++ b/frontend/src/components/features/chat/event-message-components/task-tracking-event-message.tsx @@ -1,11 +1,7 @@ -import React from "react"; -import { useTranslation } from "react-i18next"; import { OpenHandsObservation } from "#/types/core/observations"; import { isTaskTrackingObservation } from "#/types/core/guards"; -import { GenericEventMessage } from "../generic-event-message"; import { TaskTrackingObservationContent } from "../task-tracking-observation-content"; import { ConfirmationButtons } from "#/components/shared/buttons/confirmation-buttons"; -import { getObservationResult } from "../event-content-helpers/get-observation-result"; interface TaskTrackingEventMessageProps { event: OpenHandsObservation; @@ -16,34 +12,13 @@ export function TaskTrackingEventMessage({ event, shouldShowConfirmationButtons, }: TaskTrackingEventMessageProps) { - const { t } = useTranslation(); - if (!isTaskTrackingObservation(event)) { return null; } - const { command } = event.extras; - let title: React.ReactNode; - let initiallyExpanded = false; - - // Determine title and expansion state based on command - if (command === "plan") { - title = t("OBSERVATION_MESSAGE$TASK_TRACKING_PLAN"); - initiallyExpanded = true; - } else { - // command === "view" - title = t("OBSERVATION_MESSAGE$TASK_TRACKING_VIEW"); - initiallyExpanded = false; - } - return (
- } - success={getObservationResult(event)} - initiallyExpanded={initiallyExpanded} - /> + {shouldShowConfirmationButtons && }
); diff --git a/frontend/src/components/features/chat/task-tracking-observation-content.tsx b/frontend/src/components/features/chat/task-tracking-observation-content.tsx index e4dd95c2bfa2..7d9e7ff1467a 100644 --- a/frontend/src/components/features/chat/task-tracking-observation-content.tsx +++ b/frontend/src/components/features/chat/task-tracking-observation-content.tsx @@ -1,6 +1,5 @@ import { TaskTrackingObservation } from "#/types/core/observations"; import { TaskListSection } from "./task-tracking/task-list-section"; -import { ResultSection } from "./task-tracking/result-section"; interface TaskTrackingObservationContentProps { event: TaskTrackingObservation; @@ -16,11 +15,6 @@ export function TaskTrackingObservationContent({
{/* Task List section - only show for 'plan' command */} {shouldShowTaskList && } - - {/* Result message - only show if there's meaningful content */} - {event.content && event.content.trim() && ( - - )}
); } diff --git a/frontend/src/components/features/chat/task-tracking/result-section.tsx b/frontend/src/components/features/chat/task-tracking/result-section.tsx deleted file mode 100644 index 0cd06e3a4a96..000000000000 --- a/frontend/src/components/features/chat/task-tracking/result-section.tsx +++ /dev/null @@ -1,21 +0,0 @@ -import { useTranslation } from "react-i18next"; -import { Typography } from "#/ui/typography"; - -interface ResultSectionProps { - content: string; -} - -export function ResultSection({ content }: ResultSectionProps) { - const { t } = useTranslation(); - - return ( -
-
- {t("TASK_TRACKING_OBSERVATION$RESULT")} -
-
- 
{content.trim()}
-
-
- ); -} diff --git a/frontend/src/components/features/chat/task-tracking/task-item.tsx b/frontend/src/components/features/chat/task-tracking/task-item.tsx index 923ed8ea3f51..72e9e74aacea 100644 --- a/frontend/src/components/features/chat/task-tracking/task-item.tsx +++ b/frontend/src/components/features/chat/task-tracking/task-item.tsx @@ -1,7 +1,11 @@ +import { useMemo } from "react"; import { useTranslation } from "react-i18next"; +import CircleIcon from "#/icons/u-circle.svg?react"; +import CheckCircleIcon from "#/icons/u-check-circle.svg?react"; +import LoadingIcon from "#/icons/loading.svg?react"; +import { I18nKey } from "#/i18n/declaration"; +import { cn } from "#/utils/utils"; import { Typography } from "#/ui/typography"; -import { StatusIcon } from "./status-icon"; -import { StatusBadge } from "./status-badge"; interface TaskItemProps { task: { @@ -10,33 +14,47 @@ interface TaskItemProps { status: "todo" | "in_progress" | "done"; notes?: string; }; - index: number; } -export function TaskItem({ task, index }: TaskItemProps) { +export function TaskItem({ task }: TaskItemProps) { const { t } = useTranslation(); + const icon = useMemo(() => { + switch (task.status) { + case "todo": + return ; + case "in_progress": + return ; + case "done": + return ; + default: + return ; + } + }, [task.status]); + + const isDoneStatus = task.status === "done"; + return ( -
-
- -
-
- - {index + 1}. - - -
-

{task.title}

- - {t("TASK_TRACKING_OBSERVATION$TASK_ID")}: {task.id} - - {task.notes && ( - - {t("TASK_TRACKING_OBSERVATION$TASK_NOTES")}: {task.notes} - +
+
{icon}
+
+ + > + {task.title} + + + {t(I18nKey.TASK_TRACKING_OBSERVATION$TASK_ID)}: {task.id} + + + {t(I18nKey.TASK_TRACKING_OBSERVATION$TASK_NOTES)}: {task.notes} +
); diff --git a/frontend/src/components/features/chat/task-tracking/task-list-section.tsx b/frontend/src/components/features/chat/task-tracking/task-list-section.tsx index 912920252242..075517aacd1d 100644 --- a/frontend/src/components/features/chat/task-tracking/task-list-section.tsx +++ b/frontend/src/components/features/chat/task-tracking/task-list-section.tsx @@ -1,5 +1,7 @@ import { useTranslation } from "react-i18next"; import { TaskItem } from "./task-item"; +import LessonPlanIcon from "#/icons/lesson-plan.svg?react"; +import { I18nKey } from "#/i18n/declaration"; import { Typography } from "#/ui/typography"; interface TaskListSectionProps { @@ -15,19 +17,20 @@ export function TaskListSection({ taskList }: TaskListSectionProps) { const { t } = useTranslation(); return ( -
-
- - {t("TASK_TRACKING_OBSERVATION$TASK_LIST")} ({taskList.length}{" "} - {taskList.length === 1 ? "item" : "items"}) - +
+ {/* Header Tabs */} +
+ + + {t(I18nKey.COMMON$TASKS)} +
-
-
- {taskList.map((task, index) => ( - - ))} -
+ + {/* Task Items */} +
+ {taskList.map((task) => ( + + ))}
); diff --git a/frontend/src/components/v1/chat/event-content-helpers/get-event-content.tsx b/frontend/src/components/v1/chat/event-content-helpers/get-event-content.tsx index b2e7d69868d8..7eab7df1a77d 100644 --- a/frontend/src/components/v1/chat/event-content-helpers/get-event-content.tsx +++ b/frontend/src/components/v1/chat/event-content-helpers/get-event-content.tsx @@ -1,10 +1,13 @@ import { Trans } from "react-i18next"; -import { OpenHandsEvent } from "#/types/v1/core"; +import React from "react"; +import { OpenHandsEvent, ObservationEvent } from "#/types/v1/core"; import { isActionEvent, isObservationEvent } from "#/types/v1/type-guards"; import { MonoComponent } from "../../../features/chat/mono-component"; import { PathComponent } from "../../../features/chat/path-component"; import { getActionContent } from "./get-action-content"; import { getObservationContent } from "./get-observation-content"; +import { TaskTrackingObservationContent } from "../task-tracking/task-tracking-observation-content"; +import { TaskTrackerObservation } from "#/types/v1/core/base/observation"; import i18n from "#/i18n"; const trimText = (text: string, maxLength: number): string => { @@ -158,14 +161,24 @@ const getObservationEventTitle = (event: OpenHandsEvent): React.ReactNode => { export const getEventContent = (event: OpenHandsEvent) => { let title: React.ReactNode = ""; - let details: string = ""; + let details: string | React.ReactNode = ""; if (isActionEvent(event)) { title = getActionEventTitle(event); details = getActionContent(event); } else if (isObservationEvent(event)) { title = getObservationEventTitle(event); - details = getObservationContent(event); + + // For TaskTrackerObservation, use React component instead of markdown + if (event.observation.kind === "TaskTrackerObservation") { + details = ( + } + /> + ); + } else { + details = getObservationContent(event); + } } return { diff --git a/frontend/src/components/v1/chat/event-message-components/finish-event-message.tsx b/frontend/src/components/v1/chat/event-message-components/finish-event-message.tsx index 6ad385e8f095..3a30b0c4345a 100644 --- a/frontend/src/components/v1/chat/event-message-components/finish-event-message.tsx +++ b/frontend/src/components/v1/chat/event-message-components/finish-event-message.tsx @@ -27,13 +27,16 @@ export function FinishEventMessage({ microagentPRUrl, actions, }: FinishEventMessageProps) { + const eventContent = getEventContent(event); + // For FinishAction, details is always a string (getActionContent returns string) + const message = + typeof eventContent.details === "string" + ? eventContent.details + : String(eventContent.details); + return ( <> - + {details}
; + } + return (
{ + switch (task.status) { + case "todo": + return ; + case "in_progress": + return ( + + ); + case "done": + return ; + default: + return ; + } + }, [task.status]); + + const isDoneStatus = task.status === "done"; + + return ( +
+
{icon}
+
+ + {task.title} + + + {t(I18nKey.TASK_TRACKING_OBSERVATION$TASK_NOTES)}: {task.notes} + +
+
+ ); +} diff --git a/frontend/src/components/v1/chat/task-tracking/task-list-section.tsx b/frontend/src/components/v1/chat/task-tracking/task-list-section.tsx new file mode 100644 index 000000000000..aa3821036f4e --- /dev/null +++ b/frontend/src/components/v1/chat/task-tracking/task-list-section.tsx @@ -0,0 +1,33 @@ +import { useTranslation } from "react-i18next"; +import { TaskItem } from "./task-item"; +import LessonPlanIcon from "#/icons/lesson-plan.svg?react"; +import { TaskItem as TaskItemType } from "#/types/v1/core/base/common"; +import { I18nKey } from "#/i18n/declaration"; +import { Typography } from "#/ui/typography"; + +interface TaskListSectionProps { + taskList: TaskItemType[]; +} + +export function TaskListSection({ taskList }: TaskListSectionProps) { + const { t } = useTranslation(); + + return ( +
+ {/* Header Tabs */} +
+ + + {t(I18nKey.COMMON$TASKS)} + +
+ + {/* Task Items */} +
+ {taskList.map((task, index) => ( + + ))} +
+
+ ); +} diff --git a/frontend/src/components/v1/chat/task-tracking/task-tracking-observation-content.tsx b/frontend/src/components/v1/chat/task-tracking/task-tracking-observation-content.tsx new file mode 100644 index 000000000000..167429cae84a --- /dev/null +++ b/frontend/src/components/v1/chat/task-tracking/task-tracking-observation-content.tsx @@ -0,0 +1,23 @@ +import React from "react"; +import { ObservationEvent } from "#/types/v1/core"; +import { TaskTrackerObservation } from "#/types/v1/core/base/observation"; +import { TaskListSection } from "./task-list-section"; + +interface TaskTrackingObservationContentProps { + event: ObservationEvent; +} + +export function TaskTrackingObservationContent({ + event, +}: TaskTrackingObservationContentProps): React.ReactNode { + const { observation } = event; + const { command, task_list: taskList } = observation; + const shouldShowTaskList = command === "plan" && taskList.length > 0; + + return ( +
+ {/* Task List section - only show for 'plan' command */} + {shouldShowTaskList && } +
+ ); +} diff --git a/frontend/src/i18n/declaration.ts b/frontend/src/i18n/declaration.ts index f3fa1744e707..7fabded8df05 100644 --- a/frontend/src/i18n/declaration.ts +++ b/frontend/src/i18n/declaration.ts @@ -937,6 +937,7 @@ export enum I18nKey { AGENT_STATUS$WAITING_FOR_USER_CONFIRMATION = "AGENT_STATUS$WAITING_FOR_USER_CONFIRMATION", COMMON$MORE_OPTIONS = "COMMON$MORE_OPTIONS", COMMON$CREATE_A_PLAN = "COMMON$CREATE_A_PLAN", + COMMON$TASKS = "COMMON$TASKS", COMMON$PLAN_MD = "COMMON$PLAN_MD", COMMON$READ_MORE = "COMMON$READ_MORE", COMMON$BUILD = "COMMON$BUILD", diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index 3765faee4f64..992bc69ca7a1 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -14991,6 +14991,22 @@ "de": "Einen Plan erstellen", "uk": "Створити план" }, + "COMMON$TASKS": { + "en": "Tasks", + "ja": "タスク", + "zh-CN": "任务", + "zh-TW": "任務", + "ko-KR": "작업", + "no": "Oppgaver", + "it": "Attività", + "pt": "Tarefas", + "es": "Tareas", + "ar": "مهام", + "fr": "Tâches", + "tr": "Görevler", + "de": "Aufgaben", + "uk": "Завдання" + }, "COMMON$PLAN_MD": { "en": "Plan.md", "ja": "Plan.md", diff --git a/frontend/src/icons/loading.svg b/frontend/src/icons/loading.svg new file mode 100644 index 000000000000..2da678957f05 --- /dev/null +++ b/frontend/src/icons/loading.svg @@ -0,0 +1,3 @@ + + + diff --git a/frontend/src/icons/u-check-circle.svg b/frontend/src/icons/u-check-circle.svg new file mode 100644 index 000000000000..e98e0c8f3714 --- /dev/null +++ b/frontend/src/icons/u-check-circle.svg @@ -0,0 +1,3 @@ + + + diff --git a/frontend/src/icons/u-circle.svg b/frontend/src/icons/u-circle.svg new file mode 100644 index 000000000000..c562817d9b3e --- /dev/null +++ b/frontend/src/icons/u-circle.svg @@ -0,0 +1,3 @@ + + + From 8192184d3e07dfc158e9c0ffd26d53ec9df77d74 Mon Sep 17 00:00:00 2001 From: "sp.wack" <83104063+amanape@users.noreply.github.com> Date: Wed, 12 Nov 2025 20:47:21 +0400 Subject: [PATCH 002/108] chore(backend): Add better PostHog tracking (#11655) Co-authored-by: openhands --- enterprise/server/routes/auth.py | 7 + enterprise/server/routes/billing.py | 15 + openhands/controller/agent_controller.py | 30 ++ openhands/server/routes/git.py | 10 + openhands/utils/posthog_tracker.py | 270 +++++++++++++ .../test_agent_controller_posthog.py | 242 ++++++++++++ tests/unit/utils/test_posthog_tracker.py | 356 ++++++++++++++++++ 7 files changed, 930 insertions(+) create mode 100644 openhands/utils/posthog_tracker.py create mode 100644 tests/unit/controller/test_agent_controller_posthog.py create mode 100644 tests/unit/utils/test_posthog_tracker.py diff --git a/enterprise/server/routes/auth.py b/enterprise/server/routes/auth.py index d5f5cbd1ed44..3976363ee40f 100644 --- a/enterprise/server/routes/auth.py +++ b/enterprise/server/routes/auth.py @@ -30,6 +30,7 @@ from openhands.server.shared import config from openhands.server.user_auth import get_access_token from openhands.server.user_auth.user_auth import get_user_auth +from openhands.utils.posthog_tracker import track_user_signup_completed with warnings.catch_warnings(): warnings.simplefilter('ignore') @@ -362,6 +363,12 @@ async def accept_tos(request: Request): logger.info(f'User {user_id} accepted TOS') + # Track user signup completion in PostHog + track_user_signup_completed( + user_id=user_id, + signup_timestamp=user_settings.accepted_tos.isoformat(), + ) + response = JSONResponse( status_code=status.HTTP_200_OK, content={'redirect_url': redirect_url} ) diff --git a/enterprise/server/routes/billing.py b/enterprise/server/routes/billing.py index 5a8b59e2d76b..f1c0c5376bec 100644 --- a/enterprise/server/routes/billing.py +++ b/enterprise/server/routes/billing.py @@ -28,6 +28,7 @@ from openhands.server.user_auth import get_user_id from openhands.utils.http_session import httpx_verify_option +from openhands.utils.posthog_tracker import track_credits_purchased stripe.api_key = STRIPE_API_KEY billing_router = APIRouter(prefix='/api/billing') @@ -457,6 +458,20 @@ async def success_callback(session_id: str, request: Request): ) session.commit() + # Track credits purchased in PostHog + try: + track_credits_purchased( + user_id=billing_session.user_id, + amount_usd=amount_subtotal / 100, # Convert cents to dollars + credits_added=add_credits, + stripe_session_id=session_id, + ) + except Exception as e: + logger.warning( + f'Failed to track credits purchase: {e}', + extra={'user_id': billing_session.user_id, 'error': str(e)}, + ) + return RedirectResponse( f'{request.base_url}settings/billing?checkout=success', status_code=302 ) diff --git a/openhands/controller/agent_controller.py b/openhands/controller/agent_controller.py index e9616c66b5fa..ef3d162d9d98 100644 --- a/openhands/controller/agent_controller.py +++ b/openhands/controller/agent_controller.py @@ -42,6 +42,10 @@ from openhands.core.logger import LOG_ALL_EVENTS from openhands.core.logger import openhands_logger as logger from openhands.core.schema import AgentState +from openhands.utils.posthog_tracker import ( + track_agent_task_completed, + track_credit_limit_reached, +) from openhands.events import ( EventSource, EventStream, @@ -709,6 +713,20 @@ async def set_agent_state_to(self, new_state: AgentState) -> None: EventSource.ENVIRONMENT, ) + # Track agent task completion in PostHog + if new_state == AgentState.FINISHED: + try: + # Get app_mode from environment, default to 'oss' + app_mode = os.environ.get('APP_MODE', 'oss') + track_agent_task_completed( + conversation_id=self.id, + user_id=self.user_id, + app_mode=app_mode, + ) + except Exception as e: + # Don't let tracking errors interrupt the agent + self.log('warning', f'Failed to track agent completion: {e}') + # Save state whenever agent state changes to ensure we don't lose state # in case of crashes or unexpected circumstances self.save_state() @@ -887,6 +905,18 @@ async def _step(self) -> None: self.state_tracker.run_control_flags() except Exception as e: logger.warning('Control flag limits hit') + # Track credit limit reached if it's a budget exception + if 'budget' in str(e).lower() and self.state.budget_flag: + try: + track_credit_limit_reached( + conversation_id=self.id, + user_id=self.user_id, + current_budget=self.state.budget_flag.current_value, + max_budget=self.state.budget_flag.max_value, + ) + except Exception as track_error: + # Don't let tracking errors interrupt the agent + self.log('warning', f'Failed to track credit limit: {track_error}') await self._react_to_exception(e) return diff --git a/openhands/server/routes/git.py b/openhands/server/routes/git.py index 753cf6f12f16..d5db83999a4a 100644 --- a/openhands/server/routes/git.py +++ b/openhands/server/routes/git.py @@ -26,11 +26,13 @@ ) from openhands.server.dependencies import get_dependencies from openhands.server.shared import server_config +from openhands.server.types import AppMode from openhands.server.user_auth import ( get_access_token, get_provider_tokens, get_user_id, ) +from openhands.utils.posthog_tracker import alias_user_identities app = APIRouter(prefix='/api/user', dependencies=get_dependencies()) @@ -115,6 +117,14 @@ async def get_user( try: user: User = await client.get_user() + + # Alias git provider login with Keycloak user ID in PostHog (SaaS mode only) + if user_id and user.login and server_config.app_mode == AppMode.SAAS: + alias_user_identities( + keycloak_user_id=user_id, + git_login=user.login, + ) + return user except UnknownException as e: diff --git a/openhands/utils/posthog_tracker.py b/openhands/utils/posthog_tracker.py new file mode 100644 index 000000000000..c0859eddc717 --- /dev/null +++ b/openhands/utils/posthog_tracker.py @@ -0,0 +1,270 @@ +"""PostHog tracking utilities for OpenHands events.""" + +import os + +from openhands.core.logger import openhands_logger as logger + +# Lazy import posthog to avoid import errors in environments where it's not installed +posthog = None + + +def _init_posthog(): + """Initialize PostHog client lazily.""" + global posthog + if posthog is None: + try: + import posthog as ph + + posthog = ph + posthog.api_key = os.environ.get( + 'POSTHOG_CLIENT_KEY', 'phc_3ESMmY9SgqEAGBB6sMGK5ayYHkeUuknH2vP6FmWH9RA' + ) + posthog.host = os.environ.get('POSTHOG_HOST', 'https://us.i.posthog.com') + except ImportError: + logger.warning( + 'PostHog not installed. Analytics tracking will be disabled.' + ) + posthog = None + + +def track_agent_task_completed( + conversation_id: str, + user_id: str | None = None, + app_mode: str | None = None, +) -> None: + """Track when an agent completes a task. + + Args: + conversation_id: The ID of the conversation/session + user_id: The ID of the user (optional, may be None for unauthenticated users) + app_mode: The application mode (saas/oss), optional + """ + _init_posthog() + + if posthog is None: + return + + # Use conversation_id as distinct_id if user_id is not available + # This ensures we can track completions even for anonymous users + distinct_id = user_id if user_id else f'conversation_{conversation_id}' + + try: + posthog.capture( + distinct_id=distinct_id, + event='agent_task_completed', + properties={ + 'conversation_id': conversation_id, + 'user_id': user_id, + 'app_mode': app_mode or 'unknown', + }, + ) + logger.debug( + 'posthog_track', + extra={ + 'event': 'agent_task_completed', + 'conversation_id': conversation_id, + 'user_id': user_id, + }, + ) + except Exception as e: + logger.warning( + f'Failed to track agent_task_completed to PostHog: {e}', + extra={ + 'conversation_id': conversation_id, + 'error': str(e), + }, + ) + + +def track_user_signup_completed( + user_id: str, + signup_timestamp: str, +) -> None: + """Track when a user completes signup by accepting TOS. + + Args: + user_id: The ID of the user (Keycloak user ID) + signup_timestamp: ISO format timestamp of when TOS was accepted + """ + _init_posthog() + + if posthog is None: + return + + try: + posthog.capture( + distinct_id=user_id, + event='user_signup_completed', + properties={ + 'user_id': user_id, + 'signup_timestamp': signup_timestamp, + }, + ) + logger.debug( + 'posthog_track', + extra={ + 'event': 'user_signup_completed', + 'user_id': user_id, + }, + ) + except Exception as e: + logger.warning( + f'Failed to track user_signup_completed to PostHog: {e}', + extra={ + 'user_id': user_id, + 'error': str(e), + }, + ) + + +def track_credit_limit_reached( + conversation_id: str, + user_id: str | None = None, + current_budget: float = 0.0, + max_budget: float = 0.0, +) -> None: + """Track when a user reaches their credit limit during a conversation. + + Args: + conversation_id: The ID of the conversation/session + user_id: The ID of the user (optional, may be None for unauthenticated users) + current_budget: The current budget spent + max_budget: The maximum budget allowed + """ + _init_posthog() + + if posthog is None: + return + + distinct_id = user_id if user_id else f'conversation_{conversation_id}' + + try: + posthog.capture( + distinct_id=distinct_id, + event='credit_limit_reached', + properties={ + 'conversation_id': conversation_id, + 'user_id': user_id, + 'current_budget': current_budget, + 'max_budget': max_budget, + }, + ) + logger.debug( + 'posthog_track', + extra={ + 'event': 'credit_limit_reached', + 'conversation_id': conversation_id, + 'user_id': user_id, + 'current_budget': current_budget, + 'max_budget': max_budget, + }, + ) + except Exception as e: + logger.warning( + f'Failed to track credit_limit_reached to PostHog: {e}', + extra={ + 'conversation_id': conversation_id, + 'error': str(e), + }, + ) + + +def track_credits_purchased( + user_id: str, + amount_usd: float, + credits_added: float, + stripe_session_id: str, +) -> None: + """Track when a user successfully purchases credits. + + Args: + user_id: The ID of the user (Keycloak user ID) + amount_usd: The amount paid in USD (cents converted to dollars) + credits_added: The number of credits added to the user's account + stripe_session_id: The Stripe checkout session ID + """ + _init_posthog() + + if posthog is None: + return + + try: + posthog.capture( + distinct_id=user_id, + event='credits_purchased', + properties={ + 'user_id': user_id, + 'amount_usd': amount_usd, + 'credits_added': credits_added, + 'stripe_session_id': stripe_session_id, + }, + ) + logger.debug( + 'posthog_track', + extra={ + 'event': 'credits_purchased', + 'user_id': user_id, + 'amount_usd': amount_usd, + 'credits_added': credits_added, + }, + ) + except Exception as e: + logger.warning( + f'Failed to track credits_purchased to PostHog: {e}', + extra={ + 'user_id': user_id, + 'error': str(e), + }, + ) + + +def alias_user_identities( + keycloak_user_id: str, + git_login: str, +) -> None: + """Alias a user's Keycloak ID with their git provider login for unified tracking. + + This allows PostHog to link events tracked from the frontend (using git provider login) + with events tracked from the backend (using Keycloak user ID). + + PostHog Python alias syntax: alias(previous_id, distinct_id) + - previous_id: The old/previous distinct ID that will be merged + - distinct_id: The new/canonical distinct ID to merge into + + For our use case: + - Git provider login is the previous_id (first used in frontend, before backend auth) + - Keycloak user ID is the distinct_id (canonical backend ID) + - Result: All events with git login will be merged into Keycloak user ID + + Args: + keycloak_user_id: The Keycloak user ID (canonical distinct_id) + git_login: The git provider username (GitHub/GitLab/Bitbucket) to merge + + Reference: + https://github.com/PostHog/posthog-python/blob/master/posthog/client.py + """ + _init_posthog() + + if posthog is None: + return + + try: + # Merge git provider login into Keycloak user ID + # posthog.alias(previous_id, distinct_id) - official Python SDK signature + posthog.alias(git_login, keycloak_user_id) + logger.debug( + 'posthog_alias', + extra={ + 'previous_id': git_login, + 'distinct_id': keycloak_user_id, + }, + ) + except Exception as e: + logger.warning( + f'Failed to alias user identities in PostHog: {e}', + extra={ + 'keycloak_user_id': keycloak_user_id, + 'git_login': git_login, + 'error': str(e), + }, + ) diff --git a/tests/unit/controller/test_agent_controller_posthog.py b/tests/unit/controller/test_agent_controller_posthog.py new file mode 100644 index 000000000000..998a8f5fc14f --- /dev/null +++ b/tests/unit/controller/test_agent_controller_posthog.py @@ -0,0 +1,242 @@ +"""Integration tests for PostHog tracking in AgentController.""" + +import asyncio +from unittest.mock import MagicMock, patch + +import pytest + +from openhands.controller.agent import Agent +from openhands.controller.agent_controller import AgentController +from openhands.core.config import OpenHandsConfig +from openhands.core.config.agent_config import AgentConfig +from openhands.core.config.llm_config import LLMConfig +from openhands.core.schema import AgentState +from openhands.events import EventSource, EventStream +from openhands.events.action.message import SystemMessageAction +from openhands.llm.llm_registry import LLMRegistry +from openhands.server.services.conversation_stats import ConversationStats +from openhands.storage.memory import InMemoryFileStore + + +@pytest.fixture(scope='function') +def event_loop(): + """Create event loop for async tests.""" + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + loop.close() + + +@pytest.fixture +def mock_agent_with_stats(): + """Create a mock agent with properly connected LLM registry and conversation stats.""" + import uuid + + # Create LLM registry + config = OpenHandsConfig() + llm_registry = LLMRegistry(config=config) + + # Create conversation stats + file_store = InMemoryFileStore({}) + conversation_id = f'test-conversation-{uuid.uuid4()}' + conversation_stats = ConversationStats( + file_store=file_store, conversation_id=conversation_id, user_id='test-user' + ) + + # Connect registry to stats + llm_registry.subscribe(conversation_stats.register_llm) + + # Create mock agent + agent = MagicMock(spec=Agent) + agent_config = MagicMock(spec=AgentConfig) + llm_config = LLMConfig( + model='gpt-4o', + api_key='test_key', + num_retries=2, + retry_min_wait=1, + retry_max_wait=2, + ) + agent_config.disabled_microagents = [] + agent_config.enable_mcp = True + llm_registry.service_to_llm.clear() + mock_llm = llm_registry.get_llm('agent_llm', llm_config) + agent.llm = mock_llm + agent.name = 'test-agent' + agent.sandbox_plugins = [] + agent.config = agent_config + agent.llm_registry = llm_registry + agent.prompt_manager = MagicMock() + + # Add a proper system message mock + system_message = SystemMessageAction( + content='Test system message', tools=['test_tool'] + ) + system_message._source = EventSource.AGENT + system_message._id = -1 # Set invalid ID to avoid the ID check + agent.get_system_message.return_value = system_message + + return agent, conversation_stats, llm_registry + + +@pytest.fixture +def mock_event_stream(): + """Create a mock event stream.""" + mock = MagicMock( + spec=EventStream, + event_stream=EventStream(sid='test', file_store=InMemoryFileStore({})), + ) + mock.get_latest_event_id.return_value = 0 + return mock + + +@pytest.mark.asyncio +async def test_agent_finish_triggers_posthog_tracking( + mock_agent_with_stats, mock_event_stream +): + """Test that setting agent state to FINISHED triggers PostHog tracking.""" + mock_agent, conversation_stats, llm_registry = mock_agent_with_stats + + controller = AgentController( + agent=mock_agent, + event_stream=mock_event_stream, + conversation_stats=conversation_stats, + iteration_delta=10, + sid='test-conversation-123', + user_id='test-user-456', + confirmation_mode=False, + headless_mode=True, + ) + + with ( + patch('openhands.utils.posthog_tracker.posthog') as mock_posthog, + patch('os.environ.get') as mock_env_get, + ): + # Setup mocks + mock_posthog.capture = MagicMock() + mock_env_get.return_value = 'saas' + + # Initialize posthog in the tracker module + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + # Set agent state to FINISHED + await controller.set_agent_state_to(AgentState.FINISHED) + + # Verify PostHog tracking was called + mock_posthog.capture.assert_called_once() + call_args = mock_posthog.capture.call_args + + assert call_args[1]['distinct_id'] == 'test-user-456' + assert call_args[1]['event'] == 'agent_task_completed' + assert 'conversation_id' in call_args[1]['properties'] + assert call_args[1]['properties']['user_id'] == 'test-user-456' + assert call_args[1]['properties']['app_mode'] == 'saas' + + await controller.close() + + +@pytest.mark.asyncio +async def test_agent_finish_without_user_id(mock_agent_with_stats, mock_event_stream): + """Test tracking when user_id is None.""" + mock_agent, conversation_stats, llm_registry = mock_agent_with_stats + + controller = AgentController( + agent=mock_agent, + event_stream=mock_event_stream, + conversation_stats=conversation_stats, + iteration_delta=10, + sid='test-conversation-789', + user_id=None, + confirmation_mode=False, + headless_mode=True, + ) + + with ( + patch('openhands.utils.posthog_tracker.posthog') as mock_posthog, + patch('os.environ.get') as mock_env_get, + ): + mock_posthog.capture = MagicMock() + mock_env_get.return_value = 'oss' + + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + await controller.set_agent_state_to(AgentState.FINISHED) + + mock_posthog.capture.assert_called_once() + call_args = mock_posthog.capture.call_args + + # When user_id is None, distinct_id should be conversation_id + assert call_args[1]['distinct_id'].startswith('conversation_') + assert call_args[1]['properties']['user_id'] is None + + await controller.close() + + +@pytest.mark.asyncio +async def test_other_states_dont_trigger_tracking( + mock_agent_with_stats, mock_event_stream +): + """Test that non-FINISHED states don't trigger tracking.""" + mock_agent, conversation_stats, llm_registry = mock_agent_with_stats + + controller = AgentController( + agent=mock_agent, + event_stream=mock_event_stream, + conversation_stats=conversation_stats, + iteration_delta=10, + sid='test-conversation-999', + confirmation_mode=False, + headless_mode=True, + ) + + with patch('openhands.utils.posthog_tracker.posthog') as mock_posthog: + mock_posthog.capture = MagicMock() + + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + # Try different states + await controller.set_agent_state_to(AgentState.RUNNING) + await controller.set_agent_state_to(AgentState.PAUSED) + await controller.set_agent_state_to(AgentState.STOPPED) + + # PostHog should not be called for non-FINISHED states + mock_posthog.capture.assert_not_called() + + await controller.close() + + +@pytest.mark.asyncio +async def test_tracking_error_doesnt_break_agent( + mock_agent_with_stats, mock_event_stream +): + """Test that tracking errors don't interrupt agent operation.""" + mock_agent, conversation_stats, llm_registry = mock_agent_with_stats + + controller = AgentController( + agent=mock_agent, + event_stream=mock_event_stream, + conversation_stats=conversation_stats, + iteration_delta=10, + sid='test-conversation-error', + confirmation_mode=False, + headless_mode=True, + ) + + with patch('openhands.utils.posthog_tracker.posthog') as mock_posthog: + mock_posthog.capture = MagicMock(side_effect=Exception('PostHog error')) + + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + # Should not raise an exception + await controller.set_agent_state_to(AgentState.FINISHED) + + # Agent state should still be FINISHED despite tracking error + assert controller.state.agent_state == AgentState.FINISHED + + await controller.close() diff --git a/tests/unit/utils/test_posthog_tracker.py b/tests/unit/utils/test_posthog_tracker.py new file mode 100644 index 000000000000..cec0eff0ccc0 --- /dev/null +++ b/tests/unit/utils/test_posthog_tracker.py @@ -0,0 +1,356 @@ +"""Unit tests for PostHog tracking utilities.""" + +from unittest.mock import MagicMock, patch + +import pytest + +from openhands.utils.posthog_tracker import ( + alias_user_identities, + track_agent_task_completed, + track_credit_limit_reached, + track_credits_purchased, + track_user_signup_completed, +) + + +@pytest.fixture +def mock_posthog(): + """Mock the posthog module.""" + with patch('openhands.utils.posthog_tracker.posthog') as mock_ph: + mock_ph.capture = MagicMock() + yield mock_ph + + +def test_track_agent_task_completed_with_user_id(mock_posthog): + """Test tracking agent task completion with user ID.""" + # Initialize posthog manually in the test + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_agent_task_completed( + conversation_id='test-conversation-123', + user_id='user-456', + app_mode='saas', + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='user-456', + event='agent_task_completed', + properties={ + 'conversation_id': 'test-conversation-123', + 'user_id': 'user-456', + 'app_mode': 'saas', + }, + ) + + +def test_track_agent_task_completed_without_user_id(mock_posthog): + """Test tracking agent task completion without user ID (anonymous).""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_agent_task_completed( + conversation_id='test-conversation-789', + user_id=None, + app_mode='oss', + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='conversation_test-conversation-789', + event='agent_task_completed', + properties={ + 'conversation_id': 'test-conversation-789', + 'user_id': None, + 'app_mode': 'oss', + }, + ) + + +def test_track_agent_task_completed_default_app_mode(mock_posthog): + """Test tracking with default app_mode.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_agent_task_completed( + conversation_id='test-conversation-999', + user_id='user-111', + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='user-111', + event='agent_task_completed', + properties={ + 'conversation_id': 'test-conversation-999', + 'user_id': 'user-111', + 'app_mode': 'unknown', + }, + ) + + +def test_track_agent_task_completed_handles_errors(mock_posthog): + """Test that tracking errors are handled gracefully.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.capture.side_effect = Exception('PostHog API error') + + # Should not raise an exception + track_agent_task_completed( + conversation_id='test-conversation-error', + user_id='user-error', + app_mode='saas', + ) + + +def test_track_agent_task_completed_when_posthog_not_installed(): + """Test tracking when posthog is not installed.""" + import openhands.utils.posthog_tracker as tracker + + # Simulate posthog not being installed + tracker.posthog = None + + # Should not raise an exception + track_agent_task_completed( + conversation_id='test-conversation-no-ph', + user_id='user-no-ph', + app_mode='oss', + ) + + +def test_track_user_signup_completed(mock_posthog): + """Test tracking user signup completion.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_user_signup_completed( + user_id='test-user-123', + signup_timestamp='2025-01-15T10:30:00Z', + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='test-user-123', + event='user_signup_completed', + properties={ + 'user_id': 'test-user-123', + 'signup_timestamp': '2025-01-15T10:30:00Z', + }, + ) + + +def test_track_user_signup_completed_handles_errors(mock_posthog): + """Test that user signup tracking errors are handled gracefully.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.capture.side_effect = Exception('PostHog API error') + + # Should not raise an exception + track_user_signup_completed( + user_id='test-user-error', + signup_timestamp='2025-01-15T12:00:00Z', + ) + + +def test_track_user_signup_completed_when_posthog_not_installed(): + """Test user signup tracking when posthog is not installed.""" + import openhands.utils.posthog_tracker as tracker + + # Simulate posthog not being installed + tracker.posthog = None + + # Should not raise an exception + track_user_signup_completed( + user_id='test-user-no-ph', + signup_timestamp='2025-01-15T13:00:00Z', + ) + + +def test_track_credit_limit_reached_with_user_id(mock_posthog): + """Test tracking credit limit reached with user ID.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_credit_limit_reached( + conversation_id='test-conversation-456', + user_id='user-789', + current_budget=10.50, + max_budget=10.00, + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='user-789', + event='credit_limit_reached', + properties={ + 'conversation_id': 'test-conversation-456', + 'user_id': 'user-789', + 'current_budget': 10.50, + 'max_budget': 10.00, + }, + ) + + +def test_track_credit_limit_reached_without_user_id(mock_posthog): + """Test tracking credit limit reached without user ID (anonymous).""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_credit_limit_reached( + conversation_id='test-conversation-999', + user_id=None, + current_budget=5.25, + max_budget=5.00, + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='conversation_test-conversation-999', + event='credit_limit_reached', + properties={ + 'conversation_id': 'test-conversation-999', + 'user_id': None, + 'current_budget': 5.25, + 'max_budget': 5.00, + }, + ) + + +def test_track_credit_limit_reached_handles_errors(mock_posthog): + """Test that credit limit tracking errors are handled gracefully.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.capture.side_effect = Exception('PostHog API error') + + # Should not raise an exception + track_credit_limit_reached( + conversation_id='test-conversation-error', + user_id='user-error', + current_budget=15.00, + max_budget=10.00, + ) + + +def test_track_credit_limit_reached_when_posthog_not_installed(): + """Test credit limit tracking when posthog is not installed.""" + import openhands.utils.posthog_tracker as tracker + + # Simulate posthog not being installed + tracker.posthog = None + + # Should not raise an exception + track_credit_limit_reached( + conversation_id='test-conversation-no-ph', + user_id='user-no-ph', + current_budget=8.00, + max_budget=5.00, + ) + + +def test_track_credits_purchased(mock_posthog): + """Test tracking credits purchased.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + + track_credits_purchased( + user_id='test-user-999', + amount_usd=50.00, + credits_added=50.00, + stripe_session_id='cs_test_abc123', + ) + + mock_posthog.capture.assert_called_once_with( + distinct_id='test-user-999', + event='credits_purchased', + properties={ + 'user_id': 'test-user-999', + 'amount_usd': 50.00, + 'credits_added': 50.00, + 'stripe_session_id': 'cs_test_abc123', + }, + ) + + +def test_track_credits_purchased_handles_errors(mock_posthog): + """Test that credits purchased tracking errors are handled gracefully.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.capture.side_effect = Exception('PostHog API error') + + # Should not raise an exception + track_credits_purchased( + user_id='test-user-error', + amount_usd=100.00, + credits_added=100.00, + stripe_session_id='cs_test_error', + ) + + +def test_track_credits_purchased_when_posthog_not_installed(): + """Test credits purchased tracking when posthog is not installed.""" + import openhands.utils.posthog_tracker as tracker + + # Simulate posthog not being installed + tracker.posthog = None + + # Should not raise an exception + track_credits_purchased( + user_id='test-user-no-ph', + amount_usd=25.00, + credits_added=25.00, + stripe_session_id='cs_test_no_ph', + ) + + +def test_alias_user_identities(mock_posthog): + """Test aliasing user identities. + + Verifies that posthog.alias(previous_id, distinct_id) is called correctly + where git_login is the previous_id and keycloak_user_id is the distinct_id. + """ + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.alias = MagicMock() + + alias_user_identities( + keycloak_user_id='keycloak-123', + git_login='git-user', + ) + + # Verify: posthog.alias(previous_id='git-user', distinct_id='keycloak-123') + mock_posthog.alias.assert_called_once_with('git-user', 'keycloak-123') + + +def test_alias_user_identities_handles_errors(mock_posthog): + """Test that aliasing errors are handled gracefully.""" + import openhands.utils.posthog_tracker as tracker + + tracker.posthog = mock_posthog + mock_posthog.alias = MagicMock(side_effect=Exception('PostHog API error')) + + # Should not raise an exception + alias_user_identities( + keycloak_user_id='keycloak-error', + git_login='git-error', + ) + + +def test_alias_user_identities_when_posthog_not_installed(): + """Test aliasing when posthog is not installed.""" + import openhands.utils.posthog_tracker as tracker + + # Simulate posthog not being installed + tracker.posthog = None + + # Should not raise an exception + alias_user_identities( + keycloak_user_id='keycloak-no-ph', + git_login='git-no-ph', + ) From b605c96796cd962c1ea5df1ec58716094b6da561 Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Wed, 12 Nov 2025 20:13:16 -0500 Subject: [PATCH 003/108] Hotfix: rm max condenser size override (#11713) --- enterprise/experiments/experiment_manager.py | 23 +------------------ .../test_saas_experiment_manager.py | 11 +-------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/enterprise/experiments/experiment_manager.py b/enterprise/experiments/experiment_manager.py index 7c53f274146b..1c212a03916a 100644 --- a/enterprise/experiments/experiment_manager.py +++ b/enterprise/experiments/experiment_manager.py @@ -5,12 +5,8 @@ EXPERIMENT_SYSTEM_PROMPT_EXPERIMENT, ) from experiments.experiment_versions import ( - handle_condenser_max_step_experiment, handle_system_prompt_experiment, ) -from experiments.experiment_versions._004_condenser_max_step_experiment import ( - handle_condenser_max_step_experiment__v1, -) from openhands.core.config.openhands_config import OpenHandsConfig from openhands.core.logger import openhands_logger as logger @@ -31,10 +27,6 @@ def run_agent_variant_tests__v1( ) return agent - agent = handle_condenser_max_step_experiment__v1( - user_id, conversation_id, agent - ) - if EXPERIMENT_SYSTEM_PROMPT_EXPERIMENT: agent = agent.model_copy( update={'system_prompt_filename': 'system_prompt_long_horizon.j2'} @@ -60,20 +52,7 @@ def run_conversation_variant_test( """ logger.debug( 'experiment_manager:run_conversation_variant_test:started', - extra={'user_id': user_id}, - ) - - # Skip all experiment processing if the experiment manager is disabled - if not ENABLE_EXPERIMENT_MANAGER: - logger.info( - 'experiment_manager:run_conversation_variant_test:skipped', - extra={'reason': 'experiment_manager_disabled'}, - ) - return conversation_settings - - # Apply conversation-scoped experiments - conversation_settings = handle_condenser_max_step_experiment( - user_id, conversation_id, conversation_settings + extra={'user_id': user_id, 'conversation_id': conversation_id}, ) return conversation_settings diff --git a/enterprise/tests/unit/experiments/test_saas_experiment_manager.py b/enterprise/tests/unit/experiments/test_saas_experiment_manager.py index ec67c7479f63..4f1eab2a920d 100644 --- a/enterprise/tests/unit/experiments/test_saas_experiment_manager.py +++ b/enterprise/tests/unit/experiments/test_saas_experiment_manager.py @@ -92,11 +92,8 @@ def test_unknown_variant_returns_original_agent_without_changes(monkeypatch): assert getattr(result, 'condenser', None) is None -@patch('experiments.experiment_manager.handle_condenser_max_step_experiment__v1') @patch('experiments.experiment_manager.ENABLE_EXPERIMENT_MANAGER', False) -def test_run_agent_variant_tests_v1_noop_when_manager_disabled( - mock_handle_condenser, -): +def test_run_agent_variant_tests_v1_noop_when_manager_disabled(): """If ENABLE_EXPERIMENT_MANAGER is False, the method returns the exact same agent and does not call the handler.""" agent = make_agent() conv_id = uuid4() @@ -109,8 +106,6 @@ def test_run_agent_variant_tests_v1_noop_when_manager_disabled( # Same object returned (no copy) assert result is agent - # Handler should not have been called - mock_handle_condenser.assert_not_called() @patch('experiments.experiment_manager.ENABLE_EXPERIMENT_MANAGER', True) @@ -131,7 +126,3 @@ def test_run_agent_variant_tests_v1_calls_handler_and_sets_system_prompt(monkeyp # Should be a different instance than the original (copied after handler runs) assert result is not agent assert result.system_prompt_filename == 'system_prompt_long_horizon.j2' - - # The condenser returned by the handler must be preserved after the system-prompt override copy - assert isinstance(result.condenser, LLMSummarizingCondenser) - assert result.condenser.max_size == 80 From d5b2d2ebc53b53bcceeb985c0b0bbdc967c71a8a Mon Sep 17 00:00:00 2001 From: "sp.wack" <83104063+amanape@users.noreply.github.com> Date: Thu, 13 Nov 2025 17:22:05 +0400 Subject: [PATCH 004/108] fix(frontend): Sync client PostHog opt-in status with server setting (#11728) --- .../src/hooks/use-sync-posthog-consent.ts | 41 +++++++++++++++++++ frontend/src/routes/root-layout.tsx | 4 ++ 2 files changed, 45 insertions(+) create mode 100644 frontend/src/hooks/use-sync-posthog-consent.ts diff --git a/frontend/src/hooks/use-sync-posthog-consent.ts b/frontend/src/hooks/use-sync-posthog-consent.ts new file mode 100644 index 000000000000..615aa9a1bf8f --- /dev/null +++ b/frontend/src/hooks/use-sync-posthog-consent.ts @@ -0,0 +1,41 @@ +import React from "react"; +import { usePostHog } from "posthog-js/react"; +import { handleCaptureConsent } from "#/utils/handle-capture-consent"; +import { useSettings } from "./query/use-settings"; + +/** + * Hook to sync PostHog opt-in/out state with backend setting on mount. + * This ensures that if the backend setting changes (e.g., via API or different client), + * the PostHog instance reflects the current user preference. + */ +export const useSyncPostHogConsent = () => { + const posthog = usePostHog(); + const { data: settings } = useSettings(); + const hasSyncedRef = React.useRef(false); + + React.useEffect(() => { + // Only run once when both PostHog and settings are available + if (!posthog || settings === undefined || hasSyncedRef.current) { + return; + } + + const backendConsent = settings.USER_CONSENTS_TO_ANALYTICS; + + // Only sync if there's a backend preference set + if (backendConsent !== null) { + const posthogHasOptedIn = posthog.has_opted_in_capturing(); + const posthogHasOptedOut = posthog.has_opted_out_capturing(); + + // Check if PostHog state is out of sync with backend + const needsSync = + (backendConsent === true && !posthogHasOptedIn) || + (backendConsent === false && !posthogHasOptedOut); + + if (needsSync) { + handleCaptureConsent(posthog, backendConsent); + } + + hasSyncedRef.current = true; + } + }, [posthog, settings]); +}; diff --git a/frontend/src/routes/root-layout.tsx b/frontend/src/routes/root-layout.tsx index 930451dae9ce..264ae541c88d 100644 --- a/frontend/src/routes/root-layout.tsx +++ b/frontend/src/routes/root-layout.tsx @@ -25,6 +25,7 @@ import { useIsOnTosPage } from "#/hooks/use-is-on-tos-page"; import { useAutoLogin } from "#/hooks/use-auto-login"; import { useAuthCallback } from "#/hooks/use-auth-callback"; import { useReoTracking } from "#/hooks/use-reo-tracking"; +import { useSyncPostHogConsent } from "#/hooks/use-sync-posthog-consent"; import { LOCAL_STORAGE_KEYS } from "#/utils/local-storage"; import { EmailVerificationGuard } from "#/components/features/guards/email-verification-guard"; import { MaintenanceBanner } from "#/components/features/maintenance/maintenance-banner"; @@ -100,6 +101,9 @@ export default function MainApp() { // Initialize Reo.dev tracking in SaaS mode useReoTracking(); + // Sync PostHog opt-in/out state with backend setting on mount + useSyncPostHogConsent(); + React.useEffect(() => { // Don't change language when on TOS page if (!isOnTosPage && settings?.LANGUAGE) { From bc86796a67194fb58d39adfe9778d1abf0817d00 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Thu, 13 Nov 2025 23:06:44 +0700 Subject: [PATCH 005/108] feat(backend): enable sub-conversation creation using a different agent (#11715) --- .../081_add_parent_conversation_id.py | 41 ++++++++++ enterprise/storage/saas_conversation_store.py | 1 + .../app_conversation_models.py | 11 +++ .../live_status_app_conversation_service.py | 62 ++++++++++++++- .../sql_app_conversation_info_service.py | 11 +++ .../app_lifespan/alembic/versions/003.py | 41 ++++++++++ .../sandbox/sandbox_spec_service.py | 2 +- .../server/routes/test_conversation_routes.py | 75 +++++++++++++++++++ 8 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 enterprise/migrations/versions/081_add_parent_conversation_id.py create mode 100644 openhands/app_server/app_lifespan/alembic/versions/003.py diff --git a/enterprise/migrations/versions/081_add_parent_conversation_id.py b/enterprise/migrations/versions/081_add_parent_conversation_id.py new file mode 100644 index 000000000000..b27c444632e4 --- /dev/null +++ b/enterprise/migrations/versions/081_add_parent_conversation_id.py @@ -0,0 +1,41 @@ +"""add parent_conversation_id to conversation_metadata + +Revision ID: 081 +Revises: 080 +Create Date: 2025-11-06 00:00:00.000000 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = '081' +down_revision: Union[str, None] = '080' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.add_column( + 'conversation_metadata', + sa.Column('parent_conversation_id', sa.String(), nullable=True), + ) + op.create_index( + op.f('ix_conversation_metadata_parent_conversation_id'), + 'conversation_metadata', + ['parent_conversation_id'], + unique=False, + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_index( + op.f('ix_conversation_metadata_parent_conversation_id'), + table_name='conversation_metadata', + ) + op.drop_column('conversation_metadata', 'parent_conversation_id') diff --git a/enterprise/storage/saas_conversation_store.py b/enterprise/storage/saas_conversation_store.py index 80a27ce957c6..160c3a80a2fb 100644 --- a/enterprise/storage/saas_conversation_store.py +++ b/enterprise/storage/saas_conversation_store.py @@ -60,6 +60,7 @@ def _to_external_model(self, conversation_metadata: StoredConversationMetadata): kwargs.pop('reasoning_tokens', None) kwargs.pop('context_window', None) kwargs.pop('per_turn_token', None) + kwargs.pop('parent_conversation_id', None) return ConversationMetadata(**kwargs) diff --git a/openhands/app_server/app_conversation/app_conversation_models.py b/openhands/app_server/app_conversation/app_conversation_models.py index 1b2f201dcd82..4ed04cf91c7f 100644 --- a/openhands/app_server/app_conversation/app_conversation_models.py +++ b/openhands/app_server/app_conversation/app_conversation_models.py @@ -16,6 +16,13 @@ from openhands.storage.data_models.conversation_metadata import ConversationTrigger +class AgentType(Enum): + """Agent type for conversation.""" + + DEFAULT = 'default' + PLAN = 'plan' + + class AppConversationInfo(BaseModel): """Conversation info which does not contain status.""" @@ -34,6 +41,8 @@ class AppConversationInfo(BaseModel): metrics: MetricsSnapshot | None = None + parent_conversation_id: OpenHandsUUID | None = None + created_at: datetime = Field(default_factory=utc_now) updated_at: datetime = Field(default_factory=utc_now) @@ -98,6 +107,8 @@ class AppConversationStartRequest(BaseModel): title: str | None = None trigger: ConversationTrigger | None = None pr_number: list[int] = Field(default_factory=list) + parent_conversation_id: OpenHandsUUID | None = None + agent_type: AgentType = Field(default=AgentType.DEFAULT) class AppConversationStartTaskStatus(Enum): diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index cc10d254e708..73f26781a605 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -21,6 +21,7 @@ AppConversationInfoService, ) from openhands.app_server.app_conversation.app_conversation_models import ( + AgentType, AppConversation, AppConversationInfo, AppConversationPage, @@ -70,6 +71,7 @@ from openhands.sdk.security.confirmation_policy import AlwaysConfirm from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace from openhands.tools.preset.default import get_default_agent +from openhands.tools.preset.planning import get_planning_agent _conversation_info_type_adapter = TypeAdapter(list[ConversationInfo | None]) _logger = logging.getLogger(__name__) @@ -168,6 +170,20 @@ async def _start_app_conversation( ) -> AsyncGenerator[AppConversationStartTask, None]: # Create and yield the start task user_id = await self.user_context.get_user_id() + + # Validate and inherit from parent conversation if provided + if request.parent_conversation_id: + parent_info = ( + await self.app_conversation_info_service.get_app_conversation_info( + request.parent_conversation_id + ) + ) + if parent_info is None: + raise ValueError( + f'Parent conversation not found: {request.parent_conversation_id}' + ) + self._inherit_configuration_from_parent(request, parent_info) + task = AppConversationStartTask( created_by_user_id=user_id, request=request, @@ -206,6 +222,8 @@ async def _start_app_conversation( request.initial_message, request.git_provider, sandbox_spec.working_dir, + request.agent_type, + request.llm_model, ) ) @@ -224,6 +242,7 @@ async def _start_app_conversation( headers={'X-Session-API-Key': sandbox.session_api_key}, timeout=self.sandbox_startup_timeout, ) + response.raise_for_status() info = ConversationInfo.model_validate(response.json()) @@ -241,6 +260,7 @@ async def _start_app_conversation( git_provider=request.git_provider, trigger=request.trigger, pr_number=request.pr_number, + parent_conversation_id=request.parent_conversation_id, ) await self.app_conversation_info_service.save_app_conversation_info( app_conversation_info @@ -452,11 +472,43 @@ def _get_agent_server_url(self, sandbox: SandboxInfo) -> str: ) return agent_server_url + def _inherit_configuration_from_parent( + self, request: AppConversationStartRequest, parent_info: AppConversationInfo + ) -> None: + """Inherit configuration from parent conversation if not explicitly provided. + + This ensures sub-conversations automatically inherit: + - Sandbox ID (to share the same workspace/environment) + - Git parameters (repository, branch, provider) + - LLM model + + Args: + request: The conversation start request to modify + parent_info: The parent conversation info to inherit from + """ + # Inherit sandbox_id from parent to share the same workspace/environment + if not request.sandbox_id: + request.sandbox_id = parent_info.sandbox_id + + # Inherit git parameters from parent if not provided + if not request.selected_repository: + request.selected_repository = parent_info.selected_repository + if not request.selected_branch: + request.selected_branch = parent_info.selected_branch + if not request.git_provider: + request.git_provider = parent_info.git_provider + + # Inherit LLM model from parent if not provided + if not request.llm_model and parent_info.llm_model: + request.llm_model = parent_info.llm_model + async def _build_start_conversation_request_for_user( self, initial_message: SendMessageRequest | None, git_provider: ProviderType | None, working_dir: str, + agent_type: AgentType = AgentType.DEFAULT, + llm_model: str | None = None, ) -> StartConversationRequest: user = await self.user_context.get_user_info() @@ -488,13 +540,19 @@ async def _build_start_conversation_request_for_user( workspace = LocalWorkspace(working_dir=working_dir) + # Use provided llm_model if available, otherwise fall back to user's default + model = llm_model or user.llm_model llm = LLM( - model=user.llm_model, + model=model, base_url=user.llm_base_url, api_key=user.llm_api_key, usage_id='agent', ) - agent = get_default_agent(llm=llm) + # Select agent based on agent_type + if agent_type == AgentType.PLAN: + agent = get_planning_agent(llm=llm) + else: + agent = get_default_agent(llm=llm) conversation_id = uuid4() agent = ExperimentManagerImpl.run_agent_variant_tests__v1( diff --git a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py index 09904114851e..c062fc3f8b13 100644 --- a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py +++ b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py @@ -88,6 +88,7 @@ class StoredConversationMetadata(Base): # type: ignore conversation_version = Column(String, nullable=False, default='V0', index=True) sandbox_id = Column(String, nullable=True, index=True) + parent_conversation_id = Column(String, nullable=True, index=True) @dataclass @@ -307,6 +308,11 @@ async def save_app_conversation_info( llm_model=info.llm_model, conversation_version='V1', sandbox_id=info.sandbox_id, + parent_conversation_id=( + str(info.parent_conversation_id) + if info.parent_conversation_id + else None + ), ) await self.db_session.merge(stored) @@ -364,6 +370,11 @@ def _to_info(self, stored: StoredConversationMetadata) -> AppConversationInfo: pr_number=stored.pr_number, llm_model=stored.llm_model, metrics=metrics, + parent_conversation_id=( + UUID(stored.parent_conversation_id) + if stored.parent_conversation_id + else None + ), created_at=created_at, updated_at=updated_at, ) diff --git a/openhands/app_server/app_lifespan/alembic/versions/003.py b/openhands/app_server/app_lifespan/alembic/versions/003.py new file mode 100644 index 000000000000..6879b4358f7b --- /dev/null +++ b/openhands/app_server/app_lifespan/alembic/versions/003.py @@ -0,0 +1,41 @@ +"""add parent_conversation_id to conversation_metadata + +Revision ID: 003 +Revises: 002 +Create Date: 2025-11-06 00:00:00.000000 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = '003' +down_revision: Union[str, None] = '002' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.add_column( + 'conversation_metadata', + sa.Column('parent_conversation_id', sa.String(), nullable=True), + ) + op.create_index( + op.f('ix_conversation_metadata_parent_conversation_id'), + 'conversation_metadata', + ['parent_conversation_id'], + unique=False, + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_index( + op.f('ix_conversation_metadata_parent_conversation_id'), + table_name='conversation_metadata', + ) + op.drop_column('conversation_metadata', 'parent_conversation_id') diff --git a/openhands/app_server/sandbox/sandbox_spec_service.py b/openhands/app_server/sandbox/sandbox_spec_service.py index fd091ca130bd..dad14297c5e1 100644 --- a/openhands/app_server/sandbox/sandbox_spec_service.py +++ b/openhands/app_server/sandbox/sandbox_spec_service.py @@ -11,7 +11,7 @@ # The version of the agent server to use for deployments. # Typically this will be the same as the values from the pyproject.toml -AGENT_SERVER_IMAGE = 'ghcr.io/openhands/agent-server:f3c0c19-python' +AGENT_SERVER_IMAGE = 'ghcr.io/openhands/agent-server:4e2ecd8-python' class SandboxSpecService(ABC): diff --git a/tests/unit/server/routes/test_conversation_routes.py b/tests/unit/server/routes/test_conversation_routes.py index f909e44cc867..3d374f688ea1 100644 --- a/tests/unit/server/routes/test_conversation_routes.py +++ b/tests/unit/server/routes/test_conversation_routes.py @@ -11,7 +11,14 @@ AppConversationInfoService, ) from openhands.app_server.app_conversation.app_conversation_models import ( + AgentType, AppConversationInfo, + AppConversationStartRequest, + AppConversationStartTask, + AppConversationStartTaskStatus, +) +from openhands.app_server.app_conversation.app_conversation_service import ( + AppConversationService, ) from openhands.microagent.microagent import KnowledgeMicroagent, RepoMicroagent from openhands.microagent.types import MicroagentMetadata, MicroagentType @@ -1125,3 +1132,71 @@ async def test_add_message_empty_message(): call_args = mock_manager.send_event_to_conversation.call_args message_data = call_args[0][1] assert message_data['args']['content'] == '' + + +@pytest.mark.sub_conversation +@pytest.mark.asyncio +async def test_create_sub_conversation_with_planning_agent(): + """Test creating a sub-conversation from a parent conversation with planning agent.""" + from uuid import uuid4 + + parent_conversation_id = uuid4() + user_id = 'test_user_456' + sandbox_id = 'test_sandbox_123' + + # Create mock parent conversation info + parent_info = AppConversationInfo( + id=parent_conversation_id, + created_by_user_id=user_id, + sandbox_id=sandbox_id, + selected_repository='test/repo', + selected_branch='main', + git_provider=None, + title='Parent Conversation', + llm_model='anthropic/claude-3-5-sonnet-20241022', + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + # Create sub-conversation request with planning agent + sub_conversation_request = AppConversationStartRequest( + parent_conversation_id=parent_conversation_id, + agent_type=AgentType.PLAN, + initial_message=None, + ) + + # Create mock app conversation service + mock_app_conversation_service = MagicMock(spec=AppConversationService) + mock_app_conversation_info_service = MagicMock(spec=AppConversationInfoService) + + # Mock the service to return parent info + mock_app_conversation_info_service.get_app_conversation_info = AsyncMock( + return_value=parent_info + ) + + # Mock the start_app_conversation method to return a task + async def mock_start_generator(request): + task = AppConversationStartTask( + id=uuid4(), + created_by_user_id=user_id, + status=AppConversationStartTaskStatus.READY, + app_conversation_id=uuid4(), + sandbox_id=sandbox_id, + agent_server_url='http://agent-server:8000', + request=request, + ) + yield task + + mock_app_conversation_service.start_app_conversation = mock_start_generator + + # Test the service method directly + async for task in mock_app_conversation_service.start_app_conversation( + sub_conversation_request + ): + # Verify the task was created with planning agent + assert task is not None + assert task.status == AppConversationStartTaskStatus.READY + assert task.request.agent_type == AgentType.PLAN + assert task.request.parent_conversation_id == parent_conversation_id + assert task.sandbox_id == sandbox_id + break From 8c3f93ddc4ba083654223e849ad872883c79d059 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Fri, 14 Nov 2025 00:10:15 +0700 Subject: [PATCH 006/108] feat(frontend): set descriptive text for all options in the change agent button (#11732) --- .../chat/change-agent-context-menu.tsx | 21 ++++------ ...ontext-menu-icon-text-with-description.tsx | 39 +++++++++++++++++++ frontend/src/i18n/declaration.ts | 2 + frontend/src/i18n/translation.json | 32 +++++++++++++++ 4 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 frontend/src/components/features/context-menu/context-menu-icon-text-with-description.tsx diff --git a/frontend/src/components/features/chat/change-agent-context-menu.tsx b/frontend/src/components/features/chat/change-agent-context-menu.tsx index 6e88ce97d42c..69961deae862 100644 --- a/frontend/src/components/features/chat/change-agent-context-menu.tsx +++ b/frontend/src/components/features/chat/change-agent-context-menu.tsx @@ -5,19 +5,14 @@ import CodeTagIcon from "#/icons/code-tag.svg?react"; import LessonPlanIcon from "#/icons/lesson-plan.svg?react"; import { ContextMenu } from "#/ui/context-menu"; import { ContextMenuListItem } from "../context-menu/context-menu-list-item"; -import { ContextMenuIconText } from "../context-menu/context-menu-icon-text"; +import { ContextMenuIconTextWithDescription } from "../context-menu/context-menu-icon-text-with-description"; import { useClickOutsideElement } from "#/hooks/use-click-outside-element"; import { cn } from "#/utils/utils"; -import { CONTEXT_MENU_ICON_TEXT_CLASSNAME } from "#/utils/constants"; const contextMenuListItemClassName = cn( "cursor-pointer p-0 h-auto hover:bg-transparent", - CONTEXT_MENU_ICON_TEXT_CLASSNAME, ); -const contextMenuIconTextClassName = - "gap-2 p-2 hover:bg-[#5C5D62] rounded h-[30px]"; - interface ChangeAgentContextMenuProps { onClose: () => void; onCodeClick?: (event: React.MouseEvent) => void; @@ -52,17 +47,17 @@ export function ChangeAgentContextMenu({ testId="change-agent-context-menu" position="top" alignment="left" - className="min-h-fit min-w-[195px] mb-2" + className="min-h-fit mb-2 min-w-[195px] max-w-[195px] gap-0" > - - diff --git a/frontend/src/components/features/context-menu/context-menu-icon-text-with-description.tsx b/frontend/src/components/features/context-menu/context-menu-icon-text-with-description.tsx new file mode 100644 index 000000000000..fd505fef5800 --- /dev/null +++ b/frontend/src/components/features/context-menu/context-menu-icon-text-with-description.tsx @@ -0,0 +1,39 @@ +import React from "react"; +import { ContextMenuIconText } from "./context-menu-icon-text"; +import { Typography } from "#/ui/typography"; +import { cn } from "#/utils/utils"; + +interface ContextMenuIconTextWithDescriptionProps { + icon: React.ComponentType<{ className?: string }>; + title: string; + description: string; + className?: string; + iconClassName?: string; +} + +export function ContextMenuIconTextWithDescription({ + icon, + title, + description, + className, + iconClassName, +}: ContextMenuIconTextWithDescriptionProps) { + return ( +
+ + + {description} + +
+ ); +} diff --git a/frontend/src/i18n/declaration.ts b/frontend/src/i18n/declaration.ts index 7fabded8df05..52f0616575f9 100644 --- a/frontend/src/i18n/declaration.ts +++ b/frontend/src/i18n/declaration.ts @@ -944,4 +944,6 @@ export enum I18nKey { COMMON$ASK = "COMMON$ASK", COMMON$PLAN = "COMMON$PLAN", COMMON$LET_S_WORK_ON_A_PLAN = "COMMON$LET_S_WORK_ON_A_PLAN", + COMMON$CODE_AGENT_DESCRIPTION = "COMMON$CODE_AGENT_DESCRIPTION", + COMMON$PLAN_AGENT_DESCRIPTION = "COMMON$PLAN_AGENT_DESCRIPTION", } diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index 992bc69ca7a1..1a542fec6dc0 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -15102,5 +15102,37 @@ "tr": "Bir plan üzerinde çalışalım", "de": "Lassen Sie uns an einem Plan arbeiten", "uk": "Давайте розробимо план" + }, + "COMMON$CODE_AGENT_DESCRIPTION": { + "en": "Write, edit, and debug with AI assistance in real time.", + "ja": "AIの支援をリアルタイムで受けながら、コードの作成、編集、デバッグを行いましょう。", + "zh-CN": "实时在 AI 协助下编写、编辑和调试。", + "zh-TW": "即時在 AI 協助下編寫、編輯和除錯。", + "ko-KR": "AI의 지원을 받아 실시간으로 작성, 편집 및 디버깅하세요.", + "no": "Skriv, rediger og feilsøk med AI-assistanse i sanntid.", + "it": "Scrivi, modifica e esegui il debug con assistenza AI in tempo reale.", + "pt": "Escreva, edite e depure com assistência de IA em tempo real.", + "es": "Escribe, edita y depura con ayuda de IA en tiempo real.", + "ar": "اكتب وعدّل وصحّح الأخطاء بمساعدة الذكاء الاصطناعي في الوقت الفعلي.", + "fr": "Rédigez, modifiez et déboguez avec l’aide de l’IA en temps réel.", + "tr": "AI desteğiyle gerçek zamanlı olarak yazın, düzenleyin ve hata ayıklayın.", + "de": "Schreiben, bearbeiten und debuggen Sie mit KI-Unterstützung in Echtzeit.", + "uk": "Пишіть, редагуйте та налагоджуйте з підтримкою ШІ у реальному часі." + }, + "COMMON$PLAN_AGENT_DESCRIPTION": { + "en": "Outline goals, structure tasks, and map your next steps.", + "ja": "目標を明確にし、タスクを構造化し、次のステップを計画しましょう。", + "zh-CN": "概述目标、结构化任务，并规划下一步。", + "zh-TW": "概述目標、結構化任務，並規劃下一步。", + "ko-KR": "목표를 개요하고, 작업을 구조화하며, 다음 단계를 구상하세요.", + "no": "Skisser mål, strukturer oppgaver og planlegg dine neste steg.", + "it": "Definisci gli obiettivi, struttura le attività e pianifica i prossimi passi.", + "pt": "Esboce objetivos, estruture tarefas e trace seus próximos passos.", + "es": "Define objetivos, estructura tareas y planifica tus próximos pasos.", + "ar": "حدد الأهداف، نظم المهام، وارسم خطواتك التالية.", + "fr": "Dressez des objectifs, structurez vos tâches et planifiez vos prochaines étapes.", + "tr": "Hedefleri belirtin, görevleri yapılandırın ve sonraki adımlarınızı belirleyin.", + "de": "Umreißen Sie Ziele, strukturieren Sie Aufgaben und planen Sie Ihre nächsten Schritte.", + "uk": "Окресліть цілі, структуруйте завдання та сплануйте наступні кроки." } } From e3d0380c2e7d2b35d277d1b4c2fd3ea0e9fe5bb0 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Fri, 14 Nov 2025 00:10:25 +0700 Subject: [PATCH 007/108] feat(frontend): add support for the shift + tab shortcut to cycle through conversation modes (#11731) --- .../features/chat/change-agent-button.tsx | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/frontend/src/components/features/chat/change-agent-button.tsx b/frontend/src/components/features/chat/change-agent-button.tsx index 706f582b5934..cf27d9655485 100644 --- a/frontend/src/components/features/chat/change-agent-button.tsx +++ b/frontend/src/components/features/chat/change-agent-button.tsx @@ -37,6 +37,37 @@ export function ChangeAgentButton() { } }, [isAgentRunning, contextMenuOpen]); + // Handle Shift + Tab keyboard shortcut to cycle through modes + useEffect(() => { + if (!shouldUsePlanningAgent || isAgentRunning) { + return undefined; + } + + const handleKeyDown = (event: KeyboardEvent) => { + // Check for Shift + Tab combination + if (event.shiftKey && event.key === "Tab") { + // Prevent default tab navigation behavior + event.preventDefault(); + event.stopPropagation(); + + // Cycle between modes: code -> plan -> code + const nextMode = conversationMode === "code" ? "plan" : "code"; + setConversationMode(nextMode); + } + }; + + document.addEventListener("keydown", handleKeyDown); + + return () => { + document.removeEventListener("keydown", handleKeyDown); + }; + }, [ + shouldUsePlanningAgent, + isAgentRunning, + conversationMode, + setConversationMode, + ]); + const handleButtonClick = (event: React.MouseEvent) => { event.preventDefault(); event.stopPropagation(); From f24d2a61e6ac62076acff774aaab467b51d8420f Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 13 Nov 2025 17:55:23 +0000 Subject: [PATCH 008/108] Fix for wrong column name (#11735) --- .../app_conversation/sql_app_conversation_info_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py index c062fc3f8b13..30fb2b753b84 100644 --- a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py +++ b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py @@ -278,7 +278,7 @@ async def save_app_conversation_info( ) result = await self.db_session.execute(query) existing = result.scalar_one_or_none() - assert existing is None or existing.created_by_user_id == user_id + assert existing is None or existing.user_id == user_id metrics = info.metrics or MetricsSnapshot() usage = metrics.accumulated_token_usage or TokenUsage() From 24a9758434370a89521e2001d3a6483d59fa9494 Mon Sep 17 00:00:00 2001 From: jpelletier1 <44589723+jpelletier1@users.noreply.github.com> Date: Thu, 13 Nov 2025 16:10:00 -0500 Subject: [PATCH 009/108] Adding an Agent Builder Skill/Microagent (#11720) --- microagents/agent-builder.md | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 microagents/agent-builder.md diff --git a/microagents/agent-builder.md b/microagents/agent-builder.md new file mode 100644 index 000000000000..7d98d2dd5410 --- /dev/null +++ b/microagents/agent-builder.md @@ -0,0 +1,38 @@ +--- +name: agent_sdk_builder +version: 1.0.0 +author: openhands +agent: CodeActAgent +triggers: + - /agent-builder +inputs: + - name: INITIAL_PROMPT + description: "Initial SDK requirements" +--- + +# Agent Builder and Interviewer Role + +You are an expert requirements gatherer and agent builder. You must progressively interview the user to understand what type of agent they are looking to build. You should ask one question at a time when interviewing to avoid overwhelming the user. + +Please refer to the user's initial promot: {INITIAL_PROMPT} + +If {INITIAL_PROMPT} is blank, your first interview question should be: "Please provide a brief description of the type of agent you are looking to build." + +# Understanding the OpenHands Software Agent SDK +At the end of the interview, respond with a summary of the requirements. Then, proceed to thoroughly understand how the OpenHands Software Agent SDK works, it's various APIs, and examples. To do this: +- Clone the examples into a temporary workspace folder (under "temp/"): https://github.com/OpenHands/software-agent-sdk/tree/main/examples/01_standalone_sdk +- Clone the SDK docs into the same temporary workspace folder: https://github.com/OpenHands/docs/tree/main/sdk + +After analyzing the OpenHands Agent SDK, you may optionally ask additional clarifying questions in case it's important for the technical design of the agent. + +# Generating the SDK Plan +You can then proceed to build a technical implementation plan based on the user requirements and your understanding of how the OpenHands Agent SDK works. +- The plan should be stored in "plan/SDK_PLAN.md" from the root of the workspace. +- A visual representation of how the agent should work based on the SDK_PLAN.md. This should look like a flow diagram with nodes and edges. This should be generated using Javascript, HTML, and CSS and then be rendered using the built-in web server. Store this in the plan/ directory. + +# Implementing the Plan +After the plan is generated, please ask the user if they are ready to generate the SDK implementation. When they approve, please make sure the code is stored in the "output/" directory. Make sure the code provides logging that a user can see in the terminal. Ideally, the SDK is a single python file. + +Additional guidelines: +- Users can configure their LLM API Key using an environment variable named "LLM_API_KEY" +- Unless otherwise specified, default to this model: openhands/claude-sonnet-4-20250514. This is configurable through the LLM_BASE_MODEL environment variable. From 34fcc503509707291f50aacfd517ad8d42496b33 Mon Sep 17 00:00:00 2001 From: jpelletier1 <44589723+jpelletier1@users.noreply.github.com> Date: Thu, 13 Nov 2025 16:42:50 -0500 Subject: [PATCH 010/108] Update to include llms.txt (#11737) --- microagents/agent-builder.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/microagents/agent-builder.md b/microagents/agent-builder.md index 7d98d2dd5410..a1b7931e9e04 100644 --- a/microagents/agent-builder.md +++ b/microagents/agent-builder.md @@ -20,8 +20,9 @@ If {INITIAL_PROMPT} is blank, your first interview question should be: "Please p # Understanding the OpenHands Software Agent SDK At the end of the interview, respond with a summary of the requirements. Then, proceed to thoroughly understand how the OpenHands Software Agent SDK works, it's various APIs, and examples. To do this: -- Clone the examples into a temporary workspace folder (under "temp/"): https://github.com/OpenHands/software-agent-sdk/tree/main/examples/01_standalone_sdk -- Clone the SDK docs into the same temporary workspace folder: https://github.com/OpenHands/docs/tree/main/sdk +- First, research the OpenHands documentation which includes references to the Software Agent SDK: https://docs.openhands.dev/llms.txt +- Then, clone the examples into a temporary workspace folder (under "temp/"): https://github.com/OpenHands/software-agent-sdk/tree/main/examples/01_standalone_sdk +- Then, clone the SDK docs into the same temporary workspace folder: https://github.com/OpenHands/docs/tree/main/sdk After analyzing the OpenHands Agent SDK, you may optionally ask additional clarifying questions in case it's important for the technical design of the agent. From 7263657937063ff9ac905e4f51d4a73cccd96fc9 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Fri, 14 Nov 2025 11:34:30 +0700 Subject: [PATCH 011/108] feat(backend): include sub-conversation ids when fetching conversation details (#11734) --- .../app_conversation_models.py | 1 + .../sql_app_conversation_info_service.py | 38 +++++++++++++++++-- .../server/data_models/conversation_info.py | 1 + .../server/routes/manage_conversations.py | 3 ++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/openhands/app_server/app_conversation/app_conversation_models.py b/openhands/app_server/app_conversation/app_conversation_models.py index 4ed04cf91c7f..d918a2d9b14c 100644 --- a/openhands/app_server/app_conversation/app_conversation_models.py +++ b/openhands/app_server/app_conversation/app_conversation_models.py @@ -42,6 +42,7 @@ class AppConversationInfo(BaseModel): metrics: MetricsSnapshot | None = None parent_conversation_id: OpenHandsUUID | None = None + sub_conversation_ids: list[OpenHandsUUID] = Field(default_factory=list) created_at: datetime = Field(default_factory=utc_now) updated_at: datetime = Field(default_factory=utc_now) diff --git a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py index 30fb2b753b84..90da5f5d4be9 100644 --- a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py +++ b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py @@ -232,6 +232,26 @@ def _apply_filters( query = query.where(*conditions) return query + async def _get_sub_conversation_ids( + self, parent_conversation_id: UUID + ) -> list[UUID]: + """Get all sub-conversation IDs for a given parent conversation. + + Args: + parent_conversation_id: The ID of the parent conversation + + Returns: + List of sub-conversation IDs + """ + query = await self._secure_select() + query = query.where( + StoredConversationMetadata.parent_conversation_id + == str(parent_conversation_id) + ) + result_set = await self.db_session.execute(query) + rows = result_set.scalars().all() + return [UUID(row.conversation_id) for row in rows] + async def get_app_conversation_info( self, conversation_id: UUID ) -> AppConversationInfo | None: @@ -242,7 +262,9 @@ async def get_app_conversation_info( result_set = await self.db_session.execute(query) result = result_set.scalar_one_or_none() if result: - return self._to_info(result) + # Fetch sub-conversation IDs + sub_conversation_ids = await self._get_sub_conversation_ids(conversation_id) + return self._to_info(result, sub_conversation_ids=sub_conversation_ids) return None async def batch_get_app_conversation_info( @@ -261,8 +283,13 @@ async def batch_get_app_conversation_info( results: list[AppConversationInfo | None] = [] for conversation_id in conversation_id_strs: info = info_by_id.get(conversation_id) + sub_conversation_ids = await self._get_sub_conversation_ids( + UUID(conversation_id) + ) if info: - results.append(self._to_info(info)) + results.append( + self._to_info(info, sub_conversation_ids=sub_conversation_ids) + ) else: results.append(None) @@ -330,7 +357,11 @@ async def _secure_select(self): ) return query - def _to_info(self, stored: StoredConversationMetadata) -> AppConversationInfo: + def _to_info( + self, + stored: StoredConversationMetadata, + sub_conversation_ids: list[UUID] | None = None, + ) -> AppConversationInfo: # V1 conversations should always have a sandbox_id sandbox_id = stored.sandbox_id assert sandbox_id is not None @@ -375,6 +406,7 @@ def _to_info(self, stored: StoredConversationMetadata) -> AppConversationInfo: if stored.parent_conversation_id else None ), + sub_conversation_ids=sub_conversation_ids or [], created_at=created_at, updated_at=updated_at, ) diff --git a/openhands/server/data_models/conversation_info.py b/openhands/server/data_models/conversation_info.py index f4c4a77809a6..78af0e3dc121 100644 --- a/openhands/server/data_models/conversation_info.py +++ b/openhands/server/data_models/conversation_info.py @@ -28,3 +28,4 @@ class ConversationInfo: created_at: datetime = field(default_factory=lambda: datetime.now(timezone.utc)) pr_number: list[int] = field(default_factory=list) conversation_version: str = 'V0' + sub_conversation_ids: list[str] = field(default_factory=list) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 8984f79e8c49..0b828c76c03f 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -1432,4 +1432,7 @@ def _to_conversation_info(app_conversation: AppConversation) -> ConversationInfo created_at=app_conversation.created_at, pr_number=app_conversation.pr_number, conversation_version='V1', + sub_conversation_ids=[ + sub_id.hex for sub_id in app_conversation.sub_conversation_ids + ], ) From 8115d82f9601cc90579019dd07802722a6299e22 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 14 Nov 2025 14:08:34 +0000 Subject: [PATCH 012/108] feat: add created_at__gte filter to search_app_conversation_start_tasks (#11740) Co-authored-by: openhands --- .../v1-conversation-service.api.ts | 8 +- .../app_conversation_router.py | 10 ++ .../app_conversation_start_task_service.py | 3 + ...sql_app_conversation_start_task_service.py | 15 ++ ...sql_app_conversation_start_task_service.py | 142 ++++++++++++++++++ 5 files changed, 176 insertions(+), 2 deletions(-) diff --git a/frontend/src/api/conversation-service/v1-conversation-service.api.ts b/frontend/src/api/conversation-service/v1-conversation-service.api.ts index 717228c79ff7..93cd2ba85e88 100644 --- a/frontend/src/api/conversation-service/v1-conversation-service.api.ts +++ b/frontend/src/api/conversation-service/v1-conversation-service.api.ts @@ -111,11 +111,11 @@ class V1ConversationService { * Search for start tasks (ongoing tasks that haven't completed yet) * Use this to find tasks that were started but the user navigated away * - * Note: Backend only supports filtering by limit. To filter by repository/trigger, + * Note: Backend supports filtering by limit and created_at__gte. To filter by repository/trigger, * filter the results client-side after fetching. * * @param limit Maximum number of tasks to return (max 100) - * @returns Array of start tasks + * @returns Array of start tasks from the last 20 minutes */ static async searchStartTasks( limit: number = 100, @@ -123,6 +123,10 @@ class V1ConversationService { const params = new URLSearchParams(); params.append("limit", limit.toString()); + // Only get tasks from the last 20 minutes + const twentyMinutesAgo = new Date(Date.now() - 20 * 60 * 1000); + params.append("created_at__gte", twentyMinutesAgo.toISOString()); + const { data } = await openHands.get( `/api/v1/app-conversations/start-tasks/search?${params.toString()}`, ); diff --git a/openhands/app_server/app_conversation/app_conversation_router.py b/openhands/app_server/app_conversation/app_conversation_router.py index 83596b64a572..997e8b6528b3 100644 --- a/openhands/app_server/app_conversation/app_conversation_router.py +++ b/openhands/app_server/app_conversation/app_conversation_router.py @@ -207,6 +207,10 @@ async def search_app_conversation_start_tasks( UUID | None, Query(title='Filter by conversation ID equal to this value'), ] = None, + created_at__gte: Annotated[ + datetime | None, + Query(title='Filter by created_at greater than or equal to this datetime'), + ] = None, sort_order: Annotated[ AppConversationStartTaskSortOrder, Query(title='Sort order for the results'), @@ -233,6 +237,7 @@ async def search_app_conversation_start_tasks( return ( await app_conversation_start_task_service.search_app_conversation_start_tasks( conversation_id__eq=conversation_id__eq, + created_at__gte=created_at__gte, sort_order=sort_order, page_id=page_id, limit=limit, @@ -246,6 +251,10 @@ async def count_app_conversation_start_tasks( UUID | None, Query(title='Filter by conversation ID equal to this value'), ] = None, + created_at__gte: Annotated[ + datetime | None, + Query(title='Filter by created_at greater than or equal to this datetime'), + ] = None, app_conversation_start_task_service: AppConversationStartTaskService = ( app_conversation_start_task_service_dependency ), @@ -253,6 +262,7 @@ async def count_app_conversation_start_tasks( """Count conversation start tasks matching the given filters.""" return await app_conversation_start_task_service.count_app_conversation_start_tasks( conversation_id__eq=conversation_id__eq, + created_at__gte=created_at__gte, ) diff --git a/openhands/app_server/app_conversation/app_conversation_start_task_service.py b/openhands/app_server/app_conversation/app_conversation_start_task_service.py index 05229411f5bf..230b26cd8ff7 100644 --- a/openhands/app_server/app_conversation/app_conversation_start_task_service.py +++ b/openhands/app_server/app_conversation/app_conversation_start_task_service.py @@ -1,5 +1,6 @@ import asyncio from abc import ABC, abstractmethod +from datetime import datetime from uuid import UUID from openhands.app_server.app_conversation.app_conversation_models import ( @@ -18,6 +19,7 @@ class AppConversationStartTaskService(ABC): async def search_app_conversation_start_tasks( self, conversation_id__eq: UUID | None = None, + created_at__gte: datetime | None = None, sort_order: AppConversationStartTaskSortOrder = AppConversationStartTaskSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, @@ -28,6 +30,7 @@ async def search_app_conversation_start_tasks( async def count_app_conversation_start_tasks( self, conversation_id__eq: UUID | None = None, + created_at__gte: datetime | None = None, ) -> int: """Count conversation start tasks.""" diff --git a/openhands/app_server/app_conversation/sql_app_conversation_start_task_service.py b/openhands/app_server/app_conversation/sql_app_conversation_start_task_service.py index 91b48ab78127..4913e795bb62 100644 --- a/openhands/app_server/app_conversation/sql_app_conversation_start_task_service.py +++ b/openhands/app_server/app_conversation/sql_app_conversation_start_task_service.py @@ -18,6 +18,7 @@ import logging from dataclasses import dataclass +from datetime import datetime from typing import AsyncGenerator from uuid import UUID @@ -75,6 +76,7 @@ class SQLAppConversationStartTaskService(AppConversationStartTaskService): async def search_app_conversation_start_tasks( self, conversation_id__eq: UUID | None = None, + created_at__gte: datetime | None = None, sort_order: AppConversationStartTaskSortOrder = AppConversationStartTaskSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, @@ -95,6 +97,12 @@ async def search_app_conversation_start_tasks( == conversation_id__eq ) + # Apply created_at__gte filter + if created_at__gte is not None: + query = query.where( + StoredAppConversationStartTask.created_at >= created_at__gte + ) + # Add sort order if sort_order == AppConversationStartTaskSortOrder.CREATED_AT: query = query.order_by(StoredAppConversationStartTask.created_at) @@ -139,6 +147,7 @@ async def search_app_conversation_start_tasks( async def count_app_conversation_start_tasks( self, conversation_id__eq: UUID | None = None, + created_at__gte: datetime | None = None, ) -> int: """Count conversation start tasks.""" query = select(func.count(StoredAppConversationStartTask.id)) @@ -156,6 +165,12 @@ async def count_app_conversation_start_tasks( == conversation_id__eq ) + # Apply created_at__gte filter + if created_at__gte is not None: + query = query.where( + StoredAppConversationStartTask.created_at >= created_at__gte + ) + result = await self.session.execute(query) count = result.scalar() return count or 0 diff --git a/tests/unit/app_server/test_sql_app_conversation_start_task_service.py b/tests/unit/app_server/test_sql_app_conversation_start_task_service.py index 017f4f1fc847..943595e141f6 100644 --- a/tests/unit/app_server/test_sql_app_conversation_start_task_service.py +++ b/tests/unit/app_server/test_sql_app_conversation_start_task_service.py @@ -639,3 +639,145 @@ async def test_search_and_count_with_user_isolation( user2_count = await user2_service.count_app_conversation_start_tasks() assert user2_count == 1 + + async def test_search_app_conversation_start_tasks_with_created_at_gte_filter( + self, + service: SQLAppConversationStartTaskService, + sample_request: AppConversationStartRequest, + ): + """Test search with created_at__gte filter.""" + from datetime import timedelta + + from openhands.agent_server.models import utc_now + + # Create tasks with different creation times + base_time = utc_now() + + # Task 1: created 2 hours ago + task1 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.WORKING, + request=sample_request, + ) + task1.created_at = base_time - timedelta(hours=2) + await service.save_app_conversation_start_task(task1) + + # Task 2: created 1 hour ago + task2 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.READY, + request=sample_request, + ) + task2.created_at = base_time - timedelta(hours=1) + await service.save_app_conversation_start_task(task2) + + # Task 3: created 30 minutes ago + task3 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.WORKING, + request=sample_request, + ) + task3.created_at = base_time - timedelta(minutes=30) + await service.save_app_conversation_start_task(task3) + + # Search for tasks created in the last 90 minutes + filter_time = base_time - timedelta(minutes=90) + result = await service.search_app_conversation_start_tasks( + created_at__gte=filter_time + ) + + # Should return task2 and task3 (created within last 90 minutes) + assert len(result.items) == 2 + task_ids = [task.id for task in result.items] + assert task2.id in task_ids + assert task3.id in task_ids + assert task1.id not in task_ids + + # Test count with the same filter + count = await service.count_app_conversation_start_tasks( + created_at__gte=filter_time + ) + assert count == 2 + + # Search for tasks created in the last 45 minutes + filter_time_recent = base_time - timedelta(minutes=45) + result_recent = await service.search_app_conversation_start_tasks( + created_at__gte=filter_time_recent + ) + + # Should return only task3 + assert len(result_recent.items) == 1 + assert result_recent.items[0].id == task3.id + + # Test count with recent filter + count_recent = await service.count_app_conversation_start_tasks( + created_at__gte=filter_time_recent + ) + assert count_recent == 1 + + async def test_search_app_conversation_start_tasks_combined_filters( + self, + service: SQLAppConversationStartTaskService, + sample_request: AppConversationStartRequest, + ): + """Test search with both conversation_id and created_at__gte filters.""" + from datetime import timedelta + + from openhands.agent_server.models import utc_now + + conversation_id1 = uuid4() + conversation_id2 = uuid4() + base_time = utc_now() + + # Task 1: conversation_id1, created 2 hours ago + task1 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.WORKING, + app_conversation_id=conversation_id1, + request=sample_request, + ) + task1.created_at = base_time - timedelta(hours=2) + await service.save_app_conversation_start_task(task1) + + # Task 2: conversation_id1, created 30 minutes ago + task2 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.READY, + app_conversation_id=conversation_id1, + request=sample_request, + ) + task2.created_at = base_time - timedelta(minutes=30) + await service.save_app_conversation_start_task(task2) + + # Task 3: conversation_id2, created 30 minutes ago + task3 = AppConversationStartTask( + id=uuid4(), + created_by_user_id='user1', + status=AppConversationStartTaskStatus.WORKING, + app_conversation_id=conversation_id2, + request=sample_request, + ) + task3.created_at = base_time - timedelta(minutes=30) + await service.save_app_conversation_start_task(task3) + + # Search for tasks with conversation_id1 created in the last hour + filter_time = base_time - timedelta(hours=1) + result = await service.search_app_conversation_start_tasks( + conversation_id__eq=conversation_id1, created_at__gte=filter_time + ) + + # Should return only task2 (conversation_id1 and created within last hour) + assert len(result.items) == 1 + assert result.items[0].id == task2.id + assert result.items[0].app_conversation_id == conversation_id1 + + # Test count with combined filters + count = await service.count_app_conversation_start_tasks( + conversation_id__eq=conversation_id1, created_at__gte=filter_time + ) + assert count == 1 From 2841e35f24671929ecdc53f5d98ba0d7bfffeb03 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 14 Nov 2025 14:55:43 +0000 Subject: [PATCH 013/108] Do not get live status updates when they are not required (#11727) Co-authored-by: openhands --- .../sandbox/remote_sandbox_service.py | 2 - .../server/routes/manage_conversations.py | 42 +- .../app_server/test_remote_sandbox_service.py | 4 +- .../server/data_models/test_conversation.py | 388 ++++++++++++------ 4 files changed, 292 insertions(+), 144 deletions(-) diff --git a/openhands/app_server/sandbox/remote_sandbox_service.py b/openhands/app_server/sandbox/remote_sandbox_service.py index c7d444c4ec7c..c96b7362c08d 100644 --- a/openhands/app_server/sandbox/remote_sandbox_service.py +++ b/openhands/app_server/sandbox/remote_sandbox_service.py @@ -318,7 +318,6 @@ async def start_sandbox(self, sandbox_spec_id: str | None = None) -> SandboxInfo created_at=utc_now(), ) self.db_session.add(stored_sandbox) - await self.db_session.commit() # Prepare environment variables environment = await self._init_environment(sandbox_spec, sandbox_id) @@ -407,7 +406,6 @@ async def delete_sandbox(self, sandbox_id: str) -> bool: if not stored_sandbox: return False await self.db_session.delete(stored_sandbox) - await self.db_session.commit() runtime_data = await self._get_runtime(sandbox_id) response = await self._send_runtime_api_request( 'POST', diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 0b828c76c03f..7d0b1f1c0c87 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -1,3 +1,4 @@ +import asyncio import base64 import itertools import json @@ -7,10 +8,11 @@ from datetime import datetime, timedelta, timezone import base62 -from fastapi import APIRouter, Depends, status +from fastapi import APIRouter, Depends, Request, status from fastapi.responses import JSONResponse from jinja2 import Environment, FileSystemLoader from pydantic import BaseModel, ConfigDict, Field +from sqlalchemy.ext.asyncio import AsyncSession from openhands.app_server.app_conversation.app_conversation_info_service import ( AppConversationInfoService, @@ -24,9 +26,11 @@ from openhands.app_server.config import ( depends_app_conversation_info_service, depends_app_conversation_service, + depends_db_session, depends_sandbox_service, ) from openhands.app_server.sandbox.sandbox_service import SandboxService +from openhands.app_server.services.db_session_injector import set_db_session_keep_open from openhands.core.config.llm_config import LLMConfig from openhands.core.config.mcp_config import MCPConfig from openhands.core.logger import openhands_logger as logger @@ -99,6 +103,7 @@ app_conversation_service_dependency = depends_app_conversation_service() app_conversation_info_service_dependency = depends_app_conversation_info_service() sandbox_service_dependency = depends_sandbox_service() +db_session_dependency = depends_db_session() def _filter_conversations_by_age( @@ -467,16 +472,22 @@ async def get_conversation( @app.delete('/conversations/{conversation_id}') async def delete_conversation( + request: Request, conversation_id: str = Depends(validate_conversation_id), user_id: str | None = Depends(get_user_id), app_conversation_service: AppConversationService = app_conversation_service_dependency, + app_conversation_info_service: AppConversationInfoService = app_conversation_info_service_dependency, sandbox_service: SandboxService = sandbox_service_dependency, + db_session: AsyncSession = db_session_dependency, ) -> bool: + set_db_session_keep_open(request.state, True) # Try V1 conversation first v1_result = await _try_delete_v1_conversation( conversation_id, app_conversation_service, + app_conversation_info_service, sandbox_service, + db_session, ) if v1_result is not None: return v1_result @@ -488,23 +499,32 @@ async def delete_conversation( async def _try_delete_v1_conversation( conversation_id: str, app_conversation_service: AppConversationService, + app_conversation_info_service: AppConversationInfoService, sandbox_service: SandboxService, + db_session: AsyncSession, ) -> bool | None: """Try to delete a V1 conversation. Returns None if not a V1 conversation.""" result = None try: conversation_uuid = uuid.UUID(conversation_id) # Check if it's a V1 conversation by trying to get it - app_conversation = await app_conversation_service.get_app_conversation( - conversation_uuid + app_conversation_info = ( + await app_conversation_info_service.get_app_conversation_info( + conversation_uuid + ) ) - if app_conversation: + if app_conversation_info: # This is a V1 conversation, delete it using the app conversation service # Pass the conversation ID for secure deletion result = await app_conversation_service.delete_app_conversation( - app_conversation.id + app_conversation_info.id + ) + # Delete the sandbox in the background + asyncio.create_task( + _delete_sandbox_and_close_connection( + sandbox_service, app_conversation_info.sandbox_id, db_session + ) ) - await sandbox_service.delete_sandbox(app_conversation.sandbox_id) except (ValueError, TypeError): # Not a valid UUID, continue with V0 logic pass @@ -515,6 +535,16 @@ async def _try_delete_v1_conversation( return result +async def _delete_sandbox_and_close_connection( + sandbox_service: SandboxService, sandbox_id: str, db_session: AsyncSession +): + try: + await sandbox_service.delete_sandbox(sandbox_id) + await db_session.commit() + finally: + await db_session.aclose() + + async def _delete_v0_conversation(conversation_id: str, user_id: str | None) -> bool: """Delete a V0 conversation using the legacy logic.""" conversation_store = await ConversationStoreImpl.get_instance(config, user_id) diff --git a/tests/unit/app_server/test_remote_sandbox_service.py b/tests/unit/app_server/test_remote_sandbox_service.py index 1d917cc76021..567ecad2e30a 100644 --- a/tests/unit/app_server/test_remote_sandbox_service.py +++ b/tests/unit/app_server/test_remote_sandbox_service.py @@ -435,7 +435,7 @@ async def test_start_sandbox_success( 9 ) # max_num_sandboxes - 1 remote_sandbox_service.db_session.add.assert_called_once() - remote_sandbox_service.db_session.commit.assert_called_once() + remote_sandbox_service.db_session.commit.assert_not_called() @pytest.mark.asyncio async def test_start_sandbox_with_specific_spec( @@ -627,7 +627,7 @@ async def test_delete_sandbox_success(self, remote_sandbox_service): # Verify assert result is True remote_sandbox_service.db_session.delete.assert_called_once_with(stored_sandbox) - remote_sandbox_service.db_session.commit.assert_called_once() + remote_sandbox_service.db_session.commit.assert_not_called() remote_sandbox_service.httpx_client.request.assert_called_once_with( 'POST', 'https://api.example.com/stop', diff --git a/tests/unit/server/data_models/test_conversation.py b/tests/unit/server/data_models/test_conversation.py index 0917dc1facfd..c3cf34dac3cd 100644 --- a/tests/unit/server/data_models/test_conversation.py +++ b/tests/unit/server/data_models/test_conversation.py @@ -911,10 +911,16 @@ async def test_delete_conversation(): # Create a mock app conversation service mock_app_conversation_service = MagicMock() - mock_app_conversation_service.get_app_conversation = AsyncMock( + + # Create a mock app conversation info service + mock_app_conversation_info_service = MagicMock() + mock_app_conversation_info_service.get_app_conversation_info = AsyncMock( return_value=None ) + # Create a mock sandbox service + mock_sandbox_service = MagicMock() + # Mock the conversation manager with patch( 'openhands.server.routes.manage_conversations.conversation_manager' @@ -932,9 +938,12 @@ async def test_delete_conversation(): # Call delete_conversation result = await delete_conversation( + request=MagicMock(), conversation_id='some_conversation_id', user_id='12345', app_conversation_service=mock_app_conversation_service, + app_conversation_info_service=mock_app_conversation_info_service, + sandbox_service=mock_sandbox_service, ) # Verify the result @@ -972,42 +981,63 @@ async def test_delete_v1_conversation_success(): mock_service = MagicMock() mock_service_dep.return_value = mock_service - # Mock the conversation exists - mock_app_conversation = AppConversation( - id=conversation_uuid, - created_by_user_id='test_user', - sandbox_id='test-sandbox-id', - title='Test V1 Conversation', - sandbox_status=SandboxStatus.RUNNING, - execution_status=ConversationExecutionStatus.RUNNING, - session_api_key='test-api-key', - selected_repository='test/repo', - selected_branch='main', - git_provider=ProviderType.GITHUB, - trigger=ConversationTrigger.GUI, - created_at=datetime.now(timezone.utc), - updated_at=datetime.now(timezone.utc), - ) - mock_service.get_app_conversation = AsyncMock( - return_value=mock_app_conversation - ) - mock_service.delete_app_conversation = AsyncMock(return_value=True) + # Mock the app conversation info service + with patch( + 'openhands.server.routes.manage_conversations.app_conversation_info_service_dependency' + ) as mock_info_service_dep: + mock_info_service = MagicMock() + mock_info_service_dep.return_value = mock_info_service - # Call delete_conversation with V1 conversation ID - result = await delete_conversation( - conversation_id=conversation_id, - user_id='test_user', - app_conversation_service=mock_service, - ) + # Mock the sandbox service + with patch( + 'openhands.server.routes.manage_conversations.sandbox_service_dependency' + ) as mock_sandbox_service_dep: + mock_sandbox_service = MagicMock() + mock_sandbox_service_dep.return_value = mock_sandbox_service + + # Mock the conversation info exists + mock_app_conversation_info = AppConversation( + id=conversation_uuid, + created_by_user_id='test_user', + sandbox_id='test-sandbox-id', + title='Test V1 Conversation', + sandbox_status=SandboxStatus.RUNNING, + execution_status=ConversationExecutionStatus.RUNNING, + session_api_key='test-api-key', + selected_repository='test/repo', + selected_branch='main', + git_provider=ProviderType.GITHUB, + trigger=ConversationTrigger.GUI, + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + mock_info_service.get_app_conversation_info = AsyncMock( + return_value=mock_app_conversation_info + ) + mock_service.delete_app_conversation = AsyncMock(return_value=True) - # Verify the result - assert result is True + # Call delete_conversation with V1 conversation ID + result = await delete_conversation( + request=MagicMock(), + conversation_id=conversation_id, + user_id='test_user', + app_conversation_service=mock_service, + app_conversation_info_service=mock_info_service, + sandbox_service=mock_sandbox_service, + ) - # Verify that get_app_conversation was called - mock_service.get_app_conversation.assert_called_once_with(conversation_uuid) + # Verify the result + assert result is True - # Verify that delete_app_conversation was called with the conversation ID - mock_service.delete_app_conversation.assert_called_once_with(conversation_uuid) + # Verify that get_app_conversation_info was called + mock_info_service.get_app_conversation_info.assert_called_once_with( + conversation_uuid + ) + + # Verify that delete_app_conversation was called with the conversation ID + mock_service.delete_app_conversation.assert_called_once_with( + conversation_uuid + ) @pytest.mark.asyncio @@ -1025,25 +1055,46 @@ async def test_delete_v1_conversation_not_found(): mock_service = MagicMock() mock_service_dep.return_value = mock_service - # Mock the conversation doesn't exist - mock_service.get_app_conversation = AsyncMock(return_value=None) - mock_service.delete_app_conversation = AsyncMock(return_value=False) + # Mock the app conversation info service + with patch( + 'openhands.server.routes.manage_conversations.app_conversation_info_service_dependency' + ) as mock_info_service_dep: + mock_info_service = MagicMock() + mock_info_service_dep.return_value = mock_info_service - # Call delete_conversation with V1 conversation ID - result = await delete_conversation( - conversation_id=conversation_id, - user_id='test_user', - app_conversation_service=mock_service, - ) + # Mock the sandbox service + with patch( + 'openhands.server.routes.manage_conversations.sandbox_service_dependency' + ) as mock_sandbox_service_dep: + mock_sandbox_service = MagicMock() + mock_sandbox_service_dep.return_value = mock_sandbox_service + + # Mock the conversation doesn't exist + mock_info_service.get_app_conversation_info = AsyncMock( + return_value=None + ) + mock_service.delete_app_conversation = AsyncMock(return_value=False) - # Verify the result - assert result is False + # Call delete_conversation with V1 conversation ID + result = await delete_conversation( + request=MagicMock(), + conversation_id=conversation_id, + user_id='test_user', + app_conversation_service=mock_service, + app_conversation_info_service=mock_info_service, + sandbox_service=mock_sandbox_service, + ) + + # Verify the result + assert result is False - # Verify that get_app_conversation was called - mock_service.get_app_conversation.assert_called_once_with(conversation_uuid) + # Verify that get_app_conversation_info was called + mock_info_service.get_app_conversation_info.assert_called_once_with( + conversation_uuid + ) - # Verify that delete_app_conversation was NOT called - mock_service.delete_app_conversation.assert_not_called() + # Verify that delete_app_conversation was NOT called + mock_service.delete_app_conversation.assert_not_called() @pytest.mark.asyncio @@ -1091,19 +1142,40 @@ async def test_delete_v1_conversation_invalid_uuid(): mock_runtime_cls.delete = AsyncMock() mock_get_runtime_cls.return_value = mock_runtime_cls - # Call delete_conversation - result = await delete_conversation( - conversation_id=conversation_id, - user_id='test_user', - app_conversation_service=mock_service, - ) + # Mock the app conversation info service + with patch( + 'openhands.server.routes.manage_conversations.app_conversation_info_service_dependency' + ) as mock_info_service_dep: + mock_info_service = MagicMock() + mock_info_service_dep.return_value = mock_info_service + + # Mock the sandbox service + with patch( + 'openhands.server.routes.manage_conversations.sandbox_service_dependency' + ) as mock_sandbox_service_dep: + mock_sandbox_service = MagicMock() + mock_sandbox_service_dep.return_value = mock_sandbox_service + + # Call delete_conversation + result = await delete_conversation( + request=MagicMock(), + conversation_id=conversation_id, + user_id='test_user', + app_conversation_service=mock_service, + app_conversation_info_service=mock_info_service, + sandbox_service=mock_sandbox_service, + ) - # Verify the result - assert result is True + # Verify the result + assert result is True - # Verify V0 logic was used - mock_store.delete_metadata.assert_called_once_with(conversation_id) - mock_runtime_cls.delete.assert_called_once_with(conversation_id) + # Verify V0 logic was used + mock_store.delete_metadata.assert_called_once_with( + conversation_id + ) + mock_runtime_cls.delete.assert_called_once_with( + conversation_id + ) @pytest.mark.asyncio @@ -1121,57 +1193,84 @@ async def test_delete_v1_conversation_service_error(): mock_service = MagicMock() mock_service_dep.return_value = mock_service - # Mock service error - mock_service.get_app_conversation = AsyncMock( - side_effect=Exception('Service error') - ) - - # Mock V0 conversation logic as fallback + # Mock the app conversation info service with patch( - 'openhands.server.routes.manage_conversations.ConversationStoreImpl.get_instance' - ) as mock_get_instance: - mock_store = MagicMock() - mock_store.get_metadata = AsyncMock( - return_value=ConversationMetadata( - conversation_id=conversation_id, - title='Test V0 Conversation', - created_at=datetime.fromisoformat('2025-01-01T00:00:00+00:00'), - last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00+00:00'), - selected_repository='test/repo', - user_id='test_user', - ) - ) - mock_store.delete_metadata = AsyncMock() - mock_get_instance.return_value = mock_store + 'openhands.server.routes.manage_conversations.app_conversation_info_service_dependency' + ) as mock_info_service_dep: + mock_info_service = MagicMock() + mock_info_service_dep.return_value = mock_info_service - # Mock conversation manager + # Mock the sandbox service with patch( - 'openhands.server.routes.manage_conversations.conversation_manager' - ) as mock_manager: - mock_manager.is_agent_loop_running = AsyncMock(return_value=False) - mock_manager.get_connections = AsyncMock(return_value={}) + 'openhands.server.routes.manage_conversations.sandbox_service_dependency' + ) as mock_sandbox_service_dep: + mock_sandbox_service = MagicMock() + mock_sandbox_service_dep.return_value = mock_sandbox_service + + # Mock service error + mock_info_service.get_app_conversation_info = AsyncMock( + side_effect=Exception('Service error') + ) - # Mock runtime + # Mock V0 conversation logic as fallback with patch( - 'openhands.server.routes.manage_conversations.get_runtime_cls' - ) as mock_get_runtime_cls: - mock_runtime_cls = MagicMock() - mock_runtime_cls.delete = AsyncMock() - mock_get_runtime_cls.return_value = mock_runtime_cls - - # Call delete_conversation - result = await delete_conversation( - conversation_id=conversation_id, - user_id='test_user', - app_conversation_service=mock_service, + 'openhands.server.routes.manage_conversations.ConversationStoreImpl.get_instance' + ) as mock_get_instance: + mock_store = MagicMock() + mock_store.get_metadata = AsyncMock( + return_value=ConversationMetadata( + conversation_id=conversation_id, + title='Test V0 Conversation', + created_at=datetime.fromisoformat( + '2025-01-01T00:00:00+00:00' + ), + last_updated_at=datetime.fromisoformat( + '2025-01-01T00:01:00+00:00' + ), + selected_repository='test/repo', + user_id='test_user', + ) ) + mock_store.delete_metadata = AsyncMock() + mock_get_instance.return_value = mock_store + + # Mock conversation manager + with patch( + 'openhands.server.routes.manage_conversations.conversation_manager' + ) as mock_manager: + mock_manager.is_agent_loop_running = AsyncMock( + return_value=False + ) + mock_manager.get_connections = AsyncMock(return_value={}) + + # Mock runtime + with patch( + 'openhands.server.routes.manage_conversations.get_runtime_cls' + ) as mock_get_runtime_cls: + mock_runtime_cls = MagicMock() + mock_runtime_cls.delete = AsyncMock() + mock_get_runtime_cls.return_value = mock_runtime_cls + + # Call delete_conversation + result = await delete_conversation( + request=MagicMock(), + conversation_id=conversation_id, + user_id='test_user', + app_conversation_service=mock_service, + app_conversation_info_service=mock_info_service, + sandbox_service=mock_sandbox_service, + ) - # Verify the result (should fallback to V0) - assert result is True + # Verify the result (should fallback to V0) + assert result is True - # Verify V0 logic was used - mock_store.delete_metadata.assert_called_once_with(conversation_id) - mock_runtime_cls.delete.assert_called_once_with(conversation_id) + # Verify V0 logic was used + mock_store.delete_metadata.assert_called_once_with( + conversation_id + ) + mock_runtime_cls.delete.assert_called_once_with( + conversation_id + ) @pytest.mark.asyncio @@ -1195,42 +1294,63 @@ async def test_delete_v1_conversation_with_agent_server(): mock_service = MagicMock() mock_service_dep.return_value = mock_service - # Mock the conversation exists with running sandbox - mock_app_conversation = AppConversation( - id=conversation_uuid, - created_by_user_id='test_user', - sandbox_id='test-sandbox-id', - title='Test V1 Conversation', - sandbox_status=SandboxStatus.RUNNING, - execution_status=ConversationExecutionStatus.RUNNING, - session_api_key='test-api-key', - selected_repository='test/repo', - selected_branch='main', - git_provider=ProviderType.GITHUB, - trigger=ConversationTrigger.GUI, - created_at=datetime.now(timezone.utc), - updated_at=datetime.now(timezone.utc), - ) - mock_service.get_app_conversation = AsyncMock( - return_value=mock_app_conversation - ) - mock_service.delete_app_conversation = AsyncMock(return_value=True) + # Mock the app conversation info service + with patch( + 'openhands.server.routes.manage_conversations.app_conversation_info_service_dependency' + ) as mock_info_service_dep: + mock_info_service = MagicMock() + mock_info_service_dep.return_value = mock_info_service - # Call delete_conversation with V1 conversation ID - result = await delete_conversation( - conversation_id=conversation_id, - user_id='test_user', - app_conversation_service=mock_service, - ) + # Mock the sandbox service + with patch( + 'openhands.server.routes.manage_conversations.sandbox_service_dependency' + ) as mock_sandbox_service_dep: + mock_sandbox_service = MagicMock() + mock_sandbox_service_dep.return_value = mock_sandbox_service + + # Mock the conversation exists with running sandbox + mock_app_conversation_info = AppConversation( + id=conversation_uuid, + created_by_user_id='test_user', + sandbox_id='test-sandbox-id', + title='Test V1 Conversation', + sandbox_status=SandboxStatus.RUNNING, + execution_status=ConversationExecutionStatus.RUNNING, + session_api_key='test-api-key', + selected_repository='test/repo', + selected_branch='main', + git_provider=ProviderType.GITHUB, + trigger=ConversationTrigger.GUI, + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + mock_info_service.get_app_conversation_info = AsyncMock( + return_value=mock_app_conversation_info + ) + mock_service.delete_app_conversation = AsyncMock(return_value=True) - # Verify the result - assert result is True + # Call delete_conversation with V1 conversation ID + result = await delete_conversation( + request=MagicMock(), + conversation_id=conversation_id, + user_id='test_user', + app_conversation_service=mock_service, + app_conversation_info_service=mock_info_service, + sandbox_service=mock_sandbox_service, + ) - # Verify that get_app_conversation was called - mock_service.get_app_conversation.assert_called_once_with(conversation_uuid) + # Verify the result + assert result is True - # Verify that delete_app_conversation was called with the conversation ID - mock_service.delete_app_conversation.assert_called_once_with(conversation_uuid) + # Verify that get_app_conversation_info was called + mock_info_service.get_app_conversation_info.assert_called_once_with( + conversation_uuid + ) + + # Verify that delete_app_conversation was called with the conversation ID + mock_service.delete_app_conversation.assert_called_once_with( + conversation_uuid + ) @pytest.mark.asyncio From 833aae1833352c28f7c0e030358c8fa89c73598e Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Sat, 15 Nov 2025 00:21:27 +0700 Subject: [PATCH 014/108] feat(backend): exclude sub-conversations when searching for conversations (#11733) --- .../app_conversation_info_service.py | 1 + .../app_conversation_router.py | 10 +- .../app_conversation_service.py | 1 + .../live_status_app_conversation_service.py | 2 + .../sql_app_conversation_info_service.py | 8 + .../server/routes/manage_conversations.py | 11 +- .../test_sql_app_conversation_info_service.py | 380 ++++++++++++++++++ .../server/routes/test_conversation_routes.py | 261 +++++++++++- 8 files changed, 671 insertions(+), 3 deletions(-) diff --git a/openhands/app_server/app_conversation/app_conversation_info_service.py b/openhands/app_server/app_conversation/app_conversation_info_service.py index 1bbd06531b3a..56c4d77faea3 100644 --- a/openhands/app_server/app_conversation/app_conversation_info_service.py +++ b/openhands/app_server/app_conversation/app_conversation_info_service.py @@ -26,6 +26,7 @@ async def search_app_conversation_info( sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, + include_sub_conversations: bool = False, ) -> AppConversationInfoPage: """Search for sandboxed conversations.""" diff --git a/openhands/app_server/app_conversation/app_conversation_router.py b/openhands/app_server/app_conversation/app_conversation_router.py index 997e8b6528b3..b66d9983621b 100644 --- a/openhands/app_server/app_conversation/app_conversation_router.py +++ b/openhands/app_server/app_conversation/app_conversation_router.py @@ -99,6 +99,12 @@ async def search_app_conversations( lte=100, ), ] = 100, + include_sub_conversations: Annotated[ + bool, + Query( + title='If True, include sub-conversations in the results. If False (default), exclude all sub-conversations.' + ), + ] = False, app_conversation_service: AppConversationService = ( app_conversation_service_dependency ), @@ -114,6 +120,7 @@ async def search_app_conversations( updated_at__lt=updated_at__lt, page_id=page_id, limit=limit, + include_sub_conversations=include_sub_conversations, ) @@ -193,7 +200,8 @@ async def stream_app_conversation_start( user_context: UserContext = user_context_dependency, ) -> list[AppConversationStartTask]: """Start an app conversation start task and stream updates from it. - Leaves the connection open until either the conversation starts or there was an error""" + Leaves the connection open until either the conversation starts or there was an error + """ response = StreamingResponse( _stream_app_conversation_start(request, user_context), media_type='application/json', diff --git a/openhands/app_server/app_conversation/app_conversation_service.py b/openhands/app_server/app_conversation/app_conversation_service.py index d910856c7692..8c39a66ae5da 100644 --- a/openhands/app_server/app_conversation/app_conversation_service.py +++ b/openhands/app_server/app_conversation/app_conversation_service.py @@ -30,6 +30,7 @@ async def search_app_conversations( sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, + include_sub_conversations: bool = False, ) -> AppConversationPage: """Search for sandboxed conversations.""" diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index 73f26781a605..e8bd8fc33174 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -105,6 +105,7 @@ async def search_app_conversations( sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 20, + include_sub_conversations: bool = False, ) -> AppConversationPage: """Search for sandboxed conversations.""" page = await self.app_conversation_info_service.search_app_conversation_info( @@ -116,6 +117,7 @@ async def search_app_conversations( sort_order=sort_order, page_id=page_id, limit=limit, + include_sub_conversations=include_sub_conversations, ) conversations: list[AppConversation] = await self._build_app_conversations( page.items diff --git a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py index 90da5f5d4be9..9b13711e920f 100644 --- a/openhands/app_server/app_conversation/sql_app_conversation_info_service.py +++ b/openhands/app_server/app_conversation/sql_app_conversation_info_service.py @@ -111,10 +111,18 @@ async def search_app_conversation_info( sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, + include_sub_conversations: bool = False, ) -> AppConversationInfoPage: """Search for sandboxed conversations without permission checks.""" query = await self._secure_select() + # Conditionally exclude sub-conversations based on the parameter + if not include_sub_conversations: + # Exclude sub-conversations (only include top-level conversations) + query = query.where( + StoredConversationMetadata.parent_conversation_id.is_(None) + ) + query = self._apply_filters( query=query, title__contains=title__contains, diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 7d0b1f1c0c87..56f6b95f6c5e 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -6,9 +6,10 @@ import re import uuid from datetime import datetime, timedelta, timezone +from typing import Annotated import base62 -from fastapi import APIRouter, Depends, Request, status +from fastapi import APIRouter, Depends, Query, Request, status from fastapi.responses import JSONResponse from jinja2 import Environment, FileSystemLoader from pydantic import BaseModel, ConfigDict, Field @@ -309,6 +310,12 @@ async def search_conversations( limit: int = 20, selected_repository: str | None = None, conversation_trigger: ConversationTrigger | None = None, + include_sub_conversations: Annotated[ + bool, + Query( + title='If True, include sub-conversations in the results. If False (default), exclude all sub-conversations.' + ), + ] = False, conversation_store: ConversationStore = Depends(get_conversation_store), app_conversation_service: AppConversationService = app_conversation_service_dependency, ) -> ConversationInfoResultSet: @@ -343,6 +350,7 @@ async def search_conversations( limit=limit, # Apply age filter at the service level if possible created_at__gte=age_filter_date, + include_sub_conversations=include_sub_conversations, ) # Convert V1 conversations to ConversationInfo format @@ -1187,6 +1195,7 @@ async def _fetch_v1_conversations_safe( app_conversation_service: App conversation service for V1 v1_page_id: Page ID for V1 pagination limit: Maximum number of results + include_sub_conversations: If True, include sub-conversations in results Returns: Tuple of (v1_conversations, v1_next_page_id) diff --git a/tests/unit/app_server/test_sql_app_conversation_info_service.py b/tests/unit/app_server/test_sql_app_conversation_info_service.py index 2ff5974f738e..393e2e654bce 100644 --- a/tests/unit/app_server/test_sql_app_conversation_info_service.py +++ b/tests/unit/app_server/test_sql_app_conversation_info_service.py @@ -623,3 +623,383 @@ async def test_complex_date_range_filters( created_at__gte=start_time, created_at__lt=end_time ) assert count == 2 + + @pytest.mark.asyncio + async def test_search_excludes_sub_conversations_by_default( + self, + service: SQLAppConversationInfoService, + ): + """Test that search excludes sub-conversations by default.""" + # Create a parent conversation + parent_id = uuid4() + parent_info = AppConversationInfo( + id=parent_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent', + title='Parent Conversation', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations + sub_info_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub1', + title='Sub Conversation 1', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 13, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc), + ) + + sub_info_2 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub2', + title='Sub Conversation 2', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 14, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 14, 30, 0, tzinfo=timezone.utc), + ) + + # Save all conversations + await service.save_app_conversation_info(parent_info) + await service.save_app_conversation_info(sub_info_1) + await service.save_app_conversation_info(sub_info_2) + + # Search without include_sub_conversations (default False) + page = await service.search_app_conversation_info() + + # Should only return the parent conversation + assert len(page.items) == 1 + assert page.items[0].id == parent_id + assert page.items[0].title == 'Parent Conversation' + assert page.items[0].parent_conversation_id is None + + @pytest.mark.asyncio + async def test_search_includes_sub_conversations_when_flag_true( + self, + service: SQLAppConversationInfoService, + ): + """Test that search includes sub-conversations when include_sub_conversations=True.""" + # Create a parent conversation + parent_id = uuid4() + parent_info = AppConversationInfo( + id=parent_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent', + title='Parent Conversation', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations + sub_info_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub1', + title='Sub Conversation 1', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 13, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc), + ) + + sub_info_2 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub2', + title='Sub Conversation 2', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 14, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 14, 30, 0, tzinfo=timezone.utc), + ) + + # Save all conversations + await service.save_app_conversation_info(parent_info) + await service.save_app_conversation_info(sub_info_1) + await service.save_app_conversation_info(sub_info_2) + + # Search with include_sub_conversations=True + page = await service.search_app_conversation_info( + include_sub_conversations=True + ) + + # Should return all conversations (1 parent + 2 sub-conversations) + assert len(page.items) == 3 + + # Verify all conversations are present + conversation_ids = {item.id for item in page.items} + assert parent_id in conversation_ids + assert sub_info_1.id in conversation_ids + assert sub_info_2.id in conversation_ids + + # Verify parent conversation has no parent_conversation_id + parent_item = next(item for item in page.items if item.id == parent_id) + assert parent_item.parent_conversation_id is None + + # Verify sub-conversations have parent_conversation_id set + sub_item_1 = next(item for item in page.items if item.id == sub_info_1.id) + assert sub_item_1.parent_conversation_id == parent_id + + sub_item_2 = next(item for item in page.items if item.id == sub_info_2.id) + assert sub_item_2.parent_conversation_id == parent_id + + @pytest.mark.asyncio + async def test_search_sub_conversations_with_filters( + self, + service: SQLAppConversationInfoService, + ): + """Test that include_sub_conversations works correctly with other filters.""" + # Create a parent conversation + parent_id = uuid4() + parent_info = AppConversationInfo( + id=parent_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent', + title='Parent Conversation', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations with different titles + sub_info_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub1', + title='Sub Conversation Alpha', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 13, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc), + ) + + sub_info_2 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub2', + title='Sub Conversation Beta', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 14, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 14, 30, 0, tzinfo=timezone.utc), + ) + + # Save all conversations + await service.save_app_conversation_info(parent_info) + await service.save_app_conversation_info(sub_info_1) + await service.save_app_conversation_info(sub_info_2) + + # Search with title filter and include_sub_conversations=False (default) + page = await service.search_app_conversation_info(title__contains='Alpha') + # Should only find parent if it matches, but parent doesn't have "Alpha" + # So should find nothing or only sub if we include them + assert len(page.items) == 0 + + # Search with title filter and include_sub_conversations=True + page = await service.search_app_conversation_info( + title__contains='Alpha', include_sub_conversations=True + ) + # Should find the sub-conversation with "Alpha" in title + assert len(page.items) == 1 + assert page.items[0].title == 'Sub Conversation Alpha' + assert page.items[0].parent_conversation_id == parent_id + + # Search with title filter for "Parent" and include_sub_conversations=True + page = await service.search_app_conversation_info( + title__contains='Parent', include_sub_conversations=True + ) + # Should find the parent conversation + assert len(page.items) == 1 + assert page.items[0].title == 'Parent Conversation' + assert page.items[0].parent_conversation_id is None + + @pytest.mark.asyncio + async def test_search_sub_conversations_with_date_filters( + self, + service: SQLAppConversationInfoService, + ): + """Test that include_sub_conversations works correctly with date filters.""" + # Create a parent conversation + parent_id = uuid4() + parent_info = AppConversationInfo( + id=parent_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent', + title='Parent Conversation', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations at different times + sub_info_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub1', + title='Sub Conversation 1', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 13, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc), + ) + + sub_info_2 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub2', + title='Sub Conversation 2', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 14, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 14, 30, 0, tzinfo=timezone.utc), + ) + + # Save all conversations + await service.save_app_conversation_info(parent_info) + await service.save_app_conversation_info(sub_info_1) + await service.save_app_conversation_info(sub_info_2) + + # Search with date filter and include_sub_conversations=False (default) + cutoff_time = datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc) + page = await service.search_app_conversation_info(created_at__gte=cutoff_time) + # Should only return parent if it matches the filter, but parent is at 12:00 + assert len(page.items) == 0 + + # Search with date filter and include_sub_conversations=True + page = await service.search_app_conversation_info( + created_at__gte=cutoff_time, include_sub_conversations=True + ) + # Should find sub-conversations created after cutoff (sub_info_2 at 14:00) + assert len(page.items) == 1 + assert page.items[0].id == sub_info_2.id + assert page.items[0].parent_conversation_id == parent_id + + @pytest.mark.asyncio + async def test_search_multiple_parents_with_sub_conversations( + self, + service: SQLAppConversationInfoService, + ): + """Test search with multiple parent conversations and their sub-conversations.""" + # Create first parent conversation + parent1_id = uuid4() + parent1_info = AppConversationInfo( + id=parent1_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent1', + title='Parent 1', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create second parent conversation + parent2_id = uuid4() + parent2_info = AppConversationInfo( + id=parent2_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent2', + title='Parent 2', + created_at=datetime(2024, 1, 1, 13, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations for parent1 + sub1_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub1_1', + title='Sub 1-1', + parent_conversation_id=parent1_id, + created_at=datetime(2024, 1, 1, 14, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 14, 30, 0, tzinfo=timezone.utc), + ) + + # Create sub-conversations for parent2 + sub2_1 = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id='sandbox_sub2_1', + title='Sub 2-1', + parent_conversation_id=parent2_id, + created_at=datetime(2024, 1, 1, 15, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 15, 30, 0, tzinfo=timezone.utc), + ) + + # Save all conversations + await service.save_app_conversation_info(parent1_info) + await service.save_app_conversation_info(parent2_info) + await service.save_app_conversation_info(sub1_1) + await service.save_app_conversation_info(sub2_1) + + # Search without include_sub_conversations (default False) + page = await service.search_app_conversation_info() + # Should only return the 2 parent conversations + assert len(page.items) == 2 + conversation_ids = {item.id for item in page.items} + assert parent1_id in conversation_ids + assert parent2_id in conversation_ids + assert sub1_1.id not in conversation_ids + assert sub2_1.id not in conversation_ids + + # Search with include_sub_conversations=True + page = await service.search_app_conversation_info( + include_sub_conversations=True + ) + # Should return all 4 conversations (2 parents + 2 sub-conversations) + assert len(page.items) == 4 + conversation_ids = {item.id for item in page.items} + assert parent1_id in conversation_ids + assert parent2_id in conversation_ids + assert sub1_1.id in conversation_ids + assert sub2_1.id in conversation_ids + + @pytest.mark.asyncio + async def test_search_sub_conversations_with_pagination( + self, + service: SQLAppConversationInfoService, + ): + """Test that include_sub_conversations works correctly with pagination.""" + # Create a parent conversation + parent_id = uuid4() + parent_info = AppConversationInfo( + id=parent_id, + created_by_user_id='test_user_123', + sandbox_id='sandbox_parent', + title='Parent Conversation', + created_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 12, 30, 0, tzinfo=timezone.utc), + ) + + # Create multiple sub-conversations + sub_conversations = [] + for i in range(5): + sub_info = AppConversationInfo( + id=uuid4(), + created_by_user_id='test_user_123', + sandbox_id=f'sandbox_sub{i}', + title=f'Sub Conversation {i}', + parent_conversation_id=parent_id, + created_at=datetime(2024, 1, 1, 13 + i, 0, 0, tzinfo=timezone.utc), + updated_at=datetime(2024, 1, 1, 13 + i, 30, 0, tzinfo=timezone.utc), + ) + sub_conversations.append(sub_info) + await service.save_app_conversation_info(sub_info) + + # Save parent + await service.save_app_conversation_info(parent_info) + + # Search with include_sub_conversations=True and pagination + page1 = await service.search_app_conversation_info( + include_sub_conversations=True, limit=3 + ) + # Should return 3 items (1 parent + 2 sub-conversations) + assert len(page1.items) == 3 + assert page1.next_page_id is not None + + # Get next page + page2 = await service.search_app_conversation_info( + include_sub_conversations=True, limit=3, page_id=page1.next_page_id + ) + # Should return remaining items + assert len(page2.items) == 3 + assert page2.next_page_id is None + + # Verify all conversations are present across pages + all_ids = {item.id for item in page1.items} | {item.id for item in page2.items} + assert parent_id in all_ids + for sub_info in sub_conversations: + assert sub_info.id in all_ids diff --git a/tests/unit/server/routes/test_conversation_routes.py b/tests/unit/server/routes/test_conversation_routes.py index 3d374f688ea1..343894cefa87 100644 --- a/tests/unit/server/routes/test_conversation_routes.py +++ b/tests/unit/server/routes/test_conversation_routes.py @@ -13,6 +13,7 @@ from openhands.app_server.app_conversation.app_conversation_models import ( AgentType, AppConversationInfo, + AppConversationPage, AppConversationStartRequest, AppConversationStartTask, AppConversationStartTaskStatus, @@ -22,6 +23,9 @@ ) from openhands.microagent.microagent import KnowledgeMicroagent, RepoMicroagent from openhands.microagent.types import MicroagentMetadata, MicroagentType +from openhands.server.data_models.conversation_info_result_set import ( + ConversationInfoResultSet, +) from openhands.server.routes.conversation import ( AddMessageRequest, add_message, @@ -29,11 +33,15 @@ ) from openhands.server.routes.manage_conversations import ( UpdateConversationRequest, + search_conversations, update_conversation, ) from openhands.server.session.conversation import ServerConversation from openhands.storage.conversation.conversation_store import ConversationStore -from openhands.storage.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_metadata import ( + ConversationMetadata, + ConversationTrigger, +) @pytest.mark.asyncio @@ -1200,3 +1208,254 @@ async def mock_start_generator(request): assert task.request.parent_conversation_id == parent_conversation_id assert task.sandbox_id == sandbox_id break + + +@pytest.mark.asyncio +async def test_search_conversations_include_sub_conversations_default_false(): + """Test that include_sub_conversations defaults to False when not provided.""" + with patch('openhands.server.routes.manage_conversations.config') as mock_config: + mock_config.conversation_max_age_seconds = 864000 # 10 days + with patch( + 'openhands.server.routes.manage_conversations.conversation_manager' + ) as mock_manager: + + async def mock_get_running_agent_loops(*args, **kwargs): + return set() + + async def mock_get_connections(*args, **kwargs): + return {} + + async def get_agent_loop_info(*args, **kwargs): + return [] + + mock_manager.get_running_agent_loops = mock_get_running_agent_loops + mock_manager.get_connections = mock_get_connections + mock_manager.get_agent_loop_info = get_agent_loop_info + with patch( + 'openhands.server.routes.manage_conversations.datetime' + ) as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat( + '2025-01-01T00:00:00+00:00' + ) + mock_datetime.fromisoformat = datetime.fromisoformat + mock_datetime.timezone = timezone + + # Mock the conversation store + mock_store = MagicMock() + mock_store.search = AsyncMock( + return_value=ConversationInfoResultSet(results=[]) + ) + + # Create a mock app conversation service + mock_app_conversation_service = AsyncMock() + mock_app_conversation_service.search_app_conversations.return_value = ( + AppConversationPage(items=[]) + ) + + # Call search_conversations without include_sub_conversations parameter + await search_conversations( + page_id=None, + limit=20, + selected_repository=None, + conversation_trigger=None, + conversation_store=mock_store, + app_conversation_service=mock_app_conversation_service, + ) + + # Verify that search_app_conversations was called with include_sub_conversations=False (default) + mock_app_conversation_service.search_app_conversations.assert_called_once() + call_kwargs = ( + mock_app_conversation_service.search_app_conversations.call_args[1] + ) + assert call_kwargs.get('include_sub_conversations') is False + + +@pytest.mark.asyncio +async def test_search_conversations_include_sub_conversations_explicit_false(): + """Test that include_sub_conversations=False is properly passed through.""" + with patch('openhands.server.routes.manage_conversations.config') as mock_config: + mock_config.conversation_max_age_seconds = 864000 # 10 days + with patch( + 'openhands.server.routes.manage_conversations.conversation_manager' + ) as mock_manager: + + async def mock_get_running_agent_loops(*args, **kwargs): + return set() + + async def mock_get_connections(*args, **kwargs): + return {} + + async def get_agent_loop_info(*args, **kwargs): + return [] + + mock_manager.get_running_agent_loops = mock_get_running_agent_loops + mock_manager.get_connections = mock_get_connections + mock_manager.get_agent_loop_info = get_agent_loop_info + with patch( + 'openhands.server.routes.manage_conversations.datetime' + ) as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat( + '2025-01-01T00:00:00+00:00' + ) + mock_datetime.fromisoformat = datetime.fromisoformat + mock_datetime.timezone = timezone + + # Mock the conversation store + mock_store = MagicMock() + mock_store.search = AsyncMock( + return_value=ConversationInfoResultSet(results=[]) + ) + + # Create a mock app conversation service + mock_app_conversation_service = AsyncMock() + mock_app_conversation_service.search_app_conversations.return_value = ( + AppConversationPage(items=[]) + ) + + # Call search_conversations with include_sub_conversations=False + await search_conversations( + page_id=None, + limit=20, + selected_repository=None, + conversation_trigger=None, + include_sub_conversations=False, + conversation_store=mock_store, + app_conversation_service=mock_app_conversation_service, + ) + + # Verify that search_app_conversations was called with include_sub_conversations=False + mock_app_conversation_service.search_app_conversations.assert_called_once() + call_kwargs = ( + mock_app_conversation_service.search_app_conversations.call_args[1] + ) + assert call_kwargs.get('include_sub_conversations') is False + + +@pytest.mark.asyncio +async def test_search_conversations_include_sub_conversations_explicit_true(): + """Test that include_sub_conversations=True is properly passed through.""" + with patch('openhands.server.routes.manage_conversations.config') as mock_config: + mock_config.conversation_max_age_seconds = 864000 # 10 days + with patch( + 'openhands.server.routes.manage_conversations.conversation_manager' + ) as mock_manager: + + async def mock_get_running_agent_loops(*args, **kwargs): + return set() + + async def mock_get_connections(*args, **kwargs): + return {} + + async def get_agent_loop_info(*args, **kwargs): + return [] + + mock_manager.get_running_agent_loops = mock_get_running_agent_loops + mock_manager.get_connections = mock_get_connections + mock_manager.get_agent_loop_info = get_agent_loop_info + with patch( + 'openhands.server.routes.manage_conversations.datetime' + ) as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat( + '2025-01-01T00:00:00+00:00' + ) + mock_datetime.fromisoformat = datetime.fromisoformat + mock_datetime.timezone = timezone + + # Mock the conversation store + mock_store = MagicMock() + mock_store.search = AsyncMock( + return_value=ConversationInfoResultSet(results=[]) + ) + + # Create a mock app conversation service + mock_app_conversation_service = AsyncMock() + mock_app_conversation_service.search_app_conversations.return_value = ( + AppConversationPage(items=[]) + ) + + # Call search_conversations with include_sub_conversations=True + await search_conversations( + page_id=None, + limit=20, + selected_repository=None, + conversation_trigger=None, + include_sub_conversations=True, + conversation_store=mock_store, + app_conversation_service=mock_app_conversation_service, + ) + + # Verify that search_app_conversations was called with include_sub_conversations=True + mock_app_conversation_service.search_app_conversations.assert_called_once() + call_kwargs = ( + mock_app_conversation_service.search_app_conversations.call_args[1] + ) + assert call_kwargs.get('include_sub_conversations') is True + + +@pytest.mark.asyncio +async def test_search_conversations_include_sub_conversations_with_other_filters(): + """Test that include_sub_conversations works correctly with other filters.""" + with patch('openhands.server.routes.manage_conversations.config') as mock_config: + mock_config.conversation_max_age_seconds = 864000 # 10 days + with patch( + 'openhands.server.routes.manage_conversations.conversation_manager' + ) as mock_manager: + + async def mock_get_running_agent_loops(*args, **kwargs): + return set() + + async def mock_get_connections(*args, **kwargs): + return {} + + async def get_agent_loop_info(*args, **kwargs): + return [] + + mock_manager.get_running_agent_loops = mock_get_running_agent_loops + mock_manager.get_connections = mock_get_connections + mock_manager.get_agent_loop_info = get_agent_loop_info + with patch( + 'openhands.server.routes.manage_conversations.datetime' + ) as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat( + '2025-01-01T00:00:00+00:00' + ) + mock_datetime.fromisoformat = datetime.fromisoformat + mock_datetime.timezone = timezone + + # Mock the conversation store + mock_store = MagicMock() + mock_store.search = AsyncMock( + return_value=ConversationInfoResultSet(results=[]) + ) + + # Create a mock app conversation service + mock_app_conversation_service = AsyncMock() + mock_app_conversation_service.search_app_conversations.return_value = ( + AppConversationPage(items=[]) + ) + + # Create a valid base64-encoded page_id for testing + import base64 + + page_id_data = json.dumps({'v0': None, 'v1': 'test_v1_page_id'}) + encoded_page_id = base64.b64encode(page_id_data.encode()).decode() + + # Call search_conversations with include_sub_conversations and other filters + await search_conversations( + page_id=encoded_page_id, + limit=50, + selected_repository='test/repo', + conversation_trigger=ConversationTrigger.GUI, + include_sub_conversations=True, + conversation_store=mock_store, + app_conversation_service=mock_app_conversation_service, + ) + + # Verify that search_app_conversations was called with all parameters including include_sub_conversations=True + mock_app_conversation_service.search_app_conversations.assert_called_once() + call_kwargs = ( + mock_app_conversation_service.search_app_conversations.call_args[1] + ) + assert call_kwargs.get('include_sub_conversations') is True + assert call_kwargs.get('page_id') == 'test_v1_page_id' + assert call_kwargs.get('limit') == 50 From d6fab190bfb55f07cc380993b45ec4f781706a92 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Sat, 15 Nov 2025 09:43:21 +0700 Subject: [PATCH 015/108] feat(frontend): integrate with the API to create a sub-conversation for the planning agent (#11730) --- .../v1-conversation-service.api.ts | 4 ++ .../v1-conversation-service.types.ts | 2 + frontend/src/api/open-hands.types.ts | 1 + .../features/chat/change-agent-button.tsx | 66 +++++++++++++++---- .../hooks/mutation/use-create-conversation.ts | 6 ++ frontend/src/i18n/declaration.ts | 1 + frontend/src/i18n/translation.json | 16 +++++ 7 files changed, 84 insertions(+), 12 deletions(-) diff --git a/frontend/src/api/conversation-service/v1-conversation-service.api.ts b/frontend/src/api/conversation-service/v1-conversation-service.api.ts index 93cd2ba85e88..5ca7daf09a19 100644 --- a/frontend/src/api/conversation-service/v1-conversation-service.api.ts +++ b/frontend/src/api/conversation-service/v1-conversation-service.api.ts @@ -60,6 +60,8 @@ class V1ConversationService { selected_branch?: string, conversationInstructions?: string, trigger?: ConversationTrigger, + parent_conversation_id?: string, + agent_type?: "default" | "plan", ): Promise { const body: V1AppConversationStartRequest = { selected_repository: selectedRepository, @@ -67,6 +69,8 @@ class V1ConversationService { selected_branch, title: conversationInstructions, trigger, + parent_conversation_id: parent_conversation_id || null, + agent_type, }; // Add initial message if provided diff --git a/frontend/src/api/conversation-service/v1-conversation-service.types.ts b/frontend/src/api/conversation-service/v1-conversation-service.types.ts index b48ce5bd6b9b..34414484721d 100644 --- a/frontend/src/api/conversation-service/v1-conversation-service.types.ts +++ b/frontend/src/api/conversation-service/v1-conversation-service.types.ts @@ -30,6 +30,8 @@ export interface V1AppConversationStartRequest { title?: string | null; trigger?: ConversationTrigger | null; pr_number?: number[]; + parent_conversation_id?: string | null; + agent_type?: "default" | "plan"; } export type V1AppConversationStartTaskStatus = diff --git a/frontend/src/api/open-hands.types.ts b/frontend/src/api/open-hands.types.ts index 9a30e46027b0..47d34fe567db 100644 --- a/frontend/src/api/open-hands.types.ts +++ b/frontend/src/api/open-hands.types.ts @@ -77,6 +77,7 @@ export interface Conversation { session_api_key: string | null; pr_number?: number[] | null; conversation_version?: "V0" | "V1"; + sub_conversation_ids?: string[]; } export interface ResultSet { diff --git a/frontend/src/components/features/chat/change-agent-button.tsx b/frontend/src/components/features/chat/change-agent-button.tsx index cf27d9655485..6d41f5cfc1a7 100644 --- a/frontend/src/components/features/chat/change-agent-button.tsx +++ b/frontend/src/components/features/chat/change-agent-button.tsx @@ -1,4 +1,4 @@ -import React, { useMemo, useEffect } from "react"; +import React, { useMemo, useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import { Typography } from "#/ui/typography"; import { I18nKey } from "#/i18n/declaration"; @@ -11,10 +11,12 @@ import { cn } from "#/utils/utils"; import { USE_PLANNING_AGENT } from "#/utils/feature-flags"; import { useAgentState } from "#/hooks/use-agent-state"; import { AgentState } from "#/types/agent-state"; +import { useActiveConversation } from "#/hooks/query/use-active-conversation"; +import { useCreateConversation } from "#/hooks/mutation/use-create-conversation"; +import { displaySuccessToast } from "#/utils/custom-toast-handlers"; export function ChangeAgentButton() { - const { t } = useTranslation(); - const [contextMenuOpen, setContextMenuOpen] = React.useState(false); + const [contextMenuOpen, setContextMenuOpen] = useState(false); const conversationMode = useConversationStore( (state) => state.conversationMode, @@ -28,8 +30,14 @@ export function ChangeAgentButton() { const { curAgentState } = useAgentState(); + const { t } = useTranslation(); + const isAgentRunning = curAgentState === AgentState.RUNNING; + const { data: conversation } = useActiveConversation(); + const { mutate: createConversation, isPending: isCreatingConversation } = + useCreateConversation(); + // Close context menu when agent starts running useEffect(() => { if (isAgentRunning && contextMenuOpen) { @@ -37,6 +45,40 @@ export function ChangeAgentButton() { } }, [isAgentRunning, contextMenuOpen]); + const handlePlanClick = ( + event: React.MouseEvent | KeyboardEvent, + ) => { + event.preventDefault(); + event.stopPropagation(); + + // Set conversation mode to "plan" immediately + setConversationMode("plan"); + + // Check if sub_conversation_ids is not empty + if ( + (conversation?.sub_conversation_ids && + conversation.sub_conversation_ids.length > 0) || + !conversation?.conversation_id + ) { + // Do nothing if both conditions are true + return; + } + + // Create a new sub-conversation if we have a current conversation ID + createConversation( + { + parentConversationId: conversation.conversation_id, + agentType: "plan", + }, + { + onSuccess: () => + displaySuccessToast( + t(I18nKey.PLANNING_AGENTT$PLANNING_AGENT_INITIALIZED), + ), + }, + ); + }; + // Handle Shift + Tab keyboard shortcut to cycle through modes useEffect(() => { if (!shouldUsePlanningAgent || isAgentRunning) { @@ -52,7 +94,11 @@ export function ChangeAgentButton() { // Cycle between modes: code -> plan -> code const nextMode = conversationMode === "code" ? "plan" : "code"; - setConversationMode(nextMode); + if (nextMode === "plan") { + handlePlanClick(event); + } else { + setConversationMode(nextMode); + } } }; @@ -80,12 +126,6 @@ export function ChangeAgentButton() { setConversationMode("code"); }; - const handlePlanClick = (event: React.MouseEvent) => { - event.preventDefault(); - event.stopPropagation(); - setConversationMode("plan"); - }; - const isExecutionAgent = conversationMode === "code"; const buttonLabel = useMemo(() => { @@ -102,6 +142,8 @@ export function ChangeAgentButton() { return ; }, [isExecutionAgent]); + const isButtonDisabled = isAgentRunning || isCreatingConversation; + if (!shouldUsePlanningAgent) { return null; } @@ -111,11 +153,11 @@ export function ChangeAgentButton() { diff --git a/frontend/src/utils/format-time-delta.ts b/frontend/src/utils/format-time-delta.ts index 8f2425a234fe..6785d9c845cc 100644 --- a/frontend/src/utils/format-time-delta.ts +++ b/frontend/src/utils/format-time-delta.ts @@ -1,16 +1,45 @@ +/** + * Parses a date string as UTC if it doesn't have a timezone indicator. + * This fixes the issue where ISO strings without timezone info are interpreted as local time. + * @param dateString ISO 8601 date string + * @returns Date object parsed as UTC + * + * @example + * parseDateAsUTC("2025-12-01T11:53:37.273886"); // Parsed as UTC + * parseDateAsUTC("2025-12-01T11:53:37.273886Z"); // Already has timezone, parsed correctly + * parseDateAsUTC("2025-12-01T11:53:37+00:00"); // Already has timezone, parsed correctly + */ +const parseDateAsUTC = (dateString: string): Date => { + // Check if the string already has a timezone indicator + // Look for 'Z' (UTC), '+' (positive offset), or '-' after the time part (negative offset) + const hasTimezone = + dateString.includes("Z") || dateString.match(/[+-]\d{2}:\d{2}$/) !== null; + + if (hasTimezone) { + // Already has timezone info, parse normally + return new Date(dateString); + } + + // No timezone indicator - append 'Z' to force UTC parsing + return new Date(`${dateString}Z`); +}; + /** * Formats a date into a compact string representing the time delta between the given date and the current date. - * @param date The date to format + * @param date The date to format (Date object or ISO 8601 string) * @returns A compact string representing the time delta between the given date and the current date * * @example * // now is 2024-01-01T00:00:00Z * formatTimeDelta(new Date("2023-12-31T23:59:59Z")); // "1s" - * formatTimeDelta(new Date("2022-01-01T00:00:00Z")); // "2y" + * formatTimeDelta("2023-12-31T23:59:59Z"); // "1s" + * formatTimeDelta("2025-12-01T11:53:37.273886"); // Parsed as UTC automatically */ -export const formatTimeDelta = (date: Date) => { +export const formatTimeDelta = (date: Date | string) => { + // Parse string dates as UTC if needed, or use Date object directly + const dateObj = typeof date === "string" ? parseDateAsUTC(date) : date; const now = new Date(); - const delta = now.getTime() - date.getTime(); + const delta = now.getTime() - dateObj.getTime(); const seconds = Math.floor(delta / 1000); const minutes = Math.floor(seconds / 60); From d9731b68509ea953ec66e86a52536d5dbd34fbbe Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Mon, 1 Dec 2025 20:42:44 +0700 Subject: [PATCH 071/108] feat(frontend): show plan content in the planning tab (#11807) --- .../v1-conversation-service.api.ts | 19 ++++ .../components/features/markdown/headings.tsx | 2 +- .../conversation-websocket-context.tsx | 82 ++++++++++++++++- .../mutation/use-read-conversation-file.ts | 17 ++++ frontend/src/routes/planner-tab.tsx | 2 +- frontend/src/state/conversation-store.ts | 91 ++----------------- frontend/src/types/v1/core/base/base.ts | 3 +- .../src/types/v1/core/base/observation.ts | 35 ++++++- frontend/src/types/v1/type-guards.ts | 10 ++ 9 files changed, 171 insertions(+), 90 deletions(-) create mode 100644 frontend/src/hooks/mutation/use-read-conversation-file.ts diff --git a/frontend/src/api/conversation-service/v1-conversation-service.api.ts b/frontend/src/api/conversation-service/v1-conversation-service.api.ts index 5ca7daf09a19..bd37fa818043 100644 --- a/frontend/src/api/conversation-service/v1-conversation-service.api.ts +++ b/frontend/src/api/conversation-service/v1-conversation-service.api.ts @@ -296,6 +296,25 @@ class V1ConversationService { const { data } = await openHands.get<{ runtime_id: string }>(url); return data; } + + /** + * Read a file from a specific conversation's sandbox workspace + * @param conversationId The conversation ID + * @param filePath Path to the file to read within the sandbox workspace (defaults to /workspace/project/PLAN.md) + * @returns The content of the file or an empty string if the file doesn't exist + */ + static async readConversationFile( + conversationId: string, + filePath: string = "/workspace/project/PLAN.md", + ): Promise { + const params = new URLSearchParams(); + params.append("file_path", filePath); + + const { data } = await openHands.get( + `/api/v1/app-conversations/${conversationId}/file?${params.toString()}`, + ); + return data; + } } export default V1ConversationService; diff --git a/frontend/src/components/features/markdown/headings.tsx b/frontend/src/components/features/markdown/headings.tsx index 2e12fc7db4fe..3098a4514a64 100644 --- a/frontend/src/components/features/markdown/headings.tsx +++ b/frontend/src/components/features/markdown/headings.tsx @@ -8,7 +8,7 @@ export function h1({ React.HTMLAttributes & ExtraProps) { return ( -

+

{children}

); diff --git a/frontend/src/contexts/conversation-websocket-context.tsx b/frontend/src/contexts/conversation-websocket-context.tsx index b0b0119f75b2..ae115c41a9af 100644 --- a/frontend/src/contexts/conversation-websocket-context.tsx +++ b/frontend/src/contexts/conversation-websocket-context.tsx @@ -26,6 +26,7 @@ import { isExecuteBashActionEvent, isExecuteBashObservationEvent, isConversationErrorEvent, + isPlanningFileEditorObservationEvent, } from "#/types/v1/type-guards"; import { ConversationStateUpdateEventStats } from "#/types/v1/core/events/conversation-state-event"; import { handleActionEventCacheInvalidation } from "#/utils/cache-utils"; @@ -38,6 +39,7 @@ import EventService from "#/api/event-service/event-service.api"; import { useConversationStore } from "#/state/conversation-store"; import { isBudgetOrCreditError } from "#/utils/error-handler"; import { useTracking } from "#/hooks/use-tracking"; +import { useReadConversationFile } from "#/hooks/mutation/use-read-conversation-file"; import useMetricsStore from "#/stores/metrics-store"; // eslint-disable-next-line @typescript-eslint/naming-convention @@ -102,12 +104,22 @@ export function ConversationWebSocketProvider({ number | null >(null); - const { conversationMode } = useConversationStore(); + const { conversationMode, setPlanContent } = useConversationStore(); + + // Hook for reading conversation file + const { mutate: readConversationFile } = useReadConversationFile(); // Separate received event count tracking per connection const receivedEventCountRefMain = useRef(0); const receivedEventCountRefPlanning = useRef(0); + // Track the latest PlanningFileEditorObservation event during history replay + // We'll only call the API once after history loading completes + const latestPlanningFileEventRef = useRef<{ + path: string; + conversationId: string; + } | null>(null); + // Helper function to update metrics from stats event const updateMetricsFromStats = useCallback( (event: ConversationStateUpdateEventStats) => { @@ -235,11 +247,40 @@ export function ConversationWebSocketProvider({ receivedEventCountRefPlanning, ]); + // Call API once after history loading completes if we tracked any PlanningFileEditorObservation events + useEffect(() => { + if (!isLoadingHistoryPlanning && latestPlanningFileEventRef.current) { + const { path, conversationId: currentPlanningConversationId } = + latestPlanningFileEventRef.current; + + readConversationFile( + { + conversationId: currentPlanningConversationId, + filePath: path, + }, + { + onSuccess: (fileContent) => { + setPlanContent(fileContent); + }, + onError: (error) => { + // eslint-disable-next-line no-console + console.warn("Failed to read conversation file:", error); + }, + }, + ); + + // Clear the ref after calling the API + latestPlanningFileEventRef.current = null; + } + }, [isLoadingHistoryPlanning, readConversationFile, setPlanContent]); + useEffect(() => { hasConnectedRefMain.current = false; setIsLoadingHistoryPlanning(!!subConversationIds?.length); setExpectedEventCountPlanning(null); receivedEventCountRefPlanning.current = 0; + // Reset the tracked event ref when sub-conversations change + latestPlanningFileEventRef.current = null; }, [subConversationIds]); // Merged loading history state - true if either connection is still loading @@ -254,6 +295,8 @@ export function ConversationWebSocketProvider({ setIsLoadingHistoryMain(true); setExpectedEventCountMain(null); receivedEventCountRefMain.current = 0; + // Reset the tracked event ref when conversation changes + latestPlanningFileEventRef.current = null; }, [conversationId]); // Separate message handlers for each connection @@ -438,6 +481,41 @@ export function ConversationWebSocketProvider({ .join("\n"); appendOutput(textContent); } + + // Handle PlanningFileEditorObservation events - read and update plan content + if (isPlanningFileEditorObservationEvent(event)) { + const planningAgentConversation = subConversations?.[0]; + const planningConversationId = planningAgentConversation?.id; + + if (planningConversationId && event.observation.path) { + // During history replay, track the latest event but don't call API + // After history loading completes, we'll call the API once with the latest event + if (isLoadingHistoryPlanning) { + latestPlanningFileEventRef.current = { + path: event.observation.path, + conversationId: planningConversationId, + }; + } else { + // History loading is complete - this is a new real-time event + // Call the API immediately for real-time updates + readConversationFile( + { + conversationId: planningConversationId, + filePath: event.observation.path, + }, + { + onSuccess: (fileContent) => { + setPlanContent(fileContent); + }, + onError: (error) => { + // eslint-disable-next-line no-console + console.warn("Failed to read conversation file:", error); + }, + }, + ); + } + } + } } } catch (error) { // eslint-disable-next-line no-console @@ -455,6 +533,8 @@ export function ConversationWebSocketProvider({ setExecutionStatus, appendInput, appendOutput, + readConversationFile, + setPlanContent, updateMetricsFromStats, ], ); diff --git a/frontend/src/hooks/mutation/use-read-conversation-file.ts b/frontend/src/hooks/mutation/use-read-conversation-file.ts new file mode 100644 index 000000000000..5dd8c51eb965 --- /dev/null +++ b/frontend/src/hooks/mutation/use-read-conversation-file.ts @@ -0,0 +1,17 @@ +import { useMutation } from "@tanstack/react-query"; +import V1ConversationService from "#/api/conversation-service/v1-conversation-service.api"; + +interface UseReadConversationFileVariables { + conversationId: string; + filePath?: string; +} + +export const useReadConversationFile = () => + useMutation({ + mutationKey: ["read-conversation-file"], + mutationFn: async ({ + conversationId, + filePath, + }: UseReadConversationFileVariables): Promise => + V1ConversationService.readConversationFile(conversationId, filePath), + }); diff --git a/frontend/src/routes/planner-tab.tsx b/frontend/src/routes/planner-tab.tsx index a3002c665119..4fb46f993979 100644 --- a/frontend/src/routes/planner-tab.tsx +++ b/frontend/src/routes/planner-tab.tsx @@ -23,7 +23,7 @@ function PlannerTab() { const { planContent, setConversationMode } = useConversationStore(); - if (planContent) { + if (planContent !== null && planContent !== undefined) { return (
void; setConversationMode: (conversationMode: ConversationMode) => void; setSubConversationTaskId: (taskId: string | null) => void; + setPlanContent: (planContent: string | null) => void; } type ConversationStore = ConversationState & ConversationActions; @@ -81,91 +82,7 @@ export const useConversationStore = create()( submittedMessage: null, shouldHideSuggestions: false, hasRightPanelToggled: true, - planContent: ` -# Improve Developer Onboarding and Examples - -## Overview - -Based on the analysis of Browser-Use's current documentation and examples, this plan addresses gaps in developer onboarding by creating a progressive learning path, troubleshooting resources, and practical examples that address real-world scenarios (like the LM Studio/local LLM integration issues encountered). - -## Current State Analysis - -**Strengths:** - -- Good quickstart documentation in \`docs/quickstart.mdx\` -- Extensive examples across multiple categories (60+ example files) -- Well-structured docs with multiple LLM provider examples -- Active community support via Discord - -**Gaps Identified:** - -- No progressive tutorial series that builds complexity gradually -- Limited troubleshooting documentation for common issues -- Sparse comments in example files explaining what's happening -- Local LLM setup (Ollama/LM Studio) not prominently featured -- No "first 10 minutes" success path -- Missing visual/conceptual architecture guides for beginners -- Error messages don't always point to solutions - -## Proposed Improvements - -### 1. Create Interactive Tutorial Series (\`examples/tutorials/\`) - -**New folder structure:** - -\`\`\` -examples/tutorials/ -├── README.md # Tutorial overview and prerequisites -├── 00_hello_world.py # Absolute minimal example -├── 01_your_first_search.py # Basic search with detailed comments -├── 02_understanding_actions.py # How actions work -├── 03_data_extraction_basics.py # Extract data step-by-step -├── 04_error_handling.py # Common errors and solutions -├── 05_custom_tools_intro.py # First custom tool -├── 06_local_llm_setup.py # Ollama/LM Studio complete guide -└── 07_debugging_tips.py # Debugging strategies -\`\`\` - -**Key Features:** - -- Each file 50–80 lines max -- Extensive inline comments explaining every concept -- Clear learning objectives at the top of each file -- "What you'll learn" and "Prerequisites" sections -- Common pitfalls highlighted -- Expected output shown in comments - -### 2. Troubleshooting Guide (\`docs/troubleshooting.mdx\`) - -**Sections:** - -- Installation issues (Chromium, dependencies, virtual environments) -- LLM provider connection errors (API keys, timeouts, rate limits) -- Local LLM setup (Ollama vs LM Studio, model compatibility) -- Browser automation issues (element not found, timeout errors) -- Common error messages with solutions -- Performance optimization tips -- When to ask for help (Discord/GitHub) - -**Format:** - -**Error: "LLM call timed out after 60 seconds"** - -**What it means:** -The model took too long to respond - -**Common causes:** - -1. Model is too slow for the task -2. LM Studio/Ollama not responding properly -3. Complex page overwhelming the model - -**Solutions:** - -- Use flash_mode for faster execution -- Try a faster model (Gemini Flash, GPT-4 Turbo Mini) -- Simplify the task -- Check model server logs`, + planContent: null, conversationMode: "code", subConversationTaskId: null, @@ -304,6 +221,7 @@ The model took too long to respond shouldHideSuggestions: false, conversationMode: "code", subConversationTaskId: null, + planContent: null, }, false, "resetConversationState", @@ -317,6 +235,9 @@ The model took too long to respond setSubConversationTaskId: (subConversationTaskId) => set({ subConversationTaskId }, false, "setSubConversationTaskId"), + + setPlanContent: (planContent) => + set({ planContent }, false, "setPlanContent"), }), { name: "conversation-store", diff --git a/frontend/src/types/v1/core/base/base.ts b/frontend/src/types/v1/core/base/base.ts index 30c39cec89f4..7704f1105de5 100644 --- a/frontend/src/types/v1/core/base/base.ts +++ b/frontend/src/types/v1/core/base/base.ts @@ -6,7 +6,8 @@ type EventType = | "Terminal" | "FileEditor" | "StrReplaceEditor" - | "TaskTracker"; + | "TaskTracker" + | "PlanningFileEditor"; type ActionOnlyType = | "BrowserNavigate" diff --git a/frontend/src/types/v1/core/base/observation.ts b/frontend/src/types/v1/core/base/observation.ts index 0625c9d3364c..062d7ddf6e21 100644 --- a/frontend/src/types/v1/core/base/observation.ts +++ b/frontend/src/types/v1/core/base/observation.ts @@ -190,6 +190,38 @@ export interface TaskTrackerObservation task_list: TaskItem[]; } +export interface PlanningFileEditorObservation + extends ObservationBase<"PlanningFileEditorObservation"> { + /** + * Content returned from the tool as a list of TextContent/ImageContent objects. + */ + content: Array; + /** + * Whether the call resulted in an error. + */ + is_error: boolean; + /** + * The commands to run. Allowed options are: `view`, `create`, `str_replace`, `insert`, `undo_edit`. + */ + command: "view" | "create" | "str_replace" | "insert" | "undo_edit"; + /** + * The file path that was edited. + */ + path: string | null; + /** + * Indicates if the file previously existed. If not, it was created. + */ + prev_exist: boolean; + /** + * The content of the file before the edit. + */ + old_content: string | null; + /** + * The content of the file after the edit. + */ + new_content: string | null; +} + export type Observation = | MCPToolObservation | FinishObservation @@ -199,4 +231,5 @@ export type Observation = | TerminalObservation | FileEditorObservation | StrReplaceEditorObservation - | TaskTrackerObservation; + | TaskTrackerObservation + | PlanningFileEditorObservation; diff --git a/frontend/src/types/v1/type-guards.ts b/frontend/src/types/v1/type-guards.ts index e73a5e0e7437..306661e854b3 100644 --- a/frontend/src/types/v1/type-guards.ts +++ b/frontend/src/types/v1/type-guards.ts @@ -5,6 +5,7 @@ import { ExecuteBashAction, TerminalAction, ExecuteBashObservation, + PlanningFileEditorObservation, TerminalObservation, } from "./core"; import { AgentErrorEvent } from "./core/events/observation-event"; @@ -116,6 +117,15 @@ export const isExecuteBashObservationEvent = ( (event.observation.kind === "ExecuteBashObservation" || event.observation.kind === "TerminalObservation"); +/** + * Type guard function to check if an observation event is a PlanningFileEditorObservation + */ +export const isPlanningFileEditorObservationEvent = ( + event: OpenHandsEvent, +): event is ObservationEvent => + isObservationEvent(event) && + event.observation.kind === "PlanningFileEditorObservation"; + /** * Type guard function to check if an event is a system prompt event */ From 96f13b15e702805805f4230ba0fca8f585ead5ac Mon Sep 17 00:00:00 2001 From: "sp.wack" <83104063+amanape@users.noreply.github.com> Date: Mon, 1 Dec 2025 17:58:03 +0400 Subject: [PATCH 072/108] Revert "chore(backend): Add better PostHog tracking" (#11749) --- enterprise/server/routes/auth.py | 7 - enterprise/server/routes/billing.py | 15 - openhands/controller/agent_controller.py | 30 -- openhands/server/routes/git.py | 10 - openhands/utils/posthog_tracker.py | 270 ------------- .../test_agent_controller_posthog.py | 243 ------------ tests/unit/utils/test_posthog_tracker.py | 356 ------------------ 7 files changed, 931 deletions(-) delete mode 100644 openhands/utils/posthog_tracker.py delete mode 100644 tests/unit/controller/test_agent_controller_posthog.py delete mode 100644 tests/unit/utils/test_posthog_tracker.py diff --git a/enterprise/server/routes/auth.py b/enterprise/server/routes/auth.py index c9e92d54f7f5..ba7aadb88316 100644 --- a/enterprise/server/routes/auth.py +++ b/enterprise/server/routes/auth.py @@ -31,7 +31,6 @@ from openhands.server.shared import config from openhands.server.user_auth import get_access_token from openhands.server.user_auth.user_auth import get_user_auth -from openhands.utils.posthog_tracker import track_user_signup_completed with warnings.catch_warnings(): warnings.simplefilter('ignore') @@ -370,12 +369,6 @@ async def accept_tos(request: Request): logger.info(f'User {user_id} accepted TOS') - # Track user signup completion in PostHog - track_user_signup_completed( - user_id=user_id, - signup_timestamp=user_settings.accepted_tos.isoformat(), - ) - response = JSONResponse( status_code=status.HTTP_200_OK, content={'redirect_url': redirect_url} ) diff --git a/enterprise/server/routes/billing.py b/enterprise/server/routes/billing.py index f1c0c5376bec..5a8b59e2d76b 100644 --- a/enterprise/server/routes/billing.py +++ b/enterprise/server/routes/billing.py @@ -28,7 +28,6 @@ from openhands.server.user_auth import get_user_id from openhands.utils.http_session import httpx_verify_option -from openhands.utils.posthog_tracker import track_credits_purchased stripe.api_key = STRIPE_API_KEY billing_router = APIRouter(prefix='/api/billing') @@ -458,20 +457,6 @@ async def success_callback(session_id: str, request: Request): ) session.commit() - # Track credits purchased in PostHog - try: - track_credits_purchased( - user_id=billing_session.user_id, - amount_usd=amount_subtotal / 100, # Convert cents to dollars - credits_added=add_credits, - stripe_session_id=session_id, - ) - except Exception as e: - logger.warning( - f'Failed to track credits purchase: {e}', - extra={'user_id': billing_session.user_id, 'error': str(e)}, - ) - return RedirectResponse( f'{request.base_url}settings/billing?checkout=success', status_code=302 ) diff --git a/openhands/controller/agent_controller.py b/openhands/controller/agent_controller.py index 3f2ad876748b..958e5cb34837 100644 --- a/openhands/controller/agent_controller.py +++ b/openhands/controller/agent_controller.py @@ -42,10 +42,6 @@ from openhands.core.logger import LOG_ALL_EVENTS from openhands.core.logger import openhands_logger as logger from openhands.core.schema import AgentState -from openhands.utils.posthog_tracker import ( - track_agent_task_completed, - track_credit_limit_reached, -) from openhands.events import ( EventSource, EventStream, @@ -713,20 +709,6 @@ async def set_agent_state_to(self, new_state: AgentState) -> None: EventSource.ENVIRONMENT, ) - # Track agent task completion in PostHog - if new_state == AgentState.FINISHED: - try: - # Get app_mode from environment, default to 'oss' - app_mode = os.environ.get('APP_MODE', 'oss') - track_agent_task_completed( - conversation_id=self.id, - user_id=self.user_id, - app_mode=app_mode, - ) - except Exception as e: - # Don't let tracking errors interrupt the agent - self.log('warning', f'Failed to track agent completion: {e}') - # Save state whenever agent state changes to ensure we don't lose state # in case of crashes or unexpected circumstances self.save_state() @@ -905,18 +887,6 @@ async def _step(self) -> None: self.state_tracker.run_control_flags() except Exception as e: logger.warning('Control flag limits hit') - # Track credit limit reached if it's a budget exception - if 'budget' in str(e).lower() and self.state.budget_flag: - try: - track_credit_limit_reached( - conversation_id=self.id, - user_id=self.user_id, - current_budget=self.state.budget_flag.current_value, - max_budget=self.state.budget_flag.max_value, - ) - except Exception as track_error: - # Don't let tracking errors interrupt the agent - self.log('warning', f'Failed to track credit limit: {track_error}') await self._react_to_exception(e) return diff --git a/openhands/server/routes/git.py b/openhands/server/routes/git.py index 1401bb0dcd34..a6807a2e2a4a 100644 --- a/openhands/server/routes/git.py +++ b/openhands/server/routes/git.py @@ -26,13 +26,11 @@ ) from openhands.server.dependencies import get_dependencies from openhands.server.shared import server_config -from openhands.server.types import AppMode from openhands.server.user_auth import ( get_access_token, get_provider_tokens, get_user_id, ) -from openhands.utils.posthog_tracker import alias_user_identities app = APIRouter(prefix='/api/user', dependencies=get_dependencies()) @@ -119,14 +117,6 @@ async def get_user( try: user: User = await client.get_user() - - # Alias git provider login with Keycloak user ID in PostHog (SaaS mode only) - if user_id and user.login and server_config.app_mode == AppMode.SAAS: - alias_user_identities( - keycloak_user_id=user_id, - git_login=user.login, - ) - return user except UnknownException as e: diff --git a/openhands/utils/posthog_tracker.py b/openhands/utils/posthog_tracker.py deleted file mode 100644 index c0859eddc717..000000000000 --- a/openhands/utils/posthog_tracker.py +++ /dev/null @@ -1,270 +0,0 @@ -"""PostHog tracking utilities for OpenHands events.""" - -import os - -from openhands.core.logger import openhands_logger as logger - -# Lazy import posthog to avoid import errors in environments where it's not installed -posthog = None - - -def _init_posthog(): - """Initialize PostHog client lazily.""" - global posthog - if posthog is None: - try: - import posthog as ph - - posthog = ph - posthog.api_key = os.environ.get( - 'POSTHOG_CLIENT_KEY', 'phc_3ESMmY9SgqEAGBB6sMGK5ayYHkeUuknH2vP6FmWH9RA' - ) - posthog.host = os.environ.get('POSTHOG_HOST', 'https://us.i.posthog.com') - except ImportError: - logger.warning( - 'PostHog not installed. Analytics tracking will be disabled.' - ) - posthog = None - - -def track_agent_task_completed( - conversation_id: str, - user_id: str | None = None, - app_mode: str | None = None, -) -> None: - """Track when an agent completes a task. - - Args: - conversation_id: The ID of the conversation/session - user_id: The ID of the user (optional, may be None for unauthenticated users) - app_mode: The application mode (saas/oss), optional - """ - _init_posthog() - - if posthog is None: - return - - # Use conversation_id as distinct_id if user_id is not available - # This ensures we can track completions even for anonymous users - distinct_id = user_id if user_id else f'conversation_{conversation_id}' - - try: - posthog.capture( - distinct_id=distinct_id, - event='agent_task_completed', - properties={ - 'conversation_id': conversation_id, - 'user_id': user_id, - 'app_mode': app_mode or 'unknown', - }, - ) - logger.debug( - 'posthog_track', - extra={ - 'event': 'agent_task_completed', - 'conversation_id': conversation_id, - 'user_id': user_id, - }, - ) - except Exception as e: - logger.warning( - f'Failed to track agent_task_completed to PostHog: {e}', - extra={ - 'conversation_id': conversation_id, - 'error': str(e), - }, - ) - - -def track_user_signup_completed( - user_id: str, - signup_timestamp: str, -) -> None: - """Track when a user completes signup by accepting TOS. - - Args: - user_id: The ID of the user (Keycloak user ID) - signup_timestamp: ISO format timestamp of when TOS was accepted - """ - _init_posthog() - - if posthog is None: - return - - try: - posthog.capture( - distinct_id=user_id, - event='user_signup_completed', - properties={ - 'user_id': user_id, - 'signup_timestamp': signup_timestamp, - }, - ) - logger.debug( - 'posthog_track', - extra={ - 'event': 'user_signup_completed', - 'user_id': user_id, - }, - ) - except Exception as e: - logger.warning( - f'Failed to track user_signup_completed to PostHog: {e}', - extra={ - 'user_id': user_id, - 'error': str(e), - }, - ) - - -def track_credit_limit_reached( - conversation_id: str, - user_id: str | None = None, - current_budget: float = 0.0, - max_budget: float = 0.0, -) -> None: - """Track when a user reaches their credit limit during a conversation. - - Args: - conversation_id: The ID of the conversation/session - user_id: The ID of the user (optional, may be None for unauthenticated users) - current_budget: The current budget spent - max_budget: The maximum budget allowed - """ - _init_posthog() - - if posthog is None: - return - - distinct_id = user_id if user_id else f'conversation_{conversation_id}' - - try: - posthog.capture( - distinct_id=distinct_id, - event='credit_limit_reached', - properties={ - 'conversation_id': conversation_id, - 'user_id': user_id, - 'current_budget': current_budget, - 'max_budget': max_budget, - }, - ) - logger.debug( - 'posthog_track', - extra={ - 'event': 'credit_limit_reached', - 'conversation_id': conversation_id, - 'user_id': user_id, - 'current_budget': current_budget, - 'max_budget': max_budget, - }, - ) - except Exception as e: - logger.warning( - f'Failed to track credit_limit_reached to PostHog: {e}', - extra={ - 'conversation_id': conversation_id, - 'error': str(e), - }, - ) - - -def track_credits_purchased( - user_id: str, - amount_usd: float, - credits_added: float, - stripe_session_id: str, -) -> None: - """Track when a user successfully purchases credits. - - Args: - user_id: The ID of the user (Keycloak user ID) - amount_usd: The amount paid in USD (cents converted to dollars) - credits_added: The number of credits added to the user's account - stripe_session_id: The Stripe checkout session ID - """ - _init_posthog() - - if posthog is None: - return - - try: - posthog.capture( - distinct_id=user_id, - event='credits_purchased', - properties={ - 'user_id': user_id, - 'amount_usd': amount_usd, - 'credits_added': credits_added, - 'stripe_session_id': stripe_session_id, - }, - ) - logger.debug( - 'posthog_track', - extra={ - 'event': 'credits_purchased', - 'user_id': user_id, - 'amount_usd': amount_usd, - 'credits_added': credits_added, - }, - ) - except Exception as e: - logger.warning( - f'Failed to track credits_purchased to PostHog: {e}', - extra={ - 'user_id': user_id, - 'error': str(e), - }, - ) - - -def alias_user_identities( - keycloak_user_id: str, - git_login: str, -) -> None: - """Alias a user's Keycloak ID with their git provider login for unified tracking. - - This allows PostHog to link events tracked from the frontend (using git provider login) - with events tracked from the backend (using Keycloak user ID). - - PostHog Python alias syntax: alias(previous_id, distinct_id) - - previous_id: The old/previous distinct ID that will be merged - - distinct_id: The new/canonical distinct ID to merge into - - For our use case: - - Git provider login is the previous_id (first used in frontend, before backend auth) - - Keycloak user ID is the distinct_id (canonical backend ID) - - Result: All events with git login will be merged into Keycloak user ID - - Args: - keycloak_user_id: The Keycloak user ID (canonical distinct_id) - git_login: The git provider username (GitHub/GitLab/Bitbucket) to merge - - Reference: - https://github.com/PostHog/posthog-python/blob/master/posthog/client.py - """ - _init_posthog() - - if posthog is None: - return - - try: - # Merge git provider login into Keycloak user ID - # posthog.alias(previous_id, distinct_id) - official Python SDK signature - posthog.alias(git_login, keycloak_user_id) - logger.debug( - 'posthog_alias', - extra={ - 'previous_id': git_login, - 'distinct_id': keycloak_user_id, - }, - ) - except Exception as e: - logger.warning( - f'Failed to alias user identities in PostHog: {e}', - extra={ - 'keycloak_user_id': keycloak_user_id, - 'git_login': git_login, - 'error': str(e), - }, - ) diff --git a/tests/unit/controller/test_agent_controller_posthog.py b/tests/unit/controller/test_agent_controller_posthog.py deleted file mode 100644 index 630c18e3aa53..000000000000 --- a/tests/unit/controller/test_agent_controller_posthog.py +++ /dev/null @@ -1,243 +0,0 @@ -"""Integration tests for PostHog tracking in AgentController.""" - -import asyncio -from unittest.mock import MagicMock, patch - -import pytest - -from openhands.controller.agent import Agent -from openhands.controller.agent_controller import AgentController -from openhands.core.config import OpenHandsConfig -from openhands.core.config.agent_config import AgentConfig -from openhands.core.config.llm_config import LLMConfig -from openhands.core.schema import AgentState -from openhands.events import EventSource, EventStream -from openhands.events.action.message import SystemMessageAction -from openhands.llm.llm_registry import LLMRegistry -from openhands.server.services.conversation_stats import ConversationStats -from openhands.storage.memory import InMemoryFileStore - - -@pytest.fixture(scope='function') -def event_loop(): - """Create event loop for async tests.""" - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() - - -@pytest.fixture -def mock_agent_with_stats(): - """Create a mock agent with properly connected LLM registry and conversation stats.""" - import uuid - - # Create LLM registry - config = OpenHandsConfig() - llm_registry = LLMRegistry(config=config) - - # Create conversation stats - file_store = InMemoryFileStore({}) - conversation_id = f'test-conversation-{uuid.uuid4()}' - conversation_stats = ConversationStats( - file_store=file_store, conversation_id=conversation_id, user_id='test-user' - ) - - # Connect registry to stats - llm_registry.subscribe(conversation_stats.register_llm) - - # Create mock agent - agent = MagicMock(spec=Agent) - agent_config = MagicMock(spec=AgentConfig) - llm_config = LLMConfig( - model='gpt-4o', - api_key='test_key', - num_retries=2, - retry_min_wait=1, - retry_max_wait=2, - ) - agent_config.disabled_microagents = [] - agent_config.enable_mcp = True - agent_config.enable_stuck_detection = True - llm_registry.service_to_llm.clear() - mock_llm = llm_registry.get_llm('agent_llm', llm_config) - agent.llm = mock_llm - agent.name = 'test-agent' - agent.sandbox_plugins = [] - agent.config = agent_config - agent.llm_registry = llm_registry - agent.prompt_manager = MagicMock() - - # Add a proper system message mock - system_message = SystemMessageAction( - content='Test system message', tools=['test_tool'] - ) - system_message._source = EventSource.AGENT - system_message._id = -1 # Set invalid ID to avoid the ID check - agent.get_system_message.return_value = system_message - - return agent, conversation_stats, llm_registry - - -@pytest.fixture -def mock_event_stream(): - """Create a mock event stream.""" - mock = MagicMock( - spec=EventStream, - event_stream=EventStream(sid='test', file_store=InMemoryFileStore({})), - ) - mock.get_latest_event_id.return_value = 0 - return mock - - -@pytest.mark.asyncio -async def test_agent_finish_triggers_posthog_tracking( - mock_agent_with_stats, mock_event_stream -): - """Test that setting agent state to FINISHED triggers PostHog tracking.""" - mock_agent, conversation_stats, llm_registry = mock_agent_with_stats - - controller = AgentController( - agent=mock_agent, - event_stream=mock_event_stream, - conversation_stats=conversation_stats, - iteration_delta=10, - sid='test-conversation-123', - user_id='test-user-456', - confirmation_mode=False, - headless_mode=True, - ) - - with ( - patch('openhands.utils.posthog_tracker.posthog') as mock_posthog, - patch('os.environ.get') as mock_env_get, - ): - # Setup mocks - mock_posthog.capture = MagicMock() - mock_env_get.return_value = 'saas' - - # Initialize posthog in the tracker module - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - # Set agent state to FINISHED - await controller.set_agent_state_to(AgentState.FINISHED) - - # Verify PostHog tracking was called - mock_posthog.capture.assert_called_once() - call_args = mock_posthog.capture.call_args - - assert call_args[1]['distinct_id'] == 'test-user-456' - assert call_args[1]['event'] == 'agent_task_completed' - assert 'conversation_id' in call_args[1]['properties'] - assert call_args[1]['properties']['user_id'] == 'test-user-456' - assert call_args[1]['properties']['app_mode'] == 'saas' - - await controller.close() - - -@pytest.mark.asyncio -async def test_agent_finish_without_user_id(mock_agent_with_stats, mock_event_stream): - """Test tracking when user_id is None.""" - mock_agent, conversation_stats, llm_registry = mock_agent_with_stats - - controller = AgentController( - agent=mock_agent, - event_stream=mock_event_stream, - conversation_stats=conversation_stats, - iteration_delta=10, - sid='test-conversation-789', - user_id=None, - confirmation_mode=False, - headless_mode=True, - ) - - with ( - patch('openhands.utils.posthog_tracker.posthog') as mock_posthog, - patch('os.environ.get') as mock_env_get, - ): - mock_posthog.capture = MagicMock() - mock_env_get.return_value = 'oss' - - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - await controller.set_agent_state_to(AgentState.FINISHED) - - mock_posthog.capture.assert_called_once() - call_args = mock_posthog.capture.call_args - - # When user_id is None, distinct_id should be conversation_id - assert call_args[1]['distinct_id'].startswith('conversation_') - assert call_args[1]['properties']['user_id'] is None - - await controller.close() - - -@pytest.mark.asyncio -async def test_other_states_dont_trigger_tracking( - mock_agent_with_stats, mock_event_stream -): - """Test that non-FINISHED states don't trigger tracking.""" - mock_agent, conversation_stats, llm_registry = mock_agent_with_stats - - controller = AgentController( - agent=mock_agent, - event_stream=mock_event_stream, - conversation_stats=conversation_stats, - iteration_delta=10, - sid='test-conversation-999', - confirmation_mode=False, - headless_mode=True, - ) - - with patch('openhands.utils.posthog_tracker.posthog') as mock_posthog: - mock_posthog.capture = MagicMock() - - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - # Try different states - await controller.set_agent_state_to(AgentState.RUNNING) - await controller.set_agent_state_to(AgentState.PAUSED) - await controller.set_agent_state_to(AgentState.STOPPED) - - # PostHog should not be called for non-FINISHED states - mock_posthog.capture.assert_not_called() - - await controller.close() - - -@pytest.mark.asyncio -async def test_tracking_error_doesnt_break_agent( - mock_agent_with_stats, mock_event_stream -): - """Test that tracking errors don't interrupt agent operation.""" - mock_agent, conversation_stats, llm_registry = mock_agent_with_stats - - controller = AgentController( - agent=mock_agent, - event_stream=mock_event_stream, - conversation_stats=conversation_stats, - iteration_delta=10, - sid='test-conversation-error', - confirmation_mode=False, - headless_mode=True, - ) - - with patch('openhands.utils.posthog_tracker.posthog') as mock_posthog: - mock_posthog.capture = MagicMock(side_effect=Exception('PostHog error')) - - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - # Should not raise an exception - await controller.set_agent_state_to(AgentState.FINISHED) - - # Agent state should still be FINISHED despite tracking error - assert controller.state.agent_state == AgentState.FINISHED - - await controller.close() diff --git a/tests/unit/utils/test_posthog_tracker.py b/tests/unit/utils/test_posthog_tracker.py deleted file mode 100644 index cec0eff0ccc0..000000000000 --- a/tests/unit/utils/test_posthog_tracker.py +++ /dev/null @@ -1,356 +0,0 @@ -"""Unit tests for PostHog tracking utilities.""" - -from unittest.mock import MagicMock, patch - -import pytest - -from openhands.utils.posthog_tracker import ( - alias_user_identities, - track_agent_task_completed, - track_credit_limit_reached, - track_credits_purchased, - track_user_signup_completed, -) - - -@pytest.fixture -def mock_posthog(): - """Mock the posthog module.""" - with patch('openhands.utils.posthog_tracker.posthog') as mock_ph: - mock_ph.capture = MagicMock() - yield mock_ph - - -def test_track_agent_task_completed_with_user_id(mock_posthog): - """Test tracking agent task completion with user ID.""" - # Initialize posthog manually in the test - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_agent_task_completed( - conversation_id='test-conversation-123', - user_id='user-456', - app_mode='saas', - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='user-456', - event='agent_task_completed', - properties={ - 'conversation_id': 'test-conversation-123', - 'user_id': 'user-456', - 'app_mode': 'saas', - }, - ) - - -def test_track_agent_task_completed_without_user_id(mock_posthog): - """Test tracking agent task completion without user ID (anonymous).""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_agent_task_completed( - conversation_id='test-conversation-789', - user_id=None, - app_mode='oss', - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='conversation_test-conversation-789', - event='agent_task_completed', - properties={ - 'conversation_id': 'test-conversation-789', - 'user_id': None, - 'app_mode': 'oss', - }, - ) - - -def test_track_agent_task_completed_default_app_mode(mock_posthog): - """Test tracking with default app_mode.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_agent_task_completed( - conversation_id='test-conversation-999', - user_id='user-111', - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='user-111', - event='agent_task_completed', - properties={ - 'conversation_id': 'test-conversation-999', - 'user_id': 'user-111', - 'app_mode': 'unknown', - }, - ) - - -def test_track_agent_task_completed_handles_errors(mock_posthog): - """Test that tracking errors are handled gracefully.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.capture.side_effect = Exception('PostHog API error') - - # Should not raise an exception - track_agent_task_completed( - conversation_id='test-conversation-error', - user_id='user-error', - app_mode='saas', - ) - - -def test_track_agent_task_completed_when_posthog_not_installed(): - """Test tracking when posthog is not installed.""" - import openhands.utils.posthog_tracker as tracker - - # Simulate posthog not being installed - tracker.posthog = None - - # Should not raise an exception - track_agent_task_completed( - conversation_id='test-conversation-no-ph', - user_id='user-no-ph', - app_mode='oss', - ) - - -def test_track_user_signup_completed(mock_posthog): - """Test tracking user signup completion.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_user_signup_completed( - user_id='test-user-123', - signup_timestamp='2025-01-15T10:30:00Z', - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='test-user-123', - event='user_signup_completed', - properties={ - 'user_id': 'test-user-123', - 'signup_timestamp': '2025-01-15T10:30:00Z', - }, - ) - - -def test_track_user_signup_completed_handles_errors(mock_posthog): - """Test that user signup tracking errors are handled gracefully.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.capture.side_effect = Exception('PostHog API error') - - # Should not raise an exception - track_user_signup_completed( - user_id='test-user-error', - signup_timestamp='2025-01-15T12:00:00Z', - ) - - -def test_track_user_signup_completed_when_posthog_not_installed(): - """Test user signup tracking when posthog is not installed.""" - import openhands.utils.posthog_tracker as tracker - - # Simulate posthog not being installed - tracker.posthog = None - - # Should not raise an exception - track_user_signup_completed( - user_id='test-user-no-ph', - signup_timestamp='2025-01-15T13:00:00Z', - ) - - -def test_track_credit_limit_reached_with_user_id(mock_posthog): - """Test tracking credit limit reached with user ID.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_credit_limit_reached( - conversation_id='test-conversation-456', - user_id='user-789', - current_budget=10.50, - max_budget=10.00, - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='user-789', - event='credit_limit_reached', - properties={ - 'conversation_id': 'test-conversation-456', - 'user_id': 'user-789', - 'current_budget': 10.50, - 'max_budget': 10.00, - }, - ) - - -def test_track_credit_limit_reached_without_user_id(mock_posthog): - """Test tracking credit limit reached without user ID (anonymous).""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_credit_limit_reached( - conversation_id='test-conversation-999', - user_id=None, - current_budget=5.25, - max_budget=5.00, - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='conversation_test-conversation-999', - event='credit_limit_reached', - properties={ - 'conversation_id': 'test-conversation-999', - 'user_id': None, - 'current_budget': 5.25, - 'max_budget': 5.00, - }, - ) - - -def test_track_credit_limit_reached_handles_errors(mock_posthog): - """Test that credit limit tracking errors are handled gracefully.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.capture.side_effect = Exception('PostHog API error') - - # Should not raise an exception - track_credit_limit_reached( - conversation_id='test-conversation-error', - user_id='user-error', - current_budget=15.00, - max_budget=10.00, - ) - - -def test_track_credit_limit_reached_when_posthog_not_installed(): - """Test credit limit tracking when posthog is not installed.""" - import openhands.utils.posthog_tracker as tracker - - # Simulate posthog not being installed - tracker.posthog = None - - # Should not raise an exception - track_credit_limit_reached( - conversation_id='test-conversation-no-ph', - user_id='user-no-ph', - current_budget=8.00, - max_budget=5.00, - ) - - -def test_track_credits_purchased(mock_posthog): - """Test tracking credits purchased.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - - track_credits_purchased( - user_id='test-user-999', - amount_usd=50.00, - credits_added=50.00, - stripe_session_id='cs_test_abc123', - ) - - mock_posthog.capture.assert_called_once_with( - distinct_id='test-user-999', - event='credits_purchased', - properties={ - 'user_id': 'test-user-999', - 'amount_usd': 50.00, - 'credits_added': 50.00, - 'stripe_session_id': 'cs_test_abc123', - }, - ) - - -def test_track_credits_purchased_handles_errors(mock_posthog): - """Test that credits purchased tracking errors are handled gracefully.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.capture.side_effect = Exception('PostHog API error') - - # Should not raise an exception - track_credits_purchased( - user_id='test-user-error', - amount_usd=100.00, - credits_added=100.00, - stripe_session_id='cs_test_error', - ) - - -def test_track_credits_purchased_when_posthog_not_installed(): - """Test credits purchased tracking when posthog is not installed.""" - import openhands.utils.posthog_tracker as tracker - - # Simulate posthog not being installed - tracker.posthog = None - - # Should not raise an exception - track_credits_purchased( - user_id='test-user-no-ph', - amount_usd=25.00, - credits_added=25.00, - stripe_session_id='cs_test_no_ph', - ) - - -def test_alias_user_identities(mock_posthog): - """Test aliasing user identities. - - Verifies that posthog.alias(previous_id, distinct_id) is called correctly - where git_login is the previous_id and keycloak_user_id is the distinct_id. - """ - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.alias = MagicMock() - - alias_user_identities( - keycloak_user_id='keycloak-123', - git_login='git-user', - ) - - # Verify: posthog.alias(previous_id='git-user', distinct_id='keycloak-123') - mock_posthog.alias.assert_called_once_with('git-user', 'keycloak-123') - - -def test_alias_user_identities_handles_errors(mock_posthog): - """Test that aliasing errors are handled gracefully.""" - import openhands.utils.posthog_tracker as tracker - - tracker.posthog = mock_posthog - mock_posthog.alias = MagicMock(side_effect=Exception('PostHog API error')) - - # Should not raise an exception - alias_user_identities( - keycloak_user_id='keycloak-error', - git_login='git-error', - ) - - -def test_alias_user_identities_when_posthog_not_installed(): - """Test aliasing when posthog is not installed.""" - import openhands.utils.posthog_tracker as tracker - - # Simulate posthog not being installed - tracker.posthog = None - - # Should not raise an exception - alias_user_identities( - keycloak_user_id='keycloak-no-ph', - git_login='git-no-ph', - ) From 6c821ab73e6ae077f3cf3f94aa3e6cad9ac78b6b Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Mon, 1 Dec 2025 21:29:18 +0700 Subject: [PATCH 073/108] fix(frontend): the content of the FinishObservation event is not being rendered correctly. (#11846) --- .../components/features/chat/chat-message.tsx | 21 +---- .../features/chat/error-message.tsx | 19 +---- .../features/chat/expandable-message.tsx | 19 +---- .../features/chat/generic-event-message.tsx | 17 +--- .../features/markdown/markdown-renderer.tsx | 80 +++++++++++++++++++ ...ent-management-view-microagent-content.tsx | 21 +---- .../get-observation-content.ts | 17 +++- .../generic-event-message-wrapper.tsx | 17 ++-- frontend/src/routes/planner-tab.tsx | 35 +------- .../src/types/v1/core/base/observation.ts | 8 +- 10 files changed, 128 insertions(+), 126 deletions(-) create mode 100644 frontend/src/components/features/markdown/markdown-renderer.tsx diff --git a/frontend/src/components/features/chat/chat-message.tsx b/frontend/src/components/features/chat/chat-message.tsx index a3dc93447585..6f2f388682c1 100644 --- a/frontend/src/components/features/chat/chat-message.tsx +++ b/frontend/src/components/features/chat/chat-message.tsx @@ -1,15 +1,9 @@ import React from "react"; -import Markdown from "react-markdown"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; -import { code } from "../markdown/code"; import { cn } from "#/utils/utils"; -import { ul, ol } from "../markdown/list"; import { CopyToClipboardButton } from "#/components/shared/buttons/copy-to-clipboard-button"; -import { anchor } from "../markdown/anchor"; import { OpenHandsSourceType } from "#/types/core/base"; -import { paragraph } from "../markdown/paragraph"; import { TooltipButton } from "#/components/shared/buttons/tooltip-button"; +import { MarkdownRenderer } from "../markdown/markdown-renderer"; interface ChatMessageProps { type: OpenHandsSourceType; @@ -116,18 +110,7 @@ export function ChatMessage({ wordBreak: "break-word", }} > - - {message} - + {message}
{children} diff --git a/frontend/src/components/features/chat/error-message.tsx b/frontend/src/components/features/chat/error-message.tsx index 8de367a9a2bf..da40b3786e5a 100644 --- a/frontend/src/components/features/chat/error-message.tsx +++ b/frontend/src/components/features/chat/error-message.tsx @@ -1,13 +1,9 @@ import React from "react"; -import Markdown from "react-markdown"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; import { useTranslation } from "react-i18next"; -import { code } from "../markdown/code"; -import { ol, ul } from "../markdown/list"; import ArrowDown from "#/icons/angle-down-solid.svg?react"; import ArrowUp from "#/icons/angle-up-solid.svg?react"; import i18n from "#/i18n"; +import { MarkdownRenderer } from "../markdown/markdown-renderer"; interface ErrorMessageProps { errorId?: string; @@ -40,18 +36,7 @@ export function ErrorMessage({ errorId, defaultMessage }: ErrorMessageProps) {
- {showDetails && ( - - {defaultMessage} - - )} + {showDetails && {defaultMessage}}
); } diff --git a/frontend/src/components/features/chat/expandable-message.tsx b/frontend/src/components/features/chat/expandable-message.tsx index 918eafd6b859..cf9ae550d240 100644 --- a/frontend/src/components/features/chat/expandable-message.tsx +++ b/frontend/src/components/features/chat/expandable-message.tsx @@ -1,9 +1,6 @@ import { useEffect, useState } from "react"; import { Trans, useTranslation } from "react-i18next"; -import Markdown from "react-markdown"; import { Link } from "react-router"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; import { useConfig } from "#/hooks/query/use-config"; import { I18nKey } from "#/i18n/declaration"; import ArrowDown from "#/icons/angle-down-solid.svg?react"; @@ -13,9 +10,7 @@ import XCircle from "#/icons/x-circle-solid.svg?react"; import { OpenHandsAction } from "#/types/core/actions"; import { OpenHandsObservation } from "#/types/core/observations"; import { cn } from "#/utils/utils"; -import { code } from "../markdown/code"; -import { ol, ul } from "../markdown/list"; -import { paragraph } from "../markdown/paragraph"; +import { MarkdownRenderer } from "../markdown/markdown-renderer"; import { MonoComponent } from "./mono-component"; import { PathComponent } from "./path-component"; @@ -192,17 +187,7 @@ export function ExpandableMessage({
{showDetails && (
- - {details} - + {details}
)}
diff --git a/frontend/src/components/features/chat/generic-event-message.tsx b/frontend/src/components/features/chat/generic-event-message.tsx index e5124b69fef9..ff2ab633b189 100644 --- a/frontend/src/components/features/chat/generic-event-message.tsx +++ b/frontend/src/components/features/chat/generic-event-message.tsx @@ -1,13 +1,9 @@ import React from "react"; -import Markdown from "react-markdown"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; -import { code } from "../markdown/code"; -import { ol, ul } from "../markdown/list"; import ArrowDown from "#/icons/angle-down-solid.svg?react"; import ArrowUp from "#/icons/angle-up-solid.svg?react"; import { SuccessIndicator } from "./success-indicator"; import { ObservationResultStatus } from "./event-content-helpers/get-observation-result"; +import { MarkdownRenderer } from "../markdown/markdown-renderer"; interface GenericEventMessageProps { title: React.ReactNode; @@ -49,16 +45,7 @@ export function GenericEventMessage({ {showDetails && (typeof details === "string" ? ( - - {details} - + {details} ) : ( details ))} diff --git a/frontend/src/components/features/markdown/markdown-renderer.tsx b/frontend/src/components/features/markdown/markdown-renderer.tsx new file mode 100644 index 000000000000..0cb55498d63f --- /dev/null +++ b/frontend/src/components/features/markdown/markdown-renderer.tsx @@ -0,0 +1,80 @@ +import Markdown, { Components } from "react-markdown"; +import remarkGfm from "remark-gfm"; +import remarkBreaks from "remark-breaks"; +import { code } from "./code"; +import { ul, ol } from "./list"; +import { paragraph } from "./paragraph"; +import { anchor } from "./anchor"; +import { h1, h2, h3, h4, h5, h6 } from "./headings"; + +interface MarkdownRendererProps { + /** + * The markdown content to render. Can be passed as children (string) or content prop. + */ + children?: string; + content?: string; + /** + * Additional or override components for markdown elements. + * Default components (code, ul, ol) are always included unless overridden. + */ + components?: Partial; + /** + * Whether to include standard components (anchor, paragraph). + * Defaults to false. + */ + includeStandard?: boolean; + /** + * Whether to include heading components (h1-h6). + * Defaults to false. + */ + includeHeadings?: boolean; +} + +/** + * A reusable Markdown renderer component that provides consistent + * markdown rendering across the application. + * + * By default, includes: + * - code, ul, ol components + * - remarkGfm and remarkBreaks plugins + * + * Can be extended with: + * - includeStandard: adds anchor and paragraph components + * - includeHeadings: adds h1-h6 heading components + * - components prop: allows custom overrides or additional components + */ +export function MarkdownRenderer({ + children, + content, + components: customComponents, + includeStandard = false, + includeHeadings = false, +}: MarkdownRendererProps) { + // Build the components object with defaults and optional additions + const components: Components = { + code, + ul, + ol, + ...(includeStandard && { + a: anchor, + p: paragraph, + }), + ...(includeHeadings && { + h1, + h2, + h3, + h4, + h5, + h6, + }), + ...customComponents, // Custom components override defaults + }; + + const markdownContent = content ?? children ?? ""; + + return ( + + {markdownContent} + + ); +} diff --git a/frontend/src/components/features/microagent-management/microagent-management-view-microagent-content.tsx b/frontend/src/components/features/microagent-management/microagent-management-view-microagent-content.tsx index dc5b5fecaa59..2994946731e3 100644 --- a/frontend/src/components/features/microagent-management/microagent-management-view-microagent-content.tsx +++ b/frontend/src/components/features/microagent-management/microagent-management-view-microagent-content.tsx @@ -1,16 +1,10 @@ import { useTranslation } from "react-i18next"; import { Spinner } from "@heroui/react"; -import Markdown from "react-markdown"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; -import { code } from "../markdown/code"; -import { ul, ol } from "../markdown/list"; -import { paragraph } from "../markdown/paragraph"; -import { anchor } from "../markdown/anchor"; import { useMicroagentManagementStore } from "#/state/microagent-management-store"; import { useRepositoryMicroagentContent } from "#/hooks/query/use-repository-microagent-content"; import { I18nKey } from "#/i18n/declaration"; import { extractRepositoryInfo } from "#/utils/utils"; +import { MarkdownRenderer } from "../markdown/markdown-renderer"; export function MicroagentManagementViewMicroagentContent() { const { t } = useTranslation(); @@ -49,18 +43,9 @@ export function MicroagentManagementViewMicroagentContent() {
)} {microagentData && !isLoading && !error && ( - + {microagentData.content} - + )}
); diff --git a/frontend/src/components/v1/chat/event-content-helpers/get-observation-content.ts b/frontend/src/components/v1/chat/event-content-helpers/get-observation-content.ts index a227e99cfcf6..35d01b0655df 100644 --- a/frontend/src/components/v1/chat/event-content-helpers/get-observation-content.ts +++ b/frontend/src/components/v1/chat/event-content-helpers/get-observation-content.ts @@ -184,7 +184,22 @@ const getFinishObservationContent = ( event: ObservationEvent, ): string => { const { observation } = event; - return observation.message || ""; + + // Extract text content from the observation + const textContent = observation.content + .filter((c) => c.type === "text") + .map((c) => c.text) + .join("\n"); + + let content = ""; + + if (observation.is_error) { + content += `**Error:**\n${textContent}`; + } else { + content += textContent; + } + + return content; }; export const getObservationContent = (event: ObservationEvent): string => { diff --git a/frontend/src/components/v1/chat/event-message-components/generic-event-message-wrapper.tsx b/frontend/src/components/v1/chat/event-message-components/generic-event-message-wrapper.tsx index 94f35aec66b2..95c2652549a8 100644 --- a/frontend/src/components/v1/chat/event-message-components/generic-event-message-wrapper.tsx +++ b/frontend/src/components/v1/chat/event-message-components/generic-event-message-wrapper.tsx @@ -9,6 +9,7 @@ import { } from "../event-content-helpers/create-skill-ready-event"; import { V1ConfirmationButtons } from "#/components/shared/buttons/v1-confirmation-buttons"; import { ObservationResultStatus } from "../../../features/chat/event-content-helpers/get-observation-result"; +import { MarkdownRenderer } from "#/components/features/markdown/markdown-renderer"; interface GenericEventMessageWrapperProps { event: OpenHandsEvent | SkillReadyEvent; @@ -23,11 +24,17 @@ export function GenericEventMessageWrapper({ // SkillReadyEvent is not an observation event, so skip the observation checks if (!isSkillReadyEvent(event)) { - if ( - isObservationEvent(event) && - event.observation.kind === "TaskTrackerObservation" - ) { - return
{details}
; + if (isObservationEvent(event)) { + if (event.observation.kind === "TaskTrackerObservation") { + return
{details}
; + } + if (event.observation.kind === "FinishObservation") { + return ( + + {details as string} + + ); + } } } diff --git a/frontend/src/routes/planner-tab.tsx b/frontend/src/routes/planner-tab.tsx index 4fb46f993979..989e85596e92 100644 --- a/frontend/src/routes/planner-tab.tsx +++ b/frontend/src/routes/planner-tab.tsx @@ -1,22 +1,8 @@ import { useTranslation } from "react-i18next"; -import Markdown from "react-markdown"; -import remarkGfm from "remark-gfm"; -import remarkBreaks from "remark-breaks"; import { I18nKey } from "#/i18n/declaration"; import LessonPlanIcon from "#/icons/lesson-plan.svg?react"; import { useConversationStore } from "#/state/conversation-store"; -import { code } from "#/components/features/markdown/code"; -import { ul, ol } from "#/components/features/markdown/list"; -import { paragraph } from "#/components/features/markdown/paragraph"; -import { anchor } from "#/components/features/markdown/anchor"; -import { - h1, - h2, - h3, - h4, - h5, - h6, -} from "#/components/features/markdown/headings"; +import { MarkdownRenderer } from "#/components/features/markdown/markdown-renderer"; function PlannerTab() { const { t } = useTranslation(); @@ -26,24 +12,9 @@ function PlannerTab() { if (planContent !== null && planContent !== undefined) { return (
- + {planContent} - +
); } diff --git a/frontend/src/types/v1/core/base/observation.ts b/frontend/src/types/v1/core/base/observation.ts index 062d7ddf6e21..7e510888f0f4 100644 --- a/frontend/src/types/v1/core/base/observation.ts +++ b/frontend/src/types/v1/core/base/observation.ts @@ -25,9 +25,13 @@ export interface MCPToolObservation export interface FinishObservation extends ObservationBase<"FinishObservation"> { /** - * Final message sent to the user + * Content returned from the finish action as a list of TextContent/ImageContent objects. */ - message: string; + content: Array; + /** + * Whether the finish action resulted in an error + */ + is_error: boolean; } export interface ThinkObservation extends ObservationBase<"ThinkObservation"> { From 6c2862ae082100990c2533f07461a7649199936f Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Mon, 1 Dec 2025 11:08:00 -0500 Subject: [PATCH 074/108] feat(frontend): add handler for 'create a plan' button click (#11806) --- .../features/chat/change-agent-button.tsx | 55 ++------------ frontend/src/hooks/use-handle-plan-click.ts | 71 +++++++++++++++++++ frontend/src/routes/planner-tab.tsx | 6 +- 3 files changed, 81 insertions(+), 51 deletions(-) create mode 100644 frontend/src/hooks/use-handle-plan-click.ts diff --git a/frontend/src/components/features/chat/change-agent-button.tsx b/frontend/src/components/features/chat/change-agent-button.tsx index 62575879631e..68a0bd26997a 100644 --- a/frontend/src/components/features/chat/change-agent-button.tsx +++ b/frontend/src/components/features/chat/change-agent-button.tsx @@ -12,20 +12,15 @@ import { USE_PLANNING_AGENT } from "#/utils/feature-flags"; import { useAgentState } from "#/hooks/use-agent-state"; import { AgentState } from "#/types/agent-state"; import { useActiveConversation } from "#/hooks/query/use-active-conversation"; -import { useCreateConversation } from "#/hooks/mutation/use-create-conversation"; -import { displaySuccessToast } from "#/utils/custom-toast-handlers"; import { useUnifiedWebSocketStatus } from "#/hooks/use-unified-websocket-status"; import { useSubConversationTaskPolling } from "#/hooks/query/use-sub-conversation-task-polling"; +import { useHandlePlanClick } from "#/hooks/use-handle-plan-click"; export function ChangeAgentButton() { const [contextMenuOpen, setContextMenuOpen] = useState(false); - const { - conversationMode, - setConversationMode, - setSubConversationTaskId, - subConversationTaskId, - } = useConversationStore(); + const { conversationMode, setConversationMode, subConversationTaskId } = + useConversationStore(); const webSocketStatus = useUnifiedWebSocketStatus(); @@ -40,8 +35,6 @@ export function ChangeAgentButton() { const isAgentRunning = curAgentState === AgentState.RUNNING; const { data: conversation } = useActiveConversation(); - const { mutate: createConversation, isPending: isCreatingConversation } = - useCreateConversation(); // Poll sub-conversation task and invalidate parent conversation when ready useSubConversationTaskPolling( @@ -49,6 +42,9 @@ export function ChangeAgentButton() { conversation?.conversation_id || null, ); + // Get handlePlanClick and isCreatingConversation from custom hook + const { handlePlanClick, isCreatingConversation } = useHandlePlanClick(); + // Close context menu when agent starts running useEffect(() => { if ((isAgentRunning || !isWebSocketConnected) && contextMenuOpen) { @@ -56,45 +52,6 @@ export function ChangeAgentButton() { } }, [isAgentRunning, contextMenuOpen, isWebSocketConnected]); - const handlePlanClick = ( - event: React.MouseEvent | KeyboardEvent, - ) => { - event.preventDefault(); - event.stopPropagation(); - - // Set conversation mode to "plan" immediately - setConversationMode("plan"); - - // Check if sub_conversation_ids is not empty - if ( - (conversation?.sub_conversation_ids && - conversation.sub_conversation_ids.length > 0) || - !conversation?.conversation_id - ) { - // Do nothing if both conditions are true - return; - } - - // Create a new sub-conversation if we have a current conversation ID - createConversation( - { - parentConversationId: conversation.conversation_id, - agentType: "plan", - }, - { - onSuccess: (data) => { - displaySuccessToast( - t(I18nKey.PLANNING_AGENTT$PLANNING_AGENT_INITIALIZED), - ); - // Track the task ID to poll for sub-conversation creation - if (data.v1_task_id) { - setSubConversationTaskId(data.v1_task_id); - } - }, - }, - ); - }; - const isButtonDisabled = isAgentRunning || isCreatingConversation || diff --git a/frontend/src/hooks/use-handle-plan-click.ts b/frontend/src/hooks/use-handle-plan-click.ts new file mode 100644 index 000000000000..9734bab8da86 --- /dev/null +++ b/frontend/src/hooks/use-handle-plan-click.ts @@ -0,0 +1,71 @@ +import { useCallback } from "react"; +import { useTranslation } from "react-i18next"; +import { I18nKey } from "#/i18n/declaration"; +import { useConversationStore } from "#/state/conversation-store"; +import { useActiveConversation } from "#/hooks/query/use-active-conversation"; +import { useCreateConversation } from "#/hooks/mutation/use-create-conversation"; +import { displaySuccessToast } from "#/utils/custom-toast-handlers"; + +/** + * Custom hook that encapsulates the logic for handling plan creation. + * Returns a function that can be called to create a plan conversation and + * the pending state of the conversation creation. + * + * @returns An object containing handlePlanClick function and isCreatingConversation boolean + */ +export const useHandlePlanClick = () => { + const { t } = useTranslation(); + const { setConversationMode, setSubConversationTaskId } = + useConversationStore(); + const { data: conversation } = useActiveConversation(); + const { mutate: createConversation, isPending: isCreatingConversation } = + useCreateConversation(); + + const handlePlanClick = useCallback( + (event?: React.MouseEvent | KeyboardEvent) => { + event?.preventDefault(); + event?.stopPropagation(); + + // Set conversation mode to "plan" immediately + setConversationMode("plan"); + + // Check if sub_conversation_ids is not empty + if ( + (conversation?.sub_conversation_ids && + conversation.sub_conversation_ids.length > 0) || + !conversation?.conversation_id + ) { + // Do nothing if both conditions are true + return; + } + + // Create a new sub-conversation if we have a current conversation ID + createConversation( + { + parentConversationId: conversation.conversation_id, + agentType: "plan", + }, + { + onSuccess: (data) => { + displaySuccessToast( + t(I18nKey.PLANNING_AGENTT$PLANNING_AGENT_INITIALIZED), + ); + // Track the task ID to poll for sub-conversation creation + if (data.v1_task_id) { + setSubConversationTaskId(data.v1_task_id); + } + }, + }, + ); + }, + [ + conversation, + createConversation, + setConversationMode, + setSubConversationTaskId, + t, + ], + ); + + return { handlePlanClick, isCreatingConversation }; +}; diff --git a/frontend/src/routes/planner-tab.tsx b/frontend/src/routes/planner-tab.tsx index 989e85596e92..fee7c9efc8de 100644 --- a/frontend/src/routes/planner-tab.tsx +++ b/frontend/src/routes/planner-tab.tsx @@ -3,11 +3,13 @@ import { I18nKey } from "#/i18n/declaration"; import LessonPlanIcon from "#/icons/lesson-plan.svg?react"; import { useConversationStore } from "#/state/conversation-store"; import { MarkdownRenderer } from "#/components/features/markdown/markdown-renderer"; +import { useHandlePlanClick } from "#/hooks/use-handle-plan-click"; function PlannerTab() { const { t } = useTranslation(); - const { planContent, setConversationMode } = useConversationStore(); + const { planContent } = useConversationStore(); + const { handlePlanClick } = useHandlePlanClick(); if (planContent !== null && planContent !== undefined) { return ( @@ -27,7 +29,7 @@ function PlannerTab() {