Migrate Claude ACP adapter to @agentclientprotocol/claude-agent-acp (#257)#280
Open
vanzue wants to merge 4 commits into
Open
Migrate Claude ACP adapter to @agentclientprotocol/claude-agent-acp (#257)#280vanzue wants to merge 4 commits into
vanzue wants to merge 4 commits into
Conversation
…257) The Claude ACP adapter package @zed-industries/claude-code-acp is deprecated and renamed to @agentclientprotocol/claude-agent-acp. Beyond the rename, the new adapter (>= 0.24) advertises its model selector via NewSessionResponse.config_options (a Select with category=Model) instead of the legacy models field, and only accepts model switches via session/set_config_option (session/set_model returns -32601 Method not found). A bare rename therefore regresses the model picker (empty dropdown + failed switching). This change: - Renames the adapter command in both the C++ resolver (TerminalPage.cpp, AIAgentsViewModel.cpp) and the Rust registry (agent_registry.rs), plus docs and comments. - Adds protocol/acp/model_select.rs with models_from_new_session() (legacy models field, falling back to config_options[category=Model]) and apply_session_model() (routes to set_session_model or set_session_config_option based on the channel the agent advertised). One wta process drives one agent CLI, so the channel is recorded process-wide at extraction and read back at hot-swap. - Replaces the six duplicated model-extraction blocks and three switch sites in client.rs/probe.rs with the shared helper. Verified: live wta probe-models against the new package now returns the model list (previously empty); unit + registry tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the Claude ACP adapter from the deprecated @zed-industries/claude-code-acp package to @agentclientprotocol/claude-agent-acp, and updates the WTA ACP client to support the adapter’s newer “config_options”-based model enumeration and model switching so the model dropdown and hot-swap behavior don’t regress.
Changes:
- Rename the Claude ACP adapter command across C++, Rust, and docs to
npx -y @agentclientprotocol/claude-agent-acp. - Add shared ACP model extraction + model-switch routing that supports both legacy
NewSessionResponse.modelsandNewSessionResponse.config_options. - Replace duplicated legacy model-extraction / set-model call sites with the new helper.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/wta/src/protocol/acp/probe.rs | Uses shared model extraction helper during probe-models. |
| tools/wta/src/protocol/acp/model_select.rs | New shared logic for extracting models (legacy vs config-options) and applying model changes. |
| tools/wta/src/protocol/acp/mod.rs | Exposes the new model_select module. |
| tools/wta/src/protocol/acp/client.rs | Routes model extraction + model switching through model_select helpers (direct + over-pipe + hot-apply). |
| tools/wta/src/main.rs | Updates CLI help/example to reference the new Claude adapter package. |
| tools/wta/src/coordinator.rs | Updates adapter-launch examples/tests to the new Claude adapter package. |
| tools/wta/src/agent_registry.rs | Updates the Claude ACP launch command to the new adapter package. |
| tools/wta/AGENTS.md | Updates documented wta --agent "npx -y ..." example for Claude. |
| src/cascadia/TerminalSettingsEditor/AIAgentsViewModel.cpp | Updates C++ resolver command string for Claude adapter launch. |
| src/cascadia/TerminalApp/TerminalPage.cpp | Updates C++ resolver command string for Claude adapter launch. |
| src/cascadia/inc/AgentRegistry.h | Updates header comment describing which adapter package Claude uses. |
| doc/installing-dependencies.md | Updates dependency/install docs and npmjs link to the new Claude adapter package. |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
✅ C++ build verification: full incremental |
… spelling
- model_select.rs: model_option_from_config() now uses find_map to pick the
first config option that is BOTH a model selector (category=Model or
id="model") AND a Select, instead of bailing on the first id/category match
regardless of kind. Adds a category-only (id != "model") extraction test.
- client.rs: reword the two propagated model-set errors and the hot-swap warn
to be channel-agnostic ("failed to set requested model" / "model hot-update
failed"), since the call may route through session/set_config_option, not
just session/set_model.
- patterns.txt: add the @agentclientprotocol/claude-agent-acp npm package as a
check-spelling pattern (the scope token can't be reworded).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
The patterns.txt entry @agentclientprotocol/claude-agent-acp covers every
package reference, but a comment in agent_registry.rs used the bare scope as
a prose word ('agentclientprotocol-maintained'), which check-spelling still
flagged. Per repo convention (reword prose rather than allowlist), changed it
to 'ACP-project-maintained'. No other bare occurrences remain in code; the
@.../claude-agent-acp package refs stay covered by the pattern, and the
pre-existing agentclientprotocol.com / NOTICE.md URLs are unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MODEL_CONFIG_ID was a first-writer-wins OnceLock, so the recorded config-option
id could never change once set. With an in-process agent restart (run_inner can
reconnect a different agent) or a session whose model selector advertises a
non-"model" config id, a later new_session could legitimately need a different
id, and model switching would keep sending the stale one.
Replace the AtomicU8 channel flag + OnceLock id with a single
RwLock<ModelSwitchChannel> enum (Legacy | Config { config_id }) that is
overwritten on every models_from_new_session() extraction. apply_session_model
snapshots it under the read lock (released before the await, since the guard
isn't Send) and routes accordingly. Test step 4 now asserts the id updates to
the new session's selector id ("llm") instead of staying frozen at "model".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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
Fixes #257. The Claude ACP adapter package
@zed-industries/claude-code-acpis deprecated and renamed to@agentclientprotocol/claude-agent-acp.Beyond the rename, the new adapter (>= 0.24, currently 0.44) changed how it advertises models:
NewSessionResponse.models(legacySessionModelState) intoNewSessionResponse.config_options(aSelectoption withcategory=Model).session/set_modeltosession/set_config_option— the new adapter returns-32601 Method not foundforsession/set_model(verified live).So a bare package rename would ship a regression: empty model dropdown + broken model switching. This PR handles both.
Changes
Part A — rename (both sides hardcode the command and must match):
TerminalPage.cpp,AIAgentsViewModel.cppagent_registry.rs(+coordinator.rstest)installing-dependencies.md,AgentRegistry.h,main.rs,AGENTS.mdPart B — config_options model support (new
tools/wta/src/protocol/acp/model_select.rs):models_from_new_session(): prefers legacymodels, falls back toconfig_options[category=Model].apply_session_model(): routes toset_session_modelorset_session_config_optionbased on the channel the agent advertised (recorded process-wide — one wta = one agent CLI).client.rs/probe.rswith the shared helper.The
agent-client-protocol0.10 crate already exposesconfig_options+set_session_config_option, so no crate bump was needed.Verification
wta probe-modelsagainst the new package now returns the 4-model list (was 0 before).cargo buildgreen; new unit testmodel_extraction_across_channels(real Claude wire JSON) +agent_registry/coordinatortests pass.