Conversation
Introduces a cross-platform system tray application for the agent, accessible via the new 'tray' command. Implements a local IPC HTTP server for tray/agent communication, exposes agent status, metrics, logs, and restart functionality. Updates build scripts, configuration, and documentation to support the tray app and IPC integration.
There was a problem hiding this comment.
Pull request overview
Adds a cross-platform system tray UI and a local IPC HTTP API to let the tray communicate with the agent, plus packaging/build updates to ship and auto-start the tray on Windows.
Changes:
- Introduces a
trayCLI command with a systray-based UI for status + basic controls. - Adds a localhost IPC HTTP server exposing status/metrics/logs and a restart endpoint.
- Updates packaging/build/release tooling and defaults to enable IPC and start the tray on Windows login.
Reviewed changes
Copilot reviewed 15 out of 22 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| agent/packaging/msi/build.ps1 | Adds IPC config defaults to MSI-generated config. |
| agent/packaging/msi/Product.wxs | Adds HKCU Run key component to auto-start tray after login. |
| agent/internal/tray/tray.go | Implements systray UI, polling IPC, and service control helpers. |
| agent/internal/tray/client.go | Adds IPC HTTP client used by the tray app. |
| agent/internal/tray/icons.go | Embeds tray icon assets and selects by state. |
| agent/internal/tray/icons/connected.ico | Adds embedded icon asset. |
| agent/internal/tray/icons/disconnected.ico | Adds embedded icon asset. |
| agent/internal/tray/icons/error.ico | Adds embedded icon asset. |
| agent/internal/tray/icons/stopped.ico | Adds embedded icon asset. |
| agent/internal/ipc/server.go | Introduces IPC HTTP server with CORS middleware and route wiring. |
| agent/internal/ipc/handlers.go | Implements IPC endpoints for status/metrics/connection/logs/restart/health. |
| agent/internal/config/config.go | Adds IPC config struct + defaults. |
| agent/internal/agent/agent.go | Wires IPC server into agent lifecycle + exposes IPC provider methods. |
| agent/cmd/agent/main.go | Adds tray command and dashboard URL derivation helper. |
| agent/go.mod | Adds fyne.io/systray dependency. |
| agent/go.sum | Adds sums for systray and transitive deps. |
| agent/Makefile | Extends ldflags to set agent package version var. |
| .github/workflows/agent-release.yml | Updates release build ldflags and release notes mentioning tray. |
| .gitignore | Expands ignore patterns for build outputs, env files, IDEs, etc. |
| VERSION | Bumps version to 1.2.62. |
| EXPLORATIONS_AND_FUTURE.md | Adds a large roadmap/notes document (not tray/IPC-specific). |
Comments suppressed due to low confidence (1)
agent/internal/agent/agent.go:209
- When a restart is requested, Agent.Run exits the select and then returns ctx.Err(). Since the context is not canceled on restart, ctx.Err() is nil, so the process exits cleanly rather than actually restarting. If the service manager is not configured to restart on clean exit, /restart will just stop the agent. Return a distinct restart signal/error and have the caller (runAgent/service wrapper) re-exec or loop, or implement an explicit restart strategy.
// Wait for context cancellation or restart request
select {
case <-ctx.Done():
case <-a.restartCh:
a.log.Info("Restart requested")
}
// Cleanup
a.cleanup()
return ctx.Err()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Register handlers | ||
| handlers := NewHandlers(s.provider, s.log) | ||
| mux.HandleFunc("/status", handlers.HandleStatus) | ||
| mux.HandleFunc("/metrics", handlers.HandleMetrics) | ||
| mux.HandleFunc("/connection", handlers.HandleConnection) | ||
| mux.HandleFunc("/logs", handlers.HandleLogs) | ||
| mux.HandleFunc("/restart", handlers.HandleRestart) | ||
| mux.HandleFunc("/health", handlers.HandleHealth) |
There was a problem hiding this comment.
The IPC API is exposing sensitive operations (e.g., /logs and /restart) but there is no authentication/authorization layer applied to any route. Binding to localhost is not sufficient to prevent other local users/processes or browser-based localhost attacks from invoking these endpoints. Add an authentication mechanism (e.g., a random per-install token stored with restrictive permissions and required via Authorization header) or switch to an OS-permissioned transport (Unix domain socket / Windows named pipe).
| AutoInstall: false, // Require manual confirmation by default | ||
| }, | ||
| IPC: IPCConfig{ | ||
| Enabled: true, |
There was a problem hiding this comment.
IPC is enabled by default. Given the IPC server exposes logs and restart functionality, enabling it by default increases the attack surface on every agent install. Consider defaulting IPC.Enabled to false (and enabling it explicitly for tray installs) or gating it behind a required auth token to make the default configuration safe.
| Enabled: true, | |
| Enabled: false, |
| file, err := os.Open(logFile) | ||
| if err != nil { | ||
| return []string{} | ||
| } | ||
| defer file.Close() | ||
|
|
||
| // Read all lines and keep the last N | ||
| var allLines []string | ||
| scanner := bufio.NewScanner(file) | ||
| for scanner.Scan() { | ||
| allLines = append(allLines, scanner.Text()) | ||
| } |
There was a problem hiding this comment.
GetRecentLogs reads the entire log file into memory on every request and ignores scanner.Err(). This can become very slow/high-memory for large logs and can silently truncate/stop on long lines (bufio.Scanner token limit). Consider implementing a tail-style reader that only reads the last N lines (bounded memory), and handle scanner.Err()/increase buffer size if you keep Scanner.
| case <-a.menuDashboard.ClickedCh: | ||
| a.openDashboard() | ||
| case <-a.menuQuit.ClickedCh: | ||
| systray.Quit() |
There was a problem hiding this comment.
The Quit menu handler calls systray.Quit() directly and returns without closing quitCh. That leaves refreshLoop/handleMenuClicks relying on process teardown rather than clean shutdown. Prefer calling a.Quit() here so quitCh is closed and background goroutines exit deterministically.
| systray.Quit() | |
| a.Quit() |
| func startService() error { | ||
| if runtime.GOOS == "windows" { | ||
| return exec.Command("sc", "start", "ServerKitAgent").Run() | ||
| } | ||
| return exec.Command("systemctl", "start", "serverkit-agent").Run() | ||
| } |
There was a problem hiding this comment.
startService/stopService/restartService assume systemctl for all non-Windows platforms. On darwin this will fail (no systemctl). Add a darwin-specific implementation (e.g., launchctl) or return a clear error stating service control is unsupported on that platform.
| # ServerKit: Explorations and Future Directions | ||
|
|
||
| > A deep dive into where ServerKit stands, where it should go, and practical next steps. | ||
| > | ||
| > *Written: January 2026* | ||
|
|
||
| --- | ||
|
|
||
| ## Table of Contents | ||
|
|
There was a problem hiding this comment.
This new document appears unrelated to the PR’s stated goal of adding the tray app + IPC server (it contains a broad product roadmap and WordPress environment discussion, and does not mention tray/IPC). Consider moving it to a separate PR to keep review scope and history focused.
| // corsMiddleware adds CORS headers for local development | ||
| func corsMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| // Only allow requests from localhost | ||
| origin := r.Header.Get("Origin") | ||
| if origin == "" || isLocalhost(origin) { | ||
| w.Header().Set("Access-Control-Allow-Origin", origin) | ||
| w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS") | ||
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type") | ||
| } | ||
|
|
||
| if r.Method == "OPTIONS" { | ||
| w.WriteHeader(http.StatusOK) | ||
| return | ||
| } | ||
|
|
||
| next.ServeHTTP(w, r) | ||
| }) |
There was a problem hiding this comment.
corsMiddleware currently only adds CORS headers for localhost origins but still serves requests from any Origin. A malicious website can POST to http://127.0.0.1:/restart from the browser (CORS blocks reading the response, not sending the request). Reject requests with a non-local Origin (e.g., 403) and avoid setting Access-Control-Allow-Origin to an empty string when Origin is missing.
| // GetConnectionInfo returns WebSocket connection information for the IPC API | ||
| func (a *Agent) GetConnectionInfo() ipc.ConnectionInfo { | ||
| info := ipc.ConnectionInfo{ | ||
| Connected: a.ws.IsConnected(), | ||
| ServerURL: a.cfg.Server.URL, | ||
| ReconnectCount: a.reconnectCount, | ||
| } | ||
|
|
||
| if !a.lastConnected.IsZero() { | ||
| info.LastConnected = a.lastConnected.UnixMilli() | ||
| } | ||
|
|
||
| if session := a.ws.Session(); session != nil { | ||
| info.SessionExpires = session.ExpiresAt.UnixMilli() | ||
| } | ||
|
|
||
| return info |
There was a problem hiding this comment.
GetConnectionInfo reports lastConnected and reconnectCount, but these fields are never updated anywhere in Agent. As implemented, reconnectCount will always be 0 and lastConnected will always be omitted, which makes the IPC connection endpoint misleading. Either wire these values from the ws.Client (add exported getters/state) or update Agent on connect/disconnect events.
| // Quit requests the tray app to quit | ||
| func (a *App) Quit() { | ||
| close(a.quitCh) | ||
| systray.Quit() | ||
| } |
There was a problem hiding this comment.
Quit closes quitCh directly; if Quit is called more than once (e.g., signal handler + systray onExit path), this will panic with "close of closed channel". Guard the close with sync.Once or a non-blocking pattern, and consider closing quitCh in onExit so all goroutines reliably terminate.
| <!-- Auto-start tray on user login (uses main binary with tray command) --> | ||
| <Component Id="TrayAutoStart" Directory="INSTALLFOLDER" Guid="E5F6A7B8-C9D0-1234-EF01-567890123456"> | ||
| <RegistryValue | ||
| Root="HKCU" | ||
| Key="Software\Microsoft\Windows\CurrentVersion\Run" | ||
| Name="ServerKitTray" | ||
| Type="string" | ||
| Value=""[INSTALLFOLDER]serverkit-agent.exe" tray" | ||
| KeyPath="yes" /> |
There was a problem hiding this comment.
This MSI is Scope="perMachine" but the tray autostart is written to HKCU...\Run, which only applies to the user who performed the installation. If the intent is to start the tray for all users who log in, consider using HKLM...\Run, Active Setup, or an advertised shortcut in the Startup folder.
Introduces a ThemeContext for managing theme state and switching between dark, light, and system themes. Adds CSS custom properties for theme variables, updates LESS files to use theme-sensitive variables, and refactors frontend to support runtime theme changes. Also updates .env.example files to include SERVERKIT_GITHUB_REPO.
Refactored Sidebar.jsx to update markup for a cleaner, more professional look, including new class names, improved navigation structure, and a revised footer with a GitHub promo button. Updated _sidebar.less to match the new layout, enhance navigation and user profile styling, and improve overall sidebar appearance and UX.
Changed the default value of SERVERKIT_GITHUB_REPO from 'serverkit/serverkit' to 'jhd3197/ServerKit' to point to the new repository for agent release checks.
Updated secondary and tertiary text colors for improved contrast and consistency. Introduced a CSS variable for grid color and applied it to the main content background. Removed the unused density settings from the appearance settings page. Enhanced color usage in the downloads page for better readability. Sidebar promo tag now uses a star icon.
Introduces a cross-platform system tray application for the agent, accessible via the new 'tray' command. Implements a local IPC HTTP server for tray/agent communication, exposes agent status, metrics, logs, and restart functionality. Updates build scripts, configuration, and documentation to support the tray app and IPC integration.