harden(acp): fall back to the other model-select mechanism on method-not-found

Response-detection (`_session_selects_model_via_config_option`) picks
set_config_option vs set_session_model from the session response and is correct
for every validated CLI. But it reads an UNSTABLE capability whose shape can
vary by build/auth state, so a misdetect would crash session init or a model
switch with JSON-RPC -32601 "Method not found".

Add `_apply_acp_model_with_fallback`: if the chosen model-apply call returns
-32601, retry once with the other mechanism (and, for runtime switches, remember
the working one). Wired into init (`_maybe_set_session_model`) and
`ACPAgent.set_acp_model`; resume already tolerates rejection by keeping the
server default, so it's left as-is. A non-method-not-found error (e.g. -32602
invalid model) still propagates — it's a real client error, not a
wrong-mechanism signal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Simon Rosenberg

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

@@ -140,6 +140,13 @@
 #          upstream model 500s, and transient infrastructure errors.
 _RETRIABLE_SERVER_ERROR_CODES: frozenset[int] = frozenset({-32603})
 
+# -32601 = "Method not found" (JSON-RPC spec). An ACP server raises this when a
+# model-selection call uses the mechanism it does *not* implement — e.g.
+# `session/set_model` on a CLI that moved model selection to `configOptions`
+# (codex-acp 0.16+, claude-agent-acp 0.46+), or vice versa. We use it to fall
+# back to the other mechanism if response-detection picked the wrong one.
+_METHOD_NOT_FOUND_CODE: Final[int] = -32601
+
 
 # Maximum characters for ACP tool call content — matches MAX_CMD_OUTPUT_SIZE
 # used by the terminal tool and the default max_message_chars in LLM config.
