fix(observability): forward /metrics from outer mux to chi router (MCP-3135)

The /metrics handler is registered on the httpapi chi router but the outer
http.ServeMux only forwarded /api/, /events, and the health endpoints, so
GET /metrics returned 404 even with observability.metrics.enabled=true.

Extract the route forwarding into Server.registerHTTPHandlers and add a
gated mux.Handle("/metrics", httpAPIServer) so the endpoint is reachable
only when the Prometheus exporter is enabled. Covered by a unit test on the
real outer mux and a full-binary e2e that scrapes /metrics for 200 +
mcpproxy_uptime_seconds.

Related #746

Author: Dumbris

internal/server/e2e_metrics_test.go

@@ -0,0 +1,63 @@
+package server
+
+import (
+	"io"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/testutil"
+)
+
+// TestBinaryMetricsEndpoint is the MCP-3135 integration regression: boot the
+// real binary with observability.metrics.enabled=true and assert that GET
+// /metrics (at the root, NOT under /api) responds 200 with a scrapeable body.
+//
+// Before the routing fix the /metrics handler was registered on the httpapi chi
+// router but never forwarded by the outer mux, so this returned 404. The config
+// here intentionally has zero upstream servers, so the test needs no Node/npx
+// dependency — it exercises only the proxy's own HTTP listener wiring.
+func TestBinaryMetricsEndpoint(t *testing.T) {
+	env := testutil.NewBinaryTestEnv(t)
+	defer env.Cleanup()
+
+	// Overwrite the default config with a metrics-enabled, zero-upstream config
+	// before the binary starts. The data dir was created next to the config by
+	// NewBinaryTestEnv (tempDir/data).
+	dataDir := filepath.Join(filepath.Dir(env.GetConfigPath()), "data")
+	writeMetricsEnabledConfig(t, env.GetConfigPath(), env.GetPort(), dataDir)
+
+	env.Start()
+
+	resp, err := http.Get(env.GetBaseURL() + "/metrics")
+	require.NoError(t, err)
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+
+	assert.Equal(t, http.StatusOK, resp.StatusCode, "GET /metrics should be reachable when metrics enabled; body=%s", string(body))
+	assert.Contains(t, string(body), "mcpproxy_uptime_seconds", "metrics body should expose the uptime gauge")
+}
+
+func writeMetricsEnabledConfig(t *testing.T, configPath string, port int, dataDir string) {
+	t.Helper()
+	content := `{
+  "listen": ":` + strconv.Itoa(port) + `",
+  "data_dir": "` + dataDir + `",
+  "api_key": "` + testutil.TestAPIKey + `",
+  "enable_tray": false,
+  "mcpServers": [],
+  "quarantine_enabled": false,
+  "docker_isolation": { "enabled": false },
+  "observability": {
+    "metrics": { "enabled": true }
+  }
+}`
+	require.NoError(t, os.WriteFile(configPath, []byte(content), 0600))
+}

internal/server/metrics_routing_test.go

@@ -0,0 +1,74 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/observability"
+)
+
+// MCP-3135 regression: the /metrics handler is registered on the httpapi chi
+// router, but the OUTER http.ServeMux in startCustomHTTPServer must forward
+// /metrics to it. Before the fix the outer mux only forwarded /api/, /events,
+// and the health endpoints, so GET /metrics returned 404 even with metrics
+// enabled. These tests exercise the real outer mux via registerHTTPHandlers.
+
+// sentinel stands in for the httpapi chi handler: it serves a scrapeable body
+// for any request that the outer mux actually forwards to it.
+func metricsSentinel() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("mcpproxy_uptime_seconds 1\n"))
+	})
+}
+
+func TestRegisterHTTPHandlers_MetricsRoutedWhenEnabled(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.Observability.Metrics = &config.MetricsExporterConfig{Enabled: true}
+	obsCfg := buildObservabilityConfig(cfg)
+	mgr, err := observability.NewManager(zap.NewNop().Sugar(), &obsCfg)
+	require.NoError(t, err)
+	require.NotNil(t, mgr.Metrics(), "metrics manager should exist when enabled")
+
+	s := &Server{logger: zap.NewNop(), observability: mgr}
+	mux := http.NewServeMux()
+	s.registerHTTPHandlers(mux, metricsSentinel())
+
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", http.NoBody))
+
+	// Before the fix this was 404 — the outer mux never forwarded /metrics.
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Body.String(), "mcpproxy_uptime_seconds")
+}
+
+func TestRegisterHTTPHandlers_MetricsNotRoutedWhenDisabled(t *testing.T) {
+	// observability nil == metrics disabled (default). /metrics must stay 404.
+	s := &Server{logger: zap.NewNop(), observability: nil}
+	mux := http.NewServeMux()
+	s.registerHTTPHandlers(mux, metricsSentinel())
+
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", http.NoBody))
+
+	assert.Equal(t, http.StatusNotFound, rec.Code, "/metrics must not be routed when metrics are disabled")
+}
+
+// The fix must not regress the routes the outer mux already forwarded.
+func TestRegisterHTTPHandlers_ForwardsAPIAndHealth(t *testing.T) {
+	s := &Server{logger: zap.NewNop(), observability: nil}
+	mux := http.NewServeMux()
+	s.registerHTTPHandlers(mux, metricsSentinel())
+
+	for _, path := range []string{"/api/v1/status", "/events", "/healthz", "/readyz", "/livez", "/ready", "/health"} {
+		rec := httptest.NewRecorder()
+		mux.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, http.NoBody))
+		assert.Equal(t, http.StatusOK, rec.Code, "outer mux should forward %s", path)
+	}
+}

