fix(acp): expose session model selector via configOptions

acp_adapter/server.py

@@ -49,6 +49,8 @@
     SessionForkCapabilities,
     SessionInfoUpdate,
     SessionListCapabilities,
+    SessionConfigOptionSelect,
+    SessionConfigSelectOption,
     SessionMode,
     SessionModeState,
     SessionModelState,
@@ -620,6 +622,7 @@ class HermesACPAgent(acp.Agent):
     )
 
     _EDIT_APPROVAL_POLICY_CONFIG_ID = "edit_approval_policy"
+    _MODEL_CONFIG_ID = "model"
     _EDIT_APPROVAL_POLICY_DEFAULT = "ask"
     _MODE_DEFAULT = "default"
     _MODE_ACCEPT_EDITS = "accept_edits"
@@ -819,6 +822,58 @@ def _build_model_state(self, state: SessionState) -> SessionModelState | None:
             current_model_id=fallback_choice,
         )
 
+    def _session_config_options(
+        self,
+        state: SessionState,
+        model_state: "SessionModelState | None" = None,
+    ) -> list[SessionConfigOptionSelect]:
+        """Expose the session model list as an ACP ``configOptions`` selector.
+
+        Newer ACP clients (Zed and others) render their per-session model
+        picker from ``configOptions`` with ``category: "model"`` and drive
+        changes through ``session/set_config_option``. The older
+        ``SessionModelState`` (``models``) field is kept in the session
+        responses for backwards compatibility, but on its own it no longer
+        produces a visible selector in those clients.
+
+        ``model_state`` may be passed in by callers that already built it (the
+        session responses do, via ``_build_model_state``) so the potentially
+        network-bound inventory is assembled once per request rather than
+        twice. When omitted it is built on demand.
+        """
+        if model_state is None:
+            model_state = self._build_model_state(state)
+        if model_state is None or not getattr(model_state, "available_models", None):
+            return []
+
+        options: list[SessionConfigSelectOption] = []
+        for model in model_state.available_models:
+            value = getattr(model, "model_id", "") or ""
+            if not value:
+                continue
+            options.append(
+                SessionConfigSelectOption(
+                    value=value,
+                    name=getattr(model, "name", None) or value,
+                    description=getattr(model, "description", None),
+                )
+            )
+        if not options:
+            return []
+
+        current_value = getattr(model_state, "current_model_id", None) or options[0].value
+        return [
+            SessionConfigOptionSelect(
+                type="select",
+                id=self._MODEL_CONFIG_ID,
+                name="Model",
+                category="model",
+                description="Model used for this session (does not change your global default).",
+                current_value=current_value,
+                options=options,
+            )
+        ]
+
     @staticmethod
     def _resolve_model_selection(raw_model: str, current_provider: str) -> tuple[str, str]:
         """Resolve ``provider:model`` input into the provider and normalized model id."""
