diff --git a/src/benchflow/agents/registry.py b/src/benchflow/agents/registry.py index ac8e495e..8d5adb32 100644 --- a/src/benchflow/agents/registry.py +++ b/src/benchflow/agents/registry.py @@ -700,6 +700,12 @@ def _acpx_wrap(config: AgentConfig) -> AgentConfig: acpx_agent_name = alias break + # The acpx wrapper only overrides name/install_cmd/launch_cmd. Every other + # AgentConfig field must pass through from the underlying agent so that + # routing-relevant attributes (api_protocol, default_model, env_mapping, + # requires_env, credentials, …) survive when the wrapped config is cached + # into AGENTS and later read by resolve_provider_env. ``protocol`` stays + # "acp" because acpx itself speaks ACP regardless of the inner agent. return AgentConfig( # ``acpx:`` runtime key — see acpx_runtime_key / module-level contract. name=acpx_runtime_key(config.name), @@ -712,6 +718,8 @@ def _acpx_wrap(config: AgentConfig) -> AgentConfig: description=f"{config.description} (via acpx)", skill_paths=config.skill_paths, install_timeout=config.install_timeout, + default_model=config.default_model, + api_protocol=config.api_protocol, env_mapping=config.env_mapping, credential_files=config.credential_files, home_dirs=config.home_dirs, diff --git a/tests/test_agent_spec.py b/tests/test_agent_spec.py index 4e70f233..530221d7 100644 --- a/tests/test_agent_spec.py +++ b/tests/test_agent_spec.py @@ -66,6 +66,49 @@ def test_resolve_acpx_agent(self): assert config.name == "acpx:claude-agent-acp" assert "acpx" in config.launch_cmd + def test_acpx_wrap_carries_routing_fields(self): + """Regression for PR #322: _acpx_wrap must inherit api_protocol and + default_model (and all other non-overridden fields) from the underlying + agent. The wrapped config is cached into AGENTS and later read by + resolve_provider_env; if api_protocol is dropped, multi-endpoint + providers route to the wrong base URL/protocol. + """ + underlying = AGENTS["claude-agent-acp"] + wrapped = resolve_agent("acpx/claude") + + # The two fields the PR #322 review flagged as dropped. + assert wrapped.api_protocol == underlying.api_protocol + assert wrapped.api_protocol == "anthropic-messages" + assert wrapped.default_model == underlying.default_model + + # The acpx wrapper only legitimately overrides name/install/launch. + # Everything else routing-relevant must pass through unchanged. + assert wrapped.requires_env == underlying.requires_env + assert wrapped.env_mapping == underlying.env_mapping + assert wrapped.skill_paths == underlying.skill_paths + assert wrapped.credential_files == underlying.credential_files + assert wrapped.home_dirs == underlying.home_dirs + assert wrapped.acp_model_format == underlying.acp_model_format + assert wrapped.subscription_auth == underlying.subscription_auth + assert wrapped.supports_acp_set_model == underlying.supports_acp_set_model + assert wrapped.install_timeout == underlying.install_timeout + assert ( + wrapped.disallow_web_tools_setup_cmd + == underlying.disallow_web_tools_setup_cmd + ) + assert ( + wrapped.disallow_web_tools_launch_suffix + == underlying.disallow_web_tools_launch_suffix + ) + + def test_acpx_cached_config_keeps_api_protocol(self): + """Regression for PR #322: the cached acpx runtime key in AGENTS must + expose the underlying agent's api_protocol so resolve_provider_env + picks the agent-required endpoint for multi-endpoint providers. + """ + key = resolve_agent_key("acpx/claude") + assert AGENTS[key].api_protocol == "anthropic-messages" + def test_resolve_unknown_raises(self): with pytest.raises(KeyError, match="Unknown agent"): resolve_agent("nonexistent-agent")