fix(delegation): assemble subagent credential bundle atomically
_resolve_child_runtime resolves provider and base_url with independent per-field fallbacks, so a delegation.provider override whose runtime comes back without a base_url silently inherits the PARENT's endpoint. The child then sends e.g. copilot credentials to the parent's Codex URL: every request 404s, and the fallback chain can't rescue it because its dedup matches provider+model and skips the entry as a self-loop. Build the bundle all-or-nothing: with a provider override, base_url comes from the override only; otherwise everything is inherited from the parent as before. _runtime_provider_credentials now refuses a provider that resolves without a base_url, unless it is an ACP transport (addressed by command) or a native-SDK provider (bedrock/vertex/google), which legitimately have none.
This commit is contained in:
@@ -2284,5 +2284,57 @@ class TestFallbackModelInheritance(unittest.TestCase):
|
||||
self.assertIn("missing-acp-binary", str(ctx.exception))
|
||||
|
||||
|
||||
class TestAtomicChildCredentialBundle(unittest.TestCase):
|
||||
"""provider/base_url reach the child as one bundle: all override or all parent."""
|
||||
|
||||
def _build(self, parent, **overrides):
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
MockAgent.return_value = MagicMock()
|
||||
_build_child_agent(
|
||||
task_index=0, goal="bundle", context=None, toolsets=None, model=None,
|
||||
max_iterations=10, parent_agent=parent, task_count=1, **overrides,
|
||||
)
|
||||
return MockAgent.call_args[1]
|
||||
|
||||
def test_provider_override_never_borrows_parent_base_url(self):
|
||||
parent = _make_mock_parent(depth=0)
|
||||
kwargs = self._build(parent, override_provider="copilot", override_base_url=None, override_api_key="gh-x")
|
||||
self.assertEqual(kwargs["provider"], "copilot")
|
||||
self.assertIsNone(kwargs["base_url"])
|
||||
self.assertNotEqual(kwargs["base_url"], parent.base_url)
|
||||
|
||||
def test_provider_override_uses_its_own_endpoint(self):
|
||||
parent = _make_mock_parent(depth=0)
|
||||
kwargs = self._build(
|
||||
parent, override_provider="minimax", override_base_url="https://api.minimax.example/v1",
|
||||
override_api_key="sk-mm-x")
|
||||
self.assertEqual(kwargs["provider"], "minimax")
|
||||
self.assertEqual(kwargs["base_url"], "https://api.minimax.example/v1")
|
||||
self.assertEqual(kwargs["api_key"], "sk-mm-x")
|
||||
|
||||
def test_no_override_inherits_whole_parent_bundle(self):
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent._client_kwargs = {}
|
||||
parent.client = None
|
||||
kwargs = self._build(parent)
|
||||
self.assertEqual(kwargs["provider"], parent.provider)
|
||||
self.assertEqual(kwargs["base_url"], parent.base_url)
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_provider_without_base_url_is_refused(self, mock_resolve):
|
||||
mock_resolve.return_value = {"provider": "copilot", "base_url": "", "api_key": "gh-x", "api_mode": None}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
_resolve_delegation_credentials({"provider": "copilot", "model": "gpt-5"}, parent)
|
||||
self.assertIn("without a base_url", str(ctx.exception))
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_native_sdk_provider_without_base_url_is_allowed(self, mock_resolve):
|
||||
mock_resolve.return_value = {"provider": "bedrock", "base_url": "", "api_key": "aws", "api_mode": None}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
creds = _resolve_delegation_credentials({"provider": "bedrock", "model": "claude"}, parent)
|
||||
self.assertEqual(creds["provider"], "bedrock")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -381,6 +381,16 @@ def _runtime_provider_credentials(v: dict, explicit_request_overrides) -> dict:
|
||||
f"'{pinned_command}' command, which was not found on PATH. "
|
||||
f"Install it or choose a different delegation provider.",
|
||||
)
|
||||
# A provider override is assembled into the child as one bundle (see _resolve_child_runtime), so it must
|
||||
# carry its own endpoint. ACP transports are addressed by command and native-SDK providers by their SDK,
|
||||
# so neither needs a URL; anything else without one would otherwise be built with no endpoint at all.
|
||||
if (not runtime.get("base_url") and not pinned_command
|
||||
and configured_provider.strip().lower() not in _NATIVE_SDK_PROVIDERS):
|
||||
raise ValueError(
|
||||
f"Delegation provider '{configured_provider}' resolved without a base_url. "
|
||||
f"Refusing to build a subagent with an incomplete credential bundle — check the provider's "
|
||||
f"configuration / auth, or set delegation.base_url for a direct endpoint."
|
||||
)
|
||||
return _credential_bundle(
|
||||
v["model"] or runtime.get("model") or None,
|
||||
configured_provider if runtime.get("provider") == _RUNTIME_PROVIDER_CUSTOM else runtime.get("provider"),
|
||||
@@ -473,8 +483,17 @@ def _resolve_child_runtime(
|
||||
``override_provider`` clears the parent's ACP transport, fallback chain and OpenRouter routing filters so the
|
||||
pinned provider is actually honoured."""
|
||||
effective_model = model or parent_agent.model
|
||||
effective_provider = override_provider or getattr(parent_agent, "provider", None)
|
||||
effective_base_url = override_base_url or _inherit_parent_base_url(parent_agent, parent_agent.base_url)
|
||||
# provider/base_url are one bundle: all from the override, or all from the parent. Per-field fallback built
|
||||
# children like an override provider pointed at the PARENT's endpoint (e.g. copilot credentials on the
|
||||
# parent's Codex URL), which 404s on every request and can't be rescued by the fallback chain, whose dedup
|
||||
# matches provider+model and so skips the entry as a self-loop. _inherit_parent_base_url recovers the
|
||||
# parent's live endpoint, which is meaningless for a different provider.
|
||||
if override_provider:
|
||||
effective_provider = override_provider
|
||||
effective_base_url = override_base_url
|
||||
else:
|
||||
effective_provider = getattr(parent_agent, "provider", None)
|
||||
effective_base_url = override_base_url or _inherit_parent_base_url(parent_agent, parent_agent.base_url)
|
||||
# api_mode: each provider has its own wire, so a different provider re-derives (None) instead of inheriting (404s
|
||||
# otherwise). Nous Portal is dual-wire within one provider (anthropic/* → Messages, else chat_completions), so
|
||||
# same-provider inheritance would pin the child on the wrong wire — re-derive.
|
||||
|
||||
Reference in New Issue
Block a user