Skip to content

Commit 98c02c2

Browse files
committed
Refactor autostart checks and improve menu item updates
- Replaced string literals for OS checks with constants for better readability and maintainability. - Updated file permissions for plist file writing to enhance security. - Simplified menu item update logic in MenuManager to reduce nesting and improve clarity.
1 parent 56f6863 commit 98c02c2

3 files changed

Lines changed: 38 additions & 24 deletions

File tree

internal/tray/autostart.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func NewAutostartManager() (*AutostartManager, error) {
7474

7575
// IsEnabled checks if autostart is currently enabled
7676
func (m *AutostartManager) IsEnabled() bool {
77-
if runtime.GOOS != "darwin" {
77+
if runtime.GOOS != osDarwin {
7878
return false
7979
}
8080

@@ -90,7 +90,7 @@ func (m *AutostartManager) IsEnabled() bool {
9090

9191
// Enable enables autostart functionality
9292
func (m *AutostartManager) Enable() error {
93-
if runtime.GOOS != "darwin" {
93+
if runtime.GOOS != osDarwin {
9494
return fmt.Errorf("autostart is only supported on macOS")
9595
}
9696

@@ -119,7 +119,7 @@ func (m *AutostartManager) Enable() error {
119119

120120
// Write plist file
121121
plistPath := filepath.Join(launchAgentsDir, "com.smartmcpproxy.mcpproxy.plist")
122-
if err := os.WriteFile(plistPath, []byte(plistContent), 0644); err != nil {
122+
if err := os.WriteFile(plistPath, []byte(plistContent), 0600); err != nil {
123123
return fmt.Errorf("failed to write plist file: %w", err)
124124
}
125125

@@ -137,7 +137,7 @@ func (m *AutostartManager) Enable() error {
137137

138138
// Disable disables autostart functionality
139139
func (m *AutostartManager) Disable() error {
140-
if runtime.GOOS != "darwin" {
140+
if runtime.GOOS != osDarwin {
141141
return fmt.Errorf("autostart is only supported on macOS")
142142
}
143143

internal/tray/managers.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,15 +330,18 @@ func (m *MenuManager) UpdateUpstreamServersMenu(servers []map[string]interface{}
330330
} else {
331331
// No new servers - just update existing items
332332
for _, serverName := range currentServerNames {
333-
if menuItem, exists := m.serverMenuItems[serverName]; exists {
334-
serverData := currentServerMap[serverName]
335-
// Server exists, update its display and ensure it's visible
336-
status, tooltip := m.getServerStatusDisplay(serverData)
337-
menuItem.SetTitle(status)
338-
menuItem.SetTooltip(tooltip)
339-
m.updateServerActionMenus(serverName, serverData) // Update sub-menu items too
340-
menuItem.Show()
333+
menuItem, exists := m.serverMenuItems[serverName]
334+
if !exists {
335+
continue
341336
}
337+
338+
serverData := currentServerMap[serverName]
339+
// Server exists, update its display and ensure it's visible
340+
status, tooltip := m.getServerStatusDisplay(serverData)
341+
menuItem.SetTitle(status)
342+
menuItem.SetTooltip(tooltip)
343+
m.updateServerActionMenus(serverName, serverData) // Update sub-menu items too
344+
menuItem.Show()
342345
}
343346

344347
// Hide servers that are no longer in the config

internal/tray/tray.go

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,17 @@ import (
2929
)
3030

3131
const (
32-
repo = "smart-mcp-proxy/mcpproxy-go" // Actual repository
33-
osDarwin = "darwin"
32+
repo = "smart-mcp-proxy/mcpproxy-go"
33+
checkPeriod = 1 * time.Hour
34+
35+
// OS constants
36+
osWindows = "windows"
37+
osDarwin = "darwin"
38+
osLinux = "linux"
39+
40+
// String constants
41+
strTrue = "true"
42+
strUnknown = "unknown"
3443
)
3544

3645
//go:embed icon-mono-44.png
@@ -503,7 +512,7 @@ func (a *App) updateStatusFromData(statusData interface{}) {
503512
// DON'T clear quarantine menu - quarantine data is persistent storage,
504513
// not runtime connection data. Users should manage quarantined servers
505514
// even when server is stopped.
506-
//a.menuManager.UpdateQuarantineMenu([]map[string]interface{}{})
515+
// a.menuManager.UpdateQuarantineMenu([]map[string]interface{}{})
507516
}
508517
}
509518
}
@@ -695,19 +704,19 @@ func (a *App) onExit() {
695704
// checkForUpdates checks for new releases on GitHub
696705
func (a *App) checkForUpdates() {
697706
// Check if auto-update is disabled by environment variable
698-
if os.Getenv("MCPPROXY_DISABLE_AUTO_UPDATE") == "true" {
707+
if os.Getenv("MCPPROXY_DISABLE_AUTO_UPDATE") == strTrue {
699708
a.logger.Info("Auto-update disabled by environment variable")
700709
return
701710
}
702711

703712
// Disable auto-update for app bundles by default (DMG installation should be manual)
704-
if a.isAppBundle() && os.Getenv("MCPPROXY_UPDATE_APP_BUNDLE") != "true" {
713+
if a.isAppBundle() && os.Getenv("MCPPROXY_UPDATE_APP_BUNDLE") != strTrue {
705714
a.logger.Info("Auto-update disabled for app bundle installations (use DMG for updates)")
706715
return
707716
}
708717

709718
// Check if notification-only mode is enabled
710-
notifyOnly := os.Getenv("MCPPROXY_UPDATE_NOTIFY_ONLY") == "true"
719+
notifyOnly := os.Getenv("MCPPROXY_UPDATE_NOTIFY_ONLY") == strTrue
711720

712721
a.statusItem.SetTitle("Checking for updates...")
713722
defer a.updateStatus() // Restore original status after check
@@ -772,7 +781,7 @@ func (a *App) findAssetURL(release *GitHubRelease) (string, error) {
772781
// Determine file extension based on platform
773782
var extension string
774783
switch runtime.GOOS {
775-
case "windows":
784+
case osWindows:
776785
extension = ".zip"
777786
default: // macOS, Linux
778787
extension = ".tar.gz"
@@ -813,7 +822,7 @@ func (a *App) isHomebrewInstallation() bool {
813822

814823
// isAppBundle checks if running from macOS app bundle
815824
func (a *App) isAppBundle() bool {
816-
if runtime.GOOS != "darwin" {
825+
if runtime.GOOS != osDarwin {
817826
return false
818827
}
819828

@@ -900,7 +909,9 @@ func (a *App) applyTarGzUpdate(body io.Reader) error {
900909
}
901910

902911
// Open the tar.gz file and extract the binary
903-
tmpfile.Seek(0, 0)
912+
if _, err := tmpfile.Seek(0, 0); err != nil {
913+
return fmt.Errorf("failed to seek to beginning of file: %w", err)
914+
}
904915

905916
gzr, err := gzip.NewReader(tmpfile)
906917
if err != nil {
@@ -963,11 +974,11 @@ func (a *App) openLogsDir() {
963974
func (a *App) openDirectory(dirPath, dirType string) {
964975
var cmd *exec.Cmd
965976
switch runtime.GOOS {
966-
case "darwin":
977+
case osDarwin:
967978
cmd = exec.Command("open", dirPath)
968-
case "linux":
979+
case osLinux:
969980
cmd = exec.Command("xdg-open", dirPath)
970-
case "windows":
981+
case osWindows:
971982
cmd = exec.Command("explorer", dirPath)
972983
default:
973984
a.logger.Warn("Unsupported OS for opening directory", zap.String("os", runtime.GOOS))

0 commit comments

Comments
 (0)