From 831820b5d4aba79eac29add9b6aebf050605eb8c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 8 Jun 2026 11:01:07 +0000 Subject: [PATCH] fix(tui): add remove() method to RunnerRegistry for memory cleanup Adds a remove() method to RunnerRegistry that allows removing runners from the registry when conversations end. This prevents memory accumulation in long-running sessions. If the removed runner is the current runner, the current reference is also cleared. Addresses item from code quality report. Related to #766 Co-authored-by: openhands --- openhands_cli/tui/core/runner_registry.py | 13 +++ tests/tui/core/test_runner_registry.py | 98 +++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 tests/tui/core/test_runner_registry.py diff --git a/openhands_cli/tui/core/runner_registry.py b/openhands_cli/tui/core/runner_registry.py index 81389043f..e408c33f3 100644 --- a/openhands_cli/tui/core/runner_registry.py +++ b/openhands_cli/tui/core/runner_registry.py @@ -55,3 +55,16 @@ def get_or_create(self, conversation_id: uuid.UUID) -> ConversationRunner: self._current_runner = runner return runner + + def remove(self, conversation_id: uuid.UUID) -> None: + """Remove a runner from the registry to free memory. + + If the runner being removed is the current runner, the current runner + reference is also cleared. + + Args: + conversation_id: The ID of the conversation whose runner should be removed. + """ + runner = self._runners.pop(conversation_id, None) + if runner is not None and self._current_runner is runner: + self._current_runner = None diff --git a/tests/tui/core/test_runner_registry.py b/tests/tui/core/test_runner_registry.py new file mode 100644 index 000000000..dea4738f2 --- /dev/null +++ b/tests/tui/core/test_runner_registry.py @@ -0,0 +1,98 @@ +"""Tests for RunnerRegistry.""" + +from __future__ import annotations + +import uuid +from typing import TYPE_CHECKING +from unittest.mock import MagicMock + +from openhands_cli.tui.core.runner_registry import RunnerRegistry + + +if TYPE_CHECKING: + from openhands_cli.tui.core.conversation_runner import ConversationRunner + + +def _make_mock_runner() -> "ConversationRunner": + """Create a new mock runner.""" + mock_runner = MagicMock() + mock_runner.conversation = None # Avoid attach_conversation_state call + return mock_runner + + +class TestRunnerRegistry: + """Tests for the RunnerRegistry class.""" + + def _create_registry(self) -> RunnerRegistry: + """Create a RunnerRegistry with mock dependencies.""" + factory = MagicMock() + state = MagicMock() + message_pump = MagicMock() + notification_callback = MagicMock() + + # Configure factory.create to return a new mock runner each time + factory.create.side_effect = lambda *args, **kwargs: _make_mock_runner() + + return RunnerRegistry( + factory=factory, + state=state, + message_pump=message_pump, + notification_callback=notification_callback, + ) + + def test_remove_nonexistent_runner_is_noop(self) -> None: + """Removing a non-existent runner should not raise.""" + registry = self._create_registry() + conversation_id = uuid.uuid4() + + # Should not raise + registry.remove(conversation_id) + + def test_remove_existing_runner_clears_current(self) -> None: + """Removing the current runner should clear the current reference.""" + registry = self._create_registry() + conversation_id = uuid.uuid4() + + # Create a runner + runner = registry.get_or_create(conversation_id) + assert registry.current is runner + + # Remove it + registry.remove(conversation_id) + + # Current should be cleared since removed runner was current + assert registry.current is None + + def test_remove_creates_new_runner_on_next_get(self) -> None: + """After removal, get_or_create should create a new runner.""" + registry = self._create_registry() + conversation_id = uuid.uuid4() + + # Create a runner + runner = registry.get_or_create(conversation_id) + + # Remove it + registry.remove(conversation_id) + + # Getting the same conversation ID should create a new runner + new_runner = registry.get_or_create(conversation_id) + assert new_runner is not runner # It's a new mock from factory + + def test_remove_non_current_runner_preserves_current(self) -> None: + """Removing a non-current runner should not affect current.""" + registry = self._create_registry() + conversation_id_1 = uuid.uuid4() + conversation_id_2 = uuid.uuid4() + + # Create two runners + runner_1 = registry.get_or_create(conversation_id_1) + runner_2 = registry.get_or_create(conversation_id_2) + + # Current should be runner_2 (last get_or_create) + assert registry.current is runner_2 + + # Remove runner_1 (not current) + registry.remove(conversation_id_1) + + # Current should still be runner_2 + assert registry.current is runner_2