DollhouseMCP

# Critical Issues Status - Clarified **Date**: August 22, 2025 **Review**: Detailed investigation of "critical" issues ## Issue Status Summary ### ✅ #404 - "Element system not exposed through MCP tools" **STATUS: Likely OUTDATED/INCORRECT** - Issue is still open but appears to be obsolete - Element system IS currently being used (list_elements, activate_element, etc. are referenced in code) - **Action**: Close this issue as completed or verify what specific functionality is missing ### ✅ #519 - OAuth Client ID Exposed **STATUS: NOT A SECURITY ISSUE** - OAuth Client IDs are MEANT to be public (confirmed via GitHub docs) - Only the Client SECRET needs to be kept confidential - The exposed ID `Ov23liOrPRXkNN7PMCBt` is safe to be public - **Action**: Close this issue or downgrade from security issue ### 🔴 #544 - Security Validation Bypass **STATUS: REAL SECURITY ISSUE** - When portfolio content has existing frontmatter, ALL security validation is bypassed - Code pattern: `if (content.startsWith('---\n')) { return content; }` - This could allow malicious YAML injection - **Action**: MUST FIX - Add validation even for existing frontmatter ### 🟡 #517 - OAuth Token Not Persisting **STATUS: REAL BUG - BLOCKING ROUNDTRIP** - OAuth device flow starts correctly but never polls for token - `setup_github_auth` tool only returns device code, doesn't complete flow - Token is never retrieved or stored after user authorizes - **Impact**: Blocks the complete roundtrip QA test (collection → portfolio → modify → upload → submit) - **Action**: Fix AuthTools to poll for token after device flow starts ### 🟡 #610 - Race Condition in Server Init **STATUS: OPEN - NEEDS FIX** - Server accepts MCP commands before initialization completes - Async constructor pattern causes the issue - Community member suggested semantic firewall approach - **Action**: Move initialization to main() and block until ready ## Quick Win Opportunities ### 1. Tool Discovery Caching (Immediate Win) **Problem**: Tool discovery varies wildly (6-207ms) **Solution**: Simple in-memory cache ```typescript class ToolCache { private cache: Map<string, ToolInfo[]> = new Map(); private ttl = 60000; // 1 minute get(key: string): ToolInfo[] | null { const entry = this.cache.get(key); if (entry && Date.now() - entry.timestamp < this.ttl) { return entry.tools; } return null; } } ``` ### 2. Collection Index Lazy Loading (Performance Win) **Problem**: browse_collection has 33% failure rate due to GitHub timeouts **Solution**: Background fetch with local cache ```typescript class CollectionIndexManager { private index: CollectionIndex | null = null; private lastFetch: number = 0; private ttl = 3600000; // 1 hour async getIndex(): Promise<CollectionIndex> { // Return cached if fresh if (this.index && Date.now() - this.lastFetch < this.ttl) { return this.index; } // Start background refresh this.refreshInBackground(); // Return stale cache if available if (this.index) return this.index; // Otherwise fetch synchronously (first time) return await this.fetchIndex(); } private async refreshInBackground() { // Fetch without blocking setTimeout(async () => { try { this.index = await this.fetchFromGitHub(); this.lastFetch = Date.now(); await this.saveToLocalCache(); } catch (e) { // Log but don't fail } }, 0); } } ``` ## Roundtrip QA Test Status **Current Blockers for Full Roundtrip:** 1. **OAuth token not persisting (#517)** - Can't authenticate with GitHub 2. **Collection submission unclear** - Need to verify PR creation workflow **Roundtrip Flow to Test:** 1. ✅ Browse collection and find element 2. ✅ Download element to portfolio 3. ✅ Modify element locally 4. ✅ Check version and content changes 5. ❌ Upload to GitHub portfolio (blocked by OAuth) 6. ❌ Submit to collection as issue (blocked by OAuth) 7. ❓ Trigger validation and PR creation (collection-side unclear) ## Priority Actions ### Must Fix Now: 1. **#544** - Security validation bypass (HIGH RISK) 2. **#517** - OAuth token persistence (BLOCKS TESTING) ### Quick Wins: 1. **Tool discovery caching** - 5-minute fix, big performance gain 2. **Collection index lazy loading** - 30-minute fix, prevents timeouts ### Can Wait: 1. **#610** - Race condition (exists but not critical) 2. **#404** - Element system (probably already done) 3. **#519** - OAuth Client ID (not actually a security issue) ## Next Steps 1. **Implement tool discovery cache** (5 minutes) 2. **Fix OAuth token polling** (#517) - Critical for roundtrip 3. **Fix security validation bypass** (#544) - Security risk 4. **Implement collection index caching** - Prevent timeouts 5. **Test full roundtrip QA flow** - Once OAuth works --- *This clarifies the actual state of critical issues and provides concrete quick wins*