feat: add variable detection and confirmation dialogs in server forms (#205)

frontend/src/components/AddServerForm.tsx

@@ -1,7 +1,8 @@
 import { useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import ServerForm from './ServerForm'
-import { getApiUrl } from '../utils/runtime';
+import { getApiUrl } from '../utils/runtime'
+import { detectVariables } from '../utils/variableDetection'
 
 interface AddServerFormProps {
   onAdd: () => void
@@ -11,13 +12,26 @@ const AddServerForm = ({ onAdd }: AddServerFormProps) => {
   const { t } = useTranslation()
   const [modalVisible, setModalVisible] = useState(false)
   const [error, setError] = useState<string | null>(null)
+  const [confirmationVisible, setConfirmationVisible] = useState(false)
+  const [pendingPayload, setPendingPayload] = useState<any>(null)
+  const [detectedVariables, setDetectedVariables] = useState<string[]>([])
 
   const toggleModal = () => {
     setModalVisible(!modalVisible)
     setError(null) // Clear any previous errors when toggling modal
+    setConfirmationVisible(false) // Close confirmation dialog
+    setPendingPayload(null) // Clear pending payload
   }
 
-  const handleSubmit = async (payload: any) => {
+  const handleConfirmSubmit = async () => {
+    if (pendingPayload) {
+      await submitServer(pendingPayload)
+      setConfirmationVisible(false)
+      setPendingPayload(null)
+    }
+  }
+
+  const submitServer = async (payload: any) => {
     try {
       setError(null)
       const token = localStorage.getItem('mcphub_token');
@@ -65,6 +79,26 @@ const AddServerForm = ({ onAdd }: AddServerFormProps) => {
     }
   }
 
+  const handleSubmit = async (payload: any) => {
+    try {
+      // Check for variables in the payload
+      const variables = detectVariables(payload)
+
+      if (variables.length > 0) {
+        // Show confirmation dialog
+        setDetectedVariables(variables)
+        setPendingPayload(payload)
+        setConfirmationVisible(true)
+      } else {
+        // Submit directly if no variables found
+        await submitServer(payload)
+      }
+    } catch (err) {
+      console.error('Error processing server submission:', err)
+      setError(t('errors.serverAdd'))
+    }
+  }
+
   return (
     <div>
       <button
@@ -87,6 +121,60 @@ const AddServerForm = ({ onAdd }: AddServerFormProps) => {
           />
         </div>
       )}
+
+      {confirmationVisible && (
+        <div className="fixed inset-0 bg-black/50 z-[60] flex items-center justify-center p-4">
+          <div className="bg-white rounded-lg p-6 w-full max-w-md">
+            <h3 className="text-lg font-semibold text-gray-900 mb-4">
+              {t('server.confirmVariables')}
+            </h3>
+            <p className="text-gray-600 mb-4">
+              {t('server.variablesDetected')}
+            </p>
+            <div className="bg-yellow-50 border border-yellow-200 rounded p-3 mb-4">
+              <div className="flex items-start">
+                <div className="flex-shrink-0">
+                  <svg className="h-5 w-5 text-yellow-400" viewBox="0 0 20 20" fill="currentColor">
+                    <path fillRule="evenodd" d="M8.257 3.099c.765-1.36 2.722-1.36 3.486 0l5.58 9.92c.75 1.334-.213 2.98-1.742 2.98H4.42c-1.53 0-2.493-1.646-1.743-2.98l5.58-9.92zM11 13a1 1 0 11-2 0 1 1 0 012 0zm-1-8a1 1 0 00-1 1v3a1 1 0 002 0V6a1 1 0 00-1-1z" clipRule="evenodd" />
+                  </svg>
+                </div>
+                <div className="ml-3">
+                  <h4 className="text-sm font-medium text-yellow-800">
+                    {t('server.detectedVariables')}:
+                  </h4>
+                  <ul className="mt-1 text-sm text-yellow-700">
+                    {detectedVariables.map((variable, index) => (
+                      <li key={index} className="font-mono">
+                        ${`{${variable}}`}
+                      </li>
+                    ))}
+                  </ul>
+                </div>
+              </div>
+            </div>
+            <p className="text-gray-600 text-sm mb-6">
+              {t('server.confirmVariablesMessage')}
+            </p>
+            <div className="flex justify-end space-x-3">
+              <button
+                onClick={() => {
+                  setConfirmationVisible(false)
+                  setPendingPayload(null)
+                }}
+                className="px-4 py-2 text-gray-600 border border-gray-300 rounded hover:bg-gray-50 btn-secondary"
+              >
+                {t('common.cancel')}
+              </button>
+              <button
+                onClick={handleConfirmSubmit}
+                className="px-4 py-2 bg-blue-600 text-white rounded hover:bg-blue-700 btn-primary"
+              >
+                {t('server.confirmAndAdd')}
+              </button>
+            </div>
+          </div>
+        </div>
+      )}
     </div>
   )
 }

frontend/src/components/MarketServerDetail.tsx

@@ -2,6 +2,7 @@ import React, { useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { MarketServer, MarketServerInstallation } from '@/types';
 import ServerForm from './ServerForm';
+import { detectVariables } from '../utils/variableDetection';
 
 import { ServerConfig } from '@/types';
 
@@ -23,6 +24,9 @@ const MarketServerDetail: React.FC<MarketServerDetailProps> = ({
   const { t } = useTranslation();
   const [modalVisible, setModalVisible] = useState(false);
   const [error, setError] = useState<string | null>(null);
+  const [confirmationVisible, setConfirmationVisible] = useState(false);
+  const [pendingPayload, setPendingPayload] = useState<any>(null);
+  const [detectedVariables, setDetectedVariables] = useState<string[]>([]);
 
   // Helper function to determine button state
   const getButtonProps = () => {
@@ -50,6 +54,27 @@ const MarketServerDetail: React.FC<MarketServerDetailProps> = ({
   const toggleModal = () => {
     setModalVisible(!modalVisible);
     setError(null); // Clear any previous errors when toggling modal
+    setConfirmationVisible(false);
+    setPendingPayload(null);
+  };
+
+  const handleConfirmInstall = async () => {
+    if (pendingPayload) {
+      await proceedWithInstall(pendingPayload);
+      setConfirmationVisible(false);
+      setPendingPayload(null);
+    }
+  };
+
+  const proceedWithInstall = async (payload: any) => {
+    try {
+      setError(null);
+      onInstall(server, payload.config);
+      setModalVisible(false);
+    } catch (err) {
+      console.error('Error installing server:', err);
+      setError(t('errors.serverInstall'));
+    }
   };
 
   const handleInstall = () => {
@@ -84,12 +109,20 @@ const MarketServerDetail: React.FC<MarketServerDetailProps> = ({
 
   const handleSubmit = async (payload: any) => {
     try {
-      setError(null);
-      // Pass the server object and the payload (includes env changes) for installation
-      onInstall(server, payload.config);
-      setModalVisible(false);
+      // Check for variables in the payload
+      const variables = detectVariables(payload);
+
+      if (variables.length > 0) {
+        // Show confirmation dialog
+        setDetectedVariables(variables);
+        setPendingPayload(payload);
+        setConfirmationVisible(true);
+      } else {
+        // Install directly if no variables found
+        await proceedWithInstall(payload);
+      }
     } catch (err) {
-      console.error('Error installing server:', err);
+      console.error('Error processing server installation:', err);
       setError(t('errors.serverInstall'));
     }
   };
@@ -292,6 +325,60 @@ const MarketServerDetail: React.FC<MarketServerDetailProps> = ({
           />
         </div>
       )}
+
+      {confirmationVisible && (
+        <div className="fixed inset-0 bg-black/50 z-[60] flex items-center justify-center p-4">
+          <div className="bg-white rounded-lg p-6 w-full max-w-md">
+            <h3 className="text-lg font-semibold text-gray-900 mb-4">
+              {t('server.confirmVariables')}
+            </h3>
+            <p className="text-gray-600 mb-4">
+              {t('server.variablesDetected')}
+            </p>
+            <div className="bg-yellow-50 border border-yellow-200 rounded p-3 mb-4">
+              <div className="flex items-start">
+                <div className="flex-shrink-0">
+                  <svg className="h-5 w-5 text-yellow-400" viewBox="0 0 20 20" fill="currentColor">
+                    <path fillRule="evenodd" d="M8.257 3.099c.765-1.36 2.722-1.36 3.486 0l5.58 9.92c.75 1.334-.213 2.98-1.742 2.98H4.42c-1.53 0-2.493-1.646-1.743-2.98l5.58-9.92zM11 13a1 1 0 11-2 0 1 1 0 012 0zm-1-8a1 1 0 00-1 1v3a1 1 0 002 0V6a1 1 0 00-1-1z" clipRule="evenodd" />
+                  </svg>
+                </div>
+                <div className="ml-3">
+                  <h4 className="text-sm font-medium text-yellow-800">
+                    {t('server.detectedVariables')}:
+                  </h4>
+                  <ul className="mt-1 text-sm text-yellow-700">
+                    {detectedVariables.map((variable, index) => (
+                      <li key={index} className="font-mono">
+                        ${`{${variable}}`}
+                      </li>
+                    ))}
+                  </ul>
+                </div>
+              </div>
+            </div>
+            <p className="text-gray-600 text-sm mb-6">
+              {t('market.confirmVariablesMessage')}
+            </p>
+            <div className="flex justify-end space-x-3">
+              <button
+                onClick={() => {
+                  setConfirmationVisible(false)
+                  setPendingPayload(null)
+                }}
+                className="px-4 py-2 text-gray-600 border border-gray-300 rounded hover:bg-gray-50 btn-secondary"
+              >
+                {t('common.cancel')}
+              </button>
+              <button
+                onClick={handleConfirmInstall}
+                className="px-4 py-2 bg-blue-600 text-white rounded hover:bg-blue-700 btn-primary"
+              >
+                {t('market.confirmAndInstall')}
+              </button>
+            </div>
+          </div>
+        </div>
+      )}
     </div>
   );
 };

frontend/src/components/ServerForm.tsx

@@ -264,7 +264,7 @@ const ServerForm = ({ onSubmit, onCancel, initialData = null, modalTitle, formEr
     <div className="bg-white shadow rounded-lg p-6 w-full max-w-xl max-h-screen overflow-y-auto">
       <div className="flex justify-between items-center mb-4">
         <h2 className="text-xl font-semibold text-gray-900">{modalTitle}</h2>
-        <button onClick={onCancel} className="text-gray-500 hover:text-gray-700 btn-secondary">
+        <button onClick={onCancel} className="text-gray-500 hover:text-gray-700">
           ✕
         </button>
       </div>

frontend/src/locales/en.json

@@ -117,6 +117,11 @@
     "argumentsPlaceholder": "Enter arguments",
     "errorDetails": "Error Details",
     "viewErrorDetails": "View error details",
+    "confirmVariables": "Confirm Variable Configuration",
+    "variablesDetected": "Variables detected in configuration. Please confirm these variables are properly configured:",
+    "detectedVariables": "Detected Variables",
+    "confirmVariablesMessage": "Please ensure these variables are properly defined in your runtime environment. Continue adding server?",
+    "confirmAndAdd": "Confirm and Add",
     "openapi": {
       "inputMode": "Input Mode",
       "inputModeUrl": "Specification URL",
@@ -297,7 +302,9 @@
     "tagFilterError": "Error filtering servers by tag",
     "noInstallationMethod": "No installation method available for this server",
     "showing": "Showing {{from}}-{{to}} of {{total}} servers",
-    "perPage": "Per page"
+    "perPage": "Per page",
+    "confirmVariablesMessage": "Please ensure these variables are properly defined in your runtime environment. Continue installing server?",
+    "confirmAndInstall": "Confirm and Install"
   },
   "tool": {
     "run": "Run",

frontend/src/locales/zh.json

@@ -117,6 +117,11 @@
     "argumentsPlaceholder": "请输入参数",
     "errorDetails": "错误详情",
     "viewErrorDetails": "查看错误详情",
+    "confirmVariables": "确认变量配置",
+    "variablesDetected": "检测到配置中包含变量，请确认这些变量是否已正确配置：",
+    "detectedVariables": "检测到的变量",
+    "confirmVariablesMessage": "请确保这些变量在运行环境中已正确定义。是否继续添加服务器？",
+    "confirmAndAdd": "确认并添加",
     "openapi": {
       "inputMode": "输入模式",
       "inputModeUrl": "规范 URL",
@@ -298,7 +303,9 @@
     "tagFilterError": "按标签筛选服务器失败",
     "noInstallationMethod": "该服务器没有可用的安装方法",
     "showing": "显示 {{from}}-{{to}}/{{total}} 个服务器",
-    "perPage": "每页显示"
+    "perPage": "每页显示",
+    "confirmVariablesMessage": "请确保这些变量在运行环境中已正确定义。是否继续安装服务器？",
+    "confirmAndInstall": "确认并安装"
   },
   "tool": {
     "run": "运行",

frontend/src/utils/variableDetection.ts

@@ -0,0 +1,27 @@
+// Utility function to detect ${} variables in server configurations
+export const detectVariables = (payload: any): string[] => {
+  const variables = new Set<string>();
+  const variableRegex = /\$\{([^}]+)\}/g;
+
+  const checkString = (str: string) => {
+    let match;
+    while ((match = variableRegex.exec(str)) !== null) {
+      variables.add(match[1]);
+    }
+  };
+
+  const checkObject = (obj: any, path: string = '') => {
+    if (typeof obj === 'string') {
+      checkString(obj);
+    } else if (Array.isArray(obj)) {
+      obj.forEach((item, index) => checkObject(item, `${path}[${index}]`));
+    } else if (obj && typeof obj === 'object') {
+      Object.entries(obj).forEach(([key, value]) => {
+        checkObject(value, path ? `${path}.${key}` : key);
+      });
+    }
+  };
+
+  checkObject(payload);
+  return Array.from(variables);
+};