feat: add MCP tool support with new form and dropdown options

[shaohuzhang1]

feat: add MCP tool support with new form and dropdown options

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The code appears to be functional but contains some improvements that could enhance its readability and maintainability:

Improvements Suggestions

1. Code Optimization:

   • Use more descriptive function names to improve code readability.
   • Refactor duplicate code blocks into separate functions.

2. Consistency:

   • Ensure consistent indentation throughout the file.

3. Security and Input Validation:

   • Validate inputs strictly.
   • Sanitize and sanitize outputs appropriately.

4. Performance:

   • Optimize event handling and lifecycle methods.

5. Accessibility: Ensure compliance with accessibility standards.

Here's an optimized version of the code with these improvements:

<template>
  <el-drawer v-model="isVisible" size="60%" :before-close="handleClose">
    <template #header>
      <h4>{{ title }}</h4>
    </template>

    <div>
      <!-- Base Info -->
      <h4 class="title-decoration-1 mb-16">{{ $t('views.model.modelForm.title.baseInfo') }}</h4>
      <el-form
        ref="formRef"
        :model="formData"
        :rules="formRules"
        label-position="top"
        require-asterisk-position="right"
        v-loading:loading="isLoading"
        @submit.prevent
      >
        <el-form-item :label="$t('views.tool.form.toolName.label')" prop="name">
          <div class="flex w-full">
            <el-avatar
              v-if="formData.id"
              class="edit-avatar mr-12"
              @mouseenter="toggleEditIcon(true)"
              @mouseleave="toggleEditIcon(false)"
            >
              <AvatarComponent
                v-if="isAppIcon(formData.icon)"
                shape="square"
                size="32"
              />
              <AvatarComponent v-else class="avatar-green" shape="square" size="32"/>
              <AvatarComponent
                v-if="isEditActive"
                shape="square"
                class="edit-mask"
                size="32"
                @click="openEditAvatar"
              />
            </el-avatar>
            <el-image v-else class="avatar-green mr-12" fit="cover" lazy src="@/assets/workflow/icon_tool.svg" alt="" height="32"></el-image>
            <ElInput
              v-model="formData.name"
              :placeholder="$t('views.tool.form.toolName.placeholder')"
              maxlength="64"
              show-word-limit
              @blur="handleBlur('name')"
            />
          </div>
        </el-form-item>

        <!-- Description -->
        <el-form-item :label="$t('views.tool.form.toolDescription.label')">
          <ElInput
            v-model="formData.desc"
            type="textarea"
            placeholder="$t('views.tool.form.toolDescription.placeholder')"
            max-length="128"
            show-word-limit
            :autosize="{ minRows: 3 }"
            @blur="handleBlur('desc')"
          />
        </el-form-item>
      </el-form>

      <!-- MCP Config -->
      <h4 class="title-decoration-1 mb-16">
        {{ $t('views.tool.form.mcp.label') }} *
        <ElText type="info" class="color-secondary">{{ $t('views.tool.form.mcp.tip') }}</ElText>
      </h4>
      
      <div class="mb-8">
        <ElInpu
          v-model="formData.code"
          :placeholder="mcpServerJSON"
          type="textarea"
          autosize="{ minRows: 5 }"
        />
      </div>
    </div>

    <template slot="footer">
      <div>
        <ElButton :disabled="isLoading" @click="handleCancel">{{ $t('common.cancel') }}</ElButton>
        <ElButton 
          type="primary" 
          @click="handleSubmit" 
          :disabled="!hasValidInputs()"
          v-if="isEditing || canCreate"
        >
          {{ isEditing ? $t('common.save') : $t('common.create') }}
        </ElButton>
      </div>
    </template>
    
    <EditAvatarDialog ref="editarAvatarModal" @on-refresh="updateFormDataFromAvatar"/>
  </el-drawer>
</template>

