diff --git a/openhands-agent-server/openhands/agent_server/file_router.py b/openhands-agent-server/openhands/agent_server/file_router.py index fc93220d10..abbd4df815 100644 --- a/openhands-agent-server/openhands/agent_server/file_router.py +++ b/openhands-agent-server/openhands/agent_server/file_router.py @@ -30,7 +30,7 @@ @file_router.post("/upload/{path:path}") async def upload_file( path: Annotated[str, FastApiPath(alias="path", description="Absolute file path.")], - file: UploadFile = File(...), + file: Annotated[UploadFile, File(...)], ) -> Success: """Upload a file to the workspace.""" logger.info(f"Uploading file: {path}") diff --git a/openhands-sdk/openhands/sdk/agent/agent.py b/openhands-sdk/openhands/sdk/agent/agent.py index 66a9cbe5d1..4ce2b09518 100644 --- a/openhands-sdk/openhands/sdk/agent/agent.py +++ b/openhands-sdk/openhands/sdk/agent/agent.py @@ -309,9 +309,9 @@ def _get_action_event( tool_call: MessageToolCall, llm_response_id: str, on_event: ConversationCallbackType, - thought: list[TextContent] = [], + thought: list[TextContent] | None = None, reasoning_content: str | None = None, - thinking_blocks: list[ThinkingBlock | RedactedThinkingBlock] = [], + thinking_blocks: list[ThinkingBlock | RedactedThinkingBlock] | None = None, responses_reasoning_item: ReasoningItemModel | None = None, ) -> ActionEvent | None: """Converts a tool call into an ActionEvent, validating arguments. @@ -328,9 +328,9 @@ def _get_action_event( # Persist assistant function_call so next turn has matching call_id tc_event = ActionEvent( source="agent", - thought=thought, + thought=thought or [], reasoning_content=reasoning_content, - thinking_blocks=thinking_blocks, + thinking_blocks=thinking_blocks or [], responses_reasoning_item=responses_reasoning_item, tool_call=tool_call, tool_name=tool_call.name, @@ -380,9 +380,9 @@ def _get_action_event( # Persist assistant function_call so next turn has matching call_id tc_event = ActionEvent( source="agent", - thought=thought, + thought=thought or [], reasoning_content=reasoning_content, - thinking_blocks=thinking_blocks, + thinking_blocks=thinking_blocks or [], responses_reasoning_item=responses_reasoning_item, tool_call=tool_call, tool_name=tool_call.name, @@ -401,9 +401,9 @@ def _get_action_event( action_event = ActionEvent( action=action, - thought=thought, + thought=thought or [], reasoning_content=reasoning_content, - thinking_blocks=thinking_blocks, + thinking_blocks=thinking_blocks or [], responses_reasoning_item=responses_reasoning_item, tool_name=tool.name, tool_call_id=tool_call.id, diff --git a/pyproject.toml b/pyproject.toml index 65b42c0ca4..db1508c773 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,11 +43,25 @@ select = [ "UP", # pyupgrade "ARG", # flake8-unused-arguments ] +# Enforce rules that catch mutable defaults and related pitfalls +# - B006: mutable-argument-default +# - B008: function-call-in-default-argument +# - B039: mutable-contextvar-default +# - RUF012: mutable-class-default +extend-select = ["B006", "B008", "B039", "RUF012"] [tool.ruff.lint.per-file-ignores] # Test files often have unused arguments (fixtures, mocks, interface implementations) "tests/**/*.py" = ["ARG"] + +# Allowlist safe default calls for flake8-bugbear rules (e.g., FastAPI Depends) +[tool.ruff.lint.flake8-bugbear] +extend-immutable-calls = [ + "fastapi.Depends", + "fastapi.params.Depends", +] + [tool.ruff.lint.isort] known-first-party = ["openhands"] combine-as-imports = true diff --git a/tests/sdk/agent/test_agent_llms_are_discoverable.py b/tests/sdk/agent/test_agent_llms_are_discoverable.py index 687080244c..b654d27414 100644 --- a/tests/sdk/agent/test_agent_llms_are_discoverable.py +++ b/tests/sdk/agent/test_agent_llms_are_discoverable.py @@ -1,3 +1,5 @@ +from pydantic import Field + from openhands.sdk import LLM, Agent, LLMSummarizingCondenser from openhands.sdk.llm.router import MultimodalRouter @@ -36,7 +38,7 @@ def test_automatic_llm_discovery_for_multiple_llms(): def test_automatic_llm_discovery_for_custom_agent_with_duplicates(): class CustomAgent(Agent): - model_routers: list[LLM] = [] + model_routers: list[LLM] = Field(default_factory=list) llm_service_id = "main-agent" router_service_id = "secondary_llm" @@ -94,8 +96,8 @@ def test_automatic_llm_discovery_with_multimodal_router(): def test_automatic_llm_discovery_with_llm_as_base_class(): class NewLLM(LLM): - list_llms: list[LLM] = [] - dict_llms: dict[str, LLM] = {} + list_llms: list[LLM] = Field(default_factory=list) + dict_llms: dict[str, LLM] = Field(default_factory=dict) raw_llm: LLM | None = None list_llm = LLM(model="list-model", usage_id="list-model") diff --git a/tests/sdk/conversation/test_visualizer.py b/tests/sdk/conversation/test_visualizer.py index 07355914f1..ce2995c0ed 100644 --- a/tests/sdk/conversation/test_visualizer.py +++ b/tests/sdk/conversation/test_visualizer.py @@ -2,6 +2,7 @@ import json +from pydantic import Field from rich.text import Text from openhands.sdk.conversation.visualizer import ( @@ -34,7 +35,7 @@ class VisualizerMockAction(Action): class VisualizerCustomAction(Action): """Custom action with overridden visualize method.""" - task_list: list[dict] = [] + task_list: list[dict] = Field(default_factory=list) @property def visualize(self) -> Text: diff --git a/tests/sdk/security/test_security_analyzer.py b/tests/sdk/security/test_security_analyzer.py index ffae8f4252..61c272816b 100644 --- a/tests/sdk/security/test_security_analyzer.py +++ b/tests/sdk/security/test_security_analyzer.py @@ -1,5 +1,7 @@ """Tests for the SecurityAnalyzer class.""" +from pydantic import Field + from openhands.sdk.event import ActionEvent, PauseEvent from openhands.sdk.llm import MessageToolCall, TextContent from openhands.sdk.security.analyzer import SecurityAnalyzerBase @@ -19,9 +21,9 @@ class SecurityAnalyzer(SecurityAnalyzerBase): """ risk_return_value: SecurityRisk = SecurityRisk.LOW - security_risk_calls: list[ActionEvent] = [] - handle_api_request_calls: list[dict] = [] - close_calls: list[bool] = [] + security_risk_calls: list[ActionEvent] = Field(default_factory=list) + handle_api_request_calls: list[dict] = Field(default_factory=list) + close_calls: list[bool] = Field(default_factory=list) def security_risk(self, action: ActionEvent) -> SecurityRisk: """Return configurable risk level for testing.""" @@ -131,11 +133,13 @@ def test_analyze_pending_actions_mixed_risks() -> None: class VariableRiskAnalyzer(SecurityAnalyzer): call_count: int = 0 - risks: list[SecurityRisk] = [ - SecurityRisk.LOW, - SecurityRisk.HIGH, - SecurityRisk.MEDIUM, - ] + risks: list[SecurityRisk] = Field( + default_factory=lambda: [ + SecurityRisk.LOW, + SecurityRisk.HIGH, + SecurityRisk.MEDIUM, + ] + ) def security_risk(self, action: ActionEvent) -> SecurityRisk: risk = self.risks[self.call_count % len(self.risks)]