refactor(mcp): migrate server to official go-sdk#366
Merged
Conversation
- `.go-arch-lint.yml`: remove go-jsonschema vendor alias; guard against accidental direct import - `docs/ADR/017-mcp-proxy-stdio-subprocess-for-tool-interception.md`: update public-package note to reflect migration - `docs/ADR/019-mcp-server-sdk-adapter.md`: add ADR documenting migration decision, rationale, and trade-offs - `docs/ADR/README.md`: register ADR 019 - `docs/development/architecture.md`: document internal/infrastructure/mcp in infra layer - `docs/reference/package-documentation.md`: add internal/infrastructure/mcp (28 total packages) - `go.mod`: mark github.com/google/jsonschema-go as indirect - `internal/domain/ports/tool_provider.go`: document ToolProvider contract for nil args, nil result, and dual error reporting - `internal/infrastructure/agents/mcp_proxy_purge.go`: classify timeout as WARN, exit 127 as DEBUG; add resolveListTimeout with AWF_MCP_PROXY_LIST_TIMEOUT; extract firstLine helper - `internal/infrastructure/agents/mcp_proxy_purge_test.go`: add timeout WARN, exit 127 quiet, and env-var resolution tests - `internal/infrastructure/mcp/architecture_test.go`: remove jsonschema-go from allowed import prefixes - `internal/infrastructure/mcp/doc.go`: update to reflect schemaFromMap removal and transitive-only jsonschema dependency - `internal/infrastructure/mcp/handler.go`: add nil result guard and nil params guard; annotate malformed JSON fallback - `internal/infrastructure/mcp/handler_test.go`: migrate to shared fakeProvider; add NilResult, NilParams, MalformedJSONArgs tests - `internal/infrastructure/mcp/mapping.go`: remove schemaFromMap and jsonschema-go import; simplify toolToMCP - `internal/infrastructure/mcp/mapping_test.go`: remove TestSchemaFromMap_* and TestPackageCoverageSample; clean unused wantErr fields - `internal/infrastructure/mcp/mcp_test.go`: migrate to testhelpers; add parallel-safe ServeIO tests; assert atomicity on duplicate registration - `internal/infrastructure/mcp/server.go`: split RegisterProvider into validate+commit passes; extract serve() helper; wrap ListTools error - `internal/infrastructure/mcp/testhelpers_test.go`: add shared fakeProvider and recordingProvider test doubles - `internal/interfaces/cli/mcp_serve.go`: make os.Getwd() failure fatal (ExitSystem) instead of silently disabling sandbox - `internal/interfaces/cli/mcp_serve_helpers_test.go`: delete empty placeholder file - `internal/interfaces/cli/mcp_serve_plugin_test.go`: extract requestToolsList helper; add TestArchitecture_MCPServe_NewUsesVersion AST test; strengthen TestResolveOperationProvider assertions - `pkg/mcpserver/architecture_test.go`: delete (package removed) - `pkg/mcpserver/doc.go`: delete (package removed) - `pkg/mcpserver/protocol.go`: delete (package removed) - `pkg/mcpserver/protocol_test.go`: delete (package removed) - `pkg/mcpserver/server.go`: delete (package removed) - `pkg/mcpserver/server_test.go`: delete (package removed) - `pkg/mcpserver/types.go`: delete (package removed) - `tests/integration/mcp/plugin_bridge_test.go`: update imports for migration - `tests/integration/mcp/sdk_client_test.go`: add SDK client integration tests over in-memory transport Closes #365
b6a671d to
9740292
Compare
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
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
pkg/mcpserverJSON-RPC 2.0 implementation (~1270 LOC) with the officialgithub.com/modelcontextprotocol/go-sdk, reducing protocol maintenance burden and enabling future spec extensions (F108)internal/infrastructure/mcp/adapter that preserves hexagonal architecture — the CLI layer only depends on domain ports, not SDK typesmcp_proxy_purgenow distinguishes timeout/not-installed/error failure modes with actionable log levels, andmcp_serve.gotreats a failedos.Getwd()as a fatal error rather than silently disabling the sandboxChanges
Deleted: Custom MCP server package
pkg/mcpserver/server.go: Deleted — custom JSON-RPC 2.0 server implementationpkg/mcpserver/protocol.go: Deleted — custom wire-protocol types and constantspkg/mcpserver/types.go: Deleted — custom type definitionspkg/mcpserver/doc.go: Deleted — package documentationpkg/mcpserver/architecture_test.go: Deleted — AST import constraint testspkg/mcpserver/protocol_test.go: Deleted — protocol serialization testspkg/mcpserver/server_test.go: Deleted — server behavior testsNew: Official SDK adapter
internal/infrastructure/mcp/server.go:RegisterProvidernow uses a two-pass (validate-then-commit) pattern for atomicity; extractedserve()helper shared byServeStdioand newServeIOinternal/infrastructure/mcp/handler.go: Added nil-result guard and malformed-JSON fallback; removed redundanterr = nilin panic recoveryinternal/infrastructure/mcp/mapping.go: RemovedschemaFromMapandjsonschema-godependency;toolToMCPpassesInputSchemaraw map directly to SDKinternal/infrastructure/mcp/doc.go: Updated to reflect removal ofschemaFromMapandjsonschema-goimportinternal/infrastructure/mcp/architecture_test.go: Removedgithub.com/google/jsonschema-go/from allowed import listinternal/infrastructure/mcp/testhelpers_test.go: Extracted sharedfakeProviderandrecordingProvidertest doubles (previously duplicated across test files)internal/infrastructure/mcp/mcp_test.go: AddedTestServeIO_*parallel-safe tests using in-memory pipes; added atomicity assertion for duplicate-tool registration; documented SDK non-deterministic error behavior on context cancellationinternal/infrastructure/mcp/handler_test.go: Refactored to use shared test doubles; addedTestHandlerFor_NilResult,TestHandlerFor_NilParams,TestHandlerFor_MalformedJSONArgsinternal/infrastructure/mcp/mapping_test.go: RemovedTestSchemaFromMap_*tests (function deleted); cleaned upwantErrfieldsCLI interface
internal/interfaces/cli/mcp_serve.go:os.Getwd()failure is now a fatalExitSystemerror instead of silently leavingrootDirempty (sandbox bypass)internal/interfaces/cli/mcp_serve_plugin_test.go: ExtractedrequestToolsListhelper; addedTestArchitecture_MCPServe_NewUsesVersion(AST enforcement thatinframcp.New(Version)is called); strengthenedTestResolveOperationProvider_EmptyDepsInitializesassertionsinternal/interfaces/cli/mcp_serve_helpers_test.go: Deleted (empty file)Infrastructure: MCP proxy purge
internal/infrastructure/agents/mcp_proxy_purge.go: Extracted named constants (mcpListDefaultTimeout,mcpRemoveTimeout,exitCodeCommandNotFound); addedresolveListTimeoutwith env-var override; distinguished timeout (WARN), execution error (DEBUG), exit-127 (DEBUG), and non-zero exit (DEBUG) failure modes; addedfirstLinehelper for single-line stderr in logsinternal/infrastructure/agents/mcp_proxy_purge_test.go: AddedwarnCallsto log capture; addedTestResolveListTimeout,TestPurgeOrphanMCPRegistrations_TimeoutLogsWarn,TestPurgeOrphanMCPRegistrations_NotInstalledIsQuietDomain ports
internal/domain/ports/tool_provider.go: Added doc comment toToolProviderinterface specifying nil/empty-args equivalence, non-nil success result requirement, and dual error-reporting contractIntegration tests
tests/integration/mcp/sdk_client_test.go: New — end-to-end tests exercising the SDK transport layer with a real MCP clienttests/integration/mcp/plugin_bridge_test.go: Minor import/assertion updates for SDK adapterDocumentation & config
docs/ADR/019-mcp-server-sdk-adapter.md: New ADR documenting the migration decision, rationale, alternatives, and trade-offsdocs/ADR/017-mcp-proxy-stdio-subprocess-for-tool-interception.md: Updated implementation note to reference F104/ADR 019docs/ADR/README.md: Added ADR 019 entrydocs/development/architecture.md: Addedmcp/entry to infrastructure layerdocs/reference/package-documentation.md: Addedinternal/infrastructure/mcpto documented packages (28 total).go-arch-lint.yml: Removedgo-jsonschemavendor and itscanUseentry; added explanatory commentgo.mod: Markedgithub.com/google/jsonschema-goas// indirectTest plan
make build && make lintpass with zero violationsmake testandmake test-racepass, including newsdk_client_test.gointegration testsawf mcp-servesubprocess correctly lists and invokes builtin tools over stdio when exercised viamcp_proxyin a real workflow runpkg/mcpserverhas zero remaining importers:grep -r "pkg/mcpserver" --include="*.go" .returns no resultsCloses #365
Generated with awf commit workflow