feat(helper): start gRPC server when API.EnableGRPC is true (#21)

kevinelliott · claude · web-flow · commit 42350adc534b · 2026-04-21T02:30:46.000-07:00
The systray helper previously only started the REST API (when
API.EnableREST was set); the matching API.EnableGRPC config flag was
silently ignored. Mirror the REST startup pattern so operators who
enable gRPC actually get a listener.

Default remains OFF (no behavior change for existing users). Port
defaults to 50051. REST and gRPC can run concurrently.

- internal/systray: new grpcServer field, startGRPCServer(), Run()
  branch, and bounded-GracefulStop-with-ForceStop-fallback on exit.
- pkg/api/grpc: add ForceStop() for the hard-stop fallback path used
  during helper shutdown when a streaming RPC wedges.
- test: assert startGRPCServer populates App.grpcServer and binds.

Co-authored-by: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Systray helper now starts the gRPC API server when `API.EnableGRPC=true`
+  (default off, matching REST).
+
 ### Removed
 
 - `config.CatalogConfig.RefreshOnStart` field removed. Deprecated in
diff --git a/internal/systray/systray.go b/internal/systray/systray.go
@@ -17,6 +17,7 @@ import (
 	"github.com/kevinelliott/agentmanager/internal/orchestrator"
 	"github.com/kevinelliott/agentmanager/internal/versionfetch"
 	"github.com/kevinelliott/agentmanager/pkg/agent"
+	grpcapi "github.com/kevinelliott/agentmanager/pkg/api/grpc"
 	"github.com/kevinelliott/agentmanager/pkg/api/rest"
 	"github.com/kevinelliott/agentmanager/pkg/catalog"
 	"github.com/kevinelliott/agentmanager/pkg/config"
@@ -56,6 +57,9 @@ type App struct {
 	// REST API server (optional)
 	restServer *rest.Server
 
+	// gRPC API server (optional)
+	grpcServer *grpcapi.Server
+
 	// State
 	agents      []agent.Installation
 	agentsMu    sync.RWMutex
@@ -135,6 +139,13 @@ func (a *App) Run() error {
 		}
 	}
 
+	// Start gRPC API server if enabled
+	if a.config.API.EnableGRPC {
+		if err := a.startGRPCServer(); err != nil {
+			return fmt.Errorf("failed to start gRPC server: %w", err)
+		}
+	}
+
 	// Run systray (blocks until quit)
 	// On macOS, this must run on the main thread
 	systray.Run(a.onReady, a.onExit)
@@ -158,6 +169,18 @@ func (a *App) startRESTServer() error {
 	})
 }
 
+// startGRPCServer starts the gRPC API server.
+func (a *App) startGRPCServer() error {
+	a.grpcServer = grpcapi.NewServer(
+		a.config, a.platform, a.store, a.detector, a.catalog, a.installer,
+		grpcapi.WithVersion(a.version),
+		grpcapi.WithStartTime(a.startTime),
+	)
+	return a.grpcServer.Start(a.ctx, grpcapi.ServerConfig{
+		Address: fmt.Sprintf(":%d", a.config.API.GRPCPort),
+	})
+}
+
 // startIPCServer starts the IPC server for CLI communication.
 func (a *App) startIPCServer() error {
 	a.ipcServer = ipc.NewServer("")
@@ -392,6 +415,24 @@ func (a *App) onExit() {
 		a.restServer.Stop(ctx)
 	}
 
+	// Stop gRPC server: prefer GracefulStop with a short timeout; fall back
+	// to a hard Stop if graceful shutdown does not complete in time so a
+	// wedged in-flight RPC cannot block helper exit.
+	if a.grpcServer != nil {
+		graceCtx, graceCancel := context.WithTimeout(context.Background(), 2*time.Second)
+		gracefulDone := make(chan struct{})
+		go func() {
+			_ = a.grpcServer.Stop(graceCtx)
+			close(gracefulDone)
+		}()
+		select {
+		case <-gracefulDone:
+		case <-graceCtx.Done():
+			a.grpcServer.ForceStop()
+		}
+		graceCancel()
+	}
+
 	// Stop IPC server (drains in-flight handlers before returning).
 	if a.ipcServer != nil {
 		a.ipcServer.Stop(ctx)
diff --git a/internal/systray/systray_test.go b/internal/systray/systray_test.go
@@ -2,7 +2,11 @@ package systray
 
 import (
 	"bytes"
+	"context"
 	"testing"
+	"time"
+
+	"github.com/kevinelliott/agentmanager/pkg/config"
 )
 
 func TestGetIcon(t *testing.T) {
@@ -35,3 +39,50 @@ func TestGetIconConsistency(t *testing.T) {
 		t.Error("getIcon() should return consistent results")
 	}
 }
+
+// TestStartGRPCServerPopulatesField verifies that startGRPCServer wires up
+// the gRPC server on App when invoked (which is what Run() does when
+// API.EnableGRPC is true). Uses port :0 to avoid binding to a fixed port.
+// Also confirms that the server is nil if startGRPCServer is never called,
+// matching the EnableGRPC=false default path.
+func TestStartGRPCServerPopulatesField(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	a := &App{
+		config: &config.Config{
+			API: config.APIConfig{
+				EnableGRPC: true,
+				GRPCPort:   0, // bind ephemeral port for test isolation
+			},
+		},
+		ctx:       ctx,
+		startTime: time.Now(),
+		version:   "test",
+	}
+
+	// Default path: server should be nil when not started.
+	if a.grpcServer != nil {
+		t.Fatal("grpcServer should be nil before startGRPCServer() runs")
+	}
+
+	if err := a.startGRPCServer(); err != nil {
+		t.Fatalf("startGRPCServer() error = %v", err)
+	}
+	defer func() {
+		if a.grpcServer != nil {
+			_ = a.grpcServer.Stop(context.Background())
+		}
+	}()
+
+	if a.grpcServer == nil {
+		t.Fatal("startGRPCServer() did not populate App.grpcServer")
+	}
+
+	// Give the goroutine inside Start() a moment to register the listener,
+	// then verify it has a bound address (proves Serve is actually running).
+	time.Sleep(20 * time.Millisecond)
+	if addr := a.grpcServer.Address(); addr == "" {
+		t.Error("grpc server Address() should be non-empty after Start()")
+	}
+}
diff --git a/pkg/api/grpc/server.go b/pkg/api/grpc/server.go
@@ -197,6 +197,18 @@ func (s *Server) Stop(ctx context.Context) error {
 	return nil
 }
 
+// ForceStop immediately stops the gRPC server without waiting for in-flight
+// RPCs to complete. Intended as a fallback when a bounded GracefulStop does
+// not return in time (e.g. a wedged streaming handler during helper exit).
+func (s *Server) ForceStop() {
+	if s.grpcServer != nil {
+		s.grpcServer.Stop()
+	}
+	if s.listener != nil {
+		s.listener.Close()
+	}
+}
+
 // Address returns the server's listening address.
 func (s *Server) Address() string {
 	if s.listener != nil {