@@ -1346,10 +1401,12 @@ async def new_session(
         logger.info("New session %s (cwd=%s)", state.session_id, cwd)
         self._schedule_available_commands_update(state.session_id)
         self._schedule_usage_update(state)
+        model_state = self._build_model_state(state)
         return NewSessionResponse(
             session_id=state.session_id,
-            models=self._build_model_state(state),
+            models=model_state,
             modes=self._session_modes(state),
+            config_options=self._session_config_options(state, model_state),
             field_meta=self._provenance_meta(
                 state.session_id, getattr(state.agent, "session_id", state.session_id)
             ),
@@ -1394,9 +1451,11 @@ async def load_session(
             )
         self._schedule_available_commands_update(session_id)
         self._schedule_usage_update(state)
+        model_state = self._build_model_state(state)
         return LoadSessionResponse(
-            models=self._build_model_state(state),
+            models=model_state,
             modes=self._session_modes(state),
+            config_options=self._session_config_options(state, model_state),
             field_meta=self._provenance_meta(
                 session_id, getattr(state.agent, "session_id", session_id)
             ),
@@ -1429,9 +1488,11 @@ async def resume_session(
             )
         self._schedule_available_commands_update(state.session_id)
         self._schedule_usage_update(state)
+        model_state = self._build_model_state(state)
         return ResumeSessionResponse(
-            models=self._build_model_state(state),
+            models=model_state,
             modes=self._session_modes(state),
+            config_options=self._session_config_options(state, model_state),
             field_meta=self._provenance_meta(
                 state.session_id, getattr(state.agent, "session_id", state.session_id)
             ),
@@ -2396,6 +2457,32 @@ async def set_config_option(
         if str(config_id) == self._EDIT_APPROVAL_POLICY_CONFIG_ID:
             mode = self._EDIT_APPROVAL_POLICY_TO_MODE.get(str(value), self._MODE_DEFAULT)
             setattr(state, "mode", mode)
+        elif str(config_id) == self._MODEL_CONFIG_ID:
+            # Session-scoped model switch — mirrors set_session_model and the
+            # ``/model`` slash command. Only touches this session's state and
+            # its persisted session row; never writes config.yaml.
+            current_provider = getattr(state.agent, "provider", None) or "openrouter"
+            requested_provider, resolved_model = self._resolve_model_selection(
+                str(value), current_provider
+            )
+            provider_changed = bool(
+                current_provider and requested_provider != current_provider
+            )
+            current_base_url = (
+                None if provider_changed else getattr(state.agent, "base_url", None)
+            )
+            current_api_mode = (
+                None if provider_changed else getattr(state.agent, "api_mode", None)
+            )
+            state.model = resolved_model
+            state.agent = self.session_manager._make_agent(
+                session_id=session_id,
+                cwd=state.cwd,
+                model=resolved_model,
+                requested_provider=requested_provider,
+                base_url=current_base_url,
+                api_mode=current_api_mode,
+            )
         else:
             options = getattr(state, "config_options", None)
             if not isinstance(options, dict):
@@ -2404,4 +2491,6 @@ async def set_config_option(
             setattr(state, "config_options", options)
         self.session_manager.save_session(session_id)
         logger.info("Session %s: config option %s updated", session_id, config_id)
-        return SetSessionConfigOptionResponse(config_options=[])
+        return SetSessionConfigOptionResponse(
+            config_options=self._session_config_options(state)
+        )

contributors/emails/quan@toviacapital.com

@@ -0,0 +1,2 @@
+quanrees
+# PR: ACP model configOptions selector

tests/acp/test_server.py

@@ -61,7 +61,12 @@ def agent(mock_manager):
 async def test_new_session_exposes_edit_approvals_as_modes_not_config_options(agent):
     resp = await agent.new_session(cwd="/tmp")
 
-    assert resp.config_options is None
+    # Edit-approval policies remain modes, not config options. The model
+    # selector is advertised via configOptions (category: "model"); assert the
+    # config options carry no edit-approval entry rather than being empty.
+    config_ids = {opt.id for opt in (resp.config_options or [])}
+    assert "edit_approval_policy" not in config_ids
+    assert config_ids <= {"model"}
     assert isinstance(resp.modes, SessionModeState)
     assert resp.modes.current_mode_id == "default"
     assert [(mode.id, mode.name) for mode in resp.modes.available_modes] == [
@@ -82,7 +87,10 @@ async def test_set_config_option_persists_edit_approval_policy_without_advertisi
     state = agent.session_manager.get_session(resp.session_id)
 
     assert isinstance(update, SetSessionConfigOptionResponse)
-    assert update.config_options == []
+    # Per ACP spec, the response echoes the complete config-option set. The
+    # edit-approval policy is still applied via modes, so it must not appear as
+    # a config option, but the model selector may.
+    assert all(opt.id != "edit_approval_policy" for opt in update.config_options)
     assert getattr(state, "mode", None) == "accept_edits"
 
 
@@ -390,7 +398,10 @@ async def test_router_accepts_stable_session_config_methods(self, agent):
         )
 
         assert mode_result == {}
-        assert config_result["configOptions"] == []
+        assert all(
+            opt.get("id") != "edit_approval_policy"
+            for opt in config_result["configOptions"]
+        )
 
 
 

tests/acp_adapter/test_acp_model_config_option.py

@@ -0,0 +1,192 @@
+"""Tests for the ACP ``configOptions`` model selector (category: model).
+
+Newer ACP clients (e.g. Zed) render their per-session model picker from
+``configOptions`` rather than the legacy ``SessionModelState`` field. These
+tests assert that Hermes advertises a ``category: "model"`` select option on
+session creation and that ``session/set_config_option`` performs a
+session-scoped model switch without mutating the global default.
+"""
+
+from types import SimpleNamespace
+
+import pytest
+
+from acp_adapter.server import HermesACPAgent
+from acp_adapter.session import SessionManager
+
+from tests.acp_adapter.test_acp_commands import (
+    CaptureConn,
+    FakeAgent,
+    NoopDb,
+    make_agent_and_state,
+)
+
+
+def _install_model_state(monkeypatch, current_id="fake-provider:model-a"):
+    """Force a deterministic model inventory for _build_model_state."""
+    from acp.schema import ModelInfo, SessionModelState
+
+    def fake_build(self, state):
+        return SessionModelState(
+            available_models=[
+                ModelInfo(
+                    model_id="fake-provider:model-a",
+                    name="Fake Provider · model-a",
+                    description="Provider: Fake Provider • current",
+                ),
+                ModelInfo(
+                    model_id="fake-provider:model-b",
+                    name="Fake Provider · model-b",
+                    description="Provider: Fake Provider",
+                ),
+            ],
+            current_model_id=current_id,
+        )
+
+    monkeypatch.setattr(HermesACPAgent, "_build_model_state", fake_build)
+
+
+@pytest.mark.asyncio
+async def test_new_session_advertises_model_config_option(monkeypatch):
+    _install_model_state(monkeypatch)
+    fake = FakeAgent()
+    manager = SessionManager(agent_factory=lambda **kwargs: fake, db=NoopDb())
+    acp_agent = HermesACPAgent(session_manager=manager)
+
+    response = await acp_agent.new_session(cwd=".")
+
+    assert response.config_options, "expected configOptions on new_session"
+    model_opt = next(
+        (o for o in response.config_options if o.id == "model"), None
+    )
+    assert model_opt is not None
+    assert model_opt.category == "model"
+    assert model_opt.type == "select"
+    assert model_opt.current_value == "fake-provider:model-a"
+    assert [o.value for o in model_opt.options] == [
+        "fake-provider:model-a",
+        "fake-provider:model-b",
+    ]
+
+
+@pytest.mark.asyncio
+async def test_new_session_still_sends_legacy_models_field(monkeypatch):
+    """The legacy ``models`` field must remain for older ACP clients."""
+    _install_model_state(monkeypatch)
+    fake = FakeAgent()
+    manager = SessionManager(agent_factory=lambda **kwargs: fake, db=NoopDb())
+    acp_agent = HermesACPAgent(session_manager=manager)
+
+    response = await acp_agent.new_session(cwd=".")
+
+    assert response.models is not None
+    assert response.models.available_models
+
+
+@pytest.mark.asyncio
+async def test_set_model_config_option_switches_session_model(monkeypatch):
+    _install_model_state(monkeypatch)
+    acp_agent, state, _fake, _conn = make_agent_and_state()
+
+    # Route _resolve_model_selection deterministically (avoid live catalogs).
+    monkeypatch.setattr(
+        HermesACPAgent,
+        "_resolve_model_selection",
+        staticmethod(lambda raw, prov: ("fake-provider", raw.split(":", 1)[-1])),
+    )
+
+    response = await acp_agent.set_config_option(
+        session_id=state.session_id,
+        config_id="model",
+        value="fake-provider:model-b",
+    )
+
+    # Session state reflects the switch.
+    assert state.model == "model-b"
+    # Response echoes the full configOptions list (spec requires complete set).
+    assert response.config_options
+    assert any(o.id == "model" for o in response.config_options)
+
+
+@pytest.mark.asyncio
+async def test_set_model_config_option_does_not_write_global_config(monkeypatch):
+    """A model configOption switch must never persist to config.yaml."""
+    _install_model_state(monkeypatch)
+    acp_agent, state, _fake, _conn = make_agent_and_state()
+
+    monkeypatch.setattr(
+        HermesACPAgent,
+        "_resolve_model_selection",
+        staticmethod(lambda raw, prov: ("fake-provider", raw.split(":", 1)[-1])),
+    )
+
+    import hermes_cli.config as hermes_config
+
+    saved = {"called": False}
+
+    def _tripwire(*_args, **_kwargs):
+        saved["called"] = True
+        raise AssertionError("config.yaml must not be written on session model switch")
+
+    # Any attempt to persist global config during the switch fails the test.
+    if hasattr(hermes_config, "save_config"):
+        monkeypatch.setattr(hermes_config, "save_config", _tripwire, raising=False)
+    if hasattr(hermes_config, "save_config_value"):
+        monkeypatch.setattr(hermes_config, "save_config_value", _tripwire, raising=False)
+
+    await acp_agent.set_config_option(
+        session_id=state.session_id,
+        config_id="model",
+        value="fake-provider:model-b",
+    )
+
+    assert saved["called"] is False
+    assert state.model == "model-b"
+
+
+@pytest.mark.asyncio
+async def test_set_edit_approval_policy_still_returns_config_options(monkeypatch):
+    """Non-model config ids keep working and now echo the full option set."""
+    _install_model_state(monkeypatch)
+    acp_agent, state, _fake, _conn = make_agent_and_state()
+
+    response = await acp_agent.set_config_option(
+        session_id=state.session_id,
+        config_id="edit_approval_policy",
+        value="accept_edits",
+    )
+
+    assert response.config_options is not None
+
+
+@pytest.mark.asyncio
+async def test_config_option_response_always_includes_model_option(monkeypatch):
+    """The set_config_option response must carry the complete option set.
+
+    Zed and other ACP clients refresh their selectors from the full
+    configOptions list returned by session/set_config_option, so the model
+    option must be present after both a model switch and a non-model
+    (edit-approval) update.
+    """
+    _install_model_state(monkeypatch)
+    acp_agent, state, _fake, _conn = make_agent_and_state()
+
+    monkeypatch.setattr(
+        HermesACPAgent,
+        "_resolve_model_selection",
+        staticmethod(lambda raw, prov: ("fake-provider", raw.split(":", 1)[-1])),
+    )
+
+    after_model = await acp_agent.set_config_option(
+        session_id=state.session_id,
+        config_id="model",
+        value="fake-provider:model-b",
+    )
+    assert any(o.id == "model" for o in after_model.config_options)
+
+    after_edit = await acp_agent.set_config_option(
+        session_id=state.session_id,
+        config_id="edit_approval_policy",
+        value="accept_edits",
+    )
+    assert any(o.id == "model" for o in after_edit.config_options)