fixed: mcpstore is not init finished and server config load undefined #599
Conversation
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
Contributor
WalkthroughThe changes introduce a deferred loading mechanism for configuration data in three Vue components related to knowledge settings. Instead of calling configuration load functions immediately on mount, these are now scheduled to run after a short delay using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VueComponent
participant MCPStore
User->>VueComponent: Mounts component
VueComponent-->>VueComponent: setTimeout(delay)
Note right of VueComponent: Waits for delay (100ms or 1000ms)
VueComponent->>MCPStore: loadConfigFromMcp()
MCPStore-->>VueComponent: Returns config
VueComponent-->>User: UI updates with loaded config
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Closed
![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 (2)
src/renderer/src/components/settings/RagflowKnowledgeSettings.vue (2)
378-378: Remove debugging console.log from production code.The console.log statement appears to be leftover debugging code and should be removed from production.
- console.log('加载RAGFlow配置:', serverConfig)
435-440: Fix typo in comment and consider standardizing the approach.There's a typo in the comment: "deply" should be "delay". Also, consider adding error handling for the async call.
onMounted(() => { - // deply load serverconfig, ensure mcpStore is ready + // delay load server config to ensure mcpStore is ready setTimeout(() => { - loadRagflowConfigFromMcp() + loadRagflowConfigFromMcp().catch(console.error) }, 100) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/renderer/src/components/settings/DifyKnowledgeSettings.vue(1 hunks)src/renderer/src/components/settings/FastGptKnowledgeSettings.vue(1 hunks)src/renderer/src/components/settings/RagflowKnowledgeSettings.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/renderer/src/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/i18n.mdc
src/renderer/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/project-structure.mdc
🧠 Learnings (3)
src/renderer/src/components/settings/RagflowKnowledgeSettings.vue (4)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:24:10.749Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Implement proper state persistence for maintaining data across sessions
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:24:10.749Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Utilize actions for side effects and asynchronous operations
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-06-30T12:23:45.479Z
Learning: Applies to src/renderer/src/**/* : Do not hardcode user-facing text in code; always use the translation system (vue-i18n) for all user-visible strings
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-06-23T13:06:15.336Z
Learning: Use <script setup> syntax for concise Vue 3 component definitions.
src/renderer/src/components/settings/FastGptKnowledgeSettings.vue (4)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:24:10.749Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Utilize actions for side effects and asynchronous operations
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-06-23T13:06:15.336Z
Learning: Use <script setup> syntax for concise Vue 3 component definitions.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-06-23T13:06:15.335Z
Learning: Leverage Nuxt's built-in performance optimizations, including Suspense for async components and lazy loading for routes and components.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/main/presenter/**/*.ts : Optimize application startup time with lazy loading
src/renderer/src/components/settings/DifyKnowledgeSettings.vue (1)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:24:10.749Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Utilize actions for side effects and asynchronous operations
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
Comment on lines
406
to
408
| onMounted(() => { | ||
| // delay load server info to ensure mcpStore is ready | ||
| setTimeout(() => { | ||
| loadDifyConfigFromMcp() | ||
| }, 100) // 延时打开面板，确保UI渲染完成 | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a more robust approach for handling store initialization timing.
While the setTimeout approach addresses the immediate timing issue, it has some limitations:
- The 100ms delay is arbitrary and doesn't guarantee
mcpStoreis ready - No error handling for the async
loadDifyConfigFromMcp()call - The comment mentions UI rendering, but this is primarily about store readiness
Consider these alternatives:
- Watch for
mcpStoreinitialization state - Use a computed property that depends on store readiness
- Implement proper lifecycle management in the store itself
-onMounted(() => {
- // delay load server info to ensure mcpStore is ready
- setTimeout(() => {
- loadDifyConfigFromMcp()
- }, 100) // 延时打开面板，确保UI渲染完成
-})
+onMounted(() => {
+ // Watch for mcpStore readiness instead of arbitrary delay
+ const unwatch = watch(
+ () => mcpStore.isReady,
+ (isReady) => {
+ if (isReady) {
+ loadDifyConfigFromMcp().catch(console.error)
+ unwatch()
+ }
+ },
+ { immediate: true }
+ )
+})📝 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
| onMounted(() => { | |
| // delay load server info to ensure mcpStore is ready | |
| setTimeout(() => { | |
| loadDifyConfigFromMcp() | |
| }, 100) // 延时打开面板，确保UI渲染完成 | |
| }) | |
| onMounted(() => { | |
| // Watch for mcpStore readiness instead of arbitrary delay | |
| const unwatch = watch( | |
| () => mcpStore.isReady, | |
| (isReady) => { | |
| if (isReady) { | |
| loadDifyConfigFromMcp().catch(console.error) | |
| unwatch() | |
| } | |
| }, | |
| { immediate: true } | |
| ) | |
| }) |
🤖 Prompt for AI Agents
In src/renderer/src/components/settings/DifyKnowledgeSettings.vue around lines
406 to 411, replace the setTimeout delay used to wait for mcpStore readiness
with a more reliable approach. Instead of an arbitrary 100ms delay, watch for a
reactive property or state in mcpStore that indicates it is fully initialized,
and trigger loadDifyConfigFromMcp() only when that state is true. Also, add
error handling for the async loadDifyConfigFromMcp() call to catch and handle
any failures. Update or clarify the comment to reflect that the logic is about
store readiness rather than UI rendering timing.
Comment on lines
415
to
417
| onMounted(() => { | ||
| // delay load server config to ensure mcpStore is ready | ||
| setTimeout(() => { | ||
| loadFastGptConfigFromMcp() | ||
| }, 1000) | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address inconsistent delay timing across knowledge settings components.
The 1000ms delay is significantly longer than the 100ms used in DifyKnowledgeSettings.vue. This inconsistency suggests arbitrary delay values rather than a principled approach to handling store initialization timing.
Consider:
- Standardizing the delay across all knowledge settings components
- Using a more reliable method to detect store readiness
- Adding error handling for the async call
-onMounted(() => {
- // delay load server config to ensure mcpStore is ready
- setTimeout(() => {
- loadFastGptConfigFromMcp()
- }, 1000)
-})
+onMounted(() => {
+ // Use consistent delay and add error handling
+ setTimeout(() => {
+ loadFastGptConfigFromMcp().catch(console.error)
+ }, 100) // Standardize to 100ms like other components
+})📝 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
| onMounted(() => { | |
| // delay load server config to ensure mcpStore is ready | |
| setTimeout(() => { | |
| loadFastGptConfigFromMcp() | |
| }, 1000) | |
| }) | |
| onMounted(() => { | |
| // Use consistent delay and add error handling | |
| setTimeout(() => { | |
| loadFastGptConfigFromMcp().catch(console.error) | |
| }, 100) // Standardize to 100ms like other components | |
| }) |
🤖 Prompt for AI Agents
In src/renderer/src/components/settings/FastGptKnowledgeSettings.vue around
lines 415 to 420, the setTimeout delay for loading the config is 1000ms, which
is inconsistent with the 100ms delay used in DifyKnowledgeSettings.vue. To fix
this, standardize the delay to 100ms for consistency, or better, replace the
timeout with a reactive or event-based check to detect when mcpStore is ready
before calling loadFastGptConfigFromMcp. Additionally, wrap the async call in a
try-catch block to handle potential errors gracefully.
zerob13
requested changes
Jul 14, 2025
Collaborator
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
use watch once and immediate
https://vuejs.org/guide/essentials/watchers.html#once-watchers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Pull Request Description
Is your feature request related to a problem? Please describe.
#551
In Setting -> KnowledgeBase Tab, server config is load undefined beacuse when mcpserver is initing and component load the server config, so it will be undefined.
Describe the solution you'd like
settimeout about 100ms to wait mcpserver load finished, next load server info.
UI/UX changes for Desktop Application
nothing
Platform Compatibility Notes
nothing
Additional context
#551
Summary by CodeRabbit