Skip to main content
Glama

DollhouseMCP

by DollhouseMCP
WINDOWS_CI_FAILURE_ANALYSIS.md22.6 kB
# Windows CI Failure Analysis: Test Element Detection **Issue**: Test "should block test elements in production environment" failing on Windows CI **Status**: Investigation complete - Root cause identified **Date**: 2025-08-21 ## Problem Summary The test `DefaultElementProvider.metadata.test.ts:254` expects 1 file to be copied but receives 0 files on Windows CI. The test creates: - `test-element.md` (should be blocked) - contains `_dollhouseMCPTest: true` - `regular-element.md` (should be copied) - normal element Expected behavior: Copy 1 file (regular-element.md), block the test element Windows CI behavior: Copy 0 files (both files are being ignored) ## Root Cause Analysis ### Primary Issue: Line Ending Mismatch in Metadata Parsing **Location**: `src/portfolio/DefaultElementProvider.ts:475` ```typescript const match = header.match(/^---\n([\s\S]*?)\n---/); ``` **Problem**: The regex pattern expects Unix line endings (`\n`) but Windows uses CRLF line endings (`\r\n`). **Impact**: 1. On Windows, files created by the test have CRLF line endings 2. The regex fails to match the frontmatter delimiters 3. `readMetadataOnly()` returns `null` instead of parsing metadata 4. `isDollhouseMCPTestElement()` returns `false` for all files 5. The production safety check fails to detect test elements 6. ALL files get filtered out by some other mechanism ### Secondary Issue: Production Environment Detection in CI The test sets `FORCE_PRODUCTION_MODE=true` and expects production-like behavior. However, in Windows CI environment: **Current Environment Detection** (line 597-609): ```typescript const indicators = { hasUserHomeDir: (process.env.HOME?.includes('/Users/') || process.env.HOME?.includes('/home/')) || !!process.env.USERPROFILE, isProductionNode: process.env.NODE_ENV === 'production', notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp'), notInCI: !process.env.CI, noTestEnv: process.env.NODE_ENV !== 'test', noDevEnv: process.env.NODE_ENV !== 'development', }; ``` **Windows CI Environment Issues**: 1. `notInCI: !process.env.CI` - CI is set, so this is false (-1 score) 2. `notInTestDir` - Windows temp paths may include `/temp` which could affect scoring 3. `process.cwd()` in Windows uses backslashes, but the check only looks for forward slashes ### Potential Secondary Filter Since BOTH files are being blocked (0 copied instead of 1), there may be another filtering mechanism being triggered. Possible causes: 1. **Temp directory path detection**: Windows CI temp paths might trigger the `notInTestDir` check 2. **File system timing**: Windows file system operations may have different timing characteristics 3. **Unicode normalization issues**: File names could be getting normalized differently on Windows ## Windows vs Unix Differences ### 1. Line Endings - **Unix/Linux/macOS**: `\n` (LF) - **Windows**: `\r\n` (CRLF) - **Impact**: Regex patterns expecting `\n` fail on Windows ### 2. Path Separators - **Unix/Linux/macOS**: `/` (forward slash) - **Windows**: `\` (backslash) - **Node.js**: Generally handles this with `path.join()`, but string checks may fail ### 3. Environment Variables - **Unix/Linux/macOS**: `HOME` for user home directory - **Windows**: `USERPROFILE` for user home directory - **Impact**: Code correctly handles both, but other env vars may differ ### 4. Temporary Directory Paths - **Unix/Linux/macOS**: `/tmp` or `/var/tmp` - **Windows**: `C:\Users\{user}\AppData\Local\Temp` - **Impact**: May contain `/temp` substring which triggers test directory detection ## Specific Code Issues ### Issue 1: Line Ending Insensitive Regex (CRITICAL) **File**: `src/portfolio/DefaultElementProvider.ts:475` **Current**: ```typescript const match = header.match(/^---\n([\s\S]*?)\n---/); ``` **Problem**: Only matches LF line endings **Fix Needed**: Support both LF and CRLF ### Issue 2: Path Separator in Directory Detection **File**: `src/portfolio/DefaultElementProvider.ts:602` **Current**: ```typescript notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp') ``` **Problem**: Only checks for forward slash paths **Fix Needed**: Normalize paths or check both separators ## Recommended Fixes ### Fix 1: Cross-Platform Line Ending Support (HIGH PRIORITY) ```typescript // Current problematic code: const match = header.match(/^---\n([\s\S]*?)\n---/); // Proposed fix: const match = header.match(/^---\r?\n([\s\S]*?)\r?\n---/); ``` **Rationale**: The `\r?` makes the carriage return optional, supporting both LF and CRLF line endings. ### Fix 2: Cross-Platform Path Detection ```typescript // Current problematic code: notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp') // Proposed fix: notInTestDir: (() => { const cwd = process.cwd().toLowerCase().replace(/\\/g, '/'); return !cwd.includes('/test') && !cwd.includes('/__tests__') && !cwd.includes('/temp'); })() ``` **Rationale**: Normalize backslashes to forward slashes and use case-insensitive matching for Windows compatibility. ### Fix 3: Enhanced Debug Logging (MEDIUM PRIORITY) Add debug logging to understand exactly what's happening in the copyElementFiles method on Windows: ```typescript // In copyElementFiles method, add logging: logger.debug(`[DefaultElementProvider] Processing file: ${normalizedFile.normalizedContent}`, { sourcePath, destPath, isProductionEnv: this.isProductionEnvironment(), loadTestData: this.config.loadTestData, platform: process.platform }); // Before the isDollhouseMCPTestElement check: const metadata = await this.readMetadataOnly(sourcePath); logger.debug(`[DefaultElementProvider] Metadata read result:`, { file: normalizedFile.normalizedContent, hasMetadata: !!metadata, isDollhouseTest: !!(metadata && metadata._dollhouseMCPTest === true), rawMetadata: metadata }); ``` ### Fix 4: Test Enhancement for Platform Verification Add a specific test that verifies cross-platform line ending support: ```typescript it('should handle CRLF line endings on Windows', async () => { const testFile = path.join(tempDir, 'crlf-test.md'); // Explicitly use CRLF line endings const content = `---\r\nname: Test Element\r\n_dollhouseMCPTest: true\r\n---\r\n# Test Content`; await fs.writeFile(testFile, content); const metadata = await (provider as any).readMetadataOnly(testFile); expect(metadata).toEqual({ name: 'Test Element', _dollhouseMCPTest: true }); }); ``` ## Alternative Approaches ### Option 1: Use Built-in Line Ending Handling Instead of regex, use more robust text processing: ```typescript // Replace the regex approach with: const lines = header.split(/\r?\n/); if (lines[0] === '---') { const endIndex = lines.findIndex((line, index) => index > 0 && line === '---'); if (endIndex > 0) { const yamlContent = lines.slice(1, endIndex).join('\n'); // Process yamlContent... } } ``` ### Option 2: Force Unix Line Endings in Tests In the test setup, explicitly force LF line endings: ```typescript // In beforeEach or test setup: const content = `---\nname: Test Element\n_dollhouseMCPTest: true\n---\n# Test Content`; // Ensure LF line endings regardless of platform await fs.writeFile(testFile, content.replace(/\r?\n/g, '\n')); ``` ## Testing Strategy ### Immediate Tests Needed 1. **Line Ending Test**: Create files with CRLF and verify metadata parsing 2. **Path Detection Test**: Verify production detection works in temp directories 3. **End-to-End Windows Test**: Run copyElementFiles with Windows-style paths ### Long-term Testing 1. **Cross-Platform CI**: Ensure all three platforms (Ubuntu, Windows, macOS) pass 2. **Line Ending Matrix**: Test with LF, CRLF, and mixed line endings 3. **Path Matrix**: Test with various Windows temp directory structures ## Conclusion The root cause is the regex pattern in `readMetadataOnly()` not handling Windows CRLF line endings. This causes metadata parsing to fail, which prevents test element detection, leading to unexpected file filtering behavior. **Priority**: HIGH - This affects production safety checks on Windows systems. **Risk**: MEDIUM - Could allow test elements to be copied in production on Windows if the regex fix isn't applied. **Next Steps**: 1. Apply Fix 1 (line ending regex) immediately 2. Add debug logging to confirm the fix 3. Run Windows CI to verify resolution 4. Apply additional fixes for robustness --- ## Second Investigation - Continuing Failures **Date**: 2025-08-21 (Second Analysis) **Status**: CRITICAL - Root cause identified, line ending fix was insufficient ### Current Failure Pattern (Post Line-Ending Fix) After applying the regex fix for Windows line endings (`/^---\r?\n([\s\S]*?)\r?\n---/`), the test is still failing on Windows CI with the same pattern: - **Expected**: 1 file copied (regular-element.md) - **Received**: 0 files copied (NO files being copied at all) This indicates the line ending fix was applied correctly but there's a deeper, more fundamental issue preventing ALL file operations on Windows. ### New Root Cause Analysis The failure pattern has NOT changed after the line ending fix, which means the issue is NOT the metadata parsing regex. The test expects: 1. `test-element.md` to be blocked (contains `_dollhouseMCPTest: true`) 2. `regular-element.md` to be copied (normal element) 3. Final count: 1 file copied But Windows CI shows **0 files copied**, meaning BOTH files are being blocked or the copy operation itself is failing. ### Critical Issues Discovered #### Issue 1: Windows Working Directory Detection (CRITICAL) **Location**: `DefaultElementProvider.ts:603-604` ```typescript notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp') && !process.cwd().includes('/dist/test') ``` **Windows CI Environment**: - Working directory: `D:\a\mcp-server\mcp-server` (typical GitHub Actions Windows runner) - Uses backslashes (`\`) not forward slashes (`/`) - The check `!process.cwd().includes('/temp')` may miss `\temp` or similar Windows paths **Impact**: Production environment detection may be failing, causing different behavior than expected. #### Issue 2: Windows File Path Validation (POTENTIAL) **Location**: `DefaultElementProvider.ts:671-679` ```typescript if (!this.validateFilePath(sourcePath, [sourceDir]) || !this.validateFilePath(destPath, [destDir])) { // File gets skipped continue; } ``` **Windows Issues**: - `path.normalize()` behavior differs between Windows and Unix - Windows path validation may be more restrictive - UNC paths or long path names might trigger validation failures #### Issue 3: File System Timing/Permissions (POTENTIAL) **Windows CI Observations**: - Multiple EPERM errors seen in other tests: `EPERM: operation not permitted, rmdir` - Windows file system locking behavior is different from Unix - File handles may not be released immediately after operations ### Step-by-Step Analysis of copyElementFiles Execution The method processes files in this order: 1. **File enumeration**: `await fs.readdir(sourceDir)` - Gets both files ✓ 2. **Extension check**: Both files end with `.md` ✓ 3. **Unicode normalization**: Should pass for basic ASCII filenames ✓ 4. **Path validation**: `validateFilePath()` - **MAY FAIL ON WINDOWS** ❌ 5. **Production safety check**: Metadata parsing and test detection ❓ 6. **Existing file check**: `fs.access(destPath)` ✓ (files don't exist) 7. **File stats check**: Size validation ✓ 8. **Copy operation**: `copyFileWithVerification()` ❓ ### Most Likely Root Cause: Path Validation Failure Based on the failure pattern (0 files copied), the most likely issue is in step 4 (path validation). The `validateFilePath()` method may be rejecting Windows-style paths or temp directory paths. **Evidence**: - Both files fail to copy (suggests early rejection in the process) - No indication of metadata parsing issues (which would only affect test file) - Windows CI shows different permission behaviors elsewhere ### Specific Code Locations That Are Problematic #### Location 1: Path Validation Logic **File**: `DefaultElementProvider.ts:252-287` ```typescript private validateFilePath(filePath: string, allowedBasePaths?: string[]): boolean { const normalizedPath = path.normalize(filePath); // This check might fail on Windows temp directories if (normalizedPath.includes('..') || normalizedPath.includes('~')) { return false; } // This absolute path checking might be problematic on Windows if (path.isAbsolute(normalizedPath) && allowedBasePaths) { // Windows path matching may fail here } } ``` #### Location 2: Production Environment Score Calculation **File**: `DefaultElementProvider.ts:603-604` ```typescript notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp') && !process.cwd().includes('/dist/test') ``` ### Recommended Immediate Fix **Priority 1: Enhanced Debug Logging** Add comprehensive debug logging to the `copyElementFiles` method to understand exactly where the process is failing on Windows: ```typescript private async copyElementFiles(sourceDir: string, destDir: string, elementType: string): Promise<number> { let copiedCount = 0; // ADD: Log environment details logger.debug('[DefaultElementProvider] Starting copyElementFiles', { sourceDir, destDir, elementType, platform: process.platform, cwd: process.cwd(), isProduction: this.isProductionEnvironment(), loadTestData: this.config.loadTestData }); try { await fs.mkdir(destDir, { recursive: true }); const files = await fs.readdir(sourceDir); // ADD: Log file discovery logger.debug('[DefaultElementProvider] Files discovered', { files, count: files.length }); for (const file of files) { logger.debug('[DefaultElementProvider] Processing file', { file }); // Only copy markdown files if (!file.endsWith(FILE_CONSTANTS.ELEMENT_EXTENSION)) { logger.debug('[DefaultElementProvider] Skipped non-markdown file', { file }); continue; } // Normalize filename for security const normalizedFile = UnicodeValidator.normalize(file); if (!normalizedFile.isValid) { logger.warn(`[DefaultElementProvider] Invalid Unicode in filename: ${file}`); continue; } const sourcePath = path.join(sourceDir, normalizedFile.normalizedContent); const destPath = path.join(destDir, normalizedFile.normalizedContent); // ADD: Log path details logger.debug('[DefaultElementProvider] Path details', { file: normalizedFile.normalizedContent, sourcePath, destPath, sourceExists: await fs.access(sourcePath).then(() => true).catch(() => false) }); // SECURITY FIX: Validate file paths const sourceValid = this.validateFilePath(sourcePath, [sourceDir]); const destValid = this.validateFilePath(destPath, [destDir]); // ADD: Log validation results logger.debug('[DefaultElementProvider] Path validation', { file: normalizedFile.normalizedContent, sourceValid, destValid, sourceDir, destDir }); if (!sourceValid || !destValid) { logger.warn('[DefaultElementProvider] Path validation failed', { file: normalizedFile.normalizedContent, sourcePath, destPath, sourceValid, destValid }); continue; } // Production safety check if (!this.config.loadTestData && this.isProductionEnvironment()) { const isDollhouseTest = await this.isDollhouseMCPTestElement(sourcePath); // ADD: Log production check results logger.debug('[DefaultElementProvider] Production safety check', { file: normalizedFile.normalizedContent, isDollhouseTest, isProduction: this.isProductionEnvironment(), loadTestData: this.config.loadTestData }); if (isDollhouseTest) { logger.warn('[DefaultElementProvider] Blocked test element in production', { file: normalizedFile.normalizedContent }); continue; } } // Check if destination exists try { await fs.access(destPath); logger.debug('[DefaultElementProvider] File already exists, skipping', { file: normalizedFile.normalizedContent }); continue; } catch { // File doesn't exist, proceed } // Proceed with copy... logger.debug('[DefaultElementProvider] About to copy file', { file: normalizedFile.normalizedContent, sourcePath, destPath }); // ... rest of copy logic } // ADD: Final summary logger.debug('[DefaultElementProvider] Copy operation completed', { elementType, copiedCount, totalProcessed: files.length }); } catch (error) { logger.error('[DefaultElementProvider] copyElementFiles error', { error, elementType }); } return copiedCount; } ``` **Priority 2: Fix Windows Path Detection** ```typescript // Fix the notInTestDir detection for Windows paths notInTestDir: (() => { const cwd = process.cwd().toLowerCase(); // Normalize path separators for cross-platform checking const normalizedCwd = cwd.replace(/\\/g, '/'); return !normalizedCwd.includes('/test') && !normalizedCwd.includes('/__tests__') && !normalizedCwd.includes('/temp') && !normalizedCwd.includes('/dist/test'); })() ``` ### Next Steps 1. **Apply enhanced debug logging** to understand the exact failure point 2. **Run Windows CI** with debug logging enabled 3. **Analyze the debug output** to determine if it's path validation, production detection, or another issue 4. **Apply targeted fix** based on the debug results 5. **Verify fix** with another Windows CI run --- ## Implementation **Date**: 2025-08-21 (Implementation Phase) **Status**: DEBUG LOGGING IMPLEMENTED - Ready for Windows CI testing **Changes Applied**: Comprehensive debug logging + Windows path fixes ### Code Changes Made #### 1. Enhanced Debug Logging in copyElementFiles Method **Location**: `src/portfolio/DefaultElementProvider.ts:714-920` **Added comprehensive console.log statements** (will show in CI output): - **Lines 714-725**: Environment details at method start (platform, cwd, production detection, env vars) - **Lines 735**: File discovery results - **Lines 738**: Per-file processing start - **Lines 742**: Non-markdown file skips - **Lines 757-760**: Path details and source file existence - **Lines 770-778**: Path validation results for both source and dest - **Lines 799-806**: Production safety check results - **Lines 845**: File existence check results - **Lines 870-873**: About to copy file details - **Lines 879-882**: Successful copy confirmation - **Lines 916-922**: Final operation summary #### 2. Enhanced Debug Logging in validateFilePath Method **Location**: `src/portfolio/DefaultElementProvider.ts:258-329` **Added detailed path validation logging**: - **Lines 258-263**: Input parameters and platform info - **Lines 267-271**: Traversal pattern detection failures - **Lines 281-287**: Base path checking iterations - **Lines 289-294**: Absolute path validation results - **Lines 309-312**: Null byte detection failures - **Lines 317-320**: Successful validation confirmations - **Lines 323-326**: Exception handling #### 3. Enhanced Debug Logging in isDollhouseMCPTestElement Method **Location**: `src/portfolio/DefaultElementProvider.ts:621-639` **Added metadata parsing logging**: - **Lines 621-627**: Metadata parsing results with full details - **Lines 632-636**: Error handling for metadata failures #### 4. Fixed Windows Path Detection (CRITICAL) **Location**: `src/portfolio/DefaultElementProvider.ts:662-670` **Before** (problematic): ```typescript notInTestDir: !process.cwd().includes('/test') && !process.cwd().includes('/__tests__') && !process.cwd().includes('/temp') && !process.cwd().includes('/dist/test') ``` **After** (cross-platform): ```typescript notInTestDir: (() => { const cwd = process.cwd().toLowerCase(); // Normalize path separators for cross-platform checking (Windows uses \ but checks use /) const normalizedCwd = cwd.replace(/\\/g, '/'); return !normalizedCwd.includes('/test') && !normalizedCwd.includes('/__tests__') && !normalizedCwd.includes('/temp') && !normalizedCwd.includes('/dist/test'); })(), ``` **Fix Impact**: Windows paths like `D:\a\mcp-server\mcp-server` are now converted to `d:/a/mcp-server/mcp-server` for consistent checking. ### Verification on macOS ✅ **All tests pass** with debug logging enabled ✅ **Test flow confirmed**: - Discovers 2 files: `regular-element.md`, `test-element.md` - Validates paths correctly - Detects test metadata correctly (`_dollhouseMCPTest: true`) - Blocks test element in production environment - Copies 1 file (regular element) as expected ### Expected Windows CI Behavior The debug logging will now show exactly where the Windows CI process fails: 1. **Environment Detection**: Will show if production detection is working correctly 2. **Path Validation**: Will show if Windows paths are causing validation failures 3. **Metadata Reading**: Will show if line ending issues persist in metadata parsing 4. **Production Safety**: Will show the exact test detection results ### Next Steps 1. **Push changes** to trigger Windows CI with debug logging 2. **Analyze CI output** to pinpoint the exact failure location 3. **Apply targeted fix** based on debug results 4. **Clean up debug logging** once issue is resolved --- **Investigation completed by**: Claude Code Analysis **Files analyzed**: - `/Users/mick/Developer/Organizations/DollhouseMCP/active/mcp-server/src/portfolio/DefaultElementProvider.ts` - `/Users/mick/Developer/Organizations/DollhouseMCP/active/mcp-server/test/__tests__/unit/portfolio/DefaultElementProvider.metadata.test.ts` - `/Users/mick/Developer/Organizations/DollhouseMCP/active/mcp-server/.github/workflows/cross-platform-simple.yml` - GitHub Actions logs from run 17128335175

MCP directory API

We provide all the information about MCP servers via our MCP API.

curl -X GET 'https://glama.ai/api/mcp/v1/servers/DollhouseMCP/DollhouseMCP'

If you have feedback or need assistance with the MCP directory API, please join our Discord server