Skip to content

Commit f0ca376

Browse files
Dumbrisclaude
Dumbris
and
claude
authored
fix: add /api/v1 prefix to Swagger annotations and fix E2E tests (smart-mcp-proxy#186)
- Fix 22 @router annotations in internal/httpapi/server.go to include /api/v1 prefix - Add health endpoint documentation stubs (/healthz, /readyz) for Swagger - Update Makefile to exclude specs directory from swagger generation - Fix binary E2E tests to use API key authentication: - Add TestAPIKey constant and include in all HTTP requests - Update HTTPClient to support API key header - Fix expected status codes for nonexistent server (404 vs 500) - Fix TestGracefulShutdownNoPanic: start stderr reading goroutine immediately to prevent server blocking when debug logging fills pipe buffer - Regenerate swagger.yaml with corrected endpoint paths 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b285164 commit f0ca376

9 files changed

Lines changed: 681 additions & 885 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ swagger:
2828
@echo "📚 Generating OpenAPI 3.1 specification..."
2929
@[ -x "$(SWAGGER_BIN)" ] || { echo "⚠️ swag binary not found at $(SWAGGER_BIN). Run 'go install github.com/swaggo/swag/v2/cmd/swag@v2.0.0-rc4'"; exit 1; }
3030
@mkdir -p $(SWAGGER_OUT)
31-
$(SWAGGER_BIN) init -g $(SWAGGER_ENTRY) --output $(SWAGGER_OUT) --outputTypes go,yaml --v3.1
31+
$(SWAGGER_BIN) init -g $(SWAGGER_ENTRY) --output $(SWAGGER_OUT) --outputTypes go,yaml --v3.1 --exclude specs
3232
@echo "✅ OpenAPI 3.1 spec generated: $(SWAGGER_OUT)/swagger.yaml and $(SWAGGER_OUT)/docs.go"
3333

3434
swagger-verify: swagger

‎internal/httpapi/server.go‎

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ func (s *Server) setupRoutes() {
265265
})
266266

267267
// Health and readiness endpoints (Kubernetes-compatible with legacy aliases)
268+
// See healthzHandler() and readyzHandler() for swagger documentation
268269
livenessHandler := func(w http.ResponseWriter, _ *http.Request) {
269270
w.Header().Set("Content-Type", "application/json")
270271
w.WriteHeader(http.StatusOK)
@@ -443,6 +444,29 @@ func (rw *responseWriter) Flush() {
443444
}
444445
}
445446

447+
// Health and readiness documentation handlers (for swagger generation only)
448+
// The actual handlers are registered in setupRoutes() and may come from observability package
449+
450+
// healthzHandler godoc
451+
// @Summary Get health status
452+
// @Description Get comprehensive health status including all component health (Kubernetes-compatible liveness probe)
453+
// @Tags health
454+
// @Produce json
455+
// @Success 200 {object} observability.HealthResponse "Service is healthy"
456+
// @Failure 503 {object} observability.HealthResponse "Service is unhealthy"
457+
// @Router /healthz [get]
458+
func _healthzHandler() {} //nolint:unused // swagger documentation stub
459+
460+
// readyzHandler godoc
461+
// @Summary Get readiness status
462+
// @Description Get readiness status including all component readiness checks (Kubernetes-compatible readiness probe)
463+
// @Tags health
464+
// @Produce json
465+
// @Success 200 {object} observability.ReadinessResponse "Service is ready"
466+
// @Failure 503 {object} observability.ReadinessResponse "Service is not ready"
467+
// @Router /readyz [get]
468+
func _readyzHandler() {} //nolint:unused // swagger documentation stub
469+
446470
// JSON response helpers
447471

