From e77cd7f2d6876c4d9860a74677ac9a063bc5f960 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 3 Aug 2026 09:37:35 +0000 Subject: [PATCH] fix(sdk): reject switch_llm/switch_profile on ACP conversations 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 --- .../conversation/impl/local_conversation.py | 14 ++++++++++ tests/sdk/conversation/test_switch_model.py | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index 8efbf62594..9f4f717007 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -1539,7 +1539,21 @@ def switch_llm(self, llm: LLM) -> None: Args: llm: LLM to install on the agent. + + Raises: + ValueError: If the conversation's agent is an :class:`ACPAgent`. + Swapping OpenHands' own LLM object has no effect on an ACP + subprocess, which owns its own model. Persisting the swap would + leave base_state.json and the live session disagreeing (#4158); + use :meth:`switch_acp_model` for ACP conversations instead. """ + if isinstance(self.agent, ACPAgent): + raise ValueError( + "switch_llm/switch_profile is not supported for ACP conversations. " + "The ACP server owns its own model, so switching the OpenHands LLM " + "would not take effect on the live session. Use switch_acp_model to " + "change the model of an ACP conversation." + ) try: new_llm = self.llm_registry.get(llm.usage_id) except KeyError: diff --git a/tests/sdk/conversation/test_switch_model.py b/tests/sdk/conversation/test_switch_model.py index 75772e2c2f..39b130d3de 100644 --- a/tests/sdk/conversation/test_switch_model.py +++ b/tests/sdk/conversation/test_switch_model.py @@ -258,6 +258,34 @@ def test_switch_acp_model_disarms_discarded_agent_finalizer(tmp_path): assert switched._atexit_callback is None +def test_switch_llm_rejects_acp_agent(tmp_path): + """Regression for #4158: switch_llm must reject ACP conversations. + + Swapping the OpenHands LLM has no effect on the ACP subprocess (which owns + its own model), so persisting the swap would leave base_state.json and the + live session disagreeing. It must fail loudly instead of half-applying. + """ + conv, agent = _make_acp_conversation(tmp_path) + with pytest.raises(ValueError, match="not supported for ACP"): + conv.switch_llm(_make_llm("kimi", "kimi")) + # The agent is untouched — still the ACP agent, and no LLM swap occurred. + assert conv.agent is agent + assert isinstance(conv.state.agent, ACPAgent) + + +def test_switch_profile_rejects_acp_agent(tmp_path, profile_store): + """Regression for #4158: switch_profile must reject ACP conversations. + + The persisted state must not be rewritten to the profile's OpenHands LLM + while the live ACP session keeps the old agent. + """ + conv, agent = _make_acp_conversation(tmp_path) + with pytest.raises(ValueError, match="not supported for ACP"): + conv.switch_profile("fast") + assert conv.agent is agent + assert isinstance(conv.state.agent, ACPAgent) + + def test_switch_profile(profile_store): """switch_profile switches the agent's LLM.""" conv = _make_conversation()