Add in-session agent and model picker with runtime switching#390
Merged
Conversation
- New `AgentModelPicker` TUI overlay (press `a`) to select agent and model - `ExecutionEngine.switchToUserAgent` validates and applies changes without restart - `listModels()` added to agent plugin interface; Claude, Gemini, and Kiro implement it - `model` field persisted to StoredConfig so selection survives restarts - Rebinds `A` (shift) for "add remote"; `a` now opens the picker
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds an in-run Agent/Model picker (with optional save-as-default), a plugin model-listing API, an ExecutionEngine API to switch to a user-selected agent/model (with deferral), TUI wiring and keyboard changes, config schema/setting for top-level ChangesUser-Selected Agent Switching with Runtime Persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #390 +/- ##
==========================================
+ Coverage 48.25% 48.37% +0.12%
==========================================
Files 117 117
Lines 38723 38879 +156
==========================================
+ Hits 18685 18807 +122
- Misses 20038 20072 +34
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/commands/run.tsx (1)
269-295: ⚡ Quick winConsider adding clarifying comments for complex logic.
The code on lines 269-273 and 284-287 contains complex conditional fallback chains that would benefit from explanatory comments to clarify:
- The priority order for resolving agent names (agent → defaultAgent → default from agents array → first agent)
- Why the
shouldSwitchAgentlogic switches whenpreviousConfig === undefinedor when configs differThe
getDefaultAgentConfig(newConfig, {})call on line 290 is correct; an emptyRuntimeOptionsobject is valid here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/run.tsx` around lines 269 - 295, Add concise explanatory comments near getConfiguredAgentName and normalizeModel that state the resolution priority (agent → defaultAgent → agents.find(agent.default).name → agents[0].name) and that normalizeModel trims and treats empty strings as undefined; also add a brief comment above the shouldSwitchAgent computation explaining the two cases that trigger an agent switch (initial run when previousConfig is undefined and either a configured agent or model exists, or when previous vs new agent name or normalized model differ). Keep comments short (1-2 lines each) and mention getDefaultAgentConfig and engine.switchToUserAgent nearby to clarify that when shouldSwitchAgent is true we fetch the default agent config and call engine.switchToUserAgent with nextModel.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/index.ts`:
- Around line 1911-1913: The branch that builds completion summaries never sees
"user-selected" switches because the code only pushes switchEntry into
this.currentIterationAgentSwitches when reason !== 'user-selected'; update the
logic so that switchEntry is also added for 'user-selected' cases (either remove
the reason check or add an explicit push for reason === 'user-selected') so that
buildCompletionSummary(...) can see and process user-selected switches; refer to
currentIterationAgentSwitches, switchEntry, reason, and buildCompletionSummary
to locate and fix the code path.
---
Nitpick comments:
In `@src/commands/run.tsx`:
- Around line 269-295: Add concise explanatory comments near
getConfiguredAgentName and normalizeModel that state the resolution priority
(agent → defaultAgent → agents.find(agent.default).name → agents[0].name) and
that normalizeModel trims and treats empty strings as undefined; also add a
brief comment above the shouldSwitchAgent computation explaining the two cases
that trigger an agent switch (initial run when previousConfig is undefined and
either a configured agent or model exists, or when previous vs new agent name or
normalized model differ). Keep comments short (1-2 lines each) and mention
getDefaultAgentConfig and engine.switchToUserAgent nearby to clarify that when
shouldSwitchAgent is true we fetch the default agent config and call
engine.switchToUserAgent with nextModel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 983d39b1-963a-4b09-9c3e-1bd0d703106f
📒 Files selected for processing (28)
README.mdsrc/chat/engine.test.tssrc/commands/run.tsxsrc/config/index.tssrc/config/schema.tssrc/config/types.tssrc/engine/index.tssrc/engine/switch-to-user-agent.test.tssrc/engine/types.tssrc/logs/persistence.tssrc/logs/types.tssrc/plugins/agents/base.tssrc/plugins/agents/builtin/claude.test.tssrc/plugins/agents/builtin/claude.tssrc/plugins/agents/builtin/gemini.test.tssrc/plugins/agents/builtin/gemini.tssrc/plugins/agents/builtin/kiro.test.tssrc/plugins/agents/builtin/kiro.tssrc/plugins/agents/types.tssrc/tui/components/AgentModelPicker.test.tssrc/tui/components/AgentModelPicker.tsxsrc/tui/components/RunApp.tsxsrc/tui/components/SettingsView.tsxsrc/tui/theme.tswebsite/content/docs/cli/run.mdxwebsite/content/docs/configuration/config-file.mdxwebsite/content/docs/configuration/options.mdxwebsite/content/docs/getting-started/quick-start.mdx
- Remove guard that excluded 'user-selected' reason from switch history - Add inline comments clarifying agent resolution priority and model normalization logic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/engine/index.ts (2)
2107-2151:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock agent/model switches while an iteration is still executing.
Lines 2137-2151 mutate
this.agent,config.agent,config.model, and rate-limit state immediately. IfcurrentExecutionis still the previous handle, the rest of that iteration is attributed to the wrong agent/model.Suggested fix
async switchToUserAgent( agentConfig: AgentPluginConfig, model: string | undefined ): Promise<void> { + if (this.currentExecution) { + throw new Error('Cannot switch agent/model while an iteration is running'); + } + const normalizedModel = model?.trim() ? model.trim() : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/engine/index.ts` around lines 2107 - 2151, switchToUserAgent is mutating this.agent, this.config.agent, this.config.model, this.primaryAgentInstance, this.rateLimitConfig, this.rateLimitedAgents and this.state.currentModel immediately which can misattribute an ongoing iteration; detect if an iteration is in progress by checking the existing execution handle (e.g., this.currentExecution or similar) and do not apply the swap immediately—either queue the requested agent/model switch (store the new agentConfig, normalizedModel and rateLimit settings in a pendingSwap object) and return, or await completion of the currentExecution before applying the mutations; implement applying the pending swap at the end of the iteration (or when currentExecution resolves/rejects) and ensure switchAgent(agentConfig.plugin, 'user-selected', previousAgent) is invoked only after the swap is applied.
2151-2151:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
user-selectedcompletion summary is still unreachable in the intended flow.
switchToUserAgent()is explicitly for subsequent iterations, but the entry pushed at Line 2151 is cleared by thecurrentIterationAgentSwitches = []reset at Line 937 before the next iteration starts. That leaves Lines 2277-2279 dead for normal picker usage.Also applies to: 2277-2279
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/engine/index.ts` at line 2151, The pushed 'user-selected' switch is being cleared by the reset of currentIterationAgentSwitches, making the user-selected completion unreachable; update the logic so that instead of pushing into currentIterationAgentSwitches at the time of calling this.switchAgent(agentConfig.plugin, 'user-selected', previousAgent) you either (A) move that push into the persistent nextIterationAgentSwitches (or equivalent longer-lived buffer) so it survives the reset, or (B) call the existing switchToUserAgent(...) helper (or extend it) so the user-selected entry is scheduled after the reset; adjust code around switchAgent, switchToUserAgent, and currentIterationAgentSwitches to ensure the 'user-selected' entry persists into the next iteration rather than being cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/engine/index.ts`:
- Around line 2123-2133: The code incorrectly calls
newInstance.validateModel('') when normalizedModel is undefined, which forces
plugins to reject an empty-string override; instead, in the else/clear-override
path either skip validation or call newInstance.validateModel(undefined) so the
initialize() default-model path remains valid—update the block that references
normalizedModel and newInstance.validateModel to avoid passing '' (use undefined
or omit the call) and throw only if validateModel actually returns an error for
a real override.
---
Duplicate comments:
In `@src/engine/index.ts`:
- Around line 2107-2151: switchToUserAgent is mutating this.agent,
this.config.agent, this.config.model, this.primaryAgentInstance,
this.rateLimitConfig, this.rateLimitedAgents and this.state.currentModel
immediately which can misattribute an ongoing iteration; detect if an iteration
is in progress by checking the existing execution handle (e.g.,
this.currentExecution or similar) and do not apply the swap immediately—either
queue the requested agent/model switch (store the new agentConfig,
normalizedModel and rateLimit settings in a pendingSwap object) and return, or
await completion of the currentExecution before applying the mutations;
implement applying the pending swap at the end of the iteration (or when
currentExecution resolves/rejects) and ensure switchAgent(agentConfig.plugin,
'user-selected', previousAgent) is invoked only after the swap is applied.
- Line 2151: The pushed 'user-selected' switch is being cleared by the reset of
currentIterationAgentSwitches, making the user-selected completion unreachable;
update the logic so that instead of pushing into currentIterationAgentSwitches
at the time of calling this.switchAgent(agentConfig.plugin, 'user-selected',
previousAgent) you either (A) move that push into the persistent
nextIterationAgentSwitches (or equivalent longer-lived buffer) so it survives
the reset, or (B) call the existing switchToUserAgent(...) helper (or extend it)
so the user-selected entry is scheduled after the reset; adjust code around
switchAgent, switchToUserAgent, and currentIterationAgentSwitches to ensure the
'user-selected' entry persists into the next iteration rather than being
cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: caa09c8a-3fc0-42ee-b6db-038ed6e41108
📒 Files selected for processing (2)
src/commands/run.tsxsrc/engine/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/run.tsx
- Queue switchToUserAgent calls in pendingUserAgentSwap when an execution is running - Apply pending swap after execution finishes via applyPendingUserAgentSwap - Carry user-selected switch entries into the next iteration's tracking buffer - Drop empty-model validation when clearing model override (undefined → no call)
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
AgentModelPickeroverlay component (pressa) for switching the active agent and model without restarting the sessionExecutionEngine.switchToUserAgent()— validates agent availability and model compatibility before mutating engine statelistModels()to theAgentPlugininterface; implemented for Claude (sonnet/opus/haiku), Gemini (2.5-pro/2.5-flash), and Kiro (four model aliases); defaults to empty array for open-ended agentsmodelfield toStoredConfigschema so the model override can be persisted via the picker's "save as default" togglea→ open agent/model picker (local tab only);A→ add new remote instance (previouslya)propagateSettingsToEnginetoasyncso agent/model switches are validated before config is persisted; errors surface to the caller rather than silently failing'user-selected'as a newActiveAgentReason, with matching log formatting and iteration summary textTesting
switchToUserAgentcovering: successful switch with state mutation and event emission, rejection on invalid model without state mutation, rejection on detect failure without state mutationAgentModelPickerhelpers: agent config resolution, model listing, model validation, and model normalizationClaudeAgentPlugin.listModels,GeminiAgentPlugin.listModels, andKiroAgentPlugin.listModelsengine.test.tsupdated to includelistModelsstub on mock agentsTab,j/k,Enter,Escape), and "save as default" toggle recommended before mergeSummary by CodeRabbit
New Features
a) to switch agent and model mid-run; supports free-text models and validation.Documentation
aopens agent/model picker,Aadds remote instances.model.