448472
func (s *Server) writeJSON(w http.ResponseWriter, status int, data interface{}) {
@@ -472,7 +496,7 @@ func (s *Server) writeSuccess(w http.ResponseWriter, data interface{}) {
472496
// @Security ApiKeyQuery
473497
// @Success 200 {object} contracts.SuccessResponse "Server status information"
474498
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
475-
// @Router /status [get]
499+
// @Router /api/v1/status [get]
476500
func (s *Server) handleGetStatus(w http.ResponseWriter, _ *http.Request) {
477501
response := map[string]interface{}{
478502
"running": s.controller.IsRunning(),
@@ -495,7 +519,7 @@ func (s *Server) handleGetStatus(w http.ResponseWriter, _ *http.Request) {
495519
// @Security ApiKeyQuery
496520
// @Success 200 {object} contracts.SuccessResponse "Server information"
497521
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
498-
// @Router /info [get]
522+
// @Router /api/v1/info [get]
499523
func (s *Server) handleGetInfo(w http.ResponseWriter, r *http.Request) {
500524
listenAddr := s.controller.GetListenAddress()
501525

@@ -579,7 +603,7 @@ func getSocketPath() string {
579603
// @Security ApiKeyQuery
580604
// @Success 200 {object} contracts.GetServersResponse "Server list with statistics"
581605
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
582-
// @Router /servers [get]
606+
// @Router /api/v1/servers [get]
583607
func (s *Server) handleGetServers(w http.ResponseWriter, r *http.Request) {
584608
// Try to use management service if available
585609
if mgmtSvc := s.controller.GetManagementService(); mgmtSvc != nil {
@@ -648,7 +672,7 @@ func (s *Server) handleGetServers(w http.ResponseWriter, r *http.Request) {
648672
// @Failure 400 {object} contracts.ErrorResponse "Bad request"
649673
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
650674
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
651-
// @Router /servers/{id}/enable [post]
675+
// @Router /api/v1/servers/{id}/enable [post]
652676
func (s *Server) handleEnableServer(w http.ResponseWriter, r *http.Request) {
653677
serverID := chi.URLParam(r, "id")
654678
if serverID == "" {
@@ -714,7 +738,7 @@ func (s *Server) handleEnableServer(w http.ResponseWriter, r *http.Request) {
714738
// @Failure 400 {object} contracts.ErrorResponse "Bad request"
715739
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
716740
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
717-
// @Router /servers/{id}/disable [post]
741+
// @Router /api/v1/servers/{id}/disable [post]
718742
func (s *Server) handleDisableServer(w http.ResponseWriter, r *http.Request) {
719743
serverID := chi.URLParam(r, "id")
720744
if serverID == "" {
@@ -778,7 +802,7 @@ func (s *Server) handleDisableServer(w http.ResponseWriter, r *http.Request) {
778802
// @Param reason query string false "Reason for reconnection"
779803
// @Success 200 {object} contracts.ServerActionResponse "All servers reconnected successfully"
780804
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
781-
// @Router /servers/reconnect [post]
805+
// @Router /api/v1/servers/reconnect [post]
782806
func (s *Server) handleForceReconnectServers(w http.ResponseWriter, r *http.Request) {
783807
reason := r.URL.Query().Get("reason")
784808

@@ -809,7 +833,7 @@ func (s *Server) handleForceReconnectServers(w http.ResponseWriter, r *http.Requ
809833
// @Success 200 {object} management.BulkOperationResult "Bulk restart results with success/failure counts"
810834
// @Failure 403 {object} contracts.ErrorResponse "Forbidden (management disabled)"
811835
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
812-
// @Router /servers/restart_all [post]
836+
// @Router /api/v1/servers/restart_all [post]
813837
func (s *Server) handleRestartAll(w http.ResponseWriter, r *http.Request) {
814838
// Get management service from controller
815839
mgmtSvc, ok := s.controller.GetManagementService().(interface {
@@ -841,7 +865,7 @@ func (s *Server) handleRestartAll(w http.ResponseWriter, r *http.Request) {
841865
// @Success 200 {object} management.BulkOperationResult "Bulk enable results with success/failure counts"
842866
// @Failure 403 {object} contracts.ErrorResponse "Forbidden (management disabled)"
843867
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
844-
// @Router /servers/enable_all [post]
868+
// @Router /api/v1/servers/enable_all [post]
845869
func (s *Server) handleEnableAll(w http.ResponseWriter, r *http.Request) {
846870
// Get management service from controller
847871
mgmtSvc, ok := s.controller.GetManagementService().(interface {
@@ -873,7 +897,7 @@ func (s *Server) handleEnableAll(w http.ResponseWriter, r *http.Request) {
873897
// @Success 200 {object} management.BulkOperationResult "Bulk disable results with success/failure counts"
874898
// @Failure 403 {object} contracts.ErrorResponse "Forbidden (management disabled)"
875899
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
876-
// @Router /servers/disable_all [post]
900+
// @Router /api/v1/servers/disable_all [post]
877901
func (s *Server) handleDisableAll(w http.ResponseWriter, r *http.Request) {
878902
// Get management service from controller
879903
mgmtSvc, ok := s.controller.GetManagementService().(interface {
@@ -907,7 +931,7 @@ func (s *Server) handleDisableAll(w http.ResponseWriter, r *http.Request) {
907931
// @Failure 400 {object} contracts.ErrorResponse "Bad request"
908932
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
909933
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
910-
// @Router /servers/{id}/restart [post]
934+
// @Router /api/v1/servers/{id}/restart [post]
911935
func (s *Server) handleRestartServer(w http.ResponseWriter, r *http.Request) {
912936
serverID := chi.URLParam(r, "id")
913937
if serverID == "" {
@@ -1051,7 +1075,7 @@ func (s *Server) toggleServerAsync(serverID string, enabled bool) (bool, error)
10511075
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing server ID)"
10521076
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
10531077
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1054-
// @Router /servers/{id}/login [post]
1078+
// @Router /api/v1/servers/{id}/login [post]
10551079
func (s *Server) handleServerLogin(w http.ResponseWriter, r *http.Request) {
10561080
serverID := chi.URLParam(r, "id")
10571081
if serverID == "" {
@@ -1107,7 +1131,7 @@ func (s *Server) handleServerLogin(w http.ResponseWriter, r *http.Request) {
11071131
// @Failure 403 {object} contracts.ErrorResponse "Forbidden (management disabled or read-only mode)"
11081132
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
11091133
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1110-
// @Router /servers/{id}/logout [post]
1134+
// @Router /api/v1/servers/{id}/logout [post]
11111135
func (s *Server) handleServerLogout(w http.ResponseWriter, r *http.Request) {
11121136
serverID := chi.URLParam(r, "id")
11131137
if serverID == "" {
@@ -1162,7 +1186,7 @@ func (s *Server) handleServerLogout(w http.ResponseWriter, r *http.Request) {
11621186
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing server ID)"
11631187
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
11641188
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1165-
// @Router /servers/{id}/quarantine [post]
1189+
// @Router /api/v1/servers/{id}/quarantine [post]
11661190
func (s *Server) handleQuarantineServer(w http.ResponseWriter, r *http.Request) {
11671191
serverID := chi.URLParam(r, "id")
11681192
if serverID == "" {
@@ -1197,7 +1221,7 @@ func (s *Server) handleQuarantineServer(w http.ResponseWriter, r *http.Request)
11971221
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing server ID)"
11981222
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
11991223
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1200-
// @Router /servers/{id}/unquarantine [post]
1224+
// @Router /api/v1/servers/{id}/unquarantine [post]
12011225
func (s *Server) handleUnquarantineServer(w http.ResponseWriter, r *http.Request) {
12021226
serverID := chi.URLParam(r, "id")
12031227
if serverID == "" {
@@ -1232,7 +1256,7 @@ func (s *Server) handleUnquarantineServer(w http.ResponseWriter, r *http.Request
12321256
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing server ID)"
12331257
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
12341258
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1235-
// @Router /servers/{id}/tools [get]
1259+
// @Router /api/v1/servers/{id}/tools [get]
12361260
func (s *Server) handleGetServerTools(w http.ResponseWriter, r *http.Request) {
12371261
serverID := chi.URLParam(r, "id")
12381262
if serverID == "" {
@@ -1288,7 +1312,7 @@ func (s *Server) handleGetServerTools(w http.ResponseWriter, r *http.Request) {
12881312
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing server ID)"
12891313
// @Failure 404 {object} contracts.ErrorResponse "Server not found"
12901314
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1291-
// @Router /servers/{id}/logs [get]
1315+
// @Router /api/v1/servers/{id}/logs [get]
12921316
func (s *Server) handleGetServerLogs(w http.ResponseWriter, r *http.Request) {
12931317
serverID := chi.URLParam(r, "id")
12941318
if serverID == "" {
@@ -1332,7 +1356,7 @@ func (s *Server) handleGetServerLogs(w http.ResponseWriter, r *http.Request) {
13321356
// @Success 200 {object} contracts.SearchToolsResponse "Search results"
13331357
// @Failure 400 {object} contracts.ErrorResponse "Bad request (missing query parameter)"
13341358
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1335-
// @Router /index/search [get]
1359+
// @Router /api/v1/index/search [get]
13361360
func (s *Server) handleSearchTools(w http.ResponseWriter, r *http.Request) {
13371361
query := r.URL.Query().Get("q")
13381362
if query == "" {
@@ -1779,8 +1803,8 @@ func (s *Server) handleDeleteSecret(w http.ResponseWriter, r *http.Request) {
17791803
// @Security ApiKeyQuery
17801804
// @Success 200 {object} contracts.Diagnostics "Health diagnostics"
17811805
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1782-
// @Router /diagnostics [get]
1783-
// @Router /doctor [get]
1806+
// @Router /api/v1/diagnostics [get]
1807+
// @Router /api/v1/doctor [get]
17841808
func (s *Server) handleGetDiagnostics(w http.ResponseWriter, r *http.Request) {
17851809
if r.Method != http.MethodGet {
17861810
s.writeError(w, http.StatusMethodNotAllowed, "Method not allowed")
@@ -1872,7 +1896,7 @@ func (s *Server) handleGetDiagnostics(w http.ResponseWriter, r *http.Request) {
18721896
// @Security ApiKeyQuery
18731897
// @Success 200 {object} contracts.SuccessResponse "Token statistics"
18741898
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
1875-
// @Router /stats/tokens [get]
1899+
// @Router /api/v1/stats/tokens [get]
18761900
func (s *Server) handleGetTokenStats(w http.ResponseWriter, r *http.Request) {
18771901
if r.Method != http.MethodGet {
18781902
s.writeError(w, http.StatusMethodNotAllowed, "Method not allowed")
@@ -2336,7 +2360,7 @@ func (s *Server) handleApplyConfig(w http.ResponseWriter, r *http.Request) {
23362360
// @Success 200 {object} contracts.SuccessResponse "Tool call result"
23372361
// @Failure 400 {object} contracts.ErrorResponse "Bad request (invalid payload or missing tool name)"
23382362
// @Failure 500 {object} contracts.ErrorResponse "Internal server error or tool execution failure"
2339-
// @Router /tools/call [post]
2363+
// @Router /api/v1/tools/call [post]
23402364
func (s *Server) handleCallTool(w http.ResponseWriter, r *http.Request) {
23412365
if r.Method != http.MethodPost {
23422366
s.writeError(w, http.StatusMethodNotAllowed, "Method not allowed")
@@ -2648,7 +2672,7 @@ func (s *Server) handleGetSessionDetail(w http.ResponseWriter, r *http.Request)
26482672
// @Security ApiKeyQuery
26492673
// @Success 200 {object} contracts.SuccessResponse "Docker status information"
26502674
// @Failure 500 {object} contracts.ErrorResponse "Internal server error"
2651-
// @Router /docker/status [get]
2675+
// @Router /api/v1/docker/status [get]
26522676
func (s *Server) handleGetDockerStatus(w http.ResponseWriter, r *http.Request) {
26532677
status := s.controller.GetDockerRecoveryStatus()
26542678
if status == nil {

‎internal/server/e2e_binary_test.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestBinaryStartupAndShutdown(t *testing.T) {
3535
env.Start()
3636

3737
// Verify server is responding
38-
client := testutil.NewHTTPClient(env.GetAPIURL())
38+
client := env.GetHTTPClient()
3939
var response testutil.TestServerList
4040
err := client.GetJSON("/servers", &response)
4141
require.NoError(t, err)
@@ -50,7 +50,7 @@ func TestBinaryAPIEndpoints(t *testing.T) {
5050
env.Start()
5151
env.WaitForEverythingServer()
5252

53-
client := testutil.NewHTTPClient(env.GetAPIURL())
53+
client := env.GetHTTPClient()
5454

5555
t.Run("GET /servers", func(t *testing.T) {
5656
var response testutil.TestServerList
@@ -205,13 +205,14 @@ func TestBinaryErrorHandling(t *testing.T) {
205205
env.Start()
206206
env.WaitForEverythingServer()
207207

208-
client := testutil.NewHTTPClient(env.GetAPIURL())
208+
client := env.GetHTTPClient()
209209

210210
t.Run("GET /servers/nonexistent/tools", func(t *testing.T) {
211211
resp, err := client.Get("/servers/nonexistent/tools")
212212
require.NoError(t, err)
213213
defer resp.Body.Close()
214-
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
214+
// 404 is the expected response for a nonexistent server
215+
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
215216
})
216217

217218
t.Run("GET /index/search without query", func(t *testing.T) {
@@ -225,6 +226,7 @@ func TestBinaryErrorHandling(t *testing.T) {
225226
resp, err := client.PostJSON("/servers/nonexistent/enable", nil)
226227
require.NoError(t, err)
227228
defer resp.Body.Close()
229+
// Note: POST returns 500 for nonexistent servers (different from GET's 404)
228230
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
229231
})
230232
}
@@ -239,7 +241,7 @@ func TestBinarySSEEvents(t *testing.T) {
239241
// Wait a moment for the server to start sending events
240242
time.Sleep(500 * time.Millisecond)
241243

242-
client := testutil.NewHTTPClient(env.GetBaseURL())
244+
client := testutil.NewHTTPClientWithAPIKey(env.GetBaseURL(), testutil.TestAPIKey)
243245
resp, err := client.Get("/events")
244246
require.NoError(t, err)
245247
defer resp.Body.Close()
@@ -272,7 +274,7 @@ func TestBinaryConcurrentRequests(t *testing.T) {
272274
env.Start()
273275
env.WaitForEverythingServer()
274276

275-
client := testutil.NewHTTPClient(env.GetAPIURL())
277+
client := env.GetHTTPClient()
276278

277279
// Make multiple concurrent requests
278280
done := make(chan bool, 5)
@@ -320,7 +322,7 @@ func TestBinaryPerformance(t *testing.T) {
320322
env.Start()
321323
env.WaitForEverythingServer()
322324

323-
client := testutil.NewHTTPClient(env.GetAPIURL())
325+
client := env.GetHTTPClient()
324326

325327
t.Run("Server list response time", func(t *testing.T) {
326328
start := time.Now()
@@ -368,7 +370,7 @@ func TestBinaryHealthAndRecovery(t *testing.T) {
368370
env.Start()
369371
env.WaitForEverythingServer()
370372

371-
client := testutil.NewHTTPClient(env.GetAPIURL())
373+
client := env.GetHTTPClient()
372374

373375
t.Run("Server restart and recovery", func(t *testing.T) {
374376
// Restart the memory server

‎internal/server/info_shutdown_e2e_test.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,23 @@ func TestGracefulShutdownNoPanic(t *testing.T) {
167167
"--listen", listenAddr,
168168
"--log-level", "debug")
169169

170-
// Disable API key authentication for E2E tests
171-
cmd.Env = append(os.Environ(), "MCPPROXY_API_KEY=")
170+
// Set a fixed API key for E2E tests
171+
cmd.Env = append(os.Environ(), "MCPPROXY_API_KEY=e2e-test-api-key-12345")
172172

173173
// Capture stderr to check for panic messages
174174
stderrPipe, err := cmd.StderrPipe()
175175
require.NoError(t, err)
176176

177177
require.NoError(t, cmd.Start(), "Failed to start server")
178178

179+
// Start reading stderr immediately in a goroutine to prevent blocking
180+
// (with --log-level=debug, the pipe buffer can fill up quickly)
181+
stderrOutput := make(chan string, 1)
182+
go func() {
183+
output, _ := io.ReadAll(stderrPipe)
184+
stderrOutput <- string(output)
185+
}()
186+
179187
// Wait for server to be ready
180188
serverURL := fmt.Sprintf("http://%s", listenAddr)
181189
require.True(t, waitForServer(serverURL, 10*time.Second), "Server did not become ready")
@@ -192,13 +200,6 @@ func TestGracefulShutdownNoPanic(t *testing.T) {
192200
t.Log("Sending SIGINT to server process...")
193201
require.NoError(t, cmd.Process.Signal(syscall.SIGINT))
194202

195-
// Read stderr in a goroutine
196-
stderrOutput := make(chan string, 1)
197-
go func() {
198-
output, _ := io.ReadAll(stderrPipe)
199-
stderrOutput <- string(output)
200-
}()
201-
202203
// Wait for process to exit (with timeout)
203204
done := make(chan error, 1)
204205
go func() {
@@ -254,8 +255,8 @@ func TestSocketInfoEndpoint(t *testing.T) {
254255
"--listen", "127.0.0.1:0", // Random port for HTTP
255256
"--enable-socket", "true")
256257

257-
// Disable API key authentication for E2E tests
258-
cmd.Env = append(os.Environ(), "MCPPROXY_API_KEY=")
258+
// Set a fixed API key for E2E tests
259+
cmd.Env = append(os.Environ(), "MCPPROXY_API_KEY=e2e-test-api-key-12345")
259260

260261
require.NoError(t, cmd.Start(), "Failed to start server")
261262
defer func() {

0 commit comments

Comments
 (0)