Skip to content

fix: sync provider order changes across all tabs #765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zerob13 merged 1 commit into from
Aug 21, 2025
Merged

fix: sync provider order changes across all tabs #765

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/renderer/src/stores/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,9 @@ export const useSettingsStore = defineStore('settings', () => {
const setupProviderListener = () => {
// 监听配置变更事件
window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
console.log('changed')
console.log('Provider changed - updating providers and order')
providers.value = await configP.getProviders()
await loadSavedOrder()
await refreshAllModels()
})
Comment on lines 656 to 661
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Wrap async IPC handler in try–catch and use structured logging

Per the repo’s TS guidelines, guard presenter calls with try–catch and log with context to avoid unhandled rejections from IPC failures.

Apply this diff:

-    window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
-      console.log('Provider changed - updating providers and order')
-      providers.value = await configP.getProviders()
-      await loadSavedOrder()
-      await refreshAllModels()
-    })
+    window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
+      try {
+        console.info('[settings][PROVIDER_CHANGED]', { ts: Date.now() })
+        providers.value = await configP.getProviders()
+        await loadSavedOrder()
+        await refreshAllModels()
+      } catch (error) {
+        console.error('[settings][PROVIDER_CHANGED][ERROR]', {
+          ts: Date.now(),
+          message: 'Failed to reload providers/order after provider change',
+          error
+        })
+      }
+    })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
console.log('changed')
console.log('Provider changed - updating providers and order')
providers.value = await configP.getProviders()
await loadSavedOrder()
await refreshAllModels()
})
window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
try {
console.info('[settings][PROVIDER_CHANGED]', { ts: Date.now() })
providers.value = await configP.getProviders()
await loadSavedOrder()
await refreshAllModels()
} catch (error) {
console.error('[settings][PROVIDER_CHANGED][ERROR]', {
ts: Date.now(),
message: 'Failed to reload providers/order after provider change',
error
})
}
})
🤖 Prompt for AI Agents 
In src/renderer/src/stores/settings.ts around lines 656 to 661, the async IPC
handler passed to window.electron.ipcRenderer.on is not wrapped in try–catch and
may produce unhandled promise rejections; update the handler to be an async
function wrapped in try { ... } catch (err) { logger.error({ context:
'PROVIDER_CHANGED handler', error: err }) } (or use the repo's structured
logger) and move calls to providers.value = await configP.getProviders(); await
loadSavedOrder(); await refreshAllModels(); inside the try block so any failure
is caught and logged with contextual information.


Expand Down Expand Up @@ -1514,6 +1515,7 @@ export const useSettingsStore = defineStore('settings', () => {

// 强制更新 providers 以触发视图更新
providers.value = [...providers.value]
await configP.setProviders(providers.value)
} catch (error) {
Comment on lines 1516 to 1519
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Sanitize before persisting providers to avoid leaking transient fields

Persisting providers.value directly may write large/ephemeral fields (e.g., websites) to config, which other code paths explicitly strip before saving. Save a sanitized, raw copy instead.

Apply this diff:

-      // 强制更新 providers 以触发视图更新
-      providers.value = [...providers.value]
-      await configP.setProviders(providers.value)
+      // 强制更新 providers 以触发视图更新
+      providers.value = [...providers.value]
+      // Persist a sanitized snapshot (strip transient fields like `websites`)
+      const sanitizedProviders = providers.value.map((p) => {
+        const raw = toRaw(p) as any
+        const { websites, ...rest } = raw
+        return rest
+      })
+      await configP.setProviders(sanitizedProviders)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 强制更新 providers 以触发视图更新
providers.value = [...providers.value]
await configP.setProviders(providers.value)
} catch (error) {
// 强制更新 providers 以触发视图更新
providers.value = [...providers.value]
// Persist a sanitized snapshot (strip transient fields like `websites`)
const sanitizedProviders = providers.value.map((p) => {
const raw = toRaw(p) as any
const { websites, ...rest } = raw
return rest
})
await configP.setProviders(sanitizedProviders)
🤖 Prompt for AI Agents 
In src/renderer/src/stores/settings.ts around lines 1516 to 1519, the code
currently persists providers.value directly which can include large/ephemeral
fields (e.g., websites); before calling await configP.setProviders(...) create a
sanitized deep-copy (map over providers.value) that strips transient/large
fields (such as websites, caches, or runtime-only metadata) and persist that
sanitized array instead, while still updating providers.value for the UI;
replace setProviders(providers.value) with setProviders(sanitizedProviders).

console.error('Failed to update provider order:', error)
}
Expand Down