michaelschiemer/docs/refactoring/mcp-module-refactoring-plan.md

# MCP Module Refactoring Plan

## Problem Analysis

### Critical Issues Identified

1. **Complete Tool Duplication**: 16 tool classes exist in BOTH `/Tools` and `/Tools/Categories` with identical MCP tool names
2. **Architectural Inconsistency**: Two competing implementation patterns (old vs new)
3. **Discovery Pollution**: Framework discovers and registers both versions of same tools
4. **Maintenance Nightmare**: Changes must be made in two places

## Duplication Matrix

### Confirmed Duplicates (16 Tools)

| Tool Name | Root Location | Root Lines | Categories Location | Categories Lines | Delta | Pattern |
|-----------|---------------|------------|---------------------|------------------|-------|---------|
| **CacheTools** | `/Tools/CacheTools.php` | 143 | `/Tools/Categories/Performance/` | 758 | +615 | OLD → NEW |
| **CodeQualityTools** | `/Tools/CodeQualityTools.php` | 931 | `/Tools/Categories/Development/` | 1398 | +467 | OLD → NEW |
| **ContainerInspector** | `/Tools/ContainerInspector.php` | 761 | `/Tools/Categories/Analysis/` | 657 | -104 | OLD → NEW |
| **DatabaseOptimizationTools** | `/Tools/DatabaseOptimizationTools.php` | 297 | `/Tools/Categories/Database/` | 789 | +492 | OLD → NEW |
| **DatabaseTools** | `/Tools/DatabaseTools.php` | 88 | `/Tools/Categories/Database/` | 314 | +226 | OLD → NEW |
| **EventFlowVisualizer** | `/Tools/EventFlowVisualizer.php` | 879 | `/Tools/Categories/Analysis/` | 669 | -210 | OLD → NEW |
| **FileSystemTools** | `/Tools/FileSystemTools.php` | 247 | `/Tools/Categories/System/` | 1120 | +873 | OLD → NEW |
| **FrameworkAgents** | `/Tools/FrameworkAgents.php` | 470 | `/Tools/Categories/Development/` | 1691 | +1221 | OLD → NEW |
| **FrameworkTools** | `/Tools/FrameworkTools.php` | 377 | `/Tools/Categories/Development/` | 1207 | +830 | OLD → NEW |
| **LogTools** | `/Tools/LogTools.php` | 200 | `/Tools/Categories/Development/` | 729 | +529 | OLD → NEW |
| **MiddlewareChainAnalyzer** | `/Tools/MiddlewareChainAnalyzer.php` | 1008 | `/Tools/Categories/Analysis/` | 704 | -304 | OLD → NEW |
| **PerformanceTools** | `/Tools/PerformanceTools.php` | 378 | `/Tools/Categories/Performance/` | 857 | +479 | OLD → NEW |
| **RouteDebugger** | `/Tools/RouteDebugger.php` | 751 | `/Tools/Categories/Analysis/` | 638 | -113 | OLD → NEW |
| **SecurityAuditTools** | `/Tools/SecurityAuditTools.php` | 928 | `/Tools/Categories/Security/` | 1147 | +219 | OLD → NEW |
| **SecurityConfigurationTools** | `/Tools/SecurityConfigurationTools.php` | 702 | `/Tools/Categories/Security/` | 676 | -26 | OLD → NEW |
| **SecurityMonitoringTools** | `/Tools/SecurityMonitoringTools.php` | 569 | `/Tools/Categories/Security/` | 544 | -25 | OLD → NEW |

**Total Duplicated Lines**: 8,729 (root) + 13,698 (categories) = **22,427 lines** (with ~8,729 lines of pure duplication)

### Unique Files (No Duplicates)

| File | Location | Lines | Status |
|------|----------|-------|--------|
| **CodebaseAnalyzer** | `/Tools/CodebaseAnalyzer.php` | ~468 | ✅ KEEP - Newly added, no duplicate |
| **DependencyAnalysisTools** | `/Tools/DependencyAnalysisTools.php` | ~TBD | ⚠️ CHECK - No Categories version found |
| **HotReloadTool** | `/Tools/HotReloadTool.php` | ~TBD | ⚠️ CHECK - No Categories version found |
| **SchedulerQueuePipelineAgent** | `/Tools/SchedulerQueuePipelineAgent.php` | ~TBD | ⚠️ CHECK - No Categories version found |
| **FrameworkInitializerAgent** | `/Tools/FrameworkInitializerAgent.php` | ~TBD | ⚠️ CHECK - No Categories version found |
| **TestingTools** | `/Tools/TestingTools.php` | ~TBD | ⚠️ CHECK - No Categories version found |

## Pattern Comparison

### OLD Pattern (Root /Tools/*.php)

```php
final readonly class SomeTool
{
    public function __construct(
        private SomeDependency $dependency
    ) {}

    #[McpTool(
        name: 'tool_name',
        description: 'Basic description',
        inputSchema: [ /* manual JSON schema */ ]
    )]
    public function toolMethod(string $param): array
    {
        try {
            // Direct implementation
            return ['result' => 'value'];
        } catch (\Throwable $e) {
            return ['error' => $e->getMessage()];
        }
    }
}
```

