[file-diet] Refactor `pkg/cli/mcp_server.go` (1372 lines) into focused modules

[github-actions]

Overview

The file pkg/cli/mcp_server.go has grown to 1,372 lines — nearly double the healthy threshold of 800 lines — with no corresponding test file. The entire file lives inside a single createMCPServer function that registers all 8 MCP tool handlers inline, making it hard to navigate, test, or extend without touching unrelated logic.

Current State

Metric	Value
File	pkg/cli/mcp_server.go
Size	1,372 lines
Test file	❌ None (mcp_server_test.go does not exist)
Test-to-source ratio	0%
Complexity	High — one 885-line function (createMCPServer) owns all tool registrations

Full File Analysis

Functional Breakdown

Lines	Component	Description
1–175	Helpers & validation	mcpErrorData, boolPtr, getRepository, queryActorRole, hasWriteAccess, validateWorkflowName
176–405	Command & server lifecycle	NewMCPServerCommand, checkAndLogGHVersion, runMCPServer, checkActorPermission
406–1291	createMCPServer (885 lines)	Inline args structs, schema generation, and handler closures for 8 tools: status, compile, logs, audit, mcp-inspect, add, update, fix
1292–1372	HTTP infrastructure	sanitizeForLog, responseWriter, loggingHandler, runHTTPServer

Complexity Hotspots

• createMCPServer (lines 406–1291) — single function containing 8 distinct tool handler closures, 8 private type xArgs struct declarations, schema generation calls, and duplicated arg-building + error-handling patterns
• Duplicated patterns: Every tool handler contains an identical cancellation check (select { case <-ctx.Done(): ... }), repeated exec.ExitError stderr extraction, and a return &mcp.CallToolResult{...} response construction
• No tests: The entire permission-checking, validation, HTTP, and tool-dispatch logic is untested

Refactoring Strategy

Proposed File Splits

1. mcp_server_helpers.go (~175 lines)

   • Functions: mcpErrorData, boolPtr, getRepository, queryActorRole, hasWriteAccess, validateWorkflowName
   • Responsibility: Pure utility helpers and GitHub API access
   • Enables isolated unit tests for each helper

2. mcp_server_command.go (~230 lines)

   • Functions: NewMCPServerCommand, checkAndLogGHVersion, runMCPServer, checkActorPermission
   • Responsibility: Cobra command wiring and server lifecycle
   • Keep mcpLog var here

3. mcp_server_tools.go (~300 lines)

   • Functions: createMCPServer (coordinator only — delegates to registerXTool helpers), tool arg types
   • Responsibility: Server creation and tool registration orchestration

4. mcp_tools_readonly.go (~280 lines)

   • Functions: registerStatusTool, registerCompileTool, registerMCPInspectTool
   • Responsibility: Read-only / idempotent tool handlers

5. mcp_tools_privileged.go (~280 lines)

   • Functions: registerLogsTool, registerAuditTool
   • Responsibility: Privileged tools that check actor write-access

6. mcp_tools_management.go (~250 lines)

   • Functions: registerAddTool, registerUpdateTool, registerFixTool
   • Responsibility: Workflow management tools

7. mcp_server_http.go (~80 lines)

   • Functions: sanitizeForLog, responseWriter, loggingHandler, runHTTPServer
   • Responsibility: HTTP/SSE transport layer

Shared Utilities

Extract the duplicated cancellation pattern and exec error helper into shared private helpers in mcp_server_helpers.go:

// checkCancelled returns a jsonrpc.Error if ctx is already done.
func checkCancelled(ctx context.Context) error { ... }

// execError builds a jsonrpc.Error from an *exec.ExitError.
func execError(msg string, err error, extra map[string]any) error { ... }

Interface Abstractions

The execCmd closure passed into createMCPServer is good — keep it as-is so tool files can accept it as a func(ctx, ...string) *exec.Cmd parameter without referencing the outer closure.

Test Coverage Plan

Add mcp_server_test.go with at minimum:

1. mcp_server_helpers_test.go

   • TestMcpErrorData — nil input, valid input, marshal error
   • TestHasWriteAccess — all permission levels
   • TestValidateWorkflowName — valid names, injection attempts, empty string
   • TestSanitizeForLog — newlines, carriage returns, clean input
   • Target: >90%

2. mcp_server_command_test.go

   • TestCheckActorPermission — with/without validateActor, missing actor
   • Target: >80%

3. mcp_tools_readonly_test.go

   • TestStatusTool — mock GetWorkflowStatuses, verify JSON output
   • TestCompileTool — cancellation handling, strict flag propagation
   • Target: >80%

4. mcp_server_http_test.go

   • TestLoggingHandler — status code capture, path sanitization
   • Target: >80%

Implementation Guidelines

1. Preserve Behavior: All existing exported and unexported function signatures remain identical
2. Refactor createMCPServer last: Extract helpers first, then split tool registrations
3. Add tests as you go: Write tests for each new file before moving on
4. Run make test-unit after each split to catch regressions early
5. No API changes: NewMCPServerCommand stays the public entry point

Acceptance Criteria

• pkg/cli/mcp_server.go is split into 7 focused files (all under 350 lines each)
• Duplicated cancellation / exec-error patterns extracted to shared helpers
• mcp_server_test.go (or equivalent per-file tests) created with >80% coverage on helpers & HTTP layer
• All tests pass (make test-unit)
• Code passes linting (make lint) and formatting (make fmt)
• Build succeeds (make build)
• No breaking changes to NewMCPServerCommand or any other exported symbol

Additional Context

• Repository Guidelines: Follow patterns in AGENTS.md — prefer many smaller files grouped by functionality
• Validation Complexity Target: 100–200 lines per file, hard limit 300 lines
• Testing: Match patterns in pkg/cli/*_command_test.go
• Workflow Run: §22225948334

---

Priority: Medium
Effort: Medium (incremental splits, no logic changes)
Expected Impact: Improved navigability, testability, and reduced cognitive load when adding new MCP tools

Generated by Daily File Diet

• expires on Feb 22, 2026, 1:32 PM UTC