fix(acp): only the switch_model rejection maps session/set_model to invalid params
The `except ValueError` in set_session_model wrapped both the switch_model rejection and the _make_agent rebuild, so a rebuild ValueError (provider disabled in config, context window below the floor) was reported as -32602 by accident. _switch_model now raises a dedicated ModelRejected(ValueError) at the rejection site and set_session_model catches only that; rebuild ValueErrors keep the -32603 internal-error path. The slash /model path still sees the rejection text via str(exc).
This commit is contained in:
@@ -211,6 +211,10 @@ def _take_interrupted_prompt(state: SessionState) -> tuple[bool, str]:
|
||||
return True, text
|
||||
|
||||
|
||||
class ModelRejected(ValueError):
|
||||
"""``switch_model`` refused the requested model (no provider can serve it)."""
|
||||
|
||||
|
||||
@dataclass
|
||||
class _TurnCallbacks:
|
||||
"""Per-turn ACP streaming callbacks; all None when no client is connected."""
|
||||
@@ -333,7 +337,7 @@ class HermesACPAgent(SlashCommandsMixin, acp.Agent):
|
||||
user_providers=cfg.get("providers") if isinstance(cfg.get("providers"), dict) else {},
|
||||
custom_providers=get_compatible_custom_providers(cfg))
|
||||
if not result.success:
|
||||
raise ValueError(result.error_message or f"Cannot switch to {raw_model}")
|
||||
raise ModelRejected(result.error_message or f"Cannot switch to {raw_model}")
|
||||
target_provider, new_model = result.target_provider, result.new_model
|
||||
endpoint: dict[str, Any] = {}
|
||||
if keep_endpoint and not (current_provider and target_provider != current_provider):
|
||||
@@ -1003,9 +1007,11 @@ class HermesACPAgent(SlashCommandsMixin, acp.Agent):
|
||||
try:
|
||||
_old, requested_provider, resolved_model = await asyncio.to_thread(
|
||||
self._switch_model, state, model_id, keep_endpoint=True)
|
||||
except ValueError as exc:
|
||||
except ModelRejected as exc:
|
||||
# A model no provider can serve is a bad ``modelId`` param (-32602), not an agent
|
||||
# internal error (-32603): the client attributes it to the request, not to Hermes (#72439).
|
||||
# Only the switch_model rejection maps here; a ValueError from the rebuild itself
|
||||
# (disabled provider, context window below the floor) stays on the -32603 path.
|
||||
from acp.exceptions import RequestError
|
||||
raise RequestError.invalid_params({"details": str(exc)}) from exc
|
||||
logger.info(
|
||||
|
||||
@@ -143,3 +143,14 @@ def test_acp_set_session_model_rejection_is_invalid_params_and_leaves_session_un
|
||||
with pytest.raises(RuntimeError, match="No Codex credentials"):
|
||||
agent._switch_model(state, "other")
|
||||
assert state.model == "claude-sonnet-5" and state.agent is old_agent
|
||||
|
||||
# A ValueError raised by the rebuild itself (disabled provider, context floor) is not a bad
|
||||
# ``modelId``: it must escape as-is so acp maps it to -32603, not be relabelled -32602.
|
||||
def _rebuild_value_error(**_kw):
|
||||
raise ValueError("provider 'anthropic' is disabled in config")
|
||||
|
||||
agent.session_manager._make_agent = _rebuild_value_error
|
||||
with pytest.raises(ValueError, match="disabled in config") as rebuild_exc:
|
||||
asyncio.run(agent.set_session_model("other", "s1"))
|
||||
assert not isinstance(rebuild_exc.value, RequestError)
|
||||
assert state.model == "claude-sonnet-5" and state.agent is old_agent
|
||||
|
||||
Reference in New Issue
Block a user