DollhouseMCP

# Session Notes: September 29, 2025 - Afternoon - Final Fixes & Test Isolation ## Session Overview **Date**: September 29, 2025, Afternoon (12:05 PM - 5:00 PM) **Duration**: ~5 hours **Focus**: Final fixes for PR #1193 (ElementFormatter) and critical test isolation issue **Main Achievement**: Merged ElementFormatter PR and fixed memory portfolio test isolation ## Key Accomplishments ### 1. ElementFormatter PR #1193 - MERGED ✅ **Started with**: 1 failing test, 4 SonarCloud code smells **Completed with**: All tests passing, all issues resolved #### Fixed Issues: - **Test Failure**: "should handle deeply nested escaped content" - Root cause: YAML correctly escapes special characters in quoted strings - Solution: Updated test expectations to match correct YAML behavior - Added verification that parsed content contains actual special characters - **SonarCloud Code Smells**: String.raw usage - Issue: 4 code smells about using String.raw for escape sequences - Solution: Properly used String.raw template literals with escape map - Result: SonarCloud compliant while maintaining functionality - **YAML Formatting Consistency**: - Improved block scalar handling for multiline content - Better configuration for YAML.dump() - Consistent treatment of special characters - **Element Type Detection Enhancement**: - Created explicit mapping object for element types - Better documentation and fallback logic - More maintainable and understandable code **PR Successfully Merged**: All 14 CI/CD checks passed ### 2. Memory Portfolio Test Isolation Issue #1194 - PR #1195 Created **Problem**: Tests finding 112+ real user memories instead of 3 test memories **Root Cause**: Singleton pattern not respecting test environment isolation #### Solution Implemented: - Set `DOLLHOUSE_PORTFOLIO_DIR` environment variable for explicit isolation - Reset both PortfolioManager and PortfolioIndexManager singletons - Fixed memory YAML structure (metadata at root level, not nested) - Added flexible name matching for transformed names #### Review Feedback Addressed: 1. **Singleton Reset Pattern**: Added TODO comments for future improvement 2. **Test Assertion Flexibility**: Created helper functions for name matching 3. **Console Logging**: Made conditional on DEBUG_TESTS environment 4. **TypeScript Assertions**: Added proper interfaces 5. **Magic Numbers**: Documented with constants 6. **SonarCloud Issue**: Fixed negated condition **Current Status**: - ✅ Test isolation working (3 memories vs 112+) - ✅ 3 tests passing - ⚠️ 5 tests still failing due to metadata transformation (separate issue) ### 3. Issue Management **Created Issues**: - #1194: Memory portfolio index test isolation issue **Issues Reviewed**: - #936: MCP Server Process Leak - Confirmed fixed (only 1 process found) - #919: Duplicate sync_portfolio tool names - Likely fixed - #973: Basic Memory element type - Mostly complete - #874: MCP tool output escaped strings - Addressed by ElementFormatter ## Technical Details ### ElementFormatter Test Fix ```javascript // Before: Expected literal tab in YAML output expect(formatted).toContain('Tab\t'); // After: Check for escaped representation and verify parsed content expect(formatted).toMatch(/Tab\\t.*return\\r/); const parsedYaml = yaml.load(formatted); expect(parsedYaml.entries[1].content).toContain('\t'); ``` ### Test Isolation Solution ```javascript // Key fix: Set environment BEFORE singleton initialization process.env.DOLLHOUSE_PORTFOLIO_DIR = path.join(tempDir, '.dollhouse', 'portfolio'); // Reset singletons in proper order (PortfolioManager as any).instance = null; (PortfolioIndexManager as any).instance = null; ``` ### Helper Functions Added ```javascript // Flexible name matching for transformed names const normalizeMemoryName = (name: string): string => { return name.replace(/[\s-]/g, '-').toLowerCase(); }; const findMemoryByName = (memories: any[], expectedName: string): any => { const normalized = normalizeMemoryName(expectedName); return memories.find(m => normalizeMemoryName(m.metadata.name) === normalized); }; ``` ## Remaining Issues ### Memory Test Failures (5 remaining) - Tests expect specific metadata but get transformed values - Example: description becomes "Memory element" - This is a PortfolioIndexManager transformation issue - Separate from test isolation problem ### Suggested Follow-ups 1. Fix metadata transformation in PortfolioIndexManager 2. Add proper `resetInstance()` methods to singleton classes 3. Consider integration test strategy improvements 4. Review and potentially close old issues (#919, #936) ## Code Quality Improvements ### ElementFormatter - Better YAML formatting consistency - Improved element type detection - Proper use of String.raw for SonarCloud compliance - Enhanced test expectations for YAML behavior ### Test Suite - Proper test isolation from user environment - Conditional debug logging - Helper functions for maintainability - Better TypeScript typing - Documented magic numbers ## Lessons Learned 1. **YAML Behavior**: YAML correctly escapes special characters in quoted strings - this is standard behavior, not a bug 2. **String.raw Usage**: Works well for creating literal strings with backslashes, perfect for escape sequence maps 3. **Test Isolation**: Environment variables must be set BEFORE singleton initialization 4. **Singleton Pattern**: Need proper reset methods for testing - direct property manipulation is fragile 5. **Name Transformations**: Portfolio indexer may transform names - tests need flexible matching ## CI/CD Status - ✅ PR #1193 (ElementFormatter): Merged successfully - 🔄 PR #1195 (Test Isolation): Under review, partially fixes test failures - ⚠️ Overall test suite: Still has pre-existing failures unrelated to our changes ## Session Summary Productive session with two major accomplishments: 1. Successfully merged ElementFormatter PR with all issues resolved 2. Fixed critical test isolation issue improving CI/CD reliability The test isolation fix is particularly important as it removes dependency on developer's local portfolio state, making tests reproducible and reliable. While 5 tests still fail due to metadata transformation, the main isolation issue is resolved. ## Next Steps 1. Get PR #1195 reviewed and merged 2. Create follow-up issue for metadata transformation problem 3. Add proper singleton reset methods 4. Review and close outdated issues