feat: Refactor UI layout and simplify SSE notifications

- Replace 'Add Server' modal with an inline form on the main page.
- Rearrange page titles for better flow ('Add New MCP Server' -> 'Connected MCP Servers').
- Remove footer and unused modal templates.
- Introduce `ServerDataUpdatedEvent` for backend changes.
- Simplify SSE handler to listen only to `ServerDataUpdatedEvent` and broadcast a generic `refresh-list` message.
- Update API handlers (`AddMCPServer`, `RemoveMCPServer`) to return HTMX feedback fragments instead of redirects.
- Correct `RemoveMCPServer` route to use DELETE method.

Author: michiosw

internal/app/app.go

@@ -118,7 +118,7 @@ var MCPClientModule = fx.Module("mcp_client",
 // WebModule provides the HTTP server, API handlers, UI handlers, and SSE handler.
 var WebModule = fx.Module("web",
 	fx.Provide(
-		// Provide UI handler (depends on Logger, BackendService)
+		// Provide UI handler struct (depends on Logger, BackendService)
 		handlers.NewUIHandler,
 		// Provide API handlers (depends on BackendService, Logger)
 		func(bs backend.Service, logger log.Logger) *handlers.APIHandlers {
@@ -151,15 +151,24 @@ var WebModule = fx.Module("web",
 		web.NewServer,
 	),
 	// Register handlers with the router
-	fx.Invoke(func(mux *http.ServeMux, apiHandlers *handlers.APIHandlers, uiHandler http.Handler, sseHandler *handlers.SSEHandler) {
+	fx.Invoke(func(mux *http.ServeMux, apiHandlers *handlers.APIHandlers, uiHandler *handlers.UIHandler, sseHandler *handlers.SSEHandler) {
 		// API Routes (includes Swagger/OpenAPI docs)
 		apiHandlers.RegisterRoutes(mux)
-		// UI Route
-		mux.Handle("/ui/", http.StripPrefix("/ui/", uiHandler))                    // Serve UI under /ui/
-		mux.Handle("/", http.RedirectHandler("/ui/", http.StatusMovedPermanently)) // Redirect root to UI
-		// SSE Route
+
+		// --- UI Routes (Registered directly) ---
+		mux.HandleFunc("/ui/", uiHandler.ServeIndex) // Note trailing slash
+		// mux.HandleFunc("/ui/add", uiHandler.ServeAddPage)       // Removed
+		// mux.HandleFunc("/ui/success", uiHandler.ServeSuccessPage) // Removed
+		// mux.HandleFunc("/ui/error", uiHandler.ServeErrorPage)   // Removed
+
+		// --- Root Redirect ---
+		// Use mux.Handle for http.RedirectHandler as it returns an http.Handler
+		mux.Handle("/", http.RedirectHandler("/ui/", http.StatusMovedPermanently))
+
+		// --- SSE Route ---
 		mux.HandleFunc("/api/events", sseHandler.ServeHTTP)
-		// Metrics Route (already in api.go RegisterRoutes? If not, add here)
+
+		// Metrics Route (example, if needed)
 		// mux.Handle("/metrics", promhttp.Handler())
 	}),
 	// Register web server lifecycle hooks for starting/stopping the server

internal/backend/service.go

@@ -81,6 +81,8 @@ func (s *service) AddMCPServer(name, url string) (*models.MCPServer, error) {
 		s.logger.Debug().Interface("server", newServer).Msg("Fetched new server for event")
 	}
 	s.bus.Publish(events.NewServerAddedEvent(*newServer))
+	// Publish data update event for UI
+	s.bus.Publish(events.NewServerDataUpdatedEvent())
 	return newServer, nil
 }
 
@@ -103,6 +105,8 @@ func (s *service) RemoveMCPServer(id int64) error {
 	}
 	s.logger.Info().Str("name", server.Name).Str("url", serverURL).Int64("id", id).Msg("Removed MCP Server")
 	s.bus.Publish(events.NewServerRemovedEvent(id, serverURL))
+	// Publish data update event for UI
+	s.bus.Publish(events.NewServerDataUpdatedEvent())
 	return nil
 }
 
@@ -183,6 +187,9 @@ func (s *service) UpdateMCPServerStatus(id int64, state models.ConnectionState,
 	// s.bus.Publish(events.NewServerStatusChangedEvent(id, state, errStr))
 	s.logger.Debug().Int64("id", id).Str("state", string(state)).Msg("Updated server status in DB (event published by ConnectionManager)")
 
+	// Publish data update event for UI AFTER DB update
+	s.bus.Publish(events.NewServerDataUpdatedEvent())
+
 	// Fetching the updated server is no longer strictly necessary just for publishing the event
 	// updatedServer, getErr := s.db.GetServerByID(id)
 	// if getErr != nil || updatedServer == nil {
@@ -226,6 +233,10 @@ func (s *service) UpdateMCPServer(id int64, name, url string) (*models.MCPServer
 
 	s.logger.Info().Int64("id", id).Str("newName", name).Str("newURL", url).Msg("Updated MCP Server Details")
 	// s.bus.Publish(events.NewServerUpdatedEvent(*updatedServer)) // TODO: Define and use a ServerUpdatedEvent
+
+	// Publish data update event for UI
+	s.bus.Publish(events.NewServerDataUpdatedEvent())
+
 	return updatedServer, nil
 }
 
@@ -338,8 +349,8 @@ func (s *service) HandleToolsUpdated(event events.Event) {
 		s.logger.Warn().Int64("serverID", serverID).Msg("Skipping ToolsProcessedInDBEvent due to errors during tool upsert")
 	}
 
-	// TODO: Potentially publish another event like 'ToolsProcessed' if other components
-	// need to know the DB update is complete (e.g., to trigger MCP server tool refresh).
+	// Publish data update event for UI as tool list might have changed
+	s.bus.Publish(events.NewServerDataUpdatedEvent())
 }
 
 // RegisterEventHandlers registers the necessary event handlers for the backend service.

internal/events/events.go

@@ -24,6 +24,8 @@ const (
 	ToolsProcessedInDB EventType = "tools.processed.db"
 	// LocalToolsRefreshed indicates the agent's own exposed toolset was potentially updated.
 	LocalToolsRefreshed EventType = "local.tools.refreshed"
+	// ServerDataUpdated signals that backend data relevant to the UI has changed.
+	ServerDataUpdated EventType = "server.data.updated"
 	// Add more event types as needed
 )
 
@@ -141,6 +143,22 @@ func NewToolsProcessedInDBEvent(serverID int64, serverURL string) *ToolsProcesse
 	}
 }
 
+// --- NEW Event for UI Updates ---
+
+// ServerDataUpdatedEvent is published by the BackendService after any operation
+// that changes data relevant to the UI (add, remove, status update, etc.).
+type ServerDataUpdatedEvent struct {
+	baseEvent
+	// Add specific change details if needed later (e.g., ServerID, ChangeType)
+}
+
+// NewServerDataUpdatedEvent creates a new ServerDataUpdatedEvent.
+func NewServerDataUpdatedEvent() *ServerDataUpdatedEvent {
+	return &ServerDataUpdatedEvent{
+		baseEvent: newBaseEvent(ServerDataUpdated),
+	}
+}
+
 // LocalToolsRefreshedEvent is published by the ConnectionManager after refreshMCPServerTools completes.
 // It signals that the list of tools exposed by this agent's MCP server may have changed.
 type LocalToolsRefreshedEvent struct {

internal/mcp/server.go

@@ -956,11 +956,18 @@ func (cm *ConnectionManager) handleServerRemoved(event events.Event) {
 		if _, toolsExist := cm.toolsByServer[removedEvent.ServerURL]; toolsExist {
 			delete(cm.toolsByServer, removedEvent.ServerURL)
 			mcpToolsTotal.WithLabelValues(removedEvent.ServerURL).Set(0)
-			// Refreshing the MCP server tools might need to happen AFTER the backend confirms DB update.
-			// Let's assume ToolsProcessedInDBEvent handles this refresh.
-			// cm.refreshMCPServerTools() // Avoid calling this directly here
+			// Refreshing the MCP server tools MUST happen after removal
+			cm.logger.Info().Msg("Triggering local MCP tool refresh after server removal.")
+			cm.refreshMCPServerTools()
+			cm.mu.Unlock() // Unlock before publishing event
+
+			// Publish event to signal local tools changed
+			cm.logger.Info().Msg("Publishing LocalToolsRefreshedEvent after server removal.")
+			cm.eventBus.Publish(events.NewLocalToolsRefreshedEvent())
+
+		} else {
+			cm.mu.Unlock() // Ensure unlock even if tools didn't exist
 		}
-		cm.mu.Unlock()
 
 	} else {
 		cm.logger.Info().Str("url", removedEvent.ServerURL).Msg("No active connection found for removed server")

internal/web/handlers/api.go

@@ -12,6 +12,7 @@ import (
 	"github.com/co-browser/agent-browser/internal/backend/models"
 	"github.com/co-browser/agent-browser/internal/log"
 	"github.com/co-browser/agent-browser/internal/web/templates"
+	"github.com/co-browser/agent-browser/internal/web/templates/components"
 	// "github.com/gorilla/mux" // Example: Assuming a router like gorilla/mux for path params
 )
 
@@ -54,7 +55,7 @@ func (h *APIHandlers) RegisterRoutes(mux *http.ServeMux /* or other router type
 	mux.HandleFunc("POST /api/mcp/servers/update/", h.routeServerUpdateRequests)
 
 	// For delete requests (remove server)
-	mux.HandleFunc("POST /api/mcp/servers/remove/", h.routeServerDeleteRequests)
+	mux.HandleFunc("DELETE /api/mcp/servers/", h.routeServerDeleteRequests)
 
 	// For POST requests (rediscover tools)
 	mux.HandleFunc("POST /api/mcp/servers/", h.routeServerPostRequests)
@@ -142,15 +143,16 @@ func (h *APIHandlers) routeServerUpdateRequests(w http.ResponseWriter, r *http.R
 
 // routeServerDeleteRequests handles DELETE requests for server-specific paths (Recommended structure)
 func (h *APIHandlers) routeServerDeleteRequests(w http.ResponseWriter, r *http.Request) {
-	serverID, subpath, err := parseServerPath(r.URL.Path, "/api/mcp/servers/remove/")
+	// Update prefix to match the new route pattern
+	serverID, subpath, err := parseServerPath(r.URL.Path, "/api/mcp/servers/")
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 
 	switch subpath {
 	case "":
-		// POST /api/mcp/servers/remove/:id - Remove server
+		// DELETE /api/mcp/servers/:id - Remove server
 		h.RemoveMCPServer(w, r, serverID)
 	default:
 		http.NotFound(w, r)
@@ -191,32 +193,44 @@ func (h *APIHandlers) AddMCPServer(w http.ResponseWriter, r *http.Request) {
 	// Parse form data
 	if err := r.ParseForm(); err != nil {
 		h.logger.Error().Err(err).Msg("Failed to parse form data")
-		http.Redirect(w, r, "/error?error=Form parse error", http.StatusSeeOther)
+		w.WriteHeader(http.StatusBadRequest)
+		// Use components package directly
+		components.AddServerErrorFeedback("Form parse error").Render(r.Context(), w)
 		return
 	}
 
 	name := r.FormValue("name")
 	url := r.FormValue("url")
 
 	if name == "" || url == "" {
-		http.Redirect(w, r, "/error?error=Missing name or url", http.StatusSeeOther)
+		w.WriteHeader(http.StatusBadRequest)
+		// Use components package directly
+		components.AddServerErrorFeedback("Missing name or url").Render(r.Context(), w)
 		return
 	}
 
-	_, err := h.backendService.AddMCPServer(name, url)
+	server, err := h.backendService.AddMCPServer(name, url)
 	if err != nil {
 		h.logger.Error().Err(err).Msg("Error adding MCP server via API")
-
-		if err.Error() == fmt.Sprintf("MCP server with URL '%s' already exists", url) {
-			http.Redirect(w, r, "/error?error=Server already exists", http.StatusSeeOther)
-			return
+		errorMsg := "Internal server error adding server"
+		statusCode := http.StatusInternalServerError
+		if strings.Contains(err.Error(), "already exists") {
+			statusCode = http.StatusBadRequest
+			errorMsg = fmt.Sprintf("Server with URL '%s' already exists", url)
 		}
-
-		http.Redirect(w, r, "/error?error=Internal server error", http.StatusSeeOther)
+		w.WriteHeader(statusCode)
+		// Use components package directly
+		components.AddServerErrorFeedback(errorMsg).Render(r.Context(), w)
 		return
 	}
 
-	http.Redirect(w, r, "/success", http.StatusSeeOther)
+	// Success: Return success alert fragment for the feedback div
+	w.WriteHeader(http.StatusOK)
+	// Send HX-Trigger to reset form after successful request (Optional - requires JS listener)
+	// w.Header().Set("HX-Trigger", "resetAddForm")
+	// Use components package directly
+	components.AddServerSuccessFeedback(server.Name).Render(r.Context(), w)
+	// SSE refresh will handle list update separately
 }
 
 // ListMCPServers handles GET /api/mcp/servers - Returns full JSON
@@ -239,24 +253,16 @@ func (h *APIHandlers) ListMCPServersFragment(w http.ResponseWriter, r *http.Requ
 	servers, err := h.backendService.ListMCPServers()
 	if err != nil {
 		h.logger.Error().Err(err).Msg("Error listing MCP servers for fragment")
-		// Optionally render an error message within the fragment
-		// w.WriteHeader(http.StatusInternalServerError)
-		// templates.ServerErrorFragment("Failed to load servers").Render(r.Context(), w)
-		// For now, return empty list on error
-		servers = []models.MCPServer{}
+		w.WriteHeader(http.StatusInternalServerError)
+		// Use components package directly for error feedback
+		components.AddServerErrorFeedback("Failed to load server list").Render(r.Context(), w)
+		return
 	}
 
 	// Set content type to HTML
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
-
-	// Render only the body component
-	component := templates.ServerListBodyComponent(servers)
-	err = component.Render(r.Context(), w)
-	if err != nil {
-		h.logger.Error().Err(err).Msg("Error rendering server list fragment")
-		// Don't write header again if Render already wrote partial content
-		// http.Error(w, "Failed to render server list fragment", http.StatusInternalServerError)
-	}
+	// Render the list body component directly using parent templates package
+	templates.ServerListBodyComponent(servers).Render(r.Context(), w)
 }
 
 // GetMCPServer handles GET /api/mcp/servers/:id
@@ -337,22 +343,25 @@ func (h *APIHandlers) UpdateMCPServer(w http.ResponseWriter, r *http.Request, se
 	}
 }
 
-// RemoveMCPServer handles POST /api/mcp/servers/:id
+// RemoveMCPServer handles DELETE /api/mcp/servers/:id
 func (h *APIHandlers) RemoveMCPServer(w http.ResponseWriter, r *http.Request, serverID int64) {
 	err := h.backendService.RemoveMCPServer(serverID)
-
 	if err != nil {
-		h.logger.Error().Err(err).Int64("serverId", serverID).Msg("Error removing MCP server via API")
-
+		h.logger.Error().Err(err).Int64("serverID", serverID).Msg("Error removing MCP server via API")
+		// Return error response for HTMX
 		if strings.Contains(err.Error(), "not found") {
-			http.Redirect(w, r, "/error?error=Server not found", http.StatusSeeOther)
+			w.WriteHeader(http.StatusNotFound)
+			_, _ = w.Write([]byte(fmt.Sprintf("Server with ID %d not found", serverID)))
 		} else {
-			http.Redirect(w, r, "/error?error=Failed to remove server", http.StatusSeeOther)
+			w.WriteHeader(http.StatusInternalServerError)
+			_, _ = w.Write([]byte("Internal server error removing server"))
 		}
 		return
 	}
 
-	http.Redirect(w, r, "/success", http.StatusSeeOther)
+	// Success: Return 200 OK. List update will happen via SSE.
+	w.WriteHeader(http.StatusOK)
+	// No body needed
 }
 
 // --- Tool Management Handlers ---

internal/web/handlers/sse.go

@@ -193,27 +193,47 @@ func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 // listenToEvents subscribes to the event bus and processes events.
 func (h *SSEHandler) listenToEvents() {
-	// Subscribe to specific events using handler functions
-	h.bus.Subscribe(events.ServerAdded, h.handleEvent)
-	h.bus.Subscribe(events.ServerRemoved, h.handleEvent)
-	h.bus.Subscribe(events.ServerStatusChanged, h.handleEvent)
+	// Subscribe to the consolidated ServerDataUpdated event
+	h.bus.Subscribe(events.ServerDataUpdated, h.handleServerDataUpdated)
+	// Remove individual subscriptions
+	// h.bus.Subscribe(events.ServerAdded, h.handleEvent)
+	// h.bus.Subscribe(events.ServerRemoved, h.handleEvent)
+	// h.bus.Subscribe(events.ServerStatusChanged, h.handleEvent)
 	// h.bus.Subscribe(events.ToolsUpdated, h.handleEvent) // Add if needed
 
-	h.logger.Info().Msg("SSE handler subscribed to event bus")
+	h.logger.Info().Msg("SSE handler subscribed to ServerDataUpdated events")
 
 	// Keep the goroutine alive until the handler context is cancelled
 	<-h.ctx.Done()
 	h.logger.Info().Msg("SSE event listener stopping due to context cancellation.")
 }
 
+// handleServerDataUpdated processes the ServerDataUpdated event and triggers a broadcast.
+func (h *SSEHandler) handleServerDataUpdated(event events.Event) {
+	// Type assert to check if it's the correct event type (optional but good practice)
+	if _, ok := event.(*events.ServerDataUpdatedEvent); !ok {
+		h.logger.Warn().Str("eventType", string(event.Type())).Msg("Received unexpected event type in handleServerDataUpdated")
+		return
+	}
+
+	h.logger.Debug().Str("eventType", string(event.Type())).Msg("SSE handler processing ServerDataUpdated event")
+	// Trigger a generic refresh broadcast
+	go h.broadcastRefresh()
+}
+
 // handleEvent is called by the event bus dispatcher when a subscribed event occurs.
+// DEPRECATED: Use handleServerDataUpdated instead.
+/*
 func (h *SSEHandler) handleEvent(event events.Event) {
 	h.logger.Debug().Str("eventType", string(event.Type())).Msg("SSE handler processing event")
 	// Process and broadcast in a separate goroutine to avoid blocking the event bus dispatcher
 	go h.processAndBroadcastEvent(event)
 }
+*/
 
 // processAndBroadcastEvent prepares and broadcasts an SSE message for a given event.
+// DEPRECATED: Use broadcastRefresh instead.
+/*
 func (h *SSEHandler) processAndBroadcastEvent(event events.Event) {
 	var sseEventName string = "refresh-list" // Use a generic event name
 	var serverID int64                       // Declare without initial assignment
@@ -248,13 +268,21 @@ func (h *SSEHandler) processAndBroadcastEvent(event events.Event) {
 	// Including the server ID in the data is optional but can be useful for debugging on the client.
 	// For simplicity, we can just send an empty data payload.
 	// sseMsg := h.formatSSEMessage(sseEventName, []byte(fmt.Sprintf(`{"serverId": %d}`, serverID)))
-	sseMsg := h.formatSSEMessage(sseEventName, []byte("")) // Empty data payload
+	sseMsg := h.formatSSEMessage(sseEventName, nil)
+	h.broadcast <- sseMsg
+	h.logger.Info().Str("eventName", sseEventName).Msg("Broadcasted SSE message")
+}
+*/
 
-	h.logger.Info().Str("eventName", sseEventName).Msg("Broadcasting SSE refresh trigger")
+// broadcastRefresh formats and sends a generic 'refresh-list' SSE message.
+func (h *SSEHandler) broadcastRefresh() {
+	sseEventName := "refresh-list"
+	sseMsg := h.formatSSEMessage(sseEventName, nil) // No data needed for generic refresh
 	h.broadcast <- sseMsg
+	h.logger.Info().Str("eventName", sseEventName).Msg("Broadcasted SSE refresh message")
 }
 
-// formatSSEMessage formats data into a valid SSE message string.
+// formatSSEMessage constructs a message in SSE format.
 func (h *SSEHandler) formatSSEMessage(eventName string, data []byte) []byte {
 	var msg bytes.Buffer
 	fmt.Fprintf(&msg, "event: %s\n", eventName)