<script setup lang="ts">
import { computed, onMounted, reactive, ref, watchEffect } from 'vue';
import EditAvatarDialog from './component/EditAvatarDialog.vue'; // Assuming this import exists
import AvatarComponent from '@/components/avatar/Avatar.component' // Placeholder for actual avatar component
import type { FormData } from "@/api/types/tool"; // Assume this exists
import type { ElForm } from "element-plus";
import { msgConfirm, msgSuccess } from '@/utils/message'
import _ from 'lodash';
import { t } from '@/i18n';
import { isAppIcon } from '@/utils/common';
import router from '@/router';

const route = useRouter();
const props = defineProps<{ title: string }>();

const folder = computed(() => store.folder);
const user = computed(() => store.user);

const apiType = computed(() => {
  if (router.currentRoute.value.path.includes("shared")) {
    return "systemShare";
  } else if (router.currentRoute.value.path.includes("resource-management")) {
    return "systemManage";
  } else {
    return "workspace";
  }
});

const permissionPrecise = computed(() => permissionMap["tool"][apiType.value]);

const emit = defineEmits(["refresh"]);
const editarAvatarModal = ref();

const mcpServerJson = `
{
  "math": {
    "url": "your_server",
    "transport": "sse"
  }
}`;

const formRef = ref<Form | undefined>();
let formData = reactive<FormData>({
  name: "",
  desc: "",
  code: "",
  icon: "", // Assuming this property exists in your form data
  input_field_list: [],
  init_field_list: [],
  tool_type: "MCP",
});
const isLoading = ref(false);
const isVisible = ref(false);
let isEditActive = ref(false);

watchEffect(async () => {
  if (isVisible.value) {
    await loadSharedApi({ type: "tool", systemType: apiType.value }).getToolDetails(folder.currentFolder?.id ?? null, formRefs.value!.model)
      .then(response => {
        if ("detail" in response) {
          formData = _.cloneDeep(response.detail);
        }
      });
  }
});

const resetForm = (): void => {
  formData = reactive<FormData>({
    name: "",
    desc: "",
    code: "",
    icon: "",
    input_field_list: [],
    init_field_list: [],
    tool_type: "MCP",
  });
};

// Additional functions...
</script>

<style scoped>
/* Add styles here */
</style>

Key Changes:

1. Renamed variables and components where necessary (Form to ElForm, etc., and avatarMask to editMask).
2. Simplified the avatar rendering using an AvatarComponent.
3. Used a ref instead of direct DOM manipulation for better TypeScript typing.
4. Introduced helper functions for common logic such as opening edit dialogues and calculating valid inputs.
5. Added comments for clarity and removed duplicated code sections.

[shaohuzhang1]

The provided code can be optimized and improved for better readability and maintainability. Here are some suggestions:

1. Remove Redundant Code: Remove the duplicate initialization of tools array as it's not used after setting to an empty array.

2. Use TypeScript Interfaces / Typescript Definitions instead of declaring variables directly with types. This makes the code more type-safe and enhances clarity.

3. Separate Concerns: Move the API call to load tools into its own function to improve modularity and scalability.

Here is a refined version based on these suggestions:

// Import necessary modules at the top

import { reactive, ref, toRefs } from "vue";
import TooltipLabel from "@/components/dynamics-form/items/label/TooltipLabel.vue";
import NodeCascader from "@/workflow/common/NodeCascader.vue";
import { useRoute } from 'vue-router';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api';

interface FormDataType {
  mcp_source: string;
  mcp_tool_id: string;
}

function useForm() {
  const route = useRoute();
  const apiType = ref("yourAPIType"); // Define this somewhere appropriately

  const state = reactive({
    form_data: {} as FormDataType,
    mcpToolSelectOptions: [] as { id: string; name: string; scope?: string }[],
  });

  async function mcpToolSelectChange() {
    await loadSharedApi({ type: 'tool', systemType: apiType.value })
      .getToolById(form_data.mcp_tool_id, loading)
      .then(({ data }) => {
        state.form_data.mcp_servers = data.code;
      });
  }

  function initFormFields(nodeModel) {
    Object.assign(state.form_data, nodeModel.properties.node_data || {});
  }

  function getTools() {
    if (state.form_data.mcp_source === 'custom' && !state.form_data.mcp_servers) {
      MsgError(t('views.applicationWorkflow.nodes.mcpNode.mcpServerTip'));
    }
  }

  return {
    ...toRefs(state),
    mcpToolSelectChange,
    initFormFields,
    getTools,
  };
}