**Characteristics**:
- ❌ No `McpToolContext` dependency
- ❌ Manual `inputSchema` definition (verbose)
- ❌ No standardized error handling
- ❌ No result formatting options
- ❌ No caching support
- ❌ No category/tags metadata
- ❌ Basic error messages

### NEW Pattern (Categories /Tools/Categories/*)

```php
final readonly class SomeTool
{
    public function __construct(
        private McpToolContext $context,
        private SomeDependency $dependency
    ) {}

    #[McpTool(
        name: 'tool_name',
        description: 'Enhanced description with detail',
        category: 'Category Name',
        tags: ['tag1', 'tag2', 'searchable'],
        cacheable: true,
        defaultCacheTtl: 300
    )]
    public function toolMethod(
        string $param,
        string $format = 'array'
    ): array {
        try {
            $result = $this->performOperation($param);
            return $this->context->formatResult($result, $format);
        } catch (\Throwable $e) {
            return $this->context->handleError($e, [
                'operation' => 'tool_name',
                'param' => $param,
                'format' => $format
            ]);
        }
    }

    private function performOperation(string $param): array
    {
        // Business logic separated
        return ['result' => 'value'];
    }
}
```

**Characteristics**:
- ✅ `McpToolContext` for standardized handling
- ✅ No manual `inputSchema` needed (auto-generated from signature)
- ✅ Standardized error handling with context
- ✅ Multiple output formats (array, json, yaml, markdown)
- ✅ Built-in caching support with TTL
- ✅ Rich metadata (category, tags) for discovery
- ✅ Detailed error context and debugging info
- ✅ Separation of concerns (business logic vs MCP handling)

## Refactoring Strategy

### Phase 1: Immediate Cleanup (CRITICAL)

**Delete all 16 duplicate files from `/Tools` root**:

```bash
# Files to DELETE immediately:
rm src/Framework/Mcp/Tools/CacheTools.php
rm src/Framework/Mcp/Tools/CodeQualityTools.php
rm src/Framework/Mcp/Tools/ContainerInspector.php
rm src/Framework/Mcp/Tools/DatabaseOptimizationTools.php
rm src/Framework/Mcp/Tools/DatabaseTools.php
rm src/Framework/Mcp/Tools/EventFlowVisualizer.php
rm src/Framework/Mcp/Tools/FileSystemTools.php
rm src/Framework/Mcp/Tools/FrameworkAgents.php
rm src/Framework/Mcp/Tools/FrameworkTools.php
rm src/Framework/Mcp/Tools/LogTools.php
rm src/Framework/Mcp/Tools/MiddlewareChainAnalyzer.php
rm src/Framework/Mcp/Tools/PerformanceTools.php
rm src/Framework/Mcp/Tools/RouteDebugger.php
rm src/Framework/Mcp/Tools/SecurityAuditTools.php
rm src/Framework/Mcp/Tools/SecurityConfigurationTools.php
rm src/Framework/Mcp/Tools/SecurityMonitoringTools.php
```

**Expected Impact**:
- ✅ Removes 8,729 lines of duplicate code
- ✅ Eliminates double MCP tool registration
- ✅ Forces use of new pattern (Categories)
- ✅ Single source of truth for each tool

### Phase 2: Unique Files Migration

**Files to investigate and migrate**:

1. **CodebaseAnalyzer.php** - Already in NEW pattern, move to `/Tools/Categories/Codebase/`
2. **DependencyAnalysisTools.php** - Check if needs Categories version
3. **HotReloadTool.php** - Check if needs Categories version
4. **SchedulerQueuePipelineAgent.php** - Check if needs Categories version
5. **FrameworkInitializerAgent.php** - Likely agent, may not need migration
6. **TestingTools.php** - Move to `/Tools/Categories/Testing/`

### Phase 3: Directory Structure Optimization

**Target Structure**:

```
src/Framework/Mcp/
├── Core/                           # Framework core MCP services
│   ├── Services/
│   │   ├── McpServer.php          # Main MCP server
│   │   ├── McpToolContext.php     # Tool context & error handling
│   │   └── ResultFormatter.php    # Format conversion
│   └── JsonRpc/                   # JSON-RPC protocol handling
├── Shared/                         # Shared components
│   ├── ValueObjects/              # MCP value objects
│   ├── Exceptions/                # MCP exceptions
│   └── Contracts/                 # Interfaces
├── Attributes/                     # MCP attributes
│   ├── McpTool.php
│   └── McpResource.php
└── Tools/Categories/               # ALL MCP tools organized by category
    ├── Analysis/                  # Analysis & debugging tools
    │   ├── ContainerInspector.php
    │   ├── EventFlowVisualizer.php
    │   ├── MiddlewareChainAnalyzer.php
    │   ├── RouteDebugger.php
    │   └── DependencyAnalysisTools.php  # MIGRATED
    ├── Codebase/                  # Codebase analysis tools
    │   └── CodebaseAnalyzer.php   # MIGRATED
    ├── Database/                  # Database tools
    │   ├── DatabaseOptimizationTools.php
    │   └── DatabaseTools.php
    ├── Development/               # Development tools
    │   ├── CodeQualityTools.php
    │   ├── FrameworkAgents.php
    │   ├── FrameworkTools.php
    │   ├── LogTools.php
    │   └── HotReloadTool.php      # MIGRATED
    ├── Performance/               # Performance optimization
    │   ├── CacheTools.php
    │   └── PerformanceTools.php
    ├── Security/                  # Security tools
    │   ├── SecurityAuditTools.php
    │   ├── SecurityConfigurationTools.php
    │   └── SecurityMonitoringTools.php
    ├── System/                    # System-level tools
    │   └── FileSystemTools.php
    └── Testing/                   # Testing tools
        └── TestingTools.php       # MIGRATED
```

