fix settings data export & parsing error (#373)

2025-12-23 18:29:21 -05:00 · 2025-10-16 13:08:28 +08:00
parent 3e9e5cc3c9
commit bd4c546bba
3 changed files with 97 additions and 68 deletions
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -13,6 +13,7 @@ MCPHub is a TypeScript/Node.js MCP (Model Context Protocol) server management hu
 - **MCP Integration**: Connects multiple MCP servers (`src/services/mcpService.ts`)
 - **Authentication**: JWT-based with bcrypt password hashing
 - **Configuration**: JSON-based MCP server definitions (`mcp_settings.json`)
+- **Documentation**: API docs and usage instructions(`docs/`)

 ## Working Effectively

@@ -30,7 +31,7 @@ cp .env.example .env

 # Build and test to verify setup
 pnpm lint                    # ~3 seconds - NEVER CANCEL
-pnpm backend:build          # ~5 seconds - NEVER CANCEL  
+pnpm backend:build          # ~5 seconds - NEVER CANCEL
 pnpm test:ci                # ~16 seconds - NEVER CANCEL. Set timeout to 60+ seconds
 pnpm frontend:build         # ~5 seconds - NEVER CANCEL
 pnpm build                  # ~10 seconds total - NEVER CANCEL. Set timeout to 60+ seconds
@@ -48,7 +49,7 @@ pnpm dev                    # Backend on :3001, Frontend on :5173
 # Terminal 1: Backend only
 pnpm backend:dev            # Runs on port 3000 (or PORT env var)

-# Terminal 2: Frontend only  
+# Terminal 2: Frontend only
 pnpm frontend:dev           # Runs on port 5173, proxies API to backend
 ```

@@ -62,7 +63,7 @@ pnpm build                  # NEVER CANCEL - Set timeout to 60+ seconds

 # Individual builds
 pnpm backend:build          # TypeScript compilation - ~5 seconds
-pnpm frontend:build         # Vite build - ~5 seconds  
+pnpm frontend:build         # Vite build - ~5 seconds

 # Start production server
 pnpm start                  # Requires dist/ and frontend/dist/ to exist
@@ -91,6 +92,7 @@ pnpm format                 # Prettier formatting - ~3 seconds
 **ALWAYS perform these validation steps after making changes:**

 ### 1. Basic Application Functionality
+
 ```bash
 # Start the application
 pnpm dev
@@ -105,6 +107,7 @@ curl -I http://localhost:3000/
 ```

 ### 2. MCP Server Integration Test
+
 ```bash
 # Check MCP servers are loading (look for log messages)
 # Expected log output should include:
@@ -114,6 +117,7 @@ curl -I http://localhost:3000/
 ```

 ### 3. Build Verification
+
 ```bash
 # Verify production build works
 pnpm build
@@ -126,6 +130,7 @@ node scripts/verify-dist.js
 ## Project Structure and Key Files

 ### Critical Backend Files
+
 - `src/index.ts` - Application entry point
 - `src/server.ts` - Express server setup and middleware
 - `src/services/mcpService.ts` - **Core MCP server management logic**
@@ -136,11 +141,14 @@ node scripts/verify-dist.js
 - `src/types/index.ts` - TypeScript type definitions

 ### Critical Frontend Files
+
 - `frontend/src/` - React application source
 - `frontend/src/pages/` - Page components (development entry point)
 - `frontend/src/components/` - Reusable UI components
+- `frontend/src/utils/fetchInterceptor.js` - Backend API interaction

 ### Configuration Files
+
 - `mcp_settings.json` - **MCP server definitions and user accounts**
 - `package.json` - Dependencies and scripts
 - `tsconfig.json` - TypeScript configuration
@@ -148,6 +156,7 @@ node scripts/verify-dist.js
 - `.eslintrc.json` - Linting rules

 ### Docker and Deployment
+
 - `Dockerfile` - Multi-stage build with Python base + Node.js
 - `entrypoint.sh` - Docker startup script
 - `bin/cli.js` - NPM package CLI entry point
@@ -155,12 +164,14 @@ node scripts/verify-dist.js
 ## Development Process and Conventions

 ### Code Style Requirements
+
 - **ESM modules**: Always use `.js` extensions in imports, not `.ts`
 - **English only**: All code comments must be written in English
 - **TypeScript strict**: Follow strict type checking rules
 - **Import style**: `import { something } from './file.js'` (note .js extension)

 ### Key Configuration Notes
+
 - **MCP servers**: Defined in `mcp_settings.json` with command/args
 - **Endpoints**: `/mcp/{group|server}` and `/mcp/$smart` for routing
 - **i18n**: Frontend uses react-i18next with files in `locales/` folder
@@ -168,6 +179,7 @@ node scripts/verify-dist.js
 - **Default credentials**: admin/admin123 (configured in mcp_settings.json)

 ### Development Entry Points
+
 - **Add MCP server**: Modify `mcp_settings.json` and restart
 - **New API endpoint**: Add route in `src/routes/`, controller in `src/controllers/`
 - **Frontend feature**: Start from `frontend/src/pages/` or `frontend/src/components/`
@@ -176,29 +188,38 @@ node scripts/verify-dist.js
 ### Common Development Tasks

 #### Adding a new MCP server:
+
 1. Add server definition to `mcp_settings.json`
 2. Restart backend to load new server
 3. Check logs for successful connection
 4. Test via dashboard or API endpoints

 #### API development:
+
 1. Define route in `src/routes/`
 2. Implement controller in `src/controllers/`
 3. Add types in `src/types/index.ts` if needed
 4. Write tests in `tests/controllers/`

 #### Frontend development:
+
 1. Create/modify components in `frontend/src/components/`
 2. Add pages in `frontend/src/pages/`
 3. Update routing if needed
 4. Test in development mode with `pnpm frontend:dev`

+#### Documentation:
+
+1. Update or add docs in `docs/` folder
+2. Ensure README.md reflects any major changes
+
 ## Validation and CI Requirements

 ### Before Committing - ALWAYS Run:
+
 ```bash
 pnpm lint                   # Must pass - ~3 seconds
-pnpm backend:build          # Must compile - ~5 seconds  
+pnpm backend:build          # Must compile - ~5 seconds
 pnpm test:ci                # All tests must pass - ~16 seconds
 pnpm build                  # Full build must work - ~10 seconds
 ```
@@ -206,6 +227,7 @@ pnpm build                  # Full build must work - ~10 seconds
 **CRITICAL**: CI will fail if any of these commands fail. Fix issues locally first.

 ### CI Pipeline (.github/workflows/ci.yml)
+
 - Runs on Node.js 20.x
 - Tests: linting, type checking, unit tests with coverage
 - **NEVER CANCEL**: CI builds may take 2-3 minutes total
@@ -213,22 +235,26 @@ pnpm build                  # Full build must work - ~10 seconds
 ## Troubleshooting

 ### Common Issues
+
 - **"uvx command not found"**: Some MCP servers require `uvx` (Python package manager) - this is expected in development
 - **Port already in use**: Change PORT environment variable or kill existing processes
 - **Frontend not loading**: Ensure frontend was built with `pnpm frontend:build`
 - **MCP server connection failed**: Check server command/args in `mcp_settings.json`

 ### Build Failures
+
 - **TypeScript errors**: Run `pnpm backend:build` to see compilation errors
 - **Test failures**: Run `pnpm test:verbose` for detailed test output
 - **Lint errors**: Run `pnpm lint` and fix reported issues

 ### Development Issues
+
 - **Backend not starting**: Check for port conflicts, verify `mcp_settings.json` syntax
 - **Frontend proxy errors**: Ensure backend is running before starting frontend
 - **Hot reload not working**: Restart development server

 ## Performance Notes
+
 - **Install time**: pnpm install takes ~30 seconds
 - **Build time**: Full build takes ~10 seconds
 - **Test time**: Complete test suite takes ~16 seconds
--- a/frontend/src/pages/SettingsPage.tsx
+++ b/frontend/src/pages/SettingsPage.tsx
@@ -287,7 +287,7 @@ const SettingsPage: React.FC = () => {
    try {
      const result = await exportMCPSettings()
      console.log('Fetched MCP settings:', result)
-      const configJson = JSON.stringify(result, null, 2)
+      const configJson = JSON.stringify(result.data, null, 2)
      setMcpSettingsJson(configJson)
    } catch (error) {
      console.error('Error fetching MCP settings:', error)
--- a/src/config/index.ts
+++ b/src/config/index.ts
@@ -1,12 +1,12 @@
-import dotenv from 'dotenv';
-import fs from 'fs';
-import { McpSettings, IUser } from '../types/index.js';
-import { getConfigFilePath } from '../utils/path.js';
-import { getPackageVersion } from '../utils/version.js';
-import { getDataService } from '../services/services.js';
-import { DataService } from '../services/dataService.js';
+import dotenv from 'dotenv'
+import fs from 'fs'
+import { McpSettings, IUser } from '../types/index.js'
+import { getConfigFilePath } from '../utils/path.js'
+import { getPackageVersion } from '../utils/version.js'
+import { getDataService } from '../services/services.js'
+import { DataService } from '../services/dataService.js'

-dotenv.config();
+dotenv.config()

 const defaultConfig = {
  port: process.env.PORT || 3000,
@@ -15,71 +15,74 @@ const defaultConfig = {
  readonly: 'true' === process.env.READONLY || false,
  mcpHubName: 'mcphub',
  mcpHubVersion: getPackageVersion(),
-};
+}

-const dataService: DataService = getDataService();
+const dataService: DataService = getDataService()

 // Settings cache
-let settingsCache: McpSettings | null = null;
+let settingsCache: McpSettings | null = null

 export const getSettingsPath = (): string => {
-  return getConfigFilePath('mcp_settings.json', 'Settings');
-};
+  return getConfigFilePath('mcp_settings.json', 'Settings')
+}

 export const loadOriginalSettings = (): McpSettings => {
  // If cache exists, return cached data directly
  if (settingsCache) {
-    return settingsCache;
+    return settingsCache
+  }
+
+  const settingsPath = getSettingsPath()
+  // check if file exists
+  if (!fs.existsSync(settingsPath)) {
+    console.warn(`Settings file not found at ${settingsPath}, using default settings.`)
+    const defaultSettings = { mcpServers: {}, users: [] }
+    // Cache default settings
+    settingsCache = defaultSettings
+    return defaultSettings
  }

-  const settingsPath = getSettingsPath();
  try {
-    const settingsData = fs.readFileSync(settingsPath, 'utf8');
-    const settings = JSON.parse(settingsData);
+    // Read and parse settings file
+    const settingsData = fs.readFileSync(settingsPath, 'utf8')
+    const settings = JSON.parse(settingsData)

    // Update cache
-    settingsCache = settings;
+    settingsCache = settings

-    console.log(`Loaded settings from ${settingsPath}`);
-    return settings;
-  } catch (error: unknown) {
-    const errorMessage = error instanceof Error ? error.message : String(error);
-    console.warn(`Failed to load settings from ${settingsPath}:`, errorMessage);
-    const defaultSettings = { mcpServers: {}, users: [] };
-
-    // Cache default settings
-    settingsCache = defaultSettings;
-
-    return defaultSettings;
+    console.log(`Loaded settings from ${settingsPath}`)
+    return settings
+  } catch (error) {
+    throw new Error(`Failed to load settings from ${settingsPath}: ${error}`)
  }
-};
+}

 export const loadSettings = (user?: IUser): McpSettings => {
-  return dataService.filterSettings!(loadOriginalSettings(), user);
-};
+  return dataService.filterSettings!(loadOriginalSettings(), user)
+}

 export const saveSettings = (settings: McpSettings, user?: IUser): boolean => {
-  const settingsPath = getSettingsPath();
+  const settingsPath = getSettingsPath()
  try {
-    const mergedSettings = dataService.mergeSettings!(loadOriginalSettings(), settings, user);
-    fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2), 'utf8');
+    const mergedSettings = dataService.mergeSettings!(loadOriginalSettings(), settings, user)
+    fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2), 'utf8')

    // Update cache after successful save
