diff --git a/CHANGELOG.md b/CHANGELOG.md index 11d34031..c74b0dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Preserves graceful fallback behavior: `record_decision()` remains optional and `upsert_memory()` continues without propagating the exception - Added regression coverage in `tests/integrations/agno/test_shared_context.py` for both `store()` and `record_decision()` warning paths +- **`AgnoDecisionKit`/`AgnoKGToolkit` silently swallowed Agno tool registration failures** (#780, #818) by @Sameer6305 and @KaifAhmad1 + - Removed the `try/except: pass` wrapped around `self.register(fn)` in both toolkits' `__init__`; when Agno is installed, a registration failure now propagates immediately instead of leaving the toolkit half-registered with no signal to the caller + - Graceful degradation when Agno isn't installed (`AGNO_AVAILABLE=False`) is unchanged — `_tools` is still populated so callers can introspect available tools without the package + - Fixed a related duplicate-entry bug: `self._tools` was appended to unconditionally *before* `register()` ran, which could double-count a tool when Agno's own `Toolkit.register()` also tracks it in `self._tools` + - This is a behavior change for callers that construct these toolkits expecting instantiation to always succeed — audited: no in-repo call site relies on the old silent-failure behavior + - Expanded `tests/integrations/agno/test_decision_kit.py` and `test_kg_toolkit.py` with coverage for registration invocation counts, failure propagation, graceful degradation, and no-duplicate-`_tools` assertions + - **MCP `handle_get_causal_chain` returned an empty-but-valid-looking response when both `CausalChainAnalyzer` and the graph fallback were unavailable** (#781, #817) by @Sameer6305 and @KaifAhmad1 - Returns an explicit `{"error": "Causal chain analysis is not supported on this graph backend", "chain": []}` instead of `{"chain": [], "count": 0, "direction": ...}`, letting clients distinguish "unsupported" from a legitimately empty chain - The fallback path now introspects `graph.get_causal_chain`'s signature to forward `direction`/`max_depth` (or a `depth` kwarg, or nothing, depending on what the backend accepts) instead of always calling with just `decision_id`, matching the primary analyzer path's behavior diff --git a/integrations/agno/decision_kit.py b/integrations/agno/decision_kit.py index bcb4e66e..cc64e9c6 100644 --- a/integrations/agno/decision_kit.py +++ b/integrations/agno/decision_kit.py @@ -123,12 +123,10 @@ class AgnoDecisionKit(_ToolkitBase): # type: ignore[misc] tools_to_register.append(self.check_policy) for fn in tools_to_register: - self._tools.append(fn) if AGNO_AVAILABLE: - try: - self.register(fn) - except Exception: - pass + self.register(fn) + if fn not in self._tools: + self._tools.append(fn) logger.info("AgnoDecisionKit initialised") diff --git a/integrations/agno/kg_toolkit.py b/integrations/agno/kg_toolkit.py index 75ee26ed..c36ea63f 100644 --- a/integrations/agno/kg_toolkit.py +++ b/integrations/agno/kg_toolkit.py @@ -122,12 +122,10 @@ class AgnoKGToolkit(_ToolkitBase): # type: ignore[misc] self.export_subgraph, ] for fn in tools_to_register: - self._tools.append(fn) if AGNO_AVAILABLE: - try: - self.register(fn) - except Exception: - pass + self.register(fn) + if fn not in self._tools: + self._tools.append(fn) logger.info("AgnoKGToolkit initialised (backend=%s)", graph_store_backend) diff --git a/tests/integrations/agno/test_decision_kit.py b/tests/integrations/agno/test_decision_kit.py index 8efd4830..3f13b6c0 100644 --- a/tests/integrations/agno/test_decision_kit.py +++ b/tests/integrations/agno/test_decision_kit.py @@ -8,7 +8,7 @@ import json import sys import types import unittest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch # --------------------------------------------------------------------------- @@ -75,7 +75,31 @@ class TestAgnoDecisionKitInit(unittest.TestCase): def test_tools_registered(self): kit = AgnoDecisionKit(context=_make_context()) # Tools should be registered (Toolkit.register was called) - self.assertTrue(len(kit._tools) >= 5) + self.assertEqual(len(kit._tools), 6) + self.assertEqual(len(kit._tools), len(set(kit._tools))) + + def test_registration_invoked(self): + with patch.object(AgnoDecisionKit, "register") as mock_register: + AgnoDecisionKit(context=_make_context()) + self.assertEqual(mock_register.call_count, 6) + + def test_registration_failure_propagates(self): + with patch.object(AgnoDecisionKit, "register", side_effect=RuntimeError("Registration failed")): + with self.assertRaises(RuntimeError): + AgnoDecisionKit(context=_make_context()) + + def test_graceful_degradation_when_agno_unavailable(self): + with patch("integrations.agno.decision_kit.AGNO_AVAILABLE", False): + with patch.object(AgnoDecisionKit, "register") as mock_register: + kit = AgnoDecisionKit(context=_make_context()) + mock_register.assert_not_called() + self.assertEqual(len(kit._tools), 6) + self.assertEqual(len(kit._tools), len(set(kit._tools))) + + def test_no_duplicate_tools(self): + kit = AgnoDecisionKit(context=_make_context()) + self.assertEqual(len(kit._tools), len(set(kit._tools))) + self.assertEqual(len(kit._tools), 6) def test_policy_tool_can_be_disabled(self): kit = AgnoDecisionKit(context=_make_context(), enable_policy_check=False) diff --git a/tests/integrations/agno/test_kg_toolkit.py b/tests/integrations/agno/test_kg_toolkit.py index 8ddd25a9..d9dcdbfc 100644 --- a/tests/integrations/agno/test_kg_toolkit.py +++ b/tests/integrations/agno/test_kg_toolkit.py @@ -129,7 +129,31 @@ class TestAgnoKGToolkitInit(unittest.TestCase): def test_tools_registered(self): kit = AgnoKGToolkit() - self.assertTrue(len(kit._tools) >= 7) + self.assertEqual(len(kit._tools), 7) + self.assertEqual(len(kit._tools), len(set(kit._tools))) + + def test_registration_invoked(self): + with patch.object(AgnoKGToolkit, "register") as mock_register: + AgnoKGToolkit() + self.assertEqual(mock_register.call_count, 7) + + def test_registration_failure_propagates(self): + with patch.object(AgnoKGToolkit, "register", side_effect=RuntimeError("Registration failed")): + with self.assertRaises(RuntimeError): + AgnoKGToolkit() + + def test_graceful_degradation_when_agno_unavailable(self): + with patch("integrations.agno.kg_toolkit.AGNO_AVAILABLE", False): + with patch.object(AgnoKGToolkit, "register") as mock_register: + kit = AgnoKGToolkit() + mock_register.assert_not_called() + self.assertEqual(len(kit._tools), 7) + self.assertEqual(len(kit._tools), len(set(kit._tools))) + + def test_no_duplicate_tools(self): + kit = AgnoKGToolkit() + self.assertEqual(len(kit._tools), len(set(kit._tools))) + self.assertEqual(len(kit._tools), 7) def test_context_graph_attached(self): ctx = MagicMock()