internal/server/server.go

@@ -1705,6 +1705,36 @@ func (s *Server) profileMiddleware(next http.Handler) http.Handler {
 
 // startCustomHTTPServer creates a custom HTTP server that handles MCP endpoints
 // It supports both TCP (for browsers) and Unix socket/named pipe (for tray) listeners
+// registerHTTPHandlers forwards the REST API, SSE events, health endpoints,
+// and the observability /metrics endpoint from the outer http.ServeMux to the
+// httpapi chi router (httpAPIServer).
+//
+// MCP-3135: /metrics is registered on the chi router (httpapi.setupRoutes), but
+// the outer mux must explicitly forward it — otherwise GET /metrics returns 404
+// even when metrics are enabled. The forward is gated on metrics actually being
+// enabled so a disabled deployment keeps /metrics unrouted (404).
+func (s *Server) registerHTTPHandlers(mux *http.ServeMux, httpAPIServer http.Handler) {
+	mux.Handle("/api/", httpAPIServer)
+	mux.Handle("/events", httpAPIServer)
+
+	// Mount health endpoints directly on main mux at root level
+	healthEndpoints := []string{"/healthz", "/readyz", "/livez", "/ready", "/health"}
+	for _, endpoint := range healthEndpoints {
+		mux.Handle(endpoint, httpAPIServer)
+	}
+
+	s.logger.Info("Registered REST API endpoints", zap.Strings("api_endpoints", []string{"/api/v1/*", "/events"}))
+	s.logger.Info("Registered health endpoints", zap.Strings("health_endpoints", healthEndpoints))
+
+	// MCP-32/MCP-3135: forward /metrics to the chi router only when the
+	// Prometheus exporter is enabled. Without this the handler registered in
+	// httpapi.setupRoutes is unreachable through the outer mux.
+	if s.observability != nil && s.observability.Metrics() != nil {
+		mux.Handle("/metrics", httpAPIServer)
+		s.logger.Info("Registered metrics endpoint", zap.String("endpoint", "/metrics"))
+	}
+}
+
 func (s *Server) startCustomHTTPServer(ctx context.Context, streamableServer *server.StreamableHTTPServer) error {
 	cfg := s.runtime.Config()
 	if cfg == nil {
@@ -1937,17 +1967,10 @@ func (s *Server) startCustomHTTPServer(ctx context.Context, streamableServer *se
 	}
 	// Wire server edition multi-user OAuth (no-op in personal edition)
 	wireServerEditionOAuth(s, httpAPIServer)
-	mux.Handle("/api/", httpAPIServer)
-	mux.Handle("/events", httpAPIServer)
 
-	// Mount health endpoints directly on main mux at root level
-	healthEndpoints := []string{"/healthz", "/readyz", "/livez", "/ready", "/health"}
-	for _, endpoint := range healthEndpoints {
-		mux.Handle(endpoint, httpAPIServer)
-	}
-
-	s.logger.Info("Registered REST API endpoints", zap.Strings("api_endpoints", []string{"/api/v1/*", "/events"}))
-	s.logger.Info("Registered health endpoints", zap.Strings("health_endpoints", healthEndpoints))
+	// Forward REST API, events, health, and (MCP-32) /metrics onto the outer
+	// mux. Extracted so the routing is unit-testable (MCP-3135 regression).
+	s.registerHTTPHandlers(mux, httpAPIServer)
 
 	// Debug / profiling endpoints (API-key gated). Block & mutex profiles
 	// default to off; we enable them when the route is hit so the running