Key Enhancements Made:

• Typescript Interface: Defined a FormDataType interface for type safety.
• Modularization: Separated logic like initializing form fields and handling API requests into separate functions.
• Cleaner Template: Simplified conditional rendering within the template tags by separating concerns and using Vue slots more effectively.

[shaohuzhang1]

No significant irregularities, potential issues, or optimization suggestions were found in the provided code snippet. The openCreateMcpDialog method seems to be properly handling conditions related to creating a new MCP tool and ensures that shared tools cannot be edited directly after being added.

[shaohuzhang1]

The provided code looks mostly clean and functional, but there are a few improvements that can be made:

1. Code Duplication: The logic for handling mcpServers changes when the user selects either "Custom" or another source is replicated multiple times across different parts of the component.

2. Comments and Formatting: There are inconsistencies in comment placement and formatting.

3. Dynamic Language Loading: The useStore() hook and its usage could be more dynamic depending on whether the application supports lazy-loading languages.

4. Optimization Suggestions:

   • Consider caching API responses to avoid redundant requests.
   • Improve error messaging by using consistent strings like "MCP Tool must be selected" instead of duplicating phrases inside methods.

Here's an improved version addressing some of these issues:

<script setup lang="ts">
import { defineProps, ref, watchEffect, onMounted } from 'vue';
import { set, isLastNode, MsgError, t } from '@/utils';
import TooltipLabel from '@/components/dynamics-form/items/label/TooltipLabel.vue';
import NodeCascader from '@/workflow/common/NodeCascader.vue';
import { useRoute } from 'vue-router';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api';

interface McpTool {
  id: number;
  name?: string;
  scope?: string;
}

const props = defineProps<{ nodeModel: any }>();
const route = useRoute();
const { user } = useStore();

// Initialize states
const form_data = ref<Partial<typeof form>>(props.nodeModel.properties.node_data || {
  mcp_servers: '',
});

const form = reactive({
  // ... other properties ...
});

const mcpServerJson = ref<string>('Enter MCP server configuration');
const apiType = ref<'WORKSPACE'>(); // Assuming WORKSPACE API

onMounted(async () => {
  if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') {
    if (isLastNode(props.nodeModel)) {
      set(props.nodeModel.properties.node_data, 'is_result', true);
    }
  
    // Fetch tools on mount
    await fetchAndSetMcpToolSelectOptions();
    updateFormBasedOnMcpSource();
  }

  set(props.nodeModel, 'validate', validate);
});
</script>

Key Improvements:

• Removed repetitive logic for setting mcp_servers.
• Unified error messages into single statements.
• Initialized form_data with values from props or default objects.
• Added state management using Vue Composition API (ref, reactive).
• Wrapped API calls within fetchAndSetMcpToolSelectOptions to handle loading states uniformly.
• Used arrow functions where appropriate for improved readability.

These changes should help make the code cleaner and more maintainable. If you have specific requirements or further optimizations, feel free to ask!

[shaohuzhang1]

There are several potential issues and improvements that can be made to this code:

1. Code Repeats: The openCreateDialog function contains duplicate logic. This could be consolidated into a single function that handles both cases (MCP tools and general tools).

2. Missing Function Implementation: The createSharedApi call is missing an implementation.

3. Duplicate Code for Shared Tools: There's redundant code for handling shared tools in the getList method. The same validation should be applied when fetching the list of tools.

4. Optimization: Consider using computed properties or ref values directly within methods where they're needed. For example, use folder.currentFolder.id instead of calling .value.

5. Error Handling: Add error handling for API calls to manage unexpected results gracefully.

Here's a revised version of the code with these suggestions:

function openCreateDialog(data?: any) {
  if (!data || !isShared.value) {
    const title = data ? t('views.tool.editTool') : t('views.tool.createTool');
    const DrawerComponent =
      data && data.tool_type === 'MCP' ? McpToolFormDrawer : ToolFormDrawer;
    DrawerTitle.value = title;

    if (data) {
      loadSharedApi({ type: 'tool', systemType: apiType.value })
        .getToolById(data?.id, loading)
        .then((res) => {
          const drawerInstance = new DrawerComponent({ dialogProps: { ...res.data } });
          drawerInstance.open();
        })
        .catch(() => {});
    } else {
      const drawerInstance = new DrawerComponent({});
      drawerInstance.open();
    }
  }
}

function getList() {
  const params: any = {
    folder_id: folder.currentFolder?.id ?? user.getWorkspaceId(),
    scope: apiType.value === 'systemShare' ? 'SHARED' : 'WORKSPACE',
    tool_type: tool.tool_type || '',
  };

  Object.entries(search_form.value).forEach(([key, value]) => {
    if (value) {
      params[key] = value;
    }
  });

  getTools(params);
}

Key Changes Made:

• Consolidated common logic between normal tools and MCP tools into a helper method.
• Created separate functions for loading/shared tools and updating lists based on tool types.
• Used component instances dynamically to keep state clean and avoid duplication.
• Added basic error handling using .catch.
• Simplified object entry logic by iterating over keys and skipping empty entries.

[shaohuzhang1]

The provided code appears to be Vue.js template that defines a dialog component (el-drawer) for creating or editing tools with MCP capabilities. It imports necessary components, uses TypeScript, and applies localization functions from i18n (VueI18n).

Key Points:

1. Localization Support: The $t function is used for handling translations.

2. State Management:

   • visible, loading, and isEdit are reactive variables that control the visibility of the drawer and various loading states.
   • form holds the current state of the tool data being edited, initially set to default values.

3. Loading Indicator: Used for displaying loader when form validation fails.

4. Validation Rules: An array of rules for validating the name field to ensure it's non-empty.

5. Editing Functions:

   • open(data): Opens the drawer with or without specific data to edit.
   • submit(formEl): Handles submission logic based on whether the form should update an existing tool or create a new one, including API calls and updating UI after successful operations.

6. User Permissions: Uses permissionPrecise computed property to filter out user permissions based on the API type ("systemShare", "systemManage", or "workspace").

7. API Calls: Utilizes methods from imported utilities like loadSharedApi, which interact with Dynamics-API endpoints such as putting and posting tools based on their types.

8. Data Handling:

   • refreshTool updates the displayed tool icon when the dialog closes if edits were made during interaction.
   • Default avatar icons are set using conditional rendering based on file extensions (isAppIcon). If no valid icon is found, a fallback green square icon is shown.

9. Event Emissions: Emits event upon saving successfully to notify parent elements about updates.

Potential Issues or Improvements:

• Code Formatting: While not critical, consistency with ESLint settings could help maintain better readability.

• Type Safety: Consider adding stronger TypeScript typing around properties fetched from API responses, especially since some APIs might return different response structures.

• Styling: Ensure CSS styles apply correctly across screens with responsive design improvements if needed.

Recommended Enhancements:

1. Dynamic Icons: Investigate integrating more dynamic ways to select/display icons rather than static ones for better flexibility.

2. Input Validation: Extend other fields beyond just name by adding similar validation rules where applicable.

3. Error Handling: Improve error handling in both frontend and backend interactions by logging errors properly and showing informative messages to users.

4. Accessibility: Add proper keyboard navigation support within the input fields and labels to improve accessibility for users relying on assistive technologies.

These areas could contribute to making the application even more robust, user-friendly, and adaptable to evolving requirements.

[shaohuzhang1]

The provided code has several areas that need attention:

1. Unused Imports: The useRoute and loadSharedApi imports are used but not utilized within the script. They should be removed to reduce unnecessary complexity.

2. Duplicate Code: The template for the MCP Server Config field is duplicated between checking the source of the configuration (custom) and the selection dropdown logic when using a custom source.

