fix(kiro): use session/set_model for in-session model switching#4
Merged
Merged
Conversation
Kiro supports session/set_model but NOT session/set_config_option. AcpSessionRuntime.setModel routes through setConfigOption, which Kiro rejects with -32601 "Method not found". Call session/set_model directly via the raw request interface in KiroAdapter, keeping shared ACP infrastructure untouched so Cursor (which uses setConfigOption) is unaffected. Also updates session.model immediately after a successful switch (matching ClaudeAdapter/OpenCodeAdapter pattern) and removes the redundant model assignment from the post-prompt session update. Adds 3 integration tests: - switches model in-session without restarting the process - updates session.model immediately after in-session model switch - does not call setModel when model is unchanged between turns Mock agents updated with handleSetSessionModel for forward compat.
github-actions
Bot
added
vouch:trusted
hrishikeshmane
pushed a commit
that referenced
this pull request
Apr 25, 2026
Supersedes PR #4's set_model work with a corrected implementation covering both models (`session/set_model`) and agents (`session/set_mode`), and closes the first-turn alignment gotcha PR #4 had. Key learnings from debugging sessions on threads e3a5c1bf (ValidationException), 55afbe27 (silent model swap no-op), 23f518fe (session/load ignores --model), 3e377f8b (UI not rendering post-respawn reply), and 593e16ae (final working state with 3 set_mode + 4 set_model + 7 prompts, zero respawns): - Kiro's `session/set_model` RPC only works if kiro-cli acp was spawned WITHOUT the `--model` flag. Passing `--model <slug>` at spawn locks the model and makes later set_model calls silently no-op. - Kiro's `session/set_mode` RPC is the agent-equivalent of set_model. Kirodex uses it (connection.setSessionMode) and it works. Agent switching no longer needs to respawn kiro-cli, eliminating the UI artifact where post-respawn replies didn't render cleanly. - effect-acp's agent SDK doesn't wire `session/set_mode` through its core handler dispatcher. `handleExtRequest("session/set_mode", ...)` is the correct hook on the mock side. - AcpSessionRuntime.setConfigOption routes through `session/set_config_option` which Kiro rejects with -32601. We bypass the upstream helper and issue set_model / set_mode via the raw request channel (`ctx.acp.request(method, payload)`) — same pattern as PR #4. - `ctx.activeMode` and `ctx.activeModel` track the agent/model Kiro has confirmed via RPC. Both init to `undefined` at spawn; first turn fires the RPC to align Kiro's state with the user's selection even when the user-chosen value matches what spawn-`--agent` set (closes PR #4's first-turn no-op). Behavior: - Single kiro-cli child process per thread. No respawns on agent or model change. - Initial spawn still passes `--agent <name>` as a hint so first turn doesn't need set_mode if user stuck with the default. `--model` is deliberately NOT passed. - Known Kiro-side limitation: switching across model families mid- conversation (e.g., Claude → DeepSeek) can surface Bedrock `ValidationException` from history replay. This is a Kiro product bug to be filed upstream; our client surfaces the error rather than masking it. Tests: - Added regression tests for first-turn set_model/set_mode alignment - Inverted prior PR #4 test whose invariant was "don't respawn on model change" (correct) but failed to assert the set_model RPC - Dropped `--model` spawn-arg tests (invariant reversed) - Agent-respawn test replaced with set_mode-in-session test - Mock agents extended with handleExtRequest("session/set_mode", ...) Verified end-to-end on thread 593e16ae: 1 initialize, 3 set_mode, 4 set_model, 7 prompts, 0 failures, 0 respawns. 935 server tests pass, bun typecheck clean.
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
session/set_modelbut notsession/set_config_option. The sharedAcpSessionRuntime.setModelroutes throughsetConfigOption, which Kiro rejects with-32601 Method not found. The ndJsonRpc parser wraps this as aDie(defect), which the previousEffect.catchcouldn't intercept — crashing the turn.session/set_modeldirectly viactx.acp.request()in KiroAdapter, keeping shared ACP infrastructure untouched so Cursor (which usessetConfigOption) is unaffected.ctx.session.modelimmediately after a successful switch, matching the ClaudeAdapter/OpenCodeAdapter pattern. Previously the model was only updated after the prompt completed.modelfrom post-prompt session update.Files changed
KiroAdapter.tssession/set_modelRPC directly; immediate model state updateKiroAdapter.integration.test.tskiro-mock-agent.tsset_model/set_config_optionhandlersacp-mock-agent.tshandleSetSessionModelfor forward compatTest plan