Skip to content

fix(acp): expose session model selector via configOptions - #75358

Open
quanrees wants to merge 1 commit into
NousResearch:mainfrom
quanrees:fix/acp-model-configoptions
Open

fix(acp): expose session model selector via configOptions#75358
quanrees wants to merge 1 commit into
NousResearch:mainfrom
quanrees:fix/acp-model-configoptions

Conversation

@quanrees

Copy link
Copy Markdown

What does this PR do?

Restores the per-session model selector in ACP editor clients (Zed and others) by advertising the model list through the ACP-standard configOptions mechanism.

Problem: Newer ACP clients render their per-session model picker from configOptions (a select option with category: "model"), driven by session/set_config_option. Hermes only sent the legacy SessionModelState (models) field, which these clients no longer render — so the model selector disappeared from the editor UI. Users on Zed are left unable to switch models per-session from the panel and must fall back to typing exact /model <version> strings. (Same class of regression reported upstream for other ACP agents: zed-industries/zed #59197, #59096, #59169, #59202.)

Fix:

  • Advertise a configOptions model selector (category: "model") on session/new, session/load, and session/resume, built from the existing cross-provider inventory (_build_model_state) so it stays in sync with the legacy field.
  • Retain the legacy models field for older clients (non-breaking).
  • Handle session/set_config_option for configId="model" as a session-scoped switch that mirrors set_session_model and the /model command: it only mutates session state and persists the session row, and never writes config.yaml.
  • Return the complete configOptions set from set_config_option, per the ACP spec.
  • Build _build_model_state once per session response and reuse it for both surfaces to avoid rebuilding the potentially network-bound inventory twice.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

How to Test

  1. python -m pytest tests/acp/ tests/acp_adapter/ -q → 141 passed.
  2. In a configOptions-aware ACP client such as Zed, open a Hermes ACP thread; the model selector should list Provider · model entries.
  3. Select a different model; only the current session should switch and the global ~/.hermes/config.yaml default should remain unchanged.

Platform tested: macOS.

Checklist

  • Conventional Commit
  • Searched existing PRs; no open PR advertises a category: "model" configOption. feat(acp): advertise thought-level session config #48444 covers thought_level only.
  • Only changes related to this fix
  • pytest tests/acp/ tests/acp_adapter/ -q passes (141)
  • Added tests
  • Tested on macOS

Newer ACP clients (Zed and others) render their per-session model picker
from the ACP-standard configOptions mechanism (a select option with
category: "model"), driven by session/set_config_option. Hermes only sent
the legacy SessionModelState (models) field, which those clients no longer
render, so the model selector disappeared from the editor.

Advertise a configOptions model selector on session/new, session/load and
session/resume, built from the existing cross-provider inventory
(_build_model_state) so it stays in sync with the legacy field, which is
retained for older clients. Handle session/set_config_option for
configId="model" as a session-scoped switch that mirrors set_session_model
and the /model command: it only mutates session state and persists the
session row, never writing config.yaml. The response returns the complete
configOptions set per the ACP spec.

_build_model_state is now built once per session response and reused for
both surfaces to avoid rebuilding the (potentially network-bound) inventory
twice.
@alt-glitch alt-glitch added type/bug Something isn't working P4 Best-effort: we will get to it when we get to it (no commitment) comp/acp Agent Communication Protocol adapter area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 31, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this to the ACP config-options surface. The premise is verified on current main: acp_adapter/server.py:1349-1356, 1397-1403, and 1432-1438 return the legacy models field without config_options, and set_config_option returns an empty option list at acp_adapter/server.py:2387-2407.

Problems

  • The change misses the fork lifecycle path. ACP's ForkSessionResponse has an initial configOptions field (/home/teknium/.local/lib/python3.13/site-packages/acp/schema.py:2429-2449), but acp_adapter/server.py:1461-1479 returns only models and modes; the PR leaves that response unchanged. A forked session would therefore not advertise the model config selector.

Suggested changes

  • Populate ForkSessionResponse.config_options from the same model state and add a fork-response regression test.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/acp Agent Communication Protocol adapter P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants