WINDOWS_CI_FAILURE_ANALYSIS.md•22.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