Skip to content

fix(registry): carry api_protocol/default_model through _acpx_wrap#331

Merged
xdotli merged 1 commit into
mainfrom
fix/pr322-acpx-api-protocol
May 21, 2026
Merged

fix(registry): carry api_protocol/default_model through _acpx_wrap#331
xdotli merged 1 commit into
mainfrom
fix/pr322-acpx-api-protocol

Conversation

@xdotli
Copy link
Copy Markdown
Member

@xdotli xdotli commented May 21, 2026

Bug F — _acpx_wrap drops api_protocol / default_model

Addresses an unaddressed review-bot comment (Codex P1 + Cursor High) on merged PR #322.

Problem

_acpx_wrap in src/benchflow/agents/registry.py builds a new AgentConfig for the acpx:-wrapped agent but does not carry over api_protocol (or default_model) from the underlying agent config. PR #322 caches that wrapped config into AGENTS.

Provider routing reads AGENTS[agent].api_protocol in resolve_provider_env to pick endpoints. Cached entries like acpx:claude-agent-acp got api_protocol="", so multi-endpoint providers (e.g. zai, which has both OpenAI and Anthropic endpoints) fell back to the provider default protocol instead of the agent-required one — routing acpx/<agent> to the wrong base URL/protocol.

Confirmed live on main: resolve_agent_key("acpx/claude-agent-acp") yields a cached config with api_protocol='' while the underlying claude-agent-acp has api_protocol='anthropic-messages'.

Fix

_acpx_wrap now carries api_protocol and default_model through. The acpx wrapper only legitimately overrides name, install_cmd, and launch_cmd (protocol stays "acp" since acpx itself speaks ACP); every other AgentConfig field passes through from the underlying agent.

Tests

Two regression tests added to tests/test_agent_spec.py (docstrings name PR #322 per AGENTS.md):

  • test_acpx_wrap_carries_routing_fields — audits the full field set passes through.
  • test_acpx_cached_config_keeps_api_protocol — the cached acpx: runtime key exposes the underlying api_protocol.

CI gate: ruff format, ruff check, ty check, full pytest (1312 passed) all green.


Note

Medium Risk
Moderate risk because it changes how acpx/ agent configs are materialized/cached in AGENTS, which affects provider endpoint selection and runtime behavior for wrapped agents; scope is small and covered by targeted regression tests.

Overview
Fixes acpx-wrapped agent configs so they inherit routing-critical fields from the underlying AgentConfig when _acpx_wrap builds the wrapped entry.

_acpx_wrap now carries through api_protocol and default_model (in addition to existing pass-through fields), preventing cached acpx: entries from losing endpoint/protocol hints used during provider resolution. Adds regression tests asserting the wrapped config and the cached acpx: runtime key preserve these fields and other non-overridden attributes.

Reviewed by Cursor Bugbot for commit 77ab385. Bugbot is set up for automated code reviews on this repo. Configure here.

_acpx_wrap built a new AgentConfig for acpx-wrapped agents but dropped
api_protocol and default_model from the underlying config. PR #322 caches
that wrapped config into AGENTS, and resolve_provider_env reads
AGENTS[agent].api_protocol to pick provider endpoints. Cached entries like
acpx:claude-agent-acp got api_protocol="", so multi-endpoint providers
(e.g. zai) fell back to the provider default protocol instead of the
agent-required one, routing acpx/<agent> to the wrong base URL.

Carry api_protocol and default_model through the wrapper. The acpx wrapper
only legitimately overrides name/install_cmd/launch_cmd; every other
AgentConfig field now passes through. Add regression tests in
test_agent_spec.py.
@xdotli xdotli merged commit feda074 into main May 21, 2026
3 of 4 checks passed
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@xdotli xdotli deleted the fix/pr322-acpx-api-protocol branch May 22, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant