fix(sdk): reject switch_llm/switch_profile on ACP conversations - #4333
Draft
VascoSch92 wants to merge 1 commit into
Draft
fix(sdk): reject switch_llm/switch_profile on ACP conversations#4333VascoSch92 wants to merge 1 commit into
VascoSch92 wants to merge 1 commit into
Conversation
An ACP conversation runs its model inside the ACP subprocess, which owns its own model. switch_llm/switch_profile only swaps OpenHands' own LLM object, so calling it on an ACP conversation half-applied: base_state.json was rewritten to the new LLM while the live session kept the old ACP agent, and callers were told the switch succeeded. Guard switch_llm so it raises ValueError for ACP conversations, pointing callers at switch_acp_model. Both existing callers (the REST switch_profile endpoint and the SwitchLLM tool) already surface ValueError as a 4xx / error observation, so the half-applied path is now rejected loudly. Fixes #4158 Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
Coverage Report •
|
||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
AGENT:
Why
Fixes #4158. On an ACP conversation,
switch_profile(and the underlyingswitch_llm) half-applied:base_state.jsonwas rewritten to the new OpenHands LLM while the live ACP session kept running the old ACP agent/model. The caller (agent-canvas UI / REST endpoint /switch_llmtool) was told the switch succeeded, so persisted state and the running agent disagreed.The root cause is that an ACP conversation runs its model inside the ACP subprocess, which owns its own model.
switch_llmonly swaps OpenHands' ownLLMobject, which has no effect on an ACP subprocess — but it still persisted the swap, producing the inconsistency.Summary
LocalConversation.switch_llmto raiseValueErrorwhen the conversation's agent is anACPAgent, pointing callers atswitch_acp_modelinstead.switch_profiledelegates toswitch_llm, so it is covered too.ValueErroras a 4xx (POST /switch_profile→ 400) or an error observation (SwitchLLMtool), so the half-applied path is now rejected loudly rather than silently succeeding.switch_llmandswitch_profileagainst an ACP conversation.This matches the issue's suggested fix: "cross-agent-kind switching is out of scope, so
switch_profileshould validate agent_kind compatibility and return 4xx."Issue Number
Fixes #4158
How to Test
Run the targeted tests:
Both suites pass (26 and 15 tests respectively). New tests:
test_switch_llm_rejects_acp_agent—switch_llmon an ACP conversation raisesValueErrorand leaves the agent/state untouched.test_switch_profile_rejects_acp_agent—switch_profileon an ACP conversation raisesValueError; persisted state is not rewritten.Pre-commit (ruff, pyright, import rules) passes on the changed files.
Video/Screenshots
N/A — SDK-level behavior change covered by unit tests.
Type
Notes
This is intentionally a "reject loudly" fix rather than "tear down and re-provision the ACP agent from updated state," matching the out-of-scope guidance in the issue. Cross-agent-kind live switching can be a follow-up if desired. Downstream UIs should also avoid offering incompatible profiles mid-conversation, but that is outside this repo.
This PR was created by an AI agent (OpenHands) on behalf of the user.
@VascoSch92 can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:e77cd7f-pythonRun
All tags pushed for this build
About Multi-Architecture Support
e77cd7f-python) is a multi-arch manifest supporting both amd64 and arm64e77cd7f-python-amd64) are also available if needed