Fix agent crash on native tool calls and empty tool output

agent/core/conversation.py

@@ -49,15 +49,21 @@ def add_user(self, content: str):
         self.messages.append(Message(role="user", content=content))
 
     def add_assistant(self, content: str, tool_calls: Optional[list] = None):
-        if not content or not content.strip():
-            raise ValueError("Assistant message content cannot be None or empty")
+        # An empty assistant turn is a valid shape in the chat API, with or
+        # without tool calls: a native tool call carries no prose, and a
+        # truncated or silent final response legitimately has none either.
+        # Only None indicates a genuine caller bug.
+        if content is None:
+            raise ValueError("Assistant message content cannot be None")
         self.messages.append(
             Message(role="assistant", content=content, tool_calls=tool_calls)
         )
 
     def add_tool_result(self, tool_call_id: str, name: str, content: str):
-        if not content or not content.strip():
-            raise ValueError("Tool result content cannot be None or empty")
+        # A successful tool can legitimately produce no output (grep with no
+        # match, a write that prints nothing), so empty is allowed here.
+        if content is None:
+            raise ValueError("Tool result content cannot be None")
         self.messages.append(
             Message(role="tool", content=content, tool_call_id=tool_call_id, name=name)
         )

agent/core/engine.py

@@ -91,8 +91,10 @@ def process_message(self, user_input: str) -> str:
                 print(error(error_msg))
                 return error_msg
 
-            content = response.get("message", {}).get("content", "")
-            tool_calls_from_api = response.get("message", {}).get("tool_calls", [])
+            # `or ""` guards a present-but-null content key, which would
+            # otherwise reach add_assistant as None.
+            content = response.get("message", {}).get("content", "") or ""
+            tool_calls_from_api = response.get("message", {}).get("tool_calls", []) or []
 
             # Check for tool calls in the content (```tool blocks)
             tool_calls_from_content = self._extract_tool_calls(content)

tests/test_engine.py

@@ -182,3 +182,70 @@ def test_git_write_not_auto_approved(self):
 
 if __name__ == "__main__":
     unittest.main()
+
+
+class TestProcessMessageEmptyResponses(unittest.TestCase):
+    """End-to-end cover for the paths #27 made crashable.
+
+    These drive process_message itself rather than Conversation in
+    isolation - a unit test on Conversation alone is exactly what let #27
+    through CI.
+    """
+
+    def _engine(self):
+        config = Config()
+        config.web.enabled = False
+        config.agent.max_turns = 3
+        return AgentEngine(config)
+
+    def test_empty_final_response_does_not_crash(self):
+        engine = self._engine()
+        with patch.object(
+            engine, "_call_llm", return_value={"message": {"content": ""}}
+        ):
+            result = engine.process_message("hello")
+
+        self.assertEqual(result, "")
+        self.assertEqual(engine.conversation.messages[-1].role, "assistant")
+
+    def test_whitespace_only_final_response_does_not_crash(self):
+        engine = self._engine()
+        with patch.object(
+            engine, "_call_llm", return_value={"message": {"content": "   "}}
+        ):
+            result = engine.process_message("hello")
+
+        self.assertEqual(result.strip(), "")
+
+    def test_null_content_does_not_crash(self):
+        """A present-but-null content key must not reach add_assistant as None."""
+        engine = self._engine()
+        with patch.object(
+            engine, "_call_llm", return_value={"message": {"content": None}}
+        ):
+            result = engine.process_message("hello")
+
+        self.assertEqual(result, "")
+
+    def test_native_tool_call_with_empty_content_does_not_crash(self):
+        engine = self._engine()
+        responses = [
+            {
+                "message": {
+                    "content": "",
+                    "tool_calls": [
+                        {
+                            "id": "call_1",
+                            "function": {"name": "list_directory", "arguments": {}},
+                        }
+                    ],
+                }
+            },
+            {"message": {"content": "All done."}},
+        ]
+        with patch.object(engine, "_call_llm", side_effect=responses):
+            result = engine.process_message("list the files")
+
+        self.assertEqual(result, "All done.")
+        roles = [m.role for m in engine.conversation.messages]
+        self.assertIn("tool", roles)

tests/test_tools.py

@@ -291,6 +291,46 @@ def test_save_load(self):
         finally:
             shutil.rmtree(tmpdir)
 
+    def test_assistant_message_may_be_empty_when_tool_calls_present(self):
+        """Native tool calls arrive with empty content; that is a valid message."""
+        conv = Conversation()
+        conv.add_system("System")
+        conv.add_assistant("", [{"id": "1", "function": {"name": "read_file"}}])
+
+        messages = conv.get_messages()
+        self.assertEqual(messages[-1]["role"], "assistant")
+        self.assertEqual(messages[-1]["tool_calls"][0]["id"], "1")
+
+    def test_assistant_message_may_be_empty_without_tool_calls(self):
+        """A truncated or silent final response is still a valid assistant turn."""
+        conv = Conversation()
+        conv.add_system("System")
+        conv.add_assistant("")
+
+        messages = conv.get_messages()
+        self.assertEqual(messages[-1]["role"], "assistant")
+        self.assertEqual(messages[-1]["content"], "")
+
+    def test_assistant_message_rejects_none(self):
+        conv = Conversation()
+        with self.assertRaises(ValueError):
+            conv.add_assistant(None)
+
+    def test_tool_result_may_be_empty(self):
+        """A successful tool can legitimately produce no output (e.g. grep, no match)."""
+        conv = Conversation()
+        conv.add_system("System")
+        conv.add_tool_result("1", "grep", "")
+
+        messages = conv.get_messages()
+        self.assertEqual(messages[-1]["role"], "tool")
+        self.assertEqual(messages[-1]["content"], "")
+
+    def test_tool_result_rejects_none(self):
+        conv = Conversation()
+        with self.assertRaises(ValueError):
+            conv.add_tool_result("1", "grep", None)
+
 
 class TestSafety(unittest.TestCase):
     def test_sensitive_files(self):