Skip to content

Commit d016614

Browse files
committed
fix: Improve code readability and documentation across multiple files
1 parent 20cfcfd commit d016614

File tree

10 files changed

+52
-31
lines changed

10 files changed

+52
-31
lines changed

.github/workflows/go.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ jobs:
4242
if [ -n "$FORMAT_DIFF" ]; then
4343
echo "Code is not formatted correctly. Run 'make fmt' locally."
4444
echo "The following files need formatting:"
45-
git diff --name-only
46-
echo "The following files are formatted:"
45+
git diff --name-only
4746
exit 1
4847
fi
4948

cmd/agent-browser/main.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff 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.
13
package main
24

35
import (

internal/backend/service.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ import (
1111
"github.com/co-browser/agent-browser/internal/log" // Import log
1212
)
1313

14-
// ... Service interface ...
15-
16-
// service implements the Service interface.
14+
// Service provides backend operations for managing MCP servers and tools.
1715
type Service struct {
1816
db database.DBInterface
1917
bus events.Bus
@@ -29,6 +27,8 @@ func NewService(db database.DBInterface, bus events.Bus, logger log.Logger) Serv
2927
}
3028
}
3129

30+
// AddMCPServer adds a new MCP server with the given name and URL.
31+
// Returns the newly created server or an error if the operation fails.
3232
func (s *Service) AddMCPServer(name, url string) (*models.MCPServer, error) {
3333
if name == "" || url == "" {
3434
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) {
6060
return newServer, nil
6161
}
6262

63+
// RemoveMCPServer removes an MCP server with the specified ID.
64+
// Returns an error if the server doesn't exist or if the operation fails.
6365
func (s *Service) RemoveMCPServer(id int64) error {
6466
server, err := s.db.GetServerByID(id)
6567
if err != nil {
@@ -80,6 +82,8 @@ func (s *Service) RemoveMCPServer(id int64) error {
8082
return nil
8183
}
8284

85+
// ListMCPServers returns a list of all MCP servers.
86+
// Returns an error if the operation fails.
8387
func (s *Service) ListMCPServers() ([]models.MCPServer, error) {
8488
servers, err := s.db.ListServers()
8589
if err != nil {
@@ -89,6 +93,8 @@ func (s *Service) ListMCPServers() ([]models.MCPServer, error) {
8993
return servers, nil
9094
}
9195

96+
// GetMCPServer retrieves an MCP server by its ID.
97+
// Returns the server or an error if it doesn't exist or if the operation fails.
9298
func (s *Service) GetMCPServer(id int64) (*models.MCPServer, error) {
9399
server, err := s.db.GetServerByID(id)
94100
if err != nil {
@@ -101,6 +107,8 @@ func (s *Service) GetMCPServer(id int64) (*models.MCPServer, error) {
101107
return server, nil
102108
}
103109

110+
// ProcessFetchedTools processes the tools fetched from an MCP server.
111+
// Returns the number of added and updated tools, or an error if the operation fails.
104112
func (s *Service) ProcessFetchedTools(serverID int64, fetchedTools []models.FetchedTool) (added, updated int, err error) {
105113
hadError := false
106114
for _, ft := range fetchedTools {
@@ -120,6 +128,7 @@ func (s *Service) ProcessFetchedTools(serverID int64, fetchedTools []models.Fetc
120128
return 0, 0, nil
121129
}
122130

131+
// UpdateMCPServerStatus updates the status of an MCP server with the given error (if any).
123132
func (s *Service) UpdateMCPServerStatus(id int64, checkErr error) {
124133
var errStr *string
125134
if checkErr != nil {

internal/backend/service_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ func (m *mockDatabase) GetServerByID(id int64) (*models.MCPServer, error) {
7373
return nil, m.getServerError
7474
}
7575
if s, exists := m.servers[id]; exists {
76-
copy := s
77-
return &copy, nil
76+
serverCopy := s
77+
return &serverCopy, nil
7878
}
7979
return nil, nil
8080
}
@@ -84,8 +84,8 @@ func (m *mockDatabase) GetServerByURL(url string) (*models.MCPServer, error) {
8484
}
8585
if id, exists := m.serverURLIndex[url]; exists {
8686
if s, serverExists := m.servers[id]; serverExists {
87-
copy := s
88-
return &copy, nil
87+
serverCopy := s
88+
return &serverCopy, nil
8989
}
9090
}
9191
return nil, nil

internal/config/exporter.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Package config provides functionality for generating and managing
2+
// configuration for agent-browser, including exporting server configuration
3+
// data for consumption by other services.
14
package config
25

36
import (

internal/events/bus.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff 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.
13
package events
24

35
import (

internal/events/events.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ import (
1010
type EventType string
1111

1212
const (
13-
ServerAdded EventType = "server.added"
13+
// ServerAdded is the event type for when a new server is added
14+
ServerAdded EventType = "server.added"
15+
// ServerRemoved is the event type for when a server is removed
1416
ServerRemoved EventType = "server.removed"
15-
ToolsUpdated EventType = "tools.updated" // Indicates tools for a specific server were updated
17+
// ToolsUpdated indicates tools for a specific server were updated
18+
ToolsUpdated EventType = "tools.updated"
1619
// Add more event types as needed
1720
)
1821

@@ -46,6 +49,7 @@ type ServerAddedEvent struct {
4649
Server models.MCPServer
4750
}
4851

52+
// NewServerAddedEvent creates a new event for when a server is added
4953
func NewServerAddedEvent(server models.MCPServer) *ServerAddedEvent {
5054
return &ServerAddedEvent{
5155
baseEvent: newBaseEvent(ServerAdded),
@@ -60,6 +64,7 @@ type ServerRemovedEvent struct {
6064
ServerURL string // Include URL for potential listeners that don't have the ID cached
6165
}
6266

67+
// NewServerRemovedEvent creates a new event for when a server is removed
6368
func NewServerRemovedEvent(serverID int64, serverURL string) *ServerRemovedEvent {
6469
return &ServerRemovedEvent{
6570
baseEvent: newBaseEvent(ServerRemoved),
@@ -77,6 +82,7 @@ type ToolsUpdatedEvent struct {
7782
// TODO: Optionally include list of changed tool IDs/names?
7883
}
7984

85+
// NewToolsUpdatedEvent creates a new event for when tools are updated for a server
8086
func NewToolsUpdatedEvent(serverID int64, fetchedCount int) *ToolsUpdatedEvent {
8187
return &ToolsUpdatedEvent{
8288
baseEvent: newBaseEvent(ToolsUpdated),

internal/updater/service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func (s *service) performPollCycle(ctx context.Context) {
146146
}
147147

148148
// 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) {
150150
s.logger.Debug().Int64("serverID", server.ID).Str("url", server.URL).Msg("Polling single server")
151151

152152
fetchedTools, fetchErr := s.mcpClient.FetchTools(server.URL)

internal/web/handlers/api.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (h *APIHandlers) RegisterRoutes(mux *http.ServeMux /* or other router type
4545
// For now, demonstrating basic registration.
4646

4747
// Health check endpoint
48-
mux.HandleFunc("GET /api/health", func(w http.ResponseWriter, r *http.Request) {
48+
mux.HandleFunc("GET /api/health", func(w http.ResponseWriter, _ *http.Request) {
4949
w.Header().Set("Content-Type", "application/json")
5050
w.WriteHeader(http.StatusOK)
5151
_, _ = w.Write([]byte(`{"status":"ok"}`))
@@ -117,7 +117,7 @@ func (h *APIHandlers) AddMCPServer(w http.ResponseWriter, r *http.Request) {
117117
}
118118

119119
// ListMCPServers handles GET /api/mcp/servers
120-
func (h *APIHandlers) ListMCPServers(w http.ResponseWriter, r *http.Request) {
120+
func (h *APIHandlers) ListMCPServers(w http.ResponseWriter, _ *http.Request) {
121121
servers, err := h.backendService.ListMCPServers()
122122
if err != nil {
123123
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) {
268268

269269
// ListTools handles GET /api/mcp/tools
270270
func (h *APIHandlers) ListTools(w http.ResponseWriter, r *http.Request) {
271-
// Get server ID from query parameter if provided
271+
// Parse server ID from query parameter if provided
272272
serverIDStr := r.URL.Query().Get("server_id")
273273

274274
if serverIDStr != "" {
275-
// If server ID is provided, get tools for that server
276-
serverID, parseErr := strconv.ParseInt(serverIDStr, 10, 64)
277-
if parseErr != nil {
275+
// List tools for a specific server
276+
serverID, err := strconv.ParseInt(serverIDStr, 10, 64)
277+
if err != nil {
278278
http.Error(w, "Invalid server_id parameter", http.StatusBadRequest)
279279
return
280280
}
281281

282282
// Check if server exists
283-
server, serveErr := h.backendService.GetMCPServer(serverID)
284-
if serveErr != nil {
285-
h.logger.Error().Err(serveErr).Int64("serverId", serverID).Msg("Error checking server for tools listing")
283+
server, err := h.backendService.GetMCPServer(serverID)
284+
if err != nil {
285+
h.logger.Error().Err(err).Int64("serverId", serverID).Msg("Error checking server for tools list")
286286
http.Error(w, "Failed to check server existence", http.StatusInternalServerError)
287287
return
288288
}
@@ -296,12 +296,12 @@ func (h *APIHandlers) ListTools(w http.ResponseWriter, r *http.Request) {
296296
h.logger.Info().Int64("serverId", serverID).Msg("Listing tools for server is not yet implemented")
297297
http.Error(w, "Listing tools by server is not implemented yet", http.StatusNotImplemented)
298298
return
299-
} else {
300-
// Get all tools - not implemented yet
301-
h.logger.Info().Msg("Listing all tools is not yet implemented")
302-
http.Error(w, "Listing all tools is not implemented yet", http.StatusNotImplemented)
303-
return
304299
}
300+
301+
// Get all tools - not implemented yet
302+
h.logger.Info().Msg("Listing all tools is not yet implemented")
303+
http.Error(w, "Listing all tools is not implemented yet", http.StatusNotImplemented)
304+
return
305305
}
306306

307307
// RediscoverTools handles POST /api/mcp/rediscover-tools
@@ -359,7 +359,7 @@ func (h *APIHandlers) RediscoverTools(w http.ResponseWriter, r *http.Request) {
359359
}
360360

361361
// HandleConfigExport handles the config export API endpoint
362-
func (h *APIHandlers) HandleConfigExport(w http.ResponseWriter, r *http.Request) {
362+
func (h *APIHandlers) HandleConfigExport(w http.ResponseWriter, _ *http.Request) {
363363
jsonData, err := h.configExporter.GenerateConfigJSON()
364364
if err != nil {
365365
h.logger.Error().Err(err).Msg("Failed to generate config JSON")

internal/web/handlers/api_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ func (m *mockBackendServiceForAPI) AddMCPServer(name, url string) (*models.MCPSe
6464
m.servers[id] = s
6565
m.serverURLIndex[url] = id
6666
// Return a copy for safety
67-
copy := s
68-
return &copy, nil
67+
serverCopy := s
68+
return &serverCopy, nil
6969
}
7070

7171
//nolint:unused
@@ -107,8 +107,8 @@ func (m *mockBackendServiceForAPI) GetMCPServer(id int64) (*models.MCPServer, er
107107
return nil, m.getErr
108108
}
109109
if s, exists := m.servers[id]; exists {
110-
copy := s
111-
return &copy, nil
110+
serverCopy := s
111+
return &serverCopy, nil
112112
}
113113
// Simulate not found for Get (though handler doesn't explicitly use Get)
114114
return nil, fmt.Errorf("MCP server with ID %d not found", id)

0 commit comments

Comments
 (0)