DollhouseMCP

# PR #700 Fixes Coordination Document **Date**: August 22, 2025 **PR**: #700 - Performance Quick Wins **Orchestrator**: Opus 4.1 **Objective**: Address all review feedback and fix build failures ## Current Status ### Build Failures ❌ - Test suite failing on all platforms (Ubuntu, macOS, Windows) - Likely TypeScript compilation or import issues ### Review Feedback Summary 1. **Critical Gap**: No unit tests for CollectionIndexManager (594 lines untested) 2. **Security**: Low severity - missing audit logging in ServerSetup 3. **Minor Issues**: Magic numbers, lack of constants 4. **Build Failures**: Tests not passing on CI ## Task Breakdown for Agents ### Task 1: Fix Build Failures (Agent: Sonnet) **Priority**: CRITICAL - Blocks everything **Estimated Time**: 20 minutes #### Requirements - [ ] Run `npm test` locally to identify failures - [ ] Check TypeScript compilation errors - [ ] Fix import/export issues - [ ] Ensure all type definitions are correct - [ ] Verify tests pass locally before committing #### Specific Areas to Check - `src/types/collection.js` import in CollectionIndexManager - Missing type exports - Circular dependency issues - Test file imports #### Success Criteria - All tests pass locally - TypeScript compilation succeeds - No import errors --- ### Task 2: Add CollectionIndexManager Tests (Agent: Sonnet) **Priority**: HIGH - Critical gap **Estimated Time**: 45 minutes #### Requirements - [ ] Create `test/__tests__/unit/collection/CollectionIndexManager.test.ts` - [ ] Test all public methods - [ ] Test background refresh mechanism - [ ] Test circuit breaker behavior - [ ] Test file caching and persistence - [ ] Test error handling and retries - [ ] Test ETags and conditional requests - [ ] Achieve >80% code coverage #### Test Scenarios to Cover 1. **Happy Path** - Fetch index successfully - Cache hit returns immediately - Background refresh works 2. **Error Handling** - Network failures trigger retries - Circuit breaker opens after failures - Falls back to cached data 3. **File Operations** - Saves to local cache - Loads from cache on startup - Handles corrupt cache files 4. **Performance** - Stale-while-revalidate works - Doesn't block on refresh - Respects TTL #### Success Criteria - At least 15 comprehensive tests - All edge cases covered - Mock GitHub API calls properly - Tests run fast (<5 seconds total) --- ### Task 3: Fix Minor Issues & Add Security Logging (Agent: Sonnet) **Priority**: MEDIUM **Estimated Time**: 20 minutes #### Requirements - [ ] Extract magic numbers to named constants - [ ] Add security audit logging to ServerSetup - [ ] Add cache statistics getter methods - [ ] Extract jitter calculation to separate method #### Specific Fixes 1. **CollectionIndexManager.ts** ```typescript // Line 176 - Add constant private readonly REFRESH_THRESHOLD = 0.8; // Line 281 - Extract method private addJitter(delay: number): number { const jitter = delay * 0.25 * (Math.random() - 0.5); return Math.max(0, delay + jitter); } ``` 2. **ServerSetup.ts** ```typescript // Add security logging when cache is invalidated import { SecurityMonitor } from '../security/securityMonitor.js'; // In invalidateToolCache() SecurityMonitor.logSecurityEvent({ type: 'TOOL_CACHE_INVALIDATED', severity: 'LOW', source: 'ServerSetup.invalidateToolCache', details: 'Tool discovery cache manually invalidated' }); ``` 3. **Add Statistics Getter** ```typescript getCacheStatistics() { return { toolCache: this.toolCache.getStats(), // Add collection stats if available }; } ``` #### Success Criteria - No magic numbers in code - Security events logged appropriately - Code more maintainable - All existing tests still pass --- ## Agent Instructions ### For All Tasks 1. **Test First**: Run tests before making changes to understand failures 2. **Incremental Changes**: Make one fix at a time and test 3. **Clear Comments**: Add comments explaining any complex fixes 4. **Preserve Functionality**: Don't break existing features ### Coordination Notes - Task 1 MUST be completed first (build failures block everything) - Tasks 2 and 3 can run in parallel after Task 1 - All tasks should be completed on the same branch ### Git Workflow ```bash # You're already on feature/performance-quick-wins # Make changes and test npm test # Commit with clear message git add -A git commit -m "fix: [specific fix description]" # Don't push - orchestrator will handle that ``` ## Success Metrics - ✅ All CI checks passing - ✅ 100% of CollectionIndexManager covered by tests - ✅ Security audit shows 0 findings (or acknowledges the logging) - ✅ Code review concerns addressed - ✅ No magic numbers remaining ## Testing Commands ```bash # Run all tests npm test # Run specific test file npm test -- CollectionIndexManager # Check TypeScript compilation npm run build # Run linter npm run lint ``` ## Notes for Agents - The build is currently failing - fix this FIRST - CollectionIndexManager is complex - focus on critical paths in tests - Security logging should use existing SecurityMonitor patterns - Keep changes focused and minimal --- *Ready for agent delegation - Task 1 is most critical*