From 81c754ec6546c475c38fc1e56a4c3a8bb72c2ccc Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Tue, 13 May 2025 11:57:32 -0400 Subject: [PATCH] [Fix]: Don't allow endpoint to modify conversation trigger (#8396) --- .../components/features/home/home-header.test.tsx | 1 - .../components/features/home/repo-connector.test.tsx | 1 - .../__tests__/components/features/home/task-card.test.tsx | 1 - frontend/src/api/open-hands.ts | 3 --- frontend/src/components/features/home/home-header.tsx | 2 +- .../src/components/features/home/repo-selection-form.tsx | 1 - frontend/src/components/features/home/tasks/task-card.tsx | 1 - frontend/src/components/shared/task-form.tsx | 2 +- frontend/src/hooks/mutation/use-create-conversation.ts | 3 --- openhands/server/routes/manage_conversations.py | 4 ++-- openhands/storage/data_models/conversation_metadata.py | 1 + tests/unit/test_conversation.py | 8 -------- 12 files changed, 5 insertions(+), 23 deletions(-) diff --git a/frontend/__tests__/components/features/home/home-header.test.tsx b/frontend/__tests__/components/features/home/home-header.test.tsx index fc0201ce13..befb348931 100644 --- a/frontend/__tests__/components/features/home/home-header.test.tsx +++ b/frontend/__tests__/components/features/home/home-header.test.tsx @@ -43,7 +43,6 @@ describe("HomeHeader", () => { await userEvent.click(launchButton); expect(createConversationSpy).toHaveBeenCalledExactlyOnceWith( - "gui", undefined, undefined, undefined, diff --git a/frontend/__tests__/components/features/home/repo-connector.test.tsx b/frontend/__tests__/components/features/home/repo-connector.test.tsx index 7a076c912e..d4dcb7923f 100644 --- a/frontend/__tests__/components/features/home/repo-connector.test.tsx +++ b/frontend/__tests__/components/features/home/repo-connector.test.tsx @@ -173,7 +173,6 @@ describe("RepoConnector", () => { await userEvent.click(launchButton); expect(createConversationSpy).toHaveBeenCalledExactlyOnceWith( - "gui", "rbren/polaris", "github", undefined, diff --git a/frontend/__tests__/components/features/home/task-card.test.tsx b/frontend/__tests__/components/features/home/task-card.test.tsx index 3aad56ed10..83bf2c1763 100644 --- a/frontend/__tests__/components/features/home/task-card.test.tsx +++ b/frontend/__tests__/components/features/home/task-card.test.tsx @@ -85,7 +85,6 @@ describe("TaskCard", () => { await userEvent.click(launchButton); expect(createConversationSpy).toHaveBeenCalledWith( - "suggested_task", MOCK_RESPOSITORIES[0].full_name, MOCK_RESPOSITORIES[0].git_provider, undefined, diff --git a/frontend/src/api/open-hands.ts b/frontend/src/api/open-hands.ts index 41c25c6a89..19d0dafc3e 100644 --- a/frontend/src/api/open-hands.ts +++ b/frontend/src/api/open-hands.ts @@ -10,7 +10,6 @@ import { GetTrajectoryResponse, GitChangeDiff, GitChange, - ConversationTrigger, } from "./open-hands.types"; import { openHands } from "./open-hands-axios"; import { ApiSettings, PostApiSettings, Provider } from "#/types/settings"; @@ -144,7 +143,6 @@ class OpenHands { } static async createConversation( - conversation_trigger: ConversationTrigger = "gui", selectedRepository?: string, git_provider?: Provider, initialUserMsg?: string, @@ -154,7 +152,6 @@ class OpenHands { selected_branch?: string, ): Promise { const body = { - conversation_trigger, repository: selectedRepository, git_provider, selected_branch, diff --git a/frontend/src/components/features/home/home-header.tsx b/frontend/src/components/features/home/home-header.tsx index f7744e1271..ea60c65036 100644 --- a/frontend/src/components/features/home/home-header.tsx +++ b/frontend/src/components/features/home/home-header.tsx @@ -28,7 +28,7 @@ export function HomeHeader() { testId="header-launch-button" variant="primary" type="button" - onClick={() => createConversation({ conversation_trigger: "gui" })} + onClick={() => createConversation({})} isDisabled={isCreatingConversation} > {!isCreatingConversation && "Launch from Scratch"} diff --git a/frontend/src/components/features/home/repo-selection-form.tsx b/frontend/src/components/features/home/repo-selection-form.tsx index 2bc870eadd..2b09040cdc 100644 --- a/frontend/src/components/features/home/repo-selection-form.tsx +++ b/frontend/src/components/features/home/repo-selection-form.tsx @@ -200,7 +200,6 @@ export function RepositorySelectionForm({ onClick={() => createConversation({ selectedRepository, - conversation_trigger: "gui", selected_branch: selectedBranch?.name, }) } diff --git a/frontend/src/components/features/home/tasks/task-card.tsx b/frontend/src/components/features/home/tasks/task-card.tsx index c86dcdc05b..cddc6764f2 100644 --- a/frontend/src/components/features/home/tasks/task-card.tsx +++ b/frontend/src/components/features/home/tasks/task-card.tsx @@ -40,7 +40,6 @@ export function TaskCard({ task }: TaskCardProps) { const repo = getRepo(task.repo, task.git_provider); return createConversation({ - conversation_trigger: "suggested_task", selectedRepository: repo, suggested_task: task, }); diff --git a/frontend/src/components/shared/task-form.tsx b/frontend/src/components/shared/task-form.tsx index 278682c360..f6d35c8423 100644 --- a/frontend/src/components/shared/task-form.tsx +++ b/frontend/src/components/shared/task-form.tsx @@ -52,7 +52,7 @@ export function TaskForm({ ref }: TaskFormProps) { const formData = new FormData(event.currentTarget); const q = formData.get("q")?.toString(); - createConversation({ q, conversation_trigger: "gui" }); + createConversation({ q }); }; return ( diff --git a/frontend/src/hooks/mutation/use-create-conversation.ts b/frontend/src/hooks/mutation/use-create-conversation.ts index 83d2c42fcf..1f515f7fc3 100644 --- a/frontend/src/hooks/mutation/use-create-conversation.ts +++ b/frontend/src/hooks/mutation/use-create-conversation.ts @@ -6,7 +6,6 @@ import OpenHands from "#/api/open-hands"; import { setInitialPrompt } from "#/state/initial-query-slice"; import { RootState } from "#/store"; import { GitRepository } from "#/types/git"; -import { ConversationTrigger } from "#/api/open-hands.types"; import { SuggestedTask } from "#/components/features/home/tasks/task.types"; export const useCreateConversation = () => { @@ -21,7 +20,6 @@ export const useCreateConversation = () => { return useMutation({ mutationKey: ["create-conversation"], mutationFn: async (variables: { - conversation_trigger: ConversationTrigger; q?: string; selectedRepository?: GitRepository | null; selected_branch?: string; @@ -30,7 +28,6 @@ export const useCreateConversation = () => { if (variables.q) dispatch(setInitialPrompt(variables.q)); return OpenHands.createConversation( - variables.conversation_trigger, variables.selectedRepository ? variables.selectedRepository.full_name : undefined, diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 35620416eb..06922f0f6e 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -50,7 +50,6 @@ app = APIRouter(prefix='/api') class InitSessionRequest(BaseModel): - conversation_trigger: ConversationTrigger = ConversationTrigger.GUI repository: str | None = None git_provider: ProviderType | None = None selected_branch: str | None = None @@ -181,9 +180,10 @@ async def new_conversation( image_urls = data.image_urls or [] replay_json = data.replay_json suggested_task = data.suggested_task - conversation_trigger = data.conversation_trigger git_provider = data.git_provider + conversation_trigger = ConversationTrigger.GUI + if suggested_task: initial_user_msg = suggested_task.get_prompt_for_task() conversation_trigger = ConversationTrigger.SUGGESTED_TASK diff --git a/openhands/storage/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py index 041e25c570..18d84c076c 100644 --- a/openhands/storage/data_models/conversation_metadata.py +++ b/openhands/storage/data_models/conversation_metadata.py @@ -8,6 +8,7 @@ class ConversationTrigger(Enum): GUI = 'gui' SUGGESTED_TASK = 'suggested_task' REMOTE_API_KEY = 'openhands_api' + SLACK = 'slack' @dataclass diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 5276020619..4f70b748a6 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -240,7 +240,6 @@ async def test_new_conversation_success(provider_handler_mock): mock_create_conversation.return_value = 'test_conversation_id' test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, repository='test/repo', selected_branch='main', initial_user_msg='Hello, agent!', @@ -297,7 +296,6 @@ async def test_new_conversation_with_suggested_task(provider_handler_mock): ) test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.SUGGESTED_TASK, repository='test/repo', selected_branch='main', suggested_task=test_task, @@ -347,7 +345,6 @@ async def test_new_conversation_missing_settings(provider_handler_mock): ) test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, repository='test/repo', selected_branch='main', initial_user_msg='Hello, agent!', @@ -377,7 +374,6 @@ async def test_new_conversation_invalid_api_key(provider_handler_mock): ) test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, repository='test/repo', selected_branch='main', initial_user_msg='Hello, agent!', @@ -469,7 +465,6 @@ async def test_new_conversation_with_bearer_auth(provider_handler_mock): # Create the request object test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, # This should be overridden repository='test/repo', selected_branch='main', initial_user_msg='Hello, agent!', @@ -503,7 +498,6 @@ async def test_new_conversation_with_null_repository(): # Create the request object with null repository test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, repository=None, # Explicitly set to None selected_branch=None, initial_user_msg='Hello, agent!', @@ -541,7 +535,6 @@ async def test_new_conversation_with_provider_authentication_error( # Create the request object test_request = InitSessionRequest( - conversation_trigger=ConversationTrigger.GUI, repository='test/repo', selected_branch='main', initial_user_msg='Hello, agent!', @@ -571,7 +564,6 @@ async def test_new_conversation_with_provider_authentication_error( @pytest.mark.asyncio async def test_new_conversation_with_unsupported_params(test_client): test_request_data = { - 'conversation_trigger': 'GUI', # This is a valid parameter 'repository': 'test/repo', # This is valid 'selected_branch': 'main', # This is valid 'initial_user_msg': 'Hello, agent!', # Valid parameter