Skip to content

Commit c616fc8

Browse files
Debug Agentclaude
andcommitted
refactor(acp): inline vision probe into supports_vision()
Map values were all True and the helper's only caller was ACPAgent.supports_vision(); drop the wrapper and check substring membership inline. Replaces the provider→bool dict with a plain tuple of known vision-capable ACP server markers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0288dd6 commit c616fc8

2 files changed

Lines changed: 33 additions & 90 deletions

File tree

openhands-sdk/openhands/sdk/agent/acp_agent.py

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -142,40 +142,21 @@ def _make_dummy_llm() -> LLM:
142142
}
143143
_DEFAULT_BYPASS_MODE = "full-access"
144144

145-
# ACP server identifier substrings → whether the server forwards ACP
146-
# ImageContentBlock prompts to a vision-capable underlying model.
147-
#
148-
# Verified against upstream source (all declare prompt_capabilities.image=true
149-
# and map ContentBlock::Image to their provider's native image format):
150-
# claude-agent-acp → Claude Sonnet/Opus/Haiku 3.5+ (vision)
145+
# ACP servers known to forward ImageContentBlock prompts to a vision-capable
146+
# underlying model. Verified against upstream source — all declare
147+
# prompt_capabilities.image=true and map ContentBlock::Image to their
148+
# provider's native image format:
149+
# claude-agent-acp → Claude Sonnet/Opus/Haiku 3.5+
151150
# src/acp-agent.ts: promptToClaude() case "image"
152-
# gemini-cli → Gemini 1.5/2.x Pro/Flash (vision)
151+
# gemini-cli → Gemini 1.5/2.x Pro/Flash
153152
# packages/cli/src/acp/acpClient.ts: #resolvePrompt() case "image"
154-
# codex-acp → GPT-5 family via Codex CLI (vision)
153+
# codex-acp → GPT-5 family via Codex CLI
155154
# src/thread.rs: build_prompt_items() image → UserInput::Image
156-
_ACP_VISION_PROVIDER_MARKERS: dict[str, bool] = {
157-
"claude-agent-acp": True,
158-
"gemini-cli": True,
159-
"codex-acp": True,
160-
}
161-
162-
163-
def _acp_command_supports_vision(command: list[str], agent_name: str) -> bool:
164-
"""Return True if the ACP server forwards inline images to a vision model.
165-
166-
Matches on either the runtime agent name from InitializeResponse (when
167-
available) or the configured launch command (always available). Falls
168-
back to False for unknown servers so we do not silently send images a
169-
server may drop.
170-
"""
171-
haystacks: list[str] = []
172-
if agent_name:
173-
haystacks.append(agent_name.lower())
174-
haystacks.append(" ".join(command).lower())
175-
for marker, supports in _ACP_VISION_PROVIDER_MARKERS.items():
176-
if any(marker in h for h in haystacks):
177-
return supports
178-
return False
155+
_VISION_CAPABLE_ACP_SERVERS: tuple[str, ...] = (
156+
"claude-agent-acp",
157+
"gemini-cli",
158+
"codex-acp",
159+
)
179160

180161

