fix: address review feedback for server DI

handleKickUser return; MetarQueue.Run; registry sentinel re-exports; compile-time store asserts.

internal/server/conn.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/renorris/openfsd/db"
 	"github.com/renorris/openfsd/internal/auth"
-	"github.com/renorris/openfsd/internal/postoffice"
 	"github.com/renorris/openfsd/internal/session"
 	"github.com/renorris/openfsd/pkg/protocol"
 )
@@ -69,7 +68,7 @@ func (s *Server) handleConn(ctx context.Context, conn net.Conn) {
 
 	// Attempt to register to registry
 	if err = s.registry.Register(client); err != nil {
-		if errors.Is(err, postoffice.ErrCallsignInUse) {
+		if errors.Is(err, ErrCallsignInUse) {
 			sendError(conn, CallsignInUseError, "Callsign already in use")
 		}
 		return

internal/server/deps.go

@@ -7,9 +7,18 @@ import (
 	"time"
 
 	"github.com/renorris/openfsd/db"
+	"github.com/renorris/openfsd/internal/postoffice"
 	"github.com/renorris/openfsd/internal/session"
 )
 
+// Registry sentinel errors — re-exported from postoffice so handlers/conn/HTTP
+// can use errors.Is without importing the concrete registry package.
+// Registry implementations should return these (or errors that wrap them).
+var (
+	ErrCallsignInUse        = postoffice.ErrCallsignInUse
+	ErrCallsignDoesNotExist = postoffice.ErrCallsignDoesNotExist
+)
+
 // UserStore is the consumer-side user repository surface used by login.
 // Signatures match db.UserRepository (no context yet).
 type UserStore interface {
@@ -24,6 +33,7 @@ type ConfigStore interface {
 }
 
 // Registry abstracts the callsign/geo registry (postoffice.PostOffice).
+// Register/Find/Send should use ErrCallsignInUse / ErrCallsignDoesNotExist.
 type Registry interface {
 	Register(s *session.Session) error
 	Release(s *session.Session)
@@ -36,8 +46,11 @@ type Registry interface {
 }
 
 // MetarQueue is the METAR fetch queue (metar.Service).
+// Run starts workers; Request enqueues a fetch. Both are required so injectors
+// cannot silently omit worker startup.
 type MetarQueue interface {
 	Request(ctx context.Context, s session.Sender, icao string)
+	Run(ctx context.Context)
 }
 
 // Clock provides the current time (nil Deps.Clock => real wall clock).

internal/server/deps_test.go

@@ -3,10 +3,12 @@ package server
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/renorris/openfsd/db"
+	"github.com/renorris/openfsd/internal/postoffice"
 	"github.com/renorris/openfsd/internal/session"
 )
 
@@ -37,26 +39,30 @@ func (stubRegistry) Snapshot() []*session.Session                   { return nil
 type stubMetar struct{}
 
 func (stubMetar) Request(ctx context.Context, s session.Sender, icao string) {}
+func (stubMetar) Run(ctx context.Context)                                    {}
 
 type fixedClock struct{ t time.Time }
 
 func (c fixedClock) Now() time.Time { return c.t }
 
+func fullDeps() Deps {
+	return Deps{
+		Config:   &Config{FsdListenAddrs: []string{":0"}},
+		Users:    stubUserStore{},
+		ConfigKV: stubConfigStore{},
+		Registry: stubRegistry{},
+		Metar:    stubMetar{},
+		Clock:    fixedClock{t: time.Unix(1_700_000_000, 0)},
+	}
+}
+
 func TestNewRequiresDeps(t *testing.T) {
 	_, err := New(Deps{})
 	if err == nil {
 		t.Fatal("expected error for empty Deps")
 	}
 
-	cfg := &Config{FsdListenAddrs: []string{":0"}}
-	srv, err := New(Deps{
-		Config:   cfg,
-		Users:    stubUserStore{},
-		ConfigKV: stubConfigStore{},
-		Registry: stubRegistry{},
-		Metar:    stubMetar{},
-		Clock:    fixedClock{t: time.Unix(1_700_000_000, 0)},
-	})
+	srv, err := New(fullDeps())
 	if err != nil {
 		t.Fatalf("New: %v", err)
 	}
@@ -74,14 +80,37 @@ func TestNewRequiresDeps(t *testing.T) {
 	}
 }
 
+func TestNewMissingRequiredFields(t *testing.T) {
+	cases := []struct {
+		name    string
+		mutate  func(*Deps)
+		wantSub string
+	}{
+		{"nil Config", func(d *Deps) { d.Config = nil }, "Config"},
+		{"nil Users", func(d *Deps) { d.Users = nil }, "Users"},
+		{"nil ConfigKV", func(d *Deps) { d.ConfigKV = nil }, "ConfigKV"},
+		{"nil Registry", func(d *Deps) { d.Registry = nil }, "Registry"},
+		{"nil Metar", func(d *Deps) { d.Metar = nil }, "Metar"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			d := fullDeps()
+			tc.mutate(&d)
+			_, err := New(d)
+			if err == nil {
+				t.Fatal("expected error")
+			}
+			if !strings.Contains(err.Error(), tc.wantSub) {
+				t.Fatalf("error %q should mention %q", err.Error(), tc.wantSub)
+			}
+		})
+	}
+}
+
 func TestNewNilClockUsesReal(t *testing.T) {
-	srv, err := New(Deps{
-		Config:   &Config{},
-		Users:    stubUserStore{},
-		ConfigKV: stubConfigStore{},
-		Registry: stubRegistry{},
-		Metar:    stubMetar{},
-	})
+	d := fullDeps()
+	d.Clock = nil
+	srv, err := New(d)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -94,3 +123,16 @@ func TestNewNilClockUsesReal(t *testing.T) {
 		t.Fatalf("real clock skew too large: %v", delta)
 	}
 }
+
+func TestRegistrySentinelsMatchPostoffice(t *testing.T) {
+	// server re-exports must be identical for errors.Is across package boundaries.
+	if !errors.Is(ErrCallsignInUse, postoffice.ErrCallsignInUse) {
+		t.Fatal("ErrCallsignInUse mismatch")
+	}
+	if !errors.Is(ErrCallsignDoesNotExist, postoffice.ErrCallsignDoesNotExist) {
+		t.Fatal("ErrCallsignDoesNotExist mismatch")
+	}
+	if !errors.Is(postoffice.ErrCallsignInUse, ErrCallsignInUse) {
+		t.Fatal("reverse ErrCallsignInUse mismatch")
+	}
+}

internal/server/http_service.go

@@ -11,7 +11,6 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/renorris/openfsd/db"
 	"github.com/renorris/openfsd/internal/auth"
-	"github.com/renorris/openfsd/internal/postoffice"
 )
 
 // runServiceHTTP starts the admin service HTTP server used for
@@ -155,11 +154,12 @@ func (s *Server) handleKickUser(c *gin.Context) {
 	var reqBody RequestBody
 	if err := c.ShouldBindJSON(&reqBody); err != nil {
 		c.AbortWithStatus(http.StatusBadRequest)
+		return
 	}
 
 	client, err := s.registry.Find(reqBody.Callsign)
 	if err != nil {
-		if !errors.Is(err, postoffice.ErrCallsignDoesNotExist) {
+		if !errors.Is(err, ErrCallsignDoesNotExist) {
 			c.AbortWithStatus(http.StatusInternalServerError)
 			return
 		}

internal/server/server.go

@@ -174,10 +174,8 @@ func generateDefaultAdminUser(dbRepo *db.Repositories) (user *db.User, err error
 // Run starts METAR workers, the admin HTTP service, and FSD listeners.
 // It blocks until ctx is cancelled (or a listener fails to start).
 func (s *Server) Run(ctx context.Context) (err error) {
-	// Start metar workers when the concrete service supports Run.
-	if r, ok := s.metar.(interface{ Run(context.Context) }); ok {
-		go r.Run(ctx)
-	}
+	// Start metar worker pool (MetarQueue.Run is required on the interface).
+	go s.metar.Run(ctx)
 
 	// Start HTTP service
 	go s.runServiceHTTP(ctx)
@@ -245,8 +243,14 @@ func (s *Server) listenLoop(ctx context.Context, addr string, errCh chan<- error
 	}
 }
 
-// Compile-time interface satisfaction checks.
+// Compile-time interface satisfaction checks against production implementors.
 var (
-	_ Registry   = (*postoffice.PostOffice)(nil)
-	_ MetarQueue = (*metar.Service)(nil)
+	_ Registry    = (*postoffice.PostOffice)(nil)
+	_ MetarQueue  = (*metar.Service)(nil)
+	_ UserStore   = (*db.SQLiteUserRepository)(nil)
+	_ UserStore   = (*db.PostgresUserRepository)(nil)
+	_ UserStore   = db.UserRepository(nil)
+	_ ConfigStore = (*db.SQLiteConfigRepository)(nil)
+	_ ConfigStore = (*db.PostgresConfigRepository)(nil)
+	_ ConfigStore = db.ConfigRepository(nil)
 )