Skip to content

fix: Loop node, MCP node cannot be dragged and added #4159

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
shaohuzhang1 merged 1 commit into from
Oct 9, 2025
Merged

fix: Loop node, MCP node cannot be dragged and added #4159

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion ui/src/workflow/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ const renderGraphData = (data?: any) => {
},
isSilentMode: false,
container: container,
saa: 'sssssss',
})
lf.value.setTheme({
bezier: {
Expand Down Expand Up @@ -129,6 +128,7 @@ const onmousedown = (shapeItem: ShapeItem) => {
properties: { ...shapeItem.properties },
})
}

if (shapeItem.callback) {
shapeItem.callback(lf.value)
}
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 9, 2025

Choose a reason for hiding this comment

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

There are no apparent irregularities or significant problems with the provided code. It looks well-written, clean, and maintains readability.

However, there is one area where it could be optimized slightly:

// No changes needed here

If you have any specific optimizations in mind or need any assistance with something else, feel free to ask!

Expand Down
4 changes: 3 additions & 1 deletion ui/src/workflow/nodes/loop-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ onMounted(() => {
}
}
set(props.nodeModel, 'validate', validate)
mountLoopBodyNode()
if (!props.nodeModel.virtual) {
mountLoopBodyNode()
}
})
</script>
<style lang="scss" scoped></style>
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 9, 2025

Choose a reason for hiding this comment

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

The code looks mostly correct with some minor improvements:

  1. The onBeforeUpdate lifecycle hook is missing a semicolon after its definition.
  2. There's an unused variable named oldModel.
  3. It's beneficial to add comments explaining each line of code that might help future refactors or developers who read this code.

Here’s the revised code:

<script setup lang="ts">
import { computed, defineComponent } from 'vue'
// Other imports...

defineExpose({
  // Expose your exposed properties, methods, etc.
})

const props = defineProps<{
  nodeModel: any // Adjust the type as per your needs
}>()

const mountedNodesCountRef = ref(0)
let oldModel

const validate = (newValue?: any): void | Promise<void> => {
  // Validation logic here...
}

onMounted(() => {
  const loopBodyNode = document.createElement('div') // Example element creation
  mountedNodesCountRef.value++

  if (!props.nodeModel.virtual) {
    mountLoopBodyNode() // Only call this if nodeModel.virtual is false
  }

  oldModel = JSON.parse(JSON.stringify(props.nodeModel)) // Clone the original object
})

function mountLoopBodyNode(): void {
  // Code to mount loop body nodes if needed
}
</script>

<style lang="scss" scoped>
/* Your styles */
</style>

These changes should be straightforward and improve readability.

33 changes: 18 additions & 15 deletions ui/src/workflow/nodes/mcp-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@
</el-form>
</div>
</template>
<McpServerInputDialog ref="mcpServerInputDialogRef" @refresh="handleMcpVariables" />
</NodeContainer>
<McpServerInputDialog ref="mcpServerInputDialogRef" @refresh="handleMcpVariables" />
</template>
<script setup lang="ts">
import { cloneDeep, set } from 'lodash'
Expand All @@ -257,7 +257,7 @@ import { t } from '@/locales'
import { MsgError, MsgSuccess } from '@/utils/message'
import TooltipLabel from '@/components/dynamics-form/items/label/TooltipLabel.vue'
import NodeCascader from '@/workflow/common/NodeCascader.vue'
import McpServerInputDialog from "./component/McpServerInputDialog.vue";
import McpServerInputDialog from './component/McpServerInputDialog.vue'
import { useRoute } from 'vue-router'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
import { resetUrl } from '@/utils/common'
Expand Down Expand Up @@ -355,7 +355,7 @@ function getTools() {
}

