feat(acp): typed switchAcpModel client method#189
Merged
Conversation
Wraps the agent-server POST /conversations/{id}/switch_acp_model endpoint
(OpenHands/software-agent-sdk#3390) so consumers (e.g. agent-canvas) call a
typed method instead of raw-fetching the route. The ACP analog of switchLlm —
mirrors its surface (ConversationClient + RemoteConversation).
- ConversationClient.switchAcpModel(conversationId, model)
- RemoteConversation.switchAcpModel(model)
Body is { model } (matches the endpoint's embedded `model` field); returns
void on Success. Server-side errors (400 non-ACP, 404 missing, 409 before the
session exists) surface as HttpError to the caller.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
15 tasks
all-hands-bot
approved these changes
May 26, 2026
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean, simple implementation that correctly mirrors the existing switchLlm pattern.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Purely additive client method that wraps a new agent-server endpoint. Mirrors the existingswitchLlmimplementation with appropriate tests. No breaking changes, no security concerns, no dependency changes.
VERDICT:
✅ Worth merging: Well-implemented feature addition with consistent API design
KEY INSIGHT:
Consistency in API design - this method follows the exact same pattern as switchLlm, making it intuitive for developers already familiar with the codebase.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/typescript-client/actions/runs/26457027996
The switchAcpModel unit tests mock fetch, so nothing verified the method against a live agent-server that actually has the /switch_acp_model route (added in software-agent-sdk #3390). Close that gap: - Bump the integration AGENT_SERVER_IMAGE to an image containing #3390 (main @ 9d35320; bump to the SDK release image once it's cut). - Add a deterministic integration test: switchAcpModel on a non-ACP conversation must return 400 (not 404) — proving the route exists on the pinned image and the client targets the right path/body. No ACP credentials or real model switch needed, so it runs in the deterministic group. Verified locally against an agent-server built from #3390: the new test plus the existing deterministic-api suite (4/4) pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Resolved two conflicts (both "added at the same spot"): - integration-tests.yml: keep AGENT_SERVER_IMAGE at 9d35320 (the #3390 merge, which has /switch_acp_model) rather than main's 71b070d (v1.23.1, predates #3390); the switchAcpModel integration test needs the route. - deterministic-api.integration.test.ts: keep BOTH new tests — main's profile round-trip test and this branch's switchAcpModel contract test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…th integration tests)
software-agent-sdk #3390 added supports_runtime_model_switch to ACP_PROVIDERS (true for claude-code, codex, gemini-cli). The validate-acp-providers drift check tracks SDK main, so the ts-side mirror must include it. Add the field to the ACPProviderInfo interface and to acp-providers.json. It's the registry flag that gates whether switchAcpModel is offered for a provider. Verified: scripts/check-acp-drift.py passes against the #3390 SDK; build, eslint, and 213 unit tests green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
2 participants
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 a typed
switchAcpModelclient method wrapping the agent-server endpointPOST /conversations/{id}/switch_acp_model(OpenHands/software-agent-sdk#3390), so consumers — notably agent-canvas — call a typed method instead of raw-fetching the route. It's the ACP analog ofswitchLlmand mirrors its surface.Part of the unified-ACP-model-selection effort tracked in OpenHands/agent-canvas#769.
Changes
ConversationClient.switchAcpModel(conversationId, model)RemoteConversation.switchAcpModel(model)Both
POST { model }(matching the endpoint's embeddedmodelbody field) and resolvevoidonSuccess. Server-side errors surface asHttpError: 400 (non-ACP conversation), 404 (unknown conversation), 409 (no ACP session yet — set the default via settings before the first message instead).Validation
npm run build(tsc) clean;npm run format:checkclean;npm run lint0 errors.npx jest api-clients→ 41 passed (2 new:ConversationClient.switchAcpModel,RemoteConversation.switchAcpModel).Notes
switchLlm's placement (client +RemoteConversation); deliberately not added toConversationManager, which doesn't exposeswitchLlmeither. Happy to add a manager passthrough if preferred.🤖 Generated with Claude Code