fix: address CodeRabbitAI review feedback

- Replace busy-wait loop in refreshModels with Promise-based approach
- Remove duplicate error logging in opencode-models.ts handlers
- Fix multi-slash parsing in provider-icon.tsx (only handle exactly one slash)
- Use dynamic icon resolution for selected OpenCode model in trigger
- Fix misleading comment about merge precedence (static takes precedence)
- Add enabledOpencodeModels and opencodeDefaultModel to settings sync
- Add clarifying comments about session-only dynamic model settings

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Stefan de Vogelaere
2026-01-11 22:03:59 +01:00
committed by DhanushSantosh
parent 20cc401238
commit edcc4e789b
4 changed files with 65 additions and 52 deletions

View File

@@ -122,7 +122,6 @@ export function createRefreshOpencodeModelsHandler() {
res.json(response);
} catch (error) {
logger.error('Refresh OpenCode models failed:', error);
logError(error, 'Refresh OpenCode models failed');
res.status(500).json({
success: false,
@@ -155,7 +154,6 @@ export function createGetOpencodeProvidersHandler() {
res.json(response);
} catch (error) {
logger.error('Get OpenCode providers failed:', error);
logError(error, 'Get OpenCode providers failed');
res.status(500).json({
success: false,

View File

@@ -444,56 +444,64 @@ function getUnderlyingModelIcon(model?: AgentModel | string): ProviderIconKey {
// Check for dynamic OpenCode provider models (provider/model format)
// e.g., zai-coding-plan/glm-4.5, github-copilot/gpt-4o, google/gemini-2.5-pro
if (modelStr.includes('/')) {
const modelName = modelStr.split('/')[1] || '';
// Check model name for known patterns
if (modelName.includes('glm')) {
return 'glm';
// Only handle strings with exactly one slash (not URLs or paths)
if (!modelStr.includes('://')) {
const slashIndex = modelStr.indexOf('/');
if (slashIndex !== -1 && slashIndex === modelStr.lastIndexOf('/')) {
const providerName = modelStr.slice(0, slashIndex);
const modelName = modelStr.slice(slashIndex + 1);
// Skip if either part is empty
if (providerName && modelName) {
// Check model name for known patterns
if (modelName.includes('glm')) {
return 'glm';
}
if (
modelName.includes('claude') ||
modelName.includes('sonnet') ||
modelName.includes('opus')
) {
return 'anthropic';
}
if (modelName.includes('gpt') || modelName.includes('o1') || modelName.includes('o3')) {
return 'openai';
}
if (modelName.includes('gemini')) {
return 'gemini';
}
if (modelName.includes('grok')) {
return 'grok';
}
if (modelName.includes('deepseek')) {
return 'deepseek';
}
if (modelName.includes('llama')) {
return 'meta';
}
if (modelName.includes('qwen')) {
return 'qwen';
}
if (modelName.includes('mistral')) {
return 'mistral';
}
// Check provider name for hints
if (providerName.includes('google')) {
return 'gemini';
}
if (providerName.includes('anthropic')) {
return 'anthropic';
}
if (providerName.includes('openai')) {
return 'openai';
}
if (providerName.includes('xai')) {
return 'grok';
}
// Default for unknown dynamic models
return 'opencode';
}
}
if (
modelName.includes('claude') ||
modelName.includes('sonnet') ||
modelName.includes('opus')
) {
return 'anthropic';
}
if (modelName.includes('gpt') || modelName.includes('o1') || modelName.includes('o3')) {
return 'openai';
}
if (modelName.includes('gemini')) {
return 'gemini';
}
if (modelName.includes('grok')) {
return 'grok';
}
if (modelName.includes('deepseek')) {
return 'deepseek';
}
if (modelName.includes('llama')) {
return 'meta';
}
if (modelName.includes('qwen')) {
return 'qwen';
}
if (modelName.includes('mistral')) {
return 'mistral';
}
// Check provider name for hints
const providerName = modelStr.split('/')[0] || '';
if (providerName.includes('google')) {
return 'gemini';
}
if (providerName.includes('anthropic')) {
return 'anthropic';
}
if (providerName.includes('openai')) {
return 'openai';
}
if (providerName.includes('xai')) {
return 'grok';
}
// Default for unknown dynamic models
return 'opencode';
}
// Check for Cursor-specific models with underlying providers

View File

@@ -44,6 +44,8 @@ const SETTINGS_FIELDS_TO_SYNC = [
'phaseModels',
'enabledCursorModels',
'cursorDefaultModel',
'enabledOpencodeModels',
'opencodeDefaultModel',
'autoLoadClaudeMd',
'keyboardShortcuts',
'mcpServers',

View File

@@ -584,10 +584,13 @@ export interface AppState {
codexEnableImages: boolean; // Enable image processing
// OpenCode CLI Settings (global)
// Static OpenCode settings are persisted via SETTINGS_FIELDS_TO_SYNC
enabledOpencodeModels: OpencodeModelId[]; // Which static OpenCode models are available
opencodeDefaultModel: OpencodeModelId; // Default OpenCode model selection
// Dynamic models are session-only (not persisted) because they're discovered at runtime
// from `opencode models` CLI and depend on current provider authentication state
dynamicOpencodeModels: ModelDefinition[]; // Dynamically discovered models from OpenCode CLI
enabledDynamicModelIds: string[]; // Which dynamic models are enabled (model IDs)
enabledDynamicModelIds: string[]; // Which dynamic models are enabled (session-only)
cachedOpencodeProviders: Array<{
id: string;
name: string;
@@ -2036,6 +2039,8 @@ export const useAppStore = create<AppState & AppActions>()((set, get) => ({
: state.enabledOpencodeModels.filter((m) => m !== model),
})),
setDynamicOpencodeModels: (models) => {
// Dynamic models are session-only (not persisted to server) because they depend on
// current CLI authentication state and are re-discovered each session
// When setting dynamic models, auto-enable all of them if enabledDynamicModelIds is empty
const currentEnabled = get().enabledDynamicModelIds;
const newModelIds = models.map((m) => m.id);