### Phase 4: Cleanup Legacy `/Tools` Directory

**After migration, `/Tools` root should contain ONLY**:
- Possibly: Agent classes (if they have different semantics)
- Nothing else - completely empty or removed

## Implementation Checklist

### Pre-Refactoring Verification

- [x] Identify all duplicate files (16 confirmed)
- [x] Compare OLD vs NEW pattern implementation
- [x] Document line count differences
- [ ] Run Discovery to see current tool registration
- [ ] Verify which tools are actually used by MCP server
- [ ] Check if any external code references old tool locations

### Phase 1 Execution (Delete Duplicates)

- [ ] Backup `/Tools` directory before deletion
- [ ] Delete 16 duplicate files from `/Tools`
- [ ] Run Discovery to verify only Categories tools are registered
- [ ] Run MCP server and verify tool count reduced by ~16
- [ ] Test sample tools to ensure they still work

### Phase 2 Execution (Migrate Unique Files)

- [ ] Analyze 6 unique files for migration needs
- [ ] Create appropriate Categories subdirectories if needed
- [ ] Migrate CodebaseAnalyzer to `/Tools/Categories/Codebase/`
- [ ] Migrate TestingTools to `/Tools/Categories/Testing/`
- [ ] Update any imports/references to migrated files
- [ ] Verify all tools still discoverable

### Phase 3 Execution (Optimize Structure)

- [ ] Remove empty `/Tools` directory (or keep for Agents only)
- [ ] Update documentation to reflect new structure
- [ ] Update CLAUDE.md MCP documentation
- [ ] Create developer guide for adding new MCP tools

### Post-Refactoring Validation

- [ ] Run full Discovery test suite
- [ ] Verify MCP server starts without errors
- [ ] Test representative tools from each category
- [ ] Verify tool count matches expected
- [ ] Performance test Discovery with new structure
- [ ] Update MCP integration documentation

## Expected Benefits

### Immediate Benefits

1. **-8,729 Lines of Duplicate Code** removed
2. **Single Source of Truth** for each tool
3. **Consistent Error Handling** across all tools
4. **Standardized Output Formats** (array, json, yaml, markdown)
5. **Better Discoverability** with categories and tags

### Long-term Benefits

1. **Easier Maintenance** - Changes in one place only
2. **Better Performance** - No duplicate Discovery scanning
3. **Clearer Architecture** - Organized by category
4. **Enhanced Features** - Caching, formatting built-in
5. **Framework Compliance** - McpToolContext pattern everywhere

## Risk Assessment

### Low Risk

- ✅ Deleting old duplicates (Categories versions proven to work)
- ✅ Directory reorganization (no code changes)
- ✅ Documentation updates

### Medium Risk

- ⚠️ Migrating unique files (need to verify no external dependencies)
- ⚠️ Updating imports if any code directly imports old locations

### Mitigation

1. **Backup before deletion** - Can rollback if needed
2. **Test Discovery after each phase** - Catch issues early
3. **Incremental approach** - Phase by phase validation
4. **Keep git history** - Can revert individual changes

## Timeline Estimate

- **Phase 1** (Delete Duplicates): 30 minutes
- **Phase 2** (Migrate Unique): 1-2 hours
- **Phase 3** (Optimize Structure): 30 minutes
- **Testing & Validation**: 1 hour
- **Documentation Updates**: 1 hour

**Total Estimated Time**: 4-5 hours

## Success Criteria

1. ✅ Zero duplicate tool files in `/Tools` vs `/Tools/Categories`
2. ✅ All MCP tools use McpToolContext pattern
3. ✅ Discovery finds correct number of tools (~84 tools expected)
4. ✅ MCP server starts and all tools accessible
5. ✅ All tools organized in logical categories
6. ✅ Documentation reflects new structure
7. ✅ No broken imports or references
8. ✅ Test suite passes

## Next Steps

1. Get approval for refactoring plan
2. Create feature branch for refactoring
3. Execute Phase 1 (delete duplicates)
4. Validate and test
5. Execute remaining phases
6. Merge to main after full validation