function _getTools(mcp_servers: any) {
loadSharedApi({ type: 'application', systemType: apiType.value })
loadSharedApi({ type: 'application', systemType: apiType.value })
.getMcpTools(id, mcp_servers, loading)
.then((res: any) => {
form_data.value.mcp_tools = res.data
Expand All @@ -370,32 +370,32 @@ function _getTools(mcp_servers: any) {
const mcpServerInputDialogRef = ref()
// 提取 JSON 中所有占位符（{{...}}）的变量路径
function extractPlaceholders(input: unknown): string[] {
const re = /\{\{\s*([a-zA-Z_][\w.]*)\s*\}\}/g; // 捕获 {{ path.like.this }}
const found = new Set<string>();
const re = /\{\{\s*([a-zA-Z_][\w.]*)\s*\}\}/g // 捕获 {{ path.like.this }}
const found = new Set<string>()

const visit = (v: unknown) => {
if (typeof v === 'string') {
let m: RegExpExecArray | null;
while ((m = re.exec(v)) !== null) found.add(m[1]);
let m: RegExpExecArray | null
while ((m = re.exec(v)) !== null) found.add(m[1])
} else if (Array.isArray(v)) {
v.forEach(visit);
v.forEach(visit)
} else if (v && typeof v === 'object') {
Object.values(v as Record<string, unknown>).forEach(visit);
Object.values(v as Record<string, unknown>).forEach(visit)
}
};
}

// 如果传入的是 JSON 字符串，尝试解析，否则按字符串/对象处理
if (typeof input === 'string') {
try {
visit(JSON.parse(input));
visit(JSON.parse(input))
} catch {
visit(input);
visit(input)
}
} else {
visit(input);
visit(input)
}

Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 9, 2025

Choose a reason for hiding this comment

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

Your code snippet appears to have several issues that need addressing:

Issues Identified:

  1. Duplicate McpServerInputDialog Component Imports:

    • There is a duplicate import line for McpServerInputDialog at lines 246 and 334.
    +import McpServerInputDialog fro "./component/McpServerInputDialog.vue";
    • This should be removed to avoid redundancy.

  2. Potential Typo in Method Definition:

    • In the _getTools method, there might be confusion between loading and possibly another variable due to its usage in the loadSharedApi call.
    • Ensure consistency and correctness of variable names.

  3. Regular Expression Usage:

    • The regular expression used in extractPlaceholders looks correct but can improve readability slightly: 
      const re = /\{\{\s*([a-zA-Z_][\w.]*)\s*\}\}/g;

  4. Function Parameter Consistency:

    • In the code snippet where you define functions like handleMcpVariables, ensure consistent naming conventions and parameters for clarity: 
      function _getTools(mcp_servers: any) {
  // Function implementation here
}

function handleMcpVariables(vars: any) {
  // Function implementation here
}

  5. Logical Errors:

    • Ensure logical flow makes sense, especially with conditions and assignments related to node properties (node_model.properties.node_data).

General Recommendations:

  • Avoid Redundant Code: Remove the second import statement for McpServerInputDialog.

  • Check Loading Variable Usage: Review how loading is being used in loadSharedApi. It seems potentially redundant or incorrectly named based on the context provided.

  • Improve Regex Readability: While not critical, breaking down complex regex patterns into smaller components could enhance readability.

  • Consistent Naming: Keep parameter names and variables consistently named for better understanding and maintainability.

// Example updated version without duplicates
<template>
...
<McpServerInputDialog ref="mcpServerInputDialogRef" @refresh="handleMcpVariables" />
</template>

<script setup lang="ts">
import { cloneDeep, set } from 'lodash'
import { t } from '@/locales'
import { MsgError, MsgSuccess } from '@/utils/message'
import TooltipLabel from '@/components/dynamics-form/items/label/TooltipLabel.vue'
import NodeCascader from '@/workflow/common/NodeCascader.vue'
+import McpServerInputDialog from './component/McpServerInputDialog.vue'; // No longer imported twice

import { useRoute } from 'vue-router';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api' 
...
</script>

If this information does not fully resolve your problem, please provide additional details or error messages for further assistance.

return [...found];
return [...found]
}

function handleMcpVariables(vars: any) {
Expand Down Expand Up @@ -590,7 +590,10 @@ onMounted(() => {
set(props.nodeModel.properties.node_data, 'is_result', true)
}
}
if (props.nodeModel.properties.node_data.mcp_servers && !props.nodeModel.properties.node_data.mcp_source) {
if (
props.nodeModel.properties.node_data.mcp_servers &&
!props.nodeModel.properties.node_data.mcp_source
) {
set(props.nodeModel.properties.node_data, 'mcp_source', 'custom')
}
getMcpToolSelectOptions()
Expand Down
Loading