feat: filter /switch-model to show only authenticated models#1776
Closed
singbong wants to merge 4 commits intocode-yeongyu:devfrom
Closed
feat: filter /switch-model to show only authenticated models#1776singbong wants to merge 4 commits intocode-yeongyu:devfrom
singbong wants to merge 4 commits intocode-yeongyu:devfrom
Conversation
- Added model_candidates: z.array(z.string()).optional() to AgentOverrideConfigSchema - Updated JSON schema generation (build-schema.ts uses correct Zod 4 method) - Maintains backward compatibility (field is optional) - JSON schema now includes model_candidates field Preparation for Task 3: Runtime state management module
…l override - createSisyphusAgent: add agentName?: string parameter - createHephaestusAgent: add agentName?: string parameter - createAtlasAgent: add agentName to OrchestratorContext - Each create function uses getActiveModel(agentName) for override - Builtin-agents pass agentName when creating agents This enables runtime model switching for Sisyphus, Hephaestus, and Atlas through getActiveModel() integration with the model-switcher state module.
- Add switch-model command template with model switching instructions - Update BuiltinCommandName type to include 'switch-model' - Update BuiltinCommandNameSchema enum to include 'switch-model' - Add switch-model to BUILTIN_COMMAND_DEFINITIONS with argumentHint The command provides: - Current model assignments display for all agents - Model candidates listing for specific agents - Model switching instructions - Validation for model candidates - Clear error messages for invalid models Template includes detailed instructions for parsing arguments, getting current model info, validating inputs, and performing model switches using model-switcher APIs.
- Test loadCandidates() parsing with/without/empty model_candidates - Test setActiveModel() and getActiveModel() override functionality - Test getCandidates() and getCurrentModelInfo() state queries - Test backward compatibility (config without model_candidates) - Test invalid agent name handling - 12 test cases all passing Tests use unique agent names to avoid singleton state interference between test cases.
Contributor
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with: This is a one-time requirement. Once signed, all your future contributions will be automatically accepted. I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
There was a problem hiding this comment.
1 issue found across 25 files
Confidence score: 4/5
- This PR looks safe to merge, with the main risk coming from test reliability rather than production behavior.
- Shared mutable state in
src/features/model-switcher/model-switcher.test.tscould cause flaky or order-dependent tests if run in parallel or with other suites. - Pay close attention to
src/features/model-switcher/model-switcher.test.ts- shared mutable state without isolation or cleanup.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/features/model-switcher/model-switcher.test.ts">
<violation number="1" location="src/features/model-switcher/model-switcher.test.ts:1">
P2: Shared mutable state in tests without isolation or cleanup mechanism</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,169 @@ | |||
| import { describe, expect, test } from "bun:test" | |||
There was a problem hiding this comment.
P2: Shared mutable state in tests without isolation or cleanup mechanism
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/model-switcher/model-switcher.test.ts:
<comment>Shared mutable state in tests without isolation or cleanup mechanism</comment>
<file context>
@@ -0,0 +1,169 @@
+import { describe, expect, test } from "bun:test"
+import type { AgentOverrides } from "../../agents/types"
+
+import {
+ getCurrentModelInfo,
+ getCandidates,
+ getActiveModel,
+ setActiveModel,
+ loadCandidates,
</file context>
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.
Problem
Currently, the
/switch-modelcommand displays all available models frommodel_candidatesconfiguration, regardless of whether they are connected via auth login or not.This causes confusion and potential issues:
Solution
Add authentication-aware filtering to
/switch-modelcommand to only show and allow switching to models that are connected/authenticated in the current OpenCode session.Implementation Plan
Add OpenCode client API call
model_candidatesbased on availabilityUpdate /switch-model template
Add utility functions
getAvailableModels(agentName: string): Filter candidates by availabilityfilterAvailableModels(candidates: string[]): Get only available models from listUsage
Breaking Changes
None. This adds filtering only; existing functionality remains unchanged.
Technical Notes
getConnectedProviders()or similarSummary by cubic
Add session-scoped model switching with a new /switch-model command and runtime overrides, so agents can switch to valid, configured models without touching config files.
New Features
Dependencies
Written for commit 530a571. Summary will update on new commits.