Fix tool calling (#13)

* Reorganize ServerOption and methods

Remove unused Start and Shutdown methods

* Remove default server info

* Define constant for MCP version

* Clean up and comment code

* Inline applyAuthHeaders

* Define jsonrpc.Result type

* Fixup server info

* Rename Handle to HandleRequest

* Create protocol.go

* Refactor server with formalized protocol

* Use url.PathEscape to properly escape path parameters according to RFC 3986

* Move logging to top-level handler

* Add test case for NWS point tool and fix implementation

* Only escape characters that are invalid in URL path segments

Author: mattt

cmd/emcee/main.go

@@ -21,8 +21,8 @@ import (
 
 var rootCmd = &cobra.Command{
 	Use:   "emcee [spec-path-or-url]",
-	Short: "An MCP server for a given OpenAPI specification",
-	Long: `emcee is a CLI tool that provides an MCP stdio transport for a given OpenAPI specification.
+	Short: "Creates an MCP server for an OpenAPI specification",
+	Long: `emcee is a CLI tool that provides an Model Context Protocol (MCP) stdio transport for a given OpenAPI specification.
 It takes an OpenAPI specification path or URL as input and processes JSON-RPC requests
 from stdin, making corresponding API calls and returning JSON-RPC responses to stdout.
 
@@ -32,11 +32,14 @@ The spec-path-or-url argument can be:
 - "-" to read from stdin`,
 	Args: cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
+		// Set up context and signal handling
 		ctx, cancel := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM)
 		defer cancel()
 
+		// Set up error group
 		g, ctx := errgroup.WithContext(ctx)
 
+		// Set up logger
 		logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
 			Level: slog.LevelDebug,
 		}))
@@ -47,15 +50,19 @@ The spec-path-or-url argument can be:
 		g.Go(func() error {
 			var opts []mcp.ServerOption
 
+			// Set server info
+			opts = append(opts, mcp.WithServerInfo(cmd.Name(), version))
+
+			// Set logger
 			opts = append(opts, mcp.WithLogger(logger))
 
+			// Configure HTTP client
 			retryClient := retryablehttp.NewClient()
 			retryClient.RetryMax = retries
 			retryClient.RetryWaitMin = 1 * time.Second
 			retryClient.RetryWaitMax = 30 * time.Second
 			retryClient.HTTPClient.Timeout = timeout
 			retryClient.Logger = logger
-
 			if rps > 0 {
 				retryClient.Backoff = func(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
 					// Ensure we wait at least 1/rps between requests
@@ -66,19 +73,24 @@ The spec-path-or-url argument can be:
 					return retryablehttp.DefaultBackoff(min, max, attemptNum, resp)
 				}
 			}
-
 			client := retryClient.StandardClient()
 			opts = append(opts, mcp.WithClient(client))
 
+			// Set Authentication header if provided
+			if auth != "" {
+				opts = append(opts, mcp.WithAuth(auth))
+			}
+
+			// Read OpenAPI specification data
+			var rpcInput io.Reader = os.Stdin
 			var specData []byte
 			var err error
-			var rpcInput io.Reader = os.Stdin
 
 			if args[0] == "-" {
 				logger.Info("reading spec from stdin")
 
-				// When reading spec from stdin, we need to use /dev/tty for RPC input
-				// because stdin isn't a TTY when reading from a pipe
+				// When reading the OpenAPI spec from stdin, we need to read RPC input from /dev/tty
+				// since stdin is being used for the spec data and isn't available for interactive I/O
 				tty, err := os.Open("/dev/tty")
 				if err != nil {
 					return fmt.Errorf("error opening /dev/tty: %w", err)
@@ -94,6 +106,7 @@ The spec-path-or-url argument can be:
 			} else if strings.HasPrefix(args[0], "http://") || strings.HasPrefix(args[0], "https://") {
 				logger.Info("reading spec from URL", "url", args[0])
 
+				// Create HTTP request
 				req, err := http.NewRequest(http.MethodGet, args[0], nil)
 				if err != nil {
 					return fmt.Errorf("error creating request: %w", err)
@@ -104,6 +117,7 @@ The spec-path-or-url argument can be:
 					req.Header.Set("Authorization", auth)
 				}
 
+				// Make HTTP request
 				resp, err := client.Do(req)
 				if err != nil {
 					return fmt.Errorf("error downloading spec: %w", err)
@@ -113,6 +127,7 @@ The spec-path-or-url argument can be:
 				}
 				defer resp.Body.Close()
 
+				// Read spec from response body
 				specData, err = io.ReadAll(resp.Body)
 				if err != nil {
 					return fmt.Errorf("error reading spec from %s: %w", args[0], err)
@@ -142,30 +157,25 @@ The spec-path-or-url argument can be:
 					return fmt.Errorf("spec file too large (max 100MB): %s", cleanPath)
 				}
 
+				// Read spec from file
 				specData, err = os.ReadFile(cleanPath)
 				if err != nil {
 					return fmt.Errorf("error reading spec file %s: %w", cleanPath, err)
 				}
 			}
 
-			if len(specData) == 0 {
-				return fmt.Errorf("no OpenAPI spec data provided")
-			}
+			// Set spec data
 			opts = append(opts, mcp.WithSpecData(specData))
 
-			if auth != "" {
-				opts = append(opts, mcp.WithAuth(auth))
-			}
-
-			opts = append(opts, mcp.WithLogger(logger))
-
+			// Create server
 			server, err := mcp.NewServer(opts...)
 			if err != nil {
 				return fmt.Errorf("error creating server: %w", err)
 			}
 
+			// Create and run transport
 			transport := mcp.NewStdioTransport(rpcInput, os.Stdout, os.Stderr)
-			return transport.Run(ctx, server.Handle)
+			return transport.Run(ctx, server.HandleRequest)
 		})
 
 		return g.Wait()

jsonrpc/response.go

@@ -1,15 +1,18 @@
 package jsonrpc
 
+// Result represents a map of string keys to arbitrary values
+type Result interface{}
+
 // Response represents a JSON-RPC response object
 type Response struct {
-	Version string      `json:"jsonrpc"`
-	Result  interface{} `json:"result,omitempty"`
-	Error   *Error      `json:"error,omitempty"`
-	ID      ID          `json:"id"`
+	Version string `json:"jsonrpc"`
+	Result  Result `json:"result,omitempty"`
+	Error   *Error `json:"error,omitempty"`
+	ID      ID     `json:"id"`
 }
 
 // NewResponse creates a new Response object
-func NewResponse(id interface{}, result interface{}, err *Error) Response {
+func NewResponse(id interface{}, result Result, err *Error) Response {
 	respID, _ := NewID(id)
 
 	return Response{

main_test.go

@@ -78,4 +78,40 @@ func TestIntegration(t *testing.T) {
 	assert.Equal(t, "2.0", response.JSONRPC)
 	assert.Equal(t, 1, response.ID)
 	assert.NotEmpty(t, response.Result.Tools, "Expected at least one tool in response")
+
+	// Find and verify the point tool
+	var pointTool struct {
+		Name        string
+		Description string
+		InputSchema struct {
+			Type       string                 `json:"type"`
+			Properties map[string]interface{} `json:"properties"`
+			Required   []string               `json:"required"`
+		}
+	}
+
+	foundPointTool := false
+	for _, tool := range response.Result.Tools {
+		if tool.Name == "point" {
+			foundPointTool = true
+			err := json.Unmarshal(tool.InputSchema, &pointTool.InputSchema)
+			require.NoError(t, err)
+			pointTool.Name = tool.Name
+			pointTool.Description = tool.Description
+			break
+		}
+	}
+
+	require.True(t, foundPointTool, "Expected to find point tool")
+	assert.Equal(t, "point", pointTool.Name)
+	assert.Contains(t, pointTool.Description, "Returns metadata about a given latitude/longitude point")
+
+	// Verify point tool has proper parameter schema
+	assert.Equal(t, "object", pointTool.InputSchema.Type)
+	assert.Contains(t, pointTool.InputSchema.Properties, "point", "Point tool should have 'point' parameter")
+
+	pointParam := pointTool.InputSchema.Properties["point"].(map[string]interface{})
+	assert.Equal(t, "string", pointParam["type"])
+	assert.Contains(t, pointParam["description"].(string), "Point (latitude, longitude)")
+	assert.Contains(t, pointTool.InputSchema.Required, "point", "Point parameter should be required")
 }