Merge branch 'main' into fix/utils-type-hints

- Use --all-files instead of glob patterns for consistent behavior
- Improve error handling and failure conditions
- Add clearer comments

openhands/runtime/browser/utils.py

@@ -31,37 +31,81 @@ async def browse(
     try:
         # obs provided by BrowserGym: see https://github.com/ServiceNow/BrowserGym/blob/main/core/src/browsergym/core/env.py#L396
         obs = await call_sync_from_async(browser.step, action_str)
+
+        # Extract values with type checking
+        text_content = obs.get('text_content', '')
+        if not isinstance(text_content, str):
+            text_content = str(text_content)
+
+        url = obs.get('url', '')
+        if not isinstance(url, str):
+            url = str(url)
+
+        image_content = obs.get('image_content', [])
+        if not isinstance(image_content, list):
+            image_content = []
+
+        open_pages_urls = obs.get('open_pages_urls', [])
+        if not isinstance(open_pages_urls, list):
+            open_pages_urls = []
+
+        active_page_index = obs.get('active_page_index', -1)
+        if not isinstance(active_page_index, int):
+            try:
+                active_page_index = int(active_page_index)
+            except (ValueError, TypeError):
+                active_page_index = -1
+
+        dom_object = obs.get('dom_object', {})
+        if not isinstance(dom_object, dict):
+            dom_object = {}
+
+        axtree_object = obs.get('axtree_object', {})
+        if not isinstance(axtree_object, dict):
+            axtree_object = {}
+
+        extra_element_properties = obs.get('extra_element_properties', {})
+        if not isinstance(extra_element_properties, dict):
+            extra_element_properties = {}
+
+        last_action = obs.get('last_action', '')
+        if not isinstance(last_action, str):
+            last_action = str(last_action)
+
+        last_action_error = obs.get('last_action_error', '')
+        if not isinstance(last_action_error, str):
+            last_action_error = str(last_action_error)
+
         return BrowserOutputObservation(
-            content=obs['text_content'],  # text content of the page
-            url=obs.get('url', ''),  # URL of the page
+            content=text_content,  # text content of the page
+            url=url,  # URL of the page
             screenshot=obs.get('screenshot', None),  # base64-encoded screenshot, png
             set_of_marks=obs.get(
                 'set_of_marks', None
             ),  # base64-encoded Set-of-Marks annotated screenshot, png,
-            goal_image_urls=obs.get('image_content', []),
-            open_pages_urls=obs.get('open_pages_urls', []),  # list of open pages
-            active_page_index=obs.get(
-                'active_page_index', -1
-            ),  # index of the active page
-            dom_object=obs.get('dom_object', {}),  # DOM object
-            axtree_object=obs.get('axtree_object', {}),  # accessibility tree object
-            extra_element_properties=obs.get('extra_element_properties', {}),
+            goal_image_urls=image_content,
+            open_pages_urls=open_pages_urls,  # list of open pages
+            active_page_index=active_page_index,  # index of the active page
+            dom_object=dom_object,  # DOM object
+            axtree_object=axtree_object,  # accessibility tree object
+            extra_element_properties=extra_element_properties,
             focused_element_bid=obs.get(
                 'focused_element_bid', None
             ),  # focused element bid
-            last_browser_action=obs.get(
-                'last_action', ''
-            ),  # last browser env action performed
-            last_browser_action_error=obs.get('last_action_error', ''),
-            error=True if obs.get('last_action_error', '') else False,  # error flag
+            last_browser_action=last_action,  # last browser env action performed
+            last_browser_action_error=last_action_error,
+            error=bool(last_action_error),  # error flag
             trigger_by_action=action.action,
         )
     except Exception as e:
+        error_message = str(e)
+        url_value = asked_url if action.action == ActionType.BROWSE else ''
+
         return BrowserOutputObservation(
-            content=str(e),
+            content=error_message,
             screenshot='',
             error=True,
-            last_browser_action_error=str(e),
-            url=asked_url if action.action == ActionType.BROWSE else '',
+            last_browser_action_error=error_message,
+            url=url_value,
             trigger_by_action=action.action,
         )

openhands/runtime/impl/daytona/daytona_runtime.py

@@ -184,6 +184,8 @@ class DaytonaRuntime(ActionExecutionClient):
 
         self.api_url = self._construct_api_url(self._sandbox_port)
 
+        # Ensure workspace is not None before accessing its attributes
+        assert self.workspace is not None, 'Workspace should not be None at this point'
         state = self.workspace.instance.state
 
         if state == 'stopping':

openhands/server/routes/files.py

@@ -33,6 +33,14 @@ from openhands.server.file_config import (
 )
 from openhands.utils.async_utils import call_sync_from_async
 
+
+def _write_to_file(file_path: str, contents: bytes) -> None:
+    """Helper function to write contents to a file."""
+    with open(file_path, 'wb') as file:
+        file.write(contents)
+        file.flush()
+
+
 app = APIRouter(prefix='/api/conversations/{conversation_id}')
 
 
@@ -195,9 +203,8 @@ async def upload_file(request: Request, conversation_id: str, files: list[Upload
             # copy the file to the runtime
             with tempfile.TemporaryDirectory() as tmp_dir:
                 tmp_file_path = os.path.join(tmp_dir, safe_filename)
-                with open(tmp_file_path, 'wb') as tmp_file:
-                    tmp_file.write(file_contents)
-                    tmp_file.flush()
+                # Use a helper function to write to the file
+                await call_sync_from_async(_write_to_file, tmp_file_path, file_contents)
 
                 runtime: Runtime = request.state.conversation.runtime
                 try:

openhands/server/routes/manage_conversations.py

@@ -112,7 +112,9 @@ async def _create_new_conversation(
             title=conversation_title,
             user_id=user_id,
             github_user_id=None,
-            selected_repository=selected_repository.full_name if selected_repository else selected_repository,
+            selected_repository=selected_repository.full_name
+            if selected_repository
+            else selected_repository,
             selected_branch=selected_branch,
         )
     )
@@ -380,7 +382,7 @@ async def delete_conversation(
 async def _get_conversation_info(
     conversation: ConversationMetadata,
     is_running: bool,
-) -> ConversationInfo | None:
+) -> ConversationInfo:
     try:
         title = conversation.title
         if not title:
@@ -400,4 +402,12 @@ async def _get_conversation_info(
             f'Error loading conversation {conversation.conversation_id}: {str(e)}',
             extra={'session_id': conversation.conversation_id},
         )
-        return None
+        # Create a default ConversationInfo object instead of returning None
+        return ConversationInfo(
+            conversation_id=conversation.conversation_id,
+            title=get_default_conversation_title(conversation.conversation_id),
+            last_updated_at=conversation.last_updated_at,
+            created_at=conversation.created_at,
+            selected_repository='',
+            status=ConversationStatus.STOPPED,
+        )

openhands/server/routes/public.py

@@ -15,6 +15,13 @@ from openhands.core.config import LLMConfig
 from openhands.core.logger import openhands_logger as logger
 from openhands.llm import bedrock
 from openhands.server.shared import config, server_config
+from openhands.utils.async_utils import call_sync_from_async
+
+
+def _get_ollama_models(url: str) -> list:
+    """Helper function to get Ollama models."""
+    return httpx.get(url, timeout=3).json()['models']
+
 
 app = APIRouter(prefix='/api/options')
 
@@ -60,7 +67,9 @@ async def get_litellm_models() -> list[str]:
         if ollama_base_url:
             ollama_url = ollama_base_url.strip('/') + '/api/tags'
             try:
-                ollama_models_list = httpx.get(ollama_url, timeout=3).json()['models']
+                ollama_models_list = await call_sync_from_async(
+                    _get_ollama_models, ollama_url
+                )
                 for model in ollama_models_list:
                     model_list.append('ollama/' + model['name'])
                 break

openhands/utils/async_utils.py

@@ -1,17 +1,22 @@
 import asyncio
 from concurrent import futures
 from concurrent.futures import ThreadPoolExecutor
-from typing import Callable, Coroutine, Iterable, List
+from typing import Any, Callable, Coroutine, Iterable, List, TypeVar
+
+T = TypeVar('T')  # Return type of the async function
+R = TypeVar('R')  # Return type of the sync function
 
 GENERAL_TIMEOUT: int = 15
 EXECUTOR = ThreadPoolExecutor()
 
 
-async def call_sync_from_async(fn: Callable, *args, **kwargs):
+async def call_sync_from_async(fn: Callable[..., R], *args: Any, **kwargs: Any) -> R:
     """
     Shorthand for running a function in the default background thread pool executor
     and awaiting the result. The nature of synchronous code is that the future
-    returned by this function is not cancellable
+    returned by this function is not cancellable.
+
+    Preserves the return type of the original function.
     """
     loop = asyncio.get_event_loop()
     coro = loop.run_in_executor(None, lambda: fn(*args, **kwargs))
@@ -20,24 +25,28 @@ async def call_sync_from_async(fn: Callable, *args, **kwargs):
 
 
 def call_async_from_sync(
-    corofn: Callable, timeout: float = GENERAL_TIMEOUT, *args, **kwargs
-):
+    corofn: Callable[..., Coroutine[Any, Any, T]],
+    timeout: float = GENERAL_TIMEOUT,
+    *args: Any,
+    **kwargs: Any,
+) -> T:
     """
     Shorthand for running a coroutine in the default background thread pool executor
-    and awaiting the result
-    """
+    and awaiting the result.
 
+    Preserves the return type of the original coroutine function.
+    """
     if corofn is None:
         raise ValueError('corofn is None')
     if not asyncio.iscoroutinefunction(corofn):
         raise ValueError('corofn is not a coroutine function')
 
-    async def arun():
+    async def arun() -> T:
         coro = corofn(*args, **kwargs)
         result = await coro
         return result
 
-    def run():
+    def run() -> T:
         loop_for_thread = asyncio.new_event_loop()
         try:
             asyncio.set_event_loop(loop_for_thread)
@@ -52,20 +61,31 @@ def call_async_from_sync(
 
 
 async def call_coro_in_bg_thread(
-    corofn: Callable, timeout: float = GENERAL_TIMEOUT, *args, **kwargs
-):
-    """Function for running a coroutine in a background thread."""
-    await call_sync_from_async(call_async_from_sync, corofn, timeout, *args, **kwargs)
+    corofn: Callable[..., Coroutine[Any, Any, T]],
+    timeout: float = GENERAL_TIMEOUT,
+    *args: Any,
+    **kwargs: Any,
+) -> T:
+    """
+    Function for running a coroutine in a background thread.
+
+    Preserves the return type of the original coroutine function.
+    """
+    return await call_sync_from_async(
+        call_async_from_sync, corofn, timeout, *args, **kwargs
+    )
 
 
 async def wait_all(
-    iterable: Iterable[Coroutine], timeout: int = GENERAL_TIMEOUT
-) -> List:
+    iterable: Iterable[Coroutine[Any, Any, T]], timeout: int = GENERAL_TIMEOUT
+) -> List[T]:
     """
     Shorthand for waiting for all the coroutines in the iterable given in parallel. Creates
     a task for each coroutine.
     Returns a list of results in the original order. If any single task raised an exception, this is raised.
     If multiple tasks raised exceptions, an AsyncException is raised containing all exceptions.
+
+    Preserves the return type of the original coroutines.
     """
     tasks = [asyncio.create_task(c) for c in iterable]
     if not tasks:
@@ -90,8 +110,8 @@ async def wait_all(
 
 
 class AsyncException(Exception):
-    def __init__(self, exceptions):
+    def __init__(self, exceptions: list[Exception]) -> None:
         self.exceptions = exceptions
 
-    def __str__(self):
+    def __str__(self) -> str:
         return '\n'.join(str(e) for e in self.exceptions)

openhands/utils/chunk_localizer.py

@@ -25,7 +25,7 @@ class Chunk(BaseModel):
         return ret
 
 
-def _create_chunks_from_raw_string(content: str, size: int):
+def _create_chunks_from_raw_string(content: str, size: int) -> list[Chunk]:
     lines = content.split('\n')
     ret = []
     for i in range(0, len(lines), size):
@@ -65,7 +65,7 @@ def normalized_lcs(chunk: str, query: str) -> float:
     """
     if len(chunk) == 0:
         return 0.0
-    _score = pylcs.lcs_sequence_length(chunk, query)
+    _score = float(pylcs.lcs_sequence_length(chunk, query))
     return _score / len(chunk)
 
 

openhands/utils/ensure_httpx_close.py

@@ -15,15 +15,17 @@ Hopefully, this will be fixed soon and we can remove this abomination.
 """
 
 import contextlib
-from typing import Callable
+from typing import Any, Callable, Iterator, TypeVar
 
 import httpx
 
+T = TypeVar('T')
+
 
 @contextlib.contextmanager
-def ensure_httpx_close():
+def ensure_httpx_close() -> Iterator[None]:
     wrapped_class = httpx.Client
-    proxys = []
+    proxys: list[Any] = []
 
     class ClientProxy:
         """
@@ -32,24 +34,24 @@ def ensure_httpx_close():
         where a client is reused, we need to be able to reuse the client even after closing it.
         """
 
-        client_constructor: Callable
-        args: tuple
-        kwargs: dict
-        client: httpx.Client
+        client_constructor: Callable[..., Any]
+        args: tuple[Any, ...]
+        kwargs: dict[str, Any]
+        client: httpx.Client | None
 
-        def __init__(self, *args, **kwargs):
+        def __init__(self, *args: Any, **kwargs: Any) -> None:
             self.args = args
             self.kwargs = kwargs
             self.client = wrapped_class(*self.args, **self.kwargs)
             proxys.append(self)
 
-        def __getattr__(self, name):
+        def __getattr__(self, name: str) -> Any:
             # Invoke a method on the proxied client - create one if required
             if self.client is None:
                 self.client = wrapped_class(*self.args, **self.kwargs)
             return getattr(self.client, name)
 
-        def close(self):
+        def close(self) -> None:
             # Close the client if it is open
             if self.client:
                 self.client.close()
@@ -62,17 +64,21 @@ def ensure_httpx_close():
             return object.__getattribute__(self, 'iter')(*args, **kwargs)
 
         @property
-        def is_closed(self):
+        def is_closed(self) -> bool:
             # Check if closed
             if self.client is None:
                 return True
-            return self.client.is_closed
+            # Convert to bool to ensure we return a bool
+            return bool(self.client.is_closed)
 
+    # We need to monkey patch the Client class to track instances
+    # This is a hack until LiteLLM fixes their client lifecycle management
+    original_client = httpx.Client
     httpx.Client = ClientProxy
     try:
         yield
     finally:
-        httpx.Client = wrapped_class
+        httpx.Client = original_client
         while proxys:
             proxy = proxys.pop()
             proxy.close()

openhands/utils/import_utils.py

@@ -5,12 +5,15 @@ from typing import Type, TypeVar
 T = TypeVar('T')
 
 
-def import_from(qual_name: str):
+def import_from(qual_name: str) -> type:
     """Import the value from the qualified name given"""
     parts = qual_name.split('.')
     module_name = '.'.join(parts[:-1])
     module = importlib.import_module(module_name)
     result = getattr(module, parts[-1])
+    assert isinstance(
+        result, type
+    ), f'Expected {qual_name} to be a type, got {type(result)}'
     return result
 
 

openhands/utils/search_utils.py

@@ -1,5 +1,5 @@
 import base64
-from typing import AsyncIterator, Callable
+from typing import Any, AsyncIterator, Callable
 
 
 def offset_to_page_id(offset: int, has_next: bool) -> str | None:
@@ -16,7 +16,7 @@ def page_id_to_offset(page_id: str | None) -> int:
     return offset
 
 
-async def iterate(fn: Callable, **kwargs) -> AsyncIterator:
+async def iterate(fn: Callable[..., Any], **kwargs: Any) -> AsyncIterator[Any]:
     """Iterate over paged result sets. Assumes that the results sets contain an array of result objects, and a next_page_id"""
     kwargs = {**kwargs}
     kwargs['page_id'] = None

openhands/utils/term_color.py

@@ -22,4 +22,7 @@ def colorize(text: str, color: TermColor = TermColor.WARNING) -> str:
     Returns:
         str: Colored text
     """
-    return colored(text, color.value)
+    # colored() returns a string with ANSI color codes
+    result = colored(text, color.value)
+    assert isinstance(result, str)
+    return result

tests/unit/test_agent_session.py

@@ -59,6 +59,7 @@ async def test_agent_session_start_with_no_state(mock_agent):
 
     # Create a mock runtime and set it up
     mock_runtime = MagicMock(spec=Runtime)
+    mock_runtime.get_microagents_from_selected_repo.return_value = []
 
     # Mock the runtime creation to set up the runtime attribute
     async def mock_create_runtime(*args, **kwargs):
@@ -142,6 +143,7 @@ async def test_agent_session_start_with_restored_state(mock_agent):
 
     # Create a mock runtime and set it up
     mock_runtime = MagicMock(spec=Runtime)
+    mock_runtime.get_microagents_from_selected_repo.return_value = []
 
     # Mock the runtime creation to set up the runtime attribute
     async def mock_create_runtime(*args, **kwargs):