@@ -437,6 +444,48 @@ def _apply_acp_model(
     return conn.set_session_model(model_id=model, session_id=session_id)
 
 
+def _is_method_not_found(exc: ACPRequestError) -> bool:
+    """Whether ``exc`` is a JSON-RPC "method not found" — i.e. the server does
+    not implement the model-selection call we used."""
+    return exc.code == _METHOD_NOT_FOUND_CODE
+
+
+async def _apply_acp_model_with_fallback(
+    conn: ClientSideConnection,
+    session_id: str,
+    model: str,
+    *,
+    via_config_option: bool,
+) -> bool:
+    """Apply ``model`` via the detected mechanism, falling back to the other if
+    the server reports the method missing.
+
+    Response-detection (``_session_selects_model_via_config_option``) is correct
+    for every CLI we've validated, but it reads an UNSTABLE capability and the
+    response shape can vary by build/auth state. If the chosen call raises
+    ``-32601 "Method not found"``, the server simply uses the *other* mechanism,
+    so we retry with it instead of crashing session init. Returns the
+    ``via_config_option`` value that actually applied the model.
+    """
+    try:
+        await _apply_acp_model(
+            conn, session_id, model, via_config_option=via_config_option
+        )
+        return via_config_option
+    except ACPRequestError as exc:
+        if not _is_method_not_found(exc):
+            raise
+        logger.info(
+            "ACP model-apply via %s rejected as method-not-found; retrying via %s",
+            "set_config_option" if via_config_option else "set_session_model",
+            "set_session_model" if via_config_option else "set_config_option",
+        )
+        await _apply_acp_model(
+            conn, session_id, model, via_config_option=not via_config_option
+        )
+        return not via_config_option
+
+
 def _extract_session_models(
     response: Any,
 ) -> tuple[str | None, list[ACPModelInfo] | None]:
@@ -654,7 +703,7 @@ async def _maybe_set_session_model(
         return False
     provider = detect_acp_provider_by_agent_name(agent_name)
     if provider is not None and provider.supports_set_session_model:
-        await _apply_acp_model(
+        await _apply_acp_model_with_fallback(
             conn, session_id, acp_model, via_config_option=via_config_option
         )
         return True
@@ -727,7 +776,9 @@ async def _reapply_session_model_on_resume(
         if provider is not None:
             # Known provider: apply via the mechanism the resumed session uses
             # (set_config_option for codex-acp 0.16+/claude-agent-acp 0.46+,
-            # else set_session_model).
+            # else set_session_model). A rejection is already tolerated below
+            # (the session keeps the server default), so resume doesn't need the
+            # cross-mechanism fallback the init/switch paths use.
             await _apply_acp_model(
                 conn, session_id, acp_model, via_config_option=via_config_option
             )
@@ -3643,25 +3694,48 @@ def set_acp_model(self, model: str) -> None:
                 timeout=self.acp_prompt_timeout,
             )
         except ACPRequestError as e:
-            # Server-internal failures (JSON-RPC -32603) are not the caller's
-            # fault, and the prompt path already treats them as retriable. Let
-            # them propagate (-> 5xx) instead of mislabeling them as a 400
-            # client error.
-            if e.code in _RETRIABLE_SERVER_ERROR_CODES:
-                raise
-            # acp.exceptions.RequestError derives from Exception (not
-            # RuntimeError); surface a true client/protocol rejection (e.g.
-            # method-not-found, invalid model id) as a ValueError so callers —
-            # and the agent-server route — treat it as a 400-class client error
-            # rather than an opaque 500.
-            method = (
-                "set_config_option(model)"
-                if self._model_via_config_option
-                else "set_session_model"
-            )
-            raise ValueError(
-                f"ACP server rejected {method}(model={model!r}): {e}"
-            ) from e
+            pending_error: ACPRequestError | None = e
+            if _is_method_not_found(e):
+                # The session uses the other model-selection mechanism (or the
+                # init-time detection picked the wrong one): retry once with it,
+                # and remember the working mechanism for later switches. If the
+                # retry also fails, fall through to the error translation below.
+                flipped = not self._model_via_config_option
+                try:
+                    self._executor.run_async(
+                        _apply_acp_model(
+                            self._conn,
+                            self._session_id,
+                            model,
+                            via_config_option=flipped,
+                        ),
+                        timeout=self.acp_prompt_timeout,
+                    )
+                except ACPRequestError as e2:
+                    pending_error = e2
+                else:
+                    self._model_via_config_option = flipped
+                    pending_error = None  # both selections applied
+            if pending_error is not None:
+                # Server-internal failures (JSON-RPC -32603) are not the caller's
+                # fault, and the prompt path already treats them as retriable.
+                # Let them propagate (-> 5xx) instead of mislabeling them as a
+                # 400 client error.
+                if pending_error.code in _RETRIABLE_SERVER_ERROR_CODES:
+                    raise pending_error
+                # acp.exceptions.RequestError derives from Exception (not
+                # RuntimeError); surface a true client/protocol rejection (e.g.
+                # method-not-found on both mechanisms, invalid model id) as a
+                # ValueError so callers — and the agent-server route — treat it
+                # as a 400-class client error rather than an opaque 500.
+                method = (
+                    "set_config_option(model)"
+                    if self._model_via_config_option
+                    else "set_session_model"
+                )
+                raise ValueError(
+                    f"ACP server rejected {method}(model={model!r}): {pending_error}"
+                ) from pending_error
         # Reflect the live model on the sentinel LLM + metrics so cost/token
         # accounting and serialized state show the model actually in use
         # (mirrors model_post_init). The ``acp_model`` field is frozen, so the

tests/sdk/agent/test_acp_agent.py

@@ -20,13 +20,15 @@
 
 from openhands.sdk.agent.acp_agent import (
     ACPAgent,
+    _apply_acp_model_with_fallback,
     _classify_acp_init_error,
     _codex_auth_file,
     _codex_base_url_overrides,
     _estimate_cost_from_tokens,
     _extract_session_models,
     _extract_token_usage,
     _image_url_to_acp_block,
+    _is_method_not_found,
     _mask_json_value,
     _maybe_set_session_model,
     _mcp_config_to_acp_servers,
@@ -4711,6 +4713,33 @@ def test_switches_model_via_config_option_session(self):
         assert agent.llm.model == "gpt-5.4/low"
         assert agent._current_model_id == "gpt-5.4/low"
 
+    def test_switch_falls_back_on_method_not_found(self):
+        # Detected set_session_model, but the live server only has the
+        # configOptions mechanism: the first apply raises -32601, the switch
+        # retries with the other call and remembers it.
+        agent = self._wire(_make_agent(), "codex-acp", via_config_option=False)
+        agent._executor.run_async.side_effect = [
+            ACPRequestError(code=-32601, message="Method not found"),
+            None,  # retry with the flipped mechanism succeeds
+        ]
+        agent.set_acp_model("gpt-5.4/low")
+        assert agent._executor.run_async.call_count == 2
+        # The working mechanism is remembered for later switches.
+        assert agent._model_via_config_option is True
+        assert agent.llm.model == "gpt-5.4/low"
+        assert agent._current_model_id == "gpt-5.4/low"
+
+    def test_switch_invalid_model_does_not_fall_back(self):
+        # A -32602 invalid-params is a real client error, not a wrong-mechanism
+        # signal: surface it as ValueError without a second call.
+        agent = self._wire(_make_agent(), "codex-acp", via_config_option=True)
+        agent._executor.run_async.side_effect = ACPRequestError(
+            code=-32602, message="Invalid params"
+        )
+        with pytest.raises(ValueError, match="rejected set_config_option"):
+            agent.set_acp_model("nonsense")
+        agent._executor.run_async.assert_called_once()
+
     def test_claude_provider_supports_runtime_switch(self):
         agent = self._wire(_make_agent(), "claude-agent-acp")
         agent.set_acp_model("claude-haiku-4-5-20251001")
@@ -7187,6 +7216,77 @@ def test_none_response_is_not_config_option(self):
         assert _session_selects_model_via_config_option(None) is False
 
 
+class TestApplyAcpModelFallback:
+    """Model-apply falls back to the other mechanism on -32601 method-not-found.
+
+    Response-detection is correct for every validated CLI, but it reads an
+    UNSTABLE capability; a misdetect (or a server that moved between mechanisms)
+    must self-heal rather than crash session init / a switch.
+    """
+
+    def test_is_method_not_found(self):
+        assert _is_method_not_found(ACPRequestError(code=-32601, message="x")) is True
+        assert _is_method_not_found(ACPRequestError(code=-32603, message="x")) is False
+        assert _is_method_not_found(ACPRequestError(code=-32602, message="x")) is False
+
+    @pytest.mark.asyncio
+    async def test_config_option_falls_back_to_set_session_model(self):
+        # Detected configOptions, but the server only has set_session_model.
+        conn = AsyncMock()
+        conn.set_config_option.side_effect = ACPRequestError(
+            code=-32601, message="Method not found"
+        )
+        effective = await _apply_acp_model_with_fallback(
+            conn, "sess-1", "gemini-3-flash", via_config_option=True
+        )
+        conn.set_config_option.assert_awaited_once()
+        conn.set_session_model.assert_awaited_once_with(
+            model_id="gemini-3-flash", session_id="sess-1"
+        )
+        assert effective is False  # the mechanism that actually worked
+
+    @pytest.mark.asyncio
+    async def test_set_session_model_falls_back_to_config_option(self):
+        # Detected set_session_model, but the server moved to configOptions
+        # (codex 0.16 / claude 0.46 reached via a stale/degraded detection).
+        conn = AsyncMock()
+        conn.set_session_model.side_effect = ACPRequestError(
+            code=-32601, message="Method not found"
+        )
+        effective = await _apply_acp_model_with_fallback(
+            conn, "sess-1", "opus[1m]", via_config_option=False
+        )
+        conn.set_session_model.assert_awaited_once()
+        conn.set_config_option.assert_awaited_once_with(
+            config_id="model", value="opus[1m]", session_id="sess-1"
+        )
+        assert effective is True
+
+    @pytest.mark.asyncio
+    async def test_no_fallback_when_primary_succeeds(self):
+        conn = AsyncMock()
+        effective = await _apply_acp_model_with_fallback(
+            conn, "sess-1", "sonnet", via_config_option=True
+        )
+        conn.set_config_option.assert_awaited_once()
+        conn.set_session_model.assert_not_called()
+        assert effective is True
+
+    @pytest.mark.asyncio
+    async def test_non_method_not_found_error_propagates(self):
+        # An invalid-model / server error is NOT a wrong-mechanism signal — it
+        # must propagate, not silently try the other call.
+        conn = AsyncMock()
+        conn.set_config_option.side_effect = ACPRequestError(
+            code=-32602, message="Invalid params"
+        )
+        with pytest.raises(ACPRequestError):
+            await _apply_acp_model_with_fallback(
+                conn, "sess-1", "bad", via_config_option=True
+            )
+        conn.set_session_model.assert_not_called()
+
+
 class TestACPAgentAvailableModelsProperty:
     """``available_models`` exposes the server's model list verbatim.