Faces MCP by kflynn · Pull Request #62 · BuoyantIO/faces-demo

kflynn · 2026-01-17T18:08:23Z

Have an MCP server for Faces. At present, it supports tools to query and change the smiley and color in use, for the center, the edge, or all.

…miley and color Signed-off-by: Flynn <flynn@buoyant.io>

Signed-off-by: Flynn <flynn@buoyant.io>

Helm fix: Adding steps to faces mcp workload

Copilot

Pull request overview

This PR adds a Model Context Protocol (MCP) server to the Faces demo application, providing tools to programmatically query and modify smiley and color settings for the center, edge, or all faces. The MCP server supports both streaming HTTP (SSE) and stdio transports.

Changes:

Added MCP server implementation with five tools (query/update for smiley, color, and face services)
Integrated MCP server into build system and Helm charts
Configured Docker images and Kubernetes deployments with Rollout support

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 20 comments.

Show a summary per file

File	Description
make-gorel.py	Adds MCP build configuration to the build system
.goreleaser.yaml	Adds MCP binary builds and Docker images for arm64/amd64
faces-chart/values.yaml	Adds MCP service configuration and rollout strategy
faces-chart/templates/mcp.yaml	Defines Kubernetes Service, Rollout, and Deployment resources for MCP
faces-chart/templates/_frontend.yaml	Adds helper templates for MCP image, affinity, and resources
cmd/generic/mcp/main.go	MCP server entry point with transport mode selection
cmd/generic/mcp/server.go	Core MCP server implementation with JSON-RPC 2.0 support
cmd/generic/mcp/types.go	Type definitions for MCP protocol
cmd/generic/mcp/tool_*.go	Tool implementations for querying and updating services
cmd/generic/mcp/README.md	Documentation for MCP server usage and configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T16:37:39Z

cmd/generic/mcp/main.go

+			log.Fatal(err)
+		}
+	} else {
+		// stdio mode (default) - standard MCP protocol


Misleading comment. The code on line 39 sets the default to "sse", but this comment incorrectly states that stdio is the default. The comment should match the code, which defaults to SSE mode.

Suggested change

// stdio mode (default) - standard MCP protocol

// stdio mode - standard MCP protocol

Copilot · 2026-01-30T16:37:39Z

cmd/generic/mcp/server.go

+				return
+			}
+			fmt.Fprintf(w, "data: %s\n\n", string(msg))
+			w.(http.Flusher).Flush()


Unsafe type assertion without a check. If the ResponseWriter doesn't implement http.Flusher, this will panic. Use the comma-ok idiom to safely check if the type assertion succeeds before calling Flush.

Copilot · 2026-01-30T16:37:39Z

cmd/generic/mcp/server.go

+			"endpoint": "/messages",
+		},
+	}
+	endpointBytes, _ := json.Marshal(endpoint)


JSON marshaling error is silently ignored. If marshaling the endpoint discovery message fails, the error is discarded. Check the error and handle it appropriately, perhaps by logging it or aborting the SSE connection.

Suggested change

endpointBytes, _ := json.Marshal(endpoint)

endpointBytes, err := json.Marshal(endpoint)

if err != nil {

log.Printf("failed to marshal SSE endpoint discovery message for client %d: %v", clientID, err)

http.Error(w, "Internal Server Error", http.StatusInternalServerError)

return

}

Copilot · 2026-01-30T16:37:40Z

cmd/generic/mcp/server.go

+	}
+	endpointBytes, _ := json.Marshal(endpoint)
+	fmt.Fprintf(w, "data: %s\n\n", string(endpointBytes))
+	w.(http.Flusher).Flush()


Unsafe type assertion without a check. If the ResponseWriter doesn't implement http.Flusher (though http.ResponseWriter typically does), this will panic. Use the comma-ok idiom to safely check if the type assertion succeeds before calling Flush.

Copilot · 2026-01-30T16:37:40Z

faces-chart/templates/mcp.yaml

@@ -0,0 +1,192 @@
+---


The mcp.yaml template is missing a check for mcp.enabled. Based on the values.yaml configuration (line 24), mcp can be disabled, but this template will always deploy resources. The entire template should be wrapped with a check like {{- if .Values.mcp.enabled }} at the beginning and {{- end }} at the end, similar to how ingress.yaml checks .Values.ingress.enabled.

Copilot · 2026-01-30T16:37:43Z

cmd/generic/mcp/tool_query_face.go

+		return CallToolResponse{}, err
+	}
+	defer resp.Body.Close()
+


The HTTP response status code is not checked. Non-200 responses should be treated as errors. Check resp.StatusCode and return an error if it's not http.StatusOK (200) to ensure failures in the face service are properly reported.

Suggested change

if resp.StatusCode != http.StatusOK {

errorBody, _ := io.ReadAll(resp.Body)

return CallToolResponse{}, fmt.Errorf("face service returned non-200 status %d (%s): %s", resp.StatusCode, resp.Status, string(errorBody))

}

Copilot · 2026-01-30T16:37:43Z

cmd/generic/mcp/server.go

+	w.Header().Set("Access-Control-Allow-Origin", "*")
+


