Merge pull request #107 from Avtrkrb/fix/mcp-configuration-empty-fields-and-auth

Fix: MCP configuration empty fields and auth issues

Author: will-lamerton

source/lsp/lsp-manager.ts

@@ -72,7 +72,7 @@ export class LSPManager extends EventEmitter {
 
 		// Auto-discover additional servers if enabled (default: true)
 		if (config.autoDiscover !== false) {
-			const discovered = discoverLanguageServers();
+			const discovered = await discoverLanguageServers();
 
 			// Only add discovered servers for languages not already covered
 			const coveredLanguages = new Set<string>();

source/lsp/server-discovery.spec.ts

@@ -465,3 +465,41 @@ test('getServerForLanguage - handles empty extension', t => {
 	const result = getServerForLanguage(servers, '');
 	t.is(result, undefined);
 });
+
+// Tests for verificationMethod functionality
+test('getKnownServersStatus - all servers have proper structure', t => {
+	const result = getKnownServersStatus();
+
+	// Check that servers have proper structure
+	for (const server of result) {
+		t.truthy(server.name);
+		t.true(typeof server.available === 'boolean');
+		t.true(Array.isArray(server.languages));
+	}
+});
+
+test('getKnownServersStatus - key servers are present with correct names', t => {
+	const result = getKnownServersStatus();
+
+	// Check that key servers are present with their correct names
+	const keyServers = [
+		'typescript-language-server',
+		'pyright',
+		'pylsp',
+		'rust-analyzer',
+		'gopls',
+		'clangd',
+		'vscode-json-languageserver',
+		'vscode-html-languageserver',
+		'vscode-css-languageserver',
+		'yaml-language-server',
+		'bash-language-server',
+		'lua-language-server',
+	];
+
+	for (const serverName of keyServers) {
+		const server = result.find(s => s.name === serverName);
+		t.truthy(server, `Server ${serverName} should be present`);
+		t.is(server!.name, serverName);
+	}
+});

source/lsp/server-discovery.ts

@@ -3,7 +3,7 @@
  * Detects installed language servers on the system
  */
 
-import {execSync} from 'child_process';
+import {execSync, spawn} from 'child_process';
 import {existsSync} from 'fs';
 import {join} from 'path';
 import type {LSPServerConfig} from './lsp-client';
@@ -14,6 +14,7 @@ interface LanguageServerDefinition {
 	args: string[];
 	languages: string[];
 	checkCommand?: string; // Command to verify installation
+	verificationMethod?: 'version' | 'lsp' | 'none'; // New verification method
 	installHint?: string;
 }
 
@@ -28,6 +29,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs'],
 		checkCommand: 'typescript-language-server --version',
+		verificationMethod: 'version',
 		installHint: 'npm install -g typescript-language-server typescript',
 	},
 	// Python - Pyright (preferred)
@@ -37,6 +39,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['py', 'pyi'],
 		checkCommand: 'pyright-langserver --version',
+		verificationMethod: 'lsp',
 		installHint: 'npm install -g pyright',
 	},
 	// Python - pylsp (alternative)
@@ -46,6 +49,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: [],
 		languages: ['py', 'pyi'],
 		checkCommand: 'pylsp --version',
+		verificationMethod: 'version',
 		installHint: 'pip install python-lsp-server',
 	},
 	// Rust
@@ -55,6 +59,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: [],
 		languages: ['rs'],
 		checkCommand: 'rust-analyzer --version',
+		verificationMethod: 'version',
 		installHint: 'rustup component add rust-analyzer',
 	},
 	// Go
@@ -64,6 +69,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['serve'],
 		languages: ['go'],
 		checkCommand: 'gopls version',
+		verificationMethod: 'version',
 		installHint: 'go install golang.org/x/tools/gopls@latest',
 	},
 	// C/C++
@@ -73,6 +79,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--background-index'],
 		languages: ['c', 'cpp', 'cc', 'cxx', 'h', 'hpp', 'hxx'],
 		checkCommand: 'clangd --version',
+		verificationMethod: 'version',
 		installHint: 'Install via system package manager (apt, brew, etc.)',
 	},
 	// JSON
@@ -82,6 +89,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['json', 'jsonc'],
 		checkCommand: 'vscode-json-language-server --version',
+		verificationMethod: 'lsp',
 		installHint: 'npm install -g vscode-langservers-extracted',
 	},
 	// HTML
@@ -91,6 +99,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['html', 'htm'],
 		checkCommand: 'vscode-html-language-server --version',
+		verificationMethod: 'lsp',
 		installHint: 'npm install -g vscode-langservers-extracted',
 	},
 	// CSS
@@ -100,6 +109,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['css', 'scss', 'less'],
 		checkCommand: 'vscode-css-language-server --version',
+		verificationMethod: 'lsp',
 		installHint: 'npm install -g vscode-langservers-extracted',
 	},
 	// YAML
@@ -109,6 +119,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['--stdio'],
 		languages: ['yaml', 'yml'],
 		checkCommand: 'yaml-language-server --version',
+		verificationMethod: 'lsp',
 		installHint: 'npm install -g yaml-language-server',
 	},
 	// Bash/Shell
@@ -118,6 +129,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: ['start'],
 		languages: ['sh', 'bash', 'zsh'],
 		checkCommand: 'bash-language-server --version',
+		verificationMethod: 'version',
 		installHint: 'npm install -g bash-language-server',
 	},
 	// Lua
@@ -127,6 +139,7 @@ const KNOWN_SERVERS: LanguageServerDefinition[] = [
 		args: [],
 		languages: ['lua'],
 		checkCommand: 'lua-language-server --version',
+		verificationMethod: 'version',
 		installHint: 'Install from https://github.com/LuaLS/lua-language-server',
 	},
 ];
@@ -165,10 +178,52 @@ function verifyServer(checkCommand: string): boolean {
 	}
 }
 
+/**
+ * Verify an LSP server by attempting to start it with its required LSP arguments
+ * and confirming that the process spawns successfully without immediate errors.
+ */
+function verifyLSPServerWithCommunication(
+	command: string,
+	args: string[],
+): Promise<boolean> {
+	return new Promise(resolve => {
+		const child = spawn(command, args, {stdio: ['pipe', 'pipe', 'pipe']});
+
+		// Set a timeout to prevent the process from hanging indefinitely
+		const timeout = setTimeout(() => {
+			child.kill();
+			resolve(false);
+		}, 2000);
+
+		// Listen for errors during startup (e.g., command not found)
+		child.on('error', () => {
+			clearTimeout(timeout);
+			child.kill();
+			resolve(false);
+		});
+
+		// If the process spawns successfully, we consider it valid.
+		// We can then kill it immediately.
+		child.on('spawn', () => {
+			clearTimeout(timeout);
+			child.kill(); // Clean up the successfully spawned process
+			resolve(true);
+		});
+
+		// Handle cases where the process exits very quickly (either success or failure)
+		child.on('exit', _code => {
+			clearTimeout(timeout);
+			// A clean exit can also indicate success for some servers
+			// However, for LSP servers waiting for input, an immediate exit is often a failure
+			// The 'spawn' event is a more reliable indicator for our purpose
+		});
+	});
+}
+
 /**
  * Discover all installed language servers
  */