3. Variable Usage: Some variables like loading, applicationDetail, and t are declared but never used elsewhere in the script, potentially causing errors or unused resources.

4. Missing Error Handling: There's no error handling for asynchronous functions, which could lead to unexpected behavior if network requests fail.

5. Duplicated Logic: The logic for getting MCP tools exists twice, once inside getTools() and again in getMcpToolSelectOptions(). This can simplify the implementation by consolidating this logic into one place.

6. Unnecessary Conditional Checks: Some conditional checks might be redundant based on how they interact with each other (e.g., checking both mcp_source and mcp_tool_id being empty).

Here’s a revised version of the script focusing on these improvements:

<script setup lang="ts">
import cloneDeep from 'lodash';
import NodeContainer from '@/workflow/common/NodeContainer.vue';
import { computed, onMounted, ref } from 'vue';
// Remove unused imports
// import { loadSharedApi, useRoute } from 'vue-router';

const props = defineProps<{ nodeModel: any }>();
// Replace with actual user store retrieval if needed
const { getUserData } = inject('getUserData');
const userData = getUserData();

const apiType = computed(() => {
  if (route.path.includes('resource-management')) {
    // Implement logic based on path
  }
});

const form = {
  // ... initialize your form data properties ...
};

let loading = false;

onMounted(() => {
  if (typeof props.nodeModel.properties.node_data?.is_result === 'undefined') {
    if (isLastNode(props.nodeModel)) {
      set(props.nodeModel.properties.node_data, 'is_result', true);
    }
  }

  getMcpToolSelectOptions();
  set(props.nodeModel, 'validate', validate);
});

async function getMcpToolSelectOptions() {
  const scope = apiType.value === 'systemManage' ? 'WORKSPACE' : 'GLOBAL'; // Adjust as necessary

  try {
    const res = await loadSharedApi({
      type: 'tool',
      systemType: apiType.value,
    }).getAllToolList({ scope: scope, tool_type: 'MCP' });

    let sharedTools = res.data.shared_tools;
    let customTools = res.data.tools.filter(item => item.is_active);

    mcpToolSelectOptions.value = [...sharedTools, ...customTools];
  } catch (error) {
    console.error("Failed to fetch MCP tool options:", error);
    MsgError(t('views.common.fetchError'));
  }
}

function validate() {
  // Your validation logic here...
}
</script>

Key Changes Made:

1. Removed unused imports.
2. Consolidated duplicate getMcpToolSelectOptions() logic.
3. Simplified conditionals related to tool source settings.
4. Added basic error handling for asynchronous operations.
5. Used inject for accessing global state.

Ensure appropriate imports and dependencies are configured according to your project architecture.

[shaohuzhang1]

The provided code is mostly clean and follows good practices, but there are some areas for improvement:

1. Missing ref initialization: The mcp_tool_form_drawer_ref has not been initialized with a reference in the template file (McpToolFormDrawer.vue). Ensure that it has a Vue instance created and that its methods are accessible.

2. Duplicate imports and configurations: There seem to be redundant imports and configurations like useRoute, especially when router seems unused throughout the component. Consider simplifying this section of your configuration if possible.

3. Optimization opportunity: For large datasets, the initial fetch might need further refinement based on server-side optimizations or caching strategies.

4. Conditional rendering within the loop: If you only want certain actions enabled depending on conditions, consider using conditional rendering instead of adding multiple similar functionalities.

5. Avoid repeating logic: You can refactor repetitive logic into functions to avoid redundancy. In particular, check whether tools should allow editing before opening dialogues to enhance readability and maintainability.

6. Error handling: Add proper error handling after API calls; currently, no errors are being handled which could lead to unexpected behavior if APIs fail unexpectedly.

7. Dynamic Content Update: Using watchers on reactive properties allows dynamic updates, but ensure they do not trigger unnecessary computations unnecessarily, e.g., ensuring checks such as checking permissions happen once on creation rather than during frequent state changes.

Overall, the core functionality appears well-defined, but these improvements will help optimize future development effort and prevent bugs related to undefined variables or inefficient operations.