fix: Improve code readability and documentation across multiple files

hashkode · hashkode · commit d01661402d3e · 2025-04-11T22:44:39.000+02:00
diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml
@@ -42,8 +42,7 @@ jobs:
           if [ -n "$FORMAT_DIFF" ]; then
             echo "Code is not formatted correctly. Run 'make fmt' locally."
             echo "The following files need formatting:"
-            git diff --name-only  
-            echo "The following files are formatted:"
+            git diff --name-only
             exit 1
           fi
 
diff --git a/cmd/agent-browser/main.go b/cmd/agent-browser/main.go
@@ -1,3 +1,5 @@
+// Package main is the entry point for the agent-browser application.
+// It initializes and runs the application using the fx dependency injection framework.
 package main
 
 import (
diff --git a/internal/backend/service.go b/internal/backend/service.go
@@ -11,9 +11,7 @@ import (
 	"github.com/co-browser/agent-browser/internal/log" // Import log
 )
 
-// ... Service interface ...
-
-// service implements the Service interface.
+// Service provides backend operations for managing MCP servers and tools.
 type Service struct {
 	db     database.DBInterface
 	bus    events.Bus
@@ -29,6 +27,8 @@ func NewService(db database.DBInterface, bus events.Bus, logger log.Logger) Serv
 	}
 }
 
