diff --git a/.github/workflows/run-eval.yml b/.github/workflows/run-eval.yml index fe22baac1a..eaeb918ee1 100644 --- a/.github/workflows/run-eval.yml +++ b/.github/workflows/run-eval.yml @@ -25,7 +25,8 @@ on: sdk_ref: description: SDK commit/ref to evaluate (must be a semantic version like v1.0.0 unless 'Allow unreleased branches' is checked) required: true - default: v1.23.1 + default: v1.24.0 + diff --git a/openhands-agent-server/pyproject.toml b/openhands-agent-server/pyproject.toml index f5569286c7..51fbaa00c1 100644 --- a/openhands-agent-server/pyproject.toml +++ b/openhands-agent-server/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "openhands-agent-server" -version = "1.23.1" +version = "1.24.0" description = "OpenHands Agent Server - REST/WebSocket interface for OpenHands AI Agent" requires-python = ">=3.12" diff --git a/openhands-sdk/openhands/sdk/__init__.py b/openhands-sdk/openhands/sdk/__init__.py index 4191f2545a..a98dd195b6 100644 --- a/openhands-sdk/openhands/sdk/__init__.py +++ b/openhands-sdk/openhands/sdk/__init__.py @@ -1,7 +1,6 @@ from __future__ import annotations from importlib.metadata import PackageNotFoundError, version -from typing import TYPE_CHECKING, Any from openhands.sdk.agent import ( Agent, @@ -71,10 +70,6 @@ get_acp_provider, validate_agent_settings, ) - - -if TYPE_CHECKING: - from openhands.sdk.settings import LLMAgentSettings from openhands.sdk.settings.metadata import ( SettingProminence, SettingsFieldMetadata, @@ -119,35 +114,6 @@ # Print startup banner _print_banner(__version__) -_DEPRECATED_SDK_EXPORTS: dict[str, dict[str, str]] = { - "LLMAgentSettings": { - "deprecated_in": "1.19.0", - "removed_in": "1.24.0", - "details": ( - "Use ``OpenHandsAgentSettings`` directly. " - "``LLMAgentSettings`` was renamed in v1.19.0." - ), - }, -} - - -def __getattr__(name: str) -> Any: - if name in _DEPRECATED_SDK_EXPORTS: - from openhands.sdk.utils.deprecation import warn_deprecated - - info = _DEPRECATED_SDK_EXPORTS[name] - warn_deprecated( - f"Importing {name!r} from openhands.sdk", - deprecated_in=info["deprecated_in"], - removed_in=info["removed_in"], - details=info["details"], - stacklevel=3, - ) - from openhands.sdk import settings as _settings - - return getattr(_settings, name) - raise AttributeError(f"module {__name__!r} has no attribute {name!r}") - __all__ = [ "LLM", @@ -197,7 +163,6 @@ def __getattr__(name: str) -> Any: "ACPProviderInfo", "AgentSettingsBase", "AgentSettingsConfig", - "LLMAgentSettings", "OpenHandsAgentSettings", "build_session_model_meta", "default_agent_settings", diff --git a/openhands-sdk/openhands/sdk/settings/__init__.py b/openhands-sdk/openhands/sdk/settings/__init__.py index 91c1fd148e..436c23b2c1 100644 --- a/openhands-sdk/openhands/sdk/settings/__init__.py +++ b/openhands-sdk/openhands/sdk/settings/__init__.py @@ -37,7 +37,6 @@ AgentSettingsConfig, CondenserSettings, ConversationSettings, - LLMAgentSettings, OpenHandsAgentSettings, SettingsChoice, SettingsFieldSchema, @@ -86,7 +85,6 @@ "AgentSettingsConfig", "CondenserSettings", "ConversationSettings", - "LLMAgentSettings", "OpenHandsAgentSettings", "SETTINGS_METADATA_KEY", "SETTINGS_SECTION_METADATA_KEY", @@ -116,22 +114,6 @@ def __getattr__(name: str) -> Any: - if name == "LLMAgentSettings": - from openhands.sdk.utils.deprecation import warn_deprecated - - warn_deprecated( - f"Importing {name!r} from openhands.sdk.settings", - deprecated_in="1.19.0", - removed_in="1.24.0", - details=( - "Use ``OpenHandsAgentSettings`` directly. " - "``LLMAgentSettings`` was renamed in v1.19.0." - ), - stacklevel=3, - ) - from . import model - - return getattr(model, name) if name in _MODEL_EXPORTS: from . import model diff --git a/openhands-sdk/openhands/sdk/settings/model.py b/openhands-sdk/openhands/sdk/settings/model.py index 0a0c8d9cb5..a613daea25 100644 --- a/openhands-sdk/openhands/sdk/settings/model.py +++ b/openhands-sdk/openhands/sdk/settings/model.py @@ -433,7 +433,7 @@ def _migrate_agent_settings_v1_to_v2(payload: dict[str, Any]) -> dict[str, Any]: persisted payloads carried ``agent_kind: 'llm'``. The two classes are field-compatible (``LLMAgentSettings`` is a subclass of ``OpenHandsAgentSettings`` that only narrows the discriminator literal), - and ``LLMAgentSettings`` is scheduled for removal in v1.24.0. Rewriting + and ``LLMAgentSettings``'s import aliases were removed in v1.24.0. Rewriting the discriminator on read lets callers that explicitly validate as ``OpenHandsAgentSettings`` (the canonical class) accept legacy data without losing any fields. @@ -1250,17 +1250,18 @@ def create_agent(self) -> ACPAgent: class LLMAgentSettings(OpenHandsAgentSettings): - """Deprecated name for :class:`OpenHandsAgentSettings`. + """Legacy ``agent_kind='llm'`` variant of :class:`OpenHandsAgentSettings`. ``LLMAgentSettings`` was the public class name before the v1.19.0 rename. - It is kept as a :class:`OpenHandsAgentSettings` subclass so existing - callers keep working. Importing this name from ``openhands.sdk.settings`` - (or ``openhands.sdk``) emits a :class:`DeprecationWarning` via the - module-level ``__getattr__`` — no construction-time overhead. - - Use :class:`OpenHandsAgentSettings` for all new code. - - Scheduled for removal in v1.24.0. + The public import aliases (``from openhands.sdk import LLMAgentSettings`` and + ``from openhands.sdk.settings import LLMAgentSettings``) were removed in + v1.24.0 — use :class:`OpenHandsAgentSettings` for all new code. + + The class itself is retained (reachable at + ``openhands.sdk.settings.model.LLMAgentSettings``) because it remains a + member of the settings discriminated union: it keeps ``agent_kind='llm'`` so + persisted legacy payloads still deserialize and the API-breakage checker + sees no field-value change versus the published release. """ # Keep agent_kind as Literal["llm"] so the API-breakage checker sees no diff --git a/openhands-sdk/openhands/sdk/tool/registry.py b/openhands-sdk/openhands/sdk/tool/registry.py index b7c623a42e..588427a742 100644 --- a/openhands-sdk/openhands/sdk/tool/registry.py +++ b/openhands-sdk/openhands/sdk/tool/registry.py @@ -6,7 +6,6 @@ from openhands.sdk.logger import get_logger from openhands.sdk.tool.spec import Tool from openhands.sdk.tool.tool import ToolDefinition -from openhands.sdk.utils.deprecation import warn_deprecated if TYPE_CHECKING: @@ -54,32 +53,6 @@ def _resolve( return _resolve -def _resolver_from_callable( - name: str, factory: Callable[..., Sequence[ToolDefinition]] -) -> Resolver: - def _resolve( - params: dict[str, Any], conv_state: "ConversationState" - ) -> Sequence[ToolDefinition]: - try: - # Try to call with conv_state parameter first - created = factory(conv_state=conv_state, **params) - except TypeError as exc: - raise TypeError( - f"Unable to resolve tool '{name}': factory could not be called with " - f"params {params}." - ) from exc - if not isinstance(created, Sequence) or not all( - isinstance(t, ToolDefinition) for t in created - ): - raise TypeError( - f"Factory '{name}' must return Sequence[ToolDefinition], " - f"got {type(created)}" - ) - return created - - return _resolve - - def _is_abstract_method(cls: type, name: str) -> bool: try: attr = inspect.getattr_static(cls, name) @@ -127,13 +100,6 @@ def _usability_from_subclass(cls: type[ToolDefinition]) -> UsabilityChecker: return lambda: cls.is_usable() -def _usability_from_callable( - _factory: Callable[..., Sequence[ToolDefinition]], -) -> UsabilityChecker: - # Callable factories are deprecated and have no usability hook. - return lambda: True - - def _check_tool_usable(name: str, checker: UsabilityChecker) -> bool: try: return checker() @@ -146,9 +112,7 @@ def _check_tool_usable(name: str, checker: UsabilityChecker) -> bool: def register_tool( name: str, - factory: ToolDefinition - | type[ToolDefinition] - | Callable[..., Sequence[ToolDefinition]], + factory: ToolDefinition | type[ToolDefinition], ) -> None: if not isinstance(name, str) or not name.strip(): raise ValueError("ToolDefinition name must be a non-empty string") @@ -159,32 +123,16 @@ def register_tool( elif isinstance(factory, type) and issubclass(factory, ToolDefinition): resolver = _resolver_from_subclass(name, factory) usability_checker = _usability_from_subclass(factory) - elif callable(factory): - warn_deprecated( - "register_tool(callable_factory)", - deprecated_in="1.19.1", - removed_in="1.24.0", - details=( - "Register a ToolDefinition subclass with create(...) or a " - "ToolDefinition instance instead." - ), - stacklevel=2, - ) - resolver = _resolver_from_callable(name, factory) - usability_checker = _usability_from_callable(factory) else: raise TypeError( "register_tool(...) only accepts: (1) a ToolDefinition instance with " - ".executor, (2) a ToolDefinition subclass with .create(**params), or " - "(3) a callable factory returning a Sequence[ToolDefinition]" + ".executor, or (2) a ToolDefinition subclass with .create(**params)" ) # Track the module qualname for this tool module_qualname = None if isinstance(factory, type): module_qualname = factory.__module__ - elif callable(factory): - module_qualname = getattr(factory, "__module__", None) elif isinstance(factory, ToolDefinition): module_qualname = factory.__class__.__module__ diff --git a/openhands-sdk/pyproject.toml b/openhands-sdk/pyproject.toml index 413482d913..ab34666abd 100644 --- a/openhands-sdk/pyproject.toml +++ b/openhands-sdk/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "openhands-sdk" -version = "1.23.1" +version = "1.24.0" description = "OpenHands SDK - Core functionality for building AI agents" requires-python = ">=3.12" diff --git a/openhands-tools/pyproject.toml b/openhands-tools/pyproject.toml index d98bfad756..c5fa9fe1cd 100644 --- a/openhands-tools/pyproject.toml +++ b/openhands-tools/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "openhands-tools" -version = "1.23.1" +version = "1.24.0" description = "OpenHands Tools - Runtime tools for AI agents" requires-python = ">=3.12" diff --git a/openhands-workspace/pyproject.toml b/openhands-workspace/pyproject.toml index 94350ac3bf..f2ab7fc924 100644 --- a/openhands-workspace/pyproject.toml +++ b/openhands-workspace/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "openhands-workspace" -version = "1.23.1" +version = "1.24.0" description = "OpenHands Workspace - Docker and container-based workspace implementations" requires-python = ">=3.12" diff --git a/tests/cross/test_registry_qualnames.py b/tests/cross/test_registry_qualnames.py index 75992a26b4..b06dd8cc15 100644 --- a/tests/cross/test_registry_qualnames.py +++ b/tests/cross/test_registry_qualnames.py @@ -1,8 +1,5 @@ """Tests for tool registry module qualname tracking.""" -import pytest -from deprecation import DeprecatedWarning - from openhands.sdk.tool.registry import ( get_tool_module_qualnames, list_registered_tools, @@ -25,24 +22,6 @@ def test_get_tool_module_qualnames_with_class(): assert qualnames["test_glob_class"] == "openhands.tools.glob.definition" -def test_get_tool_module_qualnames_with_callable(): - """Test that module qualnames are tracked when registering a callable.""" - - def test_factory(conv_state): - return [] - - # Register the callable - with pytest.warns(DeprecatedWarning, match=r"register_tool\(callable_factory\)"): - register_tool("test_callable", test_factory) - - # Get the module qualnames - qualnames = get_tool_module_qualnames() - - # Verify the tool is tracked with its module - assert "test_callable" in qualnames - assert "test_registry_qualnames" in qualnames["test_callable"] - - def test_get_tool_module_qualnames_after_import(): """Test that importing a tool module registers it with qualname.""" # Import glob tool module to trigger auto-registration diff --git a/tests/sdk/agent/test_agent_tool_init.py b/tests/sdk/agent/test_agent_tool_init.py index 2539610551..28745d07f3 100644 --- a/tests/sdk/agent/test_agent_tool_init.py +++ b/tests/sdk/agent/test_agent_tool_init.py @@ -48,13 +48,9 @@ def create(cls, conv_state=None, **params) -> Sequence["_UpperTool"]: ] -def _make_tool(conv_state=None, **kwargs) -> Sequence[ToolDefinition]: - return _UpperTool.create(conv_state, **kwargs) - - def test_agent_initializes_tools_from_toolspec_locally(monkeypatch): # Register a simple local tool via registry - register_tool("upper", _make_tool) + register_tool("upper", _UpperTool) llm = LLM(model="test-model", usage_id="test-llm") agent = Agent(llm=llm, tools=[Tool(name="upper")]) @@ -161,13 +157,9 @@ def create(cls, conv_state=None, **params) -> Sequence["_CustomFinishTool"]: ] -def _make_custom_finish_tool(conv_state=None, **kwargs) -> Sequence[ToolDefinition]: - return _CustomFinishTool.create(conv_state, **kwargs) - - def test_agent_replace_finish_with_custom_tool(): """Test that the finish tool can be replaced with a custom implementation.""" - register_tool("custom_finish", _make_custom_finish_tool) + register_tool("custom_finish", _CustomFinishTool) llm = LLM(model="test-model", usage_id="test-llm") agent = Agent( diff --git a/tests/sdk/agent/test_message_while_finishing.py b/tests/sdk/agent/test_message_while_finishing.py index 45fa9c80bf..4872d3727b 100644 --- a/tests/sdk/agent/test_message_while_finishing.py +++ b/tests/sdk/agent/test_message_while_finishing.py @@ -146,13 +146,8 @@ def create(cls, conv_state=None, **params) -> Sequence["SleepTool"]: ] -def _make_sleep_tool(conv_state=None, **kwargs) -> Sequence[ToolDefinition]: - """Create sleep tool for testing.""" - return SleepTool.create(conv_state, **kwargs) - - # Register the tool -register_tool("SleepTool", _make_sleep_tool) +register_tool("SleepTool", SleepTool) class TestMessageWhileFinishing: diff --git a/tests/sdk/conversation/local/test_agent_status_transition.py b/tests/sdk/conversation/local/test_agent_status_transition.py index c69818322c..dac4db88ad 100644 --- a/tests/sdk/conversation/local/test_agent_status_transition.py +++ b/tests/sdk/conversation/local/test_agent_status_transition.py @@ -98,12 +98,10 @@ def test_execution_status_transitions_to_running_from_idle(): """Test that agent status transitions to RUNNING when run() is called from IDLE.""" status_during_execution: list[ConversationExecutionStatus] = [] - def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - return StatusTransitionTestTool.create( - executor=StatusCheckingExecutor(status_during_execution) - ) - - register_tool("test_tool", _make_tool) + test_tool = StatusTransitionTestTool.create( + executor=StatusCheckingExecutor(status_during_execution) + )[0] + register_tool("test_tool", test_tool) # Use TestLLM with a scripted response llm = TestLLM.from_messages( @@ -153,10 +151,8 @@ def __call__( status_during_execution.append(conversation.state.execution_status) return StatusTransitionMockObservation(result=f"Executed: {action.command}") - def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - return StatusTransitionTestTool.create(executor=SignalingExecutor()) - - register_tool("test_tool", _make_tool) + test_tool = StatusTransitionTestTool.create(executor=SignalingExecutor())[0] + register_tool("test_tool", test_tool) # Use TestLLM with scripted responses: first a tool call, then completion llm = TestLLM.from_messages( @@ -256,10 +252,8 @@ def test_execution_status_transitions_from_waiting_for_confirmation(): """Test WAITING_FOR_CONFIRMATION -> RUNNING transition when run() is called.""" from openhands.sdk.security.confirmation_policy import AlwaysConfirm - def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - return StatusTransitionTestTool.create(executor=StatusCheckingExecutor([])) - - register_tool("test_tool", _make_tool) + test_tool = StatusTransitionTestTool.create(executor=StatusCheckingExecutor([]))[0] + register_tool("test_tool", test_tool) # Use TestLLM with scripted responses: first a tool call, then completion llm = TestLLM.from_messages( @@ -422,12 +416,10 @@ def test_execution_status_error_on_max_iterations(): status_during_execution: list[ConversationExecutionStatus] = [] events_received: list = [] - def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - return StatusTransitionTestTool.create( - executor=StatusCheckingExecutor(status_during_execution) - ) - - register_tool("test_tool", _make_tool) + test_tool = StatusTransitionTestTool.create( + executor=StatusCheckingExecutor(status_during_execution) + )[0] + register_tool("test_tool", test_tool) # Create a tool call message that will be returned repeatedly tool_call_message = Message( @@ -487,10 +479,8 @@ def test_execution_status_finished_on_final_iteration(): events_received: list = [] - def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - return StatusTransitionTestTool.create(executor=StatusCheckingExecutor([])) - - register_tool("test_tool", _make_tool) + test_tool = StatusTransitionTestTool.create(executor=StatusCheckingExecutor([]))[0] + register_tool("test_tool", test_tool) # Two tool-call iterations followed by a text response on the 3rd (final) iteration. # A text-only assistant message causes the agent to set status to FINISHED. diff --git a/tests/sdk/conversation/local/test_confirmation_mode.py b/tests/sdk/conversation/local/test_confirmation_mode.py index 0a8533baca..4d84f56226 100644 --- a/tests/sdk/conversation/local/test_confirmation_mode.py +++ b/tests/sdk/conversation/local/test_confirmation_mode.py @@ -94,11 +94,6 @@ def create(cls, conv_state=None, **params) -> Sequence["ConfirmationTestTool"]: ] -def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - """Factory function for creating test tools.""" - return ConfirmationTestTool.create(conv_state, **params) - - class TestConfirmationMode: """Test suite for confirmation mode functionality.""" @@ -132,7 +127,7 @@ def setup_method(self): ) self.mock_llm.metrics.get_snapshot.return_value = mock_metrics_snapshot - register_tool("test_tool", _make_tool) + register_tool("test_tool", ConfirmationTestTool) self.agent: Agent = Agent( llm=self.llm, diff --git a/tests/sdk/conversation/local/test_conversation_pause_functionality.py b/tests/sdk/conversation/local/test_conversation_pause_functionality.py index 0890a4add7..02983ee87d 100644 --- a/tests/sdk/conversation/local/test_conversation_pause_functionality.py +++ b/tests/sdk/conversation/local/test_conversation_pause_functionality.py @@ -112,11 +112,6 @@ def create( ] -def _make_tool(conv_state=None, **params) -> Sequence[ToolDefinition]: - """Factory function for creating test tools.""" - return PauseFunctionalityTestTool.create(conv_state, **params) - - class BlockingTestTool( ToolDefinition[PauseFunctionalityMockAction, PauseFunctionalityMockObservation] ): @@ -150,7 +145,7 @@ def setup_method(self): model="gpt-4o-mini", api_key=SecretStr("test-key"), usage_id="test-llm" ) - register_tool("test_tool", _make_tool) + register_tool("test_tool", PauseFunctionalityTestTool) self.agent: Agent = Agent( llm=self.llm, @@ -369,12 +364,8 @@ def test_multiple_pause_calls_create_one_event(self): def test_pause_while_running_continuous_actions(self, mock_completion): step_entered = threading.Event() - def _make_blocking_tool(conv_state=None, **kwargs) -> Sequence[ToolDefinition]: - return BlockingTestTool.create( - conv_state, step_entered=step_entered, **kwargs - ) - - register_tool("test_tool", _make_blocking_tool) + blocking_tool = BlockingTestTool.create(step_entered=step_entered)[0] + register_tool("test_tool", blocking_tool) agent = Agent( llm=self.llm, tools=[Tool(name="test_tool")], diff --git a/tests/sdk/test_settings.py b/tests/sdk/test_settings.py index ff27ca5a1b..2f8222d72f 100644 --- a/tests/sdk/test_settings.py +++ b/tests/sdk/test_settings.py @@ -1,5 +1,4 @@ import json -import warnings import pytest from fastmcp.mcp_config import MCPConfig @@ -666,23 +665,25 @@ def test_acp_create_agent_passes_resolved_env_and_agent_context() -> None: assert agent.agent_context == context -def test_llm_agent_settings_deprecated_alias_emits_warning() -> None: - """Importing ``LLMAgentSettings`` emits DeprecationWarning at import time.""" +def test_llm_agent_settings_public_alias_removed() -> None: + """The deprecated ``LLMAgentSettings`` public import aliases were removed in + v1.24.0; the class itself is retained (internal-only) for the union.""" + import openhands.sdk as _sdk_mod import openhands.sdk.settings as _settings_mod - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - cls = getattr(_settings_mod, "LLMAgentSettings") + with pytest.raises(AttributeError): + getattr(_settings_mod, "LLMAgentSettings") + with pytest.raises(AttributeError): + getattr(_sdk_mod, "LLMAgentSettings") - assert any("LLMAgentSettings" in str(w.message) for w in caught), ( - f"expected deprecation warning, got: {[str(w.message) for w in caught]}" - ) - assert issubclass(cls, OpenHandsAgentSettings) - # Construction itself does not emit a second warning. - settings = cls(llm=LLM(model="test-model")) + # The class is still reachable at its canonical internal location and keeps + # agent_kind="llm" so the discriminated union deserializes legacy payloads + # and the API-breakage checker sees no field-value change. + from openhands.sdk.settings.model import LLMAgentSettings + + assert issubclass(LLMAgentSettings, OpenHandsAgentSettings) + settings = LLMAgentSettings(llm=LLM(model="test-model")) assert isinstance(settings, OpenHandsAgentSettings) - # LLMAgentSettings keeps its own agent_kind="llm" so the API-breakage - # checker sees no field-value change vs the published PyPI release. assert settings.agent_kind == "llm" assert settings.llm.model == "test-model" diff --git a/tests/sdk/tool/test_registry.py b/tests/sdk/tool/test_registry.py index 2b691313aa..07fcb5a1cf 100644 --- a/tests/sdk/tool/test_registry.py +++ b/tests/sdk/tool/test_registry.py @@ -2,7 +2,6 @@ from unittest.mock import MagicMock import pytest -from deprecation import DeprecatedWarning from openhands.sdk import register_tool from openhands.sdk.conversation.state import ConversationState @@ -94,15 +93,12 @@ def _hello_tool_factory(conv_state=None, **params) -> list[ToolDefinition]: return list(_SimpleHelloTool.create(conv_state, **params)) -def test_register_and_resolve_callable_factory(): - with pytest.warns(DeprecatedWarning, match=r"register_tool\(callable_factory\)"): - register_tool("say_hello", _hello_tool_factory) - - tools = resolve_tool(Tool(name="say_hello"), _create_mock_conv_state()) - assert len(tools) == 1 - assert isinstance(tools[0], ToolDefinition) - assert tools[0].name == "__simple_hello" - assert "say_hello" in list_usable_tools() +def test_register_tool_rejects_callable_factory(): + # Callable factories were removed in v1.24.0; register_tool now accepts only + # a ToolDefinition instance or a ToolDefinition subclass. Passing a callable + # is now both a static type error and a runtime TypeError. + with pytest.raises(TypeError, match=r"only accepts"): + register_tool("say_hello", _hello_tool_factory) # type: ignore[arg-type] def test_register_tool_type_respects_is_usable(): diff --git a/tests/tools/terminal/test_conversation_cleanup.py b/tests/tools/terminal/test_conversation_cleanup.py index 6723d19134..836a134803 100644 --- a/tests/tools/terminal/test_conversation_cleanup.py +++ b/tests/tools/terminal/test_conversation_cleanup.py @@ -6,13 +6,54 @@ """ import tempfile +from collections.abc import Sequence +from typing import TYPE_CHECKING, ClassVar, Literal from unittest.mock import Mock from openhands.sdk import Agent, Conversation -from openhands.sdk.tool import Tool, register_tool +from openhands.sdk.tool import Tool, ToolExecutor, register_tool from openhands.tools.terminal import TerminalExecutor, TerminalTool +if TYPE_CHECKING: + from openhands.sdk.conversation.state import ConversationState + + +class _InjectedExecutorTerminalTool(TerminalTool): + """TerminalTool variant that injects a test-provided executor at resolve time. + + ``TerminalTool.create`` needs ``conv_state`` (for the working dir), so this + subclass builds the real tool from ``conv_state`` and then swaps in the + executor captured on the class attribute. Each test sets ``injected_executor`` + before registering this subclass. + """ + + injected_executor: ClassVar[ToolExecutor | None] = None + + @classmethod + def create( + cls, + conv_state: "ConversationState", + username: str | None = None, + no_change_timeout_seconds: int | None = None, + terminal_type: Literal["tmux", "subprocess", "powershell"] | None = None, + shell_path: str | None = None, + executor: ToolExecutor | None = None, + ) -> Sequence[TerminalTool]: + tools = TerminalTool.create( + conv_state, + username=username, + no_change_timeout_seconds=no_change_timeout_seconds, + terminal_type=terminal_type, + shell_path=shell_path, + executor=executor, + ) + return [ + tool.model_copy(update={"executor": cls.injected_executor}) + for tool in tools + ] + + def test_conversation_close_calls_executor_close(mock_llm): """Test that Conversation.close() calls close() on all tool executors.""" with tempfile.TemporaryDirectory() as temp_dir: @@ -22,12 +63,8 @@ def test_conversation_close_calls_executor_close(mock_llm): ) terminal_executor.close = Mock() - def _make_tool(conv_state, **params): - tools = TerminalTool.create(conv_state) - tool = tools[0] - return [tool.model_copy(update={"executor": terminal_executor})] - - register_tool("test_terminal", _make_tool) + _InjectedExecutorTerminalTool.injected_executor = terminal_executor + register_tool("test_terminal", _InjectedExecutorTerminalTool) # Create agent and conversation agent = Agent( @@ -56,12 +93,8 @@ def test_conversation_close_calls_executor_close_without_delete(mock_llm): ) terminal_executor.close = Mock() - def _make_tool(conv_state, **params): - tools = TerminalTool.create(conv_state) - tool = tools[0] - return [tool.model_copy(update={"executor": terminal_executor})] - - register_tool("test_terminal", _make_tool) + _InjectedExecutorTerminalTool.injected_executor = terminal_executor + register_tool("test_terminal", _InjectedExecutorTerminalTool) agent = Agent( llm=mock_llm, @@ -85,12 +118,8 @@ def test_conversation_del_calls_close(mock_llm): ) terminal_executor.close = Mock() - def _make_tool(conv_state, **params): - tools = TerminalTool.create(conv_state) - tool = tools[0] - return [tool.model_copy(update={"executor": terminal_executor})] - - register_tool("test_terminal", _make_tool) + _InjectedExecutorTerminalTool.injected_executor = terminal_executor + register_tool("test_terminal", _InjectedExecutorTerminalTool) # Create agent and conversation agent = Agent( @@ -123,12 +152,8 @@ def test_conversation_close_handles_executor_exceptions(mock_llm): ) terminal_executor.close = Mock(side_effect=Exception("Test exception")) - def _make_tool(conv_state, **params): - tools = TerminalTool.create(conv_state) - tool = tools[0] - return [tool.model_copy(update={"executor": terminal_executor})] - - register_tool("test_terminal", _make_tool) + _InjectedExecutorTerminalTool.injected_executor = terminal_executor + register_tool("test_terminal", _InjectedExecutorTerminalTool) # Create agent and conversation agent = Agent( @@ -148,12 +173,8 @@ def test_conversation_close_skips_none_executors(mock_llm): # Create a mock LLM to avoid actual API calls # Create a tool with no executor - register_tool( - "test_terminal", - lambda conv_state, **params: [ - TerminalTool.create(conv_state)[0].model_copy(update={"executor": None}) - ], - ) + _InjectedExecutorTerminalTool.injected_executor = None + register_tool("test_terminal", _InjectedExecutorTerminalTool) # Create agent and conversation agent = Agent( diff --git a/uv.lock b/uv.lock index ecae9b34ad..3104f92e8f 100644 --- a/uv.lock +++ b/uv.lock @@ -8,7 +8,7 @@ resolution-markers = [ ] [options] -exclude-newer = "2026-05-12T18:17:37.961907255Z" +exclude-newer = "0001-01-01T00:00:00Z" # This has no effect and is included for backwards compatibility when using relative exclude-newer values. exclude-newer-span = "P7D" [manifest] @@ -2454,7 +2454,7 @@ wheels = [ [[package]] name = "openhands-agent-server" -version = "1.23.1" +version = "1.24.0" source = { editable = "openhands-agent-server" } dependencies = [ { name = "aiosqlite" }, @@ -2485,7 +2485,7 @@ requires-dist = [ [[package]] name = "openhands-sdk" -version = "1.23.1" +version = "1.24.0" source = { editable = "openhands-sdk" } dependencies = [ { name = "agent-client-protocol" }, @@ -2537,7 +2537,7 @@ provides-extras = ["boto3"] [[package]] name = "openhands-tools" -version = "1.23.1" +version = "1.24.0" source = { editable = "openhands-tools" } dependencies = [ { name = "binaryornot" }, @@ -2568,7 +2568,7 @@ requires-dist = [ [[package]] name = "openhands-workspace" -version = "1.23.1" +version = "1.24.0" source = { editable = "openhands-workspace" } dependencies = [ { name = "openhands-agent-server" },