refactor: Split protocol.go by Concern (1083 lines)

[severity1]

Summary

The internal/control/protocol.go file has grown to 1083 lines and contains multiple distinct concerns that could be separated for better maintainability and readability.

Current State

File: internal/control/protocol.go - 1083 lines

The file currently handles:

1. Core Protocol struct and lifecycle (Start, Close, Initialize)
2. Request/response correlation
3. Hook management (registration, callback handling, response generation)
4. MCP routing (tools/list, tools/call, initialize)
5. Permission system (can_use_tool handling)
6. Message routing and discrimination

Analysis Findings

Note: The original analysis incorrectly claimed BuildCommand() was 150+ lines with high complexity. Actual findings:

• BuildCommand() in internal/cli/discovery.go is only 22 lines with complexity ~2 - well-designed
• routeMcpMethod() is 85 lines with complexity ~6-8 - moderate, justified by protocol routing

The primary refactoring target is protocol.go at 1083 lines.

Proposed File Split

internal/control/
├── protocol.go (~500 lines)
│   ├── Protocol struct
│   ├── Start()/Close()
│   ├── Initialize handshake (60s timeout)
│   ├── Request correlation (generateRequestID, sendRequest, awaitResponse)
│   ├── Core message routing
│   └── handleReceivedMessage()
│
├── protocol_hooks.go (~250 lines)
│   ├── generateHookRegistrations()
│   ├── buildHooksConfig()
│   ├── handleHookCallbackRequest()
│   ├── parseHookInput()
│   ├── sendHookResponse()
│   └── Hook-specific validation
│
├── protocol_mcp.go (~200 lines)
│   ├── handleMcpMessageRequest()
│   ├── routeMcpMethod()
│   ├── handleMcpInitialize()
│   ├── handleMcpToolsList()
│   ├── handleMcpToolsCall()
│   ├── sendMcpResponse()
│   └── sendMcpErrorResponse()
│
└── protocol_permissions.go (~130 lines)
    ├── handleCanUseToolRequest()
    ├── sendPermissionResponse()
    ├── parsePermissionSuggestions()
    └── Permission result builders

Implementation Approach

1. Keep Protocol struct in protocol.go - Central state remains in main file
2. Extract methods, not types - Move related methods to topic files
3. Maintain internal visibility - All files in same package, no API changes
4. Test files follow same pattern - Already have protocol_hook_test.go, protocol_mcp_test.go

Specific Extractions

routeMcpMethod Improvement

The routeMcpMethod function (lines 969-1053, 85 lines) could have its "tools/call" case extracted:

// Before: 37-line case in switch
case "tools/call":
    // ... complex logic

// After: Extracted helper
case "tools/call":
    return p.handleMcpToolsCall(ctx, server, msgID, params)

func (p *Protocol) handleMcpToolsCall(ctx context.Context, server McpServer, 
    msgID any, params map[string]any) (map[string]any, error) {
    // ... implementation
}

What NOT to Refactor

1. transport.go (849 lines) - Already well-organized with extracted helpers (buildProtocolOptions(), setupStderr(), buildEnvironment())
2. BuildCommand (22 lines) - Simple, focused, no changes needed
3. options.go (788 lines) - Could be split later, lower priority

Acceptance Criteria

• Split protocol.go into 4 files by concern
• Extract handleMcpToolsCall from routeMcpMethod
• All existing tests pass without modification
• No public API changes
• Each file has clear, focused responsibility

Priority

Low-Medium - Improves maintainability but not urgent. Current code is functional.

Notes

• Test files already follow the split pattern (protocol_test.go, protocol_hook_test.go, protocol_mcp_test.go)
• The codebase follows good patterns; this is refinement, not fixing broken code

[severity1]

Completed in PR #82. Split protocol.go (1084 lines) into 4 focused files by concern:

• protocol.go (~507 lines) - Core protocol lifecycle, request/response correlation
• hooks.go (~285 lines) - Hook callback handling and registration
• mcp.go (~168 lines) - MCP JSONRPC message routing
• permissions.go (~147 lines) - Permission callback handling

Also renamed test files for consistency:

• protocol_hook_test.go -> hooks_test.go
• protocol_mcp_test.go -> mcp_test.go

All tests pass with 81.6% coverage.