From 72464503b418c4b25b6ce44791bb29a29dfc3446 Mon Sep 17 00:00:00 2001 From: Ayush Nangia Date: Thu, 6 Aug 2026 22:27:54 +0530 Subject: [PATCH] fix(delegation): resolve the child fallback chain through the full pin x config matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composes #80465's pin semantics with #80438/#80421's delegation.fallback_providers semantics (#65038) and settles the composition cell the three PRs leave undefined (#80450 map). _resolve_child_fallback_chain implements the whole decision table in one pure function: pinned children get no chain unless the delegation section declares one (a delegation-scoped chain honors both explicit intents; the silent drag in #80450 is specifically the PARENT chain substituting a pin); an explicit empty list disables fallback; absent config preserves historical parent inheritance exactly. Malformed declared values log and fall back PIN-AWARE — never the parent chain on a pinned child, so the config-error path cannot reintroduce the drag. Entries normalize through the canonical get_fallback_chain. The two existing inheritance tests are made hermetic against the real user config (as #80438 also did). --- tests/tools/test_delegate.py | 10 +- tests/tools/test_delegate_fallback_matrix.py | 184 +++++++++++++++++++ 2 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 tests/tools/test_delegate_fallback_matrix.py diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index 58346a41a1..dd3fee0841 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -2125,7 +2125,10 @@ class TestFallbackModelInheritance(unittest.TestCase): fallback_entry = {"provider": "openrouter", "model": "gpt-4o-mini", "api_key": "sk-or-x"} parent._fallback_chain = [fallback_entry] - with patch("run_agent.AIAgent") as MockAgent: + with ( + patch("run_agent.AIAgent") as MockAgent, + patch("tools.delegate_tool._load_config", return_value={}), + ): MockAgent.return_value = MagicMock() _build_child_agent( task_index=0, @@ -2146,7 +2149,10 @@ class TestFallbackModelInheritance(unittest.TestCase): parent = _make_mock_parent(depth=0) parent._fallback_chain = [] - with patch("run_agent.AIAgent") as MockAgent: + with ( + patch("run_agent.AIAgent") as MockAgent, + patch("tools.delegate_tool._load_config", return_value={}), + ): MockAgent.return_value = MagicMock() _build_child_agent( task_index=0, diff --git a/tests/tools/test_delegate_fallback_matrix.py b/tests/tools/test_delegate_fallback_matrix.py new file mode 100644 index 0000000000..0b8cc028d0 --- /dev/null +++ b/tests/tools/test_delegate_fallback_matrix.py @@ -0,0 +1,184 @@ +"""Full decision matrix for _resolve_child_fallback_chain (#80450 class). + +Composes the pin semantics of PR #80465 (@teknium1), the +delegation.fallback_providers semantics of PRs #80438 (@wz-heng) / +#80421 (@andrexibiza) for issue #65038 (@mlahatte), and settles the +pin+declared-chain composition cell raised in the #80450 cross-PR map. +""" + +import unittest +from unittest.mock import MagicMock, patch + +from tools.delegate_tool import _build_child_agent, _resolve_child_fallback_chain +from tests.tools.test_delegate import _make_mock_parent + +PARENT_CHAIN = [ + {"provider": "openrouter", "model": "gpt-4o-mini", "api_key": "sk-or-parent"} +] +DECLARED_CHAIN = [ + {"provider": "deepseek", "model": "deepseek-chat", "api_key": "sk-ds-child"} +] + + +def _parent(chain=None): + parent = _make_mock_parent(depth=0) + parent._fallback_chain = chain + return parent + + +class TestResolveChildFallbackChainMatrix(unittest.TestCase): + """All six cells of the pin x declared-chain matrix.""" + + # pin=yes ------------------------------------------------------------ + + def test_pin_declared_nonempty_uses_declared_chain(self): + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": list(DECLARED_CHAIN)}, + pinned=True, + ) + providers = {e.get("provider") for e in (chain or [])} + self.assertIn("deepseek", providers) + self.assertNotIn("openrouter", providers) + + def test_pin_declared_empty_disables_chain(self): + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), {"fallback_providers": []}, pinned=True + ) + self.assertIsNone(chain) + + def test_pin_absent_gets_no_chain(self): + """#80450: a pinned child must fail loudly, not silently reroute.""" + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), {}, pinned=True + ) + self.assertIsNone(chain) + + # pin=no ------------------------------------------------------------- + + def test_unpinned_declared_nonempty_uses_declared_chain(self): + """#65038: delegation.fallback_providers reaches the child.""" + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": list(DECLARED_CHAIN)}, + pinned=False, + ) + providers = {e.get("provider") for e in (chain or [])} + self.assertIn("deepseek", providers) + self.assertNotIn("openrouter", providers) + + def test_unpinned_declared_empty_disables_chain(self): + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), {"fallback_providers": []}, pinned=False + ) + self.assertIsNone(chain) + + def test_unpinned_absent_inherits_parent_chain(self): + """Historical default is preserved exactly.""" + chain = _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), {}, pinned=False + ) + self.assertEqual(chain, PARENT_CHAIN) + + # edges ---------------------------------------------------------------- + + def test_unpinned_absent_with_empty_parent_chain_is_none(self): + self.assertIsNone(_resolve_child_fallback_chain(_parent([]), {}, pinned=False)) + + def test_declared_chain_is_normalized_and_deduped(self): + """Entries route through the canonical get_fallback_chain normalizer.""" + chain = _resolve_child_fallback_chain( + _parent(None), + {"fallback_providers": list(DECLARED_CHAIN) + list(DECLARED_CHAIN)}, + pinned=False, + ) + routes = [(e.get("provider"), e.get("model")) for e in (chain or [])] + self.assertEqual(len(routes), len(set(routes))) + + def test_malformed_declared_value_pin_aware_fallback(self): + """Malformed config logs and falls back pin-aware: None when pinned + (never reintroduce the silent drag through the error path), parent + chain otherwise — extends #80421's log-and-inherit contract.""" + with patch( + "hermes_cli.fallback_config.get_fallback_chain", + side_effect=TypeError("boom"), + ): + self.assertIsNone( + _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": "not-a-list"}, + pinned=True, + ) + ) + self.assertEqual( + _resolve_child_fallback_chain( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": "not-a-list"}, + pinned=False, + ), + PARENT_CHAIN, + ) + + def test_non_dict_config_uses_default(self): + self.assertEqual( + _resolve_child_fallback_chain(_parent(list(PARENT_CHAIN)), None, pinned=False), + PARENT_CHAIN, + ) + + +class TestBuildChildAgentWiring(unittest.TestCase): + """End-to-end through _build_child_agent: the resolver is actually wired.""" + + def _spawn(self, parent, cfg, **overrides): + with ( + patch("tools.delegate_tool._load_config", return_value=cfg), + patch("run_agent.AIAgent") as MockAgent, + ): + MockAgent.return_value = MagicMock() + _build_child_agent( + task_index=0, + goal="matrix wiring", + context=None, + toolsets=None, + model=None, + max_iterations=10, + parent_agent=parent, + task_count=1, + **overrides, + ) + return MockAgent.call_args[1] + + def test_pinned_child_gets_no_parent_chain(self): + kwargs = self._spawn( + _parent(list(PARENT_CHAIN)), {}, override_provider="minimax", + override_base_url="https://api.minimax.example/v1", override_api_key="sk-mm", + ) + self.assertIsNone(kwargs["fallback_model"]) + + def test_configured_delegation_chain_reaches_child(self): + kwargs = self._spawn( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": list(DECLARED_CHAIN)}, + ) + providers = {e.get("provider") for e in (kwargs["fallback_model"] or [])} + self.assertIn("deepseek", providers) + + def test_pin_plus_declared_chain_uses_declared(self): + kwargs = self._spawn( + _parent(list(PARENT_CHAIN)), + {"fallback_providers": list(DECLARED_CHAIN)}, + override_provider="deepseek", + override_base_url="https://api.deepseek.example/v1", + override_api_key="sk-ds", + ) + providers = {e.get("provider") for e in (kwargs["fallback_model"] or [])} + self.assertIn("deepseek", providers) + self.assertNotIn("openrouter", providers) + + def test_default_inheritance_preserved(self): + kwargs = self._spawn(_parent(list(PARENT_CHAIN)), {}) + self.assertEqual(kwargs["fallback_model"], PARENT_CHAIN) + + +if __name__ == "__main__": + unittest.main()