+// AddMCPServer adds a new MCP server with the given name and URL.
+// Returns the newly created server or an error if the operation fails.
 func (s *Service) AddMCPServer(name, url string) (*models.MCPServer, error) {
 	if name == "" || url == "" {
 		return nil, fmt.Errorf("server name and URL cannot be empty")
@@ -60,6 +60,8 @@ func (s *Service) AddMCPServer(name, url string) (*models.MCPServer, error) {
 	return newServer, nil
 }
 
+// RemoveMCPServer removes an MCP server with the specified ID.
+// Returns an error if the server doesn't exist or if the operation fails.
 func (s *Service) RemoveMCPServer(id int64) error {
 	server, err := s.db.GetServerByID(id)
 	if err != nil {
@@ -80,6 +82,8 @@ func (s *Service) RemoveMCPServer(id int64) error {
 	return nil
 }
 
+// ListMCPServers returns a list of all MCP servers.
+// Returns an error if the operation fails.
 func (s *Service) ListMCPServers() ([]models.MCPServer, error) {
 	servers, err := s.db.ListServers()
 	if err != nil {
@@ -89,6 +93,8 @@ func (s *Service) ListMCPServers() ([]models.MCPServer, error) {
 	return servers, nil
 }
 
+// GetMCPServer retrieves an MCP server by its ID.
+// Returns the server or an error if it doesn't exist or if the operation fails.
 func (s *Service) GetMCPServer(id int64) (*models.MCPServer, error) {
 	server, err := s.db.GetServerByID(id)
 	if err != nil {
@@ -101,6 +107,8 @@ func (s *Service) GetMCPServer(id int64) (*models.MCPServer, error) {
 	return server, nil
 }
 
+// ProcessFetchedTools processes the tools fetched from an MCP server.
+// Returns the number of added and updated tools, or an error if the operation fails.
 func (s *Service) ProcessFetchedTools(serverID int64, fetchedTools []models.FetchedTool) (added, updated int, err error) {
 	hadError := false
 	for _, ft := range fetchedTools {
@@ -120,6 +128,7 @@ func (s *Service) ProcessFetchedTools(serverID int64, fetchedTools []models.Fetc
 	return 0, 0, nil
 }
 
+// UpdateMCPServerStatus updates the status of an MCP server with the given error (if any).
 func (s *Service) UpdateMCPServerStatus(id int64, checkErr error) {
 	var errStr *string
 	if checkErr != nil {
diff --git a/internal/backend/service_test.go b/internal/backend/service_test.go
@@ -73,8 +73,8 @@ func (m *mockDatabase) GetServerByID(id int64) (*models.MCPServer, error) {
 		return nil, m.getServerError
 	}
 	if s, exists := m.servers[id]; exists {
-		copy := s
-		return &copy, nil
+		serverCopy := s
+		return &serverCopy, nil
 	}
 	return nil, nil
 }
@@ -84,8 +84,8 @@ func (m *mockDatabase) GetServerByURL(url string) (*models.MCPServer, error) {
 	}
 	if id, exists := m.serverURLIndex[url]; exists {
 		if s, serverExists := m.servers[id]; serverExists {
-			copy := s
-			return &copy, nil
+			serverCopy := s
+			return &serverCopy, nil
 		}
 	}
 	return nil, nil
diff --git a/internal/config/exporter.go b/internal/config/exporter.go
@@ -1,3 +1,6 @@
+// Package config provides functionality for generating and managing
+// configuration for agent-browser, including exporting server configuration
+// data for consumption by other services.
 package config
 
 import (
diff --git a/internal/events/bus.go b/internal/events/bus.go
@@ -1,3 +1,5 @@
+// Package events provides an event bus implementation for internal application events.
+// It supports publishing events and subscribing to specific event types.
 package events
 
 import (
diff --git a/internal/events/events.go b/internal/events/events.go
@@ -10,9 +10,12 @@ import (
 type EventType string
 
 const (
-	ServerAdded   EventType = "server.added"
+	// ServerAdded is the event type for when a new server is added
+	ServerAdded EventType = "server.added"
+	// ServerRemoved is the event type for when a server is removed
 	ServerRemoved EventType = "server.removed"
-	ToolsUpdated  EventType = "tools.updated" // Indicates tools for a specific server were updated
+	// ToolsUpdated indicates tools for a specific server were updated
+	ToolsUpdated EventType = "tools.updated"
 	// Add more event types as needed
 )
 
@@ -46,6 +49,7 @@ type ServerAddedEvent struct {
 	Server models.MCPServer
 }
 
+// NewServerAddedEvent creates a new event for when a server is added
 func NewServerAddedEvent(server models.MCPServer) *ServerAddedEvent {
 	return &ServerAddedEvent{
 		baseEvent: newBaseEvent(ServerAdded),
@@ -60,6 +64,7 @@ type ServerRemovedEvent struct {
 	ServerURL string // Include URL for potential listeners that don't have the ID cached
 }
 
+// NewServerRemovedEvent creates a new event for when a server is removed
 func NewServerRemovedEvent(serverID int64, serverURL string) *ServerRemovedEvent {
 	return &ServerRemovedEvent{
 		baseEvent: newBaseEvent(ServerRemoved),
@@ -77,6 +82,7 @@ type ToolsUpdatedEvent struct {
 	// TODO: Optionally include list of changed tool IDs/names?
 }
 
+// NewToolsUpdatedEvent creates a new event for when tools are updated for a server
 func NewToolsUpdatedEvent(serverID int64, fetchedCount int) *ToolsUpdatedEvent {
 	return &ToolsUpdatedEvent{
 		baseEvent:    newBaseEvent(ToolsUpdated),
diff --git a/internal/updater/service.go b/internal/updater/service.go
@@ -146,7 +146,7 @@ func (s *service) performPollCycle(ctx context.Context) {
 }
 
 // pollSingleServer handles fetching and processing tools for one server.
-func (s *service) pollSingleServer(ctx context.Context, server models.MCPServer) {
+func (s *service) pollSingleServer(_ context.Context, server models.MCPServer) {
 	s.logger.Debug().Int64("serverID", server.ID).Str("url", server.URL).Msg("Polling single server")
 
 	fetchedTools, fetchErr := s.mcpClient.FetchTools(server.URL)
diff --git a/internal/web/handlers/api.go b/internal/web/handlers/api.go
@@ -45,7 +45,7 @@ func (h *APIHandlers) RegisterRoutes(mux *http.ServeMux /* or other router type
 	// For now, demonstrating basic registration.
 
 	// Health check endpoint
-	mux.HandleFunc("GET /api/health", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("GET /api/health", func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
 		_, _ = w.Write([]byte(`{"status":"ok"}`))
@@ -117,7 +117,7 @@ func (h *APIHandlers) AddMCPServer(w http.ResponseWriter, r *http.Request) {
 }
 
 // ListMCPServers handles GET /api/mcp/servers
-func (h *APIHandlers) ListMCPServers(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandlers) ListMCPServers(w http.ResponseWriter, _ *http.Request) {
 	servers, err := h.backendService.ListMCPServers()
 	if err != nil {
 		h.logger.Error().Err(err).Msg("Error listing MCP servers via API")
@@ -268,21 +268,21 @@ func (h *APIHandlers) RemoveMCPServer(w http.ResponseWriter, r *http.Request) {
 
 // ListTools handles GET /api/mcp/tools
 func (h *APIHandlers) ListTools(w http.ResponseWriter, r *http.Request) {
-	// Get server ID from query parameter if provided
+	// Parse server ID from query parameter if provided
 	serverIDStr := r.URL.Query().Get("server_id")
 
 	if serverIDStr != "" {
-		// If server ID is provided, get tools for that server
-		serverID, parseErr := strconv.ParseInt(serverIDStr, 10, 64)
-		if parseErr != nil {
+		// List tools for a specific server
+		serverID, err := strconv.ParseInt(serverIDStr, 10, 64)
+		if err != nil {
 			http.Error(w, "Invalid server_id parameter", http.StatusBadRequest)
 			return
 		}
 
 		// Check if server exists
-		server, serveErr := h.backendService.GetMCPServer(serverID)
-		if serveErr != nil {
-			h.logger.Error().Err(serveErr).Int64("serverId", serverID).Msg("Error checking server for tools listing")
+		server, err := h.backendService.GetMCPServer(serverID)
+		if err != nil {
+			h.logger.Error().Err(err).Int64("serverId", serverID).Msg("Error checking server for tools list")
 			http.Error(w, "Failed to check server existence", http.StatusInternalServerError)
 			return
 		}
@@ -296,12 +296,12 @@ func (h *APIHandlers) ListTools(w http.ResponseWriter, r *http.Request) {
 		h.logger.Info().Int64("serverId", serverID).Msg("Listing tools for server is not yet implemented")
 		http.Error(w, "Listing tools by server is not implemented yet", http.StatusNotImplemented)
 		return
-	} else {
-		// Get all tools - not implemented yet
-		h.logger.Info().Msg("Listing all tools is not yet implemented")
-		http.Error(w, "Listing all tools is not implemented yet", http.StatusNotImplemented)
-		return
 	}
+
+	// Get all tools - not implemented yet
+	h.logger.Info().Msg("Listing all tools is not yet implemented")
+	http.Error(w, "Listing all tools is not implemented yet", http.StatusNotImplemented)
+	return
 }
 
 // RediscoverTools handles POST /api/mcp/rediscover-tools
@@ -359,7 +359,7 @@ func (h *APIHandlers) RediscoverTools(w http.ResponseWriter, r *http.Request) {
 }
 
 // HandleConfigExport handles the config export API endpoint
-func (h *APIHandlers) HandleConfigExport(w http.ResponseWriter, r *http.Request) {
+func (h *APIHandlers) HandleConfigExport(w http.ResponseWriter, _ *http.Request) {
 	jsonData, err := h.configExporter.GenerateConfigJSON()
 	if err != nil {
 		h.logger.Error().Err(err).Msg("Failed to generate config JSON")
diff --git a/internal/web/handlers/api_test.go b/internal/web/handlers/api_test.go
@@ -64,8 +64,8 @@ func (m *mockBackendServiceForAPI) AddMCPServer(name, url string) (*models.MCPSe
 	m.servers[id] = s
 	m.serverURLIndex[url] = id
 	// Return a copy for safety
-	copy := s
-	return &copy, nil
+	serverCopy := s
+	return &serverCopy, nil
 }
 
 //nolint:unused
@@ -107,8 +107,8 @@ func (m *mockBackendServiceForAPI) GetMCPServer(id int64) (*models.MCPServer, er
 		return nil, m.getErr
 	}
 	if s, exists := m.servers[id]; exists {
-		copy := s
-		return &copy, nil
+		serverCopy := s
+		return &serverCopy, nil
 	}
 	// Simulate not found for Get (though handler doesn't explicitly use Get)
 	return nil, fmt.Errorf("MCP server with ID %d not found", id)

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+// Package main is the entry point for the agent-browser application.`
	`2`	`+// It initializes and runs the application using the fx dependency injection framework.`
`1`	`3`	`package main`
`2`	`4`
`3`	`5`	`import (`
Original file line number	Diff line number	Diff line change
`@@ -73,8 +73,8 @@ func (m mockDatabase) GetServerByID(id int64) (models.MCPServer, error) {`
`73`	`73`	`return nil, m.getServerError`
`74`	`74`	`}`
`75`	`75`	`if s, exists := m.servers[id]; exists {`
`76`		`- copy := s`
`77`		`- return &copy, nil`
	`76`	`+ serverCopy := s`
	`77`	`+ return &serverCopy, nil`
`78`	`78`	`}`
`79`	`79`	`return nil, nil`
`80`	`80`	`}`
`@@ -84,8 +84,8 @@ func (m mockDatabase) GetServerByURL(url string) (models.MCPServer, error) {`
`84`	`84`	`}`
`85`	`85`	`if id, exists := m.serverURLIndex[url]; exists {`
`86`	`86`	`if s, serverExists := m.servers[id]; serverExists {`
`87`		`- copy := s`
`88`		`- return &copy, nil`
	`87`	`+ serverCopy := s`
	`88`	`+ return &serverCopy, nil`
`89`	`89`	`}`
`90`	`90`	`}`
`91`	`91`	`return nil, nil`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+// Package events provides an event bus implementation for internal application events.`
	`2`	`+// It supports publishing events and subscribing to specific event types.`
`1`	`3`	`package events`
`2`	`4`
`3`	`5`	`import (`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ func (s *service) performPollCycle(ctx context.Context) {`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`// pollSingleServer handles fetching and processing tools for one server.`
`149`		`-func (s *service) pollSingleServer(ctx context.Context, server models.MCPServer) {`
	`149`	`+func (s *service) pollSingleServer(_ context.Context, server models.MCPServer) {`
`150`	`150`	`s.logger.Debug().Int64("serverID", server.ID).Str("url", server.URL).Msg("Polling single server")`
`151`	`151`
`152`	`152`	`fetchedTools, fetchErr := s.mcpClient.FetchTools(server.URL)`