diff --git a/src/rai_core/rai/messages/artifacts.py b/src/rai_core/rai/messages/artifacts.py index 870174344..d8711dfce 100644 --- a/src/rai_core/rai/messages/artifacts.py +++ b/src/rai_core/rai/messages/artifacts.py @@ -26,6 +26,10 @@ def store_artifacts( tool_call_id: str, artifacts: List[Any], db_path="artifact_database.pkl" ): # TODO(boczekbartek): refactor + if not isinstance(tool_call_id, str) or not tool_call_id.strip(): + raise ValueError( + f"tool_call_id must be a non-empty string, got {tool_call_id!r}" + ) path = Path(db_path) if not path.is_file(): artifact_database: dict = {} @@ -45,6 +49,10 @@ def get_stored_artifacts( tool_call_id: str, db_path="artifact_database.pkl" ) -> List[Any]: # TODO(boczekbartek): refactor + if not isinstance(tool_call_id, str) or not tool_call_id.strip(): + raise ValueError( + f"tool_call_id must be a non-empty string, got {tool_call_id!r}" + ) db_path = Path(db_path) if not db_path.is_file(): return [] diff --git a/tests/communication/__init__.py b/tests/communication/__init__.py index e69de29bb..97ceef6f0 100644 --- a/tests/communication/__init__.py +++ b/tests/communication/__init__.py @@ -0,0 +1,13 @@ +# Copyright (C) 2025 Robotec.AI +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/tests/communication/ros2/test_ros2_async.py b/tests/communication/ros2/test_ros2_async.py index 25ec290c3..bd50055c9 100644 --- a/tests/communication/ros2/test_ros2_async.py +++ b/tests/communication/ros2/test_ros2_async.py @@ -125,10 +125,10 @@ def cancel_future(): # Edge case timeout tests def test_get_future_result_zero_timeout(): - """Test with zero timeout.""" + """Test that a zero timeout is rejected as non-positive.""" future = Future() - result = get_future_result(future, timeout_sec=0.0) - assert result is None + with pytest.raises(ValueError, match="timeout_sec must be positive"): + get_future_result(future, timeout_sec=0.0) def test_get_future_result_very_short_timeout(): diff --git a/tests/messages/test_artifacts_db_path.py b/tests/messages/test_artifacts_db_path.py index bb11eb4f0..c80d150fa 100644 --- a/tests/messages/test_artifacts_db_path.py +++ b/tests/messages/test_artifacts_db_path.py @@ -32,3 +32,14 @@ def test_store_artifacts_creates_file_at_path(tmp_path: Path): db.parent.mkdir(parents=True) store_artifacts("x", [1], db_path=str(db)) assert get_stored_artifacts("x", db_path=str(db)) == [1] + + +def test_store_artifacts_rejects_empty_tool_call_id(tmp_path: Path): + import pytest + + db = tmp_path / "db.pkl" + for bad in ("", " ", None, 123): + with pytest.raises(ValueError, match="tool_call_id must be a non-empty string"): + store_artifacts(bad, ["a"], db_path=str(db)) # type: ignore[arg-type] + with pytest.raises(ValueError, match="tool_call_id must be a non-empty string"): + get_stored_artifacts(bad, db_path=str(db)) # type: ignore[arg-type]