diff --git a/Server/api/absence_contract_test.go b/Server/api/absence_contract_test.go index 2272524c..0dd6da37 100644 --- a/Server/api/absence_contract_test.go +++ b/Server/api/absence_contract_test.go @@ -14,6 +14,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" "github.com/go-chi/chi/v5" ) @@ -55,7 +56,8 @@ func fullRouter(t *testing.T) http.Handler { GIF: config.GIFConfig{APIKey: "absence-test"}, } - handler, _, cleanup := api.NewRouter(cfg, database, "test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "test", nil, nil, rt) t.Cleanup(cleanup) return handler } diff --git a/Server/api/diagnostics_handler_test.go b/Server/api/diagnostics_handler_test.go index 7304c31a..454b306d 100644 --- a/Server/api/diagnostics_handler_test.go +++ b/Server/api/diagnostics_handler_test.go @@ -11,6 +11,7 @@ import ( "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" "github.com/J3vb/OwnCord/Server/permissions" ) @@ -35,7 +36,8 @@ func setupDiagnosticsRouter(t *testing.T) (http.Handler, string, *db.DB) { }, } - handler, _, cleanup := api.NewRouter(cfg, database, "1.0.0-test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "1.0.0-test", nil, nil, rt) t.Cleanup(cleanup) // Create a user and session for authenticated requests. @@ -106,7 +108,8 @@ func TestDiagnosticsConnectivity_HonoursTrustedProxies(t *testing.T) { }, } - handler, _, cleanup := api.NewRouter(cfg, database, "1.0.0-test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "1.0.0-test", nil, nil, rt) t.Cleanup(cleanup) uid, _ := database.CreateUser(context.Background(), "diagproxyuser", "$2a$12$fake", 1) diff --git a/Server/api/router.go b/Server/api/router.go index b41f8c37..7ac08ed2 100644 --- a/Server/api/router.go +++ b/Server/api/router.go @@ -9,7 +9,6 @@ import ( "fmt" "log/slog" "net/http" - "net/url" "slices" "time" @@ -31,13 +30,37 @@ import ( "github.com/go-chi/chi/v5/middleware" ) -// NewRouter builds and returns the fully configured HTTP handler, the -// WebSocket hub (so the caller can call hub.GracefulStop on shutdown), and a -// cleanup function that stops background goroutines (e.g. rate-limiter cleanup). +// Runtime holds the process-level collaborators NewRouter mounts its routes +// over. Until B3-3 NewRouter built all of them itself and returned the hub, +// while main.go set the hub's event persister and event store after it +// returned — two owners of one hub. internal/app builds them now +// (app.StartRuntime), applies every pre-Run setter from that one place, and +// hands the result in here; B3-4 turns the required setters into validated +// constructor options at the same single call site. +type Runtime struct { + // Hub is already wired and running: StartRuntime starts its dispatch + // goroutine after the last pre-Run setter, exactly where NewRouter used + // to. Stopping it is the caller's job (App.Close's "hub" step). + Hub *ws.Hub + // Limiter backs both the hub and every rate-limited route. One instance: + // it persists auth lockouts, so a second copy would split that state. + Limiter *auth.RateLimiter + // Services is the shared service layer — the same instance the hub holds, + // so the permission cache the hub invalidates is the one the handlers read. + Services *service.Services + // VoiceEnabled is whether StartRuntime's LiveKit client was built. The + // webhook, LiveKit health and signalling-proxy routes are mounted only + // then — the `lkErr == nil` guard that used to live in this package. + VoiceEnabled bool +} + +// NewRouter builds and returns the fully configured HTTP handler and a +// cleanup function that stops background goroutines (e.g. rate-limiter +// cleanup). // // pluginRegistry may be nil — in that case the plugin admin endpoints respond // with 503 on lifecycle calls and an empty list on read. -func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.RingBuffer, pluginRegistry *plugin.Registry) (http.Handler, *ws.Hub, func()) { +func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.RingBuffer, pluginRegistry *plugin.Registry, rt Runtime) (http.Handler, func()) { // Install the auth rate multiplier before any route mounts read it. setAuthRateScale(cfg.Security.AuthRateLimitMultiplier) @@ -60,9 +83,10 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri healthHandler := handleHealth(routerHealthDeps(cfg, database, &getOnlineUsers, &hubAlive)) r.Get("/health", healthHandler) - // Shared rate limiter for auth endpoints. Lockouts are persisted to the - // database so they survive server restarts (M2 security hardening). - limiter := auth.NewPersistentRateLimiter(database) + // Shared rate limiter for auth endpoints, built by internal/app so the + // hub and these routes share one instance (its lockouts are persisted to + // the database and survive restarts — M2 security hardening). + limiter := rt.Limiter // Start background cleanup of stale rate-limiter entries to prevent // unbounded memory growth. The goroutine exits when stopCh is closed. @@ -76,9 +100,8 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri }) // Service layer — centralizes business logic for REST and WS handlers. - // *db.DB satisfies service.Store directly (the store abstraction was - // removed in D3). - svc := service.New(database, limiter) + // Built by internal/app alongside the hub, which holds the same instance. + svc := rt.Services // Auth routes are mounted after hub creation (below) so self-service // account deletion can broadcast member_ban like the admin ban path does. @@ -103,23 +126,22 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri // File upload and serving routes. store, storeErr := routerUploadRoutes(r, database, limiter, cfg, svc.Permissions) - // WebSocket hub — WS does its own in-band auth, so no AuthMiddleware here. - hub := ws.NewHub(database, limiter, svc) - // Replay budget knobs must land before hub.Run starts (below). - hub.ConfigureReplay(cfg.EventPersistence.ReplayRingSize, cfg.EventPersistence.ReplayColdLimit) + // WebSocket hub — built, wired and started by internal/app; WS does its + // own in-band auth, so no AuthMiddleware here. + hub := rt.Hub getOnlineUsers = func() int { return hub.ClientCount() } hubAlive = func() bool { return hub.DispatchAlive() } // Auth routes. The service is built after the hub, with the hub as its // AuthBroadcaster, so DELETE /api/v1/auth/account fans out member_ban and // force-disconnects the deleted user's own socket exactly like the admin - // ban path does for the same DB state. B3-3 moves this to internal/app. + // ban path does for the same DB state. MountAuthRoutes(r, service.NewAuthService(database, limiter, totpKey, hub), AuthMiddleware(database), limiter, cfg.Server.TrustedProxies) - routerPluginWiring(hub, pluginRegistry) - - // Voice: LiveKit client, optional companion process, webhook and proxy routes. - routerVoiceRoutes(r, cfg, limiter, hub) + // Voice: webhook, LiveKit health and signalling-proxy routes. The client + // and the companion process are built by internal/app, which reports + // through rt.VoiceEnabled whether there is anything to mount. + routerVoiceRoutes(r, cfg, limiter, hub, rt.VoiceEnabled) // Profile routes: update profile, change password, session management. // Mounted after hub creation so the hub can broadcast user_update events. @@ -158,7 +180,6 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri Get("/api/v1/diagnostics/connectivity", handleDiagnosticsConnectivity(cfg, ver, hub)) - go hub.Run() r.Get("/api/v1/ws", ws.ServeWS(hub, database, cfg.Server.AllowedOrigins, cfg.Server.MaxWSConnections)) routerMetricsRoutes(r, cfg, database, svc, hub) @@ -205,7 +226,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri close(limiterStopCh) } - return r, hub, cleanup + return r, cleanup } // routerTOTPKey loads (or auto-generates) the AES-256 key NewRouter hands to the @@ -320,64 +341,14 @@ func routerUploadRoutes(r chi.Router, database *db.DB, limiter *auth.RateLimiter return store, storeErr } -// routerPluginWiring wires the plugin registry and its event sink into the hub. -func routerPluginWiring(hub *ws.Hub, pluginRegistry *plugin.Registry) { - // Phase C Step 9 — wire plugin registry and event sink into the hub. - // nil pluginRegistry means plugins are disabled; the hub no-ops cleanly. - if pluginRegistry != nil { - hub.SetPluginRegistry(pluginRegistry) - sink := pluginRegistry.Sink() - sink.SetBroadcaster(hub.BroadcastToChannel) - hub.SetPluginEventSink(sink) - } -} - -// routerVoiceRoutes creates the LiveKit client, optionally starts the companion -// LiveKit process, and mounts the webhook, LiveKit health and signaling-proxy -// routes. Voice is disabled — and none of those routes are mounted — when the -// client fails to build. -func routerVoiceRoutes(r chi.Router, cfg *config.Config, limiter *auth.RateLimiter, hub *ws.Hub) { - // Create LiveKit client if voice config is present; voice is disabled on failure. - lk, lkErr := ws.NewLiveKitClient(&cfg.Voice) - if lkErr != nil { - slog.Warn("failed to create LiveKit client, voice disabled", "error", lkErr) - } else { - hub.SetLiveKit(lk) - - // Optionally start a companion LiveKit process — either from a - // configured binary or via checksum-verified auto-download (the - // download happens in the background inside Start). - if cfg.Voice.LiveKitBinaryPath != "" || cfg.Voice.AutoDownloadLiveKit { - proc := ws.NewLiveKitProcess(&cfg.Voice, &cfg.TLS, cfg.Server.DataDir) - // Register the process with the hub BEFORE calling Start(), and - // keep it registered even if Start() fails (OC-0019). The only - // consumer of h.lkProcess is the voice_join guard - // (`h.lkProcess != nil && !h.lkProcess.IsRunning()`), which reads - // a nil process as "LiveKit is externally managed, don't check". - // That is the wrong reading here: OwnCord was told to manage - // LiveKit and failed to launch it, so joins must fail closed via - // IsRunning() == false, not be waved through with no SFU - // running. IsRunning() is false for a proc whose Start() never - // got as far as spawning cmd, and Hub.Stop's lkProcess.Stop() is - // safe to call on a never-started proc. - hub.SetLiveKitProcess(proc) - if startErr := proc.Start(); startErr != nil { - slog.Error("failed to start LiveKit process", "error", startErr) - } - } - } - - // Warn if LiveKit is externally managed and webhook may be blocked by admin CIDRs. - if lkErr == nil && cfg.Voice.LiveKitBinaryPath == "" && !cfg.Voice.AutoDownloadLiveKit { - lkHost := "" - if u, parseErr := url.Parse(cfg.Voice.LiveKitURL); parseErr == nil { - lkHost = u.Hostname() - } - if lkHost != "" && lkHost != "localhost" && lkHost != "127.0.0.1" && lkHost != "::1" { - slog.Warn("LiveKit is externally managed but webhook endpoint is admin-IP-restricted — "+ - "add the LiveKit server's IP to livekit_webhook_allowed_cidrs or webhooks will be silently dropped", - "livekit_host", lkHost) - } +// routerVoiceRoutes mounts the LiveKit webhook, health and signalling-proxy +// routes. voiceEnabled is internal/app's report that the LiveKit client was +// built (StartRuntime); voice is disabled — and none of these routes are +// mounted — when it was not. Until B3-3 this function also created the client +// and the companion process, which is what gave the hub a second owner. +func routerVoiceRoutes(r chi.Router, cfg *config.Config, limiter *auth.RateLimiter, hub *ws.Hub, voiceEnabled bool) { + if !voiceEnabled { + return } // LiveKit webhook endpoint (no auth middleware — uses LiveKit JWT @@ -386,28 +357,26 @@ func routerVoiceRoutes(r chi.Router, cfg *config.Config, limiter *auth.RateLimit // externally-hosted LiveKit can be admitted WITHOUT widening the admin // panel's perimeter to the SFU's network. Falls back to // admin_allowed_cidrs when unset. - if lkErr == nil { - webhookCIDRs := cfg.Server.LiveKitWebhookCIDRs() - r.With(AdminIPRestrict(webhookCIDRs, cfg.Server.TrustedProxies)). - Post("/api/v1/livekit/webhook", - ws.MountWebhookRoute(hub, cfg.Voice.LiveKitAPIKey, cfg.Voice.LiveKitAPISecret)) + webhookCIDRs := cfg.Server.LiveKitWebhookCIDRs() + r.With(AdminIPRestrict(webhookCIDRs, cfg.Server.TrustedProxies)). + Post("/api/v1/livekit/webhook", + ws.MountWebhookRoute(hub, cfg.Voice.LiveKitAPIKey, cfg.Voice.LiveKitAPISecret)) - // LiveKit health check — same perimeter as the webhook. - r.With(AdminIPRestrict(webhookCIDRs, cfg.Server.TrustedProxies)). - Get("/api/v1/livekit/health", handleLiveKitHealth(hub)) + // LiveKit health check — same perimeter as the webhook. + r.With(AdminIPRestrict(webhookCIDRs, cfg.Server.TrustedProxies)). + Get("/api/v1/livekit/health", handleLiveKitHealth(hub)) - // Reverse proxy LiveKit signaling through OwnCord's HTTPS server. - // This avoids mixed-content blocks (secure page → insecure WS). - // Client connects to wss://server:8443/livekit/* → ws://localhost:7880/* - // - // NOTE: AuthMiddleware is intentionally omitted. The LiveKit JS SDK's - // signal requests don't carry OwnCord session tokens — authentication - // is handled by the LiveKit JWT (access_token query param) which the - // LiveKit server validates. Users can only obtain a valid JWT through - // the authenticated voice_join WS flow. Rate limiting prevents abuse. - r.With(rateLimitMiddlewareWithPrefix(limiter, "livekit_proxy:", livekitProxyRateLimitPerMinute, time.Minute, cfg.Server.TrustedProxies)). - Handle("/livekit/*", http.StripPrefix("/livekit", NewLiveKitProxy(cfg.Voice.LiveKitURL, cfg.Server.AllowedOrigins))) - } + // Reverse proxy LiveKit signaling through OwnCord's HTTPS server. + // This avoids mixed-content blocks (secure page → insecure WS). + // Client connects to wss://server:8443/livekit/* → ws://localhost:7880/* + // + // NOTE: AuthMiddleware is intentionally omitted. The LiveKit JS SDK's + // signal requests don't carry OwnCord session tokens — authentication + // is handled by the LiveKit JWT (access_token query param) which the + // LiveKit server validates. Users can only obtain a valid JWT through + // the authenticated voice_join WS flow. Rate limiting prevents abuse. + r.With(rateLimitMiddlewareWithPrefix(limiter, "livekit_proxy:", livekitProxyRateLimitPerMinute, time.Minute, cfg.Server.TrustedProxies)). + Handle("/livekit/*", http.StripPrefix("/livekit", NewLiveKitProxy(cfg.Voice.LiveKitURL, cfg.Server.AllowedOrigins))) } // routerMetricsRoutes mounts the JSON metrics endpoint and, when an OTel diff --git a/Server/api/router_delete_account_broadcast_test.go b/Server/api/router_delete_account_broadcast_test.go index a831bd52..41d825da 100644 --- a/Server/api/router_delete_account_broadcast_test.go +++ b/Server/api/router_delete_account_broadcast_test.go @@ -35,6 +35,7 @@ import ( "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" ) // dialAndAuthWS opens a WS connection against srv and completes the auth @@ -97,7 +98,8 @@ func TestNewRouter_DeleteAccount_BroadcastsMemberBanOverWS(t *testing.T) { }, } - handler, _, cleanup := api.NewRouter(cfg, database, "test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "test", nil, nil, rt) t.Cleanup(cleanup) newUserSession := func(username string) (int64, string) { diff --git a/Server/api/router_livekit_process_test.go b/Server/api/router_livekit_process_test.go index d8f32445..c2dfc477 100644 --- a/Server/api/router_livekit_process_test.go +++ b/Server/api/router_livekit_process_test.go @@ -30,6 +30,7 @@ import ( "github.com/J3vb/OwnCord/Server/auth" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" ) // voiceJoinWSMsg builds a raw voice_join WebSocket frame for the given channel. @@ -72,7 +73,8 @@ func TestNewRouter_LiveKitProcessStartFailure_VoiceJoinFailsClosed(t *testing.T) }, } - handler, _, cleanup := api.NewRouter(cfg, database, "test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "test", nil, nil, rt) t.Cleanup(cleanup) // role_id=1 -> Owner, so CONNECT_VOICE is granted and the test isolates diff --git a/Server/api/router_test.go b/Server/api/router_test.go index 6695141a..43ff6305 100644 --- a/Server/api/router_test.go +++ b/Server/api/router_test.go @@ -10,6 +10,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" ) // setupRouter creates a test router with an in-memory database. @@ -32,7 +33,8 @@ func setupRouter(t *testing.T) http.Handler { }, } - handler, _, cleanup := api.NewRouter(cfg, database, "test", nil, nil) + rt := app.StartRuntime(cfg, database, nil) + handler, cleanup := api.NewRouter(cfg, database, "test", nil, nil, rt) t.Cleanup(cleanup) return handler } diff --git a/Server/api/router_totp_key_fatal_test.go b/Server/api/router_totp_key_fatal_test.go index 49631cd6..da730289 100644 --- a/Server/api/router_totp_key_fatal_test.go +++ b/Server/api/router_totp_key_fatal_test.go @@ -6,6 +6,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" ) // TestNewRouterRefusesToStartWithMalformedTOTPKey pins OC-0228: a malformed @@ -49,7 +50,7 @@ func TestNewRouterRefusesToStartWithMalformedTOTPKey(t *testing.T) { panicked = true } }() - api.NewRouter(cfg, database, "test", nil, nil) + api.NewRouter(cfg, database, "test", nil, nil, app.StartRuntime(cfg, database, nil)) }() if !panicked { diff --git a/Server/cmd/gendocs/main.go b/Server/cmd/gendocs/main.go index d6848753..84d0d95b 100644 --- a/Server/cmd/gendocs/main.go +++ b/Server/cmd/gendocs/main.go @@ -45,6 +45,7 @@ import ( "github.com/J3vb/OwnCord/Server/api" "github.com/J3vb/OwnCord/Server/config" "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/internal/app" "github.com/J3vb/OwnCord/Server/telemetry" "github.com/go-chi/chi/v5" ) @@ -253,7 +254,11 @@ func genRoutes(w io.Writer) error { return errors.New("telemetry.Init left no Prometheus handler: run this tool as `go run -tags otel,wazero ./cmd/gendocs`, the build the route index is generated from") } - handler, _, cleanup := api.NewRouter(cfg, database, "gendocs", nil, nil) + // internal/app owns hub construction since B3-3, so the route index is + // generated over the same collaborators the server runs with. + rt := app.StartRuntime(cfg, database, nil) + defer rt.Hub.GracefulStop() + handler, cleanup := api.NewRouter(cfg, database, "gendocs", nil, nil, rt) defer cleanup() routes, ok := handler.(chi.Routes) diff --git a/Server/addrinuse.go b/Server/internal/app/addrinuse.go similarity index 98% rename from Server/addrinuse.go rename to Server/internal/app/addrinuse.go index b5734986..f05d7124 100644 --- a/Server/addrinuse.go +++ b/Server/internal/app/addrinuse.go @@ -1,4 +1,4 @@ -package main +package app import "strings" diff --git a/Server/addrinuse_test.go b/Server/internal/app/addrinuse_test.go similarity index 99% rename from Server/addrinuse_test.go rename to Server/internal/app/addrinuse_test.go index 14f15ddd..4883df0a 100644 --- a/Server/addrinuse_test.go +++ b/Server/internal/app/addrinuse_test.go @@ -1,4 +1,4 @@ -package main +package app import ( "errors" diff --git a/Server/addrinuse_unix.go b/Server/internal/app/addrinuse_unix.go similarity index 95% rename from Server/addrinuse_unix.go rename to Server/internal/app/addrinuse_unix.go index cdcbae0f..c63f9b8a 100644 --- a/Server/addrinuse_unix.go +++ b/Server/internal/app/addrinuse_unix.go @@ -1,6 +1,6 @@ //go:build !windows -package main +package app import ( "errors" diff --git a/Server/addrinuse_windows.go b/Server/internal/app/addrinuse_windows.go similarity index 98% rename from Server/addrinuse_windows.go rename to Server/internal/app/addrinuse_windows.go index 43051780..31a0d3a0 100644 --- a/Server/addrinuse_windows.go +++ b/Server/internal/app/addrinuse_windows.go @@ -1,6 +1,6 @@ //go:build windows -package main +package app import ( "errors" diff --git a/Server/internal/app/app.go b/Server/internal/app/app.go new file mode 100644 index 00000000..292cd869 --- /dev/null +++ b/Server/internal/app/app.go @@ -0,0 +1,163 @@ +package app + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "log/slog" + "net/http" + + "github.com/J3vb/OwnCord/Server/admin" + "github.com/J3vb/OwnCord/Server/api" + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/plugin" + "github.com/J3vb/OwnCord/Server/ws" +) + +// Deps are the pieces main() owns and hands in: the build version it injects +// with `-ldflags -X main.version`, the logger and the ring buffer both log +// sinks are already wired against, and the restart coordinator whose handoff +// main() performs after Run returns. Everything else the process needs, the +// App builds. +type Deps struct { + Version string + Log *slog.Logger + LogBuf *admin.RingBuffer + Restart *RestartCoordinator +} + +// closeStep is one teardown step: the stage that registered it, and how that +// stage stops. Steps are appended in START order and Close walks them +// backwards. +type closeStep struct { + stage string + stop func(context.Context) error +} + +// App is one server process. Each stage the lifecycle starts owns a field +// here and registers exactly one closeStep as it comes up, so there is a +// single teardown path — taken on a failed start, a serve error and a clean +// shutdown alike — instead of the `defer` stack run() used to carry. +type App struct { + cfg *config.Config + deps Deps + log *slog.Logger + + // rootCtx is the context Run was given: the process context every other + // context here descends from, so cancelling it stops the server the same + // way a signal or a restart request does. + rootCtx context.Context + // bgCtx is the context every background goroutine (event persister, + // event pruner, plugin loader, maintenance loop) runs under. bgCancel is + // registered as the FIRST closer, so it runs LAST — the persistence and + // maintenance steps cancel it and join their goroutines earlier, and + // this is only their backstop. + bgCtx context.Context + bgCancel context.CancelFunc + + tlsCfg *tls.Config + httpHandler http.Handler // ACME HTTP-01 handler; nil outside acme mode + database *db.DB + plugins *plugin.Registry + runtime api.Runtime + hub *ws.Hub + router http.Handler + addr string + srv *http.Server + acmeSrv *http.Server + persister *ws.EventPersister + prunerDone <-chan struct{} + auditWriter *db.AuditWriter + serveCtx context.Context + + closers []closeStep + closed bool + + // failStage makes the named start stage fail instead of running. It is + // the failure-injection seam lifecycle_test.go drives: every stage before + // it starts for real, so the assertions are about what teardown does with + // what was already up. Never set outside tests. + failStage string + + // onCloseStep is called with each stage's name just before its close step + // runs. It makes the teardown walk observable, which is how the tests + // assert what is still alive at a given point in it. Never set outside + // tests. + onCloseStep func(stage string) +} + +// errStageInjected is what a failStage-selected stage returns. It exists so +// the injection is visibly a test seam in a stack trace rather than a +// plausible production error. +var errStageInjected = errors.New("start failed (injected)") + +// New builds the App around an already-loaded configuration. It starts +// nothing and opens nothing: Run does that, so every started stage has a +// matching close on every return path and an App that is never run has +// nothing to release. The error is for a missing dependency — a nil logger +// or coordinator would only surface as a panic several stages in. +func New(cfg *config.Config, deps Deps) (*App, error) { + switch { + case cfg == nil: + return nil, errors.New("app: New needs a configuration") + case deps.Log == nil: + return nil, errors.New("app: New needs a logger") + case deps.Restart == nil: + return nil, errors.New("app: New needs a restart coordinator") + } + + return &App{cfg: cfg, deps: deps, log: deps.Log}, nil +} + +// onClose registers stage's teardown step. Order of registration is start +// order; Close reverses it. +func (a *App) onClose(stage string, stop func(context.Context) error) { + a.closers = append(a.closers, closeStep{stage: stage, stop: stop}) +} + +// Close stops every started stage in the reverse of the order they started, +// and is the only teardown path in the process. Three ordering facts depend +// on it and are what the reverse walk is FOR +// (docs/architecture/server-boundaries.md, "Start, drain, stop"): +// +// - the audit writer starts after the database opens, so it stops before +// database.Close and its queue is flushed while the handle is still live; +// - event persistence likewise stops (cancelling bgCtx and joining the +// pruner) before the handle goes, so no prune is mid-query against a +// closing pool; +// - the hub's GracefulStop runs on EVERY return from Run, including an +// early one, because it is the only caller of LiveKitProcess.Stop and +// skipping it orphans the supervised livekit-server process (OC-0027). +// +// It reports the FIRST error and runs every later step regardless: the steps +// below a failing one are the ones that release the database handle, the +// LiveKit process and the audit queue, so aborting the walk would leak +// exactly what teardown exists to reclaim. Later errors are logged. +// Calling it twice is a no-op. +func (a *App) Close(ctx context.Context) error { + if a.closed { + return nil + } + a.closed = true + + var first error + for i := len(a.closers) - 1; i >= 0; i-- { + step := a.closers[i] + if a.onCloseStep != nil { + a.onCloseStep(step.stage) + } + err := step.stop(ctx) + if err == nil { + continue + } + if first == nil { + first = fmt.Errorf("stopping %s: %w", step.stage, err) + continue + } + a.log.Warn("shutdown step failed after an earlier failure", + "stage", step.stage, "error", err) + } + return first +} diff --git a/Server/internal/app/app_test.go b/Server/internal/app/app_test.go new file mode 100644 index 00000000..e3111f04 --- /dev/null +++ b/Server/internal/app/app_test.go @@ -0,0 +1,95 @@ +package app + +import ( + "context" + "io" + "log/slog" + "strings" + "testing" + "time" + + "go.uber.org/goleak" + + "github.com/J3vb/OwnCord/Server/admin" +) + +// runApp is what main()'s runServer does, condensed: load the configuration, +// build the App, run it until it stops and has closed every stage it started. +// The end-to-end tests drive the lifecycle through this rather than through a +// package-level entry point, because that is now exactly what the process +// does. +func runApp(log *slog.Logger, logBuf *admin.RingBuffer, levelVar *slog.LevelVar, rc *RestartCoordinator) error { + cfg, err := LoadConfig(log, levelVar, rc) + if err != nil { + return err + } + a, err := New(cfg, Deps{Version: "test", Log: log, LogBuf: logBuf, Restart: rc}) + if err != nil { + return err + } + return a.Run(context.Background()) +} + +// bootTestApp builds a real App the way main() does — LoadConfig against a +// generated default config.yaml in a temp directory, then New — with TLS and +// the LiveKit auto-download turned off so the test stays offline and never +// generates a certificate. failStage, when non-empty, makes that start stage +// fail instead of running (see App.start); every stage before it starts for +// real, which is the point: the assertions are about what teardown does with +// what was already up. +func bootTestApp(t *testing.T, port, failStage string) *App { + t.Helper() + t.Chdir(t.TempDir()) + t.Setenv("OWNCORD_SERVER_PORT", port) + t.Setenv("OWNCORD_TLS_MODE", "off") + t.Setenv("OWNCORD_VOICE_AUTO_DOWNLOAD_LIVEKIT", "false") + + levelVar := new(slog.LevelVar) + log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: levelVar})) + rc := NewRestartCoordinator(time.Hour, nil) + + cfg, err := LoadConfig(log, levelVar, rc) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + a, err := New(cfg, Deps{Version: "test", Log: log, LogBuf: admin.NewRingBuffer(64), Restart: rc}) + if err != nil { + t.Fatalf("New: %v", err) + } + a.failStage = failStage + return a +} + +// TestAppRun_LateStageFailure_StopsTheHubAndClosesTheDatabase is the third +// property of the composite close, and the one OC-0027 is about: when a +// stage that starts AFTER the router fails, the hub is already running and +// its dispatch goroutine owns the companion livekit-server process. +// hub.GracefulStop is the only caller of LiveKitProcess.Stop, so a teardown +// that skips it orphans a real process. Before B3-3 this held only because +// `defer hub.GracefulStop()` sat above every early return in run(); now it +// is a closer, and this is what proves the closer actually runs. +func TestAppRun_LateStageFailure_StopsTheHubAndClosesTheDatabase(t *testing.T) { + leakOpt := goleak.IgnoreCurrent() + a := bootTestApp(t, "0", "maintenance") + + err := a.Run(context.Background()) + if err == nil { + t.Fatal("Run() = nil, want the injected maintenance failure") + } + if !strings.Contains(err.Error(), "maintenance") { + t.Errorf("Run() = %v, want an error naming the stage that failed (maintenance)", err) + } + + if a.hub == nil { + t.Fatal("the router stage runs before maintenance, so the hub must have been built") + } + if a.hub.DispatchAlive() { + t.Error("hub dispatch is still alive after Run returned — GracefulStop was skipped, so a supervised LiveKit process would be orphaned") + } + if err := a.database.PingRead(context.Background()); err == nil { + t.Error("the database handle is still open after Run returned") + } + if err := goleak.Find(leakOpt); err != nil { + t.Fatalf("goroutine leaked after a failed start: %v", err) + } +} diff --git a/Server/internal/app/banner.go b/Server/internal/app/banner.go new file mode 100644 index 00000000..bb5bb10a --- /dev/null +++ b/Server/internal/app/banner.go @@ -0,0 +1,106 @@ +package app + +import ( + "fmt" + "log/slog" + "net" + "os" + "runtime" + + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/diskutil" +) + +// printBanner writes the startup banner to stderr (so it doesn't mix with +// the structured log output on stdout). +func printBanner(cfg *config.Config, ver string, tls bool) { + scheme := "http" + if tls { + scheme = "https" + } + + localIP := getOutboundIP() + port := cfg.Server.Port + baseURL := fmt.Sprintf("%s://%s:%d", scheme, localIP, port) + adminURL := baseURL + "/admin" + + tlsStatus := "disabled" + if tls { + tlsStatus = "enabled" + } + + banner := fmt.Sprintf(` + + ___ ____ _ + / _ \__ ___ __ / ___|___ _ __ __| | + | | | \ \ /\ / / '_ \| | / _ \| '__/ _`+"`"+` | + | |_| |\ V V /| | | | |__| (_) | | | (_| | + \___/ \_/\_/ |_| |_|\____\___/|_| \__,_| + + ───────────────────────────────────────────── + Server %s + Version %s + TLS %s + Platform %s/%s + ───────────────────────────────────────────── + API %s/api/v1/info + WebSocket %s/api/v1/ws + Admin %s + Health %s/health + ───────────────────────────────────────────── + Press Ctrl+C to stop the server. + +`, cfg.Server.Name, ver, tlsStatus, runtime.GOOS, runtime.GOARCH, + baseURL, wsURL(scheme, localIP, port), adminURL, baseURL) + + _, _ = fmt.Fprint(os.Stderr, banner) +} + +// wsURL builds the WebSocket URL with the correct scheme. +func wsURL(httpScheme, ip string, port int) string { + ws := "ws" + if httpScheme == "https" { + ws = "wss" + } + return fmt.Sprintf("%s://%s:%d", ws, ip, port) +} + +// Free-space thresholds for the boot-time disk warning. /health uses its own +// (lower) continuous threshold; these only shape startup log noise. +const ( + diskWarnBytes = 1 << 30 // 1 GiB — warn + diskCriticalBytes = 256 << 20 // 256 MiB — error +) + +// warnLowDisk logs when the volume holding path is low on space. Probe +// failures (unsupported platform, missing dir) are silent — unknown ≠ full. +func warnLowDisk(log *slog.Logger, label, path string) { + free, err := diskutil.FreeBytes(path) + if err != nil { + return + } + switch { + case free < diskCriticalBytes: + log.Error("disk space critically low — writes will start failing soon", + "volume", label, "path", path, "free_mb", free>>20) + case free < diskWarnBytes: + log.Warn("disk space low", "volume", label, "path", path, "free_mb", free>>20) + } +} + +// getOutboundIP returns the preferred outbound IP of this machine by dialing +// a known external address (no actual connection is made with UDP). +func getOutboundIP() string { + conn, err := net.Dial("udp", "8.8.8.8:80") + if err != nil { + return "localhost" + } + defer conn.Close() //nolint:errcheck + addr, ok := conn.LocalAddr().(*net.UDPAddr) + if !ok { + slog.Warn("getOutboundIP: unexpected LocalAddr type, falling back to localhost", + "type", fmt.Sprintf("%T", conn.LocalAddr())) + return "localhost" + } + return addr.IP.String() +} diff --git a/Server/internal/app/bootstrap.go b/Server/internal/app/bootstrap.go new file mode 100644 index 00000000..2424cc8d --- /dev/null +++ b/Server/internal/app/bootstrap.go @@ -0,0 +1,92 @@ +package app + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "time" + + "github.com/J3vb/OwnCord/Server/config" +) + +// removeOldBinary deletes the binary a previous self-update left behind. +// The data-dir stage's first act, before anything is opened. +func removeOldBinary(log *slog.Logger) { + // Clean up old binary from a previous update. Bounded retry: in spawn + // mode the predecessor spawns this process as its very last act, so for + // the first few hundred milliseconds it may not have fully exited — and + // on Windows its image file (the .old after the swap) stays locked until + // it does. + exePath, exeErr := os.Executable() + if exeErr != nil { + log.Warn("failed to determine executable path", "error", exeErr) + return + } + + oldPath := exePath + ".old" + if _, statErr := os.Stat(oldPath); statErr != nil { + return + } + + var rmErr error + for attempt := range 5 { + if attempt > 0 { + time.Sleep(250 * time.Millisecond) + } + if rmErr = os.Remove(oldPath); rmErr == nil { + break + } + } + if rmErr != nil { + log.Warn("failed to remove old binary", "path", oldPath, "error", rmErr) + } else { + log.Info("removed old binary from previous update", "path", oldPath) + } +} + +// LoadConfig loads the on-disk configuration, applies its logging level +// and resolves the restart handoff mode. main() calls it before app.New: +// the level applies to main's own log sinks, and the mode is read back from +// the coordinator after Run returns. +func LoadConfig(log *slog.Logger, levelVar *slog.LevelVar, rc *RestartCoordinator) (*config.Config, error) { + cfg, err := config.Load(config.DefaultPath) + if err != nil { + return nil, fmt.Errorf("loading config: %w", err) + } + + // Apply the configured log level. The admin panel's live log view (ring + // buffer) follows the same threshold — set logging.level to "debug" to + // capture debug records there. + if lvl, ok := config.ParseLevel(cfg.Logging.Level); ok { + levelVar.Set(lvl) + } else { + log.Warn("unknown logging.level, keeping info", "value", cfg.Logging.Level) + } + + // Resolve how a self-restart hands off (spawn the replacement vs exit + // for a supervisor) now that config is loaded — main() reads it back + // after Run() returns. + rc.SetMode(resolveRestartMode(cfg.Server.RestartMode, log)) + + return cfg, nil +} + +// prepareDataDir creates the configured data directory and warns when the +// volumes the server writes to are low on free space. The data-dir stage. +func prepareDataDir(log *slog.Logger, cfg *config.Config) error { + if mkdirErr := os.MkdirAll(cfg.Server.DataDir, 0o750); mkdirErr != nil { + return fmt.Errorf("creating data dir %s: %w", cfg.Server.DataDir, mkdirErr) + } + + // Disk-space awareness: the database (WAL growth included), uploads, + // certs, and by default backups all live on this volume, and running it + // dry breaks several of them at once. Probe errors are ignored — unknown + // is not "full". /health repeats this check continuously at 256 MiB. + warnLowDisk(log, "data dir", cfg.Server.DataDir) + if cfg.Backup.Dir != "" && cfg.Backup.Dir != filepath.Join(cfg.Server.DataDir, "backups") { + warnLowDisk(log, "backup dir", cfg.Backup.Dir) + } + + return nil +} diff --git a/Server/internal/app/close_test.go b/Server/internal/app/close_test.go new file mode 100644 index 00000000..7c3f08fd --- /dev/null +++ b/Server/internal/app/close_test.go @@ -0,0 +1,111 @@ +package app + +import ( + "context" + "errors" + "io" + "log/slog" + "slices" + "testing" +) + +// The composite-close contract, as three properties. Before B3-3's rewrite +// there was no close function at all: teardown was a LIFO `defer` stack +// inside run(), so "reverse of start" was an emergent property of where each +// `defer` happened to be registered, nothing returned a teardown error but +// the one HTTP shutdown, and a stage that returned early simply skipped +// whatever it had not reached yet. These pin the replacement. + +// TestAppClose_StopsInReverseOfStartOrder pins the ordering half of the +// contract: closers are appended in START order and Close walks them +// backwards, so the last stage to come up is the first to go down. Three +// facts in docs/architecture/server-boundaries.md depend on exactly this — +// the audit writer and the event persister must both stop before +// database.Close, and they are started after it. +func TestAppClose_StopsInReverseOfStartOrder(t *testing.T) { + var order []string + a := newTestApp() + for _, name := range []string{"database", "telemetry", "router", "audit-writer", "http"} { + a.onClose(name, func(context.Context) error { + order = append(order, name) + return nil + }) + } + + if err := a.Close(context.Background()); err != nil { + t.Fatalf("Close() = %v, want nil when no closer fails", err) + } + + want := []string{"http", "audit-writer", "router", "telemetry", "database"} + if !slices.Equal(order, want) { + t.Errorf("close order = %v, want %v (the reverse of the start order)", order, want) + } +} + +// TestAppClose_ReturnsFirstErrorAndStillRunsEveryLaterClose pins the error +// half: a failing stop must not abort the walk. The database handle, the +// LiveKit process and the audit queue are all closed by steps that run +// AFTER the HTTP shutdown, which is the one step that could realistically +// fail — so "return early on the first error" would leak exactly the +// resources teardown exists to release. +func TestAppClose_ReturnsFirstErrorAndStillRunsEveryLaterClose(t *testing.T) { + errHTTP := errors.New("graceful shutdown: context deadline exceeded") + errDatabase := errors.New("closing the database") + + var ran []string + a := newTestApp() + a.onClose("database", func(context.Context) error { + ran = append(ran, "database") + return errDatabase + }) + a.onClose("router", func(context.Context) error { + ran = append(ran, "router") + return nil + }) + a.onClose("http", func(context.Context) error { + ran = append(ran, "http") + return errHTTP + }) + + err := a.Close(context.Background()) + + if !errors.Is(err, errHTTP) { + t.Errorf("Close() = %v, want the FIRST error in close order (%v)", err, errHTTP) + } + if errors.Is(err, errDatabase) { + t.Errorf("Close() = %v, want the first error only, not the last one", err) + } + want := []string{"http", "router", "database"} + if !slices.Equal(ran, want) { + t.Errorf("closers run = %v, want %v — a failing stop must not skip the ones below it", ran, want) + } +} + +// TestAppClose_IsIdempotent pins that a second Close does nothing: Run +// closes on every return path, and main() must be able to call it again (or +// a test through t.Cleanup) without double-stopping a hub or double-closing +// a database handle. +func TestAppClose_IsIdempotent(t *testing.T) { + calls := 0 + a := newTestApp() + a.onClose("database", func(context.Context) error { + calls++ + return nil + }) + + if err := a.Close(context.Background()); err != nil { + t.Fatalf("first Close() = %v, want nil", err) + } + if err := a.Close(context.Background()); err != nil { + t.Fatalf("second Close() = %v, want nil", err) + } + if calls != 1 { + t.Errorf("closer ran %d times, want exactly 1", calls) + } +} + +// newTestApp is an App with only what Close needs: the logger it reports +// through. The stages are supplied by each test. +func newTestApp() *App { + return &App{log: slog.New(slog.NewTextHandler(io.Discard, nil))} +} diff --git a/Server/internal/app/database.go b/Server/internal/app/database.go new file mode 100644 index 00000000..1e9d1fd3 --- /dev/null +++ b/Server/internal/app/database.go @@ -0,0 +1,67 @@ +package app + +import ( + "context" + "fmt" + "log/slog" + + "github.com/J3vb/OwnCord/Server/admin" + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" +) + +// openDatabase validates the configured backend and opens the database. +// The database stage; App.startDatabase registers its close before +// migrating, so a migration failure still releases the handle. +func openDatabase(cfg *config.Config) (*db.DB, error) { + // SQLite is the only supported backend; the unfinished Postgres + // scaffolding (stubbed query layer, never wired into the runtime) was + // removed rather than completed. + if t := cfg.Database.Type; t != "" && t != "sqlite" { + return nil, fmt.Errorf("database.type=%q is not supported; set \"sqlite\" or omit it", t) + } + + database, err := db.OpenWithMaxReaders(cfg.Database.Path, cfg.Database.MaxReaders) + if err != nil { + return nil, fmt.Errorf("opening database: %w", err) + } + + return database, nil +} + +// initDatabase points the admin panel at the live database, runs the +// migrations and clears state left over from a previous run. Extracted from +// run. +func initDatabase(log *slog.Logger, cfg *config.Config, database *db.DB, rc *RestartCoordinator) error { + // The admin "Restore backup" handler needs the real database file path: + // without this, it falls back to a hardcoded "data/chatserver.db" and + // silently no-ops on any server with a configured database.path. + admin.SetDatabasePath(cfg.Database.Path) + // Backup handlers and the scheduled-backup maintenance write to the + // configured backup directory (defaults to data/backups). + admin.SetBackupDir(cfg.Backup.Dir) + // Admin restart requests (update apply, backup restore, setup wizard) + // land in the coordinator, which drains this process and lets main() + // perform the handoff. Wired before the listener starts serving, so no + // admin request can ever hit the unwired default hook. + admin.SetRestartHandoff(rc.Request) + + if err := db.Migrate(database); err != nil { + return fmt.Errorf("running migrations: %w", err) + } + + // Clear stale state from a previous run or crash. Startup work — nothing + // to inherit a context from yet. + if err := database.ResetAllUserStatuses(context.Background()); err != nil { + log.Warn("failed to reset stale user statuses", "error", err) + } else { + log.Info("reset all user statuses to offline") + } + if err := database.ClearAllVoiceStates(context.Background()); err != nil { + log.Warn("failed to clear stale voice states", "error", err) + } else { + log.Info("cleared stale voice states") + } + + return nil +} diff --git a/Server/internal/app/healthcheck.go b/Server/internal/app/healthcheck.go new file mode 100644 index 00000000..2b86b2d0 --- /dev/null +++ b/Server/internal/app/healthcheck.go @@ -0,0 +1,148 @@ +package app + +import ( + "bytes" + "crypto/tls" + "encoding/pem" + "errors" + "fmt" + "io" + "net/http" + "os" + "strconv" + "time" + + "gopkg.in/yaml.v3" + + "github.com/J3vb/OwnCord/Server/config" +) + +// RunHealthcheckCLI probes the local server's /health endpoint and returns a +// process exit code: 0 healthy, 1 degraded or unreachable. /health answers +// 503 with a subsystem reason when the hub, database, or disk is unhealthy, +// so a container orchestrator's healthcheck surfaces those too. +func RunHealthcheckCLI() int { + // Deliberately NOT config.Load: that writes a default config.yaml when + // none exists, and a probe must have no side effects. Peek at the file + // (and the env overrides) for just the values that shape the URL and the + // certificate pin. + port := 8443 + scheme := "https" + certFile := "data/cert.pem" + tlsMode := "" + acmeDomain := "" + if raw, err := os.ReadFile(config.DefaultPath); err == nil { + var partial struct { + Server struct { + Port int `yaml:"port"` + } `yaml:"server"` + TLS struct { + Mode string `yaml:"mode"` + CertFile string `yaml:"cert_file"` + Domain string `yaml:"domain"` + } `yaml:"tls"` + } + if yaml.Unmarshal(raw, &partial) == nil { + if partial.Server.Port > 0 { + port = partial.Server.Port + } + tlsMode = partial.TLS.Mode + if partial.TLS.Mode == "off" { + scheme = "http" + } + if partial.TLS.CertFile != "" { + certFile = partial.TLS.CertFile + } + acmeDomain = partial.TLS.Domain + } + } + if env := os.Getenv("OWNCORD_SERVER_PORT"); env != "" { + if p, err := strconv.Atoi(env); err == nil && p > 0 { + port = p + } + } + if env := os.Getenv("OWNCORD_TLS_MODE"); env != "" { + tlsMode = env + if env == "off" { + scheme = "http" + } + } + if env := os.Getenv("OWNCORD_TLS_DOMAIN"); env != "" { + acmeDomain = env + } + client := &http.Client{ + Timeout: 5 * time.Second, + Transport: &http.Transport{ + TLSClientConfig: healthcheckTLSConfig(tlsMode, certFile, acmeDomain), + }, + } + if port < 1 || port > 65535 { + port = 8443 + } + resp, err := client.Get(fmt.Sprintf("%s://127.0.0.1:%d/health", scheme, port)) //nolint:gosec // G704: host is hardcoded loopback; only the port comes from the operator's own config + if err != nil { + fmt.Fprintln(os.Stderr, "healthcheck: unreachable:", err) + return 1 + } + defer resp.Body.Close() //nolint:errcheck + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + fmt.Fprintf(os.Stderr, "healthcheck: status %d: %s\n", resp.StatusCode, body) + return 1 + } + return 0 +} + +// healthcheckTLSConfig builds the probe's TLS config, per TLS mode: +// +// - acme: the served cert is CA-issued for the configured domain, so +// standard WebPKI verification works — but the probe dials 127.0.0.1, so +// ServerName must be overridden to the domain or hostname verification +// fails unconditionally and the probe reports a healthy server as down. +// A stale pre-ACME data/cert.pem must NOT be pinned in this mode either; +// the pin would mismatch the served ACME leaf forever. +// - self_signed / manual: the cert can never pass WebPKI (the generated one +// has no SANs and IsCA=false), so hostname/chain checks are replaced (not +// skipped) by pinning: the presented leaf must be byte-identical to the +// local cert file. +// - anything else with no readable local cert: plain WebPKI. +func healthcheckTLSConfig(tlsMode, certFile, acmeDomain string) *tls.Config { + if tlsMode == "acme" && acmeDomain != "" { + return &tls.Config{MinVersion: tls.VersionTLS12, ServerName: acmeDomain} + } + pinned := loadPinnedCert(certFile) + if pinned == nil { + return &tls.Config{MinVersion: tls.VersionTLS12} + } + return &tls.Config{ + MinVersion: tls.VersionTLS12, + // Chain/hostname verification is replaced by the exact-match pin + // below, which is strictly stronger for a cert we hold on disk. + // VerifyConnection (not VerifyPeerCertificate) so the pin also runs + // on resumed sessions (gosec G123). + InsecureSkipVerify: true, //nolint:gosec // G402: VerifyConnection below pins the exact local certificate + VerifyConnection: func(cs tls.ConnectionState) error { + if len(cs.PeerCertificates) == 0 { + return errors.New("healthcheck: server presented no certificate") + } + if !bytes.Equal(cs.PeerCertificates[0].Raw, pinned) { + return errors.New("healthcheck: server certificate does not match " + certFile) + } + return nil + }, + } +} + +// loadPinnedCert reads the first PEM certificate block from path, returning +// its DER bytes, or nil when unavailable. +func loadPinnedCert(path string) []byte { + raw, err := os.ReadFile(path) //nolint:gosec // G304: path is the operator's own configured cert file + if err != nil { + return nil + } + block, _ := pem.Decode(raw) + if block == nil || block.Type != "CERTIFICATE" { + return nil + } + return block.Bytes +} diff --git a/Server/internal/app/http.go b/Server/internal/app/http.go new file mode 100644 index 00000000..d0395b57 --- /dev/null +++ b/Server/internal/app/http.go @@ -0,0 +1,105 @@ +package app + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "log/slog" + "net/http" + "time" + + "github.com/J3vb/OwnCord/Server/ws" +) + +// startACMEServer starts the ACME HTTP-01 challenge server when Let's Encrypt +// is configured, and returns nil otherwise. The acme stage; the http stage +// below owns shutting both servers down, in the order the drain requires. +func startACMEServer(log *slog.Logger, httpHandler http.Handler) *http.Server { + var acmeSrv *http.Server + if httpHandler != nil { + acmeSrv = &http.Server{ + Addr: ":80", + Handler: httpHandler, + ReadTimeout: 10 * time.Second, + WriteTimeout: 10 * time.Second, + } + go func() { + log.Info("ACME HTTP challenge server starting on :80") + if err := serveWithBindRetry(log, "acme-http", acmeSrv.ListenAndServe); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Error("ACME HTTP server error — HTTP-01 challenges and certificate renewal will fail until the next restart", "error", err) + } + }() + } + + return acmeSrv +} + +// serveAndWait starts the listener and blocks until it fails or a +// shutdown or restart signal arrives. App.serve calls it after every stage +// is up. +func serveAndWait(ctx context.Context, log *slog.Logger, rc *RestartCoordinator, srv *http.Server, tlsCfg *tls.Config, addr, version string) error { + // Start serving in a goroutine. + serveErr := make(chan error, 1) + go func() { + log.Info("server starting", "addr", addr, "tls", tlsCfg != nil, "version", version) + + err := serveWithBindRetry(log, "server", func() error { + if tlsCfg != nil { + return srv.ListenAndServeTLS("", "") + } + return srv.ListenAndServe() + }) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + serveErr <- err + } + close(serveErr) + }() + + // Wait for shutdown signal or server error. + select { + case err := <-serveErr: + if err != nil { + return fmt.Errorf("server error: %w", err) + } + case <-ctx.Done(): + if reason, ok := rc.Requested(); ok { + log.Info("restart requested, draining connections (30s timeout)", "reason", reason) + } else { + log.Info("shutdown signal received, draining connections (30s timeout)") + } + } + + return nil +} + +// shutdownServers performs the ordered graceful shutdown: the ACME +// server, then in-flight HTTP handlers, then the WebSocket hub. Extracted +// from run. +func shutdownServers(shutdownCtx context.Context, log *slog.Logger, srv, acmeSrv *http.Server, hub *ws.Hub) error { + if acmeSrv != nil { + if err := acmeSrv.Shutdown(shutdownCtx); err != nil { + log.Warn("ACME HTTP server shutdown error", "error", err) + } + } + + // Drain in-flight HTTP handlers FIRST: their broadcasts must still reach + // a live hub (and the event persister) or the frames vanish from the + // replay/event store across the restart. Shutdown does not wait on + // hijacked WebSocket connections, so the hub's own stop below is not + // delayed by connected clients — they get the restart notice right after + // the drain instead of right before it. + shutdownErr := srv.Shutdown(shutdownCtx) + + // Stop the WebSocket hub: notify clients, stop LiveKit, close all client + // connections. Threaded with the same 30s budget the operator was told + // about — the notice sleep and LiveKit stop count against it rather than + // extending it. + hub.GracefulStopContext(shutdownCtx) + + if shutdownErr != nil { + return fmt.Errorf("graceful shutdown: %w", shutdownErr) + } + + return nil +} diff --git a/Server/internal/app/hub.go b/Server/internal/app/hub.go new file mode 100644 index 00000000..fa5f88d2 --- /dev/null +++ b/Server/internal/app/hub.go @@ -0,0 +1,117 @@ +package app + +import ( + "log/slog" + "net/url" + + "github.com/J3vb/OwnCord/Server/api" + "github.com/J3vb/OwnCord/Server/auth" + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/plugin" + "github.com/J3vb/OwnCord/Server/service" + "github.com/J3vb/OwnCord/Server/ws" +) + +// StartRuntime builds the collaborators the hub and the router share, applies +// every pre-Run hub setter, and starts the hub's dispatch goroutine. +// +// Before B3-3 this lived inside api.NewRouter (the ws.NewHub call at +// router.go:106 and the plugin and LiveKit setters at :325-360), while +// main.go set the event persister and the event store after NewRouter +// returned — two owners of one hub, with nothing checking that the required +// collaborators were present before Run started. There is one owner now, and +// one place B3-4 has to change when the required setters become validated +// constructor options. +// +// The limiter and the service layer are built here rather than in the router +// because the hub needs the SAME instances: the limiter persists auth +// lockouts and the service layer holds the permission cache the hub +// invalidates, so a second copy of either would silently split that state. +// +// It starts the hub, so every caller must stop it — App.Close does, through +// the "hub" close step; api's tests rely on the goleak ignore for +// ws.(*Hub).Run.func1 exactly as they did when NewRouter started it. +func StartRuntime(cfg *config.Config, database *db.DB, pluginRegistry *plugin.Registry) api.Runtime { + // Lockouts are persisted to the database so they survive restarts (M2). + limiter := auth.NewPersistentRateLimiter(database) + // Service layer — centralises business logic for REST and WS handlers. + // *db.DB satisfies service.Store directly. + svc := service.New(database, limiter) + + // WebSocket hub — WS does its own in-band auth, so no AuthMiddleware. + hub := ws.NewHub(database, limiter, svc) + // Replay budget knobs must land before hub.Run starts (below). + hub.ConfigureReplay(cfg.EventPersistence.ReplayRingSize, cfg.EventPersistence.ReplayColdLimit) + + wirePlugins(hub, pluginRegistry) + voiceEnabled := startVoice(cfg, hub) + + go hub.Run() + + return api.Runtime{Hub: hub, Limiter: limiter, Services: svc, VoiceEnabled: voiceEnabled} +} + +// wirePlugins wires the plugin registry and its event sink into the hub. +// Moved from api.routerPluginWiring. +func wirePlugins(hub *ws.Hub, pluginRegistry *plugin.Registry) { + // Phase C Step 9 — wire plugin registry and event sink into the hub. + // nil pluginRegistry means plugins are disabled; the hub no-ops cleanly. + if pluginRegistry != nil { + hub.SetPluginRegistry(pluginRegistry) + sink := pluginRegistry.Sink() + sink.SetBroadcaster(hub.BroadcastToChannel) + hub.SetPluginEventSink(sink) + } +} + +// startVoice creates the LiveKit client and, when OwnCord manages the +// companion process, starts it — the construction half of what +// api.routerVoiceRoutes did before B3-3. It reports whether voice is +// configured; the webhook, LiveKit health and signalling-proxy routes are +// still mounted by the router, on exactly that condition (the `lkErr == nil` +// guard, now api.Runtime.VoiceEnabled). +func startVoice(cfg *config.Config, hub *ws.Hub) bool { + // Create LiveKit client if voice config is present; voice is disabled on failure. + lk, lkErr := ws.NewLiveKitClient(&cfg.Voice) + if lkErr != nil { + slog.Warn("failed to create LiveKit client, voice disabled", "error", lkErr) + return false + } + hub.SetLiveKit(lk) + + // Optionally start a companion LiveKit process — either from a + // configured binary or via checksum-verified auto-download (the + // download happens in the background inside Start). + if cfg.Voice.LiveKitBinaryPath != "" || cfg.Voice.AutoDownloadLiveKit { + proc := ws.NewLiveKitProcess(&cfg.Voice, &cfg.TLS, cfg.Server.DataDir) + // Register the process with the hub BEFORE calling Start(), and + // keep it registered even if Start() fails (OC-0019). The only + // consumer of h.lkProcess is the voice_join guard + // (`h.lkProcess != nil && !h.lkProcess.IsRunning()`), which reads + // a nil process as "LiveKit is externally managed, don't check". + // That is the wrong reading here: OwnCord was told to manage + // LiveKit and failed to launch it, so joins must fail closed via + // IsRunning() == false, not be waved through with no SFU + // running. IsRunning() is false for a proc whose Start() never + // got as far as spawning cmd, and Hub.Stop's lkProcess.Stop() is + // safe to call on a never-started proc. + hub.SetLiveKitProcess(proc) + if startErr := proc.Start(); startErr != nil { + slog.Error("failed to start LiveKit process", "error", startErr) + } + return true + } + + // Warn if LiveKit is externally managed and webhook may be blocked by admin CIDRs. + lkHost := "" + if u, parseErr := url.Parse(cfg.Voice.LiveKitURL); parseErr == nil { + lkHost = u.Hostname() + } + if lkHost != "" && lkHost != "localhost" && lkHost != "127.0.0.1" && lkHost != "::1" { + slog.Warn("LiveKit is externally managed but webhook endpoint is admin-IP-restricted — "+ + "add the LiveKit server's IP to livekit_webhook_allowed_cidrs or webhooks will be silently dropped", + "livekit_host", lkHost) + } + return true +} diff --git a/Server/internal/app/lifecycle.go b/Server/internal/app/lifecycle.go new file mode 100644 index 00000000..8f0252a8 --- /dev/null +++ b/Server/internal/app/lifecycle.go @@ -0,0 +1,323 @@ +// Package app owns the server process lifecycle: it opens and migrates the +// database, starts telemetry, plugins, the HTTP router and hub, event +// persistence, the audit writer, the maintenance workers, ACME and the +// listener, serves until a shutdown or restart signal, and tears every stage +// back down through one composite close. main keeps only the CLI dispatch, +// the log sinks and the restart handoff (B3-3). +package app + +import ( + "context" + "fmt" + "io" + stdlog "log" + "net/http" + "os" + "os/signal" + "syscall" + "time" + + "github.com/J3vb/OwnCord/Server/api" + "github.com/J3vb/OwnCord/Server/auth" +) + +// shutdownBudget is the total time Close is given, and the same 30 seconds +// the operator is told about in the "draining connections" log line. The +// per-stage timeouts inside the individual stop steps are budgets of their +// own and are unchanged by the move. +const shutdownBudget = 30 * time.Second + +// stage is one start step, in start order. The name is what a failure is +// reported as, so an operator reading `starting audit-writer: ...` knows +// exactly how far the boot got — and it is the key the failure-injection +// test selects on. +type stage struct { + name string + start func() error +} + +// stages is the start sequence. Close walks the steps these register in +// reverse, so this list IS the shutdown order read backwards. Two orderings +// here are load-bearing rather than incidental: +// +// - the database opens before the audit writer and event persistence start, +// so both stop before the handle closes; +// - ACME and the HTTP server start AFTER the maintenance loop, so the +// reverse walk drains in-flight HTTP handlers (whose broadcasts must +// still reach a live hub) before anything else is stopped — which is the +// order run()'s explicit shutdown call used to impose by hand. +func (a *App) stages() []stage { + return []stage{ + {"data-dir", a.startDataDir}, + {"tls", a.startTLS}, + {"database", a.startDatabase}, + {"migrate", a.startMigrate}, + {"telemetry", a.startTelemetry}, + {"plugins", a.startPlugins}, + {"hub", a.startHub}, + {"router", a.startRouter}, + {"event-persistence", a.startEventPersistence}, + {"audit-writer", a.startAuditWriter}, + {"maintenance", a.startMaintenance}, + {"acme", a.startACME}, + {"http", a.startHTTP}, + {"signals", a.startSignals}, + } +} + +// Run starts every stage in order, serves until the listener fails or a +// shutdown or restart signal arrives, and then closes every started stage in +// the reverse order. Close runs on EVERY return path — a failed start, a +// serve error and a clean shutdown alike — which is what keeps a supervised +// LiveKit process from being orphaned by an early return (OC-0027). +// +// A start or serve error is what Run reports; a teardown error surfaces only +// when there is no earlier one to report. +func (a *App) Run(ctx context.Context) (err error) { + a.rootCtx = ctx + // WithoutCancel: bgCtx takes ctx's values but NOT its cancellation. The + // event persister, the audit writer and the maintenance loop run under + // it, and Close drains in-flight HTTP handlers FIRST precisely so their + // broadcasts and audit records still reach live consumers — inheriting + // cancellation would kill all three the instant a caller cancelled, + // before that drain, and would make caller-context shutdown behave + // differently from the SIGTERM and restart paths, which cancel only the + // serve context. Cancelling ctx stops SERVING (serveCtx descends from it + // in startSignals); when the background work stops is Close's decision. + bgCtx, bgCancel := context.WithCancel(context.WithoutCancel(ctx)) + // The deferred cancel is only a hard backstop for an App whose Close is + // somehow never reached; the ordered teardown cancels bgCtx through the + // first-registered close step, which the reverse walk runs LAST — after + // the persistence and maintenance steps have joined their goroutines. + defer bgCancel() + a.bgCtx, a.bgCancel = bgCtx, bgCancel + a.onClose("background-context", func(context.Context) error { + bgCancel() + return nil + }) + + defer func() { + // WithoutCancel, not Background: teardown must run its full budget + // even when the caller's context is what ended the server, while + // still carrying whatever values that context holds. + closeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), shutdownBudget) + defer cancel() + if closeErr := a.Close(closeCtx); closeErr != nil && err == nil { + err = closeErr + } + }() + + if startErr := a.start(); startErr != nil { + return startErr + } + return a.serve() +} + +// start brings the stages up in order, stopping at the first failure — the +// stages that did come up are already registered with Close, which Run runs +// regardless. +func (a *App) start() error { + removeOldBinary(a.log) + + for _, st := range a.stages() { + if st.name == a.failStage { + return fmt.Errorf("starting %s: %w", st.name, errStageInjected) + } + if err := st.start(); err != nil { + return fmt.Errorf("starting %s: %w", st.name, err) + } + } + return nil +} + +// serve blocks until the listener fails or the signal context is cancelled. +// The graceful shutdown that used to follow it inline is now the http stage's +// close step, so it happens on the error path too. +func (a *App) serve() error { + return serveAndWait(a.serveCtx, a.log, a.deps.Restart, a.srv, a.tlsCfg, a.addr, a.deps.Version) +} + +// startDataDir creates the configured data directory and warns about the +// volumes the server writes to. Nothing to close. +func (a *App) startDataDir() error { + return prepareDataDir(a.log, a.cfg) +} + +// startTLS resolves the serving certificate (and, in acme mode, the HTTP-01 +// challenge handler the acme stage serves) and prints the startup banner +// above the init logs, as run() did. Nothing to close. +func (a *App) startTLS() error { + tlsResult, err := auth.LoadOrGenerate(a.cfg.TLS) + if err != nil { + return fmt.Errorf("configuring TLS: %w", err) + } + a.tlsCfg = tlsResult.TLSConfig + a.httpHandler = tlsResult.HTTPHandler + + printBanner(a.cfg, a.deps.Version, a.tlsCfg != nil) + return nil +} + +// startDatabase opens the handle and registers its close BEFORE migrating, +// so a migration failure still releases it and its process lock. +func (a *App) startDatabase() error { + database, err := openDatabase(a.cfg) + if err != nil { + return err + } + a.database = database + a.onClose("database", func(context.Context) error { return database.Close() }) + return nil +} + +// startMigrate runs the migrations and clears state left by a previous run. +func (a *App) startMigrate() error { + return initDatabase(a.log, a.cfg, a.database, a.deps.Restart) +} + +// startTelemetry initialises OpenTelemetry. Its shutdown is bounded by its +// own 5s budget inside the returned step. +func (a *App) startTelemetry() error { + stop := initTelemetry(a.log, a.cfg) + a.onClose("telemetry", func(context.Context) error { + stop() + return nil + }) + return nil +} + +// startPlugins constructs the plugin runtime before the router, so the router +// can wire the live registry into the plugin admin handler. A nil registry is +// the disabled case and has nothing to close. +func (a *App) startPlugins() error { + a.plugins = initPlugins(a.bgCtx, a.log, a.cfg, a.database) + a.onClose("plugins", func(ctx context.Context) error { + closePlugins(ctx, a.plugins) + return nil + }) + return nil +} + +// startHub builds the hub and the collaborators it shares with the router, +// applies every pre-Run setter and starts the dispatch goroutine — B3-3 moved +// all of that out of api.NewRouter so the hub has exactly one owner. +// +// Its close step is GracefulStopContext, the only caller of +// LiveKitProcess.Stop and what closes the dispatch goroutine. gracefulOnce +// makes it idempotent alongside the stop the http step performs on the normal +// path, so it is reached on every return from Run and a supervised +// livekit-server process is never orphaned (OC-0027). +func (a *App) startHub() error { + a.runtime = StartRuntime(a.cfg, a.database, a.plugins) + a.hub = a.runtime.Hub + a.onClose("hub", func(ctx context.Context) error { + a.runtime.Hub.GracefulStopContext(ctx) + return nil + }) + return nil +} + +// startRouter mounts the HTTP handler over the already-built collaborators. +// Its close step stops the router's own background goroutine (rate-limiter +// cleanup); the hub it serves is stopped by the step above. +func (a *App) startRouter() error { + router, cleanup := api.NewRouter(a.cfg, a.database, a.deps.Version, a.deps.LogBuf, a.plugins, a.runtime) + a.router = router + a.onClose("router", func(context.Context) error { + cleanup() + return nil + }) + return nil +} + +// startEventPersistence seeds the hub's replay state and, when persistence is +// enabled, starts the persister and pruner. Its stop cancels bgCtx and joins +// the pruner, which the reverse walk runs before database.Close so no prune +// is mid-query against a closing pool. +func (a *App) startEventPersistence() error { + a.persister, a.prunerDone = startEventPersister(a.bgCtx, a.log, a.cfg, a.hub, a.database) + a.onClose("event-persistence", func(ctx context.Context) error { + stopEventPersister(ctx, a.log, a.bgCancel, a.persister, a.prunerDone) + return nil + }) + return nil +} + +// startAuditWriter moves audit-log INSERTs off the request path. It starts +// after the database opens, so the reverse walk drains its queue while the +// handle is still live. +func (a *App) startAuditWriter() error { + a.auditWriter = newAuditWriter(a.bgCtx, a.database) + a.onClose("audit-writer", func(ctx context.Context) error { + stopAuditWriter(ctx, a.auditWriter) + return nil + }) + return nil +} + +// startMaintenance starts the periodic purge of expired sessions, scheduled +// backups and orphaned attachments. Its stop joins the loop, bounded, so an +// in-flight tick (which can hold the writer — scheduled backups run VACUUM +// INTO) is not still using the database when the handle closes. +func (a *App) startMaintenance() error { + stop := startMaintenanceLoop(a.bgCtx, a.log, a.cfg, a.database) + a.onClose("maintenance", func(context.Context) error { + stop() + return nil + }) + return nil +} + +// startACME serves the HTTP-01 challenge and the HTTP→HTTPS redirect on :80 +// when Let's Encrypt is configured, and is a no-op otherwise. It has no close +// step of its own: the http step below shuts both servers down together, in +// the order the drain requires. +func (a *App) startACME() error { + a.acmeSrv = startACMEServer(a.log, a.httpHandler) + return nil +} + +// startHTTP builds the main server and registers the ordered graceful +// shutdown — ACME, then in-flight HTTP handlers, then the hub. It starts last +// of the real stages so that shutdown is the FIRST thing the reverse walk +// does: in-flight handlers' broadcasts must still reach a live hub and event +// persister, or the frames vanish from the replay store across the restart. +func (a *App) startHTTP() error { + a.addr = fmt.Sprintf(":%d", a.cfg.Server.Port) + a.srv = &http.Server{ + Addr: a.addr, + Handler: a.router, + TLSConfig: a.tlsCfg, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, + ErrorLog: stdlog.New(io.Discard, "", 0), // suppress TLS handshake noise + } + a.onClose("http", func(ctx context.Context) error { + return shutdownServers(ctx, a.log, a.srv, a.acmeSrv, a.hub) + }) + return nil +} + +// startSignals arms the shutdown context. The coordinator's context is the +// parent, so a programmatic restart request (rc.Request) drains exactly like +// a SIGTERM — including on Windows, where a process cannot signal itself. +// Signals arriving mid-drain are swallowed until the stop step runs, same as +// on the real-signal path. +func (a *App) startSignals() error { + parent, cancelParent := context.WithCancel(a.rootCtx) + // A restart request cancels the same context a signal would, so + // rc.Request drains through the identical path — including on Windows, + // where a process cannot signal itself. AfterFunc rather than a + // goroutine so there is nothing to leak if neither ever fires. + stopWatch := context.AfterFunc(a.deps.Restart.Context(), cancelParent) + serveCtx, stopSignals := signal.NotifyContext(parent, os.Interrupt, syscall.SIGTERM) + a.serveCtx = serveCtx + a.onClose("signals", func(context.Context) error { + stopSignals() + stopWatch() + cancelParent() + return nil + }) + return nil +} diff --git a/Server/internal/app/lifecycle_failure_test.go b/Server/internal/app/lifecycle_failure_test.go new file mode 100644 index 00000000..7dc0daa8 --- /dev/null +++ b/Server/internal/app/lifecycle_failure_test.go @@ -0,0 +1,253 @@ +package app + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "strings" + "testing" + "time" + + "go.uber.org/goleak" +) + +// The lifecycle failure-injection report the B3 exit gate asks for (plan +// §B3-3 item 3). Every stage App.start brings up is made to fail in turn and +// the same four properties are asserted each time: +// +// 1. the returned error names the stage that failed; +// 2. no goroutine is left running (goleak) — the hub's dispatch goroutine, +// the event pruner, the maintenance loop and the ACME listener are all +// started by stages that may already have run; +// 3. the database handle is closed, so the SQLite process lock is released +// for the successor a restart handoff is about to start; +// 4. the listener is not left bound, so that successor can take the port. +// +// The table is generated from App.stages() rather than written out, so a new +// stage is covered the day it is added instead of the day someone remembers +// to add a row. +// +// Before B3-3 there was no single teardown to test: run() unwound through a +// LIFO defer stack, and an early return simply skipped whatever it had not +// reached. TestAppClose_* in close_test.go pins the ordering and error +// contract of the walk; this pins what the walk actually releases. + +// freePort returns a port nothing is listening on. The tiny window in which +// something else could take it is absorbed by the server's own bind retry. +func freePort(t *testing.T) int { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("probe listen: %v", err) + } + port := l.Addr().(*net.TCPAddr).Port + _ = l.Close() + return port +} + +// assertPortFree fails when anything is still listening on port — the +// "listener is not left bound" assertion. +func assertPortFree(t *testing.T, port int) { + t.Helper() + l, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + t.Errorf("port %d is still bound after Run returned: %v", port, err) + return + } + _ = l.Close() +} + +// assertReleased is properties 2-4: nothing running, nothing open, nothing +// bound. leakOpt must have been taken before the App was booted. +func assertReleased(t *testing.T, a *App, port int, leakOpt goleak.Option) { + t.Helper() + if a.database != nil { + if err := a.database.PingRead(context.Background()); err == nil { + t.Error("the database handle is still open after Run returned — the SQLite process lock is not released") + } + } + if a.hub != nil && a.hub.DispatchAlive() { + t.Error("hub dispatch is still alive after Run returned — GracefulStop was skipped, so a supervised LiveKit process would be orphaned") + } + assertPortFree(t, port) + if err := goleak.Find(leakOpt); err != nil { + t.Errorf("goroutine leaked after Run returned: %v", err) + } +} + +func TestAppRun_EveryStageFailure_ReleasesEverythingItStarted(t *testing.T) { + for _, name := range stageNames() { + t.Run(name, func(t *testing.T) { + port := freePort(t) + leakOpt := goleak.IgnoreCurrent() + a := bootTestApp(t, fmt.Sprint(port), name) + + err := a.Run(context.Background()) + if err == nil { + t.Fatalf("Run() = nil, want the injected %s failure", name) + } + if !errors.Is(err, errStageInjected) { + t.Errorf("Run() = %v, want it to wrap the injected failure", err) + } + if !strings.Contains(err.Error(), name) { + t.Errorf("Run() = %v, want an error naming the stage that failed (%s)", err, name) + } + assertReleased(t, a, port, leakOpt) + }) + } +} + +// stageNames is the start sequence by name, read off a throwaway App so the +// table above cannot drift from the real list. +func stageNames() []string { + stages := (&App{}).stages() + names := make([]string, 0, len(stages)) + for _, st := range stages { + names = append(names, st.name) + } + return names +} + +// TestAppRun_ListenerBindFailure_ReleasesEverythingItStarted is the same +// four properties for a real failure rather than an injected one: every +// stage starts, and the listener itself refuses to bind. An out-of-range +// port fails the first attempt with an error isAddrInUse does not recognise, +// so serveAndWait takes the serve-error branch immediately instead of +// retrying for ~10s. This is the path OC-0027 was about. +func TestAppRun_ListenerBindFailure_ReleasesEverythingItStarted(t *testing.T) { + leakOpt := goleak.IgnoreCurrent() + a := bootTestApp(t, "99999", "") + + err := a.Run(context.Background()) + if err == nil { + t.Fatal("Run() = nil, want a listener error for an out-of-range port") + } + if !strings.Contains(err.Error(), "server error") { + t.Errorf("Run() = %v, want the serve error", err) + } + if a.hub == nil { + t.Fatal("every stage runs before the listener binds, so the hub must have been built") + } + // Port 99999 was never bindable, so only the release assertions apply. + if pingErr := a.database.PingRead(context.Background()); pingErr == nil { + t.Error("the database handle is still open after Run returned") + } + if a.hub.DispatchAlive() { + t.Error("hub dispatch is still alive after Run returned") + } + if leakErr := goleak.Find(leakOpt); leakErr != nil { + t.Errorf("goroutine leaked after a listener failure: %v", leakErr) + } +} + +// waitForHealth blocks until the server answers /health on port, failing the +// test if Run exits first or the server never comes up. +func waitForHealth(t *testing.T, port int, runErr <-chan error) { + t.Helper() + healthURL := fmt.Sprintf("http://127.0.0.1:%d/health", port) + for deadline := time.Now().Add(15 * time.Second); time.Now().Before(deadline); { + resp, healthErr := http.Get(healthURL) //nolint:gosec // G107: loopback URL built from the test's own port + if healthErr == nil { + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + return + } + select { + case err := <-runErr: + t.Fatalf("Run exited before serving: %v", err) + case <-time.After(50 * time.Millisecond): + } + } + t.Fatal("server never became reachable on /health") +} + +// TestAppRun_ContextCancel_DrainsAndReleases is the control the injected rows +// need: the same four properties on the path where nothing fails at all. Run +// serves for real, the caller's context is cancelled (the same cancellation a +// SIGTERM or a restart request delivers), and the composite close still +// releases the handle, the hub and the port — with a nil error, so the +// assertions above are not passing merely because something went wrong. +func TestAppRun_ContextCancel_DrainsAndReleases(t *testing.T) { + port := freePort(t) + leakOpt := goleak.IgnoreCurrent() + a := bootTestApp(t, fmt.Sprint(port), "") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + runErr := make(chan error, 1) + go func() { runErr <- a.Run(ctx) }() + + waitForHealth(t, port, runErr) + + cancel() + select { + case err := <-runErr: + if err != nil { + t.Fatalf("Run() after the context was cancelled = %v, want nil (clean drain)", err) + } + case <-time.After(60 * time.Second): + t.Fatal("Run did not return after its context was cancelled") + } + + assertReleased(t, a, port, leakOpt) +} + +// TestAppRun_CallerCancel_KeepsBackgroundWorkersAliveThroughTheDrain pins the +// one thing the HTTP-first close order exists for: when Close drains +// in-flight HTTP handlers, the consumers their work feeds — the event +// persister, the audit writer and the maintenance loop, all running under +// bgCtx — must still be alive, or those handlers' broadcasts never reach the +// replay/event store and their audit records are dropped. +// +// run() got this for free by rooting bgCtx at context.Background(): only +// bgCancel ever ended it, and the ordered teardown called that last. Deriving +// bgCtx from Run's own ctx instead would cancel all three the instant a +// caller cancelled — before the drain — and would make caller-context +// shutdown behave differently from the SIGTERM and restart paths, which +// cancel only the serve context. So bgCtx inherits ctx's values but not its +// cancellation, and this asserts that at the exact point it matters. +func TestAppRun_CallerCancel_KeepsBackgroundWorkersAliveThroughTheDrain(t *testing.T) { + port := freePort(t) + a := bootTestApp(t, fmt.Sprint(port), "") + + // Record what bgCtx looked like as each close step was about to run. + bgErrAt := map[string]error{} + a.onCloseStep = func(stage string) { bgErrAt[stage] = a.bgCtx.Err() } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + runErr := make(chan error, 1) + go func() { runErr <- a.Run(ctx) }() + waitForHealth(t, port, runErr) + + cancel() + select { + case err := <-runErr: + if err != nil { + t.Fatalf("Run() after the context was cancelled = %v, want nil (clean drain)", err) + } + case <-time.After(60 * time.Second): + t.Fatal("Run did not return after its context was cancelled") + } + + // The steps that must find bgCtx still live, in the order Close runs them. + for _, stage := range []string{"signals", "http", "maintenance", "audit-writer"} { + if err, ran := bgErrAt[stage]; !ran { + t.Errorf("the %q close step never ran", stage) + } else if err != nil { + t.Errorf("bgCtx was already cancelled (%v) when the %q close step ran — the event persister, audit writer and maintenance loop had exited before the HTTP drain, so an in-flight handler's broadcasts and audit records are dropped", err, stage) + } + } + // event-persistence is the step that cancels bgCtx and joins the pruner, + // so from there down it is expected to be done. + if err, ran := bgErrAt["database"]; !ran { + t.Error(`the "database" close step never ran`) + } else if err == nil { + t.Error("bgCtx was still live when the database closed — the event pruner and maintenance loop had not been joined, so a query could still be in flight against a closing pool") + } +} diff --git a/Server/main_test.go b/Server/internal/app/lifecycle_test.go similarity index 92% rename from Server/main_test.go rename to Server/internal/app/lifecycle_test.go index ba44942e..6f8556f5 100644 --- a/Server/main_test.go +++ b/Server/internal/app/lifecycle_test.go @@ -1,4 +1,4 @@ -package main +package app import ( "context" @@ -25,13 +25,13 @@ import ( // hub.GracefulStop() (the only caller of LiveKitProcess.Stop(), and what // closes the hub's dispatch goroutine) is a plain statement reached only on // the graceful-shutdown path. The serve-error branch — `case err := -// <-serveErr: ... return fmt.Errorf(...)` — returns from run() before ever +// <-serveErr: ... return fmt.Errorf(...)` — returns from Run before ever // reaching it, so the hub's `go hub.Run()` dispatch goroutine (started by // api.NewRouter) is left running, and in production the companion // livekit-server process it owns is left running with it. // // An out-of-range port fails the first listen attempt with an error that -// isAddrInUse does not recognize, so run() takes the servErr branch +// isAddrInUse does not recognize, so Run takes the servErr branch // immediately instead of retrying for ~10s. func TestRun_ServeErrorReturn_StopsHubDispatchGoroutine(t *testing.T) { t.Chdir(t.TempDir()) @@ -46,19 +46,19 @@ func TestRun_ServeErrorReturn_StopsHubDispatchGoroutine(t *testing.T) { leakOpt := goleak.IgnoreCurrent() - rc := newRestartCoordinator(time.Hour, nil) - if err := run(log, logBuf, levelVar, rc); err == nil { - t.Fatal("expected run() to return an error for an out-of-range port") + rc := NewRestartCoordinator(time.Hour, nil) + if err := runApp(log, logBuf, levelVar, rc); err == nil { + t.Fatal("expected Run to return an error for an out-of-range port") } if _, requested := rc.Requested(); requested { t.Error("no restart was requested, but the coordinator reports one") } // hub.Run's dispatch goroutine only exits once hub.stop is closed, which - // only happens inside hub.GracefulStop(). If run() returned without + // only happens inside hub.GracefulStop(). If Run returned without // calling it, this goroutine is still alive here. if err := goleak.Find(leakOpt); err != nil { - t.Fatalf("hub dispatch goroutine (and, in production, its LiveKit process) leaked after run() returned early: %v", err) + t.Fatalf("hub dispatch goroutine (and, in production, its LiveKit process) leaked after Run returned early: %v", err) } } @@ -75,7 +75,7 @@ func TestRun_ServeErrorReturn_StopsHubDispatchGoroutine(t *testing.T) { // // This seeds a DB with a contiguous run of persisted events (simulating a // prior boot that reached seq 520), then calls seedHubReplayState exactly as -// run() does, then reconnects a client with last_seq=500 (<= the restored +// Run does, then reconnects a client with last_seq=500 (<= the restored // max) and asserts the resume is forced onto the full-ready tier. Before the // fix, last_seq=500 converges via the ordinary DB cold-tier replay instead // (the persisted run 501..520 is contiguous and complete), silently proving @@ -119,7 +119,7 @@ func TestSeedHubReplayState_ForcesFullResyncForOfflineClient(t *testing.T) { go hub.Run() defer hub.Stop() - // The exact startup call run() makes once event persistence is enabled — + // The exact startup call Run makes once event persistence is enabled — // no ring-buffer events are pushed, so a resuming client's replay can // only be satisfied via the DB cold tier or forced full. log := slog.New(slog.NewTextHandler(io.Discard, nil)) @@ -262,8 +262,8 @@ func TestRunStartEventPersistence_DisabledMode_StaleLastSeqForcesFullResync(t *t // scheme is in effect (raw 1..N pre-fix, or a seeded floor post-fix). --- hubOld := ws.NewHub(database, limiter, nil) go hubOld.Run() - if persister, prunerDone := runStartEventPersistence(ctx, log, cfg, hubOld, database); persister != nil || prunerDone != nil { - t.Fatalf("runStartEventPersistence with Enabled=false: want (nil, nil), got (%v, %v)", persister, prunerDone) + if persister, prunerDone := startEventPersister(ctx, log, cfg, hubOld, database); persister != nil || prunerDone != nil { + t.Fatalf("startEventPersister with Enabled=false: want (nil, nil), got (%v, %v)", persister, prunerDone) } for range 40 { hubOld.BroadcastToAll([]byte(`{"type":"broadcast"}`)) @@ -278,8 +278,8 @@ func TestRunStartEventPersistence_DisabledMode_StaleLastSeqForcesFullResync(t *t hubNew := ws.NewHub(database, limiter, nil) go hubNew.Run() defer hubNew.Stop() - if persister, prunerDone := runStartEventPersistence(ctx, log, cfg, hubNew, database); persister != nil || prunerDone != nil { - t.Fatalf("runStartEventPersistence with Enabled=false: want (nil, nil), got (%v, %v)", persister, prunerDone) + if persister, prunerDone := startEventPersister(ctx, log, cfg, hubNew, database); persister != nil || prunerDone != nil { + t.Fatalf("startEventPersister with Enabled=false: want (nil, nil), got (%v, %v)", persister, prunerDone) } // Other clients reconnect first and push hub B's own new epoch forward by diff --git a/Server/listen_retry.go b/Server/internal/app/listen_retry.go similarity index 98% rename from Server/listen_retry.go rename to Server/internal/app/listen_retry.go index a89faf9a..f8c13dad 100644 --- a/Server/listen_retry.go +++ b/Server/internal/app/listen_retry.go @@ -1,4 +1,4 @@ -package main +package app import ( "errors" diff --git a/Server/internal/app/maintenance.go b/Server/internal/app/maintenance.go new file mode 100644 index 00000000..5ff102ae --- /dev/null +++ b/Server/internal/app/maintenance.go @@ -0,0 +1,114 @@ +package app + +import ( + "context" + "log/slog" + "time" + + "github.com/J3vb/OwnCord/Server/admin" + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/storage" +) + +// startMaintenanceLoop starts the periodic maintenance loop and returns the +// stop step the maintenance stage registers with App.Close. +func startMaintenanceLoop(bgCtx context.Context, log *slog.Logger, cfg *config.Config, database *db.DB) func() { + // Periodically purge expired sessions and orphaned attachments. + fileStorage, fileStorageErr := storage.New(cfg.Upload.StorageDir, cfg.Upload.MaxSizeMB) + if fileStorageErr != nil { + log.Warn("failed to create file storage for maintenance; orphan file cleanup disabled", "error", fileStorageErr) + } + + stopMaintenance := make(chan struct{}) + maintenanceDone := make(chan struct{}) + go maintenanceLoop(bgCtx, log, database, fileStorage, stopMaintenance, maintenanceDone) + + return func() { + // Backstop for early returns below (see hub.GracefulStop defer above), + // and a bounded join so an in-flight tick (which can hold the writer — + // scheduled backups run VACUUM INTO) isn't still using the database + // while the LIFO-later Close defer tears it down. + close(stopMaintenance) + select { + case <-maintenanceDone: + case <-time.After(5 * time.Second): + log.Warn("maintenance loop did not exit before shutdown timeout") + } + } +} + +// maintenanceLoop is the periodic maintenance goroutine started by +// startMaintenanceLoop. +func maintenanceLoop(bgCtx context.Context, log *slog.Logger, database *db.DB, fileStorage *storage.Storage, stopMaintenance, maintenanceDone chan struct{}) { + defer close(maintenanceDone) + ticker := time.NewTicker(15 * time.Minute) + defer ticker.Stop() + consecutiveFailures := 0 + const maxConsecutiveFailures = 5 + for { + select { + case <-ticker.C: + if consecutiveFailures >= maxConsecutiveFailures { + log.Error("maintenance loop: circuit breaker open, skipping tick", + "consecutive_failures", consecutiveFailures) + // Reset after one skip to allow retry next tick. + consecutiveFailures = maxConsecutiveFailures - 1 + continue + } + + if maintenanceTick(bgCtx, log, database, fileStorage) { + consecutiveFailures++ + } else { + consecutiveFailures = 0 + } + case <-stopMaintenance: + return + } + } +} + +// maintenanceTick runs one maintenance pass and reports whether any step +// of it failed. +func maintenanceTick(bgCtx context.Context, log *slog.Logger, database *db.DB, fileStorage *storage.Storage) bool { + tickFailed := false + if err := database.DeleteExpiredSessions(bgCtx); err != nil { + log.Warn("failed to delete expired sessions", "error", err) + tickFailed = true + } + + // Scheduled backups + retention pruning, driven by the + // backup_schedule / backup_retention admin settings. + if err := admin.MaintainBackups(bgCtx, database); err != nil { + log.Warn("backup maintenance failed", "error", err) + tickFailed = true + } + + // Clean up orphaned attachments (uploaded but never linked to a message). + // + // Skipped entirely with no file storage configured: the delete is + // atomic (row goes the instant it's selected, by design — see + // db/attachment_queries.go), so with fileStorage nil the returned + // stored_as names — the only remaining handle on those blobs — + // would just be discarded and the files stranded on disk with no + // query left able to name them. Leaving the rows in place keeps + // them reclaimable once storage is available again. + if fileStorage != nil { + cutoff := time.Now().Add(-1 * time.Hour) + orphanFiles, orphanErr := database.DeleteOrphanedAttachments(bgCtx, cutoff) + if orphanErr != nil { + log.Warn("failed to delete orphaned attachments", "error", orphanErr) + tickFailed = true + } else if len(orphanFiles) > 0 { + // Best-effort file cleanup. + for _, filename := range orphanFiles { + if delErr := fileStorage.Delete(filename); delErr != nil { + log.Warn("failed to delete orphan file", "file", filename, "error", delErr) + } + } + log.Info("cleaned up orphaned attachments", "count", len(orphanFiles)) + } + } + + return tickFailed +} diff --git a/Server/internal/app/persistence.go b/Server/internal/app/persistence.go new file mode 100644 index 00000000..314d6509 --- /dev/null +++ b/Server/internal/app/persistence.go @@ -0,0 +1,195 @@ +package app + +import ( + "context" + "errors" + "log/slog" + "strconv" + "time" + + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/ws" +) + +// startEventPersister starts the event persister and pruner, returning +// both as (nil, nil) when event persistence is disabled. +// +// seedHubReplayState runs unconditionally (whenever hub is non-nil), NOT +// gated on cfg.EventPersistence.Enabled: it seeds the hub's seq counter from +// a persisted floor even in ring-buffer-only mode, which is what closes +// OC-0210 — see its doc comment. +func startEventPersister(bgCtx context.Context, log *slog.Logger, cfg *config.Config, hub *ws.Hub, database *db.DB) (*ws.EventPersister, <-chan struct{}) { + if hub == nil { + return nil, nil + } + + seedHubReplayState(bgCtx, hub, database, log) + + if !cfg.EventPersistence.Enabled { + return nil, nil + } + + persister := ws.NewEventPersister( + database, + 4096, + cfg.EventPersistence.BatchSize, + time.Duration(cfg.EventPersistence.BatchFlushMs)*time.Millisecond, + ) + persister.Start(bgCtx) + hub.SetEventPersister(persister) + hub.SetEventStore(database) + + retention := time.Duration(cfg.EventPersistence.RetentionHours) * time.Hour + prunerInterval := time.Duration(cfg.EventPersistence.PrunerIntervalMinutes) * time.Minute + prunerDone := ws.StartEventPruner(bgCtx, database, retention, prunerInterval) + + return persister, prunerDone +} + +// stopEventPersister drains the event persister and pruner. Registered +// unconditionally, so a nil persister is the disabled case and must leave +// bgCtx alone — Run's backstop close step cancels it instead. ctx is +// App.Close's shutdown budget; the 5s cap is this step's share of it. +func stopEventPersister(ctx context.Context, log *slog.Logger, bgCancel context.CancelFunc, persister *ws.EventPersister, prunerDone <-chan struct{}) { + if persister == nil { + return + } + + stopCtx, stopCancel := context.WithTimeout(ctx, 5*time.Second) + defer stopCancel() + persister.Stop(stopCtx) + // Cancel the shared background context and JOIN the pruner before + // the (LIFO-later) database.Close defer runs, so no prune is still + // mid-query against a closing pool. Bounded: a stuck prune delays + // shutdown by at most the timeout, then Close proceeds anyway. + bgCancel() + select { + case <-prunerDone: + case <-stopCtx.Done(): + log.Warn("event pruner did not exit before shutdown timeout") + } +} + +// newAuditWriter installs the async audit writer: audit-log INSERTs move off +// the request path, and a background goroutine batches the writes. Paths that +// never install a writer — the token CLI, tests — keep the synchronous +// behaviour. It starts after the database opens, so App.Close drains its +// queue while the handle is still live. +func newAuditWriter(bgCtx context.Context, database *db.DB) *db.AuditWriter { + auditWriter := db.NewAuditWriter(database, 1024, 50, 100*time.Millisecond) + auditWriter.Start(bgCtx) + database.SetAuditWriter(auditWriter) + + return auditWriter +} + +// stopAuditWriter drains the async audit writer, within its share of +// App.Close's shutdown budget. +func stopAuditWriter(ctx context.Context, auditWriter *db.AuditWriter) { + stopCtx, stopCancel := context.WithTimeout(ctx, 5*time.Second) + defer stopCancel() + auditWriter.Stop(stopCtx) +} + +// wsSeqFloorSettingKey is the generic settings-table key (see db.GetSetting / +// db.SetSetting) seedHubSeqFloor persists its reserved floor under. +const wsSeqFloorSettingKey = "ws_seq_floor" + +// wsSeqFloorReserve is the block seedHubSeqFloor reserves above the persisted +// floor on every single boot (OC-0210). It only has to exceed the number of +// hub-sequenced broadcasts any one boot could plausibly emit before its own +// next restart — comfortably true at 1e9 for a self-hosted chat server — so +// this leaves an enormous safety margin while uint64's range still allows +// billions of restarts before the floor could ever wrap. +const wsSeqFloorReserve = 1_000_000_000 + +// seedHubReplayState seeds the hub's monotonic seq counter at startup from +// two independent, composable sources — both go through hub.SeedSeq, which +// only ever moves h.seq forward (CAS-max), so it doesn't matter which of the +// two runs first or whether either is available: +// +// 1. seedHubSeqFloor (below) reserves and persists a fresh block of seq +// space on every boot, regardless of whether event persistence is +// enabled. This is what closes OC-0210: previously this function did +// nothing at all when event_persistence.enabled is false (the +// documented "ring-buffer-only behaviour", config.go's +// EventPersistenceConfig.Enabled), so every boot's h.seq — and +// therefore its ring buffer's first entries — started back at 0/1. A +// client reconnecting with a last_seq remembered from a PRIOR boot +// could then coincidentally land inside the new boot's own live ring +// window: EventRingBuffer.EventsSinceFiltered has no way to tell that +// watermark apart from a legitimate one from this boot, and would +// silently serve a partial cross-epoch replay as if it were an +// ordinary resume. Seeding a floor far above anything a single boot +// could reach guarantees every previous boot's real seq values now sit +// below the new ring buffer's oldest entry, so a stale last_seq is +// correctly rejected by the pre-existing "afterSeq <= oldestSeq" guard +// in ringbuffer.go and falls through to a full ready instead +// (serve.go's handleReconnect, the `events == nil` branch) — the same +// path any other unrecoverable resume already takes, with no protocol +// change required. +// 2. When event persistence is enabled and the events table has history, +// MAX(events.seq) is exact (not a heuristic reserve) and naturally +// wins if it is the higher of the two. This branch is also what forces +// the paired visibilityChangeSeq watermark forward via +// MarkVisibilityChanged: h.seq is restored here, but the watermark +// that tells a resuming client whether a channel-visibility change +// happened since its last_seq (visibilityChangeSeq) is in-memory only +// and always starts at 0 on a fresh process — see +// ws/hub_events.go's mustFullResync. Channel-visibility changes made to +// an offline client (RefreshChannelVisibility, revokeUnreadableChannels) +// are sent as targeted, unsequenced messages that are never written to +// the events table, so replay can never recover them. Without the +// MarkVisibilityChanged call below, a client resuming with last_seq at +// or before the pre-restart max would sail straight through +// mustFullResync's zeroed watermark and could silently miss a +// visibility change it should have converged on. +func seedHubReplayState(ctx context.Context, hub *ws.Hub, database *db.DB, log *slog.Logger) { + seedHubSeqFloor(ctx, hub, database, log) + + maxSeq, seedErr := database.GetMaxEventSeq(ctx) + if seedErr != nil { + log.Warn("event persistence: failed to read MAX(events.seq); hub seq still advanced from the persisted floor for this boot", "error", seedErr) + return + } + if maxSeq <= 0 { + return + } + hub.SeedSeq(uint64(maxSeq)) + log.Info("event persistence: seeded hub seq from persisted events", "seq", maxSeq) + hub.MarkVisibilityChanged() +} + +// seedHubSeqFloor reserves and persists a fresh block of the hub's sequence +// space on every boot, independent of event persistence (OC-0210) — see +// seedHubReplayState's doc for why this is what actually closes the bug. A +// read or write failure against the settings table is logged and skipped +// rather than fatal: it leaves this one boot with the pre-fix exposure +// (plain Phase A ring-buffer behaviour) instead of blocking startup over a +// heuristic safety net. +func seedHubSeqFloor(ctx context.Context, hub *ws.Hub, database *db.DB, log *slog.Logger) { + var floor uint64 + raw, err := database.GetSetting(ctx, wsSeqFloorSettingKey) + switch { + case err == nil: + parsed, perr := strconv.ParseUint(raw, 10, 64) + if perr != nil { + log.Warn("event persistence: stored ws seq floor is not a valid uint64, resetting to 0", "value", raw, "error", perr) + break + } + floor = parsed + case errors.Is(err, db.ErrNotFound): + // No prior boot has ever reserved a floor — start from 0. + default: + log.Warn("event persistence: failed to read persisted ws seq floor; hub seq not advanced this boot", "error", err) + return + } + + newFloor := floor + wsSeqFloorReserve + if err := database.SetSetting(ctx, wsSeqFloorSettingKey, strconv.FormatUint(newFloor, 10)); err != nil { + log.Warn("event persistence: failed to persist advanced ws seq floor; hub seq not advanced this boot", "error", err) + return + } + hub.SeedSeq(newFloor) +} diff --git a/Server/internal/app/plugins.go b/Server/internal/app/plugins.go new file mode 100644 index 00000000..fa1e6cd6 --- /dev/null +++ b/Server/internal/app/plugins.go @@ -0,0 +1,50 @@ +package app + +import ( + "context" + "log/slog" + "time" + + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/db" + "github.com/J3vb/OwnCord/Server/plugin" +) + +// initPlugins constructs the plugin runtime, returning nil when plugins +// are disabled or failed to start. +func initPlugins(bgCtx context.Context, log *slog.Logger, cfg *config.Config, database *db.DB) *plugin.Registry { + var pluginRegistry *plugin.Registry + if cfg.Plugins.Enabled { + registry, plugErr := plugin.NewRegistry(plugin.Config{ + Directory: cfg.Plugins.Directory, + MaxMemoryMB: cfg.Plugins.MaxMemoryMB, + CPUBudgetMs: cfg.Plugins.CPUBudgetMs, + HTTPAllowlist: cfg.Plugins.HTTPAllowlist, + Store: database, + }) + if plugErr != nil { + log.Warn("plugin runtime init failed; continuing without plugins", "error", plugErr) + } else { + pluginRegistry = registry + if err := registry.LoadAll(bgCtx); err != nil { + log.Warn("plugin loader: failed to scan directory", "error", err) + } + } + } + + return pluginRegistry +} + +// closePlugins shuts the plugin runtime down. A nil registry is the disabled +// case and has nothing to close. ctx is App.Close's shutdown budget: the 5s +// cap here is this step's own share of it, not a fresh root, so a wedged +// plugin cannot push teardown past the budget the operator was told about. +func closePlugins(ctx context.Context, registry *plugin.Registry) { + if registry == nil { + return + } + + closeCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + _ = registry.Close(closeCtx) +} diff --git a/Server/restart.go b/Server/internal/app/restart.go similarity index 76% rename from Server/restart.go rename to Server/internal/app/restart.go index 36894e59..0b98083c 100644 --- a/Server/restart.go +++ b/Server/internal/app/restart.go @@ -2,18 +2,19 @@ // setup wizard). // // The admin package never stops or spawns processes; it requests a restart -// through admin.SetRestartHandoff, which lands in restartCoordinator.Request -// here. Request cancels the parent context of run()'s signal.NotifyContext, +// through admin.SetRestartHandoff, which lands in RestartCoordinator.Request +// here. Request cancels the parent context of Run's signal.NotifyContext, // so the process drains through the exact same graceful path a SIGTERM takes // — including on Windows, where a process cannot deliver a signal to itself. -// Only after run() has fully torn down (HTTP listeners closed, hub and +// Only after Run has fully torn down (HTTP listeners closed, hub and // LiveKit stopped, queues flushed, database closed and its process lock // released) does main() perform the handoff: spawn the replacement binary // when self-managed, or just exit and let the process supervisor relaunch // the service. The old process being completely gone before the successor // starts is what makes the handoff deterministic — the DB-lock and bind -// retries in db/ and main.go survive only as safety nets. -package main +// retries in db/ and internal/app survive only as safety nets. + +package app import ( "context" @@ -30,20 +31,20 @@ const ( restartModeSpawn = "spawn" restartModeSupervised = "supervised" - // restartBackstopDelay bounds how long a requested restart may drain - // before the process force-exits (performing the handoff first). run()'s + // RestartBackstopDelay bounds how long a requested restart may drain + // before the process force-exits (performing the handoff first). Run's // worst-case legitimate teardown is ≈55s — the 30s shutdown budget plus // its sequential bounded defers — so 90s only ever fires on a genuinely // wedged teardown. The successor's lock/bind retries absorb whatever a // backstop exit leaves unreleased. - restartBackstopDelay = 90 * time.Second + RestartBackstopDelay = 90 * time.Second ) -// restartCoordinator owns the lifecycle of one restart request. It is -// created in main(), threaded into run() (as a parameter, so tests drive -// run() with their own instance), and consulted by main() again after run() +// RestartCoordinator owns the lifecycle of one restart request. It is +// created in main(), threaded into Run (as a parameter, so tests drive +// Run with their own instance), and consulted by main() again after Run // returns. -type restartCoordinator struct { +type RestartCoordinator struct { ctx context.Context cancel context.CancelFunc @@ -57,13 +58,13 @@ type restartCoordinator struct { backstop *time.Timer } -// newRestartCoordinator builds a coordinator whose Context() is the parent -// for run()'s signal.NotifyContext. onBackstop runs once if a requested +// NewRestartCoordinator builds a coordinator whose Context() is the parent +// for Run's signal.NotifyContext. onBackstop runs once if a requested // restart's drain exceeds backstopDelay; production passes handoff+os.Exit, // tests pass a recorder. -func newRestartCoordinator(backstopDelay time.Duration, onBackstop func()) *restartCoordinator { +func NewRestartCoordinator(backstopDelay time.Duration, onBackstop func()) *RestartCoordinator { ctx, cancel := context.WithCancel(context.Background()) - return &restartCoordinator{ + return &RestartCoordinator{ ctx: ctx, cancel: cancel, backstopDelay: backstopDelay, @@ -71,22 +72,22 @@ func newRestartCoordinator(backstopDelay time.Duration, onBackstop func()) *rest } } -// Context is the parent context for run()'s signal handling: cancelling it +// Context is the parent context for Run's signal handling: cancelling it // (Request) is indistinguishable from a shutdown signal to everything -// downstream. A Request issued before run() reaches NotifyContext is safe — -// NotifyContext over an already-cancelled parent starts out done, and run() +// downstream. A Request issued before Run reaches NotifyContext is safe — +// NotifyContext over an already-cancelled parent starts out done, and Run // falls straight through to graceful teardown. -func (rc *restartCoordinator) Context() context.Context { return rc.ctx } +func (rc *RestartCoordinator) Context() context.Context { return rc.ctx } // SetMode records the resolved restart mode ("spawn"/"supervised") once // config is loaded; Mode reads it back for the handoff. -func (rc *restartCoordinator) SetMode(mode string) { +func (rc *RestartCoordinator) SetMode(mode string) { rc.mu.Lock() rc.mode = mode rc.mu.Unlock() } -func (rc *restartCoordinator) Mode() string { +func (rc *RestartCoordinator) Mode() string { rc.mu.Lock() defer rc.mu.Unlock() return rc.mode @@ -95,7 +96,7 @@ func (rc *restartCoordinator) Mode() string { // Request records a restart request and starts the drain. Idempotent: the // first reason wins, duplicates are logged and dropped. Arms the backstop // timer before cancelling so a wedged teardown can never outlive it. -func (rc *restartCoordinator) Request(reason string) { +func (rc *RestartCoordinator) Request(reason string) { rc.mu.Lock() if rc.requested { pending := rc.reason @@ -117,16 +118,16 @@ func (rc *restartCoordinator) Request(reason string) { } // Requested reports whether a restart was requested, and its reason. -func (rc *restartCoordinator) Requested() (reason string, ok bool) { +func (rc *RestartCoordinator) Requested() (reason string, ok bool) { rc.mu.Lock() defer rc.mu.Unlock() return rc.reason, rc.requested } -// disarm stops the backstop timer. main() calls it the moment run() returns: +// Disarm stops the backstop timer. main() calls it the moment Run returns: // from there the handoff is in main()'s hands and a delayed force-exit would // only race it. -func (rc *restartCoordinator) disarm() { +func (rc *RestartCoordinator) Disarm() { rc.mu.Lock() if rc.backstop != nil { rc.backstop.Stop() @@ -159,14 +160,14 @@ func resolveRestartMode(cfgVal string, log *slog.Logger) string { // (which must not start real processes). var spawnReplacement = updater.SpawnDetached -// performRestartHandoff completes a requested restart after run() has fully +// PerformRestartHandoff completes a requested restart after Run has fully // drained. In supervised mode the handoff IS the exit — the supervisor // (systemd Restart=, NSSM AppExit, Docker restart policy) relaunches the // service, now running the swapped binary. In spawn mode the replacement is // started directly; every resource is already released, so the successor // boots with no lock or port contention. A failed spawn leaves the server // down — loudly logged; there is no hub left to notify clients through. -func performRestartHandoff(reason, mode string, log *slog.Logger) { +func PerformRestartHandoff(reason, mode string, log *slog.Logger) { if mode == restartModeSupervised { log.Info("restart: exiting for the supervisor to relaunch", "reason", reason, "mode", mode) return diff --git a/Server/restart_test.go b/Server/internal/app/restart_test.go similarity index 84% rename from Server/restart_test.go rename to Server/internal/app/restart_test.go index 2b36ff58..635c7d35 100644 --- a/Server/restart_test.go +++ b/Server/internal/app/restart_test.go @@ -1,4 +1,4 @@ -package main +package app import ( "fmt" @@ -17,8 +17,8 @@ import ( ) func TestRestartCoordinator_RequestIdempotent(t *testing.T) { - rc := newRestartCoordinator(time.Hour, nil) - defer rc.disarm() + rc := NewRestartCoordinator(time.Hour, nil) + defer rc.Disarm() if reason, ok := rc.Requested(); ok || reason != "" { t.Fatalf("fresh coordinator Requested() = %q, %v; want none", reason, ok) @@ -40,10 +40,10 @@ func TestRestartCoordinator_RequestIdempotent(t *testing.T) { // Cancelling the coordinator's context must drive a signal.NotifyContext // built on top of it — that is the whole mechanism by which a restart -// request drains run() exactly like a SIGTERM, on every platform. +// request drains Run exactly like a SIGTERM, on every platform. func TestRestartCoordinator_CancelDrivesNotifyContext(t *testing.T) { - rc := newRestartCoordinator(time.Hour, nil) - defer rc.disarm() + rc := NewRestartCoordinator(time.Hour, nil) + defer rc.Disarm() ctx, stop := signal.NotifyContext(rc.Context(), os.Interrupt) defer stop() @@ -60,7 +60,7 @@ func TestRestartCoordinator_CancelDrivesNotifyContext(t *testing.T) { func TestRestartCoordinator_Backstop(t *testing.T) { t.Run("fires after the delay", func(t *testing.T) { fired := make(chan struct{}) - rc := newRestartCoordinator(10*time.Millisecond, func() { close(fired) }) + rc := NewRestartCoordinator(10*time.Millisecond, func() { close(fired) }) rc.Request("test") select { case <-fired: @@ -69,21 +69,21 @@ func TestRestartCoordinator_Backstop(t *testing.T) { } }) - t.Run("disarm stops it", func(t *testing.T) { + t.Run("Disarm stops it", func(t *testing.T) { fired := make(chan struct{}) - rc := newRestartCoordinator(50*time.Millisecond, func() { close(fired) }) + rc := NewRestartCoordinator(50*time.Millisecond, func() { close(fired) }) rc.Request("test") - rc.disarm() + rc.Disarm() select { case <-fired: - t.Fatal("backstop fired after disarm") + t.Fatal("backstop fired after Disarm") case <-time.After(200 * time.Millisecond): } }) t.Run("not armed without a request", func(t *testing.T) { fired := make(chan struct{}) - _ = newRestartCoordinator(10*time.Millisecond, func() { close(fired) }) + _ = NewRestartCoordinator(10*time.Millisecond, func() { close(fired) }) select { case <-fired: t.Fatal("backstop fired without a restart request") @@ -156,12 +156,12 @@ func TestPerformRestartHandoff(t *testing.T) { } defer func() { spawnReplacement = prev }() - performRestartHandoff("update", restartModeSupervised, log) + PerformRestartHandoff("update", restartModeSupervised, log) if len(calls) != 0 { t.Fatalf("supervised handoff spawned a process: %+v — the supervisor owns the relaunch", calls) } - performRestartHandoff("update", restartModeSpawn, log) + PerformRestartHandoff("update", restartModeSpawn, log) if len(calls) != 1 { t.Fatalf("spawn handoff made %d spawn calls, want 1", len(calls)) } @@ -172,20 +172,20 @@ func TestPerformRestartHandoff(t *testing.T) { // A failing spawn must be survivable (logged, no panic) — there is no // hub left to notify at this point. spawnReplacement = func(string, []string) error { return fmt.Errorf("injected spawn failure") } - performRestartHandoff("update", restartModeSpawn, log) + PerformRestartHandoff("update", restartModeSpawn, log) } -// TestRun_RestartRequest_DrainsCleanly drives run() end to end: boot on a +// TestRun_RestartRequest_DrainsCleanly drives Run end to end: boot on a // real port, request a restart through the coordinator (exactly what an -// admin update/restore does via admin.SetRestartHandoff), and assert run() +// admin update/restore does via admin.SetRestartHandoff), and assert Run // drains and returns nil with no leaked goroutines — the property main()'s // post-run handoff depends on (DB closed and lock released, port free, // LiveKit stopped) before it starts the successor. func TestRun_RestartRequest_DrainsCleanly(t *testing.T) { t.Chdir(t.TempDir()) - // Grab a free port, release it, and hand it to run(). The tiny window - // where something else could take it is absorbed by run()'s bind retry. + // Grab a free port, release it, and hand it to Run. The tiny window + // where something else could take it is absorbed by Run's bind retry. l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("probe listen: %v", err) @@ -203,9 +203,9 @@ func TestRun_RestartRequest_DrainsCleanly(t *testing.T) { leakOpt := goleak.IgnoreCurrent() - rc := newRestartCoordinator(time.Hour, nil) + rc := NewRestartCoordinator(time.Hour, nil) runErr := make(chan error, 1) - go func() { runErr <- run(log, logBuf, levelVar, rc) }() + go func() { runErr <- runApp(log, logBuf, levelVar, rc) }() // Wait for the server to actually serve before requesting the restart. healthURL := fmt.Sprintf("http://127.0.0.1:%d/health", port) @@ -220,7 +220,7 @@ func TestRun_RestartRequest_DrainsCleanly(t *testing.T) { } select { case err := <-runErr: - t.Fatalf("run() exited before serving: %v", err) + t.Fatalf("Run exited before serving: %v", err) case <-time.After(50 * time.Millisecond): } } @@ -233,12 +233,12 @@ func TestRun_RestartRequest_DrainsCleanly(t *testing.T) { select { case err := <-runErr: if err != nil { - t.Fatalf("run() after restart request = %v, want nil (clean drain)", err) + t.Fatalf("Run after restart request = %v, want nil (clean drain)", err) } case <-time.After(60 * time.Second): - t.Fatal("run() did not return after the restart request") + t.Fatal("Run did not return after the restart request") } - rc.disarm() + rc.Disarm() if reason, ok := rc.Requested(); !ok || reason != "test-restart" { t.Errorf("Requested() = %q, %v; want the recorded restart", reason, ok) diff --git a/Server/internal/app/telemetry.go b/Server/internal/app/telemetry.go new file mode 100644 index 00000000..a5f4d67b --- /dev/null +++ b/Server/internal/app/telemetry.go @@ -0,0 +1,33 @@ +package app + +import ( + "context" + "log/slog" + "time" + + "github.com/J3vb/OwnCord/Server/config" + "github.com/J3vb/OwnCord/Server/telemetry" +) + +// initTelemetry initialises OpenTelemetry and returns the shutdown step +// stop step the telemetry stage registers with App.Close. +func initTelemetry(log *slog.Logger, cfg *config.Config) func() { + // Init can return (nil, err) when the otel build-tag skeleton hasn't been + // finished wiring to the upstream SDK. Normalise to a no-op shutdown so + // the deferred closure never calls a nil function. + telemetryShutdown, telErr := telemetry.Init(context.Background(), cfg.Telemetry) + if telErr != nil { + log.Warn("telemetry init failed; continuing without OpenTelemetry", "error", telErr) + } + if telemetryShutdown == nil { + telemetryShutdown = func(context.Context) error { return nil } + } + + return func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := telemetryShutdown(shutdownCtx); err != nil { + log.Warn("telemetry shutdown returned error", "error", err) + } + } +} diff --git a/Server/invariants/db_import_boundary.go b/Server/invariants/db_import_boundary.go index 8095b17d..725d6bf7 100644 --- a/Server/invariants/db_import_boundary.go +++ b/Server/invariants/db_import_boundary.go @@ -64,17 +64,24 @@ var DBImportAllow = map[string]DBImportEntry{ "api/middleware.go": {"move", "auth", "session/API-token touch and revoke"}, "api/plugins_handler.go": {"adapter", "", "db.Auditor is the seam; WriteAudit only"}, "api/profile_handler.go": {"move", "upload", "avatar upload creates the attachment row"}, - "api/router.go": {"boundary", "", "health probe (PingRead, SQLDb); hub construction leaves in B3-3"}, + "api/router.go": {"boundary", "", "health probe (PingRead, SQLDb); hub construction left in B3-3"}, "api/upload_handler.go": {"move", "upload", "attachment access + a raw QueryRowContext"}, // ── auth ────────────────────────────────────────────────────────────── "auth/helpers.go": {"adapter", "", "db.User type in a helper signature"}, "auth/resolve.go": {"adapter", "", "Session/APIToken/Role/User types; resolution is injected"}, // ── composition roots and tools ─────────────────────────────────────── - "main.go": {"boundary", "", "process composition root; B3-3 moves it to internal/app"}, - "token_cli.go": {"move", "auth", "API-token CLI duplicates admin/handlers_tokens.go"}, - "cmd/seed/main.go": {"boundary", "", "developer seeding tool owns its handle"}, - "cmd/gendocs/main.go": {"boundary", "", "docs generator migrates its own in-memory catalog"}, - "plugin/pluginstore.go": {"adapter", "", "PluginRow type only; the store is injected"}, + // B3-3 moved the process composition root out of main.go: internal/app + // owns the handle from open to close, and main.go no longer imports db. + "internal/app/app.go": {"boundary", "", "the App holds the handle for its lifetime; no calls"}, + "internal/app/database.go": {"boundary", "", "opens the handle, migrates, clears stale state at boot"}, + "internal/app/hub.go": {"boundary", "", "hands the handle to the hub and the service layer it builds"}, + "internal/app/maintenance.go": {"boundary", "", "periodic worker: expired sessions, backups, orphan attachments"}, + "internal/app/persistence.go": {"boundary", "", "event persister, audit writer and the boot seq seed own the handle"}, + "internal/app/plugins.go": {"boundary", "", "passes the handle to the plugin registry as its store; no calls"}, + "token_cli.go": {"move", "auth", "API-token CLI duplicates admin/handlers_tokens.go"}, + "cmd/seed/main.go": {"boundary", "", "developer seeding tool owns its handle"}, + "cmd/gendocs/main.go": {"boundary", "", "docs generator migrates its own in-memory catalog"}, + "plugin/pluginstore.go": {"adapter", "", "PluginRow type only; the store is injected"}, // ── ws ──────────────────────────────────────────────────────────────── "ws/client.go": {"adapter", "", "db.User type on the connection"}, "ws/deps.go": {"move", "channel", "role and DM-membership reads behind the hub's deps"}, diff --git a/Server/main.go b/Server/main.go index 54ab800b..b8089c64 100644 --- a/Server/main.go +++ b/Server/main.go @@ -3,41 +3,19 @@ package main import ( - "bytes" "context" - "crypto/tls" - "encoding/pem" - "errors" "fmt" - "io" - stdlog "log" "log/slog" - "net" - "net/http" "os" - "os/signal" - "path/filepath" - "runtime" - "strconv" - "syscall" - "time" - - "gopkg.in/yaml.v3" "github.com/J3vb/OwnCord/Server/admin" - "github.com/J3vb/OwnCord/Server/api" - "github.com/J3vb/OwnCord/Server/auth" - "github.com/J3vb/OwnCord/Server/config" - "github.com/J3vb/OwnCord/Server/db" - "github.com/J3vb/OwnCord/Server/diskutil" + "github.com/J3vb/OwnCord/Server/internal/app" "github.com/J3vb/OwnCord/Server/logctx" - "github.com/J3vb/OwnCord/Server/plugin" - "github.com/J3vb/OwnCord/Server/storage" - "github.com/J3vb/OwnCord/Server/telemetry" - "github.com/J3vb/OwnCord/Server/ws" ) // version is overridden at build time via -ldflags "-X main.version=1.0.0". +// It stays in package main because that is the symbol the Makefile, +// release.yml and the Dockerfile name; app.Deps carries it into the process. var version = "dev" func main() { @@ -45,7 +23,7 @@ func main() { // 0/1. It exists for container healthchecks: the distroless image has no // shell or curl, so the binary is its own probe. if len(os.Args) > 1 && os.Args[1] == "healthcheck" { - os.Exit(runHealthcheckCLI()) + os.Exit(app.RunHealthcheckCLI()) } // `server token ...` is a direct-to-DB CLI (mint/list/revoke API tokens) — // handled before any server/logging setup so it stays quiet and standalone. @@ -57,11 +35,12 @@ func main() { // that tees log records to both stdout and the ring buffer. logBuf := admin.NewRingBuffer(2000) // levelVar controls both handlers' thresholds. It starts at INFO (the - // zero value) so early-startup logs are captured, then run() raises/lowers - // it once config.yaml / OWNCORD_LOGGING_LEVEL is loaded. The ring buffer - // shares it rather than hard-wiring DEBUG: with both sinks gated, Enabled - // returns false for suppressed levels and every gated Debug call across - // the server becomes a no-op instead of formatting a ring entry. + // zero value) so early-startup logs are captured, then app.LoadConfig + // raises or lowers it once config.yaml / OWNCORD_LOGGING_LEVEL is loaded. + // The ring buffer shares it rather than hard-wiring DEBUG: with both + // sinks gated, Enabled returns false for suppressed levels and every + // gated Debug call across the server becomes a no-op instead of + // formatting a ring entry. levelVar := new(slog.LevelVar) stdoutHandler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: levelVar}) multiHandler := admin.NewMultiHandler(stdoutHandler, logBuf, levelVar) @@ -71,27 +50,29 @@ func main() { slog.SetDefault(log) // The restart coordinator carries a self-restart request (update apply, - // backup restore, setup wizard) across run()'s teardown — see restart.go. - // The backstop closure fires only if a requested restart's drain wedges - // past restartBackstopDelay: it performs the handoff and force-exits, - // mirroring what the code below does on the healthy path. - var rc *restartCoordinator - rc = newRestartCoordinator(restartBackstopDelay, func() { + // backup restore, setup wizard) across the lifecycle's teardown — see + // internal/app/restart.go. main owns it and hands it in; the handoff + // below is the last thing this process does. The backstop closure fires + // only if a requested restart's drain wedges past RestartBackstopDelay: + // it performs the handoff and force-exits, mirroring what the code below + // does on the healthy path. + var rc *app.RestartCoordinator + rc = app.NewRestartCoordinator(app.RestartBackstopDelay, func() { slog.Error("restart backstop fired — teardown exceeded its budget, exiting for handoff") reason, _ := rc.Requested() - performRestartHandoff(reason, rc.Mode(), slog.Default()) + app.PerformRestartHandoff(reason, rc.Mode(), slog.Default()) os.Exit(0) }) - err := run(log, logBuf, levelVar, rc) - rc.disarm() + err := runServer(log, logBuf, levelVar, rc) + rc.Disarm() - // Perform the handoff even when run() returned an error: a restart is - // only ever requested after a committed binary swap or a restore that - // closed the database, so not restarting is strictly worse than - // restarting into whatever the error was. + // Perform the handoff even when the lifecycle returned an error: a + // restart is only ever requested after a committed binary swap or a + // restore that closed the database, so not restarting is strictly worse + // than restarting into whatever the error was. if reason, ok := rc.Requested(); ok { - performRestartHandoff(reason, rc.Mode(), log) + app.PerformRestartHandoff(reason, rc.Mode(), log) } if err != nil { @@ -101,919 +82,18 @@ func main() { } } -// run is the real entrypoint — separated for testability. rc carries a -// self-restart request out to main(), which performs the actual handoff once -// everything here has drained (see restart.go). -func run(log *slog.Logger, logBuf *admin.RingBuffer, levelVar *slog.LevelVar, rc *restartCoordinator) error { - // bgCtx is a cancellable context shared by all background goroutines - // (event persister, event pruner, plugin loader, maintenance loop). - // - // This first deferred bgCancel is only the LIFO backstop — because it is - // registered before `defer database.Close()`, it would otherwise run - // AFTER the database is closed, leaving background goroutines running - // through teardown. The persistence and maintenance blocks below register - // their own later (= earlier-running) defers that cancel bgCtx and JOIN - // their goroutines before the database closes. - bgCtx, bgCancel := context.WithCancel(context.Background()) - defer bgCancel() - - runRemoveOldBinary(log) - - // ── 1. Load configuration ────────────────────────────────────────────── - cfg, err := runLoadConfig(log, levelVar, rc) +// runServer is the whole server lifecycle: load the configuration main's log +// sinks and restart coordinator are wired against, build the App around it, +// and run until it stops and has closed every stage it started. Split out of +// main() only so the restart handoff above runs on every return path. +func runServer(log *slog.Logger, logBuf *admin.RingBuffer, levelVar *slog.LevelVar, rc *app.RestartCoordinator) error { + cfg, err := app.LoadConfig(log, levelVar, rc) if err != nil { return err } - - // ── 2. Ensure data directory exists ──────────────────────────────────── - if err := runPrepareDataDir(log, cfg); err != nil { - return err - } - - // ── 3. TLS ──────────────────────────────────────────────────────────── - tlsResult, err := auth.LoadOrGenerate(cfg.TLS) - if err != nil { - return fmt.Errorf("configuring TLS: %w", err) - } - tlsCfg := tlsResult.TLSConfig - - // Print startup banner first so it appears above all init logs. - printBanner(cfg, version, tlsCfg != nil) - - // ── 4. Open database + run migrations ───────────────────────────────── - database, err := runOpenDatabase(cfg) + a, err := app.New(cfg, app.Deps{Version: version, Log: log, LogBuf: logBuf, Restart: rc}) if err != nil { return err } - defer database.Close() //nolint:errcheck - - if err := runInitDatabase(log, cfg, database, rc); err != nil { - return err - } - - // ── 4b. Telemetry (Phase B Step 8) ───────────────────────────────────── - telemetryStop := runInitTelemetry(log, cfg) - defer telemetryStop() - - // ── 5a. Construct plugin runtime BEFORE the router so the router can - // wire the live registry into the plugin admin handler. ──────────────── - pluginRegistry := runInitPlugins(bgCtx, log, cfg, database) - defer runClosePlugins(pluginRegistry) - - // ── 5b. Build HTTP router ────────────────────────────────────────────── - router, hub, routerCleanup := api.NewRouter(cfg, database, version, logBuf, pluginRegistry) - defer routerCleanup() - // Backstop for every early return below (serve error, ACME shutdown - // failure, etc.): hub.GracefulStop is the only caller of - // LiveKitProcess.Stop(), so skipping it orphans the companion - // livekit-server process and leaves the hub's dispatch goroutine - // running. gracefulOnce makes it idempotent alongside the explicit call - // on the normal shutdown path below. - defer hub.GracefulStop() - - // ── 5c. Wire event persistence (Phase B Step 7) ──────────────────────── - persister, prunerDone := runStartEventPersistence(bgCtx, log, cfg, hub, database) - defer runStopEventPersistence(log, bgCancel, persister, prunerDone) - - // ── 5d. Async audit writer ───────────────────────────────────────────── - // Moves audit-log INSERTs off the request path: once the writer is - // installed, WriteAudit enqueues here and a background goroutine batches - // the writes (same shape as the event persister above). Paths that never - // install a writer — the token CLI, tests — keep the synchronous - // behavior. This defer is registered after `defer database.Close()` so - // LIFO ordering drains the queue before the database is torn down. - auditWriter := runStartAuditWriter(bgCtx, database) - defer runStopAuditWriter(auditWriter) - - // ── 6. Start server ──────────────────────────────────────────────────── - addr := fmt.Sprintf(":%d", cfg.Server.Port) - srv := &http.Server{ - Addr: addr, - Handler: router, - TLSConfig: tlsCfg, - ReadTimeout: 30 * time.Second, - WriteTimeout: 30 * time.Second, - IdleTimeout: 120 * time.Second, - ErrorLog: stdlog.New(io.Discard, "", 0), // suppress TLS handshake noise - } - - // ── 6b. ACME HTTP challenge server on :80 ───────────────────────────── - // When using Let's Encrypt (tls.mode: acme), an HTTP server on port 80 - // is needed for HTTP-01 challenge validation and HTTP→HTTPS redirect. - acmeSrv := runStartACME(log, tlsResult.HTTPHandler) - - // ── 7. Background maintenance ──────────────────────────────────────── - maintenanceStop := runStartMaintenance(bgCtx, log, cfg, database) - defer maintenanceStop() - - // Listen for OS signals for graceful shutdown. The coordinator's context - // is the parent, so a programmatic restart request (rc.Request) drains - // exactly like a SIGTERM — including on Windows, where a process cannot - // signal itself. Signals arriving mid-drain are swallowed until stop() - // runs, same as on the real-signal path. - ctx, stop := signal.NotifyContext(rc.Context(), os.Interrupt, syscall.SIGTERM) - defer stop() - - if err := runServeAndWait(ctx, log, rc, srv, tlsCfg, addr); err != nil { - return err - } - - // Graceful shutdown. - shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - if err := runShutdownServers(shutdownCtx, log, srv, acmeSrv, hub); err != nil { - return err - } - - log.Info("server stopped cleanly") - return nil -} - -// runRemoveOldBinary deletes the binary a previous self-update left behind. -// Extracted from run. -func runRemoveOldBinary(log *slog.Logger) { - // Clean up old binary from a previous update. Bounded retry: in spawn - // mode the predecessor spawns this process as its very last act, so for - // the first few hundred milliseconds it may not have fully exited — and - // on Windows its image file (the .old after the swap) stays locked until - // it does. - exePath, exeErr := os.Executable() - if exeErr != nil { - log.Warn("failed to determine executable path", "error", exeErr) - return - } - - oldPath := exePath + ".old" - if _, statErr := os.Stat(oldPath); statErr != nil { - return - } - - var rmErr error - for attempt := range 5 { - if attempt > 0 { - time.Sleep(250 * time.Millisecond) - } - if rmErr = os.Remove(oldPath); rmErr == nil { - break - } - } - if rmErr != nil { - log.Warn("failed to remove old binary", "path", oldPath, "error", rmErr) - } else { - log.Info("removed old binary from previous update", "path", oldPath) - } -} - -// runLoadConfig loads the on-disk configuration, applies its logging level -// and resolves the restart handoff mode. Extracted from run. -func runLoadConfig(log *slog.Logger, levelVar *slog.LevelVar, rc *restartCoordinator) (*config.Config, error) { - cfg, err := config.Load(config.DefaultPath) - if err != nil { - return nil, fmt.Errorf("loading config: %w", err) - } - - // Apply the configured log level. The admin panel's live log view (ring - // buffer) follows the same threshold — set logging.level to "debug" to - // capture debug records there. - if lvl, ok := config.ParseLevel(cfg.Logging.Level); ok { - levelVar.Set(lvl) - } else { - log.Warn("unknown logging.level, keeping info", "value", cfg.Logging.Level) - } - - // Resolve how a self-restart hands off (spawn the replacement vs exit - // for a supervisor) now that config is loaded — main() reads it back - // after run() returns. - rc.SetMode(resolveRestartMode(cfg.Server.RestartMode, log)) - - return cfg, nil -} - -// runPrepareDataDir creates the configured data directory and warns when the -// volumes the server writes to are low on free space. Extracted from run. -func runPrepareDataDir(log *slog.Logger, cfg *config.Config) error { - if mkdirErr := os.MkdirAll(cfg.Server.DataDir, 0o750); mkdirErr != nil { - return fmt.Errorf("creating data dir %s: %w", cfg.Server.DataDir, mkdirErr) - } - - // Disk-space awareness: the database (WAL growth included), uploads, - // certs, and by default backups all live on this volume, and running it - // dry breaks several of them at once. Probe errors are ignored — unknown - // is not "full". /health repeats this check continuously at 256 MiB. - warnLowDisk(log, "data dir", cfg.Server.DataDir) - if cfg.Backup.Dir != "" && cfg.Backup.Dir != filepath.Join(cfg.Server.DataDir, "backups") { - warnLowDisk(log, "backup dir", cfg.Backup.Dir) - } - - return nil -} - -// runOpenDatabase validates the configured backend and opens the database. -// Extracted from run. -func runOpenDatabase(cfg *config.Config) (*db.DB, error) { - // SQLite is the only supported backend; the unfinished Postgres - // scaffolding (stubbed query layer, never wired into the runtime) was - // removed rather than completed. - if t := cfg.Database.Type; t != "" && t != "sqlite" { - return nil, fmt.Errorf("database.type=%q is not supported; set \"sqlite\" or omit it", t) - } - - database, err := db.OpenWithMaxReaders(cfg.Database.Path, cfg.Database.MaxReaders) - if err != nil { - return nil, fmt.Errorf("opening database: %w", err) - } - - return database, nil -} - -// runInitDatabase points the admin panel at the live database, runs the -// migrations and clears state left over from a previous run. Extracted from -// run. -func runInitDatabase(log *slog.Logger, cfg *config.Config, database *db.DB, rc *restartCoordinator) error { - // The admin "Restore backup" handler needs the real database file path: - // without this, it falls back to a hardcoded "data/chatserver.db" and - // silently no-ops on any server with a configured database.path. - admin.SetDatabasePath(cfg.Database.Path) - // Backup handlers and the scheduled-backup maintenance write to the - // configured backup directory (defaults to data/backups). - admin.SetBackupDir(cfg.Backup.Dir) - // Admin restart requests (update apply, backup restore, setup wizard) - // land in the coordinator, which drains this process and lets main() - // perform the handoff. Wired before the listener starts serving, so no - // admin request can ever hit the unwired default hook. - admin.SetRestartHandoff(rc.Request) - - if err := db.Migrate(database); err != nil { - return fmt.Errorf("running migrations: %w", err) - } - - // Clear stale state from a previous run or crash. Startup work — nothing - // to inherit a context from yet. - if err := database.ResetAllUserStatuses(context.Background()); err != nil { - log.Warn("failed to reset stale user statuses", "error", err) - } else { - log.Info("reset all user statuses to offline") - } - if err := database.ClearAllVoiceStates(context.Background()); err != nil { - log.Warn("failed to clear stale voice states", "error", err) - } else { - log.Info("cleared stale voice states") - } - - return nil -} - -// runInitTelemetry initialises OpenTelemetry and returns the shutdown step -// run defers. Extracted from run. -func runInitTelemetry(log *slog.Logger, cfg *config.Config) func() { - // Init can return (nil, err) when the otel build-tag skeleton hasn't been - // finished wiring to the upstream SDK. Normalise to a no-op shutdown so - // the deferred closure never calls a nil function. - telemetryShutdown, telErr := telemetry.Init(context.Background(), cfg.Telemetry) - if telErr != nil { - log.Warn("telemetry init failed; continuing without OpenTelemetry", "error", telErr) - } - if telemetryShutdown == nil { - telemetryShutdown = func(context.Context) error { return nil } - } - - return func() { - shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - if err := telemetryShutdown(shutdownCtx); err != nil { - log.Warn("telemetry shutdown returned error", "error", err) - } - } -} - -// runInitPlugins constructs the plugin runtime, returning nil when plugins -// are disabled or failed to start. Extracted from run. -func runInitPlugins(bgCtx context.Context, log *slog.Logger, cfg *config.Config, database *db.DB) *plugin.Registry { - var pluginRegistry *plugin.Registry - if cfg.Plugins.Enabled { - registry, plugErr := plugin.NewRegistry(plugin.Config{ - Directory: cfg.Plugins.Directory, - MaxMemoryMB: cfg.Plugins.MaxMemoryMB, - CPUBudgetMs: cfg.Plugins.CPUBudgetMs, - HTTPAllowlist: cfg.Plugins.HTTPAllowlist, - Store: database, - }) - if plugErr != nil { - log.Warn("plugin runtime init failed; continuing without plugins", "error", plugErr) - } else { - pluginRegistry = registry - if err := registry.LoadAll(bgCtx); err != nil { - log.Warn("plugin loader: failed to scan directory", "error", err) - } - } - } - - return pluginRegistry -} - -// runClosePlugins shuts the plugin runtime down. Registered by run as a defer -// only once the registry exists, so a nil registry is the disabled case and -// has nothing to close. Extracted from run. -func runClosePlugins(registry *plugin.Registry) { - if registry == nil { - return - } - - closeCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - _ = registry.Close(closeCtx) -} - -// runStartEventPersistence starts the event persister and pruner, returning -// both as (nil, nil) when event persistence is disabled. Extracted from run. -// -// seedHubReplayState runs unconditionally (whenever hub is non-nil), NOT -// gated on cfg.EventPersistence.Enabled: it seeds the hub's seq counter from -// a persisted floor even in ring-buffer-only mode, which is what closes -// OC-0210 — see its doc comment. -func runStartEventPersistence(bgCtx context.Context, log *slog.Logger, cfg *config.Config, hub *ws.Hub, database *db.DB) (*ws.EventPersister, <-chan struct{}) { - if hub == nil { - return nil, nil - } - - seedHubReplayState(bgCtx, hub, database, log) - - if !cfg.EventPersistence.Enabled { - return nil, nil - } - - persister := ws.NewEventPersister( - database, - 4096, - cfg.EventPersistence.BatchSize, - time.Duration(cfg.EventPersistence.BatchFlushMs)*time.Millisecond, - ) - persister.Start(bgCtx) - hub.SetEventPersister(persister) - hub.SetEventStore(database) - - retention := time.Duration(cfg.EventPersistence.RetentionHours) * time.Hour - prunerInterval := time.Duration(cfg.EventPersistence.PrunerIntervalMinutes) * time.Minute - prunerDone := ws.StartEventPruner(bgCtx, database, retention, prunerInterval) - - return persister, prunerDone -} - -// runStopEventPersistence drains the event persister and pruner. Registered by -// run as a defer unconditionally, so a nil persister is the disabled case and -// must leave bgCtx alone — the LIFO backstop in run cancels it instead. -// Extracted from run. -func runStopEventPersistence(log *slog.Logger, bgCancel context.CancelFunc, persister *ws.EventPersister, prunerDone <-chan struct{}) { - if persister == nil { - return - } - - stopCtx, stopCancel := context.WithTimeout(context.Background(), 5*time.Second) - defer stopCancel() - persister.Stop(stopCtx) - // Cancel the shared background context and JOIN the pruner before - // the (LIFO-later) database.Close defer runs, so no prune is still - // mid-query against a closing pool. Bounded: a stuck prune delays - // shutdown by at most the timeout, then Close proceeds anyway. - bgCancel() - select { - case <-prunerDone: - case <-stopCtx.Done(): - log.Warn("event pruner did not exit before shutdown timeout") - } -} - -// runStartAuditWriter installs the async audit writer. Extracted from run. -func runStartAuditWriter(bgCtx context.Context, database *db.DB) *db.AuditWriter { - auditWriter := db.NewAuditWriter(database, 1024, 50, 100*time.Millisecond) - auditWriter.Start(bgCtx) - database.SetAuditWriter(auditWriter) - - return auditWriter -} - -// runStopAuditWriter drains the async audit writer. Extracted from run. -func runStopAuditWriter(auditWriter *db.AuditWriter) { - stopCtx, stopCancel := context.WithTimeout(context.Background(), 5*time.Second) - defer stopCancel() - auditWriter.Stop(stopCtx) -} - -// runStartACME starts the ACME HTTP-01 challenge server when Let's Encrypt -// is configured, and returns nil otherwise. Extracted from run. -func runStartACME(log *slog.Logger, httpHandler http.Handler) *http.Server { - var acmeSrv *http.Server - if httpHandler != nil { - acmeSrv = &http.Server{ - Addr: ":80", - Handler: httpHandler, - ReadTimeout: 10 * time.Second, - WriteTimeout: 10 * time.Second, - } - go func() { - log.Info("ACME HTTP challenge server starting on :80") - if err := serveWithBindRetry(log, "acme-http", acmeSrv.ListenAndServe); err != nil && !errors.Is(err, http.ErrServerClosed) { - log.Error("ACME HTTP server error — HTTP-01 challenges and certificate renewal will fail until the next restart", "error", err) - } - }() - } - - return acmeSrv -} - -// runStartMaintenance starts the periodic maintenance loop and returns the -// stop step run defers. Extracted from run. -func runStartMaintenance(bgCtx context.Context, log *slog.Logger, cfg *config.Config, database *db.DB) func() { - // Periodically purge expired sessions and orphaned attachments. - fileStorage, fileStorageErr := storage.New(cfg.Upload.StorageDir, cfg.Upload.MaxSizeMB) - if fileStorageErr != nil { - log.Warn("failed to create file storage for maintenance; orphan file cleanup disabled", "error", fileStorageErr) - } - - stopMaintenance := make(chan struct{}) - maintenanceDone := make(chan struct{}) - go runMaintenanceLoop(bgCtx, log, database, fileStorage, stopMaintenance, maintenanceDone) - - return func() { - // Backstop for early returns below (see hub.GracefulStop defer above), - // and a bounded join so an in-flight tick (which can hold the writer — - // scheduled backups run VACUUM INTO) isn't still using the database - // while the LIFO-later Close defer tears it down. - close(stopMaintenance) - select { - case <-maintenanceDone: - case <-time.After(5 * time.Second): - log.Warn("maintenance loop did not exit before shutdown timeout") - } - } -} - -// runMaintenanceLoop is the periodic maintenance goroutine started by -// runStartMaintenance. Extracted from run. -func runMaintenanceLoop(bgCtx context.Context, log *slog.Logger, database *db.DB, fileStorage *storage.Storage, stopMaintenance, maintenanceDone chan struct{}) { - defer close(maintenanceDone) - ticker := time.NewTicker(15 * time.Minute) - defer ticker.Stop() - consecutiveFailures := 0 - const maxConsecutiveFailures = 5 - for { - select { - case <-ticker.C: - if consecutiveFailures >= maxConsecutiveFailures { - log.Error("maintenance loop: circuit breaker open, skipping tick", - "consecutive_failures", consecutiveFailures) - // Reset after one skip to allow retry next tick. - consecutiveFailures = maxConsecutiveFailures - 1 - continue - } - - if runMaintenanceTick(bgCtx, log, database, fileStorage) { - consecutiveFailures++ - } else { - consecutiveFailures = 0 - } - case <-stopMaintenance: - return - } - } -} - -// runMaintenanceTick runs one maintenance pass and reports whether any step -// of it failed. Extracted from run. -func runMaintenanceTick(bgCtx context.Context, log *slog.Logger, database *db.DB, fileStorage *storage.Storage) bool { - tickFailed := false - if err := database.DeleteExpiredSessions(bgCtx); err != nil { - log.Warn("failed to delete expired sessions", "error", err) - tickFailed = true - } - - // Scheduled backups + retention pruning, driven by the - // backup_schedule / backup_retention admin settings. - if err := admin.MaintainBackups(bgCtx, database); err != nil { - log.Warn("backup maintenance failed", "error", err) - tickFailed = true - } - - // Clean up orphaned attachments (uploaded but never linked to a message). - // - // Skipped entirely with no file storage configured: the delete is - // atomic (row goes the instant it's selected, by design — see - // db/attachment_queries.go), so with fileStorage nil the returned - // stored_as names — the only remaining handle on those blobs — - // would just be discarded and the files stranded on disk with no - // query left able to name them. Leaving the rows in place keeps - // them reclaimable once storage is available again. - if fileStorage != nil { - cutoff := time.Now().Add(-1 * time.Hour) - orphanFiles, orphanErr := database.DeleteOrphanedAttachments(bgCtx, cutoff) - if orphanErr != nil { - log.Warn("failed to delete orphaned attachments", "error", orphanErr) - tickFailed = true - } else if len(orphanFiles) > 0 { - // Best-effort file cleanup. - for _, filename := range orphanFiles { - if delErr := fileStorage.Delete(filename); delErr != nil { - log.Warn("failed to delete orphan file", "file", filename, "error", delErr) - } - } - log.Info("cleaned up orphaned attachments", "count", len(orphanFiles)) - } - } - - return tickFailed -} - -// runServeAndWait starts the listener and blocks until it fails or a -// shutdown or restart signal arrives. Extracted from run. -func runServeAndWait(ctx context.Context, log *slog.Logger, rc *restartCoordinator, srv *http.Server, tlsCfg *tls.Config, addr string) error { - // Start serving in a goroutine. - serveErr := make(chan error, 1) - go func() { - log.Info("server starting", "addr", addr, "tls", tlsCfg != nil, "version", version) - - err := serveWithBindRetry(log, "server", func() error { - if tlsCfg != nil { - return srv.ListenAndServeTLS("", "") - } - return srv.ListenAndServe() - }) - if err != nil && !errors.Is(err, http.ErrServerClosed) { - serveErr <- err - } - close(serveErr) - }() - - // Wait for shutdown signal or server error. - select { - case err := <-serveErr: - if err != nil { - return fmt.Errorf("server error: %w", err) - } - case <-ctx.Done(): - if reason, ok := rc.Requested(); ok { - log.Info("restart requested, draining connections (30s timeout)", "reason", reason) - } else { - log.Info("shutdown signal received, draining connections (30s timeout)") - } - } - - return nil -} - -// runShutdownServers performs the ordered graceful shutdown: the ACME -// server, then in-flight HTTP handlers, then the WebSocket hub. Extracted -// from run. -func runShutdownServers(shutdownCtx context.Context, log *slog.Logger, srv, acmeSrv *http.Server, hub *ws.Hub) error { - if acmeSrv != nil { - if err := acmeSrv.Shutdown(shutdownCtx); err != nil { - log.Warn("ACME HTTP server shutdown error", "error", err) - } - } - - // Drain in-flight HTTP handlers FIRST: their broadcasts must still reach - // a live hub (and the event persister) or the frames vanish from the - // replay/event store across the restart. Shutdown does not wait on - // hijacked WebSocket connections, so the hub's own stop below is not - // delayed by connected clients — they get the restart notice right after - // the drain instead of right before it. - shutdownErr := srv.Shutdown(shutdownCtx) - - // Stop the WebSocket hub: notify clients, stop LiveKit, close all client - // connections. Threaded with the same 30s budget the operator was told - // about — the notice sleep and LiveKit stop count against it rather than - // extending it. - hub.GracefulStopContext(shutdownCtx) - - if shutdownErr != nil { - return fmt.Errorf("graceful shutdown: %w", shutdownErr) - } - - return nil -} - -// runHealthcheckCLI probes the local server's /health endpoint and returns a -// process exit code: 0 healthy, 1 degraded or unreachable. /health answers -// 503 with a subsystem reason when the hub, database, or disk is unhealthy, -// so a container orchestrator's healthcheck surfaces those too. -func runHealthcheckCLI() int { - // Deliberately NOT config.Load: that writes a default config.yaml when - // none exists, and a probe must have no side effects. Peek at the file - // (and the env overrides) for just the values that shape the URL and the - // certificate pin. - port := 8443 - scheme := "https" - certFile := "data/cert.pem" - tlsMode := "" - acmeDomain := "" - if raw, err := os.ReadFile(config.DefaultPath); err == nil { - var partial struct { - Server struct { - Port int `yaml:"port"` - } `yaml:"server"` - TLS struct { - Mode string `yaml:"mode"` - CertFile string `yaml:"cert_file"` - Domain string `yaml:"domain"` - } `yaml:"tls"` - } - if yaml.Unmarshal(raw, &partial) == nil { - if partial.Server.Port > 0 { - port = partial.Server.Port - } - tlsMode = partial.TLS.Mode - if partial.TLS.Mode == "off" { - scheme = "http" - } - if partial.TLS.CertFile != "" { - certFile = partial.TLS.CertFile - } - acmeDomain = partial.TLS.Domain - } - } - if env := os.Getenv("OWNCORD_SERVER_PORT"); env != "" { - if p, err := strconv.Atoi(env); err == nil && p > 0 { - port = p - } - } - if env := os.Getenv("OWNCORD_TLS_MODE"); env != "" { - tlsMode = env - if env == "off" { - scheme = "http" - } - } - if env := os.Getenv("OWNCORD_TLS_DOMAIN"); env != "" { - acmeDomain = env - } - client := &http.Client{ - Timeout: 5 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: healthcheckTLSConfig(tlsMode, certFile, acmeDomain), - }, - } - if port < 1 || port > 65535 { - port = 8443 - } - resp, err := client.Get(fmt.Sprintf("%s://127.0.0.1:%d/health", scheme, port)) //nolint:gosec // G704: host is hardcoded loopback; only the port comes from the operator's own config - if err != nil { - fmt.Fprintln(os.Stderr, "healthcheck: unreachable:", err) - return 1 - } - defer resp.Body.Close() //nolint:errcheck - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) - fmt.Fprintf(os.Stderr, "healthcheck: status %d: %s\n", resp.StatusCode, body) - return 1 - } - return 0 -} - -// healthcheckTLSConfig builds the probe's TLS config, per TLS mode: -// -// - acme: the served cert is CA-issued for the configured domain, so -// standard WebPKI verification works — but the probe dials 127.0.0.1, so -// ServerName must be overridden to the domain or hostname verification -// fails unconditionally and the probe reports a healthy server as down. -// A stale pre-ACME data/cert.pem must NOT be pinned in this mode either; -// the pin would mismatch the served ACME leaf forever. -// - self_signed / manual: the cert can never pass WebPKI (the generated one -// has no SANs and IsCA=false), so hostname/chain checks are replaced (not -// skipped) by pinning: the presented leaf must be byte-identical to the -// local cert file. -// - anything else with no readable local cert: plain WebPKI. -func healthcheckTLSConfig(tlsMode, certFile, acmeDomain string) *tls.Config { - if tlsMode == "acme" && acmeDomain != "" { - return &tls.Config{MinVersion: tls.VersionTLS12, ServerName: acmeDomain} - } - pinned := loadPinnedCert(certFile) - if pinned == nil { - return &tls.Config{MinVersion: tls.VersionTLS12} - } - return &tls.Config{ - MinVersion: tls.VersionTLS12, - // Chain/hostname verification is replaced by the exact-match pin - // below, which is strictly stronger for a cert we hold on disk. - // VerifyConnection (not VerifyPeerCertificate) so the pin also runs - // on resumed sessions (gosec G123). - InsecureSkipVerify: true, //nolint:gosec // G402: VerifyConnection below pins the exact local certificate - VerifyConnection: func(cs tls.ConnectionState) error { - if len(cs.PeerCertificates) == 0 { - return errors.New("healthcheck: server presented no certificate") - } - if !bytes.Equal(cs.PeerCertificates[0].Raw, pinned) { - return errors.New("healthcheck: server certificate does not match " + certFile) - } - return nil - }, - } -} - -// loadPinnedCert reads the first PEM certificate block from path, returning -// its DER bytes, or nil when unavailable. -func loadPinnedCert(path string) []byte { - raw, err := os.ReadFile(path) //nolint:gosec // G304: path is the operator's own configured cert file - if err != nil { - return nil - } - block, _ := pem.Decode(raw) - if block == nil || block.Type != "CERTIFICATE" { - return nil - } - return block.Bytes -} - -// wsSeqFloorSettingKey is the generic settings-table key (see db.GetSetting / -// db.SetSetting) seedHubSeqFloor persists its reserved floor under. -const wsSeqFloorSettingKey = "ws_seq_floor" - -// wsSeqFloorReserve is the block seedHubSeqFloor reserves above the persisted -// floor on every single boot (OC-0210). It only has to exceed the number of -// hub-sequenced broadcasts any one boot could plausibly emit before its own -// next restart — comfortably true at 1e9 for a self-hosted chat server — so -// this leaves an enormous safety margin while uint64's range still allows -// billions of restarts before the floor could ever wrap. -const wsSeqFloorReserve = 1_000_000_000 - -// seedHubReplayState seeds the hub's monotonic seq counter at startup from -// two independent, composable sources — both go through hub.SeedSeq, which -// only ever moves h.seq forward (CAS-max), so it doesn't matter which of the -// two runs first or whether either is available: -// -// 1. seedHubSeqFloor (below) reserves and persists a fresh block of seq -// space on every boot, regardless of whether event persistence is -// enabled. This is what closes OC-0210: previously this function did -// nothing at all when event_persistence.enabled is false (the -// documented "ring-buffer-only behaviour", config.go's -// EventPersistenceConfig.Enabled), so every boot's h.seq — and -// therefore its ring buffer's first entries — started back at 0/1. A -// client reconnecting with a last_seq remembered from a PRIOR boot -// could then coincidentally land inside the new boot's own live ring -// window: EventRingBuffer.EventsSinceFiltered has no way to tell that -// watermark apart from a legitimate one from this boot, and would -// silently serve a partial cross-epoch replay as if it were an -// ordinary resume. Seeding a floor far above anything a single boot -// could reach guarantees every previous boot's real seq values now sit -// below the new ring buffer's oldest entry, so a stale last_seq is -// correctly rejected by the pre-existing "afterSeq <= oldestSeq" guard -// in ringbuffer.go and falls through to a full ready instead -// (serve.go's handleReconnect, the `events == nil` branch) — the same -// path any other unrecoverable resume already takes, with no protocol -// change required. -// 2. When event persistence is enabled and the events table has history, -// MAX(events.seq) is exact (not a heuristic reserve) and naturally -// wins if it is the higher of the two. This branch is also what forces -// the paired visibilityChangeSeq watermark forward via -// MarkVisibilityChanged: h.seq is restored here, but the watermark -// that tells a resuming client whether a channel-visibility change -// happened since its last_seq (visibilityChangeSeq) is in-memory only -// and always starts at 0 on a fresh process — see -// ws/hub_events.go's mustFullResync. Channel-visibility changes made to -// an offline client (RefreshChannelVisibility, revokeUnreadableChannels) -// are sent as targeted, unsequenced messages that are never written to -// the events table, so replay can never recover them. Without the -// MarkVisibilityChanged call below, a client resuming with last_seq at -// or before the pre-restart max would sail straight through -// mustFullResync's zeroed watermark and could silently miss a -// visibility change it should have converged on. -func seedHubReplayState(ctx context.Context, hub *ws.Hub, database *db.DB, log *slog.Logger) { - seedHubSeqFloor(ctx, hub, database, log) - - maxSeq, seedErr := database.GetMaxEventSeq(ctx) - if seedErr != nil { - log.Warn("event persistence: failed to read MAX(events.seq); hub seq still advanced from the persisted floor for this boot", "error", seedErr) - return - } - if maxSeq <= 0 { - return - } - hub.SeedSeq(uint64(maxSeq)) - log.Info("event persistence: seeded hub seq from persisted events", "seq", maxSeq) - hub.MarkVisibilityChanged() -} - -// seedHubSeqFloor reserves and persists a fresh block of the hub's sequence -// space on every boot, independent of event persistence (OC-0210) — see -// seedHubReplayState's doc for why this is what actually closes the bug. A -// read or write failure against the settings table is logged and skipped -// rather than fatal: it leaves this one boot with the pre-fix exposure -// (plain Phase A ring-buffer behaviour) instead of blocking startup over a -// heuristic safety net. -func seedHubSeqFloor(ctx context.Context, hub *ws.Hub, database *db.DB, log *slog.Logger) { - var floor uint64 - raw, err := database.GetSetting(ctx, wsSeqFloorSettingKey) - switch { - case err == nil: - parsed, perr := strconv.ParseUint(raw, 10, 64) - if perr != nil { - log.Warn("event persistence: stored ws seq floor is not a valid uint64, resetting to 0", "value", raw, "error", perr) - break - } - floor = parsed - case errors.Is(err, db.ErrNotFound): - // No prior boot has ever reserved a floor — start from 0. - default: - log.Warn("event persistence: failed to read persisted ws seq floor; hub seq not advanced this boot", "error", err) - return - } - - newFloor := floor + wsSeqFloorReserve - if err := database.SetSetting(ctx, wsSeqFloorSettingKey, strconv.FormatUint(newFloor, 10)); err != nil { - log.Warn("event persistence: failed to persist advanced ws seq floor; hub seq not advanced this boot", "error", err) - return - } - hub.SeedSeq(newFloor) -} - -// printBanner writes the startup banner to stderr (so it doesn't mix with -// the structured log output on stdout). -func printBanner(cfg *config.Config, ver string, tls bool) { - scheme := "http" - if tls { - scheme = "https" - } - - localIP := getOutboundIP() - port := cfg.Server.Port - baseURL := fmt.Sprintf("%s://%s:%d", scheme, localIP, port) - adminURL := baseURL + "/admin" - - tlsStatus := "disabled" - if tls { - tlsStatus = "enabled" - } - - banner := fmt.Sprintf(` - - ___ ____ _ - / _ \__ ___ __ / ___|___ _ __ __| | - | | | \ \ /\ / / '_ \| | / _ \| '__/ _`+"`"+` | - | |_| |\ V V /| | | | |__| (_) | | | (_| | - \___/ \_/\_/ |_| |_|\____\___/|_| \__,_| - - ───────────────────────────────────────────── - Server %s - Version %s - TLS %s - Platform %s/%s - ───────────────────────────────────────────── - API %s/api/v1/info - WebSocket %s/api/v1/ws - Admin %s - Health %s/health - ───────────────────────────────────────────── - Press Ctrl+C to stop the server. - -`, cfg.Server.Name, ver, tlsStatus, runtime.GOOS, runtime.GOARCH, - baseURL, wsURL(scheme, localIP, port), adminURL, baseURL) - - _, _ = fmt.Fprint(os.Stderr, banner) -} - -// wsURL builds the WebSocket URL with the correct scheme. -func wsURL(httpScheme, ip string, port int) string { - ws := "ws" - if httpScheme == "https" { - ws = "wss" - } - return fmt.Sprintf("%s://%s:%d", ws, ip, port) -} - -// Free-space thresholds for the boot-time disk warning. /health uses its own -// (lower) continuous threshold; these only shape startup log noise. -const ( - diskWarnBytes = 1 << 30 // 1 GiB — warn - diskCriticalBytes = 256 << 20 // 256 MiB — error -) - -// warnLowDisk logs when the volume holding path is low on space. Probe -// failures (unsupported platform, missing dir) are silent — unknown ≠ full. -func warnLowDisk(log *slog.Logger, label, path string) { - free, err := diskutil.FreeBytes(path) - if err != nil { - return - } - switch { - case free < diskCriticalBytes: - log.Error("disk space critically low — writes will start failing soon", - "volume", label, "path", path, "free_mb", free>>20) - case free < diskWarnBytes: - log.Warn("disk space low", "volume", label, "path", path, "free_mb", free>>20) - } -} - -// getOutboundIP returns the preferred outbound IP of this machine by dialing -// a known external address (no actual connection is made with UDP). -func getOutboundIP() string { - conn, err := net.Dial("udp", "8.8.8.8:80") - if err != nil { - return "localhost" - } - defer conn.Close() //nolint:errcheck - addr, ok := conn.LocalAddr().(*net.UDPAddr) - if !ok { - slog.Warn("getOutboundIP: unexpected LocalAddr type, falling back to localhost", - "type", fmt.Sprintf("%T", conn.LocalAddr())) - return "localhost" - } - return addr.IP.String() + return a.Run(context.Background()) } diff --git a/docs/architecture/server-boundaries.md b/docs/architecture/server-boundaries.md index accd7090..cd29f0f2 100644 --- a/docs/architecture/server-boundaries.md +++ b/docs/architecture/server-boundaries.md @@ -3,7 +3,8 @@ **Written:** 2026-08-29 (B3-0), measured at `dev` `ad4defc2`. **Re-measured:** 2026-08-30 (B3-2) — the first table and the auth slice's after-state at `fe1d11b8` (pre-squash; the squash SHA is in the plan's B3-2 -evidence block). +evidence block); 2026-08-30 (B3-3) — the first table again, and the hub +lifecycle section's after-state rows, on `feat/b3-3-lifecycle`. **Owner:** the B3 plan, [plans/b3-server-architecture-guardrails-2026-08-29.md](../plans/b3-server-architecture-guardrails-2026-08-29.md). **Regenerate the first table:** `cd Server && go run ./cmd/dbinventory` and @@ -23,7 +24,7 @@ happens to that use — one of four dispositions from the | ----------- | --------------------------------------------------------------------------------------------------------------------- | ---: | | `move` | persistence or a domain decision that belongs behind a service; **Family** names the service B3-8 (or B3-2) builds | 26 | | `adapter` | a transport adapter that uses `db` types or pure helpers only — response shapes, status helpers — no persistence call | 17 | -| `boundary` | an explicit composition or transaction boundary that legitimately owns a handle (process entry, CLIs, health probe) | 6 | +| `boundary` | an explicit composition or transaction boundary that legitimately owns a handle (process entry, CLIs, health probe) | 12 | | `remove` | the import is unnecessary and goes | 0 | The rows live in code, not only here: `Server/invariants/db_import_boundary.go` @@ -58,61 +59,66 @@ which is a row worth reading, and none exists today. -| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | -| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | --------------------------------------------------------------- | -| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | -| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | -| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | -| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | -| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | -| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | -| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | -| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | -| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | -| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | -| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | -| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | -| `admin/middleware.go` | `DB×3` `Role` `User` | — | `GetRoleByID` | calls | move | auth | owner gate re-reads the role — OC-0345 | -| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | -| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | -| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | -| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | -| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | -| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | -| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | -| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | -| `api/middleware.go` | `DB` `Role` `Session` `User` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | -| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | -| `api/profile_handler.go` | `DB×2` `Session×2` `User×7` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | -| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction leaves in B3-3 | -| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | -| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | -| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | -| `cmd/gendocs/main.go` | `DB×3` | `Migrate()` `Open()` | `Close×2` `QueryContext×2` | calls | boundary | — | docs generator migrates its own in-memory catalog | -| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | -| `main.go` | `AuditWriter×2` `DB×10` | `ErrNotFound` `Migrate()` `NewAuditWriter()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `Close` `DeleteExpiredSessions` `DeleteOrphanedAttachments` `GetMaxEventSeq` `GetSetting` `ResetAllUserStatuses` `SetAuditWriter` `SetSetting` | calls | boundary | — | process composition root; B3-3 moves it to internal/app | -| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | -| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | -| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | -| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | -| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | -| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | -| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | -| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | -| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | -| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | -| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | -| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | -| `ws/serve.go` | `ChannelOverride` `DB×9` `PersistedEvent` `User` `VoiceState` | `ConnectStatus()` `StatusOffline` `WriteAudit()` | `GetChannelOverridesFor` `GetRoleByID×4` `GetUserByID×2` `GetUserDMChannelIDs` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `MarkUserDisconnected` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | -| `ws/serve_auth.go` | `DB` `User` | — | `GetSessionByTokenHash` `GetUserByID` | calls | move | auth | session lookup on the WebSocket handshake | -| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | -| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×5` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×4` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetUserDMChannels` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot: channels, overrides, unreads, DMs, members | -| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | -| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | +| File | `db.*` types | `db.*` funcs and sentinels | `*db.DB` method calls | Shape | Disposition | Family | Why | +| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- | ------------ | ------------------------------------------------------------------ | +| `admin/admin.go` | `DB` | — | — | type-only | boundary | — | holds the handle for the admin mux; no calls | +| `admin/api.go` | `DB` | — | — | type-only | boundary | — | passes the handle to handlers; no calls | +| `admin/backup_maintenance.go` | `DB×3` | `CheckBackupIntegrity()` `ErrNotFound×2` `WriteAudit()×2` | `BackupToSafe` `GetSetting×2` | calls | move | settings-ops | BackupToSafe, integrity check, settings reads | +| `admin/handlers_backup.go` | `DB×5` | `CheckBackupIntegrity()×2` `WriteAudit()×2` | `BackupToSafe×2` `Close` `LogAudit` `SQLDb` | calls | move | settings-ops | backup trigger and download; raw SQLDb for VACUUM INTO | +| `admin/handlers_channel_perms.go` | `Channel` `ChannelRoleOverride×2` `ChannelUserOverride×2` `DB×8` `Role×2` `User×2` | `WriteAudit()×4` | `DeleteChannelOverride` `DeleteChannelUserOverride` `GetChannel` `GetChannelPermissions×2` `GetRoleByID×3` `GetUserByID` `GetUserChannelPermissions×2` `ListChannelRoleOverrides` `ListChannelUserOverrides` `ListUserIDsByRole×2` `UpsertChannelOverride` `UpsertChannelUserOverride` | calls | move | channel | override CRUD decides permission policy in the handler | +| `admin/handlers_channels.go` | `Channel×2` `ChannelUpdate×2` `DB×6` | `WriteAudit()×3` | `AdminCreateChannel` `AdminDeleteChannel` `AdminUpdateChannel×2` `GetAuditLog` `GetChannel×4` `ListChannels` | calls | move | channel | channel CRUD + audit | +| `admin/handlers_roles.go` | `DB×4` | — | `GetRoleByID` `ListRoles` | calls | move | role | two reads; service/role.go already owns the writes | +| `admin/handlers_settings.go` | `DB×4` | `ErrNotFound` `WriteAudit()` | `BeginTx` `CountUsersWithoutTOTP` `GetAllSettings×2` `GetSetting` | calls | move | settings-ops | BeginTx in a handler; TOTP census | +| `admin/handlers_tokens.go` | `DB×3` `User` | `WriteAudit()×2` | `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` | calls | move | auth | API-token CRUD duplicated in token_cli.go | +| `admin/handlers_users.go` | `DB×4` `Role` `User` | — | `GetServerStats` `GetUserByID×2` `ListAllUsers` | calls | move | user | user list, stats, lookups | +| `admin/helpers.go` | `Role×2` `User` | — | — | type-only | adapter | — | Role/User types in response helpers | +| `admin/logstream.go` | `DB×3` | — | — | type-only | boundary | — | handle threaded to the SSE stream's auth check; no calls | +| `admin/middleware.go` | `DB×3` `Role` `User` | — | `GetRoleByID` | calls | move | auth | owner gate re-reads the role — OC-0345 | +| `admin/setup_handler.go` | `DB×4` | `ErrConflict` `WriteAudit()×2` | `CreateChannel×2` `CreateInvite` `CreateOwnerIfEmpty` `CreateSession` `GetSetting×3` `UserCount` | calls | move | auth | first-run owner creation (setup sub-family) | +| `admin/setup_wizard.go` | `DB` | — | `BeginTx` | calls | move | auth | BeginTx for the wizard; setup sub-family | +| `admin/types.go` | `Channel×3` `DB` `Role` `User` `UserWithRole` | — | `GetRoleByID` | calls | adapter | — | response DTOs; the one GetRoleByID moves with handlers_users | +| `api/channel_handler.go` | `DB` `MessageAPIResponse×2` `MessageSearchResult×2` `ReactionUser` `User×8` | — | — | type-only | adapter | — | response types only; service owns the calls | +| `api/dm_handler.go` | `DB` `DMChannelInfo` `DMUser×2` `User×8` | `NewDMChannelInfo()` `StatusForViewer()` | — | calls | adapter | — | DM response types + pure status helpers | +| `api/emoji_handler.go` | `DB` `Emoji×3` `User×2` | — | — | type-only | adapter | — | Emoji/User types only | +| `api/gif_handler.go` | `DB` | — | — | type-only | adapter | — | handle in the signature, unused for calls | +| `api/invite_handler.go` | `DB` `Invite` `User×2` | — | — | type-only | adapter | — | Invite/User types only | +| `api/middleware.go` | `DB` `Role` `Session` `User` | — | `DeleteSession` `TouchAPIToken` `TouchSession` | calls | move | auth | session/API-token touch and revoke | +| `api/plugins_handler.go` | `Auditor×2` | `WriteAudit()` | — | calls | adapter | — | db.Auditor is the seam; WriteAudit only | +| `api/profile_handler.go` | `DB×2` `Session×2` `User×7` | — | `CreateAttachment` | calls | move | upload | avatar upload creates the attachment row | +| `api/router.go` | `DB×4` | — | `PingRead` `SQLDb` | calls | boundary | — | health probe (PingRead, SQLDb); hub construction left in B3-3 | +| `api/upload_handler.go` | `AttachmentAccess×2` `DB×5` `Role` `User×3` | — | `CreateAttachment` `GetAttachmentWithChannel` `IsAvatarFileURL` `IsDMParticipant` `QueryRowContext` | calls | move | upload | attachment access + a raw QueryRowContext | +| `auth/helpers.go` | `User` | — | — | type-only | adapter | — | db.User type in a helper signature | +| `auth/resolve.go` | `APIToken` `Role×2` `Session×2` `User×2` | — | — | type-only | adapter | — | Session/APIToken/Role/User types; resolution is injected | +| `cmd/gendocs/main.go` | `DB×3` | `Migrate()` `Open()` | `Close×2` `QueryContext×2` | calls | boundary | — | docs generator migrates its own in-memory catalog | +| `cmd/seed/main.go` | `DB×6` | `Migrate()` `Open()` | `Close` `CreateChannel` `CreateMessage×2` `CreateUser` `GetOrCreateDMChannel` `GetUserByUsername` `ListChannels` `QueryRowContext` | calls | boundary | — | developer seeding tool owns its handle | +| `internal/app/app.go` | `AuditWriter` `DB` | — | — | type-only | boundary | — | the App holds the handle for its lifetime; no calls | +| `internal/app/database.go` | `DB×2` | `Migrate()` `OpenWithMaxReaders()` | `ClearAllVoiceStates` `ResetAllUserStatuses` | calls | boundary | — | opens the handle, migrates, clears stale state at boot | +| `internal/app/hub.go` | `DB` | — | — | type-only | boundary | — | hands the handle to the hub and the service layer it builds | +| `internal/app/maintenance.go` | `DB×3` | — | `DeleteExpiredSessions` `DeleteOrphanedAttachments` | calls | boundary | — | periodic worker: expired sessions, backups, orphan attachments | +| `internal/app/persistence.go` | `AuditWriter×2` `DB×4` | `ErrNotFound` `NewAuditWriter()` | `GetMaxEventSeq` `GetSetting` `SetAuditWriter` `SetSetting` | calls | boundary | — | event persister, audit writer and the boot seq seed own the handle | +| `internal/app/plugins.go` | `DB` | — | — | type-only | boundary | — | passes the handle to the plugin registry as its store; no calls | +| `plugin/pluginstore.go` | `PluginRow×3` | — | — | type-only | adapter | — | PluginRow type only; the store is injected | +| `token_cli.go` | `DB×3` `User` | `Migrate()` `OpenShared()` `WriteAudit()×3` | `Close` `CreateAPIToken` `GetOwnerUser` `GetUserByUsername` `ListAPITokens` `RevokeAPIToken` `RevokeAPITokenByLabel` | calls | move | auth | API-token CLI duplicates admin/handlers_tokens.go | +| `ws/client.go` | `User×2` | — | — | type-only | adapter | — | db.User type on the connection | +| `ws/deps.go` | `Channel` `DB×5` | — | `GetRoleForUser×3` `IsDMParticipant` | calls | move | channel | role and DM-membership reads behind the hub's deps | +| `ws/event.go` | — | `BroadcastStatus()` | — | calls | adapter | — | pure BroadcastStatus helper | +| `ws/event_persister.go` | `PersistedEvent×2` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/eventstore.go` | `PersistedEvent×3` | — | — | type-only | adapter | — | PersistedEvent type; store is an interface | +| `ws/handlers.go` | `User` | — | `GetChannel` `GetRoleForUser` `GetSessionWithBanStatus` `IsDMParticipant` | calls | move | channel | channel, role, session-ban and DM reads in command handlers | +| `ws/handlers_chat.go` | — | `NewDMChannelInfo()` | — | calls | adapter | — | pure NewDMChannelInfo helper | +| `ws/hub.go` | `DB×2` | — | `GetSetting×2` | calls | move | settings-ops | GetSetting at construction | +| `ws/hub_broadcast.go` | `Channel×4` `Emoji` `Role` | `BroadcastStatus()` | `GetChannel×2` `GetDMParticipantIDs` `GetRoleForUser` `GetUserByID×2` `ListChannels` | calls | move | channel | visibility refresh reads channels, roles, users | +| `ws/hub_sweep.go` | `User` | — | `GetAllVoiceStates` `GetChannel` `GetChannelVoiceStates` `GetSessionsWithBanStatusBatch` `LeaveVoiceChannelIfMatch×2` | calls | move | voice | stale-voice sweep reads and leaves | +| `ws/messages.go` | `Channel×4` `DMChannelInfo×2` `DMUser×2` `Emoji` `Role×3` `User×2` `VoiceState` | `BroadcastStatus()` `StatusForViewer()` | — | calls | adapter | — | wire types + pure status helpers | +| `ws/serve.go` | `ChannelOverride` `DB×9` `PersistedEvent` `User` `VoiceState` | `ConnectStatus()` `StatusOffline` `WriteAudit()` | `GetChannelOverridesFor` `GetRoleByID×4` `GetUserByID×2` `GetUserDMChannelIDs` `GetVoiceState` `LeaveVoiceChannelIfMatch` `ListChannels` `MarkUserDisconnected` `UpdateUserStatus` | calls | move | connection | connect/disconnect lifecycle; B3-5 splits it by family first | +| `ws/serve_auth.go` | `DB` `User` | — | `GetSessionByTokenHash` `GetUserByID` | calls | move | auth | session lookup on the WebSocket handshake | +| `ws/serve_pumps.go` | — | `StatusOffline` | `MarkUserDisconnected` | calls | move | user | MarkUserDisconnected on pump exit | +| `ws/serve_ready.go` | `Channel×10` `ChannelOverride×6` `ChannelUnread×2` `DB×5` `DMChannelInfo×4` `MemberSummary×3` `Role×4` `User` `VoiceState×4` | `StatusOffline×3` | `GetAllVoiceStates` `GetChannelOverridesFor` `GetChannelUnreadCounts` `GetUserDMChannels` `ListChannels` `ListMembers` `ListRoles` | calls | move | channel | ready snapshot: channels, overrides, unreads, DMs, members | +| `ws/voice_join.go` | `Channel×3` `ChannelOverride` `VoiceState×5` | `ErrChannelFull` | `GetChannel×2` `GetChannelOverridesFor` `GetChannelVoiceStates` `GetRoleForUser` `GetVoiceState×6` `JoinVoiceChannel` `JoinVoiceChannelIfCapacity` `LeaveVoiceChannelIfMatch` `SetVoiceServerDeafen` `SetVoiceServerMute` | calls | move | voice | voice state reads and writes | +| `ws/voice_moderation.go` | `Role` `VoiceState×3` | `WriteAudit()` | `CountChannelVoiceUsers` `GetChannel×2` `GetRoleForUser` `GetVoiceState×2` `SetVoiceServerDeafen×2` `SetVoiceServerMute×2` | calls | move | voice | mute/deafen/move persist voice state | -50 files import `db` outside `db/` and `service/` (. 2, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 1, plugin 1, ws 17); 14 are type-only; 0 unlisted. -Dispositions: adapter 17, boundary 7, move 26. Move targets: auth 7, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. +55 files import `db` outside `db/` and `service/` (. 1, admin 16, api 10, auth 2, cmd/gendocs 1, cmd/seed 1, internal/app 6, plugin 1, ws 17); 17 are type-only; 0 unlisted. +Dispositions: adapter 17, boundary 12, move 26. Move targets: auth 7, channel 6, connection 1, role 1, settings-ops 4, upload 2, user 2, voice 3. @@ -142,14 +148,15 @@ Reading the table: ## Hub lifecycle inventory -Input to B3-3 (`internal/app/`) and B3-4 (constructor options). Measured at -the same commit. +Input to B3-3 (`internal/app/`) and B3-4 (constructor options). The +before-state was measured at `ad4defc2`; the after-state rows are B3-3's, on +`feat/b3-3-lifecycle`, and are what B3-4 starts from. -### Construction and setters (S-11) +### Construction and setters (S-11) — before B3-3 -`ws.NewHub(database, limiter, svc)` is called **once**, inside +`ws.NewHub(database, limiter, svc)` was called **once**, inside `api.NewRouter` (`Server/api/router.go:106`) — not in `main.go`. The seven -post-construction setters and where they are called: +post-construction setters and where they were called: | Setter | Declared | Called from | Required before `Run`? | | ------------------------- | ---------------------------- | --------------------------------- | ------------------------------------------------------------------------------------------ | @@ -162,11 +169,34 @@ post-construction setters and where they are called: | `SetPendingVoiceModFlags` | `ws/voice_moderation.go:599` | voice moderation paths at runtime | no — genuinely replaceable runtime state; stays a setter | Two owners (the router and `main.go`) set collaborators on one hub, and the -hub starts (`hub.Run`, `ws/hub.go:273`) with no check that the required ones -are present. B3-3 moves construction into `internal/app/` so there is one -call site; B3-4 turns the four "yes" rows into validated `HubOptions` and -leaves the three optional ones as setters with that reason written beside -them. +hub started (`hub.Run`, `ws/hub.go:273`) with no check that the required ones +were present. + +### Construction and setters (S-11) — after B3-3 + +`ws.NewHub` is called from `app.StartRuntime` +(`Server/internal/app/hub.go`), which also applies every setter that must +land before `hub.Run` and then starts the dispatch goroutine. +`api.NewRouter` takes the built hub as part of `api.Runtime` and returns only +the handler and its cleanup. + +| Setter | Called from (before) | Called from (after) | Still required before `Run`? | +| ------------------------- | --------------------------------- | ----------------------------------------------------- | ------------------------------ | +| `SetPluginRegistry` | `api/router.go:325` | `internal/app/hub.go` (`wirePlugins`) | optional | +| `SetPluginEventSink` | `api/router.go:328` | `internal/app/hub.go` (`wirePlugins`) | optional | +| `SetLiveKit` | `api/router.go:342` | `internal/app/hub.go` (`startVoice`) | **yes** for voice | +| `SetLiveKitProcess` | `api/router.go:360` | `internal/app/hub.go` (`startVoice`) | when supervised | +| `SetEventPersister` | `main.go:453` | `internal/app/persistence.go` (`startEventPersister`) | **yes** when persistence is on | +| `SetEventStore` | `main.go:454` | `internal/app/persistence.go` (`startEventPersister`) | **yes** for replay | +| `SetPendingVoiceModFlags` | voice moderation paths at runtime | unchanged | no | + +One **owner**: every row is now inside `internal/app`. Two of them are still +in a second file — the persister and the store are set where the persister is +built, one lifecycle stage after the hub, because both setters are explicitly +safe to call after `Run` has started (`ws/hub_events.go`) and moving them +earlier would reorder the boot. B3-4 is what collapses them into validated +`HubOptions` at the single construction point; the router is no longer one of +the places that has to change for it. ### Locks @@ -186,12 +216,12 @@ Five locks on `Hub`, all `syncutil` (so the `-tags deadlock` pass sees them): package comment before it moves a single function, so every pure-move commit has something to be checked against. -### Start, drain, stop (`main.go` `run`, `main.go:107`) +### Start, drain, stop — before B3-3 (`main.go` `run`, `main.go:107`) -Start order, then the `defer` stack that undoes it (LIFO — the last started -is the first stopped): +Start order, then the `defer` stack that undid it (LIFO — the last started +was the first stopped): -| # | Start (`main.go`) | Stop (`defer`, in registration order — runs in reverse) | +| # | Start (`main.go`) | Stop (`defer`, in registration order — ran in reverse) | | --- | -------------------------------------------------------- | ---------------------------------------------------------- | | 1 | background context | `bgCancel()` `:118` | | 2 | `runOpenDatabase` → `db.OpenWithMaxReaders` `:314-322` | `database.Close()` `:148` | @@ -206,14 +236,62 @@ is the first stopped): | 11 | `signal.NotifyContext` `:214` | `stop()` `:215` | | 12 | `runServeAndWait` `:629` → `runShutdownServers` `:667` | `srv.Shutdown`, `acmeSrv.Shutdown`, hub drain | -Three facts B3-3's composite close must preserve, each already encoded in a +Three facts B3-3's composite close had to preserve, each already encoded in a comment at the cited line: the audit writer's stop is registered **after** `database.Close` so it flushes before the handle goes (`:183-186`); event persistence stops before the LIFO-later `database.Close` so no prune is still running (`:476`); `hub.GracefulStop` must run even on an early return so the supervised LiveKit process is not orphaned (`:168-172`). Any early `return -err` between steps 2 and 12 relies on this defer stack — there is no single -close function, which is exactly what B3-3's failure-injection test pins. +err` between steps 2 and 12 relied on this defer stack — there was no single +close function, which is exactly what B3-3's failure-injection test now pins. + +### Start, drain, stop — after B3-3 (`App.stages()` / `App.Close`) + +`Server/internal/app/lifecycle.go` declares the start sequence as a list, and +`App.Close` walks the close step each stage registered in the reverse of that +order. There is no `defer` stack and no second teardown path: `App.Run` closes +on every return — a failed start, a serve error and a clean shutdown alike. + +| # | Stage (`App.stages()`) | Close step, and what it does | +| --- | ---------------------- | --------------------------------------------------------------------------------- | +| 1 | (in `Run`) `bgCtx` | `background-context` — cancels bgCtx; registered first, so it runs **last** | +| 2 | `data-dir` | — | +| 3 | `tls` | — | +| 4 | `database` | `database` — `database.Close()`, registered before the migration runs | +| 5 | `migrate` | — | +| 6 | `telemetry` | `telemetry` — bounded OTel shutdown | +| 7 | `plugins` | `plugins` — `registry.Close` | +| 8 | `hub` | `hub` — `GracefulStopContext`, the only caller of `LiveKitProcess.Stop` | +| 9 | `router` | `router` — the rate-limiter cleanup goroutine | +| 10 | `event-persistence` | `event-persistence` — drains the persister, cancels bgCtx, joins the pruner | +| 11 | `audit-writer` | `audit-writer` — drains the audit queue | +| 12 | `maintenance` | `maintenance` — joins the maintenance loop | +| 13 | `acme` | — (shut down by the `http` step, in the order the drain requires) | +| 14 | `http` | `http` — ACME shutdown, then in-flight handlers, then the hub, on one 30s budget | +| 15 | `signals` | `signals` — unregisters the signal handler; registered last, so it runs **first** | + +Close order is therefore `signals`, `http`, `maintenance`, `audit-writer`, +`event-persistence`, `router`, `hub`, `plugins`, `telemetry`, `database`, +`background-context`. All three facts hold, and now hold **because of the +ordering rule** rather than because of where a `defer` happened to sit: + +- the audit writer and event persistence both start after the database opens, + so both stop before `database.Close`; +- the `http` step runs first, so in-flight handlers drain while the hub and + the event persister are still live — which is why ACME and the HTTP server + start one stage after the maintenance loop rather than before it; +- the `hub` step is reached on every return from `Run`, so a supervised + livekit-server process is never orphaned (OC-0027). + +`App.Close` reports the **first** error and still runs every later step: the +steps below a failing one are the ones that release the database handle, the +LiveKit process and the audit queue. `internal/app/close_test.go` pins the +order, the first-error rule and idempotence; +`internal/app/lifecycle_failure_test.go` fails each stage in turn — the table +is generated from `App.stages()`, so a new stage is covered the day it is +added — and asserts on every row that the error names the stage, no goroutine +is left running, the database handle is closed and the listener is not left +bound. ## Auth slice — before-state dependency graph diff --git a/docs/plans/README.md b/docs/plans/README.md index 0feaffeb..6ccd9f18 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -10,22 +10,22 @@ authority**. ## Active — these drive current work -| Plan | State | -| ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [beta-product-requirements-2026-08-23](beta-product-requirements-2026-08-23.md) | Approved beta scope, frozen. 57 `BPR-*` requirements. | -| [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) | Phase order and gates, B0–B10. **B0, B1 and B2 complete** (HP-0, HP-1 and HP-2 all accepted); **B3 is next** — opens with the layout-refactor first slice. B3–B10 not started. **Amended 2026-08-28:** dated lines in B3–B10, a "Phase execution pattern" section, and a truthful status header. **Amended 2026-08-29:** B3/B7/B9 lines binding the layout-refactor supplement below; current slice updated for B2-2. | -| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 88 planning rows. Public-safe; not a replacement for the ledger. | -| [beta-requirements-traceability-2026-08-23](beta-requirements-traceability-2026-08-23.md) | Requirement → phase → evidence map. No row is release-qualified. | -| [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) | **Supersedes the roadmap's "current evidence snapshot."** B0 measurements and dispositions. | -| [b1-repository-foundation-2026-08-25](b1-repository-foundation-2026-08-25.md) | **B1-0 through B1-8 all done.** B1 execution plan. Re-verifies every RL-\* claim against HEAD; several are refuted. | -| [hp-0-scorecard-2026-08-25](hp-0-scorecard-2026-08-25.md) | **HP-0 accepted 2026-08-25.** The single baseline-acceptance artifact. Part-closes `R-08`. | -| [hp-1-scorecard-2026-08-27](hp-1-scorecard-2026-08-27.md) | **HP-1 accepted 2026-08-27.** Structural-diff proofs for the flatten and module rename, plus the B1 exit gate. | -| [b2-protocol-trust-compat-2026-08-28](b2-protocol-trust-compat-2026-08-28.md) | **B2 complete — HP-2 accepted 2026-08-29.** B2-0, B2-1, B2-8 done 2026-08-28; B2-2 (B2-3/B2-4 folded in), B2-5, B2-6, B2-7, B2-9 done 2026-08-29. Scorecard below. | -| [hp-2-scorecard-2026-08-29](hp-2-scorecard-2026-08-29.md) | **HP-2 accepted 2026-08-29.** Seven questions answered with commands; B2 exit gate, nine conditions met (1 at the slim epoch scope, 4 with one E2EE gap disclaimed). Owner follow-ups that do not gate B3: BPR-051 reader line, SEC-01/SEC-04 advisory IDs. | -| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) and B3-1 (auth characterization) merged 2026-08-29; B3-2 (auth vertical slice) merged 2026-08-30; B3-9 (five of the six B3-tagged findings; OC-0323 rides B3-8) merged 2026-08-30; HP-3 accepted 2026-08-30 — B3-3 next. | -| [hp-3-scorecard-2026-08-29](hp-3-scorecard-2026-08-29.md) | **HP-3 accepted 2026-08-30 by the owner.** Five questions on the auth vertical slice answered with commands: frozen set green at every pre-squash SHA, `api` db importers 12 → 10, B2 contracts unchanged, the pattern written as D4 in server.md, guardrails as they exist. | -| [b3-bench-baseline-2026-08-30](b3-bench-baseline-2026-08-30.md) | **Recorded 2026-08-30, not gated.** The six B3-6 `Benchmark*` through `benchstat` at `ec8ef24a`, produced by `make bench-baseline`. Nothing in CI reads these numbers; the performance gate is B6's. Regenerating writes a new dated file — replace this row and delete the superseded document, so only the newest baseline is kept. | -| [audit-2026-08-19-remediation](audit-2026-08-19-remediation.md) | Phases 1–6 done 2026-08-20; **phase 7 pending**. Its header still reads "in progress 2026-08-19" — stale; the phase table is correct. | +| Plan | State | +| ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [beta-product-requirements-2026-08-23](beta-product-requirements-2026-08-23.md) | Approved beta scope, frozen. 57 `BPR-*` requirements. | +| [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) | Phase order and gates, B0–B10. **B0, B1 and B2 complete** (HP-0, HP-1 and HP-2 all accepted); **B3 is next** — opens with the layout-refactor first slice. B3–B10 not started. **Amended 2026-08-28:** dated lines in B3–B10, a "Phase execution pattern" section, and a truthful status header. **Amended 2026-08-29:** B3/B7/B9 lines binding the layout-refactor supplement below; current slice updated for B2-2. | +| [repo-health-issue-register-2026-08-23](repo-health-issue-register-2026-08-23.md) | 88 planning rows. Public-safe; not a replacement for the ledger. | +| [beta-requirements-traceability-2026-08-23](beta-requirements-traceability-2026-08-23.md) | Requirement → phase → evidence map. No row is release-qualified. | +| [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) | **Supersedes the roadmap's "current evidence snapshot."** B0 measurements and dispositions. | +| [b1-repository-foundation-2026-08-25](b1-repository-foundation-2026-08-25.md) | **B1-0 through B1-8 all done.** B1 execution plan. Re-verifies every RL-\* claim against HEAD; several are refuted. | +| [hp-0-scorecard-2026-08-25](hp-0-scorecard-2026-08-25.md) | **HP-0 accepted 2026-08-25.** The single baseline-acceptance artifact. Part-closes `R-08`. | +| [hp-1-scorecard-2026-08-27](hp-1-scorecard-2026-08-27.md) | **HP-1 accepted 2026-08-27.** Structural-diff proofs for the flatten and module rename, plus the B1 exit gate. | +| [b2-protocol-trust-compat-2026-08-28](b2-protocol-trust-compat-2026-08-28.md) | **B2 complete — HP-2 accepted 2026-08-29.** B2-0, B2-1, B2-8 done 2026-08-28; B2-2 (B2-3/B2-4 folded in), B2-5, B2-6, B2-7, B2-9 done 2026-08-29. Scorecard below. | +| [hp-2-scorecard-2026-08-29](hp-2-scorecard-2026-08-29.md) | **HP-2 accepted 2026-08-29.** Seven questions answered with commands; B2 exit gate, nine conditions met (1 at the slim epoch scope, 4 with one E2EE gap disclaimed). Owner follow-ups that do not gate B3: BPR-051 reader line, SEC-01/SEC-04 advisory IDs. | +| [b3-server-architecture-guardrails-2026-08-29](b3-server-architecture-guardrails-2026-08-29.md) | **B3 in progress from 2026-08-29.** Execution plan: B3-0 inventory → B3-1/B3-2 auth slice → HP-3 → lifecycle, hub options, `ws` split, families; guardrails and the alpha dataset beside the slice. B3-0 (inventory + `db-import-boundary` rule) and B3-1 (auth characterization) merged 2026-08-29; B3-2 (auth vertical slice) merged 2026-08-30; B3-9 (five of the six B3-tagged findings; OC-0323 rides B3-8) merged 2026-08-30; HP-3 accepted 2026-08-30; B3-3 (lifecycle extraction into `Server/internal/app/`, one composite close, hub construction out of `api.NewRouter`) is PR #1464, opened 2026-08-30 — B3-4 next. | +| [hp-3-scorecard-2026-08-29](hp-3-scorecard-2026-08-29.md) | **HP-3 accepted 2026-08-30 by the owner.** Five questions on the auth vertical slice answered with commands: frozen set green at every pre-squash SHA, `api` db importers 12 → 10, B2 contracts unchanged, the pattern written as D4 in server.md, guardrails as they exist. | +| [b3-bench-baseline-2026-08-30](b3-bench-baseline-2026-08-30.md) | **Recorded 2026-08-30, not gated.** The six B3-6 `Benchmark*` through `benchstat` at `ec8ef24a`, produced by `make bench-baseline`. Nothing in CI reads these numbers; the performance gate is B6's. Regenerating writes a new dated file — replace this row and delete the superseded document, so only the newest baseline is kept. | +| [audit-2026-08-19-remediation](audit-2026-08-19-remediation.md) | Phases 1–6 done 2026-08-20; **phase 7 pending**. Its header still reads "in progress 2026-08-19" — stale; the phase table is correct. | ## Partially implemented diff --git a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md index f7000cc5..d7fe6ae9 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -8,7 +8,9 @@ verified at `bf7b886d` B3-0 merged 2026-08-29 (PR #1448 = `d383d8c7`; closes entry-gate item 3); B3-1 merged 2026-08-29 (PR #1449 = `71d867cb`); B3-2 merged 2026-08-30 (PR #1450 = `75d64dd4`); B3-9 merged 2026-08-30 (PR #1454 = `123c0899`; -OC-0323 rides B3-8); HP-3 accepted 2026-08-30 by the owner — B3-3 next. +OC-0323 rides B3-8); HP-3 accepted 2026-08-30 by the owner (PR #1461 = +`52601114`); B3-3 (lifecycle extraction) — PR #1464 to `dev`, opened +2026-08-30; its squash SHA is recorded here at merge. B3-4 next. Update this line, not only the step table, when a step lands. Primary inputs: @@ -39,7 +41,7 @@ surface to it. | **B3-1** | Auth characterization tests — enumeration, sentinels, sessions, TOTP, rate limits, failure paths — **DONE 2026-08-29 (PR #1449)** | 1 day | B3-6, B3-7 | | **B3-2** | The auth vertical slice (S-10): route → `service.AuthService` → `db`, behaviour-neutral — **DONE 2026-08-30 (PR #1450)** | 2–3 days | B3-6, B3-7 | | **HP-3** | First vertical-slice review — scorecard — **ACCEPTED 2026-08-30** ([hp-3-scorecard-2026-08-29.md](hp-3-scorecard-2026-08-29.md)) | — | — | -| **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract | 1–2 days | B3-4 | +| **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract — **PR #1464 open 2026-08-30** | 1–2 days | B3-4 | | **B3-4** | Hub constructor options (S-11): required collaborators validated at construction | 1 day | after B3-3 | | **B3-5** | `ws` in-package split (S-08): responsibilities into named files, pure moves + adjacent rewrites | 2–3 days | after B3-3/B3-4 | | **B3-6** | Guardrails: coverage floor (S-06), hub simulation + fault transport + fuzz seeds, benchmarks, rules | 3–4 days | B3-0..B3-2 | @@ -526,6 +528,179 @@ app.Run(ctx)`. Exit: `main.go` under 150 lines; the failure-injection test green under `-race`; four tag variants build. One PR. +**Evidence, 2026-08-30** — branch `feat/b3-3-lifecycle` from `dev` `52601114` +(HP-3, PR #1461); PR #1464 to `dev`. + +- **Pre-squash SHAs**, one per numbered item. The full server gate ran before + each commit, and `go test -count=1 -run TestAuthCharacterization ./api/` + after it — the only allowed touch in the auth tests was `NewRouter`'s + signature at the call site, and no auth test file changed at all: + + | Commit | Item | Gate | `TestAuthCharacterization` | + | ----------- | --------------------------------------------------------------------------------------------------------------- | ----------- | -------------------------- | + | `b5827d23` | status line: HP-3's merge SHA, B3-3 in progress | docs only | — | + | `03c1295c` | 1 — pure move of every `run*` block into `Server/internal/app/` | full, green | ok 0.946 s | + | `556cdb11` | 2 — `type App`, `app.New`/`Run`, one composite `App.Close` | full, green | ok 0.939 s | + | `beebd3f9` | 3 — hub construction out of `api.NewRouter`; `NewRouter` takes `api.Runtime` | full, green | ok 0.943 s | + | `ca59ad44` | 4 — `internal/app/lifecycle_failure_test.go`, the failure-injection report | full, green | ok 0.929 s | + | `0c29636c` | 5 — this block, the after-state rows in `server-boundaries.md`, status line, step table, `docs/plans/README.md` | docs only | — | + | this commit | 6 — Codex P2: `bgCtx` must not inherit `Run`'s caller cancellation | full, green | ok 0.9 s | + +- **`main.go`: 1,019 → 99 lines.** What is left is the two CLI dispatches + (`healthcheck`, `token`), the ring buffer and log handlers, the restart + coordinator and its handoff, and eleven lines of `runServer`: + `app.LoadConfig` → `app.New` → `a.Run(ctx)`. The exit target was 150. + +- **Normalised-diff proof for the pure move (`03c1295c`).** HP-1's shape: + re-apply the substitutions the commit claims to make, then look for any + `+`/`-` line left unpaired. + + ```bash + git diff b5827d23 03c1295c -- Server/ ':!Server/invariants/' \ + | grep -E '^[+-]' | grep -v '^[+-][+-]' \ + | sed -E 's/^[+-]//; + s/^package app$/package main/; + s/\bapp\.//g; + s/\bRunHealthcheckCLI\b/runHealthcheckCLI/g; + s/\bNewRestartCoordinator\b/newRestartCoordinator/g; + s/\bRestartCoordinator\b/restartCoordinator/g; + s/\bRestartBackstopDelay\b/restartBackstopDelay/g; + s/\bPerformRestartHandoff\b/performRestartHandoff/g; + s/\bDisarm\b/disarm/g; + s/\brun\(\)/Run/g; + s/\bRun\(\)/Run/g; + s/func Run\(version string, log /func run(log /; + s/Run\("test", log, logBuf, levelVar, rc\)/run(log, logBuf, levelVar, rc)/; + s/Run\(version, log, logBuf, levelVar, rc\)/run(log, logBuf, levelVar, rc)/; + s/srv \*http\.Server, tlsCfg \*tls\.Config, addr, version string/srv *http.Server, tlsCfg *tls.Config, addr string/; + s/runServeAndWait\(ctx, log, rc, srv, tlsCfg, addr, version\)/runServeAndWait(ctx, log, rc, srv, tlsCfg, addr)/;' \ + | sort | uniq -u + ``` + + **Result: 45 unpaired lines, every one of them comment prose or the new + import.** No code line is unpaired. + + | Unpaired | Where | What | + | -------: | --------------------------- | ---------------------------------------------------------------------------------------------------------- | + | 1 | `main.go` | the new `internal/app` import | + | 28 | `main.go` | three comment blocks inside `main()` re-wrapped (the level var, the coordinator, the handoff) — same prose | + | 2 | `main.go` | `version`'s doc gains why the symbol stays in `package main` (`-X main.version`) | + | 12 | `internal/app/lifecycle.go` | the package doc comment (5, new) and `Run`'s doc re-wrapped for the `version` parameter (3 out, 4 in) | + | 2 | `internal/app/restart.go` | "the DB-lock and bind retries in `db/` and `main.go`" → "… in `db/` and `internal/app`" | + + The five substitutions are exactly what the commit message names: the + package clause, `run`→`Run` and `runHealthcheckCLI`→`RunHealthcheckCLI` + (the two entry points `main()` calls), the five restart-coordinator + identifiers `main()` still names, and `version` becoming a parameter + instead of a package-level var. + +- **Composite close, test-first.** `internal/app/close_test.go` was written + before the rewrite and failed to compile against `03c1295c` (`undefined: +App`, `undefined: New`, `undefined: LoadConfig`, `undefined: Deps`). Each + row has a negative control run on this branch: + + | Property pinned | Mutation applied | Result | + | ---------------------------------------------------- | ------------------------------- | -------------- | + | close order is the reverse of start order | walk the closers forward | FAIL | + | the first error is returned, later closes still run | `return` on the first error | FAIL | + | the hub stops when a stage after the router fails | skip teardown on a failed start | FAIL | + | the database close step actually releases the handle | drop the `database` close step | FAIL (11 rows) | + | the hub close step actually stops the dispatch loop | drop the `hub` close step | FAIL (12 rows) | + +- **Codex review (P2), fixed test-first.** Codex read the rewrite and found + that `Run(ctx)` derived `bgCtx` from its caller's context, so cancelling + that context killed the event persister, the audit writer and the + maintenance loop _before_ `Close` ran its HTTP-first drain — the one + ordering the drain exists for, since in-flight handlers' broadcasts and + audit records have to reach live consumers. It also made caller-context + shutdown behave unlike the SIGTERM and restart paths, which cancel only the + serve context. `run()` had this right for free by rooting `bgCtx` at + `context.Background()`. `main.go` passes `context.Background()`, so no + released build was affected; the defect was in B3-3's own new `Run(ctx)` + contract. Fixed with `context.WithoutCancel(ctx)` — values inherited, + cancellation not — and pinned by + `TestAppRun_CallerCancel_KeepsBackgroundWorkersAliveThroughTheDrain`, which + records `bgCtx.Err()` as each close step runs (a new test-only + `onCloseStep` seam makes the walk observable) and requires it still live at + `signals`, `http`, `maintenance` and `audit-writer`, and already cancelled + by `database`. RED before the fix on all four rows; the negative control — + restoring `context.WithCancel(ctx)` — fails it again. + +- **Failure-injection report** (item 3, and exit-gate row 4's evidence). + `internal/app/lifecycle_failure_test.go`. The table is generated from + `App.stages()`, so a stage added later is covered the day it is added. Every + row asserts the same four properties: the error names the stage, no + goroutine is left running (`goleak`), the database handle is closed, and the + listener is not left bound. Green under `go test -race ./internal/app/`. + + | Stage failed | Error names the stage | No goroutine leak | DB handle closed | Listener free | + | --------------------------------------- | --------------------- | ----------------- | ---------------- | ----------------- | + | `data-dir` | PASS | PASS | n/a (not opened) | PASS | + | `tls` | PASS | PASS | n/a (not opened) | PASS | + | `database` | PASS | PASS | n/a (not opened) | PASS | + | `migrate` | PASS | PASS | PASS | PASS | + | `telemetry` | PASS | PASS | PASS | PASS | + | `plugins` | PASS | PASS | PASS | PASS | + | `hub` | PASS | PASS | PASS | PASS | + | `router` | PASS | PASS | PASS | PASS | + | `event-persistence` | PASS | PASS | PASS | PASS | + | `audit-writer` | PASS | PASS | PASS | PASS | + | `maintenance` | PASS | PASS | PASS | PASS | + | `acme` | PASS | PASS | PASS | PASS | + | `http` | PASS | PASS | PASS | PASS | + | `signals` | PASS | PASS | PASS | PASS | + | listener bind (real, out-of-range port) | PASS | PASS | PASS | n/a (never bound) | + | none — context cancelled while serving | n/a (nil error) | PASS | PASS | PASS | + + The last row is the control: the same four properties on the path where + nothing fails, so the rows above are not passing merely because something + went wrong. + +- **Hub ownership.** `ws.NewHub` moves from `api.NewRouter` (`router.go:106`) + to `app.StartRuntime` (`internal/app/hub.go`), which also applies the four + pre-`Run` setters that were at `router.go:325-360` and starts the dispatch + goroutine. `NewRouter` gains an `api.Runtime` parameter (the hub, the + limiter, the service layer, and `VoiceEnabled` — the `lkErr == nil` guard + the voice routes were already mounted behind) and returns + `(http.Handler, func())`. The limiter and the service layer move with the + hub because it needs the same instances: the limiter persists auth + lockouts and the services hold the permission cache the hub invalidates. + Six `api_test` files and `cmd/gendocs` were updated at the call site only — + wiring, no assertion changes — and `gendocs` still emits a byte-identical + route index (its drift check is in the gate). Before/after tables: + [server-boundaries.md](../architecture/server-boundaries.md#hub-lifecycle-inventory). + +- **Build and packaging** (item 4). `Server/Makefile`, `Server/Dockerfile` + and `.github/workflows/release.yml` are untouched, and B2-7's + no-`wazero`-tag note stays true. The release build still resolves the + version symbol: `go build -o chatserver -ldflags "-s -w -X +main.version=9.9.9-b33check" .` embeds the string (3 occurrences in the + binary), because `version` deliberately stays in `package main` and is + passed into the App through `app.Deps`. Note for the record that a _plain_ + `go build .` from `Server/` produces a binary named `Server`, not + `chatserver` — that is derived from the module path (`.../OwnCord/Server`) + and is unchanged by B3-3; every packaging path passes `-o` explicitly. + +- **Gate**, run in full before every commit through one `set -euo pipefail` + script: four build-tag variants (default, `otel`, `wazero`, `otel,wazero`), + `go vet ./...`, `go test -race -timeout 20m ./... -coverprofile -cover`, + `scripts/coverage-floor.sh`, `go test -tags deadlock -count=1 ./ws/`, + `golangci-lint run ./...` at CI's pinned v2.11.3 (**0 issues**), sqlc and + genprotocol and gendocs drift, `check:docs`, `check:hygiene`. + +- **Coverage.** Aggregate 80.1% before (11446/14273 statements) → 80.2% after + (floor 79.8%); no core-package floor moved. The `Server` root package drops + to 0.0% because everything testable left it; `internal/app` carries it at + 65.9%, up from the root package's pre-move 45.7% because `main()` — never + covered — is no longer counted with the lifecycle. + +- **Inventory.** `DBImportAllow` loses its `main.go` row and gains six + `internal/app` rows, all `boundary`; `api/router.go`'s note is updated now + that hub construction has left. `docs/architecture/server-boundaries.md` is + regenerated from the map (50 → 55 importers, `boundary` 7 → 12) and its + summary table's stale `boundary 6` is corrected to agree with the generated + line. + ## B3-4 — Hub constructor options (S-11) Roadmap workstream 6; supplement Phase 3 item 2. **After B3-3, not