diff --git a/mothership/internal/fleet/fleethandler.go b/mothership/internal/fleet/fleethandler.go index 85ff81d..e1baa5a 100644 --- a/mothership/internal/fleet/fleethandler.go +++ b/mothership/internal/fleet/fleethandler.go @@ -34,29 +34,20 @@ func (h *FleetHandler) SetUnpairedProvider(p UnpairedProvider) { h.unpairedProvider = p } -// RegisterRoutes mounts fleet endpoints on r. +// RegisterRoutes mounts fleet health/optimisation endpoints on r. // -// GET /api/fleet — all provisioned nodes with full details // GET /api/fleet/health — current fleet health status // GET /api/fleet/history — recent optimisation history // POST /api/fleet/optimise — trigger manual re-optimisation // GET /api/fleet/simulate — simulate node removal impact -// PATCH /api/nodes/{mac}/label — update node label -// POST /api/nodes/{mac}/locate — send identify command -// POST /api/nodes/{mac}/role — assign new role -// DELETE /api/nodes/{mac} — remove from fleet +// +// Node-specific routes (role, label, locate, delete) are handled by fleet.Handler +// to avoid duplicate route registration. func (h *FleetHandler) RegisterRoutes(r chi.Router) { - r.Get("/api/fleet", h.getFleet) r.Get("/api/fleet/health", h.getFleetHealth) r.Get("/api/fleet/history", h.getFleetHistory) r.Post("/api/fleet/optimise", h.triggerOptimise) r.Get("/api/fleet/simulate", h.simulateNodeRemoval) - - // Node-specific routes - r.Patch("/api/nodes/{mac}/label", h.setNodeLabel) - r.Post("/api/nodes/{mac}/locate", h.locateNode) - r.Post("/api/nodes/{mac}/role", h.setNodeRole) - r.Delete("/api/nodes/{mac}", h.removeNode) } // fleetHealthResponse is the wire format for /api/fleet/health diff --git a/mothership/internal/fleet/handler_test.go b/mothership/internal/fleet/handler_test.go index 50f11c8..b17946f 100644 --- a/mothership/internal/fleet/handler_test.go +++ b/mothership/internal/fleet/handler_test.go @@ -2510,3 +2510,55 @@ func TestHandlerDisableEnableRoundTrip(t *testing.T) { t.Errorf("Expected role_before_disable to be cleared after enable, got %s", savedRole) } } + +// TestRouteRegistrationNoPanic verifies that registering both Handler and FleetHandler +// on the same router does not cause chi to panic with duplicate route registration. +// This is a startup smoke test to catch accidental route conflicts early. +func TestRouteRegistrationNoPanic(t *testing.T) { + reg := newTestRegistry(t) + mgr := NewManager(reg) + selfHealMgr := NewSelfHealManager(reg, NewRoleOptimiser(DefaultOptimisationConfig()), DefaultSelfHealConfig()) + + // Create both handlers as main.go does + h := NewHandler(mgr) + fleetHealthHandler := NewFleetHandler(selfHealMgr, reg) + + // This should not panic - if there are duplicate routes, chi will panic here + r := chi.NewRouter() + h.RegisterRoutes(r) + fleetHealthHandler.RegisterRoutes(r) + + // Verify expected routes are registered + // From Handler: /api/nodes, /api/fleet (list), /api/nodes/{mac}/role, etc. + // From FleetHandler: /api/fleet/health, /api/fleet/history, /api/fleet/optimise, /api/fleet/simulate + // The key is no panic on duplicate routes like POST /api/nodes/{mac}/role + + // Try to walk the routes to ensure they're registered + routes := chi.Routes(r) + if len(routes) == 0 { + t.Fatal("No routes registered") + } + + // Verify some expected routes exist + expectedRoutes := []string{ + "/api/nodes", + "/api/fleet", + "/api/fleet/health", + "/api/fleet/history", + "/api/fleet/optimise", + "/api/fleet/simulate", + } + + for _, expected := range expectedRoutes { + found := false + for _, route := range routes { + if route.Pattern == expected { + found = true + break + } + } + if !found { + t.Errorf("Expected route %s not found", expected) + } + } +}