-    settingsCache = mergedSettings;
+    settingsCache = mergedSettings

-    return true;
+    return true
  } catch (error) {
-    console.error(`Failed to save settings to ${settingsPath}:`, error);
-    return false;
+    console.error(`Failed to save settings to ${settingsPath}:`, error)
+    return false
  }
-};
+}

 /**
 * Clear settings cache, force next loadSettings call to re-read from file
 */
 export const clearSettingsCache = (): void => {
-  settingsCache = null;
-};
+  settingsCache = null
+}

 /**
 * Get current cache status (for debugging)
@@ -87,60 +90,60 @@ export const clearSettingsCache = (): void => {
 export const getSettingsCacheInfo = (): { hasCache: boolean } => {
  return {
    hasCache: settingsCache !== null,
-  };
-};
+  }
+}

-export function replaceEnvVars(input: Record<string, any>): Record<string, any>;
-export function replaceEnvVars(input: string[] | undefined): string[];
-export function replaceEnvVars(input: string): string;
+export function replaceEnvVars(input: Record<string, any>): Record<string, any>
+export function replaceEnvVars(input: string[] | undefined): string[]
+export function replaceEnvVars(input: string): string
 export function replaceEnvVars(
  input: Record<string, any> | string[] | string | undefined,
 ): Record<string, any> | string[] | string {
  // Handle object input
  if (input && typeof input === 'object' && !Array.isArray(input)) {
-    const res: Record<string, string> = {};
+    const res: Record<string, string> = {}
    for (const [key, value] of Object.entries(input)) {
      if (typeof value === 'string') {
-        res[key] = expandEnvVars(value);
+        res[key] = expandEnvVars(value)
      } else {
-        res[key] = String(value);
+        res[key] = String(value)
      }
    }
-    return res;
+    return res
  }

  // Handle array input
  if (Array.isArray(input)) {
-    return input.map((item) => expandEnvVars(item));
+    return input.map((item) => expandEnvVars(item))
  }

  // Handle string input
  if (typeof input === 'string') {
-    return expandEnvVars(input);
+    return expandEnvVars(input)
  }

  // Handle undefined/null array input
  if (input === undefined || input === null) {
-    return [];
+    return []
  }

-  return input;
+  return input
 }

 export const expandEnvVars = (value: string): string => {
  if (typeof value !== 'string') {
-    return String(value);
+    return String(value)
  }
  // Replace ${VAR} format
-  let result = value.replace(/\$\{([^}]+)\}/g, (_, key) => process.env[key] || '');
+  let result = value.replace(/\$\{([^}]+)\}/g, (_, key) => process.env[key] || '')
  // Also replace $VAR format (common on Unix-like systems)
-  result = result.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, key) => process.env[key] || '');
-  return result;
-};
+  result = result.replace(/\$([A-Z_][A-Z0-9_]*)/g, (_, key) => process.env[key] || '')
+  return result
+}

-export default defaultConfig;
+export default defaultConfig

 export function getNameSeparator(): string {
-  const settings = loadSettings();
-  return settings.systemConfig?.nameSeparator || '-';
+  const settings = loadSettings()
+  return settings.systemConfig?.nameSeparator || '-'
 }