181162
# ACP auth method ID → environment variable that supplies the credential.
@@ -826,9 +807,11 @@ def supports_vision(self) -> bool:
826807
does not recognise, so the base-class ``llm.vision_is_active()``
827808
always returns False for ACP agents. Resolve the real capability
828809
from the ACP server identity (launch command or the agent name
829-
reported by InitializeResponse) instead.
810+
reported by InitializeResponse) instead. Unknown servers fall back
811+
to False so we don't silently send images a server may drop.
830812
"""
831-
return _acp_command_supports_vision(self.acp_command, self._agent_name)
813+
probe = f"{self._agent_name} {' '.join(self.acp_command)}".lower()
814+
return any(marker in probe for marker in _VISION_CAPABLE_ACP_SERVERS)
832815

833816
def get_all_llms(self) -> Generator[LLM]:
834817
yield self.llm

tests/sdk/agent/test_acp_agent.py

Lines changed: 17 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
from openhands.sdk.agent.acp_agent import (
1515
ACPAgent,
16-
_acp_command_supports_vision,
1716
_build_session_meta,
1817
_estimate_cost_from_tokens,
1918
_extract_token_usage,
@@ -2081,80 +2080,41 @@ def test_empty_name_defaults_to_full_access(self):
20812080

20822081

20832082
# ---------------------------------------------------------------------------
2084-
# _acp_command_supports_vision + ACPAgent.supports_vision()
2083+
# ACPAgent.supports_vision()
20852084
# ---------------------------------------------------------------------------
20862085

20872086

2088-
class TestACPCommandSupportsVision:
2087+
class TestACPAgentSupportsVision:
20892088
"""All three ACP providers OpenHands supports — claude-agent-acp,
20902089
gemini-cli, codex-acp — declare prompt_capabilities.image=true and
20912090
forward images to a vision-capable underlying model."""
20922091

2093-
def test_claude_agent_acp_command(self):
2094-
assert (
2095-
_acp_command_supports_vision(
2096-
["npx", "-y", "@agentclientprotocol/claude-agent-acp"],
2097-
"",
2098-
)
2099-
is True
2100-
)
2101-
2102-
def test_claude_agent_acp_from_agent_name(self):
2103-
assert _acp_command_supports_vision(["/some/opaque/binary"], "claude-agent-acp")
2104-
assert _acp_command_supports_vision([], "claude-agent-acp")
2105-
2106-
def test_gemini_cli_command(self):
2107-
assert (
2108-
_acp_command_supports_vision(["gemini-cli", "--experimental-acp"], "")
2109-
is True
2110-
)
2111-
2112-
def test_gemini_cli_from_agent_name(self):
2113-
assert _acp_command_supports_vision(["node", "server.js"], "gemini-cli")
2114-
2115-
def test_codex_acp_command(self):
2116-
assert _acp_command_supports_vision(["codex-acp"], "") is True
2117-
2118-
def test_codex_acp_from_agent_name(self):
2119-
assert _acp_command_supports_vision([], "codex-acp")
2120-
2121-
def test_agent_name_takes_precedence_over_generic_command(self):
2122-
# A user can launch any of our three servers via ``node`` or a
2123-
# local path; the agent name from InitializeResponse still lets
2124-
# us identify the provider.
2125-
assert (
2126-
_acp_command_supports_vision(
2127-
["/usr/local/bin/node", "/opt/server.js"],
2128-
"claude-agent-acp",
2129-
)
2130-
is True
2131-
)
2132-
2133-
def test_unknown_server_defaults_to_false(self):
2134-
# Safe default: unknown servers may silently drop image blocks,
2135-
# so advertise no-vision rather than send content that gets lost.
2136-
assert _acp_command_supports_vision(["some-random-acp-server"], "") is False
2137-
2138-
def test_empty_inputs_default_to_false(self):
2139-
assert _acp_command_supports_vision([], "") is False
2140-
2141-
2142-
class TestACPAgentSupportsVision:
2143-
def test_claude_agent_supports_vision(self):
2092+
def test_claude_agent_via_command(self):
21442093
agent = ACPAgent(
21452094
acp_command=["npx", "-y", "@agentclientprotocol/claude-agent-acp"]
21462095
)
21472096
assert agent.supports_vision() is True
21482097

2149-
def test_gemini_cli_supports_vision(self):
2098+
def test_gemini_cli_via_command(self):
21502099
agent = ACPAgent(acp_command=["gemini-cli", "--experimental-acp"])
21512100
assert agent.supports_vision() is True
21522101

2153-
def test_codex_acp_supports_vision(self):
2102+
def test_codex_acp_via_command(self):
21542103
agent = ACPAgent(acp_command=["codex-acp"])
21552104
assert agent.supports_vision() is True
21562105

2157-
def test_unknown_acp_server_does_not_claim_vision(self):
2106+
def test_agent_name_resolves_opaque_launcher(self):
2107+
# A user can launch any of our three servers via ``node`` or a
2108+
# local path; the agent name from InitializeResponse still lets
2109+
# us identify the provider.
2110+
agent = ACPAgent(acp_command=["/usr/local/bin/node", "/opt/server.js"])
2111+
assert agent.supports_vision() is False
2112+
agent._agent_name = "claude-agent-acp"
2113+
assert agent.supports_vision() is True
2114+
2115+
def test_unknown_acp_server_defaults_to_false(self):
2116+
# Unknown servers may silently drop image blocks, so advertise
2117+
# no-vision rather than send content that gets lost.
21582118
agent = ACPAgent(acp_command=["echo", "test"])
21592119
assert agent.supports_vision() is False
21602120

0 commit comments

Comments
 (0)