Overly permissive CORS configuration. The Access-Control-Allow-Origin header is set to "*" which allows any origin to access the MCP server. This could expose the service to unauthorized access. Consider restricting this to specific allowed origins or making it configurable via environment variable.

Suggested change

w.Header().Set("Access-Control-Allow-Origin", "*")

// Configure CORS origin via environment variable to avoid overly-permissive default.

// If MCP_ALLOWED_ORIGIN is not set, no Access-Control-Allow-Origin header is sent.

if allowedOrigin := os.Getenv("MCP_ALLOWED_ORIGIN"); allowedOrigin != "" {

w.Header().Set("Access-Control-Allow-Origin", allowedOrigin)

}

Copilot · 2026-01-30T16:37:43Z

faces-chart/templates/mcp.yaml

+        {{- end }}
+        ports:
+        - name: http
+          containerPort: 8000


Inconsistent container port configuration in Rollout. The containerPort is set to 8000 here, but in the Deployment section (line 189) it's set to 3000. The application code defaults to port 3000 (main.go:43), so both sections should use containerPort 3000 for consistency.

Suggested change

containerPort: 8000

containerPort: 3000

Copilot · 2026-01-30T16:37:43Z

cmd/generic/mcp/tool_update_color.go

+}
+
+func (t *UpdateColorTool) Handle(ctx context.Context, s *MCPServer, args map[string]interface{}) (CallToolResponse, error) {
+	newColor := args["color"].(string)


Unsafe type assertion without a check. If the "color" argument is missing or not a string, this will panic. Add a type check with the comma-ok idiom and return an error if the required parameter is not provided or has the wrong type.

Suggested change

newColor := args["color"].(string)

colorValue, ok := args["color"]

if !ok {

return CallToolResponse{}, fmt.Errorf("missing required parameter 'color'")

}

newColor, ok := colorValue.(string)

if !ok {

return CallToolResponse{}, fmt.Errorf("parameter 'color' must be a string")

}

Copilot · 2026-01-30T16:37:44Z

cmd/generic/mcp/README.md

+- **update_smiley_emojis**: Update success and failure emojis for the
+  smiley service via HTTP PUT
+- **update_color_colors**: Update success and failure colors for the


Tool names in README don't match actual implementation. The README lists "update_smiley_emojis" and "update_color_colors", but the actual tool names are "update_smiley" and "update_color" (as seen in tool_update_smiley.go:34 and tool_update_color.go:34).

Suggested change

- **update_smiley_emojis**: Update success and failure emojis for the

smiley service via HTTP PUT

- **update_color_colors**: Update success and failure colors for the

- **update_smiley**: Update success and failure emojis for the

smiley service via HTTP PUT

- **update_color**: Update success and failure colors for the

kflynn and others added 4 commits January 17, 2026 13:06

feat(MCP): First cut at the Faces MCP server, allowing changing the s…

0d7bc62

…miley and color Signed-off-by: Flynn <flynn@buoyant.io>

feat(chart): Helm support for the MCP server

e7e7aa6

Signed-off-by: Flynn <flynn@buoyant.io>

Adding default steps for mcp rollout to fix helm templating

2f25aef

Merge pull request #63 from BuoyantIO/fragglehunter-faces-mcp

03af5c9

Helm fix: Adding steps to faces mcp workload

kflynn requested review from Copilot and fragglehunter and removed request for Copilot January 30, 2026 16:30

Copilot started reviewing on behalf of kflynn January 30, 2026 16:31 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Faces MCP#62

Faces MCP#62
kflynn wants to merge 4 commits intomainfrom
faces-mcp

kflynn commented Jan 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// stdio mode (default) - standard MCP protocol
	// stdio mode - standard MCP protocol

-	endpointBytes, _ := json.Marshal(endpoint)
+	endpointBytes, err := json.Marshal(endpoint)
+	if err != nil {
+		log.Printf("failed to marshal SSE endpoint discovery message for client %d: %v", clientID, err)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}

+	if resp.StatusCode != http.StatusOK {
+		errorBody, _ := io.ReadAll(resp.Body)
+		return CallToolResponse{}, fmt.Errorf("face service returned non-200 status %d (%s): %s", resp.StatusCode, resp.Status, string(errorBody))
+	}

-	w.Header().Set("Access-Control-Allow-Origin", "*")
+	// Configure CORS origin via environment variable to avoid overly-permissive default.
+	// If MCP_ALLOWED_ORIGIN is not set, no Access-Control-Allow-Origin header is sent.
+	if allowedOrigin := os.Getenv("MCP_ALLOWED_ORIGIN"); allowedOrigin != "" {
+		w.Header().Set("Access-Control-Allow-Origin", allowedOrigin)
+	}

-	newColor := args["color"].(string)
+	colorValue, ok := args["color"]
+	if !ok {
+		return CallToolResponse{}, fmt.Errorf("missing required parameter 'color'")
+	}
+	newColor, ok := colorValue.(string)
+	if !ok {
+		return CallToolResponse{}, fmt.Errorf("parameter 'color' must be a string")
+	}

Comments

Conversation

kflynn commented Jan 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants