feat(acp): support model and mode switching#1478
Open
mawag wants to merge 3 commits into
Open
Conversation
Add /model and /mode support for ACP agents, covering both selector mechanisms in the ACP ecosystem: - the early models/modes blocks (session/set_model, session/set_mode): kiro-cli, mimo, codebuddy, hermes - the newer configOptions RFD (session/set_config_option): opencode Selectors from either mechanism are normalised into the same caches. Live switching on an active session routes to the matching RPC via the new core.LiveModelSwitcher interface (mode reuses LiveModeSwitcher). When no session is active, a one-shot probe (initialize + session/new) fetches the selectors so /model and /mode can list options without starting a session first; it uses an independent 20s timeout so slow cold starts (e.g. hermes ~11.4s) are not bounded by the caller's 10s ctx. Verified end-to-end against kiro-cli, mimo, codebuddy, hermes and opencode. Unit tests use neutral fixtures; integration tests are gated on CC_ACP_BIN and pick switch targets dynamically from the live list.
mawag
force-pushed
the
feat/acp-model-mode-switch
branch
from
023c4d6 to
9ee570c
Compare
…t to 30s When an agent advertises both the legacy models/modes blocks and the newer configOptions selectors, configOptions now wins deterministically and order-independently: it is absorbed first and records its configId, and the legacy-block absorbers skip any selector whose configId is already set (so a block can no longer overwrite a configOptions selector, which previously left an inconsistent state — the switch routed via set_config_option while the listed models came from the block). Also raise sessionConfigProbeTimeout 20s -> 30s. The probe is an upper bound, not a fixed wait — fast agents return in well under a second and are unaffected. The slowest tested agent (hermes, 108 models from a dynamic backend) has a highly variable probe that was observed above 20s under load, causing intermittent timeouts where /model showed "not supported"; 30s reliably covers it. Adds TestSession_bothMechanisms_configOptionsWins to lock down the order-independent precedence. Re-verified end-to-end against kiro, mimo, codebuddy, hermes, opencode and copilot.
…e failure Add TestSession_mixedMechanisms_modelCfgModeBlock covering the case where an agent provides a model selector via configOptions but a mode selector only via the legacy modes block: the two must resolve independently (model from configOptions, mode from the block), order-independently. Also log (slog.Warn) when probeSessionConfig fails to parse the session/new response instead of returning silently, so a malformed response is diagnosable rather than surfacing only as /model and /mode reporting "not supported".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
/modeland/modesupport for ACP agents. cc-connect now reads amodel/mode selector list from the ACP agent and switches live on the active
session, covering both selector mechanisms in the ACP ecosystem — the early
models/modesblocks (session/set_model/session/set_mode) and thenewer
configOptionsRFD (session/set_config_option). Previously ACPagents could not list or switch models/modes.
Type of change
Testing
Verified end-to-end against six real ACP agents:
models/modesblocksmodels/modesblocksmodels/modesblocksmodels/modesblocksconfigOptionsmodesblock (mode-only)Graceful degradation is covered too: copilot exposes only a mode selector
(no models), so
/modelreports "not supported" while/modeworks — and itsmodeIds are full ACP spec URLs (e.g.
.../session-modes#plan), which switchcorrectly since the matcher makes no assumption about id format.
go test -race ./agent/acp/andgo test ./core/pass. Integration tests aregated on
CC_ACP_BIN(skip cleanly in CI) and pick switch targets dynamicallyfrom the live list — no model/mode names are hard-coded.
Automated tests added in this PR
Unit tests (
agent/acp/models_test.go, neutral fixtures):TestAgent_reportModels_populatesCache/TestAgent_GetModel_NoModels/TestAgent_SetModel_PendingWinsOverCurrent/TestAgent_GetModel_FallbackToServerCurrent— model cache & precedenceTestAgent_PermissionModes_returnsCachedModes/TestAgent_PermissionModes_skipsProbeWhenProbed/TestAgent_SharedProbeFlag/TestAgent_ensureProbe_noCmdNoSpawn— probe gatingTestSession_absorbModels_*/TestSession_matchAvailableModel/TestSession_maybeAbsorbCurrentModelUpdate*— models block pathTestSession_absorbConfigOptions_model/TestSession_absorbConfigOptions_mode/TestSession_maybeAbsorbConfigOptionUpdate/TestFlattenModelOptions_flatAndGrouped— configOptions pathTestAgent_ConcurrentModelAccess—-raceconcurrency guardIntegration tests (
agent/acp/acp_integration_test.go, gated onCC_ACP_BIN):TestIntegration_ACP_ModelSwitch/TestIntegration_ACP_ModeSwitch— full list→switch path against a real agentTestIntegration_ACP_ProbeShortCtx— locks down that the probe uses its own timeout, not the caller's short 10s ctxFor bug fixes only — regression test
N/A (new feature). Note:
TestIntegration_ACP_ProbeShortCtxwas added to lockdown a probe-timeout regression discovered during development (a slow cold
start exceeding the caller's 10s ctx).
Critical User Journeys (CUJ) impact
C — agent execution control (
/mode)F — config switching (
/model)go test ./core/ -run TestCUJpasses locally.Change is additive: non-ACP agents fall through the existing code path
unchanged, so existing CUJ behavior is preserved.
Manual / user-visible behavior change
For ACP agents that expose model/mode selectors,
/modeland/modenow listthe available options and switch live (no session restart), including before
the first session (a one-shot probe fetches the list). Agents exposing neither
report "not supported" as before. Non-ACP agents are unaffected.
Checklist (reviewer will verify)
go build ./core/... ./agent/... ./config/... ./platform/...passes(note:
go build ./...requires a prebuiltweb/dist, unrelated to this change)go test ./...passes for touched packages (agent/acp,core) incl.-racegolangci-lint run --new-from-rev origin/main ./...passes (0 issues)core/(uses the newcore.LiveModelSwitchercapability interface + type assertion)/model/modei18n keys)Related
None.