From 4377d7da0d1475f390096ed6efb0650eb6f734a3 Mon Sep 17 00:00:00 2001 From: ogzerber Date: Thu, 16 Apr 2026 18:41:12 +0530 Subject: [PATCH] fix(honcho): improve conclude descriptions and add exactly-one validation Improve honcho_conclude tool descriptions to explicitly tell the model not to send both params together. Add runtime validation that rejects calls with both or neither of conclusion/delete_id. Add schema regression test and both-params rejection test. Consolidates #10847 by @ygd58, #10864 by @cola-runner, #10870 by @vominh1919, and #10952 by @ogzerber. The anyOf removal itself was already merged; this adds the runtime validation and tests those PRs contributed. Co-authored-by: ygd58 Co-authored-by: cola-runner Co-authored-by: vominh1919 --- plugins/memory/honcho/__init__.py | 16 ++++++++++------ tests/honcho_plugin/test_session.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 6f7e52f0b..fcd64b881 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -160,11 +160,11 @@ CONCLUDE_SCHEMA = { "properties": { "conclusion": { "type": "string", - "description": "A factual statement to persist. Required when not using delete_id.", + "description": "A factual statement to persist. Provide this when creating a conclusion. Do not send it together with delete_id.", }, "delete_id": { "type": "string", - "description": "Conclusion ID to delete (for PII removal). Required when not using conclusion.", + "description": "Conclusion ID to delete for PII removal. Provide this when deleting a conclusion. Do not send it together with conclusion.", }, "peer": { "type": "string", @@ -1009,15 +1009,19 @@ class HonchoMemoryProvider(MemoryProvider): elif tool_name == "honcho_conclude": delete_id = args.get("delete_id") + conclusion = args.get("conclusion", "") peer = args.get("peer", "user") - if delete_id: + + has_delete_id = bool(delete_id) + has_conclusion = bool(conclusion) + if has_delete_id == has_conclusion: + return tool_error("Exactly one of conclusion or delete_id must be provided.") + + if has_delete_id: ok = self._manager.delete_conclusion(self._session_key, delete_id, peer=peer) if ok: return json.dumps({"result": f"Conclusion {delete_id} deleted."}) return tool_error(f"Failed to delete conclusion {delete_id}.") - conclusion = args.get("conclusion", "") - if not conclusion: - return tool_error("Missing required parameter: conclusion or delete_id") ok = self._manager.create_conclusion(self._session_key, conclusion, peer=peer) if ok: return json.dumps({"result": f"Conclusion saved for {peer}: {conclusion}"}) diff --git a/tests/honcho_plugin/test_session.py b/tests/honcho_plugin/test_session.py index 69c024efe..404f82120 100644 --- a/tests/honcho_plugin/test_session.py +++ b/tests/honcho_plugin/test_session.py @@ -366,6 +366,17 @@ class TestPeerLookupHelpers: class TestConcludeToolDispatch: + def test_conclude_schema_has_no_anyof(self): + """anyOf/oneOf/allOf breaks Anthropic and Fireworks APIs — schema must be plain object.""" + from plugins.memory.honcho import CONCLUDE_SCHEMA + params = CONCLUDE_SCHEMA["parameters"] + assert params["type"] == "object" + assert "conclusion" in params["properties"] + assert "delete_id" in params["properties"] + assert "anyOf" not in params + assert "oneOf" not in params + assert "allOf" not in params + def test_honcho_conclude_defaults_to_user_peer(self): provider = HonchoMemoryProvider() provider._session_initialized = True @@ -470,7 +481,23 @@ class TestConcludeToolDispatch: result = provider.handle_tool_call("honcho_conclude", {}) parsed = json.loads(result) - assert "error" in parsed or "Missing required" in parsed.get("result", "") + assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} + provider._manager.create_conclusion.assert_not_called() + provider._manager.delete_conclusion.assert_not_called() + + def test_honcho_conclude_rejects_both_params_at_once(self): + """Sending both conclusion and delete_id should be rejected.""" + import json + provider = HonchoMemoryProvider() + provider._session_initialized = True + provider._session_key = "telegram:123" + provider._manager = MagicMock() + result = provider.handle_tool_call( + "honcho_conclude", + {"conclusion": "User prefers dark mode", "delete_id": "conc-123"}, + ) + parsed = json.loads(result) + assert parsed == {"error": "Exactly one of conclusion or delete_id must be provided."} provider._manager.create_conclusion.assert_not_called() provider._manager.delete_conclusion.assert_not_called()