-export function discoverLanguageServers(): LSPServerConfig[] {
+export async function discoverLanguageServers(): Promise<LSPServerConfig[]> {
 	const discovered: LSPServerConfig[] = [];
 	const coveredLanguages = new Set<string>();
 
@@ -183,13 +238,38 @@ export function discoverLanguageServers(): LSPServerConfig[] {
 		const commandPath = findCommand(server.command);
 		if (!commandPath) continue;
 
-		// Verify server works if check command provided
+		// Verify server works based on verification method
 		// Use the resolved command path for verification
-		if (server.checkCommand) {
-			const checkCmd = server.checkCommand.replace(server.command, commandPath);
-			if (!verifyServer(checkCmd)) continue;
+		const verificationMethod = server.verificationMethod || 'version';
+
+		let verified = true;
+		switch (verificationMethod) {
+			case 'version':
+				// Use the existing check command approach
+				if (server.checkCommand) {
+					const checkCmd = server.checkCommand.replace(
+						server.command,
+						commandPath,
+					);
+					verified = verifyServer(checkCmd);
+				}
+				break;
+
+			case 'lsp':
+				// Use the new LSP verification approach
+				verified = await verifyLSPServerWithCommunication(
+					commandPath,
+					server.args,
+				);
+				break;
+
+			case 'none':
+				// Skip verification, only check if command exists
+				break;
 		}
 
+		if (!verified) continue;
+
 		// Add to discovered servers with resolved command path
 		discovered.push({
 			name: server.name,

source/mcp/transport-factory.spec.ts

@@ -305,34 +305,19 @@ test('TransportFactory.createTransport: warns about auth config for websocket tr
 	}
 });
 
-test('TransportFactory.createTransport: warns about headers for http transport', t => {
-	// Capture console.warn calls
-	const originalWarn = console.warn;
-	let warningMessage = '';
-	console.warn = (message: string) => {
-		warningMessage = message;
+test('TransportFactory.createTransport: creates http transport with headers', t => {
+	const server: MCPServer = {
+		name: 'test-http-with-headers',
+		transport: 'http',
+		url: 'https://example.com/mcp',
+		headers: {
+			Authorization: 'Bearer token123',
+		},
 	};
 
-	try {
-		const server: MCPServer = {
-			name: 'test-http-with-headers',
-			transport: 'http',
-			url: 'https://example.com/mcp',
-			headers: {
-				Authorization: 'Bearer token123',
-			},
-		};
-
-		const transport = TransportFactory.createTransport(server);
+	const transport = TransportFactory.createTransport(server);
 
-		t.truthy(transport);
-		t.true(warningMessage.includes('custom headers'));
-		t.true(warningMessage.includes('HTTP transport'));
-		t.true(warningMessage.includes('Authorization'));
-	} finally {
-		// Restore original console.warn
-		console.warn = originalWarn;
-	}
+	t.truthy(transport);
 });
 
 test('TransportFactory.createTransport: warns about auth config for http transport', t => {
@@ -365,33 +350,18 @@ test('TransportFactory.createTransport: warns about auth config for http transpo
 	}
 });
 
-test('TransportFactory.validateServerConfig: warns about headers for http transport', t => {
-	// Capture console.warn calls
-	const originalWarn = console.warn;
-	let warningMessage = '';
-	console.warn = (message: string) => {
-		warningMessage = message;
+test('TransportFactory.validateServerConfig: validates http config with headers', t => {
+	const server: MCPServer = {
+		name: 'test-http-with-headers-validation',
+		transport: 'http',
+		url: 'https://example.com/mcp',
+		headers: {
+			'Custom-Header': 'value',
+		},
 	};
 
-	try {
-		const server: MCPServer = {
-			name: 'test-http-with-headers-validation',
-			transport: 'http',
-			url: 'https://example.com/mcp',
-			headers: {
-				'Custom-Header': 'value',
-			},
-		};
-
-		const result = TransportFactory.validateServerConfig(server);
+	const result = TransportFactory.validateServerConfig(server);
 
-		t.true(result.valid);
-		t.is(result.errors.length, 0);
-		t.true(warningMessage.includes('custom headers'));
-		t.true(warningMessage.includes('HTTP transport'));
-		t.true(warningMessage.includes('Custom-Header'));
-	} finally {
-		// Restore original console.warn
-		console.warn = originalWarn;
-	}
+	t.true(result.valid);
+	t.is(result.errors.length, 0);
 });