Split MCPServer.swift into per-tool handler files behind ToolHandler#162
Merged
Split MCPServer.swift into per-tool handler files behind ToolHandler#162
Conversation
Step #1 of the architectural roadmap. The previous shape had three sources of truth that had to stay in lockstep: the `ToolName` enum, the `mcpToolSchemas()` array in MCPToolSchemas.swift, and the `switch tool` dispatch in `configureMCPServer`. Adding a tool meant touching three places. Introduce a `ToolHandler` protocol with `static var name`, `static var schema`, and `static func handle(_:ctx:)`. Each of the 12 tools moves to its own file under `Sources/PreviewsCLI/Handlers/`, carrying the schema next to the implementation. `configureMCPServer` derives both the `ListTools` response and the `CallTool` dispatch from a single `handlerRegistry` array — the array IS the inventory. `HandlerContext` bundles the engine-layer dependencies (host, iosState, configCache, router, macCompiler, server) so handlers receive deps via parameter rather than reaching for file-private vars. Static handler types are `Sendable`-trivially since they have no instance state. `MCPServer.swift` shrinks from 1131 to 92 lines (target was <200); shared infrastructure moves to `MCPServerSupport.swift` (heartbeat, progress reporter, version advertisement, `parseTraits`, `configQualityForSession`); `MCPToolSchemas.swift` is deleted. The acceptance gate from the architectural plan — "ListTools response bytes are identical pre/post" — is enforced by a new `ListToolsSnapshotTests` snapshot that pins the JSON-encoded schemas against a fixture file. The pre-refactor bytes were captured on the prior commit, and the test passes against the post-refactor registry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small comment additions from the code-reviewer pass: - Document why `Dictionary(uniqueKeysWithValues:)` is the right fail-fast for a future contributor who copies a handler and forgets to update `static let name` — the trap fires at server construction (visible in tests) rather than on the first call. - Explain the bless-mode early-return in `ListToolsSnapshotTests` — we deliberately skip the equality check after writing the snapshot because it would tautologically pass; the issue-record is the signal to commit the new fixture. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Step #1 of the architectural-refactor roadmap. The previous shape kept three sources of truth in lockstep: the `ToolName` enum, the `mcpToolSchemas()` array, and the `switch tool` dispatch in `configureMCPServer`. Adding a tool meant touching all three.
Introduces a `ToolHandler` protocol with `static var name`, `static var schema`, and `static func handle(_:ctx:)`. Each of the 12 tools moves to its own file under `Sources/PreviewsCLI/Handlers/`, carrying the schema next to the implementation. `configureMCPServer` derives both the `ListTools` response and the `CallTool` dispatch from a single `handlerRegistry` array — the array IS the inventory. Adding a tool now means writing one file plus appending one line.
`HandlerContext` bundles the engine-layer dependencies (host, iosState, configCache, router, macCompiler, server) so handlers receive them via parameter instead of reaching for file-private vars. Static handler types are trivially `Sendable`.
Sizes
Acceptance gate
The plan's acceptance criterion — "`ListTools` response bytes are identical pre/post" — is enforced by a new `ListToolsSnapshotTests` snapshot. The pre-refactor JSON was captured on the prior commit (a068f20) and stored as `Tests/PreviewsCLITests/list_tools_snapshot.json`. The post-refactor registry produces byte-identical output. Future contributors who change a schema must intentionally re-bless the snapshot, making the change visible in the diff.
Behavior preserved
No functional change. Same wire schemas, same handlers, same dispatch — just split across files and threaded through a context bundle.
Test plan
🤖 Generated with Claude Code