From a03ad1079c683fa44e0301e2433bf273b229a10d Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Fri, 21 Mar 2025 23:23:15 +0100 Subject: [PATCH] Rename oh_action to oh_user_action for clarity (#7368) Co-authored-by: openhands --- .../context/ws-client-provider.test.tsx | 63 ++++++++++++++++++- frontend/src/context/ws-client-provider.tsx | 2 +- frontend/src/mocks/handlers.ws.ts | 2 +- openhands/server/listen_socket.py | 7 +++ openhands/server/session/README.md | 2 +- pyproject.toml | 2 - tests/unit/test_socket_events.py | 43 +++++++++++++ 7 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_socket_events.py diff --git a/frontend/__tests__/context/ws-client-provider.test.tsx b/frontend/__tests__/context/ws-client-provider.test.tsx index 6c3bf564b3..7b5cec4a9c 100644 --- a/frontend/__tests__/context/ws-client-provider.test.tsx +++ b/frontend/__tests__/context/ws-client-provider.test.tsx @@ -1,9 +1,12 @@ -import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; import * as ChatSlice from "#/state/chat-slice"; import { updateStatusWhenErrorMessagePresent, + WsClientProvider, + useWsClient, } from "#/context/ws-client-provider"; +import React from "react"; describe("Propagate error message", () => { it("should do nothing when no message was passed from server", () => { @@ -41,3 +44,59 @@ describe("Propagate error message", () => { }); }); }); + +// Create a mock for socket.io-client +const mockEmit = vi.fn(); +const mockOn = vi.fn(); +const mockOff = vi.fn(); +const mockDisconnect = vi.fn(); + +vi.mock("socket.io-client", () => { + return { + io: vi.fn(() => ({ + emit: mockEmit, + on: mockOn, + off: mockOff, + disconnect: mockDisconnect, + io: { + opts: { + query: {}, + }, + }, + })), + }; +}); + +// Mock component to test the hook +const TestComponent = () => { + const { send } = useWsClient(); + + React.useEffect(() => { + // Send a test event + send({ type: "test_event" }); + }, [send]); + + return
Test Component
; +}; + +describe("WsClientProvider", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should emit oh_user_action event when send is called", async () => { + const { getByText } = render( + + + + ); + + // Assert + expect(getByText("Test Component")).toBeInTheDocument(); + + // Wait for the emit call to happen (useEffect needs time to run) + await waitFor(() => { + expect(mockEmit).toHaveBeenCalledWith("oh_user_action", { type: "test_event" }); + }, { timeout: 1000 }); + }); +}); diff --git a/frontend/src/context/ws-client-provider.tsx b/frontend/src/context/ws-client-provider.tsx index a5610fc50f..4113f2064c 100644 --- a/frontend/src/context/ws-client-provider.tsx +++ b/frontend/src/context/ws-client-provider.tsx @@ -118,7 +118,7 @@ export function WsClientProvider({ EventLogger.error("WebSocket is not connected."); return; } - sioRef.current.emit("oh_action", event); + sioRef.current.emit("oh_user_action", event); } function handleConnect() { diff --git a/frontend/src/mocks/handlers.ws.ts b/frontend/src/mocks/handlers.ws.ts index 0fcc03ffbf..35c86ab4c9 100644 --- a/frontend/src/mocks/handlers.ws.ts +++ b/frontend/src/mocks/handlers.ws.ts @@ -35,7 +35,7 @@ export const handlers: WebSocketHandler[] = [ ); } - io.client.on("oh_action", async (_, data) => { + io.client.on("oh_user_action", async (_, data) => { if (isInitConfig(data)) { io.client.emit( "oh_event", diff --git a/openhands/server/listen_socket.py b/openhands/server/listen_socket.py index 910e849fdd..c09771c0c0 100644 --- a/openhands/server/listen_socket.py +++ b/openhands/server/listen_socket.py @@ -75,8 +75,15 @@ async def connect(connection_id: str, environ): logger.info(f'Finished replaying event stream for conversation {conversation_id}') +@sio.event +async def oh_user_action(connection_id: str, data: dict): + await conversation_manager.send_to_event_stream(connection_id, data) + + @sio.event async def oh_action(connection_id: str, data: dict): + # TODO: Remove this handler once all clients are updated to use oh_user_action + # Keeping for backward compatibility with in-progress sessions await conversation_manager.send_to_event_stream(connection_id, data) diff --git a/openhands/server/session/README.md b/openhands/server/session/README.md index 86cce9194d..c4d5f63b65 100644 --- a/openhands/server/session/README.md +++ b/openhands/server/session/README.md @@ -8,7 +8,7 @@ interruptions are recoverable. There are 3 main server side event handlers: * `connect` - Invoked when a new connection to the server is established. (This may be via http or WebSocket) -* `oh_action` - Invoked when a connected client sends an event (such as a prompt for the Agent) - +* `oh_user_action` - Invoked when a connected client sends an event (such as a prompt for the Agent) - this is distinct from the `oh_event` sent from the server to the client. * `disconnect` - Invoked when a connected client disconnects from the server. diff --git a/pyproject.toml b/pyproject.toml index d40c76ec96..9d182a37af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -100,7 +100,6 @@ reportlab = "*" concurrency = ["gevent"] - [tool.poetry.group.runtime.dependencies] jupyterlab = "*" notebook = "*" @@ -130,7 +129,6 @@ ignore = ["D1"] convention = "google" - [tool.poetry.group.evaluation.dependencies] streamlit = "*" whatthepatch = "*" diff --git a/tests/unit/test_socket_events.py b/tests/unit/test_socket_events.py new file mode 100644 index 0000000000..1c50396cbb --- /dev/null +++ b/tests/unit/test_socket_events.py @@ -0,0 +1,43 @@ +from unittest.mock import AsyncMock, patch + +import pytest + +from openhands.server.listen_socket import oh_action, oh_user_action + + +@pytest.mark.asyncio +async def test_oh_user_action(): + """Test that oh_user_action correctly forwards data to the conversation manager.""" + connection_id = 'test_connection_id' + test_data = {'action': 'test_action', 'data': 'test_data'} + + # Mock the conversation_manager + with patch('openhands.server.listen_socket.conversation_manager') as mock_manager: + mock_manager.send_to_event_stream = AsyncMock() + + # Call the function + await oh_user_action(connection_id, test_data) + + # Verify the conversation manager was called with the correct arguments + mock_manager.send_to_event_stream.assert_called_once_with( + connection_id, test_data + ) + + +@pytest.mark.asyncio +async def test_oh_action(): + """Test that oh_action (legacy handler) correctly forwards data to the conversation manager.""" + connection_id = 'test_connection_id' + test_data = {'action': 'test_action', 'data': 'test_data'} + + # Mock the conversation_manager + with patch('openhands.server.listen_socket.conversation_manager') as mock_manager: + mock_manager.send_to_event_stream = AsyncMock() + + # Call the function + await oh_action(connection_id, test_data) + + # Verify the conversation manager was called with the correct arguments + mock_manager.send_to_event_stream.assert_called_once_with( + connection_id, test_data + )